Author: jukka Date: Fri May 17 12:59:03 2013 New Revision: 1483779 URL: http://svn.apache.org/r1483779 Log: OAK-822: TypeEditor: Missing validation for mandatory items
Improved handling of mandatory items: Now the presence of *all* mandatory items is only checked when a new node is being added. Afterwards the "mandatoriness" of items is only checked when they are being removed. Error codes documented in http://wiki.apache.org/jackrabbit/OakErrorCodes. Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/nodetype/EffectiveType.java jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/nodetype/TypeEditor.java jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/authorization/evaluation/JcrUUIDTest.java Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/nodetype/EffectiveType.java URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/nodetype/EffectiveType.java?rev=1483779&r1=1483778&r2=1483779&view=diff ============================================================================== --- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/nodetype/EffectiveType.java (original) +++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/nodetype/EffectiveType.java Fri May 17 12:59:03 2013 @@ -30,10 +30,11 @@ import static org.apache.jackrabbit.JcrC import static org.apache.jackrabbit.JcrConstants.JCR_PRIMARYTYPE; import static org.apache.jackrabbit.JcrConstants.JCR_UUID; import static org.apache.jackrabbit.oak.api.CommitFailedException.CONSTRAINT; -import static org.apache.jackrabbit.oak.api.Type.NAMES; import static org.apache.jackrabbit.oak.api.Type.UNDEFINED; import static org.apache.jackrabbit.oak.api.Type.UNDEFINEDS; import static org.apache.jackrabbit.oak.plugins.nodetype.NodeTypeConstants.JCR_IS_ABSTRACT; +import static org.apache.jackrabbit.oak.plugins.nodetype.NodeTypeConstants.OAK_MANDATORY_CHILD_NODES; +import static org.apache.jackrabbit.oak.plugins.nodetype.NodeTypeConstants.OAK_MANDATORY_PROPERTIES; import static org.apache.jackrabbit.oak.plugins.nodetype.NodeTypeConstants.OAK_NAMED_CHILD_NODE_DEFINITIONS; import static org.apache.jackrabbit.oak.plugins.nodetype.NodeTypeConstants.OAK_RESIDUAL_CHILD_NODE_DEFINITIONS; import static org.apache.jackrabbit.oak.plugins.nodetype.NodeTypeConstants.OAK_SUPERTYPES; @@ -50,7 +51,6 @@ import org.apache.jackrabbit.oak.api.Typ import org.apache.jackrabbit.oak.spi.state.ChildNodeEntry; import org.apache.jackrabbit.oak.spi.state.NodeState; -import com.google.common.collect.ImmutableSet; import com.google.common.collect.Lists; class EffectiveType { @@ -84,29 +84,22 @@ class EffectiveType { return false; } - @Nonnull - Set<String> findMissingMandatoryItems(NodeState node) { - ImmutableSet.Builder<String> builder = ImmutableSet.builder(); + boolean isMandatoryProperty(String name) { + return nameSetContains(OAK_MANDATORY_PROPERTIES, name); + } - for (NodeState type : types) { - PropertyState properties = - type.getProperty("oak:mandatoryProperties"); - for (String name : properties.getValue(NAMES)) { - if (!node.hasProperty(name)) { - builder.add(name); - } - } + @Nonnull + Set<String> getMandatoryProperties() { + return getNameSet(OAK_MANDATORY_PROPERTIES); + } - PropertyState childNodes = - type.getProperty("oak:mandatoryChildNodes"); - for (String name : childNodes.getValue(NAMES)) { - if (!node.hasChildNode(name)) { - builder.add(name); - } - } - } + boolean isMandatoryChildNode(String name) { + return nameSetContains(OAK_MANDATORY_CHILD_NODES, name); + } - return builder.build(); + @Nonnull + Set<String> getMandatoryChildNodes() { + return getNameSet(OAK_MANDATORY_CHILD_NODES); } /** @@ -364,4 +357,22 @@ class EffectiveType { return name; } + private boolean nameSetContains(String set, String name) { + for (NodeState type : types) { + if (contains(type.getNames(set), name)) { + return true; + } + } + return false; + } + + @Nonnull + private Set<String> getNameSet(String set) { + Set<String> names = newHashSet(); + for (NodeState type : types) { + addAll(names, type.getNames(set)); + } + return names; + } + } Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/nodetype/TypeEditor.java URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/nodetype/TypeEditor.java?rev=1483779&r1=1483778&r2=1483779&view=diff ============================================================================== --- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/nodetype/TypeEditor.java (original) +++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/nodetype/TypeEditor.java Fri May 17 12:59:03 2013 @@ -16,11 +16,9 @@ */ package org.apache.jackrabbit.oak.plugins.nodetype; -import java.util.Set; import javax.jcr.PropertyType; import javax.jcr.Value; -import com.google.common.base.Joiner; import com.google.common.base.Predicate; import org.apache.jackrabbit.oak.api.CommitFailedException; import org.apache.jackrabbit.oak.api.PropertyState; @@ -103,13 +101,6 @@ class TypeEditor extends DefaultEditor { public void leave(NodeState before, NodeState after) throws CommitFailedException { // TODO: add any auto-created items that are still missing - - // verify the presence of all mandatory items - Set<String> missing = effective.findMissingMandatoryItems(builder.getNodeState()); - if (!missing.isEmpty()) { - throw constraintViolation( - 2, "Missing mandatory items " + Joiner.on(", ").join(missing)); - } } @Override @@ -139,13 +130,41 @@ class TypeEditor extends DefaultEditor { } @Override + public void propertyDeleted(PropertyState before) + throws CommitFailedException { + String name = before.getName(); + if (effective.isMandatoryProperty(name)) { + throw constraintViolation( + 22, "Mandatory property " + name + " can not be removed"); + } + } + + @Override public Editor childNodeAdded(String name, NodeState after) throws CommitFailedException { - return childNodeChanged(name, MISSING_NODE, after); + TypeEditor editor = childNodeChanged(name, MISSING_NODE, after); + + // verify the presence of all mandatory items + for (String property : editor.effective.getMandatoryProperties()) { + if (!after.hasProperty(property)) { + throw constraintViolation( + 21, "Mandatory property " + property + + " not found in a new node"); + } + } + for (String child : editor.effective.getMandatoryChildNodes()) { + if (!after.hasChildNode(child)) { + throw constraintViolation( + 25, "Mandatory child node " + child + + " not found in a new node"); + } + } + + return editor; } @Override - public Editor childNodeChanged( + public TypeEditor childNodeChanged( String name, NodeState before, NodeState after) throws CommitFailedException { NodeBuilder childBuilder = builder.getChildNode(name); @@ -176,6 +195,17 @@ class TypeEditor extends DefaultEditor { return new TypeEditor(this, name, childType, childBuilder); } + @Override + public Editor childNodeDeleted(String name, NodeState before) + throws CommitFailedException { + if (effective.isMandatoryChildNode(name)) { + throw constraintViolation( + 26, "Mandatory child node " + name + " can not be removed"); + } else { + return null; // no further checking needed for the removed subtree + } + } + //-----------------------------------------------------------< private >-- private void checkValueConstraints( Modified: jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/authorization/evaluation/JcrUUIDTest.java URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/authorization/evaluation/JcrUUIDTest.java?rev=1483779&r1=1483778&r2=1483779&view=diff ============================================================================== --- jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/authorization/evaluation/JcrUUIDTest.java (original) +++ jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/authorization/evaluation/JcrUUIDTest.java Fri May 17 12:59:03 2013 @@ -216,7 +216,7 @@ public class JcrUUIDTest extends Abstrac fail("Removing the jcr:uuid property of a referenceable node must fail."); } catch (CommitFailedException e) { assertEquals(CommitFailedException.CONSTRAINT, e.getType()); - assertEquals(2, e.getCode()); + assertEquals(22, e.getCode()); } } } \ No newline at end of file