jackson-module-kotlin icon indicating copy to clipboard operation
jackson-module-kotlin copied to clipboard

@JacksonXmlElementWrapper doesn’t work with data classes

Open stanislasbonifetto opened this issue 6 years ago • 22 comments

I want to mapping a xml using kotlin data class, my xml wrapper a list of elements in a wapper node. Xml example: <MyPojo><elements><element value="e1"/></elements></MyPojo>

Here the node is a collection, wrapped in the node.

When I use the data class to map the ‘MyPojo’ and ‘element’ object and I annotated the collection property with @JacksonXmlElementWrapper and @JacksonXmlProperty it doesn’t work when I try to deserialise form xml. The exception is: com.fasterxml.jackson.databind.exc.InvalidDefinitionException: Invalid definition for property elements (of type MyDataPojo): Could not find creator property with name 'elements' (known Creator properties: [element])

I use :

  • Java 1.8
  • Kotlin 1.2.31
  • Jackson: 2.9.5

I have write a test to compare the mapping using the standard class and the data class. The standard class test passed, the data class test failed.

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.fasterxml.jackson.module.kotlin.KotlinModule
import junit.framework.Assert.assertEquals
import org.junit.Test

/**
 * Mapping the Xml with standard classes
 */
@JacksonXmlRootElement(localName = "MyPojo")
class MyPojo {

  @JacksonXmlElementWrapper(localName = "elements")
  @JacksonXmlProperty(localName = "element")
  var list: List<MyElement>? = null

}

/**
 * Mapping the Xml with standard classes
 */
class MyElement {
  @JacksonXmlProperty(localName = "value", isAttribute = true)
  var value: String? = null

}

/**
 * Mapping the Xml with data classes
 */
@JacksonXmlRootElement(localName = "MyPojo")
data class MyDataPojo (
    @JacksonXmlElementWrapper(localName = "elements")
    @JacksonXmlProperty(localName = "element")
    val list
    : List<MyDataElement>
)

/**
 * Mapping the Xml with data classes
 */
data class MyDataElement (
    @JacksonXmlProperty(localName = "value", isAttribute = true)
    var value: String
)

class TextKotlinXmlWrapper {
  // xml example I want to deserialize
  val xml = """<MyPojo><elements><element value="e1"/></elements></MyPojo>"""

  val mapper = XmlMapper()
      .registerModule(KotlinModule())

  @Test
  @Throws(Exception::class)
  fun test_class() {

    // I create a pojo from the xml using the standard classes
    val pojoFromXml = mapper.readValue(xml, MyPojo::class.java)

    //I create a xml from the pojo
    val xmlFromPojo = mapper.writeValueAsString(pojoFromXml)

    // I compare the original xml with the xml generated from the pojo
    assertEquals(xml, xmlFromPojo)

  }

  @Test
  @Throws(Exception::class)
  fun test_data_class() {

    // I create a pojo from the xml using the data classes
    val pojoFromXml = mapper.readValue(xml, MyDataPojo::class.java)

    //I create a xml from the pojo
    val xmlFromPojo = mapper.writeValueAsString(pojoFromXml)

    // I compare the original xml with the xml generated from the pojo
    assertEquals(xml, xmlFromPojo)

  }

}

stanislasbonifetto avatar May 15 '18 09:05 stanislasbonifetto

Hi anyone has idea where could be the issue and help us to understand if exist a temporary workaround or do the code change? Where in the Jackson kotlin module may I look in to?

stanislasbonifetto avatar May 22 '18 08:05 stanislasbonifetto

Good Morning FasterXML folks! Just to let you know. I also came across the same problem as described above. Just to confirm. We used Java 8, Kotlin 1.2.41, Jackson 2.9.5 and Spring Boot 2.0.1

Is it a bug or I did something wrong? Let me know if I could assist you in any way.

pastorcmentarny avatar May 23 '18 08:05 pastorcmentarny

I have no idea unfortunately. I do not know how Kotlin works, or expects things to work. Perhaps @apatrida does.

For me Java equivalent version would be needed; exception message itself suggests this might not be XML specific.

cowtowncoder avatar May 23 '18 20:05 cowtowncoder

Thank you @cowtowncoder. Hi @apatrida can you help us about that? Maybe give me some suggestion where I can look in to the Kotlin Jackson code to find the Issue and try to find a fix.

Thanks

