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

[11.x]: Factories: Ensure afterMaking is called prior to create() attributes being set #53294

Draft
wants to merge 1 commit into
base: 11.x
Choose a base branch
from

Conversation

claudiodekker
Copy link
Contributor

@claudiodekker claudiodekker commented Oct 24, 2024

The Problem

Today, the create attributes in the Factory are ignored/overwritten by those "defaulted" in afterMaking, because of the following sequence of events:

  1. create($attributes)
  2. state($attributes) (Sets the $attributes passed to create as the last-to-run state transformer)
  3. make($attributes)
  4. makeInstance() (Actually unpacks/resolves the to-be-made attributes)
  5. callAfterMaking()
  6. save
  7. callAfterCreating()

Instead, the attributes passed to create should be filled after callAfterMaking has finished, just prior to save being called on the model.

This however isn't as simple as it sounds, because it should be possible to reference things like factory methods in create() that would override those in make(). For example:

->create([
    'user_id' => User::factory()->subscribed()->verified(),
]);

This effectively means we've got a chicken-egg problem, where the model is already instantiated (but not yet created), while the create method itself still has state to set/resolve.

This Solution

Since the state($attributes) called in step 2 is appended to a (potential) list of other state transformers, as well as because they're ran in sequence, we know that it's the last transformer to run prior to Model instantiation / attribute expansion.

By wrapping this very last $attributes state call with an interceptor method, we're able to tap in to the raw attributes (e.g. related Factory instances, per-attribute resolver callbacks) right before they're resolved/unpacked by expandAttributes().

This is useful, because while we don't necessarily have access to useful values yet at this point, it does mean that we're able to see which attributes keys the user passed in to create($attributes), and track them in a property on the Factory instance.

Side-note: While this could be done by doing something like $keys = array_keys($attributes) as the first line in create($attributes), this won't work in case the user's provided a callback/resolver, and while we could resolve this in-place first as well, this could then lead to issues due to the end-user's custom logic being ran more than once.

Next, during step 4 in the makeInstance method, the Eloquent-ready attributes are expanded (using getExpandedAttributes) just prior to calling new Model($attributes). This again provides us with an opportunity, because at this point we have access to the actual attributes that would otherwise be used to instantiate the model.

Now, because we only track the custom attribute keys when create() has already been called (which again, is the last step in a Factory chain), we can simply check if there are any keys on our "tracked" property, and if there are, we can just grab that subset of values and have what is effectively a complete "create()"-method dataset. The only downside at this point however is that we're still in the middle of the make() call, meaning that the only thing we can do for now is to track them in the same way as we tracked the keys themselves.

Finally, once the make() call returns in the create() method, with the Model effectively instantiated and the afterMaking calls having ran as part of the make method, we use forceFill to set the collected/tracked subset of "create" attributes on the model, leading to the intended result.

This comment was marked as resolved.

@claudiodekker claudiodekker marked this pull request as ready for review October 24, 2024 21:40
@claudiodekker claudiodekker marked this pull request as draft October 24, 2024 21:57
@claudiodekker claudiodekker marked this pull request as ready for review October 24, 2024 22:24
@claudiodekker claudiodekker marked this pull request as draft October 24, 2024 22:31
@claudiodekker claudiodekker force-pushed the fix-factory-after-making-create-override branch from ca9ef56 to db7c530 Compare October 25, 2024 14:05
@claudiodekker claudiodekker marked this pull request as ready for review October 25, 2024 14:05
@claudiodekker claudiodekker changed the title Factories: Ensure afterMaking gets called prior to create() attributes being set [11.x]: Factories: Ensure afterMaking is called prior to create() attributes being set Oct 25, 2024
@taylorotwell taylorotwell marked this pull request as draft October 29, 2024 20:30
@morloderex
Copy link
Contributor

I wonder how these changes effects sequences?

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

Successfully merging this pull request may close these issues.

2 participants