Skip to content

Commit

Permalink
Fix for bug #8229 (id column from parent class renamed in child class) (
Browse files Browse the repository at this point in the history 
#8234)

This fixes problems with id columns defined in the parent class but renamed in the child class using an attribute override. Before this change always the child column name was used (which was not present in the parent class), now the correct column names are used for the parent table when creating inserts, joins and deletions for it.

Co-authored-by: Crossjoin <[email protected]>
  • Loading branch information
@cziegenberg
cziegenberg and Crossjoin authored Aug 29, 2020
1 parent da18985 commit ccae8f7
Show file tree
Hide file tree
Showing 6 changed files with 375 additions and 48 deletions.
7 changes: 7 additions & 0 deletions lib/Doctrine/ORM/Mapping/ClassMetadataInfo.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
use ReflectionClass;
use ReflectionProperty;
use RuntimeException;
use function array_key_exists;
use function explode;

/**
Expand Down Expand Up @@ -2262,6 +2263,12 @@ 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']]);
Expand Down
50 changes: 40 additions & 10 deletions lib/Doctrine/ORM/Persisters/Entity/BasicEntityPersister.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@
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;
Expand Down Expand Up @@ -447,14 +448,26 @@ protected final function updateTable($entity, $quotedTableName, array $updateDat
$types[] = $this->columnTypes[$columnName];
}

$where = [];
$identifier = $this->em->getUnitOfWork()->getEntityIdentifier($entity);
$where = [];
$identifier = $this->em->getUnitOfWork()->getEntityIdentifier($entity);
$quotedClassTableName = $this->quoteStrategy->getTableName($this->class, $this->platform);

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'];

// 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);

continue;
}
Expand Down Expand Up @@ -632,7 +645,18 @@ protected function prepareUpdateData($entity)
$newVal = $change[1];

if ( ! isset($this->class->associationMappings[$field])) {
$fieldMapping = $this->class->fieldMappings[$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];
$columnName = $fieldMapping['columnName'];

$this->columnTypes[$columnName] = $fieldMapping['type'];
Expand Down Expand Up @@ -1672,11 +1696,17 @@ 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;
// 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;
}

return [$this->getSQLTableAlias($className) . '.' . $this->quoteStrategy->getColumnName($field, $this->class, $this->platform)];
return [$this->getSQLTableAlias($className) . '.' . $this->quoteStrategy->getColumnName($field, $class, $this->platform)];
}

if (isset($this->class->associationMappings[$field])) {
Expand Down
71 changes: 44 additions & 27 deletions lib/Doctrine/ORM/Persisters/Entity/JoinedSubclassPersister.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@

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
Expand Down Expand Up @@ -271,35 +273,42 @@ public function update($entity)
public function delete($entity)
{
$identifier = $this->em->getUnitOfWork()->getEntityIdentifier($entity);
$id = array_combine($this->class->getIdentifierColumnNames(), $identifier);

$this->deleteJoinTableRecords($identifier);

// 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) {
// 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) {
$parentMetadata = $this->em->getClassMetadata($parentClass);
$parentTable = $this->quoteStrategy->getTableName($parentMetadata, $this->platform);
$parentTypes = $this->getClassIdentifiersTypes($parentMetadata);
$parentId = array_combine($parentMetadata->getIdentifierColumnNames(), $identifier);

$this->conn->delete($parentTable, $id, $parentTypes);
$affectedRows = $this->conn->delete($parentTable, $parentId, $parentTypes);
$deleted = $deleted ?: ($affectedRows > 0);
}

return (bool) $affectedRows;
// 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;
}

/**
Expand Down Expand Up @@ -412,8 +421,11 @@ protected function getLockTablesSql($lockMode)
$parentClass = $this->em->getClassMetadata($parentClassName);
$joinSql .= ' INNER JOIN ' . $this->quoteStrategy->getTableName($parentClass, $this->platform) . ' ' . $tableAlias . ' ON ';

foreach ($identifierColumns as $idColumn) {
$conditions[] = $baseTableAlias . '.' . $idColumn . ' = ' . $tableAlias . '.' . $idColumn;
// 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];
}

$joinSql .= implode(' AND ', $conditions);
Expand Down Expand Up @@ -589,9 +601,11 @@ private function getJoinSql($baseTableAlias)
$tableAlias = $this->getSQLTableAlias($parentClassName);
$joinSql .= ' INNER JOIN ' . $this->quoteStrategy->getTableName($parentClass, $this->platform) . ' ' . $tableAlias . ' ON ';


foreach ($identifierColumn as $idColumn) {
$conditions[] = $baseTableAlias . '.' . $idColumn . ' = ' . $tableAlias . '.' . $idColumn;
// 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];
}

$joinSql .= implode(' AND ', $conditions);
Expand All @@ -604,8 +618,11 @@ private function getJoinSql($baseTableAlias)
$tableAlias = $this->getSQLTableAlias($subClassName);
$joinSql .= ' LEFT JOIN ' . $this->quoteStrategy->getTableName($subClass, $this->platform) . ' ' . $tableAlias . ' ON ';

foreach ($identifierColumn as $idColumn) {
$conditions[] = $baseTableAlias . '.' . $idColumn . ' = ' . $tableAlias . '.' . $idColumn;
// 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];
}

$joinSql .= implode(' AND ', $conditions);
Expand Down
38 changes: 29 additions & 9 deletions lib/Doctrine/ORM/Query/SqlWalker.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -366,8 +366,12 @@ private function _generateClassTableInheritanceJoins($class, $dqlAlias)

$sqlParts = [];

foreach ($this->quoteStrategy->getIdentifierColumnNames($class, $this->platform) as $columnName) {
$sqlParts[] = $baseTableAlias . '.' . $columnName . ' = ' . $tableAlias . '.' . $columnName;
// 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];
}

// Add filters on the root class
Expand Down Expand Up @@ -661,11 +665,21 @@ 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 .= $this->quoteStrategy->getColumnName($fieldName, $class, $this->platform);
$sql .= $quotedColumnName;
break;

case AST\PathExpression::TYPE_SINGLE_VALUED_ASSOCIATION:
Expand Down Expand Up @@ -1405,13 +1419,19 @@ public function walkSelectExpression($selectExpression)
continue;
}

$tableName = (isset($mapping['inherited']))
? $this->em->getClassMetadata($mapping['inherited'])->getTableName()
: $class->getTableName();
// 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);
}

$sqlTableAlias = $this->getSQLTableAlias($tableName, $dqlAlias);
$columnAlias = $this->getSQLColumnAlias($mapping['columnName']);
$quotedColumnName = $this->quoteStrategy->getColumnName($fieldName, $class, $this->platform);
$sqlTableAlias = $this->getSQLTableAlias($tableName, $dqlAlias);
$columnAlias = $this->getSQLColumnAlias($mapping['columnName']);

$col = $sqlTableAlias . '.' . $quotedColumnName;

Expand Down
11 changes: 9 additions & 2 deletions lib/Doctrine/ORM/Tools/SchemaTool.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
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
Expand Down Expand Up @@ -248,14 +249,20 @@ 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(
$this->em->getClassMetadata($class->rootEntityName),
$parentClass,
$this->platform
),
$inheritedKeyColumns,
$inheritedKeyColumns,
$parentKeyColumns,
['onDelete' => 'CASCADE']
);
}
Expand Down
Loading

0 comments on commit ccae8f7

Please sign in to comment.