Skip to content

Commit

Permalink
Merge pull request #6177 from doctrine/fix/#6174-#5570-merging-new-en…
Browse files Browse the repository at this point in the history 
…tities-should-also-trigger-prepersist-lifecycle-callbacks

Fix #6174 #5570: merging new entities should also trigger prepersist lifecycle callbacks with merged entity data
  • Loading branch information
@Ocramius
Ocramius authored Dec 18, 2016
2 parents 6e6be3f + 21a5d8c commit cd1a5fc
Show file tree
Hide file tree
Showing 3 changed files with 157 additions and 30 deletions.
61 changes: 41 additions & 20 deletions lib/Doctrine/ORM/UnitOfWork.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1784,7 +1784,7 @@ public function merge($entity)
* @throws OptimisticLockException If the entity uses optimistic locking through a version
* attribute and the version check against the managed copy fails.
* @throws ORMInvalidArgumentException If the entity instance is NEW.
* @throws EntityNotFoundException
* @throws EntityNotFoundException if an assigned identifier is used in the entity, but none is provided
*/
private function doMerge($entity, array &$visited, $prevManagedCopy = null, array $assoc = [])
{
Expand Down Expand Up @@ -1816,6 +1816,7 @@ private function doMerge($entity, array &$visited, $prevManagedCopy = null, arra
if ( ! $id) {
$managedCopy = $this->newInstance($class);

$this->mergeEntityStateIntoManagedCopy($entity, $managedCopy);
$this->persistNew($class, $managedCopy);
} else {
$flatId = ($class->containsForeignIdentifier)
Expand Down Expand Up @@ -1847,31 +1848,16 @@ private function doMerge($entity, array &$visited, $prevManagedCopy = null, arra
$managedCopy = $this->newInstance($class);
$class->setIdentifierValues($managedCopy, $id);

$this->mergeEntityStateIntoManagedCopy($entity, $managedCopy);
$this->persistNew($class, $managedCopy);
}
}

if ($class->isVersioned && $this->isLoaded($managedCopy) && $this->isLoaded($entity)) {
$reflField = $class->reflFields[$class->versionField];
$managedCopyVersion = $reflField->getValue($managedCopy);
$entityVersion = $reflField->getValue($entity);

// Throw exception if versions don't match.
if ($managedCopyVersion != $entityVersion) {
throw OptimisticLockException::lockFailedVersionMismatch($entity, $entityVersion, $managedCopyVersion);
} else {
$this->ensureVersionMatch($class, $entity, $managedCopy);
$this->mergeEntityStateIntoManagedCopy($entity, $managedCopy);
}
}

$visited[$oid] = $managedCopy; // mark visited

if ($this->isLoaded($entity)) {
if ($managedCopy instanceof Proxy && ! $managedCopy->__isInitialized()) {
$managedCopy->__load();
}

$this->mergeEntityStateIntoManagedCopy($entity, $managedCopy);
}

if ($class->isChangeTrackingDeferredExplicit()) {
$this->scheduleForDirtyCheck($entity);
}
Expand All @@ -1889,6 +1875,33 @@ private function doMerge($entity, array &$visited, $prevManagedCopy = null, arra
return $managedCopy;
}

/**
* @param ClassMetadata $class
* @param object $entity
* @param object $managedCopy
*
* @return void
*
* @throws OptimisticLockException
*/
private function ensureVersionMatch(ClassMetadata $class, $entity, $managedCopy)
{
if (! ($class->isVersioned && $this->isLoaded($managedCopy) && $this->isLoaded($entity))) {
return;
}

$reflField = $class->reflFields[$class->versionField];
$managedCopyVersion = $reflField->getValue($managedCopy);
$entityVersion = $reflField->getValue($entity);

// Throw exception if versions don't match.
if ($managedCopyVersion == $entityVersion) {
return;
}

throw OptimisticLockException::lockFailedVersionMismatch($entity, $entityVersion, $managedCopyVersion);
}

/**
* Tests if an entity is loaded - must either be a loaded proxy or not a proxy
*
Expand Down Expand Up @@ -3338,6 +3351,14 @@ private function isIdentifierEquals($entity1, $entity2)
*/
private function mergeEntityStateIntoManagedCopy($entity, $managedCopy)
{
if (! $this->isLoaded($entity)) {
return;
}

if (! $this->isLoaded($managedCopy)) {
$managedCopy->__load();
}

$class = $this->em->getClassMetadata(get_class($entity));

foreach ($this->reflectionPropertiesGetter->getProperties($class->name) as $prop) {
Expand Down
9 changes: 4 additions & 5 deletions tests/Doctrine/Tests/Models/Company/CompanyContractListener.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,17 +6,17 @@ class CompanyContractListener
{
public $postPersistCalls;
public $prePersistCalls;

public $postUpdateCalls;
public $preUpdateCalls;

public $postRemoveCalls;
public $preRemoveCalls;

public $preFlushCalls;

public $postLoadCalls;

/**
* @PostPersist
*/
Expand Down Expand Up @@ -80,5 +80,4 @@ public function postLoadHandler(CompanyContract $contract)
{
$this->postLoadCalls[] = func_get_args();
}

}
117 changes: 112 additions & 5 deletions tests/Doctrine/Tests/ORM/UnitOfWorkTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,11 @@
namespace Doctrine\Tests\ORM;

use Doctrine\Common\Collections\ArrayCollection;
use Doctrine\Common\EventManager;
use Doctrine\Common\NotifyPropertyChanged;
use Doctrine\Common\Persistence\Event\LifecycleEventArgs;
use Doctrine\Common\PropertyChangedListener;
use Doctrine\ORM\Events;
use Doctrine\ORM\Mapping\ClassMetadata;
use Doctrine\ORM\ORMInvalidArgumentException;
use Doctrine\ORM\UnitOfWork;
Expand Down Expand Up @@ -48,18 +51,22 @@ class UnitOfWorkTest extends OrmTestCase
*/
private $_emMock;

protected function setUp() {
/**
* @var EventManager|\PHPUnit_Framework_MockObject_MockObject
*/
private $eventManager;

protected function setUp()
{
parent::setUp();
$this->_connectionMock = new ConnectionMock([], new DriverMock());
$this->_emMock = EntityManagerMock::create($this->_connectionMock);
$this->eventManager = $this->getMockBuilder(EventManager::class)->getMock();
$this->_emMock = EntityManagerMock::create($this->_connectionMock, null, $this->eventManager);
// SUT
$this->_unitOfWork = new UnitOfWorkMock($this->_emMock);
$this->_emMock->setUnitOfWork($this->_unitOfWork);
}

protected function tearDown() {
}

public function testRegisterRemovedOnNewEntityIsIgnored()
{
$user = new ForumUser();
Expand Down Expand Up @@ -488,6 +495,88 @@ public function testObjectHashesOfMergedEntitiesAreNotUsedInOriginalEntityDataMa

self::assertSame([], $this->_unitOfWork->getOriginalEntityData($newUser), 'No original data was stored');
}

/**
* @group DDC-1955
* @group 5570
* @group 6174
*/
public function testMergeWithNewEntityWillPersistItAndTriggerPrePersistListenersWithMergedEntityData()
{
$entity = new EntityWithRandomlyGeneratedField();

$generatedFieldValue = $entity->generatedField;

$this
->eventManager
->expects(self::any())
->method('hasListeners')
->willReturnCallback(function ($eventName) {
return $eventName === Events::prePersist;
});
$this
->eventManager
->expects(self::once())
->method('dispatchEvent')
->with(
self::anything(),
self::callback(function (LifecycleEventArgs $args) use ($entity, $generatedFieldValue) {
/* @var $object EntityWithRandomlyGeneratedField */
$object = $args->getObject();

self::assertInstanceOf(EntityWithRandomlyGeneratedField::class, $object);
self::assertNotSame($entity, $object);
self::assertSame($generatedFieldValue, $object->generatedField);

return true;
})
);

/* @var $object EntityWithRandomlyGeneratedField */
$object = $this->_unitOfWork->merge($entity);

self::assertNotSame($object, $entity);
self::assertInstanceOf(EntityWithRandomlyGeneratedField::class, $object);
self::assertSame($object->generatedField, $entity->generatedField);
}

/**
* @group DDC-1955
* @group 5570
* @group 6174
*/
public function testMergeWithExistingEntityWillNotPersistItNorTriggerPrePersistListeners()
{
$persistedEntity = new EntityWithRandomlyGeneratedField();
$mergedEntity = new EntityWithRandomlyGeneratedField();

$mergedEntity->id = $persistedEntity->id;
$mergedEntity->generatedField = mt_rand(
$persistedEntity->generatedField + 1,
$persistedEntity->generatedField + 1000
);

$this
->eventManager
->expects(self::any())
->method('hasListeners')
->willReturnCallback(function ($eventName) {
return $eventName === Events::prePersist;
});
$this->eventManager->expects(self::never())->method('dispatchEvent');

$this->_unitOfWork->registerManaged(
$persistedEntity,
['id' => $persistedEntity->id],
['generatedField' => $persistedEntity->generatedField]
);

/* @var $merged EntityWithRandomlyGeneratedField */
$merged = $this->_unitOfWork->merge($mergedEntity);

self::assertSame($merged, $persistedEntity);
self::assertSame($persistedEntity->generatedField, $mergedEntity->generatedField);
}
}

/**
Expand Down Expand Up @@ -634,3 +723,21 @@ class EntityWithCompositeStringIdentifier
*/
public $id2;
}

/** @Entity */
class EntityWithRandomlyGeneratedField
{
/** @Id @Column(type="string") */
public $id;

/**
* @Column(type="integer")
*/
public $generatedField;

public function __construct()
{
$this->id = uniqid('id', true);
$this->generatedField = mt_rand(0, 100000);
}
}

0 comments on commit cd1a5fc

Please sign in to comment.