Cleanup

Author: schmidt-sebastian

packages/firestore/exp/src/api/components.ts

@@ -26,13 +26,20 @@ import {
   ComponentProvider,
   MemoryComponentProvider
 } from '../../../src/core/component_provider';
+import { DEFAULT_HOST, DEFAULT_SSL } from '../../../lite/src/api/components';
 
+/**
+ * An instance map that ensures only one FirestoreClient exists per Firestore
+ * instance.
+ */
 const firestoreClientInstances = new Map<Firestore, Promise<FirestoreClient>>();
 
-// settings() defaults:
-export const DEFAULT_HOST = 'firestore.googleapis.com';
-export const DEFAULT_SSL = true;
-
+/**
+ * Returns the initialized and started FirestoreClient for the given Firestore
+ * instance. If none exists, creates a new FirestoreClient with memory
+ * persistence. Callers must invoke removeFirestoreClient() when the Firestore
+ * instance is terminated.
+ */
 export function getFirestoreClient(
   firestore: Firestore
 ): Promise<FirestoreClient> {
@@ -42,7 +49,6 @@ export function getFirestoreClient(
       'The client has already been terminated.'
     );
   }
-
   if (!firestoreClientInstances.has(firestore)) {
     // eslint-disable-next-line @typescript-eslint/no-floating-promises
     initializeFirestoreClient(firestore, new MemoryComponentProvider(), {
@@ -52,12 +58,21 @@ export function getFirestoreClient(
   return firestoreClientInstances.get(firestore)!;
 }
 
+/**
+ * Creates a new FirestoreClient for the given Firestore instance. Throws if the
+ * instance exists.
+ *
+ * @param firestore The Firestore instance for which to create the
+ * FirestoreClient.
+ * @param componentProvider The component provider to use.
+ * @param persistenceSettings Settings for the component provider.
+ */
 export function initializeFirestoreClient(
   firestore: Firestore,
   componentProvider: ComponentProvider,
   persistenceSettings: PersistenceSettings
 ): Promise<void> {
-  if (firestore._terminated || hasFirestoreClient(firestore)) {
+  if (firestore._initialized) {
     throw new FirestoreError(
       Code.FAILED_PRECONDITION,
       'Firestore has already been started and persistence can no longer ' +
@@ -69,7 +84,7 @@ export function initializeFirestoreClient(
   const settings = firestore._getSettings();
   const databaseInfo = new DatabaseInfo(
     firestore._databaseId,
-    /* persistenceKey= */ firestore._persistenceKey,
+    firestore._persistenceKey,
     settings.host ?? DEFAULT_HOST,
     settings.ssl ?? DEFAULT_SSL,
     /** forceLongPolling= */ false
@@ -90,10 +105,10 @@ export function initializeFirestoreClient(
   return initializationPromise;
 }
 
-export function hasFirestoreClient(firestore: Firestore): boolean {
-  return firestoreClientInstances.has(firestore);
-}
-
+/**
+ * Removes and terminates the FirestoreClient for the given instance if it has
+ * been started.
+ */
 export async function removeFirestoreClient(
   firestore: Firestore
 ): Promise<void> {

packages/firestore/exp/src/api/database.ts

@@ -34,14 +34,12 @@ import { Code, FirestoreError } from '../../../src/util/error';
 import { Deferred } from '../../../src/util/promise';
 import { LruParams } from '../../../src/local/lru_garbage_collector';
 import { CACHE_SIZE_UNLIMITED } from '../../../src/api/database';
-import { DatabaseId } from '../../../src/core/database_info';
 import {
   indexedDbClearPersistence,
   indexedDbStoragePrefix
 } from '../../../src/local/indexeddb_persistence';
 import {
   getFirestoreClient,
-  hasFirestoreClient,
   initializeFirestoreClient,
   removeFirestoreClient
 } from './components';
@@ -74,36 +72,6 @@ export class Firestore extends LiteFirestore
     return this._settings;
   }
 
-  /**
-   * Verifies that the client is not running and clears persistence by invoking
-   * `delegate` on the async queue.
-   *
-   * @param delegate A function that clears the clients
-   * backing storage.
-   */
-  _clearPersistence(
-    delegate: (databaseId: DatabaseId, persistenceKey: string) => Promise<void>
-  ): Promise<void> {
-    if (hasFirestoreClient(this)) {
-      throw new FirestoreError(
-        Code.FAILED_PRECONDITION,
-        'Persistence can only be cleared before a Firestore instance is ' +
-          'initialized or after it is terminated.'
-      );
-    }
-
-    const deferred = new Deferred<void>();
-    this._queue.enqueueAndForgetEvenAfterShutdown(async () => {
-      try {
-        await delegate(this._databaseId, this._persistenceKey);
-        deferred.resolve();
-      } catch (e) {
-        deferred.reject(e);
-      }
-    });
-    return deferred.promise;
-  }
-
   async _terminate(): Promise<void> {
     await super._terminate();
     await removeFirestoreClient(this);
@@ -148,8 +116,10 @@ export function enableIndexedDbPersistence(
     new IndexedDbComponentProvider(),
     {
       durable: true,
+      synchronizeTabs: false,
       cacheSizeBytes:
-        settings.cacheSizeBytes || LruParams.DEFAULT_CACHE_SIZE_BYTES
+        settings.cacheSizeBytes || LruParams.DEFAULT_CACHE_SIZE_BYTES,
+      forceOwningTab: false
     }
   );
 }
@@ -166,20 +136,39 @@ export function enableMultiTabIndexedDbPersistence(
       durable: true,
       synchronizeTabs: true,
       cacheSizeBytes:
-        settings.cacheSizeBytes || LruParams.DEFAULT_CACHE_SIZE_BYTES
+        settings.cacheSizeBytes || LruParams.DEFAULT_CACHE_SIZE_BYTES,
+      forceOwningTab: false
     }
-  ).then(() => {});
+  );
 }
 
 export function clearIndexedDbPersistence(
   firestore: firestore.FirebaseFirestore
 ): Promise<void> {
   const firestoreImpl = cast(firestore, Firestore);
-  return firestoreImpl._clearPersistence((databaseId, persistenceKey) => {
-    return indexedDbClearPersistence(
-      indexedDbStoragePrefix(databaseId, persistenceKey)
+  if (firestoreImpl._initialized && !firestoreImpl._terminated) {
+    throw new FirestoreError(
+      Code.FAILED_PRECONDITION,
+      'Persistence can only be cleared before a Firestore instance is ' +
+        'initialized or after it is terminated.'
     );
+  }
+
+  const deferred = new Deferred<void>();
+  firestoreImpl._queue.enqueueAndForgetEvenAfterShutdown(async () => {
+    try {
+      await indexedDbClearPersistence(
+        indexedDbStoragePrefix(
+          firestoreImpl._databaseId,
+          firestoreImpl._persistenceKey
+        )
+      );
+      deferred.resolve();
+    } catch (e) {
+      deferred.reject(e);
+    }
   });
+  return deferred.promise;
 }
 
 export function waitForPendingWrites(

packages/firestore/lite/index.node.ts

@@ -23,7 +23,8 @@ import { Component, ComponentType } from '@firebase/component';
 export {
   Firestore as FirebaseFirestore,
   initializeFirestore,
-  getFirestore
+  getFirestore,
+  terminate
 } from './src/api/database';
 
 export {
@@ -94,4 +95,3 @@ export function registerFirestore(): void {
 }
 
 registerFirestore();
-export { terminate } from './src/api/database';

packages/firestore/lite/src/api/components.ts

@@ -25,12 +25,25 @@ import { newSerializer } from '../../../src/platform/serializer';
 import { Firestore } from './database';
 import { DatabaseInfo } from '../../../src/core/database_info';
 
-const datastoreInstances = new Map<Firestore, Promise<Datastore>>();
-
 // settings() defaults:
 export const DEFAULT_HOST = 'firestore.googleapis.com';
 export const DEFAULT_SSL = true;
 
+// The components module manages the lifetime of dependencies of the Firestore
+// client. Dependencies can be lazily constructed and only one exists per
+// Firestore instance.
+
+/**
+ * An instance map that ensures only one Datastore exists per Firestore
+ * instance.
+ */
+const datastoreInstances = new Map<Firestore, Promise<Datastore>>();
+
+/**
+ * Returns an initialized and started Datastore for the given Firestore
+ * instance. Callers must invoke removeDatastore() when the Firestore
+ * instance is terminated.
+ */
 export function getDatastore(firestore: Firestore): Promise<Datastore> {
   if (!datastoreInstances.has(firestore)) {
     const settings = firestore._getSettings();
@@ -50,6 +63,10 @@ export function getDatastore(firestore: Firestore): Promise<Datastore> {
   return datastoreInstances.get(firestore)!;
 }
 
+/**
+ * Removes and terminates the Datastore for the given instance if it has
+ * been started.
+ */
 export async function removeDatastore(firestore: Firestore): Promise<void> {
   const datastore = await datastoreInstances.get(firestore);
   if (datastore) {

packages/firestore/lite/src/api/database.ts

@@ -16,7 +16,6 @@
  */
 
 import * as firestore from '../../';
-import { Settings } from '../../';
 
 import { _getProvider, _removeServiceInstance } from '@firebase/app-exp';
 import { FirebaseApp, _FirebaseService } from '@firebase/app-types-exp';
@@ -41,44 +40,49 @@ export class Firestore
   readonly _databaseId: DatabaseId;
   readonly _credentials: CredentialsProvider;
   readonly _persistenceKey: string = '(lite)';
-  private readonly _firebaseApp: FirebaseApp;
-
-  // docs
-  _terminated?: Promise<void>;
 
   // Assigned via _configureClient()
   protected _settings?: firestore.Settings;
+  private _settingsFrozen = false;
+
+  // A task that is assigned when the terminate() is invoked and resolved when
+  // all components have shut down.
+  private _terminateTask?: Promise<void>;
 
   constructor(
-    app: FirebaseApp,
+    readonly app: FirebaseApp,
     authProvider: Provider<FirebaseAuthInternalName>
   ) {
-    this._firebaseApp = app;
     this._databaseId = Firestore.databaseIdFromApp(app);
     this._credentials = new FirebaseCredentialsProvider(authProvider);
   }
 
-  get app(): FirebaseApp {
-    return this._firebaseApp;
+  get _initialized(): boolean {
+    return this._settingsFrozen;
+  }
+
+  get _terminated(): boolean {
+    return !this._terminateTask;
   }
 
   _configureClient(settings: firestore.Settings): void {
-    if (this._settings) {
+    if (this._settingsFrozen) {
       throw new FirestoreError(
         Code.FAILED_PRECONDITION,
         'Firestore has already been started and its settings can no longer ' +
           'be changed. initializeFirestore() cannot be called after calling ' +
           'getFirestore().'
       );
     }
+    this._settingsFrozen = true;
     this._settings = settings;
   }
 
-  _getSettings(): Settings {
+  _getSettings(): firestore.Settings {
     if (!this._settings) {
-      this._settings = {};
+      this._configureClient({});
     }
-    return this._settings;
+    return this._settings!;
   }
 
   private static databaseIdFromApp(app: FirebaseApp): DatabaseId {
@@ -93,26 +97,25 @@ export class Firestore
   }
 
   delete(): Promise<void> {
-    if (!this._terminated) {
-      this._terminated = this._terminate();
+    if (!this._terminateTask) {
+      this._terminateTask = this._terminate();
     }
-    return this._terminated;
+    return this._terminateTask;
   }
 
+  /**
+   * Terminates all components used by this client. Subclasses can override
+   * this method to clean up their own dependencies, but must also call this
+   * method.
+   *
+   * Only ever called once.
+   */
   protected _terminate(): Promise<void> {
-    debugAssert(!this._terminated, 'fff');
+    debugAssert(!this._terminated, 'Cannot invoke _terminate() more than once');
     return removeDatastore(this);
   }
 }
 
-export function terminate(
-  firestore: firestore.FirebaseFirestore
-): Promise<void> {
-  _removeServiceInstance(firestore.app, 'firestore/lite');
-  const firestoreClient = cast(firestore, Firestore);
-  return firestoreClient.delete();
-}
-
 export function initializeFirestore(
   app: FirebaseApp,
   settings: firestore.Settings
@@ -128,3 +131,11 @@ export function initializeFirestore(
 export function getFirestore(app: FirebaseApp): Firestore {
   return _getProvider(app, 'firestore/lite').getImmediate() as Firestore;
 }
+
+export function terminate(
+  firestore: firestore.FirebaseFirestore
+): Promise<void> {
+  _removeServiceInstance(firestore.app, 'firestore/lite');
+  const firestoreClient = cast(firestore, Firestore);
+  return firestoreClient.delete();
+}

packages/firestore/src/core/firestore_client.ts

@@ -65,8 +65,8 @@ export type PersistenceSettings =
   | {
       readonly durable: true;
       readonly cacheSizeBytes: number;
-      readonly synchronizeTabs?: boolean;
-      readonly forceOwningTab?: boolean;
+      readonly synchronizeTabs: boolean;
+      readonly forceOwningTab: boolean;
     };
 
 /**