diff --git a/lib/Doctrine/ORM/Cache/Persister/Collection/AbstractCollectionPersister.php b/lib/Doctrine/ORM/Cache/Persister/Collection/AbstractCollectionPersister.php index f6215676307..3172fe9994b 100644 --- a/lib/Doctrine/ORM/Cache/Persister/Collection/AbstractCollectionPersister.php +++ b/lib/Doctrine/ORM/Cache/Persister/Collection/AbstractCollectionPersister.php @@ -23,6 +23,7 @@ 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 @@ public function loadCollectionCache(PersistentCollection $collection, Collection */ 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 @@ public function get(PersistentCollection $collection, $index) */ 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 @@ public function loadCriteria(PersistentCollection $collection, Criteria $criteri { 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); + } + } } diff --git a/lib/Doctrine/ORM/Persisters/Collection/OneToManyPersister.php b/lib/Doctrine/ORM/Persisters/Collection/OneToManyPersister.php index 9b14afde59b..9dc2fad548d 100644 --- a/lib/Doctrine/ORM/Persisters/Collection/OneToManyPersister.php +++ b/lib/Doctrine/ORM/Persisters/Collection/OneToManyPersister.php @@ -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 @@ public function removeElement(PersistentCollection $collection, $element) $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; } /** diff --git a/tests/Doctrine/Tests/Models/Tweet/Tweet.php b/tests/Doctrine/Tests/Models/Tweet/Tweet.php index 5455452c2b6..8a315df9ef0 100644 --- a/tests/Doctrine/Tests/Models/Tweet/Tweet.php +++ b/tests/Doctrine/Tests/Models/Tweet/Tweet.php @@ -8,6 +8,8 @@ */ class Tweet { + const CLASSNAME = __CLASS__; + /** * @Id * @GeneratedValue diff --git a/tests/Doctrine/Tests/Models/Tweet/User.php b/tests/Doctrine/Tests/Models/Tweet/User.php index cb715702d12..4722e63176b 100644 --- a/tests/Doctrine/Tests/Models/Tweet/User.php +++ b/tests/Doctrine/Tests/Models/Tweet/User.php @@ -10,6 +10,8 @@ */ class User { + const CLASSNAME = __CLASS__; + /** * @Id * @GeneratedValue diff --git a/tests/Doctrine/Tests/ORM/Cache/Persister/Collection/AbstractCollectionPersisterTest.php b/tests/Doctrine/Tests/ORM/Cache/Persister/Collection/AbstractCollectionPersisterTest.php index 17e7c4d7e3f..0d767aca9d9 100644 --- a/tests/Doctrine/Tests/ORM/Cache/Persister/Collection/AbstractCollectionPersisterTest.php +++ b/tests/Doctrine/Tests/ORM/Cache/Persister/Collection/AbstractCollectionPersisterTest.php @@ -240,22 +240,6 @@ public function testInvokeRemoveElement() $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"); diff --git a/tests/Doctrine/Tests/ORM/Functional/ExtraLazyCollectionTest.php b/tests/Doctrine/Tests/ORM/Functional/ExtraLazyCollectionTest.php index 2807ccfd7c1..6021a9efd8f 100644 --- a/tests/Doctrine/Tests/ORM/Functional/ExtraLazyCollectionTest.php +++ b/tests/Doctrine/Tests/ORM/Functional/ExtraLazyCollectionTest.php @@ -5,13 +5,16 @@ 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 @@ public function testRemoveElementOneToMany() $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 @@ public function testRemoveElementOneToMany() $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 @@ public function testRemovalOfManagedElementFromOneToManyJoinedInheritanceCollect $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 @@ public function testRemovalOfNewManagedElementFromOneToManyJoinedInheritanceColl $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 @@ private function loadFixture() $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); + } } diff --git a/tests/Doctrine/Tests/ORM/Functional/ValueConversionType/OneToManyExtraLazyTest.php b/tests/Doctrine/Tests/ORM/Functional/ValueConversionType/OneToManyExtraLazyTest.php index bb90067824b..75b5403fe55 100644 --- a/tests/Doctrine/Tests/ORM/Functional/ValueConversionType/OneToManyExtraLazyTest.php +++ b/tests/Doctrine/Tests/ORM/Functional/ValueConversionType/OneToManyExtraLazyTest.php @@ -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;