fix: use a "placeholder" for inversed one-to-one (#755)

src/Persistence/PersistMode.php

@@ -0,0 +1,21 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Zenstruck\Foundry\Persistence;
+
+/**
+ * @internal
+ * @author Nicolas PHILIPPE <[email protected]>
+ */
+enum PersistMode
+{
+    case PERSIST;
+    case WITHOUT_PERSISTING;
+    case NO_PERSIST_BUT_SCHEDULE_FOR_INSERT;
+
+    public function isPersisting(): bool
+    {
+        return $this === self::PERSIST;
+    }
+}

src/Persistence/PersistenceManager.php

@@ -94,6 +94,17 @@ public function scheduleForInsert(object $object): object
         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

@@ -22,7 +22,7 @@
 use Zenstruck\Foundry\Persistence\Exception\NotEnoughObjects;
 use Zenstruck\Foundry\Persistence\Exception\RefreshObjectFailed;
 
-use function Zenstruck\Foundry\get;
+use function Zenstruck\Foundry\set;
 
 /**
  * @author Kevin Bond <[email protected]>
@@ -34,11 +34,14 @@
  */
 abstract class PersistentObjectFactory extends ObjectFactory
 {
-    private bool $persist;
+    private PersistMode $persist;
 
     /** @phpstan-var list<callable(T, Parameters, static):void> */
     private array $afterPersist = [];
 
+    /** @var list<callable(T):void> */
+    private array $tempAfterInstantiate = [];
+
     /** @var list<callable(T):void> */
     private array $tempAfterPersist = [];
 
@@ -196,6 +199,12 @@ public function create(callable|array $attributes = []): object
     {
         $object = parent::create($attributes);
 
+        foreach ($this->tempAfterInstantiate as $callback) {
+            $callback($object);
+        }
+
+        $this->tempAfterInstantiate = [];
+
         $this->throwIfCannotCreateObject();
 
         if (!$this->isPersisting()) {
@@ -232,15 +241,23 @@ public function create(callable|array $attributes = []): object
     final public function andPersist(): static
     {
         $clone = clone $this;
-        $clone->persist = true;
+        $clone->persist = PersistMode::PERSIST;
 
         return $clone;
     }
 
     final public function withoutPersisting(): static
     {
         $clone = clone $this;
-        $clone->persist = false;
+        $clone->persist = PersistMode::WITHOUT_PERSISTING;
+
+        return $clone;
+    }
+
+    private function withoutPersistingButScheduleForInsert(): static
+    {
+        $clone = clone $this;
+        $clone->persist = PersistMode::NO_PERSIST_BUT_SCHEDULE_FOR_INSERT;
 
         return $clone;
     }
@@ -263,9 +280,11 @@ protected function normalizeParameter(string $field, mixed $value): mixed
         }
 
         if ($value instanceof self && isset($this->persist)) {
-            $value = $this->isPersisting()
-                ? $value->andPersist()
-                : $value->withoutPersisting();
+            $value = match($this->persist) {
+                PersistMode::PERSIST => $value->andPersist(),
+                PersistMode::WITHOUT_PERSISTING => $value->withoutPersisting(),
+                PersistMode::NO_PERSIST_BUT_SCHEDULE_FOR_INSERT => $value->withoutPersistingButScheduleForInsert(),
+            };
         }
 
         if ($value instanceof self) {
@@ -277,21 +296,21 @@ protected function normalizeParameter(string $field, mixed $value): mixed
             if ($inversedRelationshipMetadata && !$inversedRelationshipMetadata->isCollection) {
                 $inverseField = $inversedRelationshipMetadata->inverseField;
 
-                // we create now the object to prevent "non-nullable" property errors,
-                // but we'll need to remove it once the current object is created
-
-                $inversedObject = unproxy($value->create());
-                $this->tempAfterPersist[] = static function(object $object) use ($value, $inverseField, $pm, $inversedObject) {
-                    // we cannot use the already created $inversedObject:
-                    // because we must also remove its potential newly created owner (here: "$oldObj")
-                    // but a cascade:["remove"] would remove too many things
-                    $value->create([$inverseField => $object]);
-                    $pm->refresh($object);
-                    $oldObj = get($inversedObject, $inverseField);
-                    delete($inversedObject);
-                    if ($oldObj) {
-                        delete($oldObj); // @phpstan-ignore argument.templateType
-                    }
+                // 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
+                $inversedObject = $value->withoutPersistingButScheduleForInsert()
+                    ->create([$inverseField => $placeholder = (new \ReflectionClass(static::class()))->newInstanceWithoutConstructor()]);
+
+                // auto-refresh computes changeset and prevents the placeholder object to be cleanly
+                // forgotten fom the persistence manager
+                if ($inversedObject instanceof Proxy) {
+                    $inversedObject->_disableAutoRefresh();
+                    $inversedObject = $inversedObject->_real();
+                }
+
+                $this->tempAfterInstantiate[] = static function(object $object) use ($inversedObject, $inverseField, $pm, $placeholder) {
+                    $pm->forget($placeholder);
+                    set($inversedObject, $inverseField, $object);
                 };
 
                 return $inversedObject;
@@ -359,11 +378,13 @@ final protected function isPersisting(): bool
     {
         $config = Configuration::instance();
 
-        if ($config->isPersistenceAvailable() && !$config->persistence()->isEnabled()) {
+        if (!$config->isPersistenceEnabled()) {
             return false;
         }
 
-        return $this->persist ?? $config->isPersistenceAvailable() && $config->persistence()->isEnabled() && $config->persistence()->autoPersist(static::class());
+        $persistMode = $this->persist ?? ($config->persistence()->autoPersist(static::class()) ? PersistMode::PERSIST : PersistMode::WITHOUT_PERSISTING);
+
+        return $persistMode->isPersisting();
     }
 
     /**
@@ -373,7 +394,7 @@ final protected function initializeInternal(): static
     {
         return $this->afterInstantiate(
             static function(object $object, array $parameters, PersistentObjectFactory $factory): void {
-                if (!$factory->isPersisting()) {
+                if (!$factory->isPersisting() && (!isset($factory->persist) || $factory->persist !== PersistMode::NO_PERSIST_BUT_SCHEDULE_FOR_INSERT)) {
                     return;
                 }
 

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

@@ -14,19 +14,15 @@
 namespace Zenstruck\Foundry\Tests\Fixture\Entity\EdgeCases\InversedOneToOneWithNonNullableOwning;
 
 use Doctrine\ORM\Mapping as ORM;
+use Zenstruck\Foundry\Tests\Fixture\Model\Base;
 
 /**
  * @author Nicolas PHILIPPE <[email protected]>
  */
 #[ORM\Entity]
 #[ORM\Table('inversed_one_to_one_with_non_nullable_owning_owning_side')]
-class OwningSide
+class OwningSide extends Base
 {
-    #[ORM\Id]
-    #[ORM\Column]
-    #[ORM\GeneratedValue(strategy: 'AUTO')]
-    public ?int $id = null;
-
     #[ORM\OneToOne(inversedBy: 'owningSide')]
     public ?InverseSide $inverseSide = null;
 }

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

@@ -0,0 +1,39 @@
+<?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\InversedOneToOneWithSetter;
+
+use Doctrine\ORM\Mapping as ORM;
+use Zenstruck\Foundry\Tests\Fixture\Model\Base;
+
+/**
+ * @author Nicolas PHILIPPE <[email protected]>
+ */
+#[ORM\Entity]
+#[ORM\Table('inversed_one_to_one_with_setter_inverse_side')]
+class InverseSide extends Base
+{
+    #[ORM\OneToOne(mappedBy: 'inverseSide')]
+    private OwningSide|null $owningSide = null;
+
+    public function getOwningSide(): ?OwningSide
+    {
+        return $this->owningSide;
+    }
+
+    public function setOwningSide(OwningSide $owningSide): void
+    {
+        $this->owningSide = $owningSide;
+        $owningSide->inverseSide = $this;
+    }
+}

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

@@ -0,0 +1,28 @@
+<?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\InversedOneToOneWithSetter;
+
+use Doctrine\ORM\Mapping as ORM;
+use Zenstruck\Foundry\Tests\Fixture\Model\Base;
+
+/**
+ * @author Nicolas PHILIPPE <[email protected]>
+ */
+#[ORM\Entity]
+#[ORM\Table('inversed_one_to_one_with_setter_owning_side')]
+class OwningSide extends Base
+{
+    #[ORM\OneToOne(inversedBy: 'owningSide')]
+    public ?InverseSide $inverseSide = null;
+}

tests/Integration/ORM/EdgeCasesRelationshipTest.php

@@ -21,6 +21,7 @@
 use Zenstruck\Foundry\Test\ResetDatabase;
 use Zenstruck\Foundry\Tests\Fixture\DoctrineCascadeRelationship\ChangesEntityRelationshipCascadePersist;
 use Zenstruck\Foundry\Tests\Fixture\DoctrineCascadeRelationship\UsingRelationships;
+use Zenstruck\Foundry\Tests\Fixture\Entity\EdgeCases\InversedOneToOneWithSetter;
 use Zenstruck\Foundry\Tests\Fixture\Entity\EdgeCases\InversedOneToOneWithNonNullableOwning;
 use Zenstruck\Foundry\Tests\Fixture\Entity\EdgeCases\ManyToOneToSelfReferencing;
 use Zenstruck\Foundry\Tests\Fixture\Entity\EdgeCases\RelationshipWithGlobalEntity;
@@ -86,9 +87,11 @@ public function inversed_relationship_mandatory(): void
         $inversedSideEntityFactory::assert()->count(1);
     }
 
-    /**
-     * @test
-     */
+    /** @test */
+    #[Test]
+    #[DataProvider('provideCascadeRelationshipsCombinations')]
+    #[UsingRelationships(InversedOneToOneWithNonNullableOwning\OwningSide::class, ['inverseSide'])]
+    #[RequiresPhpunit('^11.4')]
     public function inverse_one_to_one_with_non_nullable_inverse_side(): void
     {
         $owningSideFactory = persistent_factory(InversedOneToOneWithNonNullableOwning\OwningSide::class);
@@ -102,6 +105,24 @@ public function inverse_one_to_one_with_non_nullable_inverse_side(): void
         self::assertSame($inverseSide, $inverseSide->owningSide->inverseSide);
     }
 
+    /** @test */
+    #[Test]
+    #[DataProvider('provideCascadeRelationshipsCombinations')]
+    #[UsingRelationships(InversedOneToOneWithSetter\OwningSide::class, ['inverseSide'])]
+    #[RequiresPhpunit('^11.4')]
+    public function inverse_one_to_one_with_both_nullable(): void
+    {
+        $owningSideFactory = persistent_factory(InversedOneToOneWithSetter\OwningSide::class);
+        $inverseSideFactory = persistent_factory(InversedOneToOneWithSetter\InverseSide::class);
+
+        $inverseSide = $inverseSideFactory->create(['owningSide' => $owningSideFactory]);
+
+        $owningSideFactory::assert()->count(1);
+        $inverseSideFactory::assert()->count(1);
+
+        self::assertSame($inverseSide, $inverseSide->getOwningSide()?->inverseSide);
+    }
+
     /**
      * @test
      */