feat(elasticsearch): filtering on nested fields (#5835)

* feat(elasticsearch): filtering on nested fields * Add additional checks and function doc for less confusion * Fixing tests
api-platform · Oct 17, 2023 · 6b79b6f · 6b79b6f
1 parent 6f90626
commit 6b79b6f
Show file tree

Hide file tree

Showing 11 changed files with 242 additions and 9 deletions.
diff --git a/features/elasticsearch/match_filter.feature b/features/elasticsearch/match_filter.feature
@@ -429,3 +429,54 @@ Feature: Match filter on collections from Elasticsearch
       }
     }
     """
+
+  Scenario: Match filter on a multi-level nested property of text type with new elasticsearch operations
+    When I send a "GET" request to "/books?library.relatedGenres.name=Fiction"
+    Then the response status code should be 200
+    And the response should be in JSON
+    And the header "Content-Type" should be equal to "application/ld+json; charset=utf-8"
+    And the JSON should be valid according to this schema:
+    """
+    {
+      "type": "object",
+      "properties": {
+        "@context": {"pattern": "^/contexts/Book$"},
+        "@id": {"pattern": "^/books$"},
+        "@type": {"pattern": "^hydra:Collection$"},
+        "hydra:member": {
+          "type": "array",
+          "additionalItems": false,
+          "maxItems": 2,
+          "minItems": 2,
+          "items": [
+            {
+              "type": "object",
+              "properties": {
+                "@id": {
+                  "type": "string",
+                  "pattern": "^/books/0acfd90d-5bfe-4e42-b708-dc38bf20677c$"
+                }
+              }
+            },
+            {
+              "type": "object",
+              "properties": {
+                "@id": {
+                  "type": "string",
+                  "pattern": "^/books/f36a0026-0635-4865-86a6-5adb21d94d64$"
+                }
+              }
+            }
+          ]
+        },
+        "hydra:view": {
+          "type": "object",
+          "properties": {
+            "@id": {"pattern": "^/books\\?library.relatedGenres.name=Fiction$"},
+            "@type": {"pattern": "^hydra:PartialCollectionView$"}
+          }
+        }
+      }
+    }
+    """
+
diff --git a/features/elasticsearch/read.feature b/features/elasticsearch/read.feature
@@ -499,7 +499,7 @@ Feature: Retrieve from Elasticsearch
       },
       "hydra:search": {
         "@type": "hydra:IriTemplate",
-        "hydra:template": "/books{?order[id],order[library.id],message,message[],library.firstName,library.firstName[]}",
+        "hydra:template": "/books{?order[id],order[library.id],message,message[],library.firstName,library.firstName[],library.relatedGenres.name,library.relatedGenres.name[]}",
         "hydra:variableRepresentation": "BasicRepresentation",
         "hydra:mapping": [
           {
@@ -537,6 +537,18 @@ Feature: Retrieve from Elasticsearch
             "variable": "library.firstName[]",
             "property": "library.firstName",
             "required": false
+          },
+          {
+            "@type": "IriTemplateMapping",
+            "variable": "library.relatedGenres.name",
+            "property": "library.relatedGenres.name",
+            "required": false
+          },
+          {
+            "@type": "IriTemplateMapping",
+            "variable": "library.relatedGenres.name[]",
+            "property": "library.relatedGenres.name",
+            "required": false
           }
         ]
       }
@@ -615,7 +627,7 @@ Feature: Retrieve from Elasticsearch
       },
       "hydra:search": {
         "@type": "hydra:IriTemplate",
-        "hydra:template": "/books{?order[id],order[library.id],message,message[],library.firstName,library.firstName[]}",
+        "hydra:template": "/books{?order[id],order[library.id],message,message[],library.firstName,library.firstName[],library.relatedGenres.name,library.relatedGenres.name[]}",
         "hydra:variableRepresentation": "BasicRepresentation",
         "hydra:mapping": [
           {
@@ -653,6 +665,18 @@ Feature: Retrieve from Elasticsearch
             "variable": "library.firstName[]",
             "property": "library.firstName",
             "required": false
+          },
+          {
+            "@type": "IriTemplateMapping",
+            "variable": "library.relatedGenres.name",
+            "property": "library.relatedGenres.name",
+            "required": false
+          },
+          {
+            "@type": "IriTemplateMapping",
+            "variable": "library.relatedGenres.name[]",
+            "property": "library.relatedGenres.name",
+            "required": false
           }
         ]
       }
@@ -714,7 +738,7 @@ Feature: Retrieve from Elasticsearch
       },
       "hydra:search": {
         "@type": "hydra:IriTemplate",
-        "hydra:template": "/books{?order[id],order[library.id],message,message[],library.firstName,library.firstName[]}",
+        "hydra:template": "/books{?order[id],order[library.id],message,message[],library.firstName,library.firstName[],library.relatedGenres.name,library.relatedGenres.name[]}",
         "hydra:variableRepresentation": "BasicRepresentation",
         "hydra:mapping": [
           {
@@ -752,6 +776,18 @@ Feature: Retrieve from Elasticsearch
             "variable": "library.firstName[]",
             "property": "library.firstName",
             "required": false
+          },
+          {
+            "@type": "IriTemplateMapping",
+            "variable": "library.relatedGenres.name",
+            "property": "library.relatedGenres.name",
+            "required": false
+          },
+          {
+            "@type": "IriTemplateMapping",
+            "variable": "library.relatedGenres.name[]",
+            "property": "library.relatedGenres.name",
+            "required": false
           }
         ]
       }

diff --git a/src/Elasticsearch/Tests/Filter/MatchFilterTest.php b/src/Elasticsearch/Tests/Filter/MatchFilterTest.php
@@ -87,7 +87,7 @@ public function testApply(): void
         );
     }
 
-    public function testApplyWithNestedProperty(): void
+    public function testApplyWithNestedArrayProperty(): void
     {
         $fooType = new Type(Type::BUILTIN_TYPE_ARRAY, false, Foo::class, true, new Type(Type::BUILTIN_TYPE_INT), new Type(Type::BUILTIN_TYPE_OBJECT, false, Foo::class));
         $barType = new Type(Type::BUILTIN_TYPE_STRING);
@@ -119,6 +119,37 @@ public function testApplyWithNestedProperty(): void
         );
     }
 
+    public function testApplyWithNestedObjectProperty(): void
+    {
+        $fooType = new Type(Type::BUILTIN_TYPE_OBJECT, false, Foo::class);
+        $barType = new Type(Type::BUILTIN_TYPE_STRING);
+
+        $propertyMetadataFactoryProphecy = $this->prophesize(PropertyMetadataFactoryInterface::class);
+        $propertyMetadataFactoryProphecy->create(Foo::class, 'foo')->willReturn((new ApiProperty())->withBuiltinTypes([$fooType]))->shouldBeCalled();
+        $propertyMetadataFactoryProphecy->create(Foo::class, 'bar')->willReturn((new ApiProperty())->withBuiltinTypes([$barType]))->shouldBeCalled();
+
+        $resourceClassResolverProphecy = $this->prophesize(ResourceClassResolverInterface::class);
+        $resourceClassResolverProphecy->isResourceClass(Foo::class)->willReturn(true)->shouldBeCalled();
+
+        $nameConverterProphecy = $this->prophesize(NameConverterInterface::class);
+        $nameConverterProphecy->normalize('foo.bar', Foo::class, null, Argument::type('array'))->willReturn('foo.bar')->shouldBeCalled();
+
+        $matchFilter = new MatchFilter(
+            $this->prophesize(PropertyNameCollectionFactoryInterface::class)->reveal(),
+            $propertyMetadataFactoryProphecy->reveal(),
+            $resourceClassResolverProphecy->reveal(),
+            $this->prophesize(IriConverterInterface::class)->reveal(),
+            $this->prophesize(PropertyAccessorInterface::class)->reveal(),
+            $nameConverterProphecy->reveal(),
+            ['foo.bar' => null]
+        );
+
+        self::assertSame(
+            ['bool' => ['must' => [['match' => ['foo.bar' => 'Krupicka']]]]],
+            $matchFilter->apply([], Foo::class, null, ['filters' => ['foo.bar' => 'Krupicka']])
+        );
+    }
+
     public function testApplyWithInvalidFilters(): void
     {
         $propertyNameCollectionFactoryProphecy = $this->prophesize(PropertyNameCollectionFactoryInterface::class);

diff --git a/src/Elasticsearch/Tests/Filter/TermFilterTest.php b/src/Elasticsearch/Tests/Filter/TermFilterTest.php
@@ -87,7 +87,7 @@ public function testApply(): void
         );
     }
 
-    public function testApplyWithNestedProperty(): void
+    public function testApplyWithNestedArrayProperty(): void
     {
         $fooType = new Type(Type::BUILTIN_TYPE_ARRAY, false, Foo::class, true, new Type(Type::BUILTIN_TYPE_INT), new Type(Type::BUILTIN_TYPE_OBJECT, false, Foo::class));
         $barType = new Type(Type::BUILTIN_TYPE_STRING);
@@ -119,6 +119,37 @@ public function testApplyWithNestedProperty(): void
         );
     }
 
+    public function testApplyWithNestedObjectProperty(): void
+    {
+        $fooType = new Type(Type::BUILTIN_TYPE_OBJECT, false, Foo::class);
+        $barType = new Type(Type::BUILTIN_TYPE_STRING);
+
+        $propertyMetadataFactoryProphecy = $this->prophesize(PropertyMetadataFactoryInterface::class);
+        $propertyMetadataFactoryProphecy->create(Foo::class, 'foo')->willReturn((new ApiProperty())->withBuiltinTypes([$fooType]))->shouldBeCalled();
+        $propertyMetadataFactoryProphecy->create(Foo::class, 'bar')->willReturn((new ApiProperty())->withBuiltinTypes([$barType]))->shouldBeCalled();
+
+        $resourceClassResolverProphecy = $this->prophesize(ResourceClassResolverInterface::class);
+        $resourceClassResolverProphecy->isResourceClass(Foo::class)->willReturn(true)->shouldBeCalled();
+
+        $nameConverterProphecy = $this->prophesize(NameConverterInterface::class);
+        $nameConverterProphecy->normalize('foo.bar', Foo::class, null, Argument::type('array'))->willReturn('foo.bar')->shouldBeCalled();
+
+        $termFilter = new TermFilter(
+            $this->prophesize(PropertyNameCollectionFactoryInterface::class)->reveal(),
+            $propertyMetadataFactoryProphecy->reveal(),
+            $resourceClassResolverProphecy->reveal(),
+            $this->prophesize(IriConverterInterface::class)->reveal(),
+            $this->prophesize(PropertyAccessorInterface::class)->reveal(),
+            $nameConverterProphecy->reveal(),
+            ['foo.bar' => null]
+        );
+
+        self::assertSame(
+            ['bool' => ['must' => [['term' => ['foo.bar' => 'Krupicka']]]]],
+            $termFilter->apply([], Foo::class, null, ['filters' => ['foo.bar' => 'Krupicka']])
+        );
+    }
+
     public function testApplyWithInvalidFilters(): void
     {
         $propertyNameCollectionFactoryProphecy = $this->prophesize(PropertyNameCollectionFactoryInterface::class);

diff --git a/src/Elasticsearch/Tests/Util/FieldDatatypeTraitTest.php b/src/Elasticsearch/Tests/Util/FieldDatatypeTraitTest.php
@@ -32,6 +32,18 @@ public function testGetNestedFieldPath(): void
         $fieldDatatype = $this->getValidFieldDatatype();
 
         self::assertSame('foo.bar', $fieldDatatype->getNestedFieldPath(Foo::class, 'foo.bar.baz'));
+        self::assertNull($fieldDatatype->getNestedFieldPath(Foo::class, 'foo.baz'));
+        self::assertNull($fieldDatatype->getNestedFieldPath(Foo::class, 'baz'));
+    }
+
+    public function testGetNestedFieldInNestedCollection(): void
+    {
+        $fieldDatatype = $this->getValidFieldDatatype();
+
+        self::assertSame('foo.foo.bar', $fieldDatatype->getNestedFieldPath(Foo::class, 'foo.foo.bar.baz'));
+        self::assertNull($fieldDatatype->getNestedFieldPath(Foo::class, 'foo.foo.baz'));
+        self::assertSame('foo.bar', $fieldDatatype->getNestedFieldPath(Foo::class, 'foo.bar.foo.baz'));
+        self::assertSame('bar', $fieldDatatype->getNestedFieldPath(Foo::class, 'bar.foo'));
         self::assertNull($fieldDatatype->getNestedFieldPath(Foo::class, 'baz'));
     }
 
@@ -72,17 +84,20 @@ public function testIsNestedField(): void
         $fieldDatatype = $this->getValidFieldDatatype();
 
         self::assertTrue($fieldDatatype->isNestedField(Foo::class, 'foo.bar.baz'));
+        self::assertFalse($fieldDatatype->isNestedField(Foo::class, 'foo.baz'));
         self::assertFalse($fieldDatatype->isNestedField(Foo::class, 'baz'));
     }
 
     private function getValidFieldDatatype()
     {
         $fooType = new Type(Type::BUILTIN_TYPE_OBJECT, false, Foo::class);
         $barType = new Type(Type::BUILTIN_TYPE_ARRAY, false, Foo::class, true, new Type(Type::BUILTIN_TYPE_INT), new Type(Type::BUILTIN_TYPE_OBJECT, false, Foo::class));
+        $bazType = new Type(Type::BUILTIN_TYPE_STRING, false, Foo::class);
 
         $propertyMetadataFactoryProphecy = $this->prophesize(PropertyMetadataFactoryInterface::class);
         $propertyMetadataFactoryProphecy->create(Foo::class, 'foo')->willReturn((new ApiProperty())->withBuiltinTypes([$fooType]))->shouldBeCalled();
         $propertyMetadataFactoryProphecy->create(Foo::class, 'bar')->willReturn((new ApiProperty())->withBuiltinTypes([$barType]))->shouldBeCalled();
+        $propertyMetadataFactoryProphecy->create(Foo::class, 'baz')->willReturn((new ApiProperty())->withBuiltinTypes([$bazType]));
 
         $resourceClassResolverProphecy = $this->prophesize(ResourceClassResolverInterface::class);
         $resourceClassResolverProphecy->isResourceClass(Foo::class)->willReturn(true)->shouldBeCalled();

diff --git a/src/Elasticsearch/Util/FieldDatatypeTrait.php b/src/Elasticsearch/Util/FieldDatatypeTrait.php
@@ -43,6 +43,11 @@ private function isNestedField(string $resourceClass, string $property): bool
 
     /**
      * Get the nested path to the decomposed given property (e.g.: foo.bar.baz => foo.bar).
+     *
+     * Elasticsearch can save arrays of Objects as nested documents.
+     * In the case of foo.bar.baz
+     *   foo.bar will be returned if foo.bar is an array of objects.
+     *   If neither foo nor bar is an array, it is not a nested property and will return null.
      */
     private function getNestedFieldPath(string $resourceClass, string $property): ?string
     {
@@ -78,7 +83,9 @@ private function getNestedFieldPath(string $resourceClass, string $property): ?s
                 && null !== ($className = $type->getClassName())
                 && $this->resourceClassResolver->isResourceClass($className)
             ) {
-                return $currentProperty;
+                $nestedPath = $this->getNestedFieldPath($className, implode('.', $properties));
+
+                return null === $nestedPath ? $currentProperty : "$currentProperty.$nestedPath";
             }
         }
 

diff --git a/tests/Fixtures/Elasticsearch/Fixtures/book.json b/tests/Fixtures/Elasticsearch/Fixtures/book.json
@@ -6,7 +6,17 @@
       "gender": "male",
       "age": 31,
       "firstName": "Kilian",
-      "lastName": "Jornet"
+      "lastName": "Jornet",
+      "relatedGenres": [
+        {
+          "id": "5b45d21a-8cdf-4dac-9237-4fea7bb1535f%",
+          "name": "Fiction"
+        },
+        {
+          "id": "e7a24c7d-84a8-4133-b6b2-4bf050b36023",
+          "name": "Contemporary"
+        }
+      ]
     },
     "date": "2017-01-01 01:01:01",
     "message": "The north summit, Store Vengetind Thanks for t... These Top 10 Women of a fk... Francois is the field which."
@@ -90,7 +100,17 @@
       "gender": "male",
       "age": 35,
       "firstName": "Anton",
-      "lastName": "Krupicka"
+      "lastName": "Krupicka",
+      "relatedGenres": [
+        {
+          "id": "5b45d21a-8cdf-4dac-9237-4fea7bb1535f%",
+          "name": "Fiction"
+        },
+        {
+          "id": "e7a24c7d-84a8-4133-b6b2-4bf050b36023",
+          "name": "Contemporary"
+        }
+      ]
     },
     "date": "2017-08-08 08:08:08",
     "message": "Whoever curates the Marathon yesterday. Truly inspiring stuff. The new is straight. Such a couple!"

diff --git a/tests/Fixtures/Elasticsearch/Mappings/book.json b/tests/Fixtures/Elasticsearch/Mappings/book.json
@@ -20,6 +20,18 @@
           },
           "lastName": {
             "type": "text"
+          },
+          "relatedGenres": {
+            "type": "nested",
+            "properties": {
+              "id": {
+                "type": "keyword"
+              },
+              "name": {
+                "type": "text"
+              }
+            },
+            "dynamic": "strict"
           }
         },
         "dynamic": "strict"

diff --git a/tests/Fixtures/Elasticsearch/Model/Book.php b/tests/Fixtures/Elasticsearch/Model/Book.php
@@ -27,7 +27,7 @@
 
 #[ApiResource(operations: [new Get(provider: ItemProvider::class), new GetCollection(provider: CollectionProvider::class)], normalizationContext: ['groups' => ['book:read']], stateOptions: new Options(index: 'book'))]
 #[ApiFilter(OrderFilter::class, properties: ['id', 'library.id'])]
-#[ApiFilter(MatchFilter::class, properties: ['message', 'library.firstName'])]
+#[ApiFilter(MatchFilter::class, properties: ['message', 'library.firstName', 'library.relatedGenres.name'])]
 class Book
 {
     #[Groups(['book:read', 'library:read'])]

diff --git a/tests/Fixtures/Elasticsearch/Model/Genre.php b/tests/Fixtures/Elasticsearch/Model/Genre.php
@@ -0,0 +1,27 @@
+<?php
+
+/*
+ * This file is part of the API Platform project.
+ *
+ * (c) Kévin Dunglas <dunglas@gmail.com>
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+declare(strict_types=1);
+
+namespace ApiPlatform\Tests\Fixtures\Elasticsearch\Model;
+
+use ApiPlatform\Metadata\ApiResource;
+use Symfony\Component\Serializer\Annotation\Groups;
+
+#[ApiResource]
+class Genre
+{
+    #[Groups(['genre:read', 'book:read'])]
+    public ?string $id = null;
+
+    #[Groups(['genre:read', 'book:read'])]
+    public ?string $name = null;
+}
diff --git a/tests/Fixtures/Elasticsearch/Model/Library.php b/tests/Fixtures/Elasticsearch/Model/Library.php
@@ -46,4 +46,7 @@ class Library
     /** @var Book[] */
     #[Groups(['library:read'])]
     public array $books = [];
+
+    /** @var Genre[] */
+    public array $relatedGenres = [];
 }