Merge pull request #908 from FabioBatSilva/DDC-2862

[DDC-2862][SLC] Fix lazy association load

lib/Doctrine/ORM/Cache/AssociationCacheEntry.php

@@ -0,0 +1,66 @@
+<?php
+
+/*
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+ * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+ * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+ * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+ * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+ * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+ * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+ * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+ * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ *
+ * This software consists of voluntary contributions made by many individuals
+ * and is licensed under the MIT license. For more information, see
+ * <http://www.doctrine-project.org>.
+ */
+
+namespace Doctrine\ORM\Cache;
+
+/**
+ * Association cache entry
+ *
+ * @since   2.5
+ * @author  Fabio B. Silva <fabio.bat.silva@gmail.com>
+ */
+class AssociationCacheEntry implements CacheEntry
+{
+    /**
+     * READ-ONLY: Public only for performance reasons, it should be considered immutable.
+     *
+     * @var array The entity identifier
+     */
+    public $identifier;
+
+    /**
+     * READ-ONLY: Public only for performance reasons, it should be considered immutable.
+     *
+     * @var string The entity class name
+     */
+    public $class;
+
+    /**
+     * @param string $class       The entity class.
+     * @param array  $identifier  The entity identifier.
+     */
+    public function __construct($class, array $identifier)
+    {
+        $this->class       = $class;
+        $this->identifier  = $identifier;
+    }
+
+    /**
+     * Creates a new AssociationCacheEntry
+     *
+     * This method allow Doctrine\Common\Cache\PhpFileCache compatibility
+     *
+     * @param array $values array containing property values
+     */
+    public static function __set_state(array $values)
+    {
+        return new self($values['class'], $values['identifier']);
+    }
+}

lib/Doctrine/ORM/Cache/DefaultEntityHydrator.php

@@ -20,6 +20,8 @@
 
 namespace Doctrine\ORM\Cache;
 
+use Doctrine\Common\Util\ClassUtils;
+
 use Doctrine\ORM\Query;
 use Doctrine\ORM\Cache\EntityCacheKey;
 use Doctrine\ORM\Mapping\ClassMetadata;
@@ -79,13 +81,15 @@ class DefaultEntityHydrator implements EntityHydrator
             }
 
             if ( ! isset($assoc['id'])) {
-                $data[$name] = $this->uow->getEntityIdentifier($data[$name]);
+                $targetClass = ClassUtils::getClass($data[$name]);
+                $targetId    = $this->uow->getEntityIdentifier($data[$name]);
+                $data[$name] = new AssociationCacheEntry($targetClass, $targetId);
 
                 continue;
             }
 
             // handle association identifier
-            $targetId = is_object($data[$name]) && $this->em->getMetadataFactory()->hasMetadataFor(get_class($data[$name]))
+            $targetId = is_object($data[$name]) && $this->uow->isInIdentityMap($data[$name])
                 ? $this->uow->getEntityIdentifier($data[$name])
                 : $data[$name];
 
@@ -99,7 +103,7 @@ class DefaultEntityHydrator implements EntityHydrator
                 $targetId     = array($targetEntity->identifier[0] => $targetId);
             }
 
-            $data[$name] = $targetId;
+            $data[$name] = new AssociationCacheEntry($assoc['targetEntity'], $targetId);
         }
 
         return new EntityCacheEntry($metadata->name, $data);
@@ -123,18 +127,26 @@ class DefaultEntityHydrator implements EntityHydrator
                 continue;
             }
 
-            $assocId        = $data[$name];
+            $assocClass     = $data[$name]->class;
+            $assocId        = $data[$name]->identifier;
+            $isEagerLoad    = ($assoc['fetch'] === ClassMetadata::FETCH_EAGER || ($assoc['type'] === ClassMetadata::ONE_TO_ONE && ! $assoc['isOwningSide']));
+
+            if ( ! $isEagerLoad) {
+                $data[$name] = $this->em->getReference($assocClass, $assocId);
+
+                continue;
+            }
+
+            $assocKey       = new EntityCacheKey($assoc['targetEntity'], $assocId);
             $assocPersister = $this->uow->getEntityPersister($assoc['targetEntity']);
             $assocRegion    = $assocPersister->getCacheRegion();
-            $assocEntry     = $assocRegion->get(new EntityCacheKey($assoc['targetEntity'], $assocId));
+            $assocEntry     = $assocRegion->get($assocKey);
 
             if ($assocEntry === null) {
                 return null;
             }
 
-            $data[$name] = $assoc['fetch'] === ClassMetadata::FETCH_EAGER || ($assoc['type'] === ClassMetadata::ONE_TO_ONE && ! $assoc['isOwningSide'])
+            $data[$name] = $this->uow->createEntity($assocEntry->class, $assocEntry->data, $hints);
-                ? $this->uow->createEntity($assocEntry->class, $assocEntry->data, $hints)
-                : $this->em->getReference($assocEntry->class, $assocId);
         }
 
         if ($entity !== null) {

tests/Doctrine/Tests/ORM/Cache/DefaultEntityHydratorTest.php

@@ -2,13 +2,15 @@
 
 namespace Doctrine\Tests\ORM\Cache;
 
-use Doctrine\ORM\UnitOfWork;
 use Doctrine\Tests\OrmTestCase;
-use Doctrine\ORM\Cache\EntityCacheKey;
-use Doctrine\ORM\Cache\EntityCacheEntry;
 use Doctrine\Tests\Models\Cache\State;
 use Doctrine\Tests\Models\Cache\Country;
+
+use Doctrine\ORM\UnitOfWork;
+use Doctrine\ORM\Cache\EntityCacheKey;
+use Doctrine\ORM\Cache\EntityCacheEntry;
 use Doctrine\ORM\Cache\DefaultEntityHydrator;
+use Doctrine\ORM\Cache\AssociationCacheEntry;
 
 /**
  * @group DDC-2183
@@ -92,7 +94,7 @@ class DefaultEntityHydratorTest extends OrmTestCase
         ), $cache->data);
     }
 
-    public function testBuildCacheEntryOwningSide()
+    public function testBuildCacheEntryAssociation()
     {
         $country        = new Country('Foo');
         $state          = new State('Bat', $country);
@@ -119,7 +121,7 @@ class DefaultEntityHydratorTest extends OrmTestCase
         $this->assertEquals(array(
             'id'        => 11,
             'name'      => 'Bar',
-            'country'   => array ('id' => 11),
+            'country'   => new AssociationCacheEntry(Country::CLASSNAME, array('id' => 11)),
         ), $cache->data);
     }
 
@@ -147,7 +149,7 @@ class DefaultEntityHydratorTest extends OrmTestCase
         $this->assertEquals(array(
             'id'        => 11,
             'name'      => 'Bar',
-            'country'   => array ('id' => 11),
+            'country'   => new AssociationCacheEntry(Country::CLASSNAME, array('id' => 11)),
         ), $cache->data);
     }
 }

tests/Doctrine/Tests/ORM/Functional/SecondLevelCacheManyToOneTest.php

@@ -4,6 +4,7 @@ namespace Doctrine\Tests\ORM\Functional;
 
 use Doctrine\Tests\Models\Cache\Country;
 use Doctrine\Tests\Models\Cache\State;
+use Doctrine\ORM\Mapping\ClassMetadata;
 
 /**
  * @group DDC-2183
@@ -94,27 +95,48 @@ class SecondLevelCacheManyToOneTest extends SecondLevelCacheAbstractTest
         $this->assertEquals($this->states[1]->getCountry()->getName(), $c4->getCountry()->getName());
     }
 
-    public function testLoadFromDatabaseWhenAssociationIsMissing()
+    public function testShouldNotReloadWhenAssociationIsMissing()
     {
         $this->loadFixturesCountries();
         $this->loadFixturesStates();
         $this->_em->clear();
 
-        $this->assertTrue($this->cache->containsEntity(Country::CLASSNAME, $this->states[0]->getCountry()->getId()));
+        $stateId1 = $this->states[0]->getId();
-        $this->assertTrue($this->cache->containsEntity(Country::CLASSNAME, $this->states[1]->getCountry()->getId()));
+        $stateId2 = $this->states[3]->getId();
-        $this->assertTrue($this->cache->containsEntity(State::CLASSNAME, $this->states[0]->getId()));
+
-        $this->assertTrue($this->cache->containsEntity(State::CLASSNAME, $this->states[1]->getId()));
+        $countryId1 = $this->states[0]->getCountry()->getId();
+        $countryId2 = $this->states[3]->getCountry()->getId();
+
+        $this->assertTrue($this->cache->containsEntity(Country::CLASSNAME, $countryId1));
+        $this->assertTrue($this->cache->containsEntity(Country::CLASSNAME, $countryId2));
+        $this->assertTrue($this->cache->containsEntity(State::CLASSNAME, $stateId1));
+        $this->assertTrue($this->cache->containsEntity(State::CLASSNAME, $stateId2));
 
         $this->cache->evictEntityRegion(Country::CLASSNAME);
-        $this->assertFalse($this->cache->containsEntity(Country::CLASSNAME, $this->states[0]->getCountry()->getId()));
+
-        $this->assertFalse($this->cache->containsEntity(Country::CLASSNAME, $this->states[1]->getCountry()->getId()));
+        $this->assertFalse($this->cache->containsEntity(Country::CLASSNAME, $countryId1));
+        $this->assertFalse($this->cache->containsEntity(Country::CLASSNAME, $countryId2));
 
         $this->_em->clear();
 
         $queryCount = $this->getCurrentQueryCount();
 
-        $state1 = $this->_em->find(State::CLASSNAME, $this->states[0]->getId());
+        $state1 = $this->_em->find(State::CLASSNAME, $stateId1);
-        $state2 = $this->_em->find(State::CLASSNAME, $this->states[1]->getId());
+        $state2 = $this->_em->find(State::CLASSNAME, $stateId2);
+
+        $this->assertEquals($queryCount, $this->getCurrentQueryCount());
+
+        $this->assertInstanceOf(State::CLASSNAME, $state1);
+        $this->assertInstanceOf(State::CLASSNAME, $state2);
+        $this->assertInstanceOf(Country::CLASSNAME, $state1->getCountry());
+        $this->assertInstanceOf(Country::CLASSNAME, $state2->getCountry());
+
+        $queryCount = $this->getCurrentQueryCount();
+
+        $this->assertNotNull($state1->getCountry()->getName());
+        $this->assertNotNull($state2->getCountry()->getName());
+        $this->assertEquals($countryId1, $state1->getCountry()->getId());
+        $this->assertEquals($countryId2, $state2->getCountry()->getId());
 
         $this->assertEquals($queryCount + 2, $this->getCurrentQueryCount());
     }

tests/Doctrine/Tests/ORM/Functional/SingleTableInheritanceTest.php

@@ -240,6 +240,9 @@ class SingleTableInheritanceTest extends \Doctrine\Tests\OrmFunctionalTestCase
         $this->assertEquals(1000, $contract->getFixPrice());
     }
 
+    /**
+     * @group non-cacheable
+     */
     public function testUpdateChildClassWithCondition()
     {
         $this->loadFullFixture();

tests/Doctrine/Tests/ORM/Functional/Ticket/DDC2862Test.php

@@ -4,19 +4,23 @@ namespace Doctrine\Tests\ORM\Functional\Ticket;
 
 /**
  * @group DDC-2862
+ * @group DDC-2183
  */
 class DDC2862Test extends \Doctrine\Tests\OrmFunctionalTestCase
 {
-
     public function setUp()
     {
         $this->enableSecondLevelCache();
         parent::setUp();
 
-        $this->_schemaTool->createSchema(array(
+        try {
-            $this->_em->getClassMetadata(DDC2862User::CLASSNAME),
+            $this->_schemaTool->createSchema(array(
-            $this->_em->getClassMetadata(DDC2862Driver::CLASSNAME),
+                $this->_em->getClassMetadata(DDC2862User::CLASSNAME),
-        ));
+                $this->_em->getClassMetadata(DDC2862Driver::CLASSNAME),
+            ));
+        } catch (\Doctrine\ORM\Tools\ToolsException $exc) {
+            $this->assertInstanceOf('Doctrine\DBAL\Exception\TableExistsException', $exc->getPrevious());
+        }
     }
 
     public function testIssue()
@@ -57,6 +61,52 @@ class DDC2862Test extends \Doctrine\Tests\OrmFunctionalTestCase
         $this->assertEquals('Foo', $driver3->getUserProfile()->getName());
     }
 
+    public function testIssueReopened()
+    {
+        $user1    = new DDC2862User('Foo');
+        $driver1  = new DDC2862Driver('Bar' , $user1);
+
+        $this->_em->persist($user1);
+        $this->_em->persist($driver1);
+        $this->_em->flush();
+        $this->_em->clear();
+
+        $this->_em->getCache()->evictEntityRegion(DDC2862User::CLASSNAME);
+        $this->_em->getCache()->evictEntityRegion(DDC2862Driver::CLASSNAME);
+
+        $this->assertFalse($this->_em->getCache()->containsEntity(DDC2862User::CLASSNAME, array('id' => $user1->getId())));
+        $this->assertFalse($this->_em->getCache()->containsEntity(DDC2862Driver::CLASSNAME, array('id' => $driver1->getId())));
+
+        $queryCount = $this->getCurrentQueryCount();
+        $driver2    = $this->_em->find(DDC2862Driver::CLASSNAME, $driver1->getId());
+
+        $this->assertInstanceOf(DDC2862Driver::CLASSNAME, $driver2);
+        $this->assertInstanceOf(DDC2862User::CLASSNAME, $driver2->getUserProfile());
+        $this->assertEquals($queryCount + 1, $this->getCurrentQueryCount());
+
+        $this->_em->clear();
+
+        $this->assertFalse($this->_em->getCache()->containsEntity(DDC2862User::CLASSNAME, array('id' => $user1->getId())));
+        $this->assertTrue($this->_em->getCache()->containsEntity(DDC2862Driver::CLASSNAME, array('id' => $driver1->getId())));
+
+        $queryCount = $this->getCurrentQueryCount();
+        $driver3    = $this->_em->find(DDC2862Driver::CLASSNAME, $driver1->getId());
+
+        $this->assertInstanceOf(DDC2862Driver::CLASSNAME, $driver3);
+        $this->assertInstanceOf(DDC2862User::CLASSNAME, $driver3->getUserProfile());
+        $this->assertEquals($queryCount, $this->getCurrentQueryCount());
+        $this->assertEquals('Foo', $driver3->getUserProfile()->getName());
+        $this->assertEquals($queryCount + 1, $this->getCurrentQueryCount());
+
+        $queryCount = $this->getCurrentQueryCount();
+        $driver4    = $this->_em->find(DDC2862Driver::CLASSNAME, $driver1->getId());
+
+        $this->assertInstanceOf(DDC2862Driver::CLASSNAME, $driver4);
+        $this->assertInstanceOf(DDC2862User::CLASSNAME, $driver4->getUserProfile());
+        $this->assertEquals($queryCount, $this->getCurrentQueryCount());
+        $this->assertEquals('Foo', $driver4->getUserProfile()->getName());
+        $this->assertEquals($queryCount, $this->getCurrentQueryCount());
+    }
 }
 
 /**