Simplify getLastDocumentChange (#2566)

Author: schmidt-sebastian

packages/firestore/src/local/indexeddb_remote_document_cache.ts

@@ -291,11 +291,6 @@ export class IndexedDbRemoteDocumentCache implements RemoteDocumentCache {
       .next(() => results);
   }
 
-  /**
-   * Returns the set of documents that have been updated since the specified read
-   * time.
-   */
-  // PORTING NOTE: This is only used for multi-tab synchronization.
   getNewDocumentChanges(
     transaction: PersistenceTransaction,
     sinceReadTime: SnapshotVersion
@@ -328,37 +323,25 @@ export class IndexedDbRemoteDocumentCache implements RemoteDocumentCache {
       });
   }
 
-  /**
-   * Returns the last document that has changed, as well as the read time of the
-   * last change. If no document has changed, returns SnapshotVersion.MIN.
-   */
-  // PORTING NOTE: This is only used for multi-tab synchronization.
-  getLastDocumentChange(
+  getLastReadTime(
     transaction: PersistenceTransaction
-  ): PersistencePromise<{
-    changedDoc: MaybeDocument | undefined;
-    readTime: SnapshotVersion;
-  }> {
+  ): PersistencePromise<SnapshotVersion> {
     const documentsStore = remoteDocumentsStore(transaction);
 
     // If there are no existing entries, we return SnapshotVersion.MIN.
     let readTime = SnapshotVersion.MIN;
-    let changedDoc: MaybeDocument | undefined;
 
     return documentsStore
       .iterate(
         { index: DbRemoteDocument.readTimeIndex, reverse: true },
         (key, dbRemoteDoc, control) => {
-          changedDoc = this.serializer.fromDbRemoteDocument(dbRemoteDoc);
           if (dbRemoteDoc.readTime) {
             readTime = this.serializer.fromDbTimestampKey(dbRemoteDoc.readTime);
           }
           control.done();
         }
       )
-      .next(() => {
-        return { changedDoc, readTime };
-      });
+      .next(() => readTime);
   }
 
   newChangeBuffer(options?: {

packages/firestore/src/local/local_store.ts

@@ -1112,18 +1112,11 @@ export class LocalStore {
    */
   // PORTING NOTE: Multi-tab only.
   async synchronizeLastDocumentChangeReadTime(): Promise<void> {
-    if (this.remoteDocuments instanceof IndexedDbRemoteDocumentCache) {
-      const remoteDocumentCache = this.remoteDocuments;
-      return this.persistence
-        .runTransaction(
-          'Synchronize last document change read time',
-          'readonly-idempotent',
-          txn => remoteDocumentCache.getLastDocumentChange(txn)
-        )
-        .then(({ readTime }) => {
-          this.lastDocumentChangeReadTime = readTime;
-        });
-    }
+    this.lastDocumentChangeReadTime = await this.persistence.runTransaction(
+      'Synchronize last document change read time',
+      'readonly-idempotent',
+      txn => this.remoteDocuments.getLastReadTime(txn)
+    );
   }
 }
 

packages/firestore/src/local/memory_remote_document_cache.ts

@@ -188,6 +188,12 @@ export class MemoryRemoteDocumentCache implements RemoteDocumentCache {
     );
   }
 
+  getLastReadTime(
+    transaction: PersistenceTransaction
+  ): PersistencePromise<SnapshotVersion> {
+    return PersistencePromise.resolve(SnapshotVersion.MIN);
+  }
+
   newChangeBuffer(options?: {
     trackRemovals: boolean;
   }): RemoteDocumentChangeBuffer {

packages/firestore/src/local/remote_document_cache.ts

@@ -95,6 +95,15 @@ export interface RemoteDocumentCache {
     readTime: SnapshotVersion;
   }>;
 
+  /**
+   * 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.
+  getLastReadTime(
+    transaction: PersistenceTransaction
+  ): PersistencePromise<SnapshotVersion>;
+
   /**
    * Provides access to add or update the contents of the cache. The buffer
    * handles proper size accounting for the change.

packages/firestore/test/integration/api_internal/database.test.ts

@@ -68,7 +68,7 @@ apiDescribe('Database (with internal API)', (persistence: boolean) => {
       }
     );
   });
-  
+
   it('app delete leads to instance termination', async () => {
     await withTestDoc(persistence, async docRef => {
       await docRef.set({ foo: 'bar' });

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

@@ -115,6 +115,7 @@ export class CountingQueryEngine implements QueryEngine {
         });
       },
       getNewDocumentChanges: subject.getNewDocumentChanges,
+      getLastReadTime: subject.getLastReadTime,
       getSize: subject.getSize,
       newChangeBuffer: subject.newChangeBuffer
     };

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

@@ -90,16 +90,13 @@ describe('IndexedDbRemoteDocumentCache', () => {
     await persistenceHelpers.clearTestPersistence();
   });
 
-  function getLastDocumentChange(): Promise<{
-    changedDoc: MaybeDocument | undefined;
-    readTime: SnapshotVersion;
-  }> {
+  function getLastReadTime(): Promise<SnapshotVersion> {
     return persistence.runTransaction(
-      'getLastDocumentChange',
+      'getLastReadTime',
       'readonly-idempotent',
       txn => {
         const remoteDocuments = persistence.getRemoteDocumentCache();
-        return remoteDocuments.getLastDocumentChange(txn);
+        return remoteDocuments.getLastReadTime(txn);
       }
     );
   }
@@ -115,7 +112,7 @@ describe('IndexedDbRemoteDocumentCache', () => {
       dontPurgeData: true
     });
     cache = new TestRemoteDocumentCache(persistence);
-    const { readTime } = await getLastDocumentChange();
+    const readTime = await getLastReadTime();
     const { changedDocs } = await cache.getNewDocumentChanges(readTime);
     assertMatches([], changedDocs);
   });