8000 Merge pull request #1282 from Ocramius/hotfix/#1169-extra-lazy-one-to… · symfonyaml/orm@97afe00 · GitHub
[go: up one dir, main page]
Skip to content

Commit 97afe00

Browse files
OcramiusOcramius
committed
Merge pull request doctrine#1282 from Ocramius/hotfix/doctrine#1169-extra-lazy-one-to-many-should-not-delete-referenced-entities-2.4
Hotfix/doctrine#1169 extra lazy one to many should not delete referenced entities (backport to 2.4)
2 parents ba04c98 + 51250e9 commit 97afe00

File tree

5 files changed

+202
-7
lines changed

5 files changed

+202
-7
lines changed

‎lib/Doctrine/ORM/Persisters/OneToManyPersister.php

Lines changed: 15 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919

2020
namespace Doctrine\ORM\Persisters;
2121

22+
use Doctrine\Common\Proxy\Proxy;
2223
use Doctrine\ORM\PersistentCollection;
2324
use Doctrine\ORM\UnitOfWork;
2425

@@ -237,11 +238,20 @@ public function removeElement(PersistentCollection $coll, $element)
237238
return false;
238239
}
239240

240-
$mapping = $coll->getMapping();
241-
$class = $this->em->getClassMetadata($mapping['targetEntity']);
242-
$sql = 'DELETE FROM ' . $this->quoteStrategy->getTableName($class, $this->platform)
243-
. ' WHERE ' . implode('= ? AND ', $class->getIdentifierColumnNames()) . ' = ?';
241+
$mapping = $coll->getMapping();
242+
$persister = $this->uow->getEntityPersister($mapping['targetEntity']);
243+
$targetMetadata = $this->em->getClassMetadata($mapping['targetEntity']);
244+
245+
if ($element instanceof Proxy && ! $element->__isInitialized()) {
246+
$element->__load();
247+
}
248+
249+
// clearing owning side value
250+
$targetMetadata->reflFields[$mapping['mappedBy']]->setValue($element, null);
251+
252+
$this->uow->computeChangeSet($targetMetadata, $element);
253+
$persister->update($element);
244254

245-
return (bool) $this->conn->executeUpdate($sql, $this->getDeleteRowSQLParameters($coll, $element));
255+
return true;
246256
}
247257
}

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

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
<?php
2+
3+
namespace Doctrine\Tests\Models\Tweet;
4+
5+
/**
6+
* @Entity
7+
* @Table(name="tweet_tweet")
8+
*/
9+
class Tweet
10+
{
11+
const CLASSNAME = __CLASS__;
12+
13+
/**
14+
* @Id
15+
* @GeneratedValue
16+
* @Column(type="integer")
17+
*/
18+
public $id;
19+
20+
/**
21+
* @Column(type="string")
22+
*/
23+
public $content;
24+
25+
/**
26+
* @ManyToOne(targetEntity="User", inversedBy="tweets")
27+
*/
28+
public $author;
29+
30+
public function setAuthor(User $user)
31+
{
32+
$this->author = $user;
33+
}
34+
}

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

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
<?php
2+
3+
namespace Doctrine\Tests\Models\Tweet;
4+
5+
use Doctrine\Common\Collections\ArrayCollection;
6+
7+
/**
8+
* @Entity
9+
* @Table(name="tweet_user")
10+
*/
11+
class User
12+
{
13+
const CLASSNAME = __CLASS__;
14+
15+
/**
16+
* @Id
17+
* @GeneratedValue
18+
* @Column(type="integer")
19+
*/
20+
public $id;
21+
22+
/**
23+
* @Column(type="string")
24+
*/
25+
public $name;
26+
27+
/**
28+
* @OneToMany(targetEntity="Tweet", mappedBy="author", cascade={"persist"}, fetch="EXTRA_LAZY")
29+
*/
30+
public $tweets;
31+
32+
public function __construct()
33+
{
34+
$this->tweets = new ArrayCollection();
35+
}
36+
37+
public function addTweet(Tweet $tweet)
38+
{
39+
$tweet->setAuthor($this);
40+
$this->tweets->add($tweet);
41+
}
42+
}

‎tests/Doctrine/Tests/ORM/Functional/ExtraLazyCollectionTest.php

Lines changed: 103 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,8 @@
33
namespace Doctrine\Tests\ORM\Functional;
44

55
use Doctrine\ORM\Mapping\ClassMetadataInfo;
6+
use Doctrine\Tests\Models\Tweet\Tweet;
7+
use Doctrine\Tests\Models\Tweet\User;
68

79
require_once __DIR__ . '/../../TestInit.php';
810

@@ -22,6 +24,7 @@ class ExtraLazyCollectionTest extends \Doctrine\Tests\OrmFunctionalTestCase
2224

2325
public function setUp()
2426
{
27+
$this->useModelSet('tweet');
2528
$this->useModelSet('cms');
2629
parent::setUp();
2730

@@ -363,7 +366,8 @@ public function testRemoveElementOneToMany()
363366
$user->articles->removeElement($article);
364367

365368
$this->assertFalse($user->articles->isInitialized(), "Post-Condition: Collection is not initialized.");
366-
$this->assertEquals($queryCount + 1, $this->getCurrentQueryCount());
369+
// NOTE: +2 queries because CmsArticle is a versioned entity, and that needs to be handled accordingly
370+
$this->assertEquals($queryCount + 2, $this->getCurrentQueryCount());
367371

368372
// Test One to Many removal with Entity state as new
369373
$article = new \Doctrine\Tests\Models\CMS\CmsArticle();
@@ -384,7 +388,7 @@ public function testRemoveElementOneToMany()
384388

385389
$user->articles->removeElement($article);
386390

387-
$this->assertEquals($queryCount + 1, $this->getCurrentQueryCount(), "Removing a persisted entity should cause one query to be executed.");
391+
$this->assertEquals($queryCount, $this->getCurrentQueryCount(), "Removing a persisted entity will not cause queries when the owning side doesn't actually change.");
388392
$this->assertFalse($user->articles->isInitialized(), "Post-Condition: Collection is not initialized.");
389393

390394
// Test One to Many removal with Entity state as managed
@@ -650,4 +654,101 @@ private function loadFixture()
650654
$this->topic = $article1->topic;
651655
$this->phonenumber = $phonenumber1->phonenumber;
652656
}
657+
658+
/**
659+
* @group DDC-3343
660+
*/
661+
public function testRemovesManagedElementFromOneToManyExtraLazyCollection()
662+
{
663+
list($userId, $tweetId) = $this->loadTweetFixture();
664+
665+
/* @var $user User */
666+
$user = $this->_em->find(User::CLASSNAME, $userId);
667+
668+
$user->tweets->removeElement($this->_em->find(Tweet::CLASSNAME, $tweetId));
669+
670+
$this->_em->clear();
671+
672+
/* @var $user User */
673+
$user = $this->_em->find(User::CLASSNAME, $userId);
674+
675+
$this->assertCount(0, $user->tweets);
676+
}
677+
678+
/**
679+
* @group DDC-3343
680+
*/
681+
public function testRemovesManagedElementFromOneToManyExtraLazyCollectionWithoutDeletingTheTargetEntityEntry()
682+
{
683+
list($userId, $tweetId) = $this->loadTweetFixture();
684+
685+
/* @var $user User */
686+
$user = $this->_em->find(User::CLASSNAME, $userId);
687+
688+
$user->tweets->removeElement($this->_em->find(Tweet::CLASSNAME, $tweetId));
689+
690+
$this->_em->clear();
691+
692+
/* @var $tweet Tweet */
693+
$tweet = $this->_em->find(Tweet::CLASSNAME, $tweetId);
694+
$this->assertInstanceOf(
695+
Tweet::CLASSNAME,
696+
$tweet,
697+
'Even though the collection is extra lazy, the tweet should not have been deleted'
698+
);
699+
700+
$this->assertNull($tweet->author, 'Tweet author link has been removed');
701+
}
702+
703+
/**
704+
* @group DDC-3343
705+
*/
706+
public function testRemovingManagedLazyProxyFromExtraLazyOneToManyDoesRemoveTheAssociationButNotTheEntity()
707+
{
708+
list($userId, $tweetId) = $this->loadTweetFixture();
709+
710+
/* @var $user User */
711+
$user = $this->_em->find(User::CLASSNAME, $userId);
712+
$tweet = $this->_em->getReference(Tweet::CLASSNAME, $tweetId);
713+
714+
$user->tweets->removeElement($this->_em->getReference(Tweet::CLASSNAME, $tweetId));
715+
716+
$this->_em->clear();
717+
718+
/* @var $tweet Tweet */
719+
$tweet = $this->_em->find(Tweet::CLASSNAME, $tweet->id);
720+
$this->assertInstanceOf(
721+
Tweet::CLASSNAME,
722+
$tweet,
723+
'Even though the collection is extra lazy, the tweet should not have been deleted'
724+
);
725+
726+
$this->assertNull($tweet->author);
727+
728+
/* @var $user User */
729+
$user = $this->_em->find(User::CLASSNAME, $userId);
730+
731+
$this->assertCount(0, $user->tweets);
732+
}
733+
734+
/**
735+
* @return int[] ordered tuple: user id and tweet id
736+
*/
737+
private function loadTweetFixture()
738+
{
739+
$user = new User();
740+
$tweet = new Tweet();
741+
742+
$user->name = 'ocramius';
743+
$tweet->content = 'The cat is on the table';
744+
745+
$user->addTweet($tweet);
746+
747+
$this->_em->persist($user);
748+
$this->_em->persist($tweet);
749+
$this->_em->flush();
750+
$this->_em->clear();
751+
752+
return array($user->id, $tweet->id);
753+
}
653754
}

‎tests/Doctrine/Tests/OrmFunctionalTestCase.php

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -139,6 +139,10 @@ abstract class OrmFunctionalTestCase extends OrmTestCase
139139
'Doctrine\Tests\Models\StockExchange\Stock',
140140
'Doctrine\Tests\Models\StockExchange\Market',
141141
),
142+
'tweet' => array(
143+
'Doctrine\Tests\Models\Tweet\Tweet',
144+
'Doctrine\Tests\Models\Tweet\User',
145+
),
142146
'legacy' => array(
143147
'Doctrine\Tests\Models\Legacy\LegacyUser',
144148
'Doctrine\Tests\Models\Legacy\LegacyUserReference',
@@ -269,6 +273,10 @@ protected function tearDown()
269273
$conn->executeUpdate('DELETE FROM exchange_stocks');
270274
$conn->executeUpdate('DELETE FROM exchange_markets');
271275
}
276+
if (isset($this->_usedModelSets['tweet'])) {
277+
$conn->executeUpdate('DELETE FROM tweet_tweet');
278+
$conn->executeUpdate('DELETE FROM tweet_user');
279+
}
272280
if (isset($this->_usedModelSets['legacy'])) {
273281
$conn->executeUpdate('DELETE FROM legacy_users_cars');
274282
$conn->executeUpdate('DELETE FROM legacy_users_reference');

0 commit comments

Comments
 (0)
0