Skip to content

Allow user to stop listening after shutdown + other minor fixes. #701

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

Merged
merged 3 commits into from
Aug 12, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -1099,6 +1099,22 @@ public void testShutdownCalledMultipleTimes() {
expectError(() -> waitFor(reference.get()), expectedMessage);
}

@Test
public void testCanStopListeningAfterShutdown() {
FirebaseFirestore instance = testFirestore();
DocumentReference reference = instance.document("abc/123");
EventAccumulator<DocumentSnapshot> eventAccumulator = new EventAccumulator<>();
ListenerRegistration registration = reference.addSnapshotListener(eventAccumulator.listener());
eventAccumulator.await();

waitFor(instance.shutdown());

// This should proceed without error.
registration.remove();
// Multiple calls should proceed as an effectively no-op.
registration.remove();
}

@Test
public void testWaitForPendingWritesResolves() {
DocumentReference documentReference = testCollection("abc").document("123");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,11 @@
*/
public class FirebaseFirestore {

/** Provides a registry management interface for {@code FirebaseFirestore} instances. */
/**
* Provides a registry management interface for {@code FirebaseFirestore} instances.
*
* @hide
*/
public interface InstanceRegistry {
/** Removes the Cloud Firestore instance with given name from registry. */
void remove(@NonNull String databaseId);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -93,23 +93,20 @@ public int addQueryListener(QueryListener queryListener) {
return queryInfo.targetId;
}

/** Removes a previously added listener and returns true if the listener was found. */
public boolean removeQueryListener(QueryListener listener) {
/** Removes a previously added listener. It's a no-op if the listener is not found. */
public void removeQueryListener(QueryListener listener) {
Query query = listener.getQuery();
QueryListenersInfo queryInfo = queries.get(query);
boolean lastListen = false;
boolean found = false;
if (queryInfo != null) {
found = queryInfo.listeners.remove(listener);
queryInfo.listeners.remove(listener);
lastListen = queryInfo.listeners.isEmpty();
}

if (lastListen) {
queries.remove(query);
syncEngine.stopListening(query);
}

return found;
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -165,7 +165,6 @@ public QueryListener listen(

/** Stops listening to a query previously listened to. */
public void stopListening(QueryListener listener) {
this.verifyNotShutdown();
asyncQueue.enqueueAndForget(() -> eventManager.removeQueryListener(listener));
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -336,6 +336,7 @@ public void handleRemoteEvent(RemoteEvent event) {
/** Applies an OnlineState change to the sync engine and notifies any views of the change. */
@Override
public void handleOnlineStateChange(OnlineState onlineState) {
assertCallback("handleOnlineStateChange");
ArrayList<ViewSnapshot> newViewSnapshots = new ArrayList<>();
for (Map.Entry<Query, QueryView> entry : queryViewsByQuery.entrySet()) {
View view = entry.getValue().getView();
Expand Down