From bb8970286d9356ab6da92e83efcb9a78cb234aea Mon Sep 17 00:00:00 2001 From: Tyler Romeo Date: Sun, 27 Aug 2017 02:07:48 -0400 Subject: [PATCH] Allow association mappings as IDs for joined-table inherited entity SchemaTool has custom logic for creating the primary key of a joined-table inherited entity. This logic overlooked association maps as a possible source for identity columns, resulting in a fatal error when fetching the primary key list for child entities. Removed any custom logic for generating primary keys for root entities in joined-table inheritance, deferring to the common logic used for other entities. Also adjusted the child entity logic, scanning association maps for identity columns, and including the column as appropriate. It also ensures that the primary key columns are in the correct order. --- lib/Doctrine/ORM/Tools/SchemaTool.php | 80 ++++++++++++------- .../JoinedDerivedChildClass.php | 22 +++++ .../JoinedDerivedIdentityClass.php | 25 ++++++ .../JoinedDerivedRootClass.php | 29 +++++++ .../Tests/ORM/Tools/SchemaToolTest.php | 47 +++++++++++ 5 files changed, 174 insertions(+), 29 deletions(-) create mode 100644 tests/Doctrine/Tests/Models/CompositeKeyInheritance/JoinedDerivedChildClass.php create mode 100644 tests/Doctrine/Tests/Models/CompositeKeyInheritance/JoinedDerivedIdentityClass.php create mode 100644 tests/Doctrine/Tests/Models/CompositeKeyInheritance/JoinedDerivedRootClass.php diff --git a/lib/Doctrine/ORM/Tools/SchemaTool.php b/lib/Doctrine/ORM/Tools/SchemaTool.php index 5a3eb32ab..2a858c493 100644 --- a/lib/Doctrine/ORM/Tools/SchemaTool.php +++ b/lib/Doctrine/ORM/Tools/SchemaTool.php @@ -183,19 +183,9 @@ class SchemaTool } } elseif ($class->isInheritanceTypeJoined()) { // Add all non-inherited fields as columns - $pkColumns = []; foreach ($class->fieldMappings as $fieldName => $mapping) { if ( ! isset($mapping['inherited'])) { - $columnName = $this->quoteStrategy->getColumnName( - $mapping['fieldName'], - $class, - $this->platform - ); $this->gatherColumn($class, $mapping, $table); - - if ($class->isIdentifier($fieldName)) { - $pkColumns[] = $columnName; - } } } @@ -206,10 +196,12 @@ class SchemaTool $this->addDiscriminatorColumnDefinition($class, $table); } else { // Add an ID FK column to child tables + $pkColumns = []; $inheritedKeyColumns = []; + foreach ($class->identifier as $identifierField) { - $idMapping = $class->fieldMappings[$identifierField]; - if (isset($idMapping['inherited'])) { + if (isset($class->fieldMappings[$identifierField]['inherited'])) { + $idMapping = $class->fieldMappings[$identifierField]; $this->gatherColumn($class, $idMapping, $table); $columnName = $this->quoteStrategy->getColumnName( $identifierField, @@ -221,9 +213,39 @@ class SchemaTool $pkColumns[] = $columnName; $inheritedKeyColumns[] = $columnName; + + continue; + } + + if (isset($class->associationMappings[$identifierField]['inherited'])) { + $idMapping = $class->associationMappings[$identifierField]; + + $targetEntity = current( + array_filter( + $classes, + function (ClassMetadata $class) use ($idMapping) : bool { + return $class->name === $idMapping['targetEntity']; + } + ) + ); + + foreach ($idMapping['joinColumns'] as $joinColumn) { + if (isset($targetEntity->fieldMappings[$joinColumn['referencedColumnName']])) { + $idMapping = $targetEntity->fieldMappings[$joinColumn['referencedColumnName']]; + $columnName = $this->quoteStrategy->getJoinColumnName( + $joinColumn, + $class, + $this->platform + ); + + $pkColumns[] = $columnName; + $inheritedKeyColumns[] = $columnName; + } + } } } - if (!empty($inheritedKeyColumns)) { + + if ( ! empty($inheritedKeyColumns)) { // Add a FK constraint on the ID column $table->addForeignKeyConstraint( $this->quoteStrategy->getTableName( @@ -236,10 +258,10 @@ class SchemaTool ); } + if ( ! empty($pkColumns)) { + $table->setPrimaryKey($pkColumns); + } } - - $table->setPrimaryKey($pkColumns); - } elseif ($class->isInheritanceTypeTablePerClass()) { throw ORMException::notSupported(); } else { @@ -330,7 +352,7 @@ class SchemaTool } } - if ( ! $this->platform->supportsSchemas() && ! $this->platform->canEmulateSchemas() ) { + if ( ! $this->platform->supportsSchemas() && ! $this->platform->canEmulateSchemas()) { $schema->visit(new RemoveNamespacedAssets()); } @@ -496,8 +518,8 @@ class SchemaTool */ private function gatherRelationsSql($class, $table, $schema, &$addedFks, &$blacklistedFks) { - foreach ($class->associationMappings as $mapping) { - if (isset($mapping['inherited'])) { + foreach ($class->associationMappings as $id => $mapping) { + if (isset($mapping['inherited']) && array_search($id, $class->identifier) === false) { continue; } @@ -633,21 +655,21 @@ class SchemaTool if ( ! $definingClass) { throw new \Doctrine\ORM\ORMException( - "Column name `".$joinColumn['referencedColumnName']."` referenced for relation from ". - $mapping['sourceEntity'] . " towards ". $mapping['targetEntity'] . " does not exist." + 'Column name `' . $joinColumn['referencedColumnName'] . '` referenced for relation from ' + . $mapping['sourceEntity'] . ' towards ' . $mapping['targetEntity'] . ' does not exist.' ); } - $quotedColumnName = $this->quoteStrategy->getJoinColumnName($joinColumn, $class, $this->platform); - $quotedRefColumnName = $this->quoteStrategy->getReferencedJoinColumnName( + $quotedColumnName = $this->quoteStrategy->getJoinColumnName($joinColumn, $class, $this->platform); + $quotedRefColumnName = $this->quoteStrategy->getReferencedJoinColumnName( $joinColumn, $class, $this->platform ); - $primaryKeyColumns[] = $quotedColumnName; - $localColumns[] = $quotedColumnName; - $foreignColumns[] = $quotedRefColumnName; + $primaryKeyColumns[] = $quotedColumnName; + $localColumns[] = $quotedColumnName; + $foreignColumns[] = $quotedRefColumnName; if ( ! $theJoinTable->hasColumn($quotedColumnName)) { // Only add the column to the table if it does not exist already. @@ -666,7 +688,7 @@ class SchemaTool $columnOptions = ['notnull' => false, 'columnDefinition' => $columnDef]; if (isset($joinColumn['nullable'])) { - $columnOptions['notnull'] = !$joinColumn['nullable']; + $columnOptions['notnull'] = ! $joinColumn['nullable']; } if (isset($fieldMapping['options'])) { @@ -713,7 +735,7 @@ class SchemaTool } } $blacklistedFks[$compositeName] = true; - } elseif (!isset($blacklistedFks[$compositeName])) { + } elseif ( ! isset($blacklistedFks[$compositeName])) { $addedFks[$compositeName] = ['foreignTableName' => $foreignTableName, 'foreignColumns' => $foreignColumns]; $theJoinTable->addUnnamedForeignKeyConstraint( $foreignTableName, @@ -820,7 +842,7 @@ class SchemaTool if ($table->hasPrimaryKey()) { $columns = $table->getPrimaryKey()->getColumns(); if (count($columns) == 1) { - $checkSequence = $table->getName() . "_" . $columns[0] . "_seq"; + $checkSequence = $table->getName() . '_' . $columns[0] . '_seq'; if ($fullSchema->hasSequence($checkSequence)) { $visitor->acceptSequence($fullSchema->getSequence($checkSequence)); } diff --git a/tests/Doctrine/Tests/Models/CompositeKeyInheritance/JoinedDerivedChildClass.php b/tests/Doctrine/Tests/Models/CompositeKeyInheritance/JoinedDerivedChildClass.php new file mode 100644 index 000000000..8b4836199 --- /dev/null +++ b/tests/Doctrine/Tests/Models/CompositeKeyInheritance/JoinedDerivedChildClass.php @@ -0,0 +1,22 @@ +assertEquals(255, $column->getLength()); } + + public function testDerivedCompositeKey() : void + { + $em = $this->_getTestEntityManager(); + $schemaTool = new SchemaTool($em); + + $schema = $schemaTool->getSchemaFromMetadata( + [ + $em->getClassMetadata(JoinedDerivedIdentityClass::class), + $em->getClassMetadata(JoinedDerivedRootClass::class), + $em->getClassMetadata(JoinedDerivedChildClass::class), + ] + ); + + self::assertTrue($schema->hasTable('joined_derived_identity')); + self::assertTrue($schema->hasTable('joined_derived_root')); + self::assertTrue($schema->hasTable('joined_derived_child')); + + $rootTable = $schema->getTable('joined_derived_root'); + self::assertNotNull($rootTable->getPrimaryKey()); + self::assertSame(['keyPart1_id', 'keyPart2'], $rootTable->getPrimaryKey()->getColumns()); + + $childTable = $schema->getTable('joined_derived_child'); + self::assertNotNull($childTable->getPrimaryKey()); + self::assertSame(['keyPart1_id', 'keyPart2'], $childTable->getPrimaryKey()->getColumns()); + + $childTableForeignKeys = $childTable->getForeignKeys(); + + self::assertCount(2, $childTableForeignKeys); + + $expectedColumns = [ + 'joined_derived_identity' => [['keyPart1_id'], ['id']], + 'joined_derived_root' => [['keyPart1_id', 'keyPart2'], ['keyPart1_id', 'keyPart2']], + ]; + + foreach ($childTableForeignKeys as $foreignKey) { + self::assertArrayHasKey($foreignKey->getForeignTableName(), $expectedColumns); + + [$localColumns, $foreignColumns] = $expectedColumns[$foreignKey->getForeignTableName()]; + + self::assertSame($localColumns, $foreignKey->getLocalColumns()); + self::assertSame($foreignColumns, $foreignKey->getForeignColumns()); + } + } } /**