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 <[email protected]> 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); } }
