Skip to content

Naive attempt at fixing #306 #754

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

Original file line number Diff line number Diff line change
Expand Up @@ -37,12 +37,19 @@ public class JacksonXmlAnnotationIntrospector

protected boolean _cfgDefaultUseWrapper;

protected final JacksonXmlAnnotationIntrospectorConfig _cfgIntrospectorConfig;

public JacksonXmlAnnotationIntrospector() {
this(DEFAULT_USE_WRAPPER);
}

public JacksonXmlAnnotationIntrospector(boolean defaultUseWrapper) {
this(defaultUseWrapper, new JacksonXmlAnnotationIntrospectorConfig());
}

public JacksonXmlAnnotationIntrospector(boolean defaultUseWrapper, JacksonXmlAnnotationIntrospectorConfig introspectorConfig) {
_cfgDefaultUseWrapper = defaultUseWrapper;
_cfgIntrospectorConfig = introspectorConfig;
}

/*
Expand Down Expand Up @@ -208,6 +215,13 @@ public PropertyName findNameForDeserialization(MapperConfig<?> config, Annotated
PropertyName pn = PropertyName.merge(_findXmlName(a),
super.findNameForDeserialization(config, a));
if (pn == null) {
JacksonXmlText jacksonXmlTextAnnotation = _findAnnotation(a, JacksonXmlText.class);

if (jacksonXmlTextAnnotation != null && jacksonXmlTextAnnotation.value() &&
!_cfgIntrospectorConfig.inferXmlTextPropertyName()) {
return _cfgIntrospectorConfig.xmlTextPropertyName();
}

if (_hasOneOf(a, ANNOTATIONS_TO_INFER_XML_PROP)) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will look for @JacksonXmlText OR @JacksonXmlElementWrapper and handling will now differ -- former should use configured "text element name", latter should not (should return "use default").

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do not quite understand, the highlighted line only looks for @JacksonXmlText

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, I meant line 225 right after (if (_hasOneOf(...)).

return PropertyName.USE_DEFAULT;
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
package tools.jackson.dataformat.xml;

import tools.jackson.databind.PropertyName;
import tools.jackson.dataformat.xml.deser.FromXmlParser;

import java.io.Serializable;

public record JacksonXmlAnnotationIntrospectorConfig(
boolean inferXmlTextPropertyName,
PropertyName xmlTextPropertyName //Only honored if inferXmlTextPropertyName is false
) implements Serializable {

/**
* Constructs a JacksonXmlAnnotationIntrospectorConfig with the default configuration
* Does not infer the XmlTextPropertyName by default and uses {@link FromXmlParser#DEFAULT_TEXT_PROPERTY} for the {@link PropertyName}.
*/
public JacksonXmlAnnotationIntrospectorConfig() {
this(false, PropertyName.construct(FromXmlParser.DEFAULT_TEXT_PROPERTY));
}

public JacksonXmlAnnotationIntrospectorConfig withInferXmlTextPropertyName(boolean inferXmlTextPropertyName) {
return new JacksonXmlAnnotationIntrospectorConfig(inferXmlTextPropertyName, this.xmlTextPropertyName);
}

public JacksonXmlAnnotationIntrospectorConfig withXmlTextPropertyName(PropertyName xmlTextPropertyName) {
return new JacksonXmlAnnotationIntrospectorConfig(this.inferXmlTextPropertyName, xmlTextPropertyName);
}
}
2 changes: 1 addition & 1 deletion src/main/java/tools/jackson/dataformat/xml/XmlFactory.java
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ public XmlFactory(XMLInputFactory xmlIn) {
public XmlFactory(XMLInputFactory xmlIn, XMLOutputFactory xmlOut) {
this(DEFAULT_XML_READ_FEATURE_FLAGS, DEFAULT_XML_WRITE_FEATURE_FLAGS,
xmlIn, xmlOut, XmlNameProcessors.newPassthroughProcessor(),
FromXmlParser.DEFAULT_UNNAMED_TEXT_PROPERTY);
FromXmlParser.DEFAULT_TEXT_PROPERTY);
}

protected XmlFactory(int xpFeatures, int xgFeatures,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ protected XmlFactoryBuilder() {
XmlFactory.DEFAULT_XML_WRITE_FEATURE_FLAGS);
_classLoaderForStax = null;
_nameProcessor = XmlNameProcessors.newPassthroughProcessor();
_nameForTextElement = FromXmlParser.DEFAULT_UNNAMED_TEXT_PROPERTY;
_nameForTextElement = FromXmlParser.DEFAULT_TEXT_PROPERTY;
}

