fix(state): query and header parameter with the smae name

soyuka · Jul 5, 2024 · 02c45ea · 02c45ea
1 parent 0edc738
commit 02c45ea
Show file tree

Hide file tree

Showing 15 changed files with 205 additions and 67 deletions.
diff --git a/src/Doctrine/Common/ParameterValueExtractorTrait.php b/src/Doctrine/Common/ParameterValueExtractorTrait.php
@@ -0,0 +1,36 @@
+<?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\Doctrine\Common;
+
+trait ParameterValueExtractorTrait
+{
+    /**
+     * @param array<string, mixed> $values
+     *
+     * @return array<string, mixed>
+     */
+    private function extractParameterValue(array $values): array
+    {
+        if (!$values) {
+            return $values;
+        }
+
+        $key = key($values);
+        if (!str_contains($key, ':property')) {
+            return $values;
+        }
+
+        return [str_replace('[:property]', '', $key) => $values[$key]];
+    }
+}
diff --git a/src/Doctrine/Odm/Extension/ParameterExtension.php b/src/Doctrine/Odm/Extension/ParameterExtension.php
@@ -13,6 +13,7 @@
 
 namespace ApiPlatform\Doctrine\Odm\Extension;
 
+use ApiPlatform\Doctrine\Common\ParameterValueExtractorTrait;
 use ApiPlatform\Doctrine\Odm\Filter\FilterInterface;
 use ApiPlatform\Metadata\Operation;
 use Doctrine\ODM\MongoDB\Aggregation\Builder;
@@ -25,14 +26,16 @@
  */
 final class ParameterExtension implements AggregationCollectionExtensionInterface, AggregationItemExtensionInterface
 {
+    use ParameterValueExtractorTrait;
+
     public function __construct(private readonly ContainerInterface $filterLocator)
     {
     }
 
     private function applyFilter(Builder $aggregationBuilder, ?string $resourceClass = null, ?Operation $operation = null, array &$context = []): void
     {
         foreach ($operation->getParameters() ?? [] as $parameter) {
-            $values = $parameter->getExtraProperties()['_api_values'] ?? [];
+            $values = $this->extractParameterValue($parameter->getValue() ?? []);
             if (!$values) {
                 continue;
             }

diff --git a/src/Doctrine/Orm/Extension/ParameterExtension.php b/src/Doctrine/Orm/Extension/ParameterExtension.php
@@ -13,6 +13,7 @@
 
 namespace ApiPlatform\Doctrine\Orm\Extension;
 
+use ApiPlatform\Doctrine\Common\ParameterValueExtractorTrait;
 use ApiPlatform\Doctrine\Orm\Filter\FilterInterface;
 use ApiPlatform\Doctrine\Orm\Util\QueryNameGeneratorInterface;
 use ApiPlatform\Metadata\Operation;
@@ -26,6 +27,8 @@
  */
 final class ParameterExtension implements QueryCollectionExtensionInterface, QueryItemExtensionInterface
 {
+    use ParameterValueExtractorTrait;
+
     public function __construct(private readonly ContainerInterface $filterLocator)
     {
     }
@@ -36,7 +39,7 @@ public function __construct(private readonly ContainerInterface $filterLocator)
     private function applyFilter(QueryBuilder $queryBuilder, QueryNameGeneratorInterface $queryNameGenerator, string $resourceClass, ?Operation $operation = null, array $context = []): void
     {
         foreach ($operation?->getParameters() ?? [] as $parameter) {
-            $values = $parameter->getExtraProperties()['_api_values'] ?? [];
+            $values = $this->extractParameterValue($parameter->getValue() ?? []);
             if (!$values) {
                 continue;
             }

diff --git a/src/Metadata/Parameter.php b/src/Metadata/Parameter.php
@@ -50,7 +50,7 @@ public function getKey(): ?string
     }
 
     /**
-     * @return array{type?: string}|null $schema
+     * @return (array<string, mixed>&array{type?: string, default?: string})|null $schema
      */
     public function getSchema(): ?array
     {
@@ -100,6 +100,16 @@ public function getConstraints(): Constraint|array|null
         return $this->constraints;
     }
 
+    /**
+     * The computed value of this parameter, located into extraProperties['_api_values'].
+     *
+     * @readonly
+     */
+    public function getValue(): mixed
+    {
+        return $this->extraProperties['_api_values'] ?? null;
+    }
+
     /**
      * @return array<string, mixed>
      */

diff --git a/src/Metadata/Parameters.php b/src/Metadata/Parameters.php
@@ -13,6 +13,8 @@
 
 namespace ApiPlatform\Metadata;
 
+use ApiPlatform\Metadata\Exception\RuntimeException;
+
 /**
  * A parameter dictionnary.
  *
@@ -53,7 +55,7 @@ public function getIterator(): \Traversable
     public function add(string $key, Parameter $value): self
     {
         foreach ($this->parameters as $i => [$parameterName, $parameter]) {
-            if ($parameterName === $key) {
+            if ($parameterName === $key && $value::class === $parameter::class) {
                 $this->parameters[$i] = [$key, $value];
 
                 return $this;
@@ -65,34 +67,43 @@ public function add(string $key, Parameter $value): self
         return $this;
     }
 
-    public function get(string $key): ?Parameter
+    /**
+     * @param class-string $parameterClass
+     */
+    public function remove(string $key, string $parameterClass): self
     {
         foreach ($this->parameters as $i => [$parameterName, $parameter]) {
-            if ($parameterName === $key) {
-                return $parameter;
+            if ($parameterName === $key && $parameterClass === $parameter::class) {
+                unset($this->parameters[$i]);
+
+                return $this;
             }
         }
 
-        return null;
+        throw new RuntimeException(sprintf('Could not remove parameter "%s".', $key));
     }
 
-    public function remove(string $key): self
+    /**
+     * @param class-string $parameterClass
+     */
+    public function get(string $key, string $parameterClass): ?Parameter
     {
-        foreach ($this->parameters as $i => [$parameterName, $parameter]) {
-            if ($parameterName === $key) {
-                unset($this->parameters[$i]);
-
-                return $this;
+        foreach ($this->parameters as [$parameterName, $parameter]) {
+            if ($parameterName === $key && $parameterClass === $parameter::class) {
+                return $parameter;
             }
         }
 
-        throw new \RuntimeException(sprintf('Could not remove parameter "%s".', $key));
+        return null;
     }
 
-    public function has(string $key): bool
+    /**
+     * @param class-string $parameterClass
+     */
+    public function has(string $key, string $parameterClass): bool
     {
-        foreach ($this->parameters as $i => [$parameterName, $parameter]) {
-            if ($parameterName === $key) {
+        foreach ($this->parameters as [$parameterName, $parameter]) {
+            if ($parameterName === $key && $parameterClass === $parameter::class) {
                 return true;
             }
         }

diff --git a/src/Metadata/Resource/Factory/AttributesResourceMetadataCollectionFactory.php b/src/Metadata/Resource/Factory/AttributesResourceMetadataCollectionFactory.php
@@ -245,7 +245,7 @@ private function mergeOperationParameters(Metadata $resource, Parameters $global
                 $parameterName = $key;
             }
 
-            if (!$parameters->has($parameterName)) {
+            if (!$parameters->has($parameterName, $parameter::class)) {
                 $parameters->add($parameterName, $parameter);
             }
         }

diff --git a/src/Metadata/Resource/Factory/ParameterResourceMetadataCollectionFactory.php b/src/Metadata/Resource/Factory/ParameterResourceMetadataCollectionFactory.php
@@ -55,34 +55,38 @@ public function create(string $resourceClass): ResourceMetadataCollection
 
             $internalPriority = -1;
             foreach ($operations as $operationName => $operation) {
-                $parameters = [];
-                foreach ($operation->getParameters() ?? [] as $key => $parameter) {
+                $parameters = $operation->getParameters() ?? new Parameters();
+                foreach ($parameters as $key => $parameter) {
+                    $key = $parameter->getKey() ?? $key;
                     $parameter = $this->setDefaults($key, $parameter, $resourceClass);
                     $priority = $parameter->getPriority() ?? $internalPriority--;
-                    $parameters[$key] = $parameter->withPriority($priority);
+                    $parameters->add($key, $parameter->withPriority($priority));
                 }
 
-                $operations->add($operationName, $operation->withParameters(new Parameters($parameters)));
+                $operations->add($operationName, $operation->withParameters($parameters));
             }
 
             $resourceMetadataCollection[$i] = $resource->withOperations($operations->sort());
 
-            $internalPriority = -1;
             $graphQlOperations = $resource->getGraphQlOperations();
-            foreach ($graphQlOperations ?? [] as $operationName => $operation) {
-                $parameters = [];
+            if (!$graphQlOperations) {
+                continue;
+            }
+
+            $internalPriority = -1;
+            foreach ($graphQlOperations as $operationName => $operation) {
+                $parameters = $operation->getParameters() ?? new Parameters();
                 foreach ($operation->getParameters() ?? [] as $key => $parameter) {
+                    $key = $parameter->getKey() ?? $key;
                     $parameter = $this->setDefaults($key, $parameter, $resourceClass);
                     $priority = $parameter->getPriority() ?? $internalPriority--;
-                    $parameters[$key] = $parameter->withPriority($priority);
+                    $parameters->add($key, $parameter->withPriority($priority));
                 }
 
-                $graphQlOperations[$operationName] = $operation->withParameters(new Parameters($parameters));
+                $graphQlOperations[$operationName] = $operation->withParameters($parameters);
             }
 
-            if ($graphQlOperations) {
-                $resourceMetadataCollection[$i] = $resource->withGraphQlOperations($graphQlOperations);
-            }
+            $resourceMetadataCollection[$i] = $resource->withGraphQlOperations($graphQlOperations);
         }
 
         return $resourceMetadataCollection;

diff --git a/src/Metadata/Tests/Resource/Factory/ParameterResourceMetadataCollectionFactoryTests.php b/src/Metadata/Tests/Resource/Factory/ParameterResourceMetadataCollectionFactoryTests.php
@@ -15,6 +15,7 @@
 
 use ApiPlatform\Metadata\FilterInterface;
 use ApiPlatform\Metadata\Parameters;
+use ApiPlatform\Metadata\QueryParameter;
 use ApiPlatform\Metadata\Resource\Factory\AttributesResourceMetadataCollectionFactory;
 use ApiPlatform\Metadata\Resource\Factory\ParameterResourceMetadataCollectionFactory;
 use ApiPlatform\Metadata\Tests\Fixtures\ApiResource\WithParameter;
@@ -50,10 +51,10 @@ public function getDescription(string $resourceClass): array
         $parameter = new ParameterResourceMetadataCollectionFactory(new AttributesResourceMetadataCollectionFactory(), $filterLocator);
         $operation = $parameter->create(WithParameter::class)->getOperation('collection');
         $this->assertInstanceOf(Parameters::class, $parameters = $operation->getParameters());
-        $hydraParameter = $parameters->get('hydra');
+        $hydraParameter = $parameters->get('hydra', QueryParameter::class);
         $this->assertEquals(['type' => 'foo'], $hydraParameter->getSchema());
         $this->assertEquals(new Parameter('test', 'query'), $hydraParameter->getOpenApi());
-        $everywhere = $parameters->get('everywhere');
+        $everywhere = $parameters->get('everywhere', QueryParameter::class);
         $this->assertEquals(new Parameter('everywhere', 'query', allowEmptyValue: true), $everywhere->getOpenApi());
     }
 }
diff --git a/src/State/ParameterNotFound.php b/src/State/ParameterNotFound.php
@@ -0,0 +1,18 @@
+<?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\State;
+
+final class ParameterNotFound
+{
+}
diff --git a/src/State/Provider/ParameterProvider.php b/src/State/Provider/ParameterProvider.php
@@ -14,9 +14,8 @@
 namespace ApiPlatform\State\Provider;
 
 use ApiPlatform\Metadata\Operation;
-use ApiPlatform\Metadata\Parameter;
-use ApiPlatform\Metadata\Parameters;
 use ApiPlatform\State\Exception\ProviderNotFoundException;
+use ApiPlatform\State\ParameterNotFound;
 use ApiPlatform\State\ParameterProviderInterface;
 use ApiPlatform\State\ProviderInterface;
 use ApiPlatform\State\Util\ParameterParserTrait;
@@ -51,20 +50,22 @@ public function provide(Operation $operation, array $uriVariables = [], array $c
         }
 
         $context = ['operation' => $operation] + $context;
-        $p = $operation->getParameters() ?? [];
-        $parameters = $p instanceof Parameters ? iterator_to_array($p) : $p;
-        foreach ($parameters as $parameter) {
-            $key = $parameter->getKey();
-            $values = $this->extractParameterValues($parameter, $request, $context);
-            $key = $this->getParameterFlattenKey($key, $values);
-
-            if (!isset($values[$key])) {
+        $parameters = $operation->getParameters();
+        foreach ($parameters ?? [] as $parameter) {
+            $values = $this->getParameterValues($parameter, $request, $context);
+            $value = $this->extractParameterValues($parameter, $values);
+
+            if ((!$value || $value instanceof ParameterNotFound) && ($default = $parameter->getSchema()['default'] ?? false)) {
+                $value = $default;
+            }
+
+            if ($value instanceof ParameterNotFound) {
                 continue;
             }
 
-            $parameters[$parameter->getKey()] = $parameter = $parameter->withExtraProperties(
-                $parameter->getExtraProperties() + ['_api_values' => [$key => $values[$key]]]
-            );
+            $parameters->add($parameter->getKey(), $parameter = $parameter->withExtraProperties(
+                $parameter->getExtraProperties() + ['_api_values' => [$parameter->getKey() => $value]]
+            ));
 
             if (null === ($provider = $parameter->getProvider())) {
                 continue;
@@ -89,7 +90,9 @@ public function provide(Operation $operation, array $uriVariables = [], array $c
             }
         }
 
-        $operation = $operation->withParameters(new Parameters($parameters));
+        if ($parameters) {
+            $operation = $operation->withParameters($parameters);
+        }
         $request?->attributes->set('_api_operation', $operation);
         $context['operation'] = $operation;
 

diff --git a/src/State/Tests/ParameterProviderTest.php b/src/State/Tests/ParameterProviderTest.php
@@ -59,8 +59,8 @@ public function has(string $id): bool
         $operation = $request->attributes->get('_api_operation');
 
         $this->assertEquals('ok', $operation->getName());
-        $this->assertEquals(['order' => ['foo' => 'asc']], $operation->getParameters()->get('order')->getExtraProperties()['_api_values']);
-        $this->assertEquals(['search' => ['a' => 'bar']], $operation->getParameters()->get('search[:property]')->getExtraProperties()['_api_values']);
+        $this->assertEquals(['order' => ['foo' => 'asc']], $operation->getParameters()->get('order', QueryParameter::class)->getValue());
+        $this->assertEquals(['search[:property]' => ['a' => 'bar']], $operation->getParameters()->get('search[:property]', QueryParameter::class)->getValue());
     }
 
     public static function provide(): void