Merge

Author: schmidt-sebastian

packages/firestore/src/core/sync_engine.ts

@@ -802,6 +802,7 @@ export class SyncEngine implements RemoteSyncer, SharedClientStateSyncer {
 
                 newSnaps.push(viewChange.snapshot);
                 const docChanges = LocalViewChanges.fromSnapshot(
+                  queryView.targetId,
                   viewChange.snapshot
                 );
                 docChangesInAllViews.push(docChanges);
@@ -813,7 +814,7 @@ export class SyncEngine implements RemoteSyncer, SharedClientStateSyncer {
 
     await Promise.all(queriesProcessed);
     this.syncEngineListener.onWatchChange(newSnaps);
-    await this.localStore.notifyLocalViewChanges(docChangesInAllViews);
+    this.localStore.notifyLocalViewChanges(docChangesInAllViews);
     // TODO(multitab): Multitab garbage collection
     if (this.isPrimary) {
       await this.localStore

packages/firestore/src/local/local_store.ts

@@ -632,36 +632,17 @@ export class LocalStore {
   /**
    * Notify local store of the changed views to locally pin documents.
    */
-  notifyLocalViewChanges(viewChanges: LocalViewChanges[]): Promise<void> {
-    return this.persistence.runTransaction(
-      'Notify local view changes',
-      false,
-      txn => {
-        const promises = [] as Array<PersistencePromise<void>>;
-        for (const view of viewChanges) {
-          promises.push(
-            this.queryCache
-              .getQueryData(txn, view.query)
-              .next((queryData: QueryData | null) => {
-                assert(
-                  queryData !== null,
-                  'Local view changes contain unallocated query.'
-                );
-                const targetId = queryData!.targetId;
-                this.localViewReferences.addReferences(
-                  view.addedKeys,
-                  targetId
-                );
-                this.localViewReferences.removeReferences(
-                  view.removedKeys,
-                  targetId
-                );
-              })
-          );
-        }
-        return PersistencePromise.waitFor(promises);
-      }
-    );
+  notifyLocalViewChanges(viewChanges: LocalViewChanges[]): void {
+    for (const viewChange of viewChanges) {
+      this.localViewReferences.addReferences(
+        viewChange.addedKeys,
+        viewChange.targetId
+      );
+      this.localViewReferences.removeReferences(
+        viewChange.removedKeys,
+        viewChange.targetId
+      );
+    }
   }
 
   /**

packages/firestore/src/local/local_view_changes.ts

@@ -14,9 +14,9 @@
  * limitations under the License.
  */
 
-import { Query } from '../core/query';
 import { ChangeType, ViewSnapshot } from '../core/view_snapshot';
 import { documentKeySet, DocumentKeySet } from '../model/collections';
+import { TargetId } from '../core/types';
 
 /**
  * A set of changes to what documents are currently in view and out of view for
@@ -25,12 +25,15 @@ import { documentKeySet, DocumentKeySet } from '../model/collections';
  */
 export class LocalViewChanges {
   constructor(
-    readonly query: Query,
+    readonly targetId: TargetId,
     readonly addedKeys: DocumentKeySet,
     readonly removedKeys: DocumentKeySet
   ) {}
 
-  static fromSnapshot(viewSnapshot: ViewSnapshot): LocalViewChanges {
+  static fromSnapshot(
+    targetId: TargetId,
+    viewSnapshot: ViewSnapshot
+  ): LocalViewChanges {
     let addedKeys = documentKeySet();
     let removedKeys = documentKeySet();
 
@@ -47,6 +50,6 @@ export class LocalViewChanges {
       }
     }
 
-    return new LocalViewChanges(viewSnapshot.query, addedKeys, removedKeys);
+    return new LocalViewChanges(targetId, addedKeys, removedKeys);
   }
 }

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

@@ -24,6 +24,8 @@ import {
   withTestCollectionSettings,
   withTestDoc
 } from '../util/helpers';
+import * as log from '../../../src/util/log';
+import { LogLevel } from '../../../src/util/log';
 
 const FieldPath = firebase.firestore.FieldPath;
 const Timestamp = firebase.firestore.Timestamp;
@@ -349,12 +351,18 @@ apiDescribe('Timestamp Fields in snapshots', persistence => {
   };
 
   it('are returned as native dates if timestampsInSnapshots is not set', () => {
+    // Avoid the verbose log message triggered by timestampsInSnapshots ==
+    // false.
+    const logLevel = log.getLogLevel();
+    log.setLogLevel(LogLevel.SILENT);
+
     const settings = { ...DEFAULT_SETTINGS };
     settings['timestampsInSnapshots'] = false;
 
     const timestamp = new Timestamp(100, 123456789);
     const testDocs = { a: testDataWithTimestamps(timestamp) };
     return withTestCollectionSettings(persistence, settings, testDocs, coll => {
+      log.setLogLevel(logLevel);
       return coll
         .doc('a')
         .get()
@@ -411,13 +419,17 @@ apiDescribe('Timestamp Fields in snapshots', persistence => {
   });
 
   it('timestampsInSnapshots affects server timestamps', () => {
+    const logLevel = log.getLogLevel();
+    log.setLogLevel(LogLevel.SILENT);
+
     const settings = { ...DEFAULT_SETTINGS };
     settings['timestampsInSnapshots'] = false;
     const testDocs = {
       a: { timestamp: firebase.firestore.FieldValue.serverTimestamp() }
     };
 
     return withTestCollectionSettings(persistence, settings, testDocs, coll => {
+      log.setLogLevel(logLevel);
       return coll
         .doc('a')
         .get()

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

@@ -116,9 +116,9 @@ class LocalStoreTester {
   }
 
   afterViewChanges(viewChanges: LocalViewChanges): LocalStoreTester {
-    this.promiseChain = this.promiseChain.then(() => {
-      return this.localStore.notifyLocalViewChanges([viewChanges]);
-    });
+    this.promiseChain = this.promiseChain.then(() =>
+      this.localStore.notifyLocalViewChanges([viewChanges])
+    );
     return this;
   }
 
@@ -790,14 +790,14 @@ function genericLocalStoreTests(
       .afterGC()
       .toContain(doc('foo/bar', 1, { foo: 'bar' }))
       .toContain(doc('foo/baz', 0, { foo: 'baz' }, { hasLocalMutations: true }))
-      .after(localViewChanges(query, { added: ['foo/bar', 'foo/baz'] }))
+      .after(localViewChanges(2, { added: ['foo/bar', 'foo/baz'] }))
       .after(docUpdateRemoteEvent(doc('foo/bar', 1, { foo: 'bar' }), [], [2]))
       .after(docUpdateRemoteEvent(doc('foo/baz', 2, { foo: 'baz' }), [2]))
       .afterAcknowledgingMutation({ documentVersion: 2 })
       .afterGC()
       .toContain(doc('foo/bar', 1, { foo: 'bar' }))
       .toContain(doc('foo/baz', 2, { foo: 'baz' }))
-      .after(localViewChanges(query, { removed: ['foo/bar', 'foo/baz'] }))
+      .after(localViewChanges(2, { removed: ['foo/bar', 'foo/baz'] }))
       .afterReleasingQuery(query)
       .afterGC()
       .toNotContain('foo/bar')

packages/firestore/test/unit/specs/perf_spec.test.ts

@@ -15,7 +15,7 @@
  */
 
 import { Query } from '../../../src/core/query';
-import { doc, orderBy, path } from '../../util/helpers';
+import { doc, filter, orderBy, path } from '../../util/helpers';
 
 import { describeSpec, specTest } from './describe_spec';
 import { spec } from './spec_builder';
@@ -242,5 +242,52 @@ describeSpec(
         return steps;
       }
     );
+
+    specTest('Process 25 target updates and wait for snapshot', [], () => {
+      const queriesPerStep = 25;
+
+      let currentVersion = 1;
+      let steps = spec().withGCEnabled(false);
+
+      for (let i = 1; i <= STEP_COUNT; ++i) {
+        // We use a different subcollection for each iteration to ensure
+        // that we use distinct and non-overlapping collection queries.
+        const collPath = `collection/${i}/coll`;
+        const matchingDoc = doc(`${collPath}/matches`, ++currentVersion, {
+          val: -1
+        });
+
+        const queries = [];
+
+        // Create `queriesPerStep` listens, each against collPath but with a
+        // unique query constraint.
+        for (let j = 0; j < queriesPerStep; ++j) {
+          const query = Query.atPath(path(collPath)).addFilter(
+            filter('val', '<=', j)
+          );
+          queries.push(query);
+          steps = steps.userListens(query).watchAcks(query);
+        }
+
+        steps = steps
+          .watchSends({ affects: queries }, matchingDoc)
+          .watchSnapshots(++currentVersion);
+
+        // Registers the snapshot expectations with the spec runner.
+        for (const query of queries) {
+          steps = steps.expectEvents(query, {
+            added: [matchingDoc],
+            fromCache: true
+          });
+        }
+
+        // Unlisten and clean up the query.
+        for (const query of queries) {
+          steps = steps.userUnlistens(query).watchRemoves(query);
+        }
+      }
+
+      return steps;
+    });
   }
 );

packages/firestore/test/util/api_helpers.ts

@@ -46,15 +46,16 @@ export const FIRESTORE = new Firestore({
 });
 
 export function firestore(): Firestore {
+  FIRESTORE.settings({ timestampsInSnapshots: true });
   return FIRESTORE;
 }
 
 export function collectionReference(path: string): CollectionReference {
-  return new CollectionReference(pathFrom(path), FIRESTORE);
+  return new CollectionReference(pathFrom(path), firestore());
 }
 
 export function documentReference(path: string): DocumentReference {
-  return new DocumentReference(key(path), FIRESTORE);
+  return new DocumentReference(key(path), firestore());
 }
 
 export function documentSnapshot(
@@ -64,18 +65,18 @@ export function documentSnapshot(
 ): DocumentSnapshot {
   if (data) {
     return new DocumentSnapshot(
-      FIRESTORE,
+      firestore(),
       key(path),
       doc(path, 1, data),
       fromCache
     );
   } else {
-    return new DocumentSnapshot(FIRESTORE, key(path), null, fromCache);
+    return new DocumentSnapshot(firestore(), key(path), null, fromCache);
   }
 }
 
 export function query(path: string): Query {
-  return new Query(InternalQuery.atPath(pathFrom(path)), FIRESTORE);
+  return new Query(InternalQuery.atPath(pathFrom(path)), firestore());
 }
 
 /**
@@ -121,5 +122,5 @@ export function querySnapshot(
     syncStateChanged,
     /* excludesMetadataChanges= */ false
   );
-  return new QuerySnapshot(FIRESTORE, query, viewSnapshot);
+  return new QuerySnapshot(firestore(), query, viewSnapshot);
 }

packages/firestore/test/util/helpers.ts

@@ -30,7 +30,6 @@ import {
   Direction,
   Filter,
   OrderBy,
-  Query,
   RelationOp
 } from '../../src/core/query';
 import { SnapshotVersion } from '../../src/core/snapshot_version';
@@ -375,7 +374,7 @@ export function limboChanges(changes: {
 }
 
 export function localViewChanges(
-  query: Query,
+  targetId: TargetId,
   changes: { added?: string[]; removed?: string[] }
 ): LocalViewChanges {
   if (!changes.added) changes.added = [];
@@ -390,7 +389,7 @@ export function localViewChanges(
     keyStr => (removedKeys = removedKeys.add(key(keyStr)))
   );
 
-  return new LocalViewChanges(query, addedKeys, removedKeys);
+  return new LocalViewChanges(targetId, addedKeys, removedKeys);
 }
 
 /** Creates a resume token to match the given snapshot version. */