Repository: doctrine/orm (php)

Issue:
No ForeignKeyConstraintViolationException exception when trying to remove an entity
### Bug Report

<!-- Fill in the relevant information below to help triage your issue. -->

|    Q        |   A
|------------ | ------
| BC Break    | kinda
| Version     | 2.15.2

#### Summary

Since the following change
<img width="1449" alt="CleanShot 2023-06-03 at 08 04 00@2x" src="https://github.com/doctrine/orm/assets/80641364/7e024efb-8a54-46de-9990-df19fa1de179">
Doctrine stopped throwing `ForeignKeyConstraintViolationException` and (from what I noticed) it removes the entity from the database. What's more interesting, if we add the deleted lines, and leave the same code at the end of the method – it doesn't work. So putting this `if` statement somehow breaks the expected behavior.

#### Current behavior

No `ForeignKeyConstraintViolationException` 🤷🏼‍♂️.
#### How to reproduce

#10753

#### Expected behavior

`ForeignKeyConstraintViolationException` should be thrown.


Pull request description:
Defer removing removed entities from to-many collections until after transaction commit
This PR fixes regressions introduced in 2.15.1 related to how the `UnitOfWork` and `EntityPersister`s implement removal of entities that are part of many-to-many collections.

#### Background

To better understand the problem and regressions caused, we need to take a look at how the ORM deals with many-to-many associations when an entity shall be removed that is part of such an association.

`BasicEntityPersister::deleteJoinTableRecords()` is the method responsible for efficient removal of join-table rows in the database when a single entity is removed. It will `DELETE` all join-table rows referring to the entity in question before the entity is removed itself. This only works when the many-to-many association is declared on the entity's side (ie. its a bi-directional many-to-many association, or the entity is the owning side). Otherwise, since the ORM cannot detect associations that are declared in other classes only, join tables are left alone and database-level constraints apply.

`BasicEntityPersister::deleteJoinTableRecords()` implements a rather efficient join-table row removal, since a single `DELETE` query per removed entity and association can be used to possibly delete multiple rows (for associations with different other entities).

Besides deleting join-table rows in the database, we also want to update collections to no longer contain the removed entity. Removing the entity from these collections is the in-memory equivalent to removing the join table rows in the database.

Early during the commit phase, the `UnitOfWork::computeAssociationChanges()` method is responsible for looking at all associations in all in-memory entities. Among a few other tasks, it will identify all "dirty" collections – those that have been changed and need to be flushed to the database. This method will also check all collections for entities pending removal, and it will remove (unset) these entities from the collections.

#### Current situation

#10485 complained that the `computeAssociationChanges` would first take note of the "dirty" collections and then unset entities pending removal. The consequences of this were that 
* in collections that were already dirty (changed) _before_ `flush()` was called, the join-table row would be removed with a dedicated `DELETE` statement, while bringing the collection and database in sync
* more importantly, previously clean collections would be dirty after `flush()` returns (the opposite of what `flush()` is supposed to achieve), and subsequent `flush()` operations would fail since a `DELETE` statement would be issued for an entity that was no longer in the identity map.

The fix in #10486 (contained in 2.15.1) tried to address this by _first_ unsetting the to-be-removed entities _before_ taking note of (then) dirty collections. This caused unintended behavior changes that were not spotted by tests:
* Since collection updates are done first, we now use single `DELETE` statements for every join-table row deletion. `BasicEntityPersister::deleteJoinTableRecords()` was effectively bypassed for those collections that were loaded into memory at that time. More queries are used than necessary.
* #10752 reports a use case where the to-be-removed entity _does not_ declare the many-to-many association. Pre #10486, this would always mean database-level constraints apply, since the ORM does not touch join-table rows in that case. But with the #10486 changes _and_ given a collection containing the to-be-removed entity is in memory, we ended up doing join-table row removals first – circumventing database-level constraints.

#### Suggested fix

* Improve `ManyToManyBasicAssociationTest` to make sure collections are clean after `flush()` returns, and include assertions regarding the expected query count. This would have spotted per-row `DELETE`s instead of using the optimized persister method
* Do not modify collections during `computeAssociationChanges`, but instead track which entity needs to be removed from which collection in a dedicated array
* This way, join-tables will be dealt with by the `deleteJoinTableRecords` method only
* After all changes have been flushed successfully, in the finishing phase of the UoW commit, go back to the list of tracked entity removals. Remove the entities from the respective collections immediately before taking new snapshots (= defining the new "clean" state). This makes sure we only do in-memory cleanups and cause no additional database operations due to removals.
* Improve documentation to raise awareness of how this proess works

