fix: one to one no more uses placeholder

Author: nikophil

Diff for: src/Persistence/PersistenceManager.php

@@ -111,17 +111,6 @@ public function scheduleForInsert(object $object, array $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
      *

Diff for: src/Persistence/PersistentObjectFactory.php

@@ -23,6 +23,7 @@
 use Zenstruck\Foundry\Persistence\Exception\NotEnoughObjects;
 use Zenstruck\Foundry\Persistence\Exception\RefreshObjectFailed;
 
+use function Zenstruck\Foundry\force;
 use function Zenstruck\Foundry\get;
 use function Zenstruck\Foundry\set;
 
@@ -290,30 +291,40 @@ protected function normalizeParameter(string $field, mixed $value): mixed
         if ($value instanceof self) {
             $pm = Configuration::instance()->persistence();
 
-            $inversedRelationshipMetadata = $pm->inverseRelationshipMetadata(static::class(), $value::class(), $field);
+            $relationshipMetadata = $pm->inverseRelationshipMetadata(static::class(), $value::class(), $field);
 
-            // handle inversed OneToOne
-            if ($inversedRelationshipMetadata && !$inversedRelationshipMetadata->isCollection) {
-                $inverseField = $inversedRelationshipMetadata->inverseField;
+            // handle inverse OneToOne
+            if ($relationshipMetadata && !$relationshipMetadata->isCollection) {
+                $inverseField = $relationshipMetadata->inverseField;
 
-                $inversedObject = $value->withPersistMode(
+                $value = $value->withPersistMode(
                     $this->isPersisting() ? PersistMode::NO_PERSIST_BUT_SCHEDULE_FOR_INSERT : PersistMode::WITHOUT_PERSISTING
-                )
+                );
+
+                if ((new \ReflectionClass(static::class()))->getProperty($field)->getType()?->allowsNull()) {
+                    $this->tempAfterInstantiate[] = static function(object $object) use ($value, $inverseField, $field) {
+                        $inverseObject = $value->create([$inverseField => $object]);
 
-                    // 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(),
-                    ]);
+                        set($object, $field, unproxy($inverseObject, withAutoRefresh: false));
+                    };
 
-                $inversedObject = unproxy($inversedObject, withAutoRefresh: false);
+                    // we're using "force" here to avoid a potential type check in a setter
+                    return force(null);
+                } elseif ((new \ReflectionClass($value::class()))->getProperty($inverseField)->getType()?->allowsNull()) {
+                    $inverseObject = unproxy(
+                        // we're using "force" here to avoid a potential type check in a setter
+                        $value->create([$inverseField => force(null)]),
+                        withAutoRefresh: false
+                    );
 
-                $this->tempAfterInstantiate[] = static function(object $object) use ($inversedObject, $inverseField, $pm, $placeholder) {
-                    $pm->forget($placeholder);
-                    set($inversedObject, $inverseField, $object);
-                };
+                    $this->tempAfterInstantiate[] = static function(object $object) use ($inverseObject, $inverseField) {
+                        set($inverseObject, $inverseField, $object);
+                    };
 
-                return $inversedObject;
+                    return $inverseObject;
+                } else {
+                    throw new \InvalidArgumentException('Cannot handle inverse OneToOne relationship because both side are not nullable, which will result in a circular dependency.');
+                }
             }
         }
 
@@ -352,7 +363,7 @@ protected function normalizeCollection(string $field, FactoryCollection $collect
                 set($object, $field, $inverseObjects);
             };
 
-            // creation delegated to afterPersist hook - return empty array here
+            // creation delegated to tempAfterInstantiate hook - return empty array here
             return [];
         }
 

Diff for: tests/Fixture/Entity/EdgeCases/InversedOneToOneWithManyToOne/InverseSide.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\InversedOneToOneWithManyToOne;
+
+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_many_to_one_inverse_side')]
+class InverseSide extends Base
+{
+    #[ORM\OneToOne(mappedBy: 'inverseSide')]
+    public ?OwningSide $owningSide = null;
+
+    #[ORM\ManyToOne()]
+    public ?Item $item = null;
+
+    public function __construct(
+        #[ORM\Column()]
+        public string $mandatoryField,
+    ) {
+    }
+}

Diff for: tests/Fixture/Entity/EdgeCases/InversedOneToOneWithManyToOne/Item.php

@@ -0,0 +1,26 @@
+<?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\InversedOneToOneWithManyToOne;
+
+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_many_to_one_item_if_collection')]
+class Item extends Base
+{
+}

Diff for: tests/Fixture/Entity/EdgeCases/InversedOneToOneWithManyToOne/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\InversedOneToOneWithManyToOne;
+
+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_many_to_one_owning_side')]
+class OwningSide extends Base
+{
+    #[ORM\OneToOne(inversedBy: 'owningSide')]
+    public ?InverseSide $inverseSide = null;
+}

Diff for: tests/Fixture/Entity/EdgeCases/InversedOneToOneWithNonNullableOwning/InverseSide.php

