From 866a424963aef890e984a1321465ae87fd2b51cb Mon Sep 17 00:00:00 2001 From: xhuberty Date: Tue, 12 Jan 2016 15:01:34 +0100 Subject: [PATCH 1/8] Fix issue when using notify tracking policy with multiple flush on entity --- lib/Doctrine/ORM/UnitOfWork.php | 42 ++++++++++++++++++--- tests/Doctrine/Tests/ORM/UnitOfWorkTest.php | 31 ++++++++++++++- 2 files changed, 67 insertions(+), 6 deletions(-) diff --git a/lib/Doctrine/ORM/UnitOfWork.php b/lib/Doctrine/ORM/UnitOfWork.php index c0b0dd8c3..367d9f4d2 100644 --- a/lib/Doctrine/ORM/UnitOfWork.php +++ b/lib/Doctrine/ORM/UnitOfWork.php @@ -324,7 +324,7 @@ class UnitOfWork implements PropertyChangedListener } // Compute changes done since last commit. - if ($entity === null) { + if (null === $entity) { $this->computeChangeSets(); } elseif (is_object($entity)) { $this->computeSingleEntityChangeSet($entity); @@ -414,17 +414,40 @@ class UnitOfWork implements PropertyChangedListener $this->dispatchPostFlushEvent(); + $this->postCommitClear($entity); + } + + /** + * @param null|object|array $entity + */ + private function postCommitClear($entity = null) + { + // Clear up $this->entityInsertions = $this->entityUpdates = $this->entityDeletions = $this->extraUpdates = - $this->entityChangeSets = $this->collectionUpdates = $this->collectionDeletions = $this->visitedCollections = - $this->scheduledForSynchronization = - $this->orphanRemovals = []; + $this->orphanRemovals = array(); + + if (null === $entity) { + $this->entityChangeSets = $this->scheduledForSynchronization = []; + return; + } + + if (is_object($entity)) { + $entity = [$entity]; + } + + foreach ($entity as $object) { + $oid = spl_object_hash($object); + $class = $this->em->getClassMetadata(get_class($object)); + $this->clearEntityChangeSet($oid); + $this->clearScheduledForSynchronization($class, $oid); + } } /** @@ -3099,11 +3122,20 @@ class UnitOfWork implements PropertyChangedListener * * @return void */ - public function clearEntityChangeSet($oid) + private function clearEntityChangeSet($oid) { $this->entityChangeSets[$oid] = []; } + /** + * @param $class + * @param string $oid + */ + private function clearScheduledForSynchronization($class, $oid) + { + unset($this->scheduledForSynchronization[$class->rootEntityName][$oid]); + } + /* PropertyChangedListener implementation */ /** diff --git a/tests/Doctrine/Tests/ORM/UnitOfWorkTest.php b/tests/Doctrine/Tests/ORM/UnitOfWorkTest.php index ff025df4f..3af8fc5cd 100644 --- a/tests/Doctrine/Tests/ORM/UnitOfWorkTest.php +++ b/tests/Doctrine/Tests/ORM/UnitOfWorkTest.php @@ -163,7 +163,8 @@ class UnitOfWorkTest extends OrmTestCase $this->_unitOfWork->persist($entity); $this->_unitOfWork->commit(); - $this->assertEquals(1, count($persister->getInserts())); + $this->assertCount(1, $persister->getInserts()); + $persister->reset(); $this->assertTrue($this->_unitOfWork->isInIdentityMap($entity)); @@ -360,6 +361,34 @@ class UnitOfWorkTest extends OrmTestCase $this->assertFalse($this->_unitOfWork->isScheduledForInsert($entity2)); } + /** + * @group 5579 + */ + public function testEntityChangeSetNotClearAfterFlushOnEntityOrArrayOfEntity() + { + // Create and Set first entity + $entity1 = new NotifyChangedEntity; + $entity1->setData('thedata'); + $this->_unitOfWork->persist($entity1); + + // Create and Set second entity + $entity2 = new NotifyChangedEntity; + $entity2->setData('thedata'); + $this->_unitOfWork->persist($entity2); + + $this->_unitOfWork->commit($entity1); + $this->assertCount(1, $this->_unitOfWork->getEntityChangeSet($entity2)); + + // Create and Set third entity + $entity3 = new NotifyChangedEntity; + $entity3->setData('thedata'); + $this->_unitOfWork->persist($entity3); + + $this->_unitOfWork->commit([$entity1,$entity2]); + $this->assertCount(1, $this->_unitOfWork->getEntityChangeSet($entity3)); + + } + /** * Data Provider * From df1577db0c519bdd5ce52113c1e0c9009205f929 Mon Sep 17 00:00:00 2001 From: Marco Pivetta Date: Fri, 18 Aug 2017 09:13:56 +0200 Subject: [PATCH 2/8] #5579 cleaning up test case, adding assertions critical to the understanding of the code --- tests/Doctrine/Tests/ORM/UnitOfWorkTest.php | 22 ++++++++++----------- 1 file changed, 10 insertions(+), 12 deletions(-) diff --git a/tests/Doctrine/Tests/ORM/UnitOfWorkTest.php b/tests/Doctrine/Tests/ORM/UnitOfWorkTest.php index 3af8fc5cd..d67525696 100644 --- a/tests/Doctrine/Tests/ORM/UnitOfWorkTest.php +++ b/tests/Doctrine/Tests/ORM/UnitOfWorkTest.php @@ -362,31 +362,29 @@ class UnitOfWorkTest extends OrmTestCase } /** - * @group 5579 + * @group #5579 */ - public function testEntityChangeSetNotClearAfterFlushOnEntityOrArrayOfEntity() + public function testEntityChangeSetIsNotClearedAfterFlushOnEntityOrArrayOfEntity() : void { - // Create and Set first entity $entity1 = new NotifyChangedEntity; - $entity1->setData('thedata'); - $this->_unitOfWork->persist($entity1); - - // Create and Set second entity $entity2 = new NotifyChangedEntity; + $entity3 = new NotifyChangedEntity; + + $entity1->setData('thedata'); $entity2->setData('thedata'); + $entity3->setData('thedata'); + + $this->_unitOfWork->persist($entity1); $this->_unitOfWork->persist($entity2); $this->_unitOfWork->commit($entity1); + $this->assertEmpty($this->_unitOfWork->getEntityChangeSet($entity1)); $this->assertCount(1, $this->_unitOfWork->getEntityChangeSet($entity2)); - // Create and Set third entity - $entity3 = new NotifyChangedEntity; - $entity3->setData('thedata'); $this->_unitOfWork->persist($entity3); - $this->_unitOfWork->commit([$entity1,$entity2]); + $this->_unitOfWork->commit([$entity1, $entity2]); $this->assertCount(1, $this->_unitOfWork->getEntityChangeSet($entity3)); - } /** From 9707701d1056b1860d159abf732985e702fa3b9a Mon Sep 17 00:00:00 2001 From: Marco Pivetta Date: Fri, 18 Aug 2017 09:15:48 +0200 Subject: [PATCH 3/8] #5579 isolating multi-entity-commit and single-entity-commit scenarios for the tracking policy changeset clearing --- tests/Doctrine/Tests/ORM/UnitOfWorkTest.php | 33 ++++++++++++++++----- 1 file changed, 25 insertions(+), 8 deletions(-) diff --git a/tests/Doctrine/Tests/ORM/UnitOfWorkTest.php b/tests/Doctrine/Tests/ORM/UnitOfWorkTest.php index d67525696..1d3ca771e 100644 --- a/tests/Doctrine/Tests/ORM/UnitOfWorkTest.php +++ b/tests/Doctrine/Tests/ORM/UnitOfWorkTest.php @@ -364,7 +364,26 @@ class UnitOfWorkTest extends OrmTestCase /** * @group #5579 */ - public function testEntityChangeSetIsNotClearedAfterFlushOnEntityOrArrayOfEntity() : void + public function testEntityChangeSetIsNotClearedAfterFlushOnSingleEntity() : void + { + $entity1 = new NotifyChangedEntity; + $entity2 = new NotifyChangedEntity; + + $entity1->setData('thedata'); + $entity2->setData('thedata'); + + $this->_unitOfWork->persist($entity1); + $this->_unitOfWork->persist($entity2); + + $this->_unitOfWork->commit($entity1); + $this->assertEmpty($this->_unitOfWork->getEntityChangeSet($entity1)); + $this->assertCount(1, $this->_unitOfWork->getEntityChangeSet($entity2)); + } + + /** + * @group #5579 + */ + public function testEntityChangeSetIsNotClearedAfterFlushOnArrayOfEntities() : void { $entity1 = new NotifyChangedEntity; $entity2 = new NotifyChangedEntity; @@ -376,15 +395,13 @@ class UnitOfWorkTest extends OrmTestCase $this->_unitOfWork->persist($entity1); $this->_unitOfWork->persist($entity2); - - $this->_unitOfWork->commit($entity1); - $this->assertEmpty($this->_unitOfWork->getEntityChangeSet($entity1)); - $this->assertCount(1, $this->_unitOfWork->getEntityChangeSet($entity2)); - $this->_unitOfWork->persist($entity3); - $this->_unitOfWork->commit([$entity1, $entity2]); - $this->assertCount(1, $this->_unitOfWork->getEntityChangeSet($entity3)); + $this->_unitOfWork->commit([$entity1, $entity3]); + + $this->assertEmpty($this->_unitOfWork->getEntityChangeSet($entity1)); + $this->assertEmpty($this->_unitOfWork->getEntityChangeSet($entity3)); + $this->assertCount(1, $this->_unitOfWork->getEntityChangeSet($entity2)); } /** From 2921f068b7df94fa2576249f83ea0a7ef961a0b4 Mon Sep 17 00:00:00 2001 From: Marco Pivetta Date: Fri, 18 Aug 2017 09:21:53 +0200 Subject: [PATCH 4/8] #5579 cleaning up `postCommitClear` implementation --- lib/Doctrine/ORM/UnitOfWork.php | 33 ++++++++++++--------------------- 1 file changed, 12 insertions(+), 21 deletions(-) diff --git a/lib/Doctrine/ORM/UnitOfWork.php b/lib/Doctrine/ORM/UnitOfWork.php index 367d9f4d2..0e8fb4caa 100644 --- a/lib/Doctrine/ORM/UnitOfWork.php +++ b/lib/Doctrine/ORM/UnitOfWork.php @@ -418,12 +418,10 @@ class UnitOfWork implements PropertyChangedListener } /** - * @param null|object|array $entity + * @param null|object|object[] $entity */ - private function postCommitClear($entity = null) + private function postCommitClear($entity) : void { - - // Clear up $this->entityInsertions = $this->entityUpdates = $this->entityDeletions = @@ -431,22 +429,24 @@ class UnitOfWork implements PropertyChangedListener $this->collectionUpdates = $this->collectionDeletions = $this->visitedCollections = - $this->orphanRemovals = array(); + $this->orphanRemovals = []; if (null === $entity) { $this->entityChangeSets = $this->scheduledForSynchronization = []; + return; } - if (is_object($entity)) { - $entity = [$entity]; - } + $entities = \is_object($entity) + ? [$entity] + : $entity; + + foreach ($entities as $object) { + $oid = \spl_object_hash($object); - foreach ($entity as $object) { - $oid = spl_object_hash($object); - $class = $this->em->getClassMetadata(get_class($object)); $this->clearEntityChangeSet($oid); - $this->clearScheduledForSynchronization($class, $oid); + + unset($this->scheduledForSynchronization[$this->em->getClassMetadata(\get_class($object))->rootEntityName][$oid]); } } @@ -3127,15 +3127,6 @@ class UnitOfWork implements PropertyChangedListener $this->entityChangeSets[$oid] = []; } - /** - * @param $class - * @param string $oid - */ - private function clearScheduledForSynchronization($class, $oid) - { - unset($this->scheduledForSynchronization[$class->rootEntityName][$oid]); - } - /* PropertyChangedListener implementation */ /** From 9d7be0f927fdd4db955f41e4fc939860a34c6399 Mon Sep 17 00:00:00 2001 From: Marco Pivetta Date: Fri, 18 Aug 2017 09:22:23 +0200 Subject: [PATCH 5/8] #5579 s/clear/cleanup --- lib/Doctrine/ORM/UnitOfWork.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/Doctrine/ORM/UnitOfWork.php b/lib/Doctrine/ORM/UnitOfWork.php index 0e8fb4caa..4cdf26f96 100644 --- a/lib/Doctrine/ORM/UnitOfWork.php +++ b/lib/Doctrine/ORM/UnitOfWork.php @@ -414,13 +414,13 @@ class UnitOfWork implements PropertyChangedListener $this->dispatchPostFlushEvent(); - $this->postCommitClear($entity); + $this->postCommitCleanup($entity); } /** * @param null|object|object[] $entity */ - private function postCommitClear($entity) : void + private function postCommitCleanup($entity) : void { $this->entityInsertions = $this->entityUpdates = From ff15a2bc79451c54c607800caee506aa00462835 Mon Sep 17 00:00:00 2001 From: Marco Pivetta Date: Fri, 18 Aug 2017 09:23:28 +0200 Subject: [PATCH 6/8] #5579 correcting visibility of `clearEntityChangeSet`, which should be reverted to `public` --- lib/Doctrine/ORM/UnitOfWork.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/Doctrine/ORM/UnitOfWork.php b/lib/Doctrine/ORM/UnitOfWork.php index 4cdf26f96..349be6dcb 100644 --- a/lib/Doctrine/ORM/UnitOfWork.php +++ b/lib/Doctrine/ORM/UnitOfWork.php @@ -3122,7 +3122,7 @@ class UnitOfWork implements PropertyChangedListener * * @return void */ - private function clearEntityChangeSet($oid) + public function clearEntityChangeSet($oid) { $this->entityChangeSets[$oid] = []; } From 1bf8465f439e17689c30a7dcf3c64d0f04259c7f Mon Sep 17 00:00:00 2001 From: Marco Pivetta Date: Fri, 18 Aug 2017 09:25:47 +0200 Subject: [PATCH 7/8] #5579 correcting assertion static/instance method usage --- tests/Doctrine/Tests/ORM/UnitOfWorkTest.php | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/tests/Doctrine/Tests/ORM/UnitOfWorkTest.php b/tests/Doctrine/Tests/ORM/UnitOfWorkTest.php index 1d3ca771e..bfad30be8 100644 --- a/tests/Doctrine/Tests/ORM/UnitOfWorkTest.php +++ b/tests/Doctrine/Tests/ORM/UnitOfWorkTest.php @@ -376,8 +376,8 @@ class UnitOfWorkTest extends OrmTestCase $this->_unitOfWork->persist($entity2); $this->_unitOfWork->commit($entity1); - $this->assertEmpty($this->_unitOfWork->getEntityChangeSet($entity1)); - $this->assertCount(1, $this->_unitOfWork->getEntityChangeSet($entity2)); + self::assertEmpty($this->_unitOfWork->getEntityChangeSet($entity1)); + self::assertCount(1, $this->_unitOfWork->getEntityChangeSet($entity2)); } /** @@ -399,9 +399,9 @@ class UnitOfWorkTest extends OrmTestCase $this->_unitOfWork->commit([$entity1, $entity3]); - $this->assertEmpty($this->_unitOfWork->getEntityChangeSet($entity1)); - $this->assertEmpty($this->_unitOfWork->getEntityChangeSet($entity3)); - $this->assertCount(1, $this->_unitOfWork->getEntityChangeSet($entity2)); + self::assertEmpty($this->_unitOfWork->getEntityChangeSet($entity1)); + self::assertEmpty($this->_unitOfWork->getEntityChangeSet($entity3)); + self::assertCount(1, $this->_unitOfWork->getEntityChangeSet($entity2)); } /** From 496c22db0eab243c5dd54368150fce4f93f787e1 Mon Sep 17 00:00:00 2001 From: Marco Pivetta Date: Fri, 18 Aug 2017 09:27:47 +0200 Subject: [PATCH 8/8] #5579 completely removing a changeset when cleaning up --- lib/Doctrine/ORM/UnitOfWork.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/Doctrine/ORM/UnitOfWork.php b/lib/Doctrine/ORM/UnitOfWork.php index 349be6dcb..4f8ce32db 100644 --- a/lib/Doctrine/ORM/UnitOfWork.php +++ b/lib/Doctrine/ORM/UnitOfWork.php @@ -3124,7 +3124,7 @@ class UnitOfWork implements PropertyChangedListener */ public function clearEntityChangeSet($oid) { - $this->entityChangeSets[$oid] = []; + unset($this->entityChangeSets[$oid]); } /* PropertyChangedListener implementation */