When the `localName` property of `@JacksonXmlProperty` is not set, the `required` of `@JsonProperty` does not detect the property name of POJO

linghengqian commented 5 months ago

TL;

When the localName property of @JacksonXmlProperty is not set, the required of @JsonProperty does not detect the property name of POJO.

I have an example to verify this phenomenon. I don't seem to see this documented in the documentation. This unit testing and reproduction process is located at https://github.com/linghengqian/jackson-mixed-annotations-test .

public class RequiredTest {
@Test
void assertRequiredValue() {
    ObjectMapper XML_MAPPER = XmlMapper.builder().build();
    Assertions.assertDoesNotThrow(() -> {
        XML_MAPPER.readValue("""
                <Record type="">
                    <driverClassName>
                    </driverClassName>
                </Record>
                """, TestRecord.class);
    });
    Assertions.assertDoesNotThrow(() -> {
        XML_MAPPER.readValue("""
                <Record type="">
                    <driverClassName>
                    </driverClassName>
                </Record>
                """, TestRecordWithoutLocalName.class);
    });
}

@Getter
@NoArgsConstructor
@JacksonXmlRootElement(localName = "Record")
static class TestRecord {
    private String type;

    private String driverClassName;

    @JsonCreator
    public TestRecord(
            @JsonProperty(required = true) @JacksonXmlProperty(localName = "type", isAttribute = true)
            String type,
            @JsonProperty(required = true) @JacksonXmlProperty(localName = "driverClassName")
            String driverClassName) {
        this.type = type;
        this.driverClassName = driverClassName;
    }
}

@Getter
@NoArgsConstructor
@JacksonXmlRootElement(localName = "Record")
static class TestRecordWithoutLocalName {
    private String type;

    private String driverClassName;

    @JsonCreator
    public TestRecordWithoutLocalName(
            @JsonProperty(required = true) @JacksonXmlProperty(isAttribute = true)
            String type,
            @JsonProperty(required = true) @JacksonXmlProperty(localName = "driverClassName")
            String driverClassName) {
        this.type = type;
        this.driverClassName = driverClassName;
    }
}
}

The second assertion will fail.

From my understanding, the localName attribute of @JacksonXmlProperty has the same function as https://github.com/FasterXML/jackson-annotations?tab=readme-ov-file#annotations-for-renaming-properties , but is used renaming of POJO properties. Not setting localName should not cause this error.

[ERROR] Tests run: 1, Failures: 1, Errors: 0, Skipped: 0, Time elapsed: 0.273 s <<< FAILURE! -- in com.lingh.RequiredTest
[ERROR] com.lingh.RequiredTest.assertRequiredValue -- Time elapsed: 0.243 s <<< FAILURE!
org.opentest4j.AssertionFailedError: 
Unexpected exception thrown: com.fasterxml.jackson.databind.exc.MismatchedInputException: Missing required creator property '' (index 0)
at [Source: (StringReader); line: 4, column: 1] (through reference chain: com.lingh.RequiredTest$TestRecordWithoutLocalName[""])
    at org.junit.jupiter.api.AssertionFailureBuilder.build(AssertionFailureBuilder.java:152)
    at org.junit.jupiter.api.AssertDoesNotThrow.createAssertionFailedError(AssertDoesNotThrow.java:84)
    at org.junit.jupiter.api.AssertDoesNotThrow.assertDoesNotThrow(AssertDoesNotThrow.java:53)
    at org.junit.jupiter.api.AssertDoesNotThrow.assertDoesNotThrow(AssertDoesNotThrow.java:36)
    at org.junit.jupiter.api.Assertions.assertDoesNotThrow(Assertions.java:3168)
    at com.lingh.RequiredTest.assertRequiredValue(RequiredTest.java:26)
    at java.base/java.lang.reflect.Method.invoke(Method.java:580)
    at java.base/java.util.ArrayList.forEach(ArrayList.java:1596)
    at java.base/java.util.ArrayList.forEach(ArrayList.java:1596)
Caused by: com.fasterxml.jackson.databind.exc.MismatchedInputException: Missing required creator property '' (index 0)
at [Source: (StringReader); line: 4, column: 1] (through reference chain: com.lingh.RequiredTest$TestRecordWithoutLocalName[""])
    at com.fasterxml.jackson.databind.exc.MismatchedInputException.from(MismatchedInputException.java:59)
    at com.fasterxml.jackson.databind.DeserializationContext.reportInputMismatch(DeserializationContext.java:1781)
    at com.fasterxml.jackson.databind.deser.impl.PropertyValueBuffer._findMissing(PropertyValueBuffer.java:192)
    at com.fasterxml.jackson.databind.deser.impl.PropertyValueBuffer.getParameters(PropertyValueBuffer.java:158)
    at com.fasterxml.jackson.databind.deser.ValueInstantiator.createFromObjectWith(ValueInstantiator.java:301)
    at com.fasterxml.jackson.databind.deser.impl.PropertyBasedCreator.build(PropertyBasedCreator.java:202)
    at com.fasterxml.jackson.databind.deser.BeanDeserializer._deserializeUsingPropertyBased(BeanDeserializer.java:526)
    at com.fasterxml.jackson.databind.deser.BeanDeserializerBase.deserializeFromObjectUsingNonDefault(BeanDeserializerBase.java:1493)
    at com.fasterxml.jackson.databind.deser.BeanDeserializer.deserializeFromObject(BeanDeserializer.java:348)
    at com.fasterxml.jackson.databind.deser.BeanDeserializer.deserialize(BeanDeserializer.java:185)
    at com.fasterxml.jackson.dataformat.xml.deser.XmlDeserializationContext.readRootValue(XmlDeserializationContext.java:104)
    at com.fasterxml.jackson.databind.ObjectMapper._readMapAndClose(ObjectMapper.java:4899)
    at com.fasterxml.jackson.databind.ObjectMapper.readValue(ObjectMapper.java:3846)
    at com.fasterxml.jackson.databind.ObjectMapper.readValue(ObjectMapper.java:3814)
    at com.lingh.RequiredTest.lambda$assertRequiredValue$1(RequiredTest.java:27)
    at org.junit.jupiter.api.AssertDoesNotThrow.assertDoesNotThrow(AssertDoesNotThrow.java:49)
    ... 6 more

cowtowncoder commented 5 months ago

Ok the problem here is, I think, that for naming purposes @JsonProperty and @JacksonXmlProperty are sort of synonyms. And specifically so that one with higher precedence (as they are detected by different AnnotationIntrospectors, I think by default is @JacksonXmlProperty) will be used for all name aspects -- both namespace and local name. So in this case it'd be as if @JsonProperty did not even exist.

I think this should be fixable, but has to be done in jackson-databind AnnotationIntrospectorPair which handles merging of annotation information from different source: method findNameForDeserialization() (and findNameForSerialization()) at least would need to be changed a bit.

cowtowncoder commented 5 months ago

Actually, I take that back: constructor parameters' names are only discovered if using jackson-module-parameter-names (from https://github.com/FasterXML/jackson-modules-java8/). So this:

                @JsonProperty(required = true) @JacksonXmlProperty(isAttribute = true)
                String type,

Does not have name available: either @JsonProperty(value = "type") or @JacksonXmlProperty(localName="type") would indeed be needed.

linghengqian commented 5 months ago

@cowtowncoder Hi, I used XmlMapper.builder().addModule(new ParameterNamesModule()).build() at https://github.com/linghengqian/jackson-mixed-annotations-test/commit/cb7e2506855c8836684a28a961b1e7a1c903b278 to verify what you said things. I'm not sure if I understand you correctly, but it looks like jackson-module-parameter-names doesn't take effect.
```
org.opentest4j.AssertionFailedError: Unexpected exception thrown: com.fasterxml.jackson.databind.exc.MismatchedInputException: Missing required creator property '' (index 0)
at [Source: (StringReader); line: 4, column: 1] (through reference chain: com.lingh.RequiredTest$TestRecordWithoutLocalName[""])
```

cowtowncoder commented 5 months ago

@linghengqian Ok. In that case, that's problematic. I'll need to figure out if Parameter names module (or something in. Jackson databind) needs improvement -- ideally missing/empty local name should not override "implicit" name brought in by module.

I do have a related fix to make, via https://github.com/FasterXML/jackson-databind/issues/4364 but not sure that is enough. I also have another fix to make in this module (see #637), but fundamentally something else might be needed.

EDIT: now both fixes are in so test case might have a chance to work -- but I have not tested full set up yet (test needs to be in jackson-modules-java8 repo to find parameter names).

EDIT 2: tested against jackson-parameter-names -- https://github.com/FasterXML/jackson-modules-java8/issues/301 -- and things appear to work.

cowtowncoder commented 5 months ago

@linghengqian If you have a way to test against 2.17.0-SNAPSHOT the issue may have been resolved. I would be interested if this is (or is not) the case, with fixes I have made.

linghengqian commented 5 months ago

@cowtowncoder I experimented with the following two commits.
1. https://github.com/FasterXML/jackson-dataformat-xml/commit/9f1aa0f5054a7a75acfc8155df9aee50059fb5da
2. https://github.com/FasterXML/jackson-modules-java8/commit/1876ea82f326e05757d2a439af2b56290bb5d7c3

This slightly changes the experimental procedure


sdk install java 21.0.2-graalce
sdk use java 21.0.2-graalce

git clone git@github.com:FasterXML/jackson-dataformat-xml.git -b 2.17 cd ./jackson-dataformat-xml/ git reset --hard 9f1aa0f5054a7a75acfc8155df9aee50059fb5da ./mvnw clean install -T1C -DskipTests

cd ../

git clone git@github.com:FasterXML/jackson-modules-java8.git -b 2.17 cd ./jackson-modules-java8/ git reset --hard 1876ea82f326e05757d2a439af2b56290bb5d7c3 ./mvnw clean install -T1C -DskipTests

cd ../

git clone git@github.com:linghengqian/jackson-mixed-annotations-test.git cd ./jackson-mixed-annotations-test/ ./mvnw clean test

- But unfortunately, I still get the same error at https://github.com/linghengqian/jackson-mixed-annotations-test/commit/dab01c22b7603323f9379f60c8a076a580aea681 .
```shell
Caused by: com.fasterxml.jackson.databind.exc.MismatchedInputException: Missing required creator property '' (index 0)
 at [Source: (StringReader); line: 4, column: 1] (through reference chain: com.lingh.RequiredTest$TestRecordWithoutLocalName[""])
        at com.fasterxml.jackson.databind.exc.MismatchedInputException.from(MismatchedInputException.java:59)
        at com.fasterxml.jackson.databind.DeserializationContext.reportInputMismatch(DeserializationContext.java:1781)
        at com.fasterxml.jackson.databind.deser.impl.PropertyValueBuffer._findMissing(PropertyValueBuffer.java:192)
        at com.fasterxml.jackson.databind.deser.impl.PropertyValueBuffer.getParameters(PropertyValueBuffer.java:158)
        at com.fasterxml.jackson.databind.deser.ValueInstantiator.createFromObjectWith(ValueInstantiator.java:301)
        at com.fasterxml.jackson.databind.deser.impl.PropertyBasedCreator.build(PropertyBasedCreator.java:202)
        at com.fasterxml.jackson.databind.deser.BeanDeserializer._deserializeUsingPropertyBased(BeanDeserializer.java:526)
        at com.fasterxml.jackson.databind.deser.BeanDeserializerBase.deserializeFromObjectUsingNonDefault(BeanDeserializerBase.java:1493)
        at com.fasterxml.jackson.databind.deser.BeanDeserializer.deserializeFromObject(BeanDeserializer.java:348)
        at com.fasterxml.jackson.databind.deser.BeanDeserializer.deserialize(BeanDeserializer.java:185)
        at com.fasterxml.jackson.dataformat.xml.deser.XmlDeserializationContext.readRootValue(XmlDeserializationContext.java:104)
        at com.fasterxml.jackson.databind.ObjectMapper._readMapAndClose(ObjectMapper.java:4905)
        at com.fasterxml.jackson.databind.ObjectMapper.readValue(ObjectMapper.java:3848)
        at com.fasterxml.jackson.databind.ObjectMapper.readValue(ObjectMapper.java:3816)
        at com.lingh.RequiredTest.lambda$assertRequiredValue$1(RequiredTest.java:28)
        at org.junit.jupiter.api.AssertDoesNotThrow.assertDoesNotThrow(AssertDoesNotThrow.java:49)
        ... 6 more

Am I missing something?

cowtowncoder commented 5 months ago

That sounds like problem still persists: I can reproduce this with your project as well.

Odd part is that it really looks as if ParameterNamesModule was not working: if I comment out @JsonProperty and @JacksonXmlProperty annotations problem still persists (wrt no name found for constructor parameter). (I also temporarily removed Lombok annotations to ensure it had nothing to do with the issue). I'll see if I can figure out why this might be happening.

cowtowncoder commented 5 months ago

Ok. I think I know the problem: project settings are such that javac will not actually include method/constructor parameter names in bytecode -- and as such ParameterNamesModule cannot find any.

I can make test pass by adding this in pom.xml:

    <build>
      <plugins>
       <plugin>
        <groupId>org.apache.maven.plugins</groupId>
        <artifactId>maven-compiler-plugin</artifactId>
        <inherited>true</inherited>
        <configuration>
          <compilerArgs>
            <arg>-Xlint</arg>
            <arg>-parameters</arg>
          </compilerArgs>
        </configuration>
       </plugin>
      </plugins>
    </build>

linghengqian commented 5 months ago

@cowtowncoder Thanks for your help, I confirmed at https://github.com/linghengqian/jackson-mixed-annotations-test/commit/73769363e1443ae2123cd51eaf51b890b3fbb2f5 that there is no problem with jackson 2.16.1, just add the -parameters arg.

<project>
<build>
    <plugins>
        <plugin>
            <groupId>org.apache.maven.plugins</groupId>
            <artifactId>maven-compiler-plugin</artifactId>
            <version>3.12.1</version>
            <configuration>
                <compilerArgs>
                    <arg>-parameters</arg>
                </compilerArgs>
            </configuration>
        </plugin>
    </plugins>
</build>
</project>

Can I close the current issue? I noticed that instructions for this already exist at https://github.com/FasterXML/jackson-modules-java8/tree/jackson-modules-java8-2.16.1/parameter-names .

class Person must be compiled with Java 8 compliant compiler with option to store formal parameter names turned on (-parameters option). For more information about Java 8 API for accessing parameter names at runtime see this

cowtowncoder commented 5 months ago

@linghengqian Yes, let's close this issue.

FasterXML / jackson-dataformat-xml

When the `localName` property of `@JacksonXmlProperty` is not set, the `required` of `@JsonProperty` does not detect the property name of POJO #628

TL;