fix: only call om::persist() once all objects are created

Author: nikophil

src/Persistence/PersistenceManager.php

@@ -31,6 +31,9 @@ final class PersistenceManager
     private bool $flush = true;
     private bool $persist = true;
 
+    /** @var list<object> */
+    private array $objectsToPersist = [];
+
     /** @var list<callable():void> */
     private array $afterPersistCallbacks = [];
 
@@ -78,6 +81,26 @@ public function save(object $object): object
         return $object;
     }
 
+    public function saveAll(): void
+    {
+        $objectManagers = [];
+
+        foreach ($this->objectsToPersist as $object) {
+            $om = $this->strategyFor($object::class)->objectManagerFor($object::class);
+            $om->persist($object);
+
+            if (!in_array($om, $objectManagers, true)) {
+                $objectManagers[] = $om;
+            }
+        }
+
+        $this->objectsToPersist = [];
+
+        foreach ($objectManagers as $om) {
+            $this->flush($om);
+        }
+    }
+
     /**
      * @template T of object
      *
@@ -92,25 +115,13 @@ public function scheduleForInsert(object $object, array $afterPersistCallbacks =
             $object = unproxy($object);
         }
 
-        $om = $this->strategyFor($object::class)->objectManagerFor($object::class);
-        $om->persist($object);
+        $this->objectsToPersist[] = $object;
 
         $this->afterPersistCallbacks = [...$this->afterPersistCallbacks, ...$afterPersistCallbacks];
 
         return $object;
     }
 
-    public function forget(object $object): void
-    {
-        if ($this->isPersisted($object)) {
-            throw new \LogicException('Cannot forget an object already persisted.');
-        }
-
-        $om = $this->strategyFor($object::class)->objectManagerFor($object::class);
-
-        $om->detach($object);
-    }
-
     /**
      * @template T
      *

src/Persistence/PersistentObjectFactory.php

@@ -217,7 +217,7 @@ public function create(callable|array $attributes = []): object
             throw new \LogicException('Persistence cannot be used in unit tests.');
         }
 
-        $configuration->persistence()->save($object);
+        $configuration->persistence()->saveAll();
 
         return $object;
     }
@@ -293,25 +293,23 @@ protected function normalizeParameter(string $field, mixed $value): mixed
             if ($inversedRelationshipMetadata && !$inversedRelationshipMetadata->isCollection) {
                 $inverseField = $inversedRelationshipMetadata->inverseField;
 
-                $inversedObject = $value->withPersistMode(
-                    $this->isPersisting() ? PersistMode::NO_PERSIST_BUT_SCHEDULE_FOR_INSERT : PersistMode::WITHOUT_PERSISTING
-                )
-                    ->notRootFactory()
+                // we need to handle the circular dependency involved by inversed one-to-one relationship:
+                // a placeholder object is used, which will be replaced by the real object, after its instantiation
+                $inverseObjectPlaceholder = (new \ReflectionClass($value::class()))->newInstanceWithoutConstructor();
 
-                    // we need to handle the circular dependency involved by inversed one-to-one relationship:
-                    // a placeholder object is used, which will be replaced by the real object, after its instantiation
-                    ->create([
-                        $inverseField => $placeholder = (new \ReflectionClass(static::class()))->newInstanceWithoutConstructor(),
-                    ]);
+                $this->tempAfterInstantiate[] = function(object $object) use ($value, $inverseField, $field) {
+                    $inverseObject = $value->withPersistMode(
+                        $this->isPersisting() ? PersistMode::NO_PERSIST_BUT_SCHEDULE_FOR_INSERT : PersistMode::WITHOUT_PERSISTING
+                    )
+                        ->notRootFactory()
+                        ->create([$inverseField => $object]);
 
-                $inversedObject = unproxy($inversedObject, withAutoRefresh: false);
+                    $inverseObject = unproxy($inverseObject, withAutoRefresh: false);
 
-                $this->tempAfterInstantiate[] = static function(object $object) use ($inversedObject, $inverseField, $pm, $placeholder) {
-                    $pm->forget($placeholder);
-                    set($inversedObject, $inverseField, $object);
+                    set($object, $field, $inverseObject);
                 };
 
-                return $inversedObject;
+                return $inverseObjectPlaceholder;
             } else {
                 $value = $value->notRootFactory();
             }
@@ -384,6 +382,8 @@ protected function normalizeObject(object $object): object
 
         if (!$persistenceManager->isPersisted($object)) {
             $persistenceManager->scheduleForInsert($object);
+
+            return $object;
         }
 
         try {

tests/Fixture/Entity/EdgeCases/InversedOneToOneWithoutAutoGeneratedId/InverseSide.php

@@ -0,0 +1,36 @@
+<?php
+
+declare(strict_types=1);
+
+/*
+ * This file is part of the zenstruck/foundry package.
+ *
+ * (c) Kevin Bond <[email protected]>
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace Zenstruck\Foundry\Tests\Fixture\Entity\EdgeCases\InversedOneToOneWithoutAutoGeneratedId;
+
+use Doctrine\ORM\Mapping as ORM;
+use Symfony\Component\Uid\Uuid;
+
+/**
+ * @author Nicolas PHILIPPE <[email protected]>
+ */
+#[ORM\Entity]
+#[ORM\Table('inversed_one_to_one_with_custom_id_inverse')]
+class InverseSide
+{
+    #[ORM\Id]
+    #[ORM\Column(type: 'uuid')]
+    public Uuid $id;
+
+    public function __construct(
+        #[ORM\OneToOne(mappedBy: 'inverseSide', cascade: ['persist'])] // @phpstan-ignore doctrine.associationType
+        public OwningSide $owningSide,
+    ) {
+        $this->id = Uuid::v7();
+    }
+}