@@ -25,7 +25,18 @@ class InverseSide extends Base
 {
     public function __construct(
         #[ORM\OneToOne(mappedBy: 'inverseSide')] // @phpstan-ignore doctrine.associationType
-        public OwningSide $owningSide,
+        private OwningSide $owningSide,
     ) {
     }
+
+    public function getOwningSide(): OwningSide
+    {
+        return $this->owningSide;
+    }
+
+    public function setOwningSide(OwningSide $owningSide): void
+    {
+        $this->owningSide = $owningSide;
+        $owningSide->inverseSide = $this;
+    }
 }

Diff for: 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();
+    }
+}

Diff for: 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();
+    }
+}

Diff for: tests/Integration/ORM/EdgeCasesRelationshipTest.php

@@ -23,8 +23,10 @@
 use Zenstruck\Foundry\Tests\Fixture\DoctrineCascadeRelationship\ChangesEntityRelationshipCascadePersist;
 use Zenstruck\Foundry\Tests\Fixture\DoctrineCascadeRelationship\UsingRelationships;
 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\InversedOneToOneWithOneToMany;
+use Zenstruck\Foundry\Tests\Fixture\Entity\EdgeCases\InversedOneToOneWithoutAutoGeneratedId;
 use Zenstruck\Foundry\Tests\Fixture\Entity\EdgeCases\InversedOneToOneWithSetter;
 use Zenstruck\Foundry\Tests\Fixture\Entity\EdgeCases\ManyToOneToSelfReferencing;
 use Zenstruck\Foundry\Tests\Fixture\Entity\EdgeCases\OneToManyWithUnionType;
@@ -75,7 +77,7 @@ public function inverse_one_to_one_with_non_nullable_inverse_side(): void
         $owningSideFactory::assert()->count(1);
         $inverseSideFactory::assert()->count(1);
 
-        self::assertSame($inverseSide, $inverseSide->owningSide->inverseSide);
+        self::assertSame($inverseSide, $inverseSide->getOwningSide()->inverseSide);
     }
 
     /** @test */
@@ -101,7 +103,7 @@ public function inverse_one_to_one_with_both_nullable(): void
     #[DataProvider('provideCascadeRelationshipsCombinations')]
     #[UsingRelationships(InversedOneToOneWithOneToMany\OwningSide::class, ['inverseSide'])]
     #[UsingRelationships(InversedOneToOneWithOneToMany\Item::class, ['owningSide'])]
-    #[RequiresPhpunit('^11.4')]
+    #[RequiresPhpunit('>=11.4')]
     public function inverse_one_to_one_with_one_to_many(): void
     {
         $inverseSideFactory = persistent_factory(InversedOneToOneWithOneToMany\InverseSide::class);
@@ -142,7 +144,7 @@ public function many_to_many_to_self_referencing_inverse_side(): void
     #[Test]
     #[DataProvider('provideCascadeRelationshipsCombinations')]
     #[UsingRelationships(IndexedOneToMany\ParentEntity::class, ['items'])]
-    #[RequiresPhpunit('^11.4')]
+    #[RequiresPhpunit('>=11.4')]
     public function indexed_one_to_many(): void
     {
         $parentFactory = persistent_factory(IndexedOneToMany\ParentEntity::class);
@@ -162,7 +164,56 @@ public function indexed_one_to_many(): void
 
     /** @test */
     #[Test]
-    public function object_with_union_type(): void
+    #[DataProvider('provideCascadeRelationshipsCombinations')]
+    #[UsingRelationships(InversedOneToOneWithManyToOne\InverseSide::class, ['owningSide', 'item'])]
+    #[RequiresPhpunit('>=11.4')]
+    public function inversed_one_to_one_can_be_used_after_other_relationship(): void
+    {
+        $inverseSideFactory = persistent_factory(InversedOneToOneWithManyToOne\InverseSide::class);
+        $owningSideFactory = persistent_factory(InversedOneToOneWithManyToOne\OwningSide::class);
+        $itemFactory = persistent_factory(InversedOneToOneWithManyToOne\Item::class);
+
+        $inverseSide = $inverseSideFactory->create(
+            [
+                'mandatoryField' => 'foo',
+                'owningSide' => $owningSideFactory,
+                'item' => $itemFactory,
+            ]
+        );
+
+        $inverseSideFactory::assert()->count(1);
+        $owningSideFactory::assert()->count(1);
+        $itemFactory::assert()->count(1);
+
+        self::assertNotNull($inverseSide->owningSide);
+        self::assertSame($inverseSide, $inverseSide->owningSide->inverseSide);
+        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 */
+    #[Test]
+    #[DataProvider('provideCascadeRelationshipsCombinations')]
+    #[UsingRelationships(OneToManyWithUnionType\OwningSideEntity::class, ['item'])]
+    #[RequiresPhpunit('>=11.4')]
+    public function after_instantiate_flushing_using_current_object_in_relationship_one_to_one(): void
     {
         $owningSideFactory = persistent_factory(OneToManyWithUnionType\OwningSideEntity::class);
         $hasOneToManyWithUnionTypeFactory = persistent_factory(OneToManyWithUnionType\HasOneToManyWithUnionType::class);