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

Attributes added in beforeInstantiate are not passed to afterPersist #634

Draft
wants to merge 2 commits into
base: 2.x
Choose a base branch
from

Conversation

kbond
Copy link
Member

@kbond kbond commented Jun 12, 2024

Followup to quick fix in #631 that demonstrates an edge case it didn't fix.

@kbond kbond added the bug Something isn't working label Jun 12, 2024
@nikophil nikophil force-pushed the bug/before-instantiate-attribute branch from 74abce2 to 86012dd Compare June 19, 2024 14:39
@nikophil nikophil marked this pull request as ready for review June 19, 2024 14:41
* @internal
* @var Parameters|null
*/
protected array|null $normalizedParameters = null;
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nikophil, don't we need to set this back to null on clone?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

need to test this

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok, so... an easy test like this one (that I added to our test suite) was failing because of the memoization:

    public function can_create_different_objects_based_on_same_factory(): void
    {
        $factory = Object1Factory::new(['prop1' => 'first object']);
        $object1 = $factory->create();
        self::assertSame('first object-constructor', $object1->getProp1());

        $object2 = $factory->create(['prop1' => 'second object']);
        self::assertSame('second object-constructor', $object2->getProp1());

        $object3 = $factory->with(['prop1' => 'third object'])->create();
        self::assertSame('third object-constructor', $object3->getProp1());
    }

I'm really not proud of the solution I found (replace $this->normalizedParameters ??= by $this->normalizedParameters = in Factory::normalizeParameters(), but it's the only simple one I found 🤷

@nikophil nikophil marked this pull request as draft June 26, 2024 18:29
@nikophil nikophil force-pushed the bug/before-instantiate-attribute branch from 86012dd to 093330a Compare June 26, 2024 19:02
@nikophil nikophil force-pushed the bug/before-instantiate-attribute branch from 093330a to 5e44ec8 Compare June 26, 2024 19:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Development

Successfully merging this pull request may close these issues.

2 participants