[2.0] Fixed DDC-101, DDC-102 - Ensure defaults for string length and field nullability are handled consistently in both update and create schema commands of SchemaTool.

lib/Doctrine/ORM/Tools/SchemaTool.php

@@ -40,9 +40,14 @@ use Doctrine\DBAL\Types\Type,
  */
 class SchemaTool
 {
-    /** The EntityManager */
+    /**
+     * @var \Doctrine\ORM\EntityManager
+     */
     private $_em;
-    /** The DatabasePlatform */
+
+    /**
+     * @var \Doctrine\DBAL\Platforms\AbstractPlatform
+     */
     private $_platform;
 
     /**
@@ -249,10 +254,14 @@ class SchemaTool
         $column['name'] = $class->getQuotedColumnName($mapping['fieldName'], $this->_platform);
         $column['type'] = Type::getType($mapping['type']);
         $column['length'] = isset($mapping['length']) ? $mapping['length'] : null;
-        $column['notnull'] = isset($mapping['nullable']) ? ! $mapping['nullable'] : false;
+        $column['notnull'] = isset($mapping['nullable']) ? ! $mapping['nullable'] : true;
         $column['unique'] = isset($mapping['unique']) ? $mapping['unique'] : false;
         $column['version'] = $class->isVersioned && $class->versionField == $mapping['fieldName'] ? true : false;
 
+        if(strtolower($column['type']) == 'string' && $column['length'] === null) {
+            $column['length'] = 255;
+        }
+
         if (isset($mapping['precision'])) {
             $column['precision'] = $mapping['precision'];
         }
@@ -552,9 +561,8 @@ class SchemaTool
                             
                             unset($type);
                             
-                            // 4. check for length change
+                            // 4. check for scale and precision change
-                            // 5. check for scale and precision change
+                            if (strtolower($columnInfo['type']) == 'decimal') {
-                            if ($columnInfo['type'] == 'Decimal') {
                                 /*// Doesn't work yet, see DDC-89
                                 if($columnInfo['length'] != $fieldMapping['precision'] ||
                                    $columnInfo['scale'] != $fieldMapping['scale']) {
@@ -563,7 +571,13 @@ class SchemaTool
                                     $columnInfo['scale'] = $fieldMapping['scale'];
                                     $columnChanged = true;
                                 }*/
-                            } else {
+                            }
+                            // 5. check for length change of strings
+                            elseif(strtolower($fieldMapping['type']) == 'string') {
+                                if(!isset($fieldMapping['length']) || $fieldMapping['length'] === null) {
+                                    $fieldMapping['length'] = 255;
+                                }
+
                                 if($columnInfo['length'] != $fieldMapping['length']) {
                                     $columnInfo['length'] = $fieldMapping['length'];
                                     $columnChanged = true;

tests/Doctrine/Tests/ORM/Functional/SchemaTool/MySqlSchemaToolTest.php

@@ -82,6 +82,13 @@ class MySqlSchemaToolTest extends \Doctrine\Tests\OrmFunctionalTestCase
             'length' => 50,
             'nullable' => false
         ));
+
+        // Test create column with no length and nullable defaults to 255, NOT NULL
+        $classA->mapField(array(
+            'fieldName' => 'newField2',
+            'columnName' => 'new_field2',
+            'type' => 'string',
+        ));
         
         // Introduce SchemaToolEntityB
         $classB = new ClassMetadata(__NAMESPACE__ . '\SchemaToolEntityB');
@@ -105,8 +112,16 @@ class MySqlSchemaToolTest extends \Doctrine\Tests\OrmFunctionalTestCase
         
         $this->assertEquals(2, count($sql));
         $this->assertEquals("CREATE TABLE schematool_entity_b (id INT NOT NULL, field VARCHAR(255) NOT NULL, PRIMARY KEY(id)) ENGINE = InnoDB", $sql[0]);
-        $this->assertEquals("ALTER TABLE schematool_entity_a ADD new_field VARCHAR(50) NOT NULL", $sql[1]);
+        $this->assertEquals("ALTER TABLE schematool_entity_a ADD new_field VARCHAR(50) NOT NULL, ADD new_field2 VARCHAR(255) NOT NULL", $sql[1]);
-        
+
+        $tool->updateSchema($classes);
+
+        // Change from 50 to default value (by setting null)
+        $classA->fieldMappings['newField']['length'] = null;
+
+        $sql = $tool->getUpdateSchemaSql($classes);
+        $this->assertEquals(1, count($sql));
+        $this->assertEquals("ALTER TABLE schematool_entity_a CHANGE new_field new_field VARCHAR(255) NOT NULL", $sql[0]);
     }
 }
 
