FasterXML / jackson-module-kotlin

Module that adds support for serialization/deserialization of Kotlin (http://kotlinlang.org) classes and data classes.
Apache License 2.0
1.12k stars 175 forks source link

JsonMerge ignores non-contiguous elements in Kotlin #726

Closed Bram-- closed 6 months ago

Bram-- commented 9 months ago

Search before asking

Describe the bug

See Kotlin code below; Some repeated elements are ignored when they are non-contiguous even when using @JsonMerge - Using xml-dataformat and Kotlin module.

To Reproduce

  @JsonRootName("example")
  data class Example(
      val name: String,
      val type: String,
      @JsonMerge
      @JsonSetter(nulls = Nulls.SKIP)
      @JacksonXmlElementWrapper(useWrapping = false)
      @JacksonXmlProperty(localName = "repeat")
      val repeated: List<Repeated> = listOf(),
  )

  data class Repeated(
      @JacksonXmlProperty(isAttribute = true) val name: String,
      @JacksonXmlProperty(isAttribute = true) val type: String,
  )

@Test
  fun `JsonMerge ignores non-contiguous elements in Kotlin`() {
    val xml =
        """
            <example>
                <repeat name="test0" type="ignored" />
                <repeat name="test1" type="ignored" />
                <name>Example</name>
                <repeat name="test2" type="included" />
                <type>Object</type>
                <repeat name="test3" type="included" />
                <repeat name="test4" type="included" />
            </example>
        """
            .trimIndent()

    val xmlMapper =
        XmlMapper.builder()
            .apply {
              configure(DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES, true)
            }
            .build()
            .registerKotlinModule()

    val example = xmlMapper.readValue(xml, Example::class.java)

    assertContentEquals(
        listOf(
            Repeated("test0", "ignored"),
            Repeated("test1", "ignored"),
            Repeated("test2", "included"),
            Repeated("test3", "included"),
            Repeated("test4", "included"),
        ),
        example.repeated)
  }

Expected behavior

Contains all 5 elements but instead only contains 3 as 'test0' and 'test1' are ignored.

Versions

Kotlin: jackson-module-kotlin: 2.16.+ jackson-datatype-jsr310: 2.16.+ jackson-dataformat-xml: 2.16.+

Additional context

No response

cowtowncoder commented 9 months ago

Probably not Kotlin specific, but due to jackson-dataformat-xml. To fix, you probably need to override setter for repeated to append entries, instead of setting field directly.

Bram-- commented 9 months ago

Probably not Kotlin specific, but due to jackson-dataformat-xml. To fix, you probably need to override setter for repeated to append entries, instead of setting field directly.

Thanks, I'll work around it by override the setter.

I wrote the Java test for this which seems to pass just fine:

import com.fasterxml.jackson.annotation.JsonMerge;
import com.fasterxml.jackson.databind.DeserializationFeature;
import com.fasterxml.jackson.dataformat.xml.XmlMapper;
import com.fasterxml.jackson.dataformat.xml.annotation.JacksonXmlElementWrapper;
import com.fasterxml.jackson.dataformat.xml.annotation.JacksonXmlProperty;
import com.fasterxml.jackson.dataformat.xml.annotation.JacksonXmlRootElement;
import com.google.common.truth.Truth;

import org.junit.jupiter.api.Test;

import java.util.List;
import java.util.Objects;

public class ExampleTest {

    @Test
    public void testExample() throws Exception {
        String xml =
                "<example>"
                        + "<repeat name=\"test0\" type=\"ignored\" />"
                        + "<repeat name=\"test1\" type=\"ignored\" />"
                        + "<name>Example</name>"
                        + "<repeat name=\"test2\" type=\"included\" />"
                        + "<type>Object</type>"
                        + "<repeat name=\"test3\" type=\"included\" />"
                        + "<repeat name=\"test4\" type=\"included\" />"
              + "</example>";

        XmlMapper xmlMapper =
                XmlMapper.builder()
                        .configure(DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES, true)
                        .build();

        Example example = xmlMapper.readValue(xml, Example.class);

        Truth.assertThat(example.repeated)
                .containsExactly(
                        new Repeated("test0", "ignored"),
                        new Repeated("test1", "ignored"),
                        new Repeated("test2", "included"),
                        new Repeated("test3", "included"),
                        new Repeated("test4", "included"));
    }

    @JacksonXmlRootElement(localName = "example")
    static class Example {
        @JacksonXmlProperty(localName = "name")
        String name;

        @JacksonXmlProperty(localName = "type")
        String type;

        @JsonMerge @JacksonXmlElementWrapper(useWrapping = false)
        @JacksonXmlProperty(localName = "repeat")
        List<Repeated> repeated;
    }

    static class Repeated {
        @JacksonXmlProperty(localName = "name", isAttribute = true)
        String name;

        @JacksonXmlProperty(localName = "type", isAttribute = true)
        String type;

        Repeated() { }

        Repeated(String name, String type) {
            this.name = name;
            this.type = type;
        }

        @Override
        public String toString() {
            return name + " - " + type;
        }

        @Override
        public boolean equals(Object o) {
            if (this == o) return true;
            if (o == null || getClass() != o.getClass()) return false;
            Repeated repeated = (Repeated) o;
            return name.equals(repeated.name) && type.equals(repeated.type);
        }

        @Override
        public int hashCode() {
            return Objects.hash(name, type);
        }
    }
}
Bram-- commented 9 months ago

Writing the property as mutable property and overriding the setter works - as seen below - but having @JsonMerge would be nice.

@JacksonXmlElementWrapper(useWrapping = false)
@JsonProperty("repeat")
 var repeated: List<Repeated> = listOf()
     set(value) {
         field = field + value
     }

Thanks again for the reply.

cowtowncoder commented 9 months ago

@Bram-- No problem. Ideally this would not be needed, of course, but handling of non-contiguous Collections is quite tricky as they present data as separate sequences. This may also be one of the reason why JAXB prefers "Lists via getters" way of deserializing Collections (that is, instead of calling "setter", JAXB alls "getter" to get an instance and modifies that Collection by adding elements).

cowtowncoder commented 9 months ago

Actually, use of @JsonMerge should also work shouldn't it? Json prefix is a historical leftover and does not mean annotation won't work for other formats -- unlike Xml for XML module which does actually mean "xml-specific" (bit confusing to be sure).

Bram-- commented 9 months ago

Actually, use of @JsonMerge should also work shouldn't it?

I'm not quite sure what you mean here, could you elaborate?

@JsonMerge seems to work fine in the Java example, which is - with my very limited understanding of the impl - virtually equivalent to the Kotlin one.

cowtowncoder commented 9 months ago

Ugh. I was being scatter-brain. What I meant is that @JsonMerge should indeed work so that's a real problem. Not sure why it wouldn't be applied, although it might be since value is passed via Constructor and @JsonMerge just won't work that way.

Bram-- commented 9 months ago

Ugh. I was being scatter-brain. What I meant is that @JsonMerge should indeed work so that's a real problem. Not sure why it wouldn't be applied, although it might be since value is passed via Constructor and @JsonMerge just won't work that way.

Got it, thanks :) I've also tried not overriding the setter and using @JsonMerge in the data class, but unfortunately no cigar.

