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

Fix #4771: Add feature for QNAME serialization and deserialization. #4909

Open
wants to merge 1 commit into
base: 2.19
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -431,6 +431,19 @@ public enum DeserializationFeature implements ConfigFeature
*/
READ_ENUMS_USING_TO_STRING(false),

/**
* Feature that determines standard deserialization mechanism used for
* QName values: if enabled, QNames are assumed to have been serialized using
* return value of <code>QName.toString()</code>;
* if disabled, it is assumed that the QName was serialized as an object.
*<p>
* Note: this feature should usually have same value
* as {@link SerializationFeature#WRITE_QNAMES_USING_TO_STRING}.
*<p>
* Feature is disabled by default.
*/
READ_QNAMES_USING_VALUE_OF(true),
Copy link
Member

Choose a reason for hiding this comment

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

Hmmh. This is rather too specific to define global feature for... Let me think about this a bit.

But is this actually needed? Deserializer should be able to support both styles based on which value shape is seen?
(this does not remove need for feature for serialization, but would avoid DeserializationFeature)

Copy link
Author

Choose a reason for hiding this comment

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

That's a fair point. I can make that change if, after you think about it, this is still the right approach.


/**
* Feature that allows unknown Enum values to be parsed as {@code null} values.
* If disabled, unknown Enum values will throw exceptions.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -292,6 +292,18 @@ public enum SerializationFeature implements ConfigFeature
*/
WRITE_ENUMS_USING_TO_STRING(false),

/**
* Feature that determines standard serialization mechanism used for
* QName values: if enabled, return value of <code>QName.toString()</code>
* is used; if disabled, the QName is serialized as an object.
*<p>
* Note: this feature should usually have same value
* as {@link DeserializationFeature#READ_QNAMES_USING_VALUE_OF}.
*<p>
* Feature is disabled by default.
Copy link
Member

Choose a reason for hiding this comment

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

If we are to go with the addition, needs @since 2.19 tag.

*/
WRITE_QNAMES_USING_TO_STRING(true),

