address comments #1

Author: wu-hui

firebase-firestore/src/androidTest/java/com/google/firebase/firestore/FirestoreTest.java

@@ -1117,8 +1117,7 @@ public void testWaitForPendingWritesResolves() {
     assertTrue(!pendingWrite.isComplete());
     assertTrue(!awaitsPendingWrites2.isComplete());
 
-    waitFor(firestore.enableNetwork());
-    waitFor(pendingWrite);
+    firestore.enableNetwork();
     waitFor(awaitsPendingWrites2);
     assertTrue(awaitsPendingWrites2.isComplete() && awaitsPendingWrites2.isSuccessful());
   }

firebase-firestore/src/main/java/com/google/firebase/firestore/FirebaseFirestore.java

@@ -363,12 +363,10 @@ Task<Void> shutdown() {
   }
 
   /**
-   * Wait for server acknowledgement for all pending writes existing at the time of calling this
-   * method.
+   * Wait until all pending writes existed at the time of calling are sent to the backend.
    *
-   * <p>Both acceptance and rejection count as server acknowledgement.
-   *
-   * @return A {@link Task} which resolves when all pending writes are acknowledged by the server.
+   * @return A {@link Task} which resolves when all pending writes are sent to the backend. If there
+   *     is a Firebase user change, the return {@link Task} will resolve to an exception.
    */
   Task<Void> waitForPendingWrites() {
     return client.waitForPendingWrites();

firebase-firestore/src/main/java/com/google/firebase/firestore/core/FirestoreClient.java

@@ -230,12 +230,6 @@ public <TResult> Task<TResult> transaction(
    */
   public Task<Void> waitForPendingWrites() {
     this.verifyNotShutdown();
-    if (!remoteStore.canUseNetwork()) {
-      Logger.warn(
-          LOG_TAG,
-          "Network is disabled, the Task created to wait for all writes getting"
-              + " acknowledged by server will not complete until network is enabled.");
-    }
 
     final TaskCompletionSource<Void> source = new TaskCompletionSource<>();
     asyncQueue.enqueueAndForget(() -> syncEngine.registerPendingWritesTask(source));

firebase-firestore/src/main/java/com/google/firebase/firestore/core/SyncEngine.java

@@ -413,7 +413,7 @@ public void handleSuccessfulWrite(MutationBatchResult mutationBatchResult) {
     // they consistently happen before listen events.
     notifyUser(mutationBatchResult.getBatch().getBatchId(), /*status=*/ null);
 
-    resolveTasksAwaitingForPendingWritesIfAny(mutationBatchResult.getBatch().getBatchId());
+    resolvePendingWriteTasks(mutationBatchResult.getBatch().getBatchId());
 
     ImmutableSortedMap<DocumentKey, MaybeDocument> changes =
         localStore.acknowledgeBatch(mutationBatchResult);
@@ -435,16 +435,22 @@ public void handleRejectedWrite(int batchId, Status status) {
     // they consistently happen before listen events.
     notifyUser(batchId, status);
 
-    resolveTasksAwaitingForPendingWritesIfAny(batchId);
+    resolvePendingWriteTasks(batchId);
 
     emitNewSnapsAndNotifyLocalStore(changes, /*remoteEvent=*/ null);
   }
 
   /**
-   * Takes a snapshot of current local mutation queue, and register a user task which will resolve
-   * when all those mutations are either accepted or rejected by the server.
+   * Takes a snapshot of current mutation queue, and register a user task which will resolve when
+   * all those mutations are either accepted or rejected by the server.
    */
   public void registerPendingWritesTask(TaskCompletionSource<Void> userTask) {
+    if (!remoteStore.canUseNetwork()) {
+      Logger.debug(
+          TAG,
+          "The network is disabled. The task returned by 'awaitPendingWrites()' will not complete until the network is enabled.");
+    }
+
     int largestPendingBatchId = localStore.getHighestUnacknowledgedBatchId();
 
     if (largestPendingBatchId == 0) {
@@ -461,7 +467,7 @@ public void registerPendingWritesTask(TaskCompletionSource<Void> userTask) {
   }
 
   /** Resolves tasks waiting for this batch id to get acknowledged by server, if there is any. */
-  private void resolveTasksAwaitingForPendingWritesIfAny(int batchId) {
+  private void resolvePendingWriteTasks(int batchId) {
     if (pendingWritesCallbacks.containsKey(batchId)) {
       for (TaskCompletionSource<Void> task : pendingWritesCallbacks.get(batchId)) {
         task.setResult(null);

firebase-firestore/src/main/java/com/google/firebase/firestore/local/LocalStore.java

@@ -287,7 +287,7 @@ public ImmutableSortedMap<DocumentKey, MaybeDocument> rejectBatch(int batchId) {
    * Returns 0 if the queue is empty.
    */
   public int getHighestUnacknowledgedBatchId() {
-    return mutationQueue.getLargestUnacknowledgedBatchId();
+    return mutationQueue.getHighestUnacknowledgedBatchId();
   }
 
   /** Returns the last recorded stream token for the current user. */

firebase-firestore/src/main/java/com/google/firebase/firestore/local/MemoryMutationQueue.java

@@ -188,8 +188,8 @@ public MutationBatch getNextMutationBatchAfterBatchId(int batchId) {
   }
 
   @Override
-  public int getLargestUnacknowledgedBatchId() {
-    return queue.size() == 0 ? 0 : nextBatchId - 1;
+  public int getHighestUnacknowledgedBatchId() {
+    return queue.isEmpty() ? 0 : nextBatchId - 1;
   }
 
   @Override

firebase-firestore/src/main/java/com/google/firebase/firestore/local/MutationQueue.java

@@ -77,7 +77,7 @@ MutationBatch addMutationBatch(
    * @return The largest (latest) batch id in mutation queue for the current user that is pending
    *     server response, 0 if the queue is empty.
    */
-  int getLargestUnacknowledgedBatchId();
+  int getHighestUnacknowledgedBatchId();
 
   /** Returns all mutation batches in the mutation queue. */
   // TODO: PERF: Current consumer only needs mutated keys; if we can provide that

firebase-firestore/src/main/java/com/google/firebase/firestore/local/SQLiteMutationQueue.java

@@ -250,11 +250,8 @@ public MutationBatch getNextMutationBatchAfterBatchId(int batchId) {
   }
 
   @Override
-  public int getLargestUnacknowledgedBatchId() {
-    if (isEmpty()) {
-      return 0;
-    }
-    return db.query("SELECT MAX(batch_id) FROM mutations " + "WHERE uid = ?")
+  public int getHighestUnacknowledgedBatchId() {
+    return db.query("SELECT IFNULL(MAX(batch_id), 0) FROM mutations WHERE uid = ?")
         .binding(uid)
         .firstValue(row -> row.getInt(0));
   }

firebase-firestore/src/test/java/com/google/firebase/firestore/local/LocalStoreTestCase.java

@@ -1145,7 +1145,7 @@ public void testHandlesPatchMutationWithTransformThenRemoteEvent() {
   }
 
   @Test
-  public void testGetHighestUnacknowledgedBatchIdReturnsExpectedResult() {
+  public void testGetHighestUnacknowledgedBatchId() {
     assertEquals(0, localStore.getHighestUnacknowledgedBatchId());
 
     writeMutation(setMutation("foo/bar", map("abc", 123)));