Merge pull request doctrine#2160 from Ludo444/bug/2157

Bug/2157 Test (and fix) Facet's aggregation pipeline with DiscriminatorMap documents

Author: alcaeus

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,36 +219,63 @@ 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();
             },
             $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;

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),
         ];
     }

tests/Doctrine/ODM/MongoDB/Tests/Functional/Ticket/GH2157Test.php

@@ -0,0 +1,70 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Doctrine\ODM\ODM\Tests\Functional\Ticket;
+
+use Doctrine\ODM\MongoDB\Mapping\Annotations as ODM;
+use Doctrine\ODM\MongoDB\Tests\BaseTest;
+use function count;
+
+class GH2157Test extends BaseTest
+{
+    public function testFacetDiscriminatorMapCreation()
+    {
+        $this->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
+{
+}