From f84c22a86944de572789dd228fa781f67daff776 Mon Sep 17 00:00:00 2001 From: Nicolas PHILIPPE Date: Sat, 4 Jan 2025 14:31:24 +0100 Subject: [PATCH] fix: propagte "schedule for insert" to factory collection --- src/FactoryCollection.php | 30 +++++++- src/Persistence/PersistMode.php | 2 +- src/Persistence/PersistenceManager.php | 6 +- src/Persistence/PersistentObjectFactory.php | 69 ++++++++++--------- .../InverseSide.php | 39 +++++++++++ .../InversedOneToOneWithOneToMany/Item.php | 28 ++++++++ .../OwningSide.php | 63 +++++++++++++++++ .../ORM/EdgeCasesRelationshipTest.php | 29 ++++++++ 8 files changed, 231 insertions(+), 35 deletions(-) create mode 100644 tests/Fixture/Entity/EdgeCases/InversedOneToOneWithOneToMany/InverseSide.php create mode 100644 tests/Fixture/Entity/EdgeCases/InversedOneToOneWithOneToMany/Item.php create mode 100644 tests/Fixture/Entity/EdgeCases/InversedOneToOneWithOneToMany/OwningSide.php diff --git a/src/FactoryCollection.php b/src/FactoryCollection.php index 6d8f20592..44bb31b99 100644 --- a/src/FactoryCollection.php +++ b/src/FactoryCollection.php @@ -11,6 +11,9 @@ namespace Zenstruck\Foundry; +use Zenstruck\Foundry\Persistence\PersistentObjectFactory; +use Zenstruck\Foundry\Persistence\PersistMode; + /** * @author Kevin Bond * @@ -22,12 +25,28 @@ */ final class FactoryCollection implements \IteratorAggregate { + private PersistMode $persistMode; + /** * @param TFactory $factory * @phpstan-param \Closure():iterable|\Closure():iterable $items */ private function __construct(public readonly Factory $factory, private \Closure $items) { + $this->persistMode = $this->factory instanceof PersistentObjectFactory + ? $this->factory->persistMode() + : PersistMode::WITHOUT_PERSISTING; + } + + /** + * @internal + */ + public function withPersistMode(PersistMode $persistMode): static + { + $clone = clone $this; + $clone->persistMode = $persistMode; + + return $clone; } /** @@ -133,7 +152,16 @@ public function all(): array $factories[] = $this->factory->with($attributesOrFactory)->with(['__index' => $i++]); } - return $factories; // @phpstan-ignore return.type (PHPStan does not understand we have an array of factories) + return array_map( // @phpstan-ignore return.type (PHPStan does not understand we have an array of factories) + function (Factory $f) { + if ($f instanceof PersistentObjectFactory) { + return $f->withPersistMode($this->persistMode); + } + + return $f; + }, + $factories + ); } public function getIterator(): \Traversable diff --git a/src/Persistence/PersistMode.php b/src/Persistence/PersistMode.php index 69172cef7..21ff94a17 100644 --- a/src/Persistence/PersistMode.php +++ b/src/Persistence/PersistMode.php @@ -25,6 +25,6 @@ enum PersistMode public function isPersisting(): bool { - return self::PERSIST === $this; + return self::WITHOUT_PERSISTING !== $this; } } diff --git a/src/Persistence/PersistenceManager.php b/src/Persistence/PersistenceManager.php index 3525791f9..4f5072605 100644 --- a/src/Persistence/PersistenceManager.php +++ b/src/Persistence/PersistenceManager.php @@ -239,7 +239,11 @@ public function truncate(string $class): void */ public function autoPersist(string $class): bool { - return $this->strategyFor(unproxy($class))->autoPersist(); + try { + return $this->strategyFor(unproxy($class))->autoPersist(); + } catch (NoPersistenceStrategy) { + return false; + } } /** diff --git a/src/Persistence/PersistentObjectFactory.php b/src/Persistence/PersistentObjectFactory.php index 7e023d394..5e0d2bdc6 100644 --- a/src/Persistence/PersistentObjectFactory.php +++ b/src/Persistence/PersistentObjectFactory.php @@ -42,9 +42,6 @@ abstract class PersistentObjectFactory extends ObjectFactory /** @var list */ private array $tempAfterInstantiate = []; - /** @var list */ - private array $tempAfterPersist = []; - /** * @phpstan-param mixed|Parameters $criteriaOrId * @@ -207,7 +204,7 @@ public function create(callable|array $attributes = []): object $this->throwIfCannotCreateObject(); - if (!$this->isPersisting()) { + if ($this->persistMode() !== PersistMode::PERSIST) { return $object; } @@ -219,12 +216,6 @@ public function create(callable|array $attributes = []): object $configuration->persistence()->save($object); - foreach ($this->tempAfterPersist as $callback) { - $callback($object); - } - - $this->tempAfterPersist = []; - if ($this->afterPersist) { $attributes = $this->normalizedParameters ?? throw new \LogicException('Factory::$normalizedParameters has not been initialized.'); @@ -254,6 +245,17 @@ final public function withoutPersisting(): static return $clone; } + /** + * @internal + */ + public function withPersistMode(PersistMode $persistMode): static + { + $clone = clone $this; + $clone->persist = $persistMode; + + return $clone; + } + /** * @phpstan-param callable(T, Parameters, static):void $callback */ @@ -272,11 +274,7 @@ protected function normalizeParameter(string $field, mixed $value): mixed } if ($value instanceof self && isset($this->persist)) { - $value = match ($this->persist) { - PersistMode::PERSIST => $value->andPersist(), - PersistMode::WITHOUT_PERSISTING => $value->withoutPersisting(), - PersistMode::NO_PERSIST_BUT_SCHEDULE_FOR_INSERT => $value->withoutPersistingButScheduleForInsert(), - }; + $value = $value->withPersistMode($this->persist); } if ($value instanceof self) { @@ -290,7 +288,7 @@ protected function normalizeParameter(string $field, mixed $value): mixed // 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() + $inversedObject = $value->withPersistMode(PersistMode::NO_PERSIST_BUT_SCHEDULE_FOR_INSERT) ->create([$inverseField => $placeholder = (new \ReflectionClass(static::class()))->newInstanceWithoutConstructor()]); // auto-refresh computes changeset and prevents the placeholder object to be cleanly @@ -325,9 +323,9 @@ protected function normalizeCollection(string $field, FactoryCollection $collect if ($inverseRelationshipMetadata && $inverseRelationshipMetadata->isCollection) { $inverseField = $inverseRelationshipMetadata->inverseField; - $this->tempAfterPersist[] = static function(object $object) use ($collection, $inverseField, $pm) { - $collection->create([$inverseField => $object]); - $pm->refresh($object); + $this->tempAfterInstantiate[] = static function(object $object) use ($collection, $inverseField, $field) { + $inverseObjects = $collection->withPersistMode(PersistMode::NO_PERSIST_BUT_SCHEDULE_FOR_INSERT)->create([$inverseField => $object]); + set($object, $field, unproxy($inverseObjects)); }; // creation delegated to afterPersist hook - return empty array here @@ -355,12 +353,14 @@ protected function normalizeObject(object $object): object $configuration = Configuration::instance(); - if (!$configuration->isPersistenceAvailable() || !$configuration->persistence()->hasPersistenceFor($object)) { + if (!$configuration->isPersistenceAvailable() + || !$configuration->persistence()->hasPersistenceFor($object) + || !$configuration->persistence()->isPersisted($object)) { return $object; } try { - return $configuration->persistence()->refresh($object, true); + return $configuration->persistence()->refresh($object, force: true); } catch (RefreshObjectFailed|VarExportLogicException) { return $object; } @@ -374,19 +374,32 @@ final protected function isPersisting(): bool return false; } - $persistMode = $this->persist ?? ($config->persistence()->autoPersist(static::class()) ? PersistMode::PERSIST : PersistMode::WITHOUT_PERSISTING); + return $this->persistMode()->isPersisting(); + } - return $persistMode->isPersisting(); + /** + * @internal + */ + public function persistMode(): PersistMode + { + $config = Configuration::instance(); + + if (!$config->isPersistenceEnabled()) { + return PersistMode::WITHOUT_PERSISTING; + } + + return $this->persist ?? ($config->persistence()->autoPersist(static::class()) ? PersistMode::PERSIST : PersistMode::WITHOUT_PERSISTING); } /** * Schedule any new object for insert right after instantiation. + * @internal */ final protected function initializeInternal(): static { return $this->afterInstantiate( static function(object $object, array $parameters, PersistentObjectFactory $factory): void { - if (!$factory->isPersisting() && (!isset($factory->persist) || PersistMode::NO_PERSIST_BUT_SCHEDULE_FOR_INSERT !== $factory->persist)) { + if (!$factory->isPersisting()) { return; } @@ -395,14 +408,6 @@ static function(object $object, array $parameters, PersistentObjectFactory $fact ); } - private function withoutPersistingButScheduleForInsert(): static - { - $clone = clone $this; - $clone->persist = PersistMode::NO_PERSIST_BUT_SCHEDULE_FOR_INSERT; - - return $clone; - } - private function throwIfCannotCreateObject(): void { $configuration = Configuration::instance(); diff --git a/tests/Fixture/Entity/EdgeCases/InversedOneToOneWithOneToMany/InverseSide.php b/tests/Fixture/Entity/EdgeCases/InversedOneToOneWithOneToMany/InverseSide.php new file mode 100644 index 000000000..edec900ba --- /dev/null +++ b/tests/Fixture/Entity/EdgeCases/InversedOneToOneWithOneToMany/InverseSide.php @@ -0,0 +1,39 @@ + + * + * 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\InversedOneToOneWithOneToMany; + +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_one_to_many_inverse_side')] +class InverseSide extends Base +{ + #[ORM\OneToOne(mappedBy: 'inverseSide')] + private ?OwningSide $owningSide = null; + + public function getOwningSide(): ?OwningSide + { + return $this->owningSide; + } + + public function setOwningSide(OwningSide $owningSide): void + { + $this->owningSide = $owningSide; + $owningSide->inverseSide = $this; + } +} diff --git a/tests/Fixture/Entity/EdgeCases/InversedOneToOneWithOneToMany/Item.php b/tests/Fixture/Entity/EdgeCases/InversedOneToOneWithOneToMany/Item.php new file mode 100644 index 000000000..dfdf2aca6 --- /dev/null +++ b/tests/Fixture/Entity/EdgeCases/InversedOneToOneWithOneToMany/Item.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\InversedOneToOneWithOneToMany; + +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_one_to_many_item_if_collection')] +class Item extends Base +{ + #[ORM\ManyToOne(inversedBy: 'items')] + public ?OwningSide $owningSide = null; +} diff --git a/tests/Fixture/Entity/EdgeCases/InversedOneToOneWithOneToMany/OwningSide.php b/tests/Fixture/Entity/EdgeCases/InversedOneToOneWithOneToMany/OwningSide.php new file mode 100644 index 000000000..6506f8159 --- /dev/null +++ b/tests/Fixture/Entity/EdgeCases/InversedOneToOneWithOneToMany/OwningSide.php @@ -0,0 +1,63 @@ + + * + * 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\InversedOneToOneWithOneToMany; + +use Doctrine\Common\Collections\ArrayCollection; +use Doctrine\Common\Collections\Collection; +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_one_to_many_owning_side')] +class OwningSide extends Base +{ + #[ORM\OneToOne(inversedBy: 'owningSide')] + public ?InverseSide $inverseSide = null; + + /** @var Collection */ + #[ORM\OneToMany(targetEntity: Item::class, mappedBy: 'owningSide')] + private Collection $items; + + public function __construct() + { + $this->items = new ArrayCollection(); + } + + /** + * @return Collection + */ + public function getItems(): Collection + { + return $this->items; + } + + public function addItem(Item $item): void + { + if (!$this->items->contains($item)) { + $this->items->add($item); + $item->owningSide = $this; + } + } + + public function removeItem(Item $item): void + { + if ($this->items->contains($item)) { + $this->items->removeElement($item); + $item->owningSide = null; + } + } +} diff --git a/tests/Integration/ORM/EdgeCasesRelationshipTest.php b/tests/Integration/ORM/EdgeCasesRelationshipTest.php index 1bf563f33..a6b380bc5 100644 --- a/tests/Integration/ORM/EdgeCasesRelationshipTest.php +++ b/tests/Integration/ORM/EdgeCasesRelationshipTest.php @@ -22,6 +22,7 @@ use Zenstruck\Foundry\Tests\Fixture\DoctrineCascadeRelationship\ChangesEntityRelationshipCascadePersist; use Zenstruck\Foundry\Tests\Fixture\DoctrineCascadeRelationship\UsingRelationships; use Zenstruck\Foundry\Tests\Fixture\Entity\EdgeCases\InversedOneToOneWithNonNullableOwning; +use Zenstruck\Foundry\Tests\Fixture\Entity\EdgeCases\InversedOneToOneWithOneToMany; use Zenstruck\Foundry\Tests\Fixture\Entity\EdgeCases\InversedOneToOneWithSetter; use Zenstruck\Foundry\Tests\Fixture\Entity\EdgeCases\ManyToOneToSelfReferencing; use Zenstruck\Foundry\Tests\Fixture\Entity\EdgeCases\RelationshipWithGlobalEntity; @@ -123,6 +124,34 @@ public function inverse_one_to_one_with_both_nullable(): void self::assertSame($inverseSide, $inverseSide->getOwningSide()?->inverseSide); } + /** @test */ + #[Test] + #[DataProvider('provideCascadeRelationshipsCombinations')] + #[UsingRelationships(InversedOneToOneWithOneToMany\OwningSide::class, ['inverseSide'])] + #[UsingRelationships(InversedOneToOneWithOneToMany\Item::class, ['owningSide'])] + #[RequiresPhpunit('^11.4')] + public function inverse_one_to_one_with_one_to_many(): void + { + $inverseSideFactory = persistent_factory(InversedOneToOneWithOneToMany\InverseSide::class); + $owningSideFactory = persistent_factory(InversedOneToOneWithOneToMany\OwningSide::class); + $itemFactory = persistent_factory(InversedOneToOneWithOneToMany\Item::class) + // "with()" attribute emulates what would be found in the "defaults()" method in a real factory + ->with(['owningSide' => $owningSideFactory]); + + $inverseSide = $inverseSideFactory->create([ + 'owningSide' => $owningSideFactory->with([ + 'items' => $itemFactory->many(2), + ]) + ]); + + $owningSideFactory::assert()->count(1); + $inverseSideFactory::assert()->count(1); + $itemFactory::assert()->count(2); + + self::assertSame($inverseSide, $inverseSide->getOwningSide()?->inverseSide); + self::assertCount(2, $inverseSide->getOwningSide()->getItems()); + } + /** * @test */