angela created OAK-3165:
---------------------------
Summary: 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
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)
