`@JsonCreator` not working properly with `@JsonIdentityInfo` when deserializing a collection of interfaces (immutables.io compatibility)

[balysv]

Tested on 2.8.* and 2.9.0.pr4

Given an interface

@JsonSerialize(as = ImmutableTest.class)
@JsonDeserialize(as = ImmutableTest.class)
@JsonIdentityInfo(generator = ObjectIdGenerators.IntSequenceGenerator.class)
public interface ITest {
    Integer getId();
    String getName();
}

that has two implementations

public static class ImmutableTest implements ITest {
    private final Integer id;
    private final String name;
    ...
    @JsonCreator(mode = DELEGATING)
    static ImmutableTest fromJson(JsonTest json) {
        return new ImmutableTest(json.id, json.name);
    }
}

@JsonDeserialize
@JsonAutoDetect(fieldVisibility = JsonAutoDetect.Visibility.NONE)
public static class JsonTest implements ITest {
    private String name;
    private Integer id;
    ...
}

and a collection of the interface as input for deserialization

[
   { "@id":1, "id":1, "name":"test" }, 
   1
]

the method annotated with @JsonCreator is not being invoked when deserializing the second collection item (referred by @id:1), resulting in an invalid collection - one item being of type ImmutableTest while the other one is JsonTest.

The setup mimics the way immutables.io library generated classes implement jackson compatibility.

Minimal test case that has classes identical to those generated by immutables.io library: https://gist.github.com/balysv/bae96686cbbb745d07b198927f1577f0

I'm not quite sure if the issue is a misuse of jackson annotations by the generated classes or something internal in jackson-databind. Would appreciate hints of how to get the given setup work.

[cowtowncoder]

If I understand setup correctly, I don't see how it could work, if polymorphic deserialization (@JsonTypeInfo) is not being used. Without this, it is not clear how actual implementation could be determined -- serialization works fine, of course, but upon deserialization the type available is presumably ITest. It is possible to map this into just one implementation type; or, by polymorphic handling, force addition of "type id" of some type. Either would allow this to work.

If I misunderstood something, it would be helpful to add actual test method -- most of information is there, but bit of wiring not I think, to fully reproduce the case.

[balysv]

The setup seems to work fine as long as there are no @JsonIdentityInfo substitutes to handle in collections of the interface. I will have some more play with @JsonTypeInfo but had no luck so far.

This is a runnable test case for the issue. Note that, when using immutables.io, ImmutableTest and JsonTest would be the generated classes

package test;

import static com.fasterxml.jackson.annotation.JsonCreator.Mode.DELEGATING;
import static org.junit.Assert.assertEquals;

import java.io.IOException;
import java.util.Arrays;
import java.util.List;

import org.junit.Test;

import com.fasterxml.jackson.annotation.JsonAutoDetect;
import com.fasterxml.jackson.annotation.JsonCreator;
import com.fasterxml.jackson.annotation.JsonIdentityInfo;
import com.fasterxml.jackson.annotation.ObjectIdGenerators;
import com.fasterxml.jackson.core.type.TypeReference;
import com.fasterxml.jackson.databind.ObjectMapper;
import com.fasterxml.jackson.databind.annotation.JsonDeserialize;
import com.fasterxml.jackson.databind.annotation.JsonSerialize;

public class T {
    private ObjectMapper objectMapper = new ObjectMapper();

    @Test
    public void test() throws IOException {
        ImmutableTest test = new ImmutableTest(1, "test");
        List<ITest> input = Arrays.asList(test, test);

        // [{"@id":1,"id":1,"name":"test"},1]
        String json = objectMapper.writeValueAsString(input);

        List<ITest> result = objectMapper.readValue(json, new TypeReference<List<ITest>>() {});

        // java.lang.AssertionError:
        // Expected :[test.T$ImmutableTest@3644b1, test.T$ImmutableTest@3644b1]
        // Actual   :[test.T$ImmutableTest@3644b1, test.T$JsonTest@3b94d659]
        assertEquals(input, result);
    }

    @JsonSerialize(as = ImmutableTest.class)
    @JsonDeserialize(as = ImmutableTest.class)
    @JsonIdentityInfo(generator = ObjectIdGenerators.IntSequenceGenerator.class)
    public interface ITest {
        Integer getId();
        String getName();
    }

    public static class ImmutableTest implements ITest {
        private final Integer id;
        private final String name;

        private ImmutableTest(Integer id, String name) {
            this.id = id;
            this.name = name;
        }

        @Override
        public Integer getId() {
            return id;
        }

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

        @JsonCreator(mode = DELEGATING)
        static ImmutableTest fromJson(JsonTest json) {
            return new ImmutableTest(json.id, json.name);
        }

        @Override
        public boolean equals(Object o) {
            if (this == o)
                return true;
            if (o == null || getClass() != o.getClass())
                return false;
            ImmutableTest test = (ImmutableTest) o;
            return id.equals(test.id) && name.equals(test.name);
        }

        @Override
        public int hashCode() {
            int result = id.hashCode();
            result = 31 * result + name.hashCode();
            return result;
        }
    }

    @JsonDeserialize
    @JsonAutoDetect(fieldVisibility = JsonAutoDetect.Visibility.NONE)
    public static class JsonTest implements ITest {
        private String name;
        private Integer id;

        public void setId(Integer id) {
            this.id = id;
        }

        public void setName(String name) {
            this.name = name;
        }

        @Override
        public String getName() {
            throw new UnsupportedOperationException();
        }

        @Override
        public Integer getId() {
            throw new UnsupportedOperationException();
        }
    }
}

[cowtowncoder]

Thank you. This makes more sense: specifically that there is 1-to-1 relationship between interface and its implementation.

[cowtowncoder]

I can reproduce the test locally. No idea yet what is going on.

[a5phyx]

It seems JsonTest instance (i.e. mutable, intermediate object used to create ImmutableTest via method annotated with@JsonCreator) is put to ObjectIdResolver and when reference 1 is encountered later in the Json, resolver is used to get the object, so it returns proviously bound JsonTest instance.

Why ObjectIdResolver gets the (not fully initialized)JsonTest instance so early, instead of the fully created ImmutableTest object, result of passing JsonTest to the method annotated with @JsonCreator? Probably to support circular references (e.g. to serialize object which points to itself). And it works perfectly, when deserializer works on target instances and not the intermediate ones.

Library client-side workaround for Immutables (no circular references) is to create custom ObjectIdResolver which converts intemediate instances to target ones based on method annotated with @JsonCreator. We are able to point custom resolver class via@JsonIdentityInfo(resolver = ImmutableObjectIdResolver.class). There are some downsides of the workaround and the worst is that in case circular references, deserializer doesn't tell us that something is wrong (it can return incomplete object). Solution for that is to create deserializers which deserialize objects (via delegation to standard deserializer) and in addition based on @id for the object update state of ObjectIdResolver that particular id can be resolvable.

Working client-side workaround:

package test;

import com.fasterxml.jackson.annotation.JsonAutoDetect;
import com.fasterxml.jackson.annotation.JsonCreator;
import com.fasterxml.jackson.annotation.JsonIdentityInfo;
import com.fasterxml.jackson.annotation.ObjectIdGenerator;
import com.fasterxml.jackson.annotation.ObjectIdGenerators;
import com.fasterxml.jackson.annotation.ObjectIdResolver;
import com.fasterxml.jackson.annotation.SimpleObjectIdResolver;
import com.fasterxml.jackson.core.JsonParser;
import com.fasterxml.jackson.core.JsonProcessingException;
import com.fasterxml.jackson.core.type.TypeReference;
import com.fasterxml.jackson.databind.BeanDescription;
import com.fasterxml.jackson.databind.DeserializationConfig;
import com.fasterxml.jackson.databind.DeserializationContext;
import com.fasterxml.jackson.databind.JavaType;
import com.fasterxml.jackson.databind.JsonDeserializer;
import com.fasterxml.jackson.databind.JsonMappingException;
import com.fasterxml.jackson.databind.JsonNode;
import com.fasterxml.jackson.databind.ObjectMapper;
import com.fasterxml.jackson.databind.annotation.JsonDeserialize;
import com.fasterxml.jackson.databind.annotation.JsonSerialize;
import com.fasterxml.jackson.databind.deser.BeanDeserializerModifier;
import com.fasterxml.jackson.databind.deser.DefaultDeserializationContext;
import com.fasterxml.jackson.databind.deser.ResolvableDeserializer;
import com.fasterxml.jackson.databind.deser.std.StdDeserializer;
import com.fasterxml.jackson.databind.jsontype.TypeDeserializer;
import com.fasterxml.jackson.databind.module.SimpleModule;
import com.fasterxml.jackson.databind.node.ValueNode;
import org.junit.Test;

import java.io.IOException;
import java.lang.reflect.Modifier;
import java.util.Arrays;
import java.util.List;

import static com.fasterxml.jackson.annotation.JsonCreator.Mode.DELEGATING;
import static org.junit.Assert.assertEquals;

public class T
{
    private ObjectMapper objectMapper = new ObjectMapper()
            .registerModule(new SimpleModule().setDeserializerModifier(new BeanDeserializerModifier() {
                @Override
                public JsonDeserializer<?> modifyDeserializer(final DeserializationConfig config,
                                                              final BeanDescription beanDesc,
                                                              final JsonDeserializer<?> deserializer) {
                    // TODO: Only types annotated with @JsonIdentityInfo should use DelegatingDeserializer
                    return new DelegatingDeserializer<>(beanDesc.getType(), deserializer);
                }
            }));

    @Test
    public void test() throws IOException {
        ImmutableTest test = new ImmutableTest(1, "test");
        List<ITest> input = Arrays.asList(test, test);

        // [{"@id":1,"id":1,"name":"test"},1]
        String json = objectMapper.writeValueAsString(input);

        List<ITest> result = objectMapper.readValue(json, new TypeReference<List<ITest>>() {});

        // java.lang.AssertionError:
        // Expected :[test.T$ImmutableTest@3644b1, test.T$ImmutableTest@3644b1]
        // Actual   :[test.T$ImmutableTest@3644b1, test.T$JsonTest@3b94d659]
        assertEquals(input, result);
    }

    @JsonSerialize(as = ImmutableTest.class)
    @JsonDeserialize(as = ImmutableTest.class)
    @JsonIdentityInfo(generator = ObjectIdGenerators.IntSequenceGenerator.class, resolver = ImmutableObjectIdResolver.class)
    public interface ITest {
        Integer getId();
        String getName();
    }

    public static class ImmutableTest implements ITest {
        private final Integer id;
        private final String name;

        private ImmutableTest(Integer id, String name) {
            this.id = id;
            this.name = name;
        }

        @Override
        public Integer getId() {
            return id;
        }

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

        @JsonCreator(mode = DELEGATING)
        static ImmutableTest fromJson(Json json) {
            return new ImmutableTest(json.id, json.name);
        }

        @Override
        public boolean equals(Object o) {
            if (this == o)
                return true;
            if (o == null || getClass() != o.getClass())
                return false;
            ImmutableTest test = (ImmutableTest) o;
            return id.equals(test.id) && name.equals(test.name);
        }

        @Override
        public int hashCode() {
            int result = id.hashCode();
            result = 31 * result + name.hashCode();
            return result;
        }
    }

    @Deprecated
    @JsonDeserialize
    @JsonAutoDetect(fieldVisibility = JsonAutoDetect.Visibility.NONE)
    static class Json implements ITest {
        private String name;
        private Integer id;

        public void setId(Integer id) {
            this.id = id;
        }

        public void setName(String name) {
            this.name = name;
        }

        @Override
        public String getName() {
            throw new UnsupportedOperationException();
        }

        @Override
        public Integer getId() {
            throw new UnsupportedOperationException();
        }
    }

    public static class ImmutableObjectIdResolver extends SimpleObjectIdResolver
    {
        @Override
        public ObjectIdResolver newForDeserialization(final Object context) {
            return new ImmutableObjectIdResolver();
        }

