WIP

Author: schmidt-sebastian

common/api-review/database.api.md

@@ -7,7 +7,7 @@
 import { EmulatorMockTokenOptions } from '@firebase/util';
 import { FirebaseApp } from '@firebase/app';
 
-// @public
+// @public (undocumented)
 export function child(parent: Reference, path: string): Reference;
 
 // @public

packages/firestore/src/api/credentials.ts

@@ -82,12 +82,6 @@ export class OAuthToken implements Token {
  */
 export type CredentialChangeListener = (user: User) => void;
 
-/**
- * A Listener for credential change events. The listener blocks until a user
- * is available.
- */
-export type AsyncCredentialChangeListener = (user: Promise<User>) => void;
-
 /**
  * Provides methods for getting the uid and token for the current user and
  * listening for changes.
@@ -107,7 +101,10 @@ export interface CredentialsProvider {
    * (sign-in / sign-out, token changes). It is immediately called once with the
    * initial user.
    */
-  setChangeListener(changeListener: AsyncCredentialChangeListener): void;
+  setChangeListener(
+    asyncQueue: AsyncQueue,
+    changeListener: CredentialChangeListener
+  ): void;
 
   /** Removes the previously-set change listener. */
   removeChangeListener(): void;
@@ -120,22 +117,25 @@ export class EmptyCredentialsProvider implements CredentialsProvider {
    * This isn't actually necessary since the UID never changes, but we use this
    * to verify the listen contract is adhered to in tests.
    */
-  private changeListener: AsyncCredentialChangeListener | null = null;
+  private changeListener: CredentialChangeListener | null = null;
 
   getToken(): Promise<Token | null> {
     return Promise.resolve<Token | null>(null);
   }
 
   invalidateToken(): void {}
 
-  setChangeListener(changeListener: AsyncCredentialChangeListener): void {
+  setChangeListener(
+    asyncQueue: AsyncQueue,
+    changeListener: CredentialChangeListener
+  ): void {
     debugAssert(
       !this.changeListener,
       'Can only call setChangeListener() once.'
     );
     this.changeListener = changeListener;
     // Fire with initial user.
-    changeListener(Promise.resolve(User.UNAUTHENTICATED));
+    changeListener(User.UNAUTHENTICATED);
   }
 
   removeChangeListener(): void {
@@ -161,23 +161,25 @@ export class FirebaseCredentialsProvider implements CredentialsProvider {
   private tokenCounter = 0;
 
   /** The listener registered with setChangeListener(). */
-  private changeListener: AsyncCredentialChangeListener | null = null;
+  private changeListener: CredentialChangeListener | null = null;
 
   private forceRefresh = false;
 
   private auth: FirebaseAuthInternal | null = null;
 
-  private authInitialized = new Deferred<void>();
+  private asyncQueue: AsyncQueue | null = null;
+
+  private authListeners: Array<Deferred> = [];
 
   constructor(authProvider: Provider<FirebaseAuthInternalName>) {
     this.tokenListener = () => {
       this.tokenCounter++;
-      this.authInitialized.resolve();
+      this.authListeners.forEach(listener => listener.resolve());
       this.currentUser = this.getUser();
-      this.receivedInitialUser = true;
-      if (this.changeListener) {
-        this.changeListener(Promise.resolve(this.currentUser));
+      if (this.receivedInitialUser && this.changeListener) {
+        this.changeListener(this.currentUser);
       }
+      this.receivedInitialUser = true;
     };
 
     this.tokenCounter = 0;
@@ -186,15 +188,10 @@ export class FirebaseCredentialsProvider implements CredentialsProvider {
       logDebug('FirebaseCredentialsProvider', 'Auth detected');
       this.auth = auth;
       this.receivedInitialUser = false;
-      // tokenListener/changeListener can be removed by removeChangeListener()
-      if (this.tokenListener && this.changeListener) {
+      // tokenListener can be removed by removeChangeListener()
+      if (this.tokenListener) {
+        this.awaitToken();
         this.auth.addAuthTokenListener(this.tokenListener);
-        // Block the change listener until auth becomes available
-        this.changeListener(
-          new Promise<User>(resolve => {
-            this.authInitialized.promise.then(() => resolve(this.getUser()));
-          })
-        );
       }
     };
 
@@ -216,6 +213,8 @@ export class FirebaseCredentialsProvider implements CredentialsProvider {
         }
       }
     }, 0);
+
+    this.awaitToken();
   }
 
   getToken(): Promise<Token | null> {
@@ -263,16 +262,22 @@ export class FirebaseCredentialsProvider implements CredentialsProvider {
     this.forceRefresh = true;
   }
 
-  setChangeListener(changeListener: AsyncCredentialChangeListener): void {
+  setChangeListener(
+    asyncQueue: AsyncQueue,
+    changeListener: CredentialChangeListener
+  ): void {
     debugAssert(
       !this.changeListener,
       'Can only call setChangeListener() once.'
     );
     this.changeListener = changeListener;
+    this.asyncQueue = asyncQueue;
 
     // Fire the initial event
     if (this.receivedInitialUser) {
-      changeListener(Promise.resolve(this.currentUser));
+      asyncQueue.enqueueRetryable(() => {
+        changeListener(this.currentUser);
+      });
     }
   }
 
@@ -296,6 +301,25 @@ export class FirebaseCredentialsProvider implements CredentialsProvider {
     );
     return new User(currentUid);
   }
+
+  /**
+   * 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.
+   */
+  private awaitToken(): void {
+    if (this.asyncQueue) {
+      this.authListeners.push(new Deferred<void>());
+      this.asyncQueue.enqueueRetryable(async () => {
+        console.log('Blocking queue');
+        await Promise.all(this.authListeners.map(deferred => deferred.promise));
+        if (this.changeListener) {
+          this.changeListener(this.currentUser);
+        }
+      });
+    }
+  }
 }
 
 // Manual type definition for the subset of Gapi we use.
@@ -358,9 +382,12 @@ export class FirstPartyCredentialsProvider implements CredentialsProvider {
     );
   }
 
-  setChangeListener(changeListener: AsyncCredentialChangeListener): void {
+  setChangeListener(
+    asyncQueue: AsyncQueue,
+    changeListener: CredentialChangeListener
+  ): void {
     // Fire with initial uid.
-    changeListener(Promise.resolve(User.FIRST_PARTY));
+    changeListener(User.FIRST_PARTY);
   }
 
   removeChangeListener(): void {}

packages/firestore/src/core/firestore_client.ts

@@ -116,20 +116,15 @@ export class FirestoreClient {
     public asyncQueue: AsyncQueue,
     private databaseInfo: DatabaseInfo
   ) {
-    this.credentials.setChangeListener((userProvider: Promise<User>) => {
-      this.asyncQueue.enqueueAndForget(async () => {
-        const user = await userProvider;
-        logDebug(LOG_TAG, 'Received user=', user.uid);
-        this.user = user;
-        this.credentialListener(user);
-        this.receivedInitialUser.resolve();
-      });
+    this.credentials.setChangeListener(asyncQueue, user => {
+      logDebug(LOG_TAG, 'Received user=', user.uid);
+      this.user = user;
+      this.credentialListener(user);
+      this.receivedInitialUser.resolve();
     });
   }
 
   async getConfiguration(): Promise<ComponentConfiguration> {
-    await this.receivedInitialUser.promise;
-
     return {
       asyncQueue: this.asyncQueue,
       databaseInfo: this.databaseInfo,
@@ -204,12 +199,7 @@ export async function setOfflineComponentProvider(
   client.setCredentialChangeListener(user => {
     if (!currentUser.isEqual(user)) {
       currentUser = user;
-      client.asyncQueue.enqueueRetryable(async () => {
-        await localStoreHandleUserChange(
-          offlineComponentProvider.localStore,
-          user
-        );
-      });
+      localStoreHandleUserChange(offlineComponentProvider.localStore, user);
     }
   });
 

packages/firestore/test/integration/util/internal_helpers.ts

@@ -18,7 +18,6 @@
 import * as firestore from '@firebase/firestore-types';
 
 import {
-  AsyncCredentialChangeListener,
   CredentialChangeListener,
   CredentialsProvider,
   EmptyCredentialsProvider
@@ -36,6 +35,7 @@ import { key } from '../../util/helpers';
 
 import { withTestDbsSettings } from './helpers';
 import { DEFAULT_PROJECT_ID, DEFAULT_SETTINGS } from './settings';
+import { AsyncQueue } from '../../../src/util/async_queue';
 
 export function asyncQueue(db: firestore.FirebaseFirestore): AsyncQueueImpl {
   return (db as Firestore)._delegate._queue as AsyncQueueImpl;
@@ -65,14 +65,17 @@ export function withTestDatastore(
 }
 
 export class MockCredentialsProvider extends EmptyCredentialsProvider {
-  private listener: AsyncCredentialChangeListener | null = null;
+  private listener: CredentialChangeListener | null = null;
 
   triggerUserChange(newUser: User): void {
-    this.listener!(Promise.resolve(newUser));
+    this.listener!(newUser);
   }
 
-  setChangeListener(listener: AsyncCredentialChangeListener): void {
-    super.setChangeListener(listener);
+  setChangeListener(
+    asyncQueue: AsyncQueue,
+    listener: CredentialChangeListener
+  ): void {
+    super.setChangeListener(asyncQueue, listener);
     this.listener = listener;
   }
 }