Merge pull request #537 from Powerhamster/joined-composite-keys

fixed problems with joined inheritance and composite keys

lib/Doctrine/ORM/Persisters/JoinedSubclassPersister.php

@@ -184,6 +184,7 @@ class JoinedSubclassPersister extends AbstractEntityInheritancePersister
             // Execute inserts on subtables.
             // The order doesn't matter because all child tables link to the root table via FK.
             foreach ($subTableStmts as $tableName => $stmt) {
+                /** @var \Doctrine\DBAL\Statement $stmt */
                 $paramIndex = 1;
                 $data       = isset($insertData[$tableName])
                     ? $insertData[$tableName]
@@ -196,7 +197,9 @@ class JoinedSubclassPersister extends AbstractEntityInheritancePersister
                 }
 
                 foreach ($data as $columnName => $value) {
-                    $stmt->bindValue($paramIndex++, $value, $this->columnTypes[$columnName]);
+                    if (!isset($id[$columnName])) {
+                        $stmt->bindValue($paramIndex++, $value, $this->columnTypes[$columnName]);
+                    }
                 }
 
                 $stmt->execute();

lib/Doctrine/ORM/Tools/SchemaTool.php

@@ -93,7 +93,7 @@ class SchemaTool
         foreach ($createSchemaSql as $sql) {
             try {
                 $conn->executeQuery($sql);
-            } catch(\Exception $e) {
+            } catch (\Exception $e) {
                 throw ToolsException::schemaToolFailure($sql, $e);
             }
         }