public XmlFactoryBuilder(XmlFactory base) {
Expand Down
2 changes: 1 addition & 1 deletion src/main/java/tools/jackson/dataformat/xml/XmlMapper.java
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ public Builder(XmlFactory f) {
// String into `null` (where it otherwise is an error) is very useful.
enable(DeserializationFeature.ACCEPT_EMPTY_STRING_AS_NULL_OBJECT);
_defaultUseWrapper = JacksonXmlAnnotationIntrospector.DEFAULT_USE_WRAPPER;
_nameForTextElement = FromXmlParser.DEFAULT_UNNAMED_TEXT_PROPERTY;
_nameForTextElement = FromXmlParser.DEFAULT_TEXT_PROPERTY;

// as well as AnnotationIntrospector: note, however, that "use wrapper" may well
// change later on
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,10 +35,14 @@ public class FromXmlParser
implements ElementWrappable
{
/**
* The default name placeholder for XML text segments is empty
* String ("").
* The default name placeholder for XML text segments: used because Token stream
* requires all values inside "Objects" to have names associated.
* For Jackson 3.x this is {@code <xml:text>}; in 2.x matching constant was defined
* as empty String ({@code ""}).
*
* @since 3.0 Constant was renamed: was {@code DEFAULT_UNNAMED_TEXT_PROPERTY} in 2.x
*/
public final static String DEFAULT_UNNAMED_TEXT_PROPERTY = "";
public final static String DEFAULT_TEXT_PROPERTY = "<xml:text>";

/**
* XML format has some peculiarities, indicated via capability
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,74 @@
package tools.jackson.dataformat.xml.deser;

import org.junit.jupiter.api.Test;
import tools.jackson.databind.DatabindException;
import tools.jackson.databind.PropertyName;
import tools.jackson.dataformat.xml.JacksonXmlAnnotationIntrospector;
import tools.jackson.dataformat.xml.JacksonXmlAnnotationIntrospectorConfig;
import tools.jackson.dataformat.xml.XmlMapper;
import tools.jackson.dataformat.xml.XmlTestUtil;
import tools.jackson.dataformat.xml.annotation.JacksonXmlProperty;
import tools.jackson.dataformat.xml.annotation.JacksonXmlText;

import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.junit.jupiter.api.Assertions.assertTrue;

public class DifferentDeserializationPropertyNameTest extends XmlTestUtil
{
static class TestBean {
@JacksonXmlProperty(localName = "wrong")
String wrong;

@JacksonXmlText
String name;
}

/*
/**********************************************************************
/* Test methods
/**********************************************************************
*/

@Test
public void testWithExplicitProperty() {
final XmlMapper mapper = XmlMapper.builder()
.annotationIntrospector(new JacksonXmlAnnotationIntrospector(false,
new JacksonXmlAnnotationIntrospectorConfig(false, new PropertyName("name"))))
.build();

String xmlInput = "<testBean>ABC123</testBean>";

TestBean testBean = mapper.readValue(xmlInput, TestBean.class);

assertEquals("ABC123", testBean.name);
}

@Test
public void testWithInferName() {
final XmlMapper mapper = XmlMapper.builder()
.annotationIntrospector(new JacksonXmlAnnotationIntrospector(false,
new JacksonXmlAnnotationIntrospectorConfig(true, null)))
.build();

String xmlInput = "<testBean>DEF</testBean>";

TestBean testBean = mapper.readValue(xmlInput, TestBean.class);

assertEquals("DEF", testBean.name);
}

@Test
public void testWithDuplicateExplicitProperty() {
final XmlMapper mapper = XmlMapper.builder()
.annotationIntrospector(new JacksonXmlAnnotationIntrospector(false,
new JacksonXmlAnnotationIntrospectorConfig(false, new PropertyName("wrong"))))
.build();

String xmlInput = "<testBean>DEF</testBean>";

Exception result = assertThrows(DatabindException.class, () -> mapper.readValue(xmlInput, TestBean.class));

assertTrue(result.getMessage().contains("Multiple fields representing property \"wrong\""));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ public void testMapWithAttr() throws Exception
assertEquals(1, map.size());
Map<String,Object> inner = new LinkedHashMap<>();
inner.put("lang", "en");
inner.put("", "John Smith");
inner.put(FromXmlParser.DEFAULT_TEXT_PROPERTY, "John Smith");
assertEquals(Collections.singletonMap("person", inner), map);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ public void testMixedContent() throws Exception
final String XML = "<root>first<a>123</a>second<b>abc</b>last</root>";
final JsonNode fromXml = XML_MAPPER.valueToTree(XML_MAPPER.readValue(XML, Object.class));
final ObjectNode exp = XML_MAPPER.createObjectNode();
exp.putArray("")
exp.putArray(FromXmlParser.DEFAULT_TEXT_PROPERTY)
.add("first")
.add("second")
.add("last");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
import tools.jackson.databind.node.JsonNodeType;
import tools.jackson.databind.node.ObjectNode;
import tools.jackson.dataformat.xml.XmlTestUtil;
import tools.jackson.dataformat.xml.deser.FromXmlParser;

import static org.junit.jupiter.api.Assertions.*;

Expand Down Expand Up @@ -43,7 +44,7 @@ public void testMixedContent() throws Exception
{
JsonNode fromXml = XML_MAPPER.readTree("<root>first<a>123</a>second<b>abc</b>last</root>");
final ObjectNode exp = XML_MAPPER.createObjectNode();
exp.putArray("")
exp.putArray(FromXmlParser.DEFAULT_TEXT_PROPERTY)
Copy link
Member

@cowtowncoder cowtowncoder May 13, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ohhhhhhh. This is NOT good -- what used to be something like:

{
  "":["first","second","last"],
  "a":"123",
  "b":"abc"
}

now looks like:

{
  "<xml:text>":["first","second","last"],
  "a":"123",
  "b":"abc"
}

which I don't think is what anyone likes to see :-(

So for JsonNode at least exposing XML character data sections should be with nominal key of "".
And I don't think it is reasonable to expected those using XmlMapper.readTree() to have explicitly configure things to work this way.

We need to figure out another way to handle the issue, I think.

Copy link
Contributor Author

@duoduobingbing duoduobingbing May 13, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need to figure out another way to handle the issue, I think.

I mean this PR was just a naive shot at solving #306 by changing the property name, if it has to be "" I'm fine with closing this PR - because I do not know how to make it so, that the property name differs from the nominal key.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right. I'll leave this open because I'd really like to figure out a way and feel there probably is a way (despite not seeing it yet). :)

I appreciate your attempt; sorry it took me a while to sync up to what changes really mean.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No Problem, thanks for having a look at it. I will close this for now. Feel free to reopen or use the code in any way, shape or form you see fit.

.add("first")
.add("second")
.add("last");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
import tools.jackson.databind.ObjectMapper;
import tools.jackson.databind.json.JsonMapper;
import tools.jackson.dataformat.xml.XmlTestUtil;
import tools.jackson.dataformat.xml.deser.FromXmlParser;

import static org.junit.jupiter.api.Assertions.assertEquals;

Expand All @@ -19,23 +20,23 @@ public class JsonNodeMixedContent403Test extends XmlTestUtil
public void testMixedContentBefore() throws Exception
{
// First, before elements:
assertEquals(JSON_MAPPER.readTree(a2q("{'':'before','a':'1','b':'2'}")),
assertEquals(JSON_MAPPER.readTree(a2q(String.format("{'%s':'before','a':'1','b':'2'}", FromXmlParser.DEFAULT_TEXT_PROPERTY))),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

and same here obviously.

XML_MAPPER.readTree("<root>before<a>1</a><b>2</b></root>"));
}

@Test
public void testMixedContentBetween() throws Exception
{
// Second, between
assertEquals(JSON_MAPPER.readTree(a2q("{'a':'1','':'between','b':'2'}")),
assertEquals(JSON_MAPPER.readTree(a2q(String.format("{'a':'1','%s':'between','b':'2'}", FromXmlParser.DEFAULT_TEXT_PROPERTY))),
XML_MAPPER.readTree("<root><a>1</a>between<b>2</b></root>"));
}

@Test
public void testMixedContentAfter() throws Exception
{
// and then after
assertEquals(JSON_MAPPER.readTree(a2q("{'a':'1','b':'2','':'after'}")),
assertEquals(JSON_MAPPER.readTree(a2q(String.format("{'a':'1','b':'2','%s':'after'}", FromXmlParser.DEFAULT_TEXT_PROPERTY))),
XML_MAPPER.readTree("<root><a>1</a><b>2</b>after</root>"));
}

Expand All @@ -44,7 +45,7 @@ public void testMultipleMixedContent() throws Exception
{
// and then after
assertEquals(JSON_MAPPER.readTree(
a2q("{'':['first','second','third'],'a':'1','b':'2'}")),
a2q(String.format("{'%s':['first','second','third'],'a':'1','b':'2'}", FromXmlParser.DEFAULT_TEXT_PROPERTY))),
XML_MAPPER.readTree("<root>first<a>1</a>second<b>2</b>third</root>"));
}

Expand All @@ -57,7 +58,7 @@ public void testMixed226() throws Exception
+" mixed2</a>\n"
+"</root>";
JsonNode fromJson = JSON_MAPPER.readTree(
a2q("{'a':{'':['mixed1 ',' mixed2'],'b':'leaf'}}"));
a2q(String.format("{'a':{'%s':['mixed1 ',' mixed2'],'b':'leaf'}}", FromXmlParser.DEFAULT_TEXT_PROPERTY)));
assertEquals(fromJson, XML_MAPPER.readTree(XML));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ public void testMixedContentBeforeElement442() throws Exception
// Here's what we are missing:
assertToken(JsonToken.START_OBJECT, xp.nextToken());
assertToken(JsonToken.PROPERTY_NAME, xp.nextToken());
assertEquals("", xp.currentName());
assertEquals(FromXmlParser.DEFAULT_TEXT_PROPERTY, xp.currentName());

assertToken(JsonToken.VALUE_STRING, xp.nextToken());
assertEquals("text", xp.getString().trim());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ public void testRootScalar() throws Exception
try (JsonParser p = _xmlMapper.createParser(XML)) {
assertToken(JsonToken.START_OBJECT, p.nextToken());
assertToken(JsonToken.PROPERTY_NAME, p.nextToken());
assertEquals("", p.currentName());
assertEquals(FromXmlParser.DEFAULT_TEXT_PROPERTY, p.currentName());
assertToken(JsonToken.VALUE_STRING, p.nextToken());
assertEquals("value", p.getString());
assertToken(JsonToken.END_OBJECT, p.nextToken());
Expand All @@ -118,7 +118,7 @@ public void testRootMixed() throws Exception
assertToken(JsonToken.START_OBJECT, p.nextToken());

assertToken(JsonToken.PROPERTY_NAME, p.nextToken());
assertEquals("", p.currentName());
assertEquals(FromXmlParser.DEFAULT_TEXT_PROPERTY, p.currentName());
assertToken(JsonToken.VALUE_STRING, p.nextToken());
assertEquals("value", p.getString());

Expand Down Expand Up @@ -343,7 +343,8 @@ public void testXmlAttributes() throws Exception
@Test
public void testMixedContent() throws Exception
{
String exp = a2q("{'':'first','a':'123','':'second','b':'456','':'last'}");
String exp = a2q(String.format("{'%1$s':'first','a':'123','%1$s':'second','b':'456','%1$s':'last'}",
FromXmlParser.DEFAULT_TEXT_PROPERTY));
String result = _readXmlWriteJson("<root>first<a>123</a>second<b>456</b>last</root>");

//System.err.println("result = \n"+result);
Expand Down Expand Up @@ -373,7 +374,7 @@ public void testInferredNumbers() throws Exception
assertEquals(42, xp.getIntValue());

assertToken(JsonToken.PROPERTY_NAME, xp.nextToken()); // implicit for text
assertEquals("", xp.currentName());
assertEquals(FromXmlParser.DEFAULT_TEXT_PROPERTY, xp.currentName());

assertToken(JsonToken.VALUE_STRING, xp.nextToken());
assertTrue(xp.isExpectedNumberIntToken());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,6 @@ public void testIssue306NoCtor() throws Exception
}

// [dataformat-xml#423]
@JacksonTestFailureExpected
@Test
public void testXmlTextViaCtor423() throws Exception
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ record Amount(@JacksonXmlText String value,
private final String XML =
a2q("<Amt Ccy='EUR'>1</Amt>");

@JacksonTestFailureExpected
@Test
public void testDeser() throws Exception {
XmlMapper mapper = new XmlMapper();
Expand Down