[#1548] - redefines PrincipalCollection interface to be intended as immutable#1582
Conversation
bdemers
left a comment
There was a problem hiding this comment.
I took a quick pass through, it looks great! thank you!
One nit, can you add the @Deprecated annotation as well (to MutablePrincipalCollection anywhere it's used (and set forRemoval=true)
(and possibly the @deprecated javadoc tag to SimplePrincipalCollection, with a note to use ImmutablePrincipalCollection instead.
Thanks again!!
(I'll trigger the CI builds for this)
… intended to be immutable; provide an immutable implementation including a builder class; deprecate the mutable implementation; use the immutable one in SimpleAuthenticationInfo for merging. Background: Merging AuthenticationInfo and the contained PrincipalCollections previously lead to one of the involved PrincipalCollections be selected by undefined means and being mutated, propagating the changes to other callers that did not expect such changes, including the authentication cache.
f6a10ba to
e63aeae
Compare
|
I have added @deprecated to SimplePrincipalCollection. That was the only place where MutablePrincipalCollection is referenced. |
|
@janitza-mage Can you please restore the template for this PR? |
|
I edited the initial comment. Remarks:
|
|
Thanks. |
|
Is there anything left for me to do in this PR, then? I think I can't merge it myself, right? |
|
No, you are good. |
Closes #1548. Declare PrincipalCollection interface to be intended to be immutable; provide an immutable implementation including a builder class; deprecate the mutable implementation; use the immutable one in SimpleAuthenticationInfo for merging.
Background: Merging AuthenticationInfo and the contained PrincipalCollections previously lead to one of the involved PrincipalCollections be selected by undefined means and being mutated, propagating the changes to other callers that did not expect such changes, including the authentication cache.
Following this checklist to help us incorporate your contribution quickly and easily:
for the change (usually before you start working on it). Trivial changes like typos do not
require a GitHub issue. Your pull request should address just this issue, without pulling in other changes.
[#XXX] - Fixes bug in SessionManager,where you replace
#XXXwith the appropriate GitHub issue. Best practiceis to use the GitHub issue title in the pull request title and in the first line of the commit message.
fixes #XXXif merging the PR should close a related issue.mvn verifyto make sure basic checks pass. A more thorough check will be performed on your pull request automatically.git rebase -i.Trivial changes like typos do not require a GitHub issue (javadoc, comments...).
In this case, just format the pull request title like
[DOC] - Add javadoc in SessionManager.If this is your first contribution, you have to read the Contribution Guidelines
If your pull request is about ~20 lines of code you don't need to sign an Individual Contributor License Agreement
if you are unsure please ask on the developers list.
To make clear that you license your contribution under the Apache License Version 2.0, January 2004
you have to acknowledge this by using the following check-box.