GitLiveApp / firebase-kotlin-sdk

A Kotlin-first SDK for Firebase
https://gitliveapp.github.io/firebase-kotlin-sdk/
Apache License 2.0
979 stars 148 forks source link

JsonObject deserialization is not working. #500

Open Gery711KO opened 1 month ago

Gery711KO commented 1 month ago

The object im trying to deserialize is something like this:

@Serializable
data class SomeDto(
    // ...some primitive fields
    val jsonObjectField: JsonObject = JsonObject(emptyMap()),
)

I managed the find where the exception is thrown, but it hides the actual exception for some reason. It would be cool, if you could do a quick fix just for showing the actual exception itself. image kotlinx.serialization.SerializationException: Exception during decoding com.package.app.SomeDto jsonObjectField

nbransby commented 1 month ago

It passes the cause so you can just look at the caused by in the stacktrace

krobert commented 1 month ago

@nbransby the problem is that jsonObject has a built in serializer,but its not compatible, because this sdk is customizing descriptor, encoder, decoder classes, so its failing.

This serializer can be used only with Json format.Expected Decoder to be JsonDecoder, got class dev.gitlive.firebase.FirebaseDecoder

FirebaseMapSerializer doesnt seem to be working properly.

nbransby commented 1 month ago

You need to use regular POKOs with @Serializable annotation

krobert commented 1 month ago

@nbransby belive us, we tried everything, it has the annotation, but its not working for dynamic content:

@Serializable
data class Dto(
    val id: String = "",
    val params: Map<String, @Contextual @Serializable(with = AnySerializer::class) Any?>? = null,
) 

right now it looks like this, but i tried this also:

@Serializable
data class Dto(
    val id: String = "",
     @Contextual
    @Serializable(with = FirebaseMapSerializer::class)
    val params: Map<String, @Contextual Any?>? = emptyMap(),
) 

also a lot of combinations of annotations, using jsonobject instead of map.. etc...

the sdk has this custom implementation of kotlin serialization, there is no way to get the data. In android firebase sdk i can get the data as a map<string, any> and it works pretty well, but this kmm solution just breaks this.

try it out, make a map field in firestore, add string, number, list and any other objects to it, try to read it with the sdk. if it works let me know how you did it.

for an easy fix:

can we have at least a method for DocumentSnapshot, which doesnt require parsing, just returns the raw value as map<string,any?> like the original sdk?

image
Daeda88 commented 1 month ago

Have you tried setting a custom SerializerModule? If you're doing Contextual serialization, you should set a serialization module.

Like, even on regular serialization this would fail:

class TestContextualSerialization() {

    @Serializable
    data class TestData(
        val map: Map<String, @Contextual Any>
    )

    @Test
    fun testSerialization() {
        val value = TestData(mapOf("1" to 1, "2" to "two", "3" to false))
        println(Json.encodeToString(value))
    }
}

When using @Contextual you still need to provide a Context. See https://github.com/Kotlin/kotlinx.serialization/blob/master/docs/serializers.md#contextual-serialization

On this SDK, you can access the SerializationModule in your encode or decode Settings:

DocumentSnapshot.getData {
  serializersModule = SerializersModule {
      contextual(Any::class, // Set your serializer here)
  }
}

Still, while this may work for Javascript, I dont see how you would be able to deserialize for Android/iOS here. Which is precisely why this isnt supported

Daeda88 commented 1 month ago

Digging a little bit deeper into the existing SDKs, For Android we'd have to wrap CustomClassMapper somehow, and for iOS FSTUserDataWriter to get the same behaviour. It's probably not impossible, but certainly difficult to get a proper multiplatform behaviour.

What would be the correct solution IMO would be:

@Serializable
data class Dto(
    val id: String = "",
    val params: Params? = null,
) {
   @Serializable
   data class Params(
       "argumentOne": Int? = null,
       "argumentTwo": String? = null
   )
}

Since Im gonna assume you know beforehand which params you are gonna support. Strictly typed data is for sure the recommended way to go.

krobert commented 1 month ago

@Daeda88 sorry for late answer, well yeah i know some params and ended up doing the same solution. Can i ask why didnt you use the original fun map<string,Object?> getData() from the sdk? Both ios and android has this and working pretty well. It would be nice to have that same function in the sdk and then we can use dynamic data without serialization of the sdk, which is responsable for this issue.

Daeda88 commented 1 month ago

Not the original author, nor member of the actual team, so I can't go into considerations on that. I imagine it has to do with the fact that:

a) Dynamic typing makes little sense in the first place. Either the client or the server needs to know the data type at some point, otherwise why even store it b) Because this is a multiplatform approach, some customization needs to take place. E.g. a Timestamp is not passed as the platform specific timestamp but as a wrapper class so that its behaviour can be modeled consistently across platforms. So just passing dynamic typing in itself is not good enough.

nbransby commented 1 month ago

Yes B is the reason we can't simply expose the original fun map<string,Object?> getData() from the sdks otherwise you would get different types back on different platforms