This is an automated email from the ASF dual-hosted git repository. jsedding pushed a commit to branch fix/SLING-12107-repoinit-execution-order in repository https://gitbox.apache.org/repos/asf/sling-org-apache-sling-jcr-repoinit.git
commit 6c1fedb221e751d8211e63dd59aa0dcee5d09975 Author: Julian Sedding <[email protected]> AuthorDate: Thu Oct 19 17:34:04 2023 +0200 SLING-12107 - JCR Repoinit executes operations out of order --- .../repoinit/impl/JcrRepoInitOpsProcessorImpl.java | 50 ++++++++-------- .../sling/jcr/repoinit/ExecutionOrderTest.java | 67 +++++++++++++++------- .../apache/sling/jcr/repoinit/impl/TestUtil.java | 34 ++++++++--- 3 files changed, 101 insertions(+), 50 deletions(-) diff --git a/src/main/java/org/apache/sling/jcr/repoinit/impl/JcrRepoInitOpsProcessorImpl.java b/src/main/java/org/apache/sling/jcr/repoinit/impl/JcrRepoInitOpsProcessorImpl.java index ddd8b8b..f0c6981 100644 --- a/src/main/java/org/apache/sling/jcr/repoinit/impl/JcrRepoInitOpsProcessorImpl.java +++ b/src/main/java/org/apache/sling/jcr/repoinit/impl/JcrRepoInitOpsProcessorImpl.java @@ -17,45 +17,49 @@ package org.apache.sling.jcr.repoinit.impl; import java.util.List; +import java.util.stream.Stream; import javax.jcr.Session; import org.apache.sling.jcr.repoinit.JcrRepoInitOpsProcessor; import org.apache.sling.repoinit.parser.operations.Operation; -import org.apache.sling.repoinit.parser.operations.OperationVisitor; import org.osgi.framework.Constants; import org.osgi.service.component.annotations.Component; +import static java.util.Arrays.asList; +import static java.util.Collections.singleton; + /** * Apply Operations produced by the repoinit parser to a JCR Repository */ @Component(service = JcrRepoInitOpsProcessor.class, - property = { - Constants.SERVICE_VENDOR + "=The Apache Software Foundation" - }) + property = { + Constants.SERVICE_VENDOR + "=The Apache Software Foundation" + }) public class JcrRepoInitOpsProcessorImpl implements JcrRepoInitOpsProcessor { - /** Apply the supplied operations: first the namespaces and nodetypes - * registrations, then the service users, paths and ACLs. + /** + * Apply the supplied operations: first the namespaces and nodetypes + * registrations, then the service users, paths and ACLs. */ @Override public void apply(Session session, List<Operation> ops) { - - final OperationVisitor [] visitors = { - new NamespacesVisitor(session), - new NodetypesVisitor(session), - new PrivilegeVisitor(session), - new UserVisitor(session), - new NodeVisitor(session), - new AclVisitor(session), - new GroupMembershipVisitor(session), - new NodePropertiesVisitor(session) - }; - - for(OperationVisitor v : visitors) { - for(Operation op : ops) { - op.accept(v); - } - } + Stream.of( + // register namespaces first + singleton(new NamespacesVisitor(session)), + // then create node types and privileges, both use namespaces + asList( + new NodetypesVisitor(session), + new PrivilegeVisitor(session)), + // finally apply everything else + asList( + new UserVisitor(session), + new NodeVisitor(session), + new AclVisitor(session), + new GroupMembershipVisitor(session), + new NodePropertiesVisitor(session)) + ).forEach(visitorGroup -> { + ops.forEach(op -> visitorGroup.forEach(op::accept)); + }); } } diff --git a/src/test/java/org/apache/sling/jcr/repoinit/ExecutionOrderTest.java b/src/test/java/org/apache/sling/jcr/repoinit/ExecutionOrderTest.java index 92fa512..b27295f 100644 --- a/src/test/java/org/apache/sling/jcr/repoinit/ExecutionOrderTest.java +++ b/src/test/java/org/apache/sling/jcr/repoinit/ExecutionOrderTest.java @@ -16,22 +16,29 @@ */ package org.apache.sling.jcr.repoinit; -import static org.junit.Assert.assertEquals; -import java.util.UUID; - -import javax.jcr.Node; -import javax.jcr.PathNotFoundException; -import javax.jcr.RepositoryException; - +import org.apache.jackrabbit.api.security.user.Group; +import org.apache.jackrabbit.api.security.user.UserManager; import org.apache.sling.jcr.repoinit.impl.TestUtil; +import org.apache.sling.jcr.repoinit.impl.UserUtil; import org.apache.sling.repoinit.parser.RepoInitParsingException; +import org.apache.sling.testing.mock.osgi.junit.OsgiContext; import org.apache.sling.testing.mock.sling.ResourceResolverType; import org.apache.sling.testing.mock.sling.junit.SlingContext; +import org.apache.sling.testing.mock.sling.junit.SlingContextBuilder; import org.junit.Before; import org.junit.Rule; import org.junit.Test; -/** Verify that namespaces and nodetypes are executed before path creation */ +import javax.jcr.Node; +import javax.jcr.RepositoryException; +import javax.jcr.Value; +import java.util.UUID; + +import static org.junit.Assert.assertEquals; + +/** Verify that namespaces and nodetypes are executed before path creation. But otherwise, + * the execution order should match the order of the statements. + * */ public class ExecutionOrderTest { @Rule @@ -42,24 +49,44 @@ public class ExecutionOrderTest { private static final String TEST_ID = UUID.randomUUID().toString(); private static final String NS_PREFIX = ExecutionOrderTest.class.getSimpleName(); private static final String NS_URI = "uri:" + NS_PREFIX + ":" + TEST_ID; - private static final String REL_PATH = ExecutionOrderTest.class.getSimpleName() + "-" + TEST_ID; - + private static final String PATH = "/" + NS_PREFIX + "-" + TEST_ID; + private static final String GROUP_NAME = "testGroup"; + @Before public void setup() throws RepositoryException, RepoInitParsingException { U = new TestUtil(context); - - final String stmt = - "create path (" + NS_PREFIX + ":foo) /" + REL_PATH + "\n" - + U.getTestCndStatement(NS_PREFIX, NS_URI) + "\n" - + "register namespace (" + NS_PREFIX + ") " + NS_URI + "\n"; - ; - - U.parseAndExecute(stmt); } @Test - public void pathCreated() throws PathNotFoundException, RepositoryException { - final Node n = U.getAdminSession().getNode("/" + REL_PATH); + public void pathCreated() throws RepositoryException, RepoInitParsingException { + U.parseAndExecute( + "create path (" + NS_PREFIX + ":foo) " + PATH, + "register nodetypes", + "<<===", + "[" + NS_PREFIX + ":foo] > nt:unstructured", + "===>>" + ,"register namespace (" + NS_PREFIX + ") " + NS_URI + ); + final Node n = U.getAdminSession().getNode(PATH); assertEquals(NS_PREFIX + ":foo", n.getProperty("jcr:primaryType").getString()); } + + @Test + public void createGroupWithACLsThenDeleteGroup() throws RepositoryException, RepoInitParsingException { + U.parseAndExecute( + "create path (nt:folder) " + PATH, + "create group " + GROUP_NAME, + "set ACL for " + GROUP_NAME, + " allow jcr:read on " + PATH, + " deny jcr:write on " + PATH, + "end", + "delete group " + GROUP_NAME + ); + + U.assertGroup("Group " + GROUP_NAME + " should have been deleted", GROUP_NAME, false); + + // ACLs should still be present + U.assertPrivileges(GROUP_NAME, PATH, true, "jcr:read"); + U.assertPrivileges(GROUP_NAME, PATH, false, "jcr:write"); + } } 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 406686c..7623f94 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 @@ -25,6 +25,8 @@ import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; import java.io.StringReader; +import java.util.ArrayList; +import java.util.Collections; import java.util.List; import java.util.UUID; @@ -35,7 +37,9 @@ import javax.jcr.Session; import javax.jcr.SimpleCredentials; import javax.jcr.Value; import javax.jcr.nodetype.NodeType; +import javax.jcr.security.Privilege; +import org.apache.jackrabbit.api.security.JackrabbitAccessControlManager; import org.apache.jackrabbit.api.security.user.Authorizable; import org.apache.jackrabbit.api.security.user.Group; import org.apache.jackrabbit.api.security.user.User; @@ -66,12 +70,10 @@ public class TestUtil { username = "user_" + id; } - public List<Operation> parse(String input) throws RepoInitParsingException { - List<Operation> parse = null; - try (final StringReader r = new StringReader(input)) { - parse = new RepoInitParserService().parse(r); + public List<Operation> parse(String... inputLines) throws RepoInitParsingException { + try (final StringReader r = new StringReader(String.join("\n", inputLines))) { + return new RepoInitParserService().parse(r); } - return parse; } private void assertPathContains(Authorizable u, String pathShouldContain) throws RepositoryException { @@ -244,9 +246,9 @@ public class TestUtil { } } - public boolean parseAndExecute(String input) throws RepositoryException, RepoInitParsingException { + public boolean parseAndExecute(String... inputLines) throws RepositoryException, RepoInitParsingException { final JcrRepoInitOpsProcessorImpl p = new JcrRepoInitOpsProcessorImpl(); - p.apply(adminSession, parse(input)); + p.apply(adminSession, parse(inputLines)); boolean hasChanges = adminSession.hasPendingChanges(); adminSession.save(); return hasChanges; @@ -295,4 +297,22 @@ public class TestUtil { adminSession.save(); } } + + public void assertPrivileges(String principalName, String path, boolean allowed, String... privilegeNames) throws RepositoryException { + final JackrabbitAccessControlManager acMgr = (JackrabbitAccessControlManager) adminSession.getAccessControlManager(); + + final ArrayList<Privilege> privileges = new ArrayList<>(); + for (String privilege : privilegeNames) { + privileges.add(acMgr.privilegeFromName(privilege)); + } + + assertEquals( + String.format("Expected %s to have %s %s on %s", + principalName, String.join(", ", privilegeNames), allowed ? "allowed" : "denied", path), + allowed, + acMgr.hasPrivileges(path, + Collections.singleton(() -> principalName), + privileges.toArray(new Privilege[0]))); + + } }
