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 mr...@apache.org on 2013/03/26 15:17:00 UTC

svn commit: r1461148 - /jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/nodetype/NodeTypeImpl.java

Author: mreutegg
Date: Tue Mar 26 14:16:59 2013
New Revision: 1461148

URL: http://svn.apache.org/r1461148
Log:
OAK-719: NodeTypeImpl.canRemoveItem() may return incorrect value

Modified:
    jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/nodetype/NodeTypeImpl.java

Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/nodetype/NodeTypeImpl.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/nodetype/NodeTypeImpl.java?rev=1461148&r1=1461147&r2=1461148&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/nodetype/NodeTypeImpl.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/nodetype/NodeTypeImpl.java Tue Mar 26 14:16:59 2013
@@ -29,6 +29,7 @@ import javax.annotation.CheckForNull;
 import javax.jcr.PropertyType;
 import javax.jcr.RepositoryException;
 import javax.jcr.Value;
+import javax.jcr.nodetype.ItemDefinition;
 import javax.jcr.nodetype.NoSuchNodeTypeException;
 import javax.jcr.nodetype.NodeDefinition;
 import javax.jcr.nodetype.NodeType;
@@ -383,37 +384,20 @@ class NodeTypeImpl extends AbstractTypeD
 
     @Override
     public boolean canRemoveItem(String itemName) {
-        return canRemoveNode(itemName) || canRemoveProperty(itemName);
+        List<ItemDefinition> definitions = Lists.newArrayList();
+        definitions.addAll(Arrays.asList(getChildNodeDefinitions()));
+        definitions.addAll(Arrays.asList(getPropertyDefinitions()));
+        return internalCanRemoveItem(itemName, definitions);
     }
 
     @Override
     public boolean canRemoveNode(String nodeName) {
-        // FIXME: properly calculate matching definition taking residual definitions into account.
-        NodeDefinition[] childNodeDefinitions = getChildNodeDefinitions();
-        for (NodeDefinition definition : childNodeDefinitions) {
-            String name = definition.getName();
-            if (matches(nodeName, name)) {
-                if (definition.isMandatory() || definition.isProtected()) {
-                    return false;
-                }
-            }
-        }
-        return childNodeDefinitions.length > 0;
+        return internalCanRemoveItem(nodeName, Arrays.asList(getChildNodeDefinitions()));
     }
 
     @Override
     public boolean canRemoveProperty(String propertyName) {
-        // FIXME: should properly calculate matching definition taking residual definitions into account.
-        PropertyDefinition[] propertyDefinitions = getPropertyDefinitions();
-        for (PropertyDefinition definition : propertyDefinitions) {
-            String name = definition.getName();
-            if (propertyName.equals(name)) {
-                if (definition.isMandatory() || definition.isProtected()) {
-                    return false;
-                }
-            }
-        }
-        return propertyDefinitions.length > 0;
+        return internalCanRemoveItem(propertyName, Arrays.asList(getPropertyDefinitions()));
     }
 
     //-------------------------------------------------------------< Object >---
@@ -424,6 +408,20 @@ class NodeTypeImpl extends AbstractTypeD
 
     //-----------------------------------------------------------< internal >---
 
+    private boolean internalCanRemoveItem(String itemName,
+                                          Iterable<? extends ItemDefinition> definitions) {
+        // FIXME: should properly calculate matching definition taking residual definitions into account.
+        for (ItemDefinition definition : definitions) {
+            String name = definition.getName();
+            if (matches(itemName, name)) {
+                if (definition.isMandatory() || definition.isProtected()) {
+                    return false;
+                }
+            }
+        }
+        return definitions.iterator().hasNext();
+    }
+
     private ReadOnlyNodeTypeManager getManager() {
         final Tree types = definition.getParent();
         return new ReadOnlyNodeTypeManager() {