You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@sling.apache.org by bd...@apache.org on 2020/03/26 11:27:59 UTC
[sling-org-apache-sling-jcr-repoinit] 02/02: SLING-9171 - add
missing test for the 'default' mode and factor out its handling in
NodePropertiesVisitor
This is an automated email from the ASF dual-hosted git repository.
bdelacretaz pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/sling-org-apache-sling-jcr-repoinit.git
commit d8c1a1472e2137e98d24f8ae1c08b1e0ef5b803a
Author: Bertrand Delacretaz <bd...@apache.org>
AuthorDate: Thu Mar 26 12:27:01 2020 +0100
SLING-9171 - add missing test for the 'default' mode and factor out its handling in NodePropertiesVisitor
---
.../jcr/repoinit/impl/NodePropertiesVisitor.java | 47 +++++++++++++++-------
.../sling/jcr/repoinit/SetPropertiesTest.java | 42 +++++++++++++++----
.../apache/sling/jcr/repoinit/impl/TestUtil.java | 5 ++-
3 files changed, 70 insertions(+), 24 deletions(-)
diff --git a/src/main/java/org/apache/sling/jcr/repoinit/impl/NodePropertiesVisitor.java b/src/main/java/org/apache/sling/jcr/repoinit/impl/NodePropertiesVisitor.java
index 79b2db9..d04c4c4 100644
--- a/src/main/java/org/apache/sling/jcr/repoinit/impl/NodePropertiesVisitor.java
+++ b/src/main/java/org/apache/sling/jcr/repoinit/impl/NodePropertiesVisitor.java
@@ -21,6 +21,7 @@ import java.util.Calendar;
import javax.jcr.Session;
import javax.jcr.Node;
+import javax.jcr.PathNotFoundException;
import javax.jcr.Value;
import javax.jcr.PropertyType;
import javax.jcr.RepositoryException;
@@ -35,8 +36,9 @@ import org.apache.sling.repoinit.parser.operations.PropertyLine;
import org.apache.sling.repoinit.parser.operations.SetProperties;
/**
- * OperationVisitor which processes only operations related to setting node properties. Having several such specialized visitors makes it easy to control the
- * execution order.
+ * OperationVisitor which processes only operations related to setting node
+ * properties. Having several such specialized visitors makes it easy to control
+ * the execution order.
*/
class NodePropertiesVisitor extends DoNothingVisitor {
@@ -49,22 +51,37 @@ class NodePropertiesVisitor extends DoNothingVisitor {
super(s);
}
+ /**
+ * True if the property needs to be set - if false, it is not touched. This
+ * handles the "default" repoinit instruction, which means "do not change the
+ * property if already set"
+ *
+ * @throws RepositoryException
+ * @throws PathNotFoundException
+ */
+ private static boolean needToSetProperty(Node n, PropertyLine line) throws RepositoryException {
+ if(!line.isDefault()) {
+ // It's a "set" line -> overwrite existing value if any
+ return true;
+ }
+
+ // Otherwise set the property only if not set yet
+ final String name = line.getPropertyName();
+ return(!n.hasProperty(name) || n.getProperty(name) == null);
+ }
+
@Override
public void visitSetProperties(SetProperties sp) {
- List<String> nodePaths = sp.getPaths();
- List<PropertyLine> propertyLines = sp.getPropertyLines();
- for (String nodePath : nodePaths) {
+ for (String nodePath : sp.getPaths()) {
try {
log.info("Setting properties on nodePath '{}'", nodePath);
Node n = session.getNode(nodePath);
- for (PropertyLine pl : propertyLines) {
- String pName = pl.getPropertyName();
- PropertyLine.PropertyType pType = pl.getPropertyType();
- List<Object> values = pl.getPropertyValues();
- boolean setOnlyIfNull = pl.isDefault();
- int type = PropertyType.valueFromName(pType.name());
- boolean isExistingNonNullProperty = n.hasProperty(pName) && n.getProperty(pName) != null;
- if (!setOnlyIfNull || (setOnlyIfNull && !isExistingNonNullProperty)) {
+ for (PropertyLine pl : sp.getPropertyLines()) {
+ final String pName = pl.getPropertyName();
+ final PropertyLine.PropertyType pType = pl.getPropertyType();
+ final List<Object> values = pl.getPropertyValues();
+ final int type = PropertyType.valueFromName(pType.name());
+ if (needToSetProperty(n, pl)) {
if (values.size() > 1) {
Value[] pValues = convertToValues(values);
n.setProperty(pName, pValues, type);
@@ -73,7 +90,9 @@ class NodePropertiesVisitor extends DoNothingVisitor {
n.setProperty(pName, pValue, type);
}
} else {
- log.info("Property '{}' is already set on path '{}'. Will not overwrite the default", pName, nodePath);
+ log.info(
+ "Property '{}' already set on path '{}', existing value will not be overwritten in 'default' mode",
+ pName, nodePath);
}
}
} catch (RepositoryException e) {
diff --git a/src/test/java/org/apache/sling/jcr/repoinit/SetPropertiesTest.java b/src/test/java/org/apache/sling/jcr/repoinit/SetPropertiesTest.java
index 2d572fb..67a886a 100644
--- a/src/test/java/org/apache/sling/jcr/repoinit/SetPropertiesTest.java
+++ b/src/test/java/org/apache/sling/jcr/repoinit/SetPropertiesTest.java
@@ -31,6 +31,7 @@ import javax.jcr.RepositoryException;
import javax.jcr.ValueFactory;
import javax.jcr.Value;
import java.io.IOException;
+import java.util.UUID;
/** Test the setting of properties on nodes */
@@ -40,23 +41,26 @@ public class SetPropertiesTest {
public final SlingContext context = new SlingContext(ResourceResolverType.JCR_OAK);
private TestUtil U;
- final String path1 = "/one/two/three";
- final String path2 = "/one/two/four";
+ private ValueFactory vf;
+ private static final String pathPrefix = "/one/two/";
+ private static final String path1 = pathPrefix + UUID.randomUUID();
+ private static final String path2 = pathPrefix + UUID.randomUUID();
+ private static final String path3 = pathPrefix + UUID.randomUUID();
@Before
public void setup() throws RepositoryException, IOException, RepoInitParsingException {
U = new TestUtil(context);
+ vf = U.adminSession.getValueFactory();
RepositoryUtil.registerSlingNodeTypes(U.adminSession);
- U.parseAndExecute("create path " + path1);
- U.assertNodeExists(path1);
- U.parseAndExecute("create path " + path2);
- U.assertNodeExists(path2);
+ for(String p : new String[] { path1, path2, path3 }) {
+ U.parseAndExecute("create path " + p);
+ U.assertNodeExists(p);
+ }
}
@Test
public void setStringPropertyTest() throws Exception {
U.parseAndExecute("set properties on " + path1 + " \n set sling:ResourceType{String} to /x/y/z \n end");
- ValueFactory vf = U.adminSession.getValueFactory();
Value expectedValue = vf.createValue("/x/y/z");
U.assertSVPropertyExists(path1, "sling:ResourceType", expectedValue);
}
@@ -74,7 +78,6 @@ public class SetPropertiesTest {
+ "end"
;
U.parseAndExecute(setProps);
- ValueFactory vf = U.adminSession.getValueFactory();
Value expectedValue1 = vf.createValue("/x/y/z");
U.assertSVPropertyExists(path2, "sling:ResourceType", expectedValue1);
Value[] expectedValues2 = new Value[2];
@@ -102,4 +105,27 @@ public class SetPropertiesTest {
}
}
+ @Test
+ public void setDefaultProperties() throws Exception {
+ final String setPropsA =
+ "set properties on " + path3 + "\n"
+ + "set one to oneA\n"
+ + "default two to twoA\n"
+ + "end"
+ ;
+ U.parseAndExecute(setPropsA);
+ U.assertSVPropertyExists(path3, "one", vf.createValue("oneA"));
+ U.assertSVPropertyExists(path3, "two", vf.createValue("twoA"));
+
+ final String setPropsB =
+ "set properties on " + path3 + "\n"
+ + "set one to oneB\n"
+ + "default two to twoB\n"
+ + "end"
+ ;
+
+ U.parseAndExecute(setPropsB);
+ U.assertSVPropertyExists(path3, "one", vf.createValue("oneB"));
+ U.assertSVPropertyExists(path3, "two", vf.createValue("twoA"));
+ }
}
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 ac4c1d3..828f5ae 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
@@ -21,6 +21,7 @@ import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertNull;
import static org.junit.Assert.assertTrue;
import static org.junit.Assert.fail;
+import static org.junit.Assert.assertArrayEquals;
import static org.junit.Assert.assertEquals;
import java.io.Reader;
@@ -201,7 +202,7 @@ public class TestUtil {
} else {
Property p = n.getProperty(propertyName);
Value actualValue = p.getValue();
- assertEquals("Value mismatch for property: " + propertyName, actualValue, expectedValue);
+ assertEquals("Value mismatch for property: " + propertyName, expectedValue, actualValue);
}
}
@@ -212,7 +213,7 @@ public class TestUtil {
} else {
Property p = n.getProperty(propertyName);
Value[] actualValues = p.getValues();
- assertEquals("Values mismatch for property: " + propertyName, actualValues, expectedValues);
+ assertArrayEquals("Values mismatch for property: " + propertyName, expectedValues, actualValues);
}
}