Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork9.7k
[HttpKernel] Adds @QueryParam annotation#36135
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
nicolas-grekas left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
I see a lot in common with the framework-extra-bundle here, which is nice, but can't we do some extra miles and replace the runtime logic inControllerListener by a compile-time one?
This would require using a tag to identify controllers. We could reusecontroller.service_arguments or add a newkernel.controller (that could maybe replacecontroller.service_arguments btw)
WDYT?
tsantos84 commentedMar 19, 2020
Let me see if I understand. What you're proposing is a way to fetch data from a compiled container instead of a dedicated |
nicolas-grekas commentedMar 19, 2020
Yes, something like that :) |
linaori commentedMar 20, 2020
I would recommend a different tag name, I'm not using service arguments for my controllers, and I'd still like to use this feature 😄 |
tsantos84 commentedApr 4, 2020 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
Hi@nicolas-grekas, I've thought a lot how could we leverage the compilation mechanism to go ahead with this feature. One possible solution would be make the generated container implement 1. Container implementing |
tsantos84 commentedApr 4, 2020
Thinking about tags for controllers, what if create a compile pass to read controllers from routing config? We wouldn't need to create a new tag nor use the existing one. Actually, the existing one could be deprecated. |
nicolas-grekas commentedApr 4, 2020
There's a possible link between this PR and#36320 About parsing route during compilation, that's orthogonal to the way things are designed today: routes are loadedafter the container is compiled. Not sure it's worth pursuing. |
tsantos84 commentedApr 17, 2020
Ok and agree considering the Symfony's design. My opinion about this is that the framework - in a general aspect - already knows what controllers are configured and devs wouldn't need to configure it again through tags. But this subject is not part of this PR and I don't want to discuss here to not change the main scope of this feature. About the implementation of this feature, which way should we go?@nicolas-grekas, could you readthis and explain if this is what you thoughthere? |
piku235 commentedJul 4, 2020
I've managed with dropping the annotation controller listener and extracting it to the compiler pass. I decided to compile annotations in the service container, as they are holders of information needed to process the request, and also some validation can already occur at the compile-time. Now regarding the PR itself - the Regarding what@nicolas-grekas wrote in#36037 (comment)
IMO starting it only with the WDYT? |
fabpot commentedSep 12, 2020
Closing as#37829 introduced a simpler way to implement such attributes. Implementing a simple RequestQuery attribute could be something like: diff --git a/src/Symfony/Component/HttpKernel/Attribute/RequestQuery.php b/src/Symfony/Component/HttpKernel/Attribute/RequestQuery.phpnew file mode 100644index 0000000000..133c5457ae--- /dev/null+++ b/src/Symfony/Component/HttpKernel/Attribute/RequestQuery.php@@ -0,0 +1,22 @@+<?php++/*+ * This file is part of the Symfony package.+ *+ * (c) Fabien Potencier <fabien@symfony.com>+ *+ * For the full copyright and license information, please view the LICENSE+ * file that was distributed with this source code.+ */++namespace Symfony\Component\HttpKernel\Attribute;++use Attribute;++/**+ * Gets the request query parameter of the same name as the argument name.+ */+#[Attribute(Attribute::TARGET_PARAMETER)]+class RequestQuery implements ArgumentInterface+{+}diff --git a/src/Symfony/Component/HttpKernel/Controller/ArgumentResolver.php b/src/Symfony/Component/HttpKernel/Controller/ArgumentResolver.phpindex 4285ba7631..e78e07c8c6 100644--- a/src/Symfony/Component/HttpKernel/Controller/ArgumentResolver.php+++ b/src/Symfony/Component/HttpKernel/Controller/ArgumentResolver.php@@ -14,6 +14,7 @@ namespace Symfony\Component\HttpKernel\Controller; use Symfony\Component\HttpFoundation\Request; use Symfony\Component\HttpKernel\Controller\ArgumentResolver\DefaultValueResolver; use Symfony\Component\HttpKernel\Controller\ArgumentResolver\RequestAttributeValueResolver;+use Symfony\Component\HttpKernel\Controller\ArgumentResolver\RequestQueryResolver; use Symfony\Component\HttpKernel\Controller\ArgumentResolver\RequestValueResolver; use Symfony\Component\HttpKernel\Controller\ArgumentResolver\SessionValueResolver; use Symfony\Component\HttpKernel\Controller\ArgumentResolver\VariadicValueResolver;@@ -89,6 +90,7 @@ final class ArgumentResolver implements ArgumentResolverInterface new RequestAttributeValueResolver(), new RequestValueResolver(), new SessionValueResolver(),+ new RequestQueryResolver(), new DefaultValueResolver(), new VariadicValueResolver(), ];diff --git a/src/Symfony/Component/HttpKernel/Controller/ArgumentResolver/RequestQueryResolver.php b/src/Symfony/Component/HttpKernel/Controller/ArgumentResolver/RequestQueryResolver.phpnew file mode 100644index 0000000000..f68570c40e--- /dev/null+++ b/src/Symfony/Component/HttpKernel/Controller/ArgumentResolver/RequestQueryResolver.php@@ -0,0 +1,39 @@+<?php++/*+ * This file is part of the Symfony package.+ *+ * (c) Fabien Potencier <fabien@symfony.com>+ *+ * For the full copyright and license information, please view the LICENSE+ * file that was distributed with this source code.+ */++namespace Symfony\Component\HttpKernel\Controller\ArgumentResolver;++use Symfony\Component\HttpFoundation\Request;+use Symfony\Component\HttpKernel\Attribute\RequestQuery;+use Symfony\Component\HttpKernel\Controller\ArgumentValueResolverInterface;+use Symfony\Component\HttpKernel\ControllerMetadata\ArgumentMetadata;++/**+ * @author Fabien Potencier <fabien@symfony.com>+ */+final class RequestQueryResolver implements ArgumentValueResolverInterface+{+ /**+ * {@inheritdoc}+ */+ public function supports(Request $request, ArgumentMetadata $argument): bool+ {+ return $argument->getAttribute() instanceof RequestQuery;+ }++ /**+ * {@inheritdoc}+ */+ public function resolve(Request $request, ArgumentMetadata $argument): iterable+ {+ yield $request->query->all()[$argument->getName()] ?? $argument->hasDefaultValue() ? $argument->getDefaultValue() : null;+ }+} |
fabpot commentedSep 12, 2020
Let's resume the discussion in#38162 |
Uh oh!
There was an error while loading.Please reload this page.
As discussed on#36037, this PR adds the annotation
@QueryParamto map query string in controller arguments. In addition, it allows to validate parameters usingSymfony Validatorand throwBadRequestExceptionautomatically in case of any constraint violation. In practical terms, this PR allows that a code like this:Can be written like this:
Assumptions
Some assumptions were made until now and can be changed during the development:
Symfony HttpKernelcomponent as it has already classes to deal with controllers and argument resolving.Symfony Validatorrequires add manuallysymfony/validatorpackage to yourcomposer.jsonDoubts
Future
@RequestBody,@RequestCookieand etc.