diff --git a/lib/Doctrine/ORM/Persisters/OneToManyPersister.php b/lib/Doctrine/ORM/Persisters/OneToManyPersister.php index 2915fd2ed89..cf39436d485 100644 --- a/lib/Doctrine/ORM/Persisters/OneToManyPersister.php +++ b/lib/Doctrine/ORM/Persisters/OneToManyPersister.php @@ -19,6 +19,7 @@ namespace Doctrine\ORM\Persisters; +use Doctrine\Common\Proxy\Proxy; use Doctrine\ORM\PersistentCollection; use Doctrine\ORM\UnitOfWork; @@ -237,11 +238,20 @@ public function removeElement(PersistentCollection $coll, $element) return false; } - $mapping = $coll->getMapping(); - $class = $this->em->getClassMetadata($mapping['targetEntity']); - $sql = 'DELETE FROM ' . $this->quoteStrategy->getTableName($class, $this->platform) - . ' WHERE ' . implode('= ? AND ', $class->getIdentifierColumnNames()) . ' = ?'; + $mapping = $coll->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 (bool) $this->conn->executeUpdate($sql, $this->getDeleteRowSQLParameters($coll, $element)); + return true; } } diff --git a/tests/Doctrine/Tests/Models/Tweet/Tweet.php b/tests/Doctrine/Tests/Models/Tweet/Tweet.php new file mode 100644 index 00000000000..8a315df9ef0 --- /dev/null +++ b/tests/Doctrine/Tests/Models/Tweet/Tweet.php @@ -0,0 +1,34 @@ +author = $user; + } +} diff --git a/tests/Doctrine/Tests/Models/Tweet/User.php b/tests/Doctrine/Tests/Models/Tweet/User.php new file mode 100644 index 00000000000..4722e63176b --- /dev/null +++ b/tests/Doctrine/Tests/Models/Tweet/User.php @@ -0,0 +1,42 @@ +tweets = new ArrayCollection(); + } + + public function addTweet(Tweet $tweet) + { + $tweet->setAuthor($this); + $this->tweets->add($tweet); + } +} diff --git a/tests/Doctrine/Tests/ORM/Functional/ExtraLazyCollectionTest.php b/tests/Doctrine/Tests/ORM/Functional/ExtraLazyCollectionTest.php index eb4dbe4d877..42d8020d335 100644 --- a/tests/Doctrine/Tests/ORM/Functional/ExtraLazyCollectionTest.php +++ b/tests/Doctrine/Tests/ORM/Functional/ExtraLazyCollectionTest.php @@ -3,6 +3,8 @@ namespace Doctrine\Tests\ORM\Functional; use Doctrine\ORM\Mapping\ClassMetadataInfo; +use Doctrine\Tests\Models\Tweet\Tweet; +use Doctrine\Tests\Models\Tweet\User; require_once __DIR__ . '/../../TestInit.php'; @@ -22,6 +24,7 @@ class ExtraLazyCollectionTest extends \Doctrine\Tests\OrmFunctionalTestCase public function setUp() { + $this->useModelSet('tweet'); $this->useModelSet('cms'); parent::setUp(); @@ -363,7 +366,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(); @@ -384,7 +388,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 @@ -650,4 +654,101 @@ private function loadFixture() $this->topic = $article1->topic; $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); + + $user->tweets->removeElement($this->_em->find(Tweet::CLASSNAME, $tweetId)); + + $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/OrmFunctionalTestCase.php b/tests/Doctrine/Tests/OrmFunctionalTestCase.php index d2a41cfb968..ff0f2cb531d 100644 --- a/tests/Doctrine/Tests/OrmFunctionalTestCase.php +++ b/tests/Doctrine/Tests/OrmFunctionalTestCase.php @@ -139,6 +139,10 @@ abstract class OrmFunctionalTestCase extends OrmTestCase 'Doctrine\Tests\Models\StockExchange\Stock', 'Doctrine\Tests\Models\StockExchange\Market', ), + 'tweet' => array( + 'Doctrine\Tests\Models\Tweet\Tweet', + 'Doctrine\Tests\Models\Tweet\User', + ), 'legacy' => array( 'Doctrine\Tests\Models\Legacy\LegacyUser', 'Doctrine\Tests\Models\Legacy\LegacyUserReference', @@ -269,6 +273,10 @@ protected function tearDown() $conn->executeUpdate('DELETE FROM exchange_stocks'); $conn->executeUpdate('DELETE FROM exchange_markets'); } + if (isset($this->_usedModelSets['tweet'])) { + $conn->executeUpdate('DELETE FROM tweet_tweet'); + $conn->executeUpdate('DELETE FROM tweet_user'); + } if (isset($this->_usedModelSets['legacy'])) { $conn->executeUpdate('DELETE FROM legacy_users_cars'); $conn->executeUpdate('DELETE FROM legacy_users_reference');