FirebaseUI-Android icon indicating copy to clipboard operation
FirebaseUI-Android copied to clipboard

DatabasePagingSource not working when firebase query contains orderByChild clause.

Open beyondeye opened this issue 2 years ago • 3 comments

Step 2: Describe your environment

  • Android device: Samsung S21+5G
  • Android OS version: 11
  • Google Play Services version: August 1 2021
  • Firebase/Play Services SDK version: com.google.firebase:firebase-bom:28.3.0
  • FirebaseUI version: 8.0.0

Step 3: Describe the problem:

DatabasePagingSource (and therefore also FirebaseRecyclerPagingAdapter) not working when firebase query contains orderByChild clause. I have verified that the problem is not in FirebaseUI itself but in method

private Query startAt(Node node, String key)

in Query class in package com.google.firebase.database; according code javadocs for the method:

   * Creates a query constrained to only return child nodes with a value greater than or equal to
   * the given value, using the given {@code orderBy} directive or priority as default, and
   * additionally only child nodes with a key greater than or equal to the given key.

but I have verified that if the query is using an orderByChild clause, the key parameter in startAt is ignored Because of this I am getting crashes cause by exception

    java.lang.IllegalStateException: The same value, VWGuwwgOg3tg_15vtqkJf32bhICi-zI_yt---, was passed as the nextKey in two
     sequential Pages loaded from a PagingSource. Re-using load keys in
     PagingSource is often an error, and must be explicitly enabled by
     overriding PagingSource.keyReuseSupported.

because, since the key parameter is ignored, we reread each time the same data, each time the method

 public Single<LoadResult<String, DataSnapshot>> loadSingle(@NonNull LoadParams<String> params)

is called

Steps to reproduce:

  1. see above

Proposed Fix

I have written code to fix the issue. note that I am working with a fork of the original FirebaseUI-Android, so I have the original code converted to Kotlin, but you can get the idea if you look at it. In brief what I do is keep not only the key of the last read document but also the value of the childkey, and use them together when I need to query for the next batch of documents

Code With Bug Fix:

class DatabasePagingSource(private val mQuery: Query) : PagingSource<Any, DataSnapshot>() {

    @SuppressLint("RestrictedApi")
    fun getChildValue(snapshot: DataSnapshot, index:PathIndex):Any? {
        val keypath=index.queryDefinition
        val data=snapshot.child(keypath)
        if(!data.exists()) return null
        return data.value
    }
    fun Query.startAt_childvalue(startvalue:Any?,keyvalue:String?):Query {
        return when(startvalue) {
            is String -> startAt(startvalue,keyvalue)
            is Boolean -> startAt(startvalue,keyvalue)
            is Double -> startAt(startvalue,keyvalue)
            is Long -> startAt(startvalue.toDouble(),keyvalue)
            else -> this
        }
    }
    /**
     * reason of SuppressLint: DatabaseError.fromStatus() is not meant to be public.
     */
    @SuppressLint("RestrictedApi")
    override suspend fun load(params: LoadParams<Any>): LoadResult<Any, DataSnapshot> {
        //change mQuery.startAt at value  if child index

        //if not null then what we have here is orderByChild query
        var querychildpathindex:PathIndex? = mQuery.spec.index as? PathIndex
        val pkey=params.key as Pair<Any?,String>?

        val task: Task<DataSnapshot> =

            if (params.key == null) {
                mQuery.limitToFirst(params.loadSize).get()
            } else {
                if (querychildpathindex != null)  //orderByChild query mode
                    mQuery.startAt_childvalue(pkey?.first, pkey?.second).limitToFirst(params.loadSize + 1).get()
                else
                    mQuery.startAt(null,pkey?.second).limitToFirst(params.loadSize + 1).get()
            }
        try {
            val dataSnapshot = task.await()
            if (dataSnapshot.exists()) {
                //Make List of DataSnapshot
                val data: MutableList<DataSnapshot> = ArrayList()
                var lastKey: Pair<Any?,String>? = null
                if (params.key == null) {
                    for (snapshot in dataSnapshot.children) {
                        data.add(snapshot)
                    }
                } else {
                    val iterator: Iterator<DataSnapshot> = dataSnapshot.children.iterator()

                    //Skip First Item that corresponds to lastKey read in previous batch
                    if (iterator.hasNext()) {
                        iterator.next()
                    }
                    while (iterator.hasNext()) {
                        val snapshot = iterator.next()
                        data.add(snapshot)
                    }
                }

                //Detect End of Data
                if (!data.isEmpty()) {
                    //Get Last Key
                    val lastkey_c = getLastPageChildKey(data,querychildpathindex)
                    val lastkey_k = getLastPageKey(data)
                    lastKey = if (lastkey_c == null && lastkey_k == null)
                        null
                    else
                        if (lastkey_k == null) Pair(lastkey_c, "") else Pair(lastkey_c, lastkey_k)
                }
                return toLoadResult(data, lastKey)
            } else {
                val details = DETAILS_DATABASE_NOT_FOUND + mQuery.toString()
                throw DatabaseError.fromStatus(
                    STATUS_DATABASE_NOT_FOUND,
                    MESSAGE_DATABASE_NOT_FOUND,
                    details
                ).toException()
            }
        } catch (e: ExecutionException) {
            return LoadResult.Error<Any, DataSnapshot>(e)
        }
    }


    private fun toLoadResult(
        snapshots: List<DataSnapshot>,
        nextPage: Pair<Any?,String>?
    ): LoadResult<Any, DataSnapshot> {
        return LoadResult.Page(
            snapshots,
            null,  // Only paging forward.
            nextPage,
            LoadResult.Page.COUNT_UNDEFINED,
            LoadResult.Page.COUNT_UNDEFINED
        )
    }
    private fun getLastPageChildKey(data: List<DataSnapshot>,index: PathIndex?): Any? {
        if(index==null) return null
        return if (data.isEmpty()) {
            null
        } else {
            getChildValue(data[data.size - 1],index)
        }
    }

    private fun getLastPageKey(data: List<DataSnapshot>): String? {
        return if (data.isEmpty()) {
            null
        } else {
            data[data.size - 1].key
        }
    }

    override fun getRefreshKey(state: PagingState<Any, DataSnapshot>): Pair<Any?, String>? {
        return null
    }

    companion object {
        private const val STATUS_DATABASE_NOT_FOUND = "DATA_NOT_FOUND"
        private const val MESSAGE_DATABASE_NOT_FOUND = "Data not found at given child path!"
        private const val DETAILS_DATABASE_NOT_FOUND = "No data was returned for the given query: "
    }
}

beyondeye avatar Sep 05 '21 07:09 beyondeye

Experienced same issue. This solution fixed it for me

abdulwahabhassan avatar Sep 21 '21 17:09 abdulwahabhassan

@beyondeye thank you so much for the well detailed explanation and the code solution. Maybe you could send a PR implementing your proposed solution? I understand you might be discouraged because your solution uses Kotlin and our codebase is in Java - if that's the case, I'll try to come up with a PR myself.

thatfiredev avatar Dec 31 '21 17:12 thatfiredev