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

Implement java.io.Serializable on the JVM #143

Open
Matej-Hlatky opened this issue Sep 17, 2021 · 20 comments · May be fixed by #373
Open

Implement java.io.Serializable on the JVM #143

Matej-Hlatky opened this issue Sep 17, 2021 · 20 comments · May be fixed by #373
Assignees
Milestone

Comments

@Matej-Hlatky
Copy link

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
Copy link
Collaborator

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
Copy link
Author

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
Copy link
Collaborator

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
Copy link

smallufo commented Sep 20, 2021

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 dkhalanskyjb changed the title Make Android types implement java.io.Serializable Implement java.io.Serializable on the JVM Nov 22, 2021
@dkhalanskyjb
Copy link
Collaborator

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
Copy link
Author

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

@4brunu
Copy link

4brunu commented Jan 2, 2022

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
Copy link
Collaborator

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

@4brunu
Copy link

4brunu commented Jan 12, 2022

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
Copy link

4brunu commented Jan 12, 2022

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
Copy link

hovi commented Sep 28, 2022

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
Copy link
Collaborator

Duration is not even part of this library.

@hovi
Copy link

hovi commented Sep 28, 2022

Ah 🤦

@mtrakal
Copy link

mtrakal commented Jan 16, 2024

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
Copy link
Collaborator

@mtrakal, did you try this #143 (comment) ?

@mtrakal
Copy link

mtrakal commented Jan 17, 2024

@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
Copy link
Collaborator

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
Copy link

barry-irvine commented Feb 23, 2024

@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())
    }
}

dkhalanskyjb added a commit that referenced this issue Mar 22, 2024
Implement java.io.Serializable for
* Instant
* LocalDate
* LocalTime
* LocalDateTime
* UtcOffset

TimeZone is not `Serializable` because its behavior is
system-dependent. We can make it `java.io.Serializable` later
if there is demand.

We are using string representations instead of relying on Java's
entities being `java.io.Serializable` so that we have more freedom
to change our implementation later.

Fixes #143
dkhalanskyjb added a commit that referenced this issue Mar 22, 2024
Implement java.io.Serializable for
* Instant
* LocalDate
* LocalTime
* LocalDateTime
* UtcOffset

TimeZone is not `Serializable` because its behavior is
system-dependent. We can make it `java.io.Serializable` later
if there is demand.

We are using string representations instead of relying on Java's
entities being `java.io.Serializable` so that we have more freedom
to change our implementation later.

Fixes #143
@hfhbd
Copy link
Contributor

hfhbd commented Mar 25, 2024

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: hfhbd/kotlinx-uuid#297

@dkhalanskyjb dkhalanskyjb modified the milestones: 0.9.0, 0.7.0 Apr 12, 2024
@dkhalanskyjb dkhalanskyjb self-assigned this Apr 12, 2024
dkhalanskyjb added a commit that referenced this issue May 13, 2024
Implement java.io.Serializable for
* Instant
* LocalDate
* LocalTime
* LocalDateTime
* UtcOffset

TimeZone is not `Serializable` because its behavior is
system-dependent. We can make it `java.io.Serializable` later
if there is demand.

We are using string representations instead of relying on Java's
entities being `java.io.Serializable` so that we have more freedom
to change our implementation later.

Fixes #143
dkhalanskyjb added a commit that referenced this issue May 23, 2024
Implement java.io.Serializable for
* Instant
* LocalDate
* LocalTime
* LocalDateTime
* UtcOffset

TimeZone is not `Serializable` because its behavior is
system-dependent. We can make it `java.io.Serializable` later
if there is demand.

We are using string representations instead of relying on Java's
entities being `java.io.Serializable` so that we have more freedom
to change our implementation later.

Fixes #143
@crysxd
Copy link

crysxd commented May 29, 2024

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

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

Successfully merging a pull request may close this issue.

10 participants