Fixes #10752, closes #10753.



Patch:
diff --git a/docs/en/reference/association-mapping.rst b/docs/en/reference/association-mapping.rst
index 1ff30d3df..6280eee06 100644
--- a/docs/en/reference/association-mapping.rst
+++ b/docs/en/reference/association-mapping.rst
@@ -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
 ---------------------------
 
@@ -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
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 
diff --git a/docs/en/reference/working-with-objects.rst b/docs/en/reference/working-with-objects.rst
index fb1cc1ae4..32d53212e 100644
--- a/docs/en/reference/working-with-objects.rst
+++ b/docs/en/reference/working-with-objects.rst
@@ -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
diff --git a/lib/Doctrine/ORM/Persisters/Entity/BasicEntityPersister.php b/lib/Doctrine/ORM/Persisters/Entity/BasicEntityPersister.php
index 623c2cb61..1f6d3e512 100644
--- a/lib/Doctrine/ORM/Persisters/Entity/BasicEntityPersister.php
+++ b/lib/Doctrine/ORM/Persisters/Entity/BasicEntityPersister.php
@@ -538,7 +538,7 @@ class BasicEntityPersister implements EntityPersister
     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;
             }
 
@@ -574,10 +574,6 @@ class BasicEntityPersister implements EntityPersister
                 $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);
diff --git a/lib/Doctrine/ORM/UnitOfWork.php b/lib/Doctrine/ORM/UnitOfWork.php
index bc8af25fb..ebb5f12a0 100644
--- a/lib/Doctrine/ORM/UnitOfWork.php
+++ b/lib/Doctrine/ORM/UnitOfWork.php
@@ -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.
      *
@@ -474,8 +485,15 @@ class UnitOfWork implements PropertyChangedListener
 
         $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();
         }
 
@@ -487,15 +505,16 @@ class UnitOfWork implements PropertyChangedListener
     /** @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 = [];
@@ -916,6 +935,14 @@ class UnitOfWork implements PropertyChangedListener
             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.
@@ -960,10 +987,18 @@ class UnitOfWork implements PropertyChangedListener
                 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:
@@ -976,13 +1011,6 @@ class UnitOfWork implements PropertyChangedListener
                     // 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;
-        }
     }
 
     /**
@@ -2627,23 +2655,24 @@ class UnitOfWork implements PropertyChangedListener
     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',


Test patch:
diff --git a/tests/Doctrine/Tests/ORM/Functional/GH10752Test.php b/tests/Doctrine/Tests/ORM/Functional/GH10752Test.php
new file mode 100644
index 000000000..c90d19950
--- /dev/null
+++ b/tests/Doctrine/Tests/ORM/Functional/GH10752Test.php
@@ -0,0 +1,125 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Doctrine\Tests\ORM\Functional;
+
+use Doctrine\Common\Collections\ArrayCollection;
+use Doctrine\Common\Collections\Collection;
+use Doctrine\DBAL\Exception\ForeignKeyConstraintViolationException;
+use Doctrine\ORM\Mapping as ORM;
+use Doctrine\Tests\OrmFunctionalTestCase;
+
+/**
+ * @group GH10752
+ */
+class GH10752Test extends OrmFunctionalTestCase
+{
+    protected function setUp(): void
+    {
+        parent::setUp();
+
+        $this->setUpEntitySchema([
+            GH10752Order::class,
+            GH10752Promotion::class,
+        ]);
+    }
+
+    public function testThrowExceptionWhenRemovingPromotionThatIsInUse(): void
+    {
+        if (! $this->_em->getConnection()->getDatabasePlatform()->supportsForeignKeyConstraints()) {
+            self::markTestSkipped('Platform does not support foreign keys.');
+        }
+
+        $order     = new GH10752Order();
+        $promotion = new GH10752Promotion();
+
+        $order->addPromotion($promotion);
+
+        $this->_em->persist($order);
+        $this->_em->persist($promotion);
+        $this->_em->flush();
+
+        $this->_em->remove($promotion);
+
+        $this->expectException(ForeignKeyConstraintViolationException::class);
+        $this->_em->flush();
+    }
+
+    public function testThrowExceptionWhenRemovingPromotionThatIsInUseAndOrderIsNotInMemory(): void
+    {
+        if (! $this->_em->getConnection()->getDatabasePlatform()->supportsForeignKeyConstraints()) {
+            self::markTestSkipped('Platform does not support foreign keys.');
+        }
+
+        $order     = new GH10752Order();
+        $promotion = new GH10752Promotion();
+
+        $order->addPromotion($promotion);
+
+        $this->_em->persist($order);
+        $this->_em->persist($promotion);
+        $this->_em->flush();
+
+        $this->_em->clear();
+
+        $promotion = $this->_em->find(GH10752Promotion::class, $promotion->id);
+        $this->_em->remove($promotion);
+
+        $this->expectException(ForeignKeyConstraintViolationException::class);
+        $this->_em->flush();
+    }
+}
+
+/**
+ * @ORM\Entity
+ */
+class GH10752Order
+{
+    /**
+     * @ORM\Id
+     * @ORM\GeneratedValue
+     * @ORM\Column(type="integer")
+     *
+     * @var int
+     */
+    private $id = null;
+
+    /**
+     * @ORM\ManyToMany(targetEntity="GH10752Promotion", cascade={"persist"})
+     * @ORM\JoinTable(name="order_promotion",
+     *      joinColumns={@ORM\JoinColumn(name="order_id", referencedColumnName="id", onDelete="CASCADE")},
+     *      inverseJoinColumns={@ORM\JoinColumn(name="promotion_id", referencedColumnName="id")}
+     * )
+     *
+     * @var Collection
+     */
+    private $promotions;
+
+    public function __construct()
+    {
+        $this->promotions = new ArrayCollection();
+    }
+
+    public function addPromotion(GH10752Promotion $promotion): void
+    {
+        if (! $this->promotions->contains($promotion)) {
+            $this->promotions->add($promotion);
+        }
+    }
+}
+
+/**
+ * @ORM\Entity
+ */
+class GH10752Promotion
+{
+    /**
+     * @ORM\Id
+     * @ORM\GeneratedValue
+     * @ORM\Column(type="integer")
+     *
+     * @var int
+     */
+    public $id = null;
+}
diff --git a/tests/Doctrine/Tests/ORM/Functional/ManyToManyBasicAssociationTest.php b/tests/Doctrine/Tests/ORM/Functional/ManyToManyBasicAssociationTest.php
index 71d63b751..994d2bb8a 100644
--- a/tests/Doctrine/Tests/ORM/Functional/ManyToManyBasicAssociationTest.php
+++ b/tests/Doctrine/Tests/ORM/Functional/ManyToManyBasicAssociationTest.php
@@ -101,6 +101,9 @@ class ManyToManyBasicAssociationTest extends OrmFunctionalTestCase
         //$user->getGroups()->remove(0);
 
         $this->_em->flush();
+
+        self::assertFalse($user->getGroups()->isDirty());
+
         $this->_em->clear();
 
         // Reload same user
@@ -141,8 +144,13 @@ class ManyToManyBasicAssociationTest extends OrmFunctionalTestCase
 
         $user->groups->clear();
 
+        $this->getQueryLog()->reset()->enable();
         $this->_em->flush();
 
+        // Deletions of entire collections happen in a single query
+        $this->removeTransactionCommandsFromQueryLog();
+        self::assertQueryCount(1);
+
         // Check that the links in the association table have been deleted
         $this->assertGblancoGroupCountIs(0);
     }
@@ -212,12 +220,20 @@ class ManyToManyBasicAssociationTest extends OrmFunctionalTestCase
     /** @group DDC-130 */
     public function testRemoveUserWithManyGroups(): void
     {
-        $user   = $this->addCmsUserGblancoWithGroups(2);
+        $user   = $this->addCmsUserGblancoWithGroups(10);
         $userId = $user->getId();
 
         $this->_em->remove($user);
+
+        $this->getQueryLog()->reset()->enable();
+
         $this->_em->flush();
 
+        // This takes three queries: One to delete all user -> group join table rows for the user,
+        // one to delete all user -> tags join table rows for the user, and a final one to delete the user itself.
+        $this->removeTransactionCommandsFromQueryLog();
+        self::assertQueryCount(3);
+
         $newUser = $this->_em->find(get_class($user), $userId);
         self::assertNull($newUser);
     }
