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

Global state relationships and flush_after #649

Closed pmontoya closed 5 days ago

pmontoya commented 1 week ago

Hello,

I recently migrated to version 2.

I used to use Factory::flushAfter in my functional tests.

Using DAMA + global stories, when I create a object in a story (not global this one) using a relationship created in a global story I got a A new entity was found through the relationship error only if story is load with a flush_after. I use flush_after because in my case, two entities has to created in the same transaction.

My config file :

<?php

declare(strict_types = 1);

use ...;

return static function (ZenstruckFoundryConfig $config, ContainerConfigurator $container): void {
    $config->faker()->locale('fr_FR');
    $config->globalState([
        GlobalState\ActivityStory::class,
        ...,
        GlobalState\BusinessProviderStory::class,
        GlobalState\BusinessProviderOfferStory::class,
    ]);
    $config->orm(['auto_persist' => true, 'reset' => ['connections' => ['default'], 'entity_managers' => ['default'], 'mode' => 'migrate']]);
};

My call in test : flush_after(static fn () => CreditNoteStory::load());

If I update my business logic (ignoring error on missing linked entity in the same transaction), and remove the flush_after call, I haven't any error

Thank you for your help

nikophil commented 1 week ago

Hi @pmontoya

yes, this sounds like a bug

I use flush_after because in my cas, two entities has to created in the same transaction.

seems legit

few questions:

thanks

pmontoya commented 1 week ago

Hi @nikophil,

Thank you for your answer.

Yes, I have the same behavior if I use flush_after() inside my story.

Here is the whole error and stack trace :

There was 1 error:

1) App\Tests\App\Billing\Api\CreditNote\CreditNoteDownloaderFunctionalTest::testGetDownloadItemAsManager
Doctrine\ORM\ORMInvalidArgumentException: Multiple non-persisted new entities were found through the given association graph:

 * A new entity was found through the relationship 'App\Customer\Entity\Company#activity' that was not configured to cascade persist operations for entity: App\Activity\Entity\Activity@3697. 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"}). If you cannot find out which entity causes the problem implement 'App\Activity\Entity\Activity#__toString()' to get a clue.
 * A new entity was found through the relationship 'App\Contract\Entity\Contract#productType' that was not configured to cascade persist operations for entity: App\Offer\Entity\ProductType@4210. 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"}). If you cannot find out which entity causes the problem implement 'App\Offer\Entity\ProductType#__toString()' to get a clue.
 * A new entity was found through the relationship 'App\Contract\Entity\Contract#productProvider' that was not configured to cascade persist operations for entity: App\Provider\Entity\ProductProvider@4343. 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"}). If you cannot find out which entity causes the problem implement 'App\Provider\Entity\ProductProvider#__toString()' to get a clue.
 * A new entity was found through the relationship 'App\Contract\Entity\Contract#businessProvider' that was not configured to cascade persist operations for entity: App\Provider\Entity\BusinessProvider@8518. 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"}). If you cannot find out which entity causes the problem implement 'App\Provider\Entity\BusinessProvider#__toString()' to get a clue.
 * A new entity was found through the relationship 'App\Contract\Entity\ContractEndorsement#offer' that was not configured to cascade persist operations for entity: App\Offer\Entity\OfferOffice@6208. 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"}). If you cannot find out which entity causes the problem implement 'App\Offer\Entity\Offer#__toString()' to get a clue.

/srv/api/vendor/doctrine/orm/src/ORMInvalidArgumentException.php:71
/srv/api/vendor/doctrine/orm/src/UnitOfWork.php:3169
/srv/api/vendor/doctrine/orm/src/UnitOfWork.php:369
/srv/api/vendor/doctrine/orm/src/EntityManager.php:259
/srv/api/vendor/zenstruck/foundry/src/Persistence/PersistenceManager.php:197
/srv/api/vendor/zenstruck/foundry/src/Persistence/PersistenceManager.php:189
/srv/api/vendor/zenstruck/foundry/src/Persistence/functions.php:156
/srv/api/tests/App/Billing/Api/CreditNote/Story/CreditNoteStory.php:38
/srv/api/vendor/zenstruck/foundry/src/StoryRegistry.php:53
/srv/api/vendor/zenstruck/foundry/src/Story.php:106
/srv/api/tests/App/Billing/Api/CreditNote/CreditNoteDownloaderFunctionalTest.php:31
/srv/api/vendor/zenstruck/foundry/src/Persistence/PersistenceManager.php:183
/srv/api/vendor/zenstruck/foundry/src/Persistence/functions.php:156
/srv/api/tests/App/Billing/Api/CreditNote/CreditNoteDownloaderFunctionalTest.php:31

And my CreditNoteStory content

<?php

declare(strict_types = 1);

namespace App\Tests\App\Billing\Api\CreditNote\Story;

use App\Activity\Entity\ActivityArea;
use App\Billing\Entity\CreditNote;
use App\Billing\Entity\Invoice;
use App\Customer\Entity\Customer;
use App\Customer\Model\CompanyProperty;
use App\Tests\Factory\CompanyFactory;
use App\Tests\Factory\ContractEndorsementFactory;
use App\Tests\Factory\ContractFactory;
use App\Tests\Factory\CreditNoteFactory;
use App\Tests\Factory\CustomerFactory;
use App\Tests\Factory\InvoiceFactory;
use App\Tests\Story\ActivityStory;
use App\Tests\Story\OfferOfficeStory;

use function Zenstruck\Foundry\Persistence\flush_after;

use Zenstruck\Foundry\Persistence\Proxy;
use Zenstruck\Foundry\Story;

/**
 * @method static Proxy<Customer>   customer()
 * @method static Proxy<CreditNote> credit_note_succeeded()
 */
final class CreditNoteStory extends Story
{
    #[\Override]
    public function build(): void
    {
        $customer = CustomerFactory::new([
            'company' => CompanyFactory::new([
                'answers'  => new CompanyProperty(),
                'activity' => ActivityStory::get('developpement-applications-web/'.ActivityArea::ACTIVITY_AREA_IT_TELECOM_IT_CONSULTING),
            ]),
        ])->create();
        $this->addState('customer', $customer);

        $endorsement = ContractEndorsementFactory::new()->create([
            'offer'    => OfferOfficeStory::get('essentiel_office_activity_area_1000'),
            'contract' => ContractFactory::new([
                'company' => $customer->getCompany(),
            ])->office()->linkedToStello(),
        ]);

        $this->addState('credit_note_succeeded', CreditNoteFactory::new([
            'issuedAt' => new \DateTimeImmutable('now'),
            'amount'   => 6000,
            'invoice'  => InvoiceFactory::new([
                'endorsement'  => $endorsement,
                'state'        => Invoice::STATE_PAYMENT_SUCCEEDED,
                'stripePaidAt' => new \DateTimeImmutable('-1 months'),
            ])->create(),
        ])->create());
    }
}

In my ContractFactory, office function content is

    public function office(bool $story = true): self
    {
        return $this->with(static fn () => [
            'productType'     => $story ? ProductTypeStory::get(ProductTypeEnum::Office->value) : ProductTypeFactory::new()->office(),
            'productProvider' => $story ? ProductProviderStory::hiscoxMRP() : ProductProviderFactory::new()->hiscoxMRP(),
        ]);
    }

and linkedToStello content is

    public function linkedToStello(bool $story = true): self
    {
        return $this->with(static fn () => ['businessProvider' => $story ? BusinessProviderStory::get(BusinessProvider::DEFAULT_BUSINESS_PROVIDER) : BusinessProviderFactory::new()->stello()]);
    }

ActivityStory, ProductTypeStory, ProductProviderStory, BusinessProviderStory are declared as global

nikophil commented 1 week ago

ok, so it seems that all these missing entities in the ORM are created in the global state... But I don't know why they are not managed at the time of flushing, and more over why flush_after() creates an error :thinking:

do you have multiple entity managers?

If you could create a public reproducer, this would be really helpful :pray:

pmontoya commented 1 week ago

I have multiple entity managers but all entities on error are on the same one. The second one is readonly.

I'll try to create a public reproducer soon

pmontoya commented 1 week ago

@nikophil : reproducer created : https://github.com/pmontoya/foundry-reproducer

If I remove the GroupStory from global state, test is ok.

nikophil commented 1 week ago

thank you for this reproducer!

now I understand the problem and... it is a tough one :sweat_smile:

When entities are created with global state, they are stored/retrieved within a static property. But they are "forgotten" by Doctrine because at some point $em->clear is called (because the kernel is shutdown after the global state is created). This problem is basically fixed thanks to the proxy autorefresh behavior.

On the other hand, when we call flush_after() we're doing:

// \Zenstruck\Foundry\Persistence\PersistenceManager

public function flushAfter(callable $callback): void
{
    $this->flush = false;

    $callback();

    $this->flush = true;

    // do flush...
}

But here is what the refresh() method looks like:

public function refresh(object &$object): object
{
    if (!$this->flush) {
        return $object;
    }

    // do refresh...
}

So basically, we're not refreshing the object in flush_after() and then Doctrine does not know about the objects created in global state. If you would have activated cascading persist, it would have resulted into the creation of a duplicate Group entity.

We've added this condition otherwise there would be too much problem with autorefresh in flush_after() callbacks (see this test which tests this behavior).

As a fix, I think we should force the refresh of objects in Story::getState(). I'm reluctant to add a bool $force parameter to Proxy::_refresh() (which is a public interface), but maybe we could at this param to PersistentManager::refresh(), which is internal. any thoughts @kbond ?

nikophil commented 1 week ago

see my proposal https://github.com/zenstruck/foundry/pull/653