Fix unit test

Author: schmidt-sebastian

packages/firestore/package.json

@@ -8,6 +8,8 @@
   "author": "Firebase <[email protected]> (https://firebase.google.com/)",
   "scripts": {
     "prebuild": "tsc -m es2015 --moduleResolution node scripts/*.ts ",
+    "prebuild:lite": "yarn prebuild",
+    "prebuild:exp": "yarn prebuild",
     "prebuild:release": "yarn prebuild",
     "build": "rollup -c rollup.config.es2017.js && rollup -c rollup.config.es5.js && yarn build:lite && yarn build:exp",
     "build:release": "rollup -c rollup.config.es2017.js && rollup -c rollup.config.es5.js",

packages/firestore/rollup.config.es2017.js

@@ -29,7 +29,8 @@ import pkg from './package.json';
 import memoryPkg from './memory/package.json';
 
 import {
-  firestoreTransformers,
+  removeAssertTransformer,
+  removeAssertAndPrefixInternalTransformer,
   manglePrivatePropertiesOptions,
   resolveNodeExterns,
   resolveBrowserExterns,
@@ -72,7 +73,7 @@ const browserBuildPlugins = [
       }
     },
     clean: true,
-    transformers: firestoreTransformers
+    transformers: removeAssertAndPrefixInternalTransformer
   }),
   json({ preferConst: true }),
   terser(manglePrivatePropertiesOptions)
@@ -144,7 +145,8 @@ const nodeBuildPlugins = [
         target: 'es2017'
       }
     },
-    clean: true
+    clean: true,
+    transformers: removeAssertTransformer
   }),
   json(),
   // Needed as we also use the *.proto files

packages/firestore/rollup.config.exp.js

@@ -23,11 +23,12 @@ import path from 'path';
 import { terser } from 'rollup-plugin-terser';
 
 import {
-  firestoreTransformers,
+  resolveNodeExterns,
   generateAliasConfig,
-  manglePrivatePropertiesOptions,
   resolveBrowserExterns,
-  resolveNodeExterns
+  removeAssertTransformer,
+  removeAssertAndPrefixInternalTransformer,
+  manglePrivatePropertiesOptions
 } from './rollup.shared';
 
 import pkg from './exp/package.json';
@@ -40,7 +41,8 @@ const nodePlugins = [
         target: 'es5'
       }
     },
-    clean: true
+    clean: true,
+    transformers: removeAssertTransformer
   }),
   json({ preferConst: true })
 ];
@@ -54,7 +56,7 @@ const browserPlugins = [
       }
     },
     clean: true,
-    transformers: firestoreTransformers
+    transformers: removeAssertAndPrefixInternalTransformer
   }),
   json({ preferConst: true }),
   terser(manglePrivatePropertiesOptions)

packages/firestore/rollup.config.lite.js

@@ -25,7 +25,8 @@ import {
   resolveNodeExterns,
   generateAliasConfig,
   resolveBrowserExterns,
-  firestoreTransformers,
+  removeAssertTransformer,
+  removeAssertAndPrefixInternalTransformer,
   manglePrivatePropertiesOptions
 } from './rollup.shared';
 
@@ -40,7 +41,8 @@ const nodePlugins = [
         target: 'es5'
       }
     },
-    clean: true
+    clean: true,
+    transformers: removeAssertTransformer
   }),
   json({ preferConst: true })
 ];
@@ -54,7 +56,7 @@ const browserPlugins = [
       }
     },
     clean: true,
-    transformers: firestoreTransformers
+    transformers: removeAssertAndPrefixInternalTransformer
   }),
   json({ preferConst: true }),
   terser(manglePrivatePropertiesOptions)

packages/firestore/rollup.shared.js

