Merge pull request #1282 from Ocramius/hotfix/#1169-extra-lazy-one-to-many-should-not-delete-referenced-entities-2.4

Hotfix/#1169 extra lazy one to many should not delete referenced entities (backport to 2.4)

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 @@ class OneToManyPersister extends AbstractCollectionPersister
             return false;
         }
 
-        $mapping = $coll->getMapping();
+        $mapping        = $coll->getMapping();
-        $class   = $this->em->getClassMetadata($mapping['targetEntity']);
+        $persister      = $this->uow->getEntityPersister($mapping['targetEntity']);
-        $sql     = 'DELETE FROM ' . $this->quoteStrategy->getTableName($class, $this->platform)
+        $targetMetadata = $this->em->getClassMetadata($mapping['targetEntity']);
-                 . ' WHERE ' . implode('= ? AND ', $class->getIdentifierColumnNames()) . ' = ?';
 
-        return (bool) $this->conn->executeUpdate($sql, $this->getDeleteRowSQLParameters($coll, $element));
+        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;
     }
 }

tests/Doctrine/Tests/Models/Tweet/Tweet.php

@@ -0,0 +1,34 @@
+<?php
+
+namespace Doctrine\Tests\Models\Tweet;
+
+/**
+ * @Entity
+ * @Table(name="tweet_tweet")
+ */
+class Tweet
+{
+    const CLASSNAME = __CLASS__;
+
+    /**
+     * @Id
+     * @GeneratedValue
+     * @Column(type="integer")
+     */
+    public $id;
+
+    /**
+     * @Column(type="string")
+     */
+    public $content;
+
+    /**
+     * @ManyToOne(targetEntity="User", inversedBy="tweets")
+     */
+    public $author;
+
+    public function setAuthor(User $user)
+    {
+        $this->author = $user;
+    }
+}

tests/Doctrine/Tests/Models/Tweet/User.php

@@ -0,0 +1,42 @@
+<?php
+
+namespace Doctrine\Tests\Models\Tweet;
+
+use Doctrine\Common\Collections\ArrayCollection;
+
+/**
+ * @Entity
+ * @Table(name="tweet_user")
+ */
+class User
+{
+    const CLASSNAME = __CLASS__;
+
+    /**
+     * @Id
+     * @GeneratedValue
+     * @Column(type="integer")
+     */
+    public $id;
+
+    /**
+     * @Column(type="string")
+     */
+    public $name;
+
+    /**
+     * @OneToMany(targetEntity="Tweet", mappedBy="author", cascade={"persist"}, fetch="EXTRA_LAZY")
+     */
+    public $tweets;
+
+    public function __construct()
+    {
+        $this->tweets = new ArrayCollection();
+    }
+
+    public function addTweet(Tweet $tweet)
+    {
+        $tweet->setAuthor($this);
+        $this->tweets->add($tweet);
+    }
+}

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 @@ 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();
@@ -384,7 +388,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
@@ -650,4 +654,101 @@ class ExtraLazyCollectionTest extends \Doctrine\Tests\OrmFunctionalTestCase
         $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);
+    }
 }

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 @@ abstract class OrmFunctionalTestCase extends OrmTestCase
             $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');