Skip to content

Feature FirestoreItemKeyedDatasource #1599

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

Closed
wants to merge 9 commits into from
1 change: 1 addition & 0 deletions build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -140,6 +140,7 @@ fun Project.setupPublishing() {
val javadoc = tasks.register<Javadoc>("javadoc") {
setSource(project.the<BaseExtension>().sourceSets["main"].java.srcDirs)
classpath += files(project.the<BaseExtension>().bootClasspath)
exclude("**/*.kt")

project.the<LibraryExtension>().libraryVariants.configureEach {
dependsOn(assemble)
Expand Down
71 changes: 71 additions & 0 deletions firestore/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ Before using this library, you should be familiar with the following topics:
1. [Using the FirestorePagingAdapter](#using-the-firestorepagingadapter)
1. [Adapter lifecyle](#firestorepagingadapter-lifecycle)
1. [Events](#paging-events)
1. [Using the FirestoreItemKeyedDataSource](#using-the-firestoreitemkeyeddatasource)

## Data model

Expand Down Expand Up @@ -392,6 +393,76 @@ FirestorePagingAdapter<Item, ItemViewHolder> adapter =
};
```

### Using the `FirestoreItemKeyedDataSource`

The `FirestoreItemKeyedDataSource` loads documents in pages. It also automatically applied updates
to your UI in real time when documents are added, removed, or change.

The `FirestoreItemKeyedDataSource` is built on top of the [Android Paging Support Library][paging-support].
Before using the adapter in your application, you must add a dependency on the support library:

```groovy
implementation 'android.arch.paging:runtime:1.x.x'
```

The detailed explanation of how to use `DataSource` you can find in the [Android Paging Support Library][paging-support].

Here is a short example of how to use `FirestoreItemKeyedDataSource`.

The following code snippet shows how to create a new instance of `LiveData<PagedList>` in your app's
`ViewModel` class :

```java

public class MyViewModel extends ViewModel {

CollectionReference collection = FirebaseFirestore.getInstance().collection("cities");

FirestoreItemKeyedDataSource.Factory factory = FirestoreItemKeyedDataSource.Factory(
QueryFactory.Builder(collection)
.orderBy("name")
.build(),
errorListener
);

LiveData<PagedList<DocumentSnapshot>> items = LivePagedListBuilder(factory, /* page size*/ 10).build();

FirestoreItemKeyedDataSource.ErrorCallback errorListener = new FirestoreItemKeyedDataSource.ErrorCallback() {
@Override
public void onError(@NotNull FirebaseFirestoreException exception) {
// handle error

items.getValue().getDataSource().invalidate();
}
};

}

```

You can connect an instance of `LiveData<PagedList>` to a `PagedListAdapter`, as shown in the following code snippet:

```java

public class MytActivity extends AppCompatActivity {
private MyAdapter adapter = new MyAdapter();
private MyViewModel viewModel;

@Override
public void onCreate(@Nullable Bundle savedInstanceState) {
super.onCreate(savedInstanceState);
viewModel = ViewModelProviders.of(this).get(MyViewModel.class);
viewModel.items.observe(this, adapter::submitList);
}
}

```


**Note**: your implementation of `PagedListAdapter` should indicate that each item in the data set
can be represented with a unique identifier by calling `setHasStableIds(true)` and implement `getItemId(position: Int)`.
Otherwise `RecyclerView` will loose current position after each data change.

[firestore-docs]: https://firebase.google.com/docs/firestore/
[firestore-custom-objects]: https://firebase.google.com/docs/firestore/manage-data/add-data#custom_objects
[recyclerview]: https://developer.android.com/reference/android/support/v7/widget/RecyclerView.html
Expand Down
10 changes: 10 additions & 0 deletions firestore/build.gradle.kts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
val kotlin_version: String = "1.3.21"
tasks.named("check").configure { dependsOn("compileDebugAndroidTestJavaWithJavac") }

android {
Expand Down Expand Up @@ -31,4 +32,13 @@ dependencies {
androidTestImplementation(Config.Libs.Test.rules)
androidTestImplementation(Config.Libs.Test.mockito)
androidTestImplementation(Config.Libs.Arch.paging)
implementation(kotlin("stdlib-jdk7", kotlin_version))
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Whoa there! @samtstern Is FUI accepting Kotlin now? From the FAQ:
image

Typically, libs will be written entirely in Kotlin (not postfixed like this) and only if it offers some sort of syntactic benefit that isn't available in Java (like coroutines or multi-platform stuff, see https://github.com/square/okio). Here, I have trouble seeing what's worth the 1MB and cognitive overhead of ensuring our APIs stay idiomatic in both Java and Kotlin.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah I am pushing the Kotlin question to the end. If the PR is worth having we can translate it back to Java at the last step.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SGTM 👍

}
apply {
plugin("kotlin-android")
plugin("kotlin-kapt")
plugin("kotlin-android-extensions")
}
repositories {
mavenCentral()
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,263 @@
package com.firebase.ui.firestore.paging


import android.arch.paging.DataSource
import android.arch.paging.ItemKeyedDataSource
import android.arch.paging.PagedList
import com.google.android.gms.tasks.Task
import com.google.android.gms.tasks.TaskCompletionSource
import com.google.android.gms.tasks.Tasks
import com.google.firebase.firestore.*
import com.google.firebase.firestore.Source.CACHE
import java.util.concurrent.atomic.AtomicBoolean

/**
* Implementation of [ItemKeyedDataSource] for Firebase Firestore.
*
* [errorCallback] is optional and if not provided the [defaultErrorCallback] will be used.
*
* Note: If user provide [errorCallback] he should control data source invalidation manually.
*
* @property directQuery The direct query.
* @property reverseQuery The reverse query.
* @property errorCallback The error callback.
*/
@Suppress("NOTHING_TO_INLINE")
class FirestoreItemKeyedDataSource(
private val directQuery: Query,
private val reverseQuery: Query,
private val errorCallback: ErrorCallback?
) : ItemKeyedDataSource<DocumentSnapshot, DocumentSnapshot>() {

private val registrations = mutableListOf<ListenerRegistration>()

/**
* Factory for [FirestoreItemKeyedDataSource].
*
* @property queryFactory The query factory.
* @property errorCallback The error callback.
*/
class Factory @JvmOverloads constructor(
private val queryFactory: QueryFactory,
private val errorCallback: ErrorCallback? = null
) : DataSource.Factory<DocumentSnapshot, DocumentSnapshot>() {

/**
* Creates and returns [FirestoreItemKeyedDataSource].
*
* @return The created data source.
*/
override fun create(): DataSource<DocumentSnapshot, DocumentSnapshot> =
FirestoreItemKeyedDataSource(queryFactory.create(), queryFactory.createReverse(), errorCallback)
}

override fun loadInitial(
params: LoadInitialParams<DocumentSnapshot>,
callback: LoadInitialCallback<DocumentSnapshot>
) {
val lastSeenDocument = params.requestedInitialKey
val requestedLoadSize = params.requestedLoadSize.toLong()

if (lastSeenDocument == null) { // Initial data load could be asynchronous.
loadFromBeginning(requestedLoadSize, callback)
} else { // Stale data refresh. Should be synchronous to prevent RecyclerView items disappearing and loss of current position.
loadAround(lastSeenDocument, requestedLoadSize, callback).await()
}
}

/**
* Tries to load half of [loadSize] documents before [document] and half of [loadSize] documents starting at provided [document].
*
* @param document The document to load around.
* @param loadSize Requested number of items to load.
*
* @return A Task that will be resolved with the results of the Query.
*/
private fun loadAround(
document: DocumentSnapshot,
loadSize: Long,
callback: LoadInitialCallback<DocumentSnapshot>
) = tryGetDocumentPrior(document, loadSize / 2).continueWithTask { previousTask ->
loadStartAt(previousTask.result!!, loadSize, callback)
}

/**
* Returns document before [document] maximum by [preloadSize] or [document] if there is no documents before.
*
* @param document The document to load before.
* @param preloadSize Requested number of items to preload.
*
* @return A Task that will be resolved with the results of the Query.
*/
private fun tryGetDocumentPrior(
document: DocumentSnapshot,
preloadSize: Long
): Task<DocumentSnapshot> = getItemsBefore(document, preloadSize).continueWith { task ->
if (task.isSuccessful) {
task.result!!.documents.lastOrNull() ?: document
} else {
document
}
}

private fun getItemsBefore(
document: DocumentSnapshot,
loadSize: Long
) = queryEndAt(document, loadSize).get(CACHE)

private fun loadStartAt(
document: DocumentSnapshot,
loadSize: Long,
callback: LoadInitialCallback<DocumentSnapshot>
) = PageLoader(callback).also { pageLoader ->
addPageLoader(queryStartAt(document, loadSize), pageLoader)
}.task

private fun loadFromBeginning(
loadSize: Long,
callback: LoadInitialCallback<DocumentSnapshot>
) {
addPageLoader(
queryStartFromBeginning(loadSize),
PageLoader(callback)
)
}

override fun loadAfter(
params: LoadParams<DocumentSnapshot>,
callback: LoadCallback<DocumentSnapshot>
) {
addPageLoader(
queryStartAfter(params.key, params.requestedLoadSize.toLong()),
PageLoader(callback)
)
}

override fun loadBefore(
params: LoadParams<DocumentSnapshot>,
callback: LoadCallback<DocumentSnapshot>
) {
addPageLoader(
queryEndBefore(params.key, params.requestedLoadSize.toLong()),
PageLoader(callback.reversedResults())
)
}

override fun getKey(item: DocumentSnapshot) = item

override fun invalidate() {
super.invalidate()
removeAllPageLoaders()
}

private fun removeAllPageLoaders() {
registrations.forEach { it.remove() }
registrations.clear()
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe one of the reasons this hasn't been done yet is because of how difficult it is to properly subscribe and unsub from these listeners. Can you explain a bit more how and when invalidate is called?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, I just forget to unsubscribe.

I wrote it previously. By design of Paging library to refresh data in UI you should invalidate DS.
Here quote from Paging library documentation:

A PagedList / DataSource pair are a snapshot of the data set. A new pair of PagedList / DataSource must be created if an update occurs, such as a reorder, insert, delete, or content update occurs. A DataSource must detect that it cannot continue loading its snapshot (for instance, when Database query notices a table being invalidated), and call invalidate(). Then a new PagedList / DataSource pair would be created to load data from the new state of the Database query.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, I think I understand what's going on now. 👍

I'm a little concerned about cost. Correct me if I'm wrong, but here's what I think is going on: when a page is being loaded, it adds a snapshot listener which submits the data the first time it gets triggered. Then, on the second trigger, it invalidates the entire data set and removes all the listeners. This causes the adapter to requery everything from scratch and start the process anew. Is that correct?

If so, this feature would be extremely expensive to users. They wouldn't be paying for the diff—every single update would cost them the entire query. Coming from an official Firebase library, that seems a little abusive. @samtstern WDYT?

Copy link
Author

@abond abond Mar 19, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, you right that it requery data on every change, but not all the data only recent pageSize*3 items by default or can be specified by PagedList.Config.initialLoadSizeHint. What is more when you finish #1388 there will be PagedList.Config.maxSize which limits number of item currently kept in memory and which will be involved in old with new data comparison.

What about requerying - this 3 resent pages most likely will be loaded from cache if I understand correctly how firebase works.

upd: Here is quote from firestore pricing page:

Also, if the listener is disconnected for more than 30 minutes (for example, if the user goes offline), you will be charged for reads as if you had issued a brand-new query.

According to it only if some new documents will be queried after data change detection there will be extra charges. As I understand even if we requery whole data from the beginning there will be no extra charge during immideate requery.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After some testing I come to conclusion that I was wrong - every requery leeds to extra charges.
I was wrongly assumed that since offline access is enabled by default recently accessed documents will be fetched from local cache and do not count as priced reads. But unfortunately thats not true.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, you right, but problem is that exactly same means with the same limit and start/end conditions.
But in this implementation we can't avoid creating new queries starting from different positions so I think it is expensive to use.
I have an idea how to solve this issue. If I succeed I send new PR.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Assume that the query result is identical

execute exact same query multiple times doesn't charge for the entire doc set multiple times

Is this still true? @samtstern

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes that's true, identical queries issued within 30m of each other don't double-charge unless there are new results.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What if we have 1 new document in the query result, for example, the first query return 3 documents, the second query return the same 3 documents + 1 new document. In this case, how many documents read will be charged?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In that case 4 total reads. 3 for the initial result. 1 for the update later.

}

private fun addPageLoader(
query: Query,
pageLoader: PageLoader
) {
registrations.add(query.addSnapshotListener(pageLoader))
}

private fun queryStartFromBeginning(loadSize: Long) = directQuery.limit(loadSize)

private fun queryStartAt(document: DocumentSnapshot, loadSize: Long) = directQuery.startAt(document).limit(loadSize)

private fun queryStartAfter(document: DocumentSnapshot, loadSize: Long) = directQuery.startAfter(document).limit(loadSize)

private fun queryEndAt(document: DocumentSnapshot, loadSize: Long) = reverseQuery.startAt(document).limit(loadSize)

private fun queryEndBefore(document: DocumentSnapshot, loadSize: Long) = reverseQuery.startAfter(document).limit(loadSize)

/**
* Implementation of [EventListener].
*
* Listen for loading of the requested page.
*
* The first received result is treated as page data. The subsequent results treated as data change and invoke
* data source invalidation.
*
* @property callback The callback to pass data to [PagedList].
*/
private inner class PageLoader(
private val callback: LoadCallback<DocumentSnapshot>
) : EventListener<QuerySnapshot> {
private val isWaitingForPage = AtomicBoolean(true)
private val taskCompletionSource = TaskCompletionSource<MutableList<DocumentSnapshot>>()

/**
* A [Task] that will be resolved when this page loader loads page or fails.
* Used to load initial data synchronously on stale data refresh.
*/
val task get() = taskCompletionSource.task

override fun onEvent(snapshot: QuerySnapshot?, exception: FirebaseFirestoreException?) {
if (isWaitingForPage()) {

snapshot?.documents?.also { result ->
submit(result)
} ?: submit(exception!!)

} else {
invalidate()
}
}

private fun submit(result: MutableList<DocumentSnapshot>) {
callback.onResult(result)
taskCompletionSource.setResult(result)
}

private fun submit(exception: FirebaseFirestoreException) {
notifyError(exception)
taskCompletionSource.setException(exception)
}

private inline fun isWaitingForPage() = isWaitingForPage.compareAndSet(true, false)
}

private fun notifyError(exception: FirebaseFirestoreException) {
getErrorCallback().onError(exception)
}

private inline fun getErrorCallback() = errorCallback ?: defaultErrorCallback

/**
* Callback to listen to data loading errors.
*/
interface ErrorCallback {
fun onError(exception: FirebaseFirestoreException)
}

/**
* Wraps [LoadCallback] with new one that reverses the results list.
*/
private fun LoadCallback<DocumentSnapshot>.reversedResults() = object : LoadCallback<DocumentSnapshot>() {
override fun onResult(data: MutableList<DocumentSnapshot>) {
[email protected](data.reversed())
}
}

/**
* Waits for [Task] to resolve ignoring whether it succeeded or failed.
* Used to load initial page synchronously on stale data refresh.
*/
private inline fun <TResult> Task<TResult>.await() {
try {
Tasks.await(this)
} catch (e: Throwable) { }
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What sorts of errors are you ignoring here and why?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It waits for completion of the Task that tries to get key befor one that is provided on DS recreation after invalidation(that happens after any document change, creadion or delition, if this change is in the range of existing listener thar previously load page). If it fails DS just start load new portion of documents beginning from provided key.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry! It's not true right now. Described in the previous comment is how it was working at first. Now it wait for whole initial data load finished. And error handling occurred in ResultsListener.onResults method. This code only needed because if we don't wait for this load synchronously Page library erase Recyclerview by submitting empty results.

}

/**
* Used if user do not provide [errorCallback].
* Simply invalidates this data source if loading error occurred.
*/
private val defaultErrorCallback = object : ErrorCallback {
override fun onError(exception: FirebaseFirestoreException) {
invalidate()
}
}
}
Loading