Creator lookup fails with InvalidDefinitionException for conflict between single-double/single-Double arg constructor

[cdadac]

Describe the bug

We are using Jackson to (de)serialize POJO's representing our public API in a message oriented middleware (Apache Artemis).
We are using JsonProperty annotations to explicitly annotate getters/setters for properties to use during (de)serialization and each POJO defines at least one public no-arg constructor to use for object creation upon deserialization.
Some POJO's also include additional constructors for convenience reasons, which are not intended to be used by Jackson during deserialization.
After upgrading to 2.12.1 deserialization fails from some of those classes with the following Exception:

com.fasterxml.jackson.databind.exc.InvalidDefinitionException: Conflicting from-double creators: already had implicitly discovered creator [constructor for DecVector (1 arg), annotations: [null], encountered another: [constructor for DecVector (1 arg), annotations: [null]
 at [Source: (String)"{"v":[1.0,2.0,3.0]}"; line: 1, column: 1]
	at com.fasterxml.jackson.databind.exc.InvalidDefinitionException.from(InvalidDefinitionException.java:62)
	at com.fasterxml.jackson.databind.deser.BeanDeserializerFactory.buildBeanDeserializer(BeanDeserializerFactory.java:268)
	at com.fasterxml.jackson.databind.deser.BeanDeserializerFactory.createBeanDeserializer(BeanDeserializerFactory.java:150)
	at com.fasterxml.jackson.databind.deser.DeserializerCache._createDeserializer2(DeserializerCache.java:414)
	at com.fasterxml.jackson.databind.deser.DeserializerCache._createDeserializer(DeserializerCache.java:349)
	at com.fasterxml.jackson.databind.deser.DeserializerCache._createAndCache2(DeserializerCache.java:264)
	at com.fasterxml.jackson.databind.deser.DeserializerCache._createAndCacheValueDeserializer(DeserializerCache.java:244)
	at com.fasterxml.jackson.databind.deser.DeserializerCache.findValueDeserializer(DeserializerCache.java:142)
	at com.fasterxml.jackson.databind.DeserializationContext.findRootValueDeserializer(DeserializationContext.java:591)
	at com.fasterxml.jackson.databind.ObjectMapper._findRootDeserializer(ObjectMapper.java:4733)
	at com.fasterxml.jackson.databind.ObjectMapper._readMapAndClose(ObjectMapper.java:4594)
	at com.fasterxml.jackson.databind.ObjectMapper.readValue(ObjectMapper.java:3548)
	at com.fasterxml.jackson.databind.ObjectMapper.readValue(ObjectMapper.java:3516)

The Exception is thrown by method _reportDuplicateCreator in the CreatorCollector class (line 335) which was added in 2.12

Version information

problem occurs in 2.12.1
after upgrade from 2.10.4

To Reproduce

The following code snippet shows a (simplfied) version of one of the POJOs where this problem occurs:

@JsonIgnoreProperties(ignoreUnknown = true) 
public class DecVector {

  private List<Double> elems;
  
  @JsonCreator
  public DecVector() {
    super();
  }
  
  public DecVector(double[]elems) {
    this.elems = new ArrayList<>(elems.length);
    for (double e : elems) {
      this.elems.add(Double.valueOf(e));
    }
  }
  
  public DecVector(Double[]elems) {
    this.elems = Arrays.asList(elems);
  }
  
  public DecVector(List<Double> elems) {
    this.elems = elems;
  }
  
  public DecVector(double elem) {
    this.elems = new ArrayList<>(1);
    this.elems.add(Double.valueOf(elem));
  }
  
  public DecVector(Double elem) {
    this.elems = Arrays.asList(elem);
  }
  
  @JsonProperty(required = false, value = "v", index = 1)
  @JsonInclude(value = Include.NON_NULL)
  public List<Double> getValues() {
    return elems;
  }
  public void setValues(List<Double> list) {
    this.elems = list;
  }
}

The following simple test case (using a vanilla ObjectMapper instance) works fine in 2.10.4 but throws the Exception mentioned above in 2.12.1:

  @Test
  public void testMultipleConstructor() throws Exception {
    ObjectMapper mapper = new ObjectMapper();
    DecVector vector = new DecVector(new double[] {1,2,3} );
    String result = mapper.writeValueAsString(vector);
    DecVector deser = mapper.readValue(result, DecVector.class);
    assertEquals(vector.getValues(), deser.getValues());
  }

The explicit JsonCreator annotation in the example above was added for testing purposes (with 2.10.4 we did not explicitly annotate constructors at all!) but it makes no difference if the default constructor is annotated or not.

Expected behavior

In case multiple constructors are present Jackson should default to using the parameterless default constructor if no other creator is explicitly annotated.
In case one constructor is explicitly annotated using JsonCreator Jackson should use the annotated constructor instead.

Additional context

I think the example above represents a a rather common/basic use case for (de)serialization using Jackson and should work out of the box without additional configuration.
Per default we use JSON as target format for serialized objects, however we also support Protobuf as message format, therefore the JsonProperty annotation in the example also specifies an explicit index.

[cowtowncoder]

Hmmh. Interesting. Without looking into details, I am guessing that the conflict found -- and previously erroneously missed (as per Jackson's intent, not yours -- is between

  public DecVector(double elem) { }
  public DecVector(Double elem) { }

which would be ambiguous. Previously one of them would have been arbitrarily selected (depending on order in which JDK happens to expose them, undefined although traditionally tends to be declaration order).

Discovery of a small set of single-argument delegating creators is a sort of legacy feature from Jackson 1.0 era (before introduction of @JsonCreator).

So... the fix itself was intentional, but due to long history of it not being detected I can see why this is problematic; especially given that you do not actually need either. Unfortunately trade-off here (as usual) is between surfacing a likely problem early, at point where it is not known if there is a problem, vs. indicating it only if known (in this case that would be deferral of selection of constructor until one is actually needed, when seeing JSON floating-point number value).

To prevent discovery of such constructors it is possible to change auto-detection visibility level with something like:

@JsonAutoDetect(creatorVisibility = Visibility.NONE)

(or equivalent method in ObjectMapper).

This would not change handling of 0-args constructor which will always be detected, and so annotation does not (currently) matter. Similarly you can add @JsonIgnore on one of constructors to work around the issue for now.

Now... to solve the problem otherwise requires some thinking; especially if resolution should go in a patch release (2.12.2 or later), due to backwards compatibility. I can all but guarantee that most changes would result someone else's use case breaking somewhere. :-(

First, existence and possible annotation of 0-args constructor is unfortunately not something we can use -- this constructor is only used with Object values, and double/Double taking constructor does not overlaps as it is only used with (json) Floating-point value input.

But I think another option that might work better would be to figure out how to resolve this particular case of wrapper/primitive value (assuming this is what triggers the problem which I can verify).

Question then is... which one should have precedence? My gut feeling is that it should be primitive value (double), but I suspect the important thing is that the choice is stable and defined, not necessarily which particular one.

[cowtowncoder]

Quick note: test seems to pass on 2.11(.4), fail on 2.12.2-SNAPSHOT.

Also, forgot to start my initial comment with an important thing: thank you @perdurabo476 for reporting this and other issues! I hope we can address them; at this point I am trying to finalize 2.12.2 since there are a few important fixes in already.

[cowtowncoder]

Implemented so that in case of 2 potentially conflicting constructors (usually implicit), where pairing is primitive/wrapper, primitive one is selected. This resolves reported problem as well as a few other possible kinds (wrt int/Integer, boolean/Boolean, long/Long).