Duplicate creator property "b" (index 0 vs 1) on simple java record

[richard-melvin]

Search before asking

• I searched in the issues and found nothing similar.

Describe the bug

Using jackson-databind 2.18.3 and a java record like:

    record Dummy(
            
            @JsonProperty(value = "a", access = JsonProperty.Access.READ_ONLY) String a,

            @JsonProperty(value = "b", access = JsonProperty.Access.READ_ONLY) String b) {
    }
    

then reading a value fails with:

com.fasterxml.jackson.databind.exc.InvalidDefinitionException: Duplicate creator property "b" (index 0 vs 1) for type `com.cgi.tf.openapi.test.JacksonTest$Dummy` 
 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.deser.BeanDeserializerFactory.buildBeanDeserializer(BeanDeserializerFactory.java:269)
	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)

Changing most things about the recod causes it to work . This is a minimised version of a fuller realistic case.

Version Information

No response

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
   -->

// Your code here

Expected behavior

No response

Additional context

No response

[richard-melvin]

Full reproduction test case

import org.junit.jupiter.api.Test;

import com.fasterxml.jackson.annotation.JsonProperty;
import com.fasterxml.jackson.databind.ObjectMapper;
import com.fasterxml.jackson.databind.json.JsonMapper;

public class JacksonTest {

    record Dummy(
            @JsonProperty(value = "a", access = JsonProperty.Access.READ_ONLY) String a,
            @JsonProperty(value = "b", access = JsonProperty.Access.READ_ONLY) String b) {
    }

    @Test
    void testWithDummy() throws Exception {
        
        ObjectMapper objectMapper = JsonMapper.builder().build();
        String string = objectMapper.writeValueAsString(new Dummy ("hello", "world"));
        objectMapper.readerFor(Dummy.class).readValue(string);
    } 
}

[cowtowncoder]

Thank you for reporting this @richard-melvin -- looks like a straight bug. Although your Record definition looks odd: what is the goal here wrt JsonProperty.Access.READ_ONLY? Wouldn't that make it un-deserializable (read and write are named in bit confusing way)?
What would you expected to happen -- unit test does not assert actual values from readValue() or JSON serialized.

[richard-melvin]

I guess the unit test woudl fail anyway if it didn't hit the bug - pretend it was based on a literal JSON string rather than a round trip.

The code is a redacted version of somehting much more complex that does have reasons for being that way...

[yihtserns]

Version	Ser result	Deser result
2.14.0	{"a":"hello","b":"world"}	❗ Dummy[a=hello, b=world]
2.14.2	{"a":"hello","b":"world"}	❗ Dummy[a=hello, b=world]
2.15.0	{"a":"hello","b":"world"}	❗ Dummy[a=hello, b=world]
2.15.4	{"a":"hello","b":"world"}	❗ Dummy[a=hello, b=world]
2.16.0	{"a":"hello","b":"world"}	❗ Dummy[a=hello, b=world]
2.16.2	{"a":"hello","b":"world"}	❗ Dummy[a=hello, b=world]
2.17.0	{"a":"hello","b":"world"}	❗ Dummy[a=hello, b=world]
2.17.3	{"a":"hello","b":"world"}	❗ Dummy[a=hello, b=world]
2.18.0	{"b":"world","a":"hello"}	💥 Error Duplicate creator property "b" (index 0 vs 1)...
2.18.3	{"b":"world","a":"hello"}	💥 Error Duplicate creator property "b" (index 0 vs 1)...

While it only explodes starting from 2.18.x, apparently "READ_ONLY" never worked for Records all these while...

UPDATE

Oh this is that edge case that I reported before: #4628.

It only happens if JsonProperty.value is the same as the Record's component/field name - remove the .value and it should work:

record Dummy(
            @JsonProperty(/* value = "a",*/ access = JsonProperty.Access.READ_ONLY) String a,
            @JsonProperty(/* value = "b",*/ access = JsonProperty.Access.READ_ONLY) String b) {
}

(In that ticket, I wrote "seems to be an edge case that user wouldn't/shouldn't encounter"... LOL 😭)

[pjfanning]

Best to close as dup of #4628

[cowtowncoder]

Hmmh. Not sure -- while #4628 is older, this seems better written, for actually encountered issue?

[yihtserns]

@richard-melvin just to be clear, Records + @JsonProperty.access=READ_ONLY + @JsonProperty.value=<same as Record component/field name> has never worked:

• Before 2.18.x, it silently fails: the Record is instantiated using the fields provided in the JSON (i.e. NOT read only).
• In 2.18.x, it noisily fails i.e. by throwing weird error.

So:

• If you use pre-2.18.x Jackson, you should remove any @JsonProperty.value that has same value as the Record component/field name to make it truly read only.
• If you use 2.18.x Jackson, you should also remove any @JsonProperty.value so that the error won't happen.

[cowtowncoder]

just to be clear, Records + @JsonProperty.access=READ_ONLY + @JsonProperty.value=<same as Record component/field name> has never worked:

Agreed.

I still don't quite get what the intent here was anyway: it would seem that deserialization would only succeed passing nulls for both properties...

[richard-melvin]

It comes from code generated from an openapi spec, using the 'read-only' feature:

https://swagger.io/docs/specification/v3_0/data-models/data-types/

For such a record,m round-trip serialisation obviously wouldn't wortk, but it doesn;t need to; REST in ingherently directional. A read-only field is only set by the server, and read by the client.

[pjfanning]

@richard-melvin in Jersey/OpenAPI scenario, what values should be put in the record for the read-only fields? Do you expect nulls? Are you going to handle merging the input from the client and merge it with the server side value?

[richard-melvin]

The use case is for a java openapi client. When serialising, you omit the values annotated as read-on;ly. When deserialising, they must be present.

Sp[ecifically, it is what is generated by https://openapiprocessor.io/oap/home.html

Seems straightforward rto me, what else is the annotation supposed to for?

[cowtowncoder]

@richard-melvin If so, I think this is incorrect usage, what you really want is WRITE_ONLY.
As per Javadocs (https://javadoc.io/doc/com.fasterxml.jackson.core/jackson-annotations/latest/com.fasterxml.jackson.annotation/com/fasterxml/jackson/annotation/JsonProperty.Access.html):

        /**
         * Access setting that means that the property may only be read for serialization
         * (value accessed via "getter" Method, or read from Field)
         * but not written (set) during deserialization.
         * Put another way, this would reflect "read-only POJO", in which value contained
         * may be read but not written/set.
         */
        READ_ONLY,

        /**
         * Access setting that means that the property may only be written (set)
         * as part of deserialization (using "setter" method, or assigning to Field,
         * or passed as Creator argument)
         * but will not be read (get) for serialization, that is, the value of the property
         * is not included in serialization.
         */
        WRITE_ONLY,

Naming here is bit unfortunate as read/write direction depends on how you look at it -- and I can see how Jackson's choice may be non-intuitive. Regardless annotation usage seems incorrect.

Beside that, error message is bad and behavior should be to pass in nulls in this specific configuration.

[cowtowncoder]

Not actually sure this is same as #4628, re-opening, to add failing test.