From 75c4ded64d96b22e7732fc108a3546c7980efee4 Mon Sep 17 00:00:00 2001 From: Benjamin Eberlei Date: Tue, 24 Nov 2020 09:17:54 +0100 Subject: [PATCH 01/10] [GH-8229] Prevent AttributeOverride on fields from entities, only allowed for MappedSuperclass --- lib/Doctrine/ORM/Mapping/ClassMetadataInfo.php | 4 ++++ lib/Doctrine/ORM/Mapping/MappingException.php | 12 ++++++++++++ tests/Doctrine/Tests/Models/DDC964/DDC964User.php | 2 ++ .../php/Doctrine.Tests.Models.DDC964.DDC964User.php | 2 ++ 4 files changed, 20 insertions(+) diff --git a/lib/Doctrine/ORM/Mapping/ClassMetadataInfo.php b/lib/Doctrine/ORM/Mapping/ClassMetadataInfo.php index 0fc8980c8c9..1e56729c72e 100644 --- a/lib/Doctrine/ORM/Mapping/ClassMetadataInfo.php +++ b/lib/Doctrine/ORM/Mapping/ClassMetadataInfo.php @@ -2251,6 +2251,10 @@ public function setAttributeOverride($fieldName, array $overrideMapping) $mapping = $this->fieldMappings[$fieldName]; + if (isset($fieldMappings[$fieldName]['inherited'])) { + throw MappingException::illegalOverrideOfInheritedColumn($this->name, $fieldName); + } + if (isset($mapping['id'])) { $overrideMapping['id'] = $mapping['id']; } diff --git a/lib/Doctrine/ORM/Mapping/MappingException.php b/lib/Doctrine/ORM/Mapping/MappingException.php index 7dc4405159a..1bf34042e56 100644 --- a/lib/Doctrine/ORM/Mapping/MappingException.php +++ b/lib/Doctrine/ORM/Mapping/MappingException.php @@ -820,4 +820,16 @@ public static function infiniteEmbeddableNesting($className, $propertyName) ) ); } + + public static function illegalOverrideOfInheritedColumn($className, $propertyName) + { + return new self( + sprintf( + 'Attribute Overrides in %s::%s is only allowed for attributes declared on ' . + 'a mapped superclass.', + $className, + $propertyName + ) + ); + } } diff --git a/tests/Doctrine/Tests/Models/DDC964/DDC964User.php b/tests/Doctrine/Tests/Models/DDC964/DDC964User.php index a9f15c8c948..b8f4b4130f8 100644 --- a/tests/Doctrine/Tests/Models/DDC964/DDC964User.php +++ b/tests/Doctrine/Tests/Models/DDC964/DDC964User.php @@ -109,6 +109,8 @@ public function setAddress(DDC964Address $address) public static function loadMetadata(\Doctrine\ORM\Mapping\ClassMetadataInfo $metadata) { + $metadata->isMappedSuperclass = true; + $metadata->mapField( [ 'id' => true, diff --git a/tests/Doctrine/Tests/ORM/Mapping/php/Doctrine.Tests.Models.DDC964.DDC964User.php b/tests/Doctrine/Tests/ORM/Mapping/php/Doctrine.Tests.Models.DDC964.DDC964User.php index dc445f1ca6c..6e6172bfafb 100644 --- a/tests/Doctrine/Tests/ORM/Mapping/php/Doctrine.Tests.Models.DDC964.DDC964User.php +++ b/tests/Doctrine/Tests/ORM/Mapping/php/Doctrine.Tests.Models.DDC964.DDC964User.php @@ -2,6 +2,8 @@ use Doctrine\ORM\Mapping\ClassMetadataInfo; +$metadata->isMappedSuperclass = true; + $metadata->mapField( [ 'id' => true, From e8ab2dd16dfec915acae5b85cb0da138d745c323 Mon Sep 17 00:00:00 2001 From: Benjamin Eberlei Date: Tue, 24 Nov 2020 09:20:14 +0100 Subject: [PATCH 02/10] [GH-8229] Prevent AssociationOverride on fields from entities, only allowed for MappedSuperclass --- lib/Doctrine/ORM/Mapping/ClassMetadataInfo.php | 6 +++++- lib/Doctrine/ORM/Mapping/MappingException.php | 6 +++--- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/lib/Doctrine/ORM/Mapping/ClassMetadataInfo.php b/lib/Doctrine/ORM/Mapping/ClassMetadataInfo.php index 1e56729c72e..d758432e241 100644 --- a/lib/Doctrine/ORM/Mapping/ClassMetadataInfo.php +++ b/lib/Doctrine/ORM/Mapping/ClassMetadataInfo.php @@ -2193,6 +2193,10 @@ public function setAssociationOverride($fieldName, array $overrideMapping) $mapping = $this->associationMappings[$fieldName]; + if (isset($fieldMappings[$fieldName]['inherited'])) { + throw MappingException::illegalOverrideOfInheritedProperty($this->name, $fieldName); + } + if (isset($overrideMapping['joinColumns'])) { $mapping['joinColumns'] = $overrideMapping['joinColumns']; } @@ -2252,7 +2256,7 @@ public function setAttributeOverride($fieldName, array $overrideMapping) $mapping = $this->fieldMappings[$fieldName]; if (isset($fieldMappings[$fieldName]['inherited'])) { - throw MappingException::illegalOverrideOfInheritedColumn($this->name, $fieldName); + throw MappingException::illegalOverrideOfInheritedProperty($this->name, $fieldName); } if (isset($mapping['id'])) { diff --git a/lib/Doctrine/ORM/Mapping/MappingException.php b/lib/Doctrine/ORM/Mapping/MappingException.php index 1bf34042e56..480d41890a1 100644 --- a/lib/Doctrine/ORM/Mapping/MappingException.php +++ b/lib/Doctrine/ORM/Mapping/MappingException.php @@ -821,12 +821,12 @@ public static function infiniteEmbeddableNesting($className, $propertyName) ); } - public static function illegalOverrideOfInheritedColumn($className, $propertyName) + public static function illegalOverrideOfInheritedProperty($className, $propertyName) { return new self( sprintf( - 'Attribute Overrides in %s::%s is only allowed for attributes declared on ' . - 'a mapped superclass.', + 'Override for %s::%s is only allowed for attributes/associations ' . + 'declared on a mapped superclass.', $className, $propertyName ) From d85efb0b7b80ef25aab496ae6daff69ee6c4a002 Mon Sep 17 00:00:00 2001 From: Benjamin Eberlei Date: Tue, 24 Nov 2020 17:50:08 +0100 Subject: [PATCH 03/10] Revert "Fix SQL alias generation regression for simple inheritance (#8329)" This reverts commit f4ebded63c04a0b27c4be5d0f65bee8b034477e7. --- .../Entity/BasicEntityPersister.php | 2 +- .../ORM/Functional/Ticket/GH8229Test.php | 41 ------------------- 2 files changed, 1 insertion(+), 42 deletions(-) diff --git a/lib/Doctrine/ORM/Persisters/Entity/BasicEntityPersister.php b/lib/Doctrine/ORM/Persisters/Entity/BasicEntityPersister.php index 163be61248a..ec08ac839e6 100644 --- a/lib/Doctrine/ORM/Persisters/Entity/BasicEntityPersister.php +++ b/lib/Doctrine/ORM/Persisters/Entity/BasicEntityPersister.php @@ -1698,7 +1698,7 @@ private function getSelectConditionStatementColumnSQL($field, $assoc = null) if (isset($this->class->fieldMappings[$field])) { // Fix for bug GH-8229 (id column from parent class renamed in child class): // Use the correct metadata and name for the id column - if (isset($this->class->fieldMappings[$field]['inherited']) && $this->class->inheritanceType !== ClassMetadata::INHERITANCE_TYPE_NONE) { + if (isset($this->class->fieldMappings[$field]['inherited'])) { $className = $this->class->fieldMappings[$field]['inherited']; $class = $this->em->getClassMetadata($className); } else { diff --git a/tests/Doctrine/Tests/ORM/Functional/Ticket/GH8229Test.php b/tests/Doctrine/Tests/ORM/Functional/Ticket/GH8229Test.php index c03e7edfc89..e32ff11c59e 100644 --- a/tests/Doctrine/Tests/ORM/Functional/Ticket/GH8229Test.php +++ b/tests/Doctrine/Tests/ORM/Functional/Ticket/GH8229Test.php @@ -23,8 +23,6 @@ protected function setUp() : void [ GH8229Resource::class, GH8229User::class, - GH8229EntityWithoutDiscriminator::class, - EntityExtendingGH8229EntityWithoutDiscriminator::class, ] ); } @@ -185,19 +183,6 @@ public function testCorrectColumnNamesInPessimisticWriteLock() self::assertEquals($identifier, $entity->id); } - - /** this test check alias generation not altered by inheritance in ResolvedTargetEntities mapped classes - */ - public function testBasicEntityPersisterAliasGenerationWithSimpleInheritance() - { - $this->expectNotToPerformAssertions(); - - $entity = new EntityExtendingGH8229EntityWithoutDiscriminator(); - $this->_em->persist($entity); - $this->_em->flush(); - - $this->_em->getRepository(EntityExtendingGH8229EntityWithoutDiscriminator::class)->findOneBy(['id' => $entity->getId()]); - } } @@ -259,29 +244,3 @@ public function __construct($username) $this->username = $username; } } - -/** - * @Entity - * @Table(name="gh8229_replacedentity") - */ -class GH8229EntityWithoutDiscriminator -{ - /** - * @Id - * @GeneratedValue - * @Column(type="integer") - */ - public $id; - - public function getId() - { - return $this->id; - } -} - -/** - * @Entity - */ -final class EntityExtendingGH8229EntityWithoutDiscriminator extends GH8229EntityWithoutDiscriminator -{ -} From 52b780a03f11294a0dc91abd536c332696deb22e Mon Sep 17 00:00:00 2001 From: Benjamin Eberlei Date: Tue, 24 Nov 2020 18:16:38 +0100 Subject: [PATCH 04/10] [GH-8229] Finalize checks for illegal attribute/assocation overrides. --- lib/Doctrine/ORM/Mapping/ClassMetadataInfo.php | 6 ++++-- lib/Doctrine/ORM/Mapping/MappingException.php | 2 +- tests/Doctrine/Tests/Models/DDC3579/DDC3579User.php | 2 ++ tests/Doctrine/Tests/ORM/Functional/Ticket/GH8229Test.php | 1 + .../php/Doctrine.Tests.Models.DDC3579.DDC3579User.php | 2 ++ 5 files changed, 10 insertions(+), 3 deletions(-) diff --git a/lib/Doctrine/ORM/Mapping/ClassMetadataInfo.php b/lib/Doctrine/ORM/Mapping/ClassMetadataInfo.php index d758432e241..1fa50dde9b0 100644 --- a/lib/Doctrine/ORM/Mapping/ClassMetadataInfo.php +++ b/lib/Doctrine/ORM/Mapping/ClassMetadataInfo.php @@ -2193,7 +2193,9 @@ public function setAssociationOverride($fieldName, array $overrideMapping) $mapping = $this->associationMappings[$fieldName]; - if (isset($fieldMappings[$fieldName]['inherited'])) { + if (isset($mapping['inherited']) && (count($overrideMapping) !== 1 || !isset($overrideMapping['fetch']))) { + // TODO: Deprecate overriding the fetch mode via association override for 3.0, + // users should do this with a listener and a custom attribute/annotation throw MappingException::illegalOverrideOfInheritedProperty($this->name, $fieldName); } @@ -2255,7 +2257,7 @@ public function setAttributeOverride($fieldName, array $overrideMapping) $mapping = $this->fieldMappings[$fieldName]; - if (isset($fieldMappings[$fieldName]['inherited'])) { + if (isset($mapping['inherited'])) { throw MappingException::illegalOverrideOfInheritedProperty($this->name, $fieldName); } diff --git a/lib/Doctrine/ORM/Mapping/MappingException.php b/lib/Doctrine/ORM/Mapping/MappingException.php index 480d41890a1..130eb468e75 100644 --- a/lib/Doctrine/ORM/Mapping/MappingException.php +++ b/lib/Doctrine/ORM/Mapping/MappingException.php @@ -826,7 +826,7 @@ public static function illegalOverrideOfInheritedProperty($className, $propertyN return new self( sprintf( 'Override for %s::%s is only allowed for attributes/associations ' . - 'declared on a mapped superclass.', + 'declared on a mapped superclass or a trait.', $className, $propertyName ) diff --git a/tests/Doctrine/Tests/Models/DDC3579/DDC3579User.php b/tests/Doctrine/Tests/Models/DDC3579/DDC3579User.php index 8b6193cdc3f..000b977f628 100644 --- a/tests/Doctrine/Tests/Models/DDC3579/DDC3579User.php +++ b/tests/Doctrine/Tests/Models/DDC3579/DDC3579User.php @@ -81,6 +81,8 @@ public function getGroups() public static function loadMetadata($metadata) { + $metadata->isMappedSuperclass = true; + $metadata->mapField( [ 'id' => true, diff --git a/tests/Doctrine/Tests/ORM/Functional/Ticket/GH8229Test.php b/tests/Doctrine/Tests/ORM/Functional/Ticket/GH8229Test.php index e32ff11c59e..83ef821796e 100644 --- a/tests/Doctrine/Tests/ORM/Functional/Ticket/GH8229Test.php +++ b/tests/Doctrine/Tests/ORM/Functional/Ticket/GH8229Test.php @@ -74,6 +74,7 @@ public function testCorrectColumnNameInParentClassAfterAttributeOveride() $this->_em->clear(); } + /** * This test checks if foreign keys are generated for the schema, when a joined * table inheritance is used and the identifier columns in the inheriting class diff --git a/tests/Doctrine/Tests/ORM/Mapping/php/Doctrine.Tests.Models.DDC3579.DDC3579User.php b/tests/Doctrine/Tests/ORM/Mapping/php/Doctrine.Tests.Models.DDC3579.DDC3579User.php index 49aeacb1eee..d5f22817276 100644 --- a/tests/Doctrine/Tests/ORM/Mapping/php/Doctrine.Tests.Models.DDC3579.DDC3579User.php +++ b/tests/Doctrine/Tests/ORM/Mapping/php/Doctrine.Tests.Models.DDC3579.DDC3579User.php @@ -2,6 +2,8 @@ use Doctrine\ORM\Mapping\ClassMetadataInfo; +$metadata->isMappedSuperclass = true; + $metadata->mapField( [ 'id' => true, From d70f5024912deeefe148a1a607a7aa6aee980a96 Mon Sep 17 00:00:00 2001 From: Benjamin Eberlei Date: Tue, 24 Nov 2020 18:18:52 +0100 Subject: [PATCH 05/10] [GH-8229] Revert ccae8f717684dcd871b418444d7ca7155c677169 PR #8234 --- .../ORM/Mapping/ClassMetadataInfo.php | 7 - .../Entity/BasicEntityPersister.php | 50 +--- .../Entity/JoinedSubclassPersister.php | 71 ++--- lib/Doctrine/ORM/Query/SqlWalker.php | 38 +-- lib/Doctrine/ORM/Tools/SchemaTool.php | 11 +- .../ORM/Functional/Ticket/GH8229Test.php | 247 ------------------ 6 files changed, 48 insertions(+), 376 deletions(-) delete mode 100644 tests/Doctrine/Tests/ORM/Functional/Ticket/GH8229Test.php diff --git a/lib/Doctrine/ORM/Mapping/ClassMetadataInfo.php b/lib/Doctrine/ORM/Mapping/ClassMetadataInfo.php index 1fa50dde9b0..6e95dc6046b 100644 --- a/lib/Doctrine/ORM/Mapping/ClassMetadataInfo.php +++ b/lib/Doctrine/ORM/Mapping/ClassMetadataInfo.php @@ -30,7 +30,6 @@ use ReflectionClass; use ReflectionProperty; use RuntimeException; -use function array_key_exists; use function explode; /** @@ -2277,12 +2276,6 @@ public function setAttributeOverride($fieldName, array $overrideMapping) throw MappingException::invalidOverrideFieldType($this->name, $fieldName); } - // Fix for bug GH-8229 (id column from parent class renamed in child class): - // The contained 'inherited' information was accidentally deleted by the unset() call below. - if (array_key_exists('inherited', $this->fieldMappings[$fieldName])) { - $overrideMapping['inherited'] = $this->fieldMappings[$fieldName]['inherited']; - } - unset($this->fieldMappings[$fieldName]); unset($this->fieldNames[$mapping['columnName']]); unset($this->columnNames[$mapping['fieldName']]); diff --git a/lib/Doctrine/ORM/Persisters/Entity/BasicEntityPersister.php b/lib/Doctrine/ORM/Persisters/Entity/BasicEntityPersister.php index ec08ac839e6..4ac7b2cb91c 100644 --- a/lib/Doctrine/ORM/Persisters/Entity/BasicEntityPersister.php +++ b/lib/Doctrine/ORM/Persisters/Entity/BasicEntityPersister.php @@ -37,7 +37,6 @@ use Doctrine\ORM\UnitOfWork; use Doctrine\ORM\Utility\IdentifierFlattener; use Doctrine\ORM\Utility\PersisterHelper; -use function array_key_exists; use function array_map; use function array_merge; use function assert; @@ -448,26 +447,14 @@ protected final function updateTable($entity, $quotedTableName, array $updateDat $types[] = $this->columnTypes[$columnName]; } - $where = []; - $identifier = $this->em->getUnitOfWork()->getEntityIdentifier($entity); - $quotedClassTableName = $this->quoteStrategy->getTableName($this->class, $this->platform); + $where = []; + $identifier = $this->em->getUnitOfWork()->getEntityIdentifier($entity); foreach ($this->class->identifier as $idField) { if ( ! isset($this->class->associationMappings[$idField])) { - $params[] = $identifier[$idField]; - $types[] = $this->class->fieldMappings[$idField]['type']; - - // Fix for bug GH-8229 (id column from parent class renamed in child class): - // This method is called with the updated entity, but with different table names - // (the entity table name or a table name of an inherited entity). In dependence - // of the used table, the identifier name must be adjusted. - $class = $this->class; - if (isset($class->fieldMappings[$idField]['inherited']) && $quotedTableName !== $quotedClassTableName) { - $className = $this->class->fieldMappings[$idField]['inherited']; - $class = $this->em->getClassMetadata($className); - } - - $where[] = $this->quoteStrategy->getColumnName($idField, $class, $this->platform); + $params[] = $identifier[$idField]; + $types[] = $this->class->fieldMappings[$idField]['type']; + $where[] = $this->quoteStrategy->getColumnName($idField, $this->class, $this->platform); continue; } @@ -645,18 +632,7 @@ protected function prepareUpdateData($entity) $newVal = $change[1]; if ( ! isset($this->class->associationMappings[$field])) { - $class = $this->class; - - // Fix for bug GH-8229 (id column from parent class renamed in child class): - // Get the correct class metadata - foreach ($class->parentClasses as $parentClassName) { - $parentClass = $this->em->getClassMetadata($parentClassName); - if (array_key_exists($field, $parentClass->fieldMappings)) { - $class = $parentClass; - } - } - - $fieldMapping = $class->fieldMappings[$field]; + $fieldMapping = $this->class->fieldMappings[$field]; $columnName = $fieldMapping['columnName']; $this->columnTypes[$columnName] = $fieldMapping['type']; @@ -1696,17 +1672,11 @@ public function getSelectConditionStatementSQL($field, $value, $assoc = null, $c private function getSelectConditionStatementColumnSQL($field, $assoc = null) { if (isset($this->class->fieldMappings[$field])) { - // Fix for bug GH-8229 (id column from parent class renamed in child class): - // Use the correct metadata and name for the id column - if (isset($this->class->fieldMappings[$field]['inherited'])) { - $className = $this->class->fieldMappings[$field]['inherited']; - $class = $this->em->getClassMetadata($className); - } else { - $className = $this->class->name; - $class = $this->class; - } + $className = (isset($this->class->fieldMappings[$field]['inherited'])) + ? $this->class->fieldMappings[$field]['inherited'] + : $this->class->name; - return [$this->getSQLTableAlias($className) . '.' . $this->quoteStrategy->getColumnName($field, $class, $this->platform)]; + return [$this->getSQLTableAlias($className) . '.' . $this->quoteStrategy->getColumnName($field, $this->class, $this->platform)]; } if (isset($this->class->associationMappings[$field])) { diff --git a/lib/Doctrine/ORM/Persisters/Entity/JoinedSubclassPersister.php b/lib/Doctrine/ORM/Persisters/Entity/JoinedSubclassPersister.php index c37611e92d6..1f6d40133c7 100644 --- a/lib/Doctrine/ORM/Persisters/Entity/JoinedSubclassPersister.php +++ b/lib/Doctrine/ORM/Persisters/Entity/JoinedSubclassPersister.php @@ -26,8 +26,6 @@ use Doctrine\Common\Collections\Criteria; use Doctrine\ORM\Utility\PersisterHelper; -use function array_combine; -use function array_reverse; /** * The joined subclass persister maps a single entity instance to several tables in the @@ -273,42 +271,35 @@ public function update($entity) public function delete($entity) { $identifier = $this->em->getUnitOfWork()->getEntityIdentifier($entity); + $id = array_combine($this->class->getIdentifierColumnNames(), $identifier); $this->deleteJoinTableRecords($identifier); - // Delete parent entries in reverse order (root first, until the direct parent of the current class). - // - // If foreign key are supported (and set as well as active), the first deletion will cascade and the - // following queries won't delete anything, but it's better not to rely on a possible foreign key - // functionality which cannot be checked here (foreign keys can be missing or disabled although - // supported in general). - // - // On the other hand the supportsForeignKeyConstraints() method of the platform is also not reliable - // when it returns false, because it only means that Doctrine\DBAL doesn't support foreign keys for - // this platform, but they may be set in an existing database structure or added manually, so the - // previous order of deletion (current class to root) would have failed - this could i.e. happen with - // SQLite where foreign keys are theoretically possible but not supported by DBAL. - $deleted = false; - foreach (array_reverse($this->class->parentClasses) as $parentClass) { + // If the database platform supports FKs, just + // delete the row from the root table. Cascades do the rest. + if ($this->platform->supportsForeignKeyConstraints()) { + $rootClass = $this->em->getClassMetadata($this->class->rootEntityName); + $rootTable = $this->quoteStrategy->getTableName($rootClass, $this->platform); + $rootTypes = $this->getClassIdentifiersTypes($rootClass); + + return (bool) $this->conn->delete($rootTable, $id, $rootTypes); + } + + // Delete from all tables individually, starting from this class' table up to the root table. + $rootTable = $this->quoteStrategy->getTableName($this->class, $this->platform); + $rootTypes = $this->getClassIdentifiersTypes($this->class); + + $affectedRows = $this->conn->delete($rootTable, $id, $rootTypes); + + foreach ($this->class->parentClasses as $parentClass) { $parentMetadata = $this->em->getClassMetadata($parentClass); $parentTable = $this->quoteStrategy->getTableName($parentMetadata, $this->platform); $parentTypes = $this->getClassIdentifiersTypes($parentMetadata); - $parentId = array_combine($parentMetadata->getIdentifierColumnNames(), $identifier); - $affectedRows = $this->conn->delete($parentTable, $parentId, $parentTypes); - $deleted = $deleted ?: ($affectedRows > 0); + $this->conn->delete($parentTable, $id, $parentTypes); } - // Now delete the entries from the current class (if not deleted automatically - // because of a foreign key). - $currentTable = $this->quoteStrategy->getTableName($this->class, $this->platform); - $currentTypes = $this->getClassIdentifiersTypes($this->class); - $currentId = array_combine($this->class->getIdentifierColumnNames(), $identifier); - - $affectedRows = $this->conn->delete($currentTable, $currentId, $currentTypes); - $deleted = $deleted ?: ($affectedRows > 0); - - return $deleted; + return (bool) $affectedRows; } /** @@ -421,11 +412,8 @@ protected function getLockTablesSql($lockMode) $parentClass = $this->em->getClassMetadata($parentClassName); $joinSql .= ' INNER JOIN ' . $this->quoteStrategy->getTableName($parentClass, $this->platform) . ' ' . $tableAlias . ' ON '; - // Fix for bug GH-8229 (id column from parent class renamed in child class): - // Use the correct name for the id column as named in the parent class. - $parentIdentifierColumns = $parentClass->getIdentifierColumnNames(); - foreach ($identifierColumns as $index => $idColumn) { - $conditions[] = $baseTableAlias . '.' . $idColumn . ' = ' . $tableAlias . '.' . $parentIdentifierColumns[$index]; + foreach ($identifierColumns as $idColumn) { + $conditions[] = $baseTableAlias . '.' . $idColumn . ' = ' . $tableAlias . '.' . $idColumn; } $joinSql .= implode(' AND ', $conditions); @@ -601,11 +589,9 @@ private function getJoinSql($baseTableAlias) $tableAlias = $this->getSQLTableAlias($parentClassName); $joinSql .= ' INNER JOIN ' . $this->quoteStrategy->getTableName($parentClass, $this->platform) . ' ' . $tableAlias . ' ON '; - // Fix for bug GH-8229 (id column from parent class renamed in child class): - // Use the correct name for the id column as named in the parent class. - $parentIdentifierColumn = $parentClass->getIdentifierColumnNames(); - foreach ($identifierColumn as $index => $idColumn) { - $conditions[] = $baseTableAlias . '.' . $idColumn . ' = ' . $tableAlias . '.' . $parentIdentifierColumn[$index]; + + foreach ($identifierColumn as $idColumn) { + $conditions[] = $baseTableAlias . '.' . $idColumn . ' = ' . $tableAlias . '.' . $idColumn; } $joinSql .= implode(' AND ', $conditions); @@ -618,11 +604,8 @@ private function getJoinSql($baseTableAlias) $tableAlias = $this->getSQLTableAlias($subClassName); $joinSql .= ' LEFT JOIN ' . $this->quoteStrategy->getTableName($subClass, $this->platform) . ' ' . $tableAlias . ' ON '; - // Fix for bug GH-8229 (id column from parent class renamed in child class): - // Use the correct name for the id column as named in the parent class. - $subClassIdentifierColumn = $subClass->getIdentifierColumnNames(); - foreach ($identifierColumn as $index => $idColumn) { - $conditions[] = $baseTableAlias . '.' . $idColumn . ' = ' . $tableAlias . '.' . $subClassIdentifierColumn[$index]; + foreach ($identifierColumn as $idColumn) { + $conditions[] = $baseTableAlias . '.' . $idColumn . ' = ' . $tableAlias . '.' . $idColumn; } $joinSql .= implode(' AND ', $conditions); diff --git a/lib/Doctrine/ORM/Query/SqlWalker.php b/lib/Doctrine/ORM/Query/SqlWalker.php index d5fe832d566..e110d1faf0f 100644 --- a/lib/Doctrine/ORM/Query/SqlWalker.php +++ b/lib/Doctrine/ORM/Query/SqlWalker.php @@ -366,12 +366,8 @@ private function _generateClassTableInheritanceJoins($class, $dqlAlias) $sqlParts = []; - // Fix for bug GH-8229 (id column from parent class renamed in child class): - // Use the correct name for the id column as named in the parent class. - $identifierColumn = $this->quoteStrategy->getIdentifierColumnNames($class, $this->platform); - $parentIdentifierColumn = $this->quoteStrategy->getIdentifierColumnNames($parentClass, $this->platform); - foreach ($identifierColumn as $index => $idColumn) { - $sqlParts[] = $baseTableAlias . '.' . $idColumn . ' = ' . $tableAlias . '.' . $parentIdentifierColumn[$index]; + foreach ($this->quoteStrategy->getIdentifierColumnNames($class, $this->platform) as $columnName) { + $sqlParts[] = $baseTableAlias . '.' . $columnName . ' = ' . $tableAlias . '.' . $columnName; } // Add filters on the root class @@ -665,21 +661,11 @@ public function walkPathExpression($pathExpr) $dqlAlias = $pathExpr->identificationVariable; $class = $this->queryComponents[$dqlAlias]['metadata']; - // Fix for bug GH-8229 (id column from parent class renamed in child class): - // Use the correct name for the id column as named in the inherited class. - $mapping = $class->fieldMappings[$fieldName]; - if (isset($mapping['inherited'])) { - $inheritedClass = $this->em->getClassMetadata($mapping['inherited']); - $quotedColumnName = $this->quoteStrategy->getColumnName($fieldName, $inheritedClass, $this->platform); - } else { - $quotedColumnName = $this->quoteStrategy->getColumnName($fieldName, $class, $this->platform); - } - if ($this->useSqlTableAliases) { $sql .= $this->walkIdentificationVariable($dqlAlias, $fieldName) . '.'; } - $sql .= $quotedColumnName; + $sql .= $this->quoteStrategy->getColumnName($fieldName, $class, $this->platform); break; case AST\PathExpression::TYPE_SINGLE_VALUED_ASSOCIATION: @@ -1419,19 +1405,13 @@ public function walkSelectExpression($selectExpression) continue; } - // Fix for bug GH-8229 (id column from parent class renamed in child class): - // Use the correct name for the id column as named in the inherited class. - if (isset($mapping['inherited'])) { - $inheritedClass = $this->em->getClassMetadata($mapping['inherited']); - $tableName = $inheritedClass->getTableName(); - $quotedColumnName = $this->quoteStrategy->getColumnName($fieldName, $inheritedClass, $this->platform); - } else { - $tableName = $class->getTableName(); - $quotedColumnName = $this->quoteStrategy->getColumnName($fieldName, $class, $this->platform); - } + $tableName = (isset($mapping['inherited'])) + ? $this->em->getClassMetadata($mapping['inherited'])->getTableName() + : $class->getTableName(); - $sqlTableAlias = $this->getSQLTableAlias($tableName, $dqlAlias); - $columnAlias = $this->getSQLColumnAlias($mapping['columnName']); + $sqlTableAlias = $this->getSQLTableAlias($tableName, $dqlAlias); + $columnAlias = $this->getSQLColumnAlias($mapping['columnName']); + $quotedColumnName = $this->quoteStrategy->getColumnName($fieldName, $class, $this->platform); $col = $sqlTableAlias . '.' . $quotedColumnName; diff --git a/lib/Doctrine/ORM/Tools/SchemaTool.php b/lib/Doctrine/ORM/Tools/SchemaTool.php index a1906bb8639..6d718bceea7 100644 --- a/lib/Doctrine/ORM/Tools/SchemaTool.php +++ b/lib/Doctrine/ORM/Tools/SchemaTool.php @@ -31,7 +31,6 @@ use Doctrine\ORM\ORMException; use Doctrine\ORM\Tools\Event\GenerateSchemaTableEventArgs; use Doctrine\ORM\Tools\Event\GenerateSchemaEventArgs; -use function array_intersect_key; /** * The SchemaTool is a tool to create/drop/update database schemas based on @@ -249,20 +248,14 @@ function (ClassMetadata $class) use ($idMapping) : bool { } if ( ! empty($inheritedKeyColumns)) { - // Fix for bug GH-8229 (id column from parent class renamed in child class): - // Use the correct name for the id columns as named in the parent class. - $parentClass = $this->em->getClassMetadata($class->rootEntityName); - $parentKeyColumnNames = $parentClass->getIdentifierColumnNames(); - $parentKeyColumns = array_intersect_key($parentKeyColumnNames, $inheritedKeyColumns); - // Add a FK constraint on the ID column $table->addForeignKeyConstraint( $this->quoteStrategy->getTableName( - $parentClass, + $this->em->getClassMetadata($class->rootEntityName), $this->platform ), $inheritedKeyColumns, - $parentKeyColumns, + $inheritedKeyColumns, ['onDelete' => 'CASCADE'] ); } diff --git a/tests/Doctrine/Tests/ORM/Functional/Ticket/GH8229Test.php b/tests/Doctrine/Tests/ORM/Functional/Ticket/GH8229Test.php deleted file mode 100644 index 83ef821796e..00000000000 --- a/tests/Doctrine/Tests/ORM/Functional/Ticket/GH8229Test.php +++ /dev/null @@ -1,247 +0,0 @@ -setUpEntitySchema( - [ - GH8229Resource::class, - GH8229User::class, - ] - ); - } - - /** - * This tests the basic functionality when working with an entity using joined - * table inheritance and renamed identifier columns. It tests inserts, updates - * and deletions. - */ - public function testCorrectColumnNameInParentClassAfterAttributeOveride() - { - // Test creation - $entity = new GH8229User('foo'); - $identifier = $entity->id; - $this->_em->persist($entity); - $this->_em->flush(); - $this->_em->clear(); - - // Test reading (parent) - $entity = $this->_em->getRepository(GH8229Resource::class)->find($identifier); - self::assertEquals($identifier, $entity->id); - - // Test update (parent) - $entity->status = 2; - $this->_em->persist($entity); - $this->_em->flush(); - $this->_em->clear(); - $entity = $this->_em->getRepository(GH8229Resource::class)->find($identifier); - self::assertEquals(2, $entity->status); - $this->_em->clear(); - - // Test reading (child) - $entity = $this->_em->getRepository(GH8229User::class)->find($identifier); - self::assertEquals($identifier, $entity->id); - - // Test update (child) - $entity->username = 'bar'; - $entity->status = 3; - $this->_em->persist($entity); - $this->_em->flush(); - $this->_em->clear(); - $entity = $this->_em->getRepository(GH8229User::class)->find($identifier); - self::assertEquals('bar', $entity->username); - self::assertEquals(3, $entity->status); - - // Test deletion - $this->_em->remove($entity); - $this->_em->flush(); - $this->_em->clear(); - } - - - /** - * This test checks if foreign keys are generated for the schema, when a joined - * table inheritance is used and the identifier columns in the inheriting class - * are renamed. - */ - public function testForeignKeyInChildClassAfterAttributeOveride() - { - $schemaTool = new SchemaTool($this->_em); - - $schema = $schemaTool->getSchemaFromMetadata( - [ - $this->_em->getClassMetadata(GH8229Resource::class), - $this->_em->getClassMetadata(GH8229User::class), - ] - ); - - $childTable = $schema->getTable('gh8229_user'); - $childTableForeignKeys = $childTable->getForeignKeys(); - - self::assertCount(1, $childTableForeignKeys); - } - - /** - * This test checks if data are completely deleted, when a joined table inheritance is used, - * and the DBAL component generally supports foreign keys for the current platform, but the - * foreign keys never existed, have been removed or disabled. - */ - public function testJoinedTableDeletionWithDisabledForeignKeys() - { - // Remove foreign key (if it exists) - $connection = $this->_em->getConnection(); - $platform = $connection->getDatabasePlatform(); - if ($platform->supportsForeignKeyConstraints()) { - $class = $this->_em->getClassMetadata(GH8229User::class); - $table = $this->_schemaTool->getSchemaFromMetadata([$class])->getTable('gh8229_user'); - $foreignKeys = $table->getForeignKeys(); - - // Check if really set (seems to be not the case in MariaDB and MySQL) - if (count($foreignKeys) > 0) { - $statement = $platform->getDropForeignKeySQL(array_values($foreignKeys)[0], $table); - $connection->exec($statement); - } - } - - // Create entity - $entity = new GH8229User('foo'); - $identifier = $entity->id; - $this->_em->persist($entity); - $this->_em->flush(); - $this->_em->clear(); - - // Delete entity - $entity = $this->_em->getRepository(GH8229User::class)->find($identifier); - $this->_em->remove($entity); - $this->_em->flush(); - $this->_em->clear(); - - // Check if the child entry has been really deleted - $result = $connection->executeQuery('SELECT 1 FROM gh8229_user WHERE user_id = ?', [$identifier]); - $foundRows = count($result->fetchAll()); // Note: $result->rowCount() returns 1 instead of 0 in SQLite! - self::assertSame(0, $foundRows); - } - - /** - * This test checks the SQL generated for a DQL, because in the SELECT part, the JOIN part - * and the ORDER BY part the wrong column names were used. - */ - public function testCorrectColumnNamesInSQLFromDQL() - { - // Create entity - $entity = new GH8229User('foo'); - $identifier = $entity->id; - $this->_em->persist($entity); - $this->_em->flush(); - $this->_em->clear(); - - // Query entity - $dql = 'SELECT o FROM Doctrine\Tests\ORM\Functional\Ticket\GH8229User o WHERE o.id = :id GROUP BY o.id, o.username ORDER BY o.id ASC'; - $query = $this->_em->createQuery($dql); - $query = $query->setParameter('id', $identifier); - self::assertEquals($identifier, $query->getSingleResult()->id); - - // Delete entity - $entity = $this->_em->getRepository(GH8229User::class)->find($identifier); - $this->_em->remove($entity); - $this->_em->flush(); - $this->_em->clear(); - } - - /** - * This test checks the the right columns are used when creating a pessimistic write lock - * for an entity with joined table inheritance. - */ - public function testCorrectColumnNamesInPessimisticWriteLock() - { - // Create entity - $entity = new GH8229User('foo'); - $identifier = $entity->id; - $this->_em->persist($entity); - $this->_em->flush(); - - // Test lock - $this->_em->getConnection()->beginTransaction(); - $this->_em->lock($entity, LockMode::PESSIMISTIC_WRITE); - $this->_em->getConnection()->commit(); - $this->_em->clear(); - - self::assertEquals($identifier, $entity->id); - } -} - - -/** - * @Entity - * @Table(name="gh8229_resource") - * @InheritanceType("JOINED") - * @DiscriminatorColumn(name="resource_type", type="string", length=191) - * @DiscriminatorMap({ - * "resource"=GH8229Resource::class, - * "user"=GH8229User::class, - * }) - */ -abstract class GH8229Resource -{ - /** - * @Id() - * @Column(name="resource_id", type="integer") - */ - public $id; - - /** - * Additional property to test update - * - * @Column(type="integer", name="resource_status", nullable=false) - */ - public $status; - - private static $sequence = 0; - - protected function __construct() - { - $this->id = ++self::$sequence; - $this->status = 1; - } -} - -/** - * @Entity - * @Table(name="gh8229_user") - * @AttributeOverrides({ - * @AttributeOverride(name="id", column=@Column(name="user_id", type="integer")), - * @AttributeOverride(name="status", column=@Column(name="user_status", type="integer")) - * }) - */ -final class GH8229User extends GH8229Resource -{ - /** - * Additional property to test update - * - * @Column(type="string", name="username", length=191, nullable=false) - */ - public $username; - - public function __construct($username) - { - parent::__construct(); - - $this->username = $username; - } -} From 3431b24ba3b39546ad2e8b78abec3ab75e227f48 Mon Sep 17 00:00:00 2001 From: Benjamin Eberlei Date: Tue, 24 Nov 2020 18:26:56 +0100 Subject: [PATCH 06/10] [GH-8229] Update documentation to clarify only mapped superclass or trait works with overrides --- docs/en/reference/inheritance-mapping.rst | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/docs/en/reference/inheritance-mapping.rst b/docs/en/reference/inheritance-mapping.rst index 3b29ad8ac29..c4998d9e97a 100644 --- a/docs/en/reference/inheritance-mapping.rst +++ b/docs/en/reference/inheritance-mapping.rst @@ -289,9 +289,15 @@ column and cascading on delete. Overrides --------- -Used to override a mapping for an entity field or relationship. -May be applied to an entity that extends a mapped superclass -to override a relationship or field mapping defined by the mapped superclass. + +Used to override a mapping for an entity field or relationship. Can only be +applied to an entity that extends a mapped superclass or uses a trait to +override a relationship or field mapping defined by the mapped superclass or +trait. + +It is not possible to override attributes or associations in entity to entity +inheritance scenarios, because this can cause unforseen edge case behavior and +increases complexity in ORM internal classes. Association Override From 1e98b3d24d725984b9031ea6d594508d577b8cdd Mon Sep 17 00:00:00 2001 From: Benjamin Eberlei Date: Tue, 24 Nov 2020 18:32:21 +0100 Subject: [PATCH 07/10] [GH-8229] Fix style violations introduced by revert --- lib/Doctrine/ORM/Mapping/ClassMetadataInfo.php | 3 ++- lib/Doctrine/ORM/Mapping/MappingException.php | 2 ++ .../ORM/Persisters/Entity/BasicEntityPersister.php | 8 ++++---- .../ORM/Persisters/Entity/JoinedSubclassPersister.php | 2 ++ 4 files changed, 10 insertions(+), 5 deletions(-) diff --git a/lib/Doctrine/ORM/Mapping/ClassMetadataInfo.php b/lib/Doctrine/ORM/Mapping/ClassMetadataInfo.php index 6e95dc6046b..67e909405b9 100644 --- a/lib/Doctrine/ORM/Mapping/ClassMetadataInfo.php +++ b/lib/Doctrine/ORM/Mapping/ClassMetadataInfo.php @@ -30,6 +30,7 @@ use ReflectionClass; use ReflectionProperty; use RuntimeException; +use function count; use function explode; /** @@ -2192,7 +2193,7 @@ public function setAssociationOverride($fieldName, array $overrideMapping) $mapping = $this->associationMappings[$fieldName]; - if (isset($mapping['inherited']) && (count($overrideMapping) !== 1 || !isset($overrideMapping['fetch']))) { + if (isset($mapping['inherited']) && (count($overrideMapping) !== 1 || ! isset($overrideMapping['fetch']))) { // TODO: Deprecate overriding the fetch mode via association override for 3.0, // users should do this with a listener and a custom attribute/annotation throw MappingException::illegalOverrideOfInheritedProperty($this->name, $fieldName); diff --git a/lib/Doctrine/ORM/Mapping/MappingException.php b/lib/Doctrine/ORM/Mapping/MappingException.php index 130eb468e75..7044e964626 100644 --- a/lib/Doctrine/ORM/Mapping/MappingException.php +++ b/lib/Doctrine/ORM/Mapping/MappingException.php @@ -19,6 +19,8 @@ namespace Doctrine\ORM\Mapping; +use function sprintf; + /** * A MappingException indicates that something is wrong with the mapping setup. * diff --git a/lib/Doctrine/ORM/Persisters/Entity/BasicEntityPersister.php b/lib/Doctrine/ORM/Persisters/Entity/BasicEntityPersister.php index 4ac7b2cb91c..eafa3ea98bd 100644 --- a/lib/Doctrine/ORM/Persisters/Entity/BasicEntityPersister.php +++ b/lib/Doctrine/ORM/Persisters/Entity/BasicEntityPersister.php @@ -452,9 +452,9 @@ protected final function updateTable($entity, $quotedTableName, array $updateDat foreach ($this->class->identifier as $idField) { if ( ! isset($this->class->associationMappings[$idField])) { - $params[] = $identifier[$idField]; - $types[] = $this->class->fieldMappings[$idField]['type']; - $where[] = $this->quoteStrategy->getColumnName($idField, $this->class, $this->platform); + $params[] = $identifier[$idField]; + $types[] = $this->class->fieldMappings[$idField]['type']; + $where[] = $this->quoteStrategy->getColumnName($idField, $this->class, $this->platform); continue; } @@ -1672,7 +1672,7 @@ public function getSelectConditionStatementSQL($field, $value, $assoc = null, $c private function getSelectConditionStatementColumnSQL($field, $assoc = null) { if (isset($this->class->fieldMappings[$field])) { - $className = (isset($this->class->fieldMappings[$field]['inherited'])) + $className = isset($this->class->fieldMappings[$field]['inherited']) ? $this->class->fieldMappings[$field]['inherited'] : $this->class->name; diff --git a/lib/Doctrine/ORM/Persisters/Entity/JoinedSubclassPersister.php b/lib/Doctrine/ORM/Persisters/Entity/JoinedSubclassPersister.php index 1f6d40133c7..65e1b4efb6a 100644 --- a/lib/Doctrine/ORM/Persisters/Entity/JoinedSubclassPersister.php +++ b/lib/Doctrine/ORM/Persisters/Entity/JoinedSubclassPersister.php @@ -27,6 +27,8 @@ use Doctrine\Common\Collections\Criteria; use Doctrine\ORM\Utility\PersisterHelper; +use function array_combine; + /** * The joined subclass persister maps a single entity instance to several tables in the * database as it is defined by the Class Table Inheritance strategy. From 0193517f992ef53bfd0301addf160f93567622e6 Mon Sep 17 00:00:00 2001 From: Benjamin Eberlei Date: Tue, 24 Nov 2020 18:53:48 +0100 Subject: [PATCH 08/10] [GH-8229] Fix style violations introduced by revert --- lib/Doctrine/ORM/Persisters/Entity/BasicEntityPersister.php | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/lib/Doctrine/ORM/Persisters/Entity/BasicEntityPersister.php b/lib/Doctrine/ORM/Persisters/Entity/BasicEntityPersister.php index eafa3ea98bd..da809358d15 100644 --- a/lib/Doctrine/ORM/Persisters/Entity/BasicEntityPersister.php +++ b/lib/Doctrine/ORM/Persisters/Entity/BasicEntityPersister.php @@ -1672,9 +1672,7 @@ public function getSelectConditionStatementSQL($field, $value, $assoc = null, $c private function getSelectConditionStatementColumnSQL($field, $assoc = null) { if (isset($this->class->fieldMappings[$field])) { - $className = isset($this->class->fieldMappings[$field]['inherited']) - ? $this->class->fieldMappings[$field]['inherited'] - : $this->class->name; + $className = $this->class->fieldMappings[$field]['inherited'] ?? $this->class->name; return [$this->getSQLTableAlias($className) . '.' . $this->quoteStrategy->getColumnName($field, $this->class, $this->platform)]; } From 81aa5a98eae4d7a9c296c6eca806f537b6472b9f Mon Sep 17 00:00:00 2001 From: Benjamin Eberlei Date: Wed, 25 Nov 2020 18:33:27 +0100 Subject: [PATCH 09/10] [GH-8229] Temporarily disable the exception until 2.8. --- lib/Doctrine/ORM/Mapping/ClassMetadataInfo.php | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/lib/Doctrine/ORM/Mapping/ClassMetadataInfo.php b/lib/Doctrine/ORM/Mapping/ClassMetadataInfo.php index 67e909405b9..6b0dbe0ddd9 100644 --- a/lib/Doctrine/ORM/Mapping/ClassMetadataInfo.php +++ b/lib/Doctrine/ORM/Mapping/ClassMetadataInfo.php @@ -2196,7 +2196,8 @@ public function setAssociationOverride($fieldName, array $overrideMapping) if (isset($mapping['inherited']) && (count($overrideMapping) !== 1 || ! isset($overrideMapping['fetch']))) { // TODO: Deprecate overriding the fetch mode via association override for 3.0, // users should do this with a listener and a custom attribute/annotation - throw MappingException::illegalOverrideOfInheritedProperty($this->name, $fieldName); + // TODO: Enable this exception in 2.8 + //throw MappingException::illegalOverrideOfInheritedProperty($this->name, $fieldName); } if (isset($overrideMapping['joinColumns'])) { @@ -2258,7 +2259,8 @@ public function setAttributeOverride($fieldName, array $overrideMapping) $mapping = $this->fieldMappings[$fieldName]; if (isset($mapping['inherited'])) { - throw MappingException::illegalOverrideOfInheritedProperty($this->name, $fieldName); + // TODO: Enable this exception in 2.8 + //throw MappingException::illegalOverrideOfInheritedProperty($this->name, $fieldName); } if (isset($mapping['id'])) { From 5238c884748e054348012fba1f65c672e6990016 Mon Sep 17 00:00:00 2001 From: Benjamin Eberlei Date: Wed, 25 Nov 2020 20:12:10 +0100 Subject: [PATCH 10/10] Make phpcs happy --- lib/Doctrine/ORM/Mapping/ClassMetadataInfo.php | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/Doctrine/ORM/Mapping/ClassMetadataInfo.php b/lib/Doctrine/ORM/Mapping/ClassMetadataInfo.php index 6b0dbe0ddd9..9890c5956ee 100644 --- a/lib/Doctrine/ORM/Mapping/ClassMetadataInfo.php +++ b/lib/Doctrine/ORM/Mapping/ClassMetadataInfo.php @@ -2193,12 +2193,12 @@ public function setAssociationOverride($fieldName, array $overrideMapping) $mapping = $this->associationMappings[$fieldName]; - if (isset($mapping['inherited']) && (count($overrideMapping) !== 1 || ! isset($overrideMapping['fetch']))) { + //if (isset($mapping['inherited']) && (count($overrideMapping) !== 1 || ! isset($overrideMapping['fetch']))) { // TODO: Deprecate overriding the fetch mode via association override for 3.0, // users should do this with a listener and a custom attribute/annotation // TODO: Enable this exception in 2.8 //throw MappingException::illegalOverrideOfInheritedProperty($this->name, $fieldName); - } + //} if (isset($overrideMapping['joinColumns'])) { $mapping['joinColumns'] = $overrideMapping['joinColumns']; @@ -2258,10 +2258,10 @@ public function setAttributeOverride($fieldName, array $overrideMapping) $mapping = $this->fieldMappings[$fieldName]; - if (isset($mapping['inherited'])) { + //if (isset($mapping['inherited'])) { // TODO: Enable this exception in 2.8 //throw MappingException::illegalOverrideOfInheritedProperty($this->name, $fieldName); - } + //} if (isset($mapping['id'])) { $overrideMapping['id'] = $mapping['id'];