Skip to content

Commit

Permalink
fix: don't use custom BeanPropertyWriter for xml module (#7170)
Browse files Browse the repository at this point in the history
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
  • Loading branch information
yawkat committed Apr 5, 2022
1 parent 2726575 commit 989e0dc
Show file tree
Hide file tree
Showing 3 changed files with 32 additions and 1 deletion.
1 change: 1 addition & 0 deletions jackson-databind/build.gradle
Expand Up @@ -31,6 +31,7 @@ dependencies {
testImplementation project(":inject-java")
testImplementation project(":inject-java-test")
testImplementation project(":inject-groovy")
testImplementation "com.fasterxml.jackson.dataformat:jackson-dataformat-xml"
if (!JavaVersion.current().isJava9Compatible()) {
testImplementation files(org.gradle.internal.jvm.Jvm.current().toolsJar)
}
Expand Down
Expand Up @@ -343,7 +343,10 @@ public JsonSerializer<?> build() {
final Optional<BeanProperty<Object, Object>> property = Optional.ofNullable(named.get(existing.getName()));
// ignore properties that are @JsonIgnore, so that we don't replace other properties of the
// same name
if (property.isPresent() && !property.get().isAnnotationPresent(JsonIgnore.class)) {
if (property.isPresent() &&
!property.get().isAnnotationPresent(JsonIgnore.class) &&
// we can't support XmlBeanPropertyWriter easily https://github.com/micronaut-projects/micronaut-core/issues/5907
!existing.getClass().getName().equals("com.fasterxml.jackson.dataformat.xml.ser.XmlBeanPropertyWriter")) { // NOSONAR
final BeanProperty<Object, Object> beanProperty = property.get();
newProperties.set(i, new BeanIntrospectionPropertyWriter(
existing,
Expand Down
Expand Up @@ -21,6 +21,8 @@ import com.fasterxml.jackson.databind.SerializerProvider
import com.fasterxml.jackson.databind.annotation.JsonDeserialize
import com.fasterxml.jackson.databind.annotation.JsonNaming
import com.fasterxml.jackson.databind.annotation.JsonSerialize
import com.fasterxml.jackson.dataformat.xml.XmlMapper
import com.fasterxml.jackson.dataformat.xml.annotation.JacksonXmlElementWrapper
import groovy.transform.EqualsAndHashCode
import groovy.transform.PackageScope
import io.micronaut.context.ApplicationContext
Expand Down Expand Up @@ -965,6 +967,31 @@ class BeanIntrospectionModuleSpec extends Specification {
}
}

@Issue('https://github.com/micronaut-projects/micronaut-core/issues/5907')
void "xml modifier"() {
given:
ApplicationContext ctx = ApplicationContext.run()
XmlMapper objectMapper = new XmlMapper()
objectMapper.registerModule(ctx.getBean(BeanIntrospectionModule))

expect:
objectMapper.writeValueAsString(new UsesXmlElementWrapper()) ==
'<UsesXmlElementWrapper><nestedItems><strings>foo</strings><strings>bar</strings></nestedItems></UsesXmlElementWrapper>'

cleanup:
ctx.close()
}

@Introspected
static class UsesXmlElementWrapper {
private List<String> strings = ["foo", "bar"];

@JacksonXmlElementWrapper(/*useWrapping = false, */localName = "nestedItems")
public List<String> getStrings() {
return strings
}
}

@Unroll("JsonProperty annotation is supported with ignoreReflectiveProperties: #ignoreReflectiveProperties")
void "JsonProperty support"(boolean ignoreReflectiveProperties) {
given:
Expand Down

0 comments on commit 989e0dc

Please sign in to comment.