Firestore can go back to blocking for auth

Author: schmidt-sebastian

packages/firestore/src/api/credentials.ts

@@ -100,6 +100,8 @@ export interface CredentialsProvider {
    * Specifies a listener to be notified of credential changes
    * (sign-in / sign-out, token changes). It is immediately called once with the
    * initial user.
+   *
+   * The change listener is invoked on the provided AsyncQueue.
    */
   setChangeListener(
     asyncQueue: AsyncQueue,
@@ -135,7 +137,10 @@ export class EmptyCredentialsProvider implements CredentialsProvider {
     );
     this.changeListener = changeListener;
     // Fire with initial user.
-    changeListener(User.UNAUTHENTICATED);
+    asyncQueue.enqueueRetryable(() => {
+      changeListener(User.FIRST_PARTY);
+      return Promise.resolve();
+    });
   }
 
   removeChangeListener(): void {
@@ -152,7 +157,21 @@ export class FirebaseCredentialsProvider implements CredentialsProvider {
 
   /** Tracks the current User. */
   private currentUser: User = User.UNAUTHENTICATED;
-  private receivedInitialUser: boolean = false;
+
+  /**
+   * Promise that allows blocking on the next tokenChange event. The Promise is
+   * re-assgined in `awaitTokenAndRaiseInitialEvent()` to allow blocking on
+   * an a "lazily" loaded Auth instance. In this case, `this.receivedUser`
+   * resolves first when the SDK first detects that there is no synchronous
+   * auth, and then gets re-assigned and re-resolved when auth is loaded.
+   */
+  private receivedUser = new Deferred();
+
+  /**
+   * Whether the initial token even has been raised. This can go back to `false`
+   * if Firestore first starts without Auth and Auth is loaded later.
+   */
+  private initialEventRaised: boolean = false;
 
   /**
    * Counter used to detect if the token changed while a getToken request was
@@ -169,28 +188,27 @@ export class FirebaseCredentialsProvider implements CredentialsProvider {
 
   private asyncQueue: AsyncQueue | null = null;
 
-  private authListeners: Array<Deferred> = [];
-
   constructor(authProvider: Provider<FirebaseAuthInternalName>) {
     this.tokenListener = () => {
       this.tokenCounter++;
-      this.authListeners.forEach(listener => listener.resolve());
+      this.receivedUser.resolve();
       this.currentUser = this.getUser();
-      if (this.receivedInitialUser && this.changeListener) {
+      if (this.initialEventRaised && this.changeListener) {
+        // We only invoke the change listener here if the initial event has been
+        // raised. The initial event itself is invoked synchronously in
+        // `awaitTokenAndRaiseInitialEvent()`.
         this.changeListener(this.currentUser);
       }
-      this.receivedInitialUser = true;
     };
 
     this.tokenCounter = 0;
 
     const registerAuth = (auth: FirebaseAuthInternal): void => {
       logDebug('FirebaseCredentialsProvider', 'Auth detected');
       this.auth = auth;
-      this.receivedInitialUser = false;
       // tokenListener can be removed by removeChangeListener()
       if (this.tokenListener) {
-        this.awaitToken();
+        this.awaitTokenAndRaiseInitialEvent();
         this.auth.addAuthTokenListener(this.tokenListener);
       }
     };
@@ -214,7 +232,7 @@ export class FirebaseCredentialsProvider implements CredentialsProvider {
       }
     }, 0);
 
-    this.awaitToken();
+    this.awaitTokenAndRaiseInitialEvent();
   }
 
   getToken(): Promise<Token | null> {
@@ -272,13 +290,6 @@ export class FirebaseCredentialsProvider implements CredentialsProvider {
     );
     this.changeListener = changeListener;
     this.asyncQueue = asyncQueue;
-
-    // Fire the initial event
-    if (this.receivedInitialUser) {
-      asyncQueue.enqueueRetryable(() => {
-        changeListener(this.currentUser);
-      });
-    }
   }
 
   removeChangeListener(): void {
@@ -305,20 +316,29 @@ export class FirebaseCredentialsProvider implements CredentialsProvider {
   /**
    * Blocks the AsyncQueue until the next user is available. This is invoked
    * on SDK start to wait for the first user token (or `null` if Auth is not yet
-   * loaded). If Auth is loaded after Firestore, `awaitToken()` is also used to
-   * block Firestore until auth is fully initialized.
+   * loaded). If Auth is loaded after Firestore,
+   * `awaitTokenAndRaiseInitialEvent()` is also used to block Firestore until
+   * Auth is fully initialized.
+   *
+   * This function also invokes the change listener synchronously once a token
+   * is available.
    */
-  private awaitToken(): void {
+  private awaitTokenAndRaiseInitialEvent(): void {
+    this.initialEventRaised = false;
     if (this.asyncQueue) {
-      this.authListeners.push(new Deferred<void>());
+      // Create a new deferred Promise that gets resolved when we receive the
+      // next token. Ensure that all previous Promises also get resolved.
+      const awaitToken = new Deferred<void>();
+      awaitToken.promise.then(() => awaitToken.resolve());
+      this.receivedUser = awaitToken;
       this.asyncQueue.enqueueRetryable(async () => {
-        console.log('Blocking queue');
-        await Promise.all(this.authListeners.map(deferred => deferred.promise));
+        await awaitToken.promise;
         if (this.changeListener) {
           this.changeListener(this.currentUser);
         }
       });
     }
+    this.initialEventRaised = true;
   }
 }
 
@@ -387,7 +407,10 @@ export class FirstPartyCredentialsProvider implements CredentialsProvider {
     changeListener: CredentialChangeListener
   ): void {
     // Fire with initial uid.
-    changeListener(User.FIRST_PARTY);
+    asyncQueue.enqueueRetryable(() => {
+      changeListener(User.FIRST_PARTY);
+      return Promise.resolve();
+    });
   }
 
   removeChangeListener(): void {}

packages/firestore/src/core/firestore_client.ts

@@ -98,7 +98,6 @@ export class FirestoreClient {
   private user = User.UNAUTHENTICATED;
   private readonly clientId = AutoId.newId();
   private credentialListener: CredentialChangeListener = () => {};
-  private readonly receivedInitialUser = new Deferred<void>();
 
   offlineComponents?: OfflineComponentProvider;
   onlineComponents?: OnlineComponentProvider;
@@ -120,7 +119,6 @@ export class FirestoreClient {
       logDebug(LOG_TAG, 'Received user=', user.uid);
       this.user = user;
       this.credentialListener(user);
-      this.receivedInitialUser.resolve();
     });
   }
 
@@ -137,10 +135,6 @@ export class FirestoreClient {
 
   setCredentialChangeListener(listener: (user: User) => void): void {
     this.credentialListener = listener;
-    // eslint-disable-next-line @typescript-eslint/no-floating-promises
-    this.receivedInitialUser.promise.then(() =>
-      this.credentialListener(this.user)
-    );
   }
 
   /**
@@ -198,8 +192,8 @@ export async function setOfflineComponentProvider(
   let currentUser = configuration.initialUser;
   client.setCredentialChangeListener(user => {
     if (!currentUser.isEqual(user)) {
-      currentUser = user;
       localStoreHandleUserChange(offlineComponentProvider.localStore, user);
+      currentUser = user;
     }
   });
 
@@ -229,12 +223,7 @@ export async function setOnlineComponentProvider(
   // The CredentialChangeListener of the online component provider takes
   // precedence over the offline component provider.
   client.setCredentialChangeListener(user =>
-    client.asyncQueue.enqueueRetryable(() =>
-      remoteStoreHandleCredentialChange(
-        onlineComponentProvider.remoteStore,
-        user
-      )
-    )
+    remoteStoreHandleCredentialChange(onlineComponentProvider.remoteStore, user)
   );
   client.onlineComponents = onlineComponentProvider;
 }

packages/firestore/test/integration/api/batch_writes.test.ts

@@ -48,7 +48,7 @@ apiDescribe('Database batch writes', (persistence: boolean) => {
     });
   });
 
-  it.only('can set documents with merge', () => {
+  it('can set documents with merge', () => {
     return integrationHelpers.withTestDoc(persistence, doc => {
       return doc.firestore
         .batch()