-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
[HttpKernel] Add RequestQuery attribute #38162
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Hi @fabpot, thanks for continuing the discussion about request attributes. The whole discussion actually started from #36037. To quickly summarize it, the feelings about the request annotations/attributes were quite mixed. In my opinion, they're a great addition, I myself I've been using them for more than a year already. I published not so long ago a stable version of my bundle JungiFrameworkExtraBundle that contains RequestBody, QueryParam, RequestParam and more. For now, it contains only annotations, but I've finished working on attributes and I plan soon to publish them. I used a different way of accessing annotations/attributes in a request-time, instead of the lastest feature #37829, I used a method that @nicolas-grekas mentioned in #36135 (review). Thanks to that, I didn't lose on performance in a request-time, and also could perform additional validation in the compile-time (especially for annotations). As I wrote before in #36135 (comment), my only worry with publishing only RequestQuery (before QueryParam) is it may pollute a controller interface in some cases, it introduces a mixed style - using attributes exchangeably with a In my opinion, if we really want to have it in the core, it'll be good to publish RequestQuery with RequestBody attribute. Trough my experience, I've been using the most RequestBody and sometimes QueryParam, they should cover most of the use cases in a controller. |
I'm currently working on a project where the API makes heavy use of query parameters. To me, an attribute like this would really make sense if it is combined with input validation and error handling, so that a request with an invalid or missing mandatory query parameter won't event hit the controller. This way, I could remove a lot of redundant boilerplate. And I don't think that we would necessarily need constraints for that. Parameter types could also indicate validation. Take for example a URL like this: public function searchAction(
#[RequestQuery] string $q,
#[RequestQuery] int $page = 1
) { In this case, we could already perform the following validations:
We could for instance throw a |
We've just moved away from |
Hello, |
I think, this PR was merely meant as a PoC. At least I would want to pick up this topic for Symfony 5.3. 🙃 |
Implements a RequestQuery attribute (see #37829 for the feature) like described in #36135. As we don't have yet constraint attributes (#38096), I've submitted the PR to start a discussion (not even sure we want to support it in core).