Skip to content

Commit

Permalink
Merge pull request #10787 from doctrine/2.15.x-merge-up-into-2.16.x_X…
Browse files Browse the repository at this point in the history
…V3tWpNu
  • Loading branch information
greg0ire authored Jun 22, 2023
2 parents 41f704c + 5f6501f commit 6c0a5ec
Show file tree
Hide file tree
Showing 15 changed files with 674 additions and 214 deletions.
18 changes: 18 additions & 0 deletions docs/en/reference/association-mapping.rst
Original file line number Diff line number Diff line change
Expand Up @@ -881,6 +881,15 @@ Generated MySQL Schema:
replaced by one-to-many/many-to-one associations between the 3
participating classes.

.. note::

For many-to-many associations, the ORM takes care of managing rows
in the join table connecting both sides. Due to the way it deals
with entity removals, database-level constraints may not work the
way one might intuitively assume. Thus, be sure not to miss the section
on :ref:`join table management <remove_object_many_to_many_join_tables>`.


Many-To-Many, Bidirectional
---------------------------

Expand Down Expand Up @@ -1019,6 +1028,15 @@ one is bidirectional.
The MySQL schema is exactly the same as for the Many-To-Many
uni-directional case above.

.. note::

For many-to-many associations, the ORM takes care of managing rows
in the join table connecting both sides. Due to the way it deals
with entity removals, database-level constraints may not work the
way one might intuitively assume. Thus, be sure not to miss the section
on :ref:`join table management <remove_object_many_to_many_join_tables>`.


Owning and Inverse Side on a ManyToMany Association
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Expand Down
50 changes: 43 additions & 7 deletions docs/en/reference/working-with-objects.rst
Original file line number Diff line number Diff line change
Expand Up @@ -286,17 +286,53 @@ as follows:
After an entity has been removed, its in-memory state is the same as
before the removal, except for generated identifiers.

Removing an entity will also automatically delete any existing
records in many-to-many join tables that link this entity. The
action taken depends on the value of the ``@joinColumn`` mapping
attribute "onDelete". Either Doctrine issues a dedicated ``DELETE``
statement for records of each join table or it depends on the
foreign key semantics of onDelete="CASCADE".
During the ``EntityManager#flush()`` operation, the removed entity
will also be removed from all collections in entities currently
loaded into memory.

.. _remove_object_many_to_many_join_tables:

Join-table management when removing from many-to-many collections
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Regarding existing rows in many-to-many join tables that refer to
an entity being removed, the following applies.

When the entity being removed does not declare the many-to-many association
itself (that is, the many-to-many association is unidirectional and
the entity is on the inverse side), the ORM has no reasonable way to
detect associations targeting the entity's class. Thus, no ORM-level handling
of join-table rows is attempted and database-level constraints apply.
In case of database-level ``ON DELETE RESTRICT`` constraints, the
``EntityManager#flush()`` operation may abort and a ``ConstraintViolationException``
may be thrown. No in-memory collections will be modified in this case.
With ``ON DELETE CASCADE``, the RDBMS will take care of removing rows
from join tables.

When the entity being removed is part of bi-directional many-to-many
association, either as the owning or inverse side, the ORM will
delete rows from join tables before removing the entity itself. That means
database-level ``ON DELETE RESTRICT`` constraints on join tables are not
effective, since the join table rows are removed first. Removal of join table
rows happens through specialized methods in entity and collection persister
classes and take one query per entity and join table. In case the association
uses a ``@JoinColumn`` configuration with ``onDelete="CASCADE"``, instead
of using a dedicated ``DELETE`` query the database-level operation will be
relied upon.

.. note::

In case you rely on database-level ``ON DELETE RESTRICT`` constraints,
be aware that by making many-to-many associations bidirectional the
assumed protection may be lost.


Performance of different deletion strategies
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Deleting an object with all its associated objects can be achieved
in multiple ways with very different performance impacts.


1. If an association is marked as ``CASCADE=REMOVE`` Doctrine ORM
will fetch this association. If its a Single association it will
pass this entity to
Expand Down
11 changes: 10 additions & 1 deletion lib/Doctrine/ORM/Persisters/Collection/OneToManyPersister.php
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
use function array_values;
use function assert;
use function implode;
use function is_int;
use function is_string;

/**
Expand Down Expand Up @@ -174,16 +175,22 @@ private function deleteEntityCollection(PersistentCollection $collection): int
$targetClass = $this->em->getClassMetadata($mapping['targetEntity']);
$columns = [];
$parameters = [];
$types = [];

foreach ($targetClass->associationMappings[$mapping['mappedBy']]['joinColumns'] as $joinColumn) {
$columns[] = $this->quoteStrategy->getJoinColumnName($joinColumn, $targetClass, $this->platform);
$parameters[] = $identifier[$sourceClass->getFieldForColumn($joinColumn['referencedColumnName'])];
$types[] = PersisterHelper::getTypeOfColumn($joinColumn['referencedColumnName'], $sourceClass, $this->em);
}

$statement = 'DELETE FROM ' . $this->quoteStrategy->getTableName($targetClass, $this->platform)
. ' WHERE ' . implode(' = ? AND ', $columns) . ' = ?';

return $this->conn->executeStatement($statement, $parameters);
$numAffected = $this->conn->executeStatement($statement, $parameters, $types);

assert(is_int($numAffected));

return $numAffected;
}

/**
Expand Down Expand Up @@ -247,6 +254,8 @@ private function deleteJoinedEntityCollection(PersistentCollection $collection):

$this->conn->executeStatement($statement);

assert(is_int($numDeleted));

return $numDeleted;
}
}
6 changes: 1 addition & 5 deletions lib/Doctrine/ORM/Persisters/Entity/BasicEntityPersister.php
Original file line number Diff line number Diff line change
Expand Up @@ -538,7 +538,7 @@ final protected function updateTable(
protected function deleteJoinTableRecords(array $identifier, array $types): void
{
foreach ($this->class->associationMappings as $mapping) {
if ($mapping['type'] !== ClassMetadata::MANY_TO_MANY) {
if ($mapping['type'] !== ClassMetadata::MANY_TO_MANY || isset($mapping['isOnDeleteCascade'])) {
continue;
}

Expand Down Expand Up @@ -574,10 +574,6 @@ protected function deleteJoinTableRecords(array $identifier, array $types): void
$otherKeys[] = $this->quoteStrategy->getJoinColumnName($joinColumn, $class, $this->platform);
}

if (isset($mapping['isOnDeleteCascade'])) {
continue;
}

$joinTableName = $this->quoteStrategy->getJoinTableName($association, $this->class, $this->platform);

$this->conn->delete($joinTableName, array_combine($keys, $identifier), $types);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,13 @@ protected function configure()
by the ORM, you can use a DBAL functionality to filter the tables and sequences down
on a global level:
$config->setFilterSchemaAssetsExpression($regexp);
$config->setSchemaAssetsFilter(function (string|AbstractAsset $assetName): bool {
if ($assetName instanceof AbstractAsset) {
$assetName = $assetName->getName();
}
return !str_starts_with($assetName, 'audit_');
});
EOT
);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,13 @@ protected function configure()
by the ORM, you can use a DBAL functionality to filter the tables and sequences down
on a global level:
$config->setFilterSchemaAssetsExpression($regexp);
$config->setSchemaAssetsFilter(function (string|AbstractAsset $assetName): bool {
if ($assetName instanceof AbstractAsset) {
$assetName = $assetName->getName();
}
return !str_starts_with($assetName, 'audit_');
});
EOT
);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,13 @@ protected function configure()
by the ORM, you can use a DBAL functionality to filter the tables and sequences down
on a global level:
$config->setFilterSchemaAssetsExpression($regexp);
$config->setSchemaAssetsFilter(function (string|AbstractAsset $assetName): bool {
if ($assetName instanceof AbstractAsset) {
$assetName = $assetName->getName();
}
return !str_starts_with($assetName, 'audit_');
});
EOT
);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,13 @@ protected function configure()
by the ORM, you can use a DBAL functionality to filter the tables and sequences down
on a global level:
$config->setFilterSchemaAssetsExpression($regexp);
$config->setSchemaAssetsFilter(function (string|AbstractAsset $assetName): bool {
if ($assetName instanceof AbstractAsset) {
$assetName = $assetName->getName();
}
return !str_starts_with($assetName, 'audit_');
});
EOT
);
}
Expand Down
103 changes: 66 additions & 37 deletions lib/Doctrine/ORM/UnitOfWork.php
Original file line number Diff line number Diff line change
Expand Up @@ -240,6 +240,17 @@ class UnitOfWork implements PropertyChangedListener
*/
private $visitedCollections = [];

/**
* List of collections visited during the changeset calculation that contain to-be-removed
* entities and need to have keys removed post commit.
*
* Indexed by Collection object ID, which also serves as the key in self::$visitedCollections;
* values are the key names that need to be removed.
*
* @psalm-var array<int, array<array-key, true>>
*/
private $pendingCollectionElementRemovals = [];

