`ConstructorDetector.USE_PROPERTIES_BASED` does not work with multiple constructors since 2.18

[Saljack]

Search before asking

• I searched in the issues and found nothing similar.

Describe the bug

I have this class:

public class Foo {
    private String id;
    private String name;

    public Foo() {
    }

    public Foo(String id) {
      this.id = id;
    }

    // getter setter omitted
} 

I use ConstructorDetector.USE_PROPERTIES_BASED configuration.

JsonMapper objectMapper = JsonMapper.builder().constructorDetector(ConstructorDetector.USE_PROPERTIES_BASED).build();

Since Jackson 2.18.0 I am no able to deserialize any JSON to this object. It worked in previous versions.
The error is:

com.fasterxml.jackson.databind.exc.InvalidDefinitionException: 
Invalid type definition for type `com.example.Foo`: Argument #0 of Creator [constructor for `com.example.Foo` (1 arg), annotations: [null] has no property name (and is not Injectable): can not use as property-based Creator
 at [Source: REDACTED (`StreamReadFeature.INCLUDE_SOURCE_IN_LOCATION` disabled); line: 1, column: 1]
	at com.fasterxml.jackson.databind.exc.InvalidDefinitionException.from(InvalidDefinitionException.java:62)
	at com.fasterxml.jackson.databind.DeserializationContext.reportBadTypeDefinition(DeserializationContext.java:1866)
	at com.fasterxml.jackson.databind.deser.BasicDeserializerFactory._addSelectedPropertiesBasedCreator(BasicDeserializerFactory.java:549)
	at com.fasterxml.jackson.databind.deser.BasicDeserializerFactory._constructDefaultValueInstantiator(BasicDeserializerFactory.java:269)
	at com.fasterxml.jackson.databind.deser.BasicDeserializerFactory.findValueInstantiator(BasicDeserializerFactory.java:219)
	at com.fasterxml.jackson.databind.deser.BeanDeserializerFactory.buildBeanDeserializer(BeanDeserializerFactory.java:262)
	at com.fasterxml.jackson.databind.deser.BeanDeserializerFactory.createBeanDeserializer(BeanDeserializerFactory.java:151)
	at com.fasterxml.jackson.databind.deser.DeserializerCache._createDeserializer2(DeserializerCache.java:471)
	at com.fasterxml.jackson.databind.deser.DeserializerCache._createDeserializer(DeserializerCache.java:415)
	at com.fasterxml.jackson.databind.deser.DeserializerCache._createAndCache2(DeserializerCache.java:317)
	at com.fasterxml.jackson.databind.deser.DeserializerCache._createAndCacheValueDeserializer(DeserializerCache.java:284)
	at com.fasterxml.jackson.databind.deser.DeserializerCache.findValueDeserializer(DeserializerCache.java:174)
	at com.fasterxml.jackson.databind.DeserializationContext.findRootValueDeserializer(DeserializationContext.java:669)
	at com.fasterxml.jackson.databind.ObjectMapper._findRootDeserializer(ObjectMapper.java:5048)
	at com.fasterxml.jackson.databind.ObjectMapper._readMapAndClose(ObjectMapper.java:4918)
	at com.fasterxml.jackson.databind.ObjectMapper.readValue(ObjectMapper.java:3860)
	at com.fasterxml.jackson.databind.ObjectMapper.readValue(ObjectMapper.java:3828)

It does not deserialize these JSONs

{}
{"id": "something"}
{"id": "something", "name": "name"}
"something"

I would expect that it will work at least for {} (no args constructor) and {"id": "something"} (1 arg constructor with the same parameter name).

Version Information

2.18.0, 2.18.1 and 2.18.2 is affected
2.17.2 works

Reproduction

<-- Any of the following

1. Brief code sample/snippet: include here in preformatted/code section
2. Longer example stored somewhere else (diff repo, snippet), add a link
3. Textual explanation: include here
   -->

public class Foo {
    private String id;
    private String name;

