diff --git a/source/java/org/alfresco/util/schemacomp/ComparisonUtils.java b/source/java/org/alfresco/util/schemacomp/ComparisonUtils.java index 0791d80c20..fa881e7101 100644 --- a/source/java/org/alfresco/util/schemacomp/ComparisonUtils.java +++ b/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 objects, final DbObject objToFind); + List findEquivalentObjects(Schema schema, DbObject objToMatch); + void compareSimpleCollections(DbProperty leftProperty, DbProperty rightProperty, DiffContext ctx, Strength strength); diff --git a/source/java/org/alfresco/util/schemacomp/DefaultComparisonUtils.java b/source/java/org/alfresco/util/schemacomp/DefaultComparisonUtils.java index c9c2290883..e3114dabc5 100644 --- a/source/java/org/alfresco/util/schemacomp/DefaultComparisonUtils.java +++ b/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 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 matches = new ArrayList(); + 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 getMatches() + { + return this.matches; + } + } } diff --git a/source/java/org/alfresco/util/schemacomp/DefaultComparisonUtilsTest.java b/source/java/org/alfresco/util/schemacomp/DefaultComparisonUtilsTest.java index 4797e8fd9d..7abf4b7db1 100644 --- a/source/java/org/alfresco/util/schemacomp/DefaultComparisonUtilsTest.java +++ b/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()); + ctx = new DiffContext(dialect, differences, new ArrayList(), null, null); } @Test diff --git a/source/java/org/alfresco/util/schemacomp/DiffContext.java b/source/java/org/alfresco/util/schemacomp/DiffContext.java index 07db61112f..c861befcb9 100644 --- a/source/java/org/alfresco/util/schemacomp/DiffContext.java +++ b/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 validationResults; + private final Schema referenceSchema; + private final Schema targetSchema; /** * @param dialect * @param differences */ - public DiffContext(Dialect dialect, Results differences, List validationResults) + public DiffContext(Dialect dialect, Results differences, List 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; + } } diff --git a/source/java/org/alfresco/util/schemacomp/SchemaComparator.java b/source/java/org/alfresco/util/schemacomp/SchemaComparator.java index 372e7be661..1e588feec6 100644 --- a/source/java/org/alfresco/util/schemacomp/SchemaComparator.java +++ b/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()); + this.ctx = new DiffContext(dialect, new Results(), new ArrayList(), leftSchema, rightSchema); } diff --git a/source/java/org/alfresco/util/schemacomp/ValidatingVisitor.java b/source/java/org/alfresco/util/schemacomp/ValidatingVisitor.java index 7f2e697306..9f07d13385 100644 --- a/source/java/org/alfresco/util/schemacomp/ValidatingVisitor.java +++ b/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()); - validator.validate(dbObject, ctx); + DbValidator validator = getValidatorFor(referenceObj.getClass()); + List 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); + } } } diff --git a/source/java/org/alfresco/util/schemacomp/ValidatingVisitorTest.java b/source/java/org/alfresco/util/schemacomp/ValidatingVisitorTest.java index 9d7db3130d..60e05fcaef 100644 --- a/source/java/org/alfresco/util/schemacomp/ValidatingVisitorTest.java +++ b/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()); + index = new Index(table, "index_name", new ArrayList()); + ctx = new DiffContext(new MySQL5InnoDBDialect(), new Results(), + new ArrayList(), 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()); + 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); } } diff --git a/source/java/org/alfresco/util/schemacomp/model/AbstractDbObjectTest.java b/source/java/org/alfresco/util/schemacomp/model/AbstractDbObjectTest.java index f23fc0d507..0832c767f5 100644 --- a/source/java/org/alfresco/util/schemacomp/model/AbstractDbObjectTest.java +++ b/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()); + ctx = new DiffContext(dialect, differences, new ArrayList(), null, null); } @Test diff --git a/source/java/org/alfresco/util/schemacomp/model/DbObjectTestBase.java b/source/java/org/alfresco/util/schemacomp/model/DbObjectTestBase.java index df2c805fde..c4e594957d 100644 --- a/source/java/org/alfresco/util/schemacomp/model/DbObjectTestBase.java +++ b/source/java/org/alfresco/util/schemacomp/model/DbObjectTestBase.java @@ -61,7 +61,7 @@ public abstract class DbObjectTestBase // Check that the correct calls happened in the correct order. List mocks = getMocksUsedInDiff(); inOrder = inOrder(mocks.toArray()); - ctx = new DiffContext(dialect, differences, new ArrayList()); + ctx = new DiffContext(dialect, differences, new ArrayList(), null, null); } diff --git a/source/java/org/alfresco/util/schemacomp/model/Index.java b/source/java/org/alfresco/util/schemacomp/model/Index.java index c3c949eb19..99c4acc4c0 100644 --- a/source/java/org/alfresco/util/schemacomp/model/Index.java +++ b/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 true; - } - else - { - return columnNames.equals(other.getColumnNames()); - } + return false; } + + // 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; diff --git a/source/java/org/alfresco/util/schemacomp/model/IndexTest.java b/source/java/org/alfresco/util/schemacomp/model/IndexTest.java index b21d4de1f6..47e8168f3a 100644 --- a/source/java/org/alfresco/util/schemacomp/model/IndexTest.java +++ b/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 { + 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")); - thatIndex = new Index(null, "that_index", Arrays.asList("a", "b")); + thisTable = new Table("this_table"); + 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 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")))); } diff --git a/source/java/org/alfresco/util/schemacomp/model/Table.java b/source/java/org/alfresco/util/schemacomp/model/Table.java index 159552b469..f6fab0d998 100644 --- a/source/java/org/alfresco/util/schemacomp/model/Table.java +++ b/source/java/org/alfresco/util/schemacomp/model/Table.java @@ -220,8 +220,11 @@ public class Table extends AbstractDbObject private List getChildren() { List children = new ArrayList(); - children.addAll(columns); - children.add(primaryKey); + children.addAll(columns); + if (primaryKey != null) + { + children.add(primaryKey); + } children.addAll(foreignKeys); children.addAll(indexes); return children; diff --git a/source/java/org/alfresco/util/schemacomp/validator/DbValidator.java b/source/java/org/alfresco/util/schemacomp/validator/DbValidator.java index 4c60b4a28e..7bfe1025a7 100644 --- a/source/java/org/alfresco/util/schemacomp/validator/DbValidator.java +++ b/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); } diff --git a/source/java/org/alfresco/util/schemacomp/validator/NameValidator.java b/source/java/org/alfresco/util/schemacomp/validator/NameValidator.java index 171086083a..7e2c637ed0 100644 --- a/source/java/org/alfresco/util/schemacomp/validator/NameValidator.java +++ b/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, Pattern> namePatterns = new HashMap, Pattern>(); - private Pattern defaultNamePattern; + private Pattern pattern; @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(dbo, "name")); + ValidationResult result = new ValidationResult(new DbProperty(target, "name")); if (pattern != null && !pattern.matcher(name).matches()) { ctx.getValidationResults().add(result); } - else if (defaultNamePattern != null && !defaultNamePattern.matcher(name).matches()) - { - ctx.getValidationResults().add(result); - } } - /** - * Specify the set of mappings of database dialect to acceptable name patterns. - * - * @param namePatterns - */ - public void setNamePatterns(Map, Pattern> namePatterns) + public void setPattern(Pattern pattern) { - this.namePatterns = namePatterns; - } - - /** - * 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. - *

