[HttpKernel] Create Attributes `#[MapRequestPayload]` and `#[MapQueryString]` to map Request input to typed objects by Koc · Pull Request #49138 · symfony/symfony

Koc · 2023-01-27T21:01:14Z

Q	A
Branch?	6.3
Bug fix?	no
New feature?	yes
Deprecations?	no
Tickets	#36037, #36093, #45628, #47425, #49002, #49134
License	MIT
Doc PR	TBD

Yet another variation of how we can map raw Request data to typed objects with validation. We can even build OpenApi Specification based on this DTO classes using NelmioApiDocBundle.

Usage Example 🔧

`#[MapRequestPayload]`

class PostProductReviewPayload
{
    public function __construct(
        #[Assert\NotBlank]
        #[Assert\Length(min: 10, max: 500)]
        public readonly string $comment,
        #[Assert\GreaterThanOrEqual(1)]
        #[Assert\LessThanOrEqual(5)]
        public readonly int $rating,
    ) {
    }
}

class PostJsonApiController
{
    public function __invoke(
        #[MapRequestPayload] PostProductReviewPayload $payload,
    ): Response {
        // $payload is validated and fully typed representation of the request body $request->getContent() 
        //  or $request->request->all()
    }
}

`#[MapQueryString]`

class GetOrdersQuery
{
    public function __construct(
        #[Assert\Valid]
        public readonly ?GetOrdersFilter $filter,
        #[Assert\LessThanOrEqual(500)]
        public readonly int $limit = 25,
        #[Assert\LessThanOrEqual(10_000)]
        public readonly int $offset = 0,
    ) {
    }
}

class GetOrdersFilter
{
    public function __construct(
        #[Assert\Choice(['placed', 'shipped', 'delivered'])]
        public readonly ?string $status,
        public readonly ?float $total,
    ) {
    }
}

class GetApiController
{
    public function __invoke(
        #[MapQueryString] GetOrdersQuery $query,
    ): Response {
        // $query is validated and fully typed representation of the query string $request->query->all()
    }
}

Exception handling 💥

Validation errors will result in an HTTP 422 response, accompanied by a serialized ConstraintViolationList.
Malformed data will be responded to with an HTTP 400 error.
Unsupported deserialization formats will be responded to with an HTTP 415 error.

Comparison to another implementations 📑

Differences to #49002:

separate Attributes for explicit definition of the used source
no need to define which class use to map because Attributes already associated with typed argument
used ArgumentValueResolver mechanism instead of Subscribers
functional tests

Differences to #49134:

it is possible to map whole query string to object, not per parameter
there is validation of the mapped object
supports both $request->getContent() and $request->request->all() mapping
functional tests

Differences to #45628:

separate Attributes for explicit definition of the used source
supports $request->request->all() and $request->query->all() mapping
Exception handling opt-in
functional tests

Bonus part 🎁

Extracted UnsupportedFormatException which thrown when there is no decoder for a given format

OskarStark · 2023-01-28T07:27:22Z

It looks like your committer email is not associated with your Github account

ruudk · 2023-01-28T08:39:22Z

Was this a coincidence that we both did something similar or did you create this afterwards?

Koc · 2023-01-28T10:00:23Z

@OskarStark thanx, fixed now, please check

@ruudk it is similar but has few differences comparing to your implementation. I've updated PR description to highlight the differences.

BTW anybody know how to fix unrelated fabbot failure?

ro0NL · 2023-01-28T12:16:35Z

this looks promising :)

there is validation of the mapped object

what about deserializing errors vs. ValidationFailedException? Does either produce a 400 bad request? is only the latter i18n friendly?

Koc · 2023-01-28T18:35:04Z

My idea was start from something like simple implementation in this PR and improve it later before 6.3 release.

Add generic Exception Handler which will catch ValidationFailedException and convert it to http 400 with serialized Constraint Violations. We already have Normalizer for it but anyway it requires some effort.
Handle PartialDenormalizationException in this two Argument Value Resolvers and convert it to ValidationFailedException. We need somehow translate messages about missmatched types

ro0NL · 2023-01-28T18:49:08Z

i like a simple implementation as first iteration, but i like http/i18n compatibility more ;)

y4roc · 2023-01-30T09:15:20Z

Your PR also makes this one #47425 obsolete.

y4roc · 2023-01-30T10:44:23Z

I would use a separate exception, which builds on the ValidationFailedException. So that your listener does not filter out other ValidationFailedException.

faizanakram99 · 2023-04-07T12:32:37Z

Is the naming of attributes final, MapRequestPayload seems a bit mouthful.

What about names suggested here (FromQuery, etc) #49138 (comment)

renanbr · 2023-04-07T13:00:10Z

Landing late to the discussion :hi:

This PR overlaps some work I participate currently, so here are some feedbacks:

I would rename MapRequestPayload to MapRequestBody because it targets specifically the content (not uploaded files e.g.)
It would be great if users could configure the format they want to receive data
- I didn't get why the proposal uses ->getRequestFormat() to detect the format and not the content-type header
- I think the resolver should throw a proper exception for any unexpected format
It would be great if users could pick the validation group they want to apply, or even disable it if they want
It would be great if users could define presets (serialization format + serialization context + validation groups) and let users use them through their endpoints

Most of these ideas are implemented in https://github.com/acsiomatic/http-payload-bundle (ping @renedelima)

All of them can be done in further PR.

I think the current PR presents no blockers regarding these ideas in this comment.

nicolas-grekas · 2023-04-07T13:13:18Z

What about names suggested here (FromQuery, etc)

We have a convention to help discovery that controller-related attributes should start with Map. I submitted symfony/symfony-docs#18176 to clarify this.

rename MapRequestPayload to MapRequestBody

"body" is not a word we use in the request object so this would be MapRequestContent. It's an open topic no my side. "payload" is also new vocabulary so we might prefer "content", which is a term we use already.

targets specifically the content (not uploaded files e.g.)

Which leads to the idea of adding #[MapUploadedFile] ;)

It would be great if users could define presets

My suggestion for this: make the attribute non final. Then a preset is just a child attribute. We do something similar for DI-attributes and it works great.

I left your other questions open for someone else :)

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

Koc · 2023-04-07T16:54:28Z

Mates, can we finally agree on naming, please? 🙏 I've renamed it few times already: MapRequestContent -> MapRequestBody -> MapRequestPayload

Koc · 2023-04-07T17:01:13Z

I think the resolver should throw a proper exception for any unexpected format

@renanbr Have you seen Functional Tests which checks for 415 Unsupported Media Type?

Make class non-final is always easier than vise versa. Let's collect more feedback about all possible "child" implementation first and then remove final flag?

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

nicolas-grekas · 2023-04-11T08:10:05Z

Make class non-final is always easier than vise versa. Let's collect more feedback about all possible "child" implementation first and then remove final flag?

I'd prefer removing the final now. We do have a use case already, and attributes are not good candidates for final types (they don't define abstractions but semantics, and inheritance is the way to extend semantics.)

nicolas-grekas

please check tests, some failures look related to me

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

...y/Component/HttpKernel/Tests/Controller/ArgumentResolver/RequestPayloadValueResolverTest.php

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

…String]` to map Request input to typed objects

nicolas-grekas

I just made some minor and final changes:
https://github.com/symfony/symfony/compare/50c06d123c1c0539a1e47cb202bce6d430247ff7..d987093a538c70fc4a2d01b046913a2820341699

nicolas-grekas · 2023-04-14T10:12:47Z

Thank you @Koc.

OskarStark · 2023-04-14T10:23:47Z

and a big thank you! I really appreciate this new feature! 🎉

nicolas-grekas · 2023-04-14T10:50:39Z

Note that if there are remaining topics to discuss (naming?), we're still in feature freeze so we can do any changes!

Koc · 2023-04-14T12:18:58Z

@nicolas-grekas thanx! I'm fine with MapRequestPayload as it merged or MapRequestContent as it was at the very beginning.

carsonbot added Status: Needs Review Feature labels Jan 27, 2023

carsonbot added this to the 6.3 milestone Jan 27, 2023

Koc force-pushed the feature/mapped-request branch 5 times, most recently from 87eb66c to a08be6b Compare January 27, 2023 21:29

Koc changed the title ~~Create Attributes to map Query String and Request Content to typed objects~~ [HttpKernel] Create Attributes #[MapQueryString and #[MapRequestContent] to map Request input to typed objects Jan 27, 2023

Koc changed the title ~~[HttpKernel] Create Attributes #[MapQueryString and #[MapRequestContent] to map Request input to typed objects~~ [HttpKernel] Create Attributes #[MapQueryString] and #[MapRequestContent] to map Request input to typed objects Jan 27, 2023

Koc force-pushed the feature/mapped-request branch 4 times, most recently from 078781e to b3bbe43 Compare January 27, 2023 22:08

Koc force-pushed the feature/mapped-request branch from b3bbe43 to 4e8cb8e Compare January 28, 2023 09:38

Koc force-pushed the feature/mapped-request branch from 4e8cb8e to 1b619a2 Compare January 28, 2023 18:40

Koc force-pushed the feature/mapped-request branch 3 times, most recently from 19069b1 to 052f625 Compare January 28, 2023 21:35

Koc force-pushed the feature/mapped-request branch from 052f625 to 25e7d70 Compare January 30, 2023 10:31

Koc force-pushed the feature/mapped-request branch 2 times, most recently from cf31004 to a2b5f96 Compare January 30, 2023 11:29

fancyweb suggested changes Apr 7, 2023

View reviewed changes

src/Symfony/Component/HttpKernel/Controller/ArgumentResolver/RequestPayloadValueResolver.php Outdated Show resolved Hide resolved

renedelima mentioned this pull request Apr 8, 2023

[HttpKernel] Introduce #[MapUploadedFile] controller argument attribute #49978

Merged

nicolas-grekas reviewed Apr 11, 2023

View reviewed changes

src/Symfony/Component/HttpKernel/Controller/ArgumentResolver/RequestPayloadValueResolver.php Outdated Show resolved Hide resolved

nicolas-grekas reviewed Apr 11, 2023

View reviewed changes

src/Symfony/Component/HttpKernel/Controller/ArgumentResolver/RequestPayloadValueResolver.php Outdated Show resolved Hide resolved

nicolas-grekas reviewed Apr 11, 2023

View reviewed changes

...y/Component/HttpKernel/Tests/Controller/ArgumentResolver/RequestPayloadValueResolverTest.php Outdated Show resolved Hide resolved

nicolas-grekas reviewed Apr 11, 2023

View reviewed changes

...y/Component/HttpKernel/Tests/Controller/ArgumentResolver/RequestPayloadValueResolverTest.php Outdated Show resolved Hide resolved

GromNaN reviewed Apr 11, 2023

View reviewed changes

src/Symfony/Component/HttpKernel/Controller/ArgumentResolver/RequestPayloadValueResolver.php Show resolved Hide resolved

[HttpKernel] Create Attributes #[MapRequestPayload] and `#[MapQuery…

