Skip to content

Commit

Permalink
Merge pull request doctrine#6178 from doctrine/fix/doctrine#6174-doct…
Browse files Browse the repository at this point in the history 
…rine#5570-merging-new-entities-should-also-trigger-prepersist-lifecycle-callbacks-2.5

Backport doctrine#6177 - fix doctrine#6174 doctrine#5570: merging new entities should also trigger prepersist lifecycle callbacks with the merged data
  • Loading branch information
@Ocramius
Ocramius authored Dec 18, 2016
2 parents 20cb504 + d52dbe6 commit e6c4341
Show file tree
Hide file tree
Showing 3 changed files with 198 additions and 31 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 @@ -1799,7 +1799,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, $assoc = null)
{
Expand Down Expand Up @@ -1831,6 +1831,7 @@ private function doMerge($entity, array &$visited, $prevManagedCopy = null, $ass
if ( ! $id) {
$managedCopy = $this->newInstance($class);

$this->mergeEntityStateIntoManagedCopy($entity, $managedCopy);
$this->persistNew($class, $managedCopy);
} else {
$flatId = ($class->containsForeignIdentifier)
Expand Down Expand Up @@ -1862,31 +1863,16 @@ private function doMerge($entity, array &$visited, $prevManagedCopy = null, $ass
$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 @@ -1904,6 +1890,33 @@ private function doMerge($entity, array &$visited, $prevManagedCopy = null, $ass
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 @@ -3356,6 +3369,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();
}

}
159 changes: 153 additions & 6 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\UnitOfWork;
use Doctrine\Tests\Mocks\ConnectionMock;
Expand Down Expand Up @@ -47,18 +50,22 @@ class UnitOfWorkTest extends \Doctrine\Tests\OrmTestCase
*/
private $_emMock;

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

protected function setUp()
{
parent::setUp();
$this->_connectionMock = new ConnectionMock(array(), new DriverMock());
$this->_emMock = EntityManagerMock::create($this->_connectionMock);
$this->_connectionMock = new ConnectionMock([], new DriverMock());
$this->eventManager = $this->getMockBuilder('Doctrine\Common\EventManager')->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 @@ -392,6 +399,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('Doctrine\Tests\ORM\EntityWithRandomlyGeneratedField', $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('Doctrine\Tests\ORM\EntityWithRandomlyGeneratedField', $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 @@ -498,3 +587,61 @@ class VersionedAssignedIdentifierEntity
*/
public $version;
}

/** @Entity */
class EntityWithStringIdentifier
{
/**
* @Id @Column(type="string")
*
* @var string|null
*/
public $id;
}

/** @Entity */
class EntityWithBooleanIdentifier
{
/**
* @Id @Column(type="boolean")
*
* @var bool|null
*/
public $id;
}

/** @Entity */
class EntityWithCompositeStringIdentifier
{
/**
* @Id @Column(type="string")
*
* @var string|null
*/
public $id1;

/**
* @Id @Column(type="string")
*
* @var string|null
*/
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 e6c4341

Please sign in to comment.