Better code

Author: sylfabre

lib/Doctrine/ORM/Persisters/Entity/BasicEntityPersister.php

@@ -725,29 +725,31 @@ protected function prepareUpdateData($entity, bool $isInsert = false)
      */
     private function isExtraUpdateRequired($entity, $newVal): bool
     {
+        // The association is left empty
         if ($newVal === null) {
             return false;
         }
 
-        $oid = spl_object_id($newVal);
         $uow = $this->em->getUnitOfWork();
-        if (! (isset($this->queuedInserts[$oid]) || $uow->isScheduledForInsert($newVal))) {
+        $oid = spl_object_id($newVal);
+
+        // The value is not scheduled for insert so an extra update is not required
+        if (! isset($this->queuedInserts[$oid]) && ! $uow->isScheduledForInsert($newVal)) {
             return false;
         }
 
+        // The entity is not self-referencing
         if ($newVal !== $entity) {
             return true;
         }
 
-        $identifiers = $this->class->getIdentifier();
-        // Only single-column identifiers are supported
+        // An extra update is useless for application-generated single-column identifiers
+        $identifiers     = $this->class->getIdentifier();
         $entityChangeSet = $uow->getEntityChangeSet($entity);
-        if (count($identifiers) === 1 && isset($entityChangeSet[$identifiers[0]])) {
-            // Extra update is required if the current entity does not have yet a value for its identifier
-            return $entityChangeSet[$identifiers[0]][1] === null;
-        }
 
-        return true;
+        return count($identifiers) !== 1
+            || ! isset($entityChangeSet[$identifiers[0]])
+            || $entityChangeSet[$identifiers[0]][1] === null;
     }
 
     /**

tests/Doctrine/Tests/ORM/Functional/GH7877Test.php

@@ -35,21 +35,15 @@ public function testNoExtraUpdateWithApplicationGeneratedId(): void
         }
 
         $this->_em->flush();
-        if ($this->isQueryLogAvailable()) {
-            if ($this->getQueryLog()->queries[0]['sql'] === '"START TRANSACTION"') {
-                self::assertQueryCount(3);
-            } else {
-                self::assertQueryCount(1);
-            }
-        }
+        $this->checkQueryCount(false);
 
         $this->_em->clear();
 
         $child = $this->_em->find(GH7877ApplicationGenerated::class, $entityId);
         $this->assertSame($entityId, $child->parent->id);
     }
 
-    public function textExtraUpdateWithDatabaseGeneratedId(): void
+    public function testExtraUpdateWithDatabaseGeneratedId(): void
     {
         $entity         = new GH7877DatabaseGenerated();
         $entity->parent = $entity;
@@ -60,20 +54,27 @@ public function textExtraUpdateWithDatabaseGeneratedId(): void
         }
 
         $this->_em->flush();
-        if ($this->isQueryLogAvailable()) {
-            if ($this->getQueryLog()->queries[0]['sql'] === '"START TRANSACTION"') {
-                self::assertQueryCount(4);
-            } else {
-                self::assertQueryCount(2);
-            }
-        }
+        $this->checkQueryCount(
+            $this->_em->getClassMetadata(GH7877DatabaseGenerated::class)->idGenerator->isPostInsertGenerator()
+        );
 
         $entityId = $entity->id;
         $this->_em->clear();
 
         $child = $this->_em->find(GH7877DatabaseGenerated::class, $entityId);
         $this->assertSame($entityId, $child->parent->id);
     }
+
+    private function checkQueryCount(bool $extra): void
+    {
+        if ($this->isQueryLogAvailable()) {
+            if ($this->getQueryLog()->queries[0]['sql'] === '"START TRANSACTION"') {
+                self::assertQueryCount($extra ? 4 : 3);
+            } else {
+                self::assertQueryCount($extra ? 2 : 1);
+            }
+        }
+    }
 }
 
 /**