quarkusio / quarkus

Quarkus: Supersonic Subatomic Java.
https://quarkus.io
Apache License 2.0
13.75k stars 2.67k forks source link

RestEasy Reactive UriBuilder.buildFromMap should use registered parameter converters #25965

Open kdubb opened 2 years ago

kdubb commented 2 years ago

Description

Currently it uses toString() on the values from the provided map. It seems very much like this api should be using parameter converters, since it's converting parameter values!

I initially was going to submit this as a bug but I checked RestEasy Classic and it appears to be essentially the same code. Also, the JAX-RS spec seems to say only that in 2.0 all UriBuilder now "encodes" all components; which I believe is referring to URL encoding.

Implementation ideas

No response

quarkus-bot[bot] commented 2 years ago

/cc @FroMage, @geoand, @stuartwdouglas

kdubb commented 2 years ago

Additional comments with use case...

The reason for this is that when building URLs/URIs with UriBuilder you must manually match the encoding to what is expected when the URL is returned back to the server.

A specific use case is the Location header when returning 201 Created. We have a registered parameter converter for UUIDs (FriendlyId) so we expect all URL parameters that map to UUIDs to be in "friendly id" (aka base62) format.

When using buildFromMap(...) we expected the following code to encode parameters using the registered converter...

    val locationUri =
      uriInfo.requestUriBuilder
        .path("{id}")
        .buildFromMap(mapOf("id" to team.id))

Instead we have to match this with this with the registered converter and convert team.id to "friendly id" format manually.

In the end, using the registered parameter converters seems to make a lot of sense to reduce development and maintenance burden.

geoand commented 2 years ago

I agree that param converters seems to make sense here, but I honestly don't know if the TCK would pass. You can try the patch you had in mind by building it locally and the running mvn clean install on a local version of https://github.com/quarkusio/resteasy-reactive-testsuite