Skip to content

[HttpKernel] Create Attributes #[MapRequestPayload] and #[MapQueryString] to map Request input to typed objects #49138

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

Merged
merged 1 commit into from
Apr 14, 2023

Conversation

Koc
Copy link
Contributor

@Koc Koc commented Jan 27, 2023

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

@carsonbot carsonbot added this to the 6.3 milestone Jan 27, 2023
@Koc Koc force-pushed the feature/mapped-request branch 5 times, most recently from 87eb66c to a08be6b Compare January 27, 2023 21:29
@Koc 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 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 Koc force-pushed the feature/mapped-request branch 4 times, most recently from 078781e to b3bbe43 Compare January 27, 2023 22:08
@OskarStark
Copy link
Contributor

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

@ruudk
Copy link
Contributor

ruudk commented Jan 28, 2023

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

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

Koc commented Jan 28, 2023

@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
Copy link
Contributor

ro0NL commented Jan 28, 2023

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
Copy link
Contributor Author

Koc commented Jan 28, 2023

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

  1. 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.
  2. Handle PartialDenormalizationException in this two Argument Value Resolvers and convert it to ValidationFailedException. We need somehow translate messages about missmatched types

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

ro0NL commented Jan 28, 2023

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

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

y4roc commented Jan 30, 2023

Your PR also makes this one #47425 obsolete.

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

y4roc commented Jan 30, 2023

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

@Koc Koc force-pushed the feature/mapped-request branch 2 times, most recently from cf31004 to a2b5f96 Compare January 30, 2023 11:29
@Koc Koc force-pushed the feature/mapped-request branch 4 times, most recently from f18decf to 50c06d1 Compare April 13, 2023 20:13
@nicolas-grekas nicolas-grekas force-pushed the feature/mapped-request branch from 50c06d1 to d3bdaea Compare April 14, 2023 10:06
…String]` to map Request input to typed objects
@nicolas-grekas nicolas-grekas force-pushed the feature/mapped-request branch from d3bdaea to d987093 Compare April 14, 2023 10:11
Copy link
Member

@nicolas-grekas nicolas-grekas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nicolas-grekas
Copy link
Member

Thank you @Koc.

@OskarStark
Copy link
Contributor

giphy

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

@nicolas-grekas
Copy link
Member

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

@Koc
Copy link
Contributor Author

Koc commented Apr 14, 2023

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

nicolas-grekas added a commit that referenced this pull request Apr 20, 2023
…nd validation group (renanbr)

This PR was merged into the 6.3 branch.

Discussion
----------

[HttpKernel] Enhance MapRequestPayload adding format and validation group

| Q             | A
| ------------- | ---
| Branch?       | 6.3
| Bug fix?      | no
| New feature?  | yes
| Deprecations? | no
| Tickets       | Implements #49138 (comment)
| License       | MIT
| Doc PR        | not yet

This PR enhances `MapRequestPayload` (merged via #49138) by allowing users to restrict format and pick validation groups.

```php
# src/Controller/HelloController.php

namespace App\Controller;

use Symfony\Component\HttpFoundation\Response;
use Symfony\Component\HttpKernel\Attribute\AsController;
use Symfony\Component\HttpKernel\Attribute\MapRequestPayload;
use Symfony\Component\Routing\Annotation\Route;
use Symfony\Component\Validator\Constraints as Assert;

#[AsController]
final class HelloController
{
    #[Route('/hello', methods: ['POST'])]
    public function __invoke(
        #[MapRequestPayload(acceptFormat: 'json', validationGroups: 'strict')] MyObject $object,
    ): Response {
        // framework will...
        //     1. deserialized HTTP content from json, and throws exception in case of mismatch
        //     2. validate ONLY constraint in the "strict" group, and throws exception in case of invalidation
    }
}
```

Commits
-------

068fb4d [HttpKernel] Enhance MapRequestPayload adding format and validation group
fabpot added a commit that referenced this pull request Apr 21, 2023
…group (renanbr)

This PR was merged into the 6.3 branch.

Discussion
----------

[HttpKernel] Enhance MapQueryString adding validation group

| Q             | A
| ------------- | ---
| Branch?       | 6.3
| Bug fix?      | no
| New feature?  | yes
| Deprecations? | no
| Tickets       | continuity of #50029 and #49138
| License       | MIT
| Doc PR        | n/a

- Introduces `$validationGroups` property to the `MapQueryString` attribute to work as same as in `MapRequestPayload`
- Renames `MapQueryString` property `$context` to `$serializationContext` to harmonize with `MapRequestPayload`

```php
# src/Controller/HelloController.php

