Unable to associate parameter name with single non-annotated constructor argument

[michaelhixson]

With Jackson 2.6.0-rc2 and the parameter names module, I am running into errors when deserializing JSON into objects with one constructor argument. Here is some sample code demonstrating the issue:

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

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

public class Example {
  static class NoAnnotations {
    NoAnnotations(List<Integer> numbers) {
      System.out.println(numbers);
    }
  }

  static class CreatorOnly {
    @JsonCreator
    CreatorOnly(List<Integer> numbers) {
      System.out.println(numbers);
    }
  }

  static class CreatorAndProperty {
    @JsonCreator
    CreatorAndProperty(@JsonProperty("numbers") List<Integer> numbers) {
      System.out.println(numbers);
    }
  }

  static class PropertyOnly {
    PropertyOnly(@JsonProperty("numbers") List<Integer> numbers) {
      System.out.println(numbers);
    }
  }

  static class SecondArgument {
    SecondArgument(List<Integer> numbers, List<Integer> moreNumbers) {
      System.out.println(numbers);
    }
  }

  public static void main(String[] args) {
    String json = "{\"numbers\":[1,2,3]}";
    ObjectMapper mapper = new ObjectMapper()
        .registerModule(new ParameterNamesModule());
    try {
      // This throws.
      mapper.readValue(json, NoAnnotations.class);
    } catch (IOException e) {
      e.printStackTrace();
    }
    try {
      // This throws.
      mapper.readValue(json, CreatorOnly.class);
    } catch (IOException e) {
      e.printStackTrace();
    }
    try {
      // This is ok.
      mapper.readValue(json, CreatorAndProperty.class);
    } catch (IOException e) {
      e.printStackTrace();
    }
    try {
      // This is ok.
      mapper.readValue(json, PropertyOnly.class);
    } catch (IOException e) {
      e.printStackTrace();
    }
    try {
      // This is ok.
      mapper.readValue(json, SecondArgument.class);
    } catch (IOException e) {
      e.printStackTrace();
    }
  }
}

Here is the output of the above.

com.fasterxml.jackson.databind.JsonMappingException: No suitable constructor found for type [simple type, class Example$None]: can not instantiate from JSON object (missing default constructor or creator, or perhaps need to add/enable type information?)
 at [Source: {"numbers":[1,2,3]}; line: 1, column: 2]
    at com.fasterxml.jackson.databind.JsonMappingException.from(JsonMappingException.java:148)
    at com.fasterxml.jackson.databind.deser.BeanDeserializerBase.deserializeFromObjectUsingNonDefault(BeanDeserializerBase.java:1107)
    at com.fasterxml.jackson.databind.deser.BeanDeserializer.deserializeFromObject(BeanDeserializer.java:294)
    at com.fasterxml.jackson.databind.deser.BeanDeserializer.deserialize(BeanDeserializer.java:131)
    at com.fasterxml.jackson.databind.ObjectMapper._readMapAndClose(ObjectMapper.java:3707)
    at com.fasterxml.jackson.databind.ObjectMapper.readValue(ObjectMapper.java:2719)
    at Example.main(Example.java:47) <5 internal calls>
com.fasterxml.jackson.databind.JsonMappingException: Can not deserialize instance of java.util.ArrayList out of FIELD_NAME token
 at [Source: {"numbers":[1,2,3]}; line: 1, column: 2]
    at com.fasterxml.jackson.databind.JsonMappingException.from(JsonMappingException.java:148)
    at com.fasterxml.jackson.databind.DeserializationContext.mappingException(DeserializationContext.java:850)
    at com.fasterxml.jackson.databind.DeserializationContext.mappingException(DeserializationContext.java:846)
    at com.fasterxml.jackson.databind.deser.std.CollectionDeserializer.handleNonArray(CollectionDeserializer.java:288)
    at com.fasterxml.jackson.databind.deser.std.CollectionDeserializer.deserialize(CollectionDeserializer.java:227)
    at com.fasterxml.jackson.databind.deser.std.CollectionDeserializer.deserialize(CollectionDeserializer.java:217)
    at com.fasterxml.jackson.databind.deser.std.CollectionDeserializer.deserialize(CollectionDeserializer.java:25)
    at com.fasterxml.jackson.databind.deser.BeanDeserializerBase.deserializeFromObjectUsingNonDefault(BeanDeserializerBase.java:1096)
    at com.fasterxml.jackson.databind.deser.BeanDeserializer.deserializeFromObject(BeanDeserializer.java:294)
    at com.fasterxml.jackson.databind.deser.BeanDeserializer.deserialize(BeanDeserializer.java:131)
    at com.fasterxml.jackson.databind.ObjectMapper._readMapAndClose(ObjectMapper.java:3707)
    at com.fasterxml.jackson.databind.ObjectMapper.readValue(ObjectMapper.java:2719)
    at Example.main(Example.java:52) <5 internal calls>
[1, 2, 3]
[1, 2, 3]
[1, 2, 3]

My real code most closely resembles the NoAnnotations example. Is it a bug that that example throws, or is that an expected limitation?

[cowtowncoder]

One-argument variant is bit problematic since it can ambiguous and could be taken mean either:

1. Delegating creator, in which case the whole JSON value is to be mapped into value of the argument, OR
2. Property-based creator, in which case JSON value must be an Object, argument must have name, and then property of that JSON Object with specified name is mapped to type of argument.

By default, assumption is that (1) is used, but explicit name (with @JsonProperty) should imply (2).
As to whether implicit name (one derived from bytecode with JDK 8) does anything could be argued, but I think it's just taken as (1).

But with 2.5 it is possible to explicit define type like so:

public class Pojo {
  @JsonCreator(mode=JsonCreator.Mode.PROPERTIES)
  public Pojo(String name) {
      ...
  }

so that it could be mapped from JSON like

{ "name" : "Bill" }

instead of what delegating constructor would expect:

"value"

[michaelhixson]

I take it that (1) as the default is what the stable release does now?

If so, or you don't want to revisit that decision for other reasons, is it possible for me to override the default @JsonCreator mode for a given ObjectMapper to be (2)? That happens to fit my own code much better. We're sharing a global singleton ObjectMapper already, so configuring that differently is no problem. I see (1) causing some confusion for us, because say we have:

Foo(int a, int b) { ... }

and later we decide we don't need b so we change it to:

Foo(int a) { ... }

it's not immediately obvious that, by removing that second parameter, we've fundamentally changed how JSON deserialization works for that class.

[cowtowncoder]

Yes, the description above was the intended behavior of 2.5.

Changing default mode is easiest done by overriding method:

    public JsonCreator.Mode findCreatorBinding(Annotated a)

in JacksonAnnotationIntrospector to return mode you want (default implementation just returns null meaning "use defaults"), and then configure mapper with it. ObjectMapper itself does not specify default behavior.

[michaelhixson]

Gotcha. Closing this since it's intended. Will use my own annotation introspector.

[lpandzic]

I've seen more than once questions regarding this issue and it would make sense to provide an API in the ParameterNamesAnnotationIntrospector to override this behavior so users don't have to override a method in JacksonAnnotationIntrospector.

The end goal, in the spirit of this module, would be to go even further and make that behavior the default and users that don't want this change can change it with the new API.

Opinions?

[cowtowncoder]

@lpandzic Just to make sure I understand the proposal, are you suggesting defaulting the mode for single-argument creator with implicit name to assume properties, instead of delegating?

[lpandzic]

In case of this module, yes. The module is still new so this might be the best time as ever.

[cowtowncoder]

I am looking at jackson-databind again, and I think there is one problem that prevents detection improvements. Intent is to allow detection of properties-based creator even for single-parameter case, as long as there is matching accessor (getter, field). Unfortunately what is happening is that information is missing, leading to detection failing.

[lpandzic]

Do you have a test so we can reproduce it in parameter names module?
Do you agree that we change the default behavior to use property based creator mode?

[cowtowncoder]

I don't have a specific test. Addition of an alternate constructor with similar visibility should be fine.

I am ok with the change of default behavior for this module. Just needs to be documented on README, explained what the options are.

As to value or boolean: I think I now think your idea of specifying value, and allowing null (to mean, just use defaults from core) make sense. The main change, I think, is just to use code from JacksonAnnotationIntrospector:

    @Override
    public JsonCreator.Mode findCreatorBinding(Annotated a) {
        JsonCreator ann = a.getAnnotation(JsonCreator.class);
        if (ann != null) {
          return ann.mode();
       }
       // otherwise use override we want
      return creatorMode;
    }

which while not ideal is, I think, better than making user change priority of introspectors or such.
Doing this will allow users to specify different mode in case they actually need delegating creator (if defaulting to properties), or vice versa.

So: with this check, and some unit testing to rule out possibility of exceptions with multiple non-annotated constructors I can merge the patch.

Thank you for helping getting this straightened and merged in 2.6.0.

[lpandzic]

I've changed the PR to use null value as default instead of JsonCreator.Mode.DEFAULT. It makes sense to let core decide default behavior instead of reimplementing it here.