From 58a92a2c25cad6a0aecfb5912754867c66d2fd67 Mon Sep 17 00:00:00 2001 From: ludo444 Date: Tue, 28 Jan 2020 20:06:38 +0200 Subject: [PATCH] Don't apply filters when getting nested pipelines --- .../ODM/MongoDB/Aggregation/Builder.php | 50 ++++++++++--- .../ODM/MongoDB/Aggregation/Stage/Facet.php | 2 +- .../Tests/Functional/Ticket/GH2157Test.php | 70 +++++++++++++++++++ 3 files changed, 112 insertions(+), 10 deletions(-) create mode 100644 tests/Doctrine/ODM/MongoDB/Tests/Functional/Ticket/GH2157Test.php diff --git a/lib/Doctrine/ODM/MongoDB/Aggregation/Builder.php b/lib/Doctrine/ODM/MongoDB/Aggregation/Builder.php index c2ff35196e..5d201957e8 100644 --- a/lib/Doctrine/ODM/MongoDB/Aggregation/Builder.php +++ b/lib/Doctrine/ODM/MongoDB/Aggregation/Builder.php @@ -15,11 +15,16 @@ use MongoDB\Collection; use MongoDB\Driver\Cursor; use OutOfRangeException; +use TypeError; use function array_map; use function array_merge; use function array_unshift; use function assert; +use function func_get_arg; +use function func_num_args; +use function gettype; use function is_array; +use function is_bool; use function sprintf; /** @@ -214,16 +219,35 @@ public function geoNear($x, $y = null) : Stage\GeoNear return $stage; } + // phpcs:disable Squiz.Commenting.FunctionComment.ExtraParamComment /** * Returns the assembled aggregation pipeline * + * @param bool $applyFilters Whether to apply filters on the aggregation + * pipeline stage + * * For pipelines where the first stage is a $geoNear stage, it will apply * the document filters and discriminator queries to the query portion of * the geoNear operation. For all other pipelines, it prepends a $match stage * containing the required query. + * + * For aggregation pipelines that will be nested (e.g. in a facet stage), + * you should not apply filters as this may cause wrong results to be + * given. */ - public function getPipeline() : array + // phpcs:enable Squiz.Commenting.FunctionComment.ExtraParamComment + public function getPipeline(/* bool $applyFilters = true */) : array { + $applyFilters = func_num_args() > 0 ? func_get_arg(0) : true; + + if (! is_bool($applyFilters)) { + throw new TypeError(sprintf( + 'Argument 1 passed to %s must be of the type bool, %s given', + __METHOD__, + gettype($applyFilters) + )); + } + $pipeline = array_map( static function (Stage $stage) { return $stage->getExpression(); @@ -231,19 +255,27 @@ static function (Stage $stage) { $this->stages ); - 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) { // Don't apply any filters when using an IndexStats stage: since it // needs to be the first pipeline stage, prepending a match stage // with discriminator information will not work + $applyFilters = false; + } + + if (! $applyFilters) { return $pipeline; - } else { - $matchExpression = $this->applyFilters([]); - if ($matchExpression !== []) { - array_unshift($pipeline, ['$match' => $matchExpression]); - } + } + + if ($this->getStage(0) instanceof Stage\GeoNear) { + $pipeline[0]['$geoNear']['query'] = $this->applyFilters($pipeline[0]['$geoNear']['query']); + + return $pipeline; + } + + $matchExpression = $this->applyFilters([]); + if ($matchExpression !== []) { + array_unshift($pipeline, ['$match' => $matchExpression]); } return $pipeline; diff --git a/lib/Doctrine/ODM/MongoDB/Aggregation/Stage/Facet.php b/lib/Doctrine/ODM/MongoDB/Aggregation/Stage/Facet.php index 2563ef53c5..4e1ef20a52 100644 --- a/lib/Doctrine/ODM/MongoDB/Aggregation/Stage/Facet.php +++ b/lib/Doctrine/ODM/MongoDB/Aggregation/Stage/Facet.php @@ -28,7 +28,7 @@ public function getExpression() : array { return [ '$facet' => array_map(static function (Builder $builder) { - return $builder->getPipeline(); + return $builder->getPipeline(false); }, $this->pipelines), ]; } diff --git a/tests/Doctrine/ODM/MongoDB/Tests/Functional/Ticket/GH2157Test.php b/tests/Doctrine/ODM/MongoDB/Tests/Functional/Ticket/GH2157Test.php new file mode 100644 index 0000000000..3831aca653 --- /dev/null +++ b/tests/Doctrine/ODM/MongoDB/Tests/Functional/Ticket/GH2157Test.php @@ -0,0 +1,70 @@ +dm->persist(new GH2157FirstType()); + $this->dm->persist(new GH2157FirstType()); + $this->dm->persist(new GH2157FirstType()); + $this->dm->persist(new GH2157FirstType()); + $this->dm->flush(); + + $result = $this->dm->createAggregationBuilder(GH2157FirstType::class) + ->project() + ->includeFields(['id']) + ->facet() + ->field('count') + ->pipeline( + $this->dm->createAggregationBuilder(GH2157FirstType::class) + ->count('count') + ) + ->field('limitedResults') + ->pipeline( + $this->dm->createAggregationBuilder(GH2157FirstType::class) + ->limit(2) + ) + ->execute()->toArray(); + + $this->assertEquals(4, $result[0]['count'][0]['count']); + $this->assertEquals(2, count($result[0]['limitedResults'])); + } +} + +/** + * @ODM\Document(collection="documents") + * @ODM\InheritanceType("SINGLE_COLLECTION") + * @ODM\DiscriminatorField("type") + * @ODM\DiscriminatorMap({"firsttype"=GH2157FirstType::class, "secondtype"=GH2157SecondType::class}) + */ +abstract class GH2157Abstract +{ + /** + * @ODM\Id + * + * @var string + */ + protected $id; +} + +/** + * @ODM\Document + */ +class GH2157FirstType extends GH2157Abstract +{ +} + +/** + * @ODM\Document + */ +class GH2157SecondType extends GH2157Abstract +{ +}