[2.0] DDC-271 - Add columnDefinition for Join-Column in Annotation, XML and...

[2.0] DDC-271 - Add columnDefinition for Join-Column in Annotation, XML and YAML driver, updated SchemaTool to use Join Column definition instead of using the column's columnDefinition if specified. Refactored MappingDriverTest to allow for feature specific test-cases instead of one large assertion block. Fixed typos in XmlDriver - Updated doctrine-mapping.xsd Schema File

doctrine-mapping.xsd

@@ -123,7 +123,7 @@
     <xs:attribute name="unique" type="xs:boolean" default="false" />
     <xs:attribute name="nullable" type="xs:boolean" default="false" />
     <xs:attribute name="version" type="xs:boolean" />
-    <xs:attribute name="columnDefinition" type="xs:string" />
+    <xs:attribute name="column-definition" type="xs:string" />
   </xs:complexType>
   
   <xs:complexType name="discriminator-column">
@@ -192,6 +192,7 @@
     <xs:attribute name="nullable" type="xs:boolean" default="true" />
     <xs:attribute name="on-delete" type="orm:fk-action" />
     <xs:attribute name="on-update" type="orm:fk-action" />
+    <xs:attribute name="column-definition" type="xs:string" />
   </xs:complexType>
 
   <xs:complexType name="join-columns">

lib/Doctrine/ORM/Mapping/Driver/AnnotationDriver.php

@@ -218,7 +218,8 @@ class AnnotationDriver implements Driver
                     'unique' => $joinColumnAnnot->unique,
                     'nullable' => $joinColumnAnnot->nullable,
                     'onDelete' => $joinColumnAnnot->onDelete,
-                    'onUpdate' => $joinColumnAnnot->onUpdate
+                    'onUpdate' => $joinColumnAnnot->onUpdate,
+                    'columnDefinition' => $joinColumnAnnot->columnDefinition,
                 );
             } else if ($joinColumnsAnnot = $this->_reader->getPropertyAnnotation($property, 'Doctrine\ORM\Mapping\JoinColumns')) {
                 foreach ($joinColumnsAnnot->value as $joinColumn) {
@@ -228,7 +229,8 @@ class AnnotationDriver implements Driver
                         'unique' => $joinColumn->unique,
                         'nullable' => $joinColumn->nullable,
                         'onDelete' => $joinColumn->onDelete,
-                        'onUpdate' => $joinColumn->onUpdate
+                        'onUpdate' => $joinColumn->onUpdate,
+                        'columnDefinition' => $joinColumn->columnDefinition,
                     );
                 }
             }
@@ -319,7 +321,8 @@ class AnnotationDriver implements Driver
                             'unique' => $joinColumn->unique,
                             'nullable' => $joinColumn->nullable,
                             'onDelete' => $joinColumn->onDelete,
-                            'onUpdate' => $joinColumn->onUpdate
+                            'onUpdate' => $joinColumn->onUpdate,
+                            'columnDefinition' => $joinColumn->columnDefinition,
                         );
                     }
 
@@ -330,7 +333,8 @@ class AnnotationDriver implements Driver
                             'unique' => $joinColumn->unique,
                             'nullable' => $joinColumn->nullable,
                             'onDelete' => $joinColumn->onDelete,
-                            'onUpdate' => $joinColumn->onUpdate
+                            'onUpdate' => $joinColumn->onUpdate,
+                            'columnDefinition' => $joinColumn->columnDefinition,
                         );
                     }
                 }

lib/Doctrine/ORM/Mapping/Driver/DoctrineAnnotations.php

@@ -51,6 +51,7 @@ final class JoinColumn extends Annotation {
     public $nullable = true;
     public $onDelete;
     public $onUpdate;
+    public $columnDefinition;
 }
 final class JoinColumns extends Annotation {}
 final class Column extends Annotation {

lib/Doctrine/ORM/Mapping/Driver/XmlDriver.php

@@ -164,8 +164,8 @@ class XmlDriver extends AbstractFileDriver
                     $metadata->setVersionMapping($mapping);
                 }
                 
