You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@sling.apache.org by en...@apache.org on 2022/06/11 22:15:05 UTC
[sling-org-apache-sling-jcr-repoinit] branch master updated: SLING-11293 improve handling of autocreated properties (#26)
This is an automated email from the ASF dual-hosted git repository.
enorman 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 d5a8079 SLING-11293 improve handling of autocreated properties (#26)
d5a8079 is described below
commit d5a8079144d6e632a84e4b6c104292173a4341d4
Author: Eric Norman <en...@apache.org>
AuthorDate: Sat Jun 11 15:15:01 2022 -0700
SLING-11293 improve handling of autocreated properties (#26)
allow set default properties instruction to change autocreated property values if they are unchanged from the initial value
---
.../jcr/repoinit/impl/NodePropertiesVisitor.java | 119 +++++++++++++-
.../sling/jcr/repoinit/SetPropertiesTest.java | 182 +++++++++++++++++++--
2 files changed, 284 insertions(+), 17 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 465685b..9ed5979 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
@@ -16,9 +16,13 @@
*/
package org.apache.sling.jcr.repoinit.impl;
+import java.nio.file.Path;
+import java.nio.file.Paths;
import java.util.ArrayList;
+import java.util.Arrays;
import java.util.Calendar;
import java.util.List;
+import java.util.stream.Stream;
import javax.jcr.Node;
import javax.jcr.PathNotFoundException;
@@ -26,6 +30,8 @@ import javax.jcr.PropertyType;
import javax.jcr.RepositoryException;
import javax.jcr.Session;
import javax.jcr.Value;
+import javax.jcr.nodetype.NodeType;
+import javax.jcr.nodetype.PropertyDefinition;
import org.apache.jackrabbit.api.security.user.Authorizable;
import org.apache.jackrabbit.util.Text;
@@ -37,6 +43,7 @@ import org.apache.jackrabbit.value.StringValue;
import org.apache.sling.repoinit.parser.operations.PropertyLine;
import org.apache.sling.repoinit.parser.operations.SetProperties;
import org.jetbrains.annotations.NotNull;
+import org.jetbrains.annotations.Nullable;
/**
* OperationVisitor which processes only operations related to setting node
@@ -64,6 +71,88 @@ class NodePropertiesVisitor extends DoNothingVisitor {
super(s);
}
+ /**
+ * Find the PropertyDefinition for the specified propName
+ *
+ * @param propName the propertyName to check
+ * @param parentNode the parent node where the property will be set
+ * @return the property definition of the property or null if it could not be determined
+ */
+ private static @Nullable PropertyDefinition resolvePropertyDefinition(@NotNull String propName, @NotNull Node parentNode) throws RepositoryException {
+ NodeType primaryNodeType = parentNode.getPrimaryNodeType();
+ // try the primary type
+ PropertyDefinition propDef = resolvePropertyDefinition(propName, primaryNodeType);
+ if (propDef == null) {
+ // not found in the primary type, so try the mixins
+ NodeType[] mixinNodeTypes = parentNode.getMixinNodeTypes();
+ for (NodeType mixinNodeType : mixinNodeTypes) {
+ propDef = resolvePropertyDefinition(propName, mixinNodeType);
+ if (propDef != null) {
+ break;
+ }
+ }
+ }
+ return propDef;
+ }
+
+ /**
+ * Inspect the NodeType definition to try to determine the
+ * requiredType for the specified property
+ *
+ * @param propName the propertyName to check
+ * @param parentNode the parent node where the property will be set
+ * @return the required type of the property or {@link PropertyType#UNDEFINED} if it could not be determined
+ */
+ private static @Nullable PropertyDefinition resolvePropertyDefinition(@NotNull String propName, @NotNull NodeType nodeType) {
+ return Stream.of(nodeType.getPropertyDefinitions())
+ .filter(pd -> propName.equals(pd.getName()))
+ .findFirst()
+ .orElse(null);
+ }
+
+ /**
+ * SLING-11293 - Check if a property is defined as autocreated and the current value
+ * is the same as the autocreated default value
+ *
+ * @param n the node to check
+ * @param pRelPath the property relative path to check
+ * @return true or false
+ */
+ protected static boolean isUnchangedAutocreatedProperty(Node n, final String pRelPath)
+ throws RepositoryException {
+ boolean sameAsDefault = false;
+
+ // deal with the pRelPath nesting
+ Path path = Paths.get(pRelPath);
+ Path parentPath = path.getParent();
+ String name = path.getFileName().toString();
+ if (parentPath != null) {
+ String relPath = parentPath.toString();
+ if (n.hasNode(relPath)) {
+ n = n.getNode(relPath);
+ } else {
+ n = null;
+ }
+ }
+
+ // if the property has been set by being autocreated and the value is still
+ // the same as the default values then also allow changing the value
+ if (n != null && n.hasProperty(name)) {
+ @Nullable
+ PropertyDefinition pd = resolvePropertyDefinition(name, n);
+ if (pd != null && pd.isAutoCreated()) {
+ // if the current value is the same as the autocreated default values
+ // then allow the value to be changed.
+ if (pd.isMultiple()) {
+ sameAsDefault = Arrays.equals(pd.getDefaultValues(), n.getProperty(name).getValues());
+ } else {
+ sameAsDefault = Arrays.equals(pd.getDefaultValues(), new Value[] {n.getProperty(name).getValue()});
+ }
+ }
+ }
+ return sameAsDefault;
+ }
+
/**
* 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
@@ -72,15 +161,21 @@ class NodePropertiesVisitor extends DoNothingVisitor {
* @throws RepositoryException
* @throws PathNotFoundException
*/
- private static boolean needToSetProperty(Node n, PropertyLine line) throws RepositoryException {
- if(!line.isDefault()) {
+ private static boolean needToSetProperty(@NotNull Node n, @NotNull 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);
+ boolean needToSet;
+ if (isUnchangedAutocreatedProperty(n, name)) { // SLING-11293
+ needToSet = true;
+ } else {
+ needToSet = (!n.hasProperty(name) || n.getProperty(name) == null);
+ }
+ return needToSet;
}
/**
@@ -91,14 +186,24 @@ class NodePropertiesVisitor extends DoNothingVisitor {
* @throws RepositoryException
* @throws PathNotFoundException
*/
- private static boolean needToSetProperty(Authorizable a, String pRelPath, boolean isDefault) throws RepositoryException {
- if (!isDefault) {
+ private static boolean needToSetProperty(Session session, Authorizable a, String pRelPath, 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
- return(!a.hasProperty(pRelPath) || a.getProperty(pRelPath) == null);
+ boolean needToSet;
+ Node n = null;
+ if (session.nodeExists(a.getPath())) {
+ n = session.getNode(a.getPath());
+ }
+ if (n != null && isUnchangedAutocreatedProperty(n, pRelPath)) { // SLING-11293
+ needToSet = true;
+ } else {
+ needToSet = (!a.hasProperty(pRelPath) || a.getProperty(pRelPath) == null);
+ }
+ return needToSet;
}
/**
@@ -157,7 +262,7 @@ class NodePropertiesVisitor extends DoNothingVisitor {
for (PropertyLine pl : propertyLines) {
final String pName = pl.getPropertyName();
final String pRelPath = toRelPath(subTreePath, pName);
- if (needToSetProperty(a, pRelPath, pl.isDefault())) {
+ if (needToSetProperty(session, a, pRelPath, pl)) {
final List<Object> values = pl.getPropertyValues();
if (values.size() > 1) {
Value[] pValues = convertToValues(values);
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 94e6713..92d0446 100644
--- a/src/test/java/org/apache/sling/jcr/repoinit/SetPropertiesTest.java
+++ b/src/test/java/org/apache/sling/jcr/repoinit/SetPropertiesTest.java
@@ -16,8 +16,25 @@
*/
package org.apache.sling.jcr.repoinit;
+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.fail;
+
+import java.io.IOException;
+import java.nio.file.Paths;
+import java.util.UUID;
+
+import javax.jcr.Node;
+import javax.jcr.PropertyType;
+import javax.jcr.RepositoryException;
+import javax.jcr.Value;
+import javax.jcr.ValueFactory;
+import javax.jcr.nodetype.NoSuchNodeTypeException;
+import javax.jcr.nodetype.NodeType;
+
+import org.apache.jackrabbit.api.security.user.Authorizable;
import org.apache.sling.commons.testing.jcr.RepositoryUtil;
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.sling.ResourceResolverType;
import org.apache.sling.testing.mock.sling.junit.SlingContext;
@@ -26,16 +43,6 @@ import org.junit.Before;
import org.junit.Rule;
import org.junit.Test;
-import javax.jcr.PropertyType;
-import javax.jcr.RepositoryException;
-import javax.jcr.ValueFactory;
-import javax.jcr.Value;
-
-import static org.junit.Assert.fail;
-
-import java.io.IOException;
-import java.util.UUID;
-
/** Test the setting of properties on nodes */
public class SetPropertiesTest {
@@ -215,6 +222,161 @@ public class SetPropertiesTest {
}
}
+ /**
+ * SLING-11293 "set default properties" instruction to change autocreated property value
+ */
+ @Test
+ public void setAutocreatedDefaultPropertiesFromPrimaryType() throws Exception {
+ registerSling11293NodeType();
+
+ // create the test node
+ String name = UUID.randomUUID().toString();
+ String testPath = pathPrefix + name;
+ Node parentNode = U.getAdminSession()
+ .getNode(Paths.get(testPath).getParent().toString());
+ parentNode.addNode(name, "slingtest:sling11293");
+ changeAutocreatedDefaultProperties(testPath);
+ }
+
+ /**
+ * SLING-11293 "set default properties" instruction to change autocreated property value
+ */
+ @Test
+ public void setAutocreatedDefaultPropertiesFromMixinType() throws Exception {
+ registerSling11293NodeType();
+
+ // create the test node
+ String name = UUID.randomUUID().toString();
+ String testPath = pathPrefix + name;
+ Node parentNode = U.getAdminSession()
+ .getNode(Paths.get(testPath).getParent().toString());
+ parentNode.addNode(name).addMixin("slingtest:sling11293mixin");
+ changeAutocreatedDefaultProperties(testPath);
+ }
+
+ protected void changeAutocreatedDefaultProperties(String testPath)
+ throws RepositoryException, RepoInitParsingException {
+ U.assertNodeExists(testPath);
+ // verify the initial autocreated property values
+ U.assertSVPropertyExists(testPath, "singleVal", vf.createValue("autocreated value"));
+ U.assertMVPropertyExists(testPath, "multiVal", new Value[] {
+ vf.createValue("autocreated value1"),
+ vf.createValue("autocreated value2")
+ });
+
+ // setting "default" the first time should change the value as the value is now
+ // the same as the autocreated default values
+ final String setPropsA =
+ "set properties on " + testPath + "\n" +
+ "default singleVal to sChanged1a\n" +
+ "default multiVal to mChanged1a, mChanged2a\n" +
+ "end";
+ U.parseAndExecute(setPropsA);
+ U.assertSVPropertyExists(testPath, "singleVal", vf.createValue("sChanged1a"));
+ U.assertMVPropertyExists(testPath, "multiVal", new Value[] {
+ vf.createValue("mChanged1a"),
+ vf.createValue("mChanged2a")
+ });
+
+ // setting again should do nothing as the value is now
+ // not the same as the autocreated default values
+ final String setPropsB =
+ "set properties on " + testPath + "\n" +
+ "default singleVal to sChanged1b\n" +
+ "default multiVal to mChanged1b, mChanged2b\n" +
+ "end";
+ U.parseAndExecute(setPropsB);
+ U.assertSVPropertyExists(testPath, "singleVal", vf.createValue("sChanged1a"));
+ U.assertMVPropertyExists(testPath, "multiVal", new Value[] {
+ vf.createValue("mChanged1a"),
+ vf.createValue("mChanged2a")
+ });
+ }
+
+ /**
+ * SLING-11293 "set default properties" instruction to change autocreated property value
+ * for an authorizable
+ */
+ @Test
+ public void setAutocreatedDefaultUserProperties() throws Exception {
+ registerSling11293NodeType();
+
+ String userid = "user" + UUID.randomUUID();
+
+ U.assertUser("before creating user", userid, false);
+ U.parseAndExecute("create user " + userid);
+ U.assertUser("after creating user", userid, true);
+
+ final Authorizable a = UserUtil.getUserManager(U.getAdminSession()).getAuthorizable(userid);
+
+ // create the test node
+ String name = UUID.randomUUID().toString();
+ U.getAdminSession()
+ .getNode(a.getPath())
+ .addNode(name, "slingtest:sling11293");
+ String testPath = a.getPath() + "/" + name;
+ U.assertNodeExists(testPath);
+ // verify the initial autocreated property values
+ U.assertSVPropertyExists(testPath, "singleVal", vf.createValue("autocreated value"));
+ U.assertMVPropertyExists(testPath, "multiVal", new Value[] {
+ vf.createValue("autocreated value1"),
+ vf.createValue("autocreated value2")
+ });
+
+ // setting "default" the first time should change the value as the value is now
+ // the same as the autocreated default values
+ final String setPropsA =
+ "set properties on authorizable(" + userid + ")/" + name + "\n" +
+ "default singleVal to sChanged1a\n" +
+ "default multiVal to mChanged1a, mChanged2a\n" +
+ "end";
+ U.parseAndExecute(setPropsA);
+ U.assertAuthorizableSVPropertyExists(userid, name + "/singleVal", vf.createValue("sChanged1a"));
+ U.assertAuthorizableMVPropertyExists(userid, name + "/multiVal", new Value[] {
+ vf.createValue("mChanged1a"),
+ vf.createValue("mChanged2a")
+ });
+
+ // setting again should do nothing as the value is now
+ // not the same as the autocreated default values
+ final String setPropsB =
+ "set properties on authorizable(" + userid + ")/" + name + "\n" +
+ "default singleVal to sChanged1b\n" +
+ "default multiVal to mChanged1b, mChanged2b\n" +
+ "end";
+ U.parseAndExecute(setPropsB);
+ U.assertAuthorizableSVPropertyExists(userid, name + "/singleVal", vf.createValue("sChanged1a"));
+ U.assertAuthorizableMVPropertyExists(userid, name + "/multiVal", new Value[] {
+ vf.createValue("mChanged1a"),
+ vf.createValue("mChanged2a")
+ });
+ }
+
+ protected void registerSling11293NodeType()
+ throws RepositoryException, RepoInitParsingException, NoSuchNodeTypeException {
+ final String registerNodeTypes =
+ "register nodetypes\n" +
+ "<<===\n" +
+ "<< <slingtest='http://sling.apache.org/ns/test/repoinit-it/v1.0'>\n" +
+ "<< [slingtest:sling11293] > nt:unstructured\n" +
+ "<< - singleVal (String) = 'autocreated value'\n" +
+ "<< autocreated\n" +
+ "<< - multiVal (String) = 'autocreated value1', 'autocreated value2'\n" +
+ "<< multiple autocreated\n" +
+ "<< [slingtest:sling11293mixin]\n" +
+ "<< mixin\n" +
+ "<< - singleVal (String) = 'autocreated value'\n" +
+ "<< autocreated\n" +
+ "<< - multiVal (String) = 'autocreated value1', 'autocreated value2'\n" +
+ "<< multiple autocreated\n" +
+ "===>>";
+ U.parseAndExecute(registerNodeTypes);
+ NodeType nodeType = U.getAdminSession().getWorkspace().getNodeTypeManager().getNodeType("slingtest:sling11293");
+ assertNotNull(nodeType);
+ NodeType mixinType = U.getAdminSession().getWorkspace().getNodeTypeManager().getNodeType("slingtest:sling11293mixin");
+ assertNotNull(mixinType);
+ }
+
/**
* Set properties on an authorizable and then verify that the values were set
*/