@@ -154,6 +154,7 @@ class SchemaTool
         $blacklistedFks = array();
 
         foreach ($classes as $class) {
+            /** @var \Doctrine\ORM\Mapping\ClassMetadata $class */
             if ($this->processingNotRequired($class, $processedClasses)) {
                 continue;
             }
@@ -161,7 +162,7 @@ class SchemaTool
             $table = $schema->createTable($this->quoteStrategy->getTableName($class, $this->platform));
 
             if ($class->isInheritanceTypeSingleTable()) {
-                $columns = $this->gatherColumns($class, $table);
+                $this->gatherColumns($class, $table);
                 $this->gatherRelationsSql($class, $table, $schema, $addedFks, $blacklistedFks);
 
                 // Add the discriminator column
@@ -184,7 +185,11 @@ class SchemaTool
                 $pkColumns = array();
                 foreach ($class->fieldMappings as $fieldName => $mapping) {
                     if ( ! isset($mapping['inherited'])) {
-                        $columnName = $this->quoteStrategy->getColumnName($mapping['fieldName'], $class, $this->platform);
+                        $columnName = $this->quoteStrategy->getColumnName(
+                            $mapping['fieldName'],
+                            $class,
+                            $this->platform
+                        );
                         $this->gatherColumn($class, $mapping, $table);
 
                         if ($class->isIdentifier($fieldName)) {
@@ -200,20 +205,36 @@ class SchemaTool
                     $this->addDiscriminatorColumnDefinition($class, $table);
                 } else {
                     // Add an ID FK column to child tables
-                    /* @var \Doctrine\ORM\Mapping\ClassMetadata $class */
+                    $inheritedKeyColumns = array();
-                    $idMapping = $class->fieldMappings[$class->identifier[0]];
+                    foreach ($class->identifier as $identifierField) {
-                    $this->gatherColumn($class, $idMapping, $table);
+                        $idMapping = $class->fieldMappings[$identifierField];
-                    $columnName = $this->quoteStrategy->getColumnName($class->identifier[0], $class, $this->platform);
+                        if (isset($idMapping['inherited'])) {
-                    // TODO: This seems rather hackish, can we optimize it?
+                            $this->gatherColumn($class, $idMapping, $table);
-                    $table->getColumn($columnName)->setAutoincrement(false);
+                            $columnName = $this->quoteStrategy->getColumnName(
+                                $identifierField,
+                                $class,
+                                $this->platform
+                            );
+                            // TODO: This seems rather hackish, can we optimize it?
+                            $table->getColumn($columnName)->setAutoincrement(false);
 
-                    $pkColumns[] = $columnName;
+                            $pkColumns[] = $columnName;
+                            $inheritedKeyColumns[] = $columnName;
+                        }
+                    }
+                    if (!empty($inheritedKeyColumns)) {
+                        // Add a FK constraint on the ID column
+                        $table->addForeignKeyConstraint(
+                            $this->quoteStrategy->getTableName(
+                                $this->em->getClassMetadata($class->rootEntityName),
+                                $this->platform
+                            ),
+                            $inheritedKeyColumns,
+                            $inheritedKeyColumns,
+                            array('onDelete' => 'CASCADE')
+                        );
+                    }
 
-                    // Add a FK constraint on the ID column
-                    $table->addForeignKeyConstraint(
-                        $this->quoteStrategy->getTableName($this->em->getClassMetadata($class->rootEntityName), $this->platform),
-                        array($columnName), array($columnName), array('onDelete' => 'CASCADE')
-                    );
                 }
 
                 $table->setPrimaryKey($pkColumns);
@@ -275,7 +296,10 @@ class SchemaTool
             }
 
             if ($eventManager->hasListeners(ToolEvents::postGenerateSchemaTable)) {
-                $eventManager->dispatchEvent(ToolEvents::postGenerateSchemaTable, new GenerateSchemaTableEventArgs($class, $schema, $table));
+                $eventManager->dispatchEvent(
+                    ToolEvents::postGenerateSchemaTable,
+                    new GenerateSchemaTableEventArgs($class, $schema, $table)
+                );
             }
         }
 
@@ -284,7 +308,10 @@ class SchemaTool
         }
 
         if ($eventManager->hasListeners(ToolEvents::postGenerateSchema)) {
-            $eventManager->dispatchEvent(ToolEvents::postGenerateSchema, new GenerateSchemaEventArgs($this->em, $schema));
+            $eventManager->dispatchEvent(
+                ToolEvents::postGenerateSchema,
+                new GenerateSchemaEventArgs($this->em, $schema)
+            );
         }
 
         return $schema;
@@ -304,7 +331,9 @@ class SchemaTool
     {
         $discrColumn = $class->discriminatorColumn;
 
-        if ( ! isset($discrColumn['type']) || (strtolower($discrColumn['type']) == 'string' && $discrColumn['length'] === null)) {
+        if ( ! isset($discrColumn['type']) ||
+            (strtolower($discrColumn['type']) == 'string' && $discrColumn['length'] === null)
+        ) {
             $discrColumn['type'] = 'string';
             $discrColumn['length'] = 255;
         }
@@ -348,7 +377,7 @@ class SchemaTool
 
         // For now, this is a hack required for single table inheritence, since this method is called
         // twice by single table inheritence relations
-        if(!$table->hasIndex('primary')) {
+        if (!$table->hasIndex('primary')) {
             //$table->setPrimaryKey($pkColumns);
         }
     }
@@ -377,7 +406,7 @@ class SchemaTool
         $options['platformOptions'] = array();
         $options['platformOptions']['version'] = $class->isVersioned && $class->versionField == $mapping['fieldName'] ? true : false;
 
-        if(strtolower($columnType) == 'string' && $options['length'] === null) {
+        if (strtolower($columnType) == 'string' && $options['length'] === null) {
             $options['length'] = 255;
         }
 
@@ -457,9 +486,18 @@ class SchemaTool
             if ($mapping['type'] & ClassMetadata::TO_ONE && $mapping['isOwningSide']) {
                 $primaryKeyColumns = $uniqueConstraints = array(); // PK is unnecessary for this relation-type
 
-                $this->_gatherRelationJoinColumns($mapping['joinColumns'], $table, $foreignClass, $mapping, $primaryKeyColumns, $uniqueConstraints, $addedFks, $blacklistedFks);
+                $this->gatherRelationJoinColumns(
+                    $mapping['joinColumns'],
+                    $table,
+                    $foreignClass,
+                    $mapping,
+                    $primaryKeyColumns,
+                    $uniqueConstraints,
+                    $addedFks,
+                    $blacklistedFks
+                );
 
-                foreach($uniqueConstraints as $indexName => $unique) {
+                foreach ($uniqueConstraints as $indexName => $unique) {
                     $table->addUniqueIndex($unique['columns'], is_numeric($indexName) ? null : $indexName);
                 }
             } elseif ($mapping['type'] == ClassMetadata::ONE_TO_MANY && $mapping['isOwningSide']) {
@@ -469,19 +507,39 @@ class SchemaTool
                 // create join table
                 $joinTable = $mapping['joinTable'];
 
-                $theJoinTable = $schema->createTable($this->quoteStrategy->getJoinTableName($mapping, $foreignClass, $this->platform));
+                $theJoinTable = $schema->createTable(
+                    $this->quoteStrategy->getJoinTableName($mapping, $foreignClass, $this->platform)
+                );
 
                 $primaryKeyColumns = $uniqueConstraints = array();
 
                 // Build first FK constraint (relation table => source table)
-                $this->_gatherRelationJoinColumns($joinTable['joinColumns'], $theJoinTable, $class, $mapping, $primaryKeyColumns, $uniqueConstraints, $addedFks, $blacklistedFks);
+                $this->gatherRelationJoinColumns(
+                    $joinTable['joinColumns'],
+                    $theJoinTable,
+                    $class,
+                    $mapping,
+                    $primaryKeyColumns,
+                    $uniqueConstraints,
+                    $addedFks,
+                    $blacklistedFks
+                );
 
                 // Build second FK constraint (relation table => target table)
-                $this->_gatherRelationJoinColumns($joinTable['inverseJoinColumns'], $theJoinTable, $foreignClass, $mapping, $primaryKeyColumns, $uniqueConstraints, $addedFks, $blacklistedFks);
+                $this->gatherRelationJoinColumns(
+                    $joinTable['inverseJoinColumns'],
+                    $theJoinTable,
+                    $foreignClass,
+                    $mapping,
+                    $primaryKeyColumns,
+                    $uniqueConstraints,
+                    $addedFks,
+                    $blacklistedFks
+                );
 
                 $theJoinTable->setPrimaryKey($primaryKeyColumns);
 
-                foreach($uniqueConstraints as $indexName => $unique) {
+                foreach ($uniqueConstraints as $indexName => $unique) {
                     $theJoinTable->addUniqueIndex($unique['columns'], is_numeric($indexName) ? null : $indexName);
                 }
             }
@@ -513,7 +571,8 @@ class SchemaTool
         if (in_array($referencedColumnName, $class->getIdentifierColumnNames())) {
             // it seems to be an entity as foreign key
             foreach ($class->getIdentifierFieldNames() as $fieldName) {
-                if ($class->hasAssociation($fieldName) && $class->getSingleAssociationJoinColumnName($fieldName) == $referencedColumnName) {
+                if ($class->hasAssociation($fieldName)
+                    && $class->getSingleAssociationJoinColumnName($fieldName) == $referencedColumnName) {
                     return $this->getDefiningClass(
                         $this->em->getClassMetadata($class->associationMappings[$fieldName]['targetEntity']),
                         $class->getSingleAssociationReferencedJoinColumnName($fieldName)
@@ -541,8 +600,16 @@ class SchemaTool
      *
      * @throws \Doctrine\ORM\ORMException
      */
-    private function _gatherRelationJoinColumns($joinColumns, $theJoinTable, $class, $mapping, &$primaryKeyColumns, &$uniqueConstraints, &$addedFks, &$blacklistedFks)
+    private function gatherRelationJoinColumns(
-    {
+        $joinColumns,
+        $theJoinTable,
+        $class,
+        $mapping,
+        &$primaryKeyColumns,
+        &$uniqueConstraints,
+        &$addedFks,
+        &$blacklistedFks
+    ) {
         $localColumns       = array();
         $foreignColumns     = array();
         $fkOptions          = array();
@@ -550,7 +617,10 @@ class SchemaTool
 
         foreach ($joinColumns as $joinColumn) {
 
-            list($definingClass, $referencedFieldName) = $this->getDefiningClass($class, $joinColumn['referencedColumnName']);
+            list($definingClass, $referencedFieldName) = $this->getDefiningClass(
+                $class,
+                $joinColumn['referencedColumnName']
+            );
 
             if ( ! $definingClass) {
                 throw new \Doctrine\ORM\ORMException(
@@ -560,7 +630,11 @@ class SchemaTool
             }
 
             $quotedColumnName       = $this->quoteStrategy->getJoinColumnName($joinColumn, $class, $this->platform);
-            $quotedRefColumnName    = $this->quoteStrategy->getReferencedJoinColumnName($joinColumn, $class, $this->platform);
+            $quotedRefColumnName    = $this->quoteStrategy->getReferencedJoinColumnName(
+                $joinColumn,
+                $class,
+                $this->platform
+            );
 
             $primaryKeyColumns[]    = $quotedColumnName;
             $localColumns[]         = $quotedColumnName;
@@ -627,7 +701,10 @@ class SchemaTool
         } elseif (!isset($blacklistedFks[$compositeName])) {
             $addedFks[$compositeName] = array('foreignTableName' => $foreignTableName, 'foreignColumns' => $foreignColumns);
             $theJoinTable->addUnnamedForeignKeyConstraint(
-                $foreignTableName, $localColumns, $foreignColumns, $fkOptions
+                $foreignTableName,
+                $localColumns,
+                $foreignColumns,
+                $fkOptions
             );
         }
     }

tests/Doctrine/Tests/Models/CompositeKeyInheritance/JoinedChildClass.php

@@ -0,0 +1,21 @@
+<?php
+namespace Doctrine\Tests\Models\CompositeKeyInheritance;
+
+/**
+ * @Entity
+ */
+class JoinedChildClass extends JoinedRootClass
+{
+    /**
+     * @var string
+     * @Column(type="string")
+     */
+    public $extension = 'ext';
+
+    /**
+     * @var string
+     * @Column(type="string")
+     * @Id
+     */
+    private $additionalId = 'additional';
+}

tests/Doctrine/Tests/Models/CompositeKeyInheritance/JoinedRootClass.php

@@ -0,0 +1,25 @@
+<?php
+namespace Doctrine\Tests\Models\CompositeKeyInheritance;
+
+/**
+ * @Entity
+ * @InheritanceType("JOINED")
+ * @DiscriminatorColumn(name="discr", type="string")
+ * @DiscriminatorMap({"child" = "JoinedChildClass",})
+ */
+class JoinedRootClass
+{
+    /**
+     * @var string
+     * @Column(type="string")
+     * @Id
+     */
+    protected $keyPart1 = 'part-1';
+
+    /**
+     * @var string
+     * @Column(type="string")
+     * @Id
+     */
+    protected $keyPart2 = 'part-2';
+}

tests/Doctrine/Tests/Models/CompositeKeyInheritance/SingleChildClass.php

@@ -0,0 +1,21 @@
+<?php
+namespace Doctrine\Tests\Models\CompositeKeyInheritance;
+
+/**
+ * @Entity
+ */
+class SingleChildClass extends SingleRootClass
+{
+    /**
+     * @var string
+     * @Column(type="string")
+     */
+    public $extension = 'ext';
+
+    /**
+     * @var string
+     * @Column(type="string")
+     * @Id
+     */
+    private $additionalId = 'additional';
+}

tests/Doctrine/Tests/Models/CompositeKeyInheritance/SingleRootClass.php

@@ -0,0 +1,25 @@
+<?php
+namespace Doctrine\Tests\Models\CompositeKeyInheritance;
+
+/**
+ * @Entity
+ * @InheritanceType("SINGLE_TABLE")
+ * @DiscriminatorColumn(name="discr", type="string")
+ * @DiscriminatorMap({"child" = "SingleChildClass",})
+ */
+class SingleRootClass
+{
+    /**
+     * @var string
+     * @Column(type="string")
+     * @Id
+     */
+    protected $keyPart1 = 'part-1';
+
+    /**
+     * @var string
+     * @Column(type="string")
+     * @Id
+     */
+    protected $keyPart2 = 'part-2';
+}

tests/Doctrine/Tests/ORM/Functional/JoinedTableCompositeKeyTest.php

@@ -0,0 +1,64 @@
+<?php
+namespace Doctrine\Tests\ORM\Functional;
+
+use Doctrine\Tests\OrmFunctionalTestCase;
+use Doctrine\Tests\Models\CompositeKeyInheritance\JoinedChildClass;
+
+class JoinedTableCompositeKeyTest extends OrmFunctionalTestCase
+{
+
+    public function setUp()
+    {
+        $this->useModelSet('compositekeyinheritance');
+        parent::setUp();
+
+    }
+
+    /**
+     *
+     */
+    public function testInsertWithCompositeKey()
+    {
+        $childEntity = new JoinedChildClass();
+        $this->_em->persist($childEntity);
+        $this->_em->flush();
+
+        $this->_em->clear();
+
+        $entity = $this->findEntity();
+        $this->assertEquals($childEntity, $entity);
+    }
+
+    /**
+     *
+     */
+    public function testUpdateWithCompositeKey()
+    {
+        $childEntity = new JoinedChildClass();
+        $this->_em->persist($childEntity);
+        $this->_em->flush();
+
+        $this->_em->clear();
+
+        $entity = $this->findEntity();
+        $entity->extension = 'ext-new';
+        $this->_em->persist($entity);
+        $this->_em->flush();
+
+        $this->_em->clear();
+
+        $persistedEntity = $this->findEntity();
+        $this->assertEquals($entity, $persistedEntity);
+    }
+
+    /**
+     * @return \Doctrine\Tests\Models\CompositeKeyInheritance\JoinedChildClass
+     */
+    private function findEntity()
+    {
+        return $this->_em->find(
+            'Doctrine\Tests\Models\CompositeKeyInheritance\JoinedRootClass',
+            array('keyPart1' => 'part-1', 'keyPart2' => 'part-2')
+        );
+    }
+}

tests/Doctrine/Tests/ORM/Functional/SingleTableCompositeKeyTest.php

@@ -0,0 +1,64 @@
+<?php
+namespace Doctrine\Tests\ORM\Functional;
+
+use Doctrine\Tests\OrmFunctionalTestCase;
+use Doctrine\Tests\Models\CompositeKeyInheritance\SingleChildClass;
+
+class SingleTableCompositeKeyTest extends OrmFunctionalTestCase
+{
+
+    public function setUp()
+    {
+        $this->useModelSet('compositekeyinheritance');
+        parent::setUp();
+
+    }
+
+    /**
+     *
+     */
+    public function testInsertWithCompositeKey()
+    {
+        $childEntity = new SingleChildClass();
+        $this->_em->persist($childEntity);
+        $this->_em->flush();
+
+        $this->_em->clear();
+
+        $entity = $this->findEntity();
+        $this->assertEquals($childEntity, $entity);
+    }
+
+    /**
+     *
+     */
+    public function testUpdateWithCompositeKey()
+    {
+        $childEntity = new SingleChildClass();
+        $this->_em->persist($childEntity);
+        $this->_em->flush();
+
+        $this->_em->clear();
+
+        $entity = $this->findEntity();
+        $entity->extension = 'ext-new';
+        $this->_em->persist($entity);
+        $this->_em->flush();
+
+        $this->_em->clear();
+
+        $persistedEntity = $this->findEntity();
+        $this->assertEquals($entity, $persistedEntity);
+    }
+
+    /**
+     * @return \Doctrine\Tests\Models\CompositeKeyInheritance\JoinedChildClass
+     */
+    private function findEntity()
+    {
+        return $this->_em->find(
+            'Doctrine\Tests\Models\CompositeKeyInheritance\SingleRootClass',
+            array('keyPart1' => 'part-1', 'keyPart2' => 'part-2')
+        );
+    }
+}

tests/Doctrine/Tests/OrmFunctionalTestCase.php

@@ -150,6 +150,12 @@ abstract class OrmFunctionalTestCase extends OrmTestCase
             'Doctrine\Tests\Models\CustomType\CustomTypeParent',
             'Doctrine\Tests\Models\CustomType\CustomTypeUpperCase',
         ),
+        'compositekeyinheritance' => array(
+            'Doctrine\Tests\Models\CompositeKeyInheritance\JoinedRootClass',
+            'Doctrine\Tests\Models\CompositeKeyInheritance\JoinedChildClass',
+            'Doctrine\Tests\Models\CompositeKeyInheritance\SingleRootClass',
+            'Doctrine\Tests\Models\CompositeKeyInheritance\SingleChildClass',
+        )
     );
 
     /**
@@ -272,6 +278,13 @@ abstract class OrmFunctionalTestCase extends OrmTestCase
             $conn->executeUpdate('DELETE FROM customtype_uppercases');
         }
 
+        if (isset($this->_usedModelSets['compositekeyinheritance'])) {
+            $conn->executeUpdate('DELETE FROM JoinedChildClass');
+            $conn->executeUpdate('DELETE FROM JoinedRootClass');
+            $conn->executeUpdate('DELETE FROM SingleRootClass');
+        }
+
+
         $this->_em->clear();
     }