Refactor with type OverlayedDocumentMap. Add Comments. Fix nit.

Author: tom-andersen

packages/firestore/src/local/index_backfiller.ts

@@ -109,6 +109,13 @@ export class IndexBackfillerScheduler implements Scheduler {
 /** Implements the steps for backfilling indexes. */
 export class IndexBackfiller {
   constructor(
+    /**
+     * LocalStore provides access to IndexManager and LocalDocumentView.
+     * These properties will update when the user changes. Consequently,
+     * making a local copy of IndexManager and LocalDocumentView will require
+     * updates over time. The simpler solution is to rely on LocalStore to have
+     * an up-to-date references to IndexManager and LocalDocumentStore.
+     */
     private readonly localStore: LocalStore,
     private readonly persistence: Persistence
   ) {}

packages/firestore/src/local/indexeddb_index_manager.ts

@@ -983,7 +983,7 @@ export class IndexedDbIndexManager implements IndexManager {
       this.getSubTargets(target),
       (subTarget: Target) =>
         this.getFieldIndex(transaction, subTarget).next(index =>
-          !index ? fail('Target cannot be served from index') : index
+          index ? index : fail('Target cannot be served from index')
         )
     ).next(getMinOffsetFromFieldIndexes);
   }

packages/firestore/src/local/local_documents_view.ts

@@ -35,7 +35,9 @@ import {
   mutableDocumentMap,
   documentKeySet,
   DocumentKeyMap,
-  convertDocumentKeyMapWithOverlayedDocumentToDocumentMap
+  convertOverlayedDocumentMapToDocumentMap,
+  OverlayedDocumentMap,
+  newOverlayedDocumentMap
 } from '../model/collections';
 import { Document, MutableDocument } from '../model/document';
 import { DocumentKey } from '../model/document_key';
@@ -167,7 +169,7 @@ export class LocalDocumentsView {
   getOverlayedDocuments(
     transaction: PersistenceTransaction,
     docs: MutableDocumentMap
-  ): PersistencePromise<DocumentKeyMap<OverlayedDocument>> {
+  ): PersistencePromise<OverlayedDocumentMap> {
     const overlays = newOverlayMap();
     return this.populateOverlays(transaction, overlays, docs).next(() =>
       this.computeViews(transaction, docs, overlays, documentKeySet())
@@ -215,10 +217,10 @@ export class LocalDocumentsView {
     docs: MutableDocumentMap,
     overlays: OverlayMap,
     existenceStateChanged: DocumentKeySet
-  ): PersistencePromise<DocumentKeyMap<OverlayedDocument>> {
+  ): PersistencePromise<OverlayedDocumentMap> {
     let recalculateDocuments = mutableDocumentMap();
     const mutatedFields = newDocumentKeyMap<FieldMask | null>();
-    const results = newDocumentKeyMap<OverlayedDocument>();
+    const results = newOverlayedDocumentMap();
     docs.forEach((_, doc) => {
       const overlay = overlays.get(doc.key);
       // Recalculate an overlay if the document's existence state changed due to
@@ -442,10 +444,7 @@ export class LocalDocumentsView {
             )
             .next(localDocs => ({
               batchId: largestBatchId,
-              changes:
-                convertDocumentKeyMapWithOverlayedDocumentToDocumentMap(
-                  localDocs
-                )
+              changes: convertOverlayedDocumentMapToDocumentMap(localDocs)
             }));
         });
       });

packages/firestore/src/local/local_store_impl.ts

@@ -28,13 +28,13 @@ import { canonifyTarget, Target, targetEquals } from '../core/target';
 import { BatchId, TargetId } from '../core/types';
 import { Timestamp } from '../lite-api/timestamp';
 import {
-  convertDocumentKeyMapWithOverlayedDocumentToDocumentMap,
-  DocumentKeyMap,
+  convertOverlayedDocumentMapToDocumentMap,
   documentKeySet,
   DocumentKeySet,
   DocumentMap,
   mutableDocumentMap,
-  MutableDocumentMap
+  MutableDocumentMap,
+  OverlayedDocumentMap
 } from '../model/collections';
 import { Document } from '../model/document';
 import { DocumentKey } from '../model/document_key';
@@ -77,7 +77,6 @@ import { LocalStore } from './local_store';
 import { LocalViewChanges } from './local_view_changes';
 import { LruGarbageCollector, LruResults } from './lru_garbage_collector';
 import { MutationQueue } from './mutation_queue';
-import { OverlayedDocument } from './overlayed_document';
 import { Persistence } from './persistence';
 import { PersistencePromise } from './persistence_promise';
 import { PersistenceTransaction } from './persistence_transaction';
@@ -318,7 +317,7 @@ export function localStoreWriteLocally(
   const localWriteTime = Timestamp.now();
   const keys = mutations.reduce((keys, m) => keys.add(m.key), documentKeySet());
 
-  let overlayedDocuments: DocumentKeyMap<OverlayedDocument>;
+  let overlayedDocuments: OverlayedDocumentMap;
   let mutationBatch: MutationBatch;
 
   return localStoreImpl.persistence
@@ -350,7 +349,7 @@ export function localStoreWriteLocally(
             remoteDocs
           );
         })
-        .next((docs: DocumentKeyMap<OverlayedDocument>) => {
+        .next((docs: OverlayedDocumentMap) => {
           overlayedDocuments = docs;
 
           // For non-idempotent mutations (such as `FieldValue.increment()`),
@@ -402,10 +401,7 @@ export function localStoreWriteLocally(
     })
     .then(() => ({
       batchId: mutationBatch.batchId,
-      changes:
-        convertDocumentKeyMapWithOverlayedDocumentToDocumentMap(
-          overlayedDocuments
-        )
+      changes: convertOverlayedDocumentMapToDocumentMap(overlayedDocuments)
     }));
 }
 

packages/firestore/src/model/collections.ts

@@ -55,8 +55,13 @@ export function documentMap(...docs: Document[]): DocumentMap {
   return map;
 }
 
-export function convertDocumentKeyMapWithOverlayedDocumentToDocumentMap(
-  collection: DocumentKeyMap<OverlayedDocument>
+export type OverlayedDocumentMap = DocumentKeyMap<OverlayedDocument>;
+export function newOverlayedDocumentMap(): OverlayedDocumentMap {
+  return newDocumentKeyMap<OverlayedDocument>();
+}
+
+export function convertOverlayedDocumentMapToDocumentMap(
+  collection: OverlayedDocumentMap
 ): DocumentMap {
   let documents = EMPTY_DOCUMENT_MAP;
   collection.forEach(

packages/firestore/src/model/mutation_batch.ts

@@ -18,7 +18,6 @@
 import { SnapshotVersion } from '../core/snapshot_version';
 import { BatchId } from '../core/types';
 import { Timestamp } from '../lite-api/timestamp';
-import { OverlayedDocument } from '../local/overlayed_document';
 import { debugAssert, hardAssert } from '../util/assert';
 import { arrayEquals } from '../util/misc';
 
@@ -29,7 +28,7 @@ import {
   DocumentVersionMap,
   documentVersionMap,
   newMutationMap,
-  DocumentKeyMap
+  OverlayedDocumentMap
 } from './collections';
 import { MutableDocument } from './document';
 import { FieldMask } from './field_mask';
@@ -140,7 +139,7 @@ export class MutationBatch {
    * replace all the mutation applications.
    */
   applyToLocalDocumentSet(
-    documentMap: DocumentKeyMap<OverlayedDocument>,
+    documentMap: OverlayedDocumentMap,
     documentsWithoutRemoteVersion: DocumentKeySet
   ): MutationMap {
     // TODO(mrschmidt): This implementation is O(n^2). If we apply the mutations