tests/Fixture/Entity/EdgeCases/InversedOneToOneWithoutAutoGeneratedId/OwningSide.php

@@ -0,0 +1,37 @@
+<?php
+
+declare(strict_types=1);
+
+/*
+ * This file is part of the zenstruck/foundry package.
+ *
+ * (c) Kevin Bond <[email protected]>
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace Zenstruck\Foundry\Tests\Fixture\Entity\EdgeCases\InversedOneToOneWithoutAutoGeneratedId;
+
+use Doctrine\ORM\Mapping as ORM;
+use Symfony\Component\Uid\Uuid;
+
+/**
+ * @author Nicolas PHILIPPE <[email protected]>
+ */
+#[ORM\Entity]
+#[ORM\Table('inversed_one_to_one_with_custom_id_owning')]
+class OwningSide
+{
+    #[ORM\Id]
+    #[ORM\Column(type: 'uuid')]
+    public Uuid $id;
+
+    #[ORM\OneToOne(inversedBy: 'owningSide', cascade: ['persist'])]
+    public ?InverseSide $inverseSide = null;
+
+    public function __construct()
+    {
+        $this->id = Uuid::v7();
+    }
+}

tests/Integration/ORM/EdgeCasesRelationshipTest.php

@@ -24,6 +24,7 @@
 use Zenstruck\Foundry\Tests\Fixture\Entity\EdgeCases\IndexedOneToMany;
 use Zenstruck\Foundry\Tests\Fixture\Entity\EdgeCases\InversedOneToOneWithManyToOne;
 use Zenstruck\Foundry\Tests\Fixture\Entity\EdgeCases\InversedOneToOneWithNonNullableOwning;
+use Zenstruck\Foundry\Tests\Fixture\Entity\EdgeCases\InversedOneToOneWithoutAutoGeneratedId;
 use Zenstruck\Foundry\Tests\Fixture\Entity\EdgeCases\InversedOneToOneWithOneToMany;
 use Zenstruck\Foundry\Tests\Fixture\Entity\EdgeCases\InversedOneToOneWithSetter;
 use Zenstruck\Foundry\Tests\Fixture\Entity\EdgeCases\ManyToOneToSelfReferencing;
@@ -187,6 +188,24 @@ public function inversed_one_to_one_can_be_used_after_other_relationship(): void
         self::assertNotNull($inverseSide->item);
     }
 
+    /** @test */
+    #[Test]
+    #[DataProvider('provideCascadeRelationshipsCombinations')]
+    #[UsingRelationships(InversedOneToOneWithoutAutoGeneratedId\OwningSide::class, ['inverseSide'])]
+    #[RequiresPhpunit('>=11.4')]
+    public function inverse_one_to_one_with_custom_id(): void
+    {
+        $owningSideFactory = persistent_factory(InversedOneToOneWithoutAutoGeneratedId\OwningSide::class);
+        $inverseSideFactory = persistent_factory(InversedOneToOneWithoutAutoGeneratedId\InverseSide::class);
+
+        $inverseSide = $inverseSideFactory->create(['owningSide' => $owningSideFactory]);
+
+        $owningSideFactory::assert()->count(1);
+        $inverseSideFactory::assert()->count(1);
+
+        self::assertSame($inverseSide, $inverseSide->owningSide->inverseSide);
+    }
+
     /**
      * @test
      */