Merge pull request #6758 from soyuka/merge342

Co-authored-by: Simon <[email protected]> fix(hydra): iri template when using query parameter (#6742)
api-platform · Oct 28, 2024 · a8fdb30 · a8fdb30
2 parents 4f65ef2 + 8d66330
commit a8fdb30
Show file tree

Hide file tree

Showing 9 changed files with 230 additions and 76 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -152,6 +152,12 @@ Notes:
 
 * [0d5f35683](https://github.com/api-platform/core/commit/0d5f356839eb6aa9f536044abe4affa736553e76) feat(laravel): laravel component (#5882)
 
+## v3.4.5
+
+### Bug fixes
+
+* [fc8fa00a1](https://github.com/api-platform/core/commit/fc8fa00a19320b65547a60537261959c11f8e6a8) fix(hydra): iri template when using query parameter (#6742)
+
 ## v3.4.4
 
 ### Bug fixes

diff --git a/docs/guides/doctrine-search-filter.php b/docs/guides/doctrine-search-filter.php
@@ -120,24 +120,21 @@ public function testGetDocumentation(): void
             $this->assertJsonContains([
                 'search' => [
                     '@type' => 'IriTemplate',
-                    'template' => '/books.jsonld{?id,title,author}',
+                    'template' => '/books.jsonld{?title,author}',
                     'variableRepresentation' => 'BasicRepresentation',
                     'mapping' => [
-                        [
-                            '@type' => 'IriTemplateMapping',
-                            'variable' => 'id',
-                            'property' => 'id',
-                        ],
                         [
                             '@type' => 'IriTemplateMapping',
                             'variable' => 'title',
                             'property' => 'title',
+                            'required' => false,
                         ],
                         [
                             '@type' => 'IriTemplateMapping',
                             'variable' => 'author',
                             'property' => 'author',
-                        ]
+                            'required' => false,
+                        ],
                     ],
                 ],
             ]);

diff --git a/src/Hydra/Serializer/CollectionFiltersNormalizer.php b/src/Hydra/Serializer/CollectionFiltersNormalizer.php
@@ -17,7 +17,6 @@
 use ApiPlatform\Doctrine\Orm\State\Options;
 use ApiPlatform\JsonLd\Serializer\HydraPrefixTrait;
 use ApiPlatform\Metadata\FilterInterface;
-use ApiPlatform\Metadata\Parameter;
 use ApiPlatform\Metadata\Parameters;
 use ApiPlatform\Metadata\QueryParameterInterface;
 use ApiPlatform\Metadata\Resource\Factory\ResourceMetadataCollectionFactoryInterface;
@@ -133,10 +132,9 @@ public function setNormalizer(NormalizerInterface $normalizer): void
     /**
      * Returns the content of the Hydra search property.
      *
-     * @param FilterInterface[]        $filters
-     * @param array<string, Parameter> $parameters
+     * @param FilterInterface[] $filters
      */
-    private function getSearch(string $resourceClass, array $parts, array $filters, array|Parameters|null $parameters, string $hydraPrefix): array
+    private function getSearch(string $resourceClass, array $parts, array $filters, ?Parameters $parameters, string $hydraPrefix): array
     {
         $variables = [];
         $mapping = [];
@@ -153,13 +151,19 @@ private function getSearch(string $resourceClass, array $parts, array $filters,
                 continue;
             }
 
-            if (!($property = $parameter->getProperty()) && ($filterId = $parameter->getFilter()) && ($filter = $this->getFilter($filterId))) {
-                foreach ($filter->getDescription($resourceClass) as $variable => $description) {
-                    // This is a practice induced by PHP and is not necessary when implementing URI template
+            if (($filterId = $parameter->getFilter()) && \is_string($filterId) && ($filter = $this->getFilter($filterId))) {
+                $filterDescription = $filter->getDescription($resourceClass);
+
+                foreach ($filterDescription as $variable => $description) {
+                    // // This is a practice induced by PHP and is not necessary when implementing URI template
                     if (str_ends_with((string) $variable, '[]')) {
                         continue;
                     }
 
+                    if (($prop = $parameter->getProperty()) && ($description['property'] ?? null) !== $prop) {
+                        continue;
+                    }
+
                     // :property is a pattern allowed when defining parameters
                     $k = str_replace(':property', $description['property'], $key);
                     $variable = str_replace($description['property'], $k, $variable);
@@ -171,10 +175,12 @@ private function getSearch(string $resourceClass, array $parts, array $filters,
                     $mapping[] = $m;
                 }
 
-                continue;
+                if ($filterDescription) {
+                    continue;
+                }
             }
 
-            if (!$property) {
+            if (!($property = $parameter->getProperty())) {
                 continue;
             }
 

diff --git a/src/Metadata/Resource/Factory/ParameterResourceMetadataCollectionFactory.php b/src/Metadata/Resource/Factory/ParameterResourceMetadataCollectionFactory.php
@@ -155,16 +155,12 @@ private function addFilterMetadata(Parameter $parameter): Parameter
             return $parameter;
         }
 
-        if (null === $parameter->getSchema() && $filter instanceof JsonSchemaFilterInterface) {
-            if ($schema = $filter->getSchema($parameter)) {
-                $parameter = $parameter->withSchema($schema);
-            }
+        if (null === $parameter->getSchema() && $filter instanceof JsonSchemaFilterInterface && $schema = $filter->getSchema($parameter)) {
+            $parameter = $parameter->withSchema($schema);
         }
 
-        if (null === $parameter->getOpenApi() && $filter instanceof OpenApiParameterFilterInterface) {
-            if ($openApiParameter = $filter->getOpenApiParameters($parameter)) {
-                $parameter = $parameter->withOpenApi($openApiParameter);
-            }
+        if (null === $parameter->getOpenApi() && $filter instanceof OpenApiParameterFilterInterface && ($openApiParameter = $filter->getOpenApiParameters($parameter)) && $openApiParameter instanceof OpenApiParameter) {
+            $parameter = $parameter->withOpenApi($openApiParameter);
         }
 
         return $parameter;
@@ -194,10 +190,6 @@ private function setDefaults(string $key, Parameter $parameter, string $resource
             $parameter = $parameter->withSchema($schema);
         }
 
-        if (null === $parameter->getProperty() && ($property = $description[$key]['property'] ?? null)) {
-            $parameter = $parameter->withProperty($property);
-        }
-
         $currentKey = $key;
         if (null === $parameter->getProperty() && isset($properties[$key])) {
             $parameter = $parameter->withProperty($key);
@@ -216,10 +208,6 @@ private function setDefaults(string $key, Parameter $parameter, string $resource
             $parameter = $parameter->withRequired($required);
         }
 
-        if (null === $parameter->getOpenApi() && ($openApi = $description[$key]['openapi'] ?? null) && $openApi instanceof OpenApiParameter) {
-            $parameter = $parameter->withOpenApi($openApi);
-        }
-
         return $this->addFilterMetadata($parameter);
     }
 

diff --git a/src/OpenApi/Factory/OpenApiFactory.php b/src/OpenApi/Factory/OpenApiFactory.php
@@ -261,12 +261,26 @@ private function collectPaths(ApiResource $resource, ResourceMetadataCollection
                 }
             }
 
+            $entityClass = $this->getFilterClass($operation);
             $openapiParameters = $openapiOperation->getParameters();
             foreach ($operation->getParameters() ?? [] as $key => $p) {
                 if (false === $p->getOpenApi()) {
                     continue;
                 }
 
+                if (($f = $p->getFilter()) && \is_string($f) && $this->filterLocator && $this->filterLocator->has($f)) {
+                    $filter = $this->filterLocator->get($f);
+                    foreach ($filter->getDescription($entityClass) as $name => $description) {
+                        if ($prop = $p->getProperty()) {
+                            $name = str_replace($prop, $key, $name);
+                        }
+
+                        $openapiParameters[] = $this->getFilterParameter($name, $description, $operation->getShortName(), $f);
+                    }
+
+                    continue;
+                }
+
                 $in = $p instanceof HeaderParameterInterface ? 'header' : 'query';
                 $defaultParameter = new Parameter($key, $in, $p->getDescription() ?? "$resourceShortName $key", $p->getRequired() ?? false, false, false, $p->getSchema() ?? ['type' => 'string']);
 
@@ -557,57 +571,98 @@ private function getLinks(ResourceMetadataCollection $resourceMetadataCollection
     private function getFiltersParameters(CollectionOperationInterface|HttpOperation $operation): array
     {
         $parameters = [];
-
         $resourceFilters = $operation->getFilters();
+        $entityClass = $this->getFilterClass($operation);
+
         foreach ($resourceFilters ?? [] as $filterId) {
             if (!$this->filterLocator->has($filterId)) {
                 continue;
             }
 
             $filter = $this->filterLocator->get($filterId);
-            $entityClass = $operation->getClass();
-            if ($options = $operation->getStateOptions()) {
-                if ($options instanceof DoctrineOptions && $options->getEntityClass()) {
-                    $entityClass = $options->getEntityClass();
-                }
+            foreach ($filter->getDescription($entityClass) as $name => $description) {
+                $parameters[] = $this->getFilterParameter($name, $description, $operation->getShortName(), $filterId);
+            }
+        }
 
-                if ($options instanceof DoctrineODMOptions && $options->getDocumentClass()) {
-                    $entityClass = $options->getDocumentClass();
-                }
+        return $parameters;
+    }
+
+    private function getFilterClass(HttpOperation $operation): ?string
+    {
+        $entityClass = $operation->getClass();
+        if ($options = $operation->getStateOptions()) {
+            if ($options instanceof DoctrineOptions && $options->getEntityClass()) {
+                return $options->getEntityClass();
             }
 
-            foreach ($filter->getDescription($entityClass) as $name => $data) {
-                $schema = $data['schema'] ?? [];
+            if ($options instanceof DoctrineODMOptions && $options->getDocumentClass()) {
+                return $options->getDocumentClass();
+            }
+        }
 
-                if (isset($data['type']) && \in_array($data['type'] ?? null, Type::$builtinTypes, true) && !isset($schema['type'])) {
-                    $schema += $this->getType(new Type($data['type'], false, null, $data['is_collection'] ?? false));
-                }
+        return $entityClass;
+    }
 
-                if (!isset($schema['type'])) {
-                    $schema['type'] = 'string';
-                }
+    private function getFilterParameter(string $name, array $description, string $shortName, string $filter): Parameter
+    {
+        if (isset($description['swagger'])) {
+            trigger_deprecation('api-platform/core', '4.0', \sprintf('Using the "swagger" field of the %s::getDescription() (%s) is deprecated.', $filter, $shortName));
+        }
 
-                $style = 'array' === ($schema['type'] ?? null) && \in_array(
-                    $data['type'],
-                    [Type::BUILTIN_TYPE_ARRAY, Type::BUILTIN_TYPE_OBJECT],
-                    true
-                ) ? 'deepObject' : 'form';
+        if (!isset($description['openapi']) || $description['openapi'] instanceof Parameter) {
+            $schema = $description['schema'] ?? [];
 
-                $parameter = isset($data['openapi']) && $data['openapi'] instanceof Parameter ? $data['openapi'] : new Parameter(in: 'query', name: $name, style: $style, explode: $data['is_collection'] ?? false);
+            if (isset($description['type']) && \in_array($description['type'], Type::$builtinTypes, true) && !isset($schema['type'])) {
+                $schema += $this->getType(new Type($description['type'], false, null, $description['is_collection'] ?? false));
+            }
 
-                if ('' === $parameter->getDescription() && ($description = $data['description'] ?? '')) {
-                    $parameter = $parameter->withDescription($description);
-                }
+            if (!isset($schema['type'])) {
+                $schema['type'] = 'string';
+            }
 
-                if (false === $parameter->getRequired() && false !== ($required = $data['required'] ?? false)) {
-                    $parameter = $parameter->withRequired($required);
-                }
+            $style = 'array' === ($schema['type'] ?? null) && \in_array(
+                $description['type'],
+                [Type::BUILTIN_TYPE_ARRAY, Type::BUILTIN_TYPE_OBJECT],
+                true
+            ) ? 'deepObject' : 'form';
+
+            $parameter = isset($description['openapi']) && $description['openapi'] instanceof Parameter ? $description['openapi'] : new Parameter(in: 'query', name: $name, style: $style, explode: $description['is_collection'] ?? false);
+
+            if ('' === $parameter->getDescription() && ($str = $description['description'] ?? '')) {
+                $parameter = $parameter->withDescription($str);
+            }
 
-                $parameters[] = $parameter->withSchema($schema);
+            if (false === $parameter->getRequired() && false !== ($required = $description['required'] ?? false)) {
+                $parameter = $parameter->withRequired($required);
             }
+
+            return $parameter->withSchema($schema);
         }
 
-        return $parameters;
+        trigger_deprecation('api-platform/core', '4.0', \sprintf('Not using "%s" on the "openapi" field of the %s::getDescription() (%s) is deprecated.', Parameter::class, $filter, $shortName));
+        $schema = $description['schema'] ?? (\in_array($description['type'], Type::$builtinTypes, true) ? $this->getType(new Type($description['type'], false, null, $description['is_collection'] ?? false)) : ['type' => 'string']);
+
+        return new Parameter(
+            $name,
+            'query',
+            $description['description'] ?? '',
+            $description['required'] ?? false,
+            $description['openapi']['deprecated'] ?? false,
+            $description['openapi']['allowEmptyValue'] ?? true,
+            $schema,
+            'array' === $schema['type'] && \in_array(
+                $description['type'],
+                [Type::BUILTIN_TYPE_ARRAY, Type::BUILTIN_TYPE_OBJECT],
+                true
+            ) ? 'deepObject' : 'form',
+            $description['openapi']['explode'] ?? ('array' === $schema['type']),
+            $description['openapi']['allowReserved'] ?? false,
+            $description['openapi']['example'] ?? null,
+            isset(
+                $description['openapi']['examples']
+            ) ? new \ArrayObject($description['openapi']['examples']) : null
+        );
     }
 
     private function getPaginationParameters(CollectionOperationInterface|HttpOperation $operation): array

diff --git a/src/OpenApi/Tests/Factory/OpenApiFactoryTest.php b/src/OpenApi/Tests/Factory/OpenApiFactoryTest.php
@@ -879,7 +879,8 @@ public function testInvoke(): void
                     'type' => 'string',
                     'enum' => ['asc', 'desc'],
                 ]),
-            ]
+            ],
+            deprecated: false
         ), $filteredPath->getGet());
 
         $paginatedPath = $paths->getPath('/paginated');

diff --git a/tests/Fixtures/TestBundle/ApiResource/FilterWithStateOptions.php b/tests/Fixtures/TestBundle/ApiResource/FilterWithStateOptions.php
@@ -0,0 +1,36 @@
+<?php
+
+/*
+ * This file is part of the API Platform project.
+ *
+ * (c) Kévin Dunglas <[email protected]>
+ *
+ * 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\TestBundle\ApiResource;
+
+use ApiPlatform\Doctrine\Orm\Filter\DateFilter;
+use ApiPlatform\Doctrine\Orm\State\CollectionProvider;
+use ApiPlatform\Doctrine\Orm\State\Options;
+use ApiPlatform\Metadata\ApiFilter;
+use ApiPlatform\Metadata\GetCollection;
+use ApiPlatform\Metadata\QueryParameter;
+use ApiPlatform\Tests\Fixtures\TestBundle\Entity\FilterWithStateOptionsEntity;
+
+#[GetCollection(
+    uriTemplate: 'filter_with_state_options',
+    stateOptions: new Options(entityClass: FilterWithStateOptionsEntity::class),
+    parameters: ['date' => new QueryParameter(filter: 'filter_with_state_options_date', property: 'dummyDate')],
+    provider: CollectionProvider::class
+)]
+#[ApiFilter(DateFilter::class, alias: 'filter_with_state_options_date', properties: ['dummyDate' => DateFilter::EXCLUDE_NULL])]
+final class FilterWithStateOptions
+{
+    public function __construct(public readonly string $id, public readonly \DateTimeImmutable $dummyDate, public readonly string $name)
+    {
+    }
+}
diff --git a/tests/Fixtures/TestBundle/Entity/FilterWithStateOptionsEntity.php b/tests/Fixtures/TestBundle/Entity/FilterWithStateOptionsEntity.php
@@ -0,0 +1,32 @@
+<?php
+
+/*
+ * This file is part of the API Platform project.
+ *
+ * (c) Kévin Dunglas <[email protected]>
+ *
+ * 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\TestBundle\Entity;
+
+use Doctrine\ORM\Mapping as ORM;
+
+#[ORM\Entity]
+class FilterWithStateOptionsEntity
+{
+    public function __construct(
+        #[ORM\Column(type: 'integer')]
+        #[ORM\Id]
+        #[ORM\GeneratedValue(strategy: 'AUTO')]
+        public ?int $id = null,
+        #[ORM\Column(type: 'date_immutable', nullable: true)]
+        public ?\DateTimeImmutable $dummyDate = null,
+        #[ORM\Column(type: 'string', nullable: true)]
+        public ?string $name = null,
+    ) {
+    }
+}