@@ -225,15 +241,40 @@ class ManyToManyBasicAssociationTest extends OrmFunctionalTestCase
     /** @group DDC-130 */
     public function testRemoveGroupWithUser(): void
     {
-        $user = $this->addCmsUserGblancoWithGroups(2);
+        $user = $this->addCmsUserGblancoWithGroups(5);
+
+        $anotherUser           = new CmsUser();
+        $anotherUser->username = 'joe_doe';
+        $anotherUser->name     = 'Joe Doe';
+        $anotherUser->status   = 'QA Engineer';
+
+        foreach ($user->getGroups() as $group) {
+            $anotherUser->addGroup($group);
+        }
+
+        $this->_em->persist($anotherUser);
+        $this->_em->flush();
 
         foreach ($user->getGroups() as $group) {
             $this->_em->remove($group);
         }
 
+        $this->getQueryLog()->reset()->enable();
         $this->_em->flush();
+
+        // This takes 5 * 2 queries – for each group to be removed, one to remove all join table rows
+        // for the CmsGroup -> CmsUser inverse side association (for both users at once),
+        // and one for the group itself.
+        $this->removeTransactionCommandsFromQueryLog();
+        self::assertQueryCount(10);
+
+        // Changes to in-memory collection have been made and flushed
+        self::assertCount(0, $user->getGroups());
+        self::assertFalse($user->getGroups()->isDirty());
+
         $this->_em->clear();
 
+        // Changes have been made to the database
         $newUser = $this->_em->find(get_class($user), $user->getId());
         self::assertCount(0, $newUser->getGroups());
     }
@@ -243,7 +284,13 @@ class ManyToManyBasicAssociationTest extends OrmFunctionalTestCase
         $user         = $this->addCmsUserGblancoWithGroups(2);
         $user->groups = null;
 
+        $this->getQueryLog()->reset()->enable();
         $this->_em->flush();
+
+        // It takes one query to remove all join table rows for the user at once
+        $this->removeTransactionCommandsFromQueryLog();
+        self::assertQueryCount(1);
+
         $this->_em->clear();
 
         $newUser = $this->_em->find(get_class($user), $user->getId());
@@ -528,4 +575,15 @@ class ManyToManyBasicAssociationTest extends OrmFunctionalTestCase
 
         self::assertFalse($user->groups->isInitialized(), 'Post-condition: matching does not initialize collection');
     }
+
+    private function removeTransactionCommandsFromQueryLog(): void
+    {
+        $log = $this->getQueryLog();
+
+        foreach ($log->queries as $key => $entry) {
+            if ($entry['sql'] === '"START TRANSACTION"' || $entry['sql'] === '"COMMIT"') {
+                unset($log->queries[$key]);
+            }
+        }
+    }
 }
diff --git a/tests/Doctrine/Tests/ORM/Functional/ManyToManyEventTest.php b/tests/Doctrine/Tests/ORM/Functional/ManyToManyEventTest.php
index 03d547e6b..d9436acb2 100644
--- a/tests/Doctrine/Tests/ORM/Functional/ManyToManyEventTest.php
+++ b/tests/Doctrine/Tests/ORM/Functional/ManyToManyEventTest.php
@@ -30,21 +30,22 @@ class ManyToManyEventTest extends OrmFunctionalTestCase
 
     public function testListenerShouldBeNotifiedWhenNewCollectionEntryAdded(): void
     {
-        $user = $this->createNewValidUser();
+        $user        = $this->createNewValidUser();
+        $group       = new CmsGroup();
+        $group->name = 'admins';
+
         $this->_em->persist($user);
+        $this->_em->persist($group);
         $this->_em->flush();
         self::assertFalse($this->listener->wasNotified);
 
-        $group       = new CmsGroup();
-        $group->name = 'admins';
         $user->addGroup($group);
-        $this->_em->persist($user);
         $this->_em->flush();
 
         self::assertTrue($this->listener->wasNotified);
     }
 
-    public function testListenerShouldBeNotifiedWhenNewCollectionEntryRemoved(): void
+    public function testListenerShouldBeNotifiedWhenCollectionEntryRemoved(): void
     {
         $user        = $this->createNewValidUser();
         $group       = new CmsGroup();
@@ -52,10 +53,11 @@ class ManyToManyEventTest extends OrmFunctionalTestCase
         $user->addGroup($group);
 
         $this->_em->persist($user);
+        $this->_em->persist($group);
         $this->_em->flush();
         self::assertFalse($this->listener->wasNotified);
 
-        $this->_em->remove($group);
+        $user->getGroups()->removeElement($group);
         $this->_em->flush();
 
         self::assertTrue($this->listener->wasNotified);