From 70603ee3db1afd985d89d56350bd21eedddb03c0 Mon Sep 17 00:00:00 2001 From: Rico Humme Date: Fri, 3 Jun 2016 13:48:05 +0200 Subject: [PATCH 01/10] Clear entityInsertions for specific entityName --- lib/Doctrine/ORM/UnitOfWork.php | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/lib/Doctrine/ORM/UnitOfWork.php b/lib/Doctrine/ORM/UnitOfWork.php index a66d7b462..0b71372e3 100644 --- a/lib/Doctrine/ORM/UnitOfWork.php +++ b/lib/Doctrine/ORM/UnitOfWork.php @@ -2401,6 +2401,13 @@ class UnitOfWork implements PropertyChangedListener $this->doDetach($entity, $visited, false); } } + + foreach ($this->entityInsertions as $hash => $entity) { + if (get_class($entity) != $entityName) { + continue; + } + unset($this->entityInsertions[$hash]); + } } if ($this->evm->hasListeners(Events::onClear)) { From 3df494ddc8f3f8e3e502c13847856cd6146e12e0 Mon Sep 17 00:00:00 2001 From: Rico Humme Date: Fri, 3 Jun 2016 16:13:52 +0200 Subject: [PATCH 02/10] Test Case for Clear entityInsertions for specific entityName --- tests/Doctrine/Tests/ORM/UnitOfWorkTest.php | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/tests/Doctrine/Tests/ORM/UnitOfWorkTest.php b/tests/Doctrine/Tests/ORM/UnitOfWorkTest.php index eb8bf3a5c..c4dd4adae 100644 --- a/tests/Doctrine/Tests/ORM/UnitOfWorkTest.php +++ b/tests/Doctrine/Tests/ORM/UnitOfWorkTest.php @@ -323,6 +323,27 @@ class UnitOfWorkTest extends OrmTestCase $this->assertTrue($this->_unitOfWork->isInIdentityMap($entity)); } + public function testPersistedEntityAndClearManager() + { + $entity1 = new ForumUser(); + $entity1->id = 123; + + $entity2 = new ForumAvatar(); + $entity2->id = 456; + + $this->_unitOfWork->persist($entity1); + $this->assertTrue($this->_unitOfWork->isInIdentityMap($entity1)); + + $this->_unitOfWork->persist($entity2); + $this->assertTrue($this->_unitOfWork->isInIdentityMap($entity2)); + + $this->_unitOfWork->clear(ForumAvatar::class); + $this->assertTrue($this->_unitOfWork->isInIdentityMap($entity1)); + $this->assertFalse($this->_unitOfWork->isInIdentityMap($entity2)); + $this->assertTrue($this->_unitOfWork->isScheduledForInsert($entity1)); + $this->assertFalse($this->_unitOfWork->isScheduledForInsert($entity2)); + } + /** * Data Provider * From 4d48781e2bd9aa3aa7b0b3652a1c875321e99bf4 Mon Sep 17 00:00:00 2001 From: Rico Humme Date: Fri, 3 Jun 2016 16:27:27 +0200 Subject: [PATCH 03/10] Split of functionality in separate functions --- lib/Doctrine/ORM/UnitOfWork.php | 50 +++++++++++++++++++++------------ 1 file changed, 32 insertions(+), 18 deletions(-) diff --git a/lib/Doctrine/ORM/UnitOfWork.php b/lib/Doctrine/ORM/UnitOfWork.php index 0b71372e3..31260b15a 100644 --- a/lib/Doctrine/ORM/UnitOfWork.php +++ b/lib/Doctrine/ORM/UnitOfWork.php @@ -2390,24 +2390,8 @@ class UnitOfWork implements PropertyChangedListener $this->visitedCollections = $this->orphanRemovals = array(); } else { - $visited = array(); - - foreach ($this->identityMap as $className => $entities) { - if ($className !== $entityName) { - continue; - } - - foreach ($entities as $entity) { - $this->doDetach($entity, $visited, false); - } - } - - foreach ($this->entityInsertions as $hash => $entity) { - if (get_class($entity) != $entityName) { - continue; - } - unset($this->entityInsertions[$hash]); - } + $this->clearIdentityMap($entityName); + $this->clearIdentityInsertions($entityName); } if ($this->evm->hasListeners(Events::onClear)) { @@ -3470,4 +3454,34 @@ class UnitOfWork implements PropertyChangedListener { $this->hydrationCompleteHandler->hydrationComplete(); } + + /** + * @param $entityName + */ + private function clearIdentityMap($entityName) + { + $visited = array(); + + foreach ($this->identityMap as $className => $entities) { + if ($className !== $entityName) { + continue; + } + + foreach ($entities as $entity) { + $this->doDetach($entity, $visited, false); + } + } + } + + /** + * @param $entityName + */ + private function clearIdentityInsertions($entityName) + { + foreach ($this->entityInsertions as $hash => $entity) { + if (get_class($entity) === $entityName) { + unset($this->entityInsertions[$hash]); + } + } + } } From beb26414926757a63e65a7b860fd0f1b77121dcf Mon Sep 17 00:00:00 2001 From: Rico Humme Date: Fri, 3 Jun 2016 16:31:35 +0200 Subject: [PATCH 04/10] Correct naming convention of function. Was confusing otherwise --- 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 31260b15a..13e329c51 100644 --- a/lib/Doctrine/ORM/UnitOfWork.php +++ b/lib/Doctrine/ORM/UnitOfWork.php @@ -2391,7 +2391,7 @@ class UnitOfWork implements PropertyChangedListener $this->orphanRemovals = array(); } else { $this->clearIdentityMap($entityName); - $this->clearIdentityInsertions($entityName); + $this->clearEntityInsertions($entityName); } if ($this->evm->hasListeners(Events::onClear)) { @@ -3476,7 +3476,7 @@ class UnitOfWork implements PropertyChangedListener /** * @param $entityName */ - private function clearIdentityInsertions($entityName) + private function clearEntityInsertions($entityName) { foreach ($this->entityInsertions as $hash => $entity) { if (get_class($entity) === $entityName) { From 313e4a33e54b34015aedc3bf6c286dd1cbca7e5a Mon Sep 17 00:00:00 2001 From: Marco Pivetta Date: Sun, 5 Jun 2016 23:54:16 +0200 Subject: [PATCH 05/10] #5849 #5850 adding group annotations to the newly introduced test case --- tests/Doctrine/Tests/ORM/UnitOfWorkTest.php | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/tests/Doctrine/Tests/ORM/UnitOfWorkTest.php b/tests/Doctrine/Tests/ORM/UnitOfWorkTest.php index c4dd4adae..2d5b6a883 100644 --- a/tests/Doctrine/Tests/ORM/UnitOfWorkTest.php +++ b/tests/Doctrine/Tests/ORM/UnitOfWorkTest.php @@ -323,6 +323,10 @@ class UnitOfWorkTest extends OrmTestCase $this->assertTrue($this->_unitOfWork->isInIdentityMap($entity)); } + /** + * @group 5849 + * @group 5850 + */ public function testPersistedEntityAndClearManager() { $entity1 = new ForumUser(); From 34d8e00df72803908ee77f567bc6a9f2dd93fbda Mon Sep 17 00:00:00 2001 From: Marco Pivetta Date: Mon, 6 Jun 2016 00:08:26 +0200 Subject: [PATCH 06/10] #5849 #5850 correcting test scenario: identity map could not be built with auto-generated identities+persist --- tests/Doctrine/Tests/ORM/UnitOfWorkTest.php | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/tests/Doctrine/Tests/ORM/UnitOfWorkTest.php b/tests/Doctrine/Tests/ORM/UnitOfWorkTest.php index 2d5b6a883..8755a4eaa 100644 --- a/tests/Doctrine/Tests/ORM/UnitOfWorkTest.php +++ b/tests/Doctrine/Tests/ORM/UnitOfWorkTest.php @@ -15,6 +15,8 @@ use Doctrine\Tests\Mocks\UnitOfWorkMock; use Doctrine\Tests\Models\CMS\CmsPhonenumber; use Doctrine\Tests\Models\Forum\ForumAvatar; use Doctrine\Tests\Models\Forum\ForumUser; +use Doctrine\Tests\Models\GeoNames\City; +use Doctrine\Tests\Models\GeoNames\Country; use Doctrine\Tests\OrmTestCase; use stdClass; @@ -329,11 +331,8 @@ class UnitOfWorkTest extends OrmTestCase */ public function testPersistedEntityAndClearManager() { - $entity1 = new ForumUser(); - $entity1->id = 123; - - $entity2 = new ForumAvatar(); - $entity2->id = 456; + $entity1 = new City(123, 'London'); + $entity2 = new Country(456, 'United Kingdom'); $this->_unitOfWork->persist($entity1); $this->assertTrue($this->_unitOfWork->isInIdentityMap($entity1)); @@ -341,7 +340,7 @@ class UnitOfWorkTest extends OrmTestCase $this->_unitOfWork->persist($entity2); $this->assertTrue($this->_unitOfWork->isInIdentityMap($entity2)); - $this->_unitOfWork->clear(ForumAvatar::class); + $this->_unitOfWork->clear(Country::class); $this->assertTrue($this->_unitOfWork->isInIdentityMap($entity1)); $this->assertFalse($this->_unitOfWork->isInIdentityMap($entity2)); $this->assertTrue($this->_unitOfWork->isScheduledForInsert($entity1)); From 14e080029316819de5d9f2be106affa4933072cd Mon Sep 17 00:00:00 2001 From: Marco Pivetta Date: Mon, 6 Jun 2016 00:10:18 +0200 Subject: [PATCH 07/10] #5849 #5850 renamed `clearIdentityMap` to `clearIdentityMapForEntityName`, for clarity --- lib/Doctrine/ORM/UnitOfWork.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/Doctrine/ORM/UnitOfWork.php b/lib/Doctrine/ORM/UnitOfWork.php index 13e329c51..5c943297d 100644 --- a/lib/Doctrine/ORM/UnitOfWork.php +++ b/lib/Doctrine/ORM/UnitOfWork.php @@ -2390,7 +2390,7 @@ class UnitOfWork implements PropertyChangedListener $this->visitedCollections = $this->orphanRemovals = array(); } else { - $this->clearIdentityMap($entityName); + $this->clearIdentityMapForEntityName($entityName); $this->clearEntityInsertions($entityName); } @@ -3456,9 +3456,9 @@ class UnitOfWork implements PropertyChangedListener } /** - * @param $entityName + * @param string $entityName */ - private function clearIdentityMap($entityName) + private function clearIdentityMapForEntityName($entityName) { $visited = array(); From 20d86c5b27b8af6c567aefe7baaf7e66d13c4b95 Mon Sep 17 00:00:00 2001 From: Marco Pivetta Date: Mon, 6 Jun 2016 00:11:19 +0200 Subject: [PATCH 08/10] #5849 #5850 refactored `clearIdentityMapForEntityName` to remove useless looping --- lib/Doctrine/ORM/UnitOfWork.php | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/lib/Doctrine/ORM/UnitOfWork.php b/lib/Doctrine/ORM/UnitOfWork.php index 5c943297d..0876740da 100644 --- a/lib/Doctrine/ORM/UnitOfWork.php +++ b/lib/Doctrine/ORM/UnitOfWork.php @@ -3460,16 +3460,14 @@ class UnitOfWork implements PropertyChangedListener */ private function clearIdentityMapForEntityName($entityName) { - $visited = array(); + if (! isset($this->identityMap[$entityName])) { + return; + } - foreach ($this->identityMap as $className => $entities) { - if ($className !== $entityName) { - continue; - } + $visited = []; - foreach ($entities as $entity) { - $this->doDetach($entity, $visited, false); - } + foreach ($this->identityMap[$entityName] as $entity) { + $this->doDetach($entity, $visited, false); } } From b9b952ce8a5bb4abce188dbfeef62c827edcfcb9 Mon Sep 17 00:00:00 2001 From: Marco Pivetta Date: Mon, 6 Jun 2016 00:13:39 +0200 Subject: [PATCH 09/10] #5849 #5850 renamed `clearEntityInsertions` to `clearEntityInsertionsForEntityName`, for clarity --- lib/Doctrine/ORM/UnitOfWork.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/Doctrine/ORM/UnitOfWork.php b/lib/Doctrine/ORM/UnitOfWork.php index 0876740da..8efeb9e8f 100644 --- a/lib/Doctrine/ORM/UnitOfWork.php +++ b/lib/Doctrine/ORM/UnitOfWork.php @@ -2391,7 +2391,7 @@ class UnitOfWork implements PropertyChangedListener $this->orphanRemovals = array(); } else { $this->clearIdentityMapForEntityName($entityName); - $this->clearEntityInsertions($entityName); + $this->clearEntityInsertionsForEntityName($entityName); } if ($this->evm->hasListeners(Events::onClear)) { @@ -3472,9 +3472,9 @@ class UnitOfWork implements PropertyChangedListener } /** - * @param $entityName + * @param string $entityName */ - private function clearEntityInsertions($entityName) + private function clearEntityInsertionsForEntityName($entityName) { foreach ($this->entityInsertions as $hash => $entity) { if (get_class($entity) === $entityName) { From 68c5d761a8ccf90a6cb463036483ebb7f05aecd5 Mon Sep 17 00:00:00 2001 From: Marco Pivetta Date: Mon, 6 Jun 2016 00:25:48 +0200 Subject: [PATCH 10/10] #5849 #5850 minor performance optimization - avoiding `get_class()` calls on all entity insertions --- lib/Doctrine/ORM/UnitOfWork.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/Doctrine/ORM/UnitOfWork.php b/lib/Doctrine/ORM/UnitOfWork.php index 8efeb9e8f..7044ce57e 100644 --- a/lib/Doctrine/ORM/UnitOfWork.php +++ b/lib/Doctrine/ORM/UnitOfWork.php @@ -3477,7 +3477,8 @@ class UnitOfWork implements PropertyChangedListener private function clearEntityInsertionsForEntityName($entityName) { foreach ($this->entityInsertions as $hash => $entity) { - if (get_class($entity) === $entityName) { + // note: performance optimization - `instanceof` is much faster than a function call + if ($entity instanceof $entityName && get_class($entity) === $entityName) { unset($this->entityInsertions[$hash]); } }