diff --git a/lib/Doctrine/ORM/UnitOfWork.php b/lib/Doctrine/ORM/UnitOfWork.php index 00a41e6dffc..c0b0dd8c302 100644 --- a/lib/Doctrine/ORM/UnitOfWork.php +++ b/lib/Doctrine/ORM/UnitOfWork.php @@ -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 = []) { @@ -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) @@ -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); } @@ -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 * @@ -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) { diff --git a/tests/Doctrine/Tests/Models/Company/CompanyContractListener.php b/tests/Doctrine/Tests/Models/Company/CompanyContractListener.php index 61c1d4719f2..23714f32983 100644 --- a/tests/Doctrine/Tests/Models/Company/CompanyContractListener.php +++ b/tests/Doctrine/Tests/Models/Company/CompanyContractListener.php @@ -6,17 +6,17 @@ class CompanyContractListener { public $postPersistCalls; public $prePersistCalls; - + public $postUpdateCalls; public $preUpdateCalls; - + public $postRemoveCalls; public $preRemoveCalls; public $preFlushCalls; - + public $postLoadCalls; - + /** * @PostPersist */ @@ -80,5 +80,4 @@ public function postLoadHandler(CompanyContract $contract) { $this->postLoadCalls[] = func_get_args(); } - } diff --git a/tests/Doctrine/Tests/ORM/UnitOfWorkTest.php b/tests/Doctrine/Tests/ORM/UnitOfWorkTest.php index 72382549ebc..90c621d605a 100644 --- a/tests/Doctrine/Tests/ORM/UnitOfWorkTest.php +++ b/tests/Doctrine/Tests/ORM/UnitOfWorkTest.php @@ -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; @@ -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(); @@ -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); + } } /** @@ -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); + } +}