cowtowncoder commented 9 months ago

Hmmh. Ok, that is weird. Setter-with-annotation should definitely allow @JsonMerge to work. In that case I'd guess that setter in question was not used for some reason.

Bram-- commented 6 months ago

Looked into this a bit more and found that rewriting the Java test case I can actually reproduce it in Java as well. Will dig a bit into jackson-dataformat-xml.

Test case:

    @Test
    public void test() throws Exception {
        String xml =
                "<example>"
                        + "<repeat name=\"test0\" type=\"ignored\" />"
                        + "<repeat name=\"test1\" type=\"ignored\" />"
                        + "<name>Example</name>"
                        + "<repeat name=\"test2\" type=\"included\" />"
                        + "<type>Object</type>"
                        + "<repeat name=\"test3\" type=\"included\" />"
                        + "<repeat name=\"test4\" type=\"included\" />"
                        + "</example>";

        XmlMapper xmlMapper =
                XmlMapper.builder()
                        .configure(DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES, true)
                        .build();

        Example example = xmlMapper.readValue(xml, Example.class);

        Assert.assertEquals(Arrays.asList(
                        new Repeated("test0", "ignored"),
                        new Repeated("test1", "ignored"),
                        new Repeated("test2", "included"),
                        new Repeated("test3", "included"),
                        new Repeated("test4", "included")),
                example.getRepeated());
    }

    @JacksonXmlRootElement(localName = "example")
    static class Example {
        private final String name;

        private final String type;

        private List<Repeated> repeated;

        Example(
                @JacksonXmlProperty(localName = "name")
                String name,
                @JacksonXmlProperty(localName = "type")
                String type,
                @JsonMerge @JacksonXmlElementWrapper(useWrapping = false)
                @JacksonXmlProperty(localName = "repeat")
                List<Repeated> repeated) {
            this.name = name;
            this.type = type;
            this.repeated = repeated;
        }

        List<Repeated> getRepeated() {
            return repeated;
        }

        @JsonSetter(value = "repeat")
        void setRepeated(List<Repeated> repeated) {
            this.repeated = repeated;
        }
    }

    static class Repeated {
        @JacksonXmlProperty(localName = "name", isAttribute = true)
        String name;

        @JacksonXmlProperty(localName = "type", isAttribute = true)
        String type;

        Repeated() {
        }

        Repeated(String name, String type) {
            this.name = name;
            this.type = type;
        }

        @Override
        public String toString() {
            return name + " - " + type;
        }

        @Override
        public boolean equals(Object o) {
            if (this == o) return true;
            if (o == null || getClass() != o.getClass()) return false;
            Repeated repeated = (Repeated) o;
            return name.equals(repeated.name) && type.equals(repeated.type);
        }

        @Override
        public int hashCode() {
            return Objects.hash(name, type);
        }
    }