/**
* The EntityManager that "owns" this UnitOfWork instance.
*
Expand Down Expand Up @@ -474,8 +485,15 @@ public function commit($entity = null)

$this->afterTransactionComplete();

// Take new snapshots from visited collections
foreach ($this->visitedCollections as $coll) {
// Unset removed entities from collections, and take new snapshots from
// all visited collections.
foreach ($this->visitedCollections as $coid => $coll) {
if (isset($this->pendingCollectionElementRemovals[$coid])) {
foreach ($this->pendingCollectionElementRemovals[$coid] as $key => $valueIgnored) {
unset($coll[$key]);
}
}

$coll->takeSnapshot();
}

Expand All @@ -487,15 +505,16 @@ public function commit($entity = null)
/** @param object|object[]|null $entity */
private function postCommitCleanup($entity): void
{
$this->entityInsertions =
$this->entityUpdates =
$this->entityDeletions =
$this->extraUpdates =
$this->collectionUpdates =
$this->nonCascadedNewDetectedEntities =
$this->collectionDeletions =
$this->visitedCollections =
$this->orphanRemovals = [];
$this->entityInsertions =
$this->entityUpdates =
$this->entityDeletions =
$this->extraUpdates =
$this->collectionUpdates =
$this->nonCascadedNewDetectedEntities =
$this->collectionDeletions =
$this->pendingCollectionElementRemovals =
$this->visitedCollections =
$this->orphanRemovals = [];

if ($entity === null) {
$this->entityChangeSets = $this->scheduledForSynchronization = [];
Expand Down Expand Up @@ -916,6 +935,14 @@ private function computeAssociationChanges(array $assoc, $value): void
return;
}

// If this collection is dirty, schedule it for updates
if ($value instanceof PersistentCollection && $value->isDirty()) {
$coid = spl_object_id($value);

$this->collectionUpdates[$coid] = $value;
$this->visitedCollections[$coid] = $value;
}

// Look through the entities, and in any of their associations,
// for transient (new) entities, recursively. ("Persistence by reachability")
// Unwrap. Uninitialized collections will simply be empty.
Expand Down Expand Up @@ -960,10 +987,18 @@ private function computeAssociationChanges(array $assoc, $value): void
case self::STATE_REMOVED:
// Consume the $value as array (it's either an array or an ArrayAccess)
// and remove the element from Collection.
if ($assoc['type'] & ClassMetadata::TO_MANY) {
unset($value[$key]);
if (! ($assoc['type'] & ClassMetadata::TO_MANY)) {
break;
}

$coid = spl_object_id($value);
$this->visitedCollections[$coid] = $value;

if (! isset($this->pendingCollectionElementRemovals[$coid])) {
$this->pendingCollectionElementRemovals[$coid] = [];
}

$this->pendingCollectionElementRemovals[$coid][$key] = true;
break;

case self::STATE_DETACHED:
Expand All @@ -976,13 +1011,6 @@ private function computeAssociationChanges(array $assoc, $value): void
// during changeset calculation anyway, since they are in the identity map.
}
}

if ($value instanceof PersistentCollection && $value->isDirty()) {
$coid = spl_object_id($value);

$this->collectionUpdates[$coid] = $value;
$this->visitedCollections[$coid] = $value;
}
}

/**
Expand Down Expand Up @@ -2627,23 +2655,24 @@ public function getCommitOrderCalculator()
public function clear($entityName = null)
{
if ($entityName === null) {
$this->identityMap =
$this->entityIdentifiers =
$this->originalEntityData =
$this->entityChangeSets =
$this->entityStates =
$this->scheduledForSynchronization =
$this->entityInsertions =
$this->entityUpdates =
$this->entityDeletions =
$this->nonCascadedNewDetectedEntities =
$this->collectionDeletions =
$this->collectionUpdates =
$this->extraUpdates =
$this->readOnlyObjects =
$this->visitedCollections =
$this->eagerLoadingEntities =
$this->orphanRemovals = [];
$this->identityMap =
$this->entityIdentifiers =
$this->originalEntityData =
$this->entityChangeSets =
$this->entityStates =
$this->scheduledForSynchronization =
$this->entityInsertions =
$this->entityUpdates =
$this->entityDeletions =
$this->nonCascadedNewDetectedEntities =
$this->collectionDeletions =
$this->collectionUpdates =
$this->extraUpdates =
$this->readOnlyObjects =
$this->pendingCollectionElementRemovals =
$this->visitedCollections =
$this->eagerLoadingEntities =
$this->orphanRemovals = [];
} else {
Deprecation::triggerIfCalledFromOutside(
'doctrine/orm',
Expand Down
8 changes: 0 additions & 8 deletions psalm-baseline.xml
Original file line number Diff line number Diff line change
Expand Up @@ -1243,14 +1243,6 @@
$mapping['indexBy'] => $index,
]]]></code>
</InvalidArrayOffset>
<InvalidReturnStatement>
<code>$numDeleted</code>
<code><![CDATA[$this->conn->executeStatement($statement, $parameters)]]></code>
</InvalidReturnStatement>
<InvalidReturnType>
<code>int</code>
<code>int</code>
</InvalidReturnType>
<PossiblyNullArgument>
<code><![CDATA[$collection->getOwner()]]></code>
<code><![CDATA[$collection->getOwner()]]></code>
Expand Down
Loading

0 comments on commit 6c0a5ec

Please sign in to comment.