From eaca88451c9bfdf2d8446f4d160ff41d28e7d66d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A9r=C3=B4me=20Tamarelle?= Date: Sat, 1 Mar 2025 00:26:29 +0100 Subject: [PATCH 1/2] Add support for using $text operator in a aggregation with a filter --- .../ODM/MongoDB/Aggregation/Builder.php | 17 +++++++++- .../MongoDB/Tests/Aggregation/BuilderTest.php | 33 +++++++++++++++++++ .../MongoDB/Tests/Functional/FilterTest.php | 18 ++++++++++ 3 files changed, 67 insertions(+), 1 deletion(-) diff --git a/lib/Doctrine/ODM/MongoDB/Aggregation/Builder.php b/lib/Doctrine/ODM/MongoDB/Aggregation/Builder.php index cbc62a3b8f..58daf10e0e 100644 --- a/lib/Doctrine/ODM/MongoDB/Aggregation/Builder.php +++ b/lib/Doctrine/ODM/MongoDB/Aggregation/Builder.php @@ -264,6 +264,11 @@ public function getAggregation(array $options = []): IterableResult * @param bool $applyFilters Whether to apply filters on the aggregation * pipeline stage * + * For pipelines where the first stage is a $match stage, it will merge + * the document filters with the existing stage in a logical $and. This is + * required as $text operator can be used anywhere in the first $match stage + * or in the document filters. + * * 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 @@ -314,7 +319,17 @@ public function getPipeline(/* bool $applyFilters = true */): array $matchExpression = $this->applyFilters([]); if ($matchExpression !== []) { - array_unshift($pipeline, ['$match' => $matchExpression]); + if (isset($pipeline[0]['$match'])) { + // Merge filters into the first $match stage with a logical $and + $pipeline[0]['$match'] = [ + '$and' => [ + $matchExpression, + $pipeline[0]['$match'], + ], + ]; + } else { + array_unshift($pipeline, ['$match' => $matchExpression]); + } } return $pipeline; diff --git a/tests/Doctrine/ODM/MongoDB/Tests/Aggregation/BuilderTest.php b/tests/Doctrine/ODM/MongoDB/Tests/Aggregation/BuilderTest.php index 0fc3dad8b1..feb0c2836d 100644 --- a/tests/Doctrine/ODM/MongoDB/Tests/Aggregation/BuilderTest.php +++ b/tests/Doctrine/ODM/MongoDB/Tests/Aggregation/BuilderTest.php @@ -368,6 +368,39 @@ public function testBuilderAppliesFilterAndDiscriminatorWithMatchStage(): void self::assertEquals($expectedPipeline, $builder->getPipeline()); } + public function testBuilderMergeFilterAndDiscriminatorWithMatchStage(): void + { + $this->dm->getFilterCollection()->enable('testFilter'); + $filter = $this->dm->getFilterCollection()->getFilter('testFilter'); + $filter->setParameter('class', GuestServer::class); + $filter->setParameter('field', 'filtered'); + $filter->setParameter('value', true); + + $builder = $this->dm->createAggregationBuilder(GuestServer::class); + $builder + ->match() + ->text('Paul'); + + $expectedPipeline = [ + [ + '$match' => [ + '$and' => [ + [ + '$and' => [ + ['stype' => 'server_guest'], + ['filtered' => true], + ], + ], + ['$text' => ['$search' => 'Paul']], + ], + ], + + ], + ]; + + self::assertEquals($expectedPipeline, $builder->getPipeline()); + } + public function testBuilderAppliesFilterAndDiscriminatorWithGeoNearStage(): void { $this->dm->getFilterCollection()->enable('testFilter'); diff --git a/tests/Doctrine/ODM/MongoDB/Tests/Functional/FilterTest.php b/tests/Doctrine/ODM/MongoDB/Tests/Functional/FilterTest.php index 69f61000e8..c8d5d5d13e 100644 --- a/tests/Doctrine/ODM/MongoDB/Tests/Functional/FilterTest.php +++ b/tests/Doctrine/ODM/MongoDB/Tests/Functional/FilterTest.php @@ -334,4 +334,22 @@ public function testMultipleFiltersOnSameField(): void */ self::assertEmpty($this->getUsernamesWithFindAll()); } + + public function testFilterOnMatchStageUsingTextOperator(): void + { + $this->dm->getDocumentCollection(User::class)->createIndex(['username' => 'text']); + + $this->fc->enable('testFilter'); + $testFilter = $this->fc->getFilter('testFilter'); + $testFilter->setParameter('class', User::class); + $testFilter->setParameter('field', 'hits'); + $testFilter->setParameter('value', 10); + + $builder = $this->dm->createAggregationBuilder(User::class) + ->match() + ->field('username') + ->text('John'); + + self::assertCount(1, $builder->getAggregation()->execute()); + } } From 005ddf78f652f7a91a5138c7b48249ed97bd5ff0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A9r=C3=B4me=20Tamarelle?= Date: Mon, 3 Mar 2025 23:09:08 +0100 Subject: [PATCH 2/2] Leverage CriteriaMerger --- .../ODM/MongoDB/Aggregation/Builder.php | 35 ++++++++++--------- .../MongoDB/Persisters/DocumentPersister.php | 3 -- .../MongoDB/Tests/Aggregation/BuilderTest.php | 9 ++--- .../Tests/Aggregation/Stage/GeoNearTest.php | 4 +-- 4 files changed, 23 insertions(+), 28 deletions(-) diff --git a/lib/Doctrine/ODM/MongoDB/Aggregation/Builder.php b/lib/Doctrine/ODM/MongoDB/Aggregation/Builder.php index 58daf10e0e..f13be1b134 100644 --- a/lib/Doctrine/ODM/MongoDB/Aggregation/Builder.php +++ b/lib/Doctrine/ODM/MongoDB/Aggregation/Builder.php @@ -14,6 +14,7 @@ use GeoJson\Geometry\Point; use MongoDB\Collection; use OutOfRangeException; +use stdClass; use TypeError; use function array_map; @@ -311,25 +312,21 @@ public function getPipeline(/* bool $applyFilters = true */): array return $pipeline; } - if ($this->getStage(0) instanceof Stage\GeoNear) { + if (isset($pipeline[0]['$geoNear'])) { $pipeline[0]['$geoNear']['query'] = $this->applyFilters($pipeline[0]['$geoNear']['query']); return $pipeline; } + if (isset($pipeline[0]['$match'])) { + $pipeline[0]['$match'] = $this->applyFilters($pipeline[0]['$match']); + + return $pipeline; + } + $matchExpression = $this->applyFilters([]); - if ($matchExpression !== []) { - if (isset($pipeline[0]['$match'])) { - // Merge filters into the first $match stage with a logical $and - $pipeline[0]['$match'] = [ - '$and' => [ - $matchExpression, - $pipeline[0]['$match'], - ], - ]; - } else { - array_unshift($pipeline, ['$match' => $matchExpression]); - } + if ((array) $matchExpression !== []) { + array_unshift($pipeline, ['$match' => $matchExpression]); } return $pipeline; @@ -711,18 +708,22 @@ public function addStage(Stage $stage): Stage /** * Applies filters and discriminator queries to the pipeline * - * @param array $query + * @param array|stdClass $query * - * @return array + * @return array|stdClass */ - private function applyFilters(array $query): array + private function applyFilters(array|stdClass $query): array|stdClass { + if (! is_array($query)) { + $query = (array) $query; + } + $documentPersister = $this->getDocumentPersister(); $query = $documentPersister->addDiscriminatorToPreparedQuery($query); $query = $documentPersister->addFilterToPreparedQuery($query); - return $query; + return $query === [] ? (object) $query : $query; } private function getDocumentPersister(): DocumentPersister diff --git a/lib/Doctrine/ODM/MongoDB/Persisters/DocumentPersister.php b/lib/Doctrine/ODM/MongoDB/Persisters/DocumentPersister.php index da09225913..c322ec2d98 100644 --- a/lib/Doctrine/ODM/MongoDB/Persisters/DocumentPersister.php +++ b/lib/Doctrine/ODM/MongoDB/Persisters/DocumentPersister.php @@ -1029,9 +1029,6 @@ public function addFilterToPreparedQuery(array $preparedQuery): array { /* If filter criteria exists for this class, prepare it and merge * over the existing query. - * - * @todo Consider recursive merging in case the filter criteria and - * prepared query both contain top-level $and/$or operators. */ $filterCriteria = $this->dm->getFilterCollection()->getFilterCriteria($this->class); if ($filterCriteria) { diff --git a/tests/Doctrine/ODM/MongoDB/Tests/Aggregation/BuilderTest.php b/tests/Doctrine/ODM/MongoDB/Tests/Aggregation/BuilderTest.php index feb0c2836d..238ce0e3b6 100644 --- a/tests/Doctrine/ODM/MongoDB/Tests/Aggregation/BuilderTest.php +++ b/tests/Doctrine/ODM/MongoDB/Tests/Aggregation/BuilderTest.php @@ -386,15 +386,12 @@ public function testBuilderMergeFilterAndDiscriminatorWithMatchStage(): void '$match' => [ '$and' => [ [ - '$and' => [ - ['stype' => 'server_guest'], - ['filtered' => true], - ], + 'stype' => 'server_guest', + '$text' => ['$search' => 'Paul'], ], - ['$text' => ['$search' => 'Paul']], + ['filtered' => true], ], ], - ], ]; diff --git a/tests/Doctrine/ODM/MongoDB/Tests/Aggregation/Stage/GeoNearTest.php b/tests/Doctrine/ODM/MongoDB/Tests/Aggregation/Stage/GeoNearTest.php index 687ff1a5de..8e7d898067 100644 --- a/tests/Doctrine/ODM/MongoDB/Tests/Aggregation/Stage/GeoNearTest.php +++ b/tests/Doctrine/ODM/MongoDB/Tests/Aggregation/Stage/GeoNearTest.php @@ -72,7 +72,7 @@ public function testLimitDoesNotCreateExtraStage(): void ->geoNear(0, 0) ->limit(1); - $stage = ['near' => [0, 0], 'spherical' => false, 'distanceField' => null, 'query' => [], 'num' => 1]; - self::assertSame([['$geoNear' => $stage]], $builder->getPipeline()); + $stage = ['near' => [0, 0], 'spherical' => false, 'distanceField' => null, 'query' => (object) [], 'num' => 1]; + self::assertEquals([['$geoNear' => $stage]], $builder->getPipeline()); } }