#59612 - Add test to prove behaviour#59700
Closed
justlunix wants to merge 1 commit into
Closed
Conversation
Member
|
We don't really do anything with these types of PRs, we would need a fix PR. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Background & Problem
Eloquent models support a trait initializer convention: any trait used on a model can define an
initialize<TraitName>()method that gets called automatically during model construction.For example, some traits need to call
getCasts()during their initialization — i.e. to inspect what casts are defined on the model. The issue is that this only works correctly ifHasAttributes::initializeHasAttributes()has already run by the time those dependent trait initializers execute, sinceHasAttributesis responsible for setting up the casts infrastructure. Without a guaranteed initialization order, traits relying ongetCasts()could silently receive incomplete or empty results.With PHP 8.5 there have been fixes concerning the order on how trait methods are loaded. This results in the underlying architecture of the initialization logic to produce a different order of initializations. More info is described in #59612.
This PR
This PR adds a regression test that explicitly verifies the initialization order is correct. The test creates a model that uses a trait whose initializer calls
getCasts(), and asserts that the casts were available at that point. If the order ever regresses, the test will catch it immediately.This approach was chosen because the fix for the underlying behavior (issue #59612) lives elsewhere - this PR's role is to lock in the expected contract with a test, preventing future regressions.
Possible fix
Described in the issue #59612.
Test Protocol
getCasts()returns the model's defined casts when called from within a trait initializer