Address comments.

Author: ehsannas

packages/firestore/src/local/document_overlay_cache.ts

@@ -53,7 +53,7 @@ export interface DocumentOverlayCache {
     overlays: Map<DocumentKey, Mutation>
   ): PersistencePromise<void>;
 
-  /** Removes the overlay whose largest-batch-id equals the given Id. */
+  /** Removes the overlay whose largest-batch-id equals the given ID. */
   removeOverlaysForBatchId(
     transaction: PersistenceTransaction,
     batchId: number
@@ -62,9 +62,9 @@ export interface DocumentOverlayCache {
   /**
    * Returns all saved overlays for the given collection.
    *
-   * @param transaction The persistence transaction to use for this operation.
-   * @param collection The collection path to get the overlays for.
-   * @param sinceBatchId The minimum batch ID to filter by (exclusive).
+   * @param transaction - The persistence transaction to use for this operation.
+   * @param collection - The collection path to get the overlays for.
+   * @param sinceBatchId - The minimum batch ID to filter by (exclusive).
    * Only overlays that contain a change past `sinceBatchId` are returned.
    * @returns Mapping of each document key in the collection to its overlay.
    */
@@ -80,11 +80,11 @@ export interface DocumentOverlayCache {
    * always returns all overlays for a batch even if the last batch contains
    * more documents than the remaining limit.
    *
-   * @param transaction The persistence transaction used for this operation.
-   * @param collectionGroup The collection group to get the overlays for.
-   * @param sinceBatchId The minimum batch ID to filter by (exclusive).
+   * @param transaction - The persistence transaction used for this operation.
+   * @param collectionGroup - The collection group to get the overlays for.
+   * @param sinceBatchId - The minimum batch ID to filter by (exclusive).
    * Only overlays that contain a change past `sinceBatchId` are returned.
-   * @param count The number of overlays to return. Can be exceeded if the last
+   * @param count - The number of overlays to return. Can be exceeded if the last
    * batch contains more entries.
    * @return Mapping of each document key in the collection group to its overlay.
    */

packages/firestore/src/local/indexeddb_document_overlay_cache.ts

@@ -36,7 +36,7 @@ import { PersistenceTransaction } from './persistence_transaction';
 import { SimpleDbStore } from './simple_db';
 
 /**
- * An in-memory implementation of DocumentOverlayCache.
+ * Implementation of DocumentOverlayCache using IndexedDb.
  */
 export class IndexedDbDocumentOverlayCache implements DocumentOverlayCache {
   /**

packages/firestore/src/local/local_serializer.ts

@@ -426,7 +426,10 @@ export function toDbDocumentOverlay(
   );
 }
 
-/** Returns the DbDocumentOverlayKey corresponding to the given user and document key. */
+/**
+ * Returns the DbDocumentOverlayKey corresponding to the given user and
+ * document key.
+ */
 export function toDbDocumentOverlayKey(
   userId: string,
   docKey: DocumentKey

packages/firestore/src/local/memory_document_overlay_cache.ts

@@ -111,7 +111,6 @@ export class MemoryDocumentOverlayCache implements DocumentOverlayCache {
       const entry = iter.getNext();
       const overlay = entry.value;
       const key = overlay.getKey();
-      // TODO(overlays): We should consider using the IndexManager to get the list of collection paths first.
       if (key.getCollectionGroup() !== collectionGroup) {
         continue;
       }
@@ -162,9 +161,11 @@ export class MemoryDocumentOverlayCache implements DocumentOverlayCache {
     );
 
     // Create the association of this overlay to the given largestBatchId.
-    if (this.overlayByBatchId.get(largestBatchId) === undefined) {
-      this.overlayByBatchId.set(largestBatchId, new Set<DocumentKey>());
+    let batch = this.overlayByBatchId.get(largestBatchId);
+    if (batch === undefined) {
+      batch = new Set<DocumentKey>();
+      this.overlayByBatchId.set(largestBatchId, batch);
     }
-    this.overlayByBatchId.get(largestBatchId)!.add(mutation.key);
+    batch.add(mutation.key);
   }
 }

packages/firestore/src/model/overlay.ts

@@ -18,6 +18,12 @@
 import { DocumentKey } from './document_key';
 import { Mutation } from './mutation';
 
+/**
+ * Representation of an overlay computed by Firestore.
+ *
+ * Holds information about a mutation and the largest batch id in Firestore when
+ * the mutation was created.
+ */
 export class Overlay {
   constructor(readonly largestBatchId: number, readonly mutation: Mutation) {}
 

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

@@ -18,7 +18,7 @@
 import { expect } from 'chai';
 
 import { DatabaseId } from '../../../src/core/database_info';
-import { decodeResourcePath } from '../../../src/local/encoded_resource_path';
+import { encodeResourcePath } from '../../../src/local/encoded_resource_path';
 import { DbMutationBatch } from '../../../src/local/indexeddb_schema';
 import {
   fromDbDocumentOverlay,
@@ -31,9 +31,9 @@ import {
   SetMutation
 } from '../../../src/model/mutation';
 import { Overlay } from '../../../src/model/overlay';
+import { ResourcePath } from '../../../src/model/path';
 import { Write } from '../../../src/protos/firestore_proto_api';
 import {
-  fromMutation,
   JsonProtoSerializer,
   toDocumentMask,
   toMutation,
@@ -196,16 +196,16 @@ describe('Local Serializer', () => {
     const overlay = new Overlay(2, m);
 
     const serialized = toDbDocumentOverlay(localSerializer, userId, overlay);
-    expect(serialized.userId).to.equal(userId);
-    expect(serialized.largestBatchId).to.equal(2);
-    expect(serialized.documentId).to.equal('doc2');
-    expect(
-      decodeResourcePath(serialized.collectionPath).canonicalString()
-    ).to.equal('coll1/doc1/coll2');
-    expect(serialized.collectionGroup).to.equal('coll2');
-    expect(
-      mutationEquals(fromMutation(s, serialized.overlayMutation), m)
-    ).to.equal(true);
+    expect(serialized).to.deep.equal({
+      userId,
+      collectionPath: encodeResourcePath(
+        ResourcePath.fromString('coll1/doc1/coll2')
+      ),
+      documentId: 'doc2',
+      collectionGroup: 'coll2',
+      largestBatchId: 2,
+      overlayMutation: toMutation(localSerializer.remoteSerializer, m)
+    });
 
     const roundTripped = fromDbDocumentOverlay(localSerializer, serialized);
     expect(roundTripped.largestBatchId).to.equal(overlay.largestBatchId);