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

Don't use custom BeanPropertyWriter for xml module #7170

Merged
merged 2 commits into from Apr 5, 2022
Merged

Don't use custom BeanPropertyWriter for xml module #7170

merged 2 commits into from Apr 5, 2022

Conversation

yawkat
Copy link
Member

@yawkat yawkat commented Mar 31, 2022

Our BeanIntrospectionPropertyWriter is fundamentally incompatible with the XmlBeanPropertyWriter. This patch skips use of our writer if the xml writer is present, to preserve the proper xml serialization behavior.

The biggest disadvantage to this approach is that it only works if reflection is available. There also isn't the perf improvement of BeanIntrospectionPropertyWriter.

An alternative here would be to add support for the XML features to the BeanIntrospectionModule. I tried a limited version of this (only the features directly supported by the XmlBeanPropertyWriter), and it works, but only with reflection available. So the native-image issue remains unresolved even with the limited xml support.

Full XML support without reflection would basically need a reimpl of big parts of jackson-dataformat-xml, which is certainly out of scope for micronaut-jackson-databind, though it could be a future feature of micronaut-xml.

Fixes #5907

Our BeanIntrospectionPropertyWriter is fundamentally incompatible with the XmlBeanPropertyWriter. This patch skips use of our writer if the xml writer is present, to preserve the proper xml serialization behavior.

The biggest disadvantage to this approach is that it only works if reflection is available. There also isn't the perf improvement of BeanIntrospectionPropertyWriter.

An alternative here would be to add support for the XML features to the BeanIntrospectionModule. I tried a limited version of this (only the features directly supported by the XmlBeanPropertyWriter), and it works, but only with reflection available. So the native-image issue remains unresolved even with the limited xml support.

Full XML support without reflection would basically need a reimpl of big parts of jackson-dataformat-xml, which is certainly out of scope for micronaut-jackson-databind, though it could be a future feature of micronaut-xml.

Fixes #5907
# Conflicts:
#	jackson-databind/src/test/groovy/io/micronaut/jackson/modules/BeanIntrospectionModuleSpec.groovy
@sonarcloud
Copy link

sonarcloud bot commented Mar 31, 2022

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

100.0% 100.0% Coverage
0.0% 0.0% Duplication

@sdelamo sdelamo added this to the 3.4.2 milestone Apr 5, 2022
@sdelamo sdelamo merged commit 989e0dc into 3.4.x Apr 5, 2022
@sdelamo sdelamo deleted the xml-bim branch April 5, 2022 09:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Micronaut client does not generate proper XML for wrapped values
3 participants