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

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

@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?

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?

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

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

Your PR also makes this one #47425 obsolete.

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

I understand the feature that is a shortcut to Deserializing and Validating your data (query or request) before it hits the Controller. I think that this is helpful but I know it is already possible using API Platform, although API Platform enforces standards (for example for error validation, resource-oriented designs, but also plain json). I'm afraid that these attributes will lead to bad API designs in the future and wanted to share my concern. Last but not least, the feature you propose looks out of the scope of the HTTP Kernel (adds a dependency on the Serializer and the Validator).

We're already using such a similar arg value resolver in house, and from the reaction to this PR (and related PRs or issues), looks like it is not uncommon. It increases overall DX. I shouldn't need to import a big library just to avoid manually mapping json to PHP objects, the framework should do it for me.

My argument is more that the join of validation and serialization is probably out of the scope of the HTTP Kernel. Actually what you do here is definitely already possible using the Symfony Framework by doing (pseudo code):

class MyController extends AbstractController {
    #[Route('/route', name: 'app_route')]
    public function myRoute(Request $request): Response
    {
      $serializer = $this->container->get('serializer);
      $data = $serializer->deserialize($request->getContent(), Dto::class, 'json');
      $validator = $this->container->get('validator);
      $validator->validate($data, ...);
    }
}

By "need to import a big library " you're probably talking about API Platform, I understand that this is the current "image" of this framework although it's not that "big" and its integration with the Symfony Framework is quite good.

I see some quite interesting things in the DX here don't get me wrong, I just have some concerns about it being a part of the http-kernel.

My argument is more that the join of validation and serialization is probably out of the scope of the HTTP Kernel. Actually what you do here is definitely already possible using the Symfony Framework by doing (pseudo code):

Yes, indeed. Attribute does not allow to do things that are not possible with plain PHP. But it's a shortcut to reduce boilerplate code. Like IsGranted, Cache, MapEntity etc.

So I disagree with you. I'm a heavy user of APIP, but sometime I don't use it and this kind of entity would be very useful!

So it's a big 👍🏼 for me

My argument is more that the join of validation and serialization is probably out of the scope of the HTTP Kernel. Actually what you do here is definitely already possible using the Symfony Framework by doing (pseudo code):

class MyController extends AbstractController {
    #[Route('/route', name: 'app_route')]
    public function myRoute(Request $request): Response
    {
      $serializer = $this->container->get('serializer);
      $data = $serializer->deserialize($request->getContent(), Dto::class, 'json');
      $validator = $this->container->get('validator);
      $validator->validate($data, ...);
    }
}

Indeed, it is possible with a bit of boilerplate, that is specifically what we (users) would like to avoid.

By "need to import a big library " you're probably talking about API Platform, I understand that this is the current "image" of this framework although it's not that "big" and its integration with the Symfony Framework is quite good.

It is big imho, I mean it does a lot more than just deserialize + validate.

I see some quite interesting things in the DX here don't get me wrong, I just have some concerns about it being a part of the http-kernel.

Yeah, the concern about this being part of http-kernel is legit. I do think in an another PR @derrabus had suggested to move the changes to serializer component with optional validation

#45628 (comment)

I also saw #49134, I find the implementation quite good and it doesn't bring the issues I see here with the serializer/validator link. I love the idea behind these and this is going to inspire us for some of the upcoming features in API P:D.

It is big imho, I mean it does a lot more than just deserialize + validate.

Thing is it doesn't need to, it can do just the above with about no overhead:

#[Get(uriTemplate: '/books/{id}', controller: MyController::class)]
class Dto {
  public $id;
  #[Assert]
  public $name;
}

class MyController {
  public function __invoke(Dto $data): DtoOutput {
    return new DtoOutput();
  }
}

(NB: pseudo code not sure it works just like that!)

And you can already disable the validation ^^. We're in the process of spliting the code so that one could use only the relevant parts.

/edit: I think we'll find something even better to avoid the Resource approach, I think I get the why you did this, and I quite like the approach!

@nicolas-grekas rebased again 😃

btw, given #49614 we should consider #[MapRequestPayload] here (or a recall to consider getBody() there)

Can we merge this one? I have a local branch with some changes (merge of type error + validation ; Drop of partial violation normalizer and other nipticks).

Yes, please 🙏. We will have 2 months for tuning it

obsolete

Conversion and validation are two topics.

I am familiar with a few frameworks in Java/Jakarta EE.

Firstly compare to the existing web framework, I would like rename the attributes/annotations in this PR to #[RequestBody], #[RequestForm], and we can also add #[RequestParam], (#[MatrixParam] or request param can parse the matrix params), #[RequestPath], #[RequestPart], #[Header], #[Cookie], #[Session], etc.

<?php

class CreatingUserForm
{
    public function __construct(
        #[RequestParam(name:"name")]
        public readonly string $name,
        #[RequestParam(name:"age", required:false, defaultValue:"0")]
        public readonly int $age= 25,
        #[RequestPart(name:"file")]
        public readonly byte[] $offset = 0,
    ) {
    }
}


class CreatingUserCommand
{
    public function __construct(
        public readonly string $name,
        public readonly int $age= 25
    ) {
    }
}


#[Controller]
class UserApiController
{
// accept a traditional multiform/data
    public function saveFromWeb(
        #[ReuqestForm] CreatingUserForm $body,
    ): Response {
    }


// accept json/xml
 public function saveFromJson(
        #[ReuqestBody] CreatingUserCommand $body,
    ): Response {
    }

}

Through these attrbutes/annotaions to convert to the target object.

Applying the validation is targeted the converted object.

class CreatingUserCommand
{
    public function __construct(

        #[NotBlank]
        public readonly string $name,
        #[Positive]
        public readonly int $age= 25
    ) {
    }
}

// in controller

public function saveFromJson(
        #[ReuqestBody] #[Validated] CreatingUserCommand $body,
    ): Response {
    }

The above dummy usage of attributes/annotations is similar to Jakarta Bean Validations in Spring framework.

that's more work in terms of maintaining the mapping :), i'd prefer to control it from the calling side (the argument attribute) if there's a usecase

not validating the payload by default sounds like a footgun to me

combining multiple sources eg. body/query/cookie/header into a single DTO could be done witth a new attribute #[MapRequest] which takes a nested attribute mapping, using attributes such as #49134 or the one from this PR

then you can create pre-defined mappings in userland: MapMySpecialRequest extends MapRequest

it doesnt block this PR IMHO. please merge it :)