search

FasterXML / jackson-datatype-joda

Extension module to properly support full datatype set of Joda datetime library
Apache License 2.0
140 stars 81 forks source link

Gracefully handle Deserialization of Joda values from JSON Objects #115

Closed asaph closed 2 years ago

asaph commented 4 years ago

Gracefully handle deserialization of Joda objects that were serialized without first registering the JodaModule. Under such a situation, objects containing Joda types would be serialized with Jackson's default serialization behavior which is to use all the object's public getters to inform serialization. This results in rather verbose output, but no errors or warnings logged. More critically, the resulting json cannot be deserialized even after registering the JodaModule. Neglecting to register the JodaModule is an easy trap to fall into because doing so was not required in Jackson 1.x, which was a monolith that supported Joda types out of the box. Jackson 2.x was broken out into modules. Anyone serializing objects with Joda types who upgraded to Jackson 2.x without realizing this might have inadvertently serialized and persisted objects which are undeserializable. This fix adds support for deserializing all Joda object types which have been serialized with Jackson's default all-public-getters serializer.

cowtowncoder commented 4 years ago

I appreciate you contributing this. My first thought is that I am not sure I would want to add support for such handling.

You may want to bring this up on

https://groups.google.com/forum/#!forum/jackson-dev

and we can see how others feel about it.

asaph commented 4 years ago

Thanks for your consideration. To add a little more context, an example of an Exception error message people see when failing to deserialize these objects is:

Can not deserialize instance of org.joda.time.DateTime out of START_OBJECT token

FWIW I believe people have been running into this gotcha for years and generally not getting down to the root of the problem. One example (posted in 2 places):

I can appreciate your reservations around adding this mitigation to the library because it can potentially mask a misuse of the library (i.e. failing to register the JodaModule with your ObjectMapper prior to serialization). IMO including this mitigation is a justifiable tradeoff as the the consequence is having undeserializable data. I'm definitely open to other opinions. If you could kindly post it on the mailing list you referenced above, we'll see what the consensus is and take it from there.

cowtowncoder commented 4 years ago

Sounds reasonable. Another possibility could be to create separate module, but I can see why that might get tricky since this would need to add support in addition to canonical representations. I will send a question on dev list and see how others feel about this.

Also: now that I think about this more, I should mention this:

https://github.com/FasterXML/jackson-databind/issues/2683

which would try to address similar question, on blocking serialization if no module registered. I think something similar could be done for Joda types -- regardless of whether support was also added for deserializing from JSON Objects. I will file issue as well.

asaph commented 4 years ago

https://github.com/FasterXML/jackson-databind/issues/2683 and https://github.com/FasterXML/jackson-databind/issues/2776 are welcome enhancements because they would help developers catch mis-serialization issues at the development stage in the future. But given that Jackson 2.x is already widely deployed without these mitigations, I still think it's important for deserializers to make a best effort to recover gracefully, as undeserializable data is a most unwelcome problem.

I like the idea of addressing the problem at both the serialization and the deserialization ends. It's also worth noting that serialization and deserialization don't always occur within the same system. Consider the case of System A and System B with different codebases, library versions, owners and runtime environments. System A serializes and persists a payload, and then some time later, System B tries to deserialize the payload. It would be beneficial for System B, if it could gracefully recover from System A's mis-serialization issues. Postel's Law comes to mind.

cowtowncoder commented 4 years ago

Yes, valid points; I esp. agree that source/destination systems can be very different, and many developers do not quite realize this (and suggest use of local timezone as default for various things).

cowtowncoder commented 2 years ago

At this point I don't think I will merge this in, closing.