zenstruck / foundry

A model factory library for creating expressive, auto-completable, on-demand dev/test fixtures with Symfony and Doctrine.
https://symfony.com/bundles/ZenstruckFoundryBundle/current/index.html
MIT License
609 stars 63 forks source link

Should object be scheduled for insert before afterInstantiate call ? #537

Open aegypius opened 6 months ago

aegypius commented 6 months ago

Hi,

@nikophil and I having an issue in one of my tests concerning the way afterInstantiate is handled.

I struggle to understand what is meant to happen but my intuition is pointing to the moment where the "root" object is scheduled for insertion and the moment afterInstantiate callbacks are called.

We cannot use afterPersist callbacks because some of our tests are preformed without persistence.

To my understanding the process should be the following

flowchart LR
    A[new] --> B(beforeInstantiate) --> B
    B --> C[schedule for persistence]
    C --> D(afterInstantiate) --> D
    D --> E[persist]
    E --> F(afterPersist) --> F

But when we used this process in a factory like this :


final class PostFactory extends ModelFactory
{
    public function published(): static 
    {
        return $this->afterInstantiate(
            static function (Post $post): void {
                PublishFactory::new([
                    'post' => $post,
                ])->create();
            }
        );
    }
}

Doctrine complains about not knowing the post

Doctrine\ORM\ORMInvalidArgumentException : A new entity was found through the relationship 'App\Entity\Publish#post' >that was not configured to cascade persist operations for entity: ef8013ee-2f5b-43e2-81bb-d9e617fb8973. To solve this >issue: Either explicitly call EntityManager#persist() on this unknown entity or configure cascade persist this association in the >mapping for example @ ManyToOne(..,cascade={"persist"}).

Ending with this ugly workaround that seems to do the trick


final class PostFactory extends ModelFactory
{
    public function published(): static 
    {
        return $this->afterInstantiate(
            static function (Post $post): void {
              // Workaround doctrine cascade 
               (new Proxy($post))->save();

                PublishFactory::new([
                    'post' => $post,
                ])->create();
            }
        );
    }
}

Is this the intended behavior that the root object is not yet scheduled for insert in this case ?

kbond commented 6 months ago

Hey @aegypius, just want to confirm, are the entities in your example above using cascade persist?

I believe I chose to schedule the persist after afterInstantiate because at this point, only plain object creation is the concern. I know as soon as doctrine is involved, there are some nuances here. The cascade persist concern further compounds this nuance.

One question about your code above, in your tests where persistence is disabled, how do you access the created Publish entity - is it like a one-to-one relationship?

aegypius commented 6 months ago

Hi @kbond ! Thanks for your reply my entities did not have cascade persist, I had this since and it worked.

I believe I chose to schedule the persist after afterInstantiate because at this point, only plain object creation is the concern. I know as soon as doctrine is involved, there are some nuances here. The cascade persist concern further compounds this nuance.

I think my confusion tend to be from the fact that with doctrine we schedule persistence with $entityManager->persist($entity) but this is actually persisted when we flush.

In my mind, the new was a finite-state and therefore schedule for persistence regardless of if we actually persist or not.

One question about your code above, in your tests where persistence is disabled, how do you access the created Publish entity - is it like a one-to-one relationship?

Yes I noticed this is an odd one :smile: ! In our project we use the proposal of @nikophil here https://github.com/zenstruck/foundry/issues/533. I should have said without "database persistence" relationship but I was'nt aware that this was not a standard foundry feature.

nikophil commented 6 months ago

I think the problem is more or less the same than https://github.com/zenstruck/foundry/issues/531

the Publish is a ManyToOne (without cascade persist) on Program which is used in the constructor. something like:

class Program
{
    public function __construct(
        #[ORM\ManyToOne(targetEntity: Program::class)]
        #[ORM\JoinColumn(nullable: false)]
        private Program $program,
    ) {}
}

Then we're using this trick with afterInstantiate() to be able to create publishes related to a program, but from the "parent" perspective:

ProgramFactory::new()
    ->published()
    ->create()

I'm pretty sure that this part of the problem will be fixed in Foundry 2

but I'm wondering if we cannot add a call to $em->persist() right after the instantiation? Maybe we could automatically add a postInstantiate callback when persisting is enabled? I think, this way, we could tackle all differences between objects with cascade=['persist'] and object without it