Local docs, path

Author: var-const

packages/firestore/src/local/indexeddb_mutation_queue.ts

@@ -373,7 +373,7 @@ export class IndexedDbMutationQueue implements MutationQueue {
     );
     const maxKey = DbDocumentMutation.prefixForPath(
       this.userId,
-      documentKeys.first().path
+      documentKeys.last().path
     );
     const keyRange = IDBKeyRange.bound(minKey, maxKey);
     let uniqueBatchIDs = new SortedSet<BatchId>(primitiveComparator);

packages/firestore/src/local/local_documents_view.ts

@@ -26,6 +26,7 @@ import {
 } from '../model/collections';
 import { Document, MaybeDocument, NoDocument } from '../model/document';
 import { DocumentKey } from '../model/document_key';
+import { MutationBatch } from '../model/mutation_batch';
 import { ResourcePath } from '../model/path';
 import { fail } from '../util/assert';
 
@@ -55,11 +56,25 @@ export class LocalDocumentsView {
   getDocument(
     transaction: PersistenceTransaction,
     key: DocumentKey
+  ): PersistencePromise<MaybeDocument | null> {
+    return this.mutationQueue
+      .getAllMutationBatchesAffectingDocumentKey(transaction, key)
+      .next(batches => this.getDocumentInBatches(transaction, key, batches));
+  }
+
+  /** Internal version of `getDocument` that allows reusing batches. */
+  private getDocumentInBatches(
+    transaction: PersistenceTransaction,
+    key: DocumentKey,
+    inBatches: MutationBatch[]
   ): PersistencePromise<MaybeDocument | null> {
     return this.remoteDocumentCache
       .getEntry(transaction, key)
-      .next(remoteDoc => {
-        return this.computeLocalDocument(transaction, key, remoteDoc);
+      .next(doc => {
+        for (const batch of inBatches) {
+          doc = batch.applyToLocalView(key, doc);
+        }
+        return doc;
       });
   }
 
@@ -73,20 +88,23 @@ export class LocalDocumentsView {
     transaction: PersistenceTransaction,
     keys: DocumentKeySet
   ): PersistencePromise<MaybeDocumentMap> {
-    const promises = [] as Array<PersistencePromise<void>>;
-    let results = maybeDocumentMap();
-    keys.forEach(key => {
-      promises.push(
-        this.getDocument(transaction, key).next(maybeDoc => {
-          // TODO(http://b/32275378): Don't conflate missing / deleted.
-          if (!maybeDoc) {
-            maybeDoc = new NoDocument(key, SnapshotVersion.forDeletedDoc());
-          }
-          results = results.insert(key, maybeDoc);
-        })
-      );
+    return this.mutationQueue
+    .getAllMutationBatchesAffectingDocumentKeys(transaction, keys).next(batches => {
+      const promises = [] as Array<PersistencePromise<void>>;
+      let results = maybeDocumentMap();
+      keys.forEach(key => {
+        promises.push(
+          this.getDocumentInBatches(transaction, key, batches).next(maybeDoc => {
+            // TODO(http://b/32275378): Don't conflate missing / deleted.
+            if (!maybeDoc) {
+              maybeDoc = new NoDocument(key, SnapshotVersion.forDeletedDoc());
+            }
+            results = results.insert(key, maybeDoc);
+          })
+        );
+      });
+      return PersistencePromise.waitFor(promises).next(() => results);
     });
-    return PersistencePromise.waitFor(promises).next(() => results);
   }
 
   /** Performs a query against the local view of all documents. */
@@ -122,48 +140,37 @@ export class LocalDocumentsView {
     query: Query
   ): PersistencePromise<DocumentMap> {
     // Query the remote documents and overlay mutations.
-    // TODO(mikelehen): There may be significant overlap between the mutations
-    // affecting these remote documents and the
-    // getAllMutationBatchesAffectingQuery() mutations. Consider optimizing.
     let results: DocumentMap;
     return this.remoteDocumentCache
       .getDocumentsMatchingQuery(transaction, query)
       .next(queryResults => {
-        return this.computeLocalDocuments(transaction, queryResults);
-      })
-      .next(promisedResults => {
-        results = promisedResults;
-        // Now use the mutation queue to discover any other documents that may
-        // match the query after applying mutations.
+        results = queryResults;
         return this.mutationQueue.getAllMutationBatchesAffectingQuery(
           transaction,
           query
         );
       })
       .next(matchingMutationBatches => {
-        let matchingKeys = documentKeySet();
         for (const batch of matchingMutationBatches) {
+          // TODO(mikelehen): PERF: Check if this mutation actually affects the
+          // query to reduce work.
           for (const mutation of batch.mutations) {
-            // TODO(mikelehen): PERF: Check if this mutation actually
-            // affects the query to reduce work.
-            if (!results.get(mutation.key)) {
-              matchingKeys = matchingKeys.add(mutation.key);
+            if (!query.path.isImmediateParentOf(mutation.key.path)) {
+              continue;
+            }
+
+            const key = mutation.key;
+            const baseDoc = results.get(key);
+            const mutatedDoc = mutation.applyToLocalView(baseDoc, baseDoc, batch.localWriteTime);
+            if (!mutatedDoc || mutatedDoc instanceof NoDocument) {
+              results = results.remove(mutatedDoc.key);
+            } else if (mutatedDoc instanceof Document) {
+              results = results.insert(mutatedDoc.key, mutatedDoc);
+            } else {
+              fail('Unknown MaybeDocument: ' + mutatedDoc);
             }
           }
         }
-
-        // Now add in the results for the matchingKeys.
-        const promises = [] as Array<PersistencePromise<void>>;
-        matchingKeys.forEach(key => {
-          promises.push(
-            this.getDocument(transaction, key).next(doc => {
-              if (doc instanceof Document) {
-                results = results.insert(doc.key, doc);
-              }
-            })
-          );
-        });
-        return PersistencePromise.waitFor(promises);
       })
       .next(() => {
         // Finally, filter out any documents that don't actually match
@@ -177,57 +184,4 @@ export class LocalDocumentsView {
         return results;
       });
   }
-
-  /**
-   * Takes a remote document and applies local mutations to generate the local
-   * view of the document.
-   * @param transaction The transaction in which to perform any persistence
-   *     operations.
-   * @param documentKey The key of the document (necessary when remoteDocument
-   *     is null).
-   * @param document The base remote document to apply mutations to or null.
-   */
-  private computeLocalDocument(
-    transaction: PersistenceTransaction,
-    documentKey: DocumentKey,
-    document: MaybeDocument | null
-  ): PersistencePromise<MaybeDocument | null> {
-    return this.mutationQueue
-      .getAllMutationBatchesAffectingDocumentKey(transaction, documentKey)
-      .next(batches => {
-        for (const batch of batches) {
-          document = batch.applyToLocalView(documentKey, document);
-        }
-        return document;
-      });
-  }
-
-  /**
-   * Takes a set of remote documents and applies local mutations to generate the
-   * local view of the documents.
-   * @param transaction The transaction in which to perform any persistence
-   *     operations.
-   * @param documents The base remote documents to apply mutations to.
-   * @return The local view of the documents.
-   */
-  private computeLocalDocuments(
-    transaction: PersistenceTransaction,
-    documents: DocumentMap
-  ): PersistencePromise<DocumentMap> {
-    const promises = [] as Array<PersistencePromise<void>>;
-    documents.forEach((key, doc) => {
-      promises.push(
-        this.computeLocalDocument(transaction, key, doc).next(mutatedDoc => {
-          if (mutatedDoc instanceof Document) {
-            documents = documents.insert(mutatedDoc.key, mutatedDoc);
-          } else if (mutatedDoc instanceof NoDocument) {
-            documents = documents.remove(mutatedDoc.key);
-          } else {
-            fail('Unknown MaybeDocument: ' + mutatedDoc);
-          }
-        })
-      );
-    });
-    return PersistencePromise.waitFor(promises).next(() => documents);
-  }
 }

packages/firestore/src/local/memory_mutation_queue.ts

@@ -17,6 +17,7 @@
 import { Timestamp } from '../api/timestamp';
 import { Query } from '../core/query';
 import { BatchId, ProtoByteString } from '../core/types';
+import { DocumentKeySet } from '../model/collections';
 import { DocumentKey } from '../model/document_key';
 import { Mutation } from '../model/mutation';
 import { BATCHID_UNKNOWN, MutationBatch } from '../model/mutation_batch';
@@ -252,6 +253,29 @@ export class MemoryMutationQueue implements MutationQueue {
     return PersistencePromise.resolve(result);
   }
 
+  getAllMutationBatchesAffectingDocumentKeys(
+    transaction: PersistenceTransaction,
+    documentKeys: DocumentKeySet
+  ): PersistencePromise<MutationBatch[]> {
+    const start = new DocReference(documentKeys.first(), 0);
+    const end = new DocReference(documentKeys.last(), 0);
+    const result: MutationBatch[] = [];
+    this.batchesByDocumentKey.forEachInRange([start, end], ref => {
+      assert(
+        documentKey.isEqual(ref.key),
+        "Should only iterate over a single key's batches"
+      );
+      const batch = this.findMutationBatch(ref.targetOrBatchId);
+      assert(
+        batch !== null,
+        'Batches in the index must exist in the main table'
+      );
+      result.push(batch!);
+    });
+
+    return PersistencePromise.resolve(result);
+  }
+
   getAllMutationBatchesAffectingQuery(
     transaction: PersistenceTransaction,
     query: Query

packages/firestore/src/local/mutation_queue.ts

@@ -140,6 +140,8 @@ export interface MutationQueue extends GarbageSource {
    * document key, so when looping through the batch you'll need to check that
    * the mutation itself matches the key.
    *
+   * Batches are guaranteed to be in sorted order.
+   *
    * Note that because of this requirement implementations are free to return
    * mutation batches that don't contain the document key at all if it's
    * convenient.
@@ -157,6 +159,8 @@ export interface MutationQueue extends GarbageSource {
    * each key, so when looping through the batch you'll need to
    * check that the mutation itself matches the key.
    *
+   * Batches are guaranteed to be in sorted order.
+   *
    * Note that because of this requirement implementations are free to return
    * mutation batches that don't contain any of the document keys at all if it's
    * convenient.
@@ -174,6 +178,8 @@ export interface MutationQueue extends GarbageSource {
    * when looping through the batch you'll need to check that the mutation
    * itself matches the query.
    *
+   * Batches are guaranteed to be in sorted order.
+   *
    * Note that because of this requirement implementations are free to return
    * mutation batches that don't match the query at all if it's convenient.
    *

packages/firestore/src/model/path.ts

@@ -143,6 +143,20 @@ export abstract class Path {
     return true;
   }
 
+  isImmediateParentOf(potentialChild: this): boolean {
+    if (this.length + 1 !== potentialChild.length) {
+      return false;
+    }
+
+    for (let i = 0; i < this.length; i++) {
+      if (this.get(i) !== potentialChild.get(i)) {
+        return false;
+      }
+    }
+
+    return true;
+  }
+
   forEach(fn: (segment: string) => void): void {
     for (let i = this.offset, end = this.limit(); i < end; i++) {
       fn(this.segments[i]);