retrofit icon indicating copy to clipboard operation
retrofit copied to clipboard

JaxbConverterFactory doesn't recognize @XmlJavaTypeAdapter

Open mahmoudmohsen213 opened this issue 5 years ago • 2 comments

What kind of issue is this?

  • [ ] Question. This issue tracker is not the place for questions. If you want to ask how to do something, or to understand why something isn't working the way you expect it to, use Stack Overflow. https://stackoverflow.com/questions/tagged/retrofit

  • [ ] Bug report. If you’ve found a bug, spend the time to write a failing test. Bugs with tests get fixed. Here’s an example: https://gist.github.com/swankjesse/6608b4713ad80988cdc9

  • [x] Feature Request. Start by telling us what problem you’re trying to solve. Often a solution already exists! Don’t send pull requests to implement new features without first getting our support. Sometimes we leave features out on purpose to keep the project small.

The way JAXB deals with classes that doesn't have a no-arg constructor or classes with final fields is by using an XmlAdapter, and to configure an XmlAdapter for some class, we need to annotate this class with XmlJavaTypeAdapter.

The problem happens if the root is an immutable class, in this case, the root java class will be annotated with XmlJavaTypeAdapter, which the JaxbConverterFactory doesn't recognize (it only checks isAnnotationPresent for XmlRootElement), and will return a null value, so retrofit will assume that the request/response cannot be converted using JAXB.

It would be an easy change to also check for XmlJavaTypeAdapter annotation.

Complete tutorial: http://blog.bdoughan.com/2010/12/jaxb-and-immutable-objects.html

mahmoudmohsen213 avatar Mar 31 '20 15:03 mahmoudmohsen213

My question is "why Jaxb is suggested solution" if it does not support no-arg constructor classes? Its almost all Data Classes in Kotlin when dealing with Api.

jemshit avatar Nov 14 '20 11:11 jemshit

Well... I honestly didn't try to analyse the impact of this bug in Kotlin, I was trying to do it in Java. But, regardless of the language, Jaxb does know how to support no-arg constructor classes, it's the retrofit library that doesn't check for all annotations used by Jaxb. It is not about Jaxb being the suggested solution, I don't if it the best solution, it totally depends on what you are trying to do with retrofit, but the fact remains that retrofit has Jaxb converters, so it only make sense for this support to be complete, especially that this bug impacts only cases where the root class is immutable, so this is clearly a defect, not something that is left out by design.

Also, the simpleXML is deprecated, and the readme clearly directs the user to go for Jaxb, so in case I am dealing with an endpoint that accepts/returns an XML request/response body, I'll need to use Jaxb.

mahmoudmohsen213 avatar Nov 27 '20 18:11 mahmoudmohsen213