Auto-detection of constructor-based creator method skipped if there is an annotated factory-based creator method (regression from 2.11)

[hisener]

Describe the bug
First, I am not sure whether it's a bug or expected behavior, but I just wanted to open an issue for my understanding. Prior to Jackson 2.12.0, it was possible to bind a Number (e.g., 42) to a single field DTO (see example code below). Now, it throws MismatchedInputException with this message:

Cannot construct instance of `jackson211$ExampleDto$Json` (although at least one Creator exists):
no int/Int-argument constructor/factory method to deserialize from Number value (42) at [Source: (String)"42"; line: 1, column: 1]

Version information
2.12.0

To Reproduce
The code below can be used.

Some notes:

• I used jbang to switch between versions easily.
• The example Dto is the minimal version of an Immutables generated code.
• Jackson 2.11.3 uses the private constructor to deserialize Number to Dto object.

/// usr/bin/env jbang "$0" "$@" ; exit $?
//DEPS com.fasterxml.jackson.core:jackson-annotations:2.12.0
//DEPS com.fasterxml.jackson.core:jackson-databind:2.12.0

// It prints `true` on 2.11.3.
////DEPS com.fasterxml.jackson.core:jackson-annotations:2.11.3
////DEPS com.fasterxml.jackson.core:jackson-databind:2.11.3

import com.fasterxml.jackson.annotation.JsonAutoDetect;
import com.fasterxml.jackson.annotation.JsonCreator;
import com.fasterxml.jackson.annotation.JsonProperty;
import com.fasterxml.jackson.core.JsonProcessingException;
import com.fasterxml.jackson.databind.ObjectMapper;
import com.fasterxml.jackson.databind.annotation.JsonDeserialize;
import java.util.Objects;

public final class jackson211 {
    private static final ObjectMapper OBJECT_MAPPER = new ObjectMapper();

    public static void main(String... args) throws JsonProcessingException {
        int version = 1;
        ExampleDto deserialize = OBJECT_MAPPER.readValue(String.valueOf(version), ExampleDto.class);
        System.out.println(new ExampleDto(version).equals(deserialize));
    }

    static final class ExampleDto {
        private final int version;

        private ExampleDto(int version) {
            this.version = version;
        }

        @Override
        public boolean equals(Object o) {
            if (this == o) return true;
            if (o == null || getClass() != o.getClass()) return false;
            ExampleDto that = (ExampleDto) o;
            return version == that.version;
        }

        @Override
        public int hashCode() {
            return Objects.hash(version);
        }

        @JsonCreator(mode = JsonCreator.Mode.DELEGATING)
        static ExampleDto fromJson(Json json) {
            return new ExampleDto(json.version);
        }

        @JsonDeserialize
        @JsonAutoDetect(fieldVisibility = JsonAutoDetect.Visibility.NONE)
        static final class Json {
            int version;
            boolean versionIsSet;

            @JsonProperty
            public void setVersion(int version) {
                this.version = version;
                this.versionIsSet = true;
            }

            public int getVersion() {
                throw new UnsupportedOperationException();
            }
        }
    }
}

Expected behavior
As I wrote at the beginning, I am not sure whether it's a bug or expected behavior. If it's a bug then, the example code should print true instead of throwing an exception.

Additional context
N/A

[cowtowncoder]

My first reaction is that I have no idea how this could ever have worked, to be honest.
Assuming input is a JSON String like

"42"

and then with

        @JsonCreator(mode = JsonCreator.Mode.DELEGATING)
        static ExampleDto fromJson(Json json) {
            return new ExampleDto(json.version);

JSON String would need to be matching input for Json type; but it expects a JSON Object like

{ "version" : 42}

(although with coercion could also accept { "version" : "42" } )

I guess it is possible that formerly this constructor of ExampleDTO might have done it:

        private ExampleDto(int version) {  ... }

but it expects as int, not String.

I guess I could see that possibly working, and maybe even allowing coercion from String to int...

I'll leave this open to hopefully have another look in near future.

[hisener]

Assuming input is a JSON String like

"42"

In the given example, we use 42, not "42". That's what I meant by Number, but it looks like I copy-pasted the wrong exception message; sorry for the confusion. Updated the exception message.

Jackson 2.11.3

• 42 -> new ExampleDto(42)
• "42" -> (although at least one Creator exists): no String-argument constructor/factory method to deserialize from String value ('42')

Jackson 2.12.0

• 42 -> (although at least one Creator exists): no int/Int-argument constructor/factory method to deserialize from Number value (42)
• "42" -> (although at least one Creator exists): no String-argument constructor/factory method to deserialize from String value ('42')

I guess it is possible that formerly this constructor of ExampleDTO might have done it:

        private ExampleDto(int version) {  ... }

Indeed, it uses the private constructor to deserialize Number.

[cowtowncoder]

Ah! Thank you for clarification. That makes more sense. It may still be tricky wrt delegation aspects, but makes more sense regarding how it worked (and possibly should work).
Does addition of @JsonCreator(mode = Mode.DELEGATING) on private ExampleDto help? Visibility of constructors is bit of a gray area due to backwards compatibility -- in theory, private constructors would not be visible by default settings, I think, but in practice they are to support earlier behavior (if I remember this correctly).

[hisener]

Yes, adding @JsonCreator(mode = JsonCreator.Mode.DELEGATING) works in both 2.11 and 2.12. However, making the constructor public (without @JsonCreator annotation) doesn't help. @JsonCreator(mode = JsonCreator.Mode.DELEGATING) is still required even for public constructor.

[cowtowncoder]

Ok thank you for verifying these aspects. It sounds like there is an unintended change in behavior here, and I hope to look into it when I get a chance.

[cowtowncoder]

I think the problem is that logic is now expecting annotations for (about) everything if there is one annotation.
So for this test, commenting out annotation in

static ExampleDto fromJson(Json json) { ...

will actually return auto-detection of the constructor. But it might be possible to re-enable auto-detection of only some of delegating "legacy" creators (for int, String, long, boolean).

[cowtowncoder]

So, rewritten code in 2.12 (meant to work same way as before) had checks so that:

• Only check for implicit (non-annotated) Constructors if neither annotated factory methods NOR annotated constructors found

but apparently logic earlier was

• Only check for implicit (non-annotated) Constructors if no annotated constructors found

so that existence of annotated factory method was the difference. Will change code to use older logic, to try to keep backwards-compatibility. This is bit tricky going forward as definition is not very crisp, but seems like the right thing to do for now.