    public Foo() {
    }

    public Foo(String id) {
        this.id = id;
    }

    // getter setter omitted
   
   public static void main(String[] args) {
       // Test
       JsonMapper objectMapper = JsonMapper.builder()
          .constructorDetector(ConstructorDetector.USE_PROPERTIES_BASED).build();

        Foo foo = objectMapper.readValue("{}", Foo.class);
   }

}

Expected behavior

I would expect that it will deserialize JSON instead of throwing an exception.

Additional context

The object for deserialization is an external class and I cannot change it. I can use mixin to override it but I am afraid that this issue can happen with other external objects only if someone adds two constructors.s

[JooHyukKim]

Filed #4861, hopefully enough to fix the issue.

I excluded case with "something", it does not seem to work in 2.17 as well.
@Saljack Lemme know if you think we should include "something" case.

[cowtowncoder]

@Saljack quick note: please do not omit getters and setters for brevity -- their existence is important wrt property introspection. Reproduction should be faithful to actual use case.

[cowtowncoder]

Actually, I am not sure this should work quite the way that submitter expects.
Automatic (no-annotation) detection is meant to only work if there is EXACTLY one constructor (including no 0-args constructor). But given there is 0-args constructor, that should instead be used.

Note, too, that Jackson does not select multiple properties-based constructors; they must resolve into to one. So there is no dynamic choice from { } into 0-args and others into 1-arg.

In this case it does not seem to matter as the end result is about same.

So I guess I do not quite understand why auto-detection works.

[cowtowncoder]

@JooHyukKim plain "something" case cannot work (and should not work) without explicit delegating Creator so no it should not be tested for.

But support can be added with something like

@JsonCreator
public static Foo valueOf(String str) {
   return new Foo(str);
}

or @JsonCreator(mode = JsonCreator.Mode.DELEGATING) annotated Constructor.

But would not be auto-detected.

[Saljack]

Sorry for confusion. Let me try to explain. I use ConstructorDetector.USE_PROPERTIES_BASED be cause I do not want "something" to be deserialize into this object (it is with other ConstructorDetector options because one arg string constructor is used).
I added "something" just to show that anything works if there are multiple constructors and I do not expect (or I do not want) to deserialize it.

My expectation is that Jackson uses no args constructor if there are multiple constructors and there is no @JsonCreator annotation. The whole problem is that you cannot deserialize into this class if you use ConstructorDetector.USE_PROPERTIES_BASED.

[cowtowncoder]

@Saljack Ok, good thank you for adding more info -- that's how I think things should actually work. I am not 100% sure why auto-detection kicks in, need to dig deeper.

[cowtowncoder]

Hmmh. Ok, no, for 2.18 this was changed slightly so that an implicit properties-based Creator MAY be detected not just if there's no 0-args constructor but also if ConstructorDetector.USE_PROPERTIES_BASED is enabled.

In POJOPropertiesCollector 718:

        // One more thing: if neither explicit (constructor or factory) nor
        // canonical (constructor?), consider implicit Constructor with
        // all named.
        final ConstructorDetector ctorDetector = _config.getConstructorDetector();
        if (!creators.hasPropertiesBasedOrDelegating()
                && !ctorDetector.requireCtorAnnotation()) {
            // But only if no Default (0-args) constructor available OR if we are configured
            // to prefer properties-based Creators
            if ((_classDef.getDefaultConstructor() == null)
                    || ctorDetector.singleArgCreatorDefaultsToProperties()) {
                _addImplicitConstructor(creators, constructors, props);
            }
        }

[cowtowncoder]

Ok so what I think is specifically needed, to address the failure is this: we can only auto-detect Properties-based Constructor if (and only if) it:

• Has either Injectable definition OR implicit name for EVERY parameter

This is being checked for multi-parameter case, but not for 1-parameter.
I will add that check.

There will still be auto-detection, however; I will not change that -- but in this case without parameter-names module it won't kick in.