You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@sling.apache.org by jo...@apache.org on 2025/02/10 14:50:48 UTC

(sling-org-apache-sling-jcr-repoinit) branch SLING-12642-cache-privileges created (now 155411c)

This is an automated email from the ASF dual-hosted git repository.

joerghoh pushed a change to branch SLING-12642-cache-privileges
in repository https://gitbox.apache.org/repos/asf/sling-org-apache-sling-jcr-repoinit.git


      at 155411c  renanme the class

This branch includes the following new commits:

     new a9e282a  SLING-12642 introduce a wrapper for a session
     new a4e4b8b  cache once-retrieved privileges
     new c467584  also cache the expansion of aggregated privileges
     new a6cfe20  add principal caching
     new 269ce91  do not cache non-resolved principals
     new 155411c  renanme the class

The 6 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.



(sling-org-apache-sling-jcr-repoinit) 06/06: renanme the class

Posted by jo...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

joerghoh pushed a commit to branch SLING-12642-cache-privileges
in repository https://gitbox.apache.org/repos/asf/sling-org-apache-sling-jcr-repoinit.git

commit 155411cf87a479e96093f86180c420602fa1ad68
Author: Joerg Hoh <jo...@apache.org>
AuthorDate: Mon Feb 10 15:50:30 2025 +0100

    renanme the class
---
 .../apache/sling/jcr/repoinit/impl/AclUtil.java    | 130 ++++++++++-----------
 .../apache/sling/jcr/repoinit/impl/AclVisitor.java |  22 ++--
 ...sionWrapper.java => CachingSessionWrapper.java} |   4 +-
 .../sling/jcr/repoinit/PrincipalBasedAclTest.java  |   4 +-
 .../sling/jcr/repoinit/impl/AclUtilTest.java       |   4 +-
 .../jcr/repoinit/impl/ManyServiceUsersTest.java    |   2 +-
 6 files changed, 83 insertions(+), 83 deletions(-)

diff --git a/src/main/java/org/apache/sling/jcr/repoinit/impl/AclUtil.java b/src/main/java/org/apache/sling/jcr/repoinit/impl/AclUtil.java
index e5f5ab9..98d181c 100644
--- a/src/main/java/org/apache/sling/jcr/repoinit/impl/AclUtil.java
+++ b/src/main/java/org/apache/sling/jcr/repoinit/impl/AclUtil.java
@@ -132,13 +132,13 @@ public class AclUtil {
     }
 
     public static void setAcl(
-            PrivilegeCachingSessionWrapper pcsw, List<String> principals, List<String> paths, List<String> privileges, boolean isAllow)
+            CachingSessionWrapper csw, List<String> principals, List<String> paths, List<String> privileges, boolean isAllow)
             throws RepositoryException {
-        setAcl(pcsw, principals, paths, privileges, isAllow, Collections.emptyList(), Collections.emptyList());
+        setAcl(csw, principals, paths, privileges, isAllow, Collections.emptyList(), Collections.emptyList());
     }
 
     public static void setAcl(
-            PrivilegeCachingSessionWrapper pcsw,
+            CachingSessionWrapper csw,
             List<String> principals,
             List<String> paths,
             List<String> privileges,
@@ -147,16 +147,16 @@ public class AclUtil {
             List<String> options)
             throws RepositoryException {
 
-        for (String jcrPath : getJcrPaths(pcsw.getSession(), paths)) {
-            if (jcrPath != null && !pcsw.getSession().nodeExists(jcrPath)) {
+        for (String jcrPath : getJcrPaths(csw.getSession(), paths)) {
+            if (jcrPath != null && !csw.getSession().nodeExists(jcrPath)) {
                 throw new PathNotFoundException("Cannot set ACL on non-existent path " + jcrPath);
             }
-            setAcl(pcsw, principals, jcrPath, privileges, isAllow, restrictionClauses, options);
+            setAcl(csw, principals, jcrPath, privileges, isAllow, restrictionClauses, options);
         }
     }
 
     private static void setAcl(
-            PrivilegeCachingSessionWrapper pcsw,
+            CachingSessionWrapper csw,
             List<String> principals,
             String jcrPath,
             List<String> privileges,
@@ -165,15 +165,15 @@ public class AclUtil {
             List<String> options)
             throws RepositoryException {
 
-        AccessControlManager acMgr = pcsw.getAccessControlManager();
+        AccessControlManager acMgr = csw.getAccessControlManager();
 
         final String[] privArray = privileges.toArray(new String[privileges.size()]);
-        final Privilege[] jcrPriv = pcsw.privilegesFromNames(privArray);
+        final Privilege[] jcrPriv = csw.privilegesFromNames(privArray);
 
         JackrabbitAccessControlList acl = getAccessControlList(acMgr, jcrPath, true);
         checkState(acl != null, "No JackrabbitAccessControlList available for path {0}", jcrPath);
 
-        LocalRestrictions localRestrictions = createLocalRestrictions(restrictionClauses, acl, pcsw.getSession());
+        LocalRestrictions localRestrictions = createLocalRestrictions(restrictionClauses, acl, csw.getSession());
 
         AccessControlEntry[] existingAces = acl.getAccessControlEntries();
 
@@ -182,9 +182,9 @@ public class AclUtil {
                 .map(o -> o.contains(AclVisitor.OPTION_IGNORE_MISSING_PRINCIPAL))
                 .orElse(false);
         for (String name : principals) {
-            final Principal principal = getPrincipal(pcsw, name, ignoreMissingPrincipal);
+            final Principal principal = getPrincipal(csw, name, ignoreMissingPrincipal);
             LocalAccessControlEntry newAce =
-                    new LocalAccessControlEntry(pcsw, principal, jcrPriv, isAllow, localRestrictions);
+                    new LocalAccessControlEntry(csw, principal, jcrPriv, isAllow, localRestrictions);
             if (contains(existingAces, newAce)) {
                 LOG.info(
                         "Not adding {} to path {} since an equivalent access control entry already exists",
@@ -206,13 +206,13 @@ public class AclUtil {
     }
 
     @NotNull
-    private static Principal getPrincipal(PrivilegeCachingSessionWrapper pcsw, String name, boolean ignoreMissingPrincipal)
+    private static Principal getPrincipal(CachingSessionWrapper csw, String name, boolean ignoreMissingPrincipal)
             throws RepositoryException {
-        Principal principal = pcsw.getPrincipal(name);
+        Principal principal = csw.getPrincipal(name);
         if (principal == null) {
             // backwards compatibility: fallback to original code treating principal name as authorizable ID (see
             // SLING-8604)
-            final Authorizable authorizable = UserUtil.getAuthorizable(pcsw.getSession(), name);
+            final Authorizable authorizable = UserUtil.getAuthorizable(csw.getSession(), name);
             if (!ignoreMissingPrincipal) {
                 checkState(authorizable != null, "Authorizable not found: {0}", name);
                 principal = authorizable.getPrincipal();
@@ -236,9 +236,9 @@ public class AclUtil {
      * @param principalName
      * @throws RepositoryException
      */
-    public static void removePolicy(@NotNull PrivilegeCachingSessionWrapper pcsw, @NotNull final String principalName)
+    public static void removePolicy(@NotNull CachingSessionWrapper csw, @NotNull final String principalName)
             throws RepositoryException {
-        Principal principal = pcsw.getPrincipal(principalName);
+        Principal principal = csw.getPrincipal(principalName);
         if (principal == null) {
             LOG.info("Principal {} does not exist.", principalName);
             // using PrincipalImpl will prevent 'removePolicy' from failing with AccessControlException
@@ -246,7 +246,7 @@ public class AclUtil {
             principal = new PrincipalImpl(principalName);
         }
 
-        JackrabbitAccessControlManager acMgr = pcsw.getAccessControlManager();
+        JackrabbitAccessControlManager acMgr = csw.getAccessControlManager();
         for (JackrabbitAccessControlPolicy policy : acMgr.getPolicies(principal)) {
             // make sure not to remove the principal-based access control list but instead only drop
             // resource-based access control content for the given principal
@@ -263,11 +263,11 @@ public class AclUtil {
      * @param paths
      * @throws RepositoryException
      */
-    public static void removePolicies(@NotNull PrivilegeCachingSessionWrapper pcsw, @NotNull List<String> paths)
+    public static void removePolicies(@NotNull CachingSessionWrapper csw, @NotNull List<String> paths)
             throws RepositoryException {
-        AccessControlManager acMgr = pcsw.getAccessControlManager();
-        for (String jcrPath : getJcrPaths(pcsw.session, paths)) {
-            if (!isValidPath(pcsw.getSession(), jcrPath)) {
+        AccessControlManager acMgr = csw.getAccessControlManager();
+        for (String jcrPath : getJcrPaths(csw.session, paths)) {
+            if (!isValidPath(csw.getSession(), jcrPath)) {
                 LOG.info("Cannot remove ACL; no node at {} ", jcrPath);
                 continue;
             }
@@ -282,12 +282,12 @@ public class AclUtil {
     }
 
     public static void removeEntries(
-            @NotNull PrivilegeCachingSessionWrapper pcsw, @NotNull List<String> principals, @NotNull List<String> paths)
+            @NotNull CachingSessionWrapper csw, @NotNull List<String> principals, @NotNull List<String> paths)
             throws RepositoryException {
         Set<String> principalNames = new HashSet<>(principals);
-        AccessControlManager acMgr = pcsw.getAccessControlManager();
-        for (String jcrPath : getJcrPaths(pcsw.getSession(), paths)) {
-            if (!isValidPath(pcsw.getSession(), jcrPath)) {
+        AccessControlManager acMgr = csw.getAccessControlManager();
+        for (String jcrPath : getJcrPaths(csw.getSession(), paths)) {
+            if (!isValidPath(csw.getSession(), jcrPath)) {
                 LOG.info("Cannot remove access control entries on non-existent path {}", jcrPath);
             } else {
                 JackrabbitAccessControlList acl = getAccessControlList(acMgr, jcrPath, false);
@@ -313,7 +313,7 @@ public class AclUtil {
     }
 
     public static void removeEntries(
-            @NotNull PrivilegeCachingSessionWrapper pcsw,
+            @NotNull CachingSessionWrapper csw,
             @NotNull List<String> principals,
             @NotNull List<String> paths,
             List<String> privileges,
@@ -321,18 +321,18 @@ public class AclUtil {
             List<RestrictionClause> restrictionClauses)
             throws RepositoryException {
         Set<String> principalNames = new HashSet<>(principals);
-        AccessControlManager acMgr = pcsw.getAccessControlManager();
-        for (String jcrPath : getJcrPaths(pcsw.getSession(), paths)) {
-            if (!isValidPath(pcsw.getSession(), jcrPath)) {
+        AccessControlManager acMgr = csw.getAccessControlManager();
+        for (String jcrPath : getJcrPaths(csw.getSession(), paths)) {
+            if (!isValidPath(csw.getSession(), jcrPath)) {
                 LOG.info("Cannot remove access control entries on non-existent path {}", jcrPath);
             } else {
                 JackrabbitAccessControlList acl = getAccessControlList(acMgr, jcrPath, false);
                 if (acl != null) {
                     boolean modified = false;
 
-                    LocalRestrictions restr = createLocalRestrictions(restrictionClauses, acl, pcsw.getSession());
+                    LocalRestrictions restr = createLocalRestrictions(restrictionClauses, acl, csw.getSession());
                     Privilege[] privs =
-                            pcsw.privilegesFromNames(privileges.toArray(new String[0]));
+                            csw.privilegesFromNames(privileges.toArray(new String[0]));
 
                     for (AccessControlEntry ace : acl.getAccessControlEntries()) {
                         Principal principal = ace.getPrincipal();
@@ -340,7 +340,7 @@ public class AclUtil {
                             continue;
                         }
                         LocalAccessControlEntry entry =
-                                new LocalAccessControlEntry(pcsw, ace.getPrincipal(), privs, isAllow, restr);
+                                new LocalAccessControlEntry(csw, ace.getPrincipal(), privs, isAllow, restr);
                         if (entry.isEqual(ace)) {
                             acl.removeAccessControlEntry(ace);
                             modified = true;
@@ -368,15 +368,15 @@ public class AclUtil {
     }
 
     public static void setPrincipalAcl(
-            PrivilegeCachingSessionWrapper pcsw, String principalName, Collection<AclLine> lines, boolean isStrict)
+            CachingSessionWrapper csw, String principalName, Collection<AclLine> lines, boolean isStrict)
             throws RepositoryException {
-        final JackrabbitAccessControlManager acMgr = pcsw.getAccessControlManager();
-        Principal principal = pcsw.getPrincipal(principalName);
+        final JackrabbitAccessControlManager acMgr = csw.getAccessControlManager();
+        Principal principal = csw.getPrincipal(principalName);
         if (principal == null) {
             // due to transient nature of the repo-init the principal lookup may not succeed if completed through query
             // -> save transient changes and retry principal lookup
-            pcsw.getSession().save();
-            principal = pcsw.getPrincipal(principalName);
+            csw.getSession().save();
+            principal = csw.getPrincipal(principalName);
             checkState(principal != null, PRINCIPAL_NOT_FOUND_PATTERN, principalName);
         }
 
@@ -393,7 +393,7 @@ public class AclUtil {
         boolean modified = false;
         for (AclLine line : lines) {
             AclLine.Action action = line.getAction();
-            List<String> jcrPaths = getJcrPaths(pcsw.getSession(), line.getProperty(PROP_PATHS));
+            List<String> jcrPaths = getJcrPaths(csw.getSession(), line.getProperty(PROP_PATHS));
             if (action == AclLine.Action.DENY) {
                 throw new AccessControlException("PrincipalAccessControlList doesn't support 'deny' entries.");
             } else if (action == AclLine.Action.REMOVE) {
@@ -404,7 +404,7 @@ public class AclUtil {
                     modified = true;
                 }
             } else if (action == AclLine.Action.ALLOW) {
-                final Privilege[] privileges = pcsw.privilegesFromNames(
+                final Privilege[] privileges = csw.privilegesFromNames(
                         line.getProperty(PROP_PRIVILEGES).toArray(new String[0]));
                 for (String effectivePath : jcrPaths) {
                     if (acl == null) {
@@ -413,7 +413,7 @@ public class AclUtil {
                         // or if there exists no node at the effective path (unable to evaluate path-based entries).
                         LOG.info("No PrincipalAccessControlList available for principal {}", principal);
                         if (!containsEquivalentEntry(
-                                pcsw, effectivePath, principal, privileges, true, line.getRestrictions())) {
+                                csw, effectivePath, principal, privileges, true, line.getRestrictions())) {
                             LOG.warn(
                                     "No equivalent path-based entry exists for principal {} and effective path {} ",
                                     principal.getName(),
@@ -422,7 +422,7 @@ public class AclUtil {
                         }
                     } else {
                         final LocalRestrictions restrictions =
-                                createLocalRestrictions(line.getRestrictions(), acl, pcsw.getSession());
+                                createLocalRestrictions(line.getRestrictions(), acl, csw.getSession());
                         final boolean added = acl.addEntry(
                                 effectivePath,
                                 privileges,
@@ -447,30 +447,30 @@ public class AclUtil {
         }
     }
 
-    public static void removePrincipalEntries(@NotNull PrivilegeCachingSessionWrapper pcsw, String principalName, Collection<AclLine> lines)
+    public static void removePrincipalEntries(@NotNull CachingSessionWrapper csw, String principalName, Collection<AclLine> lines)
             throws RepositoryException {
-        final JackrabbitAccessControlManager acMgr = pcsw.getAccessControlManager();
-        Principal principal = pcsw.getPrincipal(principalName);
+        final JackrabbitAccessControlManager acMgr = csw.getAccessControlManager();
+        Principal principal = csw.getPrincipal(principalName);
         if (principal == null) {
             // due to transient nature of the repo-init the principal lookup may not succeed if completed through query
             // -> save transient changes and retry principal lookup
-            pcsw.getSession().save();
-            principal = pcsw.getPrincipal(principalName);
+            csw.getSession().save();
+            principal = csw.getPrincipal(principalName);
             checkState(principal != null, PRINCIPAL_NOT_FOUND_PATTERN, principalName);
         }
 
         final PrincipalAccessControlList acl = getPrincipalAccessControlList(acMgr, principal, true);
         boolean modified = false;
         for (AclLine line : lines) {
-            List<String> jcrPaths = getJcrPaths(pcsw.getSession(), line.getProperty(PROP_PATHS));
-            LocalRestrictions restr = createLocalRestrictions(line.getRestrictions(), acl, pcsw.getSession());
+            List<String> jcrPaths = getJcrPaths(csw.getSession(), line.getProperty(PROP_PATHS));
+            LocalRestrictions restr = createLocalRestrictions(line.getRestrictions(), acl, csw.getSession());
             List<String> privNames = line.getProperty(PROP_PRIVILEGES);
-            Privilege[] privs = pcsw.privilegesFromNames(privNames.toArray(new String[0]));
+            Privilege[] privs = csw.privilegesFromNames(privNames.toArray(new String[0]));
             Predicate<PrincipalAccessControlList.Entry> predicate = entry -> {
                 if (!jcrPaths.contains(entry.getEffectivePath())) {
                     return false;
                 }
-                LocalAccessControlEntry lace = new LocalAccessControlEntry(pcsw,
+                LocalAccessControlEntry lace = new LocalAccessControlEntry(csw,
                         entry.getPrincipal(), privs, line.getAction() == AclLine.Action.ALLOW, restr);
                 return lace.isEqual(entry);
             };
@@ -498,15 +498,15 @@ public class AclUtil {
      * @param principalName
      * @throws RepositoryException
      */
-    public static void removePrincipalPolicy(@NotNull PrivilegeCachingSessionWrapper pcsw, @NotNull String principalName)
+    public static void removePrincipalPolicy(@NotNull CachingSessionWrapper csw, @NotNull String principalName)
             throws RepositoryException {
-        Principal principal = pcsw.getPrincipal(principalName);
+        Principal principal = csw.getPrincipal(principalName);
         if (principal == null) {
             LOG.info("Cannot remove principal-based ACL. Principal {} does not exist.", principalName);
             return;
         }
 
-        JackrabbitAccessControlManager acMgr = pcsw.getAccessControlManager();
+        JackrabbitAccessControlManager acMgr = csw.getAccessControlManager();
         PrincipalAccessControlList acl = getPrincipalAccessControlList(acMgr, principal, false);
         if (acl == null) {
             LOG.info("Cannot remove principal-based ACL for principal {}. No such policy exists.", principalName);
@@ -622,25 +622,25 @@ public class AclUtil {
     }
 
     private static boolean containsEquivalentEntry(
-            PrivilegeCachingSessionWrapper pcsw,
+            CachingSessionWrapper csw,
             String absPath,
             Principal principal,
             Privilege[] privileges,
             boolean isAllow,
             List<RestrictionClause> restrictionList)
             throws RepositoryException {
-        if (absPath != null && !pcsw.getSession().nodeExists(absPath)) {
+        if (absPath != null && !csw.getSession().nodeExists(absPath)) {
             LOG.info(
                     "Cannot determine existence of equivalent path-based entry for principal {}. No node at path {} ",
                     principal.getName(),
                     absPath);
             return true;
         }
-        for (AccessControlPolicy policy : pcsw.getAccessControlManager().getPolicies(absPath)) {
+        for (AccessControlPolicy policy : csw.getAccessControlManager().getPolicies(absPath)) {
             if (policy instanceof JackrabbitAccessControlList) {
                 LocalRestrictions lr =
-                        createLocalRestrictions(restrictionList, ((JackrabbitAccessControlList) policy), pcsw.getSession());
-                LocalAccessControlEntry newEntry = new LocalAccessControlEntry(pcsw,principal, privileges, isAllow, lr);
+                        createLocalRestrictions(restrictionList, ((JackrabbitAccessControlList) policy), csw.getSession());
+                LocalAccessControlEntry newEntry = new LocalAccessControlEntry(csw,principal, privileges, isAllow, lr);
                 if (contains(((JackrabbitAccessControlList) policy).getAccessControlEntries(), newEntry)) {
                     LOG.info(
                             "Equivalent path-based entry exists for principal {} and effective path {} ",
@@ -711,19 +711,19 @@ public class AclUtil {
         private final Privilege[] privileges;
         private final boolean isAllow;
         private final LocalRestrictions restrictions;
-        private final PrivilegeCachingSessionWrapper pcsw;
+        private final CachingSessionWrapper csw;
 
-        LocalAccessControlEntry(PrivilegeCachingSessionWrapper pcsw, Principal principal, Privilege[] privileges, boolean isAllow) {
-            this(pcsw,principal, privileges, isAllow, null);
+        LocalAccessControlEntry(CachingSessionWrapper csw, Principal principal, Privilege[] privileges, boolean isAllow) {
+            this(csw,principal, privileges, isAllow, null);
         }
 
         LocalAccessControlEntry(
-                PrivilegeCachingSessionWrapper pcsw,
+                CachingSessionWrapper csw,
                 Principal principal,
                 Privilege[] privileges,
                 boolean isAllow,
                 LocalRestrictions restrictions) {
-            this.pcsw = pcsw;
+            this.csw = csw;
             this.principal = principal;
             this.privileges = privileges;
             this.isAllow = isAllow;
@@ -757,7 +757,7 @@ public class AclUtil {
 
             if (privileges != null) {
                 for (Privilege privilege : privileges) {
-                    expandedSet.addAll(pcsw.expandPrivilege(privilege));
+                    expandedSet.addAll(csw.expandPrivilege(privilege));
                 }
             }
 
diff --git a/src/main/java/org/apache/sling/jcr/repoinit/impl/AclVisitor.java b/src/main/java/org/apache/sling/jcr/repoinit/impl/AclVisitor.java
index 492961f..4dfe460 100644
--- a/src/main/java/org/apache/sling/jcr/repoinit/impl/AclVisitor.java
+++ b/src/main/java/org/apache/sling/jcr/repoinit/impl/AclVisitor.java
@@ -55,7 +55,7 @@ class AclVisitor extends DoNothingVisitor {
      */
     public static final String OPTION_IGNORE_MISSING_PRINCIPAL = "ignoreMissingPrincipal";
 
-    private final PrivilegeCachingSessionWrapper pcsw;
+    private final CachingSessionWrapper csw;
 
     private enum Instruction {
         SET,
@@ -70,7 +70,7 @@ class AclVisitor extends DoNothingVisitor {
      */
     public AclVisitor(Session s)  {
         super(s);
-        pcsw = new PrivilegeCachingSessionWrapper(s);
+        csw = new CachingSessionWrapper(s);
     }
 
     private void handleAclLine(
@@ -80,17 +80,17 @@ class AclVisitor extends DoNothingVisitor {
         if (action == AclLine.Action.REMOVE) {
             report("remove not supported. use 'remove acl' instead.");
         } else if (action == AclLine.Action.REMOVE_ALL) {
-            AclUtil.removeEntries(pcsw, principals, paths);
+            AclUtil.removeEntries(csw, principals, paths);
         } else {
             final boolean isAllow = action == AclLine.Action.ALLOW;
             final String actionName = isAllow ? "allow" : "deny";
             final List<String> privileges = line.getProperty(PROP_PRIVILEGES);
             if (instruction == Instruction.SET) {
                 log.info("Adding ACL '{}' entry '{}' for {} on {}", actionName, privileges, principals, paths);
-                AclUtil.setAcl(pcsw, principals, paths, privileges, isAllow, line.getRestrictions(), options);
+                AclUtil.setAcl(csw, principals, paths, privileges, isAllow, line.getRestrictions(), options);
             } else if (instruction == Instruction.REMOVE) {
                 log.info("Removing ACL '{}' entry '{}' for {} on {}", actionName, privileges, principals, paths);
-                AclUtil.removeEntries(pcsw, principals, paths, privileges, isAllow, line.getRestrictions());
+                AclUtil.removeEntries(csw, principals, paths, privileges, isAllow, line.getRestrictions());
             }
         }
     }
@@ -133,7 +133,7 @@ class AclVisitor extends DoNothingVisitor {
         for (String principalName : s.getPrincipals()) {
             try {
                 log.info("Adding principal-based access control entry for {}", principalName);
-                AclUtil.setPrincipalAcl(pcsw, principalName, s.getLines(), false);
+                AclUtil.setPrincipalAcl(csw, principalName, s.getLines(), false);
             } catch (Exception e) {
                 report(e, "Failed to set principal-based ACL (" + e.getMessage() + ")");
             }
@@ -145,7 +145,7 @@ class AclVisitor extends DoNothingVisitor {
         for (String principalName : s.getPrincipals()) {
             try {
                 log.info("Enforcing principal-based access control entry for {}", principalName);
-                AclUtil.setPrincipalAcl(pcsw, principalName, s.getLines(), true);
+                AclUtil.setPrincipalAcl(csw, principalName, s.getLines(), true);
             } catch (Exception e) {
                 report(e, "Failed to set principal-based ACL (" + e.getMessage() + ")");
             }
@@ -181,7 +181,7 @@ class AclVisitor extends DoNothingVisitor {
         for (String principalName : s.getPrincipals()) {
             try {
                 log.info("Removing principal-based access control entries for {}", principalName);
-                AclUtil.removePrincipalEntries(pcsw, principalName, s.getLines());
+                AclUtil.removePrincipalEntries(csw, principalName, s.getLines());
             } catch (Exception e) {
                 report(e, "Failed to remove principal-based access control entries (" + e.getMessage() + ")");
             }
@@ -193,7 +193,7 @@ class AclVisitor extends DoNothingVisitor {
         for (String principalName : s.getPrincipals()) {
             try {
                 log.info("Removing access control policy for {}", principalName);
-                AclUtil.removePolicy(pcsw, principalName);
+                AclUtil.removePolicy(csw, principalName);
             } catch (RepositoryException e) {
                 report(e, "Failed to remove ACL (" + e.getMessage() + ")");
             }
@@ -203,7 +203,7 @@ class AclVisitor extends DoNothingVisitor {
     @Override
     public void visitDeleteAclPaths(DeleteAclPaths s) {
         try {
-            AclUtil.removePolicies(pcsw, s.getPaths());
+            AclUtil.removePolicies(csw, s.getPaths());
         } catch (RepositoryException e) {
             report(e, "Failed to remove ACL (" + e.getMessage() + ")");
         }
@@ -214,7 +214,7 @@ class AclVisitor extends DoNothingVisitor {
         for (String principalName : s.getPrincipals()) {
             try {
                 log.info("Removing principal-based access control policy for {}", principalName);
-                AclUtil.removePrincipalPolicy(pcsw, principalName);
+                AclUtil.removePrincipalPolicy(csw, principalName);
             } catch (RepositoryException e) {
                 report(e, "Failed to remove principal-based ACL (" + e.getMessage() + ")");
             }
diff --git a/src/main/java/org/apache/sling/jcr/repoinit/impl/PrivilegeCachingSessionWrapper.java b/src/main/java/org/apache/sling/jcr/repoinit/impl/CachingSessionWrapper.java
similarity index 97%
rename from src/main/java/org/apache/sling/jcr/repoinit/impl/PrivilegeCachingSessionWrapper.java
rename to src/main/java/org/apache/sling/jcr/repoinit/impl/CachingSessionWrapper.java
index a903546..167c54f 100644
--- a/src/main/java/org/apache/sling/jcr/repoinit/impl/PrivilegeCachingSessionWrapper.java
+++ b/src/main/java/org/apache/sling/jcr/repoinit/impl/CachingSessionWrapper.java
@@ -39,7 +39,7 @@ import com.google.common.collect.Lists;
 /**
  * A simple wrapper around a session, which can cache the privilege resolution
  */
-public class PrivilegeCachingSessionWrapper {
+public class CachingSessionWrapper {
 
     JackrabbitSession session;
     JackrabbitAccessControlManager acMgr;
@@ -47,7 +47,7 @@ public class PrivilegeCachingSessionWrapper {
     Map<Privilege,List<Privilege>> privilegeToAggreate = new HashMap<>();
     Map<String,Principal> idToPrincipal = new HashMap<>();
     
-    public PrivilegeCachingSessionWrapper (Session session) {
+    public CachingSessionWrapper (Session session) {
         AclUtil.checkState(session instanceof JackrabbitSession,"A Jackrabbit Session is required");
         this.session = (JackrabbitSession) session;
         try {
diff --git a/src/test/java/org/apache/sling/jcr/repoinit/PrincipalBasedAclTest.java b/src/test/java/org/apache/sling/jcr/repoinit/PrincipalBasedAclTest.java
index cf1d5af..c45c5b1 100644
--- a/src/test/java/org/apache/sling/jcr/repoinit/PrincipalBasedAclTest.java
+++ b/src/test/java/org/apache/sling/jcr/repoinit/PrincipalBasedAclTest.java
@@ -57,7 +57,7 @@ import org.apache.jackrabbit.oak.spi.security.principal.SystemUserPrincipal;
 import org.apache.jackrabbit.oak.spi.security.user.UserConfiguration;
 import org.apache.jackrabbit.oak.spi.security.user.UserConstants;
 import org.apache.sling.jcr.repoinit.impl.AclUtil;
-import org.apache.sling.jcr.repoinit.impl.PrivilegeCachingSessionWrapper;
+import org.apache.sling.jcr.repoinit.impl.CachingSessionWrapper;
 import org.apache.sling.jcr.repoinit.impl.RepoInitException;
 import org.apache.sling.jcr.repoinit.impl.TestUtil;
 import org.apache.sling.repoinit.parser.RepoInitParsingException;
@@ -805,7 +805,7 @@ public class PrincipalBasedAclTest {
         line.setProperty(AclLine.PROP_PRINCIPALS, Collections.singletonList(principal.getName()));
         line.setProperty(AclLine.PROP_PRIVILEGES, Collections.singletonList(Privilege.JCR_READ));
         line.setProperty(AclLine.PROP_PATHS, Collections.singletonList(":home:" + U.username + "#"));
-        AclUtil.setPrincipalAcl(new PrivilegeCachingSessionWrapper(U.adminSession), U.username, Collections.singletonList(line), false);
+        AclUtil.setPrincipalAcl(new CachingSessionWrapper(U.adminSession), U.username, Collections.singletonList(line), false);
 
         PrincipalAccessControlList acl = getAcl(principal, U.adminSession);
         assertNotNull(acl);
diff --git a/src/test/java/org/apache/sling/jcr/repoinit/impl/AclUtilTest.java b/src/test/java/org/apache/sling/jcr/repoinit/impl/AclUtilTest.java
index 8dc7700..56f34b5 100644
--- a/src/test/java/org/apache/sling/jcr/repoinit/impl/AclUtilTest.java
+++ b/src/test/java/org/apache/sling/jcr/repoinit/impl/AclUtilTest.java
@@ -627,7 +627,7 @@ public class AclUtilTest {
         return AccessControlUtils.privilegesFromNames(U.adminSession, privilegeNames);
     }
 
-    private static PrivilegeCachingSessionWrapper toPCSessionWrapper (Session session) {
-        return new PrivilegeCachingSessionWrapper(session);
+    private static CachingSessionWrapper toPCSessionWrapper (Session session) {
+        return new CachingSessionWrapper(session);
     }
 }
diff --git a/src/test/java/org/apache/sling/jcr/repoinit/impl/ManyServiceUsersTest.java b/src/test/java/org/apache/sling/jcr/repoinit/impl/ManyServiceUsersTest.java
index 1a93836..34a57f3 100644
--- a/src/test/java/org/apache/sling/jcr/repoinit/impl/ManyServiceUsersTest.java
+++ b/src/test/java/org/apache/sling/jcr/repoinit/impl/ManyServiceUsersTest.java
@@ -85,7 +85,7 @@ public class ManyServiceUsersTest {
 
                 try {
                     AclUtil.setAcl(
-                            new PrivilegeCachingSessionWrapper(otherSession),
+                            new CachingSessionWrapper(otherSession),
                             Arrays.asList(username),
                             Arrays.asList(path),
                             Arrays.asList("jcr:read"),


(sling-org-apache-sling-jcr-repoinit) 02/06: cache once-retrieved privileges

Posted by jo...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

joerghoh pushed a commit to branch SLING-12642-cache-privileges
in repository https://gitbox.apache.org/repos/asf/sling-org-apache-sling-jcr-repoinit.git

commit a4e4b8b7be3afaace8a8eea1792f434e231a3176
Author: Joerg Hoh <jo...@apache.org>
AuthorDate: Sun Feb 9 14:11:04 2025 +0100

    cache once-retrieved privileges
---
 .../apache/sling/jcr/repoinit/impl/AclUtil.java    | 10 ++++----
 .../impl/PrivilegeCachingSessionWrapper.java       | 29 ++++++++++++++++++++++
 2 files changed, 34 insertions(+), 5 deletions(-)

diff --git a/src/main/java/org/apache/sling/jcr/repoinit/impl/AclUtil.java b/src/main/java/org/apache/sling/jcr/repoinit/impl/AclUtil.java
index 8a3cbef..a459c45 100644
--- a/src/main/java/org/apache/sling/jcr/repoinit/impl/AclUtil.java
+++ b/src/main/java/org/apache/sling/jcr/repoinit/impl/AclUtil.java
@@ -168,7 +168,7 @@ public class AclUtil {
         AccessControlManager acMgr = pcsw.getAccessControlManager();
 
         final String[] privArray = privileges.toArray(new String[privileges.size()]);
-        final Privilege[] jcrPriv = AccessControlUtils.privilegesFromNames(acMgr, privArray);
+        final Privilege[] jcrPriv = pcsw.privilegesFromNames(privArray);
 
         JackrabbitAccessControlList acl = getAccessControlList(acMgr, jcrPath, true);
         checkState(acl != null, "No JackrabbitAccessControlList available for path {0}", jcrPath);
@@ -332,7 +332,7 @@ public class AclUtil {
 
                     LocalRestrictions restr = createLocalRestrictions(restrictionClauses, acl, pcsw.getSession());
                     Privilege[] privs =
-                            AccessControlUtils.privilegesFromNames(acMgr, privileges.toArray(new String[0]));
+                            pcsw.privilegesFromNames(privileges.toArray(new String[0]));
 
                     for (AccessControlEntry ace : acl.getAccessControlEntries()) {
                         Principal principal = ace.getPrincipal();
@@ -404,8 +404,8 @@ public class AclUtil {
                     modified = true;
                 }
             } else if (action == AclLine.Action.ALLOW) {
-                final Privilege[] privileges = AccessControlUtils.privilegesFromNames(
-                        acMgr, line.getProperty(PROP_PRIVILEGES).toArray(new String[0]));
+                final Privilege[] privileges = pcsw.privilegesFromNames(
+                        line.getProperty(PROP_PRIVILEGES).toArray(new String[0]));
                 for (String effectivePath : jcrPaths) {
                     if (acl == null) {
                         // no PrincipalAccessControlList available: don't fail if an equivalent path-based entry with
@@ -465,7 +465,7 @@ public class AclUtil {
             List<String> jcrPaths = getJcrPaths(pcsw.getSession(), line.getProperty(PROP_PATHS));
             LocalRestrictions restr = createLocalRestrictions(line.getRestrictions(), acl, pcsw.getSession());
             List<String> privNames = line.getProperty(PROP_PRIVILEGES);
-            Privilege[] privs = AccessControlUtils.privilegesFromNames(acMgr, privNames.toArray(new String[0]));
+            Privilege[] privs = pcsw.privilegesFromNames(privNames.toArray(new String[0]));
             Predicate<PrincipalAccessControlList.Entry> predicate = entry -> {
                 if (!jcrPaths.contains(entry.getEffectivePath())) {
                     return false;
diff --git a/src/main/java/org/apache/sling/jcr/repoinit/impl/PrivilegeCachingSessionWrapper.java b/src/main/java/org/apache/sling/jcr/repoinit/impl/PrivilegeCachingSessionWrapper.java
index a9610cb..9c642a6 100644
--- a/src/main/java/org/apache/sling/jcr/repoinit/impl/PrivilegeCachingSessionWrapper.java
+++ b/src/main/java/org/apache/sling/jcr/repoinit/impl/PrivilegeCachingSessionWrapper.java
@@ -18,8 +18,15 @@
  */
 package org.apache.sling.jcr.repoinit.impl;
 
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.Map;
+import java.util.Set;
+
 import javax.jcr.RepositoryException;
 import javax.jcr.Session;
+import javax.jcr.security.AccessControlManager;
+import javax.jcr.security.Privilege;
 
 import org.apache.jackrabbit.api.JackrabbitSession;
 import org.apache.jackrabbit.api.security.JackrabbitAccessControlManager;
@@ -31,6 +38,7 @@ public class PrivilegeCachingSessionWrapper {
 
     JackrabbitSession session;
     JackrabbitAccessControlManager acMgr;
+    Map<String,Privilege> nameToPrivilegeMap = new HashMap<>();
     
     public PrivilegeCachingSessionWrapper (Session session) {
         AclUtil.checkState(session instanceof JackrabbitSession,"A Jackrabbit Session is required");
@@ -38,6 +46,7 @@ public class PrivilegeCachingSessionWrapper {
         try {
             AclUtil.checkState(session.getAccessControlManager() instanceof JackrabbitAccessControlManager, 
                     "A Jachrabbit AccessControlManager is required");
+            this.acMgr = (JackrabbitAccessControlManager) session.getAccessControlManager();
         } catch (RepositoryException e) {
             throw new IllegalStateException("Cannot retrieve the AcccessControlManager");
         }
@@ -51,4 +60,24 @@ public class PrivilegeCachingSessionWrapper {
         return acMgr;
     }
 
+    /**
+     * Retrieve the matching privileges from the given privilege names; uses internally a cache. The retrieval
+     * logic is identical to AccessControlUtils.privilegesFromName, but with caching
+     * @param privilegeNames the name of the privileges
+     * @return the matching privileges
+     * @throws RepositoryException in case of errors
+     */
+    public Privilege[] privilegesFromNames(String... privilegeNames) throws RepositoryException {
+        Set<Privilege> privileges = new HashSet<Privilege>(privilegeNames.length);
+        for (String privName : privilegeNames) {
+            if (nameToPrivilegeMap.containsKey(privName)) {
+                privileges.add(nameToPrivilegeMap.get(privName));
+            } else {
+                Privilege p = acMgr.privilegeFromName(privName);
+                nameToPrivilegeMap.put(privName, p);
+                privileges.add(p);
+            }
+        }
+        return privileges.toArray(new Privilege[privileges.size()]);
+    }
 }


(sling-org-apache-sling-jcr-repoinit) 05/06: do not cache non-resolved principals

Posted by jo...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

joerghoh pushed a commit to branch SLING-12642-cache-privileges
in repository https://gitbox.apache.org/repos/asf/sling-org-apache-sling-jcr-repoinit.git

commit 269ce910915c8d43b4119c4950942fccc07ad4d2
Author: Joerg Hoh <jo...@apache.org>
AuthorDate: Mon Feb 10 15:47:45 2025 +0100

    do not cache non-resolved principals
---
 .../jcr/repoinit/impl/PrivilegeCachingSessionWrapper.java | 15 +++++++++------
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/src/main/java/org/apache/sling/jcr/repoinit/impl/PrivilegeCachingSessionWrapper.java b/src/main/java/org/apache/sling/jcr/repoinit/impl/PrivilegeCachingSessionWrapper.java
index 3708d90..a903546 100644
--- a/src/main/java/org/apache/sling/jcr/repoinit/impl/PrivilegeCachingSessionWrapper.java
+++ b/src/main/java/org/apache/sling/jcr/repoinit/impl/PrivilegeCachingSessionWrapper.java
@@ -89,7 +89,7 @@ public class PrivilegeCachingSessionWrapper {
     }
 
     /**
-     * If a privilege is an aggreated, return the privilges it contains, otherwise return the privilege itself
+     * If a privilege is an aggregated, return the privileges it contains, otherwise return the privilege itself
      * @param priv the privilege 
      * @return
      */
@@ -104,12 +104,15 @@ public class PrivilegeCachingSessionWrapper {
     }
 
     public Principal getPrincipal (String principalName) throws RepositoryException {
+        // Do not cache null principals
         if (idToPrincipal.containsKey(principalName)) {
-            return idToPrincipal.get(principalName);
-        } else {
-            Principal p = AccessControlUtils.getPrincipal(this.getSession(), principalName);
-            idToPrincipal.put(principalName, p);
-            return p;
+            Principal p = idToPrincipal.get(principalName);
+            if (p != null) {
+                return p;
+            }
         }
+        Principal p = AccessControlUtils.getPrincipal(this.getSession(), principalName);
+        idToPrincipal.put(principalName, p);
+        return p;
     }
 }


(sling-org-apache-sling-jcr-repoinit) 01/06: SLING-12642 introduce a wrapper for a session

Posted by jo...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

joerghoh pushed a commit to branch SLING-12642-cache-privileges
in repository https://gitbox.apache.org/repos/asf/sling-org-apache-sling-jcr-repoinit.git

commit a9e282a4e69fabd1f1aa5adf68f80d170be0bd66
Author: Joerg Hoh <jo...@apache.org>
AuthorDate: Sun Feb 9 13:50:33 2025 +0100

    SLING-12642 introduce a wrapper for a session
---
 .../apache/sling/jcr/repoinit/impl/AclUtil.java    | 90 +++++++++++-----------
 .../apache/sling/jcr/repoinit/impl/AclVisitor.java | 23 +++---
 .../impl/PrivilegeCachingSessionWrapper.java       | 54 +++++++++++++
 .../sling/jcr/repoinit/PrincipalBasedAclTest.java  |  3 +-
 .../sling/jcr/repoinit/impl/AclUtilTest.java       | 28 ++++---
 .../jcr/repoinit/impl/ManyServiceUsersTest.java    |  2 +-
 .../apache/sling/jcr/repoinit/impl/TestUtil.java   |  4 +-
 7 files changed, 133 insertions(+), 71 deletions(-)

diff --git a/src/main/java/org/apache/sling/jcr/repoinit/impl/AclUtil.java b/src/main/java/org/apache/sling/jcr/repoinit/impl/AclUtil.java
index e8a9e0b..8a3cbef 100644
--- a/src/main/java/org/apache/sling/jcr/repoinit/impl/AclUtil.java
+++ b/src/main/java/org/apache/sling/jcr/repoinit/impl/AclUtil.java
@@ -132,13 +132,13 @@ public class AclUtil {
     }
 
     public static void setAcl(
-            Session session, List<String> principals, List<String> paths, List<String> privileges, boolean isAllow)
+            PrivilegeCachingSessionWrapper pcsw, List<String> principals, List<String> paths, List<String> privileges, boolean isAllow)
             throws RepositoryException {
-        setAcl(session, principals, paths, privileges, isAllow, Collections.emptyList(), Collections.emptyList());
+        setAcl(pcsw, principals, paths, privileges, isAllow, Collections.emptyList(), Collections.emptyList());
     }
 
     public static void setAcl(
-            Session session,
+            PrivilegeCachingSessionWrapper pcsw,
             List<String> principals,
             List<String> paths,
             List<String> privileges,
@@ -147,16 +147,16 @@ public class AclUtil {
             List<String> options)
             throws RepositoryException {
 
-        for (String jcrPath : getJcrPaths(session, paths)) {
-            if (jcrPath != null && !session.nodeExists(jcrPath)) {
+        for (String jcrPath : getJcrPaths(pcsw.getSession(), paths)) {
+            if (jcrPath != null && !pcsw.getSession().nodeExists(jcrPath)) {
                 throw new PathNotFoundException("Cannot set ACL on non-existent path " + jcrPath);
             }
-            setAcl(session, principals, jcrPath, privileges, isAllow, restrictionClauses, options);
+            setAcl(pcsw, principals, jcrPath, privileges, isAllow, restrictionClauses, options);
         }
     }
 
     private static void setAcl(
-            Session session,
+            PrivilegeCachingSessionWrapper pcsw,
             List<String> principals,
             String jcrPath,
             List<String> privileges,
@@ -165,7 +165,7 @@ public class AclUtil {
             List<String> options)
             throws RepositoryException {
 
-        AccessControlManager acMgr = session.getAccessControlManager();
+        AccessControlManager acMgr = pcsw.getAccessControlManager();
 
         final String[] privArray = privileges.toArray(new String[privileges.size()]);
         final Privilege[] jcrPriv = AccessControlUtils.privilegesFromNames(acMgr, privArray);
@@ -173,7 +173,7 @@ public class AclUtil {
         JackrabbitAccessControlList acl = getAccessControlList(acMgr, jcrPath, true);
         checkState(acl != null, "No JackrabbitAccessControlList available for path {0}", jcrPath);
 
-        LocalRestrictions localRestrictions = createLocalRestrictions(restrictionClauses, acl, session);
+        LocalRestrictions localRestrictions = createLocalRestrictions(restrictionClauses, acl, pcsw.getSession());
 
         AccessControlEntry[] existingAces = acl.getAccessControlEntries();
 
@@ -182,7 +182,7 @@ public class AclUtil {
                 .map(o -> o.contains(AclVisitor.OPTION_IGNORE_MISSING_PRINCIPAL))
                 .orElse(false);
         for (String name : principals) {
-            final Principal principal = getPrincipal(session, name, ignoreMissingPrincipal);
+            final Principal principal = getPrincipal(pcsw.getSession(), name, ignoreMissingPrincipal);
             LocalAccessControlEntry newAce =
                     new LocalAccessControlEntry(principal, jcrPriv, isAllow, localRestrictions);
             if (contains(existingAces, newAce)) {
@@ -236,9 +236,9 @@ public class AclUtil {
      * @param principalName
      * @throws RepositoryException
      */
-    public static void removePolicy(@NotNull Session session, @NotNull final String principalName)
+    public static void removePolicy(@NotNull PrivilegeCachingSessionWrapper pcsw, @NotNull final String principalName)
             throws RepositoryException {
-        Principal principal = AccessControlUtils.getPrincipal(session, principalName);
+        Principal principal = AccessControlUtils.getPrincipal(pcsw.getSession(), principalName);
         if (principal == null) {
             LOG.info("Principal {} does not exist.", principalName);
             // using PrincipalImpl will prevent 'removePolicy' from failing with AccessControlException
@@ -246,7 +246,7 @@ public class AclUtil {
             principal = new PrincipalImpl(principalName);
         }
 
-        JackrabbitAccessControlManager acMgr = getJACM(session);
+        JackrabbitAccessControlManager acMgr = pcsw.getAccessControlManager();
         for (JackrabbitAccessControlPolicy policy : acMgr.getPolicies(principal)) {
             // make sure not to remove the principal-based access control list but instead only drop
             // resource-based access control content for the given principal
@@ -263,11 +263,11 @@ public class AclUtil {
      * @param paths
      * @throws RepositoryException
      */
-    public static void removePolicies(@NotNull Session session, @NotNull List<String> paths)
+    public static void removePolicies(@NotNull PrivilegeCachingSessionWrapper pcsw, @NotNull List<String> paths)
             throws RepositoryException {
-        AccessControlManager acMgr = session.getAccessControlManager();
-        for (String jcrPath : getJcrPaths(session, paths)) {
-            if (!isValidPath(session, jcrPath)) {
+        AccessControlManager acMgr = pcsw.getAccessControlManager();
+        for (String jcrPath : getJcrPaths(pcsw.session, paths)) {
+            if (!isValidPath(pcsw.getSession(), jcrPath)) {
                 LOG.info("Cannot remove ACL; no node at {} ", jcrPath);
                 continue;
             }
@@ -282,12 +282,12 @@ public class AclUtil {
     }
 
     public static void removeEntries(
-            @NotNull Session session, @NotNull List<String> principals, @NotNull List<String> paths)
+            @NotNull PrivilegeCachingSessionWrapper pcsw, @NotNull List<String> principals, @NotNull List<String> paths)
             throws RepositoryException {
         Set<String> principalNames = new HashSet<>(principals);
-        AccessControlManager acMgr = session.getAccessControlManager();
-        for (String jcrPath : getJcrPaths(session, paths)) {
-            if (!isValidPath(session, jcrPath)) {
+        AccessControlManager acMgr = pcsw.getAccessControlManager();
+        for (String jcrPath : getJcrPaths(pcsw.getSession(), paths)) {
+            if (!isValidPath(pcsw.getSession(), jcrPath)) {
                 LOG.info("Cannot remove access control entries on non-existent path {}", jcrPath);
             } else {
                 JackrabbitAccessControlList acl = getAccessControlList(acMgr, jcrPath, false);
@@ -313,7 +313,7 @@ public class AclUtil {
     }
 
     public static void removeEntries(
-            @NotNull Session session,
+            @NotNull PrivilegeCachingSessionWrapper pcsw,
             @NotNull List<String> principals,
             @NotNull List<String> paths,
             List<String> privileges,
@@ -321,16 +321,16 @@ public class AclUtil {
             List<RestrictionClause> restrictionClauses)
             throws RepositoryException {
         Set<String> principalNames = new HashSet<>(principals);
-        AccessControlManager acMgr = session.getAccessControlManager();
-        for (String jcrPath : getJcrPaths(session, paths)) {
-            if (!isValidPath(session, jcrPath)) {
+        AccessControlManager acMgr = pcsw.getAccessControlManager();
+        for (String jcrPath : getJcrPaths(pcsw.getSession(), paths)) {
+            if (!isValidPath(pcsw.getSession(), jcrPath)) {
                 LOG.info("Cannot remove access control entries on non-existent path {}", jcrPath);
             } else {
                 JackrabbitAccessControlList acl = getAccessControlList(acMgr, jcrPath, false);
                 if (acl != null) {
                     boolean modified = false;
 
-                    LocalRestrictions restr = createLocalRestrictions(restrictionClauses, acl, session);
+                    LocalRestrictions restr = createLocalRestrictions(restrictionClauses, acl, pcsw.getSession());
                     Privilege[] privs =
                             AccessControlUtils.privilegesFromNames(acMgr, privileges.toArray(new String[0]));
 
@@ -368,15 +368,15 @@ public class AclUtil {
     }
 
     public static void setPrincipalAcl(
-            Session session, String principalName, Collection<AclLine> lines, boolean isStrict)
+            PrivilegeCachingSessionWrapper pcsw, String principalName, Collection<AclLine> lines, boolean isStrict)
             throws RepositoryException {
-        final JackrabbitAccessControlManager acMgr = getJACM(session);
-        Principal principal = AccessControlUtils.getPrincipal(session, principalName);
+        final JackrabbitAccessControlManager acMgr = pcsw.getAccessControlManager();
+        Principal principal = AccessControlUtils.getPrincipal(pcsw.getSession(), principalName);
         if (principal == null) {
             // due to transient nature of the repo-init the principal lookup may not succeed if completed through query
             // -> save transient changes and retry principal lookup
-            session.save();
-            principal = AccessControlUtils.getPrincipal(session, principalName);
+            pcsw.getSession().save();
+            principal = AccessControlUtils.getPrincipal(pcsw.getSession(), principalName);
             checkState(principal != null, PRINCIPAL_NOT_FOUND_PATTERN, principalName);
         }
 
@@ -393,7 +393,7 @@ public class AclUtil {
         boolean modified = false;
         for (AclLine line : lines) {
             AclLine.Action action = line.getAction();
-            List<String> jcrPaths = getJcrPaths(session, line.getProperty(PROP_PATHS));
+            List<String> jcrPaths = getJcrPaths(pcsw.getSession(), line.getProperty(PROP_PATHS));
             if (action == AclLine.Action.DENY) {
                 throw new AccessControlException("PrincipalAccessControlList doesn't support 'deny' entries.");
             } else if (action == AclLine.Action.REMOVE) {
@@ -413,7 +413,7 @@ public class AclUtil {
                         // or if there exists no node at the effective path (unable to evaluate path-based entries).
                         LOG.info("No PrincipalAccessControlList available for principal {}", principal);
                         if (!containsEquivalentEntry(
-                                session, effectivePath, principal, privileges, true, line.getRestrictions())) {
+                                pcsw.getSession(), effectivePath, principal, privileges, true, line.getRestrictions())) {
                             LOG.warn(
                                     "No equivalent path-based entry exists for principal {} and effective path {} ",
                                     principal.getName(),
@@ -422,7 +422,7 @@ public class AclUtil {
                         }
                     } else {
                         final LocalRestrictions restrictions =
-                                createLocalRestrictions(line.getRestrictions(), acl, session);
+                                createLocalRestrictions(line.getRestrictions(), acl, pcsw.getSession());
                         final boolean added = acl.addEntry(
                                 effectivePath,
                                 privileges,
@@ -447,23 +447,23 @@ public class AclUtil {
         }
     }
 
-    public static void removePrincipalEntries(Session session, String principalName, Collection<AclLine> lines)
+    public static void removePrincipalEntries(@NotNull PrivilegeCachingSessionWrapper pcsw, String principalName, Collection<AclLine> lines)
             throws RepositoryException {
-        final JackrabbitAccessControlManager acMgr = getJACM(session);
-        Principal principal = AccessControlUtils.getPrincipal(session, principalName);
+        final JackrabbitAccessControlManager acMgr = pcsw.getAccessControlManager();
+        Principal principal = AccessControlUtils.getPrincipal(pcsw.getSession(), principalName);
         if (principal == null) {
             // due to transient nature of the repo-init the principal lookup may not succeed if completed through query
             // -> save transient changes and retry principal lookup
-            session.save();
-            principal = AccessControlUtils.getPrincipal(session, principalName);
+            pcsw.getSession().save();
+            principal = AccessControlUtils.getPrincipal(pcsw.getSession(), principalName);
             checkState(principal != null, PRINCIPAL_NOT_FOUND_PATTERN, principalName);
         }
 
         final PrincipalAccessControlList acl = getPrincipalAccessControlList(acMgr, principal, true);
         boolean modified = false;
         for (AclLine line : lines) {
-            List<String> jcrPaths = getJcrPaths(session, line.getProperty(PROP_PATHS));
-            LocalRestrictions restr = createLocalRestrictions(line.getRestrictions(), acl, session);
+            List<String> jcrPaths = getJcrPaths(pcsw.getSession(), line.getProperty(PROP_PATHS));
+            LocalRestrictions restr = createLocalRestrictions(line.getRestrictions(), acl, pcsw.getSession());
             List<String> privNames = line.getProperty(PROP_PRIVILEGES);
             Privilege[] privs = AccessControlUtils.privilegesFromNames(acMgr, privNames.toArray(new String[0]));
             Predicate<PrincipalAccessControlList.Entry> predicate = entry -> {
@@ -498,15 +498,15 @@ public class AclUtil {
      * @param principalName
      * @throws RepositoryException
      */
-    public static void removePrincipalPolicy(@NotNull Session session, @NotNull String principalName)
+    public static void removePrincipalPolicy(@NotNull PrivilegeCachingSessionWrapper pcsw, @NotNull String principalName)
             throws RepositoryException {
-        Principal principal = AccessControlUtils.getPrincipal(session, principalName);
+        Principal principal = AccessControlUtils.getPrincipal(pcsw.getSession(), principalName);
         if (principal == null) {
             LOG.info("Cannot remove principal-based ACL. Principal {} does not exist.", principalName);
             return;
         }
 
-        JackrabbitAccessControlManager acMgr = getJACM(session);
+        JackrabbitAccessControlManager acMgr = pcsw.getAccessControlManager();
         PrincipalAccessControlList acl = getPrincipalAccessControlList(acMgr, principal, false);
         if (acl == null) {
             LOG.info("Cannot remove principal-based ACL for principal {}. No such policy exists.", principalName);
@@ -674,7 +674,7 @@ public class AclUtil {
                 + entry.isAllow() + ", restrictionNames: " + entry.getRestrictionNames() + "]";
     }
 
-    private static void checkState(boolean expression, String msgPattern, Object... args) {
+    static void checkState(boolean expression, String msgPattern, Object... args) {
         if (!expression) {
             if (args == null) {
                 throw new IllegalStateException(msgPattern);
diff --git a/src/main/java/org/apache/sling/jcr/repoinit/impl/AclVisitor.java b/src/main/java/org/apache/sling/jcr/repoinit/impl/AclVisitor.java
index f96f696..492961f 100644
--- a/src/main/java/org/apache/sling/jcr/repoinit/impl/AclVisitor.java
+++ b/src/main/java/org/apache/sling/jcr/repoinit/impl/AclVisitor.java
@@ -55,6 +55,8 @@ class AclVisitor extends DoNothingVisitor {
      */
     public static final String OPTION_IGNORE_MISSING_PRINCIPAL = "ignoreMissingPrincipal";
 
+    private final PrivilegeCachingSessionWrapper pcsw;
+
     private enum Instruction {
         SET,
         REMOVE
@@ -66,8 +68,9 @@ class AclVisitor extends DoNothingVisitor {
      * @param s must have sufficient rights to create users
      *          and set ACLs.
      */
-    public AclVisitor(Session s) {
+    public AclVisitor(Session s)  {
         super(s);
+        pcsw = new PrivilegeCachingSessionWrapper(s);
     }
 
     private void handleAclLine(
@@ -77,17 +80,17 @@ class AclVisitor extends DoNothingVisitor {
         if (action == AclLine.Action.REMOVE) {
             report("remove not supported. use 'remove acl' instead.");
         } else if (action == AclLine.Action.REMOVE_ALL) {
-            AclUtil.removeEntries(session, principals, paths);
+            AclUtil.removeEntries(pcsw, principals, paths);
         } else {
             final boolean isAllow = action == AclLine.Action.ALLOW;
             final String actionName = isAllow ? "allow" : "deny";
             final List<String> privileges = line.getProperty(PROP_PRIVILEGES);
             if (instruction == Instruction.SET) {
                 log.info("Adding ACL '{}' entry '{}' for {} on {}", actionName, privileges, principals, paths);
-                AclUtil.setAcl(session, principals, paths, privileges, isAllow, line.getRestrictions(), options);
+                AclUtil.setAcl(pcsw, principals, paths, privileges, isAllow, line.getRestrictions(), options);
             } else if (instruction == Instruction.REMOVE) {
                 log.info("Removing ACL '{}' entry '{}' for {} on {}", actionName, privileges, principals, paths);
-                AclUtil.removeEntries(session, principals, paths, privileges, isAllow, line.getRestrictions());
+                AclUtil.removeEntries(pcsw, principals, paths, privileges, isAllow, line.getRestrictions());
             }
         }
     }
@@ -130,7 +133,7 @@ class AclVisitor extends DoNothingVisitor {
         for (String principalName : s.getPrincipals()) {
             try {
                 log.info("Adding principal-based access control entry for {}", principalName);
-                AclUtil.setPrincipalAcl(session, principalName, s.getLines(), false);
+                AclUtil.setPrincipalAcl(pcsw, principalName, s.getLines(), false);
             } catch (Exception e) {
                 report(e, "Failed to set principal-based ACL (" + e.getMessage() + ")");
             }
@@ -142,7 +145,7 @@ class AclVisitor extends DoNothingVisitor {
         for (String principalName : s.getPrincipals()) {
             try {
                 log.info("Enforcing principal-based access control entry for {}", principalName);
-                AclUtil.setPrincipalAcl(session, principalName, s.getLines(), true);
+                AclUtil.setPrincipalAcl(pcsw, principalName, s.getLines(), true);
             } catch (Exception e) {
                 report(e, "Failed to set principal-based ACL (" + e.getMessage() + ")");
             }
@@ -178,7 +181,7 @@ class AclVisitor extends DoNothingVisitor {
         for (String principalName : s.getPrincipals()) {
             try {
                 log.info("Removing principal-based access control entries for {}", principalName);
-                AclUtil.removePrincipalEntries(session, principalName, s.getLines());
+                AclUtil.removePrincipalEntries(pcsw, principalName, s.getLines());
             } catch (Exception e) {
                 report(e, "Failed to remove principal-based access control entries (" + e.getMessage() + ")");
             }
@@ -190,7 +193,7 @@ class AclVisitor extends DoNothingVisitor {
         for (String principalName : s.getPrincipals()) {
             try {
                 log.info("Removing access control policy for {}", principalName);
-                AclUtil.removePolicy(session, principalName);
+                AclUtil.removePolicy(pcsw, principalName);
             } catch (RepositoryException e) {
                 report(e, "Failed to remove ACL (" + e.getMessage() + ")");
             }
@@ -200,7 +203,7 @@ class AclVisitor extends DoNothingVisitor {
     @Override
     public void visitDeleteAclPaths(DeleteAclPaths s) {
         try {
-            AclUtil.removePolicies(session, s.getPaths());
+            AclUtil.removePolicies(pcsw, s.getPaths());
         } catch (RepositoryException e) {
             report(e, "Failed to remove ACL (" + e.getMessage() + ")");
         }
@@ -211,7 +214,7 @@ class AclVisitor extends DoNothingVisitor {
         for (String principalName : s.getPrincipals()) {
             try {
                 log.info("Removing principal-based access control policy for {}", principalName);
-                AclUtil.removePrincipalPolicy(session, principalName);
+                AclUtil.removePrincipalPolicy(pcsw, principalName);
             } catch (RepositoryException e) {
                 report(e, "Failed to remove principal-based ACL (" + e.getMessage() + ")");
             }
diff --git a/src/main/java/org/apache/sling/jcr/repoinit/impl/PrivilegeCachingSessionWrapper.java b/src/main/java/org/apache/sling/jcr/repoinit/impl/PrivilegeCachingSessionWrapper.java
new file mode 100644
index 0000000..a9610cb
--- /dev/null
+++ b/src/main/java/org/apache/sling/jcr/repoinit/impl/PrivilegeCachingSessionWrapper.java
@@ -0,0 +1,54 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.sling.jcr.repoinit.impl;
+
+import javax.jcr.RepositoryException;
+import javax.jcr.Session;
+
+import org.apache.jackrabbit.api.JackrabbitSession;
+import org.apache.jackrabbit.api.security.JackrabbitAccessControlManager;
+
+/**
+ * A simple wrapper around a session, which can cache the privilege resolution
+ */
+public class PrivilegeCachingSessionWrapper {
+
+    JackrabbitSession session;
+    JackrabbitAccessControlManager acMgr;
+    
+    public PrivilegeCachingSessionWrapper (Session session) {
+        AclUtil.checkState(session instanceof JackrabbitSession,"A Jackrabbit Session is required");
+        this.session = (JackrabbitSession) session;
+        try {
+            AclUtil.checkState(session.getAccessControlManager() instanceof JackrabbitAccessControlManager, 
+                    "A Jachrabbit AccessControlManager is required");
+        } catch (RepositoryException e) {
+            throw new IllegalStateException("Cannot retrieve the AcccessControlManager");
+        }
+    }
+
+    public JackrabbitSession getSession() {
+        return session;
+    }
+
+    public JackrabbitAccessControlManager getAccessControlManager() {
+        return acMgr;
+    }
+
+}
diff --git a/src/test/java/org/apache/sling/jcr/repoinit/PrincipalBasedAclTest.java b/src/test/java/org/apache/sling/jcr/repoinit/PrincipalBasedAclTest.java
index 282a3a8..cf1d5af 100644
--- a/src/test/java/org/apache/sling/jcr/repoinit/PrincipalBasedAclTest.java
+++ b/src/test/java/org/apache/sling/jcr/repoinit/PrincipalBasedAclTest.java
@@ -57,6 +57,7 @@ import org.apache.jackrabbit.oak.spi.security.principal.SystemUserPrincipal;
 import org.apache.jackrabbit.oak.spi.security.user.UserConfiguration;
 import org.apache.jackrabbit.oak.spi.security.user.UserConstants;
 import org.apache.sling.jcr.repoinit.impl.AclUtil;
+import org.apache.sling.jcr.repoinit.impl.PrivilegeCachingSessionWrapper;
 import org.apache.sling.jcr.repoinit.impl.RepoInitException;
 import org.apache.sling.jcr.repoinit.impl.TestUtil;
 import org.apache.sling.repoinit.parser.RepoInitParsingException;
@@ -804,7 +805,7 @@ public class PrincipalBasedAclTest {
         line.setProperty(AclLine.PROP_PRINCIPALS, Collections.singletonList(principal.getName()));
         line.setProperty(AclLine.PROP_PRIVILEGES, Collections.singletonList(Privilege.JCR_READ));
         line.setProperty(AclLine.PROP_PATHS, Collections.singletonList(":home:" + U.username + "#"));
-        AclUtil.setPrincipalAcl(U.adminSession, U.username, Collections.singletonList(line), false);
+        AclUtil.setPrincipalAcl(new PrivilegeCachingSessionWrapper(U.adminSession), U.username, Collections.singletonList(line), false);
 
         PrincipalAccessControlList acl = getAcl(principal, U.adminSession);
         assertNotNull(acl);
diff --git a/src/test/java/org/apache/sling/jcr/repoinit/impl/AclUtilTest.java b/src/test/java/org/apache/sling/jcr/repoinit/impl/AclUtilTest.java
index ff04c18..2d72021 100644
--- a/src/test/java/org/apache/sling/jcr/repoinit/impl/AclUtilTest.java
+++ b/src/test/java/org/apache/sling/jcr/repoinit/impl/AclUtilTest.java
@@ -243,7 +243,7 @@ public class AclUtilTest {
             assertNull(((JackrabbitSession) session).getUserManager().getAuthorizable(EveryonePrincipal.getInstance()));
 
             AclUtil.setAcl(
-                    session,
+                    toPCSessionWrapper(session),
                     Collections.singletonList(EveryonePrincipal.NAME),
                     Collections.singletonList(PathUtils.ROOT_PATH),
                     Collections.singletonList(Privilege.JCR_READ),
@@ -273,7 +273,7 @@ public class AclUtilTest {
             U.adminSession.save();
 
             AclUtil.setAcl(
-                    U.adminSession,
+                    toPCSessionWrapper(U.adminSession),
                     Collections.singletonList(principal.getName()),
                     Collections.singletonList(PathUtils.ROOT_PATH),
                     Collections.singletonList(Privilege.JCR_READ),
@@ -307,7 +307,7 @@ public class AclUtilTest {
             U.adminSession.save();
 
             AclUtil.setAcl(
-                    U.adminSession,
+                    toPCSessionWrapper(U.adminSession),
                     Collections.singletonList(U.username),
                     Collections.singletonList(PathUtils.ROOT_PATH),
                     Collections.singletonList(Privilege.JCR_READ),
@@ -342,7 +342,7 @@ public class AclUtilTest {
 
         List<String> paths = Collections.singletonList(":home:" + U.username + "#");
         AclUtil.setAcl(
-                U.adminSession,
+                toPCSessionWrapper(U.adminSession),
                 Collections.singletonList(U.username),
                 paths,
                 Collections.singletonList(Privilege.JCR_READ),
@@ -374,7 +374,7 @@ public class AclUtilTest {
 
         List<String> paths = Collections.singletonList(":home:" + U.username + "," + gr.getID() + "#");
         AclUtil.setAcl(
-                U.adminSession,
+                toPCSessionWrapper(U.adminSession),
                 Collections.singletonList(U.username),
                 paths,
                 Collections.singletonList(Privilege.JCR_READ),
@@ -399,7 +399,7 @@ public class AclUtilTest {
 
         List<String> paths = Arrays.asList(":home:" + U.username + "#", ":repository", PathUtils.ROOT_PATH);
         AclUtil.setAcl(
-                U.adminSession,
+                toPCSessionWrapper(U.adminSession),
                 Collections.singletonList(U.username),
                 paths,
                 Collections.singletonList(Privilege.JCR_ALL),
@@ -450,7 +450,7 @@ public class AclUtilTest {
 
         List<String> paths = Collections.singletonList(":home:" + U.username + "," + gr.getID() + "#/profiles/private");
         AclUtil.setAcl(
-                U.adminSession,
+                toPCSessionWrapper(U.adminSession),
                 Collections.singletonList(U.username),
                 paths,
                 Collections.singletonList(Privilege.JCR_READ),
@@ -504,7 +504,7 @@ public class AclUtilTest {
 
         List<String> paths = Collections.singletonList(":home:" + gr.getID() + "," + U.username + "#");
         AclUtil.setAcl(
-                U.adminSession,
+                toPCSessionWrapper(U.adminSession),
                 Collections.singletonList(U.username),
                 paths,
                 Collections.singletonList(Privilege.JCR_READ),
@@ -537,7 +537,7 @@ public class AclUtilTest {
     public void testSetAclWithHomePathMissingTrailingHash() throws Exception {
         List<String> paths = Collections.singletonList(":home:" + U.username);
         AclUtil.setAcl(
-                U.adminSession,
+                toPCSessionWrapper(U.adminSession),
                 Collections.singletonList(U.username),
                 paths,
                 Collections.singletonList(Privilege.JCR_READ),
@@ -548,7 +548,7 @@ public class AclUtilTest {
     public void testSetAclWithHomePathUnknownUser() throws Exception {
         List<String> paths = Collections.singletonList(":home:alice#");
         AclUtil.setAcl(
-                U.adminSession,
+                toPCSessionWrapper(U.adminSession),
                 Collections.singletonList(U.username),
                 paths,
                 Collections.singletonList(Privilege.JCR_READ),
@@ -559,7 +559,7 @@ public class AclUtilTest {
     public void repeatedSetAclCallIsNoOp() throws Throwable {
         final Session session = U.adminSession;
         final ThrowingRunnable setAcls = () -> AclUtil.setAcl(
-                session,
+                toPCSessionWrapper(session),
                 Collections.singletonList(U.username),
                 Arrays.asList(":home:" + U.username + "#", ":repository", PathUtils.ROOT_PATH),
                 Collections.singletonList(Privilege.JCR_ALL),
@@ -578,7 +578,7 @@ public class AclUtilTest {
     public void nullRestrictionClauseAndNullOptionsAreHandled() {
         final Session session = U.adminSession;
         Assertions.assertDoesNotThrow(() -> AclUtil.setAcl(
-                session,
+                toPCSessionWrapper(session),
                 Collections.singletonList(U.username),
                 Collections.singletonList(PathUtils.ROOT_PATH),
                 Collections.singletonList(Privilege.JCR_READ),
@@ -626,4 +626,8 @@ public class AclUtilTest {
     private Privilege[] privileges(String... privilegeNames) throws RepositoryException {
         return AccessControlUtils.privilegesFromNames(U.adminSession, privilegeNames);
     }
+
+    private static PrivilegeCachingSessionWrapper toPCSessionWrapper (Session session) {
+        return new PrivilegeCachingSessionWrapper(session);
+    }
 }
diff --git a/src/test/java/org/apache/sling/jcr/repoinit/impl/ManyServiceUsersTest.java b/src/test/java/org/apache/sling/jcr/repoinit/impl/ManyServiceUsersTest.java
index 552689c..1a93836 100644
--- a/src/test/java/org/apache/sling/jcr/repoinit/impl/ManyServiceUsersTest.java
+++ b/src/test/java/org/apache/sling/jcr/repoinit/impl/ManyServiceUsersTest.java
@@ -85,7 +85,7 @@ public class ManyServiceUsersTest {
 
                 try {
                     AclUtil.setAcl(
-                            otherSession,
+                            new PrivilegeCachingSessionWrapper(otherSession),
                             Arrays.asList(username),
                             Arrays.asList(path),
                             Arrays.asList("jcr:read"),
diff --git a/src/test/java/org/apache/sling/jcr/repoinit/impl/TestUtil.java b/src/test/java/org/apache/sling/jcr/repoinit/impl/TestUtil.java
index f5b0904..b057fae 100644
--- a/src/test/java/org/apache/sling/jcr/repoinit/impl/TestUtil.java
+++ b/src/test/java/org/apache/sling/jcr/repoinit/impl/TestUtil.java
@@ -76,13 +76,13 @@ public class TestUtil {
         username = "user_" + id;
     }
 
-    public List<Operation> parse(String... inputLines) throws RepoInitParsingException {
+    public static List<Operation> parse(String... inputLines) throws RepoInitParsingException {
         try (final StringReader r = new StringReader(String.join("\n", inputLines))) {
             return new RepoInitParserService().parse(r);
         }
     }
 
-    private void assertPathContains(Authorizable u, String pathShouldContain) throws RepositoryException {
+    private static void assertPathContains(Authorizable u, String pathShouldContain) throws RepositoryException {
         if (pathShouldContain != null) {
             final String path = u.getPath();
             assertTrue("Expecting path " + path + " to contain " + pathShouldContain, path.contains(pathShouldContain));


(sling-org-apache-sling-jcr-repoinit) 03/06: also cache the expansion of aggregated privileges

Posted by jo...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

joerghoh pushed a commit to branch SLING-12642-cache-privileges
in repository https://gitbox.apache.org/repos/asf/sling-org-apache-sling-jcr-repoinit.git

commit c46758487a77ad3a7e27a16fa823736f20dc56c3
Author: Joerg Hoh <jo...@apache.org>
AuthorDate: Sun Feb 9 14:38:03 2025 +0100

    also cache the expansion of aggregated privileges
---
 .../apache/sling/jcr/repoinit/impl/AclUtil.java    | 36 ++++++++++++----------
 .../impl/PrivilegeCachingSessionWrapper.java       | 21 ++++++++++++-
 .../sling/jcr/repoinit/impl/AclUtilTest.java       |  2 +-
 3 files changed, 40 insertions(+), 19 deletions(-)

diff --git a/src/main/java/org/apache/sling/jcr/repoinit/impl/AclUtil.java b/src/main/java/org/apache/sling/jcr/repoinit/impl/AclUtil.java
index a459c45..9d8d0af 100644
--- a/src/main/java/org/apache/sling/jcr/repoinit/impl/AclUtil.java
+++ b/src/main/java/org/apache/sling/jcr/repoinit/impl/AclUtil.java
@@ -184,7 +184,7 @@ public class AclUtil {
         for (String name : principals) {
             final Principal principal = getPrincipal(pcsw.getSession(), name, ignoreMissingPrincipal);
             LocalAccessControlEntry newAce =
-                    new LocalAccessControlEntry(principal, jcrPriv, isAllow, localRestrictions);
+                    new LocalAccessControlEntry(pcsw, principal, jcrPriv, isAllow, localRestrictions);
             if (contains(existingAces, newAce)) {
                 LOG.info(
                         "Not adding {} to path {} since an equivalent access control entry already exists",
@@ -340,7 +340,7 @@ public class AclUtil {
                             continue;
                         }
                         LocalAccessControlEntry entry =
-                                new LocalAccessControlEntry(ace.getPrincipal(), privs, isAllow, restr);
+                                new LocalAccessControlEntry(pcsw, ace.getPrincipal(), privs, isAllow, restr);
                         if (entry.isEqual(ace)) {
                             acl.removeAccessControlEntry(ace);
                             modified = true;
@@ -413,7 +413,7 @@ public class AclUtil {
                         // or if there exists no node at the effective path (unable to evaluate path-based entries).
                         LOG.info("No PrincipalAccessControlList available for principal {}", principal);
                         if (!containsEquivalentEntry(
-                                pcsw.getSession(), effectivePath, principal, privileges, true, line.getRestrictions())) {
+                                pcsw, effectivePath, principal, privileges, true, line.getRestrictions())) {
                             LOG.warn(
                                     "No equivalent path-based entry exists for principal {} and effective path {} ",
                                     principal.getName(),
@@ -470,7 +470,7 @@ public class AclUtil {
                 if (!jcrPaths.contains(entry.getEffectivePath())) {
                     return false;
                 }
-                LocalAccessControlEntry lace = new LocalAccessControlEntry(
+                LocalAccessControlEntry lace = new LocalAccessControlEntry(pcsw,
                         entry.getPrincipal(), privs, line.getAction() == AclLine.Action.ALLOW, restr);
                 return lace.isEqual(entry);
             };
@@ -622,25 +622,25 @@ public class AclUtil {
     }
 
     private static boolean containsEquivalentEntry(
-            Session session,
+            PrivilegeCachingSessionWrapper pcsw,
             String absPath,
             Principal principal,
             Privilege[] privileges,
             boolean isAllow,
             List<RestrictionClause> restrictionList)
             throws RepositoryException {
-        if (absPath != null && !session.nodeExists(absPath)) {
+        if (absPath != null && !pcsw.getSession().nodeExists(absPath)) {
             LOG.info(
                     "Cannot determine existence of equivalent path-based entry for principal {}. No node at path {} ",
                     principal.getName(),
                     absPath);
             return true;
         }
-        for (AccessControlPolicy policy : session.getAccessControlManager().getPolicies(absPath)) {
+        for (AccessControlPolicy policy : pcsw.getAccessControlManager().getPolicies(absPath)) {
             if (policy instanceof JackrabbitAccessControlList) {
                 LocalRestrictions lr =
-                        createLocalRestrictions(restrictionList, ((JackrabbitAccessControlList) policy), session);
-                LocalAccessControlEntry newEntry = new LocalAccessControlEntry(principal, privileges, isAllow, lr);
+                        createLocalRestrictions(restrictionList, ((JackrabbitAccessControlList) policy), pcsw.getSession());
+                LocalAccessControlEntry newEntry = new LocalAccessControlEntry(pcsw,principal, privileges, isAllow, lr);
                 if (contains(((JackrabbitAccessControlList) policy).getAccessControlEntries(), newEntry)) {
                     LOG.info(
                             "Equivalent path-based entry exists for principal {} and effective path {} ",
@@ -711,13 +711,19 @@ public class AclUtil {
         private final Privilege[] privileges;
         private final boolean isAllow;
         private final LocalRestrictions restrictions;
+        private final PrivilegeCachingSessionWrapper pcsw;
 
-        LocalAccessControlEntry(Principal principal, Privilege[] privileges, boolean isAllow) {
-            this(principal, privileges, isAllow, null);
+        LocalAccessControlEntry(PrivilegeCachingSessionWrapper pcsw, Principal principal, Privilege[] privileges, boolean isAllow) {
+            this(pcsw,principal, privileges, isAllow, null);
         }
 
         LocalAccessControlEntry(
-                Principal principal, Privilege[] privileges, boolean isAllow, LocalRestrictions restrictions) {
+                PrivilegeCachingSessionWrapper pcsw,
+                Principal principal,
+                Privilege[] privileges,
+                boolean isAllow,
+                LocalRestrictions restrictions) {
+            this.pcsw = pcsw;
             this.principal = principal;
             this.privileges = privileges;
             this.isAllow = isAllow;
@@ -751,11 +757,7 @@ public class AclUtil {
 
             if (privileges != null) {
                 for (Privilege privilege : privileges) {
-                    if (privilege.isAggregate()) {
-                        expandedSet.addAll(Arrays.asList(privilege.getAggregatePrivileges()));
-                    } else {
-                        expandedSet.add(privilege);
-                    }
+                    expandedSet.addAll(pcsw.expandPrivilege(privilege));
                 }
             }
 
diff --git a/src/main/java/org/apache/sling/jcr/repoinit/impl/PrivilegeCachingSessionWrapper.java b/src/main/java/org/apache/sling/jcr/repoinit/impl/PrivilegeCachingSessionWrapper.java
index 9c642a6..e38b10f 100644
--- a/src/main/java/org/apache/sling/jcr/repoinit/impl/PrivilegeCachingSessionWrapper.java
+++ b/src/main/java/org/apache/sling/jcr/repoinit/impl/PrivilegeCachingSessionWrapper.java
@@ -18,19 +18,22 @@
  */
 package org.apache.sling.jcr.repoinit.impl;
 
+import java.util.Arrays;
 import java.util.HashMap;
 import java.util.HashSet;
+import java.util.List;
 import java.util.Map;
 import java.util.Set;
 
 import javax.jcr.RepositoryException;
 import javax.jcr.Session;
-import javax.jcr.security.AccessControlManager;
 import javax.jcr.security.Privilege;
 
 import org.apache.jackrabbit.api.JackrabbitSession;
 import org.apache.jackrabbit.api.security.JackrabbitAccessControlManager;
 
+import com.google.common.collect.Lists;
+
 /**
  * A simple wrapper around a session, which can cache the privilege resolution
  */
@@ -39,6 +42,7 @@ public class PrivilegeCachingSessionWrapper {
     JackrabbitSession session;
     JackrabbitAccessControlManager acMgr;
     Map<String,Privilege> nameToPrivilegeMap = new HashMap<>();
+    Map<Privilege,List<Privilege>> privilegeToAggreate = new HashMap<>();
     
     public PrivilegeCachingSessionWrapper (Session session) {
         AclUtil.checkState(session instanceof JackrabbitSession,"A Jackrabbit Session is required");
@@ -80,4 +84,19 @@ public class PrivilegeCachingSessionWrapper {
         }
         return privileges.toArray(new Privilege[privileges.size()]);
     }
+
+    /**
+     * If a privilege is an aggreated, return the privilges it contains, otherwise return the privilege itself
+     * @param priv the privilege 
+     * @return
+     */
+    public List<Privilege> expandPrivilege (Privilege priv) {
+        return privilegeToAggreate.computeIfAbsent(priv, (p) -> {
+            if (p.isAggregate()) {
+                return Arrays.asList(p.getAggregatePrivileges());
+            } else {
+                return Lists.newArrayList(p);
+            }
+        });
+    }
 }
diff --git a/src/test/java/org/apache/sling/jcr/repoinit/impl/AclUtilTest.java b/src/test/java/org/apache/sling/jcr/repoinit/impl/AclUtilTest.java
index 2d72021..8dc7700 100644
--- a/src/test/java/org/apache/sling/jcr/repoinit/impl/AclUtilTest.java
+++ b/src/test/java/org/apache/sling/jcr/repoinit/impl/AclUtilTest.java
@@ -607,7 +607,7 @@ public class AclUtilTest {
             boolean contained)
             throws RepositoryException {
         AclUtil.LocalAccessControlEntry localAce =
-                new AclUtil.LocalAccessControlEntry(principal(username), privileges(privilegeNames), isAllow);
+                new AclUtil.LocalAccessControlEntry(toPCSessionWrapper(U.adminSession),principal(username), privileges(privilegeNames), isAllow);
 
         if (contained) {
             assertTrue(


(sling-org-apache-sling-jcr-repoinit) 04/06: add principal caching

Posted by jo...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

joerghoh pushed a commit to branch SLING-12642-cache-privileges
in repository https://gitbox.apache.org/repos/asf/sling-org-apache-sling-jcr-repoinit.git

commit a6cfe20aeb4658ec676c735e8c74d86a0fb969e4
Author: Joerg Hoh <jo...@apache.org>
AuthorDate: Sun Feb 9 19:55:46 2025 +0100

    add principal caching
---
 .../org/apache/sling/jcr/repoinit/impl/AclUtil.java  | 20 ++++++++++----------
 .../impl/PrivilegeCachingSessionWrapper.java         | 13 +++++++++++++
 2 files changed, 23 insertions(+), 10 deletions(-)

diff --git a/src/main/java/org/apache/sling/jcr/repoinit/impl/AclUtil.java b/src/main/java/org/apache/sling/jcr/repoinit/impl/AclUtil.java
index 9d8d0af..e5f5ab9 100644
--- a/src/main/java/org/apache/sling/jcr/repoinit/impl/AclUtil.java
+++ b/src/main/java/org/apache/sling/jcr/repoinit/impl/AclUtil.java
@@ -182,7 +182,7 @@ public class AclUtil {
                 .map(o -> o.contains(AclVisitor.OPTION_IGNORE_MISSING_PRINCIPAL))
                 .orElse(false);
         for (String name : principals) {
-            final Principal principal = getPrincipal(pcsw.getSession(), name, ignoreMissingPrincipal);
+            final Principal principal = getPrincipal(pcsw, name, ignoreMissingPrincipal);
             LocalAccessControlEntry newAce =
                     new LocalAccessControlEntry(pcsw, principal, jcrPriv, isAllow, localRestrictions);
             if (contains(existingAces, newAce)) {
@@ -206,13 +206,13 @@ public class AclUtil {
     }
 
     @NotNull
-    private static Principal getPrincipal(Session session, String name, boolean ignoreMissingPrincipal)
+    private static Principal getPrincipal(PrivilegeCachingSessionWrapper pcsw, String name, boolean ignoreMissingPrincipal)
             throws RepositoryException {
-        Principal principal = AccessControlUtils.getPrincipal(session, name);
+        Principal principal = pcsw.getPrincipal(name);
         if (principal == null) {
             // backwards compatibility: fallback to original code treating principal name as authorizable ID (see
             // SLING-8604)
-            final Authorizable authorizable = UserUtil.getAuthorizable(session, name);
+            final Authorizable authorizable = UserUtil.getAuthorizable(pcsw.getSession(), name);
             if (!ignoreMissingPrincipal) {
                 checkState(authorizable != null, "Authorizable not found: {0}", name);
                 principal = authorizable.getPrincipal();
@@ -238,7 +238,7 @@ public class AclUtil {
      */
     public static void removePolicy(@NotNull PrivilegeCachingSessionWrapper pcsw, @NotNull final String principalName)
             throws RepositoryException {
-        Principal principal = AccessControlUtils.getPrincipal(pcsw.getSession(), principalName);
+        Principal principal = pcsw.getPrincipal(principalName);
         if (principal == null) {
             LOG.info("Principal {} does not exist.", principalName);
             // using PrincipalImpl will prevent 'removePolicy' from failing with AccessControlException
@@ -371,12 +371,12 @@ public class AclUtil {
             PrivilegeCachingSessionWrapper pcsw, String principalName, Collection<AclLine> lines, boolean isStrict)
             throws RepositoryException {
         final JackrabbitAccessControlManager acMgr = pcsw.getAccessControlManager();
-        Principal principal = AccessControlUtils.getPrincipal(pcsw.getSession(), principalName);
+        Principal principal = pcsw.getPrincipal(principalName);
         if (principal == null) {
             // due to transient nature of the repo-init the principal lookup may not succeed if completed through query
             // -> save transient changes and retry principal lookup
             pcsw.getSession().save();
-            principal = AccessControlUtils.getPrincipal(pcsw.getSession(), principalName);
+            principal = pcsw.getPrincipal(principalName);
             checkState(principal != null, PRINCIPAL_NOT_FOUND_PATTERN, principalName);
         }
 
@@ -450,12 +450,12 @@ public class AclUtil {
     public static void removePrincipalEntries(@NotNull PrivilegeCachingSessionWrapper pcsw, String principalName, Collection<AclLine> lines)
             throws RepositoryException {
         final JackrabbitAccessControlManager acMgr = pcsw.getAccessControlManager();
-        Principal principal = AccessControlUtils.getPrincipal(pcsw.getSession(), principalName);
+        Principal principal = pcsw.getPrincipal(principalName);
         if (principal == null) {
             // due to transient nature of the repo-init the principal lookup may not succeed if completed through query
             // -> save transient changes and retry principal lookup
             pcsw.getSession().save();
-            principal = AccessControlUtils.getPrincipal(pcsw.getSession(), principalName);
+            principal = pcsw.getPrincipal(principalName);
             checkState(principal != null, PRINCIPAL_NOT_FOUND_PATTERN, principalName);
         }
 
@@ -500,7 +500,7 @@ public class AclUtil {
      */
     public static void removePrincipalPolicy(@NotNull PrivilegeCachingSessionWrapper pcsw, @NotNull String principalName)
             throws RepositoryException {
-        Principal principal = AccessControlUtils.getPrincipal(pcsw.getSession(), principalName);
+        Principal principal = pcsw.getPrincipal(principalName);
         if (principal == null) {
             LOG.info("Cannot remove principal-based ACL. Principal {} does not exist.", principalName);
             return;
diff --git a/src/main/java/org/apache/sling/jcr/repoinit/impl/PrivilegeCachingSessionWrapper.java b/src/main/java/org/apache/sling/jcr/repoinit/impl/PrivilegeCachingSessionWrapper.java
index e38b10f..3708d90 100644
--- a/src/main/java/org/apache/sling/jcr/repoinit/impl/PrivilegeCachingSessionWrapper.java
+++ b/src/main/java/org/apache/sling/jcr/repoinit/impl/PrivilegeCachingSessionWrapper.java
@@ -18,6 +18,7 @@
  */
 package org.apache.sling.jcr.repoinit.impl;
 
+import java.security.Principal;
 import java.util.Arrays;
 import java.util.HashMap;
 import java.util.HashSet;
@@ -31,6 +32,7 @@ import javax.jcr.security.Privilege;
 
 import org.apache.jackrabbit.api.JackrabbitSession;
 import org.apache.jackrabbit.api.security.JackrabbitAccessControlManager;
+import org.apache.jackrabbit.commons.jackrabbit.authorization.AccessControlUtils;
 
 import com.google.common.collect.Lists;
 
@@ -43,6 +45,7 @@ public class PrivilegeCachingSessionWrapper {
     JackrabbitAccessControlManager acMgr;
     Map<String,Privilege> nameToPrivilegeMap = new HashMap<>();
     Map<Privilege,List<Privilege>> privilegeToAggreate = new HashMap<>();
+    Map<String,Principal> idToPrincipal = new HashMap<>();
     
     public PrivilegeCachingSessionWrapper (Session session) {
         AclUtil.checkState(session instanceof JackrabbitSession,"A Jackrabbit Session is required");
@@ -99,4 +102,14 @@ public class PrivilegeCachingSessionWrapper {
             }
         });
     }
+
+    public Principal getPrincipal (String principalName) throws RepositoryException {
+        if (idToPrincipal.containsKey(principalName)) {
+            return idToPrincipal.get(principalName);
+        } else {
+            Principal p = AccessControlUtils.getPrincipal(this.getSession(), principalName);
+            idToPrincipal.put(principalName, p);
+            return p;
+        }
+    }
 }