diff --git a/lib/Doctrine/ORM/PersistentCollection.php b/lib/Doctrine/ORM/PersistentCollection.php
index a4b579b7e..48daa4dc6 100644
--- a/lib/Doctrine/ORM/PersistentCollection.php
+++ b/lib/Doctrine/ORM/PersistentCollection.php
@@ -19,7 +19,6 @@
namespace Doctrine\ORM;
-use Closure;
use Doctrine\Common\Collections\AbstractLazyCollection;
use Doctrine\Common\Collections\Collection;
use Doctrine\Common\Collections\ArrayCollection;
@@ -685,21 +684,41 @@ final class PersistentCollection extends AbstractLazyCollection implements Selec
protected function doInitialize()
{
// Has NEW objects added through add(). Remember them.
- $newObjects = array();
+ $newlyAddedDirtyObjects = array();
if ($this->isDirty) {
- $newObjects = $this->collection->toArray();
+ $newlyAddedDirtyObjects = $this->collection->toArray();
}
$this->collection->clear();
$this->em->getUnitOfWork()->loadCollection($this);
$this->takeSnapshot();
- // Reattach NEW objects added through add(), if any.
- if ($newObjects) {
- foreach ($newObjects as $obj) {
- $this->collection->add($obj);
- }
+ if ($newlyAddedDirtyObjects) {
+ $this->restoreNewObjectsInDirtyCollection($newlyAddedDirtyObjects);
+ }
+ }
+
+ /**
+ * @param object[] $newObjects
+ *
+ * @return void
+ *
+ * Note: the only reason why this entire looping/complexity is performed via `spl_object_hash`
+ * is because we want to prevent using `array_udiff()`, which is likely to cause very
+ * high overhead (complexity of O(n^2)). `array_diff_key()` performs the operation in
+ * core, which is faster than using a callback for comparisons
+ */
+ private function restoreNewObjectsInDirtyCollection(array $newObjects)
+ {
+ $loadedObjects = $this->collection->toArray();
+ $newObjectsByOid = \array_combine(\array_map('spl_object_hash', $newObjects), $newObjects);
+ $loadedObjectsByOid = \array_combine(\array_map('spl_object_hash', $loadedObjects), $loadedObjects);
+ $newObjectsThatWereNotLoaded = \array_diff_key($newObjectsByOid, $loadedObjectsByOid);
+
+ if ($newObjectsThatWereNotLoaded) {
+ // Reattach NEW objects added through add(), if any.
+ \array_walk($newObjectsThatWereNotLoaded, [$this->collection, 'add']);
$this->isDirty = true;
}
diff --git a/tests/Doctrine/Tests/ORM/PersistentCollectionTest.php b/tests/Doctrine/Tests/ORM/PersistentCollectionTest.php
index 190fe2ef1..2ca179ef9 100644
--- a/tests/Doctrine/Tests/ORM/PersistentCollectionTest.php
+++ b/tests/Doctrine/Tests/ORM/PersistentCollectionTest.php
@@ -4,6 +4,7 @@ namespace Doctrine\Tests\ORM;
use Doctrine\Common\Collections\ArrayCollection;
use Doctrine\ORM\PersistentCollection;
+use Doctrine\ORM\UnitOfWork;
use Doctrine\Tests\Mocks\ConnectionMock;
use Doctrine\Tests\Mocks\DriverMock;
use Doctrine\Tests\Mocks\EntityManagerMock;
@@ -23,7 +24,7 @@ class PersistentCollectionTest extends OrmTestCase
protected $collection;
/**
- * @var \Doctrine\ORM\EntityManagerInterface
+ * @var EntityManagerMock
*/
private $_emMock;
@@ -32,6 +33,8 @@ class PersistentCollectionTest extends OrmTestCase
parent::setUp();
$this->_emMock = EntityManagerMock::create(new ConnectionMock([], new DriverMock()));
+
+ $this->setUpPersistentCollection();
}
/**
@@ -58,7 +61,6 @@ class PersistentCollectionTest extends OrmTestCase
*/
public function testCurrentInitializesCollection()
{
- $this->setUpPersistentCollection();
$this->collection->current();
$this->assertTrue($this->collection->isInitialized());
}
@@ -68,7 +70,6 @@ class PersistentCollectionTest extends OrmTestCase
*/
public function testKeyInitializesCollection()
{
- $this->setUpPersistentCollection();
$this->collection->key();
$this->assertTrue($this->collection->isInitialized());
}
@@ -78,7 +79,6 @@ class PersistentCollectionTest extends OrmTestCase
*/
public function testNextInitializesCollection()
{
- $this->setUpPersistentCollection();
$this->collection->next();
$this->assertTrue($this->collection->isInitialized());
}
@@ -88,12 +88,12 @@ class PersistentCollectionTest extends OrmTestCase
*/
public function testRemovingElementsAlsoRemovesKeys()
{
- $this->setUpPersistentCollection();
+ $dummy = new \stdClass();
- $this->collection->add('dummy');
+ $this->collection->add($dummy);
$this->assertEquals([0], array_keys($this->collection->toArray()));
- $this->collection->removeElement('dummy');
+ $this->collection->removeElement($dummy);
$this->assertEquals([], array_keys($this->collection->toArray()));
}
@@ -102,9 +102,7 @@ class PersistentCollectionTest extends OrmTestCase
*/
public function testClearWillAlsoClearKeys()
{
- $this->setUpPersistentCollection();
-
- $this->collection->add('dummy');
+ $this->collection->add(new \stdClass());
$this->collection->clear();
$this->assertEquals([], array_keys($this->collection->toArray()));
}
@@ -114,12 +112,133 @@ class PersistentCollectionTest extends OrmTestCase
*/
public function testClearWillAlsoResetKeyPositions()
{
- $this->setUpPersistentCollection();
+ $dummy = new \stdClass();
- $this->collection->add('dummy');
- $this->collection->removeElement('dummy');
+ $this->collection->add($dummy);
+ $this->collection->removeElement($dummy);
$this->collection->clear();
- $this->collection->add('dummy');
+ $this->collection->add($dummy);
$this->assertEquals([0], array_keys($this->collection->toArray()));
}
+
+ /**
+ * @group 6613
+ * @group 6614
+ * @group 6616
+ */
+ public function testWillKeepNewItemsInDirtyCollectionAfterInitialization()
+ {
+ /* @var $unitOfWork UnitOfWork|\PHPUnit_Framework_MockObject_MockObject */
+ $unitOfWork = $this->createMock(UnitOfWork::class);
+
+ $this->_emMock->setUnitOfWork($unitOfWork);
+
+ $newElement = new \stdClass();
+ $persistedElement = new \stdClass();
+
+ $this->collection->add($newElement);
+
+ self::assertFalse($this->collection->isInitialized());
+ self::assertTrue($this->collection->isDirty());
+
+ $unitOfWork
+ ->expects(self::once())
+ ->method('loadCollection')
+ ->with($this->collection)
+ ->willReturnCallback(function (PersistentCollection $persistentCollection) use ($persistedElement) {
+ $persistentCollection->unwrap()->add($persistedElement);
+ });
+
+ $this->collection->initialize();
+
+ self::assertSame([$persistedElement, $newElement], $this->collection->toArray());
+ self::assertTrue($this->collection->isInitialized());
+ self::assertTrue($this->collection->isDirty());
+ }
+
+ /**
+ * @group 6613
+ * @group 6614
+ * @group 6616
+ */
+ public function testWillDeDuplicateNewItemsThatWerePreviouslyPersistedInDirtyCollectionAfterInitialization()
+ {
+ /* @var $unitOfWork UnitOfWork|\PHPUnit_Framework_MockObject_MockObject */
+ $unitOfWork = $this->createMock(UnitOfWork::class);
+
+ $this->_emMock->setUnitOfWork($unitOfWork);
+
+ $newElement = new \stdClass();
+ $newElementThatIsAlsoPersisted = new \stdClass();
+ $persistedElement = new \stdClass();
+
+ $this->collection->add($newElementThatIsAlsoPersisted);
+ $this->collection->add($newElement);
+
+ self::assertFalse($this->collection->isInitialized());
+ self::assertTrue($this->collection->isDirty());
+
+ $unitOfWork
+ ->expects(self::once())
+ ->method('loadCollection')
+ ->with($this->collection)
+ ->willReturnCallback(function (PersistentCollection $persistentCollection) use (
+ $persistedElement,
+ $newElementThatIsAlsoPersisted
+ ) {
+ $persistentCollection->unwrap()->add($newElementThatIsAlsoPersisted);
+ $persistentCollection->unwrap()->add($persistedElement);
+ });
+
+ $this->collection->initialize();
+
+ self::assertSame(
+ [$newElementThatIsAlsoPersisted, $persistedElement, $newElement],
+ $this->collection->toArray()
+ );
+ self::assertTrue($this->collection->isInitialized());
+ self::assertTrue($this->collection->isDirty());
+ }
+
+ /**
+ * @group 6613
+ * @group 6614
+ * @group 6616
+ */
+ public function testWillNotMarkCollectionAsDirtyAfterInitializationIfNoElementsWereAdded()
+ {
+ /* @var $unitOfWork UnitOfWork|\PHPUnit_Framework_MockObject_MockObject */
+ $unitOfWork = $this->createMock(UnitOfWork::class);
+
+ $this->_emMock->setUnitOfWork($unitOfWork);
+
+ $newElementThatIsAlsoPersisted = new \stdClass();
+ $persistedElement = new \stdClass();
+
+ $this->collection->add($newElementThatIsAlsoPersisted);
+
+ self::assertFalse($this->collection->isInitialized());
+ self::assertTrue($this->collection->isDirty());
+
+ $unitOfWork
+ ->expects(self::once())
+ ->method('loadCollection')
+ ->with($this->collection)
+ ->willReturnCallback(function (PersistentCollection $persistentCollection) use (
+ $persistedElement,
+ $newElementThatIsAlsoPersisted
+ ) {
+ $persistentCollection->unwrap()->add($newElementThatIsAlsoPersisted);
+ $persistentCollection->unwrap()->add($persistedElement);
+ });
+
+ $this->collection->initialize();
+
+ self::assertSame(
+ [$newElementThatIsAlsoPersisted, $persistedElement],
+ $this->collection->toArray()
+ );
+ self::assertTrue($this->collection->isInitialized());
+ self::assertFalse($this->collection->isDirty());
+ }
}