Merge pull request #1288 from Ocramius/hotfix/#1169-extra-lazy-one-to-many-is-noop-when-not-doing-orphan-removal
Hotfix - #1169 - extra lazy one to many must be no-op when not doing orphan removal
This commit is contained in:
commit
3ed73b4a60
@ -78,9 +78,13 @@ class NonStrictReadWriteCachedEntityPersister extends AbstractEntityPersister
|
||||
*/
|
||||
public function delete($entity)
|
||||
{
|
||||
$this->persister->delete($entity);
|
||||
$key = new EntityCacheKey($this->class->rootEntityName, $this->uow->getEntityIdentifier($entity));
|
||||
|
||||
$this->queuedCache['delete'][] = new EntityCacheKey($this->class->rootEntityName, $this->uow->getEntityIdentifier($entity));
|
||||
if ($this->persister->delete($entity)) {
|
||||
$this->region->evict($key);
|
||||
}
|
||||
|
||||
$this->queuedCache['delete'][] = $key;
|
||||
}
|
||||
|
||||
/**
|
||||
|
@ -103,7 +103,9 @@ class ReadWriteCachedEntityPersister extends AbstractEntityPersister
|
||||
$key = new EntityCacheKey($this->class->rootEntityName, $this->uow->getEntityIdentifier($entity));
|
||||
$lock = $this->region->lock($key);
|
||||
|
||||
$this->persister->delete($entity);
|
||||
if ($this->persister->delete($entity)) {
|
||||
$this->region->evict($key);
|
||||
}
|
||||
|
||||
if ($lock === null) {
|
||||
return;
|
||||
|
@ -157,27 +157,21 @@ class OneToManyPersister extends AbstractCollectionPersister
|
||||
*/
|
||||
public function removeElement(PersistentCollection $collection, $element)
|
||||
{
|
||||
$mapping = $collection->getMapping();
|
||||
|
||||
if ( ! $mapping['orphanRemoval']) {
|
||||
// no-op: this is not the owning side, therefore no operations should be applied
|
||||
return false;
|
||||
}
|
||||
|
||||
if ( ! $this->isValidEntityState($element)) {
|
||||
return false;
|
||||
}
|
||||
|
||||
$mapping = $collection->getMapping();
|
||||
$persister = $this->uow->getEntityPersister($mapping['targetEntity']);
|
||||
|
||||
$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;
|
||||
return $this
|
||||
->uow
|
||||
->getEntityPersister($mapping['targetEntity'])
|
||||
->delete($element);
|
||||
}
|
||||
|
||||
/**
|
||||
|
@ -29,9 +29,15 @@ class User
|
||||
*/
|
||||
public $tweets;
|
||||
|
||||
/**
|
||||
* @OneToMany(targetEntity="UserList", mappedBy="owner", fetch="EXTRA_LAZY", orphanRemoval=true)
|
||||
*/
|
||||
public $userLists;
|
||||
|
||||
public function __construct()
|
||||
{
|
||||
$this->tweets = new ArrayCollection();
|
||||
$this->tweets = new ArrayCollection();
|
||||
$this->userLists = new ArrayCollection();
|
||||
}
|
||||
|
||||
public function addTweet(Tweet $tweet)
|
||||
@ -39,4 +45,10 @@ class User
|
||||
$tweet->setAuthor($this);
|
||||
$this->tweets->add($tweet);
|
||||
}
|
||||
|
||||
public function addUserList(UserList $userList)
|
||||
{
|
||||
$userList->owner = $this;
|
||||
$this->userLists->add($userList);
|
||||
}
|
||||
}
|
||||
|
29
tests/Doctrine/Tests/Models/Tweet/UserList.php
Normal file
29
tests/Doctrine/Tests/Models/Tweet/UserList.php
Normal file
@ -0,0 +1,29 @@
|
||||
<?php
|
||||
|
||||
namespace Doctrine\Tests\Models\Tweet;
|
||||
|
||||
/**
|
||||
* @Entity
|
||||
* @Table(name="tweet_user_list")
|
||||
*/
|
||||
class UserList
|
||||
{
|
||||
const CLASSNAME = __CLASS__;
|
||||
|
||||
/**
|
||||
* @Id
|
||||
* @GeneratedValue
|
||||
* @Column(type="integer")
|
||||
*/
|
||||
public $id;
|
||||
|
||||
/**
|
||||
* @Column(type="string")
|
||||
*/
|
||||
public $listName;
|
||||
|
||||
/**
|
||||
* @ManyToOne(targetEntity="User", inversedBy="userLists")
|
||||
*/
|
||||
public $owner;
|
||||
}
|
@ -7,6 +7,7 @@ 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\Models\Tweet\UserList;
|
||||
use Doctrine\Tests\OrmFunctionalTestCase;
|
||||
|
||||
/**
|
||||
@ -484,8 +485,7 @@ class ExtraLazyCollectionTest extends OrmFunctionalTestCase
|
||||
$user->articles->removeElement($article);
|
||||
|
||||
$this->assertFalse($user->articles->isInitialized(), "Post-Condition: Collection is not initialized.");
|
||||
// NOTE: +2 queries because CmsArticle is a versioned entity, and that needs to be handled accordingly
|
||||
$this->assertEquals($queryCount + 2, $this->getCurrentQueryCount());
|
||||
$this->assertEquals($queryCount, $this->getCurrentQueryCount());
|
||||
|
||||
// Test One to Many removal with Entity state as new
|
||||
$article = new \Doctrine\Tests\Models\CMS\CmsArticle();
|
||||
@ -528,22 +528,37 @@ class ExtraLazyCollectionTest extends OrmFunctionalTestCase
|
||||
*/
|
||||
public function testRemovalOfManagedElementFromOneToManyJoinedInheritanceCollectionDoesNotInitializeIt()
|
||||
{
|
||||
/* @var $otherClass DDC2504OtherClass */
|
||||
$otherClass = $this->_em->find(DDC2504OtherClass::CLASSNAME, $this->ddc2504OtherClassId);
|
||||
/* @var $childClass DDC2504ChildClass */
|
||||
$childClass = $this->_em->find(DDC2504ChildClass::CLASSNAME, $this->ddc2504ChildClassId);
|
||||
|
||||
$queryCount = $this->getCurrentQueryCount();
|
||||
|
||||
$otherClass->childClasses->removeElement($childClass);
|
||||
$childClass->other = null; // updating owning side
|
||||
|
||||
$this->assertFalse($otherClass->childClasses->isInitialized(), 'Collection is not initialized.');
|
||||
|
||||
$this->assertEquals(
|
||||
$queryCount + 1,
|
||||
$queryCount,
|
||||
$this->getCurrentQueryCount(),
|
||||
'The owning side of the association is updated'
|
||||
'No queries have been executed'
|
||||
);
|
||||
|
||||
$this->assertFalse($otherClass->childClasses->contains($childClass));
|
||||
$this->assertTrue(
|
||||
$otherClass->childClasses->contains($childClass),
|
||||
'Collection item still not updated (needs flushing)'
|
||||
);
|
||||
|
||||
$this->_em->flush();
|
||||
|
||||
$this->assertFalse(
|
||||
$otherClass->childClasses->contains($childClass),
|
||||
'Referenced item was removed in the transaction'
|
||||
);
|
||||
|
||||
$this->assertFalse($otherClass->childClasses->isInitialized(), 'Collection is not initialized.');
|
||||
}
|
||||
|
||||
/**
|
||||
@ -551,6 +566,7 @@ class ExtraLazyCollectionTest extends OrmFunctionalTestCase
|
||||
*/
|
||||
public function testRemovalOfNonManagedElementFromOneToManyJoinedInheritanceCollectionDoesNotInitializeIt()
|
||||
{
|
||||
/* @var $otherClass DDC2504OtherClass */
|
||||
$otherClass = $this->_em->find(DDC2504OtherClass::CLASSNAME, $this->ddc2504OtherClassId);
|
||||
$queryCount = $this->getCurrentQueryCount();
|
||||
|
||||
@ -568,6 +584,7 @@ class ExtraLazyCollectionTest extends OrmFunctionalTestCase
|
||||
*/
|
||||
public function testRemovalOfNewElementFromOneToManyJoinedInheritanceCollectionDoesNotInitializeIt()
|
||||
{
|
||||
/* @var $otherClass DDC2504OtherClass */
|
||||
$otherClass = $this->_em->find(DDC2504OtherClass::CLASSNAME, $this->ddc2504OtherClassId);
|
||||
$childClass = new DDC2504ChildClass();
|
||||
|
||||
@ -1035,7 +1052,7 @@ class ExtraLazyCollectionTest extends OrmFunctionalTestCase
|
||||
/**
|
||||
* @group DDC-3343
|
||||
*/
|
||||
public function testRemovesManagedElementFromOneToManyExtraLazyCollection()
|
||||
public function testRemoveManagedElementFromOneToManyExtraLazyCollectionIsNoOp()
|
||||
{
|
||||
list($userId, $tweetId) = $this->loadTweetFixture();
|
||||
|
||||
@ -1049,22 +1066,21 @@ class ExtraLazyCollectionTest extends OrmFunctionalTestCase
|
||||
/* @var $user User */
|
||||
$user = $this->_em->find(User::CLASSNAME, $userId);
|
||||
|
||||
$this->assertCount(0, $user->tweets);
|
||||
$this->assertCount(1, $user->tweets, 'Element was not removed - need to update the owning side first');
|
||||
}
|
||||
|
||||
/**
|
||||
* @group DDC-3343
|
||||
*/
|
||||
public function testRemovesManagedElementFromOneToManyExtraLazyCollectionWithoutDeletingTheTargetEntityEntry()
|
||||
public function testRemoveManagedElementFromOneToManyExtraLazyCollectionWithoutDeletingTheTargetEntityEntryIsNoOp()
|
||||
{
|
||||
list($userId, $tweetId) = $this->loadTweetFixture();
|
||||
|
||||
/* @var $user User */
|
||||
$user = $this->_em->find(User::CLASSNAME, $userId);
|
||||
$tweet = $this->_em->find(Tweet::CLASSNAME, $tweetId);
|
||||
|
||||
$e = $this->_em->find(Tweet::CLASSNAME, $tweetId);
|
||||
|
||||
$user->tweets->removeElement($e);
|
||||
$user->tweets->removeElement($tweet);
|
||||
|
||||
$this->_em->clear();
|
||||
|
||||
@ -1076,7 +1092,11 @@ class ExtraLazyCollectionTest extends OrmFunctionalTestCase
|
||||
'Even though the collection is extra lazy, the tweet should not have been deleted'
|
||||
);
|
||||
|
||||
$this->assertNull($tweet->author, 'Tweet author link has been removed');
|
||||
$this->assertInstanceOf(
|
||||
User::CLASSNAME,
|
||||
$tweet->author,
|
||||
'Tweet author link has not been removed - need to update the owning side first'
|
||||
);
|
||||
}
|
||||
|
||||
/**
|
||||
@ -1102,12 +1122,89 @@ class ExtraLazyCollectionTest extends OrmFunctionalTestCase
|
||||
'Even though the collection is extra lazy, the tweet should not have been deleted'
|
||||
);
|
||||
|
||||
$this->assertNull($tweet->author);
|
||||
$this->assertInstanceOf(User::CLASSNAME, $tweet->author);
|
||||
|
||||
/* @var $user User */
|
||||
$user = $this->_em->find(User::CLASSNAME, $userId);
|
||||
|
||||
$this->assertCount(0, $user->tweets);
|
||||
$this->assertCount(1, $user->tweets, 'Element was not removed - need to update the owning side first');
|
||||
}
|
||||
|
||||
/**
|
||||
* @group DDC-3343
|
||||
*/
|
||||
public function testRemoveOrphanedManagedElementFromOneToManyExtraLazyCollection()
|
||||
{
|
||||
list($userId, $userListId) = $this->loadUserListFixture();
|
||||
|
||||
/* @var $user User */
|
||||
$user = $this->_em->find(User::CLASSNAME, $userId);
|
||||
|
||||
$user->userLists->removeElement($this->_em->find(UserList::CLASSNAME, $userListId));
|
||||
|
||||
$this->_em->clear();
|
||||
|
||||
/* @var $user User */
|
||||
$user = $this->_em->find(User::CLASSNAME, $userId);
|
||||
|
||||
$this->assertCount(0, $user->userLists, 'Element was removed from association due to orphan removal');
|
||||
$this->assertNull(
|
||||
$this->_em->find(UserList::CLASSNAME, $userListId),
|
||||
'Element was deleted due to orphan removal'
|
||||
);
|
||||
}
|
||||
|
||||
/**
|
||||
* @group DDC-3343
|
||||
*/
|
||||
public function testRemoveOrphanedUnManagedElementFromOneToManyExtraLazyCollection()
|
||||
{
|
||||
list($userId, $userListId) = $this->loadUserListFixture();
|
||||
|
||||
/* @var $user User */
|
||||
$user = $this->_em->find(User::CLASSNAME, $userId);
|
||||
|
||||
$user->userLists->removeElement(new UserList());
|
||||
|
||||
$this->_em->clear();
|
||||
|
||||
/* @var $userList UserList */
|
||||
$userList = $this->_em->find(UserList::CLASSNAME, $userListId);
|
||||
$this->assertInstanceOf(
|
||||
UserList::CLASSNAME,
|
||||
$userList,
|
||||
'Even though the collection is extra lazy + orphan removal, the user list should not have been deleted'
|
||||
);
|
||||
|
||||
$this->assertInstanceOf(
|
||||
User::CLASSNAME,
|
||||
$userList->owner,
|
||||
'User list to owner link has not been removed'
|
||||
);
|
||||
}
|
||||
|
||||
/**
|
||||
* @group DDC-3343
|
||||
*/
|
||||
public function testRemoveOrphanedManagedLazyProxyFromExtraLazyOneToMany()
|
||||
{
|
||||
list($userId, $userListId) = $this->loadUserListFixture();
|
||||
|
||||
/* @var $user User */
|
||||
$user = $this->_em->find(User::CLASSNAME, $userId);
|
||||
|
||||
$user->userLists->removeElement($this->_em->getReference(UserList::CLASSNAME, $userListId));
|
||||
|
||||
$this->_em->clear();
|
||||
|
||||
/* @var $user User */
|
||||
$user = $this->_em->find(User::CLASSNAME, $userId);
|
||||
|
||||
$this->assertCount(0, $user->userLists, 'Element was removed from association due to orphan removal');
|
||||
$this->assertNull(
|
||||
$this->_em->find(UserList::CLASSNAME, $userListId),
|
||||
'Element was deleted due to orphan removal'
|
||||
);
|
||||
}
|
||||
|
||||
/**
|
||||
@ -1130,4 +1227,25 @@ class ExtraLazyCollectionTest extends OrmFunctionalTestCase
|
||||
|
||||
return array($user->id, $tweet->id);
|
||||
}
|
||||
|
||||
/**
|
||||
* @return int[] ordered tuple: user id and user list id
|
||||
*/
|
||||
private function loadUserListFixture()
|
||||
{
|
||||
$user = new User();
|
||||
$userList = new UserList();
|
||||
|
||||
$user->name = 'ocramius';
|
||||
$userList->listName = 'PHP Developers to follow closely';
|
||||
|
||||
$user->addUserList($userList);
|
||||
|
||||
$this->_em->persist($user);
|
||||
$this->_em->persist($userList);
|
||||
$this->_em->flush();
|
||||
$this->_em->clear();
|
||||
|
||||
return array($user->id, $userList->id);
|
||||
}
|
||||
}
|
||||
|
@ -296,10 +296,12 @@ class SecondLevelCacheTest extends SecondLevelCacheAbstractTest
|
||||
|
||||
$this->_em->clear();
|
||||
|
||||
$this->assertTrue($this->cache->containsEntity(Country::CLASSNAME, $countryId));
|
||||
$this->assertFalse(
|
||||
$this->cache->containsEntity(Country::CLASSNAME, $countryId),
|
||||
'Removal attempts should clear the cache entry corresponding to the entity'
|
||||
);
|
||||
|
||||
$country = $this->_em->find(Country::CLASSNAME, $countryId);
|
||||
$this->assertInstanceOf(Country::CLASSNAME, $country);
|
||||
$this->assertInstanceOf(Country::CLASSNAME, $this->_em->find(Country::CLASSNAME, $countryId));
|
||||
}
|
||||
|
||||
public function testCachedNewEntityExists()
|
||||
|
@ -186,7 +186,8 @@ abstract class OrmFunctionalTestCase extends OrmTestCase
|
||||
),
|
||||
'tweet' => array(
|
||||
'Doctrine\Tests\Models\Tweet\User',
|
||||
'Doctrine\Tests\Models\Tweet\Tweet'
|
||||
'Doctrine\Tests\Models\Tweet\Tweet',
|
||||
'Doctrine\Tests\Models\Tweet\UserList',
|
||||
),
|
||||
'ddc2504' => array(
|
||||
'Doctrine\Tests\Models\DDC2504\DDC2504RootClass',
|
||||
@ -387,6 +388,12 @@ abstract class OrmFunctionalTestCase extends OrmTestCase
|
||||
$conn->executeUpdate('DELETE FROM taxi_driver');
|
||||
}
|
||||
|
||||
if (isset($this->_usedModelSets['tweet'])) {
|
||||
$conn->executeUpdate('DELETE FROM tweet_tweet');
|
||||
$conn->executeUpdate('DELETE FROM tweet_user_list');
|
||||
$conn->executeUpdate('DELETE FROM tweet_user');
|
||||
}
|
||||
|
||||
if (isset($this->_usedModelSets['cache'])) {
|
||||
$conn->executeUpdate('DELETE FROM cache_attraction_location_info');
|
||||
$conn->executeUpdate('DELETE FROM cache_attraction_contact_info');
|
||||
|
Loading…
x
Reference in New Issue
Block a user