Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
1 task done
richard-melvin opened this issue Mar 26, 2025 · 22 comments
Closed
1 task done
Labels
2.18 Issues planned at 2.18 or later has-failing-test Indicates that there exists a test case (under `failing/`) to reproduce the issue
Milestone

Comments

@richard-melvin
Copy link

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 richard-melvin added the to-evaluate Issue that has been received but not yet evaluated label Mar 26, 2025
@richard-melvin
Copy link
Author

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
Copy link
Member

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
Copy link
Author

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
Copy link
Contributor

yihtserns commented Mar 27, 2025

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
Copy link
Member

Best to close as dup of #4628

@cowtowncoder
Copy link
Member

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

@yihtserns
Copy link
Contributor

@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
Copy link
Member

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
Copy link
Author

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
Copy link
Member

@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
Copy link
Author

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
Copy link
Member

cowtowncoder commented Apr 1, 2025

@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
Copy link
Member

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

@cowtowncoder cowtowncoder reopened this Apr 2, 2025
cowtowncoder added a commit that referenced this issue Apr 2, 2025
@cowtowncoder
Copy link
Member

Just in case this wasn't clear: equivalent POJO case works as expected, no exception, a and b passed as nulls.

@cowtowncoder
Copy link
Member

Adding test via #5061 -- this is tricky as almost all the way parameters "a" and "b" are passed just fine -- until at some point there are two "b"s. Very odd...

cowtowncoder added a commit that referenced this issue Apr 3, 2025
@cowtowncoder cowtowncoder added has-failing-test Indicates that there exists a test case (under `failing/`) to reproduce the issue and removed to-evaluate Issue that has been received but not yet evaluated labels Apr 3, 2025
@cowtowncoder
Copy link
Member

Added test; could see where name duplication is but not where it comes from. Yet.

@cowtowncoder cowtowncoder added the 2.18 Issues planned at 2.18 or later label Apr 4, 2025
cowtowncoder added a commit that referenced this issue Apr 4, 2025
@cowtowncoder cowtowncoder added 2.19 Issues planned at 2.19 or later 2.18 Issues planned at 2.18 or later and removed 2.18 Issues planned at 2.18 or later 2.19 Issues planned at 2.19 or later labels Apr 6, 2025
@yihtserns
Copy link
Contributor

With:

record Dummy(
        @JsonProperty(value = "a", access = READ_ONLY) String a,
        @JsonProperty(value = "b", access = READ_ONLY) String b) {
}
Operation Before 2.18.0 2.18.0-2.18.3 2.18.4-SNAPSHOT
Serialization: objectMapper.writeValueAsString(new Dummy("hello", "world")) {"a":"hello","b":"world"} {"b":"world","a":"hello"} {"a":"hello","b":"world"}
Deserialization: objectMapper.readValue("{\"a\":\"hello\",\"b\":\"world\"}", Dummy.class) Dummy[a=hello, b=world] InvalidDefinitionException Dummy[a=null, b=null]

@richard-melvin you said:

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.

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.

I don't really understand what you mean, but do check the behaviours documented in the table above - specifically Deserialization + 2.18.4-SNAPSHOT to make sure your usage/scenario is not broken by the fix.

@cowtowncoder
Copy link
Member

@yihtserns It seemed to be his use of READ_ONLY itself was incorrect (needs to use WRITE_ONLY).

@yihtserns
Copy link
Contributor

yihtserns commented Apr 7, 2025

@cowtowncoder not entirely sure about that since I can see a few different ways to interpret his statements... 😓

I can't afford to spend time trying to get on the same page as him, so decided to do a "here's the outcome, please check for yourself #SelfServicePlease".

@cowtowncoder
Copy link
Member

cowtowncoder commented Apr 7, 2025

@yihtserns Fair. At any rate, underlying issue from Jackson perspective is resolved as I understand.
Let's hope it will help user with their goal, whatever it actually is :)

@richard-melvin
Copy link
Author

yeah it looks fine

@cowtowncoder
Copy link
Member

cowtowncoder commented Apr 8, 2025

The fix is included in 2.19.0-rc2 released yesterday, fwtw. Will be in 2.19.0 as well as eventual 2.18.4

Also: MapperFeature.INVERSE_READ_WRITE_ACCESS is added via #2951 which allows "swapping" meaning of READ_ONLY and WRITE_ONLY -- included for 2.19.
This might be handy setting as well.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2.18 Issues planned at 2.18 or later has-failing-test Indicates that there exists a test case (under `failing/`) to reproduce the issue
Projects
None yet
Development

No branches or pull requests

4 participants