Fix regression with an edge case for the CMF::peekIfIsMappedSuperclass() method#10557
Fix regression with an edge case for the CMF::peekIfIsMappedSuperclass() method#10557mpdude wants to merge 1 commit intodoctrine:2.15.xfrom
CMF::peekIfIsMappedSuperclass() method#10557Conversation
70fc74b to
5408ffd
Compare
CMF::peekIfIsMappedSuperclass() methodCMF::peekIfIsMappedSuperclass() method
5408ffd to
a36868d
Compare
|
@derrabus Can you tell me how to proceed with Psalm errors here? |
|
Your class extends |
|
Thank you, will 👀 |
72a8f3e to
205a50a
Compare
|
Ah, I get it! I am consuming one of the baseline entries with the change I made, so we're out of baseline entries at the end of the file and the error pops up there. |
…lassMetadataFactory::peekIfIsMappedSuperclass()` method introduced in doctrine#10411 causes a failure. `CMF::peekIfIsMappedSuperclass()` has to perform improvised metadata loading in a situation where the CMF is currently loading a class. So, we cannot use the full/real `ClassMetadataFactory` mechanisms, since it would require a re-entry for a subclass of the current class, causing an infinite loop (loads parent classes first, and that's what we're currently doing). The problem is that the improvised call to `$driver->loadMetadataForClass()` cannot provide a pre-filled `ClassMetadata` instance populated with all parent class fields and associations. But, when attribute or association overrides are used, a check is made to see if the overridden field/association actually exists, and this information is missing in that situation. This PR suggests to override the methods to get around this. In fact, we do not care about all these details, we only want to ask the driver if the class is a mapped superclass or not. A much better fix would be to have a dedicated method on the driver to ask it just that particular question (also better performance-wise). But I do not see how we could get that done in a BC way – ideas? 💡 A few things that need to come together to make the bug surface: * Load an entity declaring an inheritance tree * There must be a mapped superclass in the inheritance tree to provide the field that shall be overriden * An entity class must inherit from the mapped superclass and override the field * That entity class must be an abstract, intermediate class not be declared in the discriminator map so we can "discover" it * The overriden property must be private so the mapping drivers (using reflection) do not see it when looking at the overriding entity class.
205a50a to
50d6451
Compare
Add the method to all existing drivers, and deprecate not adding it to a driver (and fallback to the old behavior when that's the case). You can also add an |
|
There hasn't been any activity on this pull request in the past 90 days, so it has been marked as stale and it will be closed automatically if no further activity occurs in the next 7 days. |
|
This pull request was closed due to inactivity. |
This comment hints to a case where the
ClassMetadataFactory::peekIfIsMappedSuperclass()method introduced in #10411 causes a failure.CMF::peekIfIsMappedSuperclass()has to perform improvised metadata loading in a situation where the CMF is currently loading a class. So, we cannot use the full/realClassMetadataFactorymechanisms, since it would require a re-entry for a subclass of the current class, causing an infinite loop (loads parent classes first, and that's what we're currently doing).The problem is that the improvised call to
$driver->loadMetadataForClass()cannot provide a pre-filledClassMetadatainstance populated with all parent class fields and associations. But, when attribute or association overrides are used, a check is made to see if the overridden field/association actually exists, and this information is missing in that situation.This PR suggests to override the methods to get around this. In fact, we do not care about all these details, we only want to ask the driver if the class is a mapped superclass or not.
A much better fix would be to have a dedicated method on the driver to ask it just that particular question (also better performance-wise). But I do not see how we could get that done in a BC way – ideas? 💡
A few things that need to come together to make the bug surface: