Permalink
Browse files

MODE-2149 Implemented eager validation for property constraints.

  • Loading branch information...
1 parent c8fdac1 commit f22f21b8c8149463caf25465584c24f8f9e71daa @hchiorean hchiorean committed Feb 24, 2014
View
38 modeshape-jcr/src/main/java/org/modeshape/jcr/JcrPropertyDefinition.java
@@ -42,7 +42,6 @@
import org.modeshape.jcr.value.ValueFactories;
import org.modeshape.jcr.value.ValueFactory;
import org.modeshape.jcr.value.ValueFormatException;
-import org.modeshape.jcr.value.basic.JodaDateTime;
/**
* ModeShape implementation of the {@link PropertyDefinition} interface. This implementation is immutable and has all fields
@@ -103,6 +102,11 @@ static Operator operatorFromSymbol( String jcrConstantValue ) {
this.defaultValues = defaultValues;
this.requiredType = requiredType;
this.valueConstraints = valueConstraints;
+ assert this.valueConstraints != null;
+ if(requiredType != PropertyType.UNDEFINED && valueConstraints.length > 0) {
+ //if we have a required type, create the default checker eagerly to detect any invalid constraint values
+ this.checker = createChecker(context, requiredType, valueConstraints);
+ }
this.multiple = multiple;
this.fullTextSearchable = fullTextSearchable;
this.queryOrderable = queryOrderable;
@@ -111,7 +115,6 @@ static Operator operatorFromSymbol( String jcrConstantValue ) {
QueryObjectModelConstants.JCR_OPERATOR_GREATER_THAN_OR_EQUAL_TO, QueryObjectModelConstants.JCR_OPERATOR_LESS_THAN,
QueryObjectModelConstants.JCR_OPERATOR_LESS_THAN_OR_EQUAL_TO, QueryObjectModelConstants.JCR_OPERATOR_LIKE,
QueryObjectModelConstants.JCR_OPERATOR_NOT_EQUAL_TO};
- assert this.valueConstraints != null;
this.id = this.declaringNodeType == null ? null : new PropertyDefinitionId(this.declaringNodeType.getInternalName(),
this.name, this.requiredType, this.multiple);
this.key = this.id == null ? prototypeKey : prototypeKey.withId("/jcr:system/jcr:nodeTypes/" + this.id.getString());
@@ -408,7 +411,7 @@ boolean canCastToTypeAndSatisfyConstraints( Value value,
assert value instanceof JcrValue : "Illegal implementation of Value interface";
((JcrValue)value).asType(getRequiredType()); // throws ValueFormatException if there's a problem
return satisfiesConstraints(value, session);
- } catch (javax.jcr.ValueFormatException vfe) {
+ } catch (javax.jcr.ValueFormatException | org.modeshape.jcr.value.ValueFormatException vfe) {
// Cast failed
return false;
}
@@ -469,6 +472,7 @@ private ConstraintChecker createChecker( ExecutionContext context,
return new ReferenceConstraintChecker(valueConstraints, context);
case org.modeshape.jcr.api.PropertyType.SIMPLE_REFERENCE:
return new SimpleReferenceConstraintChecker(valueConstraints, context);
+ case PropertyType.URI:
case PropertyType.STRING:
return new StringConstraintChecker(valueConstraints, context);
case PropertyType.DECIMAL:
@@ -618,8 +622,6 @@ protected RangeConstraintChecker( String[] valueConstraints,
protected abstract ValueFactory<T> getValueFactory( ValueFactories valueFactories );
- protected abstract T parseValue( String s );
-
@Override
public String toString() {
return constraints.toString();
@@ -683,8 +685,8 @@ protected T getMaximum() {
String rval = commaInd < valueConstraint.length() - 2 ? valueConstraint.substring(commaInd + 1,
valueConstraint.length() - 1) : null;
- final T lower = lval == null ? null : parseValue(lval.trim());
- final T upper = rval == null ? null : parseValue(rval.trim());
+ final T lower = lval == null ? null : valueFactory.create(lval.trim());
+ final T upper = rval == null ? null : valueFactory.create(rval.trim());
return new Range<T>() {
@Override
@@ -836,10 +838,6 @@ public int getType() {
return valueFactories.getLongFactory();
}
- @Override
- protected Long parseValue( String s ) {
- return Long.parseLong(s);
- }
}
@Immutable
@@ -860,10 +858,6 @@ public int getType() {
return valueFactories.getDateFactory();
}
- @Override
- protected DateTime parseValue( String s ) {
- return new JodaDateTime(s.trim());
- }
}
@Immutable
@@ -884,10 +878,6 @@ public int getType() {
return valueFactories.getDoubleFactory();
}
- @Override
- protected Double parseValue( String s ) {
- return Double.parseDouble(s);
- }
}
@Immutable
@@ -908,10 +898,6 @@ public int getType() {
return valueFactories.getDecimalFactory();
}
- @Override
- protected BigDecimal parseValue( String s ) {
- return new BigDecimal(s);
- }
}
@Immutable
@@ -1078,7 +1064,11 @@ protected StringConstraintChecker( String[] valueConstraints,
for (int i = 0; i < valueConstraints.length; i++) {
String expr = valueConstraints[i];
- constraints[i] = Pattern.compile(expr);
+ try {
+ constraints[i] = Pattern.compile(expr);
+ } catch (Exception e) {
+ throw new ValueFormatException(expr, org.modeshape.jcr.value.PropertyType.STRING, "Invalid string pattern ");
+ }
expressions.add(expr);
}
}
View
12 modeshape-jcr/src/main/java/org/modeshape/jcr/JcrPropertyDefinitionTemplate.java
@@ -67,6 +67,18 @@
}
}
}
+
+ //check if there are constraints of type Name, for which additional namespaces may need registration
+ boolean shouldCheckForNameConstraints = requiredType == PropertyType.NAME ||
+ requiredType == PropertyType.REFERENCE ||
+ requiredType == PropertyType.WEAKREFERENCE ||
+ requiredType == org.modeshape.jcr.api.PropertyType.SIMPLE_REFERENCE;
+ if (this.valueConstraints != null && this.valueConstraints.length > 0 && shouldCheckForNameConstraints) {
+ for (String constraintValue : original.getValueConstraints()) {
+ Name nameValue = original.getContext().getValueFactories().getNameFactory().create(constraintValue);
+ JcrItemDefinitionTemplate.registerMissingNamespaces(original.getContext(), context, nameValue);
+ }
+ }
}
JcrPropertyDefinitionTemplate with( ExecutionContext context ) {
View
39 modeshape-jcr/src/test/java/org/modeshape/jcr/JcrNodeTypeManagerTest.java
@@ -34,6 +34,7 @@
import org.junit.BeforeClass;
import org.junit.Test;
import org.modeshape.common.FixFor;
+import org.modeshape.jcr.value.ValueFormatException;
public class JcrNodeTypeManagerTest extends MultiUseAbstractTest {
@@ -219,7 +220,8 @@ public void shouldAllowRegisteringBooleanConstraints() throws Exception {
// definition as BOOLEAN and a String constraint like 'true'
NodeTypeTemplate nodeTypeTemplate = nodeTypeMgr.createNodeTypeTemplate();
nodeTypeTemplate.setPrimaryItemName("test");
- nodeTypeTemplate.setName(namespaceName.concat(":").concat(nodeTypeName));
+ String primaryType = namespaceName.concat(":").concat(nodeTypeName);
+ nodeTypeTemplate.setName(primaryType);
PropertyDefinitionTemplate propertyDefinition = nodeTypeMgr.createPropertyDefinitionTemplate();
propertyDefinition.setName("test");
@@ -230,7 +232,7 @@ public void shouldAllowRegisteringBooleanConstraints() throws Exception {
nodeTypeMgr.registerNodeType(nodeTypeTemplate, false);
- Node node = session.getRootNode().addNode("test", namespaceName.concat(":").concat(nodeTypeName));
+ Node node = session.getRootNode().addNode("test", primaryType);
node.setProperty("test", true);
session.save();
try {
@@ -239,6 +241,39 @@ public void shouldAllowRegisteringBooleanConstraints() throws Exception {
fail("Value which violates constraint did not raise exception");
} catch (ConstraintViolationException e) {
//expected
+ node.remove();
+ session.save();
+ nodeTypeMgr.unregisterNodeType(primaryType);
+ }
+ }
+
+ @Test
+ @FixFor( "MOE-2149" )
+ @SuppressWarnings("unchecked")
+ public void shouldValidateConstraintValue() throws Exception {
+ String namespaceName = "admb";
+ String namespaceUri = "http://www.admb.be/modeshape/admb/1.0";
+ String nodeTypeName = "test";
+
+ session.getWorkspace().getNamespaceRegistry().registerNamespace(namespaceName, namespaceUri);
+
+ NodeTypeTemplate nodeTypeTemplate = nodeTypeMgr.createNodeTypeTemplate();
+ nodeTypeTemplate.setPrimaryItemName("test");
+ String primaryType = namespaceName.concat(":").concat(nodeTypeName);
+ nodeTypeTemplate.setName(primaryType);
+
+ PropertyDefinitionTemplate propertyDefinition = nodeTypeMgr.createPropertyDefinitionTemplate();
+ propertyDefinition.setName("test");
+ propertyDefinition.setRequiredType(PropertyType.LONG);
+ propertyDefinition.setMandatory(true);
+ propertyDefinition.setValueConstraints(new String[] { "test" });
+ nodeTypeTemplate.getPropertyDefinitionTemplates().add(propertyDefinition);
+
+ try {
+ nodeTypeMgr.registerNodeType(nodeTypeTemplate, false);
+ fail("Should not allow the registration of a node type with invalid constraint");
+ } catch (ValueFormatException e) {
+ //expected
}
}
View
3 ...cers/modeshape-sequencer-sramp/src/main/resources/org/modeshape/sequencer/sramp/sramp.cnd
@@ -20,7 +20,8 @@
<jcr='http://www.jcp.org/jcr/1.0'>
<nt='http://www.jcp.org/jcr/nt/1.0'>
<mix='http://www.jcp.org/jcr/mix/1.0'>
-<sramp = "http://s-ramp.org/xmlns/2010/s-ramp">
+<sramp='http://s-ramp.org/xmlns/2010/s-ramp'>
+<owl='http://www.w3.org/2002/07/owl'>
//------------------------------------------------------------------------------
// N O D E T Y P E S

0 comments on commit f22f21b

Please sign in to comment.