With RESTEasy Reactive @QueryParam makes POST with JSON not work

ia3andy commented 1 year ago

Describe the bug

I need some Data to be consumable in both a POST (Json) and a GET (@BeanParam) endpoints

public class Data {
    @QueryParam("f")
    public String foo;

    @QueryParam("b")
    public int bar;
}


    @GET
    @Produces(MediaType.TEXT_PLAIN)
    public String hello(@BeanParam Data data) {
        return "Hello from RESTEasy Reactive " + data.foo + " " + data.bar;
    }

    @POST
    @Consumes(MediaType.APPLICATION_JSON)
    @Produces(MediaType.TEXT_PLAIN)
    public String helloBug(Data data) {
        return "Hello from RESTEasy Reactive " + data.foo + " " + data.bar;
    }

This was working fine with RESTEasy Classic.

Expected behavior

data in the hello2 POST contains the data from the json body

Actual behavior

data uses initial values

How to Reproduce?

Here is a reproducer with tests:

The tests fail with RR https://github.com/ia3andy/reproducer-query-param

The tests passes with Classic: https://github.com/ia3andy/reproducer-query-param/commit/dd1de45d79a759e3d7f84bf552728fbda7579be9

quarkus-bot[bot] commented 1 year ago

/cc @FroMage (resteasy-reactive), @Sgitario (resteasy-reactive), @geoand (resteasy-reactive), @stuartwdouglas (resteasy-reactive)

geoand commented 1 year ago

@FroMage I am assigning thia one to you.

FroMage commented 1 year ago

Oh wow. This is unexpected. So you have a class that you want to deserialise in two manners: using the JAX-RS annotations, or as a method body.

This currently doesn't work because @BeanParam is implicit now, because we can infer it based on the presence of the JAX-RS annotations in there. I didn't think anyone would dual-use a class.

I certainly don't want to force people to use @BeanParam because this use-case is far-fetched (after all, POST allows query params, I'm not sure why you alternate between passing them as query params and as body, unless they also change meaning, but then that's even more confusing). But if you really really insist, I guess we could introduce a @Body annotation you could use on the POST endpoint to disable it being seen as a @BeanParam. But then all its annotations will be ignored, this is really a bad pattern IMO, it's very confusing. Not to mention that the deserialisation rules would be completely different. It works for String types, but the minute you introduce numbers or dates, in one case you'll go via ParamConverter and in the other via whatever system Jackson has, and you'll end up with different formats and errors.

This feels wrong for lots of reasons. I could support it by adding @Body, if you justify this style of architecture, especially given all the cons I listed.

I don't know of any workaround, ATM, no.

ia3andy commented 1 year ago

Ok I get the problem..

So the reason behind this:

We are limited in length in the URL, so QueryParam is not a fit when there is a big array for example, having a POST alternative solve this problem (in the case of code.quarkus.io, it's the extension list)
There are a lot of OpenApi documentation in the beans, having a separate bean means we duplicate this doc

As an alternative, instead of having yet a new annotation, could we have a "strict" mode config that force having this annotation and all others in the same "inferred" situation.

FroMage commented 1 year ago

But why do you have the same params in GET and POST? Won't you have the length limitation in the GET call as well?

How many of these bean params do you have?

ia3andy commented 1 year ago

Just this buddy


@JsonIgnoreProperties(ignoreUnknown = true)
data class ProjectDefinition(

    @field:QueryParam("S")
    @field:Parameter(name = "S", description = "The platform stream to use to create this project ('platformKey:streamId' or 'streamId')", required = false)
    @field:Schema(
        description = "The platform stream to use to create this project ('platformKey:streamId' or 'streamId')",
        required = false
    )
    val streamKey: String? = null,

    @field:QueryParam("g")
    @field:Parameter(name = "g", description = "GAV: groupId", required = false)
    @field:DefaultValue(DEFAULT_GROUPID)
    @field:NotEmpty
    @field:Pattern(regexp = GROUPID_PATTERN)
    @field:Schema(description = "GAV: groupId", required = false, defaultValue = DEFAULT_GROUPID, pattern = GROUPID_PATTERN)
    val groupId: String = DEFAULT_GROUPID,

    @field:QueryParam("a")
    @field:Parameter(name = "a", description = "GAV: artifactId", required = false)
    @field:DefaultValue(DEFAULT_ARTIFACTID)
    @field:NotEmpty
    @field:Pattern(regexp = ARTIFACTID_PATTERN)
    @field:Schema(description = "GAV: artifactId", required = false, defaultValue = DEFAULT_ARTIFACTID, pattern = ARTIFACTID_PATTERN)
    val artifactId: String = DEFAULT_ARTIFACTID,

    @field:QueryParam("v")
    @field:Parameter(name = "v", description = "GAV: version", required = false)
    @field:DefaultValue(DEFAULT_VERSION)
    @field:NotEmpty
    @field:Schema(description = "GAV: version", required = false, defaultValue = DEFAULT_VERSION)
    val version: String = DEFAULT_VERSION,

    @field:QueryParam("c")
    @field:Parameter(name = "c", description = "The class name to use in the generated application", required = false)
    @field:Pattern(regexp = CLASSNAME_PATTERN)
    @field:Schema(description = "The class name to use in the generated application", required = false, pattern = CLASSNAME_PATTERN)
    val className: String? = null,

    @field:QueryParam("p")
    @field:Parameter(name = "p", description = "The path of the REST endpoint created in the generated application", required = false)
    @field:Pattern(regexp = PATH_PATTERN)
    @field:Schema(description = "The path of the REST endpoint created in the generated application", required = false, pattern = PATH_PATTERN)
    val path: String? = null,

    @field:QueryParam("b")
    @field:Parameter(name = "b", description = "The build tool to use (MAVEN, GRADLE or GRADLE_KOTLIN_DSL)", required = false, schema = Schema(enumeration = ["MAVEN", "GRADLE", "GRADLE_KOTLIN_DSL"]))
    @field:DefaultValue(DEFAULT_BUILDTOOL)
    @field:NotEmpty
    @field:Pattern(regexp = BUILDTOOL_PATTERN)
    @field:Schema(description = "The build tool to use (MAVEN, GRADLE or GRADLE_KOTLIN_DSL)", enumeration = ["MAVEN", "GRADLE", "GRADLE_KOTLIN_DSL"], defaultValue = DEFAULT_BUILDTOOL)
    val buildTool: String = DEFAULT_BUILDTOOL,

    @field:QueryParam("j")
    @field:Parameter(name = "j", description = "The Java version for the generation application", required = false)
    @field:NotEmpty
    @field:DefaultValue(DEFAULT_JAVA_VERSION)
    @field:Pattern(regexp = JAVA_VERSION_PATTERN)
    @field:Schema(description = "The Java version for the generation application", required = false, pattern = JAVA_VERSION_PATTERN)
    val javaVersion: String = DEFAULT_JAVA_VERSION,

    @field:QueryParam("nc")
    @field:Parameter(name = "nc", description = "No code", required = false)
    @field:DefaultValue(DEFAULT_NO_CODE.toString())
    @field:Schema(description = "No code", required = false, defaultValue = DEFAULT_NO_CODE.toString())
    val noCode: Boolean = DEFAULT_NO_CODE,

    @field:QueryParam("ne")
    @field:Parameter(name = "ne", description = "No code examples (Deprecated: use noCode (nc) instead)", required = false, schema = Schema(deprecated = true))
    @field:DefaultValue(DEFAULT_NO_CODE.toString())
    @field:Schema(description = "No code examples (Deprecated: use noCode (nc) instead)", deprecated = true, required = false, defaultValue = DEFAULT_NO_CODE.toString())
    @Deprecated(message = "Use noCode (nc) instead")
    val noExamples: Boolean = DEFAULT_NO_CODE,

    @field:QueryParam("e")
    @field:Parameter(name = "e", description = "The set of extension ids that will be included in the generated application", required = false)
    @field:Schema(description = "The set of extension ids that will be included in the generated application", required = false)
    val extensions: Set<String> = setOf()
){

ia3andy commented 1 year ago

But why do you have the same params in GET and POST? Won't you have the length limitation in the GET call as well?

the direct GET is only for code.quarkus.io which has a safety for the url size
there is a POST to retrieve the GET url for client app (with the safety)
there is a POST to download directly when possible from the client app

geoand commented 1 year ago

What's the status of this?

ia3andy commented 1 year ago

@BeanParam is implicit with RR, therefore, it seems there is no "clean" fix for it (fix is to add an opposite annotation which seems dumb), better leave it this way.

geoand commented 1 year ago

Thanks for the update

naah69 commented 8 months ago

Thanks for the update

We use 2.13.4.Final now. @QueryParam and json work.

We have the same error when we upgrade to 2.16.12.Final.

It is not compatible when the version greater than or equal to 2.14.x

quarkusio / quarkus