[FrameworkBundle][Serializer] Add an ArgumentResolver to deserialize & validate user input

Q	A
Branch?	6.1
Bug fix?	no
New feature?	yes
Deprecations?	no
Tickets	N/A
License	MIT
Doc PR	TODO

Assuming you wish to validate some DTO:

<?php

use Symfony\Component\Validator\Constraints as Assert;

class DummyDto
{
    #[Assert\NotBlank()]
    public ?string $randomText = null;

    #[Assert\IsTrue()]
    public bool $itMustBeTrue = true;
}

If you're not using API Platform, you will probably have to inject serializer & validator in your controller, check if input can be normalized & is valid:

// Before
public function __invoke(Request $request): Response
{
        $input = $this->serializer->deserialize($request->getContent(), DummyDto::class, 'json');
        $errors = $this->validator->validate($input);

        if ($errors->count()) {
            // Some logic to return error to user
        }

While this not the end of the world (:crossed_fingers:), It can be cumbersome and doesn't necessarily bring value.

Using Symfony\Component\Serializer\Annotation\Input provide an opt'in way to get a DTO deserialized and validated directly in the controller:

// After

use Symfony\Component\Serializer\Annotation\Input;

public function __invoke(#[Input()] DummyDto $input): Response
{
        // do your logic

In case the value is not valid, user will receive a HTTP 422 response. Once again, it's completely opt'in (For those of you who don't like magic :unicorn: )

Nice feature ! Thanks

I like the basic idea of this feature, thank you for the PR. I however believe that it should be implemented differently.

First of all, the marker interface. In a world with native attributes, we should not use marker interfaces anymore. Also, it does not really make sense to add a marker (interface or attribute) to a DTO class just because in some controller I might want to deserialize a request payload into it. If we assume that such a DTO class could be a Doctrine entity at the same time, the current implementation would create unpleasant ambiguites with good old param converters or its replacement (see #43854).

I strongly advice to implement this as an attribute for controller parameters. This would also solve the configuration issues mentioned.

Secondly, I'd like to move as little logic as possible into the framework bundle. I believe this argument resolver should live inside the serializer component and validation should be made optional.

@derrabus I switched to Attribute, way nicer indeed, thanks. Not sure about the optional validation though, for two reasons:

1. It imply to create custom normalization for PartialDenormalizationException instead of relying on ConstraintViolationListNormalizer
2. Not sure if it's a good practice to "not validate" user data.

And either way, If a dev really don't care about validation, adding no assertion should result in little overhead.

Not sure if it's a good practice to "not validate" user data.

No, but Symfony's validator is certainly not the only tool for that. If we add this functionality to the serializer component (which can even be used standalone), the validator should not be a mandatory dependency for using it.

And either way, If a dev really don't care about validation, adding no assertion should result in little overhead.

Sure, but installing the validator although it's not used actually is unnecessary overhead.

Would it make sense to split this into 2 different features: one that deserialize input and one that validate controller arguments (with 2 separate attributes)?

This would allow to use the deserialize attribute with custom validation logic or the validation one with other argument resolver.

Would it make sense to split this into 2 different features: one that deserialize input and one that validate controller arguments (with 2 separate attributes)?

This would allow us to validate any arguments, not only those that have been deserialized from the request payload. 🤔

Would it make sense to split this into 2 different features: one that deserialize input and one that validate controller arguments (with 2 separate attributes)?

This would allow us to validate any arguments, not only those that have been deserialized from the request payload.

Can it be multiple ArgumentResolver for one specific argument ? or does the first ArgumentResolver to supports an argument takes over?

The first argument resolver take over, the validation would have to be implemented in another way (probably with an event listener).

@derrabus @jvasseur so one way to do it would be to listen for ControllerArgumentsEvent and check by reflexion if the controller has a specific attribute (Something like #[Validate(['foo', 'bar'])] ?), but I'm not sure how it would impact performance. This may be improved by caching in memory ArgumentMetadataFactory::createArgumentMetadata() to avoid additional overhead.

Or do it at compile time and check on every controller if the specific attribute is present and do some kind of mapping controller => arguments to validate. Would probably be more effective but wouldn't work if the controller is set by an event listener.

Any opinion?

Hi there!

It would be very nice to see this landing in Symfony :)

I have a concerns regarding the naming of the attribute, Input could also refer to query params.

I think it's important that the name shows where the data come from. Some names from other frameworks:

• NestJS: @Body
• Spring Boot: @RequestBody

We have a similar argument resolver at work: #[FromRequestBody(validate: true)] MyDto $payload, where validate is optional and defaults to true.

Using a dedicated attribute for validation could be a good idea, but I would prefer an opt out attribute, like #[NoValidate], to avoid the risk of forgetting it.

@GaryPEGEOT I don't think the performance overhead would be that important, we read attributes for controller arguments on every request for the ArgumentResolver feature so doing it for a validation feature would be the same.

@lyrixx @dunglas would make sense to create an AbstractProblemNormalizer to regroup ConstraintViolationListNormalizer and the new PartialDenormalizationProblemNormalizer?

I think yes 👍🏼

I agree too.

To be honest, I would recommend using API Platform instead of adding such code to Symfony.

I've been using this feature for a few months now. I must say that I really love it. It makes is simple to validate API input and have a clear contract with your clients.

I undertand part of API Platform is also solving this issue. But I dont think that would stop us from adding this to Symfony.

Had a similar idea a couple of years ago :smiley: https://github.com/symfony/symfony/issues/36093 , i am glad we are finally getting an implementation for it (in core)

We use a similar RequestBody attribute to resolve request contents, but recently i found a flaw in it, it doesn't validate that the correct arguments are passed as per php doc block.

<?php
class Preference
{
    public const CATEGORY_1 = 'foo';
    public const CATEGORY_2 = 'bar';

    /**
     * TODO - These types are just for annotation, request body resolver won't validate these when constructing the object from request contents.
     *
     * @param non-empty-string           $option
     * @param self::CATEGORY_* $category
     */
    public function __construct(
        public string $category,
        public string $option,
        public mixed $value = null,
    ) {
    }
}

Does this pr aim to attempt validating against php doc blocks ?

Does this pr aim to attempt validating against php doc blocks ?

I don't think we should do that.

Shall we close in favor of #49138?