        @Override
        public Object resolveId(final ObjectIdGenerator.IdKey id) {
            final Object object = super.resolveId(id);
            if (object == null) {
                return null;
            }
            if (isIntermediateImmutableObject(object)) {
                throw new RuntimeException("Object not fully initialized (cyclic reference)");
            }
            return object;
        }

        public void updateItem(final ObjectIdGenerator.IdKey id, final Object object) {
            final Object old = super.resolveId(id);
            if (old == null) {
                throw new IllegalStateException("Can't update not bound item");
            }
            if (isIntermediateImmutableObject(old)) {
                _items.put(id, object);
            }
        }

        private boolean isIntermediateImmutableObject(final Object object) {
            final Class<?> type = object.getClass();
            return type.getSimpleName().equals("Json")
                    && type.getEnclosingClass() != null
                    && !Modifier.isPublic(type.getModifiers())
                    && type.getAnnotation(Deprecated.class) != null;
        }
    }

    private static class DelegatingDeserializer<T> extends StdDeserializer<T> implements ResolvableDeserializer
    {
        private static final ObjectIdGenerator<?> objectIdGenerator = new ObjectIdGenerators.IntSequenceGenerator();
        private static final ObjectIdResolver objectIdResolver = new ImmutableObjectIdResolver();

        private final JsonDeserializer<T> defaultDeserializer;

        DelegatingDeserializer(final JavaType type, final JsonDeserializer<T> defaultDeserializer) {
            super(type);
            this.defaultDeserializer = defaultDeserializer;
        }

        @Override
        public T deserialize(final JsonParser parser, final DeserializationContext context) throws IOException, JsonProcessingException {
            final JsonNode node = parser.getCodec().readTree(parser);
            final JsonParser treeParser = parser.getCodec().treeAsTokens(node);
            if (treeParser.getCurrentToken() == null) {
                treeParser.nextToken();
            }
            final T object = defaultDeserializer.deserialize(treeParser, context);

            final JsonNode objectIdNode = node.get("@id");
            if (objectIdNode instanceof ValueNode) {
                final String objectId = objectIdNode.asText();
                if (context instanceof DefaultDeserializationContext) {
                    final int id = Integer.parseInt(objectId);
                    final ObjectIdResolver resolver =
                            context.findObjectId(id, objectIdGenerator, DelegatingDeserializer.objectIdResolver).getResolver();
                    if (resolver instanceof ImmutableObjectIdResolver) {
                        ((ImmutableObjectIdResolver) resolver).updateItem(objectIdGenerator.key(id), object);
                    }
                }
            }
            return object;
        }

        @Override
        public Object deserializeWithType(final JsonParser parser,
                                          final DeserializationContext context,
                                          final TypeDeserializer typeDeserializer) throws IOException {
            return defaultDeserializer.deserializeWithType(parser, context, typeDeserializer);
        }

        @Override
        public void resolve(final DeserializationContext context) throws JsonMappingException {
            if (defaultDeserializer instanceof ResolvableDeserializer) {
                ((ResolvableDeserializer) defaultDeserializer).resolve(context);
            }
        }
    }
}

The simplest library-side fix is to parameterize when deserialized object is passed to ObjectIdResolver. By default it could be as it is now, but with possibility to inform deserializers that only fully initialized and transformed by creator methods objects can be passed to ObjectIdResolvers (in this scenario circural references are not supported).

[cowtowncoder]

Yes, passing of incomplete instances is deliberate exactly for cyclic definitions, main use case for @JsonIdentityInfo (although I guess there may be cases where DAGs benefit from ability to retain shared identity as well).

[mathansen]

I ran into this issue today. Circular references work partially with a recent version of Jackson (v. 2.15.2). The circular references are set in simple cases where the reference's target has already been decoded. However, the intermediate instances are used. In the case of the immutables project this breaks equals and hashCode and erases the type hierarchy so it is not only unusable but dangerous (as it seems to work at first glance).