Use collectionGroupReadTimeIndex for multi-tab synchronization (#6073)

Author: schmidt-sebastian

packages/firestore/src/core/bundle.ts

@@ -43,11 +43,10 @@ export interface BundledDocument {
  */
 export type BundledDocuments = BundledDocument[];
 
-export class BundleLoadResult {
-  constructor(
-    readonly progress: LoadBundleTaskProgress,
-    readonly changedDocs: DocumentMap
-  ) {}
+export interface BundleLoadResult {
+  readonly progress: LoadBundleTaskProgress;
+  readonly changedCollectionGroups: Set<string>;
+  readonly changedDocs: DocumentMap;
 }
 
 /**

packages/firestore/src/core/bundle_impl.ts

@@ -25,6 +25,7 @@ import {
 import { documentKeySet, DocumentKeySet } from '../model/collections';
 import { MutableDocument } from '../model/document';
 import { DocumentKey } from '../model/document_key';
+import { ResourcePath } from '../model/path';
 import {
   BundleMetadata as ProtoBundleMetadata,
   NamedQuery as ProtoNamedQuery
@@ -91,6 +92,8 @@ export class BundleLoader {
   private queries: ProtoNamedQuery[] = [];
   /** Batched documents to be saved into storage */
   private documents: BundledDocuments = [];
+  /** The collection groups affected by this bundle. */
+  private collectionGroups = new Set<string>();
 
   constructor(
     private bundleMetadata: ProtoBundleMetadata,
@@ -120,6 +123,14 @@ export class BundleLoader {
       if (!element.payload.documentMetadata.exists) {
         ++documentsLoaded;
       }
+      const path = ResourcePath.fromString(
+        element.payload.documentMetadata.name!
+      );
+      debugAssert(
+        path.length >= 2,
+        'The document name does not point to a document.'
+      );
+      this.collectionGroups.add(path.get(path.length - 2));
     } else if (element.payload.document) {
       debugAssert(
         this.documents.length > 0 &&
@@ -173,7 +184,7 @@ export class BundleLoader {
     );
     debugAssert(!!this.bundleMetadata.id, 'Bundle ID must be set.');
 
-    const changedDocuments = await localStoreApplyBundledDocuments(
+    const changedDocs = await localStoreApplyBundledDocuments(
       this.localStore,
       new BundleConverterImpl(this.serializer),
       this.documents,
@@ -191,7 +202,11 @@ export class BundleLoader {
     }
 
     this.progress.taskState = 'Success';
-    return new BundleLoadResult({ ...this.progress }, changedDocuments);
+    return {
+      progress: this.progress,
+      changedCollectionGroups: this.collectionGroups,
+      changedDocs
+    };
   }
 }
 

packages/firestore/src/core/component_provider.ts

@@ -22,10 +22,7 @@ import {
   IndexedDbPersistence
 } from '../local/indexeddb_persistence';
 import { LocalStore } from '../local/local_store';
-import {
-  newLocalStore,
-  localStoreSynchronizeLastDocumentChangeReadTime
-} from '../local/local_store_impl';
+import { newLocalStore } from '../local/local_store_impl';
 import { LruParams } from '../local/lru_garbage_collector';
 import { LruScheduler } from '../local/lru_garbage_collector_impl';
 import {
@@ -174,7 +171,6 @@ export class IndexedDbOfflineComponentProvider extends MemoryOfflineComponentPro
 
   async initialize(cfg: ComponentConfiguration): Promise<void> {
     await super.initialize(cfg);
-    await localStoreSynchronizeLastDocumentChangeReadTime(this.localStore);
 
     await this.onlineComponentProvider.initialize(this, cfg);
 

packages/firestore/src/core/query.ts

@@ -526,6 +526,21 @@ function queryMatchesBounds(query: Query, doc: Document): boolean {
   return true;
 }
 
+/**
+ * Returns the collection group that this query targets.
+ *
+ * PORTING NOTE: This is only used in the Web SDK to facilitate multi-tab
+ * synchronization for query results.
+ */
+export function queryCollectionGroup(query: Query): string {
+  return (
+    query.collectionGroup ||
+    (query.path.length % 2 === 1
+      ? query.path.lastSegment()
+      : query.path.get(query.path.length - 2))
+  );
+}
+
 /**
  * Returns a new comparator function that can be used to compare two documents
  * based on the Query's ordering constraint.

packages/firestore/src/core/sync_engine_impl.ts

@@ -92,6 +92,7 @@ import {
   newQueryForPath,
   Query,
   queryEquals,
+  queryCollectionGroup,
   queryToTarget,
   stringifyQuery
 } from './query';
@@ -1171,13 +1172,16 @@ async function synchronizeViewAndComputeSnapshot(
  */
 // PORTING NOTE: Multi-Tab only.
 export async function syncEngineSynchronizeWithChangedDocuments(
-  syncEngine: SyncEngine
+  syncEngine: SyncEngine,
+  collectionGroup: string
 ): Promise<void> {
   const syncEngineImpl = debugCast(syncEngine, SyncEngineImpl);
 
-  return localStoreGetNewDocumentChanges(syncEngineImpl.localStore).then(
-    changes =>
-      syncEngineEmitNewSnapsAndNotifyLocalStore(syncEngineImpl, changes)
+  return localStoreGetNewDocumentChanges(
+    syncEngineImpl.localStore,
+    collectionGroup
+  ).then(changes =>
+    syncEngineEmitNewSnapsAndNotifyLocalStore(syncEngineImpl, changes)
   );
 }
 
@@ -1432,12 +1436,14 @@ export async function syncEngineApplyTargetState(
     return;
   }
 
-  if (syncEngineImpl.queriesByTarget.has(targetId)) {
+  const query = syncEngineImpl.queriesByTarget.get(targetId);
+  if (query && query.length > 0) {
     switch (state) {
       case 'current':
       case 'not-current': {
         const changes = await localStoreGetNewDocumentChanges(
-          syncEngineImpl.localStore
+          syncEngineImpl.localStore,
+          queryCollectionGroup(query[0])
         );
         const synthesizedRemoteEvent =
           RemoteEvent.createSynthesizedRemoteEventForCurrentChange(
@@ -1565,16 +1571,17 @@ export function syncEngineLoadBundle(
   const syncEngineImpl = debugCast(syncEngine, SyncEngineImpl);
 
   // eslint-disable-next-line @typescript-eslint/no-floating-promises
-  loadBundleImpl(syncEngineImpl, bundleReader, task).then(() => {
-    syncEngineImpl.sharedClientState.notifyBundleLoaded();
+  loadBundleImpl(syncEngineImpl, bundleReader, task).then(collectionGroups => {
+    syncEngineImpl.sharedClientState.notifyBundleLoaded(collectionGroups);
   });
 }
 
+/** Loads a bundle and returns the list of affected collection groups. */
 async function loadBundleImpl(
   syncEngine: SyncEngineImpl,
   reader: BundleReader,
   task: LoadBundleTask
-): Promise<void> {
+): Promise<Set<string>> {
   try {
     const metadata = await reader.getMetadata();
     const skip = await localStoreHasNewerBundle(
@@ -1584,7 +1591,7 @@ async function loadBundleImpl(
     if (skip) {
       await reader.close();
       task._completeWith(bundleSuccessProgress(metadata));
-      return;
+      return Promise.resolve(new Set<string>());
     }
 
     task._updateProgress(bundleInitialProgress(metadata));
@@ -1609,9 +1616,6 @@ async function loadBundleImpl(
     }
 
     const result = await loader.complete();
-    // TODO(b/160876443): This currently raises snapshots with
-    // `fromCache=false` if users already listen to some queries and bundles
-    // has newer version.
     await syncEngineEmitNewSnapsAndNotifyLocalStore(
       syncEngine,
       result.changedDocs,
@@ -1621,8 +1625,10 @@ async function loadBundleImpl(
     // Save metadata, so loading the same bundle will skip.
     await localStoreSaveBundle(syncEngine.localStore, metadata);
     task._completeWith(result.progress);
+    return Promise.resolve(result.changedCollectionGroups);
   } catch (e) {
     logWarn(LOG_TAG, `Loading bundle failed with ${e}`);
     task._failWith(e);
+    return Promise.resolve(new Set<string>());
   }
 }

packages/firestore/src/local/indexeddb_remote_document_cache.ts

@@ -26,7 +26,7 @@ import { MutableDocument } from '../model/document';
 import { DocumentKey } from '../model/document_key';
 import { IndexOffset } from '../model/field_index';
 import { ResourcePath } from '../model/path';
-import { debugAssert, debugCast, hardAssert } from '../util/assert';
+import { debugAssert, hardAssert } from '../util/assert';
 import { primitiveComparator } from '../util/misc';
 import { ObjectMap } from '../util/obj_map';
 import { SortedMap } from '../util/sorted_map';
@@ -41,14 +41,12 @@ import {
   DbRemoteDocumentGlobalKey,
   DbRemoteDocumentGlobalStore,
   DbRemoteDocumentKey,
-  DbRemoteDocumentReadTimeIndex,
   DbRemoteDocumentStore,
   DbTimestampKey
 } from './indexeddb_sentinels';
 import { getStore } from './indexeddb_transaction';
 import {
   fromDbRemoteDocument,
-  fromDbTimestampKey,
   LocalSerializer,
   toDbRemoteDocument,
   toDbTimestampKey
@@ -411,77 +409,6 @@ export function newIndexedDbRemoteDocumentCache(
   return new IndexedDbRemoteDocumentCacheImpl(serializer);
 }
 
-/**
- * Returns the set of documents that have changed since the specified read
- * time.
- */
-// PORTING NOTE: This is only used for multi-tab synchronization.
-export function remoteDocumentCacheGetNewDocumentChanges(
-  remoteDocumentCache: IndexedDbRemoteDocumentCache,
-  transaction: PersistenceTransaction,
-  sinceReadTime: SnapshotVersion
-): PersistencePromise<{
-  changedDocs: MutableDocumentMap;
-  readTime: SnapshotVersion;
-}> {
-  const remoteDocumentCacheImpl = debugCast(
-    remoteDocumentCache,
-    IndexedDbRemoteDocumentCacheImpl // We only support IndexedDb in multi-tab mode.
-  );
-  let changedDocs = mutableDocumentMap();
-
-  let lastReadTime = toDbTimestampKey(sinceReadTime);
-
-  const documentsStore = remoteDocumentsStore(transaction);
-  const range = IDBKeyRange.lowerBound(lastReadTime, true);
-  return documentsStore
-    .iterate(
-      { index: DbRemoteDocumentReadTimeIndex, range },
-      (_, dbRemoteDoc) => {
-        // Unlike `getEntry()` and others, `getNewDocumentChanges()` parses
-        // the documents directly since we want to keep sentinel deletes.
-        const doc = fromDbRemoteDocument(
-          remoteDocumentCacheImpl.serializer,
-          dbRemoteDoc
-        );
-        changedDocs = changedDocs.insert(doc.key, doc);
-        lastReadTime = dbRemoteDoc.readTime!;
-      }
-    )
-    .next(() => {
-      return {
-        changedDocs,
-        readTime: fromDbTimestampKey(lastReadTime)
-      };
-    });
-}
-
-/**
- * Returns the read time of the most recently read document in the cache, or
- * SnapshotVersion.min() if not available.
- */
-// PORTING NOTE: This is only used for multi-tab synchronization.
-export function remoteDocumentCacheGetLastReadTime(
-  transaction: PersistenceTransaction
-): PersistencePromise<SnapshotVersion> {
-  const documentsStore = remoteDocumentsStore(transaction);
-
-  // If there are no existing entries, we return SnapshotVersion.min().
-  let readTime = SnapshotVersion.min();
-
-  return documentsStore
-    .iterate(
-      { index: DbRemoteDocumentReadTimeIndex, reverse: true },
-      (key, dbRemoteDoc, control) => {
-        if (dbRemoteDoc.readTime) {
-          readTime = fromDbTimestampKey(dbRemoteDoc.readTime);
-        }
-        control.done();
-      }
-    )
-    .next(() => readTime);
-}
-
 /**
  * Handles the details of adding and updating documents in the IndexedDbRemoteDocumentCache.
  *

packages/firestore/src/local/indexeddb_schema_converter.ts

@@ -93,8 +93,6 @@ import {
   DbRemoteDocumentGlobalStore,
   DbRemoteDocumentKey,
   DbRemoteDocumentKeyPath,
-  DbRemoteDocumentReadTimeIndex,
-  DbRemoteDocumentReadTimeIndexPath,
   DbRemoteDocumentStore,
   DbTargetDocumentDocumentTargetsIndex,
   DbTargetDocumentDocumentTargetsKeyPath,
@@ -532,11 +530,6 @@ function createRemoteDocumentCache(db: IDBDatabase): void {
     DbRemoteDocumentDocumentKeyIndex,
     DbRemoteDocumentDocumentKeyIndexPath
   );
-  remoteDocumentStore.createIndex(
-    DbRemoteDocumentReadTimeIndex,
-    DbRemoteDocumentReadTimeIndexPath,
-    { unique: false }
-  );
   remoteDocumentStore.createIndex(
     DbRemoteDocumentCollectionGroupIndex,
     DbRemoteDocumentCollectionGroupIndexPath

packages/firestore/src/local/indexeddb_sentinels.ts

@@ -148,17 +148,6 @@ export const DbRemoteDocumentKeyPath = [
   'documentId'
 ];
 
-/**
- * An index that provides access to all entries sorted by read time (which
- * corresponds to the last modification time of each row).
- *
- * This index is used to provide a changelog for Multi-Tab.
- */
-export const DbRemoteDocumentReadTimeIndex = 'readTimeIndex';
-
-// TODO(indexing): Consider re-working Multi-Tab to use the collectionGroupIndex
-export const DbRemoteDocumentReadTimeIndexPath = 'readTime';
-
 /** An index that provides access to documents by key. */
 export const DbRemoteDocumentDocumentKeyIndex = 'documentKeyIndex';