@@ -115,6 +130,7 @@ class SchemaToolEntityA {
     /** @Id @Column(type="integer") */
     private $id;
     private $newField;
+    private $newField2;
 }
 
 class SchemaToolEntityB {

tests/Doctrine/Tests/ORM/Mapping/ClassMetadataTest.php

@@ -48,4 +48,21 @@ class ClassMetadataTest extends \Doctrine\Tests\OrmTestCase
         $this->assertEquals('phonenumbers', $oneOneMapping->getSourceFieldName());
         $this->assertEquals('Doctrine\Tests\Models\CMS\Bar', $oneOneMapping->getTargetEntityName());
     }
+
+    public function testFieldIsNullable()
+    {
+        $cm = new ClassMetadata('Doctrine\Tests\Models\CMS\CmsUser');
+
+        // Explicit Nullable
+        $cm->mapField(array('fieldName' => 'status', 'nullable' => true, 'type' => 'string', 'length' => 50));
+        $this->assertTrue($cm->isNullable('status'));
+
+        // Explicit Not Nullable
+        $cm->mapField(array('fieldName' => 'username', 'nullable' => false, 'type' => 'string', 'length' => 50));
+        $this->assertFalse($cm->isNullable('username'));
+
+        // Implicit Not Nullable
+        $cm->mapField(array('fieldName' => 'name', 'type' => 'string', 'length' => 50));
+        $this->assertFalse($cm->isNullable('name'), "By default a field should not be nullable.");
+    }
 }

tests/Doctrine/Tests/ORM/Tools/SchemaTool/MysqlUpdateSchemaTest.php

@@ -23,7 +23,7 @@ class MysqlUpdateSchemaTest extends UpdateSchemaTestCase
 
         $this->assertEquals(1, count($sql));
         $this->assertEquals(
-            "ALTER TABLE cms_addresses ADD street VARCHAR(255) DEFAULT NULL",
+            "ALTER TABLE cms_addresses ADD street VARCHAR(255) NOT NULL",
             $sql[0]
         );
     }
@@ -57,6 +57,24 @@ class MysqlUpdateSchemaTest extends UpdateSchemaTestCase
         $this->assertEquals("ALTER TABLE cms_addresses CHANGE city city VARCHAR(50) DEFAULT NULL", $sql[0]);
     }
 
+    /**
+     * @group DDC-102
+     */
+    public function testChangeNullabilityToNull()
+    {
+        $address = new \Doctrine\Tests\Models\CMS\CmsAddress;
+
+        $st = $this->_getSchemaTool("Cms");
+        $classMetadata = $this->_getMetadataFor("\Doctrine\Tests\Models\CMS\CmsAddress");
+
+        $this->assertFalse($classMetadata->fieldMappings['city']['nullable']);
+        unset($classMetadata->fieldMappings['city']['nullable']);
+
+        $sql = $st->getUpdateSchemaSql(array($classMetadata));
+
+        $this->assertEquals(0, count($sql));
+    }
+
     public function testChangeType()
     {
         $address = new \Doctrine\Tests\Models\CMS\CmsAddress;
@@ -99,6 +117,23 @@ class MysqlUpdateSchemaTest extends UpdateSchemaTestCase
         $this->assertEquals('ALTER TABLE cms_addresses CHANGE city city VARCHAR(200) NOT NULL', $sql[0]);
     }
 
+    /**
+     * @group DDC-101
+     */
+    public function testChangeLengthToNull()
+    {
+        $address = new \Doctrine\Tests\Models\CMS\CmsAddress;
+
+        $st = $this->_getSchemaTool("Cms");
+        $classMetadata = $this->_getMetadataFor("\Doctrine\Tests\Models\CMS\CmsAddress");
+        $classMetadata->fieldMappings['city']['length'] = null;
+
+        $sql = $st->getUpdateSchemaSql(array($classMetadata));
+
+        $this->assertEquals(1, count($sql));
+        $this->assertEquals('ALTER TABLE cms_addresses CHANGE city city VARCHAR(255) NOT NULL', $sql[0]);
+    }
+
     public function testChangeDecimalLengthPrecision()
     {
         $this->markTestSkipped('Decimal Scale changes not supported yet, because of DDC-89.');