[HttpKernel] Add RequestQuery attribute

[fabpot]

Q	A
Branch?	master
Bug fix?	no
New feature?	yes
Deprecations?	no
Tickets	refs #36135
License	MIT
Doc PR

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).

[piku235]

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 Request instance. It may happen that one action uses only query parameters, so RequestQuery can be easily used, but another action in the same controller will use an entire Request instance to eg deserialize the request message body. From the pure controller interface perspective, it doesn't look so good. The great add of this annotation/attribute style is that they refrain from using an entire Request instance where the context is bigger than needed and parameters are unknown, so there's a need to check them yourself before using.

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.

[derrabus]

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: http://example.com/api/search?q=some+search+term&page=2

public function searchAction(
    #[RequestQuery] string $q,
    #[RequestQuery] int $page = 1
) {

In this case, we could already perform the following validations:

• Query parameter q is missing: The parameter is not nullable and the action does not define a default value => error.
• page=foo: Not a valid integer => error.

We could for instance throw a BadRequestHttpException in such a case.

[fabpot]

We've just moved away from master as the main branch to use 5.x instead. Unfortunately, I cannot reopen the PR and change the target branch to 5.x. Can you open a new PR referencing this one to not loose the discussion? Thank you for your understanding and for your help.

[rrajkomar]

Hello,
Just wondering what is the state of this PR ?
Has another PR been created and not been linked or has this feature been abandoned ?
Thanks.

[derrabus]

I think, this PR was merely meant as a PoC. At least I would want to pick up this topic for Symfony 5.3. 🙃