You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@sling.apache.org by pa...@apache.org on 2021/05/20 10:26:35 UTC

[sling-org-apache-sling-jcr-repoinit] branch 1.1.34_SLING-10406 created (now e56c62c)

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

pauls pushed a change to branch 1.1.34_SLING-10406
in repository https://gitbox.apache.org/repos/asf/sling-org-apache-sling-jcr-repoinit.git.


      at e56c62c  SLING-10406 : AclVisitor.visitCreatePath: retry if no default primary type can be found

This branch includes the following new commits:

     new e56c62c  SLING-10406 : AclVisitor.visitCreatePath: retry if no default primary type can be found

The 1 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] 01/01: SLING-10406 : AclVisitor.visitCreatePath: retry if no default primary type can be found

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

pauls pushed a commit to branch 1.1.34_SLING-10406
in repository https://gitbox.apache.org/repos/asf/sling-org-apache-sling-jcr-repoinit.git

commit e56c62cb0c5acb92bef1f53b5c3d0d75cf576975
Author: angela <an...@adobe.com>
AuthorDate: Thu May 20 11:41:50 2021 +0200

    SLING-10406 : AclVisitor.visitCreatePath: retry if no default primary type can be found
---
 .../apache/sling/jcr/repoinit/impl/AclVisitor.java | 84 ++++++++++++++--------
 .../apache/sling/jcr/repoinit/CreatePathsTest.java | 15 ++++
 2 files changed, 70 insertions(+), 29 deletions(-)

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 965bd80..398214e 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
@@ -24,7 +24,9 @@ import java.util.Collections;
 import java.util.List;
 
 import javax.jcr.Node;
+import javax.jcr.RepositoryException;
 import javax.jcr.Session;
+import javax.jcr.nodetype.ConstraintViolationException;
 
 import org.apache.sling.repoinit.parser.operations.AclLine;
 import org.apache.sling.repoinit.parser.operations.CreatePath;
@@ -33,16 +35,24 @@ import org.apache.sling.repoinit.parser.operations.RestrictionClause;
 import org.apache.sling.repoinit.parser.operations.SetAclPaths;
 import org.apache.sling.repoinit.parser.operations.SetAclPrincipalBased;
 import org.apache.sling.repoinit.parser.operations.SetAclPrincipals;
