GitLiveApp / firebase-kotlin-sdk

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

SerializationModule is ignored in update(vararg fieldsAndValues: Pair<FieldPath, Any?>, buildSettings: EncodeSettings.Builder.() -> Unit = {}) #632

Open dogeweb opened 1 week ago

dogeweb commented 1 week ago

the encode function in common-internal -> encoders.kt doesn't use the provided serializermodule in encodesettings

@PublishedApi
internal inline fun <reified T> encode(value: T, encodeSettings: EncodeSettings): Any? = value?.let {
    FirebaseEncoder(encodeSettings).apply {
        if (it is ValueWithSerializer<*> && it.value is T) {
            @Suppress("UNCHECKED_CAST")
            (it as ValueWithSerializer<T>).let {
                encodeSerializableValue(it.serializer, it.value)
            }
        } else {
            encodeSerializableValue(it.firebaseSerializer(), it)
        }
    }.value
}

A quick fix could be the following, but when called by update T is erased to Any?, so the serializer doesn't find the right serializer.

@PublishedApi
internal inline fun <reified T> encode(value: T, encodeSettings: EncodeSettings): Any? = value?.let {
    FirebaseEncoder(encodeSettings).apply {
        if (it is ValueWithSerializer<*> && it.value is T) {
            @Suppress("UNCHECKED_CAST")
            (it as ValueWithSerializer<T>).let {
                encodeSerializableValue(it.serializer, it.value)
            }
        } else {
            val serializer = serializersModule.serializerOrNull(typeOf<T>()) ?: it.firebaseSerializer()
            encodeSerializableValue(serializer, it)
        }
    }.value
}

A proposal would be something like this, where encode is called each time with the type correctly erased

public class UpdateContext(public val buildSettings: EncodeSettings.Builder.() -> Unit) {

    public val list: MutableList<Pair<String, Any?>> = mutableListOf()

    public inline fun <reified T> String.updateWith(value: T) {
        list.add(this to encode(value, buildSettings))
    }
}

public suspend inline fun update(
    noinline buildSettings: EncodeSettings.Builder.() -> Unit = {}, 
    update: UpdateContext.() -> Unit) {
        updateEncodedFieldsAndValues(UpdateContext(buildSettings).apply(update).list)
}

I tested the encode fix, but I wasn't able to test the update fix as whenever I add functions or classes to the firestore module then the library published to maven local has some classes not accessible.