fix(doctrine): odm order filter should use a left join on nullable fields (#5911)

Co-authored-by: Jannik Wettengel <[email protected]>

Author: skalrayi

src/Doctrine/Odm/Extension/OrderExtension.php

@@ -64,7 +64,7 @@ public function applyToCollection(Builder $aggregationBuilder, string $resourceC
                 }
 
                 if ($this->isPropertyNested($field, $resourceClass)) {
-                    [$field] = $this->addLookupsForNestedProperty($field, $aggregationBuilder, $resourceClass);
+                    [$field] = $this->addLookupsForNestedProperty($field, $aggregationBuilder, $resourceClass, true);
                 }
                 $aggregationBuilder->sort(
                     $context['mongodb_odm_sort_fields'] = ($context['mongodb_odm_sort_fields'] ?? []) + [$field => $order]

src/Doctrine/Odm/Filter/OrderFilter.php

@@ -253,7 +253,7 @@ protected function filterProperty(string $property, $direction, Builder $aggrega
         $matchField = $property;
 
         if ($this->isPropertyNested($property, $resourceClass)) {
-            [$matchField] = $this->addLookupsForNestedProperty($property, $aggregationBuilder, $resourceClass);
+            [$matchField] = $this->addLookupsForNestedProperty($property, $aggregationBuilder, $resourceClass, true);
         }
 
         $aggregationBuilder->sort(

src/Doctrine/Odm/PropertyHelperTrait.php

@@ -60,7 +60,7 @@ protected function getClassMetadata(string $resourceClass): ClassMetadata
      *               the second element is the $field name
      *               the third element is the $associations array
      */
-    protected function addLookupsForNestedProperty(string $property, Builder $aggregationBuilder, string $resourceClass): array
+    protected function addLookupsForNestedProperty(string $property, Builder $aggregationBuilder, string $resourceClass, bool $preserveNullAndEmptyArrays = false): array
     {
         $propertyParts = $this->splitPropertyParts($property, $resourceClass);
         $alias = '';
@@ -98,7 +98,8 @@ protected function addLookupsForNestedProperty(string $property, Builder $aggreg
                     ->localField($isOwningSide ? $localField : '_id')
                     ->foreignField($isOwningSide ? '_id' : $referenceMapping['mappedBy'])
                     ->alias($alias);
-                $aggregationBuilder->unwind("\$$alias");
+                $aggregationBuilder->unwind("\$$alias")
+                    ->preserveNullAndEmptyArrays($preserveNullAndEmptyArrays);
 
                 // association.property => association_lkup.property
                 $property = substr_replace($property, $propertyAlias, strpos($property, (string) $association), \strlen((string) $association));

tests/Doctrine/Common/Filter/OrderFilterTestTrait.php

@@ -325,6 +325,14 @@ private static function provideApplyTestArguments(): array
                     ],
                 ],
             ],
+            'nullable field in relation will be a LEFT JOIN' => [
+                [
+                    'relatedDummy.name' => null,
+                ],
+                [
+                    'order' => ['relatedDummy.name' => 'ASC'],
+                ],
+            ],
         ];
     }
 }

tests/Doctrine/Odm/Extension/OrderExtensionTest.php

@@ -19,6 +19,7 @@
 use Doctrine\ODM\MongoDB\Aggregation\Builder;
 use Doctrine\ODM\MongoDB\Aggregation\Stage\Lookup;
 use Doctrine\ODM\MongoDB\Aggregation\Stage\Sort;
+use Doctrine\ODM\MongoDB\Aggregation\Stage\Unwind;
 use Doctrine\ODM\MongoDB\DocumentManager;
 use Doctrine\ODM\MongoDB\Mapping\ClassMetadata;
 use Doctrine\Persistence\ManagerRegistry;
@@ -128,7 +129,9 @@ public function testApplyToCollectionWithOrderOverriddenWithAssociation(): void
         $lookupProphecy->foreignField('_id')->shouldBeCalled()->willReturn($lookupProphecy);
         $lookupProphecy->alias('author_lkup')->shouldBeCalled();
         $aggregationBuilderProphecy->lookup(Dummy::class)->shouldBeCalled()->willReturn($lookupProphecy->reveal());
-        $aggregationBuilderProphecy->unwind('$author_lkup')->shouldBeCalled();
+        $unwindProphecy = $this->prophesize(Unwind::class);
+        $unwindProphecy->preserveNullAndEmptyArrays(true)->shouldBeCalled();
+        $aggregationBuilderProphecy->unwind('$author_lkup')->shouldBeCalled()->willReturn($unwindProphecy->reveal());
         $aggregationBuilderProphecy->getStage(0)->willThrow(new \OutOfRangeException('message'));
         $aggregationBuilderProphecy->sort(['author_lkup.name' => 'ASC'])->shouldBeCalled();
 

tests/Doctrine/Odm/Filter/OrderFilterTest.php

@@ -343,7 +343,10 @@ public static function provideApplyTestData(): array
                             ],
                         ],
                         [
-                            '$unwind' => '$relatedDummy_lkup',
+                            '$unwind' => [
+                                'path' => '$relatedDummy_lkup',
+                                'preserveNullAndEmptyArrays' => true,
+                            ],
                         ],
                         [
                             '$sort' => [
@@ -514,7 +517,10 @@ public static function provideApplyTestData(): array
                             ],
                         ],
                         [
-                            '$unwind' => '$relatedDummy_lkup',
+                            '$unwind' => [
+                                'path' => '$relatedDummy_lkup',
+                                'preserveNullAndEmptyArrays' => true,
+                            ],
                         ],
                         [
                             '$sort' => [
@@ -546,6 +552,30 @@ public static function provideApplyTestData(): array
                     $orderFilterFactory,
                     EmbeddedDummy::class,
                 ],
+                'nullable field in relation will be a LEFT JOIN' => [
+                    [
+                        [
+                            '$lookup' => [
+                                'from' => 'RelatedDummy',
+                                'localField' => 'relatedDummy',
+                                'foreignField' => '_id',
+                                'as' => 'relatedDummy_lkup',
+                            ],
+                        ],
+                        [
+                            '$unwind' => [
+                                'path' => '$relatedDummy_lkup',
+                                'preserveNullAndEmptyArrays' => true,
+                            ],
+                        ],
+                        [
+                            '$sort' => [
+                                'relatedDummy_lkup.name' => 1,
+                            ],
+                        ],
+                    ],
+                    $orderFilterFactory,
+                ],
             ]
         );
     }

tests/Doctrine/Orm/Filter/OrderFilterTest.php

@@ -414,6 +414,11 @@ public static function provideApplyTestData(): array
                     $orderFilterFactory,
                     EmbeddedDummy::class,
                 ],
+                'nullable field in relation will be a LEFT JOIN' => [
+                    sprintf('SELECT o FROM %s o LEFT JOIN o.relatedDummy relatedDummy_a1 ORDER BY relatedDummy_a1.name ASC', Dummy::class),
+                    null,
+                    $orderFilterFactory,
+                ],
             ]
         );
     }