d987093

…String]` to map Request input to typed objects

nicolas-grekas approved these changes Apr 14, 2023

View reviewed changes

nicolas-grekas mentioned this pull request Apr 14, 2023

[HttpKernel] Create Attributes #[MapRequestPayload] and #[MapQueryString] to map Request input to typed objects symfony/symfony-docs#18206

Closed

OskarStark mentioned this pull request Apr 14, 2023

[Validator] [WIP] Validate controller arguments using constraints #47425

Closed

5 tasks

This was referenced Apr 14, 2023

[Validator] Attribute can not be applied on Controller method argument #43958

Closed

[HTTPFoundation] Get param or throw BadRequestHttpException #47575

Closed

This was referenced Apr 16, 2023

[HttpKernel] Enhance MapRequestPayload adding format and validation group #50029

Merged

[HttpKernel] Enhance MapQueryString adding validation group #50082

Merged

Korbeil mentioned this pull request Apr 22, 2023

Add AutoMapper component Korbeil/symfony#1

Open

nicolas-grekas mentioned this pull request Apr 23, 2023

[HttpKernel] #[MapRequestPayload] is "handled" before #[IsGranted] #50120

Closed

This was referenced Jun 22, 2023

[Router] Validate Request in Router #48908

Closed

RequestDataObject as in java spring #46991

Closed

ayyoub-afwallah mentioned this pull request Jan 17, 2026

Add check for commit attribution symfony-tools/fabbot#15

Open

Uh oh!

Conversation

Koc commented Jan 27, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Usage Example 🔧

#[MapRequestPayload]

#[MapQueryString]

Exception handling 💥

Comparison to another implementations 📑

Bonus part 🎁

Uh oh!

OskarStark commented Jan 28, 2023

Uh oh!

ruudk commented Jan 28, 2023

Uh oh!

Koc commented Jan 28, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ro0NL commented Jan 28, 2023

Uh oh!

Koc commented Jan 28, 2023

Uh oh!

ro0NL commented Jan 28, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

y4roc commented Jan 30, 2023

Uh oh!

y4roc commented Jan 30, 2023

Uh oh!

faizanakram99 commented Apr 7, 2023

Uh oh!

renanbr commented Apr 7, 2023

Uh oh!

nicolas-grekas commented Apr 7, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Koc commented Apr 7, 2023

Uh oh!

Koc commented Apr 7, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

nicolas-grekas commented Apr 11, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nicolas-grekas left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

nicolas-grekas left a comment

Choose a reason for hiding this comment

Uh oh!

nicolas-grekas commented Apr 14, 2023

Uh oh!

OskarStark commented Apr 14, 2023

Uh oh!

nicolas-grekas commented Apr 14, 2023

Uh oh!

Koc commented Apr 14, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

20 participants

Koc commented Jan 27, 2023 •

edited

Loading

`#[MapRequestPayload]`

`#[MapQueryString]`

Koc commented Jan 28, 2023 •

edited

Loading

ro0NL commented Jan 28, 2023 •

edited

Loading

nicolas-grekas commented Apr 7, 2023 •

edited

Loading

Koc commented Apr 7, 2023 •

edited

Loading

nicolas-grekas commented Apr 11, 2023 •

edited

Loading