@JsonCreator on constructor not compatible with @JsonIdentityInfo, PropertyGenerator

[symposion]

On Jackson 2.11.3

A bean annotated with @JsonIdentityInfo pointing at an id property will not deserialize correctly using @JsonCreator on its constructor. The code explicitly ignores identity fields when looking for constructor parameters (they are set after creation) but this makes JsonIdentityInfo incompatible with any sort of immutable object that requries all its properties to be set at construction time.

import com.fasterxml.jackson.annotation.JsonCreator;
import com.fasterxml.jackson.annotation.JsonIdentityInfo;
import com.fasterxml.jackson.annotation.ObjectIdGenerators;
import com.fasterxml.jackson.databind.ObjectMapper;
import com.fasterxml.jackson.module.paramnames.ParameterNamesModule;

public class Test {

    public static void main(String[] args) throws Exception {
        ObjectMapper mapper = new ObjectMapper();
        mapper.registerModule(new ParameterNamesModule());
        mapper.readValue(JSON, JsonBean.class);
    }


    private static final String JSON = "{\"id\": \"myId\",\"value\": \"myValue\"}";

    @JsonIdentityInfo(generator = ObjectIdGenerators.PropertyGenerator.class, property = "id")
    public static class JsonBean {

        private final String id;
        private final String value;

        @JsonCreator
        public JsonBean(String id, String value) {
            System.out.println(id);
            this.id = id;
            this.value = value;
        }

        public String getId() {
            return id;
        }

        public String getValue() {
            return value;
        }
    }

}

I would expect this to print out "myId"; instead it prints "null". Note that you need to compile with -parameters for this to work

[cowtowncoder]

Thank you for reporting this: I hope to have a look at it soon.

[cowtowncoder]

I can reproduce this; added failing test. Looks like CreatorProperty is somehow not linked to identity id -- so in the example case, field id is used as accessor (even if it is final, actually, unless specifically configured not to do that).

[symposion]

I spent some time looking into this. In BeanDeserializer._deserializeUsingPropertyBased (and probably various other related methods on other classes) it specifically excludes id properties from being added to the PropertyBuffer:

           // Object Id property?
            if (buffer.readIdProperty(propName)) {
                continue;
            }

This looks very deliberate but I don't understand why it's necessary. The intention seems to be that id properties are handled at the end when the object is finally built. PropertyBasedCreator.build() calls through to buffer.handeIdValue which will attempt to set the id value using a setter if one is available as well as registering the object with the id resolver. Given that PropertyBasedCreator (AIUI) is only used for those cases where we're explicitly not using setters but instead passing the value to the creator method/constructor this seems like a mistake. Perhaps a copy/paste from some other creation code? If I get a chance I'll try just removing that continue and see if it breaks anything

[cowtowncoder]

@symposion Couple of notes.

First, use of PropertyBasedCreator does not exclude possibility of also using setters/fields (although some users prefer it would; there's an issue to allow forcing that), or "any setter". So the idea is that anything not-yet handled would be buffered, and anything that is handled is skipped.

Second, I think (but am not 100% sure) that skipping of Object Id is due to most Object Ids being considered (identity) metadata, that is, something for which there is no matching property. The exception to this is the use of PropertyGenerator which relies on existing property.

I think you are probably in thinking that skipping should not occur automatically.

Somehow, the id CreatorProperty is not linked in a way it should be -- it may even be that it is removed intentionally (but for this case, incorrectly).

[cowtowncoder]

Ah. Missed the note on PR -- thank you, hope to check this either today or tomorrow!

[symposion]

Yeah, it seems to work for my use case and none of the tests break... but Jackson is complicated and covers a lot of use cases so I'm not sure if there's some corner case that my fix would cause problems for.