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
608 stars 63 forks source link

Stories ignore persistenceless factories #588

Closed Fabrn closed 1 week ago

Fabrn commented 2 months ago

Hello there !

Context

In order to explain the full thinking process and how I was led here, I need to explain what I want to achieve : my goal is, since I am working on an app that does not work properly, I need to create items with their ID pre-defined. In order to do so, I tried to define the identifier in the attributes of the item I was creating. Sadly, Doctrine seems to overlap the ID whatever.

Being blocked by the Doctrine behavior, after analysing the situation, I thought "why not trying without persistence", so I would be done with Doctrine, and should be able to retrieve my elements.

By the way, my elements are created in a Story using Story states like so :

public function build(): void
{
    self::addState(
        'my string',
        MyFactory::createOne([
            'label' => 'something',
            'id' => 99,
        ])
    );
}

Issue

Then, I put my story in the global_state configuration so it would load automatically. And then, at the very beginning of my teams, I debugged MyFactory::all() and saw that every items had their ID reset, and most importantly, the persisted boolean was set to true , which means that the Story ignored this :

protected function initialize(): self
{
    return $this->withoutPersisting();
}

Also, I confirm that loading the Story manually creates the same problem, and creating an item using MyFactory::createOne created a non-persisted Proxy.

Solution

After some research, I found this bit of code in the Story::normalizeObject method at line 189 :

// ensure proxies are persisted
if (!$object->isPersisted()) {
    $object->save();
}

Is this an intended behavior ? If yes, shouldn't it check if persistence is enabled for the Factory ? Why the Story whould essentially need the Proxies to be persisted ?

Maybe there is something I didn't get which explains it all and it's fine 😄

nikophil commented 2 months ago

Hi @Fabrn

I don't fully understand what you're trying to achieve: without Doctrine, the objects which are created in a story are not persisted at all (moreover in a global state). They are not "stored" anywhere, where you could retrieve them.

At first, I thought your issue was the same than https://github.com/zenstruck/foundry/issues/573 but they are adding a afterInstantiate() callback, in order to store their objects somewhere (in memory, in a sql table but without using doctrine, etc...)

I debugged MyFactory::all() and saw that every items had their ID reset, and most importantly, the persisted boolean was set to true

When MyFactory::all() is called, no factory is instantiated, because the underlying call is static::repository()->findAll() so it's kinda normal that persistence is not disabled at this step.

Fabrn commented 2 months ago

Hello !

Thanks for your quick answer 😄

I don't fully understand what you're trying to achieve: without Doctrine, the objects which are created in a story are not persisted at all (moreover in a global state). They are not "stored" anywhere, where you could retrieve them.

My goal was to store non-persistent Proxies into the Story states in order to retrieve them later in the code.

When MyFactory::all() is called, no factory is instantiated, because the underlying call is static::repository()->findAll() so it's kinda normal that persistence is not disabled at this step.

My bad, didn't check what was happening under the hood there. However, the bit of code that persists the Proxies happens to be called while adding a state to the Story (addState -> normalizeObject -> persists). Shouldn't it be possible to add non-persistent Proxies to the Story states ? I would find it useful to be able to have accessible set of Proxies throught my tests that are not persisted sometimes

nikophil commented 2 months ago

My goal was to store non-persistent Proxies into the Story states in order to retrieve them later in the code.

whoops, you're right stories do keep their own states, the other part of the issue misguided me: I thought you wanted to access from MyFactory::all() to your objects created with "not persisting" stories :sweat_smile:

So I guess this PR does what you need?

Fabrn commented 2 months ago

So I guess https://github.com/zenstruck/foundry/pull/580 does what you need?

Well I guess this PR does the job indeed 😄 I wonder if it wouldn't have been cleaner to check if the save method is callable (according to persistence) instead of preventing the action behind the scenes but that's fine I guess !

nikophil commented 1 week ago

Hi! I'm closing this, since this works in Foundry v2 and we won't support this in v1