Merge pull request #1281 from Ocramius/hotfix/#1169-extra-lazy-one-to-many-should-not-delete-referenced-entities
Hotfix/#1169 extra lazy one to many should not delete referenced entities
This commit is contained in:
commit
5bf18298b1
@ -23,6 +23,7 @@ namespace Doctrine\ORM\Cache\Persister\Collection;
|
||||
use Doctrine\Common\Collections\Criteria;
|
||||
use Doctrine\ORM\Cache\EntityCacheKey;
|
||||
use Doctrine\ORM\Cache\CollectionCacheKey;
|
||||
use Doctrine\ORM\Cache\Persister\Entity\CachedEntityPersister;
|
||||
use Doctrine\ORM\Persisters\Collection\CollectionPersister;
|
||||
use Doctrine\ORM\PersistentCollection;
|
||||
use Doctrine\ORM\EntityManagerInterface;
|
||||
@ -162,13 +163,13 @@ abstract class AbstractCollectionPersister implements CachedCollectionPersister
|
||||
*/
|
||||
public function storeCollectionCache(CollectionCacheKey $key, $elements)
|
||||
{
|
||||
/* @var $targetPersister CachedEntityPersister */
|
||||
$targetPersister = $this->uow->getEntityPersister($this->targetEntity->rootEntityName);
|
||||
$targetRegion = $targetPersister->getCacheRegion();
|
||||
$targetHydrator = $targetPersister->getEntityHydrator();
|
||||
$entry = $this->hydrator->buildCacheEntry($this->targetEntity, $key, $elements);
|
||||
|
||||
foreach ($entry->identifiers as $index => $entityKey) {
|
||||
|
||||
if ($targetRegion->contains($entityKey)) {
|
||||
continue;
|
||||
}
|
||||
@ -238,15 +239,13 @@ abstract class AbstractCollectionPersister implements CachedCollectionPersister
|
||||
*/
|
||||
public function removeElement(PersistentCollection $collection, $element)
|
||||
{
|
||||
return $this->persister->removeElement($collection, $element);
|
||||
}
|
||||
if ($persisterResult = $this->persister->removeElement($collection, $element)) {
|
||||
$this->evictCollectionCache($collection);
|
||||
$this->evictElementCache($this->sourceEntity->rootEntityName, $collection->getOwner());
|
||||
$this->evictElementCache($this->targetEntity->rootEntityName, $element);
|
||||
}
|
||||
|
||||
/**
|
||||
* {@inheritdoc}
|
||||
*/
|
||||
public function removeKey(PersistentCollection $collection, $key)
|
||||
{
|
||||
return $this->persister->removeKey($collection, $key);
|
||||
return $persisterResult;
|
||||
}
|
||||
|
||||
/**
|
||||
@ -264,4 +263,42 @@ abstract class AbstractCollectionPersister implements CachedCollectionPersister
|
||||
{
|
||||
return $this->persister->loadCriteria($collection, $criteria);
|
||||
}
|
||||
|
||||
/**
|
||||
* Clears cache entries related to the current collection
|
||||
*
|
||||
* @param PersistentCollection $collection
|
||||
*/
|
||||
protected function evictCollectionCache(PersistentCollection $collection)
|
||||
{
|
||||
$key = new CollectionCacheKey(
|
||||
$this->sourceEntity->rootEntityName,
|
||||
$this->association['fieldName'],
|
||||
$this->uow->getEntityIdentifier($collection->getOwner())
|
||||
);
|
||||
|
||||
$this->region->evict($key);
|
||||
|
||||
if ($this->cacheLogger) {
|
||||
$this->cacheLogger->collectionCachePut($this->regionName, $key);
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* @param string $targetEntity
|
||||
* @param object $element
|
||||
*/
|
||||
protected function evictElementCache($targetEntity, $element)
|
||||
{
|
||||
/* @var $targetPersister CachedEntityPersister */
|
||||
$targetPersister = $this->uow->getEntityPersister($targetEntity);
|
||||
$targetRegion = $targetPersister->getCacheRegion();
|
||||
$key = new EntityCacheKey($targetEntity, $this->uow->getEntityIdentifier($element));
|
||||
|
||||
$targetRegion->evict($key);
|
||||
|
||||
if ($this->cacheLogger) {
|
||||
$this->cacheLogger->entityCachePut($targetRegion->getName(), $key);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
@ -20,6 +20,7 @@
|
||||
namespace Doctrine\ORM\Persisters\Collection;
|
||||
|
||||
use Doctrine\Common\Collections\Criteria;
|
||||
use Doctrine\Common\Proxy\Proxy;
|
||||
use Doctrine\ORM\PersistentCollection;
|
||||
|
||||
/**
|
||||
@ -163,7 +164,20 @@ class OneToManyPersister extends AbstractCollectionPersister
|
||||
$mapping = $collection->getMapping();
|
||||
$persister = $this->uow->getEntityPersister($mapping['targetEntity']);
|
||||
|
||||
return $persister->delete($element);
|
||||
$targetMetadata = $this->em->getClassMetadata($mapping['targetEntity']);
|
||||
|
||||
if ($element instanceof Proxy && ! $element->__isInitialized()) {
|
||||
$element->__load();
|
||||
}
|
||||
|
||||
// clearing owning side value
|
||||
$targetMetadata->reflFields[$mapping['mappedBy']]->setValue($element, null);
|
||||
|
||||
$this->uow->computeChangeSet($targetMetadata, $element);
|
||||
|
||||
$persister->update($element);
|
||||
|
||||
return true;
|
||||
}
|
||||
|
||||
/**
|
||||
|
@ -8,6 +8,8 @@ namespace Doctrine\Tests\Models\Tweet;
|
||||
*/
|
||||
class Tweet
|
||||
{
|
||||
const CLASSNAME = __CLASS__;
|
||||
|
||||
/**
|
||||
* @Id
|
||||
* @GeneratedValue
|
||||
|
@ -10,6 +10,8 @@ use Doctrine\Common\Collections\ArrayCollection;
|
||||
*/
|
||||
class User
|
||||
{
|
||||
const CLASSNAME = __CLASS__;
|
||||
|
||||
/**
|
||||
* @Id
|
||||
* @GeneratedValue
|
||||
|
@ -240,22 +240,6 @@ abstract class AbstractCollectionPersisterTest extends OrmTestCase
|
||||
$this->assertFalse($persister->removeElement($collection, $element));
|
||||
}
|
||||
|
||||
public function testInvokeRemoveKey()
|
||||
{
|
||||
$entity = new State("Foo");
|
||||
$persister = $this->createPersisterDefault();
|
||||
$collection = $this->createCollection($entity);
|
||||
|
||||
$this->em->getUnitOfWork()->registerManaged($entity, array('id'=>1), array('id'=>1, 'name'=>'Foo'));
|
||||
|
||||
$this->collectionPersister->expects($this->once())
|
||||
->method('removeKey')
|
||||
->with($this->equalTo($collection), $this->equalTo(0))
|
||||
->will($this->returnValue(false));
|
||||
|
||||
$this->assertFalse($persister->removeKey($collection, 0));
|
||||
}
|
||||
|
||||
public function testInvokeGet()
|
||||
{
|
||||
$entity = new State("Foo");
|
||||
|
@ -5,13 +5,16 @@ namespace Doctrine\Tests\ORM\Functional;
|
||||
use Doctrine\ORM\Mapping\ClassMetadataInfo;
|
||||
use Doctrine\Tests\Models\DDC2504\DDC2504ChildClass;
|
||||
use Doctrine\Tests\Models\DDC2504\DDC2504OtherClass;
|
||||
use Doctrine\Tests\Models\Tweet\Tweet;
|
||||
use Doctrine\Tests\Models\Tweet\User;
|
||||
use Doctrine\Tests\OrmFunctionalTestCase;
|
||||
|
||||
/**
|
||||
* Description of ExtraLazyCollectionTest
|
||||
*
|
||||
* @author beberlei
|
||||
*/
|
||||
class ExtraLazyCollectionTest extends \Doctrine\Tests\OrmFunctionalTestCase
|
||||
class ExtraLazyCollectionTest extends OrmFunctionalTestCase
|
||||
{
|
||||
private $userId;
|
||||
private $userId2;
|
||||
@ -27,6 +30,7 @@ class ExtraLazyCollectionTest extends \Doctrine\Tests\OrmFunctionalTestCase
|
||||
|
||||
public function setUp()
|
||||
{
|
||||
$this->useModelSet('tweet');
|
||||
$this->useModelSet('cms');
|
||||
$this->useModelSet('ddc2504');
|
||||
parent::setUp();
|
||||
@ -480,7 +484,8 @@ class ExtraLazyCollectionTest extends \Doctrine\Tests\OrmFunctionalTestCase
|
||||
$user->articles->removeElement($article);
|
||||
|
||||
$this->assertFalse($user->articles->isInitialized(), "Post-Condition: Collection is not initialized.");
|
||||
$this->assertEquals($queryCount + 1, $this->getCurrentQueryCount());
|
||||
// NOTE: +2 queries because CmsArticle is a versioned entity, and that needs to be handled accordingly
|
||||
$this->assertEquals($queryCount + 2, $this->getCurrentQueryCount());
|
||||
|
||||
// Test One to Many removal with Entity state as new
|
||||
$article = new \Doctrine\Tests\Models\CMS\CmsArticle();
|
||||
@ -501,7 +506,7 @@ class ExtraLazyCollectionTest extends \Doctrine\Tests\OrmFunctionalTestCase
|
||||
|
||||
$user->articles->removeElement($article);
|
||||
|
||||
$this->assertEquals($queryCount + 1, $this->getCurrentQueryCount(), "Removing a persisted entity should cause one query to be executed.");
|
||||
$this->assertEquals($queryCount, $this->getCurrentQueryCount(), "Removing a persisted entity will not cause queries when the owning side doesn't actually change.");
|
||||
$this->assertFalse($user->articles->isInitialized(), "Post-Condition: Collection is not initialized.");
|
||||
|
||||
// Test One to Many removal with Entity state as managed
|
||||
@ -532,17 +537,10 @@ class ExtraLazyCollectionTest extends \Doctrine\Tests\OrmFunctionalTestCase
|
||||
|
||||
$this->assertFalse($otherClass->childClasses->isInitialized(), 'Collection is not initialized.');
|
||||
|
||||
$expectedQueryCount = $queryCount + 1;
|
||||
|
||||
if (! $this->_em->getConnection()->getDatabasePlatform()->supportsForeignKeyConstraints()) {
|
||||
// the ORM emulates cascades in a JTI if the underlying platform does not support them
|
||||
$expectedQueryCount = $queryCount + 2;
|
||||
};
|
||||
|
||||
$this->assertEquals(
|
||||
$expectedQueryCount,
|
||||
$queryCount + 1,
|
||||
$this->getCurrentQueryCount(),
|
||||
'One removal per table in the JTI has been executed'
|
||||
'The owning side of the association is updated'
|
||||
);
|
||||
|
||||
$this->assertFalse($otherClass->childClasses->contains($childClass));
|
||||
@ -601,17 +599,10 @@ class ExtraLazyCollectionTest extends \Doctrine\Tests\OrmFunctionalTestCase
|
||||
|
||||
$otherClass->childClasses->removeElement($childClass);
|
||||
|
||||
$expectedQueryCount = $queryCount + 1;
|
||||
|
||||
if (! $this->_em->getConnection()->getDatabasePlatform()->supportsForeignKeyConstraints()) {
|
||||
// the ORM emulates cascades in a JTI if the underlying platform does not support them
|
||||
$expectedQueryCount = $queryCount + 2;
|
||||
};
|
||||
|
||||
$this->assertEquals(
|
||||
$expectedQueryCount,
|
||||
$queryCount,
|
||||
$this->getCurrentQueryCount(),
|
||||
'Removing a persisted entity should cause two queries to be executed.'
|
||||
'No queries are executed, as the owning side of the association is not actually updated.'
|
||||
);
|
||||
$this->assertFalse($otherClass->childClasses->isInitialized(), 'Collection is not initialized.');
|
||||
}
|
||||
@ -1040,4 +1031,103 @@ class ExtraLazyCollectionTest extends \Doctrine\Tests\OrmFunctionalTestCase
|
||||
$this->phonenumber = $phonenumber1->phonenumber;
|
||||
|
||||
}
|
||||
|
||||
/**
|
||||
* @group DDC-3343
|
||||
*/
|
||||
public function testRemovesManagedElementFromOneToManyExtraLazyCollection()
|
||||
{
|
||||
list($userId, $tweetId) = $this->loadTweetFixture();
|
||||
|
||||
/* @var $user User */
|
||||
$user = $this->_em->find(User::CLASSNAME, $userId);
|
||||
|
||||
$user->tweets->removeElement($this->_em->find(Tweet::CLASSNAME, $tweetId));
|
||||
|
||||
$this->_em->clear();
|
||||
|
||||
/* @var $user User */
|
||||
$user = $this->_em->find(User::CLASSNAME, $userId);
|
||||
|
||||
$this->assertCount(0, $user->tweets);
|
||||
}
|
||||
|
||||
/**
|
||||
* @group DDC-3343
|
||||
*/
|
||||
public function testRemovesManagedElementFromOneToManyExtraLazyCollectionWithoutDeletingTheTargetEntityEntry()
|
||||
{
|
||||
list($userId, $tweetId) = $this->loadTweetFixture();
|
||||
|
||||
/* @var $user User */
|
||||
$user = $this->_em->find(User::CLASSNAME, $userId);
|
||||
|
||||
$e = $this->_em->find(Tweet::CLASSNAME, $tweetId);
|
||||
|
||||
$user->tweets->removeElement($e);
|
||||
|
||||
$this->_em->clear();
|
||||
|
||||
/* @var $tweet Tweet */
|
||||
$tweet = $this->_em->find(Tweet::CLASSNAME, $tweetId);
|
||||
$this->assertInstanceOf(
|
||||
Tweet::CLASSNAME,
|
||||
$tweet,
|
||||
'Even though the collection is extra lazy, the tweet should not have been deleted'
|
||||
);
|
||||
|
||||
$this->assertNull($tweet->author, 'Tweet author link has been removed');
|
||||
}
|
||||
|
||||
/**
|
||||
* @group DDC-3343
|
||||
*/
|
||||
public function testRemovingManagedLazyProxyFromExtraLazyOneToManyDoesRemoveTheAssociationButNotTheEntity()
|
||||
{
|
||||
list($userId, $tweetId) = $this->loadTweetFixture();
|
||||
|
||||
/* @var $user User */
|
||||
$user = $this->_em->find(User::CLASSNAME, $userId);
|
||||
$tweet = $this->_em->getReference(Tweet::CLASSNAME, $tweetId);
|
||||
|
||||
$user->tweets->removeElement($this->_em->getReference(Tweet::CLASSNAME, $tweetId));
|
||||
|
||||
$this->_em->clear();
|
||||
|
||||
/* @var $tweet Tweet */
|
||||
$tweet = $this->_em->find(Tweet::CLASSNAME, $tweet->id);
|
||||
$this->assertInstanceOf(
|
||||
Tweet::CLASSNAME,
|
||||
$tweet,
|
||||
'Even though the collection is extra lazy, the tweet should not have been deleted'
|
||||
);
|
||||
|
||||
$this->assertNull($tweet->author);
|
||||
|
||||
/* @var $user User */
|
||||
$user = $this->_em->find(User::CLASSNAME, $userId);
|
||||
|
||||
$this->assertCount(0, $user->tweets);
|
||||
}
|
||||
|
||||
/**
|
||||
* @return int[] ordered tuple: user id and tweet id
|
||||
*/
|
||||
private function loadTweetFixture()
|
||||
{
|
||||
$user = new User();
|
||||
$tweet = new Tweet();
|
||||
|
||||
$user->name = 'ocramius';
|
||||
$tweet->content = 'The cat is on the table';
|
||||
|
||||
$user->addTweet($tweet);
|
||||
|
||||
$this->_em->persist($user);
|
||||
$this->_em->persist($tweet);
|
||||
$this->_em->flush();
|
||||
$this->_em->clear();
|
||||
|
||||
return array($user->id, $tweet->id);
|
||||
}
|
||||
}
|
||||
|
@ -2,7 +2,6 @@
|
||||
|
||||
namespace Doctrine\Tests\ORM\Functional\ValueConversionType;
|
||||
|
||||
use Doctrine\DBAL\Types\Type as DBALType;
|
||||
use Doctrine\Tests\Models\ValueConversionType as Entity;
|
||||
use Doctrine\Tests\OrmFunctionalTestCase;
|
||||
|
||||
|
Loading…
x
Reference in New Issue
Block a user