bug #50458 [HttpKernel] Fix default value ignored with pinned resolvers (HypeMC)

nicolas-grekas · nicolas-grekas · commit 2af8f39425e2 · 2023-05-29T15:36:11.000+02:00
This PR was merged into the 6.3 branch. Discussion ---------- [HttpKernel] Fix default value ignored with pinned resolvers | Q | A | ------------- | --- | Branch? | 6.3 | Bug fix? | yes | New feature? | no | Deprecations? | no | Tickets | - | License | MIT | Doc PR | - Since #48992 the default value is ignored when, for example, `#[MapEntity]` is used: ```php #[Route('/')] #[Route('/{someId}')] public function index(#[MapEntity(id: 'someId')] ?Post $post): Response { // ... } ``` Before, `$post` would be `null` when making a request to `/`, now an exception is thrown: ``` Controller "App\Controller\TestController::index" requires that you provide a value for the "$post" argument. Either the argument is nullable and no null value has been provided, no default value has been provided or there is a non-optional argument after this one. ``` Since I can't think of a valid case when one would want to ignore the default value, I'd suggest always adding the `DefaultValueResolver` to the list when a pinned resolver is used. Commits ------- fabe7bc [HttpKernel] Fix default value ignored with pinned resolvers
diff --git a/src/Symfony/Component/HttpKernel/Controller/ArgumentResolver.php b/src/Symfony/Component/HttpKernel/Controller/ArgumentResolver.php
@@ -71,7 +71,10 @@ public function getArguments(Request $request, callable $controller, \Reflection
                         throw new ResolverNotFoundException($resolverName, $this->namedResolvers instanceof ServiceProviderInterface ? array_keys($this->namedResolvers->getProvidedServices()) : []);
                     }
 
-                    $argumentValueResolvers = [$this->namedResolvers->get($resolverName)];
+                    $argumentValueResolvers = [
+                        $this->namedResolvers->get($resolverName),
+                        new DefaultValueResolver(),
+                    ];
                 }
             }
 
diff --git a/src/Symfony/Component/HttpKernel/Controller/ArgumentResolver/QueryParameterValueResolver.php b/src/Symfony/Component/HttpKernel/Controller/ArgumentResolver/QueryParameterValueResolver.php
@@ -31,12 +31,8 @@ public function resolve(Request $request, ArgumentMetadata $argument): array
 
         $name = $attribute->name ?? $argument->getName();
         if (!$request->query->has($name)) {
-            if ($argument->hasDefaultValue()) {
-                return [$argument->getDefaultValue()];
-            }
-
-            if ($argument->isNullable()) {
-                return [null];
+            if ($argument->isNullable() || $argument->hasDefaultValue()) {
+                return [];
             }
 
             throw new NotFoundHttpException(sprintf('Missing query parameter "%s".', $name));
diff --git a/src/Symfony/Component/HttpKernel/Tests/Controller/ArgumentResolver/QueryParameterValueResolverTest.php b/src/Symfony/Component/HttpKernel/Tests/Controller/ArgumentResolver/QueryParameterValueResolverTest.php
@@ -179,14 +179,14 @@ public static function provideTestResolve(): iterable
         yield 'parameter not found but nullable' => [
             Request::create('/', 'GET'),
             new ArgumentMetadata('firstName', 'string', false, false, false, true, [new MapQueryParameter()]),
-            [null],
+            [],
             null,
         ];
 
         yield 'parameter not found but optional' => [
             Request::create('/', 'GET'),
             new ArgumentMetadata('firstName', 'string', false, true, false, attributes: [new MapQueryParameter()]),
-            [false],
+            [],
             null,
         ];
 
diff --git a/src/Symfony/Component/HttpKernel/Tests/Controller/ArgumentResolverTest.php b/src/Symfony/Component/HttpKernel/Tests/Controller/ArgumentResolverTest.php
@@ -296,6 +296,26 @@ public function testTargetedResolver()
         $this->assertSame([1], $resolver->getArguments($request, $controller));
     }
 
+    public function testTargetedResolverWithDefaultValue()
+    {
+        $resolver = self::getResolver([], [RequestAttributeValueResolver::class => new RequestAttributeValueResolver()]);
+
+        $request = Request::create('/');
+        $controller = $this->controllerTargetingResolverWithDefaultValue(...);
+
+        $this->assertSame([2], $resolver->getArguments($request, $controller));
+    }
+
+    public function testTargetedResolverWithNullableValue()
+    {
+        $resolver = self::getResolver([], [RequestAttributeValueResolver::class => new RequestAttributeValueResolver()]);
+
+        $request = Request::create('/');
+        $controller = $this->controllerTargetingResolverWithNullableValue(...);
+
+        $this->assertSame([null], $resolver->getArguments($request, $controller));
+    }
+
     public function testDisabledResolver()
     {
         $resolver = self::getResolver(namedResolvers: []);
@@ -373,6 +393,14 @@ public function controllerTargetingResolver(#[ValueResolver(DefaultValueResolver
     {
     }
 
+    public function controllerTargetingResolverWithDefaultValue(#[ValueResolver(RequestAttributeValueResolver::class)] int $foo = 2)
+    {
+    }
+
+    public function controllerTargetingResolverWithNullableValue(#[ValueResolver(RequestAttributeValueResolver::class)] ?int $foo)
+    {
+    }
+
     public function controllerDisablingResolver(#[ValueResolver(RequestAttributeValueResolver::class, disabled: true)] int $foo = 1)
     {
     }

Original file line number	Diff line number	Diff line change
`@@ -71,7 +71,10 @@ public function getArguments(Request $request, callable $controller, \Reflection`
`71`	`71`	`throw new ResolverNotFoundException($resolverName, $this->namedResolvers instanceof ServiceProviderInterface ? array_keys($this->namedResolvers->getProvidedServices()) : []);`
`72`	`72`	`}`
`73`	`73`
`74`		`- $argumentValueResolvers = [$this->namedResolvers->get($resolverName)];`
	`74`	`+ $argumentValueResolvers = [`
	`75`	`+ $this->namedResolvers->get($resolverName),`
	`76`	`+ new DefaultValueResolver(),`
	`77`	`+ ];`
`75`	`78`	`}`
`76`	`79`	`}`
`77`	`80`