Merge pull request #35 from jsedding/issues/SLING-11483
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 9ed5979..e9f4a23 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
@@ -26,6 +26,7 @@
import javax.jcr.Node;
import javax.jcr.PathNotFoundException;
+import javax.jcr.Property;
import javax.jcr.PropertyType;
import javax.jcr.RepositoryException;
import javax.jcr.Session;
@@ -45,6 +46,7 @@
import org.jetbrains.annotations.NotNull;
import org.jetbrains.annotations.Nullable;
+
/**
* OperationVisitor which processes only operations related to setting node
* properties. Having several such specialized visitors makes it easy to control
@@ -291,15 +293,18 @@
for (PropertyLine pl : propertyLines) {
final String pName = pl.getPropertyName();
if (needToSetProperty(n, pl)) {
- final PropertyLine.PropertyType pType = pl.getPropertyType();
- final int type = PropertyType.valueFromName(pType.name());
- final List<Object> values = pl.getPropertyValues();
- if (values.size() > 1) {
- Value[] pValues = convertToValues(values);
- n.setProperty(pName, pValues, type);
- } else {
- Value pValue = convertToValue(values.get(0));
- n.setProperty(pName, pValue, type);
+ final int newType = PropertyType.valueFromName(pl.getPropertyType().name());
+ Value[] newValues = convertToValues(pl.getPropertyValues());
+ Property oldProperty = n.hasProperty(pName) ? n.getProperty(pName) : null;
+ // only set property if type and/or values change
+ // note: Node#setProperty() touches the node even if the property value is unchanged
+ // and thus needs to be explicitly avoided
+ if(hasPropertyChange(oldProperty, newType, newValues)) {
+ if (newValues.length == 1) {
+ n.setProperty(pName, newValues[0], newType);
+ } else {
+ n.setProperty(pName, newValues, newType);
+ }
}
} else {
log.info("Property '{}' already set on path '{}', existing value will not be overwritten in 'default' mode",
@@ -308,6 +313,31 @@
}
}
+ private boolean hasPropertyChange(Property oldProperty, int newType, Value... newValues) throws RepositoryException {
+ if (oldProperty == null || oldProperty.getType() != newType) {
+ return true;
+ }
+
+ final Value[] oldValues = oldProperty.isMultiple() ? oldProperty.getValues() : new Value[]{ oldProperty.getValue() };
+ if (oldValues.length != newValues.length) {
+ return true;
+ }
+
+ for (int i = 0; i < oldValues.length; i++) {
+ final Value oldValue = oldValues[i];
+ final Value newValue = newValues[i];
+ if (!valueEquals(oldValue, newValue)) {
+ return true;
+ }
+ }
+ return false;
+ }
+
+ private static boolean valueEquals(Value oldValue, Value newValue) throws RepositoryException {
+ return oldValue.getType() == newValue.getType()
+ && oldValue.getString().equals(newValue.getString());
+ }
+
@Override
public void visitSetProperties(SetProperties sp) {
for (String nodePath : sp.getPaths()) {
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 92d0446..5c9b5f8 100644
--- a/src/test/java/org/apache/sling/jcr/repoinit/SetPropertiesTest.java
+++ b/src/test/java/org/apache/sling/jcr/repoinit/SetPropertiesTest.java
@@ -16,7 +16,9 @@
*/
package org.apache.sling.jcr.repoinit;
+import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertTrue;
import static org.junit.Assert.fail;
import java.io.IOException;
@@ -43,7 +45,6 @@
import org.junit.Rule;
import org.junit.Test;
-
/** Test the setting of properties on nodes */
public class SetPropertiesTest {
@@ -56,13 +57,14 @@
private static final String path1 = pathPrefix + UUID.randomUUID();
private static final String path2 = pathPrefix + UUID.randomUUID();
private static final String path3 = pathPrefix + UUID.randomUUID();
+ private static final String path4 = pathPrefix + UUID.randomUUID();
@Before
public void setup() throws RepositoryException, IOException, RepoInitParsingException {
U = new TestUtil(context);
vf = U.adminSession.getValueFactory();
RepositoryUtil.registerSlingNodeTypes(U.adminSession);
- for(String p : new String[] { path1, path2, path3 }) {
+ for(String p : new String[] { path1, path2, path3, path4 }) {
U.parseAndExecute("create path " + p);
U.assertNodeExists(p);
}
@@ -221,6 +223,19 @@
// all good
}
}
+
+ @Test
+ public void noChangeOnSameProps() throws Exception {
+ String testRT = "/my/props/test";
+ String repoinitStr = "set properties on " + path4 + " \n set sling:ResourceType{String} to "+testRT+" \n end";
+ boolean hasChanges = U.parseAndExecute(repoinitStr);
+ assertTrue("No changes first attempt - session should contain changes.", hasChanges);
+ hasChanges = U.parseAndExecute(repoinitStr);
+ assertFalse("Returning changes on second execution with same values - session should NOT contain changes.", hasChanges);
+ Value expectedValue = vf.createValue(testRT);
+ U.assertSVPropertyExists(path4, "sling:ResourceType", expectedValue);
+
+ }
/**
* SLING-11293 "set default properties" instruction to change autocreated property value
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 fd57723..406686c 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
@@ -244,10 +244,12 @@
}
}
- public void parseAndExecute(String input) throws RepositoryException, RepoInitParsingException {
+ public boolean parseAndExecute(String input) throws RepositoryException, RepoInitParsingException {
final JcrRepoInitOpsProcessorImpl p = new JcrRepoInitOpsProcessorImpl();
p.apply(adminSession, parse(input));
+ boolean hasChanges = adminSession.hasPendingChanges();
adminSession.save();
+ return hasChanges;
}
public void cleanupUser() throws RepositoryException, RepoInitParsingException {
