Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SLING-12642 cache privileges and principals in the context of the invoking Jackrabbit Session #58

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

joerghoh
Copy link
Contributor

@joerghoh joerghoh commented Feb 10, 2025

In the AclUtil there are many occassions where privileges and principals are resolved over and over again (probably many times the same).

In order to remove the redundant retrieval I added CachingSesssionWrapper which is passed instead of the session; by default the wrapped session is used, but the following 2 functionalities are executed in an optimized way by the CachingSessionWrapper:

  • privilegesFromNames (convert the privilege names into privilege objects)
  • expandPrivilege ( to expand aggregated privileges into the contained privileges)
  • getPrincipal ( to retrieve a principal for a given principal name)

All these results are cached within the CachingSessionWrapper. As this object shares the same lifetime as the session object itself, and as the results are immutable, it's safe to do so.

Note: getPrincipal is safe here, as no principals are removed here.

For review I advise to review commit by commit.

@joerghoh joerghoh requested review from jsedding and anchela February 10, 2025 14:58
Copy link
Contributor

@anchela anchela left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hi @joerghoh , i worked a bit on the patch replacing the manual privilege handling with privilegecollection.
the patch looks good to me otherwise.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants