From 967d873e477ade6120ea16b0762144f042c6852c Mon Sep 17 00:00:00 2001 From: tsantos Date: Wed, 18 Mar 2020 13:57:28 -0300 Subject: [PATCH 1/7] [HttpKernel] Adds entry to change log --- src/Symfony/Component/HttpKernel/CHANGELOG.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/Symfony/Component/HttpKernel/CHANGELOG.md b/src/Symfony/Component/HttpKernel/CHANGELOG.md index ada9fafe60102..7e96927d06eb1 100644 --- a/src/Symfony/Component/HttpKernel/CHANGELOG.md +++ b/src/Symfony/Component/HttpKernel/CHANGELOG.md @@ -1,6 +1,11 @@ CHANGELOG ========= +5.2.0 +----- + + * added @QueryParam annotation to read query string from request + 5.1.0 ----- From 2ba1bb3440dd8c98de88f213158bddbcdeac5e88 Mon Sep 17 00:00:00 2001 From: tsantos Date: Thu, 19 Mar 2020 11:56:41 -0300 Subject: [PATCH 2/7] [HttpKernel] Adds controller listener to create a list of configurations --- .../Configuration/ConfigurationAnnotation.php | 19 +++++ .../Configuration/QueryParam.php | 33 +++++++++ .../ConfigurationInterface.php | 12 ++++ .../ConfigurationList.php | 35 ++++++++++ .../HttpKernel/Event/ControllerEvent.php | 13 ++++ .../EventListener/ControllerListener.php | 69 +++++++++++++++++++ .../AnnotatedControllerListenerTest.php | 39 +++++++++++ .../Controller/AnnotatedController.php | 20 ++++++ 8 files changed, 240 insertions(+) create mode 100644 src/Symfony/Component/HttpKernel/ControllerConfiguration/Configuration/ConfigurationAnnotation.php create mode 100644 src/Symfony/Component/HttpKernel/ControllerConfiguration/Configuration/QueryParam.php create mode 100644 src/Symfony/Component/HttpKernel/ControllerConfiguration/ConfigurationInterface.php create mode 100644 src/Symfony/Component/HttpKernel/ControllerConfiguration/ConfigurationList.php create mode 100644 src/Symfony/Component/HttpKernel/EventListener/ControllerListener.php create mode 100644 src/Symfony/Component/HttpKernel/Tests/EventListener/AnnotatedControllerListenerTest.php create mode 100644 src/Symfony/Component/HttpKernel/Tests/Fixtures/Controller/AnnotatedController.php diff --git a/src/Symfony/Component/HttpKernel/ControllerConfiguration/Configuration/ConfigurationAnnotation.php b/src/Symfony/Component/HttpKernel/ControllerConfiguration/Configuration/ConfigurationAnnotation.php new file mode 100644 index 0000000000000..85595b08030de --- /dev/null +++ b/src/Symfony/Component/HttpKernel/ControllerConfiguration/Configuration/ConfigurationAnnotation.php @@ -0,0 +1,19 @@ + $v) { + if (!method_exists($this, $name = 'set'.$k)) { + throw new \RuntimeException(sprintf('Unknown key "%s" for annotation "@%s".', $k, static::class)); + } + + $this->$name($v); + } + } +} diff --git a/src/Symfony/Component/HttpKernel/ControllerConfiguration/Configuration/QueryParam.php b/src/Symfony/Component/HttpKernel/ControllerConfiguration/Configuration/QueryParam.php new file mode 100644 index 0000000000000..a9f18d5d65eb0 --- /dev/null +++ b/src/Symfony/Component/HttpKernel/ControllerConfiguration/Configuration/QueryParam.php @@ -0,0 +1,33 @@ +name; + } + + public function setName(string $name): void + { + $this->name = $name; + } + + public function setValue(string $value): void + { + $this->setName($value); + } + + public function getTarget(): array + { + return [ConfigurationInterface::TARGET_CLASS, ConfigurationInterface::TARGET_ARGUMENTS]; + } +} diff --git a/src/Symfony/Component/HttpKernel/ControllerConfiguration/ConfigurationInterface.php b/src/Symfony/Component/HttpKernel/ControllerConfiguration/ConfigurationInterface.php new file mode 100644 index 0000000000000..8748af422a82f --- /dev/null +++ b/src/Symfony/Component/HttpKernel/ControllerConfiguration/ConfigurationInterface.php @@ -0,0 +1,12 @@ +configurations[] = $configuration; + foreach ($configuration->getTarget() as $target) { + if (!isset($this->targetConfigurations[$target])) { + $this->targetConfigurations[$target] = []; + } + + $this->targetConfigurations[$target][] = $configuration; + } + } + + public function forTarget(string $target): array + { + if (!isset($this->targetConfigurations[$target])) { + return []; + } + + return $this->targetConfigurations[$target]; + } + + public function count(): int + { + return \count($this->configurations); + } +} diff --git a/src/Symfony/Component/HttpKernel/Event/ControllerEvent.php b/src/Symfony/Component/HttpKernel/Event/ControllerEvent.php index da88800e20285..05cf45c0ce484 100644 --- a/src/Symfony/Component/HttpKernel/Event/ControllerEvent.php +++ b/src/Symfony/Component/HttpKernel/Event/ControllerEvent.php @@ -12,6 +12,7 @@ namespace Symfony\Component\HttpKernel\Event; use Symfony\Component\HttpFoundation\Request; +use Symfony\Component\HttpKernel\ControllerConfiguration\ConfigurationList; use Symfony\Component\HttpKernel\HttpKernelInterface; /** @@ -28,12 +29,14 @@ final class ControllerEvent extends KernelEvent { private $controller; + private $configurations; public function __construct(HttpKernelInterface $kernel, callable $controller, Request $request, ?int $requestType) { parent::__construct($kernel, $request, $requestType); $this->setController($controller); + $this->setConfigurations(new ConfigurationList()); } public function getController(): callable @@ -45,4 +48,14 @@ public function setController(callable $controller): void { $this->controller = $controller; } + + public function setConfigurations(ConfigurationList $configurations): void + { + $this->configurations = $configurations; + } + + public function getConfigurations(): ConfigurationList + { + return $this->configurations; + } } diff --git a/src/Symfony/Component/HttpKernel/EventListener/ControllerListener.php b/src/Symfony/Component/HttpKernel/EventListener/ControllerListener.php new file mode 100644 index 0000000000000..0fc847e4d6568 --- /dev/null +++ b/src/Symfony/Component/HttpKernel/EventListener/ControllerListener.php @@ -0,0 +1,69 @@ + + */ +class ControllerListener implements EventSubscriberInterface +{ + /** + * @var Reader + */ + private $reader; + + public function __construct(Reader $reader) + { + $this->reader = $reader; + } + + public function onController(ControllerEvent $event): void + { + $controller = $event->getController(); + + if (!\is_array($controller) && method_exists($controller, '__invoke')) { + $controller = [$controller, '__invoke']; + } + + if (!\is_array($controller)) { + return; + } + + $reflectionObject = new \ReflectionObject($controller[0]); + $reflectionMethod = $reflectionObject->getMethod($controller[1]); + + $configurations = new ConfigurationList(); + + $this->appendConfigurations($configurations, $this->reader->getClassAnnotations($reflectionObject)); + $this->appendConfigurations($configurations, $this->reader->getMethodAnnotations($reflectionMethod)); + + $event->setConfigurations($configurations); + } + + private function appendConfigurations(ConfigurationList $configurations, array $annotations): void + { + foreach ($annotations as $annotation) { + if ($annotation instanceof ConfigurationInterface) { + $configurations->add($annotation); + } + } + } + + public static function getSubscribedEvents() + { + return [ + KernelEvents::CONTROLLER => 'onController', + ]; + } +} diff --git a/src/Symfony/Component/HttpKernel/Tests/EventListener/AnnotatedControllerListenerTest.php b/src/Symfony/Component/HttpKernel/Tests/EventListener/AnnotatedControllerListenerTest.php new file mode 100644 index 0000000000000..8b1f6c055acfe --- /dev/null +++ b/src/Symfony/Component/HttpKernel/Tests/EventListener/AnnotatedControllerListenerTest.php @@ -0,0 +1,39 @@ +reader = new AnnotationReader(); + $this->listener = new ControllerListener($this->reader); + } + + public function testOnController() + { + $event = $this->createControllerEvent([new AnnotatedController(), 'queryParamAction']); + $this->listener->onController($event); + $this->assertCount(2, $event->getConfigurations()); + } + + private function createControllerEvent(callable $controller): ControllerEvent + { + $kernel = new KernelForTest('test', true); + $event = new ControllerEvent($kernel, $controller, new Request(), HttpKernelInterface::MASTER_REQUEST); + + return $event; + } +} diff --git a/src/Symfony/Component/HttpKernel/Tests/Fixtures/Controller/AnnotatedController.php b/src/Symfony/Component/HttpKernel/Tests/Fixtures/Controller/AnnotatedController.php new file mode 100644 index 0000000000000..992a2f727664f --- /dev/null +++ b/src/Symfony/Component/HttpKernel/Tests/Fixtures/Controller/AnnotatedController.php @@ -0,0 +1,20 @@ + Date: Thu, 19 Mar 2020 13:13:35 -0300 Subject: [PATCH 3/7] [HttpKernel] Resolves query params from annotations --- .../Controller/ArgumentResolver.php | 2 + .../QueryParamValueResolver.php | 43 +++++++++++++++++++ .../Configuration/QueryParam.php | 23 +++++++--- .../ConfigurationInterface.php | 5 --- .../ConfigurationList.php | 35 +++++++++------ .../HttpKernel/Event/ControllerEvent.php | 13 ------ .../EventListener/ControllerListener.php | 2 +- .../Tests/Controller/ArgumentResolverTest.php | 15 +++++++ .../AnnotatedControllerListenerTest.php | 15 ++++++- 9 files changed, 113 insertions(+), 40 deletions(-) create mode 100644 src/Symfony/Component/HttpKernel/Controller/ArgumentResolver/QueryParamValueResolver.php diff --git a/src/Symfony/Component/HttpKernel/Controller/ArgumentResolver.php b/src/Symfony/Component/HttpKernel/Controller/ArgumentResolver.php index 4285ba76319b2..644f05783d6b9 100644 --- a/src/Symfony/Component/HttpKernel/Controller/ArgumentResolver.php +++ b/src/Symfony/Component/HttpKernel/Controller/ArgumentResolver.php @@ -13,6 +13,7 @@ use Symfony\Component\HttpFoundation\Request; use Symfony\Component\HttpKernel\Controller\ArgumentResolver\DefaultValueResolver; +use Symfony\Component\HttpKernel\Controller\ArgumentResolver\QueryParamValueResolver; use Symfony\Component\HttpKernel\Controller\ArgumentResolver\RequestAttributeValueResolver; use Symfony\Component\HttpKernel\Controller\ArgumentResolver\RequestValueResolver; use Symfony\Component\HttpKernel\Controller\ArgumentResolver\SessionValueResolver; @@ -91,6 +92,7 @@ public static function getDefaultArgumentValueResolvers(): iterable new SessionValueResolver(), new DefaultValueResolver(), new VariadicValueResolver(), + new QueryParamValueResolver(), ]; } } diff --git a/src/Symfony/Component/HttpKernel/Controller/ArgumentResolver/QueryParamValueResolver.php b/src/Symfony/Component/HttpKernel/Controller/ArgumentResolver/QueryParamValueResolver.php new file mode 100644 index 0000000000000..1160055d2fa0d --- /dev/null +++ b/src/Symfony/Component/HttpKernel/Controller/ArgumentResolver/QueryParamValueResolver.php @@ -0,0 +1,43 @@ +attributes->has('_configurations')) { + return false; + } + + return 1 === $this->getConfigurationsForArgument($request, $argument)->count(); + } + + public function resolve(Request $request, ArgumentMetadata $argument) + { + $configuration = $this->getConfigurationsForArgument($request, $argument)->first(); + + // todo validate query param constraints + + yield $request->query->get($configuration->getName()); + } + + private function getConfigurationsForArgument(Request $request, ArgumentMetadata $argument): ConfigurationList + { + /** @var ConfigurationList $configurations */ + $configurations = $request->attributes->get('_configurations'); + + $configuration = $configurations->filter(function (ConfigurationInterface $configuration) use ($argument): bool { + return $configuration instanceof QueryParam && $configuration->getArgumentName() === $argument->getName(); + }); + + return $configuration; + } +} diff --git a/src/Symfony/Component/HttpKernel/ControllerConfiguration/Configuration/QueryParam.php b/src/Symfony/Component/HttpKernel/ControllerConfiguration/Configuration/QueryParam.php index a9f18d5d65eb0..8304e98911155 100644 --- a/src/Symfony/Component/HttpKernel/ControllerConfiguration/Configuration/QueryParam.php +++ b/src/Symfony/Component/HttpKernel/ControllerConfiguration/Configuration/QueryParam.php @@ -2,15 +2,23 @@ namespace Symfony\Component\HttpKernel\ControllerConfiguration\Configuration; -use Symfony\Component\HttpKernel\ControllerConfiguration\ConfigurationInterface; - /** * @Annotation */ class QueryParam extends ConfigurationAnnotation { + private $argumentName; private $name; + public function __construct(array $values) + { + parent::__construct($values); + + if (null === $this->name) { + $this->name = $this->argumentName; + } + } + public function getName() { return $this->name; @@ -23,11 +31,16 @@ public function setName(string $name): void public function setValue(string $value): void { - $this->setName($value); + $this->setArgumentName($value); + } + + public function setArgumentName(string $argumentName): void + { + $this->argumentName = $argumentName; } - public function getTarget(): array + public function getArgumentName() { - return [ConfigurationInterface::TARGET_CLASS, ConfigurationInterface::TARGET_ARGUMENTS]; + return $this->argumentName; } } diff --git a/src/Symfony/Component/HttpKernel/ControllerConfiguration/ConfigurationInterface.php b/src/Symfony/Component/HttpKernel/ControllerConfiguration/ConfigurationInterface.php index 8748af422a82f..91dd74adda93d 100644 --- a/src/Symfony/Component/HttpKernel/ControllerConfiguration/ConfigurationInterface.php +++ b/src/Symfony/Component/HttpKernel/ControllerConfiguration/ConfigurationInterface.php @@ -4,9 +4,4 @@ interface ConfigurationInterface { - const TARGET_CLASS = 'class'; - const TARGET_ARGUMENTS = 'arguments'; - const TARGET_RESPONSE = 'response'; - - public function getTarget(): array; } diff --git a/src/Symfony/Component/HttpKernel/ControllerConfiguration/ConfigurationList.php b/src/Symfony/Component/HttpKernel/ControllerConfiguration/ConfigurationList.php index a27b618e87818..04c552e144bf6 100644 --- a/src/Symfony/Component/HttpKernel/ControllerConfiguration/ConfigurationList.php +++ b/src/Symfony/Component/HttpKernel/ControllerConfiguration/ConfigurationList.php @@ -2,34 +2,41 @@ namespace Symfony\Component\HttpKernel\ControllerConfiguration; -class ConfigurationList implements \Countable +class ConfigurationList implements \Countable, \IteratorAggregate { private $configurations = []; - private $targetConfigurations = []; - public function add(ConfigurationInterface $configuration): void + public function __construct(array $configurations = []) + { + foreach ($configurations as $configuration) { + $this->add($configuration); + } + } + + public function add(ConfigurationInterface $configuration): self { $this->configurations[] = $configuration; - foreach ($configuration->getTarget() as $target) { - if (!isset($this->targetConfigurations[$target])) { - $this->targetConfigurations[$target] = []; - } - $this->targetConfigurations[$target][] = $configuration; - } + return $this; } - public function forTarget(string $target): array + public function filter(callable $filter): self { - if (!isset($this->targetConfigurations[$target])) { - return []; - } + return new static(array_filter($this->configurations, $filter)); + } - return $this->targetConfigurations[$target]; + public function first(): ?ConfigurationInterface + { + return reset($this->configurations) ?? null; } public function count(): int { return \count($this->configurations); } + + public function getIterator() + { + return new \ArrayIterator($this->configurations); + } } diff --git a/src/Symfony/Component/HttpKernel/Event/ControllerEvent.php b/src/Symfony/Component/HttpKernel/Event/ControllerEvent.php index 05cf45c0ce484..da88800e20285 100644 --- a/src/Symfony/Component/HttpKernel/Event/ControllerEvent.php +++ b/src/Symfony/Component/HttpKernel/Event/ControllerEvent.php @@ -12,7 +12,6 @@ namespace Symfony\Component\HttpKernel\Event; use Symfony\Component\HttpFoundation\Request; -use Symfony\Component\HttpKernel\ControllerConfiguration\ConfigurationList; use Symfony\Component\HttpKernel\HttpKernelInterface; /** @@ -29,14 +28,12 @@ final class ControllerEvent extends KernelEvent { private $controller; - private $configurations; public function __construct(HttpKernelInterface $kernel, callable $controller, Request $request, ?int $requestType) { parent::__construct($kernel, $request, $requestType); $this->setController($controller); - $this->setConfigurations(new ConfigurationList()); } public function getController(): callable @@ -48,14 +45,4 @@ public function setController(callable $controller): void { $this->controller = $controller; } - - public function setConfigurations(ConfigurationList $configurations): void - { - $this->configurations = $configurations; - } - - public function getConfigurations(): ConfigurationList - { - return $this->configurations; - } } diff --git a/src/Symfony/Component/HttpKernel/EventListener/ControllerListener.php b/src/Symfony/Component/HttpKernel/EventListener/ControllerListener.php index 0fc847e4d6568..3d9c3b7eb94ab 100644 --- a/src/Symfony/Component/HttpKernel/EventListener/ControllerListener.php +++ b/src/Symfony/Component/HttpKernel/EventListener/ControllerListener.php @@ -48,7 +48,7 @@ public function onController(ControllerEvent $event): void $this->appendConfigurations($configurations, $this->reader->getClassAnnotations($reflectionObject)); $this->appendConfigurations($configurations, $this->reader->getMethodAnnotations($reflectionMethod)); - $event->setConfigurations($configurations); + $event->getRequest()->attributes->set('_configurations', $configurations); } private function appendConfigurations(ConfigurationList $configurations, array $annotations): void diff --git a/src/Symfony/Component/HttpKernel/Tests/Controller/ArgumentResolverTest.php b/src/Symfony/Component/HttpKernel/Tests/Controller/ArgumentResolverTest.php index d663297121887..1fa9e0aae53b9 100644 --- a/src/Symfony/Component/HttpKernel/Tests/Controller/ArgumentResolverTest.php +++ b/src/Symfony/Component/HttpKernel/Tests/Controller/ArgumentResolverTest.php @@ -19,7 +19,10 @@ use Symfony\Component\HttpKernel\Controller\ArgumentResolver; use Symfony\Component\HttpKernel\Controller\ArgumentResolver\RequestAttributeValueResolver; use Symfony\Component\HttpKernel\Controller\ArgumentValueResolverInterface; +use Symfony\Component\HttpKernel\ControllerConfiguration\Configuration\QueryParam; +use Symfony\Component\HttpKernel\ControllerConfiguration\ConfigurationList; use Symfony\Component\HttpKernel\ControllerMetadata\ArgumentMetadataFactory; +use Symfony\Component\HttpKernel\Tests\Fixtures\Controller\AnnotatedController; use Symfony\Component\HttpKernel\Tests\Fixtures\Controller\ExtendingRequest; use Symfony\Component\HttpKernel\Tests\Fixtures\Controller\ExtendingSession; use Symfony\Component\HttpKernel\Tests\Fixtures\Controller\NullableController; @@ -280,6 +283,18 @@ public function testGetSessionMissMatchOnNull() self::$resolver->getArguments($request, $controller); } + public function testGetQueryParam() + { + $request = Request::create('/?foo=foo&bar=bar'); + $request->attributes->set('_configurations', new ConfigurationList([ + new QueryParam(['value' => 'foo']), + new QueryParam(['value' => 'bar']), + ])); + $controller = [new AnnotatedController(), 'queryParamAction']; + + $this->assertEquals(['foo', 'bar'], self::$resolver->getArguments($request, $controller)); + } + public function __invoke($foo, $bar = null) { } diff --git a/src/Symfony/Component/HttpKernel/Tests/EventListener/AnnotatedControllerListenerTest.php b/src/Symfony/Component/HttpKernel/Tests/EventListener/AnnotatedControllerListenerTest.php index 8b1f6c055acfe..e4b71186520e5 100644 --- a/src/Symfony/Component/HttpKernel/Tests/EventListener/AnnotatedControllerListenerTest.php +++ b/src/Symfony/Component/HttpKernel/Tests/EventListener/AnnotatedControllerListenerTest.php @@ -24,9 +24,20 @@ protected function setUp(): void public function testOnController() { - $event = $this->createControllerEvent([new AnnotatedController(), 'queryParamAction']); + $request = new Request(); + + $kernel = new KernelForTest('test', true); + $event = new ControllerEvent( + $kernel, + [new AnnotatedController(), 'queryParamAction'], + $request, + HttpKernelInterface::MASTER_REQUEST + ); + $this->listener->onController($event); - $this->assertCount(2, $event->getConfigurations()); + + $this->assertTrue($request->attributes->has('_configurations')); + $this->assertCount(2, $request->attributes->get('_configurations')); } private function createControllerEvent(callable $controller): ControllerEvent From 15737036315263beb8a7a000fdd5c8b7df4b4cfe Mon Sep 17 00:00:00 2001 From: tsantos Date: Thu, 19 Mar 2020 13:29:43 -0300 Subject: [PATCH 4/7] [HttpKernel] Resolving arguments with default values --- .../QueryParamValueResolver.php | 19 +++++++++++-------- .../Tests/Controller/ArgumentResolverTest.php | 14 +++++++++++++- .../AnnotatedControllerListenerTest.php | 2 +- .../Controller/AnnotatedController.php | 10 +++++++++- .../Component/HttpKernel/composer.json | 1 + 5 files changed, 35 insertions(+), 11 deletions(-) diff --git a/src/Symfony/Component/HttpKernel/Controller/ArgumentResolver/QueryParamValueResolver.php b/src/Symfony/Component/HttpKernel/Controller/ArgumentResolver/QueryParamValueResolver.php index 1160055d2fa0d..412ce15674169 100644 --- a/src/Symfony/Component/HttpKernel/Controller/ArgumentResolver/QueryParamValueResolver.php +++ b/src/Symfony/Component/HttpKernel/Controller/ArgumentResolver/QueryParamValueResolver.php @@ -17,19 +17,22 @@ public function supports(Request $request, ArgumentMetadata $argument) return false; } - return 1 === $this->getConfigurationsForArgument($request, $argument)->count(); + /** @var QueryParam $configuration */ + $configuration = $this->getConfigurationsForArgument($request, $argument); + + return null !== $configuration + && !$argument->isVariadic() + && ($request->query->has($configuration->getName()) || $argument->hasDefaultValue()); } public function resolve(Request $request, ArgumentMetadata $argument) { - $configuration = $this->getConfigurationsForArgument($request, $argument)->first(); - - // todo validate query param constraints - - yield $request->query->get($configuration->getName()); + $configuration = $this->getConfigurationsForArgument($request, $argument); + $defaultValue = $argument->hasDefaultValue() ? $argument->getDefaultValue() : null; + yield $request->query->get($configuration->getName(), $defaultValue); } - private function getConfigurationsForArgument(Request $request, ArgumentMetadata $argument): ConfigurationList + private function getConfigurationsForArgument(Request $request, ArgumentMetadata $argument): ?ConfigurationInterface { /** @var ConfigurationList $configurations */ $configurations = $request->attributes->get('_configurations'); @@ -38,6 +41,6 @@ private function getConfigurationsForArgument(Request $request, ArgumentMetadata return $configuration instanceof QueryParam && $configuration->getArgumentName() === $argument->getName(); }); - return $configuration; + return $configuration->first(); } } diff --git a/src/Symfony/Component/HttpKernel/Tests/Controller/ArgumentResolverTest.php b/src/Symfony/Component/HttpKernel/Tests/Controller/ArgumentResolverTest.php index 1fa9e0aae53b9..60ae38815ee66 100644 --- a/src/Symfony/Component/HttpKernel/Tests/Controller/ArgumentResolverTest.php +++ b/src/Symfony/Component/HttpKernel/Tests/Controller/ArgumentResolverTest.php @@ -290,7 +290,19 @@ public function testGetQueryParam() new QueryParam(['value' => 'foo']), new QueryParam(['value' => 'bar']), ])); - $controller = [new AnnotatedController(), 'queryParamAction']; + $controller = [new AnnotatedController(), 'queryParam']; + + $this->assertEquals(['foo', 'bar'], self::$resolver->getArguments($request, $controller)); + } + + public function testGetQueryParamWithDefaultValues() + { + $request = Request::create('/'); + $request->attributes->set('_configurations', new ConfigurationList([ + new QueryParam(['value' => 'foo']), + new QueryParam(['value' => 'bar']), + ])); + $controller = [new AnnotatedController(), 'queryParamWithDefaultValues']; $this->assertEquals(['foo', 'bar'], self::$resolver->getArguments($request, $controller)); } diff --git a/src/Symfony/Component/HttpKernel/Tests/EventListener/AnnotatedControllerListenerTest.php b/src/Symfony/Component/HttpKernel/Tests/EventListener/AnnotatedControllerListenerTest.php index e4b71186520e5..4951cc1d0745e 100644 --- a/src/Symfony/Component/HttpKernel/Tests/EventListener/AnnotatedControllerListenerTest.php +++ b/src/Symfony/Component/HttpKernel/Tests/EventListener/AnnotatedControllerListenerTest.php @@ -29,7 +29,7 @@ public function testOnController() $kernel = new KernelForTest('test', true); $event = new ControllerEvent( $kernel, - [new AnnotatedController(), 'queryParamAction'], + [new AnnotatedController(), 'queryParam'], $request, HttpKernelInterface::MASTER_REQUEST ); diff --git a/src/Symfony/Component/HttpKernel/Tests/Fixtures/Controller/AnnotatedController.php b/src/Symfony/Component/HttpKernel/Tests/Fixtures/Controller/AnnotatedController.php index 992a2f727664f..c49af01e3590e 100644 --- a/src/Symfony/Component/HttpKernel/Tests/Fixtures/Controller/AnnotatedController.php +++ b/src/Symfony/Component/HttpKernel/Tests/Fixtures/Controller/AnnotatedController.php @@ -13,7 +13,15 @@ class AnnotatedController /** * @QueryParam("bar") */ - public function queryParamAction(string $foo, string $bar): Response + public function queryParam(string $foo, string $bar): Response + { + return new Response($foo.'.'.$bar); + } + + /** + * @QueryParam("bar") + */ + public function queryParamWithDefaultValues(string $foo = 'foo', string $bar = 'bar'): Response { return new Response($foo.'.'.$bar); } diff --git a/src/Symfony/Component/HttpKernel/composer.json b/src/Symfony/Component/HttpKernel/composer.json index e60792f689190..5fbb825095427 100644 --- a/src/Symfony/Component/HttpKernel/composer.json +++ b/src/Symfony/Component/HttpKernel/composer.json @@ -26,6 +26,7 @@ "psr/log": "~1.0" }, "require-dev": { + "doctrine/annotations": "~1.7", "symfony/browser-kit": "^4.4|^5.0", "symfony/config": "^5.0", "symfony/console": "^4.4|^5.0", From 74a304493ce434c87f566953a4071200ccc2a93e Mon Sep 17 00:00:00 2001 From: tsantos Date: Thu, 19 Mar 2020 13:43:16 -0300 Subject: [PATCH 5/7] [HttpKernel] Resolving nullable arguments --- .../ArgumentResolver/QueryParamValueResolver.php | 8 ++++---- .../ControllerConfiguration/ConfigurationList.php | 2 +- .../Tests/Controller/ArgumentResolverTest.php | 12 ++++++++++++ .../Fixtures/Controller/AnnotatedController.php | 8 ++++++++ 4 files changed, 25 insertions(+), 5 deletions(-) diff --git a/src/Symfony/Component/HttpKernel/Controller/ArgumentResolver/QueryParamValueResolver.php b/src/Symfony/Component/HttpKernel/Controller/ArgumentResolver/QueryParamValueResolver.php index 412ce15674169..aea7f49380d79 100644 --- a/src/Symfony/Component/HttpKernel/Controller/ArgumentResolver/QueryParamValueResolver.php +++ b/src/Symfony/Component/HttpKernel/Controller/ArgumentResolver/QueryParamValueResolver.php @@ -18,21 +18,21 @@ public function supports(Request $request, ArgumentMetadata $argument) } /** @var QueryParam $configuration */ - $configuration = $this->getConfigurationsForArgument($request, $argument); + $configuration = $this->getConfigurationForArgument($request, $argument); return null !== $configuration && !$argument->isVariadic() - && ($request->query->has($configuration->getName()) || $argument->hasDefaultValue()); + && ($request->query->has($configuration->getName()) || $argument->hasDefaultValue() || $argument->isNullable()); } public function resolve(Request $request, ArgumentMetadata $argument) { - $configuration = $this->getConfigurationsForArgument($request, $argument); + $configuration = $this->getConfigurationForArgument($request, $argument); $defaultValue = $argument->hasDefaultValue() ? $argument->getDefaultValue() : null; yield $request->query->get($configuration->getName(), $defaultValue); } - private function getConfigurationsForArgument(Request $request, ArgumentMetadata $argument): ?ConfigurationInterface + private function getConfigurationForArgument(Request $request, ArgumentMetadata $argument): ?ConfigurationInterface { /** @var ConfigurationList $configurations */ $configurations = $request->attributes->get('_configurations'); diff --git a/src/Symfony/Component/HttpKernel/ControllerConfiguration/ConfigurationList.php b/src/Symfony/Component/HttpKernel/ControllerConfiguration/ConfigurationList.php index 04c552e144bf6..c8855df3e12b7 100644 --- a/src/Symfony/Component/HttpKernel/ControllerConfiguration/ConfigurationList.php +++ b/src/Symfony/Component/HttpKernel/ControllerConfiguration/ConfigurationList.php @@ -27,7 +27,7 @@ public function filter(callable $filter): self public function first(): ?ConfigurationInterface { - return reset($this->configurations) ?? null; + return empty($this->configurations) ? null : reset($this->configurations); } public function count(): int diff --git a/src/Symfony/Component/HttpKernel/Tests/Controller/ArgumentResolverTest.php b/src/Symfony/Component/HttpKernel/Tests/Controller/ArgumentResolverTest.php index 60ae38815ee66..e8bbb5285bf77 100644 --- a/src/Symfony/Component/HttpKernel/Tests/Controller/ArgumentResolverTest.php +++ b/src/Symfony/Component/HttpKernel/Tests/Controller/ArgumentResolverTest.php @@ -307,6 +307,18 @@ public function testGetQueryParamWithDefaultValues() $this->assertEquals(['foo', 'bar'], self::$resolver->getArguments($request, $controller)); } + public function testGetQueryParamWithNullableValues() + { + $request = Request::create('/?foo=foo'); + $request->attributes->set('_configurations', new ConfigurationList([ + new QueryParam(['value' => 'foo']), + new QueryParam(['value' => 'bar']), + ])); + $controller = [new AnnotatedController(), 'queryParamWithNullableValues']; + + $this->assertEquals(['foo', null], self::$resolver->getArguments($request, $controller)); + } + public function __invoke($foo, $bar = null) { } diff --git a/src/Symfony/Component/HttpKernel/Tests/Fixtures/Controller/AnnotatedController.php b/src/Symfony/Component/HttpKernel/Tests/Fixtures/Controller/AnnotatedController.php index c49af01e3590e..2dec7f89fd881 100644 --- a/src/Symfony/Component/HttpKernel/Tests/Fixtures/Controller/AnnotatedController.php +++ b/src/Symfony/Component/HttpKernel/Tests/Fixtures/Controller/AnnotatedController.php @@ -25,4 +25,12 @@ public function queryParamWithDefaultValues(string $foo = 'foo', string $bar = ' { return new Response($foo.'.'.$bar); } + + /** + * @QueryParam("bar") + */ + public function queryParamWithNullableValues(string $foo, ?string $bar): Response + { + return new Response($foo.'.'.$bar); + } } From f61854bc5d22a814967f0d99f980534d1fa27ceb Mon Sep 17 00:00:00 2001 From: tsantos Date: Thu, 19 Mar 2020 14:26:48 -0300 Subject: [PATCH 6/7] [HttpKernel] Preventing duplicated configurations --- .../Configuration/QueryParam.php | 5 +++++ .../ConfigurationInterface.php | 1 + .../ConfigurationList.php | 6 +++++- ...nerTest.php => ControllerListenerTest.php} | 20 +++++++++---------- .../Controller/AnnotatedController.php | 7 +++++++ 5 files changed, 28 insertions(+), 11 deletions(-) rename src/Symfony/Component/HttpKernel/Tests/EventListener/{AnnotatedControllerListenerTest.php => ControllerListenerTest.php} (72%) diff --git a/src/Symfony/Component/HttpKernel/ControllerConfiguration/Configuration/QueryParam.php b/src/Symfony/Component/HttpKernel/ControllerConfiguration/Configuration/QueryParam.php index 8304e98911155..e383dda6ef3e9 100644 --- a/src/Symfony/Component/HttpKernel/ControllerConfiguration/Configuration/QueryParam.php +++ b/src/Symfony/Component/HttpKernel/ControllerConfiguration/Configuration/QueryParam.php @@ -43,4 +43,9 @@ public function getArgumentName() { return $this->argumentName; } + + public function getUniqueName(): string + { + return static::class.'.'.$this->argumentName; + } } diff --git a/src/Symfony/Component/HttpKernel/ControllerConfiguration/ConfigurationInterface.php b/src/Symfony/Component/HttpKernel/ControllerConfiguration/ConfigurationInterface.php index 91dd74adda93d..b3d716989eea3 100644 --- a/src/Symfony/Component/HttpKernel/ControllerConfiguration/ConfigurationInterface.php +++ b/src/Symfony/Component/HttpKernel/ControllerConfiguration/ConfigurationInterface.php @@ -4,4 +4,5 @@ interface ConfigurationInterface { + public function getUniqueName(): string; } diff --git a/src/Symfony/Component/HttpKernel/ControllerConfiguration/ConfigurationList.php b/src/Symfony/Component/HttpKernel/ControllerConfiguration/ConfigurationList.php index c8855df3e12b7..df3ff72a9df01 100644 --- a/src/Symfony/Component/HttpKernel/ControllerConfiguration/ConfigurationList.php +++ b/src/Symfony/Component/HttpKernel/ControllerConfiguration/ConfigurationList.php @@ -15,7 +15,11 @@ public function __construct(array $configurations = []) public function add(ConfigurationInterface $configuration): self { - $this->configurations[] = $configuration; + if (isset($this->configurations[$configuration->getUniqueName()])) { + throw new \LogicException(sprintf('Multiples "%s" configurations are not allowed', $configuration->getUniqueName())); + } + + $this->configurations[$configuration->getUniqueName()] = $configuration; return $this; } diff --git a/src/Symfony/Component/HttpKernel/Tests/EventListener/AnnotatedControllerListenerTest.php b/src/Symfony/Component/HttpKernel/Tests/EventListener/ControllerListenerTest.php similarity index 72% rename from src/Symfony/Component/HttpKernel/Tests/EventListener/AnnotatedControllerListenerTest.php rename to src/Symfony/Component/HttpKernel/Tests/EventListener/ControllerListenerTest.php index 4951cc1d0745e..08252e9e694e9 100644 --- a/src/Symfony/Component/HttpKernel/Tests/EventListener/AnnotatedControllerListenerTest.php +++ b/src/Symfony/Component/HttpKernel/Tests/EventListener/ControllerListenerTest.php @@ -11,7 +11,7 @@ use Symfony\Component\HttpKernel\Tests\Fixtures\Controller\AnnotatedController; use Symfony\Component\HttpKernel\Tests\Fixtures\KernelForTest; -class AnnotatedControllerListenerTest extends TestCase +class ControllerListenerTest extends TestCase { private $listener; private $reader; @@ -24,15 +24,8 @@ protected function setUp(): void public function testOnController() { - $request = new Request(); - - $kernel = new KernelForTest('test', true); - $event = new ControllerEvent( - $kernel, - [new AnnotatedController(), 'queryParam'], - $request, - HttpKernelInterface::MASTER_REQUEST - ); + $event = $this->createControllerEvent([new AnnotatedController(), 'queryParam']); + $request = $event->getRequest(); $this->listener->onController($event); @@ -40,6 +33,13 @@ public function testOnController() $this->assertCount(2, $request->attributes->get('_configurations')); } + public function testOnControllerWithDuplicatedQueryParam() + { + $this->expectException(\LogicException::class); + $event = $this->createControllerEvent([new AnnotatedController(), 'duplicatedQueryParamConfiguration']); + $this->listener->onController($event); + } + private function createControllerEvent(callable $controller): ControllerEvent { $kernel = new KernelForTest('test', true); diff --git a/src/Symfony/Component/HttpKernel/Tests/Fixtures/Controller/AnnotatedController.php b/src/Symfony/Component/HttpKernel/Tests/Fixtures/Controller/AnnotatedController.php index 2dec7f89fd881..a0b62165fcbbc 100644 --- a/src/Symfony/Component/HttpKernel/Tests/Fixtures/Controller/AnnotatedController.php +++ b/src/Symfony/Component/HttpKernel/Tests/Fixtures/Controller/AnnotatedController.php @@ -33,4 +33,11 @@ public function queryParamWithNullableValues(string $foo, ?string $bar): Respons { return new Response($foo.'.'.$bar); } + + /** + * @QueryParam("foo") + */ + public function duplicatedQueryParamConfiguration(string $foo): void + { + } } From 12576909697d4028f6748aec5191740876f63503 Mon Sep 17 00:00:00 2001 From: tsantos Date: Thu, 19 Mar 2020 15:19:10 -0300 Subject: [PATCH 7/7] [HttpKernel] Moved classes to existing namespace --- .../Controller/ArgumentResolver/QueryParamValueResolver.php | 6 +++--- .../Configuration/ConfigurationAnnotation.php | 4 +--- .../Configuration}/ConfigurationInterface.php | 2 +- .../Configuration/QueryParam.php | 2 +- .../ConfigurationList.php | 4 +++- .../HttpKernel/EventListener/ControllerListener.php | 4 ++-- .../HttpKernel/Tests/Controller/ArgumentResolverTest.php | 4 ++-- .../Tests/Fixtures/Controller/AnnotatedController.php | 2 +- 8 files changed, 14 insertions(+), 14 deletions(-) rename src/Symfony/Component/HttpKernel/{ControllerConfiguration => Controller}/Configuration/ConfigurationAnnotation.php (72%) rename src/Symfony/Component/HttpKernel/{ControllerConfiguration => Controller/Configuration}/ConfigurationInterface.php (58%) rename src/Symfony/Component/HttpKernel/{ControllerConfiguration => Controller}/Configuration/QueryParam.php (92%) rename src/Symfony/Component/HttpKernel/{ControllerConfiguration => Controller}/ConfigurationList.php (89%) diff --git a/src/Symfony/Component/HttpKernel/Controller/ArgumentResolver/QueryParamValueResolver.php b/src/Symfony/Component/HttpKernel/Controller/ArgumentResolver/QueryParamValueResolver.php index aea7f49380d79..1deeb0248c257 100644 --- a/src/Symfony/Component/HttpKernel/Controller/ArgumentResolver/QueryParamValueResolver.php +++ b/src/Symfony/Component/HttpKernel/Controller/ArgumentResolver/QueryParamValueResolver.php @@ -4,9 +4,9 @@ use Symfony\Component\HttpFoundation\Request; use Symfony\Component\HttpKernel\Controller\ArgumentValueResolverInterface; -use Symfony\Component\HttpKernel\ControllerConfiguration\Configuration\QueryParam; -use Symfony\Component\HttpKernel\ControllerConfiguration\ConfigurationInterface; -use Symfony\Component\HttpKernel\ControllerConfiguration\ConfigurationList; +use Symfony\Component\HttpKernel\Controller\Configuration\ConfigurationInterface; +use Symfony\Component\HttpKernel\Controller\Configuration\QueryParam; +use Symfony\Component\HttpKernel\Controller\ConfigurationList; use Symfony\Component\HttpKernel\ControllerMetadata\ArgumentMetadata; final class QueryParamValueResolver implements ArgumentValueResolverInterface diff --git a/src/Symfony/Component/HttpKernel/ControllerConfiguration/Configuration/ConfigurationAnnotation.php b/src/Symfony/Component/HttpKernel/Controller/Configuration/ConfigurationAnnotation.php similarity index 72% rename from src/Symfony/Component/HttpKernel/ControllerConfiguration/Configuration/ConfigurationAnnotation.php rename to src/Symfony/Component/HttpKernel/Controller/Configuration/ConfigurationAnnotation.php index 85595b08030de..0f7741ad8f932 100644 --- a/src/Symfony/Component/HttpKernel/ControllerConfiguration/Configuration/ConfigurationAnnotation.php +++ b/src/Symfony/Component/HttpKernel/Controller/Configuration/ConfigurationAnnotation.php @@ -1,8 +1,6 @@