Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

XmlMapper serializes @JsonAppend property twice #578

Closed
cowtowncoder opened this issue Mar 5, 2023 · 10 comments
Closed

XmlMapper serializes @JsonAppend property twice #578

cowtowncoder opened this issue Mar 5, 2023 · 10 comments
Labels
2.15 For issues planned for 2.15 has-failing-test Indicates that there exists a test case (under `failing/`) to reproduce the issue
Milestone

Comments

@cowtowncoder
Copy link
Member

cowtowncoder commented Mar 5, 2023

Discussed in FasterXML/jackson-databind#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("<Pojo><name>foo</name><virtual>bar</virtual></Pojo>",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 :`<Pojo><name>foo</name><virtual>bar</virtual></Pojo>`
Actual   :`<Pojo><name>foo</name><virtual>bar</virtual><virtual>bar</virtual></Pojo>`
</div>
@cowtowncoder cowtowncoder transferred this issue from FasterXML/jackson-databind Mar 5, 2023
@cowtowncoder
Copy link
Member Author

Sounds like a flaw. Thank you for reporting @stepince

@stepince
Copy link

stepince commented Mar 6, 2023

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 cowtowncoder added has-failing-test Indicates that there exists a test case (under `failing/`) to reproduce the issue 2.15 For issues planned for 2.15 labels Mar 7, 2023
@cowtowncoder
Copy link
Member Author

@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
Copy link

mukham12 commented Mar 14, 2023

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
Copy link
Member Author

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

@cowtowncoder
Copy link
Member Author

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
Copy link

mukham12 commented Mar 14, 2023

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 cowtowncoder changed the title XmlMapper serializes jsonAppend property twice XmlMapper serializes @JsonAppend property twice Mar 31, 2023
@cowtowncoder cowtowncoder added this to the 2.15.0 milestone Mar 31, 2023
@cowtowncoder
Copy link
Member Author

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
Copy link

stepince commented Apr 1, 2023

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

@cowtowncoder
Copy link
Member Author

@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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2.15 For issues planned for 2.15 has-failing-test Indicates that there exists a test case (under `failing/`) to reproduce the issue
Projects
None yet
Development

No branches or pull requests

3 participants