[2.0] DDC-169 - Implemented Index and ForeignKey comparison based on properties and not on names.

lib/Doctrine/DBAL/Schema/Comparator.php

@@ -147,14 +147,14 @@ class Comparator
         /* See if all the fields in table 1 exist in table 2 */
         foreach ( $table2->getColumns() as $columnName => $column ) {
             if ( !$table1->hasColumn($columnName) ) {
-                $tableDifferences->addedFields[$columnName] = $column;
+                $tableDifferences->addedColumns[$columnName] = $column;
                 $changes++;
             }
         }
         /* See if there are any removed fields in table 2 */
         foreach ( $table1->getColumns() as $columnName => $column ) {
             if ( !$table2->hasColumn($columnName) ) {
-                $tableDifferences->removedFields[$columnName] = true;
+                $tableDifferences->removedColumns[$columnName] = true;
                 $changes++;
             }
         }
@@ -162,7 +162,7 @@ class Comparator
         foreach ( $table1->getColumns() as $columnName => $column ) {
             if ( $table2->hasColumn($columnName) ) {
                 if ( $this->diffColumn( $column, $table2->getColumn($columnName) ) ) {
-                    $tableDifferences->changedFields[$columnName] = $table2->getColumn($columnName);
+                    $tableDifferences->changedColumns[$columnName] = $table2->getColumn($columnName);
                     $changes++;
                 }
             }
@@ -171,49 +171,59 @@ class Comparator
         $table1Indexes = $table1->getIndexes();
         $table2Indexes = $table2->getIndexes();
 
-        /* See if all the indexes in table 1 exist in table 2 */
+        foreach ($table2Indexes AS $index2Name => $index2Definition) {
-        foreach ( $table2Indexes as $indexName => $indexDefinition ) {
+            foreach ($table1Indexes AS $index1Name => $index1Definition) {
-            if ( !isset( $table1Indexes[$indexName] ) ) {
+                if ($this->diffIndex($index1Definition, $index2Definition) === false) {
-                $tableDifferences->addedIndexes[$indexName] = $indexDefinition;
+                    unset($table1Indexes[$index1Name]);
-                $changes++;
+                    unset($table2Indexes[$index2Name]);
-            }
-        }
-        /* See if there are any removed indexes in table 2 */
-        foreach ( $table1Indexes as $indexName => $indexDefinition ) {
-            if ( !isset( $table2Indexes[$indexName] ) ) {
-                $tableDifferences->removedIndexes[$indexName] = true;
-                $changes++;
-            }
-        }
-        /* See if there are any changed indexDefinitions */
-        foreach ( $table1Indexes as $indexName => $indexDefinition ) {
-            if ( isset( $table2Indexes[$indexName] ) ) {
-                if ( $this->diffIndex( $indexDefinition, $table2Indexes[$indexName] ) ) {
-                    $tableDifferences->changedIndexes[$indexName] = $table2Indexes[$indexName];
-                    $changes++;
-                }
-            }
-        }
-
-        foreach ($table2->getForeignKeys() AS $constraint) {
-            $fkName = $constraint->getName();
-            if (!$table1->hasForeignKey($fkName)) {
-                $tableDifferences->addedForeignKeys[$fkName] = $constraint;
-                $changes++;
                 } else {
-                if ($this->diffForeignKey($constraint, $table1->getForeignKey($fkName))) {
+                    if ($index1Name == $index2Name) {
-                    $tableDifferences->changedForeignKeys[$fkName] = $constraint;
+                        $tableDifferences->changedIndexes[$index2Name] = $table2Indexes[$index2Name];
+                        unset($table1Indexes[$index1Name]);
+                        unset($table2Indexes[$index2Name]);
                         $changes++;
                     }
                 }
             }
+        }
 
-        foreach ($table1->getForeignKeys() AS $constraint) {
+        foreach ($table1Indexes AS $index1Name => $index1Definition) {
-            $fkName = $constraint->getName();
+            $tableDifferences->removedIndexes[$index1Name] = true;
-            if (!$table2->hasForeignKey($fkName)) {
-                $tableDifferences->removedForeignKeys[$fkName] = $constraint;
             $changes++;
         }
+
+        foreach ($table2Indexes AS $index2Name => $index2Definition) {
+            $tableDifferences->addedIndexes[$index2Name] = $index2Definition;
+            $changes++;
+        }
+
+        $fromFkeys = $table1->getForeignKeys();
+        $toFkeys = $table2->getForeignKeys();
+
+        foreach ($fromFkeys AS $key1 => $constraint1) {
+            foreach ($toFkeys AS $key2 => $constraint2) {
+                if($this->diffForeignKey($constraint1, $constraint2) === false) {
+                    unset($fromFkeys[$key1]);
+                    unset($toFkeys[$key2]);
+                } else {
+                    if (strtolower($constraint1->getName()) == strtolower($constraint2->getName())) {
+                        $tableDifferences->changedForeignKeys[] = $constraint2;
+                        $changes++;
+                        unset($fromFkeys[$key1]);
+                        unset($toFkeys[$key2]);
+                    }
+                }
+            }
+        }
+
+        foreach ($fromFkeys AS $key1 => $constraint1) {
+            $tableDifferences->removedForeignKeys[] = $constraint1;
+            $changes++;
+        }
+
+        foreach ($toFkeys AS $key2 => $constraint2) {
+            $tableDifferences->addedForeignKeys[] = $constraint2;
+            $changes++;
         }
 
         return $changes ? $tableDifferences : false;
@@ -234,21 +244,21 @@ class Comparator
             return true;
         }
 
-        if ($key1->hasOption('onUpdate') != $key2->hasOption('onUpdate')) {
+        if ($key1->hasOption('onUpdate') && $key->hasOption('onUpdate')) {
-            return true;
-        }
-
             if ($key1->getOption('onUpdate') != $key2->getOption('onUpdate')) {
                 return true;
             }
-
+        } else if ($key1->hasOption('onUpdate') != $key2->hasOption('onUpdate')) {
-        if ($key1->hasOption('onDelete') != $key2->hasOption('onDelete')) {
             return true;
         }
 
+        if ($key1->hasOption('onDelete') && $key2->hasOption('onDelete')) {
             if ($key1->getOption('onDelete') != $key2->getOption('onDelete')) {
                 return true;
             }
+        } else if ($key1->hasOption('onDelete') != $key2->hasOption('onDelete')) {
+            return true;
+        }
 
         return false;
     }

lib/Doctrine/DBAL/Schema/Index.php

@@ -102,6 +102,8 @@ class Index extends AbstractAsset implements Constraint
      */
     public function hasColumnAtPosition($columnName, $pos=0)
     {
-        return \array_search($columnName, $this->getColumns()) === $pos;
+        $columnName = strtolower($columnName);
+        $indexColumns = \array_map('strtolower', $this->getColumns());
+        return \array_search($columnName, $indexColumns) === $pos;
     }
 }

lib/Doctrine/DBAL/Schema/TableDiff.php

@@ -37,23 +37,23 @@ class TableDiff
     /**
      * All added fields
      *
-     * @var array(string=>ezcDbSchemaField)
+     * @var array(string=>Column)
      */
-    public $addedFields;
+    public $addedColumns;
 
     /**
      * All changed fields
      *
      * @var array(string=>Column)
      */
-    public $changedFields = array();
+    public $changedColumns = array();
 
     /**
      * All removed fields
      *
      * @var array(string=>bool)
      */
-    public $removedFields = array();
+    public $removedColumns = array();
 
     /**
      * All added indexes
@@ -100,20 +100,20 @@ class TableDiff
     /**
      * Constructs an TableDiff object.
      *
-     * @param array(string=>Column) $addedFields
+     * @param array(string=>Column) $addedColumns
-     * @param array(string=>Column) $changedFields
+     * @param array(string=>Column) $changedColumns
-     * @param array(string=>bool)             $removedFields
+     * @param array(string=>bool)             $removedColumns
      * @param array(string=>Index) $addedIndexes
      * @param array(string=>Index) $changedIndexes
      * @param array(string=>bool)             $removedIndexes
      */
-    function __construct( $addedFields = array(), $changedFields = array(),
+    function __construct( $addedColumns = array(), $changedColumns = array(),
-            $removedFields = array(), $addedIndexes = array(), $changedIndexes =
+            $removedColumns = array(), $addedIndexes = array(), $changedIndexes =
             array(), $removedIndexes = array() )
     {
-        $this->addedFields = $addedFields;
+        $this->addedColumns = $addedColumns;
-        $this->changedFields = $changedFields;
+        $this->changedColumns = $changedColumns;
-        $this->removedFields = $removedFields;
+        $this->removedColumns = $removedColumns;
         $this->addedIndexes = $addedIndexes;
         $this->changedIndexes = $changedIndexes;
         $this->removedIndexes = $removedIndexes;

tests/Doctrine/Tests/DBAL/Schema/ComparatorTest.php

@@ -494,11 +494,71 @@ class ComparatorTest extends \PHPUnit_Framework_TestCase
 
     public function testSequencesCaseInsenstive()
     {
+        $schemaA = new Schema();
+        $schemaA->createSequence('foo');
+        $schemaA->createSequence('BAR');
+        $schemaA->createSequence('Baz');
+        $schemaA->createSequence('new');
 
+        $schemaB = new Schema();
+        $schemaB->createSequence('FOO');
+        $schemaB->createSequence('Bar');
+        $schemaB->createSequence('baz');
+        $schemaB->createSequence('old');
+        
+        $c = new Comparator();
+        $diff = $c->compare($schemaA, $schemaB);
+
+        $this->assertSchemaSequenceChangeCount($diff, 1, 0, 1);
+    }
+
+    public function testCompareColumnCompareCaseInsensitive()
+    {
+        $tableA = new Table("foo");
+        $tableA->createColumn('id', 'integer');
+
+        $tableB = new Table("foo");
+        $tableB->createColumn('ID', 'integer');
+
+        $c = new Comparator();
+        $tableDiff = $c->diffTable($tableA, $tableB);
+
+        $this->assertFalse($tableDiff);
+    }
+
+    public function testCompareIndexBasedOnPropertiesNotName()
+    {
+        $tableA = new Table("foo");
+        $tableA->createColumn('id', 'integer');
+        $tableA->addIndex(array("id"), "foo_bar_idx");
+
+        $tableB = new Table("foo");
+        $tableB->createColumn('ID', 'integer');
+        $tableB->addIndex(array("id"), "bar_foo_idx");
+
+        $c = new Comparator();
+        $tableDiff = $c->diffTable($tableA, $tableB);
+
+        $this->assertFalse($tableDiff);
+    }
+
+    public function testCompareForeignKeyBasedOnPropertiesNotName()
+    {
+        $tableA = new Table("foo");
+        $tableA->createColumn('id', 'integer');
+        $tableA->addNamedForeignKeyConstraint('foo_constraint', 'bar', array('id'), array('id'));
+
+        $tableB = new Table("foo");
+        $tableB->createColumn('ID', 'integer');
+        $tableB->addNamedForeignKeyConstraint('bar_constraint', 'bar', array('id'), array('id'));
+
+        $c = new Comparator();
+        $tableDiff = $c->diffTable($tableA, $tableB);
+
+        $this->assertFalse($tableDiff);
     }
 
     /**
-     *
      * @param SchemaDiff $diff
      * @param int $newTableCount
      * @param int $changeTableCount
@@ -510,4 +570,17 @@ class ComparatorTest extends \PHPUnit_Framework_TestCase
         $this->assertEquals($changeTableCount, count($diff->changedTables));
         $this->assertEquals($removeTableCount, count($diff->removedTables));
     }
+
+    /**
+     * @param SchemaDiff $diff
+     * @param int $newSequenceCount
+     * @param int $changeSequenceCount
+     * @param int $changeSequenceCount
+     */
+    public function assertSchemaSequenceChangeCount($diff, $newSequenceCount=0, $changeSequenceCount=0, $removeSequenceCount=0)
+    {
+        $this->assertEquals($newSequenceCount, count($diff->newSequences), "Expected number of new sequences is wrong.");
+        $this->assertEquals($changeSequenceCount, count($diff->changedSequences), "Expected number of changed sequences is wrong.");
+        $this->assertEquals($removeSequenceCount, count($diff->removedSequences), "Expected number of removed sequences is wrong.");
+    }
 }