pilgr / Paper

Paper is a fast NoSQL-like storage for Java/Kotlin objects on Android with automatic schema migration support.
Apache License 2.0
2.34k stars 234 forks source link

Serialization is broken for Java 8 Date Time classes with api desugaring #206

Closed filipebrandao closed 1 year ago

filipebrandao commented 1 year ago

Hi, I found this issue where using desugaring to be able to use the Time classes in older Android versions ( below API 26, I believe) doesn't go very well with Paper/Kryo.

The problem is: after desugaring the time api will be located at package j$.time instead of java.time . Then when Kryo registers the default serializers for those classes it fails to do so because there's a package name check .

As a result, devices where desugaring is applied will serialize/deserialize using the CompatibleFieldSerializer and such instead. Which gets messy and causes issues in classes like ZonedDateTime where the Zone info is lost after serialization and deserialization.

I reported this issue on the Kryo repository and, understandably, it's not going to be fixed there. But maybe since Paper is more Android focused, it could be a good thing to fix?

As a band-aid solution in my project, I copied the whole TimeSerializers class from Kryo and removed the isClassAvailable() checks while calling Paper.addSerializer() myself to register every Serializer. It's been working pretty well and that would be my recommended approach. I can proceed with a PR if there's interest.

pilgr commented 1 year ago

That's a great finding, thanks for taking time to debug issue all the way to the particular code lines.

To be honest, I don't think it worth to do the fix in Paper since:

  1. The Kryo team refuses to do that. Paper is essentially a tiny wrapper around Kryo.
  2. The issue affects a limited corner case on quite legacy devices.
  3. The workaround is available and quite easy to replicate for anyone who would face the same issue and come to this ticket via search results.

I'll close the issue for now.