Skip to content

2.11.3 regression involving generics + static @JsonCreator 'of' method #2916

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

Closed
stevestorey opened this issue Nov 2, 2020 · 5 comments
Closed

Comments

@stevestorey
Copy link
Contributor

Describe the bug
I've discovered what appears to be a regression between 2.11.2 and 2.11.3 involving a combination of generic type references, and the use of static builder methods annotated with `@JsonCreator'

Version information
2.11.3

To Reproduce
Plain unit test:

package regression;

import java.io.IOException;

import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.Test;

import com.fasterxml.jackson.annotation.JsonCreator;
import com.fasterxml.jackson.annotation.JsonProperty;
import com.fasterxml.jackson.core.type.TypeReference;
import com.fasterxml.jackson.databind.ObjectMapper;

public class Jackson2Dot11Dot3Test {

    @Test
    public void testPolymorphism() throws IOException {
        Holder<TypedChild> holder = Holder.of(new TypedChild("jackson"));
        ObjectMapper om = new ObjectMapper();
        Assertions.assertEquals("{\"value\":{\"name\":\"jackson\"}}", om.writeValueAsString(holder));
        Holder<?> readBack = om.readValue(om.writeValueAsString(holder), new HolderChildReference());
        // In Jackson 2.11.2, this is true, but in 2.11.3, 'value' has been unmarshalled as a plain LinkedHashMap
        // instead ? Seems to be the fact the JsonCreator is a public 'of' method rather than constructor
        Assertions.assertTrue(readBack.getValue() instanceof TypedChild);
    }

    private static class HolderChildReference extends TypeReference<Holder<TypedChild>> {}

    public static class Holder<V> {
        private V value;
        
        private Holder(V value) {
            this.value = value;
        }

        // Use this as the constructor instead, and the test passes in both 2.11.2 and 2.11.3.
        /*@JsonCreator
        private Holder(@JsonProperty("value") V value) {
            this.value = value;
        }*/

        public V getValue() {
            return value;
        }

        @JsonCreator
        public static <V> Holder<V> of(@JsonProperty("value") V value) {
            return new Holder<>(value);
        }
    }

    public static class TypedChild {
        private String name;
        
        @JsonCreator
        public TypedChild(@JsonProperty("name") String name) {
            this.name = name;
        }

        public String getName() {
            return name;
        }
    }
}

Expected behavior
The unmarshalled JSON uses the passed type reference information when unmarshalling the child object, so that the unit test passes.

@stevestorey stevestorey added the to-evaluate Issue that has been received but not yet evaluated label Nov 2, 2020
@stevestorey
Copy link
Contributor Author

stevestorey commented Nov 2, 2020

Having tried to fix a more complicated example in my actual codebase by moving the @JsonCreator over to the constructor instead, my more complicated implementation still fails with what also looks like another failure to resolve the generic types from the provided type reference, so perhaps it's more of a bug about the processing of type references than it is about the use of static builder methods.

I'll see if I can narrow down a test case demonstrating the 2nd failure as well which doesn't look like it requires the static builder method to trigger in my codebase.

EDIT: No, I realised that as I'm using the immutables framework (https://github.com/immutables/immutables/) in my actual codebase, that also uses static @JsonCreator methods for its objects, and so it's the same issue.

@stevestorey
Copy link
Contributor Author

stevestorey commented Nov 2, 2020

I've narrowed down the slightly more complicated case I referred to above, which does look like another symptom, but this time the test throws an exception rather than just unmarshalling as the wrong type. Again, the Parent class here uses a static of method as its @JsonCreator. Uncommenting the constructor method @JsonCreator annotation is enough to make it work again.

package regression;

import java.io.IOException;

import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.Test;

import com.fasterxml.jackson.annotation.JsonCreator;
import com.fasterxml.jackson.annotation.JsonProperty;
import com.fasterxml.jackson.core.type.TypeReference;
import com.fasterxml.jackson.databind.ObjectMapper;

public class Jackson2Dot11Dot3Test {

    @Test
    public void testPolymorphism() throws IOException {
        GrandParent<Parent<Child>, Child> holder = GrandParent.of(new Parent<>(new Child("jackson")));
        ObjectMapper om = new ObjectMapper();
        Assertions.assertEquals("{\"parent\":{\"child\":{\"name\":\"jackson\"}}}", om.writeValueAsString(holder));
        // In Jackson 2.11.2, this all works and passes, but in 2.11.3, an exception is thrown as the
        // static creation of the Parent doesn't know how to deal with the type of the Child by the looks
        // of things
        GrandParent<?, ?> readBack = om.readValue(om.writeValueAsString(holder), new AncestryReference());
        Assertions.assertTrue(readBack.getParent() instanceof Parent);
        Assertions.assertTrue(readBack.getParent().getChild() instanceof Child);
        Assertions.assertEquals("jackson", readBack.getParent().getChild().getName());
    }

    private static class AncestryReference extends TypeReference<GrandParent<Parent<Child>, Child>> {}

    public static class GrandParent<P extends IParent<C>, C extends IChild> {
        private P value;
        
        @JsonCreator
        private GrandParent(@JsonProperty("parent") P parent) {
            this.value = parent;
        }

        public P getParent() {
            return value;
        }

        public static <P extends IParent<C>, C extends IChild> GrandParent<P, C> of(@JsonProperty("parent") P parent) {
            return new GrandParent<>(parent);
        }
    }

    public interface IParent<C extends IChild> {
        C getChild();
    }
    
    public static class Parent<C extends IChild> implements IParent<C> {
        private C child;
        
        // @JsonCreator // **** Uncomment this and the test passes in both 2.11.2 and 2.11.3
        private Parent(@JsonProperty("child") C child) {
            this.child = child;
        }

        @Override
        public C getChild() {
            return child;
        }

        @JsonCreator
        public static <C extends IChild> Parent<C> of(@JsonProperty("child") C child) {
            return new Parent<>(child);
        }
    }

    public interface IChild {
        String getName();
    }

    public static class Child implements IChild {
        private String name;
        
        @JsonCreator
        public Child(@JsonProperty("name") String name) {
            this.name = name;
        }

        @Override
        public String getName() {
            return name;
        }

        // @JsonCreator
        public static Child of(@JsonProperty("name") String name) {
            return new Child(name);
        }
    }
}

@cowtowncoder
Copy link
Member

@stevestorey This may be same as #2894, patch for which is included in 2.11 branch (for 2.11.4). I will try that when I get a chance (there are a few issues I need to resolve first), but if you can either build locally against 2.11 branch or use 2.11.4-SNAPSHOT from Sonatype OSS repo maybe you could see if that would resolve this issue.

@stevestorey
Copy link
Contributor Author

@cowtowncoder yes, you're right - upgrading to 2.11.4-SNAPSHOT means that both test examples above work just fine, so I'll mark this closed as a duplicate of #2894

@cowtowncoder
Copy link
Member

@stevestorey Thank you for verifying this! (and also reporting: better to have multiple reports than none, just in case)

@cowtowncoder cowtowncoder removed the to-evaluate Issue that has been received but not yet evaluated label Dec 9, 2020
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

No branches or pull requests

2 participants