@@ -59,11 +59,22 @@ export function resolveNodeExterns(id) {
 const externsPaths = externs.map(p => path.resolve(__dirname, '../../', p));
 const publicIdentifiers = extractPublicIdentifiers(externsPaths);
 
+/**
+ * Transformers that remove calls to `debugAssert` and messages for 'fail` and
+ * `hardAssert`.
+ */
+export const removeAssertTransformer = [
+  service => ({
+    before: [removeAsserts(service.getProgram())],
+    after: []
+  })
+];
+
 /**
  * Transformers that remove calls to `debugAssert`, messages for 'fail` and
  * `hardAssert` and appends a __PRIVATE_ prefix to all internal symbols.
  */
-export const firestoreTransformers = [
+export const removeAssertAndPrefixInternalTransformer = [
   service => ({
     before: [
       removeAsserts(service.getProgram()),

packages/firestore/src/core/component_provider.ts

@@ -23,9 +23,8 @@ import {
 } from '../local/shared_client_state';
 import {
   LocalStore,
-  MultiTabLocalStore,
   newLocalStore,
-  newMultiTabLocalStore
+  synchronizeLastDocumentChangeReadTime
 } from '../local/local_store';
 import {
   MultiTabSyncEngine,
@@ -133,7 +132,6 @@ export class MemoryComponentProvider implements ComponentProvider {
       );
     this.remoteStore.syncEngine = this.syncEngine;
 
-    await this.localStore.start();
     await this.sharedClientState.start();
     await this.remoteStore.start();
 
@@ -298,7 +296,6 @@ export class IndexedDbComponentProvider extends MemoryComponentProvider {
  * `synchronizeTabs` will be enabled.
  */
 export class MultiTabIndexedDbComponentProvider extends IndexedDbComponentProvider {
-  localStore!: MultiTabLocalStore;
   syncEngine!: MultiTabSyncEngine;
 
   async initialize(cfg: ComponentConfiguration): Promise<void> {
@@ -318,14 +315,12 @@ export class MultiTabIndexedDbComponentProvider extends IndexedDbComponentProvid
         }
       }
     });
-  }
 
-  createLocalStore(cfg: ComponentConfiguration): LocalStore {
-    return newMultiTabLocalStore(
-      this.persistence,
-      new IndexFreeQueryEngine(),
-      cfg.initialUser
-    );
+    // In multi-tab mode, we need to read the last document change marker from
+    // persistence once during client initialization. The next call to
+    // `getNewDocumentChanges()` will then only read changes that were persisted
+    // since client startup.
+    await synchronizeLastDocumentChangeReadTime(this.localStore);
   }
 
   createSyncEngine(cfg: ComponentConfiguration): SyncEngine {

packages/firestore/src/core/firestore_client.ts

@@ -202,7 +202,8 @@ export class FirestoreClient {
   enableNetwork(): Promise<void> {
     this.verifyNotTerminated();
     return this.asyncQueue.enqueue(() => {
-      return this.syncEngine.enableNetwork();
+      this.persistence.setNetworkEnabled(true);
+      return this.remoteStore.enableNetwork();
     });
   }
 
@@ -334,7 +335,8 @@ export class FirestoreClient {
   disableNetwork(): Promise<void> {
     this.verifyNotTerminated();
     return this.asyncQueue.enqueue(() => {
-      return this.syncEngine.disableNetwork();
+      this.persistence.setNetworkEnabled(false);
+      return this.remoteStore.disableNetwork();
     });
   }
 

packages/firestore/src/core/sync_engine.ts

@@ -17,9 +17,13 @@
 
 import { User } from '../auth/user';
 import {
+  getNewDocumentChanges,
+  getCachedTarget,
   ignoreIfPrimaryLeaseLoss,
   LocalStore,
-  MultiTabLocalStore
+  getCurrentlyActiveClients,
+  lookupMutationDocuments,
+  removeCachedMutationBatchMetadata
 } from '../local/local_store';
 import { LocalViewChanges } from '../local/local_view_changes';
 import { ReferenceSet } from '../local/reference_set';
@@ -225,10 +229,6 @@ export interface SyncEngine extends RemoteSyncer {
 
   handleCredentialChange(user: User): Promise<void>;
 
-  enableNetwork(): Promise<void>;
-
-  disableNetwork(): Promise<void>;
-
   getRemoteKeysForTarget(targetId: TargetId): DocumentKeySet;
 }
 
@@ -940,14 +940,6 @@ class SyncEngineImpl implements SyncEngine {
     }
   }
 
-  enableNetwork(): Promise<void> {
-    return this.remoteStore.enableNetwork();
-  }
-
-  disableNetwork(): Promise<void> {
-    return this.remoteStore.disableNetwork();
-  }
-
   getRemoteKeysForTarget(targetId: TargetId): DocumentKeySet {
     const limboResolution = this.activeLimboResolutionsByTarget.get(targetId);
     if (limboResolution && limboResolution.receivedDocument) {
@@ -1016,38 +1008,10 @@ class MultiTabSyncEngineImpl extends SyncEngineImpl {
   // `isPrimary` is true.
   private _isPrimaryClient: undefined | boolean = undefined;
 
-  constructor(
-    protected localStore: MultiTabLocalStore,
-    remoteStore: RemoteStore,
-    datastore: Datastore,
-    sharedClientState: SharedClientState,
-    currentUser: User,
-    maxConcurrentLimboResolutions: number
-  ) {
-    super(
-      localStore,
-      remoteStore,
-      datastore,
-      sharedClientState,
-      currentUser,
-      maxConcurrentLimboResolutions
-    );
-  }
-
   get isPrimaryClient(): boolean {
     return this._isPrimaryClient === true;
   }
 
-  enableNetwork(): Promise<void> {
-    this.localStore.setNetworkEnabled(true);
-    return super.enableNetwork();
-  }
-
-  disableNetwork(): Promise<void> {
-    this.localStore.setNetworkEnabled(false);
-    return super.disableNetwork();
-  }
-
   /**
    * Reconcile the list of synced documents in an existing view with those
    * from persistence.
@@ -1097,7 +1061,7 @@ class MultiTabSyncEngineImpl extends SyncEngineImpl {
     error?: FirestoreError
   ): Promise<void> {
     this.assertSubscribed('applyBatchState()');
-    const documents = await this.localStore.lookupMutationDocuments(batchId);
+    const documents = await lookupMutationDocuments(this.localStore, batchId);
 
     if (documents === null) {
       // A throttled tab may not have seen the mutation before it was completed
@@ -1120,7 +1084,7 @@ class MultiTabSyncEngineImpl extends SyncEngineImpl {
       // NOTE: Both these methods are no-ops for batches that originated from
       // other clients.
       this.processUserCallback(batchId, error ? error : null);
-      this.localStore.removeCachedMutationBatchMetadata(batchId);
+      removeCachedMutationBatchMetadata(this.localStore, batchId);
     } else {
       fail(`Unknown batchState: ${batchState}`);
     }
@@ -1236,7 +1200,7 @@ class MultiTabSyncEngineImpl extends SyncEngineImpl {
         );
         // For queries that never executed on this client, we need to
         // allocate the target in LocalStore and initialize a new View.
-        const target = await this.localStore.getTarget(targetId);
+        const target = await getCachedTarget(this.localStore, targetId);
         debugAssert(!!target, `Target for id ${targetId} not found`);
         targetData = await this.localStore.allocateTarget(target);
         await this.initializeViewAndComputeSnapshot(
@@ -1277,7 +1241,7 @@ class MultiTabSyncEngineImpl extends SyncEngineImpl {
   }
 
   getActiveClients(): Promise<ClientId[]> {
-    return this.localStore.getActiveClients();
+    return getCurrentlyActiveClients(this.localStore);
   }
 
   async applyTargetState(
@@ -1296,7 +1260,7 @@ class MultiTabSyncEngineImpl extends SyncEngineImpl {
       switch (state) {
         case 'current':
         case 'not-current': {
-          const changes = await this.localStore.getNewDocumentChanges();
+          const changes = await getNewDocumentChanges(this.localStore);
           const synthesizedRemoteEvent = RemoteEvent.createSynthesizedRemoteEventForCurrentChange(
             targetId,
             state === 'current'
@@ -1336,7 +1300,7 @@ class MultiTabSyncEngineImpl extends SyncEngineImpl {
         continue;
       }
 
-      const target = await this.localStore.getTarget(targetId);
+      const target = await getCachedTarget(this.localStore, targetId);
       debugAssert(
         !!target,
         `Query data for active target ${targetId} not found`
@@ -1370,7 +1334,7 @@ class MultiTabSyncEngineImpl extends SyncEngineImpl {
 }
 
 export function newMultiTabSyncEngine(
-  localStore: MultiTabLocalStore,
+  localStore: LocalStore,
   remoteStore: RemoteStore,
   datastore: Datastore,
   sharedClientState: SharedClientState,