AR-166: Property constraints included in integrity checks

AR-194: Regular expression constraint applied to cm:name property to flag illegal characters
Other minor fixes
 - Dictionary bootstrap bean depends on I18N bean being constructed
 - RegexConstraint check can be inverted
 - Some I18N message fixes


git-svn-id: https://svn.alfresco.com/repos/alfresco-enterprise/alfresco/HEAD/root@2549 c4b6b30b-aa2e-2d43-bbcb-ca4b014f7261

config/alfresco/core-services-context.xml

@@ -400,7 +400,7 @@
     	<property name="dictionaryDAO"><ref local="dictionaryDAO"/></property>
     </bean>
     
-    <bean id="dictionaryBootstrap" parent="dictionaryModelBootstrap">
+    <bean id="dictionaryBootstrap" parent="dictionaryModelBootstrap" depends-on="resourceBundles">
         <property name="models">
             <list>
                 <value>alfresco/model/dictionaryModel.xml</value>

config/alfresco/messages/dictionary-messages.properties

@@ -4,15 +4,16 @@ d_dictionary.model.err.no_model=Model ''{0}'' does not exist
 d_dictionary.model.err.type_not_found=Failed to create anonymous type as specified type {0} not found
 d_dictionary.model.err.aspect_not_found=Failed to create anonymous type as specified aspect {0} not found
 
-d_dictionary.model.err.cyclic_ref=Constraint ''{0}'' is part of a cyclic reference of constraints
+d_dictionary.constraint.err.cyclic_ref=Constraint ''{0}'' is part of a cyclic reference of constraints
-d_dictionary.model.err.type_and_ref=Constraint ''{0}'' cannot have a ''type'' and be a ''reference'' attribute
+d_dictionary.constraint.err.type_and_ref=Constraint ''{0}'' cannot have a ''type'' and be a ''reference'' attribute
-d_dictionary.model.err.type_or_ref=Constraint ''{0}'' cannot have a ''type'' and be a ''reference'' attribute
+d_dictionary.constraint.err.type_or_ref=Constraint ''{0}'' cannot have a ''type'' and be a ''reference'' attribute
-d_dictionary.model.err.ref_not_found=Constraint reference ''{0}'' not found on constraint ''{1}''
+d_dictionary.constraint.err.ref_not_found=Constraint reference ''{0}'' not found on constraint ''{1}''
-d_dictionary.model.err.anon_needs_property=Anonymous constraints can only be declared within the context of a property
+d_dictionary.constraint.err.anon_needs_property=Anonymous constraints can only be declared within the context of a property
-d_dictionary.model.err.invalid_type=Constraint type ''{0}'' on constraint ''{1}'' is not a well-known type or a valid Constraint implementation
+d_dictionary.constraint.err.invalid_type=Constraint type ''{0}'' on constraint ''{1}'' is not a well-known type or a valid Constraint implementation
-d_dictionary.model.err.construct_failure=Failed to construct an instance of type ''{0}'' for constraint ''{1}''
+d_dictionary.constraint.err.construct_failure=Failed to construct an instance of type ''{0}'' for constraint ''{1}''
-d_dictionary.model.err.property_not_set=Property ''{0}'' has not been set on constraint ''{1}''
+d_dictionary.constraint.err.property_mismatch=Property mismatch setting property ''{0}'' on constraint ''{1}''
-d_dictionary.model.err.evaluate_exception=Exception during evaluation of constraint ''{0}'': {1}
+d_dictionary.constraint.err.property_not_set=Property ''{0}'' has not been set on constraint ''{1}''
+d_dictionary.constraint.err.evaluate_exception=Exception during evaluation of constraint ''{0}'': {1}
 
 d_dictionary.property.err.property_type_not_specified=Property type of property ''{0}'' must be specified
 d_dictionary.property.err.property_type_not_found=Property type ''{0}'' of property ''{1}'' is not found
@@ -20,3 +21,4 @@ d_dictionary.property.err.single_valued_content=Content properties must be singl
 d_dictionary.property.err.duplicate_constraint_on_property=Found duplicate constraint definition ''{0}'' within property ''{1}''
 
 d_dictionary.constraint.regex.no_match=Value ''{0}'' does not match regular expression: {1}
