[OAK-4119] Improvements Take 1

• Type: Improvement
• Status: Closed
• Priority: Major
• Resolution: Fixed
• Fix Version/s: 1.2.19, 1.4.7, 1.5.2, 1.6.0
• Component/s: core
• Epic Link: potential improvements to membership management
• Assignee: Angela Schreiber
• Reporter: Angela Schreiber
• Created: 10/Mar/16 18:39
• Updated: 08/Oct/19 15:22
• Resolved: 13/May/16 08:03

Description

First bunch of potential improvements to be tested/verified using the benchmarks:

General

• dedicated index for rep:members property defined by nt rep:MemberReferences (=> mod in UserInitializer)
• adjust nt-handling in IdentifierManager.getReferences : if a single nt name is specified in the nodeTypeNames param it is inserted in the query statement instead of looking for nt:base
• simplified, new version of IdentifierManager.getReferences to match needs of membership lookup where propName and a single nt-name is given. see below (in OAK-4119_3.patch)
• introduce hasAnyReferenceInPath, which allows to search for any references being located in a give subtree of the repository (escaping fixed in OAK-4119_3.patch)
• MembershipProvider.MemberReferenceIterator: keeping track of processed references is optional i.e. an implementation detail
• MembershipProvider.AbstractMemberIterator: no more protected fields (patch OAK-4119_2)
• MembershipProvider.AbstractMemberIterator: breadth-first iteration before performing queries for the inherited members/membership (patch OAK-4119_2)

Group.isMember

• MembershipProvider: introduce shortcut if a Group doesn't have any members
• MembershipProvider: keep old logic if the target Group has (potentially) pending changes
• MembershipProvider: for unmodified Groups use reverse membership lookup of the target authorizable instead (patch OAK-4119_2: further optimizes 'hasMembership' as the AbstractMemberIterator doesn't need to search for the complete membership; limiting the number of queries to be executed. see above)
• GroupImpl: add shortcut if member to be tested is a Group representing the EveryonePrincipal

Group.isDeclaredMember

• introduce shortcut if a Group doesn't have any members
• keep old logic if target Group (potentially) has pending changes or if the number of members is small (=> threshold with MembershipProvider)
• minor change with the old behavior, which does no longer keep track of processed references, which is not needed here
• for unmodified Groups with plenty of members use IdentifierManager.hasAnyReferenceInPath
• All modifications in MembershipProvider

Group.addMember(Authorizable)

• GroupImpl: drop extra test for circular membership, which is anyway enforced by the UserValidator => needs adjustment of test-cases that expect this to be detected immediately => intoducing save-call to verify if cycles are properly detected latest during commit.
• UserValidator: if isMember is improved by the changes above, the check for circular membership would be faster as well

Group.addMembers(String...)

• introduce new methods on MembershipProvider and MembershipWriter that don't take a single contentID but rather a Map of contentID:memberID and returns the set of memberIDs that could not be processed successfully.
• consequently finding the best rep:members property and avoiding duplicate membership is performed only once for the batch of ids to be added
• Note: once best-property reaches threshold new member-ref nodes will be created

Group.removeMembers(String...)

• introduce new methods on MembershipProvider and MembershipWriter that don't take a single contentID but rather a Map of contentID:memberID and returns the set of memberIDs that could not be processed successfully.
• consequently for a given batch of ids the declared member are traversed only once

Issue Links

• blocks: OAK-4679 Backport OAK-4119, OAK-4101, OAK-4087 and OAK-4344
• is depended upon by: OAK-4130 Simplify IdentifierManager.getReferences
• is related to: OAK-4174 SegmentToJdbcTest failing with improvements of OAK-4119
• relates to: OAK-4365 Redundant Action Class Lookup in DefaultAuthorizableActionProvider