Fixed calculation of differences of columns

lib/Doctrine/DBAL/Schema/Comparator.php

@@ -19,6 +19,8 @@
 
 namespace Doctrine\DBAL\Schema;
 
+use Doctrine\DBAL\Types;
+
 /**
  * Compares two Schemas and return an instance of SchemaDiff.
  *
@@ -365,86 +367,67 @@ class Comparator
      */
     public function diffColumn(Column $column1, Column $column2)
     {
+        $properties1 = $column1->toArray();
+        $properties2 = $column2->toArray();
+
         $changedProperties = array();
-        if ($column1->getType() != $column2->getType()) {
-            $changedProperties[] = 'type';
-        }
 
-        if ($column1->getNotnull() != $column2->getNotnull()) {
-            $changedProperties[] = 'notnull';
+        foreach (array('type', 'notnull', 'unsigned', 'autoincrement') as $property) {
+            if ($properties1[$property] != $properties2[$property]) {
+                $changedProperties[] = $property;
+            }
         }
-
-        $column1Default = $column1->getDefault();
-        $column2Default = $column2->getDefault();
-
-        if ($column1Default != $column2Default ||
+        
+        if ($properties1['default'] != $properties2['default'] ||
             // Null values need to be checked additionally as they tell whether to create or drop a default value.
             // null != 0, null != false, null != '' etc. This affects platform's table alteration SQL generation.
-            (null === $column1Default && null !== $column2Default) ||
-            (null === $column2Default && null !== $column1Default)
+            (null === $properties1['default'] && null !== $properties2['default']) ||
+            (null === $properties2['default'] && null !== $properties1['default'])
         ) {
             $changedProperties[] = 'default';
         }
 
-        if ($column1->getUnsigned() != $column2->getUnsigned()) {
-            $changedProperties[] = 'unsigned';
-        }
-
-        $column1Type = $column1->getType();
-
-        if ($column1Type instanceof \Doctrine\DBAL\Types\StringType ||
-            $column1Type instanceof \Doctrine\DBAL\Types\BinaryType
-        ) {
+        if ($properties1['type'] instanceof Types\StringType || $properties1['type'] instanceof Types\BinaryType) {
             // check if value of length is set at all, default value assumed otherwise.
-            $length1 = $column1->getLength() ?: 255;
-            $length2 = $column2->getLength() ?: 255;
+            $length1 = $properties1['length'] ?: 255;
+            $length2 = $properties2['length'] ?: 255;
             if ($length1 != $length2) {
                 $changedProperties[] = 'length';
             }
 
-            if ($column1->getFixed() != $column2->getFixed()) {
+            if ($properties1['fixed'] != $properties2['fixed']) {
                 $changedProperties[] = 'fixed';
             }
-        }
-
-        if ($column1->getType() instanceof \Doctrine\DBAL\Types\DecimalType) {
-            if (($column1->getPrecision()?:10) != ($column2->getPrecision()?:10)) {
+        } elseif ($properties1['type'] instanceof Types\DecimalType) {
+            if (($properties1['precision'] ?: 10) != ($properties2['precision'] ?: 10)) {
                 $changedProperties[] = 'precision';
             }
-            if ($column1->getScale() != $column2->getScale()) {
+            if ($properties1['scale'] != $properties2['scale']) {
                 $changedProperties[] = 'scale';
             }
         }
 
-        if ($column1->getAutoincrement() != $column2->getAutoincrement()) {
-            $changedProperties[] = 'autoincrement';
-        }
-
         // only allow to delete comment if its set to '' not to null.
-        if ($column1->getComment() !== null && $column1->getComment() != $column2->getComment()) {
+        if ($properties1['comment'] !== null && $properties1['comment'] != $properties2['comment']) {
             $changedProperties[] = 'comment';
         }
 
-        $options1 = $column1->getCustomSchemaOptions();
-        $options2 = $column2->getCustomSchemaOptions();
+        $customOptions1 = $column1->getCustomSchemaOptions();
+        $customOptions2 = $column2->getCustomSchemaOptions();
 
-        $commonKeys = array_keys(array_intersect_key($options1, $options2));
-
-        foreach ($commonKeys as $key) {
-            if ($options1[$key] !== $options2[$key]) {
+        foreach (array_merge(array_keys($customOptions1), array_keys($customOptions2)) as $key) {
+            if ( ! array_key_exists($key, $properties1) || ! array_key_exists($key, $properties2)) {
+                $changedProperties[] = $key;
+            } elseif ($properties1[$key] !== $properties2[$key]) {
                 $changedProperties[] = $key;
             }
         }
 
-        $diffKeys = array_keys(array_diff_key($options1, $options2) + array_diff_key($options2, $options1));
-
-        $changedProperties = array_merge($changedProperties, $diffKeys);
-
         $platformOptions1 = $column1->getPlatformOptions();
         $platformOptions2 = $column2->getPlatformOptions();
 
         foreach (array_keys(array_intersect_key($platformOptions1, $platformOptions2)) as $key) {
-            if ($platformOptions1[$key] !== $platformOptions2[$key]) {
+            if ($properties1[$key] !== $properties2[$key]) {
                 $changedProperties[] = $key;
             }
         }

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

@@ -1004,15 +1004,9 @@ class ComparatorTest extends \PHPUnit_Framework_TestCase
 
     public function testDiffColumnPlatformOptions()
     {
-        $column1 = new Column('foo', Type::getType('string'));
-        $column1->setPlatformOptions(array('foo' => 'foo', 'bar' => 'bar'));
-
-        $column2 = new Column('foo', Type::getType('string'));
-        $column2->setPlatformOptions(array('foo' => 'foo', 'foobar' => 'foobar'));
-
-        $column3 = new Column('foo', Type::getType('string'));
-        $column3->setPlatformOptions(array('foo' => 'foo', 'bar' => 'rab'));
-
+        $column1 = new Column('foo', Type::getType('string'), array('platformOptions' => array('foo' => 'foo', 'bar' => 'bar')));
+        $column2 = new Column('foo', Type::getType('string'), array('platformOptions' => array('foo' => 'foo', 'foobar' => 'foobar')));
+        $column3 = new Column('foo', Type::getType('string'), array('platformOptions' => array('foo' => 'foo', 'bar' => 'rab')));
         $column4 = new Column('foo', Type::getType('string'));
 
         $comparator = new Comparator();
@@ -1024,4 +1018,21 @@ class ComparatorTest extends \PHPUnit_Framework_TestCase
         $this->assertEquals(array(), $comparator->diffColumn($column1, $column4));
         $this->assertEquals(array(), $comparator->diffColumn($column4, $column1));
     }
+
+    public function testComplexDiffColumn()
+    {
+        $column1 = new Column('foo', Type::getType('string'), array(
+            'platformOptions' => array('foo' => 'foo'),
+            'customSchemaOptions' => array('foo' => 'bar'),
+        ));
+
+        $column2 = new Column('foo', Type::getType('string'), array(
+            'platformOptions' => array('foo' => 'bar'),
+        ));
+
+        $comparator = new Comparator();
+
+        $this->assertEquals(array(), $comparator->diffColumn($column1, $column2));
+        $this->assertEquals(array(), $comparator->diffColumn($column2, $column1));
+    }
 }