stanislasbonifetto avatar Jun 12 '18 09:06 stanislasbonifetto

I'm checking this out now.

apatrida avatar Jul 14 '18 18:07 apatrida

@cowtowncoder this is the issue again with annotations, and Kotlin having more areas that annotations can target and there being a conflict with where they are ending up. I'm seeing if there is a work-around but it is hard because of the wrong annotation ending up on the parameter value for the constructor.

apatrida avatar Jul 14 '18 18:07 apatrida

so @cowtowncoder

JacksonXmlElementWrapper cannot be on a parameter therefore no property name of elements gets added to the creator parameter, and instead JacksonXmlProperty ends up on the parameter which is the wrong name. So then the mapper tries to pass in elements but the parameter appears to be property element

apatrida avatar Jul 14 '18 19:07 apatrida

A fix would be to allow JacksonXmlElementWrapper to also be on the parameter.

apatrida avatar Jul 14 '18 19:07 apatrida

@apatrida @cowtowncoder Do you think I should raise this issue with Kotlin folks?

pastorcmentarny avatar Jul 27 '18 10:07 pastorcmentarny

@pastorcmentarny it isn't a Kotlin issue, it is a problem with where annotations can be placed and the inability to specialize that for a different language. This could be fixed as described

@cowtowncoder:

A fix would be to allow JacksonXmlElementWrapper to also be on the parameter.

apatrida avatar Aug 18 '18 02:08 apatrida

@apatrida Ok; could you please file an issue for JAXB annotations module? I can do that for 2.10, can not change in patch release.

cowtowncoder avatar Aug 22 '18 17:08 cowtowncoder

@apatrida Thanks for the insight! I was struggling with this as well.

If anyone is wondering, this means that the problem isn't so much with data classes, but with attributes defined in the primary constructor, i.e. this doesn't work:

data class Example(
        @JacksonXmlElementWrapper(localName = "elements")
        @JacksonXmlProperty(localName = "element")
        val elements: MutableList<String>
)

but this does:

data class Example(
    val someOtherVal: String
) {
    @JacksonXmlElementWrapper(localName = "elements")
    @JacksonXmlProperty(localName = "element")
    val elements: MutableList<String> = mutableListOf()
}

I would still be great if this could be fixed. If I understand correctly, it would only take adding ElementType.PARAMETER to the @Target of JacksonXmlElementWrapper to fix this.

darioseidl avatar Dec 09 '18 18:12 darioseidl

FYI, from Kotlin you can specify where the annotation lands, using the get: or field: prefixes between the @ and the annotation name. So the following should work:

data class Example(
        @get:JacksonXmlElementWrapper(localName = "elements")
        @get:JacksonXmlProperty(localName = "element")
        val elements: MutableList<String>
)

joffrey-bion avatar Jan 20 '19 18:01 joffrey-bion

Hi guys, Just in case it helps somebody else, I found a workaround by combining the things mentioned here with a property backed up with a constructor field.

This allow us to still use a data class and avoid writing getters/setters/hashcode/equals.

data class Example(private var _elements: MutableList<String>) {
    @get:JacksonXmlElementWrapper(localName = "elements")
    @get:JacksonXmlProperty(localName = "element")
    var elements
        set(value) { _elements = value }
        get() = _elements
}

It is not too elegant, but it works fine so hopefully it will help.

Thanks guys for helping me figuring it out!

jespeche avatar Jun 07 '19 23:06 jespeche

I also encountered this problem. It seems that for just renaming elements, using @get:JacksonXmlProperty(localName = "some_quirky_name") is sufficient. Using JacksonXmlElementWrapper doesn't seem necessary at all.

michelwilson avatar Jun 24 '19 09:06 michelwilson

Hi guys, Just in case it helps somebody else, I found a workaround by combining the things mentioned here with a property backed up with a constructor field.

This allow us to still use a data class and avoid writing getters/setters/hashcode/equals.

data class Example(private var _elements: MutableList<String>) {
    @get:JacksonXmlElementWrapper(localName = "elements")
    @get:JacksonXmlProperty(localName = "element")
    var elements
        set(value) { _elements = value }
        get() = _elements
}

It is not too elegant, but it works fine so hopefully it will help.

Thanks guys for helping me figuring it out!

I faced the same problem today. jespeche's solution is a working workaround!

Thanks,

mobal avatar Aug 27 '19 13:08 mobal

While the solution presented here works well for serialization, I've found that in jackson 2.9.8 it breaks for de-serialization due to the KotlinValueInstantiator trying to find the _elements parameter in the xml. The solution is to simply give the _elements parameter a default value:

data class Example(private var _elements: MutableList<String> = mutableListOf())

De-serializes properly with Json too, even with polymorphic elements.

Myndale avatar Sep 14 '20 12:09 Myndale

For all the exhausted gogglers out there who try to fix this issue: at this day Jackson XML de-serialization is still bugged when paired with Kotlin Data-Classes => do yourself the favor and use classes instead.

SirArtorias avatar Mar 08 '21 07:03 SirArtorias

I don't think that this issue is only related to data classes. In my case (Jackson 2.12.4) standard classes are also not able to use @JacksonXmlElementWrapper. Solution propsed by @jespeche works, but it's kinda dirty and it's quite sad that such a feature is still not supported after 3 years :/

dzikoysk avatar Aug 16 '21 15:08 dzikoysk

While the solutions of @jespeche and @joffrey-bion works for serialization, for deserialization I found Jackson not knowing what to do with the wrapper tags. I solved it like this

data class Car(
    val wheels: Wheels
)

data class Wheels(
    @get:JacksonXmlElementWrapper(useWrapping = false)
    val wheel: List<Wheel>
)

data class Wheel(
    val rim: String,
    val tire: String
)

ESchouten avatar Oct 06 '21 17:10 ESchouten

I'd like to bump this issue as still relevant and present in 2.13.1. Solution posted by @ESchouten works fine, but unfortunately code quality suffers in the process.

Quinteger avatar Feb 11 '22 13:02 Quinteger

That's our workaround but it's quite terrible:

  • https://github.com/dzikoysk/reposilite/blob/c4513ead3f138d908e242682b0c419fbc7ecc3f9/reposilite-backend/src/main/kotlin/com/reposilite/maven/api/Metadata.kt

dzikoysk avatar Feb 11 '22 19:02 dzikoysk

The same issue also affects Java 16 records. The below Testcase is successful for serialization, but fails for deserialization.

Record Definition:

import static com.fasterxml.jackson.annotation.JsonCreator.Mode.PROPERTIES;
import java.util.List;
import com.fasterxml.jackson.annotation.JsonCreator;
import com.fasterxml.jackson.dataformat.xml.annotation.JacksonXmlElementWrapper;
import com.fasterxml.jackson.dataformat.xml.annotation.JacksonXmlProperty;

public record MyRecord( //
    @JacksonXmlElementWrapper(localName = "messages") //
    @JacksonXmlProperty(localName = "message") //
    List<String> messages //
) {
  @JsonCreator(mode = PROPERTIES)
  public MyRecord {}
}

Testcase:

import java.util.Arrays;
import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.Test;
import com.fasterxml.jackson.databind.ObjectMapper;
import com.fasterxml.jackson.dataformat.xml.XmlMapper;

public class MyRecordTest {
  @Test
  void test_deserialize() throws Exception {
    // given:
    ObjectMapper objectMapper = new XmlMapper();
    MyRecord dto = new MyRecord(Arrays.asList("foo", "bar"));
    String xml = """
        <MyRecord>
          <messages>
            <message>foo</message>
            <message>bar</message>
          </messages>
        </MyRecord>
        """;

    // when:
    MyRecord actual = objectMapper.readValue(xml, MyRecord.class);

    // then:
    Assertions.assertEquals(actual, dto);
  }

  @Test
  void test_serialize() throws Exception {
    // given:
    ObjectMapper objectMapper = new XmlMapper();
    MyRecord dto = new MyRecord(Arrays.asList("foo", "bar"));
    String xml = """
        <MyRecord>
          <messages>
            <message>foo</message>
            <message>bar</message>
          </messages>
        </MyRecord>
        """;

    // when:
    String actual = objectMapper.writerWithDefaultPrettyPrinter().writeValueAsString(dto);

    // then:
    Assertions.assertEquals(actual.replace("\r", ""), xml.replace("\r", ""));
  }
}

Stacktrace of test_deserialize

com.fasterxml.jackson.databind.exc.InvalidDefinitionException: Invalid definition for property 'messages' (of type `MyRecord`): Could not find creator property with name 'messages' (known Creator properties: [message])
 at [Source: (StringReader); line: 2, column: 3]
	at com.fasterxml.jackson.databind.exc.InvalidDefinitionException.from(InvalidDefinitionException.java:62)
	at com.fasterxml.jackson.databind.DeserializationContext.reportBadPropertyDefinition(DeserializationContext.java:1759)
	at com.fasterxml.jackson.databind.deser.BeanDeserializerFactory.addBeanProps(BeanDeserializerFactory.java:630)
	at com.fasterxml.jackson.databind.deser.BeanDeserializerFactory.buildBeanDeserializer(BeanDeserializerFactory.java:277)
	at com.fasterxml.jackson.databind.deser.BeanDeserializerFactory.createBeanDeserializer(BeanDeserializerFactory.java:150)
	at com.fasterxml.jackson.databind.deser.DeserializerCache._createDeserializer2(DeserializerCache.java:414)
	at com.fasterxml.jackson.databind.deser.DeserializerCache._createDeserializer(DeserializerCache.java:349)
	at com.fasterxml.jackson.databind.deser.DeserializerCache._createAndCache2(DeserializerCache.java:264)
	at com.fasterxml.jackson.databind.deser.DeserializerCache._createAndCacheValueDeserializer(DeserializerCache.java:244)
	at com.fasterxml.jackson.databind.deser.DeserializerCache.findValueDeserializer(DeserializerCache.java:142)
	at com.fasterxml.jackson.databind.DeserializationContext.findRootValueDeserializer(DeserializationContext.java:591)
	at com.fasterxml.jackson.databind.ObjectMapper._findRootDeserializer(ObjectMapper.java:4733)
	at com.fasterxml.jackson.databind.ObjectMapper._readMapAndClose(ObjectMapper.java:4594)
	at com.fasterxml.jackson.databind.ObjectMapper.readValue(ObjectMapper.java:3548)
	at com.fasterxml.jackson.databind.ObjectMapper.readValue(ObjectMapper.java:3516)
	at MyRecordTest.test_deserialize(MyRecordTest.java:23)

Adrodoc avatar Oct 17 '22 14:10 Adrodoc

It seems this is also reported as https://github.com/FasterXML/jackson-dataformat-xml/issues/517

Adrodoc avatar Oct 17 '22 14:10 Adrodoc

@stanislasbonifetto can you please rename this issue to "@JacksonXmlElementWrapper doesn't work in constructors"? It has nothing to do with Kotlin or data classes as was discussed, removing data doesn't change a thing, it's the presence of the primary constructor that fails things. But it would break even with secondary constructors, or in Java: simply using immutable coding style in Java exhibits the same problem.

@cowtowncoder allowing JacksonXmlElementWrapper on parameters seems like it wasn't enough, I'm using jackson-dataformat-xml-2.14.1 and it's still broken. Here's a full self-contained test that you asked for in https://github.com/FasterXML/jackson-module-kotlin/issues/153#issuecomment-391489596, please do note that this bug violates one of the basic principles listed in https://github.com/FasterXML/jackson-dataformat-xml#overview

What should be guaranteed is that any XML written using this module must be readable using module as well ("read what I wrote"): that is, we do aim for full round-trip support

Behind this collapse block is a very basic JUnit Jupiter test in Java demonstrating the issue
import java.util.Arrays;
import java.util.List;

import org.junit.jupiter.api.Test;

import static org.junit.jupiter.api.Assertions.assertEquals;

import com.example.RootWithWrappedChild.Element;
import com.fasterxml.jackson.annotation.JsonProperty;
import com.fasterxml.jackson.core.JsonProcessingException;
import com.fasterxml.jackson.databind.SerializationFeature;
import com.fasterxml.jackson.dataformat.xml.JacksonXmlModule;
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;

public class SerializationTest {

	@Test
	public void serialize() throws JsonProcessingException {
		XmlMapper sut = new XmlMapper(new JacksonXmlModule());
		sut.enable(SerializationFeature.INDENT_OUTPUT);

		RootWithWrappedChild input = new RootWithWrappedChild(
				Arrays.asList(
						new Element("Text 1"),
						new Element("Text 2")
				)
		);

		String actual = sut.writeValueAsString(input);

		String expected = """
				<parent>
				  <wrapper>
				    <item>
				      <content>Text 1</content>
				    </item>
				    <item>
				      <content>Text 2</content>
				    </item>
				  </wrapper>
				</parent>
				""";
		assertEquals(clean(expected), clean(actual));
	}

	// Fails with below during sut.readValue call:
	// com.fasterxml.jackson.databind.exc.InvalidDefinitionException:
	// Invalid definition for property 'wrapper' (of type `com.example.RootWithWrappedChild`):
	// Could not find creator property with name 'wrapper' (known Creator properties: [item])
	// at [Source: (StringReader); line: 1, column: 1]
	@Test
	public void deserialize() throws JsonProcessingException {
		XmlMapper sut = new XmlMapper(new JacksonXmlModule());

		String input = """
				<parent>
				  <wrapper>
				    <item>
				      <content>Text 1</content>
				    </item>
				    <item>
				      <content>Text 2</content>
				    </item>
				  </wrapper>
				</parent>
				""";

		RootWithWrappedChild actual = sut.readValue(input, RootWithWrappedChild.class);

		assertEquals(2, actual.getElements().size());
		assertEquals("Text 1", actual.getElements().get(0).getContent());
		assertEquals("Text 2", actual.getElements().get(1).getContent());
	}

	private static String clean(String xml) {
		return xml.replaceAll("[\n\r]+", "\n").trim();
	}
}

// Note: properties and types (which give implicit names) are named distinctly
// to avoid confusion with explicit names in annotations.
@JacksonXmlRootElement(localName = "parent")
class RootWithWrappedChild {

	// These two work perfect for serialization: here, or on the getter, or both.
	@JacksonXmlElementWrapper(localName = "wrapper")
	@JacksonXmlProperty(localName = "item")
	private final List<Element> elements;

	public RootWithWrappedChild(
			// These are required for deserialization, to get creators.
			@JacksonXmlProperty(localName = "item")
			// BUG: this gets ignored, and we get the dreaded:
			// Could not find creator property with name 'wrapper' (known Creator properties: [item])
			// It all works fine if the wrapper and property names match exactly.
			@JacksonXmlElementWrapper(localName = "wrapper")
			List<Element> elements
	) {
		this.elements = elements;
	}

	// These two work perfect for serialization: here, or on the backing field, or both.
	@JacksonXmlElementWrapper(localName = "wrapper")
	@JacksonXmlProperty(localName = "item")
	public List<Element> getElements() {
		return elements;
	}

	/** Nothing to see here: it's just a standard - automatically serialized - POJO. */
	public static class Element {

		private final String content;

		// JsonProperty only necessary if there's no -parameters compiler flag as far as I can tell. Anyway, I digress.
		public Element(@JsonProperty("content") String content) {
			this.content = content;
		}

		public String getContent() {
			return content;
		}
	}
}

I do hope this helps things to move along, because Kotlin and Java records are here to stay and full support would be more than ideal.

TWiStErRob avatar Dec 06 '22 17:12 TWiStErRob

This is closed because it does not appear to be a problem caused by kotlin-module and should be handled by https://github.com/FasterXML/jackson-dataformat-xml/issues/517.

k163377 avatar Apr 02 '23 05:04 k163377

@k163377 Java records and Kotlin data classes are not the same thing, unless the Kotlin data class is annotated with @JvmRecord AND targeting a supported Java version. We can use data classes on Java 7 onwards.

TWiStErRob avatar Apr 02 '23 08:04 TWiStErRob

@TWiStErRob If the problem is reproduced only in Java and there is nothing to fix in jackson-module-kotlin, then it is not a jackson-module-kotlin problem, right? If so, I see no reason why this issue should be left open.

k163377 avatar Apr 02 '23 09:04 k163377

According to https://github.com/FasterXML/jackson-dataformat-xml/issues/517 the kotlin issue is not related to the java issue. So maybe this should be reopened? @k163377 ?

magott avatar Apr 19 '23 12:04 magott

With such a lengthy discussion as what is here, I think filing a new issue with summary of the current state, reproduction (and just a reference to this as older issue) would make sense. But reproduction itself would need to go to jackson-integration-tests since neither Kotlin module nor XML module can refer to each other directly (even wrt tests).

But one big problem we have here is that there's little overlap with people who know XML module well (myself included) and Kotlin module (not me, but @k163377) ... and has time to dig into this issue.

cowtowncoder avatar Apr 20 '23 03:04 cowtowncoder