Chain Firestore setDocument operations in offline
alexandr2levin opened this issue · 12 comments
Hello, great library! But here is a problem :)
I need to chain Firebase-s setDocument operation in offline, but I can't because library's binding trying to wait till Task's OnSuccessListener will be called.
How can I manage this?
Firebase doesn't provide you any way to check if there is internet or not when calling setDocument as you can see in the official doc: https://firebase.google.com/docs/firestore/manage-data/enable-offline?hl=es-419 you can just check it on get and listener calls.
setDocument will always sucess even if your client is offline(if you have enabled the offline capabilities).
FirebaseFirestoreSettings settings = new FirebaseFirestoreSettings.Builder()
.setPersistenceEnabled(true)
.build();
db.setFirestoreSettings(settings);
The only operations that will fail if your client is offline are WriteBatches and Transactions. Anyway, if you need to change the behaviour base on that. You can create a method checking the connection in your device and do a flatmap call with it. If it's true, returns Firebase method, if not, your custom logic.
Hope that it helps you! thx for the feedback
But how I can chain tasks in offline if OnSuccessListener doesn't called until data pushed to server?
Everyone recommend do not observe for this event as local data changed immediately.
So, there is a problem: RxFirestore.setDocument will never be completed in offline as OnSuccessListener called ONLY when data is pushed to server. What should I do if I want to chain multiple RxFirestore.setDocument while being in offline-mode?
Try4W is right. This library methods will use the firebase callback to emit data to Obversable when creating/adding documents to an collection.
The code below (from this library) will add a document to our database, but the single will never emit onSuccess either onError because these firebase's callback: .addOnCompleteListener and .addOnFailure will only be called when we sync a POJO with the server. That is the problem. The document has been already added to our database but our single will never emit if the user has no network connection.
@TRY4W, what I recommend for you is to clone this library and create some methods that focus primary on offline addition of documents. Getters methods of this library will work properly even if the user isn't connected, so what we only need to do is to create new methods and avoid those callbacks.
@NonNull
public static Single<DocumentReference> addDocument(@NonNull final CollectionReference ref,
@NonNull final Object pojo) {
return Single.create(new SingleOnSubscribe<DocumentReference>() {
@Override
public void subscribe(final SingleEmitter<DocumentReference> emitter) throws Exception {
ref.add(pojo).addOnCompleteListener(new OnCompleteListener<DocumentReference>() {
@Override
public void onComplete(@NonNull Task<DocumentReference> task) {
emitter.onSuccess(task.getResult());
}
}).addOnFailureListener(new OnFailureListener() {
@Override
public void onFailure(@NonNull Exception e) {
if (!emitter.isDisposed())
emitter.onError(e);
}
});
}
});
`
Also, this library should only use callbacks when operations like WriteBatch, because they require internet connection. Otherwise, I may focus only in offline additions and emit data immediately.
Totally agree with @itscorey , I think that I will add a few methods regarding this issues to avoid errors when working during offline data. I'm thinking in this kind of approach:
The only way to retrieve a snapshot without internet connection is using the addSnapshotListener, so I'm using it to control errors. Also I would add a try/catch in the set method to control more error cases.
@NonNull
public static Single<DocumentSnapshot> addDocumentOffline(@NonNull final DocumentReference ref,
@NonNull final Map<String, Object> data) {
return Single.create(new SingleOnSubscribe<DocumentSnapshot>() {
@Override
public void subscribe(final SingleEmitter<DocumentSnapshot> emitter) {
final ListenerRegistration listener = ref.addSnapshotListener(new EventListener<DocumentSnapshot>() {
@Override
public void onEvent(@Nullable DocumentSnapshot documentSnapshot, @Nullable FirebaseFirestoreException e) {
if (e != null) {
emitter.onError(e);
} else {
if (documentSnapshot != null) {
documentSnapshot.getReference().set(data);
} else {
emitter.onError(new NullPointerException("Empty Snapshot"));
}
emitter.onSuccess(documentSnapshot);
}
}
});
emitter.setCancellable(new Cancellable() {
@Override
public void cancel() {
listener.remove();
}
});
}
});There should be a method for Add, another for Delete and another one for Update. However, I don't know right now how to achieve this approach using Collections.addSnapshotListener because it returns a list of Documents. What do you think about this approach guys? Any better idea?
Thank you for the feedback!
Hey, @FrangSierra
I'm actually using a clone of your lib with some modifications to fit my things. I've updated my adds methods and they are looking like the one above. But, when I was testing these methods, I've found a big problem: when I add a document and change it (update fields or remove), the modification will not work and the document will be 're-added'.
For example, if I have a document with field name and default value Corey and change it to Corey2, the field will rollback to Corey.
That's weird, but I found out why this is happening.
Our set methods shouldn't be in our Snapshot listeners, because they are just listening to changes in our document. So, when we change something in the document, the listener will execute our set and set the old document again.
We should do something like that:
@NonNull
public static Single<DocumentReference> addDocument(@NonNull final CollectionReference ref,
@NonNull final Map<String, Object> data) {
return Single.create(new SingleOnSubscribe<DocumentReference>() {
@Override
public void subscribe(final SingleEmitter<DocumentReference> emitter) throws Exception {
// Create our document if it doesn't exists
// If it exists, firestore will update it
ref.document().set(data);
// Listen to changes in this document
final ListenerRegistration registration = ref.document().addSnapshotListener(new EventListener<DocumentSnapshot>() {
@Override
public void onEvent(@Nullable DocumentSnapshot documentSnapshot, @Nullable FirebaseFirestoreException e) {
if (e != null) {
emitter.onError(e);
return;
}
if (documentSnapshot != null) {
emitter.onSuccess(documentSnapshot.getReference());
} else {
emitter.onError(new NullPointerException("Empty snapshot"));
}
}
});
emitter.setCancellable(new Cancellable() {
@Override
public void cancel() {
registration.remove();
}
});
}
});
}
This bugs happens because our RegistrationListener is listening to changes in the document, if it is removed, this bug will not happen, and remove it after calling onSuccess isn't a good solution either.
I think it is better add, listen and notifies than listen, add and notifies.
Thanks for your effort to keep improving this library. I'll continue using it and help you improving it when I can. Good work!
Isn't add/set are "immediate" operations for local data?
Maybe we don't need to wait anything, maybe when ref.document().set(data); is called, data is already updated on the next line (for local db)?
I recommend to look at this library https://github.com/btrautmann/RxFirestore which are pretty good in dealing with offline requests and doesn't relay on backend callbacks at all.
Yes. add/set/update are operations made immediate in local database.
If you do ref.document().set(pojo) and in the next line call showDocumentFromDb(doc...) you will receive an error saying that the document isn't added yet or something like that.
Yes, they are added immediate, but remember that this is a operation... so you will need to wait some milliseconds.
You need to add the document and listen to changes on it to do your action. This works fine offline.
I'm actually doing that and this is ok.
I think that @itscorey it's right. And the approach that we are proposing it's ok. Anyway we could just do
try{
ref.document().set(data);
emitter.onSuccess(documentSnapshot.getReference());
} catch (e : Exception) {
emitter.onError(e);
}And it will work in the same way, but I don't know if the try catch will receive any possible error that set does on the offline database. That's why I feel more comfortable using the Listener approach. Anyway I think that I will integrate this in separate methods or using an allowOffline flag in the base methods.
@itscorey if you have the methods already done in your library version I invite you to upload a PR to the library :) If not, I will add methods as soon as possible. If finally you do it, please remember follow the same indentation rules and add some unitary tests for this new methods.
Thank you so much guys for your support ;)
I just added new offline methods on the development branch. I need to test them first on my sample app to check that everything works correctly. Anyway, feel free to suggest any change or test it in your current applications. I will merge the branch after my tests and release a new version.
@FrangSierra I wanted to give a try but addDocumentOffline is private.
Hello @tspoke , my fault, I left the method in private visibility.
It was already Fixed. Thank you for your support!