From 0252d55c678dff09050de0e7a4f44e4aeb5cfd97 Mon Sep 17 00:00:00 2001 From: Benjamin Eberlei Date: Sun, 16 Oct 2011 11:08:11 +0200 Subject: [PATCH] DDC-1358 - Fix bug where multiple NULL root entity combined with scalar results will break the object and array hydrator. This case likeli only occurs when doing native queries. A guard clause that prevents hydration from breaking when RIGHT JOIN queries with null root entities appear has been added aswell. --- .../Internal/Hydration/AbstractHydrator.php | 5 + .../ORM/Internal/Hydration/ArrayHydrator.php | 16 ++ .../ORM/Internal/Hydration/ObjectHydrator.php | 18 ++ .../Tests/ORM/Hydration/ArrayHydratorTest.php | 54 ++++++ .../ORM/Hydration/ObjectHydratorTest.php | 161 ++++++++++++++++++ 5 files changed, 254 insertions(+) diff --git a/lib/Doctrine/ORM/Internal/Hydration/AbstractHydrator.php b/lib/Doctrine/ORM/Internal/Hydration/AbstractHydrator.php index 181854e36..5899a69ca 100644 --- a/lib/Doctrine/ORM/Internal/Hydration/AbstractHydrator.php +++ b/lib/Doctrine/ORM/Internal/Hydration/AbstractHydrator.php @@ -164,6 +164,11 @@ abstract class AbstractHydrator * field names during this procedure as well as any necessary conversions on * the values applied. * + * @param array $data SQL Result Row + * @param array &$cache Cache for column to field result information + * @param array &$id Dql-Alias => ID-Hash + * @param array &$nonemptyComponents Does this DQL-Alias has at least one non NULL value? + * * @return array An array with all the fields (name => value) of the data row, * grouped by their component alias. */ diff --git a/lib/Doctrine/ORM/Internal/Hydration/ArrayHydrator.php b/lib/Doctrine/ORM/Internal/Hydration/ArrayHydrator.php index 92eb45c5c..4b1c21c6f 100644 --- a/lib/Doctrine/ORM/Internal/Hydration/ArrayHydrator.php +++ b/lib/Doctrine/ORM/Internal/Hydration/ArrayHydrator.php @@ -92,6 +92,11 @@ class ArrayHydrator extends AbstractHydrator $parent = $this->_rsm->parentAliasMap[$dqlAlias]; $path = $parent . '.' . $dqlAlias; + // missing parent data, skipping as RIGHT JOIN hydration is not supported. + if ( ! isset($nonemptyComponents[$parent]) ) { + continue; + } + // Get a reference to the right element in the result tree. // This element will get the associated element attached. if ($this->_rsm->isMixed && isset($this->_rootAliases[$parent])) { @@ -154,6 +159,17 @@ class ArrayHydrator extends AbstractHydrator // It's a root result element $this->_rootAliases[$dqlAlias] = true; // Mark as root + + // if this row has a NULL value for the root result id then make it a null result. + if ( ! isset($nonemptyComponents[$dqlAlias]) ) { + if ($this->_rsm->isMixed) { + $result[] = array(0 => null); + } else { + $result[] = null; + } + ++$this->_resultCounter; + continue; + } // Check for an existing element if ($this->_isSimpleQuery || ! isset($this->_identifierMap[$dqlAlias][$id[$dqlAlias]])) { diff --git a/lib/Doctrine/ORM/Internal/Hydration/ObjectHydrator.php b/lib/Doctrine/ORM/Internal/Hydration/ObjectHydrator.php index bb11a7431..1287a138b 100644 --- a/lib/Doctrine/ORM/Internal/Hydration/ObjectHydrator.php +++ b/lib/Doctrine/ORM/Internal/Hydration/ObjectHydrator.php @@ -302,6 +302,12 @@ class ObjectHydrator extends AbstractHydrator // seen for this parent-child relationship $path = $parentAlias . '.' . $dqlAlias; + // We have a RIGHT JOIN result here. Doctrine cannot hydrate RIGHT JOIN Object-Graphs + if (!isset($nonemptyComponents[$parentAlias])) { + // TODO: Add special case code where we hydrate the right join objects into identity map at least + continue; + } + // Get a reference to the parent object to which the joined element belongs. if ($this->_rsm->isMixed && isset($this->_rootAliases[$parentAlias])) { $first = reset($this->_resultPointers); @@ -408,6 +414,18 @@ class ObjectHydrator extends AbstractHydrator // PATH C: Its a root result element $this->_rootAliases[$dqlAlias] = true; // Mark as root alias + // if this row has a NULL value for the root result id then make it a null result. + if ( ! isset($nonemptyComponents[$dqlAlias]) ) { + if ($this->_rsm->isMixed) { + $result[] = array(0 => null); + } else { + $result[] = null; + } + ++$this->_resultCounter; + continue; + } + + // check for existing result from the iterations before if ( ! isset($this->_identifierMap[$dqlAlias][$id[$dqlAlias]])) { $element = $this->_getEntity($rowData[$dqlAlias], $dqlAlias); if (isset($this->_rsm->indexByMap[$dqlAlias])) { diff --git a/tests/Doctrine/Tests/ORM/Hydration/ArrayHydratorTest.php b/tests/Doctrine/Tests/ORM/Hydration/ArrayHydratorTest.php index 4fb5b92ac..a318f78af 100644 --- a/tests/Doctrine/Tests/ORM/Hydration/ArrayHydratorTest.php +++ b/tests/Doctrine/Tests/ORM/Hydration/ArrayHydratorTest.php @@ -763,4 +763,58 @@ class ArrayHydratorTest extends HydrationTestCase $this->assertEquals(1, count($result)); } + + /** + * @group DDC-1358 + */ + public function testMissingIdForRootEntity() + { + $rsm = new ResultSetMapping; + $rsm->addEntityResult('Doctrine\Tests\Models\CMS\CmsUser', 'u'); + $rsm->addFieldResult('u', 'u__id', 'id'); + $rsm->addFieldResult('u', 'u__status', 'status'); + $rsm->addScalarResult('sclr0', 'nameUpper'); + + // Faked result set + $resultSet = array( + //row1 + array( + 'u__id' => '1', + 'u__status' => 'developer', + 'sclr0' => 'ROMANB', + ), + array( + 'u__id' => null, + 'u__status' => null, + 'sclr0' => 'ROMANB', + ), + array( + 'u__id' => '2', + 'u__status' => 'developer', + 'sclr0' => 'JWAGE', + ), + array( + 'u__id' => null, + 'u__status' => null, + 'sclr0' => 'JWAGE', + ), + ); + + $stmt = new HydratorMockStatement($resultSet); + $hydrator = new \Doctrine\ORM\Internal\Hydration\ArrayHydrator($this->_em); + + $result = $hydrator->hydrateAll($stmt, $rsm); + + $this->assertEquals(4, count($result), "Should hydrate four results."); + + $this->assertEquals('ROMANB', $result[0]['nameUpper']); + $this->assertEquals('ROMANB', $result[1]['nameUpper']); + $this->assertEquals('JWAGE', $result[2]['nameUpper']); + $this->assertEquals('JWAGE', $result[3]['nameUpper']); + + $this->assertEquals(array('id' => 1, 'status' => 'developer'), $result[0][0]); + $this->assertNull($result[1][0]); + $this->assertEquals(array('id' => 2, 'status' => 'developer'), $result[2][0]); + $this->assertNull($result[3][0]); + } } \ No newline at end of file diff --git a/tests/Doctrine/Tests/ORM/Hydration/ObjectHydratorTest.php b/tests/Doctrine/Tests/ORM/Hydration/ObjectHydratorTest.php index f2673ac70..581a3504a 100644 --- a/tests/Doctrine/Tests/ORM/Hydration/ObjectHydratorTest.php +++ b/tests/Doctrine/Tests/ORM/Hydration/ObjectHydratorTest.php @@ -1008,4 +1008,165 @@ class ObjectHydratorTest extends HydrationTestCase $this->assertEquals(4, count($result[1]->groups)); $this->assertEquals(2, count($result[1]->phonenumbers)); } + + /** + * @group DDC-1358 + */ + public function testMissingIdForRootEntity() + { + $rsm = new ResultSetMapping; + $rsm->addEntityResult('Doctrine\Tests\Models\CMS\CmsUser', 'u'); + $rsm->addFieldResult('u', 'u__id', 'id'); + $rsm->addFieldResult('u', 'u__status', 'status'); + $rsm->addScalarResult('sclr0', 'nameUpper'); + + // Faked result set + $resultSet = array( + //row1 + array( + 'u__id' => '1', + 'u__status' => 'developer', + 'sclr0' => 'ROMANB', + ), + array( + 'u__id' => null, + 'u__status' => null, + 'sclr0' => 'ROMANB', + ), + array( + 'u__id' => '2', + 'u__status' => 'developer', + 'sclr0' => 'JWAGE', + ), + array( + 'u__id' => null, + 'u__status' => null, + 'sclr0' => 'JWAGE', + ), + ); + + $stmt = new HydratorMockStatement($resultSet); + $hydrator = new \Doctrine\ORM\Internal\Hydration\ObjectHydrator($this->_em); + + $result = $hydrator->hydrateAll($stmt, $rsm, array(Query::HINT_FORCE_PARTIAL_LOAD => true)); + + $this->assertEquals(4, count($result), "Should hydrate four results."); + + $this->assertEquals('ROMANB', $result[0]['nameUpper']); + $this->assertEquals('ROMANB', $result[1]['nameUpper']); + $this->assertEquals('JWAGE', $result[2]['nameUpper']); + $this->assertEquals('JWAGE', $result[3]['nameUpper']); + + $this->assertInstanceOf('Doctrine\Tests\Models\CMS\CmsUser', $result[0][0]); + $this->assertNull($result[1][0]); + $this->assertInstanceOf('Doctrine\Tests\Models\CMS\CmsUser', $result[2][0]); + $this->assertNull($result[3][0]); + } + + /** + * @group DDC-1358 + * @return void + */ + public function testMissingIdForCollectionValuedChildEntity() + { + $rsm = new ResultSetMapping; + $rsm->addEntityResult('Doctrine\Tests\Models\CMS\CmsUser', 'u'); + $rsm->addJoinedEntityResult( + 'Doctrine\Tests\Models\CMS\CmsPhonenumber', + 'p', + 'u', + 'phonenumbers' + ); + $rsm->addFieldResult('u', 'u__id', 'id'); + $rsm->addFieldResult('u', 'u__status', 'status'); + $rsm->addScalarResult('sclr0', 'nameUpper'); + $rsm->addFieldResult('p', 'p__phonenumber', 'phonenumber'); + + // Faked result set + $resultSet = array( + //row1 + array( + 'u__id' => '1', + 'u__status' => 'developer', + 'sclr0' => 'ROMANB', + 'p__phonenumber' => '42', + ), + array( + 'u__id' => '1', + 'u__status' => 'developer', + 'sclr0' => 'ROMANB', + 'p__phonenumber' => null + ), + array( + 'u__id' => '2', + 'u__status' => 'developer', + 'sclr0' => 'JWAGE', + 'p__phonenumber' => '91' + ), + array( + 'u__id' => '2', + 'u__status' => 'developer', + 'sclr0' => 'JWAGE', + 'p__phonenumber' => null + ) + ); + + $stmt = new HydratorMockStatement($resultSet); + $hydrator = new \Doctrine\ORM\Internal\Hydration\ObjectHydrator($this->_em); + + $result = $hydrator->hydrateAll($stmt, $rsm, array(Query::HINT_FORCE_PARTIAL_LOAD => true)); + + $this->assertEquals(2, count($result)); + $this->assertEquals(1, $result[0][0]->phonenumbers->count()); + $this->assertEquals(1, $result[1][0]->phonenumbers->count()); + } + + /** + * @group DDC-1358 + */ + public function testMissingIdForSingleValuedChildEntity() + { + $rsm = new ResultSetMapping; + $rsm->addEntityResult('Doctrine\Tests\Models\CMS\CmsUser', 'u'); + $rsm->addJoinedEntityResult( + 'Doctrine\Tests\Models\CMS\CmsAddress', + 'a', + 'u', + 'address' + ); + $rsm->addFieldResult('u', 'u__id', 'id'); + $rsm->addFieldResult('u', 'u__status', 'status'); + $rsm->addScalarResult('sclr0', 'nameUpper'); + $rsm->addFieldResult('a', 'a__id', 'id'); + $rsm->addFieldResult('a', 'a__city', 'city'); + $rsm->addMetaResult('a', 'user_id', 'user_id'); + + // Faked result set + $resultSet = array( + //row1 + array( + 'u__id' => '1', + 'u__status' => 'developer', + 'sclr0' => 'ROMANB', + 'a__id' => 1, + 'a__city' => 'Berlin', + ), + array( + 'u__id' => '2', + 'u__status' => 'developer', + 'sclr0' => 'BENJAMIN', + 'a__id' => null, + 'a__city' => null, + ), + ); + + $stmt = new HydratorMockStatement($resultSet); + $hydrator = new \Doctrine\ORM\Internal\Hydration\ObjectHydrator($this->_em); + + $result = $hydrator->hydrateAll($stmt, $rsm, array(Query::HINT_FORCE_PARTIAL_LOAD => true)); + + $this->assertEquals(2, count($result)); + $this->assertInstanceOf('Doctrine\Tests\Models\CMS\CmsAddress', $result[0][0]->address); + $this->assertNull($result[1][0]->address); + } }