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

Unable to deserialize root-level Instant value from XML #380

Closed
afayes opened this issue Dec 20, 2019 · 12 comments
Closed

Unable to deserialize root-level Instant value from XML #380

afayes opened this issue Dec 20, 2019 · 12 comments
Milestone

Comments

@afayes
Copy link

afayes commented Dec 20, 2019

Unable to deserialise XML to Instant value

Jackson version: 2.10.1
Java version: Java 8

Code to reproduce:

XmlMapper mapper = new XmlMapper();
mapper.registerModule(new JavaTimeModule());
String instant = mapper.writeValueAsString(Instant.now());
Instant instantOutput = mapper.readValue(instant, Instant.class);

Stack trace:

Exception in thread "main" com.fasterxml.jackson.databind.exc.MismatchedInputException: Unexpected token (START_OBJECT), expected one of [VALUE_STRING, VALUE_NUMBER_INT, VALUE_NUMBER_FLOAT] for java.time.Instant value
 at [Source: (StringReader); line: 1, column: 1]
	at com.fasterxml.jackson.databind.exc.MismatchedInputException.from(MismatchedInputException.java:59)
	at com.fasterxml.jackson.databind.DeserializationContext.reportInputMismatch(DeserializationContext.java:1442)
	at com.fasterxml.jackson.databind.DeserializationContext.handleUnexpectedToken(DeserializationContext.java:1216)
	at com.fasterxml.jackson.databind.DeserializationContext.handleUnexpectedToken(DeserializationContext.java:1148)
	at com.fasterxml.jackson.datatype.jsr310.deser.JSR310DeserializerBase._handleUnexpectedToken(JSR310DeserializerBase.java:105)
	at com.fasterxml.jackson.datatype.jsr310.deser.JSR310DeserializerBase._handleUnexpectedToken(JSR310DeserializerBase.java:118)
	at com.fasterxml.jackson.datatype.jsr310.deser.InstantDeserializer.deserialize(InstantDeserializer.java:231)
	at com.fasterxml.jackson.datatype.jsr310.deser.InstantDeserializer.deserialize(InstantDeserializer.java:50)
	at com.fasterxml.jackson.databind.ObjectMapper._readMapAndClose(ObjectMapper.java:4202)
	at com.fasterxml.jackson.databind.ObjectMapper.readValue(ObjectMapper.java:3205)
	at com.fasterxml.jackson.databind.ObjectMapper.readValue(ObjectMapper.java:3173)
	at uk.co.argos.availability.server.Temp.main(Temp.java:19)
@cowtowncoder
Copy link
Member

Hmmh. I'll have to figure out how to test this; issue belongs somewhere else, probably XML module (I can transfer it).

But one thing to note is this: since Instants are typically serialized either as Strings or longs (timestamp), it is not recommended to attempt to serialize them directly as root values: instead, there should be a wrapper POJO. This is same advise I would give if someone was to serialize basic String or number as well.

But I am interested in seeing what exactly happens here.

@cowtowncoder cowtowncoder transferred this issue from FasterXML/jackson-databind Dec 20, 2019
@afayes
Copy link
Author

afayes commented Dec 25, 2019

But one thing to note is this: since Instants are typically serialized either as Strings or longs (timestamp), it is not recommended to attempt to serialize them directly as root values

For the XmlMapper, the value is serialised as XML i.e. <Instance>value</Instance>

You would expect that whatever serialised value is returned by the mapper, it would be able to deserialise it back to the object value. I.e the following should hold:

Instant instant = Instant.now();
String serialised = mapper.writeValueAsString(instant);
Instant deserialised = mapper.readValue(serialised, Instant.class);

assertEquals(instant, deserialised);

@cowtowncoder
Copy link
Member

Yes, as a general rule, what Jackson writes, it should be able to read back.
I hope to have a look at what is happening here; root values pose special challenges unfortunately since there is no property name to match, which ties things back. But it may come down to details of deserializer, i.e. there are some methods of determining content that work better if String value is expected (XML backend has to use bit more complex heuristics than, say, JSON).

@afayes
Copy link
Author

afayes commented Dec 25, 2019