namespace App\Controller;

use Symfony\Component\HttpFoundation\Response;
use Symfony\Component\HttpKernel\Attribute\AsController;
use Symfony\Component\HttpKernel\Attribute\MapQueryString;
use Symfony\Component\Routing\Annotation\Route;

#[AsController]
final class HelloController
{
    #[Route('/hello', methods: ['GET'])]
    public function __invoke(
        #[MapQueryString(validationGroups: 'strict')] MyObject $object,
    ): Response {
        // ...
    }
}
```

Commits
-------

8aad8b9 [HttpKernel] Enhance MapQueryString adding validation group
chalasr added a commit that referenced this pull request Apr 18, 2024
… argument attribute (renedelima)

This PR was merged into the 7.1 branch.

Discussion
----------

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

| Q             | A
| ------------- | ---
| Branch?       | 7.1
| Bug fix?      | no
| New feature?  | yes
| Deprecations? | no
| Tickets       | #52678, #49138
| License       | MIT
| Doc PR        | -

## Usage Example

```php
# src/Controller/UserPictureController.php

namespace App\Controller;

use Symfony\Component\HttpFoundation\File\UploadedFile;
use Symfony\Component\HttpFoundation\Response;
use Symfony\Component\HttpKernel\Attribute\AsController;
use Symfony\Component\HttpKernel\Attribute\MapUploadedFile;
use Symfony\Component\Routing\Annotation\Route;
use Symfony\Component\Validator\Constraints as Assert;

#[AsController]
final class UserPictureController
{
    #[Route('/user/picture', methods: ['PUT'])]
    public function __invoke(
        #[MapUploadedFile(
            new Assert\File(mimeTypes: ['image/png', 'image/jpeg']),
        )] ?UploadedFile $picture,
    ): Response {
        return new Response('Your picture was updated');
    }
}
```

```php
# src/Controller/UserDocumentsController.php

namespace App\Controller;

use Symfony\Component\HttpFoundation\Response;
use Symfony\Component\HttpKernel\Attribute\AsController;
use Symfony\Component\HttpKernel\Attribute\MapUploadedFile;
use Symfony\Component\Routing\Annotation\Route;
use Symfony\Component\Validator\Constraints as Assert;

final class UserDocumentsController
{
    #[Route('/user/documents', methods: ['PUT'])]
    public function __invoke(
        #[MapUploadedFile(
            new Assert\File(mimeTypes: ['application/pdf'])
        )] array $documents
    ): Response {
        return new Response('Thanks for sharing your documents');
    }
}
```

```php
# src/Controller/UserDocumentsController.php

namespace App\Controller;

use Symfony\Component\HttpFoundation\File\UploadedFile;
use Symfony\Component\HttpFoundation\Response;
use Symfony\Component\HttpKernel\Attribute\AsController;
use Symfony\Component\HttpKernel\Attribute\MapUploadedFile;
use Symfony\Component\Routing\Annotation\Route;
use Symfony\Component\Validator\Constraints as Assert;

final class UserDocumentsController
{
    #[Route('/user/documents', methods: ['PUT'])]
    public function __invoke(
        #[MapUploadedFile(
            new Assert\File(mimeTypes: ['application/pdf'])
        )] UploadedFile ...$documents
    ): Response {
        return new Response('Thanks for sharing your documents');
    }
}
```

Commits
-------

b85dbd0 [HttpKernel] Add MapUploadedFile attribute
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.