Merge pull request #6178 from doctrine/fix/#6174-#5570-merging-new-entities-should-also-trigger-prepersist-lifecycle-callbacks-2.5

Backport #6177 - fix #6174 #5570: merging new entities should also trigger prepersist lifecycle callbacks with the merged data

lib/Doctrine/ORM/UnitOfWork.php

@@ -1799,7 +1799,7 @@ class UnitOfWork implements PropertyChangedListener
      * @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)
     {
@@ -1831,6 +1831,7 @@ class UnitOfWork implements PropertyChangedListener
             if ( ! $id) {
                 $managedCopy = $this->newInstance($class);
 
+                $this->mergeEntityStateIntoManagedCopy($entity, $managedCopy);
                 $this->persistNew($class, $managedCopy);
             } else {
                 $flatId = ($class->containsForeignIdentifier)
@@ -1862,31 +1863,16 @@ class UnitOfWork implements PropertyChangedListener
                     $managedCopy = $this->newInstance($class);
                     $class->setIdentifierValues($managedCopy, $id);
 
+                    $this->mergeEntityStateIntoManagedCopy($entity, $managedCopy);
                     $this->persistNew($class, $managedCopy);
-                }
+                } else {
-            }
+                    $this->ensureVersionMatch($class, $entity, $managedCopy);
-
+                    $this->mergeEntityStateIntoManagedCopy($entity, $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);
                 }
             }
 
             $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);
             }
@@ -1904,6 +1890,33 @@ class UnitOfWork implements PropertyChangedListener
         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
      *
@@ -3356,6 +3369,14 @@ class UnitOfWork implements PropertyChangedListener
      */
     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) {

tests/Doctrine/Tests/Models/Company/CompanyContractListener.php

@@ -80,5 +80,4 @@ class CompanyContractListener
     {
         $this->postLoadCalls[] = func_get_args();
     }
-
 }

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\UnitOfWork;
 use Doctrine\Tests\Mocks\ConnectionMock;
@@ -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->_connectionMock = new ConnectionMock([], new DriverMock());
-        $this->_emMock = EntityManagerMock::create($this->_connectionMock);
+        $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();
@@ -392,6 +399,88 @@ class UnitOfWorkTest extends \Doctrine\Tests\OrmTestCase
 
         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);
+    }
 }
 
 /**
@@ -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);
+    }
+}