-                if (isset($fieldMapping['columnDefinition'])) {
-                    $mapping['columnDefinition'] = (string)$fieldMapping['columnDefinition'];
+                if (isset($fieldMapping['column-definition'])) {
+                    $mapping['columnDefinition'] = (string)$fieldMapping['column-definition'];
                 }
                 
                 $metadata->mapField($mapping);
@@ -392,13 +392,17 @@ class XmlDriver extends AbstractFileDriver
             $joinColumn['nullable'] = ((string)$joinColumnElement['nullable'] == "false") ? false : true;
         }
         
-        if (isset($joinColumnElement['onDelete'])) {
+        if (isset($joinColumnElement['on-delete'])) {
             $joinColumn['onDelete'] = (string)$joinColumnElement['on-delete'];
         }
         
-        if (isset($joinColumnElement['onUpdate'])) {
+        if (isset($joinColumnElement['on-update'])) {
             $joinColumn['onUpdate'] = (string)$joinColumnElement['on-update'];
         }
+
+        if (isset($joinColumnElement['column-definition'])) {
+            $joinColumn['columnDefinition'] = (string)$joinColumnElement['column-definition'];
+        }
         
         return $joinColumn;
     }

lib/Doctrine/ORM/Mapping/Driver/YamlDriver.php

@@ -414,6 +414,10 @@ class YamlDriver extends AbstractFileDriver
         if (isset($joinColumnElement['onUpdate'])) {
             $joinColumn['onUpdate'] = $joinColumnElement['onUpdate'];
         }
+
+        if (isset($joinColumnElement['columnDefinition'])) {
+            $joinColumn['columnDefinition'] = $joinColumnElement['columnDefinition'];
+        }
         
         return $joinColumn;
     }

lib/Doctrine/ORM/Tools/SchemaTool.php

@@ -415,7 +415,13 @@ class SchemaTool
                 // property as well.
 
                 $fieldMapping = $class->getFieldMapping($referencedFieldName);
-                $columnDef = isset($fieldMapping['columnDefinition']) ? $fieldMapping['columnDefinition'] : null;
+
+                $columnDef = null;
+                if (isset($joinColumn['columnDefinition'])) {
+                    $columnDef = $joinColumn['columnDefinition'];
+                } else if (isset($fieldMapping['columnDefinition'])) {
+                    $columnDef = $fieldMapping['columnDefinition'];
+                }
                 $columnOptions = array('notnull' => false, 'columnDefinition' => $columnDef);
 
                 $theJoinTable->createColumn(

tests/Doctrine/Tests/ORM/Mapping/MappingDriverTest.php → tests/Doctrine/Tests/ORM/Mapping/AbstractMappingDriverTest.php

@@ -7,66 +7,86 @@ use Doctrine\ORM\Mapping\ClassMetadata,
     Doctrine\ORM\Mapping\Driver\YamlDriver;
 
 require_once __DIR__ . '/../../TestInit.php';
- 
-class MappingDriverTest extends \Doctrine\Tests\OrmTestCase
+
+abstract class AbstractMappingDriverTest extends \Doctrine\Tests\OrmTestCase
 {
-    public function testXmlMapping()
-    {
-        $className = 'Doctrine\Tests\ORM\Mapping\User';
-        $xmlDriver = new XmlDriver(__DIR__ . DIRECTORY_SEPARATOR . 'xml');
-        
-        $class = new ClassMetadata($className);
-        
-        $this->assertFalse($xmlDriver->isTransient($className));
-        
-        $xmlDriver->loadMetadataForClass($className, $class);
-        
-        $this->_testUserClassMapping($class);
-    }
-    
-    public function testYamlMapping()
+    abstract protected function _loadDriver();
+
+    public function testLoadMapping()
     {
         $className = 'Doctrine\Tests\ORM\Mapping\User';
-        $yamlDriver = new YamlDriver(__DIR__ . DIRECTORY_SEPARATOR . 'yaml');
-        
+        $mappingDriver = $this->_loadDriver();
+
         $class = new ClassMetadata($className);
-        
-        $this->assertFalse($yamlDriver->isTransient($className));
-        
-        $yamlDriver->loadMetadataForClass($className, $class);
-        
-        $this->_testUserClassMapping($class);
+
+        $this->assertFalse($mappingDriver->isTransient($className));
+
+        $mappingDriver->loadMetadataForClass($className, $class);
+
+        return $class;
     }
-    
-    public function testXmlGetAllClassNames()
+
+    /**
+     * @depends testLoadMapping
+     * @param ClassMetadata $class
+     */
+    public function testEntityTableNameAndInheritance($class)
     {
-        $className = 'Doctrine\Tests\ORM\Mapping\User';
-        $xmlDriver = new XmlDriver(__DIR__ . DIRECTORY_SEPARATOR . 'xml');
-        
-        $class = new ClassMetadata($className);
-        
-        $classNames = $xmlDriver->getAllClassNames();
-        
-        $this->assertEquals($className, $classNames[0]);
-        $this->assertEquals(1, count($classNames));
-    }
-    
-    private function _testUserClassMapping(ClassMetadata $class)
-    {        
         $this->assertEquals('cms_users', $class->getTableName());
         $this->assertEquals(ClassMetadata::INHERITANCE_TYPE_NONE, $class->getInheritanceType());
-        $this->assertEquals(2, count($class->fieldMappings));
+
+        return $class;
+    }
+
+    /**
+     * @depends testEntityTableNameAndInheritance
+     * @param ClassMetadata $class
+     */
+    public function testFieldMappings($class)
+    {
+        $this->assertEquals(3, count($class->fieldMappings));
         $this->assertTrue(isset($class->fieldMappings['id']));
         $this->assertTrue(isset($class->fieldMappings['name']));
+        $this->assertTrue(isset($class->fieldMappings['email']));
+
         $this->assertEquals('string', $class->fieldMappings['name']['type']);
         $this->assertTrue($class->fieldMappings['name']['nullable']);
         $this->assertTrue($class->fieldMappings['name']['unique']);
+        $this->assertEquals("user_email", $class->fieldMappings['email']['columnName']);
+
+        return $class;
+    }
+
+    /**
+     * @depends testFieldMappings
+     * @param ClassMetadata $class
+     */
+    public function testIdentifier($class)
+    {
         $this->assertEquals(array('id'), $class->identifier);
         $this->assertEquals(ClassMetadata::GENERATOR_TYPE_AUTO, $class->getIdGeneratorType());
-        
+
+        return $class;
+    }
+
+    /**
+     * @depends testIdentifier
+     * @param ClassMetadata $class
+     */
+    public function testAssocations($class)
+    {
         $this->assertEquals(3, count($class->associationMappings));
         $this->assertEquals(1, count($class->inverseMappings));
-        
+
+        return $class;
+    }
+
+    /**
+     * @depends testAssocations
+     * @param ClassMetadata $class
+     */
+    public function testOwningOneToOneAssocation($class)
+    {
         $this->assertTrue($class->associationMappings['address'] instanceof \Doctrine\ORM\Mapping\OneToOneMapping);
         $this->assertTrue(isset($class->associationMappings['address']));
         $this->assertTrue($class->associationMappings['address']->isOwningSide);
@@ -76,7 +96,16 @@ class MappingDriverTest extends \Doctrine\Tests\OrmTestCase
         $this->assertFalse($class->associationMappings['address']->isCascadeRefresh);
         $this->assertFalse($class->associationMappings['address']->isCascadeDetach);
         $this->assertFalse($class->associationMappings['address']->isCascadeMerge);
-        
+
+        return $class;
+    }
+
+    /**
+     * @depends testOwningOneToOneAssocation
+     * @param ClassMetadata $class
+     */
+    public function testInverseOneToManyAssociation($class)
+    {
         $this->assertTrue($class->associationMappings['phonenumbers'] instanceof \Doctrine\ORM\Mapping\OneToManyMapping);
         $this->assertTrue(isset($class->associationMappings['phonenumbers']));
         $this->assertFalse($class->associationMappings['phonenumbers']->isOwningSide);
@@ -86,13 +115,19 @@ class MappingDriverTest extends \Doctrine\Tests\OrmTestCase
         $this->assertFalse($class->associationMappings['phonenumbers']->isCascadeRefresh);
         $this->assertFalse($class->associationMappings['phonenumbers']->isCascadeDetach);
         $this->assertFalse($class->associationMappings['phonenumbers']->isCascadeMerge);
-        
+
+        return $class;
+    }
+
+    /**
+     * @depends testInverseOneToManyAssociation
+     * @param ClassMetadata $class
+     */
+    public function testManyToManyAssociationWithCascadeAll($class)
+    {
         $this->assertTrue($class->associationMappings['groups'] instanceof \Doctrine\ORM\Mapping\ManyToManyMapping);
         $this->assertTrue(isset($class->associationMappings['groups']));
         $this->assertTrue($class->associationMappings['groups']->isOwningSide);
-        $this->assertEquals(count($class->lifecycleCallbacks), 2);
-        $this->assertEquals($class->lifecycleCallbacks['prePersist'][0], 'doStuffOnPrePersist');
-        $this->assertEquals($class->lifecycleCallbacks['postPersist'][0], 'doStuffOnPostPersist');
         // Make sure that cascade-all works as expected
         $this->assertTrue($class->associationMappings['groups']->isCascadeRemove);
         $this->assertTrue($class->associationMappings['groups']->isCascadePersist);
@@ -100,19 +135,56 @@ class MappingDriverTest extends \Doctrine\Tests\OrmTestCase
         $this->assertTrue($class->associationMappings['groups']->isCascadeDetach);
         $this->assertTrue($class->associationMappings['groups']->isCascadeMerge);
 
+        return $class;
+    }
+
+    /**
+     * @depends testManyToManyAssociationWithCascadeAll
+     * @param ClassMetadata $class
+     */
+    public function testLifecycleCallbacks($class)
+    {
+        $this->assertEquals(count($class->lifecycleCallbacks), 2);
+        $this->assertEquals($class->lifecycleCallbacks['prePersist'][0], 'doStuffOnPrePersist');
+        $this->assertEquals($class->lifecycleCallbacks['postPersist'][0], 'doStuffOnPostPersist');
+
+        return $class;
+    }
+
+    /**
+     * @depends testLifecycleCallbacks
+     * @param ClassMetadata $class
+     */
+    public function testJoinColumnUniqueAndNullable($class)
+    {
         // Non-Nullability of Join Column
         $this->assertFalse($class->associationMappings['groups']->joinTable['joinColumns'][0]['nullable']);
         $this->assertFalse($class->associationMappings['groups']->joinTable['joinColumns'][0]['unique']);
+
+        return $class;
+    }
+
+    /**
+     * @depends testJoinColumnUniqueAndNullable
+     * @param ClassMetadata $class
+     */
+    public function testColumnDefinition($class)
+    {
+        $this->assertEquals("CHAR(32) NOT NULL", $class->fieldMappings['email']['columnDefinition']);
+        $this->assertEquals("INT NULL", $class->associationMappings['groups']->joinTable['inverseJoinColumns'][0]['columnDefinition']);
+
+        return $class;
     }
 }
 
 class User {
     private $id;
     private $name;
+    private $email;
     private $address;
     private $phonenumbers;
     private $groups;
-    
+
     // ... rest of code omitted, irrelevant for the mapping tests
 
     public function doStuffOnPrePersist()
@@ -121,6 +193,6 @@ class User {
 
     public function doStuffOnPostPersist()
     {
-        
+
     }
 }
\ No newline at end of file

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

@@ -20,7 +20,8 @@ class AllTests
         $suite = new \Doctrine\Tests\DoctrineTestSuite('Doctrine Orm Mapping');
 
         $suite->addTestSuite('Doctrine\Tests\ORM\Mapping\ClassMetadataTest');
-        $suite->addTestSuite('Doctrine\Tests\ORM\Mapping\MappingDriverTest');
+        $suite->addTestSuite('Doctrine\Tests\ORM\Mapping\XmlMappingDriverTest');
+        $suite->addTestSuite('Doctrine\Tests\ORM\Mapping\YamlMappingDriverTest');
         $suite->addTestSuite('Doctrine\Tests\ORM\Mapping\ClassMetadataFactoryTest');
         $suite->addTestSuite('Doctrine\Tests\ORM\Mapping\ClassMetadataLoadEventTest');
         $suite->addTestSuite('Doctrine\Tests\ORM\Mapping\BasicInheritanceMappingTest');

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

+<?php
+
+namespace Doctrine\Tests\ORM\Mapping;
+
+use Doctrine\ORM\Mapping\ClassMetadata,
+    Doctrine\ORM\Mapping\Driver\XmlDriver,
+    Doctrine\ORM\Mapping\Driver\YamlDriver;
+
+require_once __DIR__ . '/../../TestInit.php';
+
+class XmlMappingDriverTest extends AbstractMappingDriverTest
+{
+    protected function _loadDriver()
+    {
+        return new XmlDriver(__DIR__ . DIRECTORY_SEPARATOR . 'xml');
+    }
+}
\ No newline at end of file

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

+<?php
+
+namespace Doctrine\Tests\ORM\Mapping;
+
+use Doctrine\ORM\Mapping\ClassMetadata,
+    Doctrine\ORM\Mapping\Driver\XmlDriver,
+    Doctrine\ORM\Mapping\Driver\YamlDriver;
+
+require_once __DIR__ . '/../../TestInit.php';
+
+class YamlMappingDriverTest extends AbstractMappingDriverTest
+{
+    protected function _loadDriver()
+    {
+        return new YamlDriver(__DIR__ . DIRECTORY_SEPARATOR . 'yaml');
+    }
+}
\ No newline at end of file

tests/Doctrine/Tests/ORM/Mapping/xml/Doctrine.Tests.ORM.Mapping.User.dcm.xml

@@ -17,6 +17,7 @@
         </id>
         
         <field name="name" column="name" type="string" length="50" nullable="true" unique="true" />
+        <field name="email" column="user_email" type="string" column-definition="CHAR(32) NOT NULL" />
         
         <one-to-one field="address" target-entity="Address">
             <join-column name="address_id" referenced-column-name="id"/>
@@ -37,7 +38,7 @@
                     <join-column name="user_id" referenced-column-name="id" nullable="false" unique="false" />
                 </join-columns>
                 <inverse-join-columns>
-                    <join-column name="group_id" referenced-column-name="id"/>
+                    <join-column name="group_id" referenced-column-name="id" column-definition="INT NULL" />
                 </inverse-join-columns>
             </join-table>
             <cascade>

tests/Doctrine/Tests/ORM/Mapping/yaml/Doctrine.Tests.ORM.Mapping.User.dcm.yml

@@ -12,6 +12,10 @@ Doctrine\Tests\ORM\Mapping\User:
       length: 50
       nullable: true
       unique: true
+    email:
+      type: string
+      column: user_email
+      columnDefinition: CHAR(32) NOT NULL
   oneToOne:
     address:
       targetEntity: Address
@@ -37,6 +41,7 @@ Doctrine\Tests\ORM\Mapping\User:
         inverseJoinColumns:
           group_id:
             referencedColumnName: id
+            columnDefinition: INT NULL
       cascade:
         - all
   lifecycleCallbacks: