Skip to content

Commit 28715d1

Browse files
nikophilnikophil
committed
fix: flush only once per Factory::create() calls in userland
1 parent 8356b53 commit 28715d1

File tree

8 files changed

+199
-12
lines changed

8 files changed

+199
-12
lines changed

‎src/Persistence/PersistenceManager.php

+11-11
Original file line numberDiff line numberDiff line change
@@ -75,17 +75,6 @@ public function save(object $object): object
7575
$om->persist($object);
7676
$this->flush($om);
7777

78-
if ($this->afterPersistCallbacks) {
79-
$afterPersistCallbacks = $this->afterPersistCallbacks;
80-
$this->afterPersistCallbacks = [];
81-
82-
foreach ($afterPersistCallbacks as $afterPersistCallback) {
83-
$afterPersistCallback();
84-
}
85-
86-
$this->save($object);
87-
}
88-
8978
return $object;
9079
}
9180

@@ -150,6 +139,17 @@ public function flush(ObjectManager $om): void
150139
{
151140
if ($this->flush) {
152141
$om->flush();
142+
143+
if ($this->afterPersistCallbacks) {
144+
$afterPersistCallbacks = $this->afterPersistCallbacks;
145+
$this->afterPersistCallbacks = [];
146+
147+
foreach ($afterPersistCallbacks as $afterPersistCallback) {
148+
$afterPersistCallback();
149+
}
150+
151+
$this->flush($om);
152+
}
153153
}
154154
}
155155

‎src/Persistence/PersistentObjectFactory.php

+14-1
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,8 @@ abstract class PersistentObjectFactory extends ObjectFactory
4444
/** @var list<callable(T):void> */
4545
private array $tempAfterInstantiate = [];
4646

47+
private bool $isRootFactory = true;
48+
4749
/**
4850
* @phpstan-param mixed|Parameters $criteriaOrId
4951
*
@@ -206,7 +208,7 @@ public function create(callable|array $attributes = []): object
206208

207209
$this->throwIfCannotCreateObject();
208210

209-
if (PersistMode::PERSIST !== $this->persistMode()) {
211+
if (PersistMode::PERSIST !== $this->persistMode() || !$this->isRootFactory) {
210212
return $object;
211213
}
212214

@@ -299,6 +301,7 @@ protected function normalizeParameter(string $field, mixed $value): mixed
299301
$inversedObject = $value->withPersistMode(
300302
$this->isPersisting() ? PersistMode::NO_PERSIST_BUT_SCHEDULE_FOR_INSERT : PersistMode::WITHOUT_PERSISTING
301303
)
304+
->notRootFactory()
302305

303306
// we need to handle the circular dependency involved by inversed one-to-one relationship:
304307
// 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
314317
};
315318

316319
return $inversedObject;
320+
} else {
321+
$value = $value->notRootFactory();
317322
}
318323
}
319324

@@ -450,4 +455,12 @@ private function throwIfCannotCreateObject(): void
450455

451456
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));
452457
}
458+
459+
private function notRootFactory(): static
460+
{
461+
$clone = clone $this;
462+
$clone->isRootFactory = false;
463+
464+
return $clone;
465+
}
453466
}

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

+37
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
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\InversedOneToOneWithManyToOne;
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_many_to_one_inverse_side')]
24+
class InverseSide extends Base
25+
{
26+
#[ORM\OneToOne(mappedBy: 'inverseSide')]
27+
public ?OwningSide $owningSide = null;
28+
29+
#[ORM\ManyToOne()]
30+
public ?Item $item = null;
31+
32+
public function __construct(
33+
#[ORM\Column()]
34+
public string $mandatoryField
35+
) {
36+
}
37+
}

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

+26
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
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\InversedOneToOneWithManyToOne;
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_many_to_one_item_if_collection')]
24+
class Item extends Base
25+
{
26+
}

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

‎tests/Integration/ORM/EdgeCasesRelationshipTest.php

+29
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
use Zenstruck\Foundry\Tests\Fixture\DoctrineCascadeRelationship\ChangesEntityRelationshipCascadePersist;
2424
use Zenstruck\Foundry\Tests\Fixture\DoctrineCascadeRelationship\UsingRelationships;
2525
use Zenstruck\Foundry\Tests\Fixture\Entity\EdgeCases\IndexedOneToMany;
26+
use Zenstruck\Foundry\Tests\Fixture\Entity\EdgeCases\InversedOneToOneWithManyToOne;
2627
use Zenstruck\Foundry\Tests\Fixture\Entity\EdgeCases\InversedOneToOneWithNonNullableOwning;
2728
use Zenstruck\Foundry\Tests\Fixture\Entity\EdgeCases\InversedOneToOneWithOneToMany;
2829
use Zenstruck\Foundry\Tests\Fixture\Entity\EdgeCases\InversedOneToOneWithSetter;
@@ -160,6 +161,34 @@ public function indexed_one_to_many(): void
160161
self::assertNotNull($parent->getItems()->get('en')); // @phpstan-ignore argument.type
161162
}
162163

164+
/** @test */
165+
#[Test]
166+
#[DataProvider('provideCascadeRelationshipsCombinations')]
167+
#[UsingRelationships(InversedOneToOneWithManyToOne\InverseSide::class, ['owningSide', 'item'])]
168+
#[RequiresPhpunit('>=11.4')]
169+
public function inversed_one_to_one_can_be_used_after_other_relationship(): void
170+
{
171+
$inverseSideFactory = persistent_factory(InversedOneToOneWithManyToOne\InverseSide::class);
172+
$owningSideFactory = persistent_factory(InversedOneToOneWithManyToOne\OwningSide::class);
173+
$itemFactory = persistent_factory(InversedOneToOneWithManyToOne\Item::class);
174+
175+
$inverseSide = $inverseSideFactory->create(
176+
[
177+
'mandatoryField' => 'foo',
178+
'owningSide' => $owningSideFactory,
179+
'item' => $itemFactory,
180+
]
181+
);
182+
183+
$inverseSideFactory::assert()->count(1);
184+
$owningSideFactory::assert()->count(1);
185+
$itemFactory::assert()->count(1);
186+
187+
self::assertNotNull($inverseSide->owningSide);
188+
self::assertSame($inverseSide, $inverseSide->owningSide->inverseSide);
189+
self::assertNotNull($inverseSide->item);
190+
}
191+
163192
/** @test */
164193
#[Test]
165194
public function object_with_union_type(): void

‎tests/Integration/ORM/EntityRelationship/EntityFactoryRelationshipTestCase.php

+34
Original file line numberDiff line numberDiff line change
@@ -519,6 +519,40 @@ public function can_call_create_in_after_persist_callback(): void
519519
self::assertSame(unproxy($category), $category->getContacts()[0]?->getCategory());
520520
}
521521

522+
/** @test */
523+
#[Test]
524+
public function can_use_nested_after_persist_callback(): void
525+
{
526+
$contact = static::contactFactory()::createOne(
527+
[
528+
'address' => static::addressFactory()
529+
->afterPersist(function(Address $address) {
530+
$address->setCity('city from after persist');
531+
}),
532+
]
533+
);
534+
535+
self::assertSame('city from after persist', $contact->getAddress()->getCity());
536+
}
537+
538+
/** @test */
539+
#[Test]
540+
public function can_call_create_in_nested_after_persist_callback(): void
541+
{
542+
$contact = static::contactFactory()::createOne(
543+
[
544+
'category' => static::categoryFactory()
545+
->afterPersist(function(Category $category) {
546+
$category->addSecondaryContact(
547+
unproxy(static::contactFactory()::createOne())
548+
);
549+
}),
550+
]
551+
);
552+
553+
self::assertCount(1, $contact->getCategory()?->getSecondaryContacts() ?? []);
554+
}
555+
522556
/** @return PersistentObjectFactory<Contact> */
523557
protected static function contactFactoryWithoutCategory(): PersistentObjectFactory
524558
{

‎tests/Integration/Persistence/GenericFactoryTestCase.php

+20
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111

1212
namespace Zenstruck\Foundry\Tests\Integration\Persistence;
1313

14+
use PHPUnit\Framework\Attributes\Test;
1415
use Symfony\Bundle\FrameworkBundle\Test\KernelTestCase;
1516
use Zenstruck\Foundry\Configuration;
1617
use Zenstruck\Foundry\Exception\PersistenceDisabled;
@@ -564,6 +565,25 @@ public function can_use_after_persist_with_attributes(): void
564565
$this->assertSame(1, $object->getPropInteger());
565566
}
566567

568+
/**
569+
* @test
570+
*/
571+
#[Test]
572+
public function it_actually_calls_post_persist_hook_after_persist_when_in_flush_after(): void
573+
{
574+
$object = flush_after(
575+
function () {
576+
return static::factory()->afterPersist(
577+
static function (GenericModel $o) {
578+
$o->setProp1((string) $o->id);
579+
}
580+
)->create();
581+
}
582+
);
583+
584+
self::assertSame((string) $object->id, $object->getProp1());
585+
}
586+
567587
/**
568588
* @return class-string<GenericModel>
569589
*/

0 commit comments

Comments
 (0)