[OAK-6072] Move check for cyclic membership to GroupImpl

• Type: Improvement
• Status: Closed
• Priority: Major
• Resolution: Fixed
• Fix Version/s: 1.7.0, 1.8.0
• Component/s: core
• Epic Link: potential improvements to membership management
• Assignee: Angela Schreiber
• Reporter: Angela Schreiber
• Created: 12/Apr/17 07:49
• Updated: 30/May/17 08:38
• Resolved: 26/Apr/17 08:04

Description

while investigating potential improvements for adding/removing group members, I noticed that the way we check for cyclic group membership in the UserValidator may limit our options wrt arranging member references in the tree defined by the rep:Group node.

With the current diff mechanism the only option for spotting cyclic membership in the UserValidator is by looking at modifications/addition of rep:members properties. However, if adding a new member reference would not just be appended to an existing or new property but instead would otherwise change the structure of these properties (e.g. by inserting and splitting), we may end up re-checking member references that have already been verified; just because they look like being added in the diff.

Consequently, we may consider moving the check to the GroupImpl and limit it to those cases where the member is either passed to the addMember call or is being resolved as it may happen during addMembers.

Benefit:

• more options to improve content structure of member references to improve write performance
• only a single resolution of memberId/memberReference to the member itself (in GroupImpl)
• no additional resolution of member-reference(s) during commit validation
• improved performance for ImportBehavior.BEST_EFFORT that doesn't require the new member to exist.

Drawback

• check for cyclic membership only in the user management implementation, not enforced upon commit
• ImportBehavior.BEST_EFFORT will not spot cyclic membership

alex.parvulescu, what do you think?

Issue Links

• relates to: OAK-6105 Respect ImportBehavior when trying to add 'Everyone' as member to another group