Merge branch 'fix/#6614-clean-modified-collection-causing-double-dirty-object-persistence-2.5' into 2.5
Backport #6613 Backport #6614 Backport #6616
This commit is contained in:
Marco Pivetta
commit
095611c4b6
@ -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;
|
||||
}
|
||||
|
||||
@ -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());
|
||||
}
|
||||
}
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user