Fix to-many collections left in dirty state after entities are remove…

…d by the UoW
doctrine · May 11, 2023 · a951369 · a951369
1 parent 70477d8
commit a951369
Show file tree

Hide file tree

Showing 3 changed files with 82 additions and 8 deletions.
diff --git a/lib/Doctrine/ORM/UnitOfWork.php b/lib/Doctrine/ORM/UnitOfWork.php
@@ -916,13 +916,6 @@ private function computeAssociationChanges(array $assoc, $value): void
             return;
         }
 
-        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.
@@ -983,6 +976,13 @@ 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;
+        }
     }
 
     /**

diff --git a/tests/Doctrine/Tests/ORM/Functional/ManyToManyEventTest.php b/tests/Doctrine/Tests/ORM/Functional/ManyToManyEventTest.php
@@ -28,7 +28,7 @@ protected function setUp(): void
         $evm->addEventListener(Events::postUpdate, $this->listener);
     }
 
-    public function testListenerShouldBeNotifiedOnlyWhenUpdating(): void
+    public function testListenerShouldBeNotifiedWhenNewCollectionEntryAdded(): void
     {
         $user = $this->createNewValidUser();
         $this->_em->persist($user);
@@ -44,6 +44,23 @@ public function testListenerShouldBeNotifiedOnlyWhenUpdating(): void
         self::assertTrue($this->listener->wasNotified);
     }
 
+    public function testListenerShouldBeNotifiedWhenNewCollectionEntryRemoved(): void
+    {
+        $user        = $this->createNewValidUser();
+        $group       = new CmsGroup();
+        $group->name = 'admins';
+        $user->addGroup($group);
+
+        $this->_em->persist($user);
+        $this->_em->flush();
+        self::assertFalse($this->listener->wasNotified);
+
+        $this->_em->remove($group);
+        $this->_em->flush();
+
+        self::assertTrue($this->listener->wasNotified);
+    }
+
     private function createNewValidUser(): CmsUser
     {
         $user           = new CmsUser();

diff --git a/tests/Doctrine/Tests/ORM/UnitOfWorkTest.php b/tests/Doctrine/Tests/ORM/UnitOfWorkTest.php
@@ -31,6 +31,7 @@
 use Doctrine\Tests\Mocks\EntityManagerMock;
 use Doctrine\Tests\Mocks\EntityPersisterMock;
 use Doctrine\Tests\Mocks\UnitOfWorkMock;
+use Doctrine\Tests\Models\CMS\CmsGroup;
 use Doctrine\Tests\Models\CMS\CmsPhonenumber;
 use Doctrine\Tests\Models\CMS\CmsUser;
 use Doctrine\Tests\Models\Forum\ForumAvatar;
@@ -866,6 +867,62 @@ public function testItThrowsWhenLookingUpIdentifierForUnknownEntity(): void
         $this->expectException(EntityNotFoundException::class);
         $this->_unitOfWork->getEntityIdentifier(new stdClass());
     }
+
+    public function testRemovedEntityIsRemovedFromManyToManyCollection(): void
+    {
+        $group       = new CmsGroup();
+        $group->name = 'test';
+        $this->_unitOfWork->persist($group);
+
+        $user       = new CmsUser();
+        $user->name = 'test';
+        $user->groups->add($group);
+        $this->_unitOfWork->persist($user);
+
+        $this->_unitOfWork->commit();
+
+        self::assertFalse($user->groups->isDirty());
+
+        $this->_unitOfWork->remove($group);
+        $this->_unitOfWork->commit();
+
+        // Test that the removed entity has been removed from the many to many collection
+        self::assertEmpty(
+            $user->groups,
+            'the removed entity should have been removed from the many to many collection'
+        );
+
+        // Collection is clean, snapshot has been updated
+        self::assertFalse($user->groups->isDirty());
+        self::assertEmpty($user->groups->getSnapshot());
+    }
+
+    public function testRemovedEntityIsRemovedFromOneToManyCollection(): void
+    {
+        $user       = new CmsUser();
+        $user->name = 'test';
+
+        $phonenumber              = new CmsPhonenumber();
+        $phonenumber->phonenumber = '0800-123456';
+
+        $user->addPhonenumber($phonenumber);
+
+        $this->_unitOfWork->persist($user);
+        $this->_unitOfWork->persist($phonenumber);
+        $this->_unitOfWork->commit();
+
+        self::assertFalse($user->phonenumbers->isDirty());
+
+        $this->_unitOfWork->remove($phonenumber);
+        $this->_unitOfWork->commit();
+
+        // Test that the removed entity has been removed from the one to many collection
+        self::assertEmpty($user->phonenumbers);
+
+        // Collection is clean, snapshot has been updated
+        self::assertFalse($user->phonenumbers->isDirty());
+        self::assertEmpty($user->phonenumbers->getSnapshot());
+    }
 }
 
 /** @Entity */