@JsonNaming does not work with Java 16 record types #3102
Comments
|
Yes, alas, I think this is same as #2992 (and probably #2974). The problem is that linking of constructor properties is failing for some reason and so only mutator attached to underlying I plan on finally tackling bigger property introspection change for 2.13 (to solve the root cause), although if someone can figure out a patch for 2.12.x (with more targeted fix) that would be great too. |
|
I tried following the code in a debugger, and I saw the record constructor being processed and registered, with the raw record field names (the constructor parameter names). It looked like it was just missing the processing of the rename strategy. I couldn't follow the rest of the code, but it appears that it also registers the "renamed" the raw private fields inside the record class. Then during deserialization, it sees a snake case field, and since the constructor parameters aren't processed, it tries to set the field directly, which fails. |
|
Correct. The problem is due to Creator methods (and properties that are found via them) are handled separately due to historical reasons. If I remember this correctly if (but only if) constructor is explicitly annotated with So I think one work around, until fix, would be to add explicit, otherwise unnecessary declaration of Record constructor. I guess if above is correct, then a smaller fix within Jackson 2.12 would be to indicate that the main Record constructor was explicitly annotated -- one reason this wasn't done is because of possibility that user might designate an alternate constructor. But maybe it'd be possible to detect the case of no explicit annotations. |
|
I tried adding an explicit constructor with The only workaround I could find was the obvjous to declare the property mapping manually: |
|
Hmmh. Ok, I better add a new test for this fail, just in case. Explicit |
|
Yes, I can reproduce the issue. Also odd: exception message claims there are no known properties -- I get bit different one with JDK 14, will need to install JDK 16 to see if I can reproduce exception shown here. |
|
Ok. The odd "0 properties" was due to local changes (wrt attempt to prevent use of setters which has side effect of dropping properties); after removing that, I can reproduce the issue with JDK 16. I think I must have misremembered the logic: it must be that explicit naming of constructor properties is the only thing that establishes links, not constructor annotation itself. And obviously having to use that prevents any benefit of applying naming convention. |
|
Odd. Trying to add a reproduction with regular POJO, it looks like combination of I think this is because Record handling short-circuits otherwise default implicit-name access (since we know how to access them more reliably). So it might be possible to inject implicit names. That would solve this particular issue, although would not help the main use case (of not requiring bogus constructor to add |
cowtowncoder
removed
the
to-evaluate
|
I just ran into this without using any annotations, by configuring the jackson mapper like this (suppose this is well known as the annotation is doing the same, but I figured I'll leave the test case here): @Test
public void testSerializeDeserializeSnakeCaseRecord() throws JsonProcessingException {
final ObjectMapper mapper = new ObjectMapper()
.setPropertyNamingStrategy(PropertyNamingStrategies.SNAKE_CASE);
User user = new User("Obama", "Barack");
// serialization
final String output = mapper.writeValueAsString(user);
assertThat(output).contains("\"name\":\"Obama\"");
assertThat(output).contains("\"first_name\":\"Barack\"");
// deserialization
final User readUser = mapper.readValue(output, User.class);
assertThat(readUser.firstName()).isEqualTo("Barack");
assertThat(readUser.name()).isEqualTo("Obama");
}
public static record User(String name, /*@JsonProperty("first_name")*/ String firstName) {
}Works fine, when using the |
|
@cowtowncoder would it be possible to consider the change in FasterXML/jackson-future-ideas#61 (comment) for v2.14? |
|
@pjfanning Yes. I haven't had time to follow on those ideas but yes I'd be +1 for inclusion. |
|
I think it might be possible to sort out the issue with Record deserialization when naming strategies are involved by treating Records as a special case and changing it so that the properties are evaluated before the Record constructor is called. When there are no naming strategies, this already happens. When there is a naming strategy, the Record is constructed with null values and the code tries to use setters after the fact to set the right values (and refelction doesn't allow this for Records). |
|
@pjfanning At the high level, yes. Records should be handled in (more) special way. But aside from logics there is an existing problem wherein Creator properties (constructor detection) and "regular" property detection are done in somewhat wrong order and that is the big thing I hope to rewrite. Smaller tweaks to the existing implementation are much more complicated unfortunately; but rewrite is not a trivial task either. Renaming, in particular, was (if I remember correctly) victim of Creator not being found and linked early enough to have the logical creator properties be renamed; thereby only leaving Field-based property accessors in use (unliked with wrongly named creator properties). |
|
Hopefully this is related and might be helpful to others. They only way I can reliably get records to work is by turning off all Auto Detection ( Luckily you can make combined annotations that will turn off the auto detection. Really you just have to turn off I guess what I'm saying is OOB auto detect for records seems broken and there needs to be way to define accessors besides getters and fields. It seems like out of the box for records jackson should only care about the record components. |
|
@agentgt Yes, there is one fundamental implementation problem that causes this -- it is not mismatch between non-getter and getter as much as missing linkage between "Creator" parameters and "regular" property accessors (getters, setters). This is probably why annotations do not get properly considered for all accessors of a single logical property. I am hoping to eventually fix this, but unfortunately the change is to rewrite property introspection and I just haven't had time to allocate for this work. On plus side we have tests to verify fix, as well as guard against various regressions. In the meantime I appreciate your sharing of workarounds so others can use them too. |
Describe the bug
JsonNaming does not seem to be applied to Java 16 record types for deserialization.
Version information
2.12.2
To Reproduce
Simple unit test:
Failure stack:
The text was updated successfully, but these errors were encountered: