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
574 stars 222 forks source link

XML module not registered correctly when setting a custom `SerializerFactory` #678

Closed SimonCockx closed 3 weeks ago

SimonCockx commented 1 month ago

I'm using a custom SerializerFactory as follows.

ObjectMapper mapper = new XmlMapper().setSerializerFactory(MySerializerFactory.INSTANCE);

I noticed that in the constructor of XmlMapper, the default XML_MODULE is registered, which also configures the SerializerFactory. Since I'm overriding it after constructing, this configuration is lost, resulting in different behaviour, which is very confusing.

I currently don't see any way for registering my custom SerializerFactory before the XML_MODULE is registered. Is this intended? It seems error-prone.

My current workaround:

ObjectMapper mapper = new XmlMapper((JacksonXmlModule) null)
    .setSerializerFactory(RosettaSerialiserFactory.INSTANCE)
    .registerModule(new JacksonXmlModule());
cowtowncoder commented 1 month ago

I think that overriding of SerializerFactory is very rare thing, this is probably the first time I've heard users doing that. So I don't think the problem here was anticipated.

It might help if I understood your use case better: why are you overriding SerializerFactory?

SimonCockx commented 1 month ago

See my comment in https://github.com/FasterXML/jackson-dataformat-xml/issues/676#issuecomment-2438049500.

At the moment I'm using a custom SerializerFactory to "hack in" some solutions to that issue. :)

cowtowncoder commented 1 month ago

Ah ok. Yes, we should make it possible to configure SerializerFactory even with XML module. Right now I am bit overloaded with Jackson work so I may not have time to work on this, BUT, if you had time to maybe propose a pr, I'd make sure find time to review and help with it.

Without remembering full details, I suspect JacksonXmlModule should expose configurability of SerializerFactory -- it seems to be passed to XmlSerializerProvider (sub-class of default SerializerProvider) as it's, well, required.

But I guess I am not sure which aspects of SerializerFactory configuration you are referring to? No custom factory is used, and although some set methods probably set things on that factory indirectly, I don't see immediate paths.

SimonCockx commented 1 month ago

But I guess I am not sure which aspects of SerializerFactory configuration you are referring to? No custom factory is used, and although some set methods probably set things on that factory indirectly, I don't see immediate paths.

Just to clarify the issue: the JacksonXmlModule adds the following two modifiers:

context.addBeanSerializerModifier(new XmlBeanSerializerModifier());
context.addBeanDeserializerModifier(new XmlBeanDeserializerModifier(_cfgNameForTextElement));

When setting the SerializerFactory after this module is registered, these modifiers are lost.

BUT, if you had time to maybe propose a pr, I'd make sure find time to review and help with it.

Thanks. I'll see if I find a moment, but can't make any promise either. :)

cowtowncoder commented 1 month ago

Ah. Yes, flow of configuration gets bit hairy, since SerializerFactory is amongst oldest extant abstractions and its configuration was not as well-thought-out as newer additions. This does explain the problem well fwtw.

And yes, pr would be great but we all have limited time so I take whatever I can get. :)

cowtowncoder commented 3 weeks ago

I did #680 and I think it might be the best small thing I can do; will merge.