Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 22 additions & 6 deletions lib/Doctrine/ODM/MongoDB/Aggregation/Builder.php
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
use GeoJson\Geometry\Point;
use MongoDB\Collection;
use OutOfRangeException;
use stdClass;
use TypeError;

use function array_map;
Expand Down Expand Up @@ -264,6 +265,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
Expand Down Expand Up @@ -306,14 +312,20 @@ public function getPipeline(/* bool $applyFilters = true */): array
return $pipeline;
}

if ($this->getStage(0) instanceof Stage\GeoNear) {
if (isset($pipeline[0]['$geoNear'])) {
Copy link
Member Author

@GromNaN GromNaN Mar 3, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is more efficient and more resilient if we introduce changes on the $pipeline array like #2740.

$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 ((array) $matchExpression !== []) {
array_unshift($pipeline, ['$match' => $matchExpression]);
}

Expand Down Expand Up @@ -696,18 +708,22 @@ public function addStage(Stage $stage): Stage
/**
* Applies filters and discriminator queries to the pipeline
*
* @param array<string, mixed> $query
* @param array<string, mixed>|stdClass $query
*
* @return array<string, mixed>
* @return array<string, mixed>|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;
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have to juggle between array and empty stdClass to ensure that we have a map when converting to BSON.

}

private function getDocumentPersister(): DocumentPersister
Expand Down
3 changes: 0 additions & 3 deletions lib/Doctrine/ODM/MongoDB/Persisters/DocumentPersister.php
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
30 changes: 30 additions & 0 deletions tests/Doctrine/ODM/MongoDB/Tests/Aggregation/BuilderTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -368,6 +368,36 @@ 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' => [
[
'stype' => 'server_guest',
'$text' => ['$search' => 'Paul'],
],
['filtered' => true],
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The query here seems odd, but I understand that's because addDiscriminatorToPreparedQuery() sets the key directly and addFilterToPreparedQuery() uses CriteriaMerger, which will always adds $and for 2+ criteria.

],
],
],
];

self::assertEquals($expectedPipeline, $builder->getPipeline());
}

public function testBuilderAppliesFilterAndDiscriminatorWithGeoNearStage(): void
{
$this->dm->getFilterCollection()->enable('testFilter');
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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());
Comment on lines +75 to +76
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was an error: $geoNear.query does not accept an empty array

MongoServerError[TypeMismatch]: query must be an object

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this suggest that the geoNear() builder method never worked in previous versions when query criteria was omitted? That seems like something worthy of its own ticket for changelog visibility, even if you want to keep the fix in this PR.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Explicitly fixed in #2743.

}
}
18 changes: 18 additions & 0 deletions tests/Doctrine/ODM/MongoDB/Tests/Functional/FilterTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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());
}
}
Loading