You are viewing a plain text version of this content. The canonical link for it is here.
Posted to oak-commits@jackrabbit.apache.org by kw...@apache.org on 2022/02/14 18:00:23 UTC

[jackrabbit-oak] branch bugfix/consider-type-for-residual-property-type-definition created (now 51d0147)

This is an automated email from the ASF dual-hosted git repository.

kwin pushed a change to branch bugfix/consider-type-for-residual-property-type-definition
in repository https://gitbox.apache.org/repos/asf/jackrabbit-oak.git.


      at 51d0147  OAK-9695 correctly evaluate property type from definition while check if protected

This branch includes the following new commits:

     new 51d0147  OAK-9695 correctly evaluate property type from definition while check if protected

The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.


[jackrabbit-oak] 01/01: OAK-9695 correctly evaluate property type from definition while check if protected

Posted by kw...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

kwin pushed a commit to branch bugfix/consider-type-for-residual-property-type-definition
in repository https://gitbox.apache.org/repos/asf/jackrabbit-oak.git

commit 51d0147ce5f2e6490a4462d137c5509b915a0e0c
Author: Konrad Windszus <kw...@apache.org>
AuthorDate: Mon Feb 14 19:00:12 2022 +0100

    OAK-9695 correctly evaluate property type from definition while check if
    protected
---
 .../jackrabbit/oak/jcr/delegate/NodeDelegate.java  | 39 ++++++++++++++--------
 .../oak/jcr/delegate/PropertyDelegate.java         |  2 +-
 .../apache/jackrabbit/oak/jcr/PropertyTest.java    | 30 +++++++++++++++++
 3 files changed, 56 insertions(+), 15 deletions(-)

diff --git a/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/delegate/NodeDelegate.java b/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/delegate/NodeDelegate.java
index b851892..5403e7c 100644
--- a/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/delegate/NodeDelegate.java
+++ b/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/delegate/NodeDelegate.java
@@ -193,14 +193,14 @@ public class NodeDelegate extends ItemDelegate {
         return false;
     }
 
-    boolean isProtected(String property) throws InvalidItemStateException {
+    boolean isProtected(String propertyName, Type<?> propertyType) throws InvalidItemStateException {
         Tree tree = getTree();
         Tree typeRoot = sessionDelegate.getRoot().getTree(NODE_TYPES_PATH);
         List<Tree> types = TreeUtil.getEffectiveType(tree, typeRoot);
 
         boolean protectedResidual = false;
         for (Tree type : types) {
-            if (contains(TreeUtil.getNames(type, REP_PROTECTED_PROPERTIES), property)) {
+            if (contains(TreeUtil.getNames(type, REP_PROTECTED_PROPERTIES), propertyName)) {
                 return true;
             } else if (!protectedResidual) {
                 protectedResidual = TreeUtil.getBoolean(
@@ -209,18 +209,15 @@ public class NodeDelegate extends ItemDelegate {
         }
 
         // Special case: There are one or more protected *residual*
-        // child node definitions. Iterate through them to check whether
+        // property definitions. Iterate through them to check whether
         // there's a matching, protected one.
         if (protectedResidual) {
-            for (Tree type : types) {
-                Tree definitions = type.getChild(REP_RESIDUAL_PROPERTY_DEFINITIONS);
-                for (Tree definition : definitions.getChildren()) {
-                    // TODO: check for matching property type?
-                    if (TreeUtil.getBoolean(definition, JCR_PROTECTED)) {
-                        return true;
-                    }
+        	Tree definition = findMatchingResidualPropertyDefinition(null, types, propertyType, true);
+        	if (definition != null) {
+        		if (TreeUtil.getBoolean(definition, JCR_PROTECTED)) {
+                    return true;
                 }
-            }
+        	}
         }
 
         return false;
@@ -617,7 +614,20 @@ public class NodeDelegate extends ItemDelegate {
         }
 
         // Then look through any residual property definitions
-        for (Tree type : types) {
+        return findMatchingResidualPropertyDefinition(fuzzyMatch, types, propertyType.isArray(), definedType, undefinedType, exactTypeMatch);
+    }
+
+    private Tree findMatchingResidualPropertyDefinition(Tree fuzzyMatch, List<Tree> types, Type<?> propertyType, boolean exactTypeMatch) {
+    	String definedType = propertyType.toString();
+        String undefinedType = UNDEFINED.toString();
+        if (propertyType.isArray()) {
+            undefinedType = UNDEFINEDS.toString();
+        }
+        return findMatchingResidualPropertyDefinition(fuzzyMatch, types, propertyType.isArray(), definedType, undefinedType, exactTypeMatch);
+    }
+
+    private Tree findMatchingResidualPropertyDefinition(Tree fuzzyMatch, List<Tree> types, boolean isMultiValue, String definedType, String undefinedType, boolean exactTypeMatch) {
+    	for (Tree type : types) {
             Tree definitions = type.getChild(REP_RESIDUAL_PROPERTY_DEFINITIONS);
             Tree definition = definitions.getChild(definedType);
             if (definition.exists()) {
@@ -629,7 +639,7 @@ public class NodeDelegate extends ItemDelegate {
             }
             if (!exactTypeMatch && fuzzyMatch == null) {
                 for (Tree def : definitions.getChildren()) {
-                    if (propertyType.isArray() == TreeUtil.getBoolean(def, JCR_MULTIPLE)) {
+                    if (isMultiValue == TreeUtil.getBoolean(def, JCR_MULTIPLE)) {
                         fuzzyMatch = def;
                         break;
                     }
@@ -639,7 +649,8 @@ public class NodeDelegate extends ItemDelegate {
 
         return fuzzyMatch;
     }
-
+    
+    
     private Tree findMatchingChildNodeDefinition(
             List<Tree> types, String childNodeName, Set<String> typeNames) {
         // First look for a matching named property definition
diff --git a/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/delegate/PropertyDelegate.java b/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/delegate/PropertyDelegate.java
index 45b49e8..1376cf2 100644
--- a/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/delegate/PropertyDelegate.java
+++ b/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/delegate/PropertyDelegate.java
@@ -92,7 +92,7 @@ public class PropertyDelegate extends ItemDelegate {
 
     @Override
     public boolean isProtected() throws InvalidItemStateException {
-        return getParent().isProtected(name);
+        return getParent().isProtected(name, state.getType());
     }
 
     @NotNull
diff --git a/oak-jcr/src/test/java/org/apache/jackrabbit/oak/jcr/PropertyTest.java b/oak-jcr/src/test/java/org/apache/jackrabbit/oak/jcr/PropertyTest.java
index 1218a6e..aef92cf 100644
--- a/oak-jcr/src/test/java/org/apache/jackrabbit/oak/jcr/PropertyTest.java
+++ b/oak-jcr/src/test/java/org/apache/jackrabbit/oak/jcr/PropertyTest.java
@@ -20,16 +20,21 @@ import javax.jcr.Node;
 import javax.jcr.Property;
 import javax.jcr.PropertyIterator;
 import javax.jcr.PropertyType;
+import javax.jcr.RepositoryException;
 import javax.jcr.Value;
 import javax.jcr.ValueFormatException;
+import javax.jcr.lock.LockException;
+import javax.jcr.nodetype.ConstraintViolationException;
 import javax.jcr.nodetype.NodeTypeManager;
 import javax.jcr.nodetype.NodeTypeTemplate;
 import javax.jcr.nodetype.PropertyDefinition;
 import javax.jcr.nodetype.PropertyDefinitionTemplate;
+import javax.jcr.version.VersionException;
 
 import com.google.common.collect.Iterators;
 import org.apache.jackrabbit.JcrConstants;
 import org.apache.jackrabbit.test.AbstractJCRTest;
+import org.apache.jackrabbit.value.ValueFactoryImpl;
 
 public class PropertyTest extends AbstractJCRTest {
 
@@ -58,6 +63,12 @@ public class PropertyTest extends AbstractJCRTest {
         pdt.setRequiredType(PropertyType.LONG);
         template.getPropertyDefinitionTemplates().add(pdt);
 
+        pdt = ntMgr.createPropertyDefinitionTemplate();
+        pdt.setName("*"); // residual property type
+        pdt.setRequiredType(PropertyType.URI);
+        pdt.setProtected(true);
+        template.getPropertyDefinitionTemplates().add(pdt);
+        
         ntMgr.registerNodeType(template, true);
 
         node = testRootNode.addNode(nodeName2, NT_NAME);
@@ -274,4 +285,23 @@ public class PropertyTest extends AbstractJCRTest {
         assertEquals(3, pitr.getSize());
         assertEquals(3, Iterators.size(pitr));
     }
+    
+    public void testSetAndRemoveUnprotectedProperty() throws ValueFormatException, VersionException, LockException, ConstraintViolationException, RepositoryException {
+    	Property property = node.setProperty(BOOLEAN_PROP_NAME, true);
+    	assertNotNull(property);
+    	property.setValue(false);
+    	superuser.save();
+    	property.remove();
+    	superuser.save();
+    }
+    
+    public void testSetProtectedResidualProperty() throws ValueFormatException, VersionException, LockException, ConstraintViolationException, RepositoryException {
+    	Value uriValue = ValueFactoryImpl.getInstance().createValue("http://example.com", PropertyType.URI);
+    	try {
+    		node.setProperty("test", uriValue);
+    		fail("Setting protected property (according to residual propery type definition) must throw ConstraintViolationException");
+    	} catch (ConstraintViolationException e) {
+            // success
+        }
+    }
 }
\ No newline at end of file