- * If defaultNamePattern is null then a validation failure will be produced. - * - * @param defaultNamePattern - */ - public void setDefaultNamePattern(Pattern defaultNamePattern) - { - this.defaultNamePattern = defaultNamePattern; + this.pattern = pattern; } } diff --git a/source/java/org/alfresco/util/schemacomp/validator/NameValidatorTest.java b/source/java/org/alfresco/util/schemacomp/validator/NameValidatorTest.java index c731fc506a..3ff29a1410 100644 --- a/source/java/org/alfresco/util/schemacomp/validator/NameValidatorTest.java +++ b/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(); - 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.validate(indexForName("SYS_MYINDEX"), ctx); - validator.validate(indexForName("SYS_"), ctx); - validator.validate(indexForName("SYS_MY_INDEX"), ctx); - validator.validate(indexForName("MY_INDEX"), ctx); + validator.setPattern(Pattern.compile("SYS_[A-Z_]+")); + validator.validate(null, indexForName("SYS_MYINDEX"), ctx); + validator.validate(null, indexForName("SYS_"), ctx); + validator.validate(null, indexForName("SYS_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, Pattern> patterns = new HashMap, Pattern>(); - patterns.put(Oracle10gDialect.class, Pattern.compile("ORA_[A-Z_]+")); - validator.setNamePatterns(patterns); + validator.setPattern(Pattern.compile("ORA_[A-Z_]+")); - validator.validate(indexForName("ORA_MYINDEX"), ctx); - validator.validate(indexForName("SYS_MYINDEX"), ctx); + validator.validate(null, indexForName("ORA_MYINDEX"), ctx); + validator.validate(null, indexForName("SYS_MYINDEX"), ctx); assertEquals(1, validationResults.size()); assertEquals("SYS_MYINDEX", validationResults.get(0).getValue()); diff --git a/source/java/org/alfresco/util/schemacomp/validator/NullValidator.java b/source/java/org/alfresco/util/schemacomp/validator/NullValidator.java index aacdec8167..c9c17c9a00 100644 --- a/source/java/org/alfresco/util/schemacomp/validator/NullValidator.java +++ b/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 }