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

[mcvayc]

This commit fixes #4771 by adding serialization and deserialization features to control whether a QName is serialized to a string using the QName.toString() method (the only option currently) or if it is serialized to JSON object (a new option to fix #4771).

[mcvayc]

Hi,

This is my first contribution to the jackson project. Please let me know if there is anything I can improve or change.

Thanks,
Chris

[mcvayc]

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

[cowtowncoder]

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

[cowtowncoder]

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)

[mcvayc]

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

[cowtowncoder]

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

[cowtowncoder]

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

[cowtowncoder]

Ok, first of all: thank you for submitting this pr @mcvayc (and thank you for already sending CLA).

All in all makes sense; added some implementations suggestions.

But the big question actually has to do with configurability. Some concerns (which you couldn't really have known since they are not that obvious):

1. I have tried to keep SerializationFeature and DeserializationFeature more for "big" / global / cross-cutting aspects (although for historical reasons this is not completely true). So settings that affect a single type would ideally not be added here
2. Conceptually SerializationFeature and DeserializationFeature should be changeable on per-call basis (on ObjectWriter / ObjectReader, respectively). These feature cannot be changed as they affect registered (de)serializers. MapperFeatures, OTOH, cannot be changed so in that sense feature would belong there.
3. DeserializationFeature might not be needed: in most "polymorphic" cases deserializers actually accept various valid types -- f.ex, for java.util.Calendar either timestamp (Number) or ISO-860 (String). Only serialization has a configuration setting.

So... I am trying to think of an alternative. And actually there is another mechanism, sort of designed for this. It's bit less convenient in some ways for global definitions but conceptually better: @JsonFormat.shape.

You can annotate properties

public class Bean {
  @JsonFormat(shape = JsonFormat.Shape.OBJECT)
  public QName name;
}

but also define global defaults:

        ObjectMapper m = JsonMapper.builder()
                .withConfigOverride(QName.class,
                        cfg -> cfg.setFormat(JsonFormat.Value.forShape(JsonFormat.Shape.OBJECT))

so that would be something I'd consider as the canonical intended mechanism.
Examples of serializers that use this mechanism include:

• BooleanSerializer (as boolean or String)
• UUIDSerializer (as String or (base64-encoded) Binary)

So... this is something I would suggest. Apologies for added complexity.

[mcvayc]

@cowtowncoder, thank you for your guidance. I'll make the changes to implement this via @JsonFormat.shape.

I created this PR based on what I saw for Enum serialization

jackson-databind/src/main/java/com/fasterxml/jackson/databind/SerializationFeature.java

Lines 283 to 293 in 17858e9

	/**
	* Feature that determines standard serialization mechanism used for
	* Enum values: if enabled, return value of <code>Enum.toString()</code>
	* is used; if disabled, return value of <code>Enum.name()</code> is used.
	*<p>
	* Note: this feature should usually have same value
	* as {@link DeserializationFeature#READ_ENUMS_USING_TO_STRING}.
	*<p>
	* Feature is disabled by default.
	*/
	WRITE_ENUMS_USING_TO_STRING(false),

but understand now why this doesn't fit. Thanks for clarifying.

[cowtowncoder]

@mcvayc Great! Nothing wrong with the initial take; some existing implementations do things in similar way. Looking forward to v2! :)