The default ObjectMapper which is based on JSON is able to serialise/deserialise values such as Instant without any attribute names I.E the value is the whole JSON and its not an object with fields. You would expect different implementations such as the XmlMapper to exhibit same behaviour. This is the Liskob substitution principle from SOLID.

@kupci
Copy link
Member

kupci commented Dec 26, 2019

This is the Liskob substitution principle from SOLID.

Right, I think we are all familiar with SOLID. Btw, typo - it's the "Liskov" substitution principle, not "Liskob".

Anyway, if you can provide a full test case, that would be helpful.

@afayes
Copy link
Author

afayes commented Dec 26, 2019

I have provided the code to reproduce in the initial description along with the API version, Java version and stack trace

@cowtowncoder
Copy link
Member

cowtowncoder commented Dec 26, 2019

@afayes Question is not that of whether it would be good for mappers to be able to work similarly, but that of if it is possible to. XML and JSON format are structurally different.

@cowtowncoder
Copy link
Member

After looking a bit into this, I have come to conclusion that it is not and will not be feasible to support the specific use case of scalar (text, number) as root value. So although serialization of such value is easy -- for simple String, it would result in:

<String>string value</String>

deserialization is difficult as elements are represent as JSON Object equivalent token streams, roughly identical to

{ "String" : "string value" }

which looks like an Object value: this is different from how POJO properties look like and would need specialized heuristics to support.

Keeping also in mind that according to original JSON specification, root value MUST be either Object or Array, and as such some json libraries do not support handling of root-level String or Number values.
Similarly XML databinding libraries may or may not support non-POJO root values: my understanding is that JAXB does not support them either.

So. To handle Instant values with Jackson, they must be property values, not root values.
I will update README to indicate this limtation.

@cowtowncoder cowtowncoder added the will-not-fix Issue for which there is no plan to fix as described or requested label Dec 27, 2019
@cowtowncoder cowtowncoder changed the title Unable to deserialise XML to Instant value Unable to deserialize root-level Instant value from XML Dec 27, 2019
@afayes
Copy link
Author

afayes commented Dec 27, 2019

@cowtowncoder Thanks for your time on this.

I get your point about root level scalars. In the past whilst designing REST API's I looked into the validity of returning scalars as response for JSON. As you noted, it's not supported by some libraries although more recent standards do allow it and the Jackson library conforms to that for JSON.

I have not used XML for REST API's until the current client project I am working on as part of a team of devs. It's an existing code base and a recently developed controller returned an Instant as root and Spring REST client was throwing an exception when deserialising the response back to Java. After some further investigation I found that the XmlMapper is able to serialise but not deserialise. I tested with default ObjectMapper that uses JSON and it worked fine.

I understand that the current implementation makes it difficult to support the deserialisation part for root Instant values for XML. Given that, root scalars are not really valid XML, you may wish to never support it.

One possible suggestion is to throw an exception when trying to serialise root scalars with the XmlMapper. That way there is consistency between serialisation and deserialisation and it won't cause confusion for users of the API.

Who knows, you may decided to one day support both serialisation and deserialation :)

@cowtowncoder
Copy link
Member

@afayes No problem. It is unfortunately this is currently difficult to support -- it is obviously not something impossible to support, just... rather difficult with the way things are.
You are correct in that serialization works, deserialization less so: problem being that deserializer would need to figure out bit of magic to get from key/value "Object" into String value. That could perhaps be tackled by another low-level JsonParser accessor like "isExpectedStringValue" (there is "isExpectedStartArray()" that serves similar purpose).

I'll think about exception part: it is challenging in its own way since core Jackson databind actually does not really know possible "shapes" that are acceptable, currently, so there is no reliable way to know if type X might be possible to support or not. With 3.0 there may be more support for deserializers to indicate shapes (Object, Array, various scalars) they support so perhaps this could be checked to some degree.

Anyway, thank you for your suggestions!

@cowtowncoder cowtowncoder added 2.12 and removed will-not-fix Issue for which there is no plan to fix as described or requested labels Jun 30, 2020
@cowtowncoder
Copy link
Member

I changed my mind. Since I have been able to, I think, to support this for basic JDK scalar values, I think same might be possible for date/time types as well.
Not 100% certain yet but hope to test the idea soon.

@afayes
Copy link
Author

afayes commented Jun 30, 2020

Awesome!

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

No branches or pull requests

3 participants