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 ju...@apache.org on 2013/04/03 14:45:43 UTC
svn commit: r1463974 - in
/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/nodetype:
EffectiveType.java TypeEditor.java
Author: jukka
Date: Wed Apr 3 12:45:43 2013
New Revision: 1463974
URL: http://svn.apache.org/r1463974
Log:
OAK-411: Validator for node type management
Include more information in type validation errors
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
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=1463974&r1=1463973&r2=1463974&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 Wed Apr 3 12:45:43 2013
@@ -24,16 +24,18 @@ import static org.apache.jackrabbit.JcrC
import static org.apache.jackrabbit.oak.api.Type.BOOLEAN;
import java.util.List;
+import java.util.Set;
+import javax.annotation.CheckForNull;
import javax.annotation.Nonnull;
-import javax.jcr.nodetype.ConstraintViolationException;
-import com.google.common.base.Joiner;
import org.apache.jackrabbit.oak.api.PropertyState;
import org.apache.jackrabbit.oak.api.Type;
import org.apache.jackrabbit.oak.spi.state.ChildNodeEntry;
import org.apache.jackrabbit.oak.spi.state.NodeState;
+import com.google.common.collect.ImmutableSet;
+
class EffectiveType {
private final List<NodeState> types;
@@ -42,8 +44,10 @@ class EffectiveType {
this.types = checkNotNull(types);
}
- void checkMandatoryItems(NodeState node)
- throws ConstraintViolationException {
+ @Nonnull
+ Set<String> findMissingMandatoryItems(NodeState node) {
+ ImmutableSet.Builder<String> builder = ImmutableSet.builder();
+
for (NodeState type : types) {
NodeState properties =
type.getChildNode("oak:namedPropertyDefinitions");
@@ -59,8 +63,7 @@ class EffectiveType {
}
if (node.getProperty(name) == null
&& isMandatory(name, entry.getNodeState())) {
- throw new ConstraintViolationException(
- "Missing mandatory property " + name);
+ builder.add(name);
}
}
}
@@ -72,24 +75,23 @@ class EffectiveType {
String name = entry.getName();
if (!node.hasChildNode(name)
&& isMandatory(name, entry.getNodeState())) {
- throw new ConstraintViolationException(
- "Missing mandatory child node " + name);
+ builder.add(name);
}
}
}
}
+
+ return builder.build();
}
/**
* Finds a matching definition for a property with the given name and type.
*
* @param property modified property
- * @return matching property definition
- * @throws ConstraintViolationException if a matching definition was not found
+ * @return matching property definition, or {@code null}
*/
- @Nonnull
- NodeState getDefinition(PropertyState property)
- throws ConstraintViolationException {
+ @CheckForNull
+ NodeState getDefinition(PropertyState property) {
String propertyName = property.getName();
Type<?> propertyType = property.getType();
@@ -149,8 +151,7 @@ class EffectiveType {
}
}
- throw new ConstraintViolationException(
- "No matching definition found for property " + propertyName);
+ return null;
}
/**
@@ -159,12 +160,10 @@ class EffectiveType {
*
* @param nodeName child node name
* @param nodeType effective types of the child node
- * @return matching child node definition
- * @throws ConstraintViolationException if a matching definition was not found
+ * @return matching child node definition, or {@code null} if not found
*/
- @Nonnull
- NodeState getDefinition(String nodeName, Iterable<String> nodeType)
- throws ConstraintViolationException {
+ @CheckForNull
+ NodeState getDefinition(String nodeName, Iterable<String> nodeType) {
boolean sns = false;
int n = nodeName.length();
if (n > 3 && nodeName.charAt(n - 1) == ']') {
@@ -212,8 +211,7 @@ class EffectiveType {
}
}
- throw new ConstraintViolationException(
- "Incorrect node type of child node " + nodeName + " (" + Joiner.on(",").join(nodeType) + ')');
+ return null;
}
//-----------------------------------------------------------< private >--
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=1463974&r1=1463973&r2=1463974&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 Wed Apr 3 12:45:43 2013
@@ -47,6 +47,7 @@ import org.apache.jackrabbit.oak.spi.com
import org.apache.jackrabbit.oak.spi.commit.Editor;
import org.apache.jackrabbit.oak.spi.state.NodeState;
+import com.google.common.base.Joiner;
import com.google.common.base.Predicate;
import com.google.common.collect.Lists;
import com.google.common.collect.Queues;
@@ -83,6 +84,16 @@ class TypeEditor extends DefaultEditor {
this.types = parent.types;
}
+ private String getPath() {
+ if (parent == null) {
+ return "/";
+ } else if (parent.parent == null) {
+ return "/" + nodeName;
+ } else {
+ return parent.getPath() + "/" + nodeName;
+ }
+ }
+
/**
* Computes the effective type of the modified type.
*/
@@ -92,12 +103,12 @@ class TypeEditor extends DefaultEditor {
Iterable<String> names = computeEffectiveType(after);
// find matching entry in the parent node's effective type
- if (parent != null) {
- try {
- parent.effective.getDefinition(nodeName, names);
- } catch (ConstraintViolationException e) {
- throw new CommitFailedException(e);
- }
+ // TODO: this should be in childNodeAdded()
+ if (parent != null
+ && parent.effective.getDefinition(nodeName, names) == null) {
+ throw constraintViolation(
+ "Incorrect node type of child node " + nodeName
+ + " with types " + Joiner.on(", ").join(names));
}
}
@@ -107,37 +118,37 @@ class TypeEditor extends DefaultEditor {
// TODO: add any auto-created items that are still missing
// verify the presence of all mandatory items
- try {
- effective.checkMandatoryItems(after);
- } catch (ConstraintViolationException e) {
- throw new CommitFailedException(e);
+ Set<String> missing = effective.findMissingMandatoryItems(after);
+ if (!missing.isEmpty()) {
+ throw constraintViolation(
+ "Missing mandatory items " + Joiner.on(", ").join(missing));
}
}
private CommitFailedException constraintViolation(String message) {
return new CommitFailedException(
- new ConstraintViolationException(message));
+ new ConstraintViolationException(getPath() + ": " + message));
}
@Override
public void propertyAdded(PropertyState after) throws CommitFailedException {
- try {
- NodeState definition = effective.getDefinition(after);
- checkValueConstraints(definition, after);
- } catch (ConstraintViolationException e) {
- throw new CommitFailedException(e);
+ NodeState definition = effective.getDefinition(after);
+ if (definition == null) {
+ throw constraintViolation(
+ "No matching property definition found for " + after);
}
+ checkValueConstraints(definition, after);
}
@Override
public void propertyChanged(PropertyState before, PropertyState after)
throws CommitFailedException {
- try {
- NodeState definition = effective.getDefinition(after);
- checkValueConstraints(definition, after);
- } catch (ConstraintViolationException e) {
- throw new CommitFailedException(e);
+ NodeState definition = effective.getDefinition(after);
+ if (definition == null) {
+ throw constraintViolation(
+ "No matching property definition found for " + after);
}
+ checkValueConstraints(definition, after);
}
@Override
@@ -153,13 +164,6 @@ class TypeEditor extends DefaultEditor {
return new TypeEditor(this, name);
}
- private boolean getBoolean(NodeState node, String name) {
- PropertyState property = node.getProperty(name);
- return property != null
- && property.getType() == BOOLEAN
- && property.getValue(BOOLEAN);
- }
-
//-----------------------------------------------------------< private >--
private void checkValueConstraints(
@@ -218,7 +222,7 @@ class TypeEditor extends DefaultEditor {
}
}
}
- throw constraintViolation("Value constraint violation");
+ throw constraintViolation("Value constraint violation in " + property);
}
/**
@@ -313,4 +317,11 @@ class TypeEditor extends DefaultEditor {
return names;
}
+ private boolean getBoolean(NodeState node, String name) {
+ PropertyState property = node.getProperty(name);
+ return property != null
+ && property.getType() == BOOLEAN
+ && property.getValue(BOOLEAN);
+ }
+
}