Records with additional constructors failed to deserialize

[GeorgiPetkov]

Describe the bug
Records are not deserialized as before due to error:

com.fasterxml.jackson.databind.exc.InvalidDefinitionException: Conflicting property-based creators: already had implicitly discovered creator [constructor for RecordsDeserializationTest$TestRecord (1 arg), annotations: [null], encountered another: [constructor for RecordsDeserializationTest$TestRecord (2 args), annotations: [null]
at [Source: (String)"{"a":"a", "b":"b"}"; line: 1, column: 1]

See the test below for the exact setup.

Version information
Which Jackson version(s) was this for?
2.15.0

To Reproduce

import com.fasterxml.jackson.core.JsonProcessingException;
import com.fasterxml.jackson.databind.ObjectMapper;
import com.fasterxml.jackson.databind.cfg.ConstructorDetector;
import com.fasterxml.jackson.databind.json.JsonMapper;
import com.fasterxml.jackson.module.paramnames.ParameterNamesModule;
import org.junit.jupiter.api.Test;

import static org.junit.jupiter.api.Assertions.assertEquals;

class RecordsDeserializationTest {

    private final ObjectMapper objectMapper = JsonMapper.builder()
            // commenting only this fixes the problem
            .addModules(new ParameterNamesModule())
            // commenting only this fixes the problem
            .constructorDetector(ConstructorDetector.USE_PROPERTIES_BASED)
            .build();

    record TestRecord(String a, String b) {

        // uncommenting only this fixes the problem
        // @JsonIgnore
        TestRecord(String c) {
            this(c, c);
        }
    }

    @Test
    void testDeserialization() throws JsonProcessingException {
        assertEquals(
                new TestRecord("a", "b"),
                objectMapper.readValue("{\"a\":\"a\", \"b\":\"b\"}", TestRecord.class));
    }
}

Expected behavior
The exact combination of configuration is not working in version 2.15.0.
There are 4 ways to fix it:

• remove module ParameterNamesModule
• don't use USE_PROPERTIES_BASED constructor detection
• add @JsonIgnore annotation to the extra constructor
• use version 2.13.4 on which this works

Additional context
The caveat here is that I can't use explicit @JsonCreator annotation on the primary constructor on a record (or I have to implement it just to add the annotation). Ideally, you should restore the old behavior. The @JsonIgnore approach seems like a hack to me.

[pjfanning]

What happens if you use v2.15.2? v2.15.0 introduced some big changes for Records and some issues occurred. Some of those have been fixed since.

[yihtserns]

Still the same in v2.15.2. I'll propose a PR to fix this since it is caused by #3724.

[cowtowncoder]

Yes, I think this case should work. Ideally it would just work without changes, using the Canonical constructor and not being bothered about secondary one.

@yihtserns Why is the secondary constructor even considered here? I guess the problem is two-fold:

1. We do not want to mark Canonical constructor as explicit (annotated) to make it possible to replace with something that is, but
2. We do want to auto-detect public constructors in general.

In this case perhaps unification of POJO handling (POJOs do not have canonical constructors) and Records caused more problems than worth... again. :-/

Anyway: I hope to review the PR and see if that works. I wish it didn't have to change defaulting but would rather handle preference/priority of 3 levels we have (highest: annotated/explicit; medium: canonical; lowest: visible/public non-canonical).

[yihtserns]

Why is the secondary constructor even considered here?

Because they may be considered for implicit delegating creator:

jackson-databind/src/test-jdk14/java/com/fasterxml/jackson/databind/records/RecordImplicitCreatorsTest.java

Lines 53 to 75 in 2cea7c9

	/**
	* Similar to:
	* <pre>
	* public class MyBean {
	* ...
	* // Single-arg constructor used by delegating creator.
	* public MyBean(int id) { ... }
	*
	* // No-arg constructor used by properties-based creator.
	* public MyBean() {}
	*
	* // Setters used by properties-based creator.
	* public void setId(int id) { ... }
	* public void setName(String name) { ... }
	* }
	* </pre>
	*/
	record RecordWithAltSingleValueConstructor(int id, String name) {
	public RecordWithAltSingleValueConstructor(int id) {
	this(id, "SingleValueConstructor");
	}
	}

jackson-databind/src/test-jdk14/java/com/fasterxml/jackson/databind/records/RecordImplicitCreatorsTest.java

Lines 277 to 298 in 2cea7c9

	/*
	/**********************************************************************
	/* Test methods, implicit properties-based + delegating constructor
	/**********************************************************************
	*/
	public void testDeserializeUsingImplicitPropertiesBasedConstructor() throws Exception {
	RecordWithAltSingleValueConstructor value = MAPPER.readValue(
	"{\"id\":123,\"name\":\"PropertiesBasedConstructor\"}",
	RecordWithAltSingleValueConstructor.class);
	assertEquals(new RecordWithAltSingleValueConstructor(123, "PropertiesBasedConstructor"), value);
	}
	/**
	* @see #testDeserializeUsingImplicitSingleValueConstructor()
	*/
	public void testDeserializeUsingImplicitDelegatingConstructor() throws Exception {
	RecordWithAltSingleValueConstructor value = MAPPER.readValue("123", RecordWithAltSingleValueConstructor.class);
	assertEquals(new RecordWithAltSingleValueConstructor(123, "SingleValueConstructor"), value);
	}

In this case perhaps unification of POJO handling (POJOs do not have canonical constructors) and Records caused more problems than worth... again. :-/

Yeah sorry about that. I'm regretting it a lot.

[cowtowncoder]

@yihtserns Don't regret too much; it wasn't possible to really know all the issues... partly due to limited test coverage.