jackson-databind: `@JsonNaming` does not work with Records

@cgpassante Unfortunately there is no concrete idea: problem is that it would be fixable as part of big rewrite of property introspection. But I have not found enough contiguous time to spend on tackling this (I only work on Jackson during my spare time): it could be that I might be able to do that within next month if something opened up. And yet with all the other things going on, it may be that it won’t happen this year.

What I do not want to try is a one-off hack to work around just this problem since that tends to complicate eventual rewrite.

I agree that this problem area – proper support of Java Records – is VERY important, and I try my best to find time to resolve it properly.

Great. I will only use my fix will until 2.13. Here is a version that does not modify in place:

package com.examaple.jackson;

import com.fasterxml.jackson.databind.BeanDescription;
import com.fasterxml.jackson.databind.DeserializationConfig;
import com.fasterxml.jackson.databind.deser.SettableBeanProperty;
import com.fasterxml.jackson.databind.deser.ValueInstantiator;
import com.fasterxml.jackson.databind.deser.ValueInstantiators;
import com.fasterxml.jackson.databind.deser.std.StdValueInstantiator;
import com.fasterxml.jackson.databind.introspect.BeanPropertyDefinition;
import com.fasterxml.jackson.databind.module.SimpleModule;

import java.util.Arrays;
import java.util.Map;
import java.util.function.Function;
import java.util.stream.Collectors;

public class RecordNamingStrategyPatchModule extends SimpleModule {

    @Override
    public void setupModule(SetupContext context) {
        context.addValueInstantiators(new ValueInstantiatorsModifier());
        super.setupModule(context);
    }

    /**
     * Remove when the following issue is resolved: 
     * <a href="https://github.com/FasterXML/jackson-databind/issues/2992">Properties naming strategy do not work with Record #2992</a>
     */
    private static class ValueInstantiatorsModifier extends ValueInstantiators.Base {
        @Override
        public ValueInstantiator findValueInstantiator(
                DeserializationConfig config, BeanDescription beanDesc, ValueInstantiator defaultInstantiator
        ) {
            if (!beanDesc.getBeanClass().isRecord() || !(defaultInstantiator instanceof StdValueInstantiator) || !defaultInstantiator.canCreateFromObjectWith()) {
                return defaultInstantiator;
            }
            Map<String, BeanPropertyDefinition> map = beanDesc.findProperties().stream().collect(Collectors.toMap(p -> p.getInternalName(), Function.identity()));
            SettableBeanProperty[] renamedConstructorArgs = Arrays.stream(defaultInstantiator.getFromObjectArguments(config))
                    .map(p -> {
                        BeanPropertyDefinition prop = map.get(p.getName());
                        return prop != null ? p.withName(prop.getFullName()) : p;
                    })
                    .toArray(SettableBeanProperty[]::new);

            return new PatchedValueInstantiator((StdValueInstantiator) defaultInstantiator, renamedConstructorArgs);
        }
    }

    private static class PatchedValueInstantiator extends StdValueInstantiator {

        protected PatchedValueInstantiator(StdValueInstantiator src, SettableBeanProperty[] constructorArguments) {
            super(src);
            _constructorArguments = constructorArguments;
        }
    }
}

@M4urici0GM if there were updates, there would be notes here. Hence, no.

@nazarii-klymok the idea is to make all annotations work, but unfortunately there is no simple/easy fix: a complete overhaul of property introspection is needed. I haven’t had time to dedicate for this due to may daytime job. I do hope to get there for 2.15 but it is difficult to predict if that happens or not.

A poor man’s workaround for this issue is to just manually annotate the fields with the wrong naming pattern with @JsonProperty("user_id") String userId.

Hey @cowtowncoder, first of all, thank you so much for maintaining this project (in your spare time no less)! The community really stands on the shoulders of a giant here. Having been in that same position myself before, I relate to how you may be feel; at least I often felt overwhelmed and eventually lost the motivation to further work on the project (hence, a big boo to whoever gave that thumbs down to your comment above 😠).

On the issue itself, I don’t mean to further add to your workload, so just in case: is there by any chance a description or design doc of what that complete overhaul would look like? Or asked differently, is there a way for folks in the community to help? Oftentimes there are people who may be willing to help out, but aren’t able to do so (or don’t dare) because there’s not good enough of a description of the problem, its context and potential solution constraints which already are present in the mind of the core maintainers. I probably won’t have the bandwidth myself (as much as I’d like to do it, as said, I feel the community owes you so much here), but perhaps having a more in-depth description could help finding others willing to step up. Anyways, just a thought, and thanks again for this amazing project!

Does this workaroud work on every case? This solution didn’t work for me (a record with SnakeCaseStrategy). It still complains about Can not set final java.lang.String field ....

Great. I will only use my fix will until 2.13. Here is a version that does not modify in place:

package com.examaple.jackson;

import com.fasterxml.jackson.databind.BeanDescription;
import com.fasterxml.jackson.databind.DeserializationConfig;
import com.fasterxml.jackson.databind.deser.SettableBeanProperty;
import com.fasterxml.jackson.databind.deser.ValueInstantiator;
import com.fasterxml.jackson.databind.deser.ValueInstantiators;
import com.fasterxml.jackson.databind.deser.std.StdValueInstantiator;
import com.fasterxml.jackson.databind.introspect.BeanPropertyDefinition;
import com.fasterxml.jackson.databind.module.SimpleModule;

import java.util.Arrays;
import java.util.Map;
import java.util.function.Function;
import java.util.stream.Collectors;

public class RecordNamingStrategyPatchModule extends SimpleModule {

    @Override
    public void setupModule(SetupContext context) {
        context.addValueInstantiators(new ValueInstantiatorsModifier());
        super.setupModule(context);
    }

    /**
     * Remove when the following issue is resolved: 
     * <a href="https://github.com/FasterXML/jackson-databind/issues/2992">Properties naming strategy do not work with Record #2992</a>
     */
    private static class ValueInstantiatorsModifier extends ValueInstantiators.Base {
        @Override
        public ValueInstantiator findValueInstantiator(
                DeserializationConfig config, BeanDescription beanDesc, ValueInstantiator defaultInstantiator
        ) {
            if (!beanDesc.getBeanClass().isRecord() || !(defaultInstantiator instanceof StdValueInstantiator) || !defaultInstantiator.canCreateFromObjectWith()) {
                return defaultInstantiator;
            }
            Map<String, BeanPropertyDefinition> map = beanDesc.findProperties().stream().collect(Collectors.toMap(p -> p.getInternalName(), Function.identity()));
            SettableBeanProperty[] renamedConstructorArgs = Arrays.stream(defaultInstantiator.getFromObjectArguments(config))
                    .map(p -> {
                        BeanPropertyDefinition prop = map.get(p.getName());
                        return prop != null ? p.withName(prop.getFullName()) : p;
                    })
                    .toArray(SettableBeanProperty[]::new);

            return new PatchedValueInstantiator((StdValueInstantiator) defaultInstantiator, renamedConstructorArgs);
        }
    }

    private static class PatchedValueInstantiator extends StdValueInstantiator {

        protected PatchedValueInstantiator(StdValueInstantiator src, SettableBeanProperty[] constructorArguments) {
            super(src);
            _constructorArguments = constructorArguments;
        }
    }
}

@cgpassante how does your non-record setup (i.e. the DTO classes) looks like currently? Bean class with getters & setters?

They are standard Bean classes. The issue is really our use of PropertyNamingStrategies.KEBAB_CASE. It does not translate attribute names in records. So we must use @JsonProperty(“tracking-id”) String trackingId, to explicitly name each record field. It removes the major benefit of records over beans…brevity.

Dr. Jackson – if you have a candidate strategy in mind, you are welcome to preflight it at the amber-dev@openjdk.org list, to validate that you’re not subtly stepping on the Record contract.

Does this workaroud work on every case? This solution didn’t work for me (a record with SnakeCaseStrategy). It still complains about Can not set final java.lang.String field ....

@MRamonLeon, am not sure how your JsonMapper is setup, but are you using the ParameterNamesModule (which relies metadata generated by the java compiler using the -parameters flag) . If not it might explain why Jackson tries to set the record fields instead of using the record constructor. Here is the junit test I am using to track the bug.

import com.fasterxml.jackson.core.JsonProcessingException;
import com.fasterxml.jackson.databind.JsonMappingException;
import com.fasterxml.jackson.databind.PropertyNamingStrategies;
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;
import static org.junit.jupiter.api.Assertions.assertThrows;

class RecordNamingStrategyPatchModuleTest {

    private static final JsonMapper STANDARD_MAPPER = JsonMapper.builder()
            .addModule(new ParameterNamesModule())
            .propertyNamingStrategy(PropertyNamingStrategies.SNAKE_CASE)
            .build();

    private static final JsonMapper PATCHED_MAPPER = JsonMapper.builder()
            .addModule(new ParameterNamesModule())
            .addModule(new RecordNamingStrategyPatchModule())
            .propertyNamingStrategy(PropertyNamingStrategies.SNAKE_CASE)
            .build();

    public record SampleItem(int itemId) {
    }

    @Test
    void testNamingStrategyDeserializeRecordBug() throws JsonProcessingException {
        assertThrows(JsonMappingException.class, () ->
                STANDARD_MAPPER.readValue("""
                        {"item_id": 5}
                        """, SampleItem.class), "https://github.com/FasterXML/jackson-databind/issues/2992 has been fixed!");

        assertEquals(new SampleItem(5),
                PATCHED_MAPPER.readValue("""
                        {"item_id": 5}
                        """, SampleItem.class));
    }
}

@jedvardsson excellent – thank you very much for sharing this solution!

@GregoireW thank you for reporting the issue and digging pretty deep actually – constructor-handling part of property introspection is a mess. Apologies for lecturing on internal workings, your educated guess on behavior is pretty good considering symptoms. But more importantly I had completely missed the possibility that just because use cases seemed to work, they might have worked for the wrong reasons: that is, use of fields was intended to be completely avoided with 2.12.0. It sounds like this has not happened.

This is not completely unexpected, for what that is worth, and the specific part of functionality (discrepancy between initial POJOPropertiesCollector detection and later Basic/BeanDeserializerFactory gathering of Creator methods) has been known to be in need of rewrite for a while. 2.12 did some rewriting of latter part but I am bit afraid of full rewrite and was wondering if it could be only attempted for 3.0. But now I am beginning to think it might be one big thing to do for 2.13 (along with increasing baseline to Java 8).

So that’s a long of saying “thank you very much for doing what you did already”. 😃 I can take it from here as necessary. Additional help obviously very welcome, but not expected.

Putting a record as stand alone type in is own file and setting @JsonNaming(PropertyNamingStrategies.SnakeCaseStrategy.class) fail with the same exception as before.

From what I read in the code, Jackson first create an object from the constructor, filling parameters with properties it can match by name (setting null if it can not), then apply the naming strategy which may override some attributes values (the default naming strategy which is based on property name means there is nothing to do in the second step in case of a record).

Record are immutable, the second phase will always fail if there is at least one attributes which name do not match the name in the json.

Your test is success because there is a “bug” in jdk14 (record are not so immutable after all in JDK14). With JDK15 your test fail ( I use JDK15, and I was able to have a green if I use JDK14 and red if I use JDK15.

Turns out there is a quote in the JEP record second preview, JDK15

The implicitly declared fields corresponding to the record components of a record class are final and moreover are not modifiable via reflection (doing so will throw IllegalAccessException). These restrictions embody an immutable by default policy that is widely applicable for data-carrier classes.

On the record JEP for JDK16 (Release version) the quote become:

The fields derived from the record components are final. This restriction embodies an immutable by default policy that is widely applicable for data-carrier classes.

Even if the “IllegalAccessException” is not mention, I think it means it will be kept that way (There is nothing about relaxing this in the ‘refinement’ section).

On a positive note, the record in method local is fine, this test is success for both JDK14 and JDK15.

    @Test
    void tryLocalMethodRecord() throws Exception{
        ObjectMapper mapper=new ObjectMapper();

        record Test(String myId, String myValue){}

        var src=new Test("id", "value");
        String json=mapper.writeValueAsString(src);
        assertThat(json).contains("\"myId\":\"id\"", "\"myValue\":\"value\"");
        var after=mapper.readValue(json, Test.class);
        assertThat(after).isEqualTo(src);
    }