Skip to content

Commit

Permalink
Merge pull request #908 from FabioBatSilva/DDC-2862
Browse files Browse the repository at this point in the history 
[DDC-2862][SLC] Fix lazy association load
  • Loading branch information
@beberlei
beberlei committed Feb 8, 2014
2 parents b76e95c + 7e5a1c6 commit 960fbfc
Show file tree
Hide file tree
Showing 6 changed files with 183 additions and 28 deletions.
66 changes: 66 additions & 0 deletions lib/Doctrine/ORM/Cache/AssociationCacheEntry.php
View file
@@ -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']);
}
}
28 changes: 20 additions & 8 deletions lib/Doctrine/ORM/Cache/DefaultEntityHydrator.php
View file
Expand Up @@ -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;
Expand Down Expand Up @@ -79,13 +81,15 @@ public function buildCacheEntry(ClassMetadata $metadata, EntityCacheKey $key, $e
}

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];

Expand All @@ -99,7 +103,7 @@ public function buildCacheEntry(ClassMetadata $metadata, EntityCacheKey $key, $e
$targetId = array($targetEntity->identifier[0] => $targetId);
}

$data[$name] = $targetId;
$data[$name] = new AssociationCacheEntry($assoc['targetEntity'], $targetId);
}

return new EntityCacheEntry($metadata->name, $data);
Expand All @@ -123,18 +127,26 @@ public function loadCacheEntry(ClassMetadata $metadata, EntityCacheKey $key, Ent
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'])
? $this->uow->createEntity($assocEntry->class, $assocEntry->data, $hints)
: $this->em->getReference($assocEntry->class, $assocId);
$data[$name] = $this->uow->createEntity($assocEntry->class, $assocEntry->data, $hints);
}

if ($entity !== null) {
Expand Down
14 changes: 8 additions & 6 deletions tests/Doctrine/Tests/ORM/Cache/DefaultEntityHydratorTest.php
View file
Expand Up @@ -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
Expand Down Expand Up @@ -92,7 +94,7 @@ public function testBuildCacheEntry()
), $cache->data);
}

public function testBuildCacheEntryOwningSide()
public function testBuildCacheEntryAssociation()
{
$country = new Country('Foo');
$state = new State('Bat', $country);
Expand All @@ -119,7 +121,7 @@ public function testBuildCacheEntryOwningSide()
$this->assertEquals(array(
'id' => 11,
'name' => 'Bar',
'country' => array ('id' => 11),
'country' => new AssociationCacheEntry(Country::CLASSNAME, array('id' => 11)),
), $cache->data);
}

Expand Down Expand Up @@ -147,7 +149,7 @@ public function testBuildCacheEntryNonInitializedAssocProxy()
$this->assertEquals(array(
'id' => 11,
'name' => 'Bar',
'country' => array ('id' => 11),
'country' => new AssociationCacheEntry(Country::CLASSNAME, array('id' => 11)),
), $cache->data);
}
}
40 changes: 31 additions & 9 deletions tests/Doctrine/Tests/ORM/Functional/SecondLevelCacheManyToOneTest.php
View file
Expand Up @@ -4,6 +4,7 @@

use Doctrine\Tests\Models\Cache\Country;
use Doctrine\Tests\Models\Cache\State;
use Doctrine\ORM\Mapping\ClassMetadata;

/**
* @group DDC-2183
Expand Down Expand Up @@ -94,27 +95,48 @@ public function testPutAndLoadManyToOneRelation()
$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()));
$this->assertTrue($this->cache->containsEntity(Country::CLASSNAME, $this->states[1]->getCountry()->getId()));
$this->assertTrue($this->cache->containsEntity(State::CLASSNAME, $this->states[0]->getId()));
$this->assertTrue($this->cache->containsEntity(State::CLASSNAME, $this->states[1]->getId()));
$stateId1 = $this->states[0]->getId();
$stateId2 = $this->states[3]->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());
$state2 = $this->_em->find(State::CLASSNAME, $this->states[1]->getId());
$state1 = $this->_em->find(State::CLASSNAME, $stateId1);
$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());
}
Expand Down
3 changes: 3 additions & 0 deletions tests/Doctrine/Tests/ORM/Functional/SingleTableInheritanceTest.php
View file
Expand Up @@ -240,6 +240,9 @@ public function testQueryChildClassWithCondition()
$this->assertEquals(1000, $contract->getFixPrice());
}

/**
* @group non-cacheable
*/
public function testUpdateChildClassWithCondition()
{
$this->loadFullFixture();
Expand Down
60 changes: 55 additions & 5 deletions tests/Doctrine/Tests/ORM/Functional/Ticket/DDC2862Test.php
View file
Expand Up @@ -4,19 +4,23 @@

/**
* @group DDC-2862
* @group DDC-2183
*/
class DDC2862Test extends \Doctrine\Tests\OrmFunctionalTestCase
{

public function setUp()
{
$this->enableSecondLevelCache();
parent::setUp();

$this->_schemaTool->createSchema(array(
$this->_em->getClassMetadata(DDC2862User::CLASSNAME),
$this->_em->getClassMetadata(DDC2862Driver::CLASSNAME),
));
try {
$this->_schemaTool->createSchema(array(
$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()
Expand Down Expand Up @@ -57,6 +61,52 @@ public function testIssue()
$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());
}
}

/**
Expand Down

0 comments on commit 960fbfc

Please sign in to comment.