+d_dictionary.constraint.regex.match=Value ''{0}'' matches regular expression: {1}

config/alfresco/model/contentModel.xml

@@ -14,6 +14,12 @@
       <namespace uri="http://www.alfresco.org/model/content/1.0" prefix="cm"/>
    </namespaces>
 
+   <constraints>
+      <constraint name="cm:filename" type="REGEX">
+         <parameter name="expression"><![CDATA[[^\"\*\\\>\<\?\/\:\|\¬\£\%\&\+\;]+]]></parameter>
+         <parameter name="requiresMatch">true</parameter>
+      </constraint>
+   </constraints>
    
    <types>
    
@@ -25,6 +31,9 @@
                <title>Name</title>
                <type>d:text</type>
                <mandatory>true</mandatory>
+               <constraints>
+                  <constraint ref="cm:filename" />
+               </constraints>
             </property>
          </properties>
          <mandatory-aspects>

source/java/org/alfresco/repo/dictionary/M2ConstraintDefinition.java

@@ -27,6 +27,7 @@ import org.alfresco.service.namespace.NamespacePrefixResolver;
 import org.alfresco.service.namespace.QName;
 import org.springframework.beans.BeanWrapper;
 import org.springframework.beans.BeanWrapperImpl;
+import org.springframework.beans.TypeMismatchException;
 
 /**
  * Compiled Property Constraint
@@ -35,13 +36,14 @@ import org.springframework.beans.BeanWrapperImpl;
  */
 /*package*/ class M2ConstraintDefinition implements ConstraintDefinition
 {
-    public static final String ERR_CYCLIC_REF = "d_dictionary.model.err.cyclic_ref";
+    public static final String ERR_CYCLIC_REF = "d_dictionary.constraint.err.cyclic_ref";
     public static final String ERR_TYPE_AND_REF = "d_dictionary.constraint.err.type_and_ref";
     public static final String ERR_TYPE_OR_REF = "d_dictionary.constraint.err.type_or_ref";
     public static final String ERR_REF_NOT_FOUND = "d_dictionary.constraint.err.ref_not_found";
     public static final String ERR_ANON_NEEDS_PROPERTY = "d_dictionary.constraint.err.anon_needs_property";
     public static final String ERR_INVALID_TYPE = "d_dictionary.constraint.err.invalid_type";
     public static final String ERR_CONSTRUCT_FAILURE = "d_dictionary.constraint.err.construct_failure";
+    public static final String ERR_PROPERTY_MISMATCH = "d_dictionary.constraint.err.property_mismatch";
 
     private static int anonPropCount = 0;
     
@@ -173,9 +175,16 @@ import org.springframework.beans.BeanWrapperImpl;
             BeanWrapper beanWrapper = new BeanWrapperImpl(constraint);
             List<M2NamedValue> constraintNamedValues = m2Constraint.getParameters();
             for (M2NamedValue namedValue : constraintNamedValues)
+            {
+                try
                 {
                     beanWrapper.setPropertyValue(namedValue.getName(), namedValue.getValue());
                 }
+                catch (TypeMismatchException e)
+                {
+                    throw new DictionaryException(ERR_PROPERTY_MISMATCH, e, namedValue.getName(), shortName);
+                }
+            }
             // now initialize
             constraint.initialize();
         }

source/java/org/alfresco/repo/dictionary/constraint/AbstractConstraint.java

@@ -19,6 +19,7 @@ package org.alfresco.repo.dictionary.constraint;
 import java.util.Collection;
 
 import org.alfresco.service.cmr.dictionary.Constraint;
+import org.alfresco.service.cmr.dictionary.ConstraintException;
 import org.alfresco.service.cmr.dictionary.DictionaryException;
 
 /**
@@ -28,8 +29,8 @@ import org.alfresco.service.cmr.dictionary.DictionaryException;
  */
 public abstract class AbstractConstraint implements Constraint
 {
-    public static final String ERR_PROP_NOT_SET = "d_dictionary.model.err.property_not_set";
+    public static final String ERR_PROP_NOT_SET = "d_dictionary.constraint.err.property_not_set";
-    public static final String ERR_EVALUATE_EXCEPTION = "d_dictionary.model.err.evaluate_exception";
+    public static final String ERR_EVALUATE_EXCEPTION = "d_dictionary.constraint.err.evaluate_exception";
 
     /**
      * @see #evaluateSingleValue(Object)
@@ -51,7 +52,7 @@ public abstract class AbstractConstraint implements Constraint
                 evaluateSingleValue(value);
             }
         }
-        catch (DictionaryException e)
+        catch (ConstraintException e)
         {
             // this can go
             throw e;
@@ -82,7 +83,7 @@ public abstract class AbstractConstraint implements Constraint
      * Support for evaluation of properties.  The value passed in will never be a
      * collection.
      * 
-     * @throws DictionaryException throw this when the evaluation fails
+     * @throws ConstraintException throw this when the evaluation fails
      */
     protected abstract void evaluateSingleValue(Object value);
 }

source/java/org/alfresco/repo/dictionary/constraint/ConstraintsTest.java

@@ -23,9 +23,11 @@ import junit.framework.TestCase;
 
 import org.alfresco.i18n.I18NUtil;
 import org.alfresco.repo.dictionary.DictionaryDAOTest;
-import org.alfresco.service.cmr.dictionary.DictionaryException;
+import org.alfresco.service.cmr.dictionary.ConstraintException;
 
 /**
+ * <b>This file must be saved using UTF-8.</b>
+ * 
  * @see org.alfresco.service.cmr.dictionary.Constraint
  * @see org.alfresco.repo.dictionary.constraint.AbstractConstraint
  * @see org.alfresco.repo.dictionary.constraint.RegexConstraint
@@ -58,7 +60,7 @@ public class ConstraintsTest extends TestCase
             constraint.evaluate(dummyObjects);
             fail("Failed to detected constraint violation in collection");
         }
-        catch (DictionaryException e)
+        catch (ConstraintException e)
         {
             // expected
         }
@@ -70,6 +72,7 @@ public class ConstraintsTest extends TestCase
     {
         RegexConstraint constraint = new RegexConstraint();
         constraint.setExpression("[A-Z]*");
+        constraint.setRequiresMatch(true);
         constraint.initialize();
         
         // do some successful stuff
@@ -82,7 +85,7 @@ public class ConstraintsTest extends TestCase
             constraint.evaluate("abc");
             fail("Regular expression evaluation should have failed: abc");
         }
-        catch (DictionaryException e)
+        catch (ConstraintException e)
         {
             String msg = e.getMessage();
             assertFalse("I18N of constraint message failed", msg.startsWith("d_dictionary.constraint"));
@@ -96,9 +99,56 @@ public class ConstraintsTest extends TestCase
             constraint.evaluate(DummyEnum.abc);
             fail("Regular expression evaluation should have failed for enum: " + DummyEnum.abc);
         }
-        catch (DictionaryException e)
+        catch (ConstraintException e)
         {
         }
+        
+        // now switch the requiresMatch around
+        constraint.setRequiresMatch(false);
+        constraint.initialize();
+        
+        constraint.evaluate(DummyEnum.abc);
+    }
+    
+    public void testRegexConstraintFilename() throws Exception
+    {
+        // we assume UTF-8
+        String expression = "[^\\\"\\*\\\\\\>\\<\\?\\/\\:\\|\\¬\\£\\%\\&\\+\\;]+";
+        String invalidChars = "\"*\\><?/:|£%&+;";
+        
+        RegexConstraint constraint = new RegexConstraint();
+        constraint.setExpression(expression);
+        constraint.setRequiresMatch(true);
+        constraint.initialize();
+        
+        // check that all the invalid chars cause failures
+        for (int i = 0; i < invalidChars.length(); i++)
+        {
+            String invalidStr = invalidChars.substring(i, i+1);
+            try
+            {
+                constraint.evaluate(invalidStr);
+                fail("Regex constraint failed to detect illegal characters: \n" +
+                        "   constraint: " + constraint + "\n" +
+                        "   invalid string: " + invalidStr);
+            }
+            catch (ConstraintException e)
+            {
+                // expected
+            }
+        }
+        // check a bogus filename
+        try
+        {
+            constraint.evaluate("Bogus<>.txt");
+            fail("Failed to detect invalid filename");
+        }
+        catch (ConstraintException e)
+        {
+            // expected
+        }
+        // ... and a valid one
+        constraint.evaluate("Company Home");
     }
     
     private enum DummyEnum
@@ -135,7 +185,7 @@ public class ConstraintsTest extends TestCase
             }
             else
             {
-                throw new DictionaryException("Non-String value");
+                throw new ConstraintException("Non-String value");
             }
         }
     }

source/java/org/alfresco/repo/dictionary/constraint/RegexConstraint.java

@@ -16,7 +16,10 @@
  */
 package org.alfresco.repo.dictionary.constraint;
 
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
 
+import org.alfresco.service.cmr.dictionary.ConstraintException;
 import org.alfresco.service.cmr.dictionary.DictionaryException;
 import org.alfresco.service.cmr.repository.datatype.DefaultTypeConverter;
 
@@ -25,16 +28,35 @@ import org.alfresco.service.cmr.repository.datatype.DefaultTypeConverter;
  * Where possible, the {@link org.alfresco.service.cmr.repository.datatype.DefaultTypeConverter type converter}
  * will be used to first convert the value to a <code>String</code>, so the evaluation
  * will be against the value's <code>String</code> equivalent.
+ * <p>
+ * The failure condition can be changed to occur either on a match or on a non-match by using
+ * the {@link #setRequiresMatch(boolean) requiresMatch} property.  The default is <tt>true</tt>, i.e.
+ * failures will occur if the object value does not match the given expression.
  * 
  * @see java.lang.String#matches(java.lang.String)
+ * @see java.util.regex.Pattern
  * 
  * @author Derek Hulley
  */
 public class RegexConstraint extends AbstractConstraint
 {
     public static final String CONSTRAINT_REGEX_NO_MATCH = "d_dictionary.constraint.regex.no_match";
+    public static final String CONSTRAINT_REGEX_MATCH = "d_dictionary.constraint.regex.match";
 
     private String expression;
+    private Pattern patternMatcher;
+    private boolean requiresMatch = true;
+    
+    @Override
+    public String toString()
+    {
+        StringBuilder sb = new StringBuilder(80);
+        sb.append("RegexConstraint")
+          .append("[ expression=").append(expression)
+          .append(", requiresMatch=").append(requiresMatch)
+          .append("]");
+        return sb.toString();
+    }
 
     /**
      * Set the regular expression used to evaluate string values
@@ -45,22 +67,36 @@ public class RegexConstraint extends AbstractConstraint
         this.expression = expression;
     }
     
+    public void setRequiresMatch(boolean requiresMatch)
+    {
+        this.requiresMatch = requiresMatch;
+    }
+
     public void initialize()
     {
         if (expression == null)
         {
             throw new DictionaryException(AbstractConstraint.ERR_PROP_NOT_SET, "expression");
         }
+        this.patternMatcher = Pattern.compile(expression);
     }
 
-    public void evaluateSingleValue(Object value)
+    protected void evaluateSingleValue(Object value)
     {
         // convert the value to a String
         String valueStr = DefaultTypeConverter.INSTANCE.convert(String.class, value);
-        boolean matches = valueStr.matches(expression);
+        Matcher matcher = patternMatcher.matcher(valueStr);
-        if (!matches)
+        boolean matches = matcher.matches();
+        if (matches != requiresMatch)
         {
-            throw new DictionaryException(RegexConstraint.CONSTRAINT_REGEX_NO_MATCH, value, expression);
+            if (requiresMatch)
+            {
+                throw new ConstraintException(RegexConstraint.CONSTRAINT_REGEX_NO_MATCH, value, expression);
+            }
+            else
+            {
+                throw new ConstraintException(RegexConstraint.CONSTRAINT_REGEX_MATCH, value, expression);
+            }
         }
     }
 }

source/java/org/alfresco/repo/dictionary/dictionarydaotest_model.xml

@@ -25,6 +25,7 @@
    <constraints>
       <constraint name="test:regex1" type="REGEX">
          <parameter name="expression">[A-Z]*</parameter>
+         <parameter name="requiresMatch">false</parameter>
       </constraint>
    </constraints>
 

source/java/org/alfresco/repo/node/integrity/PropertiesIntegrityEvent.java

@@ -23,10 +23,12 @@ import java.util.Map;
 import java.util.Set;
 
 import org.alfresco.service.cmr.dictionary.AspectDefinition;
+import org.alfresco.service.cmr.dictionary.Constraint;
+import org.alfresco.service.cmr.dictionary.ConstraintDefinition;
+import org.alfresco.service.cmr.dictionary.ConstraintException;
 import org.alfresco.service.cmr.dictionary.DictionaryService;
 import org.alfresco.service.cmr.dictionary.PropertyDefinition;
 import org.alfresco.service.cmr.dictionary.TypeDefinition;
-import org.alfresco.service.cmr.repository.InvalidNodeRefException;
 import org.alfresco.service.cmr.repository.NodeRef;
 import org.alfresco.service.cmr.repository.NodeService;
 import org.alfresco.service.namespace.QName;
@@ -52,11 +54,8 @@ public class PropertiesIntegrityEvent extends AbstractIntegrityEvent
     
     public void checkIntegrity(List<IntegrityRecord> eventResults)
     {
-        try
+        NodeRef nodeRef = getNodeRef();
-        {
+        if (!nodeService.exists(nodeRef))
-            checkAllProperties(getNodeRef(), eventResults);
-        }
-        catch (InvalidNodeRefException e)
         {
             // node has gone
             if (logger.isDebugEnabled())
@@ -66,6 +65,10 @@ public class PropertiesIntegrityEvent extends AbstractIntegrityEvent
             eventResults.clear();
             return;
         }
+        else
+        {
+            checkAllProperties(getNodeRef(), eventResults);
+        }
     }
 
     /**
@@ -123,7 +126,6 @@ public class PropertiesIntegrityEvent extends AbstractIntegrityEvent
         for (PropertyDefinition propertyDef : propertyDefs)
         {
             QName propertyQName = propertyDef.getName();
-            Serializable propertyValue = nodeProperties.get(propertyQName);
             // check that mandatory properties are set
             if (propertyDef.isMandatory() && !nodeProperties.containsKey(propertyQName))
             {
@@ -136,7 +138,30 @@ public class PropertiesIntegrityEvent extends AbstractIntegrityEvent
                 // next one
                 continue;
             }
-            // TODO: Incorporate value constraint checks - JIRA AR166
+            Serializable propertyValue = nodeProperties.get(propertyQName);
+            // check constraints
+            List<ConstraintDefinition> constraintDefs = propertyDef.getConstraints();
+            for (ConstraintDefinition constraintDef : constraintDefs)
+            {
+                // get the constraint implementation
+                Constraint constraint = constraintDef.getConstraint();
+                try
+                {
+                    constraint.evaluate(propertyValue);
+                }
+                catch (ConstraintException e)
+                {
+                    IntegrityRecord result = new IntegrityRecord(
+                            "Invalid property value: \n" +
+                            "   Node: " + nodeRef + "\n" +
+                            "   Type: " + typeQName + "\n" +
+                            "   Property: " + propertyQName + "\n" +
+                            "   Constraint: " + e.getMessage());
+                    eventResults.add(result);
+                    // next one
+                    continue;
+                }
+            }
         }
     }
 }

source/java/org/alfresco/service/cmr/dictionary/DictionaryException.java

@@ -27,14 +27,14 @@ public class DictionaryException extends AlfrescoRuntimeException
 {
     private static final long serialVersionUID = 3257008761007847733L;
 
-    public DictionaryException(String msg)
+    public DictionaryException(String msgId)
     {
-       super(msg);
+       super(msgId);
     }
     
-    public DictionaryException(String msg, Throwable cause)
+    public DictionaryException(String msgId, Throwable cause)
     {
-       super(msg, cause);
+       super(msgId, cause);
     }
 
     public DictionaryException(String msgId, Object ... args)