Skip to content

Commit

Permalink
fix: propagte "schedule for insert" to factory collection
Browse files Browse the repository at this point in the history
  • Loading branch information
nikophil committed Jan 2, 2025
1 parent 482fcdd commit bc89ccf
Show file tree
Hide file tree
Showing 8 changed files with 229 additions and 33 deletions.
30 changes: 29 additions & 1 deletion src/FactoryCollection.php
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,9 @@

namespace Zenstruck\Foundry;

use Zenstruck\Foundry\Persistence\PersistentObjectFactory;
use Zenstruck\Foundry\Persistence\PersistMode;

/**
* @author Kevin Bond <kevinbond@gmail.com>
*
Expand All @@ -22,12 +25,28 @@
*/
final class FactoryCollection implements \IteratorAggregate
{
private PersistMode $persistMode;

/**
* @param TFactory $factory
* @phpstan-param \Closure():iterable<Attributes>|\Closure():iterable<TFactory> $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;
}

/**
Expand Down Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion src/Persistence/PersistMode.php
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,6 @@ enum PersistMode

public function isPersisting(): bool
{
return self::PERSIST === $this;
return self::WITHOUT_PERSISTING !== $this;
}
}
6 changes: 5 additions & 1 deletion src/Persistence/PersistenceManager.php
Original file line number Diff line number Diff line change
Expand Up @@ -249,7 +249,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;
}
}

/**
Expand Down
65 changes: 35 additions & 30 deletions src/Persistence/PersistentObjectFactory.php
Original file line number Diff line number Diff line change
Expand Up @@ -40,9 +40,6 @@ abstract class PersistentObjectFactory extends ObjectFactory
/** @var list<callable(T):void> */
private array $tempAfterInstantiate = [];

/** @var list<callable(T):void> */
private array $tempAfterPersist = [];

/**
* @phpstan-param mixed|Parameters $criteriaOrId
*
Expand Down Expand Up @@ -205,7 +202,7 @@ public function create(callable|array $attributes = []): object

$this->throwIfCannotCreateObject();

if (!$this->isPersisting()) {
if ($this->persistMode() !== PersistMode::PERSIST) {
return $object;
}

Expand All @@ -217,12 +214,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.');

Expand Down Expand Up @@ -252,6 +243,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
*/
Expand All @@ -270,11 +272,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) {
Expand All @@ -288,7 +286,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
Expand Down Expand Up @@ -323,9 +321,11 @@ 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);
$persistMode = $this->persistMode();

$this->tempAfterInstantiate[] = static function(object $object) use ($collection, $inverseField, $field, $persistMode) {
$inverseObjects = $collection->withPersistMode($persistMode)->create([$inverseField => $object]);
set($object, $field, unproxy($inverseObjects));
};

// creation delegated to afterPersist hook - return empty array here
Expand Down Expand Up @@ -368,19 +368,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;
}

Expand All @@ -389,14 +402,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();
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
<?php

declare(strict_types=1);

/*
* This file is part of the zenstruck/foundry package.
*
* (c) Kevin Bond <kevinbond@gmail.com>
*
* 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 <nikophil@gmail.com>
*/
#[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;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
<?php

declare(strict_types=1);

/*
* This file is part of the zenstruck/foundry package.
*
* (c) Kevin Bond <kevinbond@gmail.com>
*
* 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 <nikophil@gmail.com>
*/
#[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;
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
<?php

declare(strict_types=1);

/*
* This file is part of the zenstruck/foundry package.
*
* (c) Kevin Bond <kevinbond@gmail.com>
*
* 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 <nikophil@gmail.com>
*/
#[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<int, Item> */
#[ORM\OneToMany(targetEntity: Item::class, mappedBy: 'owningSide')]
private Collection $items;

public function __construct()
{
$this->items = new ArrayCollection();
}

/**
* @return Collection<int, Item>
*/
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;
}
}
}
29 changes: 29 additions & 0 deletions tests/Integration/ORM/EdgeCasesRelationshipTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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
*/
Expand Down

0 comments on commit bc89ccf

Please sign in to comment.