Kotlin / kotlinx-datetime

KotlinX multiplatform date/time library
Apache License 2.0
2.38k stars 97 forks source link

Implement java.io.Serializable on the JVM #143

Open Matej-Hlatky opened 3 years ago

Matej-Hlatky commented 3 years ago

Hello, Please make the Android actual types implement also java.io.Serializable (not Kotlin @Serializable as described in #37). This will allow us to make the complex types, which depends on kotlinx.datetime.* types either Serializable or Parcelable in Android app.

For instance, java.time.Instant IS Serializable, however the actual Android class kotlinx.datetime.Instant is NOT.

dkhalanskyjb commented 3 years ago

Hi! Could you please share the reasons why you can't use kotlinx.serialization throughout your project? There are several libraries that allow kotlinx.serialization to interact with Parcelable: https://chrynan.codes/android-parcelable-theres-a-better-way/, https://github.com/AhmedMourad0/bundlizer are some that I quickly found; maybe they are sufficient?

Matej-Hlatky commented 3 years ago

Hi @dkhalanskyjb, thanks for quick reply.

The proposed solution with KotlinX date types supporting java.io.Serializable seems to me like quick solution and it's "for free" in sense of zero additional coding requirements and dependencies.

We will then use @Parcelize on complex models which are having properties of Instant type.

However I am not sure if @Parcelize can also generate code for types annotated with kotlinx.serialization.Serializable - I will check that.

dkhalanskyjb commented 3 years ago

it's "for free" in sense of zero additional coding requirements

Not at all. We don't currently give any guarantees about the internal representation of our classes; just annotating Java with Serializable generates a serializer that is entirely dependent on the internal representation, so we would have to write our own methods if we wanted backward-compatible serialization. We would probably need to write some methods anyway to check that the internal invariants hold. Some classes that are currently in the common code, like DateTimePeriod, would have to be specialized for the JVM… All of this would also have to be tested and, of course, discussed. To top it all off, we want to discourage people from using Java's serialization, so extending all this effort to support java.io.Serializable could even be harmful.

So, we would need to have a good, solid reason to implement java.io.Serializable on our classes.

smallufo commented 3 years ago

Not related to android. I want to use kotlinx-datetime in wicket. But all wicket models mandate Serializable interface, otherwise NotSerializableException will be thrown.

dkhalanskyjb commented 2 years ago

What do you think about writing Serializable Adapters for our classes in place? We provide compatible toString and parse for everything.

An overly cautious example that can be slightly simplified if you know what you're doing:

public data class LocalDateModel(@Transient var value: LocalDate): java.io.Serializable {
    private fun writeObject(oStream: ObjectOutputStream) {
        oStream.defaultWriteObject()
        oStream.writeObject(value.toString())
    }
    private fun readObject(iStream: ObjectInputStream) {
        iStream.defaultReadObject()
        value = LocalDate.parse(iStream.readObject() as String)
    }
    private fun readObjectNoData() {
        throw InvalidObjectException("Stream data required")
    }
}

(be careful to check out serialVersionUid if you plan to change this class)

Matej-Hlatky commented 2 years ago

Hi @dkhalanskyjb, that approach, or maybe implementing java.io.Externalizable on LocalDateModel will workaround the issue I think. thanks.

4brunu commented 2 years ago

I just hit this issue. I was trying to use a property that is Instant in a data class that is parcelable, but the compiler shows the following error.

Type is not directly supported by 'Parcelize'. Annotate the parameter type with '@RawValue' if you want it to be serialized using 'writeValue()'
dkhalanskyjb commented 2 years ago

@4brunu, does the adapter pattern, shown above, not solve the issue?

4brunu commented 2 years ago

It was not strait forward, I needed to create type alias to a lot of abstractions, but in the end, it worked. Maybe this could be somehow made easier by make some changes in this library?

In case it helps anyone, I leave here the url to a sample project that I created to show how to use kotlinx-datetime with parcelable.

https://github.com/4brunu/kotlinx-datetime-parcelize-issue

4brunu commented 2 years ago

Maybe including the adapters on the android target was already a step to make the integration with Parcelize/Serialization easier. Or maybe some other idea to make the integration with Android easier, because this is a barrier to the adoption of this library. @dkhalanskyjb what are your thoughts on this?

hovi commented 1 year ago

I just ran into the same problem on android. In my case it is about kotlin.time.Duration, which is inline class represented by Long. I get the potencial technical baggage implementing Serializable overall, but perhaps on this simple class it could be easier.

dkhalanskyjb commented 1 year ago

Duration is not even part of this library.

hovi commented 1 year ago

Ah :facepalm:

mtrakal commented 8 months ago

forces to same issue when trying to use Android Navigation component with arguments - that require parcelable or serializable.

android.os.BadParcelableException: Parcelable encountered IOException writing serializable object (name = MyClass)
    at android.os.Parcel.writeSerializable(Parcel.java:2797)
    at android.os.Parcel.writeValue(Parcel.java:2563)
    at android.os.Parcel.writeValue(Parcel.java:2362)
    at android.os.Parcel.writeArrayMapInternal(Parcel.java:1298)
    at android.os.BaseBundle.writeToParcelInner(BaseBundle.java:1843)
    at android.os.Bundle.writeToParcel(Bundle.java:1389)
    at android.os.Parcel.writeBundle(Parcel.java:1367)
    at android.os.Parcel.writeValue(Parcel.java:2479)
    at android.os.Parcel.writeValue(Parcel.java:2369)
    at android.os.Parcel.writeArrayMapInternal(Parcel.java:1298)
    at android.os.BaseBundle.writeToParcelInner(BaseBundle.java:1843)
    at android.os.Bundle.writeToParcel(Bundle.java:1389)
    at android.os.Parcel.writeBundle(Parcel.java:1367)
    at android.content.Intent.writeToParcel(Intent.java:11798)
    at android.os.Parcel.writeTypedObject(Parcel.java:2203)
    at android.app.IActivityTaskManager$Stub$Proxy.startActivity(IActivityTaskManager.java:2101)
    at android.app.Instrumentation.execStartActivity(Instrumentation.java:1873)
    at android.app.Activity.startActivityForResult(Activity.java:5589)
    at androidx.activity.ComponentActivity.startActivityForResult(ComponentActivity.java:780)
    at android.app.Activity.startActivityForResult(Activity.java:5547)
    at androidx.activity.ComponentActivity.startActivityForResult(ComponentActivity.java:761)
    at android.app.Activity.startActivity(Activity.java:6045)
    at android.app.Activity.startActivity(Activity.java:6012)

Caused by: java.io.NotSerializableException: kotlinx.datetime.Instant
    at java.io.ObjectOutputStream.writeObject0(ObjectOutputStream.java:1240)
    at java.io.ObjectOutputStream.defaultWriteFields(ObjectOutputStream.java:1620)
    at java.io.ObjectOutputStream.writeSerialData(ObjectOutputStream.java:1581)
    at java.io.ObjectOutputStream.writeOrdinaryObject(ObjectOutputStream.java:1490)
    at java.io.ObjectOutputStream.writeObject0(ObjectOutputStream.java:1234)
    at java.io.ObjectOutputStream.defaultWriteFields(ObjectOutputStream.java:1620)
    at java.io.ObjectOutputStream.writeSerialData(ObjectOutputStream.java:1581)
    at java.io.ObjectOutputStream.writeOrdinaryObject(ObjectOutputStream.java:1490)
    at java.io.ObjectOutputStream.writeObject0(ObjectOutputStream.java:1234)
    at java.io.ObjectOutputStream.writeObject(ObjectOutputStream.java:354)
    at android.os.Parcel.writeSerializable(Parcel.java:2792)
dkhalanskyjb commented 8 months ago

@mtrakal, did you try this https://github.com/Kotlin/kotlinx-datetime/issues/143#issuecomment-975380754 ?

mtrakal commented 8 months ago

@dkhalanskyjb not so easy to wrap every instance of Instant into the new data class. Some classes are generated from openapiGenerator, where we are not able to encapsulate it at all.

I understand that you don't want to be dependant on java classes, on second side, it's still there java.time in kotlinx.datetime... Or implemnt kotlinx.parcelize.Parcelize could be helpfull for this kotlinx-datetime library to use it in android bundles / navigation components.


edit: Looks, that this is helpful: https://developer.android.com/kotlin/parcelize#custom_parcelers it still need some manual action (annotate class or variable), but don't need encapsulate whole data type into new class.

import android.os.Parcel
import kotlinx.parcelize.Parceler
import kotlin.time.Duration

/**
 * Usage:
 * For whole class:
 * @Parcelize
 * @TypeParceler<Duration, DurationClassParceler>()
 * class MyClass(val instant: Duration) : Parcelable
 *
 * For Property-local parceler
 * @Parcelize
 * class MyClass(@TypeParceler<Duration, DurationClassParceler>() val external: Duration) : Parcelable
 *
 * For Type-local parceler
 * @Parcelize
 * class MyClass(val external: @WriteWith<DurationClassParceler>() Duration) : Parcelable
 */
object DurationClassParceler : Parceler<Duration> {
    override fun create(parcel: Parcel) = Duration.parse(parcel.readString()!!)

    override fun Duration.write(parcel: Parcel, flags: Int) {
        parcel.writeString(this.toString())
    }
}

and

import android.os.Parcel
import kotlinx.datetime.Instant
import kotlinx.parcelize.Parceler

/**
 * Usage:
 * For whole class:
 * @Parcelize
 * @TypeParceler<Instant?, InstantClassParceler>()
 * class MyClass(val instant: Instant?) : Parcelable
 *
 * For Property-local parceler
 * @Parcelize
 * class MyClass(@TypeParceler<Instant?, InstantClassParceler>() val external: Instant?) : Parcelable
 *
 * For Type-local parceler
 * @Parcelize
 * class MyClass(val external: @WriteWith<InstantClassParceler>() Instant?) : Parcelable
 */
object InstantClassParceler : Parceler<Instant?> {
    override fun create(parcel: Parcel) = Instant.parse(parcel.readString()!!)

    override fun Instant?.write(parcel: Parcel, flags: Int) {
        parcel.writeString(this.toString())
    }
}
dkhalanskyjb commented 8 months ago

Maybe something like this https://github.com/chRyNaN/serialization-parcelable or https://github.com/AhmedMourad0/bundlizer would work, then: I don't understand the reason to write separate Parceler instances for Instant, LocalDate, etc, when all of them implement kotlinx.serialization.Serializable.

barry-irvine commented 7 months ago

@mtrakal I think the nulls on your Instant aren't handled correctly. Perhaps this?

object InstantClassParceler : Parceler<Instant?> {
    override fun create(parcel: Parcel) = parcel.readString()?.let { Instant.parse(it) }

    override fun Instant?.write(parcel: Parcel, flags: Int) {
        parcel.writeString(this?.toString())
    }
}
hfhbd commented 6 months ago

With some expect/actual code its also possible to only add Parcelable support instead adding the broken by design java.io.Serializable support. I did this here too: https://github.com/hfhbd/kotlinx-uuid/pull/297

crysxd commented 3 months ago

This is a real pain and should be easy to solve. We have to wrap Instant in every project.