GitLiveApp / firebase-kotlin-sdk

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

Suspend function is stuck on Firestore writes while device offline #518

Closed lenoch0d closed 4 months ago

lenoch0d commented 5 months ago

Hey there! First of all, thanks for this amazing project; it helps us a lot in sharing our codebase among platforms.

However, as we started integrating it more, we encountered the following issue. When the device is offline, any calls for writes/updates/add to Firestore get suspended until the device is back online, as the underlying OnCompleteListener callback will never get fired. The same way works the official suspend fun <T> Task<T>.await() on Android. However, on the native SDK, you can just call the set/update/add method, which returns immediately and doesn't wait for the sync with the server. That's convenient while offline, as the data gets stored in the local Firestore database, where we can access them with a snapshot listener, and don't block the suspend calls until the connection is back online.

I came up with this in the multiplatform code. A race between suspend and a snapshots flow. If online, the suspend will fire first, if offline, the snapshot listener will fire. I added a parameter into the method, whether the caller wants to wait for server sync. Seems to be working well.

    private suspend fun CollectionReference.add(
        data: Any,
        shouldMerge: Boolean,
        shouldWaitForSync: Boolean,
    ) = document.write(data, shouldMerge, shouldWaitForSync)

    private suspend fun CollectionReference.add(
        docId: String,
        data: Any,
        shouldMerge: Boolean,
        shouldWaitForSync: Boolean,
    ) = document(docId).write(data, shouldMerge, shouldWaitForSync)

    private suspend fun DocumentReference.update(
        data: Any,
        shouldMerge: Boolean,
        shouldWaitForSync: Boolean,
    ) = if (shouldWaitForSync) {
        set(data, merge = shouldMerge)
    } else {
        race(
            { set(data, merge = shouldMerge) },
            { snapshots().first() }
        )
    }

    private suspend fun <R> race(vararg races: suspend () -> R) {
        channelFlow {
            for (race in races) {
                launch { send(race()) }
            }
        }.first()
    }

Do you think it is something worth adding as a feature in the library? There could just be the shouldWaitForSync parameter so the caller can decide, whether they are fine with locally stored data or they want to be sure the data is on the server.

nbransby commented 5 months ago

As described here if you don't want to wait to know when the operation completes just launch a coroutine.

As for your suggestion is better you decouple the writing of data from the reading of it. So where you need to read the data you should be collecting the snapshots flow constantly (not using .first()) and then just write the data separately - the snapshots flow will fire the new value instantly on write (before its written to the local or remote database)