addressing comments #1

Author: wu-hui

Firestore/Example/Tests/Integration/API/FIRDatabaseTests.mm

@@ -1417,17 +1417,16 @@ - (void)testWaitForPendingWritesCompletes {
 }
 
 - (void)testWaitForPendingWritesFailsWhenUserChanges {
-  FIRApp *app = testutil::AppForUnitTesting(util::MakeString([FSTIntegrationTestCase projectID]));
-  FIRFirestore *firestore = [self firestoreWithApp:app];
+  // FIRApp *app = testutil::AppForUnitTesting(util::MakeString([FSTIntegrationTestCase
+  // projectID]));
+  FIRFirestore *firestore = self.db;
 
-  [firestore
-      disableNetworkWithCompletion:[self completionForExpectationWithName:@"Disable network"]];
-  [self awaitExpectations];
+  [self disableNetwork];
 
   // Writes to local to prevent immediate call to the completion of waitForPendingWrites.
   NSDictionary<NSString *, id> *data =
-      @{@"owner" : @{@"name" : @"Andy", @"email" : @"abc@xyz.com"}};
-  [[firestore documentWithPath:@"abc/123"] setData:data];
+      @{@"owner" : @{@"name" : @"Andy", @"email" : @"abc@example.com"}};
+  [[self documentRef] setData:data];
 
   XCTestExpectation *expectation = [self expectationWithDescription:@"waitForPendingWrites"];
   [firestore waitForPendingWritesWithCompletion:^(NSError *_Nullable error) {
@@ -1442,8 +1441,7 @@ - (void)testWaitForPendingWritesFailsWhenUserChanges {
 }
 
 - (void)testWaitForPendingWritesCompletesWhenOfflineIfNoPending {
-  FIRDocumentReference *doc = [self documentRef];
-  FIRFirestore *firestore = doc.firestore;
+  FIRFirestore *firestore = self.db;
 
   [firestore
       disableNetworkWithCompletion:[self completionForExpectationWithName:@"Disable network"]];

Firestore/Example/Tests/Util/FSTIntegrationTestCase.mm

@@ -85,7 +85,7 @@
 
 // Behaves the same as `EmptyCredentialsProvider` except it can also trigger a user
 // change.
-class MockCredentialsProvider : public EmptyCredentialsProvider {
+class FakeCredentialsProvider : public EmptyCredentialsProvider {
  public:
   void SetCredentialChangeListener(CredentialChangeListener changeListener) override {
     if (changeListener) {
@@ -106,13 +106,13 @@ void ChangeUser(NSString *new_id) {
 
 @implementation FSTIntegrationTestCase {
   NSMutableArray<FIRFirestore *> *_firestores;
-  std::shared_ptr<MockCredentialsProvider> _mockCredProdiver;
+  std::shared_ptr<FakeCredentialsProvider> _fakeCredentialsProvider;
 }
 
 - (void)setUp {
   [super setUp];
 
-  _mockCredProdiver = std::make_shared<MockCredentialsProvider>();
+  _fakeCredentialsProvider = std::make_shared<FakeCredentialsProvider>();
 
   [self clearPersistenceOnce];
   [self primeBackend];
@@ -271,7 +271,7 @@ - (FIRFirestore *)firestoreWithApp:(FIRApp *)app {
   FIRFirestore *firestore =
       [[FIRFirestore alloc] initWithDatabaseID:DatabaseId(projectID)
                                 persistenceKey:util::MakeString(persistenceKey)
-                           credentialsProvider:_mockCredProdiver
+                           credentialsProvider:_fakeCredentialsProvider
                                    workerQueue:std::move(workerQueue)
                                    firebaseApp:app
                               instanceRegistry:nil];
@@ -282,7 +282,7 @@ - (FIRFirestore *)firestoreWithApp:(FIRApp *)app {
 }
 
 - (void)triggerUserChangeWithUid:(NSString *)uid {
-  _mockCredProdiver->ChangeUser(uid);
+  _fakeCredentialsProvider->ChangeUser(uid);
 }
 
 - (void)primeBackend {

Firestore/Source/API/FIRFirestore+Internal.h

@@ -90,6 +90,22 @@ NS_ASSUME_NONNULL_BEGIN
 
 - (void)shutdownInternalWithCompletion:(nullable void (^)(NSError *_Nullable error))completion;
 
+/**
+ * Waits until all currently pending writes for the active user have been acknowledged by the
+ * backend.
+ *
+ * The completion block is called immediately without error if there are no outstanding writes.
+ * Otherwise, the completion block is called when all previously issued writes (including those
+ * written in a previous app session) have been acknowledged by the backend. The completion
+ * block does not wait for writes that were added after the method is called. If you
+ * wish to wait for additional writes, you have to call `waitForPendingWritesWithCompletion`
+ * again.
+ *
+ * Any outstanding `waitForPendingWritesWithCompletion` completion blocks are called with an
+ * error during user change.
+ */
+- (void)waitForPendingWritesWithCompletion:(void (^)(NSError *_Nullable error))completion;
+
 - (const std::shared_ptr<util::AsyncQueue> &)workerQueue;
 
 @property(nonatomic, assign, readonly) std::shared_ptr<api::Firestore> wrapped;

Firestore/Source/Local/FSTLocalStore.mm

@@ -428,9 +428,8 @@ - (nullable FSTMutationBatch *)nextMutationBatchAfterBatchID:(BatchId)batchID {
 }
 
 - (model::BatchId)getHighestUnacknowledgedBatchId {
-  return self.persistence.run("getHighestUnacknowledgedBatchId", [&]() -> model::BatchId {
-    return _mutationQueue->GetHighestUnacknowledgedBatchId();
-  });
+  return self.persistence.run("getHighestUnacknowledgedBatchId",
+                              [&]() { return _mutationQueue->GetHighestUnacknowledgedBatchId(); });
 }
 
 - (FSTQueryData *)allocateQuery:(Query)query {

Firestore/Source/Public/FIRFirestore.h

@@ -154,22 +154,6 @@ NS_SWIFT_NAME(Firestore)
  */
 - (FIRWriteBatch *)batch;
 
-/**
- * Waits until all currently pending writes for the active user have been acknowledged by the
- * backend.
- *
- * The completion block is called immediately without error if there are no outstanding writes.
- * Otherwise, the completion block is called when all previously issued writes (including those
- * written in a previous app session) have been acknowledged by the backend. The completion
- * block does not wait for writes that were added after the method is called. If you
- * wish to wait for additional writes, you have to call `waitForPendingWritesWithCompletion`
- * again.
- *
- * Any outstanding `waitForPendingWritesWithCompletion()` completion blocks are called with an
- * error during user change.
- */
-- (void)waitForPendingWritesWithCompletion:(void (^)(NSError *_Nullable error))completion;
-
 #pragma mark - Logging
 
 /** Enables or disables logging from the Firestore client. */

Firestore/core/src/firebase/firestore/local/leveldb_mutation_queue.mm

@@ -383,16 +383,15 @@ BatchId LoadNextBatchIdFromDb(DB* db) {
       util::PrefixSuccessor(LevelDbMutationKey::KeyPrefix(user_id_));
 
   LevelDbMutationKey row_key;
-  BatchId max_batch_id = kBatchIdUnknown;
 
   it->Seek(next_user_key);
   it->Prev();
   if (it->Valid() && row_key.Decode(MakeStringView(it->key())) &&
       row_key.user_id() == user_id_) {
-    max_batch_id = row_key.batch_id();
+    return row_key.batch_id();
   }
 
-  return max_batch_id;
+  return kBatchIdUnknown;
 }
 
 void LevelDbMutationQueue::PerformConsistencyCheck() {