From 4a87f00fab72d8cfe7f04e92ce8966cdbc9edb50 Mon Sep 17 00:00:00 2001 From: Jeremy Benoist Date: Sun, 11 Sep 2016 16:31:08 +0200 Subject: [PATCH 01/20] Avoid error when entityName isn't a string --- lib/Doctrine/ORM/UnitOfWork.php | 4 ++++ tests/Doctrine/Tests/ORM/UnitOfWorkTest.php | 14 ++++++++++++++ 2 files changed, 18 insertions(+) diff --git a/lib/Doctrine/ORM/UnitOfWork.php b/lib/Doctrine/ORM/UnitOfWork.php index fbbab7b6a..bc8d09fb8 100644 --- a/lib/Doctrine/ORM/UnitOfWork.php +++ b/lib/Doctrine/ORM/UnitOfWork.php @@ -3442,6 +3442,10 @@ class UnitOfWork implements PropertyChangedListener */ private function clearIdentityMapForEntityName($entityName) { + if (is_object($entityName)) { + return; + } + if (! isset($this->identityMap[$entityName])) { return; } diff --git a/tests/Doctrine/Tests/ORM/UnitOfWorkTest.php b/tests/Doctrine/Tests/ORM/UnitOfWorkTest.php index 0ff2572b9..84df1073d 100644 --- a/tests/Doctrine/Tests/ORM/UnitOfWorkTest.php +++ b/tests/Doctrine/Tests/ORM/UnitOfWorkTest.php @@ -349,6 +349,20 @@ class UnitOfWorkTest extends OrmTestCase $this->assertFalse($this->_unitOfWork->isScheduledForInsert($entity2)); } + public function testClearManagerWithObject() + { + $entity = new Country(456, 'United Kingdom'); + + $this->_unitOfWork->persist($entity); + $this->assertTrue($this->_unitOfWork->isInIdentityMap($entity)); + + $this->_unitOfWork->clear($entity); + + // true because entity wasn't a string so it wasn't cleared + $this->assertTrue($this->_unitOfWork->isInIdentityMap($entity)); + $this->assertTrue($this->_unitOfWork->isScheduledForInsert($entity)); + } + /** * Data Provider * From 2a7d21ad181f1ccbb18bf4fc1d7851d913b56dd7 Mon Sep 17 00:00:00 2001 From: Jeremy Benoist Date: Mon, 12 Sep 2016 12:08:42 +0200 Subject: [PATCH 02/20] Throw an exception instead of a workaround --- lib/Doctrine/ORM/UnitOfWork.php | 8 ++++---- tests/Doctrine/Tests/ORM/UnitOfWorkTest.php | 16 ++++++++++++++-- 2 files changed, 18 insertions(+), 6 deletions(-) diff --git a/lib/Doctrine/ORM/UnitOfWork.php b/lib/Doctrine/ORM/UnitOfWork.php index bc8d09fb8..1340dc891 100644 --- a/lib/Doctrine/ORM/UnitOfWork.php +++ b/lib/Doctrine/ORM/UnitOfWork.php @@ -2363,6 +2363,10 @@ class UnitOfWork implements PropertyChangedListener */ public function clear($entityName = null) { + if ($entityName !== null && !is_string($entityName)) { + throw new \InvalidArgumentException(sprintf('Argument 1 passed to %s() must be a string, %s given', __METHOD__, gettype($entityName))); + } + if ($entityName === null) { $this->identityMap = $this->entityIdentifiers = @@ -3442,10 +3446,6 @@ class UnitOfWork implements PropertyChangedListener */ private function clearIdentityMapForEntityName($entityName) { - if (is_object($entityName)) { - return; - } - if (! isset($this->identityMap[$entityName])) { return; } diff --git a/tests/Doctrine/Tests/ORM/UnitOfWorkTest.php b/tests/Doctrine/Tests/ORM/UnitOfWorkTest.php index 84df1073d..def2ae91a 100644 --- a/tests/Doctrine/Tests/ORM/UnitOfWorkTest.php +++ b/tests/Doctrine/Tests/ORM/UnitOfWorkTest.php @@ -351,16 +351,28 @@ class UnitOfWorkTest extends OrmTestCase public function testClearManagerWithObject() { + $this->expectException(\InvalidArgumentException::class); + $this->expectExceptionMessage('must be a string'); + $entity = new Country(456, 'United Kingdom'); $this->_unitOfWork->persist($entity); $this->assertTrue($this->_unitOfWork->isInIdentityMap($entity)); $this->_unitOfWork->clear($entity); + } - // true because entity wasn't a string so it wasn't cleared + public function testClearManagerWithNullValue() + { + $entity = new Country(456, 'United Kingdom'); + + $this->_unitOfWork->persist($entity); $this->assertTrue($this->_unitOfWork->isInIdentityMap($entity)); - $this->assertTrue($this->_unitOfWork->isScheduledForInsert($entity)); + + $this->_unitOfWork->clear(); + + $this->assertFalse($this->_unitOfWork->isInIdentityMap($entity)); + $this->assertFalse($this->_unitOfWork->isScheduledForInsert($entity)); } /** From be4aafd4f6aee5558842726cc7d53bc120def342 Mon Sep 17 00:00:00 2001 From: Jeremy Benoist Date: Sat, 26 Nov 2016 23:12:09 +0100 Subject: [PATCH 03/20] Use ORMException instead of a default exception --- lib/Doctrine/ORM/ORMException.php | 14 +++++++++++++- lib/Doctrine/ORM/UnitOfWork.php | 2 +- tests/Doctrine/Tests/ORM/UnitOfWorkTest.php | 3 ++- 3 files changed, 16 insertions(+), 3 deletions(-) diff --git a/lib/Doctrine/ORM/ORMException.php b/lib/Doctrine/ORM/ORMException.php index 919789d92..edfc47df6 100644 --- a/lib/Doctrine/ORM/ORMException.php +++ b/lib/Doctrine/ORM/ORMException.php @@ -101,7 +101,7 @@ class ORMException extends Exception return new self("Unrecognized field: $field"); } - /** + /** * * @param string $class * @param string $association @@ -340,4 +340,16 @@ class ORMException extends Exception { return new self("Can't use IN operator on entities that have composite keys."); } + + /** + * Used when a given entityName hasn't the good type + * + * @param mixed $entityName The given entity (which shouldn't be a string) + * + * @return ORMException + */ + public static function invalidEntityName($entityName) + { + return new self(sprintf('Entity name must be a string, %s given', gettype($entityName))); + } } diff --git a/lib/Doctrine/ORM/UnitOfWork.php b/lib/Doctrine/ORM/UnitOfWork.php index 1340dc891..8fbac956d 100644 --- a/lib/Doctrine/ORM/UnitOfWork.php +++ b/lib/Doctrine/ORM/UnitOfWork.php @@ -2364,7 +2364,7 @@ class UnitOfWork implements PropertyChangedListener public function clear($entityName = null) { if ($entityName !== null && !is_string($entityName)) { - throw new \InvalidArgumentException(sprintf('Argument 1 passed to %s() must be a string, %s given', __METHOD__, gettype($entityName))); + throw ORMException::invalidEntityName($entityName); } if ($entityName === null) { diff --git a/tests/Doctrine/Tests/ORM/UnitOfWorkTest.php b/tests/Doctrine/Tests/ORM/UnitOfWorkTest.php index def2ae91a..c60de753e 100644 --- a/tests/Doctrine/Tests/ORM/UnitOfWorkTest.php +++ b/tests/Doctrine/Tests/ORM/UnitOfWorkTest.php @@ -8,6 +8,7 @@ use Doctrine\Common\PropertyChangedListener; use Doctrine\ORM\Mapping\ClassMetadata; use Doctrine\ORM\ORMInvalidArgumentException; use Doctrine\ORM\UnitOfWork; +use Doctrine\ORM\ORMException; use Doctrine\Tests\Mocks\ConnectionMock; use Doctrine\Tests\Mocks\DriverMock; use Doctrine\Tests\Mocks\EntityManagerMock; @@ -351,7 +352,7 @@ class UnitOfWorkTest extends OrmTestCase public function testClearManagerWithObject() { - $this->expectException(\InvalidArgumentException::class); + $this->expectException(ORMException::class); $this->expectExceptionMessage('must be a string'); $entity = new Country(456, 'United Kingdom'); From 6b1d64d484b8cbf035eab575d8b194dbc250c8e1 Mon Sep 17 00:00:00 2001 From: Jeremy Benoist Date: Sun, 27 Nov 2016 18:05:18 +0100 Subject: [PATCH 04/20] Remove unecessary persist in tests --- tests/Doctrine/Tests/ORM/UnitOfWorkTest.php | 6 ------ 1 file changed, 6 deletions(-) diff --git a/tests/Doctrine/Tests/ORM/UnitOfWorkTest.php b/tests/Doctrine/Tests/ORM/UnitOfWorkTest.php index c60de753e..6e22ed498 100644 --- a/tests/Doctrine/Tests/ORM/UnitOfWorkTest.php +++ b/tests/Doctrine/Tests/ORM/UnitOfWorkTest.php @@ -357,9 +357,6 @@ class UnitOfWorkTest extends OrmTestCase $entity = new Country(456, 'United Kingdom'); - $this->_unitOfWork->persist($entity); - $this->assertTrue($this->_unitOfWork->isInIdentityMap($entity)); - $this->_unitOfWork->clear($entity); } @@ -367,9 +364,6 @@ class UnitOfWorkTest extends OrmTestCase { $entity = new Country(456, 'United Kingdom'); - $this->_unitOfWork->persist($entity); - $this->assertTrue($this->_unitOfWork->isInIdentityMap($entity)); - $this->_unitOfWork->clear(); $this->assertFalse($this->_unitOfWork->isInIdentityMap($entity)); From c4d41fe56a2beffb638e1d7e7d9ee6fb0a745f0a Mon Sep 17 00:00:00 2001 From: Marco Pivetta Date: Sun, 27 Nov 2016 18:08:03 +0100 Subject: [PATCH 05/20] #6017 moved `ORMException::invalidEntityName` to `ORMInvalidArgumentException::invalidEntityName` --- lib/Doctrine/ORM/ORMInvalidArgumentException.php | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/lib/Doctrine/ORM/ORMInvalidArgumentException.php b/lib/Doctrine/ORM/ORMInvalidArgumentException.php index 81466a0fe..accf1cc15 100644 --- a/lib/Doctrine/ORM/ORMInvalidArgumentException.php +++ b/lib/Doctrine/ORM/ORMInvalidArgumentException.php @@ -210,6 +210,18 @@ class ORMInvalidArgumentException extends \InvalidArgumentException )); } + /** + * Used when a given entityName hasn't the good type + * + * @param mixed $entityName The given entity (which shouldn't be a string) + * + * @return self + */ + public static function invalidEntityName($entityName) + { + return new self(sprintf('Entity name must be a string, %s given', gettype($entityName))); + } + /** * Helper method to show an object as string. * From 6ad9c9ea0475ffbbd585d43a93a1d60c324a16e0 Mon Sep 17 00:00:00 2001 From: Marco Pivetta Date: Sun, 27 Nov 2016 18:08:26 +0100 Subject: [PATCH 06/20] #6017 test coverage for `ORMInvalidArgumentException::invalidEntityName` --- .../ORM/ORMInvalidArgumentExceptionTest.php | 60 +++++++++++++++++++ 1 file changed, 60 insertions(+) create mode 100644 tests/Doctrine/Tests/ORM/ORMInvalidArgumentExceptionTest.php diff --git a/tests/Doctrine/Tests/ORM/ORMInvalidArgumentExceptionTest.php b/tests/Doctrine/Tests/ORM/ORMInvalidArgumentExceptionTest.php new file mode 100644 index 000000000..81f22e2b7 --- /dev/null +++ b/tests/Doctrine/Tests/ORM/ORMInvalidArgumentExceptionTest.php @@ -0,0 +1,60 @@ +getMessage()); + } + + /** + * @return string[][] + */ + public function invalidEntityNames() + { + return [ + [null, 'Entity name must be a string, NULL given'], + [true, 'Entity name must be a string, boolean given'], + [123, 'Entity name must be a string, integer given'], + [123.45, 'Entity name must be a string, double given'], + [new \stdClass(), 'Entity name must be a string, object given'], + ]; + } +} From 56598596a4343a0709358d7c88dc41ef6ca25d69 Mon Sep 17 00:00:00 2001 From: Marco Pivetta Date: Sun, 27 Nov 2016 18:09:51 +0100 Subject: [PATCH 07/20] #6017 adding `@group` annotation to newly introduced tests --- tests/Doctrine/Tests/ORM/UnitOfWorkTest.php | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/tests/Doctrine/Tests/ORM/UnitOfWorkTest.php b/tests/Doctrine/Tests/ORM/UnitOfWorkTest.php index 6e22ed498..523baf071 100644 --- a/tests/Doctrine/Tests/ORM/UnitOfWorkTest.php +++ b/tests/Doctrine/Tests/ORM/UnitOfWorkTest.php @@ -350,6 +350,9 @@ class UnitOfWorkTest extends OrmTestCase $this->assertFalse($this->_unitOfWork->isScheduledForInsert($entity2)); } + /** + * @group 6017 + */ public function testClearManagerWithObject() { $this->expectException(ORMException::class); @@ -360,6 +363,9 @@ class UnitOfWorkTest extends OrmTestCase $this->_unitOfWork->clear($entity); } + /** + * @group 6017 + */ public function testClearManagerWithNullValue() { $entity = new Country(456, 'United Kingdom'); From c97799f15174917119fd6604b5ee3b1584825e0e Mon Sep 17 00:00:00 2001 From: Marco Pivetta Date: Sun, 27 Nov 2016 18:10:57 +0100 Subject: [PATCH 08/20] #6017 expecting an `ORMInvalidArgumentException` when clearing with invalid data --- tests/Doctrine/Tests/ORM/UnitOfWorkTest.php | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/tests/Doctrine/Tests/ORM/UnitOfWorkTest.php b/tests/Doctrine/Tests/ORM/UnitOfWorkTest.php index 523baf071..61957aa1e 100644 --- a/tests/Doctrine/Tests/ORM/UnitOfWorkTest.php +++ b/tests/Doctrine/Tests/ORM/UnitOfWorkTest.php @@ -355,11 +355,10 @@ class UnitOfWorkTest extends OrmTestCase */ public function testClearManagerWithObject() { - $this->expectException(ORMException::class); - $this->expectExceptionMessage('must be a string'); - $entity = new Country(456, 'United Kingdom'); + $this->expectException(ORMInvalidArgumentException::class); + $this->_unitOfWork->clear($entity); } From 754e1f5d4221d712c1cd8cfed97bfe60a7582c03 Mon Sep 17 00:00:00 2001 From: Marco Pivetta Date: Sun, 27 Nov 2016 18:11:27 +0100 Subject: [PATCH 09/20] #6017 throwing an `ORMInvalidArgumentException` when clearing with non-string data. Also removing duplicate null checking --- lib/Doctrine/ORM/UnitOfWork.php | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/lib/Doctrine/ORM/UnitOfWork.php b/lib/Doctrine/ORM/UnitOfWork.php index 8fbac956d..e2f0616e8 100644 --- a/lib/Doctrine/ORM/UnitOfWork.php +++ b/lib/Doctrine/ORM/UnitOfWork.php @@ -2360,13 +2360,11 @@ class UnitOfWork implements PropertyChangedListener * @param string|null $entityName if given, only entities of this type will get detached. * * @return void + * + * @throws ORMInvalidArgumentException if an invalid entity name is given */ public function clear($entityName = null) { - if ($entityName !== null && !is_string($entityName)) { - throw ORMException::invalidEntityName($entityName); - } - if ($entityName === null) { $this->identityMap = $this->entityIdentifiers = @@ -2384,6 +2382,10 @@ class UnitOfWork implements PropertyChangedListener $this->visitedCollections = $this->orphanRemovals = array(); } else { + if (! is_string($entityName)) { + throw ORMInvalidArgumentException::invalidEntityName($entityName); + } + $this->clearIdentityMapForEntityName($entityName); $this->clearEntityInsertionsForEntityName($entityName); } From 8f77afdc3447724f91c655f7cbef9f60998103c6 Mon Sep 17 00:00:00 2001 From: Marco Pivetta Date: Sun, 27 Nov 2016 18:12:00 +0100 Subject: [PATCH 10/20] #6017 removed unused `ORMException::invalidEntityName` --- lib/Doctrine/ORM/ORMException.php | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/lib/Doctrine/ORM/ORMException.php b/lib/Doctrine/ORM/ORMException.php index edfc47df6..039ef2eaa 100644 --- a/lib/Doctrine/ORM/ORMException.php +++ b/lib/Doctrine/ORM/ORMException.php @@ -340,16 +340,4 @@ class ORMException extends Exception { return new self("Can't use IN operator on entities that have composite keys."); } - - /** - * Used when a given entityName hasn't the good type - * - * @param mixed $entityName The given entity (which shouldn't be a string) - * - * @return ORMException - */ - public static function invalidEntityName($entityName) - { - return new self(sprintf('Entity name must be a string, %s given', gettype($entityName))); - } } From c1038096e0fdda78e68144849ebbdf088581f9e4 Mon Sep 17 00:00:00 2001 From: Marco Pivetta Date: Sun, 27 Nov 2016 18:17:30 +0100 Subject: [PATCH 11/20] #6017 requesting `clear('nonExistingEntityName')` should raise a `MappingException` --- tests/Doctrine/Tests/ORM/UnitOfWorkTest.php | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/tests/Doctrine/Tests/ORM/UnitOfWorkTest.php b/tests/Doctrine/Tests/ORM/UnitOfWorkTest.php index 61957aa1e..a90b6d191 100644 --- a/tests/Doctrine/Tests/ORM/UnitOfWorkTest.php +++ b/tests/Doctrine/Tests/ORM/UnitOfWorkTest.php @@ -4,6 +4,7 @@ namespace Doctrine\Tests\ORM; use Doctrine\Common\Collections\ArrayCollection; use Doctrine\Common\NotifyPropertyChanged; +use Doctrine\Common\Persistence\Mapping\MappingException; use Doctrine\Common\PropertyChangedListener; use Doctrine\ORM\Mapping\ClassMetadata; use Doctrine\ORM\ORMInvalidArgumentException; @@ -362,6 +363,16 @@ class UnitOfWorkTest extends OrmTestCase $this->_unitOfWork->clear($entity); } + /** + * @group 6017 + */ + public function testClearManagerWithUnknownEntityName() + { + $this->expectException(MappingException::class); + + $this->_unitOfWork->clear(uniqid('nonExisting', true)); + } + /** * @group 6017 */ From dffd765b1e6365cb5879b43fe0cbb98789837f9b Mon Sep 17 00:00:00 2001 From: Marco Pivetta Date: Sun, 27 Nov 2016 18:17:47 +0100 Subject: [PATCH 12/20] #6017 requesting `clear('nonExistingEntityName')` now raises a `MappingException` --- lib/Doctrine/ORM/UnitOfWork.php | 2 ++ 1 file changed, 2 insertions(+) diff --git a/lib/Doctrine/ORM/UnitOfWork.php b/lib/Doctrine/ORM/UnitOfWork.php index e2f0616e8..07cf68568 100644 --- a/lib/Doctrine/ORM/UnitOfWork.php +++ b/lib/Doctrine/ORM/UnitOfWork.php @@ -2386,6 +2386,8 @@ class UnitOfWork implements PropertyChangedListener throw ORMInvalidArgumentException::invalidEntityName($entityName); } + $entityName = $this->em->getClassMetadata($entityName)->getName(); + $this->clearIdentityMapForEntityName($entityName); $this->clearEntityInsertionsForEntityName($entityName); } From fdb2af07e78d7793eae9c8b04e8ab1032ab107fb Mon Sep 17 00:00:00 2001 From: Marco Pivetta Date: Sun, 27 Nov 2016 18:21:35 +0100 Subject: [PATCH 13/20] #6017 hardened `clear()` logic, which now ensures that persisted entries are correctly cleared --- tests/Doctrine/Tests/ORM/UnitOfWorkTest.php | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/tests/Doctrine/Tests/ORM/UnitOfWorkTest.php b/tests/Doctrine/Tests/ORM/UnitOfWorkTest.php index a90b6d191..ef321e761 100644 --- a/tests/Doctrine/Tests/ORM/UnitOfWorkTest.php +++ b/tests/Doctrine/Tests/ORM/UnitOfWorkTest.php @@ -380,6 +380,11 @@ class UnitOfWorkTest extends OrmTestCase { $entity = new Country(456, 'United Kingdom'); + $this->_unitOfWork->persist($entity); + + $this->assertTrue($this->_unitOfWork->isInIdentityMap($entity)); + $this->assertTrue($this->_unitOfWork->isScheduledForInsert($entity)); + $this->_unitOfWork->clear(); $this->assertFalse($this->_unitOfWork->isInIdentityMap($entity)); From 9894dcb4b078f88945b6e5970fa2c5dc664d152d Mon Sep 17 00:00:00 2001 From: Marco Pivetta Date: Sun, 27 Nov 2016 18:22:01 +0100 Subject: [PATCH 14/20] #6017 `clear($proxyClassName)` should behave like `clear($realClassName)` --- tests/Doctrine/Tests/ORM/UnitOfWorkTest.php | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/tests/Doctrine/Tests/ORM/UnitOfWorkTest.php b/tests/Doctrine/Tests/ORM/UnitOfWorkTest.php index ef321e761..acf256a96 100644 --- a/tests/Doctrine/Tests/ORM/UnitOfWorkTest.php +++ b/tests/Doctrine/Tests/ORM/UnitOfWorkTest.php @@ -373,6 +373,26 @@ class UnitOfWorkTest extends OrmTestCase $this->_unitOfWork->clear(uniqid('nonExisting', true)); } + /** + * @group 6017 + */ + public function testClearManagerWithProxyClassName() + { + $proxy = $this->_emMock->getReference(Country::class, ['id' => random_int(457, 100000)]); + + $entity = new Country(456, 'United Kingdom'); + + $this->_unitOfWork->persist($entity); + + $this->assertTrue($this->_unitOfWork->isInIdentityMap($entity)); + $this->assertTrue($this->_unitOfWork->isScheduledForInsert($entity)); + + $this->_unitOfWork->clear(get_class($proxy)); + + $this->assertFalse($this->_unitOfWork->isInIdentityMap($entity)); + $this->assertFalse($this->_unitOfWork->isScheduledForInsert($entity)); + } + /** * @group 6017 */ From 49333867f8813af20f9780ceaeb21ec9adcc31a3 Mon Sep 17 00:00:00 2001 From: Marco Pivetta Date: Sun, 27 Nov 2016 18:25:12 +0100 Subject: [PATCH 15/20] FQCN reference correction --- tests/Doctrine/Tests/ORM/EntityManagerTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/Doctrine/Tests/ORM/EntityManagerTest.php b/tests/Doctrine/Tests/ORM/EntityManagerTest.php index d0cf2bcbb..e0bf30b6f 100644 --- a/tests/Doctrine/Tests/ORM/EntityManagerTest.php +++ b/tests/Doctrine/Tests/ORM/EntityManagerTest.php @@ -69,7 +69,7 @@ class EntityManagerTest extends OrmTestCase } /** - * @covers Doctrine\ORM\EntityManager::createNamedNativeQuery + * @covers \Doctrine\ORM\EntityManager::createNamedNativeQuery */ public function testCreateNamedNativeQuery() { From 92274124f980ad5e584387bed7e6f961835c1682 Mon Sep 17 00:00:00 2001 From: Marco Pivetta Date: Sun, 27 Nov 2016 18:28:37 +0100 Subject: [PATCH 16/20] #6017 moving tests around `clear()` into the `EntityManager` tests `UnitOfWork` assumptions are OK, since we don't want to clutter the API even more down there --- .../Doctrine/Tests/ORM/EntityManagerTest.php | 61 +++++++++++++++++++ 1 file changed, 61 insertions(+) diff --git a/tests/Doctrine/Tests/ORM/EntityManagerTest.php b/tests/Doctrine/Tests/ORM/EntityManagerTest.php index e0bf30b6f..c28ca150e 100644 --- a/tests/Doctrine/Tests/ORM/EntityManagerTest.php +++ b/tests/Doctrine/Tests/ORM/EntityManagerTest.php @@ -3,15 +3,20 @@ namespace Doctrine\Tests\ORM; use Doctrine\Common\Persistence\Mapping\Driver\MappingDriver; +use Doctrine\Common\Persistence\Mapping\MappingException; use Doctrine\ORM\Configuration; use Doctrine\ORM\EntityManager; use Doctrine\ORM\ORMException; use Doctrine\ORM\ORMInvalidArgumentException; use Doctrine\ORM\Query\ResultSetMapping; +use Doctrine\Tests\Models\GeoNames\Country; use Doctrine\Tests\OrmTestCase; class EntityManagerTest extends OrmTestCase { + /** + * @var EntityManager + */ private $_em; function setUp() @@ -217,4 +222,60 @@ class EntityManagerTest extends OrmTestCase $config->setMetadataDriverImpl($this->createMock(MappingDriver::class)); EntityManager::create(1, $config); } + + /** + * @group 6017 + */ + public function testClearManagerWithObject() + { + $entity = new Country(456, 'United Kingdom'); + + $this->expectException(ORMInvalidArgumentException::class); + + $this->_em->clear($entity); + } + + /** + * @group 6017 + */ + public function testClearManagerWithUnknownEntityName() + { + $this->expectException(MappingException::class); + + $this->_em->clear(uniqid('nonExisting', true)); + } + + /** + * @group 6017 + */ + public function testClearManagerWithProxyClassName() + { + $proxy = $this->_em->getReference(Country::class, ['id' => random_int(457, 100000)]); + + $entity = new Country(456, 'United Kingdom'); + + $this->_em->persist($entity); + + $this->assertTrue($this->_em->contains($entity)); + + $this->_em->clear(get_class($proxy)); + + $this->assertFalse($this->_em->contains($entity)); + } + + /** + * @group 6017 + */ + public function testClearManagerWithNullValue() + { + $entity = new Country(456, 'United Kingdom'); + + $this->_em->persist($entity); + + $this->assertTrue($this->_em->contains($entity)); + + $this->_em->clear(null); + + $this->assertFalse($this->_em->contains($entity)); + } } From 1d7397caf09574d7cd58652931a4f5cb64923271 Mon Sep 17 00:00:00 2001 From: Marco Pivetta Date: Sun, 27 Nov 2016 18:33:53 +0100 Subject: [PATCH 17/20] #6017 moving entity name validity checks into the `EntityManager` API, documenting newly thrown exception types --- lib/Doctrine/ORM/EntityManager.php | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/lib/Doctrine/ORM/EntityManager.php b/lib/Doctrine/ORM/EntityManager.php index 51d6dbf05..b61a2557b 100644 --- a/lib/Doctrine/ORM/EntityManager.php +++ b/lib/Doctrine/ORM/EntityManager.php @@ -19,6 +19,7 @@ namespace Doctrine\ORM; +use Doctrine\ORM\Mapping\MappingException; use Exception; use Doctrine\Common\EventManager; use Doctrine\DBAL\Connection; @@ -539,10 +540,22 @@ use Doctrine\Common\Util\ClassUtils; * @param string|null $entityName if given, only entities of this type will get detached * * @return void + * + * @throws ORMInvalidArgumentException if a non-null non-string value is given + * @throws \Doctrine\Common\Persistence\Mapping\MappingException if a $entityName is given, but that entity is not + * found in the mappings */ public function clear($entityName = null) { - $this->unitOfWork->clear($entityName); + if (null !== $entityName && ! is_string($entityName)) { + throw ORMInvalidArgumentException::invalidEntityName($entityName); + } + + $this->unitOfWork->clear( + null === $entityName + ? null + : $this->metadataFactory->getMetadataFor($entityName)->getName() + ); } /** From 53c5824a6bcc1bb5af63eddeae56ea74f8ee1b22 Mon Sep 17 00:00:00 2001 From: Marco Pivetta Date: Sun, 27 Nov 2016 18:35:05 +0100 Subject: [PATCH 18/20] #6017 removed `clear($entityName)` parameter validity checks The `UnitOfWork` is advanced stuff: don't touch if you don't know what you are doing. --- lib/Doctrine/ORM/UnitOfWork.php | 6 ------ 1 file changed, 6 deletions(-) diff --git a/lib/Doctrine/ORM/UnitOfWork.php b/lib/Doctrine/ORM/UnitOfWork.php index 07cf68568..368fdfd63 100644 --- a/lib/Doctrine/ORM/UnitOfWork.php +++ b/lib/Doctrine/ORM/UnitOfWork.php @@ -2382,12 +2382,6 @@ class UnitOfWork implements PropertyChangedListener $this->visitedCollections = $this->orphanRemovals = array(); } else { - if (! is_string($entityName)) { - throw ORMInvalidArgumentException::invalidEntityName($entityName); - } - - $entityName = $this->em->getClassMetadata($entityName)->getName(); - $this->clearIdentityMapForEntityName($entityName); $this->clearEntityInsertionsForEntityName($entityName); } From 44a61412351b279e814f63a28a09413d01c63965 Mon Sep 17 00:00:00 2001 From: Marco Pivetta Date: Sun, 27 Nov 2016 18:35:43 +0100 Subject: [PATCH 19/20] #6017 removed `clear($entityName)` tests from `UnitOfWorkTest`: now covered in `EntityManagerTest` --- tests/Doctrine/Tests/ORM/UnitOfWorkTest.php | 60 --------------------- 1 file changed, 60 deletions(-) diff --git a/tests/Doctrine/Tests/ORM/UnitOfWorkTest.php b/tests/Doctrine/Tests/ORM/UnitOfWorkTest.php index acf256a96..5a32d2882 100644 --- a/tests/Doctrine/Tests/ORM/UnitOfWorkTest.php +++ b/tests/Doctrine/Tests/ORM/UnitOfWorkTest.php @@ -351,66 +351,6 @@ class UnitOfWorkTest extends OrmTestCase $this->assertFalse($this->_unitOfWork->isScheduledForInsert($entity2)); } - /** - * @group 6017 - */ - public function testClearManagerWithObject() - { - $entity = new Country(456, 'United Kingdom'); - - $this->expectException(ORMInvalidArgumentException::class); - - $this->_unitOfWork->clear($entity); - } - - /** - * @group 6017 - */ - public function testClearManagerWithUnknownEntityName() - { - $this->expectException(MappingException::class); - - $this->_unitOfWork->clear(uniqid('nonExisting', true)); - } - - /** - * @group 6017 - */ - public function testClearManagerWithProxyClassName() - { - $proxy = $this->_emMock->getReference(Country::class, ['id' => random_int(457, 100000)]); - - $entity = new Country(456, 'United Kingdom'); - - $this->_unitOfWork->persist($entity); - - $this->assertTrue($this->_unitOfWork->isInIdentityMap($entity)); - $this->assertTrue($this->_unitOfWork->isScheduledForInsert($entity)); - - $this->_unitOfWork->clear(get_class($proxy)); - - $this->assertFalse($this->_unitOfWork->isInIdentityMap($entity)); - $this->assertFalse($this->_unitOfWork->isScheduledForInsert($entity)); - } - - /** - * @group 6017 - */ - public function testClearManagerWithNullValue() - { - $entity = new Country(456, 'United Kingdom'); - - $this->_unitOfWork->persist($entity); - - $this->assertTrue($this->_unitOfWork->isInIdentityMap($entity)); - $this->assertTrue($this->_unitOfWork->isScheduledForInsert($entity)); - - $this->_unitOfWork->clear(); - - $this->assertFalse($this->_unitOfWork->isInIdentityMap($entity)); - $this->assertFalse($this->_unitOfWork->isScheduledForInsert($entity)); - } - /** * Data Provider * From dc3b1668114893de1112745e8e3e5df1a1b6acb8 Mon Sep 17 00:00:00 2001 From: Marco Pivetta Date: Sun, 27 Nov 2016 18:38:57 +0100 Subject: [PATCH 20/20] #6017 replaced `random_int()` with `rand()`, since we still support oldstable PHP (5.6.x) --- tests/Doctrine/Tests/ORM/EntityManagerTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/Doctrine/Tests/ORM/EntityManagerTest.php b/tests/Doctrine/Tests/ORM/EntityManagerTest.php index c28ca150e..954a3f2f0 100644 --- a/tests/Doctrine/Tests/ORM/EntityManagerTest.php +++ b/tests/Doctrine/Tests/ORM/EntityManagerTest.php @@ -250,7 +250,7 @@ class EntityManagerTest extends OrmTestCase */ public function testClearManagerWithProxyClassName() { - $proxy = $this->_em->getReference(Country::class, ['id' => random_int(457, 100000)]); + $proxy = $this->_em->getReference(Country::class, ['id' => rand(457, 100000)]); $entity = new Country(456, 'United Kingdom');