Skip to content

Commit 53899af

Browse files
nicolas-grekasnicolas-grekas
committed
[HttpKernel] Don't cache controller's reflector inside the request
1 parent 0ca5051 commit 53899af

File tree

6 files changed

+34
-53
lines changed

6 files changed

+34
-53
lines changed

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

Lines changed: 2 additions & 3 deletions
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/ControllerMetadata/ArgumentMetadataFactory.php‎

Lines changed: 10 additions & 20 deletions
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/Event/ControllerArgumentsEvent.php‎

Lines changed: 1 addition & 2 deletions
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‎

Lines changed: 19 additions & 10 deletions
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‎

Lines changed: 1 addition & 2 deletions
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‎

Lines changed: 1 addition & 16 deletions
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 => [

0 commit comments

Comments
 (0)