/**
* Feature that determines whether Java Enum values are serialized
* as numbers (true), or textual values (false). If textual values are
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@
import javax.xml.namespace.QName;

import com.fasterxml.jackson.core.*;
import com.fasterxml.jackson.core.type.TypeReference;

import com.fasterxml.jackson.databind.*;
import com.fasterxml.jackson.databind.deser.Deserializers;
import com.fasterxml.jackson.databind.deser.std.FromStringDeserializer;
Expand Down Expand Up @@ -40,7 +42,10 @@ public JsonDeserializer<?> findBeanDeserializer(JavaType type,
{
Class<?> raw = type.getRawClass();
if (raw == QName.class) {
return new Std(raw, TYPE_QNAME);
if (config == null || config.isEnabled(DeserializationFeature.READ_QNAMES_USING_VALUE_OF)) {
return new Std(raw, TYPE_QNAME);
}
return new QNameObjectDeserializer();
}
if (raw == XMLGregorianCalendar.class) {
return new Std(raw, TYPE_G_CALENDAR);
Expand Down Expand Up @@ -149,4 +154,27 @@ protected XMLGregorianCalendar _gregorianFromDate(DeserializationContext ctxt,
return _dataTypeFactory.newXMLGregorianCalendar(calendar);
}
}

private class QNameObjectDeserializer extends JsonDeserializer<QName> {
private static final long serialVersionUID = 1L;
public static final TypeReference<Map<String, String>> STRING_MAP_TYPE_REFERENCE = new TypeReference<>() {};

@Override
public QName deserialize(final JsonParser p, final DeserializationContext ctxt)
throws IOException
{
Map<String, String> map;
try {
map = p.readValueAs(STRING_MAP_TYPE_REFERENCE);
Copy link
Author

@mcvayc mcvayc Jan 15, 2025

Choose a reason for hiding this comment

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

Is this the best way to deserialize the QName from a JSON object?

Copy link
Member

Choose a reason for hiding this comment

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

Ideally not reading via JsonParser, DeserializationContext methods are better (for a few reasons).
Might be easiest to read with:

JsonNode tree = ctxt.readTree(p);

and then extract values.

But before that, should probably check this is Object value:

if (!p.hasToken(JsonToken.START_OBJECT)) { // fail

or, read tree, check

if (!tree.isObject()) { // fail

} catch (IOException e) {
throw new JsonMappingException(p, "Unable to parse the QName as an object.", e);
}

return new QName(
map.getOrDefault("namespaceURI", ""),
map.getOrDefault("localPart", ""),
map.getOrDefault("prefix", "")
);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -34,9 +34,15 @@ public JsonSerializer<?> findSerializer(SerializationConfig config,
JavaType type, BeanDescription beanDesc)
{
Class<?> raw = type.getRawClass();
if (Duration.class.isAssignableFrom(raw) || QName.class.isAssignableFrom(raw)) {
if (Duration.class.isAssignableFrom(raw)){
return ToStringSerializer.instance;
}
if (QName.class.isAssignableFrom(raw)) {
if (config.isEnabled(SerializationFeature.WRITE_QNAMES_USING_TO_STRING)) {
return ToStringSerializer.instance;
}
return null;
}
if (XMLGregorianCalendar.class.isAssignableFrom(raw)) {
return XMLGregorianCalendarSerializer.instance;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,13 @@ public void testQNameSer() throws Exception
assertEquals(q(qn.toString()), MAPPER.writeValueAsString(qn));
}

@Test
public void testQNameSerWithToStringFeatureDisabled() throws Exception
{
QName qn = new QName("http://abc", "tag", "prefix");
assertEquals(a2q("{'namespaceURI':'http://abc','localPart':'tag','prefix':'prefix'}"), MAPPER.disable(SerializationFeature.WRITE_QNAMES_USING_TO_STRING).writeValueAsString(qn));
}

@Test
public void testDurationSer() throws Exception
{
Expand Down Expand Up @@ -121,6 +128,16 @@ public void testQNameDeser() throws Exception
assertEquals("", qn.getLocalPart());
}

@Test
public void testQNameDeserWithValueOfFeatureDisabled() throws Exception
{
String qstr = a2q("{'namespaceURI':'http://abc','localPart':'tag','prefix':'prefix'}");
QName qn = MAPPER.disable(DeserializationFeature.READ_QNAMES_USING_VALUE_OF).readValue(qstr, QName.class);
Copy link
Member

Choose a reason for hiding this comment

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

Construct a new mapper, changing of shared MAPPER should not be done (behavior not defined, concurrent unit test could fail etc)

assertEquals("http://abc", qn.getNamespaceURI());
assertEquals("tag", qn.getLocalPart());
assertEquals("prefix", qn.getPrefix());
}

@Test
public void testXMLGregorianCalendarDeser() throws Exception
{
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
package com.fasterxml.jackson.databind.ext;

import java.util.stream.Stream;
import javax.xml.namespace.QName;

import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.Arguments;
import org.junit.jupiter.params.provider.MethodSource;

import com.fasterxml.jackson.core.JsonProcessingException;

import com.fasterxml.jackson.databind.DeserializationFeature;
import com.fasterxml.jackson.databind.ObjectMapper;
import com.fasterxml.jackson.databind.SerializationFeature;

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

import static com.fasterxml.jackson.databind.testutil.DatabindTestUtil.newJsonMapper;

class QNameAsObjectReadWrite4771Test {

private final ObjectMapper MAPPER = newJsonMapper();

@ParameterizedTest
@MethodSource("provideAllPerumtationsOfQNameConstructor")
void testQNameWithObjectSerialization(QName originalQName) throws JsonProcessingException {
String json = MAPPER
.disable(SerializationFeature.WRITE_QNAMES_USING_TO_STRING)
.writeValueAsString(originalQName);

QName deserializedQName = MAPPER
.disable(DeserializationFeature.READ_QNAMES_USING_VALUE_OF)
.readValue(json, QName.class);

assertEquals(originalQName, deserializedQName);
}

static Stream<Arguments> provideAllPerumtationsOfQNameConstructor() {
return Stream.of(
Arguments.of(new QName("test-local-part")),
Arguments.of(new QName("test-namespace-uri", "test-local-part")),
Arguments.of(new QName("test-namespace-uri", "test-local-part", "test-prefix"))
);
}

}
Loading