From bb8970286d9356ab6da92e83efcb9a78cb234aea Mon Sep 17 00:00:00 2001 From: Tyler Romeo Date: Sun, 27 Aug 2017 02:07:48 -0400 Subject: [PATCH 1/3] 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()); + } + } } /** From 27c42d418b95d5783cbd279dd79967d1510a3ff5 Mon Sep 17 00:00:00 2001 From: Marco Pivetta Date: Tue, 19 Dec 2017 18:00:03 +0100 Subject: [PATCH 2/3] #6767 removed unused variable Ref: https://github.com/doctrine/doctrine2/pull/6767/files#r157354726 --- lib/Doctrine/ORM/Tools/SchemaTool.php | 1 - 1 file changed, 1 deletion(-) diff --git a/lib/Doctrine/ORM/Tools/SchemaTool.php b/lib/Doctrine/ORM/Tools/SchemaTool.php index 2a858c493..1e8acaa3d 100644 --- a/lib/Doctrine/ORM/Tools/SchemaTool.php +++ b/lib/Doctrine/ORM/Tools/SchemaTool.php @@ -231,7 +231,6 @@ class SchemaTool foreach ($idMapping['joinColumns'] as $joinColumn) { if (isset($targetEntity->fieldMappings[$joinColumn['referencedColumnName']])) { - $idMapping = $targetEntity->fieldMappings[$joinColumn['referencedColumnName']]; $columnName = $this->quoteStrategy->getJoinColumnName( $joinColumn, $class, From b6aa4bab15f231ea9ed8d31b516f3f657bc807de Mon Sep 17 00:00:00 2001 From: Marco Pivetta Date: Tue, 19 Dec 2017 18:01:38 +0100 Subject: [PATCH 3/3] #6767 using `in_array` rather than `array_search` Ref: https://github.com/doctrine/doctrine2/pull/6767/files#r157355050 --- lib/Doctrine/ORM/Tools/SchemaTool.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/Doctrine/ORM/Tools/SchemaTool.php b/lib/Doctrine/ORM/Tools/SchemaTool.php index 1e8acaa3d..1950446e7 100644 --- a/lib/Doctrine/ORM/Tools/SchemaTool.php +++ b/lib/Doctrine/ORM/Tools/SchemaTool.php @@ -518,7 +518,7 @@ class SchemaTool private function gatherRelationsSql($class, $table, $schema, &$addedFks, &$blacklistedFks) { foreach ($class->associationMappings as $id => $mapping) { - if (isset($mapping['inherited']) && array_search($id, $class->identifier) === false) { + if (isset($mapping['inherited']) && ! \in_array($id, $class->identifier, true)) { continue; }