fix: Don't process subclass indexes with SingleCollection inheritance #2573
Conversation
buffcode
force-pushed
the
fix/2.5.x-single-collection-indexes
branch
from
152a906
to
a9137a6
Compare
|
@malarzm I am not too familiar with psalm and how to fix this CI error, as it appears to be in the baseline file already: |
There was a problem hiding this comment.
As for the psalm error please try updating baseline (vendor/bin/psalm --threads=$(nproc) --php-version=8.1 --update-baseline --set-baseline=psalm-baseline.xml) and we'll see what it produces. Most probably it was allowing single usages of offending code and now there will be multiple :)
| $this->addInheritedIndexes($class, $parent); | ||
| if ($parent->isMappedSuperclass) { | ||
| // only inherit indexes if parent won't apply them itself | ||
| $this->addInheritedIndexes($class, $parent); |
There was a problem hiding this comment.
We can't do such a thing as it changes produced ClassMetadata too much:, especially for a patch release. I'm not sure yet if the change would be OK in a minor patch anyway.
| } else { | ||
| continue; | ||
| } | ||
| if ($class->isInheritanceTypeSingleCollection()) { |
There was a problem hiding this comment.
I'd add a failsafe throwing an \InvalidArgumentException when a document that has $class->parentClasses is passed here
| foreach ($processClasses as $class) { | ||
| $visited[$class->name] = true; | ||
|
|
||
| $indexes = array_merge($indexes, $this->prepareIndexes($class)); |
There was a problem hiding this comment.
If I'm not mistaken we should be able to work around my first comment here by applying array_unique or calculating some kind of key for an index that's shared between classes
There was a problem hiding this comment.
This will be dirtier than what you're proposing but we're not changing behaviour that was established for years
Summary
When using single collection inheritance, all sub classes are processed as well and indexes get dropped or re-created, depending on class order.
With this PR only the parent classes are processed and will ensure the indexes of all subclasses.