fix(state): parameter decorates main chain (#6434)

* fix(state): parameter decorates main chain * fix: property placeholder validation
api-platform · Jun 28, 2024 · b42e25f · b42e25f
1 parent af34e72
commit b42e25f
Show file tree

Hide file tree

Showing 16 changed files with 187 additions and 61 deletions.
diff --git a/src/State/Provider/ParameterProvider.php b/src/State/Provider/ParameterProvider.php
@@ -13,16 +13,15 @@
 
 namespace ApiPlatform\State\Provider;
 
-use ApiPlatform\Metadata\HeaderParameterInterface;
 use ApiPlatform\Metadata\Operation;
 use ApiPlatform\Metadata\Parameter;
 use ApiPlatform\Metadata\Parameters;
 use ApiPlatform\State\Exception\ProviderNotFoundException;
 use ApiPlatform\State\ParameterProviderInterface;
 use ApiPlatform\State\ProviderInterface;
+use ApiPlatform\State\Util\ParameterParserTrait;
 use ApiPlatform\State\Util\RequestParser;
 use Psr\Container\ContainerInterface;
-use Symfony\Component\HttpFoundation\Request;
 
 /**
  * Loops over parameters to:
@@ -33,6 +32,8 @@
  */
 final class ParameterProvider implements ProviderInterface
 {
+    use ParameterParserTrait;
+
     public function __construct(private readonly ?ProviderInterface $decorated = null, private readonly ?ContainerInterface $locator = null)
     {
     }
@@ -50,31 +51,27 @@ public function provide(Operation $operation, array $uriVariables = [], array $c
         }
 
         $context = ['operation' => $operation] + $context;
-        $parameters = $operation->getParameters() ?? [];
-        $operationParameters = $parameters instanceof Parameters ? iterator_to_array($parameters) : $parameters;
-        foreach ($operationParameters as $parameter) {
+        $p = $operation->getParameters() ?? [];
+        $parameters = $p instanceof Parameters ? iterator_to_array($p) : $p;
+        foreach ($parameters as $parameter) {
             $key = $parameter->getKey();
-            $parameters = $this->extractParameterValues($parameter, $request, $context);
-            $parsedKey = explode('[:property]', $key);
-
-            if (isset($parsedKey[0]) && isset($parameters[$parsedKey[0]])) {
-                $key = $parsedKey[0];
-            }
+            $values = $this->extractParameterValues($parameter, $request, $context);
+            $key = $this->getParameterFlattenKey($key, $values);
 
-            if (!isset($parameters[$key])) {
+            if (!isset($values[$key])) {
                 continue;
             }
 
-            $operationParameters[$parameter->getKey()] = $parameter = $parameter->withExtraProperties(
-                $parameter->getExtraProperties() + ['_api_values' => [$key => $parameters[$key]]]
+            $parameters[$parameter->getKey()] = $parameter = $parameter->withExtraProperties(
+                $parameter->getExtraProperties() + ['_api_values' => [$key => $values[$key]]]
             );
 
             if (null === ($provider = $parameter->getProvider())) {
                 continue;
             }
 
             if (\is_callable($provider)) {
-                if (($op = $provider($parameter, $parameters, $context)) instanceof Operation) {
+                if (($op = $provider($parameter, $values, $context)) instanceof Operation) {
                     $operation = $op;
                 }
 
@@ -87,28 +84,15 @@ public function provide(Operation $operation, array $uriVariables = [], array $c
 
             /** @var ParameterProviderInterface $providerInstance */
             $providerInstance = $this->locator->get($provider);
-            if (($op = $providerInstance->provide($parameter, $parameters, $context)) instanceof Operation) {
+            if (($op = $providerInstance->provide($parameter, $values, $context)) instanceof Operation) {
                 $operation = $op;
             }
         }
 
-        $operation = $operation->withParameters(new Parameters($operationParameters));
+        $operation = $operation->withParameters(new Parameters($parameters));
         $request?->attributes->set('_api_operation', $operation);
         $context['operation'] = $operation;
 
         return $this->decorated?->provide($operation, $uriVariables, $context);
     }
-
-    /**
-     * @param array<string, mixed> $context
-     */
-    private function extractParameterValues(Parameter $parameter, ?Request $request, array $context)
-    {
-        if ($request) {
-            return $parameter instanceof HeaderParameterInterface ? $request->attributes->get('_api_header_parameters') : $request->attributes->get('_api_query_parameters');
-        }
-
-        // GraphQl
-        return $context['args'] ?? [];
-    }
 }
diff --git a/src/State/Util/ParameterParserTrait.php b/src/State/Util/ParameterParserTrait.php
@@ -0,0 +1,53 @@
+<?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\State\Util;
+
+use ApiPlatform\Metadata\HeaderParameterInterface;
+use ApiPlatform\Metadata\Parameter;
+use Symfony\Component\HttpFoundation\Request;
+
+/**
+ * @internal
+ */
+trait ParameterParserTrait
+{
+    /**
+     * @param array<string, mixed> $values
+     */
+    private function getParameterFlattenKey(string $key, array $values): string
+    {
+        $parsedKey = explode('[:property]', $key);
+
+        if (isset($parsedKey[0]) && isset($values[$parsedKey[0]])) {
+            return $parsedKey[0];
+        }
+
+        return $key;
+    }
+
+    /**
+     * @param array<string, mixed> $context
+     *
+     * @return array<string, mixed>
+     */
+    private function extractParameterValues(Parameter $parameter, ?Request $request, array $context): array
+    {
+        if ($request) {
+            return ($parameter instanceof HeaderParameterInterface ? $request->attributes->get('_api_header_parameters') : $request->attributes->get('_api_query_parameters')) ?? [];
+        }
+
+        // GraphQl
+        return $context['args'] ?? [];
+    }
+}
diff --git a/src/Symfony/Bundle/DependencyInjection/ApiPlatformExtension.php b/src/Symfony/Bundle/DependencyInjection/ApiPlatformExtension.php
@@ -830,7 +830,6 @@ private function registerValidatorConfiguration(ContainerBuilder $container, arr
             $container->setParameter('api_platform.validator.legacy_validation_exception', $config['validator']['legacy_validation_exception'] ?? true);
             $loader->load('metadata/validator.xml');
             $loader->load('validator/validator.xml');
-            $loader->load('symfony/parameter_validator.xml');
 
             if ($this->isConfigEnabled($container, $config['graphql'])) {
                 $loader->load('graphql/validator.xml');

diff --git a/src/Symfony/Bundle/Resources/config/state/provider.xml b/src/Symfony/Bundle/Resources/config/state/provider.xml
@@ -38,5 +38,10 @@
             <argument key="$negotiator">null</argument>
             <argument key="$problemCompliantErrors">%api_platform.rfc_7807_compliant_errors%</argument>
         </service>
+
+        <service id="api_platform.state_provider.parameter" class="ApiPlatform\State\Provider\ParameterProvider" decorates="api_platform.state_provider.main" decoration-priority="190">
+            <argument type="service" id="api_platform.state_provider.parameter.inner" />
+            <argument type="tagged_locator" tag="api_platform.parameter_provider" index-by="key" />
+        </service>
     </services>
 </container>
diff --git a/src/Symfony/Bundle/Resources/config/state/state.xml b/src/Symfony/Bundle/Resources/config/state/state.xml
@@ -55,11 +55,6 @@
         </service>
         <service id="ApiPlatform\State\ObjectProvider" alias="api_platform.state_provider.object" />
 
-        <service id="api_platform.state_provider.parameter" class="ApiPlatform\State\Provider\ParameterProvider" decorates="api_platform.state_provider.read" decoration-priority="300">
-            <argument type="service" id="api_platform.state_provider.parameter.inner" />
-            <argument type="tagged_locator" tag="api_platform.parameter_provider" index-by="key" />
-        </service>
-
         <service id="ApiPlatform\State\SerializerContextBuilderInterface" alias="api_platform.serializer.context_builder" />
     </services>
 </container>
diff --git a/src/Symfony/Bundle/Resources/config/symfony/events.xml b/src/Symfony/Bundle/Resources/config/symfony/events.xml
@@ -102,6 +102,11 @@
             <argument key="$problemCompliantErrors">%api_platform.rfc_7807_compliant_errors%</argument>
         </service>
 
+        <service id="api_platform.state_provider.parameter" class="ApiPlatform\State\Provider\ParameterProvider" decorates="api_platform.state_provider.read" decoration-priority="100">
+            <argument type="service" id="api_platform.state_provider.parameter.inner" />
+            <argument type="tagged_locator" tag="api_platform.parameter_provider" index-by="key" />
+        </service>
+
         <service id="api_platform.state_processor.documentation" alias="api_platform.state_processor.respond" />
 
         <service id="api_platform.state_processor.documentation.serialize" class="ApiPlatform\State\Processor\SerializeProcessor" decorates="api_platform.state_processor.documentation" decoration-priority="200">

diff --git a/src/Symfony/Bundle/Resources/config/symfony/parameter_validator.xml b/src/Symfony/Bundle/Resources/config/symfony/parameter_validator.xml
diff --git a/src/Symfony/Bundle/Resources/config/validator/events.xml b/src/Symfony/Bundle/Resources/config/validator/events.xml
@@ -17,6 +17,11 @@
             <tag name="kernel.event_listener" event="kernel.view" method="onKernelView" priority="64" />
         </service>
 
+        <service id="api_platform.state_provider.parameter_validator" class="ApiPlatform\Symfony\Validator\State\ParameterValidatorProvider" public="true" decorates="api_platform.state_provider.read" decoration-priority="110">
+            <argument type="service" id="validator" />
+            <argument type="service" id="api_platform.state_provider.parameter_validator.inner" />
+        </service>
+
         <service id="api_platform.state_provider.query_parameter_validate" class="ApiPlatform\Symfony\Validator\State\QueryParameterValidateProvider">
             <argument>null</argument>
             <argument type="service" id="api_platform.validator.query_parameter_validator" />
@@ -26,6 +31,7 @@
             <argument type="service" id="api_platform.state_provider.query_parameter_validate" />
             <argument type="service" id="api_platform.metadata.resource.metadata_collection_factory" />
             <argument>%api_platform.validator.query_parameter_validation%</argument>
+            <argument type="service" id="api_platform.state_provider.parameter_validator" />
 
             <tag name="kernel.event_listener" event="kernel.request" method="onKernelRequest" priority="16" />
         </service>

diff --git a/src/Symfony/Bundle/Resources/config/validator/state.xml b/src/Symfony/Bundle/Resources/config/validator/state.xml
@@ -14,5 +14,10 @@
             <argument type="service" id="api_platform.state_provider.validate.inner" />
             <argument type="service" id="api_platform.validator" />
         </service>
+
+        <service id="api_platform.state_provider.parameter_validator" class="ApiPlatform\Symfony\Validator\State\ParameterValidatorProvider" public="true" decorates="api_platform.state_provider.main" decoration-priority="191">
+            <argument type="service" id="validator" />
+            <argument type="service" id="api_platform.state_provider.parameter_validator.inner" />
+        </service>
     </services>
 </container>
diff --git a/src/Symfony/Validator/State/ParameterValidatorProvider.php b/src/Symfony/Validator/State/ParameterValidatorProvider.php
@@ -15,6 +15,7 @@
 
 use ApiPlatform\Metadata\Operation;
 use ApiPlatform\State\ProviderInterface;
+use ApiPlatform\State\Util\ParameterParserTrait;
 use ApiPlatform\Validator\Exception\ValidationException;
 use Symfony\Component\HttpFoundation\Request;
 use Symfony\Component\Validator\ConstraintViolation;
@@ -28,36 +29,39 @@
  */
 final class ParameterValidatorProvider implements ProviderInterface
 {
+    use ParameterParserTrait;
+
     public function __construct(
-        private readonly ProviderInterface $decorated,
-        private readonly ValidatorInterface $validator
+        private readonly ValidatorInterface $validator,
+        private readonly ProviderInterface $decorated
     ) {
     }
 
     public function provide(Operation $operation, array $uriVariables = [], array $context = []): object|array|null
     {
-        $body = $this->decorated->provide($operation, $uriVariables, $context);
-        if (!$context['request'] instanceof Request) {
-            return $body;
+        if (!($request = $context['request']) instanceof Request) {
+            return $this->decorated->provide($operation, $uriVariables, $context);
         }
 
-        $operation = $context['request']->attributes->get('_api_operation');
+        $operation = $request->attributes->get('_api_operation') ?? $operation;
         foreach ($operation->getParameters() ?? [] as $parameter) {
             if (!$constraints = $parameter->getConstraints()) {
                 continue;
             }
 
-            $value = $parameter->getExtraProperties()['_api_values'][$parameter->getKey()] ?? null;
+            $key = $this->getParameterFlattenKey($parameter->getKey(), $this->extractParameterValues($parameter, $request, $context));
+            $value = $parameter->getExtraProperties()['_api_values'][$key] ?? null;
             $violations = $this->validator->validate($value, $constraints);
             if (0 !== \count($violations)) {
                 $constraintViolationList = new ConstraintViolationList();
                 foreach ($violations as $violation) {
+                    $propertyPath = $key !== $parameter->getKey() ? $key.$violation->getPropertyPath() : ($parameter->getProperty() ?? $key);
                     $constraintViolationList->add(new ConstraintViolation(
                         $violation->getMessage(),
                         $violation->getMessageTemplate(),
                         $violation->getParameters(),
                         $violation->getRoot(),
-                        $parameter->getProperty() ?? $parameter->getKey(),
+                        $propertyPath,
                         $violation->getInvalidValue(),
                         $violation->getPlural(),
                         $violation->getCode(),
@@ -70,6 +74,6 @@ public function provide(Operation $operation, array $uriVariables = [], array $c
             }
         }
 
-        return $body;
+        return $this->decorated->provide($operation, $uriVariables, $context);
     }
 }
diff --git a/tests/.ignored-deprecations b/tests/.ignored-deprecations
@@ -14,3 +14,5 @@
 %The "Symfony\\Bundle\\MakerBundle\\Maker\\MakeAuthenticator" class is deprecated, use any of the Security\\Make\* commands instead%
 
 %Since symfony/validator 7.1: Not passing a value for the "requireTld" option to the Url constraint is deprecated. Its default value will change to "true".%
+
+%$fieldsBuilder argument of SchemaBuilder implementing "ApiPlatform\\GraphQl\\Type\\FieldsBuilderInterface" is deprecated since API Platform 3.1. It has to implement "ApiPlatform\\GraphQl\\Type\\FieldsBuilderEnumInterface" instead.%
diff --git a/tests/Fixtures/TestBundle/ApiResource/ValidateParameterBeforeProvider.php b/tests/Fixtures/TestBundle/ApiResource/ValidateParameterBeforeProvider.php
@@ -0,0 +1,42 @@
+<?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\Metadata\GetCollection;
+use ApiPlatform\Metadata\Operation;
+use ApiPlatform\Metadata\QueryParameter;
+use Symfony\Component\HttpFoundation\JsonResponse;
+use Symfony\Component\Validator\Constraints\Choice;
+use Symfony\Component\Validator\Constraints\Collection;
+use Symfony\Component\Validator\Constraints\NotBlank;
+
+#[GetCollection(
+    uriTemplate: 'query_parameter_validate_before_read',
+    parameters: [
+        'search' => new QueryParameter(constraints: [new NotBlank()]),
+        'sort[:property]' => new QueryParameter(constraints: [new NotBlank(), new Collection(['id' => new Choice(['asc', 'desc'])], allowMissingFields: true)]),
+    ],
+    provider: [self::class, 'provide']
+)]
+class ValidateParameterBeforeProvider
+{
+    public static function provide(Operation $operation, array $uriVariables = [], array $context = [])
+    {
+        if (!$context['request']->query->get('search')) {
+            throw new \RuntimeException('Not supposed to happen');
+        }
+
+        return new JsonResponse(204);
+    }
+}
diff --git a/...s/Functional/Parameters/DoctrineTests.php → tests/Functional/Parameters/DoctrineTest.php b/...s/Functional/Parameters/DoctrineTests.php → tests/Functional/Parameters/DoctrineTest.php
@@ -19,7 +19,7 @@
 use Doctrine\ORM\EntityManagerInterface;
 use Doctrine\ORM\Tools\SchemaTool;
 
-final class DoctrineTests extends ApiTestCase
+final class DoctrineTest extends ApiTestCase
 {
     public function testDoctrineEntitySearchFilter(): void
     {
@@ -55,8 +55,15 @@ public function testDoctrineEntitySearchFilter(): void
         $this->assertArraySubset(['foo' => 'bar', 'createdAt' => '2024-01-21T00:00:00+00:00'], $members[0]);
     }
 
+    /**
+     * @group legacy
+     */
     public function testGraphQl(): void
     {
+        if ($_SERVER['EVENT_LISTENERS_BACKWARD_COMPATIBILITY_LAYER'] ?? false) {
+            $this->markTestSkipped('Parameters are not supported in BC mode.');
+        }
+
         $this->recreateSchema();
         $container = static::getContainer();
         $object = 'mongodb' === $container->getParameter('kernel.environment') ? 'searchFilterParameterDocuments' : 'searchFilterParameters';

diff --git a/tests/Functional/Parameters/HydraTests.php → tests/Functional/Parameters/HydraTest.php b/tests/Functional/Parameters/HydraTests.php → tests/Functional/Parameters/HydraTest.php
@@ -15,11 +15,11 @@
 
 use ApiPlatform\Symfony\Bundle\Test\ApiTestCase;
 
-final class HydraTests extends ApiTestCase
+final class HydraTest extends ApiTestCase
 {
     public function testHydraTemplate(): void
     {
-        $response = self::createClient()->request('GET', 'with_parameters_collection');
+        $response = self::createClient()->request('GET', 'with_parameters_collection?hydra=1');
         $this->assertArraySubset(['hydra:search' => [
             'hydra:template' => '/with_parameters_collection{?hydra}',
             'hydra:mapping' => [

diff --git a/.../Functional/Parameters/ParameterTests.php → ...s/Functional/Parameters/ParameterTest.php b/.../Functional/Parameters/ParameterTests.php → ...s/Functional/Parameters/ParameterTest.php
@@ -15,7 +15,7 @@
 
 use ApiPlatform\Symfony\Bundle\Test\ApiTestCase;
 
-final class ParameterTests extends ApiTestCase
+final class ParameterTest extends ApiTestCase
 {
     public function testWithGroupFilter(): void
     {
Original file line number	Diff line number	Diff line change
Expand Up		@@ -14,3 +14,5 @@
		%The "Symfony\\Bundle\\MakerBundle\\Maker\\MakeAuthenticator" class is deprecated, use any of the Security\\Make\* commands instead%

		%Since symfony/validator 7.1: Not passing a value for the "requireTld" option to the Url constraint is deprecated. Its default value will change to "true".%

		%$fieldsBuilder argument of SchemaBuilder implementing "ApiPlatform\\GraphQl\\Type\\FieldsBuilderInterface" is deprecated since API Platform 3.1. It has to implement "ApiPlatform\\GraphQl\\Type\\FieldsBuilderEnumInterface" instead.%