Bug/2157 Test (and fix) Facet's aggregation pipeline with DiscriminatorMap documents #2160
Conversation
| */ | ||
| public function getPipeline() : array | ||
| // phpcs:enable Squiz.Commenting.FunctionComment.ExtraParamComment | ||
| public function getPipeline(/* bool $applyFilters = true */) : array |
There was a problem hiding this comment.
Since we need to preserve BC, we can't add a new method argument. Instead, we add it commented and use func_num_args to check for it being given. The if below then emulates strict typing, throwing an exception if we get anything but a bool.
Note that I've also changed the logic of the argument: instead of calling it isFacet, we call it applyFilters. This is mainly because upcoming changes to the $lookup stage will require us using the same functionality, so isFacet would be the wrong name.
There was a problem hiding this comment.
I think we'd do just fine with new argument with default value, but since you've gone the extra mile with this approach let's keep it 👍 The only possible downside I see is PHPStan complaining about passing a parameter that is not there. Unless it'll take phpdoc's word?
There was a problem hiding this comment.
It should take PHPDoc's word. However, adding a new parameter, even with default value, is a BC break: https://3v4l.org/jobIX
There was a problem hiding this comment.
It sure is, my point was more "who extends a builder" :) Sorry for not making myself clear
There was a problem hiding this comment.
OTOH the builder is not final so my point was invalid
| if ($this->getStage(0) instanceof Stage\GeoNear) { | ||
| $pipeline[0]['$geoNear']['query'] = $this->applyFilters($pipeline[0]['$geoNear']['query']); | ||
| } elseif ($this->getStage(0) instanceof Stage\IndexStats) { | ||
| if ($this->getStage(0) instanceof Stage\IndexStats) { |
There was a problem hiding this comment.
Applied some formatting and early exits here to avoid unnecessary nesting.
Summary
Provide a bool flag for
getPipeline()if it's being called from inside theFacetand test case withDiscriminatorMapannotated document.