Skip to content
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

[WIP] Create a controller argument resolver to replace the param converter #2398

Draft
wants to merge 1 commit into
base: 3.x
Choose a base branch
from

Conversation

mbabker
Copy link
Contributor

@mbabker mbabker commented Nov 28, 2023

This is an attempt to get the ball rolling at deprecating the param converter integration in favor of the framework's native argument resolvers. It's untested, it does not have 100% feature parity with the existing implementation (some of that being by design since the two systems are different), and it's kind of a Frankenbeast trying to deal with Symfony 5.4 support, but here we are.

Note, this will only support PHP 8. Considering the Sensio bundle works fine on Symfony 5.4 and Symfony 6 requires PHP 8, as well as argument resolvers having only ever supported PHP 8 attributes (#[CurrentUser] exists in Symfony 5.4 but has no @Annotation declaration), I personally don't see that as an issue.

The attribute and resolver design try to take inspiration from the framework's #[MapRequestPayload] attribute and RequestPayloadValueResolver while only re-implementing the functionality in the existing RequestBodyParamConverter. So the validator integration is still opt-in (whereas in the framework if the validator's injected it is always used), and this bundle's serializer bridge continues to be used.

Things that have changed in the workflow:

  • No more "is optional" style conditional aborts, all errors now throw
  • Since an argument value resolver can't provide two separate arguments (the resolver itself enforces only providing one value unless working with a variadic argument), and some of the order of operations logic here is already crazy, a validation error now throws its HTTP exception instead of adding the violation list as another argument for the controller
  • The deserialized payload isn't written to the request attributes

Things that still need to be done if this is usable:

  • Tests
  • Wire services in as appropriate
  • Adapt the bundle config?

@mbabker mbabker force-pushed the argument-resolver branch from a8f5223 to 2dfafdd Compare April 4, 2024 17:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant