Skip to content

Commit 0e28848

Browse files
fabpotfabpot
committed
bug #47468 [HttpKernel] Don't cache controller's reflector inside the request (nicolas-grekas)
This PR was merged into the 6.2 branch. Discussion ---------- [HttpKernel] Don't cache controller's reflector inside the request | Q | A | ------------- | --- | Branch? | 6.2 | Bug fix? | yes | New feature? | no | Deprecations? | no | Tickets | Fix #47461 | License | MIT | Doc PR | - Issue introduced by #46001 Commits ------- c751bd0 [HttpKernel] Don't cache controller's reflector inside the request
2 parents f885f8e + c751bd0 commit 0e28848

13 files changed

+50
-60
lines changed

‎src/Symfony/Component/HttpKernel/CHANGELOG.md

+1
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ CHANGELOG
99
* Add `#[Cache]` to describe the default HTTP cache headers on controllers
1010
* Add `absolute_uri` option to surrogate fragment renderers
1111
* Add `ValueResolverInterface` and deprecate `ArgumentValueResolverInterface`
12+
* Add argument `$reflector` to `ArgumentResolverInterface` and `ArgumentMetadataFactoryInterface`
1213

1314
6.1
1415
---

‎src/Symfony/Component/HttpKernel/Controller/ArgumentResolver.php

+2-3
Original file line numberDiff line numberDiff line change
@@ -40,12 +40,11 @@ public function __construct(ArgumentMetadataFactoryInterface $argumentMetadataFa
4040
$this->argumentValueResolvers = $argumentValueResolvers ?: self::getDefaultArgumentValueResolvers();
4141
}
4242

43-
public function getArguments(Request $request, callable $controller): array
43+
public function getArguments(Request $request, callable $controller, \ReflectionFunctionAbstract $reflector = null): array
4444
{
4545
$arguments = [];
46-
$reflectors = $request->attributes->get('_controller_reflectors') ?? [];
4746

48-
foreach ($this->argumentMetadataFactory->createArgumentMetadata($controller, ...$reflectors) as $metadata) {
47+
foreach ($this->argumentMetadataFactory->createArgumentMetadata($controller, $reflector) as $metadata) {
4948
foreach ($this->argumentValueResolvers as $resolver) {
5049
if ((!$resolver instanceof ValueResolverInterface || $resolver instanceof TraceableValueResolver) && !$resolver->supports($request, $metadata)) {
5150
continue;

‎src/Symfony/Component/HttpKernel/Controller/ArgumentResolverInterface.php

+3-1
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,9 @@ interface ArgumentResolverInterface
2424
/**
2525
* Returns the arguments to pass to the controller.
2626
*
27+
* @param \ReflectionFunctionAbstract|null $reflector
28+
*
2729
* @throws \RuntimeException When no value could be provided for a required argument
2830
*/
29-
public function getArguments(Request $request, callable $controller): array;
31+
public function getArguments(Request $request, callable $controller/* , \ReflectionFunctionAbstract $reflector = null */): array;
3032
}

‎src/Symfony/Component/HttpKernel/Controller/TraceableArgumentResolver.php

+6-2
Original file line numberDiff line numberDiff line change
@@ -28,11 +28,15 @@ public function __construct(ArgumentResolverInterface $resolver, Stopwatch $stop
2828
$this->stopwatch = $stopwatch;
2929
}
3030

31-
public function getArguments(Request $request, callable $controller): array
31+
/**
32+
* @param \ReflectionFunctionAbstract|null $reflector
33+
*/
34+
public function getArguments(Request $request, callable $controller/* , \ReflectionFunctionAbstract $reflector = null */): array
3235
{
36+
$reflector = 2 < \func_num_args() ? func_get_arg(2) : null;
3337
$e = $this->stopwatch->start('controller.get_arguments');
3438

35-
$ret = $this->resolver->getArguments($request, $controller);
39+
$ret = $this->resolver->getArguments($request, $controller, $reflector);
3640

3741
$e->stop();
3842

‎src/Symfony/Component/HttpKernel/ControllerMetadata/ArgumentMetadataFactory.php

+10-20
Original file line numberDiff line numberDiff line change
@@ -18,25 +18,20 @@
1818
*/
1919
final class ArgumentMetadataFactory implements ArgumentMetadataFactoryInterface
2020
{
21-
public function createArgumentMetadata(string|object|array $controller, \ReflectionClass $class = null, \ReflectionFunctionAbstract $reflection = null): array
21+
public function createArgumentMetadata(string|object|array $controller, \ReflectionFunctionAbstract $reflector = null): array
2222
{
2323
$arguments = [];
24+
$reflector ??= new \ReflectionFunction($controller(...));
2425

25-
if (null === $reflection) {
26-
$reflection = new \ReflectionFunction($controller(...));
27-
$class = str_contains($reflection->name, '{closure}') ? null : $reflection->getClosureScopeClass();
28-
}
29-
$class = $class?->name;
30-
31-
foreach ($reflection->getParameters() as $param) {
26+
foreach ($reflector->getParameters() as $param) {
3227
$attributes = [];
3328
foreach ($param->getAttributes() as $reflectionAttribute) {
3429
if (class_exists($reflectionAttribute->getName())) {
3530
$attributes[] = $reflectionAttribute->newInstance();
3631
}
3732
}
3833

39-
$arguments[] = new ArgumentMetadata($param->getName(), $this->getType($param, $class), $param->isVariadic(), $param->isDefaultValueAvailable(), $param->isDefaultValueAvailable() ? $param->getDefaultValue() : null, $param->allowsNull(), $attributes);
34+
$arguments[] = new ArgumentMetadata($param->getName(), $this->getType($param), $param->isVariadic(), $param->isDefaultValueAvailable(), $param->isDefaultValueAvailable() ? $param->getDefaultValue() : null, $param->allowsNull(), $attributes);
4035
}
4136

4237
return $arguments;
@@ -45,22 +40,17 @@ public function createArgumentMetadata(string|object|array $controller, \Reflect
4540
/**
4641
* Returns an associated type to the given parameter if available.
4742
*/
48-
private function getType(\ReflectionParameter $parameter, ?string $class): ?string
43+
private function getType(\ReflectionParameter $parameter): ?string
4944
{
5045
if (!$type = $parameter->getType()) {
5146
return null;
5247
}
5348
$name = $type instanceof \ReflectionNamedType ? $type->getName() : (string) $type;
5449

55-
if (null !== $class) {
56-
switch (strtolower($name)) {
57-
case 'self':
58-
return $class;
59-
case 'parent':
60-
return get_parent_class($class) ?: null;
61-
}
62-
}
63-
64-
return $name;
50+
return match (strtolower($name)) {
51+
'self' => $parameter->getDeclaringClass()?->name,
52+
'parent' => get_parent_class($parameter->getDeclaringClass()?->name ?? '') ?: null,
53+
default => $name,
54+
};
6555
}
6656
}

‎src/Symfony/Component/HttpKernel/ControllerMetadata/ArgumentMetadataFactoryInterface.php

+3-1
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,9 @@
1919
interface ArgumentMetadataFactoryInterface
2020
{
2121
/**
22+
* @param \ReflectionFunctionAbstract|null $reflector
23+
*
2224
* @return ArgumentMetadata[]
2325
*/
24-
public function createArgumentMetadata(string|object|array $controller): array;
26+
public function createArgumentMetadata(string|object|array $controller/* , \ReflectionFunctionAbstract $reflector = null */): array;
2527
}

‎src/Symfony/Component/HttpKernel/Event/ControllerArgumentsEvent.php

+1-2
Original file line numberDiff line numberDiff line change
@@ -77,9 +77,8 @@ public function getNamedArguments(): array
7777

7878
$namedArguments = [];
7979
$arguments = $this->arguments;
80-
$r = $this->getRequest()->attributes->get('_controller_reflectors')[1] ?? new \ReflectionFunction($this->controllerEvent->getController());
8180

82-
foreach ($r->getParameters() as $i => $param) {
81+
foreach ($this->controllerEvent->getControllerReflector()->getParameters() as $i => $param) {
8382
if ($param->isVariadic()) {
8483
$namedArguments[$param->name] = \array_slice($arguments, $i);
8584
break;

‎src/Symfony/Component/HttpKernel/Event/ControllerEvent.php

+19-10
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@
2828
final class ControllerEvent extends KernelEvent
2929
{
3030
private string|array|object $controller;
31+
private \ReflectionFunctionAbstract $controllerReflector;
3132
private array $attributes;
3233

3334
public function __construct(HttpKernelInterface $kernel, callable $controller, Request $request, ?int $requestType)
@@ -42,6 +43,11 @@ public function getController(): callable
4243
return $this->controller;
4344
}
4445

46+
public function getControllerReflector(): \ReflectionFunctionAbstract
47+
{
48+
return $this->controllerReflector;
49+
}
50+
4551
/**
4652
* @param array<class-string, list<object>>|null $attributes
4753
*/
@@ -62,17 +68,13 @@ public function setController(callable $controller, array $attributes = null): v
6268
}
6369

6470
if (\is_array($controller) && method_exists(...$controller)) {
65-
$action = new \ReflectionMethod(...$controller);
66-
$class = new \ReflectionClass($controller[0]);
71+
$this->controllerReflector = new \ReflectionMethod(...$controller);
6772
} elseif (\is_string($controller) && false !== $i = strpos($controller, '::')) {
68-
$action = new \ReflectionMethod($controller);
69-
$class = new \ReflectionClass(substr($controller, 0, $i));
73+
$this->controllerReflector = new \ReflectionMethod($controller);
7074
} else {
71-
$action = new \ReflectionFunction($controller(...));
72-
$class = str_contains($action->name, '{closure}') ? null : $action->getClosureScopeClass();
75+
$this->controllerReflector = new \ReflectionFunction($controller(...));
7376
}
7477

75-
$this->getRequest()->attributes->set('_controller_reflectors', [$class, $action]);
7678
$this->controller = $controller;
7779
}
7880

@@ -81,13 +83,20 @@ public function setController(callable $controller, array $attributes = null): v
8183
*/
8284
public function getAttributes(): array
8385
{
84-
if (isset($this->attributes) || ![$class, $action] = $this->getRequest()->attributes->get('_controller_reflectors')) {
85-
return $this->attributes ??= [];
86+
if (isset($this->attributes)) {
87+
return $this->attributes;
8688
}
8789

90+
if (\is_array($this->controller) && method_exists(...$this->controller)) {
91+
$class = new \ReflectionClass($this->controller[0]);
92+
} elseif (\is_string($this->controller) && false !== $i = strpos($this->controller, '::')) {
93+
$class = new \ReflectionClass(substr($this->controller, 0, $i));
94+
} else {
95+
$class = str_contains($this->controllerReflector->name, '{closure}') ? null : $this->controllerReflector->getClosureScopeClass();
96+
}
8897
$this->attributes = [];
8998

90-
foreach (array_merge($class?->getAttributes() ?? [], $action->getAttributes()) as $attribute) {
99+
foreach (array_merge($class?->getAttributes() ?? [], $this->controllerReflector->getAttributes()) as $attribute) {
91100
if (class_exists($attribute->getName())) {
92101
$this->attributes[$attribute->getName()][] = $attribute->newInstance();
93102
}

‎src/Symfony/Component/HttpKernel/HttpKernel.php

+1-2
Original file line numberDiff line numberDiff line change
@@ -145,7 +145,7 @@ private function handleRaw(Request $request, int $type = self::MAIN_REQUEST): Re
145145
$controller = $event->getController();
146146

147147
// controller arguments
148-
$arguments = $this->argumentResolver->getArguments($request, $controller);
148+
$arguments = $this->argumentResolver->getArguments($request, $controller, $event->getControllerReflector());
149149

150150
$event = new ControllerArgumentsEvent($this, $event, $arguments, $request, $type);
151151
$this->dispatcher->dispatch($event, KernelEvents::CONTROLLER_ARGUMENTS);
@@ -173,7 +173,6 @@ private function handleRaw(Request $request, int $type = self::MAIN_REQUEST): Re
173173
throw new ControllerDoesNotReturnResponseException($msg, $controller, __FILE__, __LINE__ - 17);
174174
}
175175
}
176-
$request->attributes->remove('_controller_reflectors');
177176

178177
return $this->filterResponse($response, $request, $type);
179178
}

‎src/Symfony/Component/HttpKernel/Tests/Event/ControllerEventTest.php

+1-16
Original file line numberDiff line numberDiff line change
@@ -21,27 +21,12 @@
2121

2222
class ControllerEventTest extends TestCase
2323
{
24-
public function testSetAttributes()
25-
{
26-
$request = new Request();
27-
$request->attributes->set('_controller_reflectors', [1, 2]);
28-
$controller = [new AttributeController(), 'action'];
29-
$event = new ControllerEvent(new TestHttpKernel(), $controller, $request, HttpKernelInterface::MAIN_REQUEST);
30-
$event->setController($controller, []);
31-
32-
$this->assertSame([], $event->getAttributes());
33-
}
34-
3524
/**
3625
* @dataProvider provideGetAttributes
3726
*/
3827
public function testGetAttributes(callable $controller)
3928
{
40-
$request = new Request();
41-
$reflector = new \ReflectionFunction($controller(...));
42-
$request->attributes->set('_controller_reflectors', [str_contains($reflector->name, '{closure}') ? null : $reflector->getClosureScopeClass(), $reflector]);
43-
44-
$event = new ControllerEvent(new TestHttpKernel(), $controller, $request, HttpKernelInterface::MAIN_REQUEST);
29+
$event = new ControllerEvent(new TestHttpKernel(), $controller, new Request(), HttpKernelInterface::MAIN_REQUEST);
4530

4631
$expected = [
4732
Bar::class => [

‎src/Symfony/Component/HttpKernel/Tests/HttpCache/TestHttpKernel.php

+1-1
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,7 @@ public function getController(Request $request): callable|false
7272
return $this->callController(...);
7373
}
7474

75-
public function getArguments(Request $request, callable $controller): array
75+
public function getArguments(Request $request, callable $controller, \ReflectionFunctionAbstract $reflector = null): array
7676
{
7777
return [$request];
7878
}

‎src/Symfony/Component/HttpKernel/Tests/HttpCache/TestMultipleHttpKernel.php

+1-1
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@ public function getController(Request $request): callable|false
5555
return $this->callController(...);
5656
}
5757

58-
public function getArguments(Request $request, callable $controller): array
58+
public function getArguments(Request $request, callable $controller, \ReflectionFunctionAbstract $reflector = null): array
5959
{
6060
return [$request];
6161
}

‎src/Symfony/Component/HttpKernel/Tests/TestHttpKernel.php

+1-1
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ public function getController(Request $request): callable|false
3030
return $this->callController(...);
3131
}
3232

33-
public function getArguments(Request $request, callable $controller): array
33+
public function getArguments(Request $request, callable $controller, \ReflectionFunctionAbstract $reflector = null): array
3434
{
3535
return [$request];
3636
}

0 commit comments

Comments
 (0)