Internal persistence settings (#1411)

* Refactor PersistenceSettings

Author: Greg Soltis

packages/firestore/src/api/database.ts

@@ -20,7 +20,12 @@ import { FirebaseApp } from '@firebase/app-types';
 import { FirebaseService } from '@firebase/app-types/private';
 import { DatabaseId, DatabaseInfo } from '../core/database_info';
 import { ListenOptions } from '../core/event_manager';
-import { FirestoreClient } from '../core/firestore_client';
+import {
+  FirestoreClient,
+  IndexedDbPersistenceSettings,
+  InternalPersistenceSettings,
+  MemoryPersistenceSettings
+} from '../core/firestore_client';
 import {
   Bound,
   Direction,
@@ -237,38 +242,6 @@ class FirestoreConfig {
   persistence: boolean;
 }
 
-/**
- * Encapsulates the settings that can be used to configure Firestore
- * persistence.
- */
-export class PersistenceSettings {
-  /** Whether to enable multi-tab synchronization. */
-  experimentalTabSynchronization: boolean;
-
-  constructor(
-    readonly enabled: boolean,
-    settings?: firestore.PersistenceSettings
-  ) {
-    assert(
-      enabled || !settings,
-      'Can only provide PersistenceSettings with persistence enabled'
-    );
-    settings = settings || {};
-    this.experimentalTabSynchronization = objUtils.defaulted(
-      settings.experimentalTabSynchronization,
-      DEFAULT_SYNCHRONIZE_TABS
-    );
-  }
-
-  isEqual(other: PersistenceSettings): boolean {
-    return (
-      this.enabled === other.enabled &&
-      this.experimentalTabSynchronization ===
-        other.experimentalTabSynchronization
-    );
-  }
-}
-
 /**
  * The root reference to the database.
  */
@@ -371,23 +344,29 @@ export class Firestore implements firestore.FirebaseFirestore, FirebaseService {
           'any other methods on a Firestore object.'
       );
     }
-
     return this.configureClient(
-      new PersistenceSettings(/* enabled= */ true, settings)
+      new IndexedDbPersistenceSettings(
+        this._config.settings.cacheSizeBytes,
+        settings !== undefined &&
+          objUtils.defaulted(
+            settings.experimentalTabSynchronization,
+            DEFAULT_SYNCHRONIZE_TABS
+          )
+      )
     );
   }
 
   ensureClientConfigured(): FirestoreClient {
     if (!this._firestoreClient) {
       // Kick off starting the client but don't actually wait for it.
       // tslint:disable-next-line:no-floating-promises
-      this.configureClient(new PersistenceSettings(/* enabled= */ false));
+      this.configureClient(new MemoryPersistenceSettings());
     }
     return this._firestoreClient as FirestoreClient;
   }
 
   private configureClient(
-    persistenceSettings: PersistenceSettings
+    persistenceSettings: InternalPersistenceSettings
   ): Promise<void> {
     assert(
       !!this._config.settings.host,
@@ -454,10 +433,7 @@ follow these steps, YOUR APP MAY BREAK.`);
       this._config.credentials,
       this._queue
     );
-    return this._firestoreClient.start(
-      persistenceSettings,
-      this._config.settings.cacheSizeBytes
-    );
+    return this._firestoreClient.start(persistenceSettings);
   }
 
   private static databaseIdFromApp(app: FirebaseApp): DatabaseId {

packages/firestore/src/core/firestore_client.ts

@@ -45,7 +45,6 @@ import {
 import { SyncEngine } from './sync_engine';
 import { View, ViewDocumentChanges } from './view';
 
-import { PersistenceSettings } from '../api/database';
 import {
   LruGarbageCollector,
   LruParams,
@@ -56,7 +55,6 @@ import {
   SharedClientState,
   WebStorageSharedClientState
 } from '../local/shared_client_state';
-import { assert } from '../util/assert';
 import { AutoId } from '../util/misc';
 import { DatabaseId, DatabaseInfo } from './database_info';
 import { Query } from './query';
@@ -72,6 +70,23 @@ const DOM_EXCEPTION_ABORTED = 20;
 /** The DOMException code for quota exceeded. */
 const DOM_EXCEPTION_QUOTA_EXCEEDED = 22;
 
+export class IndexedDbPersistenceSettings {
+  constructor(
+    readonly cacheSizeBytes: number,
+    readonly experimentalTabSynchronization: boolean
+  ) {}
+
+  lruParams(): LruParams {
+    return LruParams.withCacheSize(this.cacheSizeBytes);
+  }
+}
+
+export class MemoryPersistenceSettings {}
+
+export type InternalPersistenceSettings =
+  | IndexedDbPersistenceSettings
+  | MemoryPersistenceSettings;
+
 /**
  * FirestoreClient is a top-level class that constructs and owns all of the
  * pieces of the client SDK architecture. It is responsible for creating the
@@ -146,10 +161,7 @@ export class FirestoreClient {
    *     start for any reason. If usePersistence is false this is
    *     unconditionally resolved.
    */
-  start(
-    persistenceSettings: PersistenceSettings,
-    cacheSizeBytes: number
-  ): Promise<void> {
+  start(persistenceSettings: InternalPersistenceSettings): Promise<void> {
     // We defer our initialization until we get the current user from
     // setChangeListener(). We block the async queue until we got the initial
     // user and the initialization is completed. This will prevent any scheduled
@@ -172,12 +184,7 @@ export class FirestoreClient {
       if (!initialized) {
         initialized = true;
 
-        this.initializePersistence(
-          persistenceSettings,
-          persistenceResult,
-          user,
-          cacheSizeBytes
-        )
+        this.initializePersistence(persistenceSettings, persistenceResult, user)
           .then(maybeLruGc => this.initializeRest(user, maybeLruGc))
           .then(initializationDone.resolve, initializationDone.reject);
       } else {
@@ -223,17 +230,12 @@ export class FirestoreClient {
    *     succeeded.
    */
   private initializePersistence(
-    persistenceSettings: PersistenceSettings,
+    persistenceSettings: InternalPersistenceSettings,
     persistenceResult: Deferred<void>,
-    user: User,
-    cacheSizeBytes: number
+    user: User
   ): Promise<LruGarbageCollector | null> {
-    if (persistenceSettings.enabled) {
-      return this.startIndexedDbPersistence(
-        user,
-        persistenceSettings,
-        cacheSizeBytes
-      )
+    if (persistenceSettings instanceof IndexedDbPersistenceSettings) {
+      return this.startIndexedDbPersistence(user, persistenceSettings)
         .then(maybeLruGc => {
           persistenceResult.resolve();
           return maybeLruGc;
@@ -301,14 +303,8 @@ export class FirestoreClient {
    */
   private startIndexedDbPersistence(
     user: User,
-    settings: PersistenceSettings,
-    cacheSizeBytes: number
+    settings: IndexedDbPersistenceSettings
   ): Promise<LruGarbageCollector> {
-    assert(
-      settings.enabled,
-      'Should only start IndexedDb persitence with offline persistence enabled.'
-    );
-
     // TODO(http://b/33384523): For now we just disable garbage collection
     // when persistence is enabled.
     const storagePrefix = IndexedDbPersistence.buildStoragePrefix(
@@ -331,7 +327,7 @@ export class FirestoreClient {
       }
 
       let persistence: IndexedDbPersistence;
-      const lruParams = LruParams.withCacheSize(cacheSizeBytes);
+      const lruParams = settings.lruParams();
       if (settings.experimentalTabSynchronization) {
         this.sharedClientState = new WebStorageSharedClientState(
           this.asyncQueue,

packages/firestore/test/unit/local/indexeddb_persistence.test.ts

@@ -15,7 +15,6 @@
  */
 
 import { expect } from 'chai';
-import { PersistenceSettings } from '../../../src/api/database';
 import { SnapshotVersion } from '../../../src/core/snapshot_version';
 import { decode, encode } from '../../../src/local/encoded_resource_path';
 import { IndexedDbPersistence } from '../../../src/local/indexeddb_persistence';
@@ -98,7 +97,7 @@ function withDb(
 
 async function withCustomPersistence(
   clientId: ClientId,
-  settings: PersistenceSettings,
+  multiClient: boolean,
   fn: (
     persistence: IndexedDbPersistence,
     platform: TestPlatform,
@@ -114,7 +113,7 @@ async function withCustomPersistence(
     PlatformSupport.getPlatform(),
     new SharedFakeWebStorage()
   );
-  const persistence = await (settings.experimentalTabSynchronization
+  const persistence = await (multiClient
     ? IndexedDbPersistence.createMultiClientIndexedDbPersistence(
         TEST_PERSISTENCE_PREFIX,
         clientId,
@@ -147,11 +146,7 @@ async function withPersistence(
     queue: AsyncQueue
   ) => Promise<void>
 ): Promise<void> {
-  return withCustomPersistence(
-    clientId,
-    new PersistenceSettings(/* enabled */ true),
-    fn
-  );
+  return withCustomPersistence(clientId, /* multiClient= */ false, fn);
 }
 
 async function withMultiClientPersistence(
@@ -162,13 +157,7 @@ async function withMultiClientPersistence(
     queue: AsyncQueue
   ) => Promise<void>
 ): Promise<void> {
-  return withCustomPersistence(
-    clientId,
-    new PersistenceSettings(/* enabled */ true, {
-      experimentalTabSynchronization: true
-    }),
-    fn
-  );
+  return withCustomPersistence(clientId, /* multiClient= */ true, fn);
 }
 
 function getAllObjectStores(db: IDBDatabase): string[] {