From db3697433bfd7e201a7b416ffd05a90441e26793 Mon Sep 17 00:00:00 2001 From: Andrea Sprega Date: Fri, 24 Oct 2014 23:10:37 +0200 Subject: [PATCH 01/16] [DDC-3343] Failing test case --- .../ORM/Functional/Ticket/DDC3343Test.php | 92 +++++++++++++++++++ 1 file changed, 92 insertions(+) create mode 100644 tests/Doctrine/Tests/ORM/Functional/Ticket/DDC3343Test.php diff --git a/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC3343Test.php b/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC3343Test.php new file mode 100644 index 00000000000..c8a1f73592a --- /dev/null +++ b/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC3343Test.php @@ -0,0 +1,92 @@ +_schemaTool->createSchema(array( + $this->_em->getClassMetadata(__NAMESPACE__ . '\DDC3343User'), + $this->_em->getClassMetadata(__NAMESPACE__ . '\DDC3343Group'), + )); + + // Save a group and an associated user (in an extra lazy association) + $group = new DDC3343Group(); + $user = new DDC3343User(); + + $group->users->add($user); + + $this->_em->persist($group); + $this->_em->persist($user); + $this->_em->flush(); + + // Fetch the group and the user again and remove the user from the collection. + $this->_em->clear(); + + $group = $this->_em->find(__NAMESPACE__ . '\DDC3343Group', $group->id); + $user = $this->_em->find(__NAMESPACE__ . '\DDC3343User', $user->id); + + $group->users->removeElement($user); + + $this->_em->persist($group); + $this->_em->flush(); + + // Even if the collection is extra lazy, the user should not have been deleted. + $this->_em->clear(); + + $user = $this->_em->find(__NAMESPACE__ . '\DDC3343User', $user->id); + $this->assertNotNull($user); + } +} + +/** + * @Entity + */ +class DDC3343User +{ + /** + * @Id + * @GeneratedValue + * @Column(type="integer") + */ + public $id; + + /** + * @ManyToOne(targetEntity="DDC3343Group", inversedBy="users") + */ + protected $group; +} + +/** + * @Entity + */ +class DDC3343Group +{ + /** + * @Id + * @GeneratedValue + * @Column(type="integer") + */ + public $id; + + /** + * @OneToMany(targetEntity="DDC3343User", mappedBy="group", fetch="EXTRA_LAZY") + */ + public $users; + + public function __construct() + { + $this->users = new ArrayCollection(); + } +} From c2b3348f996eafa5de4ebd8d4bf36e01dbe34c5d Mon Sep 17 00:00:00 2001 From: Andrea Sprega Date: Sat, 25 Oct 2014 00:02:57 +0200 Subject: [PATCH 02/16] [DDC-3343] Failing test case (updated) --- tests/Doctrine/Tests/ORM/Functional/Ticket/DDC3343Test.php | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC3343Test.php b/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC3343Test.php index c8a1f73592a..c6db00bef6a 100644 --- a/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC3343Test.php +++ b/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC3343Test.php @@ -39,14 +39,11 @@ public function testEntityNotDeletedWhenRemovedFromExtraLazyAssociation() $group->users->removeElement($user); - $this->_em->persist($group); - $this->_em->flush(); - - // Even if the collection is extra lazy, the user should not have been deleted. + // Even though the collection is extra lazy, the user should not have been deleted. $this->_em->clear(); $user = $this->_em->find(__NAMESPACE__ . '\DDC3343User', $user->id); - $this->assertNotNull($user); + $this->assertInstanceOf(__NAMESPACE__ . '\DDC3343User', $user); } } From f184956d3ae3b028386760704cda1a0fe80a4dc5 Mon Sep 17 00:00:00 2001 From: Marco Pivetta Date: Sat, 24 Jan 2015 11:40:48 +0100 Subject: [PATCH 03/16] #1169 DDC-3343 - one-to-many extra-lazy should not delete associated values when they are removed, but just update the owning side --- .../ORM/Persisters/Collection/OneToManyPersister.php | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/lib/Doctrine/ORM/Persisters/Collection/OneToManyPersister.php b/lib/Doctrine/ORM/Persisters/Collection/OneToManyPersister.php index 9b14afde59b..6687e5c5c9c 100644 --- a/lib/Doctrine/ORM/Persisters/Collection/OneToManyPersister.php +++ b/lib/Doctrine/ORM/Persisters/Collection/OneToManyPersister.php @@ -163,7 +163,14 @@ 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']); + + // clearing owning side value + $targetMetadata->reflFields[$mapping['mappedBy']]->setValue($element, null); + + $persister->update($element); + + return true; } /** From aed01ea57173f813ac742cead56641664e46f0b5 Mon Sep 17 00:00:00 2001 From: Marco Pivetta Date: Sat, 24 Jan 2015 11:41:20 +0100 Subject: [PATCH 04/16] #1169 DDC-3343 - minor refactoring: constant over string reference --- .../ORM/Functional/Ticket/DDC3343Test.php | 21 ++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-) diff --git a/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC3343Test.php b/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC3343Test.php index c6db00bef6a..4537c589fdc 100644 --- a/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC3343Test.php +++ b/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC3343Test.php @@ -17,8 +17,8 @@ class DDC3343Test extends \Doctrine\Tests\OrmFunctionalTestCase public function testEntityNotDeletedWhenRemovedFromExtraLazyAssociation() { $this->_schemaTool->createSchema(array( - $this->_em->getClassMetadata(__NAMESPACE__ . '\DDC3343User'), - $this->_em->getClassMetadata(__NAMESPACE__ . '\DDC3343Group'), + $this->_em->getClassMetadata(DDC3343User::CLASSNAME), + $this->_em->getClassMetadata(DDC3343Group::CLASSNAME), )); // Save a group and an associated user (in an extra lazy association) @@ -34,16 +34,19 @@ public function testEntityNotDeletedWhenRemovedFromExtraLazyAssociation() // Fetch the group and the user again and remove the user from the collection. $this->_em->clear(); - $group = $this->_em->find(__NAMESPACE__ . '\DDC3343Group', $group->id); - $user = $this->_em->find(__NAMESPACE__ . '\DDC3343User', $user->id); + $group = $this->_em->find(DDC3343Group::CLASSNAME, $group->id); + $user = $this->_em->find(DDC3343User::CLASSNAME, $user->id); $group->users->removeElement($user); // Even though the collection is extra lazy, the user should not have been deleted. $this->_em->clear(); - $user = $this->_em->find(__NAMESPACE__ . '\DDC3343User', $user->id); - $this->assertInstanceOf(__NAMESPACE__ . '\DDC3343User', $user); + /* @var $user DDC3343User */ + $user = $this->_em->find(DDC3343User::CLASSNAME, $user->id); + $this->assertInstanceOf(DDC3343User::CLASSNAME, $user); + + $this->assertNull($user->group); } } @@ -52,6 +55,8 @@ public function testEntityNotDeletedWhenRemovedFromExtraLazyAssociation() */ class DDC3343User { + const CLASSNAME = __CLASS__; + /** * @Id * @GeneratedValue @@ -62,7 +67,7 @@ class DDC3343User /** * @ManyToOne(targetEntity="DDC3343Group", inversedBy="users") */ - protected $group; + public $group; } /** @@ -70,6 +75,8 @@ class DDC3343User */ class DDC3343Group { + const CLASSNAME = __CLASS__; + /** * @Id * @GeneratedValue From 15397bbe4058612e2ad65b3e7f74999b16f41418 Mon Sep 17 00:00:00 2001 From: Marco Pivetta Date: Sat, 24 Jan 2015 11:54:03 +0100 Subject: [PATCH 05/16] #1169 DDC-3343 - refactoring test to use pre-existing test models --- lib/Doctrine/ORM/Persisters/Collection/OneToManyPersister.php | 2 ++ tests/Doctrine/Tests/Models/Tweet/Tweet.php | 2 ++ tests/Doctrine/Tests/Models/Tweet/User.php | 2 ++ 3 files changed, 6 insertions(+) diff --git a/lib/Doctrine/ORM/Persisters/Collection/OneToManyPersister.php b/lib/Doctrine/ORM/Persisters/Collection/OneToManyPersister.php index 6687e5c5c9c..fb376033c31 100644 --- a/lib/Doctrine/ORM/Persisters/Collection/OneToManyPersister.php +++ b/lib/Doctrine/ORM/Persisters/Collection/OneToManyPersister.php @@ -168,6 +168,8 @@ public function removeElement(PersistentCollection $collection, $element) // 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 From 43f301fdad13253b40e24067c4c1aade9aaa8a16 Mon Sep 17 00:00:00 2001 From: Marco Pivetta Date: Sat, 24 Jan 2015 11:54:17 +0100 Subject: [PATCH 06/16] #1169 DDC-3343 - refactoring test to use pre-existing test models --- .../ORM/Functional/Ticket/DDC3343Test.php | 96 +++++++------------ 1 file changed, 32 insertions(+), 64 deletions(-) diff --git a/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC3343Test.php b/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC3343Test.php index 4537c589fdc..9257883fd88 100644 --- a/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC3343Test.php +++ b/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC3343Test.php @@ -8,89 +8,57 @@ use Doctrine\ORM\Mapping\Id; use Doctrine\ORM\Mapping\ManyToOne; use Doctrine\ORM\Mapping\OneToMany; +use Doctrine\Tests\Models\Tweet\Tweet; +use Doctrine\Tests\Models\Tweet\User; /** * @group DDC-3343 */ class DDC3343Test extends \Doctrine\Tests\OrmFunctionalTestCase { + /** + * {@inheritDoc} + */ + protected function setUp() + { + $this->useModelSet('tweet'); + + parent::setUp(); + } + public function testEntityNotDeletedWhenRemovedFromExtraLazyAssociation() { - $this->_schemaTool->createSchema(array( - $this->_em->getClassMetadata(DDC3343User::CLASSNAME), - $this->_em->getClassMetadata(DDC3343Group::CLASSNAME), - )); + $user = new User(); + $tweet = new Tweet(); - // Save a group and an associated user (in an extra lazy association) - $group = new DDC3343Group(); - $user = new DDC3343User(); + $user->name = 'ocramius'; + $tweet->content = 'The cat is on the table'; - $group->users->add($user); + $user->addTweet($tweet); - $this->_em->persist($group); $this->_em->persist($user); + $this->_em->persist($tweet); $this->_em->flush(); - - // Fetch the group and the user again and remove the user from the collection. $this->_em->clear(); - $group = $this->_em->find(DDC3343Group::CLASSNAME, $group->id); - $user = $this->_em->find(DDC3343User::CLASSNAME, $user->id); + /* @var $user User */ + $user = $this->_em->find(User::CLASSNAME, $user->id); + $tweet = $this->_em->find(Tweet::CLASSNAME, $tweet->id); - $group->users->removeElement($user); - - // Even though the collection is extra lazy, the user should not have been deleted. - $this->_em->clear(); + $user->tweets->removeElement($tweet); - /* @var $user DDC3343User */ - $user = $this->_em->find(DDC3343User::CLASSNAME, $user->id); - $this->assertInstanceOf(DDC3343User::CLASSNAME, $user); + $this->assertCount(0, $user->tweets); - $this->assertNull($user->group); - } -} - -/** - * @Entity - */ -class DDC3343User -{ - const CLASSNAME = __CLASS__; - - /** - * @Id - * @GeneratedValue - * @Column(type="integer") - */ - public $id; - - /** - * @ManyToOne(targetEntity="DDC3343Group", inversedBy="users") - */ - public $group; -} - -/** - * @Entity - */ -class DDC3343Group -{ - const CLASSNAME = __CLASS__; - - /** - * @Id - * @GeneratedValue - * @Column(type="integer") - */ - public $id; + $this->_em->clear(); - /** - * @OneToMany(targetEntity="DDC3343User", mappedBy="group", fetch="EXTRA_LAZY") - */ - public $users; + /* @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' + ); - public function __construct() - { - $this->users = new ArrayCollection(); + $this->assertNull($tweet->author); } } From ed0331d25a5ab515c0ccacce74cda1a57ebd09a6 Mon Sep 17 00:00:00 2001 From: Marco Pivetta Date: Sat, 24 Jan 2015 11:54:32 +0100 Subject: [PATCH 07/16] #1169 DDC-3343 - optimized imports --- tests/Doctrine/Tests/ORM/Functional/Ticket/DDC3343Test.php | 6 ------ 1 file changed, 6 deletions(-) diff --git a/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC3343Test.php b/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC3343Test.php index 9257883fd88..1831b6bce7e 100644 --- a/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC3343Test.php +++ b/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC3343Test.php @@ -2,12 +2,6 @@ namespace Doctrine\Tests\ORM\Functional\Ticket; -use Doctrine\Common\Collections\ArrayCollection; -use Doctrine\ORM\Mapping\Column; -use Doctrine\ORM\Mapping\GeneratedValue; -use Doctrine\ORM\Mapping\Id; -use Doctrine\ORM\Mapping\ManyToOne; -use Doctrine\ORM\Mapping\OneToMany; use Doctrine\Tests\Models\Tweet\Tweet; use Doctrine\Tests\Models\Tweet\User; From ff986a9bf5dc1e8686a2b2e31789ffc4b91b9ace Mon Sep 17 00:00:00 2001 From: Marco Pivetta Date: Sat, 24 Jan 2015 11:54:58 +0100 Subject: [PATCH 08/16] #1169 DDC-3343 - importing used classe --- tests/Doctrine/Tests/ORM/Functional/Ticket/DDC3343Test.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC3343Test.php b/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC3343Test.php index 1831b6bce7e..1dd75007930 100644 --- a/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC3343Test.php +++ b/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC3343Test.php @@ -4,11 +4,12 @@ use Doctrine\Tests\Models\Tweet\Tweet; use Doctrine\Tests\Models\Tweet\User; +use Doctrine\Tests\OrmFunctionalTestCase; /** * @group DDC-3343 */ -class DDC3343Test extends \Doctrine\Tests\OrmFunctionalTestCase +class DDC3343Test extends OrmFunctionalTestCase { /** * {@inheritDoc} From a8796fa489b6c519c9deba0d956da93da83bd33b Mon Sep 17 00:00:00 2001 From: Marco Pivetta Date: Sat, 24 Jan 2015 11:58:57 +0100 Subject: [PATCH 09/16] #1169 DDC-3343 - integrating tests into the existing test suite --- .../OneToManyExtraLazyTest.php | 43 ++++++++++++++++++- 1 file changed, 42 insertions(+), 1 deletion(-) diff --git a/tests/Doctrine/Tests/ORM/Functional/ValueConversionType/OneToManyExtraLazyTest.php b/tests/Doctrine/Tests/ORM/Functional/ValueConversionType/OneToManyExtraLazyTest.php index bb90067824b..383940f5192 100644 --- a/tests/Doctrine/Tests/ORM/Functional/ValueConversionType/OneToManyExtraLazyTest.php +++ b/tests/Doctrine/Tests/ORM/Functional/ValueConversionType/OneToManyExtraLazyTest.php @@ -2,7 +2,8 @@ namespace Doctrine\Tests\ORM\Functional\ValueConversionType; -use Doctrine\DBAL\Types\Type as DBALType; +use Doctrine\Tests\Models\Tweet\Tweet; +use Doctrine\Tests\Models\Tweet\User; use Doctrine\Tests\Models\ValueConversionType as Entity; use Doctrine\Tests\OrmFunctionalTestCase; @@ -19,6 +20,7 @@ class OneToManyExtraLazyTest extends OrmFunctionalTestCase { public function setUp() { + $this->useModelSet('tweet'); $this->useModelSet('vct_onetomany_extralazy'); parent::setUp(); @@ -103,4 +105,43 @@ public function testThatASliceOfTheExtraLazyCollectionIsLoaded() $this->assertCount(2, $inversed->associatedEntities->slice(0, 2)); } + + /** + * @group DDC-3343 + */ + public function testEntityNotDeletedWhenRemovedFromExtraLazyAssociation() + { + $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(); + + /* @var $user User */ + $user = $this->_em->find(User::CLASSNAME, $user->id); + $tweet = $this->_em->find(Tweet::CLASSNAME, $tweet->id); + + $user->tweets->removeElement($tweet); + + $this->assertCount(0, $user->tweets); + + $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); + } } From 99c5650ba4425386f75d93a201c8e4281394953d Mon Sep 17 00:00:00 2001 From: Marco Pivetta Date: Sat, 24 Jan 2015 11:59:21 +0100 Subject: [PATCH 10/16] #1169 DDC-3343 - removing duplicate test --- .../ORM/Functional/Ticket/DDC3343Test.php | 59 ------------------- 1 file changed, 59 deletions(-) delete mode 100644 tests/Doctrine/Tests/ORM/Functional/Ticket/DDC3343Test.php diff --git a/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC3343Test.php b/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC3343Test.php deleted file mode 100644 index 1dd75007930..00000000000 --- a/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC3343Test.php +++ /dev/null @@ -1,59 +0,0 @@ -useModelSet('tweet'); - - parent::setUp(); - } - - public function testEntityNotDeletedWhenRemovedFromExtraLazyAssociation() - { - $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(); - - /* @var $user User */ - $user = $this->_em->find(User::CLASSNAME, $user->id); - $tweet = $this->_em->find(Tweet::CLASSNAME, $tweet->id); - - $user->tweets->removeElement($tweet); - - $this->assertCount(0, $user->tweets); - - $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); - } -} From 6a2b7c2a8e869cf750d464abf2e83113c61c74fd Mon Sep 17 00:00:00 2001 From: Marco Pivetta Date: Sat, 24 Jan 2015 12:20:33 +0100 Subject: [PATCH 11/16] #1169 DDC-3343 - correcting query count assertions on extra-lazy specific tests (some DELETE operations became UPDATE operations) --- .../Collection/OneToManyPersister.php | 5 ++++ .../Functional/ExtraLazyCollectionTest.php | 27 +++++-------------- 2 files changed, 12 insertions(+), 20 deletions(-) diff --git a/lib/Doctrine/ORM/Persisters/Collection/OneToManyPersister.php b/lib/Doctrine/ORM/Persisters/Collection/OneToManyPersister.php index fb376033c31..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; /** @@ -165,6 +166,10 @@ public function removeElement(PersistentCollection $collection, $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); diff --git a/tests/Doctrine/Tests/ORM/Functional/ExtraLazyCollectionTest.php b/tests/Doctrine/Tests/ORM/Functional/ExtraLazyCollectionTest.php index 2807ccfd7c1..2aa7241a9c7 100644 --- a/tests/Doctrine/Tests/ORM/Functional/ExtraLazyCollectionTest.php +++ b/tests/Doctrine/Tests/ORM/Functional/ExtraLazyCollectionTest.php @@ -480,7 +480,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 +502,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 +533,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 +595,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.'); } From d443d4f3b60acfe7a803354aa34a4309a6e4104d Mon Sep 17 00:00:00 2001 From: Marco Pivetta Date: Sat, 24 Jan 2015 12:21:21 +0100 Subject: [PATCH 12/16] #1169 DDC-3343 - additional test cases: removing proxies from an extra-lazy collection still updates the owning side values --- .../OneToManyExtraLazyTest.php | 84 ++++++++++++++++--- 1 file changed, 71 insertions(+), 13 deletions(-) diff --git a/tests/Doctrine/Tests/ORM/Functional/ValueConversionType/OneToManyExtraLazyTest.php b/tests/Doctrine/Tests/ORM/Functional/ValueConversionType/OneToManyExtraLazyTest.php index 383940f5192..c323a44d8cf 100644 --- a/tests/Doctrine/Tests/ORM/Functional/ValueConversionType/OneToManyExtraLazyTest.php +++ b/tests/Doctrine/Tests/ORM/Functional/ValueConversionType/OneToManyExtraLazyTest.php @@ -109,28 +109,60 @@ public function testThatASliceOfTheExtraLazyCollectionIsLoaded() /** * @group DDC-3343 */ - public function testEntityNotDeletedWhenRemovedFromExtraLazyAssociation() + public function testRemovesManagedElementFromOneToManyExtraLazyCollection() { - $user = new User(); - $tweet = new Tweet(); + list($userId, $tweetId) = $this->loadTweetFixture(); - $user->name = 'ocramius'; - $tweet->content = 'The cat is on the table'; + /* @var $user User */ + $user = $this->_em->find(User::CLASSNAME, $userId); - $user->addTweet($tweet); + $user->tweets->removeElement($this->_em->find(Tweet::CLASSNAME, $tweetId)); - $this->_em->persist($user); - $this->_em->persist($tweet); - $this->_em->flush(); $this->_em->clear(); /* @var $user User */ - $user = $this->_em->find(User::CLASSNAME, $user->id); - $tweet = $this->_em->find(Tweet::CLASSNAME, $tweet->id); - - $user->tweets->removeElement($tweet); + $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(); @@ -143,5 +175,31 @@ public function testEntityNotDeletedWhenRemovedFromExtraLazyAssociation() ); $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); } } From b7b716a6bbb5e5158bbcdb74a291ab13388c6ce1 Mon Sep 17 00:00:00 2001 From: Marco Pivetta Date: Sat, 24 Jan 2015 12:53:48 +0100 Subject: [PATCH 13/16] #1169 DDC-3343 - moved tests to correct test class --- .../Functional/ExtraLazyCollectionTest.php | 103 +++++++++++++++++- .../OneToManyExtraLazyTest.php | 98 ----------------- 2 files changed, 102 insertions(+), 99 deletions(-) diff --git a/tests/Doctrine/Tests/ORM/Functional/ExtraLazyCollectionTest.php b/tests/Doctrine/Tests/ORM/Functional/ExtraLazyCollectionTest.php index 2aa7241a9c7..525021ff32c 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(); @@ -1027,4 +1031,101 @@ 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); + + $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/ORM/Functional/ValueConversionType/OneToManyExtraLazyTest.php b/tests/Doctrine/Tests/ORM/Functional/ValueConversionType/OneToManyExtraLazyTest.php index c323a44d8cf..5d51d4ce174 100644 --- a/tests/Doctrine/Tests/ORM/Functional/ValueConversionType/OneToManyExtraLazyTest.php +++ b/tests/Doctrine/Tests/ORM/Functional/ValueConversionType/OneToManyExtraLazyTest.php @@ -20,7 +20,6 @@ class OneToManyExtraLazyTest extends OrmFunctionalTestCase { public function setUp() { - $this->useModelSet('tweet'); $this->useModelSet('vct_onetomany_extralazy'); parent::setUp(); @@ -105,101 +104,4 @@ public function testThatASliceOfTheExtraLazyCollectionIsLoaded() $this->assertCount(2, $inversed->associatedEntities->slice(0, 2)); } - - /** - * @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); - } } From 01a9dadee7ffe57cecab4282810a745352e782c0 Mon Sep 17 00:00:00 2001 From: Marco Pivetta Date: Sat, 24 Jan 2015 12:54:40 +0100 Subject: [PATCH 14/16] #1169 DDC-3343 - removed unused imports --- .../Functional/ValueConversionType/OneToManyExtraLazyTest.php | 2 -- 1 file changed, 2 deletions(-) diff --git a/tests/Doctrine/Tests/ORM/Functional/ValueConversionType/OneToManyExtraLazyTest.php b/tests/Doctrine/Tests/ORM/Functional/ValueConversionType/OneToManyExtraLazyTest.php index 5d51d4ce174..75b5403fe55 100644 --- a/tests/Doctrine/Tests/ORM/Functional/ValueConversionType/OneToManyExtraLazyTest.php +++ b/tests/Doctrine/Tests/ORM/Functional/ValueConversionType/OneToManyExtraLazyTest.php @@ -2,8 +2,6 @@ namespace Doctrine\Tests\ORM\Functional\ValueConversionType; -use Doctrine\Tests\Models\Tweet\Tweet; -use Doctrine\Tests\Models\Tweet\User; use Doctrine\Tests\Models\ValueConversionType as Entity; use Doctrine\Tests\OrmFunctionalTestCase; From 7e85c94f4812e5f40fa1068b62fb2bad49d69b3b Mon Sep 17 00:00:00 2001 From: Marco Pivetta Date: Sun, 25 Jan 2015 04:40:30 +0100 Subject: [PATCH 15/16] #1169 DDC-3343 - adapting cached collection persister logic to EXTRA_LAZY collection behavior --- .../AbstractCollectionPersister.php | 46 +++++++++++++++---- .../AbstractCollectionPersisterTest.php | 16 ------- .../Functional/ExtraLazyCollectionTest.php | 4 +- 3 files changed, 40 insertions(+), 26 deletions(-) diff --git a/lib/Doctrine/ORM/Cache/Persister/Collection/AbstractCollectionPersister.php b/lib/Doctrine/ORM/Cache/Persister/Collection/AbstractCollectionPersister.php index f6215676307..7c21535cd7b 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,33 @@ 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) + { + $this->region->evict(new CollectionCacheKey( + $this->sourceEntity->rootEntityName, + $this->association['fieldName'], + $this->uow->getEntityIdentifier($collection->getOwner()) + )); + } + + /** + * @param string $targetEntity + * @param object $element + */ + protected function evictElementCache($targetEntity, $element) + { + /* @var $targetPersister CachedEntityPersister */ + $targetPersister = $this->uow->getEntityPersister($targetEntity); + + $targetPersister->getCacheRegion()->evict(new EntityCacheKey( + $targetEntity, + $this->uow->getEntityIdentifier($element) + )); + } } 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 525021ff32c..6021a9efd8f 100644 --- a/tests/Doctrine/Tests/ORM/Functional/ExtraLazyCollectionTest.php +++ b/tests/Doctrine/Tests/ORM/Functional/ExtraLazyCollectionTest.php @@ -1062,7 +1062,9 @@ public function testRemovesManagedElementFromOneToManyExtraLazyCollectionWithout /* @var $user User */ $user = $this->_em->find(User::CLASSNAME, $userId); - $user->tweets->removeElement($this->_em->find(Tweet::CLASSNAME, $tweetId)); + $e = $this->_em->find(Tweet::CLASSNAME, $tweetId); + + $user->tweets->removeElement($e); $this->_em->clear(); From cb780e8bb6dc48e7c2a2a82cfbde902bbf621fa2 Mon Sep 17 00:00:00 2001 From: Marco Pivetta Date: Sun, 25 Jan 2015 04:45:45 +0100 Subject: [PATCH 16/16] #1169 DDC-3343 - factoring logging into cached collection persister changes --- .../AbstractCollectionPersister.php | 21 +++++++++++++------ 1 file changed, 15 insertions(+), 6 deletions(-) diff --git a/lib/Doctrine/ORM/Cache/Persister/Collection/AbstractCollectionPersister.php b/lib/Doctrine/ORM/Cache/Persister/Collection/AbstractCollectionPersister.php index 7c21535cd7b..3172fe9994b 100644 --- a/lib/Doctrine/ORM/Cache/Persister/Collection/AbstractCollectionPersister.php +++ b/lib/Doctrine/ORM/Cache/Persister/Collection/AbstractCollectionPersister.php @@ -271,11 +271,17 @@ public function loadCriteria(PersistentCollection $collection, Criteria $criteri */ protected function evictCollectionCache(PersistentCollection $collection) { - $this->region->evict(new CollectionCacheKey( + $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); + } } /** @@ -286,10 +292,13 @@ 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); - $targetPersister->getCacheRegion()->evict(new EntityCacheKey( - $targetEntity, - $this->uow->getEntityIdentifier($element) - )); + if ($this->cacheLogger) { + $this->cacheLogger->entityCachePut($targetRegion->getName(), $key); + } } }