From 28715d1a53bb209cc3609acf0fa899aeae1ac8e3 Mon Sep 17 00:00:00 2001 From: Nicolas PHILIPPE Date: Sun, 2 Mar 2025 12:02:14 +0100 Subject: [PATCH 1/5] fix: flush only once per Factory::create() calls in userland --- src/Persistence/PersistenceManager.php | 22 +++++------ src/Persistence/PersistentObjectFactory.php | 15 +++++++- .../InverseSide.php | 37 +++++++++++++++++++ .../InversedOneToOneWithManyToOne/Item.php | 26 +++++++++++++ .../OwningSide.php | 28 ++++++++++++++ .../ORM/EdgeCasesRelationshipTest.php | 29 +++++++++++++++ .../EntityFactoryRelationshipTestCase.php | 34 +++++++++++++++++ .../Persistence/GenericFactoryTestCase.php | 20 ++++++++++ 8 files changed, 199 insertions(+), 12 deletions(-) create mode 100644 tests/Fixture/Entity/EdgeCases/InversedOneToOneWithManyToOne/InverseSide.php create mode 100644 tests/Fixture/Entity/EdgeCases/InversedOneToOneWithManyToOne/Item.php create mode 100644 tests/Fixture/Entity/EdgeCases/InversedOneToOneWithManyToOne/OwningSide.php diff --git a/src/Persistence/PersistenceManager.php b/src/Persistence/PersistenceManager.php index 8a39e611..aa60b40f 100644 --- a/src/Persistence/PersistenceManager.php +++ b/src/Persistence/PersistenceManager.php @@ -75,17 +75,6 @@ public function save(object $object): object $om->persist($object); $this->flush($om); - if ($this->afterPersistCallbacks) { - $afterPersistCallbacks = $this->afterPersistCallbacks; - $this->afterPersistCallbacks = []; - - foreach ($afterPersistCallbacks as $afterPersistCallback) { - $afterPersistCallback(); - } - - $this->save($object); - } - return $object; } @@ -150,6 +139,17 @@ public function flush(ObjectManager $om): void { if ($this->flush) { $om->flush(); + + if ($this->afterPersistCallbacks) { + $afterPersistCallbacks = $this->afterPersistCallbacks; + $this->afterPersistCallbacks = []; + + foreach ($afterPersistCallbacks as $afterPersistCallback) { + $afterPersistCallback(); + } + + $this->flush($om); + } } } diff --git a/src/Persistence/PersistentObjectFactory.php b/src/Persistence/PersistentObjectFactory.php index 1a9f6ac2..8aadf1fd 100644 --- a/src/Persistence/PersistentObjectFactory.php +++ b/src/Persistence/PersistentObjectFactory.php @@ -44,6 +44,8 @@ abstract class PersistentObjectFactory extends ObjectFactory /** @var list */ private array $tempAfterInstantiate = []; + private bool $isRootFactory = true; + /** * @phpstan-param mixed|Parameters $criteriaOrId * @@ -206,7 +208,7 @@ public function create(callable|array $attributes = []): object $this->throwIfCannotCreateObject(); - if (PersistMode::PERSIST !== $this->persistMode()) { + if (PersistMode::PERSIST !== $this->persistMode() || !$this->isRootFactory) { return $object; } @@ -299,6 +301,7 @@ protected function normalizeParameter(string $field, mixed $value): mixed $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 @@ -314,6 +317,8 @@ protected function normalizeParameter(string $field, mixed $value): mixed }; return $inversedObject; + } else { + $value = $value->notRootFactory(); } } @@ -450,4 +455,12 @@ private function throwIfCannotCreateObject(): void throw new \LogicException(\sprintf('Cannot create object in a data provider for non-proxy factories. Transform your factory into a "%s", or call "create()" method in the test. See https://symfony.com/bundles/ZenstruckFoundryBundle/current/index.html#phpunit-data-providers', PersistentProxyObjectFactory::class)); } + + private function notRootFactory(): static + { + $clone = clone $this; + $clone->isRootFactory = false; + + return $clone; + } } diff --git a/tests/Fixture/Entity/EdgeCases/InversedOneToOneWithManyToOne/InverseSide.php b/tests/Fixture/Entity/EdgeCases/InversedOneToOneWithManyToOne/InverseSide.php new file mode 100644 index 00000000..85409021 --- /dev/null +++ b/tests/Fixture/Entity/EdgeCases/InversedOneToOneWithManyToOne/InverseSide.php @@ -0,0 +1,37 @@ + + * + * 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 + */ +#[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 --git a/tests/Fixture/Entity/EdgeCases/InversedOneToOneWithManyToOne/Item.php b/tests/Fixture/Entity/EdgeCases/InversedOneToOneWithManyToOne/Item.php new file mode 100644 index 00000000..cbd15e0e --- /dev/null +++ b/tests/Fixture/Entity/EdgeCases/InversedOneToOneWithManyToOne/Item.php @@ -0,0 +1,26 @@ + + * + * 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 + */ +#[ORM\Entity] +#[ORM\Table('inversed_one_to_one_with_many_to_one_item_if_collection')] +class Item extends Base +{ +} diff --git a/tests/Fixture/Entity/EdgeCases/InversedOneToOneWithManyToOne/OwningSide.php b/tests/Fixture/Entity/EdgeCases/InversedOneToOneWithManyToOne/OwningSide.php new file mode 100644 index 00000000..11b0b225 --- /dev/null +++ b/tests/Fixture/Entity/EdgeCases/InversedOneToOneWithManyToOne/OwningSide.php @@ -0,0 +1,28 @@ + + * + * 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 + */ +#[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 --git a/tests/Integration/ORM/EdgeCasesRelationshipTest.php b/tests/Integration/ORM/EdgeCasesRelationshipTest.php index 82b47243..07a2b163 100644 --- a/tests/Integration/ORM/EdgeCasesRelationshipTest.php +++ b/tests/Integration/ORM/EdgeCasesRelationshipTest.php @@ -23,6 +23,7 @@ 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\InversedOneToOneWithSetter; @@ -160,6 +161,34 @@ public function indexed_one_to_many(): void self::assertNotNull($parent->getItems()->get('en')); // @phpstan-ignore argument.type } + /** @test */ + #[Test] + #[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] public function object_with_union_type(): void diff --git a/tests/Integration/ORM/EntityRelationship/EntityFactoryRelationshipTestCase.php b/tests/Integration/ORM/EntityRelationship/EntityFactoryRelationshipTestCase.php index 844b74a9..0a9871c2 100644 --- a/tests/Integration/ORM/EntityRelationship/EntityFactoryRelationshipTestCase.php +++ b/tests/Integration/ORM/EntityRelationship/EntityFactoryRelationshipTestCase.php @@ -519,6 +519,40 @@ public function can_call_create_in_after_persist_callback(): void self::assertSame(unproxy($category), $category->getContacts()[0]?->getCategory()); } + /** @test */ + #[Test] + public function can_use_nested_after_persist_callback(): void + { + $contact = static::contactFactory()::createOne( + [ + 'address' => static::addressFactory() + ->afterPersist(function(Address $address) { + $address->setCity('city from after persist'); + }), + ] + ); + + self::assertSame('city from after persist', $contact->getAddress()->getCity()); + } + + /** @test */ + #[Test] + public function can_call_create_in_nested_after_persist_callback(): void + { + $contact = static::contactFactory()::createOne( + [ + 'category' => static::categoryFactory() + ->afterPersist(function(Category $category) { + $category->addSecondaryContact( + unproxy(static::contactFactory()::createOne()) + ); + }), + ] + ); + + self::assertCount(1, $contact->getCategory()?->getSecondaryContacts() ?? []); + } + /** @return PersistentObjectFactory */ protected static function contactFactoryWithoutCategory(): PersistentObjectFactory { diff --git a/tests/Integration/Persistence/GenericFactoryTestCase.php b/tests/Integration/Persistence/GenericFactoryTestCase.php index 4163cbad..eb51379f 100644 --- a/tests/Integration/Persistence/GenericFactoryTestCase.php +++ b/tests/Integration/Persistence/GenericFactoryTestCase.php @@ -11,6 +11,7 @@ namespace Zenstruck\Foundry\Tests\Integration\Persistence; +use PHPUnit\Framework\Attributes\Test; use Symfony\Bundle\FrameworkBundle\Test\KernelTestCase; use Zenstruck\Foundry\Configuration; use Zenstruck\Foundry\Exception\PersistenceDisabled; @@ -564,6 +565,25 @@ public function can_use_after_persist_with_attributes(): void $this->assertSame(1, $object->getPropInteger()); } + /** + * @test + */ + #[Test] + public function it_actually_calls_post_persist_hook_after_persist_when_in_flush_after(): void + { + $object = flush_after( + function () { + return static::factory()->afterPersist( + static function (GenericModel $o) { + $o->setProp1((string) $o->id); + } + )->create(); + } + ); + + self::assertSame((string) $object->id, $object->getProp1()); + } + /** * @return class-string */ From a4d38a882b6e3d33422746ca66b81f362823e861 Mon Sep 17 00:00:00 2001 From: Nicolas PHILIPPE Date: Thu, 6 Mar 2025 08:57:16 +0100 Subject: [PATCH 2/5] fix: only call om::persist() once all objects are created --- src/Persistence/PersistenceManager.php | 111 +++++++++++++----- src/Persistence/PersistentObjectFactory.php | 34 +++--- .../InverseSide.php | 2 +- .../InverseSide.php | 36 ++++++ .../OwningSide.php | 37 ++++++ .../ORM/EdgeCasesRelationshipTest.php | 23 +++- .../EntityFactoryRelationshipTestCase.php | 52 ++++++++ .../Persistence/GenericFactoryTestCase.php | 4 +- 8 files changed, 248 insertions(+), 51 deletions(-) create mode 100644 tests/Fixture/Entity/EdgeCases/InversedOneToOneWithoutAutoGeneratedId/InverseSide.php create mode 100644 tests/Fixture/Entity/EdgeCases/InversedOneToOneWithoutAutoGeneratedId/OwningSide.php diff --git a/src/Persistence/PersistenceManager.php b/src/Persistence/PersistenceManager.php index aa60b40f..93cc4cf4 100644 --- a/src/Persistence/PersistenceManager.php +++ b/src/Persistence/PersistenceManager.php @@ -31,7 +31,10 @@ final class PersistenceManager private bool $flush = true; private bool $persist = true; - /** @var list */ + /** @var array> */ + private array $objectsToPersist = []; + + /** @var array> */ private array $afterPersistCallbacks = []; /** @@ -78,6 +81,45 @@ public function save(object $object): object return $object; } + /** + * We're using so-called "transactions" to group multiple persist/flush operations + * This prevents such code to persist the whole batch of objects in the normalization phase: + * ```php + * SomeFactory::createOne(['item' => lazy(fn() => OtherFactory::createOne())]); + * ```. + */ + public function startTransaction(): void + { + $this->objectsToPersist[] = []; + $this->afterPersistCallbacks[] = []; + } + + public function commit(): void + { + $objectManagers = []; + + $objectsToPersist = \array_pop($this->objectsToPersist); + + if (null === $objectsToPersist) { + return; + } + + foreach ($objectsToPersist as $object) { + $om = $this->strategyFor($object::class)->objectManagerFor($object::class); + $om->persist($object); + + if (!\in_array($om, $objectManagers, true)) { + $objectManagers[] = $om; + } + } + + foreach ($objectManagers as $om) { + $this->flush($om); + } + + $this->callPostPersistCallbacks(); + } + /** * @template T of object * @@ -92,23 +134,19 @@ public function scheduleForInsert(object $object, array $afterPersistCallbacks = $object = unproxy($object); } - $om = $this->strategyFor($object::class)->objectManagerFor($object::class); - $om->persist($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.'); + if (0 === \count($this->objectsToPersist)) { + throw new \LogicException('No transaction started yet.'); } - $om = $this->strategyFor($object::class)->objectManagerFor($object::class); + $transactionCount = \count($this->objectsToPersist) - 1; + $this->objectsToPersist[$transactionCount][] = $object; + + $this->afterPersistCallbacks[$transactionCount] = [ + ...$this->afterPersistCallbacks[$transactionCount], + ...$afterPersistCallbacks, + ]; - $om->detach($object); + return $object; } /** @@ -126,11 +164,9 @@ public function flushAfter(callable $callback): mixed $this->flush = true; - foreach ($this->strategies as $strategy) { - foreach ($strategy->objectManagers() as $om) { - $this->flush($om); - } - } + $this->flushAllStrategies(); + + $this->callPostPersistCallbacks(); return $result; } @@ -139,17 +175,6 @@ public function flush(ObjectManager $om): void { if ($this->flush) { $om->flush(); - - if ($this->afterPersistCallbacks) { - $afterPersistCallbacks = $this->afterPersistCallbacks; - $this->afterPersistCallbacks = []; - - foreach ($afterPersistCallbacks as $afterPersistCallback) { - $afterPersistCallback(); - } - - $this->flush($om); - } } } @@ -372,6 +397,30 @@ public static function isOrmOnly(): bool })(); } + private function flushAllStrategies(): void + { + foreach ($this->strategies as $strategy) { + foreach ($strategy->objectManagers() as $om) { + $this->flush($om); + } + } + } + + private function callPostPersistCallbacks(): void + { + if (!$this->flush || [] === $this->afterPersistCallbacks) { + return; + } + + $afterPersistCallbacks = \array_pop($this->afterPersistCallbacks); + + foreach ($afterPersistCallbacks as $afterPersistCallback) { + $afterPersistCallback(); + } + + $this->flushAllStrategies(); + } + /** * @param class-string $class * diff --git a/src/Persistence/PersistentObjectFactory.php b/src/Persistence/PersistentObjectFactory.php index 8aadf1fd..949e40c1 100644 --- a/src/Persistence/PersistentObjectFactory.php +++ b/src/Persistence/PersistentObjectFactory.php @@ -198,6 +198,10 @@ final public static function truncate(): void */ public function create(callable|array $attributes = []): object { + if (PersistMode::PERSIST === $this->persistMode() && $this->isRootFactory) { + Configuration::instance()->persistence()->startTransaction(); + } + $object = parent::create($attributes); foreach ($this->tempAfterInstantiate as $callback) { @@ -218,7 +222,7 @@ public function create(callable|array $attributes = []): object throw new \LogicException('Persistence cannot be used in unit tests.'); } - $configuration->persistence()->save($object); + $configuration->persistence()->commit(); return $object; } @@ -298,25 +302,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(); } @@ -389,6 +391,8 @@ protected function normalizeObject(object $object): object if (!$persistenceManager->isPersisted($object)) { $persistenceManager->scheduleForInsert($object); + + return $object; } try { diff --git a/tests/Fixture/Entity/EdgeCases/InversedOneToOneWithManyToOne/InverseSide.php b/tests/Fixture/Entity/EdgeCases/InversedOneToOneWithManyToOne/InverseSide.php index 85409021..46bb9540 100644 --- a/tests/Fixture/Entity/EdgeCases/InversedOneToOneWithManyToOne/InverseSide.php +++ b/tests/Fixture/Entity/EdgeCases/InversedOneToOneWithManyToOne/InverseSide.php @@ -31,7 +31,7 @@ class InverseSide extends Base public function __construct( #[ORM\Column()] - public string $mandatoryField + public string $mandatoryField, ) { } } diff --git a/tests/Fixture/Entity/EdgeCases/InversedOneToOneWithoutAutoGeneratedId/InverseSide.php b/tests/Fixture/Entity/EdgeCases/InversedOneToOneWithoutAutoGeneratedId/InverseSide.php new file mode 100644 index 00000000..c4240c45 --- /dev/null +++ b/tests/Fixture/Entity/EdgeCases/InversedOneToOneWithoutAutoGeneratedId/InverseSide.php @@ -0,0 +1,36 @@ + + * + * 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 + */ +#[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 --git a/tests/Fixture/Entity/EdgeCases/InversedOneToOneWithoutAutoGeneratedId/OwningSide.php b/tests/Fixture/Entity/EdgeCases/InversedOneToOneWithoutAutoGeneratedId/OwningSide.php new file mode 100644 index 00000000..46504419 --- /dev/null +++ b/tests/Fixture/Entity/EdgeCases/InversedOneToOneWithoutAutoGeneratedId/OwningSide.php @@ -0,0 +1,37 @@ + + * + * 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 + */ +#[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 --git a/tests/Integration/ORM/EdgeCasesRelationshipTest.php b/tests/Integration/ORM/EdgeCasesRelationshipTest.php index 07a2b163..42b0857f 100644 --- a/tests/Integration/ORM/EdgeCasesRelationshipTest.php +++ b/tests/Integration/ORM/EdgeCasesRelationshipTest.php @@ -26,6 +26,7 @@ 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; @@ -102,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); @@ -143,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); @@ -189,6 +190,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 */ #[Test] public function object_with_union_type(): void diff --git a/tests/Integration/ORM/EntityRelationship/EntityFactoryRelationshipTestCase.php b/tests/Integration/ORM/EntityRelationship/EntityFactoryRelationshipTestCase.php index 0a9871c2..1e8d61c1 100644 --- a/tests/Integration/ORM/EntityRelationship/EntityFactoryRelationshipTestCase.php +++ b/tests/Integration/ORM/EntityRelationship/EntityFactoryRelationshipTestCase.php @@ -31,6 +31,7 @@ use Zenstruck\Foundry\Tests\Fixture\Entity\Contact; use Zenstruck\Foundry\Tests\Fixture\Entity\Tag; +use function Zenstruck\Foundry\lazy; use function Zenstruck\Foundry\Persistence\refresh; use function Zenstruck\Foundry\Persistence\unproxy; @@ -553,6 +554,57 @@ public function can_call_create_in_nested_after_persist_callback(): void self::assertCount(1, $contact->getCategory()?->getSecondaryContacts() ?? []); } + /** @test */ + #[Test] + #[DataProvider('provideCascadeRelationshipsCombinations')] + #[UsingRelationships(Address::class, ['contact'])] + #[UsingRelationships(Contact::class, ['category'])] + public function inverse_one_to_one_with_flush_in_before_instantiate(): void + { + $address = static::addressFactory()::createOne( + [ + 'contact' => static::contactFactory() + ->beforeInstantiate( + function(array $attributes): array { + $attributes['category'] = static::categoryFactory()->create(); + + return $attributes; + } + ), + ] + ); + + static::addressFactory()::assert()->count(1); + static::contactFactory()::assert()->count(1); + static::categoryFactory()::assert()->count(1); + + self::assertNotNull($address->getContact()); + self::assertNotNull($address->getContact()->getCategory()); + } + + /** @test */ + #[Test] + #[DataProvider('provideCascadeRelationshipsCombinations')] + #[UsingRelationships(Address::class, ['contact'])] + #[UsingRelationships(Contact::class, ['category'])] + public function inverse_one_to_one_with_lazy_flush(): void + { + $address = static::addressFactory()::createOne( + [ + 'contact' => static::contactFactory()->with([ + 'category' => lazy(fn() => static::categoryFactory()->create()), + ]), + ] + ); + + static::addressFactory()::assert()->count(1); + static::contactFactory()::assert()->count(1); + static::categoryFactory()::assert()->count(1); + + self::assertNotNull($address->getContact()); + self::assertNotNull($address->getContact()->getCategory()); + } + /** @return PersistentObjectFactory */ protected static function contactFactoryWithoutCategory(): PersistentObjectFactory { diff --git a/tests/Integration/Persistence/GenericFactoryTestCase.php b/tests/Integration/Persistence/GenericFactoryTestCase.php index eb51379f..15d26968 100644 --- a/tests/Integration/Persistence/GenericFactoryTestCase.php +++ b/tests/Integration/Persistence/GenericFactoryTestCase.php @@ -572,9 +572,9 @@ public function can_use_after_persist_with_attributes(): void public function it_actually_calls_post_persist_hook_after_persist_when_in_flush_after(): void { $object = flush_after( - function () { + function() { return static::factory()->afterPersist( - static function (GenericModel $o) { + static function(GenericModel $o) { $o->setProp1((string) $o->id); } )->create(); From 0cb916ea4bfa213195d19fd369b11a368beab24c Mon Sep 17 00:00:00 2001 From: Nicolas PHILIPPE Date: Sun, 16 Mar 2025 17:25:59 +0100 Subject: [PATCH 3/5] wip --- src/Persistence/PersistenceManager.php | 41 +++++++++++-------- src/Persistence/PersistentObjectFactory.php | 7 +++- .../EntityFactoryRelationshipTestCase.php | 19 +++++++++ 3 files changed, 47 insertions(+), 20 deletions(-) diff --git a/src/Persistence/PersistenceManager.php b/src/Persistence/PersistenceManager.php index 93cc4cf4..f0c59cfa 100644 --- a/src/Persistence/PersistenceManager.php +++ b/src/Persistence/PersistenceManager.php @@ -31,12 +31,14 @@ final class PersistenceManager private bool $flush = true; private bool $persist = true; - /** @var array> */ + /** @var list */ private array $objectsToPersist = []; - /** @var array> */ + /** @var list */ private array $afterPersistCallbacks = []; + private bool $transactinStarted = false; + /** * @param iterable $strategies */ @@ -86,23 +88,25 @@ public function save(object $object): object * This prevents such code to persist the whole batch of objects in the normalization phase: * ```php * SomeFactory::createOne(['item' => lazy(fn() => OtherFactory::createOne())]); - * ```. + * ``` */ public function startTransaction(): void { - $this->objectsToPersist[] = []; - $this->afterPersistCallbacks[] = []; + $this->transactinStarted = true; + } + + public function isTransactionStarted(): bool + { + return $this->transactinStarted; } public function commit(): void { $objectManagers = []; - $objectsToPersist = \array_pop($this->objectsToPersist); - - if (null === $objectsToPersist) { - return; - } + $objectsToPersist = $this->objectsToPersist; + $this->objectsToPersist = []; + $this->transactinStarted = false; foreach ($objectsToPersist as $object) { $om = $this->strategyFor($object::class)->objectManagerFor($object::class); @@ -134,15 +138,15 @@ public function scheduleForInsert(object $object, array $afterPersistCallbacks = $object = unproxy($object); } - if (0 === \count($this->objectsToPersist)) { - throw new \LogicException('No transaction started yet.'); - } +// if (0 === \count($this->objectsToPersist)) { +// throw new \LogicException('No transaction started yet.'); +// } - $transactionCount = \count($this->objectsToPersist) - 1; - $this->objectsToPersist[$transactionCount][] = $object; +// $transactionCount = \count($this->objectsToPersist) - 1; + $this->objectsToPersist[] = $object; - $this->afterPersistCallbacks[$transactionCount] = [ - ...$this->afterPersistCallbacks[$transactionCount], + $this->afterPersistCallbacks = [ + ...$this->afterPersistCallbacks, ...$afterPersistCallbacks, ]; @@ -412,7 +416,8 @@ private function callPostPersistCallbacks(): void return; } - $afterPersistCallbacks = \array_pop($this->afterPersistCallbacks); + $afterPersistCallbacks = $this->afterPersistCallbacks; + $this->afterPersistCallbacks = []; foreach ($afterPersistCallbacks as $afterPersistCallback) { $afterPersistCallback(); diff --git a/src/Persistence/PersistentObjectFactory.php b/src/Persistence/PersistentObjectFactory.php index 949e40c1..8f1c84bc 100644 --- a/src/Persistence/PersistentObjectFactory.php +++ b/src/Persistence/PersistentObjectFactory.php @@ -198,7 +198,10 @@ final public static function truncate(): void */ public function create(callable|array $attributes = []): object { - if (PersistMode::PERSIST === $this->persistMode() && $this->isRootFactory) { + $transactionStarted = false; + + if (Configuration::isBooted() && PersistMode::PERSIST === $this->persistMode() && $this->isRootFactory) { + $transactionStarted = Configuration::instance()->persistence()->isTransactionStarted(); Configuration::instance()->persistence()->startTransaction(); } @@ -212,7 +215,7 @@ public function create(callable|array $attributes = []): object $this->throwIfCannotCreateObject(); - if (PersistMode::PERSIST !== $this->persistMode() || !$this->isRootFactory) { + if ($transactionStarted || PersistMode::PERSIST !== $this->persistMode() || !$this->isRootFactory) { return $object; } diff --git a/tests/Integration/ORM/EntityRelationship/EntityFactoryRelationshipTestCase.php b/tests/Integration/ORM/EntityRelationship/EntityFactoryRelationshipTestCase.php index 1e8d61c1..c8f61152 100644 --- a/tests/Integration/ORM/EntityRelationship/EntityFactoryRelationshipTestCase.php +++ b/tests/Integration/ORM/EntityRelationship/EntityFactoryRelationshipTestCase.php @@ -605,6 +605,25 @@ public function inverse_one_to_one_with_lazy_flush(): void self::assertNotNull($address->getContact()->getCategory()); } + /** @test */ + #[Test] + #[DataProvider('provideCascadeRelationshipsCombinations')] + #[UsingRelationships(Contact::class, ['category'])] + public function after_instantiate_flushing_using_current_object_in_relationship(): void + { + $category = static::categoryFactory() + ->afterInstantiate( + static fn(Category $c) => static::contactFactory()->create(['category' => $c]) + ) + ->create(); + + static::contactFactory()::assert()->count(1); + static::categoryFactory()::assert()->count(1); + + self::assertCount(1, $category->getContacts()); + self::assertNotNull($category->getContacts()[0] ?? null); + } + /** @return PersistentObjectFactory */ protected static function contactFactoryWithoutCategory(): PersistentObjectFactory { From 11c807529263df53d40ae2444cd83cd6e26dc769 Mon Sep 17 00:00:00 2001 From: Nicolas PHILIPPE Date: Sat, 22 Mar 2025 11:33:39 +0100 Subject: [PATCH 4/5] wip --- src/Persistence/PersistentObjectFactory.php | 8 +++- .../InverseSide.php | 14 ++++++- .../ORM/EdgeCasesRelationshipTest.php | 28 +++++++++++++- .../EntityFactoryRelationshipTestCase.php | 38 ++++++++++++++++++- 4 files changed, 84 insertions(+), 4 deletions(-) diff --git a/src/Persistence/PersistentObjectFactory.php b/src/Persistence/PersistentObjectFactory.php index 8f1c84bc..159488df 100644 --- a/src/Persistence/PersistentObjectFactory.php +++ b/src/Persistence/PersistentObjectFactory.php @@ -438,7 +438,13 @@ static function(object $object, array $parameters, PersistentObjectFactory $fact Configuration::instance()->persistence()->scheduleForInsert($object, $afterPersistCallbacks); } - ); + ) +// ->afterPersist( +// static function(object $object): void { +// Configuration::instance()->persistence()->refresh($object); +// } +// ) + ; } private function throwIfCannotCreateObject(): void diff --git a/tests/Fixture/Entity/EdgeCases/InversedOneToOneWithNonNullableOwning/InverseSide.php b/tests/Fixture/Entity/EdgeCases/InversedOneToOneWithNonNullableOwning/InverseSide.php index f93863fb..0ecb2db0 100644 --- a/tests/Fixture/Entity/EdgeCases/InversedOneToOneWithNonNullableOwning/InverseSide.php +++ b/tests/Fixture/Entity/EdgeCases/InversedOneToOneWithNonNullableOwning/InverseSide.php @@ -25,7 +25,19 @@ 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 + { + dump(__METHOD__.'()::'.__LINE__); + $this->owningSide = $owningSide; + $owningSide->inverseSide = $this; + } } diff --git a/tests/Integration/ORM/EdgeCasesRelationshipTest.php b/tests/Integration/ORM/EdgeCasesRelationshipTest.php index 42b0857f..28ddd303 100644 --- a/tests/Integration/ORM/EdgeCasesRelationshipTest.php +++ b/tests/Integration/ORM/EdgeCasesRelationshipTest.php @@ -13,6 +13,7 @@ namespace Zenstruck\Foundry\Tests\Integration\ORM; +use DAMA\DoctrineTestBundle\Doctrine\DBAL\StaticDriver; use Doctrine\Common\Collections\Collection; use PHPUnit\Framework\Attributes\DataProvider; use PHPUnit\Framework\Attributes\RequiresPhpunit; @@ -77,7 +78,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 */ @@ -208,6 +209,31 @@ public function inverse_one_to_one_with_custom_id(): void self::assertSame($inverseSide, $inverseSide->owningSide->inverseSide); } + /** @test */ + #[Test] + #[DataProvider('provideCascadeRelationshipsCombinations')] + #[UsingRelationships(InversedOneToOneWithNonNullableOwning\OwningSide::class, ['inverseSide'])] + #[RequiresPhpunit('>=11.4')] + public function after_instantiate_flushing_using_current_object_in_relationship_inversed_one_to_one(): void + { + $owningSideFactory = persistent_factory(InversedOneToOneWithNonNullableOwning\OwningSide::class); + $inverseSideFactory = persistent_factory(InversedOneToOneWithNonNullableOwning\InverseSide::class); + + $owningSide = $owningSideFactory + ->afterInstantiate( + static function (InversedOneToOneWithNonNullableOwning\OwningSide $o) use ($inverseSideFactory) { + $inverseSideFactory->create(['owningSide' => $o]); + } + ) + ->create(); + + $owningSideFactory::assert()->count(1); + $inverseSideFactory::assert()->count(1); + + self::assertNotNull($owningSide->inverseSide); + self::assertSame($owningSide, $owningSide->inverseSide->owningSide); + } + /** @test */ #[Test] public function object_with_union_type(): void diff --git a/tests/Integration/ORM/EntityRelationship/EntityFactoryRelationshipTestCase.php b/tests/Integration/ORM/EntityRelationship/EntityFactoryRelationshipTestCase.php index c8f61152..72158350 100644 --- a/tests/Integration/ORM/EntityRelationship/EntityFactoryRelationshipTestCase.php +++ b/tests/Integration/ORM/EntityRelationship/EntityFactoryRelationshipTestCase.php @@ -609,7 +609,7 @@ public function inverse_one_to_one_with_lazy_flush(): void #[Test] #[DataProvider('provideCascadeRelationshipsCombinations')] #[UsingRelationships(Contact::class, ['category'])] - public function after_instantiate_flushing_using_current_object_in_relationship(): void + public function after_instantiate_flushing_using_current_object_in_relationship_many_to_one(): void { $category = static::categoryFactory() ->afterInstantiate( @@ -624,6 +624,42 @@ public function after_instantiate_flushing_using_current_object_in_relationship( self::assertNotNull($category->getContacts()[0] ?? null); } + /** @test */ + #[Test] + #[DataProvider('provideCascadeRelationshipsCombinations')] + #[UsingRelationships(Contact::class, ['category'])] + public function after_instantiate_flushing_using_current_object_in_relationship_one_to_many(): void + { + $contact = static::contactFactory() + ->afterInstantiate( + static fn(Contact $c) => static::categoryFactory()->create(['contacts' => [$c]]) + ) + ->create(['category' => null]); + + static::contactFactory()::assert()->count(1); + static::categoryFactory()::assert()->count(1); + + self::assertNotNull($contact->getCategory()); + self::assertCount(1, $contact->getCategory()->getContacts()); + } + + /** @test */ + #[Test] + #[DataProvider('provideCascadeRelationshipsCombinations')] + #[UsingRelationships(Contact::class, ['address'])] + public function after_instantiate_flushing_using_current_object_in_relationship_one_to_one(): void + { + $address = static::addressFactory() + ->afterInstantiate( + static fn(Address $a) => static::contactFactory()->create(['address' => $a]) + )->create(); + + static::contactFactory()::assert()->count(1); + static::addressFactory()::assert()->count(1); + + self::assertNotNull($address->getContact()); + } + /** @return PersistentObjectFactory */ protected static function contactFactoryWithoutCategory(): PersistentObjectFactory { From 28ca1e35a409f09cf1a0669e7bc04af96e68c687 Mon Sep 17 00:00:00 2001 From: Nicolas PHILIPPE Date: Sat, 22 Mar 2025 11:04:12 +0100 Subject: [PATCH 5/5] wip --- src/Factory.php | 4 +- src/ORM/OrmV2PersistenceStrategy.php | 48 +++++++--------- src/ORM/OrmV3PersistenceStrategy.php | 45 ++++++++------- src/Persistence/PersistenceManager.php | 13 +++-- src/Persistence/PersistenceStrategy.php | 3 +- src/Persistence/PersistentObjectFactory.php | 56 +++++++++++++------ .../OneToManyRelationship.php} | 19 ++++--- .../Relationship/OneToOneRelationship.php | 28 ++++++++++ .../Relationship/RelationshipMetadata.php | 22 ++++++++ .../InverseSide.php | 1 - .../ORM/EdgeCasesRelationshipTest.php | 30 +++++++++- .../EntityFactoryRelationshipTestCase.php | 12 +++- 12 files changed, 187 insertions(+), 94 deletions(-) rename src/Persistence/{InverseRelationshipMetadata.php => Relationship/OneToManyRelationship.php} (55%) create mode 100644 src/Persistence/Relationship/OneToOneRelationship.php create mode 100644 src/Persistence/Relationship/RelationshipMetadata.php diff --git a/src/Factory.php b/src/Factory.php index 54e8ce06..de8add1b 100644 --- a/src/Factory.php +++ b/src/Factory.php @@ -235,7 +235,7 @@ protected function normalizeParameter(string $field, mixed $value): mixed ); } - return \is_object($value) ? $this->normalizeObject($value) : $value; + return \is_object($value) ? $this->normalizeObject($field, $value) : $value; } /** @@ -253,7 +253,7 @@ protected function normalizeCollection(string $field, FactoryCollection $collect /** * @internal */ - protected function normalizeObject(object $object): object + protected function normalizeObject(string $field, object $object): object { return $object; } diff --git a/src/ORM/OrmV2PersistenceStrategy.php b/src/ORM/OrmV2PersistenceStrategy.php index 7e2ae2a2..564aeec4 100644 --- a/src/ORM/OrmV2PersistenceStrategy.php +++ b/src/ORM/OrmV2PersistenceStrategy.php @@ -16,7 +16,9 @@ use Doctrine\ORM\Mapping\ClassMetadataInfo; use Doctrine\ORM\Mapping\MappingException as ORMMappingException; use Doctrine\Persistence\Mapping\MappingException; -use Zenstruck\Foundry\Persistence\InverseRelationshipMetadata; +use Zenstruck\Foundry\Persistence\Relationship\OneToManyRelationship; +use Zenstruck\Foundry\Persistence\Relationship\OneToOneRelationship; +use Zenstruck\Foundry\Persistence\Relationship\RelationshipMetadata; /** * @internal @@ -25,49 +27,39 @@ */ final class OrmV2PersistenceStrategy extends AbstractORMPersistenceStrategy { - public function inversedRelationshipMetadata(string $parent, string $child, string $field): ?InverseRelationshipMetadata + public function inversedRelationshipMetadata(string $parent, string $child, string $field): ?RelationshipMetadata { - $metadata = $this->classMetadata($child); + $associationMapping = $this->getAssociationMapping($parent, $child, $field); - $inversedAssociation = $this->getAssociationMapping($parent, $child, $field); - - if (null === $inversedAssociation || !$metadata instanceof ClassMetadataInfo) { + if (null === $associationMapping) { return null; } if (!\is_a( $child, - $inversedAssociation['targetEntity'], + $associationMapping['targetEntity'], allow_string: true )) { // is_a() handles inheritance as well throw new \LogicException("Cannot find correct association named \"{$field}\" between classes [parent: \"{$parent}\", child: \"{$child}\"]"); } - // exclude "owning" side of the association (owning OneToOne or ManyToOne) - if (!\in_array( - $inversedAssociation['type'], - [ClassMetadataInfo::ONE_TO_MANY, ClassMetadataInfo::ONE_TO_ONE], - true - ) - || !isset($inversedAssociation['mappedBy']) - ) { - return null; - } + $inverseField = $associationMapping['isOwningSide'] ? $associationMapping['inversedBy'] ?? null : $associationMapping['mappedBy'] ?? null; - $association = $metadata->getAssociationMapping($inversedAssociation['mappedBy']); - - // only keep *ToOne associations - if (!$metadata->isSingleValuedAssociation($association['fieldName'])) { + if (null === $inverseField) { return null; } - $inversedAssociationMetadata = $this->classMetadata($inversedAssociation['sourceEntity']); - - return new InverseRelationshipMetadata( - inverseField: $association['fieldName'], - isCollection: $inversedAssociationMetadata->isCollectionValuedAssociation($inversedAssociation['fieldName']), - collectionIndexedBy: $inversedAssociation['indexBy'] ?? null - ); + return match (true) { + ClassMetadataInfo::ONE_TO_MANY === $associationMapping['type'] => new OneToManyRelationship( + inverseField: $inverseField, + collectionIndexedBy: $associationMapping['indexBy'] ?? null + ), + ClassMetadataInfo::ONE_TO_ONE === $associationMapping['type'] => new OneToOneRelationship( + inverseField: $inverseField, + isOwning: $associationMapping['isOwningSide'] ?? false + ), + default => null, + }; } /** diff --git a/src/ORM/OrmV3PersistenceStrategy.php b/src/ORM/OrmV3PersistenceStrategy.php index e105b176..196df921 100644 --- a/src/ORM/OrmV3PersistenceStrategy.php +++ b/src/ORM/OrmV3PersistenceStrategy.php @@ -14,50 +14,49 @@ namespace Zenstruck\Foundry\ORM; use Doctrine\ORM\Mapping\AssociationMapping; -use Doctrine\ORM\Mapping\ClassMetadata; -use Doctrine\ORM\Mapping\InverseSideMapping; use Doctrine\ORM\Mapping\MappingException as ORMMappingException; -use Doctrine\ORM\Mapping\ToManyAssociationMapping; +use Doctrine\ORM\Mapping\OneToManyAssociationMapping; +use Doctrine\ORM\Mapping\OneToOneAssociationMapping; use Doctrine\Persistence\Mapping\MappingException; -use Zenstruck\Foundry\Persistence\InverseRelationshipMetadata; +use Zenstruck\Foundry\Persistence\Relationship\OneToManyRelationship; +use Zenstruck\Foundry\Persistence\Relationship\OneToOneRelationship; +use Zenstruck\Foundry\Persistence\Relationship\RelationshipMetadata; final class OrmV3PersistenceStrategy extends AbstractORMPersistenceStrategy { - public function inversedRelationshipMetadata(string $parent, string $child, string $field): ?InverseRelationshipMetadata + public function inversedRelationshipMetadata(string $parent, string $child, string $field): ?RelationshipMetadata { - $metadata = $this->classMetadata($child); + $associationMapping = $this->getAssociationMapping($parent, $child, $field); - $inversedAssociation = $this->getAssociationMapping($parent, $child, $field); - - if (null === $inversedAssociation || !$metadata instanceof ClassMetadata) { + if (null === $associationMapping) { return null; } if (!\is_a( $child, - $inversedAssociation->targetEntity, + $associationMapping->targetEntity, allow_string: true )) { // is_a() handles inheritance as well throw new \LogicException("Cannot find correct association named \"{$field}\" between classes [parent: \"{$parent}\", child: \"{$child}\"]"); } - // exclude "owning" side of the association (owning OneToOne or ManyToOne) - if (!$inversedAssociation instanceof InverseSideMapping) { - return null; - } - - $association = $metadata->getAssociationMapping($inversedAssociation->mappedBy); + $inverseField = $associationMapping->isOwningSide() ? $associationMapping->inversedBy : $associationMapping->mappedBy; - // only keep *ToOne associations - if (!$metadata->isSingleValuedAssociation($association->fieldName)) { + if (null === $inverseField) { return null; } - return new InverseRelationshipMetadata( - inverseField: $association->fieldName, - isCollection: $inversedAssociation instanceof ToManyAssociationMapping, - collectionIndexedBy: $inversedAssociation->isIndexed() ? $inversedAssociation->indexBy() : null - ); + return match (true) { + $associationMapping instanceof OneToManyAssociationMapping => new OneToManyRelationship( + inverseField: $inverseField, + collectionIndexedBy: $associationMapping->isIndexed() ? $associationMapping->indexBy() : null + ), + $associationMapping instanceof OneToOneAssociationMapping => new OneToOneRelationship( + inverseField: $inverseField, + isOwning: $associationMapping->isOwningSide() + ), + default => null, + }; } /** diff --git a/src/Persistence/PersistenceManager.php b/src/Persistence/PersistenceManager.php index f0c59cfa..82d1facf 100644 --- a/src/Persistence/PersistenceManager.php +++ b/src/Persistence/PersistenceManager.php @@ -19,6 +19,7 @@ use Zenstruck\Foundry\ORM\AbstractORMPersistenceStrategy; use Zenstruck\Foundry\Persistence\Exception\NoPersistenceStrategy; use Zenstruck\Foundry\Persistence\Exception\RefreshObjectFailed; +use Zenstruck\Foundry\Persistence\Relationship\RelationshipMetadata; use Zenstruck\Foundry\Persistence\ResetDatabase\ResetDatabaseManager; /** @@ -88,7 +89,7 @@ public function save(object $object): object * This prevents such code to persist the whole batch of objects in the normalization phase: * ```php * SomeFactory::createOne(['item' => lazy(fn() => OtherFactory::createOne())]); - * ``` + * ```. */ public function startTransaction(): void { @@ -138,11 +139,11 @@ public function scheduleForInsert(object $object, array $afterPersistCallbacks = $object = unproxy($object); } -// if (0 === \count($this->objectsToPersist)) { -// throw new \LogicException('No transaction started yet.'); -// } + // if (0 === \count($this->objectsToPersist)) { + // throw new \LogicException('No transaction started yet.'); + // } -// $transactionCount = \count($this->objectsToPersist) - 1; + // $transactionCount = \count($this->objectsToPersist) - 1; $this->objectsToPersist[] = $object; $this->afterPersistCallbacks = [ @@ -309,7 +310,7 @@ public function repositoryFor(string $class): ObjectRepository * @param class-string $parent * @param class-string $child */ - public function inverseRelationshipMetadata(string $parent, string $child, string $field): ?InverseRelationshipMetadata + public function inverseRelationshipMetadata(string $parent, string $child, string $field): ?RelationshipMetadata { $parent = unproxy($parent); $child = unproxy($child); diff --git a/src/Persistence/PersistenceStrategy.php b/src/Persistence/PersistenceStrategy.php index 85607ccd..ed3e36c9 100644 --- a/src/Persistence/PersistenceStrategy.php +++ b/src/Persistence/PersistenceStrategy.php @@ -15,6 +15,7 @@ use Doctrine\Persistence\Mapping\ClassMetadata; use Doctrine\Persistence\Mapping\MappingException; use Doctrine\Persistence\ObjectManager; +use Zenstruck\Foundry\Persistence\Relationship\RelationshipMetadata; /** * @author Kevin Bond @@ -63,7 +64,7 @@ public function objectManagers(): array * @param class-string $parent * @param class-string $child */ - public function inversedRelationshipMetadata(string $parent, string $child, string $field): ?InverseRelationshipMetadata + public function inversedRelationshipMetadata(string $parent, string $child, string $field): ?RelationshipMetadata { return null; } diff --git a/src/Persistence/PersistentObjectFactory.php b/src/Persistence/PersistentObjectFactory.php index 159488df..8ec70765 100644 --- a/src/Persistence/PersistentObjectFactory.php +++ b/src/Persistence/PersistentObjectFactory.php @@ -22,6 +22,8 @@ use Zenstruck\Foundry\ObjectFactory; use Zenstruck\Foundry\Persistence\Exception\NotEnoughObjects; use Zenstruck\Foundry\Persistence\Exception\RefreshObjectFailed; +use Zenstruck\Foundry\Persistence\Relationship\OneToManyRelationship; +use Zenstruck\Foundry\Persistence\Relationship\OneToOneRelationship; use function Zenstruck\Foundry\get; use function Zenstruck\Foundry\set; @@ -299,11 +301,11 @@ 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; + if ($relationshipMetadata instanceof OneToOneRelationship && !$relationshipMetadata->isOwning) { + $inverseField = $relationshipMetadata->inverseField(); // 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 @@ -340,9 +342,9 @@ protected function normalizeCollection(string $field, FactoryCollection $collect $inverseRelationshipMetadata = $pm->inverseRelationshipMetadata(static::class(), $collection->factory::class(), $field); - if ($inverseRelationshipMetadata && $inverseRelationshipMetadata->isCollection) { + if ($inverseRelationshipMetadata instanceof OneToManyRelationship) { $this->tempAfterInstantiate[] = function(object $object) use ($collection, $inverseRelationshipMetadata, $field) { - $inverseField = $inverseRelationshipMetadata->inverseField; + $inverseField = $inverseRelationshipMetadata->inverseField(); $inverseObjects = $collection->withPersistMode( $this->isPersisting() ? PersistMode::NO_PERSIST_BUT_SCHEDULE_FOR_INSERT : PersistMode::WITHOUT_PERSISTING @@ -374,24 +376,39 @@ protected function normalizeCollection(string $field, FactoryCollection $collect * * @internal */ - protected function normalizeObject(object $object): object + protected function normalizeObject(string $field, object $object): object { $configuration = Configuration::instance(); - if ( - !$this->isPersisting() - || !$configuration->isPersistenceAvailable() - ) { + $object = unproxy($object, withAutoRefresh: false); + + if (!$configuration->isPersistenceAvailable()) { return $object; } - $object = unproxy($object, withAutoRefresh: false); - $persistenceManager = $configuration->persistence(); + if (!$persistenceManager->hasPersistenceFor($object)) { return $object; } + $inverseRelationship = $persistenceManager->inverseRelationshipMetadata(static::class(), $object::class, $field); + + if ($inverseRelationship instanceof OneToOneRelationship) { + $this->tempAfterInstantiate[] = static function(object $newObject) use ($object, $inverseRelationship) { + try { + set($object, $inverseRelationship->inverseField(), $newObject); + } catch (\Throwable) { + } + }; + } + + if ( + !$this->isPersisting() + ) { + return $object; + } + if (!$persistenceManager->isPersisted($object)) { $persistenceManager->scheduleForInsert($object); @@ -439,12 +456,15 @@ static function(object $object, array $parameters, PersistentObjectFactory $fact Configuration::instance()->persistence()->scheduleForInsert($object, $afterPersistCallbacks); } ) -// ->afterPersist( -// static function(object $object): void { -// Configuration::instance()->persistence()->refresh($object); -// } -// ) - ; + ->afterPersist( + static function(object $object): void { + try { + Configuration::instance()->persistence()->refresh($object); + } catch (RefreshObjectFailed) { + } + } + ) + ; } private function throwIfCannotCreateObject(): void diff --git a/src/Persistence/InverseRelationshipMetadata.php b/src/Persistence/Relationship/OneToManyRelationship.php similarity index 55% rename from src/Persistence/InverseRelationshipMetadata.php rename to src/Persistence/Relationship/OneToManyRelationship.php index 1287b8f5..d0cf3a59 100644 --- a/src/Persistence/InverseRelationshipMetadata.php +++ b/src/Persistence/Relationship/OneToManyRelationship.php @@ -1,5 +1,7 @@ - * - * @internal - */ -final class InverseRelationshipMetadata +final class OneToManyRelationship implements RelationshipMetadata { public function __construct( - public readonly string $inverseField, - public readonly bool $isCollection, + private readonly string $inverseField, public readonly ?string $collectionIndexedBy, ) { } + + public function inverseField(): string + { + return $this->inverseField; + } } diff --git a/src/Persistence/Relationship/OneToOneRelationship.php b/src/Persistence/Relationship/OneToOneRelationship.php new file mode 100644 index 00000000..28419af8 --- /dev/null +++ b/src/Persistence/Relationship/OneToOneRelationship.php @@ -0,0 +1,28 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Zenstruck\Foundry\Persistence\Relationship; + +final class OneToOneRelationship implements RelationshipMetadata +{ + public function __construct( + private readonly string $inverseField, + public readonly bool $isOwning, + ) { + } + + public function inverseField(): string + { + return $this->inverseField; + } +} diff --git a/src/Persistence/Relationship/RelationshipMetadata.php b/src/Persistence/Relationship/RelationshipMetadata.php new file mode 100644 index 00000000..83ad64f2 --- /dev/null +++ b/src/Persistence/Relationship/RelationshipMetadata.php @@ -0,0 +1,22 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Zenstruck\Foundry\Persistence\Relationship; + +/** + * @author Kevin Bond + * + * @internal + */ +interface RelationshipMetadata +{ + public function inverseField(): string; +} diff --git a/tests/Fixture/Entity/EdgeCases/InversedOneToOneWithNonNullableOwning/InverseSide.php b/tests/Fixture/Entity/EdgeCases/InversedOneToOneWithNonNullableOwning/InverseSide.php index 0ecb2db0..c90cf744 100644 --- a/tests/Fixture/Entity/EdgeCases/InversedOneToOneWithNonNullableOwning/InverseSide.php +++ b/tests/Fixture/Entity/EdgeCases/InversedOneToOneWithNonNullableOwning/InverseSide.php @@ -36,7 +36,6 @@ public function getOwningSide(): OwningSide public function setOwningSide(OwningSide $owningSide): void { - dump(__METHOD__.'()::'.__LINE__); $this->owningSide = $owningSide; $owningSide->inverseSide = $this; } diff --git a/tests/Integration/ORM/EdgeCasesRelationshipTest.php b/tests/Integration/ORM/EdgeCasesRelationshipTest.php index 28ddd303..9c7f4b52 100644 --- a/tests/Integration/ORM/EdgeCasesRelationshipTest.php +++ b/tests/Integration/ORM/EdgeCasesRelationshipTest.php @@ -13,7 +13,6 @@ namespace Zenstruck\Foundry\Tests\Integration\ORM; -use DAMA\DoctrineTestBundle\Doctrine\DBAL\StaticDriver; use Doctrine\Common\Collections\Collection; use PHPUnit\Framework\Attributes\DataProvider; use PHPUnit\Framework\Attributes\RequiresPhpunit; @@ -221,7 +220,7 @@ public function after_instantiate_flushing_using_current_object_in_relationship_ $owningSide = $owningSideFactory ->afterInstantiate( - static function (InversedOneToOneWithNonNullableOwning\OwningSide $o) use ($inverseSideFactory) { + static function(InversedOneToOneWithNonNullableOwning\OwningSide $o) use ($inverseSideFactory) { $inverseSideFactory->create(['owningSide' => $o]); } ) @@ -231,7 +230,7 @@ static function (InversedOneToOneWithNonNullableOwning\OwningSide $o) use ($inve $inverseSideFactory::assert()->count(1); self::assertNotNull($owningSide->inverseSide); - self::assertSame($owningSide, $owningSide->inverseSide->owningSide); + self::assertSame($owningSide, $owningSide->inverseSide->getOwningSide()); } /** @test */ @@ -254,6 +253,31 @@ public function object_with_union_type(): void self::assertInstanceOf(Collection::class, $object->collection); } + /** @test */ + #[Test] + #[DataProvider('provideCascadeRelationshipsCombinations')] + #[UsingRelationships(InversedOneToOneWithNonNullableOwning\OwningSide::class, ['inverseSide'])] + #[RequiresPhpunit('>=11.4')] + public function can_create_inverse_one_to_one_with_actual(): void + { + $owningSideFactory = persistent_factory(InversedOneToOneWithNonNullableOwning\OwningSide::class); + $inverseSideFactory = persistent_factory(InversedOneToOneWithNonNullableOwning\InverseSide::class); + + $owningSide = $owningSideFactory + ->afterInstantiate( + static function(InversedOneToOneWithNonNullableOwning\OwningSide $o) use ($inverseSideFactory): void { + $inverseSideFactory->create(['owningSide' => $o]); + } + ) + ->create(); + + $owningSideFactory::assert()->count(1); + $inverseSideFactory::assert()->count(1); + + self::assertNotNull($owningSide->inverseSide); + self::assertSame($owningSide, $owningSide->inverseSide->getOwningSide()); + } + /** * @test */ diff --git a/tests/Integration/ORM/EntityRelationship/EntityFactoryRelationshipTestCase.php b/tests/Integration/ORM/EntityRelationship/EntityFactoryRelationshipTestCase.php index 72158350..ee8af685 100644 --- a/tests/Integration/ORM/EntityRelationship/EntityFactoryRelationshipTestCase.php +++ b/tests/Integration/ORM/EntityRelationship/EntityFactoryRelationshipTestCase.php @@ -613,7 +613,9 @@ public function after_instantiate_flushing_using_current_object_in_relationship_ { $category = static::categoryFactory() ->afterInstantiate( - static fn(Category $c) => static::contactFactory()->create(['category' => $c]) + static function(Category $c): void { + static::contactFactory()->create(['category' => $c]); + } ) ->create(); @@ -632,7 +634,9 @@ public function after_instantiate_flushing_using_current_object_in_relationship_ { $contact = static::contactFactory() ->afterInstantiate( - static fn(Contact $c) => static::categoryFactory()->create(['contacts' => [$c]]) + static function(Contact $c): void { + static::categoryFactory()->create(['contacts' => [$c]]); + } ) ->create(['category' => null]); @@ -651,7 +655,9 @@ public function after_instantiate_flushing_using_current_object_in_relationship_ { $address = static::addressFactory() ->afterInstantiate( - static fn(Address $a) => static::contactFactory()->create(['address' => $a]) + static function(Address $a): void { + static::contactFactory()->create(['address' => $a]); + } )->create(); static::contactFactory()::assert()->count(1);