From 21b70577d37c3710d670367b6f6a1dd3f1a5a1e2 Mon Sep 17 00:00:00 2001 From: Marco Pivetta Date: Thu, 2 Apr 2015 23:43:16 +0100 Subject: [PATCH 1/9] Hydration of fetch-joined results fails when an entity has a default value of `array` for the collection property --- .../EntityWithArrayDefaultArrayValueM2M.php | 15 +++++++++++ .../Tests/Models/Hydration/SimpleEntity.php | 12 +++++++++ .../ORM/Hydration/ObjectHydratorTest.php | 27 +++++++++++++++++++ 3 files changed, 54 insertions(+) create mode 100644 tests/Doctrine/Tests/Models/Hydration/EntityWithArrayDefaultArrayValueM2M.php create mode 100644 tests/Doctrine/Tests/Models/Hydration/SimpleEntity.php diff --git a/tests/Doctrine/Tests/Models/Hydration/EntityWithArrayDefaultArrayValueM2M.php b/tests/Doctrine/Tests/Models/Hydration/EntityWithArrayDefaultArrayValueM2M.php new file mode 100644 index 000000000..8ba57db68 --- /dev/null +++ b/tests/Doctrine/Tests/Models/Hydration/EntityWithArrayDefaultArrayValueM2M.php @@ -0,0 +1,15 @@ +_em); $hydrator->hydrateAll($stmt, $rsm); } + + public function testFetchJoinCollectionValuedAssociationWithDefaultArrayValue() + { + $rsm = new ResultSetMapping; + + $rsm->addEntityResult(EntityWithArrayDefaultArrayValueM2M::CLASSNAME, 'e1', null); + $rsm->addJoinedEntityResult(SimpleEntity::CLASSNAME, 'e2', 'e1', 'collection'); + $rsm->addFieldResult('e1', 'a1__id', 'id'); + $rsm->addFieldResult('e2', 'e2__id', 'id'); + + $result = (new ObjectHydrator($this->_em)) + ->hydrateAll( + new HydratorMockStatement([[ + 'a1__id' => '1', + 'e2__id' => '1', + ]]), + $rsm + ); + + $this->assertCount(1, $result); + $this->assertInstanceOf(EntityWithArrayDefaultArrayValueM2M::CLASSNAME, $result[0]); + $this->assertInstanceOf('Doctrine\ORM\PersistentCollection', $result[0]->collection); + $this->assertCount(1, $result[0]->collection); + $this->assertInstanceOf(SimpleEntity::CLASSNAME, $result[0]->collection[0]); + } } From 3bc3aeeb529d33b120e71e2dbea3fadafdff9d65 Mon Sep 17 00:00:00 2001 From: Marco Pivetta Date: Thu, 2 Apr 2015 23:43:41 +0100 Subject: [PATCH 2/9] Minor docblock correction (discovered during testing) --- lib/Doctrine/ORM/Query/ResultSetMapping.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/Doctrine/ORM/Query/ResultSetMapping.php b/lib/Doctrine/ORM/Query/ResultSetMapping.php index 07b896695..bdd5de75e 100644 --- a/lib/Doctrine/ORM/Query/ResultSetMapping.php +++ b/lib/Doctrine/ORM/Query/ResultSetMapping.php @@ -347,7 +347,7 @@ class ResultSetMapping * @param string $class The class name of the joined entity. * @param string $alias The unique alias to use for the joined entity. * @param string $parentAlias The alias of the entity result that is the parent of this joined result. - * @param object $relation The association field that connects the parent entity result + * @param string $relation The association field that connects the parent entity result * with the joined entity result. * * @return ResultSetMapping This ResultSetMapping instance. From 670972d5c31d6caec266425ba194fefbf8def524 Mon Sep 17 00:00:00 2001 From: Marco Pivetta Date: Thu, 2 Apr 2015 23:44:12 +0100 Subject: [PATCH 3/9] `PersistentCollection` should still accept `null` and `array` as constructor argument, as it did before --- .../Tests/ORM/PersistentCollectionTest.php | 46 +++++++++++++++++-- 1 file changed, 42 insertions(+), 4 deletions(-) diff --git a/tests/Doctrine/Tests/ORM/PersistentCollectionTest.php b/tests/Doctrine/Tests/ORM/PersistentCollectionTest.php index 9b9a067fc..6c139ba30 100644 --- a/tests/Doctrine/Tests/ORM/PersistentCollectionTest.php +++ b/tests/Doctrine/Tests/ORM/PersistentCollectionTest.php @@ -3,8 +3,10 @@ namespace Doctrine\Tests\ORM; use Doctrine\Common\Collections\ArrayCollection; +use Doctrine\Common\Collections\Collection; use Doctrine\ORM\PersistentCollection; use Doctrine\Tests\Mocks\ConnectionMock; +use Doctrine\Tests\Mocks\DriverMock; use Doctrine\Tests\Mocks\EntityManagerMock; use Doctrine\Tests\Models\ECommerce\ECommerceCart; use Doctrine\Tests\OrmTestCase; @@ -21,15 +23,16 @@ class PersistentCollectionTest extends OrmTestCase */ protected $collection; - private $_connectionMock; + /** + * @var \Doctrine\ORM\EntityManagerInterface + */ private $_emMock; protected function setUp() { parent::setUp(); - // SUT - $this->_connectionMock = new ConnectionMock(array(), new \Doctrine\Tests\Mocks\DriverMock()); - $this->_emMock = EntityManagerMock::create($this->_connectionMock); + + $this->_emMock = EntityManagerMock::create(new ConnectionMock([], new DriverMock())); } /** @@ -80,4 +83,39 @@ class PersistentCollectionTest extends OrmTestCase $this->collection->next(); $this->assertTrue($this->collection->isInitialized()); } + + public function testAcceptsArrayAsConstructorArgument() + { + $metadata = $this->_emMock->getClassMetadata('Doctrine\Tests\Models\ECommerce\ECommerceCart'); + + $collection = new PersistentCollection($this->_emMock, $metadata, []); + + $this->assertEmpty($collection); + $this->tryGenericCollectionOperations($collection); + } + + public function testAcceptsNullAsConstructorArgument() + { + $metadata = $this->_emMock->getClassMetadata('Doctrine\Tests\Models\ECommerce\ECommerceCart'); + + $collection = new PersistentCollection($this->_emMock, $metadata, null); + + $this->assertEmpty($collection); + $this->tryGenericCollectionOperations($collection); + } + + private function tryGenericCollectionOperations(Collection $collection) + { + $count = count($collection); + $object = new \stdClass(); + + $collection->add($object); + + $this->assertTrue($collection->contains($object)); + $this->assertCount($count + 1, $collection); + + $collection->removeElement($object); + + $this->assertCount($count, $collection); + } } From 6b5188fee8a2a45d9d9e52bd33f0d542921349ce Mon Sep 17 00:00:00 2001 From: Marco Pivetta Date: Thu, 2 Apr 2015 23:45:12 +0100 Subject: [PATCH 4/9] FQCN reference (class was not imported correctly) --- tests/Doctrine/Tests/ORM/Hydration/ObjectHydratorTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/Doctrine/Tests/ORM/Hydration/ObjectHydratorTest.php b/tests/Doctrine/Tests/ORM/Hydration/ObjectHydratorTest.php index 3dc5025ae..eaa0b75b7 100644 --- a/tests/Doctrine/Tests/ORM/Hydration/ObjectHydratorTest.php +++ b/tests/Doctrine/Tests/ORM/Hydration/ObjectHydratorTest.php @@ -1968,7 +1968,7 @@ class ObjectHydratorTest extends HydrationTestCase $rsm->addFieldResult('e1', 'a1__id', 'id'); $rsm->addFieldResult('e2', 'e2__id', 'id'); - $result = (new ObjectHydrator($this->_em)) + $result = (new \Doctrine\ORM\Internal\Hydration\ObjectHydrator($this->_em)) ->hydrateAll( new HydratorMockStatement([[ 'a1__id' => '1', From 1993aecd4c29c3355c01724d9039854d48f87fc0 Mon Sep 17 00:00:00 2001 From: Marco Pivetta Date: Thu, 2 Apr 2015 23:45:46 +0100 Subject: [PATCH 5/9] Reverting BC break: `PersistentConnection#__construct()` now accepts `null|array|Collection` again --- lib/Doctrine/ORM/PersistentCollection.php | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/lib/Doctrine/ORM/PersistentCollection.php b/lib/Doctrine/ORM/PersistentCollection.php index 98e3b91d8..7564133b9 100644 --- a/lib/Doctrine/ORM/PersistentCollection.php +++ b/lib/Doctrine/ORM/PersistentCollection.php @@ -101,16 +101,18 @@ final class PersistentCollection extends AbstractLazyCollection implements Selec /** * Creates a new persistent collection. * - * @param EntityManagerInterface $em The EntityManager the collection will be associated with. - * @param ClassMetadata $class The class descriptor of the entity type of this collection. - * @param Collection $coll The collection elements. + * @param EntityManagerInterface $em The EntityManager the collection will be associated with. + * @param ClassMetadata $class The class descriptor of the entity type of this collection. + * @param Collection|array|null $collection The collection elements. */ - public function __construct(EntityManagerInterface $em, $class, $coll) + public function __construct(EntityManagerInterface $em, $class, $collection) { - $this->collection = $coll; $this->em = $em; $this->typeClass = $class; $this->initialized = true; + $this->collection = $collection instanceof Collection + ? $collection + : new ArrayCollection((array) $collection); } /** From d49c90793433f4f3b5d9b9277e40023925b2ed1d Mon Sep 17 00:00:00 2001 From: Marco Pivetta Date: Fri, 3 Apr 2015 15:26:38 +0100 Subject: [PATCH 6/9] a `PersistentCollection` should only allow another collection as a wrapped collection --- lib/Doctrine/ORM/PersistentCollection.php | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/lib/Doctrine/ORM/PersistentCollection.php b/lib/Doctrine/ORM/PersistentCollection.php index 7564133b9..83eab8cf1 100644 --- a/lib/Doctrine/ORM/PersistentCollection.php +++ b/lib/Doctrine/ORM/PersistentCollection.php @@ -103,16 +103,14 @@ final class PersistentCollection extends AbstractLazyCollection implements Selec * * @param EntityManagerInterface $em The EntityManager the collection will be associated with. * @param ClassMetadata $class The class descriptor of the entity type of this collection. - * @param Collection|array|null $collection The collection elements. + * @param Collection $collection The collection elements. */ - public function __construct(EntityManagerInterface $em, $class, $collection) + public function __construct(EntityManagerInterface $em, $class, Collection $collection) { + $this->collection = $collection; $this->em = $em; $this->typeClass = $class; $this->initialized = true; - $this->collection = $collection instanceof Collection - ? $collection - : new ArrayCollection((array) $collection); } /** From 95b128ce8fce01fdb643ef0f002990fff1f7eb6f Mon Sep 17 00:00:00 2001 From: Marco Pivetta Date: Fri, 3 Apr 2015 15:27:13 +0100 Subject: [PATCH 7/9] Removing irrelevant tests (as per discussion with @guilhermeblanco and @stof --- .../Tests/ORM/PersistentCollectionTest.php | 35 ------------------- 1 file changed, 35 deletions(-) diff --git a/tests/Doctrine/Tests/ORM/PersistentCollectionTest.php b/tests/Doctrine/Tests/ORM/PersistentCollectionTest.php index 6c139ba30..e77f898d1 100644 --- a/tests/Doctrine/Tests/ORM/PersistentCollectionTest.php +++ b/tests/Doctrine/Tests/ORM/PersistentCollectionTest.php @@ -83,39 +83,4 @@ class PersistentCollectionTest extends OrmTestCase $this->collection->next(); $this->assertTrue($this->collection->isInitialized()); } - - public function testAcceptsArrayAsConstructorArgument() - { - $metadata = $this->_emMock->getClassMetadata('Doctrine\Tests\Models\ECommerce\ECommerceCart'); - - $collection = new PersistentCollection($this->_emMock, $metadata, []); - - $this->assertEmpty($collection); - $this->tryGenericCollectionOperations($collection); - } - - public function testAcceptsNullAsConstructorArgument() - { - $metadata = $this->_emMock->getClassMetadata('Doctrine\Tests\Models\ECommerce\ECommerceCart'); - - $collection = new PersistentCollection($this->_emMock, $metadata, null); - - $this->assertEmpty($collection); - $this->tryGenericCollectionOperations($collection); - } - - private function tryGenericCollectionOperations(Collection $collection) - { - $count = count($collection); - $object = new \stdClass(); - - $collection->add($object); - - $this->assertTrue($collection->contains($object)); - $this->assertCount($count + 1, $collection); - - $collection->removeElement($object); - - $this->assertCount($count, $collection); - } } From f0d2e8d150001a97c128c87e1f3bde8d70d02ee0 Mon Sep 17 00:00:00 2001 From: Marco Pivetta Date: Fri, 3 Apr 2015 15:28:13 +0100 Subject: [PATCH 8/9] Correcting static introspection issue in cache specific tests (`null` was being passed to a `PersistentCollection`) --- .../Cache/Persister/Entity/AbstractEntityPersisterTest.php | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/tests/Doctrine/Tests/ORM/Cache/Persister/Entity/AbstractEntityPersisterTest.php b/tests/Doctrine/Tests/ORM/Cache/Persister/Entity/AbstractEntityPersisterTest.php index f682723c1..bf94b1e46 100644 --- a/tests/Doctrine/Tests/ORM/Cache/Persister/Entity/AbstractEntityPersisterTest.php +++ b/tests/Doctrine/Tests/ORM/Cache/Persister/Entity/AbstractEntityPersisterTest.php @@ -11,6 +11,7 @@ use Doctrine\ORM\Persisters\Entity\EntityPersister; use Doctrine\Tests\Models\Cache\Country; use Doctrine\Common\Collections\Criteria; +use Doctrine\Common\Collections\ArrayCollection; use Doctrine\ORM\Query\ResultSetMappingBuilder; use Doctrine\ORM\PersistentCollection; @@ -390,7 +391,7 @@ abstract class AbstractEntityPersisterTest extends OrmTestCase { $mapping = $this->em->getClassMetadata('Doctrine\Tests\Models\Cache\Country'); $assoc = array('type' => 1); - $coll = new PersistentCollection($this->em, $mapping, null); + $coll = new PersistentCollection($this->em, $mapping, new ArrayCollection()); $persister = $this->createPersisterDefault(); $entity = new Country("Foo"); @@ -406,7 +407,7 @@ abstract class AbstractEntityPersisterTest extends OrmTestCase { $mapping = $this->em->getClassMetadata('Doctrine\Tests\Models\Cache\Country'); $assoc = array('type' => 1); - $coll = new PersistentCollection($this->em, $mapping, null); + $coll = new PersistentCollection($this->em, $mapping, new ArrayCollection()); $persister = $this->createPersisterDefault(); $entity = new Country("Foo"); From 2a81adc1fc79eae8f9a3b7cac61bec90ceaddbb0 Mon Sep 17 00:00:00 2001 From: Marco Pivetta Date: Fri, 3 Apr 2015 15:28:53 +0100 Subject: [PATCH 9/9] Correcting `ObjectHydrator` logic: if an `array` is a default value for a collection-valued property, it should be cast to a `Collection` --- lib/Doctrine/ORM/Internal/Hydration/ObjectHydrator.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/Doctrine/ORM/Internal/Hydration/ObjectHydrator.php b/lib/Doctrine/ORM/Internal/Hydration/ObjectHydrator.php index c1bcc2591..3cedaada0 100644 --- a/lib/Doctrine/ORM/Internal/Hydration/ObjectHydrator.php +++ b/lib/Doctrine/ORM/Internal/Hydration/ObjectHydrator.php @@ -189,8 +189,8 @@ class ObjectHydrator extends AbstractHydrator $relation = $class->associationMappings[$fieldName]; $value = $class->reflFields[$fieldName]->getValue($entity); - if ($value === null) { - $value = new ArrayCollection; + if ($value === null || is_array($value)) { + $value = new ArrayCollection((array) $value); } if ( ! $value instanceof PersistentCollection) {