Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
aegypius opened this issue Dec 8, 2023 · 3 comments
Open

Comments

@aegypius
Copy link

aegypius commented Dec 8, 2023

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
Copy link
Member

kbond commented Dec 13, 2023

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
Copy link
Author

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 😄 ! In our project we use the proposal of @nikophil here #533. I should have said without "database persistence" relationship but I was'nt aware that this was not a standard foundry feature.

@nikophil
Copy link
Member

nikophil commented Dec 14, 2023

I think the problem is more or less the same than #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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

3 participants