You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@sling.apache.org by kw...@apache.org on 2023/01/12 16:09:05 UTC
[sling-org-apache-sling-jcr-repoinit] branch master updated: SLING-11736 adjust types of existing nodes with "ensure nodes ..." (#41)
This is an automated email from the ASF dual-hosted git repository.
kwin pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/sling-org-apache-sling-jcr-repoinit.git
The following commit(s) were added to refs/heads/master by this push:
new abb28aa SLING-11736 adjust types of existing nodes with "ensure nodes ..." (#41)
abb28aa is described below
commit abb28aa77f215f53d4dfa01b4de6d3dd233f20a1
Author: Konrad Windszus <kw...@apache.org>
AuthorDate: Thu Jan 12 17:08:59 2023 +0100
SLING-11736 adjust types of existing nodes with "ensure nodes ..." (#41)
Also fixes SLING-10463 by bailing out in case same name property exists
---
pom.xml | 2 +-
.../sling/jcr/repoinit/impl/DoNothingVisitor.java | 6 ++
.../sling/jcr/repoinit/impl/NodeVisitor.java | 48 ++++++++++++---
.../apache/sling/jcr/repoinit/CreatePathsTest.java | 71 ++++++++++++++++++----
4 files changed, 107 insertions(+), 20 deletions(-)
diff --git a/pom.xml b/pom.xml
index dcf0c3f..bffc8fd 100644
--- a/pom.xml
+++ b/pom.xml
@@ -226,7 +226,7 @@
<dependency>
<groupId>org.apache.sling</groupId>
<artifactId>org.apache.sling.repoinit.parser</artifactId>
- <version>1.8.0</version>
+ <version>1.8.1-SNAPSHOT</version>
<scope>provided</scope>
</dependency>
<dependency>
diff --git a/src/main/java/org/apache/sling/jcr/repoinit/impl/DoNothingVisitor.java b/src/main/java/org/apache/sling/jcr/repoinit/impl/DoNothingVisitor.java
index 4594130..b9a5f39 100644
--- a/src/main/java/org/apache/sling/jcr/repoinit/impl/DoNothingVisitor.java
+++ b/src/main/java/org/apache/sling/jcr/repoinit/impl/DoNothingVisitor.java
@@ -29,6 +29,7 @@ import org.apache.sling.repoinit.parser.operations.DeleteGroup;
import org.apache.sling.repoinit.parser.operations.DeleteServiceUser;
import org.apache.sling.repoinit.parser.operations.DeleteUser;
import org.apache.sling.repoinit.parser.operations.DisableServiceUser;
+import org.apache.sling.repoinit.parser.operations.EnsureNodes;
import org.apache.sling.repoinit.parser.operations.OperationVisitor;
import org.apache.sling.repoinit.parser.operations.RegisterNamespace;
import org.apache.sling.repoinit.parser.operations.RegisterNodetypes;
@@ -150,6 +151,11 @@ class DoNothingVisitor implements OperationVisitor {
// no-op
}
+ @Override
+ public void visitEnsureNodes(EnsureNodes en) {
+ // no-op
+ }
+
@Override
public void visitRegisterNamespace(RegisterNamespace rn) {
// no-op
diff --git a/src/main/java/org/apache/sling/jcr/repoinit/impl/NodeVisitor.java b/src/main/java/org/apache/sling/jcr/repoinit/impl/NodeVisitor.java
index 8c4c0b9..8843f02 100644
--- a/src/main/java/org/apache/sling/jcr/repoinit/impl/NodeVisitor.java
+++ b/src/main/java/org/apache/sling/jcr/repoinit/impl/NodeVisitor.java
@@ -16,6 +16,8 @@
*/
package org.apache.sling.jcr.repoinit.impl;
+import java.util.ArrayList;
+import java.util.Arrays;
import java.util.Collections;
import java.util.List;
@@ -26,6 +28,7 @@ import javax.jcr.nodetype.ConstraintViolationException;
import org.apache.sling.repoinit.parser.operations.AddMixins;
import org.apache.sling.repoinit.parser.operations.CreatePath;
+import org.apache.sling.repoinit.parser.operations.EnsureNodes;
import org.apache.sling.repoinit.parser.operations.PathSegmentDefinition;
import org.apache.sling.repoinit.parser.operations.PropertyLine;
import org.apache.sling.repoinit.parser.operations.RemoveMixins;
@@ -50,19 +53,51 @@ public class NodeVisitor extends DoNothingVisitor {
super(s);
}
+ @Override
+ public void visitEnsureNodes(EnsureNodes en) {
+ createNodes(en.getDefinitions(), en.getPropertyLines(), true);
+ }
+
@Override
public void visitCreatePath(CreatePath cp) {
+ createNodes(cp.getDefinitions(), cp.getPropertyLines(), false);
+ }
+
+ private void createNodes(List<PathSegmentDefinition> pathSegmentDefinitions, List<PropertyLine> propertyLines, boolean strict) {
StringBuilder parentPathBuilder = new StringBuilder();
- for (PathSegmentDefinition psd : cp.getDefinitions()) {
+ for (PathSegmentDefinition psd : pathSegmentDefinitions) {
String parentPath = parentPathBuilder.toString();
final String fullPath = String.format("%s/%s", 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);
+ final Node node;
+ if (strict) {
+ if (session.nodeExists(fullPath)) {
+ log.info("Node at {} already exists, checking/adjusting its types", fullPath);
+ node = session.getNode(fullPath);
+ if (psd.getPrimaryType() != null && !node.getPrimaryNodeType().getName().equals(psd.getPrimaryType())) {
+ log.info("Adjusting primary type of node {} to {}", fullPath, psd.getPrimaryType());
+ node.setPrimaryType(psd.getPrimaryType());
+ }
+ } else if (!session.propertyExists(fullPath)) {
+ final Node parent = parentPath.equals("") ? session.getRootNode() : session.getNode(parentPath);
+ log.info("Creating node {} with primary type {}", fullPath, psd.getPrimaryType());
+ node = addChildNode(parent, psd);
+
+ } else {
+ throw new RepoInitException("There is a property with the name of the to be created node already at " + fullPath + ", therefore bailing out here as potentially not supported by the underlying JCR");
+ }
} 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);
+ if (session.itemExists(fullPath)) {
+ log.info("Path already exists, nothing to do (and not checking its primary type for now): {}", fullPath);
+ node = null;
+ } else {
+ final Node parent = parentPath.equals("") ? session.getRootNode() : session.getNode(parentPath);
+ log.info("Creating node {} with primary type {}", fullPath, psd.getPrimaryType());
+ node = addChildNode(parent, psd);
+ }
+ }
+
+ if (node != null) {
List<String> mixins = psd.getMixins();
if (mixins != null) {
log.info("Adding mixins {} to node {}", mixins, fullPath);
@@ -76,7 +111,6 @@ public class NodeVisitor extends DoNothingVisitor {
}
parentPathBuilder.append("/").append(psd.getSegment());
}
- List<PropertyLine> propertyLines = cp.getPropertyLines();
if (!propertyLines.isEmpty()) {
// delegate to the NodePropertiesVisitor to set the properties
SetProperties sp = new SetProperties(Collections.singletonList(parentPathBuilder.toString()), propertyLines);
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 3ef7860..3b6b500 100644
--- a/src/test/java/org/apache/sling/jcr/repoinit/CreatePathsTest.java
+++ b/src/test/java/org/apache/sling/jcr/repoinit/CreatePathsTest.java
@@ -18,6 +18,7 @@ package org.apache.sling.jcr.repoinit;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
import java.io.IOException;
import java.util.Arrays;
@@ -29,6 +30,7 @@ import javax.jcr.RepositoryException;
import javax.jcr.ValueFactory;
import org.apache.sling.commons.testing.jcr.RepositoryUtil;
+import org.apache.sling.jcr.repoinit.impl.RepoInitException;
import org.apache.sling.jcr.repoinit.impl.TestUtil;
import org.apache.sling.repoinit.parser.RepoInitParsingException;
import org.apache.sling.testing.mock.sling.ResourceResolverType;
@@ -36,10 +38,29 @@ import org.apache.sling.testing.mock.sling.junit.SlingContext;
import org.junit.Before;
import org.junit.Rule;
import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+import org.junit.runners.Parameterized.Parameters;
/** Test the creation of paths with specific node types */
+@RunWith(Parameterized.class)
public class CreatePathsTest {
-
+
+ @Parameters(name = "{1}")
+ public static Iterable<Object[]> data() {
+ return Arrays.asList(new Object[][] {
+ { Boolean.TRUE, "ensure nodes" }, { Boolean.FALSE, "create path" }
+ });
+ }
+
+ private final String baseCreateNodesStatement;
+ private final boolean strict;
+
+ public CreatePathsTest(boolean strict, String createNodesStatement) {
+ this.strict = strict;
+ this.baseCreateNodesStatement = createNodesStatement + " ";
+ }
+
@Rule
public final SlingContext context = new SlingContext(ResourceResolverType.JCR_OAK);
@@ -59,7 +80,7 @@ public class CreatePathsTest {
* @param path the path to create
*/
protected void assertSimplePath(final String path) throws RepositoryException, RepoInitParsingException {
- U.parseAndExecute("create path " + path);
+ U.parseAndExecute(baseCreateNodesStatement + path);
U.assertNodeExists(path);
}
@@ -86,7 +107,7 @@ public class CreatePathsTest {
@Test
public void createPathWithTypes() throws Exception {
final String path = "/four/five(sling:Folder)/six(nt:folder)";
- U.parseAndExecute("create path " + path);
+ U.parseAndExecute(baseCreateNodesStatement + path);
U.assertNodeExists("/four", "nt:unstructured");
U.assertNodeExists("/four/five", "sling:Folder");
U.assertNodeExists("/four/five/six", "nt:folder");
@@ -95,7 +116,7 @@ public class CreatePathsTest {
@Test
public void createPathWithSpecificDefaultType() throws Exception {
final String path = "/seven/eight(nt:unstructured)/nine";
- U.parseAndExecute("create path (sling:Folder) " + path);
+ U.parseAndExecute(baseCreateNodesStatement + "(sling:Folder) " + path);
U.assertNodeExists("/seven", "sling:Folder");
U.assertNodeExists("/seven/eight", "nt:unstructured");
U.assertNodeExists("/seven/eight/nine", "sling:Folder");
@@ -104,7 +125,7 @@ public class CreatePathsTest {
@Test
public void createPathWithJcrDefaultType() throws Exception {
final String path = "/ten/eleven(sling:Folder)/twelve";
- U.parseAndExecute("create path " + path);
+ U.parseAndExecute(baseCreateNodesStatement + path);
U.assertNodeExists("/ten", "nt:unstructured");
U.assertNodeExists("/ten/eleven", "sling:Folder");
U.assertNodeExists("/ten/eleven/twelve", "sling:Folder");
@@ -113,7 +134,7 @@ public class CreatePathsTest {
@Test
public void createPathWithMixins() throws Exception {
final String path = "/eleven(mixin mix:lockable)/twelve(mixin mix:referenceable,mix:shareable)/thirteen";
- U.parseAndExecute("create path " + path);
+ U.parseAndExecute(baseCreateNodesStatement + path);
U.assertNodeExists("/eleven", Collections.singletonList("mix:lockable"));
U.assertNodeExists("/eleven/twelve", Arrays.asList("mix:shareable", "mix:referenceable"));
}
@@ -121,7 +142,7 @@ public class CreatePathsTest {
@Test
public void createPathWithJcrDefaultTypeAndMixins() throws Exception {
final String path = "/twelve/thirteen(mixin mix:lockable)/fourteen";
- U.parseAndExecute("create path (nt:unstructured)" + path);
+ U.parseAndExecute(baseCreateNodesStatement + "(nt:unstructured)" + path);
U.assertNodeExists("/twelve", "nt:unstructured", Collections.<String>emptyList());
U.assertNodeExists("/twelve/thirteen", "nt:unstructured", Collections.singletonList("mix:lockable"));
U.assertNodeExists("/twelve/thirteen/fourteen", "nt:unstructured", Collections.<String>emptyList());
@@ -130,7 +151,7 @@ public class CreatePathsTest {
@Test
public void createPathWithJcrTypeAndMixins() throws Exception {
final String path = "/thirteen(nt:unstructured)/fourteen(nt:unstructured mixin mix:lockable)/fifteen(mixin mix:lockable)";
- U.parseAndExecute("create path " + path);
+ U.parseAndExecute(baseCreateNodesStatement + path);
U.assertNodeExists("/thirteen", "nt:unstructured", Collections.<String>emptyList());
U.assertNodeExists("/thirteen/fourteen", "nt:unstructured", Collections.singletonList("mix:lockable"));
U.assertNodeExists("/thirteen/fourteen/fifteen", "nt:unstructured", Collections.singletonList("mix:lockable"));
@@ -139,7 +160,7 @@ public class CreatePathsTest {
@Test
public void createPathNoDefaultPrimaryType() throws Exception {
U.adminSession.getRootNode().addNode("folder", "nt:folder");
- U.parseAndExecute("create path /folder/subfolder/subfolder2");
+ U.parseAndExecute(baseCreateNodesStatement + "/folder/subfolder/subfolder2");
Node subFolder = U.adminSession.getNode("/folder/subfolder");
assertEquals("sling:Folder", subFolder.getPrimaryNodeType().getName());
@@ -154,8 +175,34 @@ public class CreatePathsTest {
folder.setProperty("nodeOrProperty", "someValue");
folder.getSession().save();
final String fullPath = "/cpwpe/nodeOrProperty";
- U.parseAndExecute("create path " + fullPath);
- assertTrue(U.adminSession.propertyExists(fullPath));
+ if (strict) {
+ try {
+ U.parseAndExecute(baseCreateNodesStatement + fullPath);
+ fail("Creating a node at a path where a property exists already should have thrown an exception");
+ } catch (RepoInitException e) {
+ assertTrue(e.getMessage().contains("There is a property with the name of the to be created node already at"));
+ }
+ } else {
+ U.parseAndExecute("create path " + fullPath);
+ assertTrue(U.adminSession.propertyExists(fullPath));
+ }
+ }
+
+ /**
+ * SLING-11736 adjust existing node types
+ */
+ @Test
+ public void createPathWhereNodeExists() throws Exception {
+ final Node folder = U.adminSession.getRootNode().addNode("mynode", "nt:folder");
+ folder.addMixin("mix:mimeType");
+ folder.getSession().save();
+ final String fullPath = "/mynode";
+ U.parseAndExecute(baseCreateNodesStatement + fullPath + " (nt:unstructured mixin mix:mimeType,mix:language)");
+ if (strict) {
+ U.assertNodeExists("/mynode", "nt:unstructured", Arrays.asList("mix:mimeType", "mix:language"));
+ } else {
+ U.assertNodeExists("/mynode", "nt:folder", Arrays.asList("mix:mimeType"));
+ }
}
/**
@@ -176,7 +223,7 @@ public class CreatePathsTest {
// create the path with the mandatory property populated
final String path = String.format("/one(%s:foo)", NS_PREFIX);
- String createPathCndStatement = "create path " + path + " with properties\n"
+ String createPathCndStatement = baseCreateNodesStatement + path + " with properties\n"
+ " default displayName{String} to \"Hello\"\n"
+ "end\n";
U.parseAndExecute(createPathCndStatement);