Skip to content

Commit e45913e

Browse files
nikophilnikophil
authored
fix: propagate "schedule for insert" to factory collection (#775)
1 parent d9262cc commit e45913e

File tree

8 files changed

+227
-33
lines changed

8 files changed

+227
-33
lines changed

‎src/FactoryCollection.php

+29-1
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,9 @@
1111

1212
namespace Zenstruck\Foundry;
1313

14+
use Zenstruck\Foundry\Persistence\PersistentObjectFactory;
15+
use Zenstruck\Foundry\Persistence\PersistMode;
16+
1417
/**
1518
* @author Kevin Bond <[email protected]>
1619
*
@@ -22,12 +25,28 @@
2225
*/
2326
final class FactoryCollection implements \IteratorAggregate
2427
{
28+
private PersistMode $persistMode;
29+
2530
/**
2631
* @param TFactory $factory
2732
* @phpstan-param \Closure():iterable<Attributes>|\Closure():iterable<TFactory> $items
2833
*/
2934
private function __construct(public readonly Factory $factory, private \Closure $items)
3035
{
36+
$this->persistMode = $this->factory instanceof PersistentObjectFactory
37+
? $this->factory->persistMode()
38+
: PersistMode::WITHOUT_PERSISTING;
39+
}
40+
41+
/**
42+
* @internal
43+
*/
44+
public function withPersistMode(PersistMode $persistMode): static
45+
{
46+
$clone = clone $this;
47+
$clone->persistMode = $persistMode;
48+
49+
return $clone;
3150
}
3251

3352
/**
@@ -133,7 +152,16 @@ public function all(): array
133152
$factories[] = $this->factory->with($attributesOrFactory)->with(['__index' => $i++]);
134153
}
135154

136-
return $factories; // @phpstan-ignore return.type (PHPStan does not understand we have an array of factories)
155+
return array_map( // @phpstan-ignore return.type (PHPStan does not understand we have an array of factories)
156+
function (Factory $f) {
157+
if ($f instanceof PersistentObjectFactory) {
158+
return $f->withPersistMode($this->persistMode);
159+
}
160+
161+
return $f;
162+
},
163+
$factories
164+
);
137165
}
138166

139167
public function getIterator(): \Traversable

‎src/Persistence/PersistMode.php

+1-1
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,6 @@ enum PersistMode
2525

2626
public function isPersisting(): bool
2727
{
28-
return self::PERSIST === $this;
28+
return self::WITHOUT_PERSISTING !== $this;
2929
}
3030
}

‎src/Persistence/PersistenceManager.php

+5-1
Original file line numberDiff line numberDiff line change
@@ -239,7 +239,11 @@ public function truncate(string $class): void
239239
*/
240240
public function autoPersist(string $class): bool
241241
{
242-
return $this->strategyFor(unproxy($class))->autoPersist();
242+
try {
243+
return $this->strategyFor(unproxy($class))->autoPersist();
244+
} catch (NoPersistenceStrategy) {
245+
return false;
246+
}
243247
}
244248

245249
/**

‎src/Persistence/PersistentObjectFactory.php

+33-30
Original file line numberDiff line numberDiff line change
@@ -42,9 +42,6 @@ abstract class PersistentObjectFactory extends ObjectFactory
4242
/** @var list<callable(T):void> */
4343
private array $tempAfterInstantiate = [];
4444

45-
/** @var list<callable(T):void> */
46-
private array $tempAfterPersist = [];
47-
4845
/**
4946
* @phpstan-param mixed|Parameters $criteriaOrId
5047
*
@@ -207,7 +204,7 @@ public function create(callable|array $attributes = []): object
207204

208205
$this->throwIfCannotCreateObject();
209206

210-
if (!$this->isPersisting()) {
207+
if ($this->persistMode() !== PersistMode::PERSIST) {
211208
return $object;
212209
}
213210

@@ -219,12 +216,6 @@ public function create(callable|array $attributes = []): object
219216

220217
$configuration->persistence()->save($object);
221218

222-
foreach ($this->tempAfterPersist as $callback) {
223-
$callback($object);
224-
}
225-
226-
$this->tempAfterPersist = [];
227-
228219
if ($this->afterPersist) {
229220
$attributes = $this->normalizedParameters ?? throw new \LogicException('Factory::$normalizedParameters has not been initialized.');
230221

@@ -254,6 +245,17 @@ final public function withoutPersisting(): static
254245
return $clone;
255246
}
256247

248+
/**
249+
* @internal
250+
*/
251+
public function withPersistMode(PersistMode $persistMode): static
252+
{
253+
$clone = clone $this;
254+
$clone->persist = $persistMode;
255+
256+
return $clone;
257+
}
258+
257259
/**
258260
* @phpstan-param callable(T, Parameters, static):void $callback
259261
*/
@@ -272,11 +274,7 @@ protected function normalizeParameter(string $field, mixed $value): mixed
272274
}
273275

274276
if ($value instanceof self && isset($this->persist)) {
275-
$value = match ($this->persist) {
276-
PersistMode::PERSIST => $value->andPersist(),
277-
PersistMode::WITHOUT_PERSISTING => $value->withoutPersisting(),
278-
PersistMode::NO_PERSIST_BUT_SCHEDULE_FOR_INSERT => $value->withoutPersistingButScheduleForInsert(),
279-
};
277+
$value = $value->withPersistMode($this->persist);
280278
}
281279

282280
if ($value instanceof self) {
@@ -290,7 +288,7 @@ protected function normalizeParameter(string $field, mixed $value): mixed
290288

291289
// we need to handle the circular dependency involved by inversed one-to-one relationship:
292290
// a placeholder object is used, which will be replaced by the real object, after its instantiation
293-
$inversedObject = $value->withoutPersistingButScheduleForInsert()
291+
$inversedObject = $value->withPersistMode(PersistMode::NO_PERSIST_BUT_SCHEDULE_FOR_INSERT)
294292
->create([$inverseField => $placeholder = (new \ReflectionClass(static::class()))->newInstanceWithoutConstructor()]);
295293

296294
// auto-refresh computes changeset and prevents the placeholder object to be cleanly
@@ -325,9 +323,9 @@ protected function normalizeCollection(string $field, FactoryCollection $collect
325323
if ($inverseRelationshipMetadata && $inverseRelationshipMetadata->isCollection) {
326324
$inverseField = $inverseRelationshipMetadata->inverseField;
327325

328-
$this->tempAfterPersist[] = static function(object $object) use ($collection, $inverseField, $pm) {
329-
$collection->create([$inverseField => $object]);
330-
$pm->refresh($object);
326+
$this->tempAfterInstantiate[] = static function(object $object) use ($collection, $inverseField, $field) {
327+
$inverseObjects = $collection->withPersistMode(PersistMode::NO_PERSIST_BUT_SCHEDULE_FOR_INSERT)->create([$inverseField => $object]);
328+
set($object, $field, unproxy($inverseObjects));
331329
};
332330

333331
// creation delegated to afterPersist hook - return empty array here
@@ -374,19 +372,32 @@ final protected function isPersisting(): bool
374372
return false;
375373
}
376374

377-
$persistMode = $this->persist ?? ($config->persistence()->autoPersist(static::class()) ? PersistMode::PERSIST : PersistMode::WITHOUT_PERSISTING);
375+
return $this->persistMode()->isPersisting();
376+
}
378377

379-
return $persistMode->isPersisting();
378+
/**
379+
* @internal
380+
*/
381+
public function persistMode(): PersistMode
382+
{
383+
$config = Configuration::instance();
384+
385+
if (!$config->isPersistenceEnabled()) {
386+
return PersistMode::WITHOUT_PERSISTING;
387+
}
388+
389+
return $this->persist ?? ($config->persistence()->autoPersist(static::class()) ? PersistMode::PERSIST : PersistMode::WITHOUT_PERSISTING);
380390
}
381391

382392
/**
383393
* Schedule any new object for insert right after instantiation.
394+
* @internal
384395
*/
385396
final protected function initializeInternal(): static
386397
{
387398
return $this->afterInstantiate(
388399
static function(object $object, array $parameters, PersistentObjectFactory $factory): void {
389-
if (!$factory->isPersisting() && (!isset($factory->persist) || PersistMode::NO_PERSIST_BUT_SCHEDULE_FOR_INSERT !== $factory->persist)) {
400+
if (!$factory->isPersisting()) {
390401
return;
391402
}
392403

@@ -395,14 +406,6 @@ static function(object $object, array $parameters, PersistentObjectFactory $fact
395406
);
396407
}
397408

398-
private function withoutPersistingButScheduleForInsert(): static
399-
{
400-
$clone = clone $this;
401-
$clone->persist = PersistMode::NO_PERSIST_BUT_SCHEDULE_FOR_INSERT;
402-
403-
return $clone;
404-
}
405-
406409
private function throwIfCannotCreateObject(): void
407410
{
408411
$configuration = Configuration::instance();

‎tests/Fixture/Entity/EdgeCases/InversedOneToOneWithOneToMany/InverseSide.php

+39
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
/*
6+
* This file is part of the zenstruck/foundry package.
7+
*
8+
* (c) Kevin Bond <[email protected]>
9+
*
10+
* For the full copyright and license information, please view the LICENSE
11+
* file that was distributed with this source code.
12+
*/
13+
14+
namespace Zenstruck\Foundry\Tests\Fixture\Entity\EdgeCases\InversedOneToOneWithOneToMany;
15+
16+
use Doctrine\ORM\Mapping as ORM;
17+
use Zenstruck\Foundry\Tests\Fixture\Model\Base;
18+
19+
/**
20+
* @author Nicolas PHILIPPE <[email protected]>
21+
*/
22+
#[ORM\Entity]
23+
#[ORM\Table('inversed_one_to_one_with_one_to_many_inverse_side')]
24+
class InverseSide extends Base
25+
{
26+
#[ORM\OneToOne(mappedBy: 'inverseSide')]
27+
private ?OwningSide $owningSide = null;
28+
29+
public function getOwningSide(): ?OwningSide
30+
{
31+
return $this->owningSide;
32+
}
33+
34+
public function setOwningSide(OwningSide $owningSide): void
35+
{
36+
$this->owningSide = $owningSide;
37+
$owningSide->inverseSide = $this;
38+
}
39+
}

‎tests/Fixture/Entity/EdgeCases/InversedOneToOneWithOneToMany/Item.php

+28
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
/*
6+
* This file is part of the zenstruck/foundry package.
7+
*
8+
* (c) Kevin Bond <[email protected]>
9+
*
10+
* For the full copyright and license information, please view the LICENSE
11+
* file that was distributed with this source code.
12+
*/
13+
14+
namespace Zenstruck\Foundry\Tests\Fixture\Entity\EdgeCases\InversedOneToOneWithOneToMany;
15+
16+
use Doctrine\ORM\Mapping as ORM;
17+
use Zenstruck\Foundry\Tests\Fixture\Model\Base;
18+
19+
/**
20+
* @author Nicolas PHILIPPE <[email protected]>
21+
*/
22+
#[ORM\Entity]
23+
#[ORM\Table('inversed_one_to_one_with_one_to_many_item_if_collection')]
24+
class Item extends Base
25+
{
26+
#[ORM\ManyToOne(inversedBy: 'items')]
27+
public ?OwningSide $owningSide = null;
28+
}

‎tests/Fixture/Entity/EdgeCases/InversedOneToOneWithOneToMany/OwningSide.php

+63
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,63 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
/*
6+
* This file is part of the zenstruck/foundry package.
7+
*
8+
* (c) Kevin Bond <[email protected]>
9+
*
10+
* For the full copyright and license information, please view the LICENSE
11+
* file that was distributed with this source code.
12+
*/
13+
14+
namespace Zenstruck\Foundry\Tests\Fixture\Entity\EdgeCases\InversedOneToOneWithOneToMany;
15+
16+
use Doctrine\Common\Collections\ArrayCollection;
17+
use Doctrine\Common\Collections\Collection;
18+
use Doctrine\ORM\Mapping as ORM;
19+
use Zenstruck\Foundry\Tests\Fixture\Model\Base;
20+
21+
/**
22+
* @author Nicolas PHILIPPE <[email protected]>
23+
*/
24+
#[ORM\Entity]
25+
#[ORM\Table('inversed_one_to_one_with_one_to_many_owning_side')]
26+
class OwningSide extends Base
27+
{
28+
#[ORM\OneToOne(inversedBy: 'owningSide')]
29+
public ?InverseSide $inverseSide = null;
30+
31+
/** @var Collection<int, Item> */
32+
#[ORM\OneToMany(targetEntity: Item::class, mappedBy: 'owningSide')]
33+
private Collection $items;
34+
35+
public function __construct()
36+
{
37+
$this->items = new ArrayCollection();
38+
}
39+
40+
/**
41+
* @return Collection<int, Item>
42+
*/
43+
public function getItems(): Collection
44+
{
45+
return $this->items;
46+
}
47+
48+
public function addItem(Item $item): void
49+
{
50+
if (!$this->items->contains($item)) {
51+
$this->items->add($item);
52+
$item->owningSide = $this;
53+
}
54+
}
55+
56+
public function removeItem(Item $item): void
57+
{
58+
if ($this->items->contains($item)) {
59+
$this->items->removeElement($item);
60+
$item->owningSide = null;
61+
}
62+
}
63+
}

‎tests/Integration/ORM/EdgeCasesRelationshipTest.php

+29
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
use Zenstruck\Foundry\Tests\Fixture\DoctrineCascadeRelationship\ChangesEntityRelationshipCascadePersist;
2323
use Zenstruck\Foundry\Tests\Fixture\DoctrineCascadeRelationship\UsingRelationships;
2424
use Zenstruck\Foundry\Tests\Fixture\Entity\EdgeCases\InversedOneToOneWithNonNullableOwning;
25+
use Zenstruck\Foundry\Tests\Fixture\Entity\EdgeCases\InversedOneToOneWithOneToMany;
2526
use Zenstruck\Foundry\Tests\Fixture\Entity\EdgeCases\InversedOneToOneWithSetter;
2627
use Zenstruck\Foundry\Tests\Fixture\Entity\EdgeCases\ManyToOneToSelfReferencing;
2728
use Zenstruck\Foundry\Tests\Fixture\Entity\EdgeCases\RelationshipWithGlobalEntity;
@@ -123,6 +124,34 @@ public function inverse_one_to_one_with_both_nullable(): void
123124
self::assertSame($inverseSide, $inverseSide->getOwningSide()?->inverseSide);
124125
}
125126

127+
/** @test */
128+
#[Test]
129+
#[DataProvider('provideCascadeRelationshipsCombinations')]
130+
#[UsingRelationships(InversedOneToOneWithOneToMany\OwningSide::class, ['inverseSide'])]
131+
#[UsingRelationships(InversedOneToOneWithOneToMany\Item::class, ['owningSide'])]
132+
#[RequiresPhpunit('^11.4')]
133+
public function inverse_one_to_one_with_one_to_many(): void
134+
{
135+
$inverseSideFactory = persistent_factory(InversedOneToOneWithOneToMany\InverseSide::class);
136+
$owningSideFactory = persistent_factory(InversedOneToOneWithOneToMany\OwningSide::class);
137+
$itemFactory = persistent_factory(InversedOneToOneWithOneToMany\Item::class)
138+
// "with()" attribute emulates what would be found in the "defaults()" method in a real factory
139+
->with(['owningSide' => $owningSideFactory]);
140+
141+
$inverseSide = $inverseSideFactory->create([
142+
'owningSide' => $owningSideFactory->with([
143+
'items' => $itemFactory->many(2),
144+
])
145+
]);
146+
147+
$owningSideFactory::assert()->count(1);
148+
$inverseSideFactory::assert()->count(1);
149+
$itemFactory::assert()->count(2);
150+
151+
self::assertSame($inverseSide, $inverseSide->getOwningSide()?->inverseSide);
152+
self::assertCount(2, $inverseSide->getOwningSide()->getItems());
153+
}
154+
126155
/**
127156
* @test
128157
*/

0 commit comments

Comments
 (0)