Skip to content

Commit c06e8d4

Browse files
malarzmmalarzm
committed
Merge pull request doctrine#1120 
2 parents 8f4362f + 6983f59 commit c06e8d4

File tree

4 files changed

+136
-11
lines changed

4 files changed

+136
-11
lines changed

‎lib/Doctrine/ODM/MongoDB/Persisters/DocumentPersister.php

-9
Original file line numberDiff line numberDiff line change
@@ -1307,15 +1307,6 @@ private function getAtomicCollectionUpdateQuery($document)
13071307
* parent of a scheduled child, the following calls are NOPs.
13081308
*/
13091309
$this->uow->unscheduleCollectionUpdate($coll);
1310-
/* TODO: The collection may have been scheduled for both update and
1311-
* deletion if the PersistentCollection instance was changed. Since
1312-
* we're including the collection's update in an atomic $set, the
1313-
* $unset is unnecessary and we can unschedule the deletion.
1314-
*
1315-
* This can be removed if UnitOfWork is ever fixed to not schedule
1316-
* collections for both updates and deletions.
1317-
*/
1318-
$this->uow->unscheduleCollectionDeletion($coll);
13191310
}
13201311

13211312
foreach ($atomicCollDeletes as $coll) {

‎lib/Doctrine/ODM/MongoDB/UnitOfWork.php

+13
Original file line numberDiff line numberDiff line change
@@ -782,6 +782,12 @@ private function computeOrRecomputeChangeSet(ClassMetadata $class, $document, $r
782782
// if embed-many or reference-many relationship
783783
if (isset($class->fieldMappings[$propName]['type']) && $class->fieldMappings[$propName]['type'] === 'many') {
784784
$changeSet[$propName] = array($orgValue, $actualValue);
785+
/* If original collection was exchanged with a non-empty value
786+
* and $set will be issued, there is no need to $unset it first
787+
*/
788+
if ($actualValue && $actualValue->isDirty() && in_array($class->fieldMappings[$propName]['strategy'], array('set', 'setArray', 'atomicSet', 'atomicSetArray'))) {
789+
continue;
790+
}
785791
if ($orgValue instanceof PersistentCollection) {
786792
$this->scheduleCollectionDeletion($orgValue);
787793
}
@@ -2741,6 +2747,13 @@ public function unscheduleCollectionDeletion(PersistentCollection $coll)
27412747
*/
27422748
public function scheduleCollectionUpdate(PersistentCollection $coll)
27432749
{
2750+
$mapping = $coll->getMapping();
2751+
if (isset($mapping['strategy']) && in_array($mapping['strategy'], array('set', 'setArray', 'atomicSet', 'atomicSetArray'))) {
2752+
/* There is no need to $unset collection if it will be $set later
2753+
* This is NOP if collection is not scheduled for deletion
2754+
*/
2755+
$this->unscheduleCollectionDeletion($coll);
2756+
}
27442757
$this->collectionUpdates[] = $coll;
27452758
$this->scheduleCollectionOwner($coll);
27462759
}

‎tests/Doctrine/ODM/MongoDB/Tests/Functional/AtomicSetTest.php

+60-2
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,10 @@ public function testAtomicUpsert()
7777
$this->assertEquals('12345678', $user->phonenumbers[0]->getPhonenumber());
7878
}
7979

80-
public function testAtomicCollectionUnset()
80+
/**
81+
* @dataProvider provideAtomicCollectionUnset
82+
*/
83+
public function testAtomicCollectionUnset($clearWith)
8184
{
8285
$user = new AtomicSetUser('Maciej');
8386
$user->phonenumbers[] = new Phonenumber('12345678');
@@ -92,7 +95,7 @@ public function testAtomicCollectionUnset()
9295
$this->assertEquals('12345678', $user->phonenumbers[0]->getPhonenumber());
9396

9497
$user->surname = "Malarz";
95-
$user->phonenumbers = null;
98+
$user->phonenumbers = $clearWith;
9699
$this->ql->clear();
97100
$this->dm->flush();
98101
$this->assertCount(1, $this->ql, 'Updating a document and unsetting its embed-many collection requires one query');
@@ -104,6 +107,61 @@ public function testAtomicCollectionUnset()
104107
$this->assertCount(0, $user->phonenumbers);
105108
}
106109

110+
public function provideAtomicCollectionUnset()
111+
{
112+
return array(
113+
array(null),
114+
array(array()),
115+
array(new ArrayCollection()),
116+
);
117+
}
118+
119+
public function testAtomicCollectionClearAndUpdate()
120+
{
121+
$user = new AtomicSetUser('Maciej');
122+
$user->phonenumbers[] = new Phonenumber('12345678');
123+
$this->dm->persist($user);
124+
$this->dm->flush();
125+
$this->assertCount(1, $this->ql, 'Inserting a document with an embed-many collection requires one query');
126+
$this->dm->clear();
127+
128+
$user = $this->dm->getRepository(get_class($user))->find($user->id);
129+
$user->phonenumbers->clear();
130+
$user->phonenumbers[] = new Phonenumber('87654321');
131+
$this->ql->clear();
132+
$this->dm->flush();
133+
$this->assertCount(1, $this->ql, 'Updating emptied collection requires one query');
134+
$this->dm->clear();
135+
136+
$user = $this->dm->getRepository(get_class($user))->find($user->id);
137+
$this->assertEquals('Maciej', $user->name);
138+
$this->assertCount(1, $user->phonenumbers);
139+
$this->assertEquals('87654321', $user->phonenumbers[0]->getPhonenumber());
140+
}
141+
142+
public function testAtomicCollectionReplacedAndUpdated()
143+
{
144+
$user = new AtomicSetUser('Maciej');
145+
$user->phonenumbers[] = new Phonenumber('12345678');
146+
$this->dm->persist($user);
147+
$this->dm->flush();
148+
$this->assertCount(1, $this->ql, 'Inserting a document with an embed-many collection requires one query');
149+
$this->dm->clear();
150+
151+
$user = $this->dm->getRepository(get_class($user))->find($user->id);
152+
$user->phonenumbers = new ArrayCollection();
153+
$user->phonenumbers[] = new Phonenumber('87654321');
154+
$this->ql->clear();
155+
$this->dm->flush();
156+
$this->assertCount(1, $this->ql, 'Updating emptied collection requires one query');
157+
$this->dm->clear();
158+
159+
$user = $this->dm->getRepository(get_class($user))->find($user->id);
160+
$this->assertEquals('Maciej', $user->name);
161+
$this->assertCount(1, $user->phonenumbers);
162+
$this->assertEquals('87654321', $user->phonenumbers[0]->getPhonenumber());
163+
}
164+
107165
public function testAtomicSetArray()
108166
{
109167
$user = new AtomicSetUser('Maciej');

‎tests/Doctrine/ODM/MongoDB/Tests/Functional/Ticket/GH1011Test.php

+63
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,63 @@
1+
<?php
2+
3+
namespace Doctrine\ODM\MongoDB\Tests\Functional\Ticket;
4+
5+
use Doctrine\Common\Collections\ArrayCollection;
6+
use Doctrine\ODM\MongoDB\Mapping\Annotations as ODM;
7+
8+
class GH1011Test extends \Doctrine\ODM\MongoDB\Tests\BaseTest
9+
{
10+
public function testClearCollection()
11+
{
12+
$doc = new GH1011Document();
13+
$doc->embeds->add(new GH1011Embedded('test1'));
14+
$this->dm->persist($doc);
15+
$this->dm->flush();
16+
$doc->embeds->clear();
17+
$doc->embeds->add(new GH1011Embedded('test2'));
18+
$this->uow->computeChangeSets();
19+
$this->assertTrue($this->uow->isCollectionScheduledForUpdate($doc->embeds));
20+
$this->assertFalse($this->uow->isCollectionScheduledForDeletion($doc->embeds));
21+
}
22+
23+
public function testReplaceCollection()
24+
{
25+
$doc = new GH1011Document();
26+
$doc->embeds->add(new GH1011Embedded('test1'));
27+
$this->dm->persist($doc);
28+
$this->dm->flush();
29+
$oldCollection = $doc->embeds;
30+
$doc->embeds = new ArrayCollection();
31+
$doc->embeds->add(new GH1011Embedded('test2'));
32+
$this->uow->computeChangeSets();
33+
$this->assertTrue($this->uow->isCollectionScheduledForUpdate($doc->embeds));
34+
$this->assertFalse($this->uow->isCollectionScheduledForDeletion($oldCollection));
35+
}
36+
}
37+
38+
/** @ODM\Document */
39+
class GH1011Document
40+
{
41+
/** @ODM\Id */
42+
public $id;
43+
44+
/** @ODM\EmbedMany(targetDocument="GH1011Embedded", strategy="set") */
45+
public $embeds;
46+
47+
public function __construct()
48+
{
49+
$this->embeds = new ArrayCollection();
50+
}
51+
}
52+
53+
/** @ODM\EmbeddedDocument */
54+
class GH1011Embedded
55+
{
56+
/** @ODM\String */
57+
public $name;
58+
59+
public function __construct($name)
60+
{
61+
$this->name = $name;
62+
}
63+
}

0 commit comments

Comments
 (0)