FasterXML / jackson-dataformat-xml

Extension for Jackson JSON processor that adds support for serializing POJOs as XML (and deserializing from XML) as an alternative to JSON
Apache License 2.0
561 stars 221 forks source link

`XmlMapper` serializes `@JsonAppend` property twice #578

Closed cowtowncoder closed 1 year ago

cowtowncoder commented 1 year ago

Discussed in https://github.com/FasterXML/jackson-databind/discussions/3806

Originally posted by **stepince** March 5, 2023 XmlMapper is serializing jsonAppend virtual property twice. ObjectMapper for json works correctly. jackson version: 2.14.1 ``` public class VirtualBeanPropertyWriterTest { @Test public void testJsonAppend() throws Exception { ObjectMapper mapper = new XmlMapper(); String xml = mapper.writeValueAsString(new Pojo("foo")); assertEquals("foobar",xml); } @JsonAppend(props = @JsonAppend.Prop(name = "virtual", value = MyVirtualPropertyWriter.class)) public static class Pojo { private final String name; public Pojo(String name) { this.name = name; } public String getName() { return name; } } public static class MyVirtualPropertyWriter extends VirtualBeanPropertyWriter { public MyVirtualPropertyWriter() {} protected MyVirtualPropertyWriter(BeanPropertyDefinition propDef, Annotations contextAnnotations, JavaType declaredType) { super(propDef, contextAnnotations, declaredType); } @Override protected Object value(Object bean, JsonGenerator jgen, SerializerProvider prov) throws Exception { return "bar"; } @Override public VirtualBeanPropertyWriter withConfig(MapperConfig config, AnnotatedClass declaringClass, BeanPropertyDefinition propDef, JavaType type) { return new MyVirtualPropertyWriter(propDef, declaringClass.getAnnotations(), type); } } } ``` output ``` org.opentest4j.AssertionFailedError: Expected :`foobar` Actual :`foobarbar`
cowtowncoder commented 1 year ago

Sounds like a flaw. Thank you for reporting @stepince

stepince commented 1 year ago

Any workaround for this issue? I am actually using a mixin class for the serialization. Maybe in VirtualBeanPropertyWriter Impl check if the property has been written. Not sure if this is possible?

cowtowncoder commented 1 year ago

@stepince Without knowing why it's failing it is hard to suggest any workaround. But no, writer should not really have to check for anything, backend should not write duplicates.

I hope I'll have time to look into this in near future -- existence of unit test should be great help.

mukham12 commented 1 year ago

Hi @cowtowncoder,

Not sure if this helps, but I was able to track down where the property is getting set twice. It is in this file (AnnotationIntrospectorPair.java) lines 594 and 595. The first virtual prop is getting set when _primary calls findAndAddVirtualProperties and also the second time when _secondary calls findAndAddVirtualProperties. Thanks.

cowtowncoder commented 1 year ago

@mukham12 that is indeed helpful! Question then being why do we have 2 JacksonAnnotationIntrospector instances registered (-Pair is used to combine 2 AIs).

cowtowncoder commented 1 year ago

Ah. That is because JacksonXmlModule inserts our special JacksonXmlAnnotationIntrospector; it won't (and basically can't) replace plain JacksonAnnotationIntrospector. But what that means, then, is that it doubles up this call.

Hmmh. In a way fix is easy, although I need to think it through -- basically have a switch to prevent some of the calls to super class. The reason for a switch (as opposed to just blocking) is that this implementation may be used directly as well, as a replacement of JacksonAnnotationIntrospector (instead of augmenting) -- if so, calls should be handled by base implementation.

But at least I now know how to implement this.

mukham12 commented 1 year ago

Happy to help.

I may be able to help with the implementation if you can just nudge me in the right direction and lay out the overall plan. Thanks.

cowtowncoder commented 1 year ago

Fixed by simply overriding and blocking findAndAddVirtualProperties() in JacksonXmlAnnotationIntrospector. Hopefully works well enough; it's bit of a design flaw (this particular method in AnnotationIntrospector)

stepince commented 1 year ago

Thx Tatu. I see that the fix will be released in 2.15 . I don't see a release date for 2.15.

cowtowncoder commented 1 year ago

@stepince Project does not have pre-planned release dates; releases occur when versions are considered ready.

Having said that, general expectations are updated sometimes on project release page:

https://github.com/FasterXML/jackson/wiki/Jackson-Release-2.15