+import org.jetbrains.annotations.NotNull;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
-/** OperationVisitor which processes only operations related to ACLs.
+/**
+ * OperationVisitor which processes only operations related to ACLs.
  * Having several such specialized visitors
  * makes it easy to control the execution order.
  */
 class AclVisitor extends DoNothingVisitor {
 
-    /** Create a visitor using the supplied JCR Session.
+    private static final Logger log = LoggerFactory.getLogger(AclVisitor.class);
+    
+    /**
+     * Create a visitor using the supplied JCR Session.
+     *
      * @param s must have sufficient rights to create users
-     *      and set ACLs.
+     *          and set ACLs.
      */
     public AclVisitor(Session s) {
         super(s);
@@ -50,7 +60,7 @@ class AclVisitor extends DoNothingVisitor {
 
     private List<String> require(AclLine line, String propertyName) {
         final List<String> result = line.getProperty(propertyName);
-        if(result == null) {
+        if (result == null) {
             throw new IllegalStateException("Missing property " + propertyName + " on " + line);
         }
         return result;
@@ -68,7 +78,7 @@ class AclVisitor extends DoNothingVisitor {
                 List<RestrictionClause> restrictions = line.getRestrictions();
                 AclUtil.setAcl(s, principals, paths, privileges, isAllow, restrictions);
             }
-        } catch(Exception e) {
+        } catch (Exception e) {
             throw new RuntimeException("Failed to set ACL (" + e.toString() + ") " + line, e);
         }
     }
@@ -85,7 +95,7 @@ class AclVisitor extends DoNothingVisitor {
                 List<RestrictionClause> restrictions = line.getRestrictions();
                 AclUtil.setRepositoryAcl(s, principals, privileges, isAllow, restrictions);
             }
-        } catch(Exception e) {
+        } catch (Exception e) {
             throw new RuntimeException("Failed to set repository level ACL (" + e.toString() + ") " + line, e);
         }
     }
@@ -95,13 +105,13 @@ class AclVisitor extends DoNothingVisitor {
         final List<String> principals = s.getPrincipals();
         for (AclLine line : s.getLines()) {
             final List<String> paths = line.getProperty(PROP_PATHS);
-            if (paths != null && ! paths.isEmpty()) {
+            if (paths != null && !paths.isEmpty()) {
                 setAcl(line, session, principals, paths, require(line, PROP_PRIVILEGES), line.getAction());
             } else {
                 setRepositoryAcl(line, session, principals, require(line, PROP_PRIVILEGES), line.getAction());
             }
         }
-     }
+    }
 
     @Override
     public void visitSetAclPaths(SetAclPaths s) {
@@ -117,7 +127,7 @@ class AclVisitor extends DoNothingVisitor {
             try {
                 log.info("Adding principal-based access control entry for {}", principalName);
                 AclUtil.setPrincipalAcl(session, principalName, s.getLines());
-            } catch(Exception e) {
+            } catch (Exception e) {
                 throw new RuntimeException("Failed to set principal-based ACL (" + e.getMessage() + ")", e);
             }
         }
@@ -126,32 +136,48 @@ class AclVisitor extends DoNothingVisitor {
     @Override
     public void visitCreatePath(CreatePath cp) {
         String parentPath = "";
-            for(PathSegmentDefinition psd : cp.getDefinitions()) {
-                final String fullPath = parentPath + "/" + psd.getSegment();
-                try {
-                    if(session.itemExists(fullPath)) {
-                        log.info("Path already exists, nothing to do (and not checking its primary type for now): {}", fullPath);
-                    } else {
-                        final Node parent = parentPath.equals("") ? session.getRootNode() : session.getNode(parentPath);
-                        log.info("Creating node {} with primary type {}", fullPath, psd.getPrimaryType());
-                        Node node = parent.addNode(psd.getSegment(), psd.getPrimaryType());
-                        List<String> mixins = psd.getMixins();
-                        if (mixins != null) {
-                            log.info("Adding mixins {} to node {}", mixins, fullPath);
-                            for (String mixin : mixins) {
-                                node.addMixin(mixin);
-                            }
+        for (PathSegmentDefinition psd : cp.getDefinitions()) {
+            final String fullPath = parentPath + "/" + psd.getSegment();
+            try {
+                if (session.itemExists(fullPath)) {
+                    log.info("Path already exists, nothing to do (and not checking its primary type for now): {}", fullPath);
+                } else {
+                    final Node parent = parentPath.equals("") ? session.getRootNode() : session.getNode(parentPath);
+                    log.info("Creating node {} with primary type {}", fullPath, psd.getPrimaryType());
+                    Node node = addChildNode(parent, psd);
+                    List<String> mixins = psd.getMixins();
+                    if (mixins != null) {
+                        log.info("Adding mixins {} to node {}", mixins, fullPath);
+                        for (String mixin : mixins) {
+                            node.addMixin(mixin);
                         }
                     }
-                } catch(Exception e) {
-                    throw new RuntimeException("CreatePath execution failed at " + psd + ": " + e, e);
                 }
-                parentPath += "/" + psd.getSegment();
+            } catch (Exception e) {
+                throw new RuntimeException("CreatePath execution failed at " + psd + ": " + e, e);
             }
+            parentPath += "/" + psd.getSegment();
+        }
         try {
             session.save();
-        } catch(Exception e) {
-            throw new RuntimeException("Session.save failed: "+ e, e);
+        } catch (Exception e) {
+            throw new RuntimeException("Session.save failed: " + e, e);
+        }
+    }
+    
+    @NotNull
+    private static Node addChildNode(@NotNull Node parent, @NotNull PathSegmentDefinition psd) throws RepositoryException {
+        String primaryType = psd.getPrimaryType();
+        if (primaryType == null) {
+            try {
+                return parent.addNode(psd.getSegment());
+            } catch (ConstraintViolationException e) {
+                // assume that no default primary type could be detected -> retry with a default
+                log.info("Adding Node without node type failed ('{}'), retry with sling:Folder", e.getMessage());
+                return parent.addNode(psd.getSegment(), "sling:Folder");
+            }
+        } else {
+            return parent.addNode(psd.getSegment(), psd.getPrimaryType());
         }
     }
 }
diff --git a/src/test/java/org/apache/sling/jcr/repoinit/CreatePathsTest.java b/src/test/java/org/apache/sling/jcr/repoinit/CreatePathsTest.java
index 38b33e6..cb356d9 100644
--- a/src/test/java/org/apache/sling/jcr/repoinit/CreatePathsTest.java
+++ b/src/test/java/org/apache/sling/jcr/repoinit/CreatePathsTest.java
@@ -20,6 +20,7 @@ import java.io.IOException;
 import java.util.Arrays;
 import java.util.Collections;
 
+import javax.jcr.Node;
 import javax.jcr.RepositoryException;
 
 import org.apache.sling.commons.testing.jcr.RepositoryUtil;
@@ -30,6 +31,8 @@ import org.junit.Before;
 import org.junit.Rule;
 import org.junit.Test;
 
+import static org.junit.Assert.assertEquals;
+
 /** Test the creation of paths with specific node types */
 public class CreatePathsTest {
     
@@ -124,4 +127,16 @@ public class CreatePathsTest {
         U.assertNodeExists("/thirteen/fourteen", "nt:unstructured", Collections.singletonList("mix:lockable"));
         U.assertNodeExists("/thirteen/fourteen/fifteen", "nt:unstructured", Collections.singletonList("mix:lockable"));
     }
+    
+    @Test
+    public void createPathNoDefaultPrimaryType() throws Exception {
+        U.adminSession.getRootNode().addNode("folder", "nt:folder");
+        U.parseAndExecute("create path /folder/subfolder/subfolder2");
+        
+        Node subFolder = U.adminSession.getNode("/folder/subfolder");
+        assertEquals("sling:Folder", subFolder.getPrimaryNodeType().getName());
+
+        Node subFolder2 = subFolder.getNode("subfolder2");
+        assertEquals("sling:Folder", subFolder2.getPrimaryNodeType().getName());
+    }
 }