angela commented on OAK-6072:

[~alexparvulescu], good catch... i will move that change to a separate issue 
such that we can discuss the consequences and decided on whether it makes sense 
and if we apply it properly track it in a separate issue that can be referenced 
in the documentation.

> Move check for cyclic membership to GroupImpl
> ---------------------------------------------
>                 Key: OAK-6072
>                 URL: https://issues.apache.org/jira/browse/OAK-6072
>             Project: Jackrabbit Oak
>          Issue Type: Improvement
>          Components: core
>            Reporter: angela
>         Attachments: OAK-6072.patch, OAK-6072-tests.patch
> 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}}.
> h6. 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.
> h6. 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?

This message was sent by Atlassian JIRA

Reply via email to