[
https://issues.apache.org/jira/browse/OAK-3165?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
angela resolved OAK-3165.
-------------------------
Resolution: Fixed
Committed revision 1693401.
> Redundant test for duplicate membership in Group.addMember
> ----------------------------------------------------------
>
> Key: OAK-3165
> URL: https://issues.apache.org/jira/browse/OAK-3165
> Project: Jackrabbit Oak
> Issue Type: Improvement
> Components: core
> Reporter: angela
> Assignee: angela
> Fix For: 1.3.4
>
> Attachments: OAK-3165.patch
>
>
> while looking at the {{MembershipWriter.addMember}} again I spotted the
> following code:
> {code}
> int bestCount = membershipSizeThreshold;
> PropertyState bestProperty = null;
> Tree bestTree = null;
> while (trees.hasNext()) {
> Tree t = trees.next();
> PropertyState refs = t.getProperty(UserConstants.REP_MEMBERS);
> if (refs != null) {
> int numRefs = 0;
> for (String ref : refs.getValue(Type.WEAKREFERENCES)) {
> if (ref.equals(memberContentId)) {
> return false;
> }
> numRefs++;
> }
> if (numRefs < bestCount) {
> bestCount = numRefs;
> bestProperty = refs;
> bestTree = t;
> }
> }
> }
> {code}
> the fact that loop doesn't stop once the first {{bestProperty}} has been
> found and there is an explicit test for the the references already containing
> the contentID of the member to be added, indicates to me that the additional
> test for {{isDeclaredMember(newMember)}} in {{GroupImpl}} is redundant and
> can be omitted.
> proposed improvement and some additional test attached as patch.
> [~tripod], i would appreciate if you could confirm that my reading of your
> code is correct.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
