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

Quarkus REST: further support for record parameter containers #42646

Open
FroMage opened this issue Aug 20, 2024 · 1 comment
Open

Quarkus REST: further support for record parameter containers #42646

FroMage opened this issue Aug 20, 2024 · 1 comment
Labels
area/rest kind/enhancement New feature or request

Comments

@FroMage
Copy link
Member

FroMage commented Aug 20, 2024

Description

During work on #42642 to support record bean params on the server, I've noted the following issues left to be done:

  • @Context, @BeanParam, @MultipartForm, MultipartDataInput are implied, but only as method parameters, not fields in resources or parameter container classes, I feel like we should change it so fields are treated the same as endpoint method parameters.
  • The docs say that @BeanParam is still required in order for OpenAPI to detect them, we should fix this.
  • Implicit parameter containers which implicit contain parameter containers are not automatically detected as such (due to above this can only happen for records). This is because auto-detection should run in a loop so that new parameter containers trigger a scan for classes that contain them.
  • REST client has parameter containers with no default constructor, the server wants to make beans out of them, but we can't because there's no default constructor, they are only useful for the client. Right now the server ignores them, but it would be better if we could have a different set of bean param containers: used by client, or server, so we can generate error messages when not used properly (client parameter containers with no default constructors are not usable by the server).
  • I don't think record parameter containers work for the REST client
  • CustomResourceProducersGenerator generates CDI producers for endpoints that have non-default constructors, but not bean params. Also, this constructor injection uses custom logic that is not the same as method parameter or bean param, so it appears to call *ParamExtractor.extractParameter (single only, no separator, not encoded, no default value, no converter, no support for multipart anything). This is wrong in any but the most trivial case.
  • We could replace ClassInjectorTransformer to use Gizmo (should be possible now, with the transformer support), or we could get rid of it entirely and convert all custom injection into CDI producers by adding the required logic to CustomResourceProducersGenerator (and fixing it). This feels like the best option IMO.

Implementation ideas

No response

Copy link

quarkus-bot bot commented Aug 21, 2024

/cc @stuartwdouglas (resteasy-reactive)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/rest kind/enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants