`@JsonNaming` does not work with Records

[GregoireW]

Hello,

When I try to use a @JsonNaming annotation on a record, I cannot unmarshall json to an object because a mapping exception occurs.

I use jackson 2.12.0 with JDK 15.

A Test example can be something like:

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

        @JsonNaming(PropertyNamingStrategies.SnakeCaseStrategy.class)
                record Test(String myId, String myValue){}

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

The json String is generated correctly, but when unmarshalling, I got an exception

com.fasterxml.jackson.databind.JsonMappingException: Can not set final java.lang.String field test.Tests$1Test.myValue to java.lang.String
    at com.fasterxml.jackson.databind.JsonMappingException.from(JsonMappingException.java:274)
    at com.fasterxml.jackson.databind.deser.SettableBeanProperty._throwAsIOE(SettableBeanProperty.java:623)
    at com.fasterxml.jackson.databind.deser.SettableBeanProperty._throwAsIOE(SettableBeanProperty.java:611)
    at com.fasterxml.jackson.databind.deser.SettableBeanProperty._throwAsIOE(SettableBeanProperty.java:634)
    at com.fasterxml.jackson.databind.deser.impl.FieldProperty.set(FieldProperty.java:193)
    at com.fasterxml.jackson.databind.deser.impl.PropertyValue$Regular.assign(PropertyValue.java:62)
    at com.fasterxml.jackson.databind.deser.impl.PropertyBasedCreator.build(PropertyBasedCreator.java:211)
    at com.fasterxml.jackson.databind.deser.BeanDeserializer._deserializeUsingPropertyBased(BeanDeserializer.java:520)
    at com.fasterxml.jackson.databind.deser.BeanDeserializerBase.deserializeFromObjectUsingNonDefault(BeanDeserializerBase.java:1390)
    at com.fasterxml.jackson.databind.deser.BeanDeserializer.deserializeFromObject(BeanDeserializer.java:362)
    at com.fasterxml.jackson.databind.deser.BeanDeserializer.deserialize(BeanDeserializer.java:195)
    at com.fasterxml.jackson.databind.deser.DefaultDeserializationContext.readRootValue(DefaultDeserializationContext.java:322)
    at com.fasterxml.jackson.databind.ObjectMapper._readMapAndClose(ObjectMapper.java:4591)
    at com.fasterxml.jackson.databind.ObjectMapper.readValue(ObjectMapper.java:3546)
    at com.fasterxml.jackson.databind.ObjectMapper.readValue(ObjectMapper.java:3514)
    at test.Tests.tryJsonNamingOnRecord(Tests.java:100)
    at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
    ...
    at org.junit.platform.commons.util.ReflectionUtils.invokeMethod(ReflectionUtils.java:688)
    at org.junit.jupiter.engine.execution.MethodInvocation.proceed(MethodInvocation.java:60)
...
Caused by: java.lang.IllegalAccessException: Can not set final java.lang.String field test.Tests$1Test.myValue to java.lang.String
    at java.base/jdk.internal.reflect.UnsafeFieldAccessorImpl.throwFinalFieldIllegalAccessException(UnsafeFieldAccessorImpl.java:76)
    at java.base/jdk.internal.reflect.UnsafeFieldAccessorImpl.throwFinalFieldIllegalAccessException(UnsafeFieldAccessorImpl.java:80)
    at java.base/jdk.internal.reflect.UnsafeQualifiedObjectFieldAccessorImpl.set(UnsafeQualifiedObjectFieldAccessorImpl.java:79)
    at java.base/java.lang.reflect.Field.set(Field.java:793)
    at com.fasterxml.jackson.databind.deser.impl.FieldProperty.set(FieldProperty.java:190)
    ... 76 more

[cowtowncoder]

I cannot quite reproduce this with the current 2.12.1-SNAPSHOT, but I do get different error, if Record is declared as a method-local class like in your example: this is not usage that is or can be supported. Reason being that you cannot deserialize non-static inner classes: they have that gnarly invisible "this" that needs to be passed: at least in case of regular POJOs and I don't think Records are different here.

PropertyNamingStrategy itself is supported.

So you'd need to declare Record as regular inner class (they are always static, at least) or as stand-alone type.

[GregoireW]

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);
    }

[cowtowncoder]

@GregoireW I don't think your explanation of how you think naming strategy works is quite accurate: naming strategy is used as part of constructing deserializer and modifies "true" names used and expected -- it is not used dynamically during deserialization itself (that would be rather inefficient) But what is possible -- and compatible with your observations -- is that due to different issue (*), the names of underlying fields and names of constructor parameters might not get linked during initial introspection, leading to only former getting renamed according to naming strategy: this would essentially leave two sets of unrelated properties instead of one unified set.

(sidenote: What is interesting, on method local Records, is that I do get actual failure: this is why I suggested it as likely root cause (it is something reported for POJOs every now and then -- but it does make sense Records differ a bit wrt static-ness) )

Anyway. If above is true, this is unfortunately much much more difficult problem to solve, and probably something I cannot fix for 2.12.x at all (due to high risk of refactoring). But first things first... good things is that as per your comments, while I cannot reproduce this with JDK 14 (latest I have installed), I can then get 15 installed (even if it's not LTS at least it'd be ok for reproduction). And what might be possible to do would be to explicitly prevent use of fields for Records.

... although while removing Fields might be the right thing to do, for longer term, it would also immediately "break" existing users, use cases, for Records with JDK 14 and Preview option. So multiple bad options.

(*) See f.ex #2974 -- not specific to Records, but to creator method handling for POJOs as well -- due to creator parameters not being recognized unless explicit annotated with @JsonProperty

[GregoireW]

I only scratch the surface on how it works, so yes I more guess on what I saw than really drill down the code.

So multiple bad options.

I was guessing so... Unfortunately this one need a drill down to multiple aspect of this library, I will not have time to check what can be done.

[cowtowncoder]

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

[GregoireW]

btw, I did a test with toolchains to check what was possible to do.

If you setup a ~/.m2/toolchains.xml file like (well update the JDK location to fit your setup):

<?xml version="1.0" encoding="UTF-8"?>

<toolchains xmlns="http://maven.apache.org/TOOLCHAINS/1.1.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
  xsi:schemaLocation="http://maven.apache.org/TOOLCHAINS/1.1.0 http://maven.apache.org/xsd/toolchains-1.1.0.xsd">

  <toolchain>
    <type>jdk</type>
    <provides>
      <version>1.8</version>
      <vendor>sun</vendor>
    </provides>
    <configuration>
      <jdkHome>/opt/java/jdk1.8.0_202</jdkHome>
    </configuration>
  </toolchain>

  <toolchain>
    <type>jdk</type>
    <provides>
      <version>14</version>
      <vendor>openjdk</vendor>
    </provides>
    <configuration>
      <jdkHome>/opt/java/jdk-14</jdkHome>
    </configuration>
  </toolchain>

  <toolchain>
    <type>jdk</type>
    <provides>
      <version>15</version>
      <vendor>openjdk</vendor>
    </provides>
    <configuration>
      <jdkHome>/opt/java/jdk-15</jdkHome>
    </configuration>
  </toolchain>

</toolchains>

you can replace the plugins section of the pom.xml by:

    <plugins>
      <plugin>
        <groupId>org.jacoco</groupId>
        <artifactId>jacoco-maven-plugin</artifactId>
        <version>0.8.6</version>
        <executions>
          <execution>
            <goals>
              <goal>prepare-agent</goal>
            </goals>
          </execution>
          <execution>
            <id>report</id>
            <phase>verify</phase>
            <goals>
              <goal>report</goal>
            </goals>
          </execution>
        </executions>
      </plugin>

      <plugin>
        <groupId>org.codehaus.mojo</groupId>
        <artifactId>build-helper-maven-plugin</artifactId>
        <executions>
          <execution>
            <id>add-test-source</id>
            <phase>generate-test-sources</phase>
            <goals>
              <goal>add-test-source</goal>
            </goals>
            <configuration>
              <sources>
                <source>src/test-jdk14/java</source>
              </sources>
            </configuration>
          </execution>
        </executions>
      </plugin>

      <plugin>
        <groupId>org.apache.maven.plugins</groupId>
        <artifactId>maven-compiler-plugin</artifactId>
        <executions>
          <execution>
            <id>default-testCompile</id>
            <goals>
              <goal>testCompile</goal>
            </goals>
            <configuration>
              <optimize>true</optimize>
              <jdkToolchain>
                <version>1.8</version>
              </jdkToolchain>
              <compileSourceRoots>
                <compileSourceRoot>${project.basedir}/src/test/java</compileSourceRoot>
              </compileSourceRoots>
            </configuration>
          </execution>
          <execution>
            <id>jdk-14</id>
            <goals>
              <goal>testCompile</goal>
            </goals>
            <configuration>
              <optimize>true</optimize>
              <jdkToolchain>
                <version>14</version>
              </jdkToolchain>
              <compileSourceRoots>
                <compileSourceRoot>${project.basedir}/src/test/java</compileSourceRoot>
                <compileSourceRoot>${project.basedir}/src/test-jdk14/java</compileSourceRoot>
              </compileSourceRoots>
              <outputDirectory>${project.build.directory}/test-classes-jdk14</outputDirectory>
              <testExcludes>
                <testExclude>**/Java9ListsTest.java</testExclude>
              </testExcludes>
              <source>14</source>
              <release>14</release>
              <compilerArgs>
                <arg>-parameters</arg>
                <arg>--enable-preview</arg>
              </compilerArgs>
            </configuration>
          </execution>
          <execution>
            <id>jdk-15</id>
            <goals>
              <goal>testCompile</goal>
            </goals>
            <configuration>
              <optimize>true</optimize>
              <jdkToolchain>
                <version>15</version>
              </jdkToolchain>
              <compileSourceRoots>
                <compileSourceRoot>${project.basedir}/src/test/java</compileSourceRoot>
                <compileSourceRoot>${project.basedir}/src/test-jdk14/java</compileSourceRoot>
              </compileSourceRoots>
              <outputDirectory>${project.build.directory}/test-classes-jdk15</outputDirectory>
              <testExcludes>
                <testExclude>**/Java9ListsTest.java</testExclude>
              </testExcludes>
              <source>15</source>
              <release>15</release>
              <compilerArgs>
                <arg>-parameters</arg>
                <arg>--enable-preview</arg>
              </compilerArgs>
            </configuration>
          </execution>
        </executions>
        <configuration>
          <!-- 17-Sep-2017, tatu: With 3.0 need to ensure parameter names compiled in -->
          <optimize>true</optimize>
          <compilerArgs>
            <arg>-parameters</arg>
          </compilerArgs>
        </configuration>
      </plugin>

      <plugin>
        <groupId>org.apache.maven.plugins</groupId>
        <artifactId>maven-resources-plugin</artifactId>
        <version>3.2.0</version>
        <executions>
          <execution>
            <id>resources-jdk14</id>
            <goals>
              <goal>testResources</goal>
            </goals>
            <configuration>
              <outputDirectory>${project.build.directory}/test-classes-jdk14</outputDirectory>
            </configuration>
          </execution>
          <execution>
            <id>resources-jdk15</id>
            <goals>
              <goal>testResources</goal>
            </goals>
            <configuration>
              <outputDirectory>${project.build.directory}/test-classes-jdk15</outputDirectory>
            </configuration>
          </execution>
        </executions>
      </plugin>

      <plugin>
        <groupId>org.apache.maven.plugins</groupId>
        <version>3.0.0-M5</version>
        <artifactId>maven-surefire-plugin</artifactId>
        <executions>
          <execution>
            <id>default-test</id>
            <goals>
              <goal>test</goal>
            </goals>
            <configuration>
              <jdkToolchain>
                <version>1.8</version>
              </jdkToolchain>
              <excludes>
                <exclude>com/fasterxml/jackson/failing/*.java</exclude>

                <exclude>**/RecordBasicsTest.class</exclude>
                <exclude>**/RecordCreatorsTest.class</exclude>
                <exclude>**/Java9ListsTest.class</exclude>
                <exclude>**/RecordWithJsonSetter2974Test.class</exclude>
              </excludes>
            </configuration>
          </execution>
          <execution>
            <id>test-jdk14</id>
            <goals>
              <goal>test</goal>
            </goals>
            <configuration>
              <jdkToolchain>
                <version>14</version>
              </jdkToolchain>
              <testClassesDirectory>${project.build.directory}/test-classes-jdk14</testClassesDirectory>
              <excludes>
                <exclude>com/fasterxml/jackson/failing/*.java</exclude>

                <exclude>**/RecordWithJsonSetter2974Test.class</exclude>
              </excludes>
              <argLine>@{argLine} --enable-preview -XX:+ShowCodeDetailsInExceptionMessages</argLine>
            </configuration>
          </execution>
          <execution>
            <id>test-jdk15</id>
            <goals>
              <goal>test</goal>
            </goals>
            <configuration>
              <jdkToolchain>
                <version>15</version>
              </jdkToolchain>
              <testClassesDirectory>${project.build.directory}/test-classes-jdk15</testClassesDirectory>
              <excludes>
                <exclude>com/fasterxml/jackson/failing/*.java</exclude>

                <exclude>**/RecordWithJsonSetter2974Test.class</exclude>
              </excludes>
              <argLine>@{argLine} --enable-preview -XX:+ShowCodeDetailsInExceptionMessages</argLine>
            </configuration>
          </execution>
        </executions>
        <configuration>
          <classpathDependencyExcludes>
            <exclude>javax.measure:jsr-275</exclude>
          </classpathDependencyExcludes>
          <!-- 26-Nov-2019, tatu: moar parallelism! Per-class basis, safe, efficient enough
                      ... although not 100% sure this makes much difference TBH
                -->
          <threadCount>4</threadCount>
          <parallel>classes</parallel>
        </configuration>
      </plugin>

      <plugin>
        <groupId>org.apache.maven.plugins</groupId>
        <artifactId>maven-javadoc-plugin</artifactId>
        <configuration>
          <links combine.children="append">
            <link>http://fasterxml.github.com/jackson-annotations/javadoc/3.0</link>
            <link>http://fasterxml.github.com/jackson-core/javadoc/3.0</link>
          </links>
        </configuration>
      </plugin>

      <plugin> <!-- default settings are fine, just need to enable here -->
        <!-- Inherited from oss-base. Generate PackageVersion.java.-->
        <groupId>com.google.code.maven-replacer-plugin</groupId>
        <artifactId>replacer</artifactId>
        <executions>
          <execution>
            <id>generate-package-version-java</id>
            <phase>prepare-package</phase>
            <goals>
              <goal>replace</goal>
            </goals>
          </execution>
        </executions>
      </plugin>

      <plugin>
        <groupId>org.moditect</groupId>
        <artifactId>moditect-maven-plugin</artifactId>
      </plugin>
      <!-- 03-Nov-2020, tatu: Add LICENSE from main level -->

      <plugin>
        <groupId>de.jjohannes</groupId>
        <artifactId>gradle-module-metadata-maven-plugin</artifactId>
      </plugin>
    </plugins>

you can remove the jdk14 profile, then running maven with a jdk 8, will compile source with this jdk, compile test 3 times (jdk8, jdk14 (preview mode) and jdk15 (preview mode) ), and run all test 3 times, with jdk8, 14 and 15 (well jdk14 test are disabled when running on jdk8).

This approach is simpler to test all needed jdk in one time, but need the toolchains setup.

The not so good aspect is we need to compile 3 times all the tests as jdk14 compiled with preview cannot be run by jdk15. It would theoricaly possible to filter test to run only some test with jdk14 or 15, but option are limited on compilation part and surefire part.

Jacoco plugin has to be bound to verify step to prevent run before all test, but I'm not sure how code coverage will be with the multiple test run.

Anyway, this approach may not be the correct approach (what will be the jdk to test in march when jdk16 is released or in september with jdk17) , but as I did some test with this I share it if you want to test too.

[cowtowncoder]

@GregoireW thank you for sharing this! It is probably not something I'd consider at this point for databind repo (if it triples build time, requires use of later JDK), but sounds like something that could work well for projects like jackson-integration-tests (external testing package that can add cross-dependencies across Jackson components) and some individual repos.

[cowtowncoder]

For what it is worth, I was able to replicate the failure by forcing removal of Field accessors for Records (in POJOPropertiesCollector); this fails naming-strategy case. Hopign to address the issue for 2.13.

[jedvardsson]

Would the preferred way to fix this be to rewrite the property names when deriving the bean description? I guess I could add a beandeserializermodifier to fix this temporarily, right?

Edit:

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.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);
    }

    /**
     * <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;
            }
            // StdValueInstantiator can be modified in place
            SettableBeanProperty[] fromObjectArguments = defaultInstantiator.getFromObjectArguments(config);
            Map<String, BeanPropertyDefinition> map = beanDesc.findProperties().stream().collect(Collectors.toMap(p -> p.getInternalName(), Function.identity()));
            for (int i = 0; i < fromObjectArguments.length; i++) {
                SettableBeanProperty arg = fromObjectArguments[i];
                BeanPropertyDefinition prop = map.get(arg.getName());
                if (prop != null) {
                    fromObjectArguments[i] = arg.withName(prop.getFullName());
                }
            }
            return defaultInstantiator;
        }
    }
}

package com.examaple.jackson;

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));
    }
}

[cowtowncoder]

Yes, I want to fix the problem by correctly linking constructor parameter -- something I am planning to achieve for 2.13.

As to fix... ability to modify the array is not part of API so I would not really recommend it as something that is likely to keep working (that is, this implementation detail could change, or might behave differently for some edge case). But if it works now it might work in future.

[jedvardsson]

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

[cowtowncoder]

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

[MRamonLeon]

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

[jedvardsson]

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));
    }
}

[rmetzger]

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.

[cgpassante]

Any idea when this will be fixed? Considering moving to record for all DTOs but we use kebab case as a standard and having to annotate every field removes a big part of the advantage of reduced verbosity that record types provide. Right now this is a blocker to adopting record.

[jedvardsson]

@cgpassante you CSN just add the workaround I posted in a comment No need for annotations.

Use the test in comment to track the bug.

[cowtowncoder]

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

[briangoetz]

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.

[cowtowncoder]

@briangoetz Thank you for suggestion. I think at this point the issues are relatively well-known (basically Record support is a tweak on earlier code that does not consider there being module-system imposed access limits, nor Fields being off-limits). Once those are cleared if there are more subtle problems we can and will reach out as necessary.

[yihtserns]

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

[cgpassante]

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

[M4urici0GM]

any updates on this?

[nazarii-klymok]

Greetings! I love jedvardsson's solution. It would be great though to support this out of the box :)

[cowtowncoder]

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

[gunnarmorling]

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!

[cowtowncoder]

Hi @gunnarmorling! I REALLY appreciate your suggestion here. I will have to think of how I could do something along those lines. I have added some vague outlines in various issues, but nothing concrete enough to let others start from somewhere other than square 1. But maybe I can do some preparation work to dig out details that I'd need myself. And I am 110% certain that the community would be happy to help -- at this point majority of changes are done by someone other than myself, which is a new & great thing to have happened.

I will add whatever I know, tho, as a separate comment in case that might help someone interested in maybe tackling the issue, or at least learn more about what it might take.

[cowtowncoder]

Ok, so the issue is roughly as follows:

1. Property introspection needed to eventually produce BasicBeanDescription starts by looking at Accessors -- Fields, Methods (getters, setters). This uses annotation-enhanced variants of accessors; some combining of annotations (from getters to setters or vice versa, to avoid having to add multiple annotations) is done now, as well as (I think?) renaming of combined properties
2. After determining properties derived from accessors, Creators are introspected -- these are annotated or compatible Constructors and static (factory) methods
3. Attempt is made to find Creator(s) to use; explicitly annotated ones tend to work well, but limited inference for non-annotated ones can fail to find expected one. If properties-style Creator is found, virtual "accessors" are created and linked to existing property (field, getter/setter) accessors (or creating new ones).
4. Full merging of annotations is done at this point; but due to problems with (3) some annotations are not properly merged/propagated

What should happen, instead, it that process should start with Creator discovery (step (3)) followed by accessor discovery. This would allow reliable linking of all accessors/properties as well as make it possible to prevent discovery of underlying fields Records have.

I will need to trace through code again to remind myself of code flow: I think it:

1. Starts with BeanDeserializerFactory/BasicDeserializerFactory (and -Serializer counterparts to a degree)
2. factory calls BasicClassIntrospector which gets annotated class (AnnotatedClass) with AnnotatedClassResolver.resolve (this is fine)
3. Introspector eventually creates POJOPropertiesCollector to perform non-Creator property discovery,
4. introspector also calls some methods in BasicBeanDescription for construction
5. After getting BasicBeanDescription, deserializer factory proceeds to Creator discovery; this uses properties collected previously for some of inference

But now I am bit confused wrt how Bean/BasicSerializerFactory does things since although Creators are not used for serialization, some annotations may be needed.

Anyway, not sure above helps... I will need to go dig deeper. I will create a Discussion for this tho.

[cowtowncoder]

Quick update: Enabled Discussions and created #3719