Revert: IndexedDB recovery for handleUserChange

Reverts #3087 Fixes #3179
firebase · Jun 9, 2020 · caebb4b · caebb4b
1 parent 1e3721c
commit caebb4b
Show file tree

Hide file tree

Showing 4 changed files with 8 additions and 53 deletions.
diff --git a/packages/firestore/src/core/firestore_client.ts b/packages/firestore/src/core/firestore_client.ts
@@ -179,7 +179,7 @@ export class FirestoreClient {
           persistenceResult
         ).then(initializationDone.resolve, initializationDone.reject);
       } else {
-        this.asyncQueue.enqueueRetryable(() => {
+        this.asyncQueue.enqueueAndForget(() => {
           return this.handleCredentialChange(user);
         });
       }

diff --git a/packages/firestore/src/core/sync_engine.ts b/packages/firestore/src/core/sync_engine.ts
@@ -862,15 +862,15 @@ export class SyncEngine implements RemoteSyncer {
 
   async handleCredentialChange(user: User): Promise<void> {
     const userChanged = !this.currentUser.isEqual(user);
+    this.currentUser = user;
 
     if (userChanged) {
-      const result = await this.localStore.handleUserChange(user);
-      this.currentUser = user;
-
       // Fails tasks waiting for pending writes requested by previous user.
       this.rejectOutstandingPendingWritesCallbacks(
         "'waitForPendingWrites' promise is rejected due to a user change."
       );
+
+      const result = await this.localStore.handleUserChange(user);
       // TODO(b/114226417): Consider calling this only in the primary tab.
       this.sharedClientState.handleUserChange(
         user,

diff --git a/packages/firestore/test/unit/specs/recovery_spec.test.ts b/packages/firestore/test/unit/specs/recovery_spec.test.ts
@@ -709,41 +709,4 @@ describeSpec('Persistence Recovery', ['no-ios', 'no-android'], () => {
         removed: [doc1]
       });
   });
-
-  specTest(
-    'User change handles transaction failures (with recovery)',
-    ['durable-persistence'],
-    () => {
-      const query = Query.atPath(path('collection'));
-      const doc1 = doc(
-        'collection/key1',
-        0,
-        { foo: 'a' },
-        { hasLocalMutations: true }
-      );
-      return spec()
-        .changeUser('user1')
-        .userSets('collection/key1', { foo: 'a' })
-        .userListens(query)
-        .expectEvents(query, {
-          added: [doc1],
-          fromCache: true,
-          hasPendingWrites: true
-        })
-        .failDatabaseTransactions('Handle user change')
-        .changeUser('user2')
-        .recoverDatabase()
-        .runTimer(TimerId.AsyncQueueRetry)
-        .expectEvents(query, { removed: [doc1], fromCache: true })
-        .failDatabaseTransactions('Handle user change')
-        .changeUser('user1')
-        .recoverDatabase()
-        .runTimer(TimerId.AsyncQueueRetry)
-        .expectEvents(query, {
-          added: [doc1],
-          fromCache: true,
-          hasPendingWrites: true
-        });
-    }
-  );
 });
diff --git a/packages/firestore/test/unit/specs/spec_test_runner.ts b/packages/firestore/test/unit/specs/spec_test_runner.ts
@@ -714,19 +714,11 @@ abstract class TestRunner {
     return Promise.resolve();
   }
 
-  private async doChangeUser(user: string | null): Promise<void> {
+  private doChangeUser(user: string | null): Promise<void> {
     this.user = new User(user);
-    const deferred = new Deferred<void>();
-    await this.queue.enqueueRetryable(async () => {
-      try {
-        await this.syncEngine.handleCredentialChange(this.user);
-      } finally {
-        // Resolve the deferred Promise even if the operation failed. This allows
-        // the spec tests to manually retry the failed user change.
-        deferred.resolve();
-      }
-    });
-    return deferred.promise;
+    return this.queue.enqueue(() =>
+      this.syncEngine.handleCredentialChange(this.user)
+    );
   }
 
   private async doFailDatabase(