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

JacksonJrDecoder is not able to deserialize generics #2296

Closed
yvasyliev opened this issue Jan 20, 2024 · 0 comments · Fixed by #2298
Closed

JacksonJrDecoder is not able to deserialize generics #2296

yvasyliev opened this issue Jan 20, 2024 · 0 comments · Fixed by #2298

Comments

@yvasyliev
Copy link
Contributor

yvasyliev commented Jan 20, 2024

Description

Although jackson-jr library is capable of generics deserialization, feign-jackson-jr module throws DecodeException when you try to decode the response.

How to reproduce

I created a simple demo to demonstrate this:

public class JacksonJrDemo {
    public static void main(String[] args) throws IOException {
        String json = "{\"data\":2024}";

        DataHolder<?> jrResult = JSON.std.beanFrom(DataHolder.class, json);
        System.out.println(jrResult); // works

        Object jrDecoderResult = new JacksonJrDecoder().decode(
            Response
                .builder()
                .request(Request.create(
                    Request.HttpMethod.GET,
                    "/v1/dummy",
                    Collections.emptyMap(),
                    Request.Body.empty(),
                    null
                ))
                .body(json, StandardCharsets.UTF_8)
                .build(),
            new TypeReference<DataHolder<Integer>>() {}.getType()
        );
        System.out.println(jrDecoderResult); // throws DecodeException
    }

    @Data
    private static class DataHolder<T> {
        private T data;
    }
}

Output:

JacksonJrDemo.DataHolder(data=2024)
Exception in thread "main" feign.codec.DecodeException: Cannot decode type: io.github.yvasyliev.JacksonJrDemo$DataHolder<java.lang.Integer>
	at feign.jackson.jr.JacksonJrDecoder.findTransformer(JacksonJrDecoder.java:109)
	at feign.jackson.jr.JacksonJrDecoder.decode(JacksonJrDecoder.java:67)
	at io.github.yvasyliev.JacksonJrDemo.main(JacksonJrDemo.java:21)

Tested with:

<dependency>
    <groupId>io.github.openfeign</groupId>
    <artifactId>feign-jackson-jr</artifactId>
    <version>13.1</version>
</dependency>

Initial investigation

I see two problems here:

  1. feign.jackson.jr.JacksonJrDecoder#findTransformer always throws DecodeException unless type is instance of Class, List or Map.
  2. This behevior is not possible to override since feign.jackson.jr.JacksonJrDecoder#findTransformer has private static modifiers. (why?)

Suggestion

  1. In this issue I suggest jackson-jr to hand the control over generics deserialization. Because now JacksonJrDecoder is very limited.
  2. I'm going to create another issue to make feign.jackson.jr.JacksonJrDecoder#findTransformer overridable.

Both problems requires very few code changes, and I'll try to handle both of them.

Actually, if problem 2 will be solved first, there is no urgent need to solve problem 1. Because in this case the user will be able to handle deserialization by extending JacksonJrDecoder. Yet both changes would be great to see.

yvasyliev added a commit to yvasyliev/feign that referenced this issue Jan 20, 2024
velo added a commit that referenced this issue Jan 22, 2024
* Allow generics deserialization by JacksonJrDecoder
Fixes #2296

* Update JacksonJrDecoder.java

* Update JacksonCodecTest.java

---------

Co-authored-by: Marvin <[email protected]>
velo added a commit that referenced this issue Oct 7, 2024
* Allow generics deserialization by JacksonJrDecoder
Fixes #2296

* Update JacksonJrDecoder.java

* Update JacksonCodecTest.java

---------

Co-authored-by: Marvin <[email protected]>
velo added a commit that referenced this issue Oct 8, 2024
* Allow generics deserialization by JacksonJrDecoder
Fixes #2296

* Update JacksonJrDecoder.java

* Update JacksonCodecTest.java

---------

Co-authored-by: Marvin <[email protected]>
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 a pull request may close this issue.

1 participant