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

RESTEasy Reactive does not call method ParamConverter#toString for collection elements #36639

Closed
timonzi opened this issue Oct 23, 2023 · 3 comments · Fixed by #36885
Closed
Assignees
Labels
area/rest kind/bug Something isn't working
Milestone

Comments

@timonzi
Copy link

timonzi commented Oct 23, 2023

Describe the bug

The fix in #35774 allows the definition of REST resources with a collection of parameterized types as a query parameter.

Example (from #35774):

@GET
@Path("/wrapperList")
public String wrapperList(@QueryParam("wrapperList") final List<WrapperClass<StatusEnum>> wrapperList) {
    // ...
}

The problem what I found now is that toString method of my ParamConverter implementation is not called for the collection elements. Because of this the fromString method gets a class reference as String value (e.g. org.acme.WrapperClass@59e082f8) instead of the expected one.

public class WrapperClassParamConverter implements ParamConverter<WrapperClass<?>> {

  // [...]

    @Override
    public WrapperClass<?> fromString(String value) {
        return new WrapperClass<>(Enum.valueOf(StatusEnum.class, value));
    }

    @Override
    public String toString(WrapperClass<?> wrapperClass) {
        // RESTEasy Reactive: Not called for elements of a collection.
        // RESTEasy Classic: Calls the method for every element
        return wrapperClass != null ? wrapperClass.getValue().toString() : null;
    }

}

So the current implementation works fine when I simply provide a String value when calling the REST resource. This is what I did when testing the fix of #35774.

But when I now use the REST client to call a resource of another service, the toString method should be called to serialize the value and on the other side the fromString method should be called. For the fromString this works fine, but the toString method is never called.

Expected behavior

The toString method from parameter converters should be used like in RESTEasy Classic - so also for collections.

Actual behavior

The toString method is never called for collection elements.

How to Reproduce?

Reproducer: https://github.com/timonzi/resteasy-reactive-wrapper-collection-elements

  1. Execute tests in ParametersResourceTest --> testCollectionViaClient will fail (because of the unexpected value in the fromString implementation)
  2. Switch to RESTEasy Classic implementation (commented dependencies are already in the pom.xml) --> tests will work

Output of uname -a or ver

Linux nb-timonz 6.2.0-31-generic #31~22.04.1-Ubuntu SMP PREEMPT_DYNAMIC Wed Aug 16 13:45:26 UTC 2 x86_64 x86_64 x86_64 GNU/Linux

Output of java -version

OpenJDK Runtime Environment Temurin-17.0.6+10 (build 17.0.6+10)

Quarkus version or git rev

3.2.7.Final

Build tool (ie. output of mvnw --version or gradlew --version)

Apache Maven 3.9.3 (21122926829f1ead511c958d89bd2f672198ae9f)

Additional information

The fix of #35774 is also included in 3.2.7.Final. This version is used in the reproducer.

@timonzi timonzi added the kind/bug Something isn't working label Oct 23, 2023
@quarkus-bot
Copy link

quarkus-bot bot commented Oct 23, 2023

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

@geoand
Copy link
Contributor

geoand commented Oct 26, 2023

Thanks for reporting.

I'll try and have a look next week

@geoand
Copy link
Contributor

geoand commented Nov 6, 2023

#36885 fixes the issue

@quarkus-bot quarkus-bot bot added this to the 3.6 - main milestone Nov 6, 2023
geoand added a commit that referenced this issue Nov 6, 2023
Handle generic types for ParamConverter in REST Client
@gsmet gsmet modified the milestones: 3.6 - main, 3.5.1 Nov 6, 2023
gsmet pushed a commit to gsmet/quarkus that referenced this issue Nov 6, 2023
DavideD pushed a commit to DavideD/quarkus that referenced this issue Nov 27, 2023
@aloubyansky aloubyansky modified the milestones: 3.5.1, 3.2.10.Final Jan 14, 2024
aloubyansky pushed a commit to aloubyansky/quarkus that referenced this issue Jan 14, 2024
holly-cummins pushed a commit to holly-cummins/quarkus that referenced this issue Feb 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/rest kind/bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants