ALF-10771: schema validation and differences rules

Laying some groundwork for the main code.


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

source/java/org/alfresco/util/schemacomp/ComparisonUtils.java

@@ -20,9 +20,11 @@
 package org.alfresco.util.schemacomp;
 
 import java.util.Collection;
+import java.util.List;
 
 import org.alfresco.util.schemacomp.Result.Strength;
 import org.alfresco.util.schemacomp.model.DbObject;
+import org.alfresco.util.schemacomp.model.Schema;
 
 /**
  * Utilities for comparing data structures in the context of comparing two database schemas.
@@ -32,11 +34,14 @@ import org.alfresco.util.schemacomp.model.DbObject;
 public interface ComparisonUtils
 {
     /**
+     * @deprecated This method ignores the fact that multiple objects may match.
      * @param objToFind
      * @return
      */
     DbObject findSameObjectAs(Collection<? extends DbObject> objects, final DbObject objToFind);
 
+    List<DbObject> findEquivalentObjects(Schema schema, DbObject objToMatch);
+    
     void compareSimpleCollections(DbProperty leftProperty, DbProperty rightProperty,
                 DiffContext ctx, Strength strength);
 

source/java/org/alfresco/util/schemacomp/DefaultComparisonUtils.java

@@ -21,10 +21,12 @@ package org.alfresco.util.schemacomp;
 
 import java.util.ArrayList;
 import java.util.Collection;
+import java.util.List;
 
-import org.alfresco.util.schemacomp.Result.Strength;
 import org.alfresco.util.schemacomp.Difference.Where;
+import org.alfresco.util.schemacomp.Result.Strength;
 import org.alfresco.util.schemacomp.model.DbObject;
+import org.alfresco.util.schemacomp.model.Schema;
 import org.apache.commons.collections.CollectionUtils;
 import org.apache.commons.collections.Predicate;
 
@@ -54,6 +56,18 @@ public class DefaultComparisonUtils implements ComparisonUtils
     }
 
     
+    
+    @Override
+    public List<DbObject> findEquivalentObjects(Schema schema, DbObject objToMatch)
+    {
+        EquivalentObjectSeeker objectSeeker = new EquivalentObjectSeeker(objToMatch);
+        schema.accept(objectSeeker);
+        
+        return objectSeeker.getMatches();
+    }
+
+
+
     @Override
     public void compareSimpleCollections(DbProperty leftProp,
                 DbProperty rightProp, DiffContext ctx, Strength strength)
@@ -235,4 +249,33 @@ public class DefaultComparisonUtils implements ComparisonUtils
                         "Property value is a DbObject - this method shouldn't be used to compare this type: " + obj);
         }
     }
+    
+    
+    private static class EquivalentObjectSeeker implements DbObjectVisitor
+    {
+        private final List<DbObject> matches = new ArrayList<DbObject>();
+        private final DbObject objToMatch;
+        
+        public EquivalentObjectSeeker(DbObject objToMatch)
+        {
+            this.objToMatch = objToMatch;
+        }
+        
+        @Override
+        public void visit(DbObject dbObject)
+        {
+            if (objToMatch.sameAs(dbObject))
+            {
+                matches.add(dbObject);
+            }
+        }
+
+        /**
+         * @return the matches
+         */
+        public List<DbObject> getMatches()
+        {
+            return this.matches;
+        }
+    }
 }

source/java/org/alfresco/util/schemacomp/DefaultComparisonUtilsTest.java

@@ -58,7 +58,7 @@ public class DefaultComparisonUtilsTest
     public void setUp()
     {
         comparisonUtils = new DefaultComparisonUtils();
-        ctx = new DiffContext(dialect, differences, new ArrayList<ValidationResult>());
+        ctx = new DiffContext(dialect, differences, new ArrayList<ValidationResult>(), null, null);
     }
     
     @Test

source/java/org/alfresco/util/schemacomp/DiffContext.java

@@ -20,6 +20,7 @@ package org.alfresco.util.schemacomp;
 
 import java.util.List;
 
+import org.alfresco.util.schemacomp.model.Schema;
 import org.hibernate.dialect.Dialect;
 
 /**
@@ -34,16 +35,21 @@ public class DiffContext
     private final Dialect dialect;
     private final Results differences;
     private final List<ValidationResult> validationResults;
+    private final Schema referenceSchema;
+    private final Schema targetSchema;
     
     /**
      * @param dialect
      * @param differences
      */
-    public DiffContext(Dialect dialect, Results differences, List<ValidationResult> validationResults)
+    public DiffContext(Dialect dialect, Results differences, List<ValidationResult> validationResults,
+                Schema referenceSchema, Schema targetSchema)
     {
         this.dialect = dialect;
         this.differences = differences;
         this.validationResults = validationResults;
+        this.referenceSchema = referenceSchema;
+        this.targetSchema = targetSchema;
     }
 
     /**
@@ -69,6 +75,20 @@ public class DiffContext
     {
         return this.validationResults;
     }
-    
+
-    
+    /**
+     * @return the referenceSchema
+     */
+    public Schema getReferenceSchema()
+    {
+        return this.referenceSchema;
+    }
+
+    /**
+     * @return the targetSchema
+     */
+    public Schema getTargetSchema()
+    {
+        return this.targetSchema;
+    }
 }

source/java/org/alfresco/util/schemacomp/SchemaComparator.java

@@ -47,7 +47,7 @@ public class SchemaComparator
     {
         this.leftSchema = left;
         this.rightSchema = right;
-        this.ctx = new DiffContext(dialect, new Results(), new ArrayList<ValidationResult>());
+        this.ctx = new DiffContext(dialect, new Results(), new ArrayList<ValidationResult>(), leftSchema, rightSchema);
     }
     
     

source/java/org/alfresco/util/schemacomp/ValidatingVisitor.java

@@ -18,6 +18,9 @@
  */
 package org.alfresco.util.schemacomp;
 
+import java.util.ArrayList;
+import java.util.List;
+
 import org.alfresco.util.schemacomp.model.DbObject;
 import org.alfresco.util.schemacomp.model.Index;
 import org.alfresco.util.schemacomp.validator.DbValidator;
@@ -33,7 +36,8 @@ public class ValidatingVisitor implements DbObjectVisitor
 {
     private DiffContext ctx;
     protected NameValidator indexNameValidator = new NameValidator();
-    protected NullValidator nullValidator = new NullValidator();
+    protected NullValidator defaultValidator = new NullValidator();
+    protected ComparisonUtils comparisonUtils = new DefaultComparisonUtils();
 
     public ValidatingVisitor(DiffContext ctx)
     {
@@ -49,14 +53,21 @@ public class ValidatingVisitor implements DbObjectVisitor
         }
         else
         {
-            return nullValidator;
+            return defaultValidator;
         }
     }
 
     @Override
-    public void visit(DbObject dbObject)
+    public void visit(DbObject referenceObj)
     {
-        DbValidator validator = getValidatorFor(dbObject.getClass());
+        DbValidator validator = getValidatorFor(referenceObj.getClass());
-        validator.validate(dbObject, ctx);
+        List<DbObject> matches = comparisonUtils.findEquivalentObjects(ctx.getTargetSchema(), referenceObj);
+        
+        // TODO: if matches.size() > 1 then warn of possible redundant database objects
+        
+        for (DbObject target : matches)
+        {
+            validator.validate(referenceObj, target, ctx);            
+        }
     }
 }

source/java/org/alfresco/util/schemacomp/ValidatingVisitorTest.java

@@ -22,8 +22,10 @@ package org.alfresco.util.schemacomp;
 import static org.junit.Assert.assertSame;
 
 import java.util.ArrayList;
+import java.util.Arrays;
 
 import org.alfresco.util.schemacomp.model.Column;
+import org.alfresco.util.schemacomp.model.DbObject;
 import org.alfresco.util.schemacomp.model.ForeignKey;
 import org.alfresco.util.schemacomp.model.Index;
 import org.alfresco.util.schemacomp.model.PrimaryKey;
@@ -46,11 +48,17 @@ public class ValidatingVisitorTest
 {
     private DiffContext ctx;
     private ValidatingVisitor visitor;
+    private Table table;
+    private Schema refSchema;
+    private Schema targetSchema;
+    private Index index;
     
     @Before
     public void setUp() throws Exception
     {
-        ctx = new DiffContext(new MySQL5InnoDBDialect(), new Results(), new ArrayList<ValidationResult>());
+        index = new Index(table, "index_name", new ArrayList<String>());
+        ctx = new DiffContext(new MySQL5InnoDBDialect(), new Results(),
+                    new ArrayList<ValidationResult>(), refSchema, targetSchema);
         visitor = new ValidatingVisitor(ctx);
     }
 
@@ -58,7 +66,7 @@ public class ValidatingVisitorTest
     public void canGetCorrectValidator()
     {
        // Get references to the validator instances to test for
-       DbValidator nullValidator = visitor.nullValidator;
+       DbValidator nullValidator = visitor.defaultValidator;
        DbValidator nameValidator = visitor.indexNameValidator;
        
        assertSame(nullValidator, visitor.getValidatorFor(Column.class)); 
@@ -74,10 +82,13 @@ public class ValidatingVisitorTest
     public void canValidate()
     {
         visitor.indexNameValidator = Mockito.mock(NameValidator.class);
-        Index index = new Index(null, "index_name", new ArrayList<String>());
+        visitor.comparisonUtils = Mockito.mock(ComparisonUtils.class);
+        Mockito.when(visitor.comparisonUtils.findEquivalentObjects(refSchema, index)).
+            thenReturn(Arrays.asList((DbObject)index));
         
+        // Validate all instances of the target schema's indexes that are equivalent to this index
         visitor.visit(index);
         
-        Mockito.verify(visitor.indexNameValidator).validate(index, ctx);
+        Mockito.verify(visitor.indexNameValidator).validate(index, index, ctx);
     }
 }

source/java/org/alfresco/util/schemacomp/model/AbstractDbObjectTest.java

@@ -60,7 +60,7 @@ public class AbstractDbObjectTest
     public void setUp() throws Exception
     {
         dbObject = new ConcreteDbObject("the_object");
-        ctx = new DiffContext(dialect, differences, new ArrayList<ValidationResult>());
+        ctx = new DiffContext(dialect, differences, new ArrayList<ValidationResult>(), null, null);
     }
 
     @Test

source/java/org/alfresco/util/schemacomp/model/DbObjectTestBase.java

@@ -61,7 +61,7 @@ public abstract class DbObjectTestBase<T extends AbstractDbObject>
         // Check that the correct calls happened in the correct order.
         List<Object> mocks = getMocksUsedInDiff();
         inOrder = inOrder(mocks.toArray());
-        ctx = new DiffContext(dialect, differences, new ArrayList<ValidationResult>());
+        ctx = new DiffContext(dialect, differences, new ArrayList<ValidationResult>(), null, null);
     }
     
     

source/java/org/alfresco/util/schemacomp/model/Index.java

@@ -122,17 +122,24 @@ public class Index extends AbstractDbObject
         {
             Index other = (Index) o;
             
-            if (getName() != null)
+            // An index can only be 'the same' if it belongs to the correct table.
+            if (!getParent().sameAs(other.getParent()))
             {
-                if (getName().equals(other.getName()))
+                return false;
-                {
-                    return true;
-                }
-                else
-                {
-                    return columnNames.equals(other.getColumnNames());
-                }
             }
+            
+            // If it has the same name, then it is intended to be the same index
+            if (getName() != null && getName().equals(other.getName()))
+            {
+                return true;
+            }
+            else
+            {
+                // The name may be different, but if it has the same parent table (see above)
+                // and indexes the same columns, then it is the same index.
+                return columnNames.equals(other.getColumnNames());
+            }
+        
         }
         
         return false;

source/java/org/alfresco/util/schemacomp/model/IndexTest.java

@@ -18,14 +18,17 @@
  */
 package org.alfresco.util.schemacomp.model;
 
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+import static org.mockito.Mockito.verify;
+
 import java.util.Arrays;
 
 import org.alfresco.util.schemacomp.DbProperty;
 import org.alfresco.util.schemacomp.Result.Strength;
 import org.junit.Before;
 import org.junit.Test;
-import static org.junit.Assert.*;
-import static org.mockito.Mockito.verify;
 
 
 /**
@@ -34,14 +37,18 @@ import static org.mockito.Mockito.verify;
  */
 public class IndexTest extends DbObjectTestBase<Index>
 {
+    private Table thisTable;
     private Index thisIndex;
+    private Table thatTable;
     private Index thatIndex;
 
     @Before
     public void setUp()
     {
-        thisIndex = new Index(null, "this_index", Arrays.asList("id", "name", "age"));
+        thisTable = new Table("this_table");
-        thatIndex = new Index(null, "that_index", Arrays.asList("a", "b"));        
+        thisIndex = new Index(thisTable, "this_index", Arrays.asList("id", "name", "age"));
+        thatTable = new Table("that_table");
+        thatIndex = new Index(thatTable, "that_index", Arrays.asList("a", "b"));        
     }
     
     @Override
@@ -82,19 +89,23 @@ public class IndexTest extends DbObjectTestBase<Index>
     public void sameAs()
     {
        assertTrue("Indexes should be logically the same.",
-                   thisIndex.sameAs(new Index(null, "this_index", Arrays.asList("id", "name", "age"))));
+                   thisIndex.sameAs(new Index(thisTable, "this_index", Arrays.asList("id", "name", "age"))));
+       
+       assertFalse("Indexes have logically different parents",
+                   thisIndex.sameAs(new Index(thatTable, "this_index", Arrays.asList("id", "name", "age"))));
+       
        
        assertTrue("Indexes should be logically the same, despite different names (as same column order)",
-                   thisIndex.sameAs(new Index(null, "different_name", Arrays.asList("id", "name", "age")))); 
+                   thisIndex.sameAs(new Index(thisTable, "different_name", Arrays.asList("id", "name", "age")))); 
 
        assertTrue("Indexes should be identified as the same despite different column order (as same name).",
-                   thisIndex.sameAs(new Index(null, "this_index", Arrays.asList("name", "id", "age")))); 
+                   thisIndex.sameAs(new Index(thisTable, "this_index", Arrays.asList("name", "id", "age")))); 
 
        assertFalse("Indexes should be identified different (different name and column order)",
-                   thisIndex.sameAs(new Index(null, "different_name", Arrays.asList("name", "id", "age")))); 
+                   thisIndex.sameAs(new Index(thisTable, "different_name", Arrays.asList("name", "id", "age")))); 
        
        assertFalse("Indexes should be identified different (different name & different columns)",
-                   thisIndex.sameAs(new Index(null, "different_name", Arrays.asList("node_ref", "url"))));
+                   thisIndex.sameAs(new Index(thisTable, "different_name", Arrays.asList("node_ref", "url"))));
     }
     
     

source/java/org/alfresco/util/schemacomp/model/Table.java

@@ -220,8 +220,11 @@ public class Table extends AbstractDbObject
     private List<DbObject> getChildren()
     {
         List<DbObject> children = new ArrayList<DbObject>();
-        children.addAll(columns); 
+        children.addAll(columns);
-        children.add(primaryKey);
+        if (primaryKey != null)
+        {
+            children.add(primaryKey);
+        }
         children.addAll(foreignKeys);
         children.addAll(indexes);
         return children;

source/java/org/alfresco/util/schemacomp/validator/DbValidator.java

@@ -29,5 +29,5 @@ import org.alfresco.util.schemacomp.model.DbObject;
  */
 public interface DbValidator
 {
-    void validate(DbObject dbo, DiffContext ctx);
+    void validate(DbObject reference, DbObject target, DiffContext ctx);
 }

source/java/org/alfresco/util/schemacomp/validator/NameValidator.java

@@ -38,49 +38,24 @@ import org.hibernate.dialect.Dialect;
  */
 public class NameValidator implements DbValidator
 {
-    private Map<Class<? extends Dialect>, Pattern> namePatterns = new HashMap<Class<? extends Dialect>, Pattern>();
+    private Pattern pattern;
-    private Pattern defaultNamePattern;
     
     @Override
-    public void validate(DbObject dbo, DiffContext ctx)
+    public void validate(DbObject reference, DbObject target, DiffContext ctx)
     {
-        String name = dbo.getName();
+        String name = target.getName();
         
-        Pattern pattern = namePatterns.get(ctx.getDialect().getClass());
+        ValidationResult result = new ValidationResult(new DbProperty(target, "name"));
-        
-        ValidationResult result = new ValidationResult(new DbProperty(dbo, "name"));
         
         if (pattern != null && !pattern.matcher(name).matches())
         {
             ctx.getValidationResults().add(result);
         }
-        else if (defaultNamePattern != null && !defaultNamePattern.matcher(name).matches())
-        {   
-            ctx.getValidationResults().add(result);
-        }
     }
 
 
-    /**
+    public void setPattern(Pattern pattern)
-     * Specify the set of mappings of database dialect to acceptable name patterns.
-     * 
-     * @param namePatterns
-     */
-    public void setNamePatterns(Map<Class<? extends Dialect>, Pattern> namePatterns)
     {
-        this.namePatterns = namePatterns; 
+        this.pattern = pattern; 
-    }
-    
-    /**
-     * If during validation, there is no specific name validation pattern for the supplied {@link Dialect}
-     * then the defaultNamePattern property will be used - if not null.
-     * <p>
-     * If defaultNamePattern is null then a validation failure will be produced.
-     * 
-     * @param defaultNamePattern
-     */
-    public void setDefaultNamePattern(Pattern defaultNamePattern)
-    {
-        this.defaultNamePattern = defaultNamePattern;
     }
 }

source/java/org/alfresco/util/schemacomp/validator/NameValidatorTest.java

@@ -22,9 +22,7 @@ package org.alfresco.util.schemacomp.validator;
 import static org.junit.Assert.assertEquals;
 
 import java.util.ArrayList;
-import java.util.HashMap;
 import java.util.List;
-import java.util.Map;
 import java.util.regex.Pattern;
 
 import org.alfresco.util.schemacomp.DiffContext;
@@ -32,7 +30,6 @@ import org.alfresco.util.schemacomp.Results;
 import org.alfresco.util.schemacomp.ValidationResult;
 import org.alfresco.util.schemacomp.model.DbObject;
 import org.alfresco.util.schemacomp.model.Index;
-import org.hibernate.dialect.Dialect;
 import org.hibernate.dialect.Oracle10gDialect;
 import org.junit.Before;
 import org.junit.Test;
@@ -53,17 +50,17 @@ public class NameValidatorTest
     {
         validator = new NameValidator();
         validationResults = new ArrayList<ValidationResult>();
-        ctx = new DiffContext(new Oracle10gDialect(), new Results(), validationResults);
+        ctx = new DiffContext(new Oracle10gDialect(), new Results(), validationResults, null, null);
     }
 
     @Test
     public void canSpecifyDefaultRequiredPattern()
     {
-        validator.setDefaultNamePattern(Pattern.compile("SYS_[A-Z_]+"));
+        validator.setPattern(Pattern.compile("SYS_[A-Z_]+"));
-        validator.validate(indexForName("SYS_MYINDEX"), ctx);
+        validator.validate(null, indexForName("SYS_MYINDEX"), ctx);
-        validator.validate(indexForName("SYS_"), ctx);
+        validator.validate(null, indexForName("SYS_"), ctx);
-        validator.validate(indexForName("SYS_MY_INDEX"), ctx);
+        validator.validate(null, indexForName("SYS_MY_INDEX"), ctx);
-        validator.validate(indexForName("MY_INDEX"), ctx);
+        validator.validate(null, indexForName("MY_INDEX"), ctx);
         
         assertEquals(2, validationResults.size());
         assertEquals("SYS_", validationResults.get(0).getValue());
@@ -73,12 +70,10 @@ public class NameValidatorTest
     @Test
     public void canValidateAgainstPatternForDialect()
     {
-        Map<Class<? extends Dialect>, Pattern> patterns = new HashMap<Class<? extends Dialect>, Pattern>();
+        validator.setPattern(Pattern.compile("ORA_[A-Z_]+"));
-        patterns.put(Oracle10gDialect.class, Pattern.compile("ORA_[A-Z_]+"));
-        validator.setNamePatterns(patterns);
         
-        validator.validate(indexForName("ORA_MYINDEX"), ctx);
+        validator.validate(null, indexForName("ORA_MYINDEX"), ctx);
-        validator.validate(indexForName("SYS_MYINDEX"), ctx);
+        validator.validate(null, indexForName("SYS_MYINDEX"), ctx);
         
         assertEquals(1, validationResults.size());
         assertEquals("SYS_MYINDEX", validationResults.get(0).getValue());

source/java/org/alfresco/util/schemacomp/validator/NullValidator.java

@@ -29,7 +29,7 @@ import org.alfresco.util.schemacomp.model.DbObject;
 public class NullValidator implements DbValidator
 {
     @Override
-    public void validate(DbObject dbo, DiffContext ctx)
+    public void validate(DbObject reference, DbObject target, DiffContext ctx)
     {
         // Do nothing
     }