WIP

Author: schmidt-sebastian

packages/firestore/src/local/indexeddb_remote_document_cache.ts

@@ -189,7 +189,7 @@ class IndexedDbRemoteDocumentCacheImpl implements IndexedDbRemoteDocumentCache {
       documentKeys,
       (key, dbRemoteDoc) => {
         const doc = this.maybeDecodeDocument(key, dbRemoteDoc);
-        if (doc) {
+        if (doc.isValidDocument()) {
           results = results.insert(key, doc);
           sizeMap = sizeMap.insert(key, dbDocumentSize(dbRemoteDoc!));
         } else {

packages/firestore/src/local/local_documents_view.ts

@@ -92,15 +92,12 @@ export class LocalDocumentsView {
   private applyLocalMutationsToDocuments(
     docs: MutableDocumentMap,
     batches: MutationBatch[]
-  ): MutableDocumentMap {
-    let results = mutableDocumentMap();
+  ): void {
     docs.forEach((key, localView) => {
       for (const batch of batches) {
         batch.applyToLocalView(localView);
       }
-      results = results.insert(key, localView);
     });
-    return results;
   }
 
   /**
@@ -113,29 +110,23 @@ export class LocalDocumentsView {
     transaction: PersistenceTransaction,
     keys: DocumentKeySet
   ): PersistencePromise<DocumentMap> {
-    return this.remoteDocumentCache
-      .getEntries(transaction, keys)
-      .next(docs => this.getLocalViewOfDocuments(transaction, docs));
+    return this.remoteDocumentCache.getEntries(transaction, keys).next(docs => {
+      this.applyLocalViewToDocuments(transaction, docs);
+      return docs as DocumentMap;
+    });
   }
 
   /**
-   * Similar to `getDocuments`, but creates the local view from the given
-   * `baseDocs` without retrieving documents from the local store.
+   * Applies the local view the given `baseDocs` without retrieving documents
+   * from the local store.
    */
-  getLocalViewOfDocuments(
+  applyLocalViewToDocuments(
     transaction: PersistenceTransaction,
     baseDocs: MutableDocumentMap
-  ): PersistencePromise<DocumentMap> {
+  ): PersistencePromise<void> {
     return this.mutationQueue
       .getAllMutationBatchesAffectingDocumentKeys(transaction, baseDocs)
-      .next(batches => {
-        const docs = this.applyLocalMutationsToDocuments(baseDocs, batches);
-        let results = mutableDocumentMap();
-        docs.forEach((key, document) => {
-          results = results.insert(key, document);
-        });
-        return results as DocumentMap;
-      });
+      .next(batches => this.applyLocalMutationsToDocuments(baseDocs, batches));
   }
 
   /**

packages/firestore/src/local/local_store_impl.ts

@@ -346,8 +346,8 @@ export function localStoreWriteLocally(
         });
     })
     .then(batch => {
-      const changes = batch.applyToLocalDocumentSet(existingDocs);
-      return { batchId: batch.batchId, changes };
+      batch.applyToLocalDocumentSet(existingDocs);
+      return { batchId: batch.batchId, changes: existingDocs };
     });
 }
 
@@ -575,10 +575,11 @@ export function localStoreApplyRemoteEventToLocalCache(
       return PersistencePromise.waitFor(promises)
         .next(() => documentBuffer.apply(txn))
         .next(() => {
-          return localStoreImpl.localDocuments.getLocalViewOfDocuments(
+          localStoreImpl.localDocuments.applyLocalViewToDocuments(
             txn,
             changedDocs
           );
+          return changedDocs;
         });
     })
     .then(changedDocs => {
@@ -1254,12 +1255,13 @@ export async function localStoreApplyBundledDocuments(
                 umbrellaTargetData.targetId
               )
             )
-            .next(() =>
-              localStoreImpl.localDocuments.getLocalViewOfDocuments(
+            .next(() => {
+              localStoreImpl.localDocuments.applyLocalViewToDocuments(
                 txn,
                 changedDocs
-              )
-            );
+              );
+              return changedDocs;
+            });
         });
     }
   );

packages/firestore/src/local/memory_remote_document_cache.ts

@@ -99,7 +99,7 @@ class MemoryRemoteDocumentCacheImpl implements MemoryRemoteDocumentCache {
     const currentSize = this.sizer(doc);
 
     this.docs = this.docs.insert(key, {
-      document: doc,
+      document: doc.clone(),
       size: currentSize,
       readTime
     });
@@ -181,7 +181,7 @@ class MemoryRemoteDocumentCacheImpl implements MemoryRemoteDocumentCache {
       if (readTime.compareTo(sinceReadTime) <= 0) {
         continue;
       }
-      if (queryMatches(query, document)) {
+      if (!queryMatches(query, document)) {
         continue;
       }
       results = results.insert(document.key, document);

packages/firestore/src/model/mutable_document.ts

@@ -45,6 +45,8 @@ const enum DocumentType {
 
 /** Describes the `hasPendingWrites` state of a document. */
 const enum DocumentState {
+  /** No mutations applied. Document was sent to us by Watch. */
+  SYNCED,
   /**
    * Local mutations applied via the mutation queue. Document is potentially
    * inconsistent.
@@ -54,9 +56,7 @@ const enum DocumentState {
    * Mutations applied based on a write acknowledgment. Document is potentially
    * inconsistent.
    */
-  HAS_COMMITTED_MUTATIONS,
-  /** No mutations applied. Document was sent to us by Watch. */
-  SYNCED
+  HAS_COMMITTED_MUTATIONS
 }
 
 /**
@@ -243,7 +243,9 @@ export class MutableDocument implements Document {
 
   toString(): string {
     return (
-      `Document(${this.key}, ${this.version}, ${this.data.toString()}, ` +
+      `Document(${this.key}, ${this.version}, ${JSON.stringify(
+        this.data.toProto()
+      )}, ` +
       `{documentType: ${this.documentType}}), ` +
       `{documentState: ${this.documentState}})`
     );

packages/firestore/src/model/mutation.ts

@@ -372,7 +372,7 @@ function verifyMutationKeyMatches(
  * post-mutation version of SnapshotVersion.min().
  */
 function getPostMutationVersion(document: MutableDocument): SnapshotVersion {
-  return document.version;
+  return document.isFoundDocument() ? document.version : SnapshotVersion.min();
 }
 
 /**

packages/firestore/src/model/mutation_batch.ts

@@ -120,20 +120,20 @@ export class MutationBatch {
    * Computes the local view for all provided documents given the mutations in
    * this batch.
    */
-  applyToLocalDocumentSet(documentMap: DocumentMap): DocumentMap {
+  applyToLocalDocumentSet(documentMap: DocumentMap): void {
     // TODO(mrschmidt): This implementation is O(n^2). If we apply the mutations
     // directly (as done in `applyToLocalView()`), we can reduce the complexity
     // to O(n).
     this.mutations.forEach(m => {
       const document = documentMap.get(m.key)!;
       // TODO(mutabledocuments): This method should take a MutableDocumentMap
       // and we should remove this cast.
-      this.applyToLocalView(document as MutableDocument);
+      const mutableDocument = document as MutableDocument;
+      this.applyToLocalView(mutableDocument);
       if (!document.isValidDocument()) {
-        documentMap = documentMap.remove(m.key);
+        mutableDocument.convertToNoDocument(SnapshotVersion.min());
       }
     });
-    return documentMap;
   }
 
   keys(): DocumentKeySet {

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

@@ -388,13 +388,7 @@ class LocalStoreTester {
       expect(this.lastChanges.size).to.equal(docs.length, 'number of changes');
       for (const doc of docs) {
         const returned = this.lastChanges.get(doc.key);
-        expectEqual(
-          doc,
-          returned,
-          `Expected '${
-            returned ? returned.toString() : null
-          }' to equal '${doc.toString()}'.`
-        );
+        expectEqual(doc, returned, `Expected '${doc}' to equal '${returned}'.`);
       }
       this.lastChanges = null;
     });
@@ -752,9 +746,7 @@ function genericLocalStoreTests(
       .after(
         docUpdateRemoteEvent(doc('foo/bar', 2, { foo: 'bar', it: 'base' }), [2])
       )
-      .toReturnChanged(
-        doc('foo/bar', 2, { foo: 'bar', it: 'base' }).setHasLocalMutations()
-      )
+      .toReturnChanged(doc('foo/bar', 2, { foo: 'bar', it: 'base' }))
       .toContain(doc('foo/bar', 2, { foo: 'bar', it: 'base' }))
       .finish();
   });
@@ -1391,7 +1383,10 @@ function genericLocalStoreTests(
         )
         .afterAcknowledgingMutation({ documentVersion: 1 })
         .toReturnChanged(
-          doc('foo/bar', 1, { sum: 0, arrayUnion: [] }).setHasLocalMutations()
+          doc('foo/bar', 1, {
+            sum: 0,
+            arrayUnion: []
+          }).setHasCommittedMutations()
         )
         .afterRemoteEvent(
           docAddedRemoteEvent(

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

@@ -528,7 +528,7 @@ function genericLruGarbageCollectorTests(
       toBeRemoved.forEach(docKey => {
         p = p.next(() => {
           return documentCache.getEntry(txn, docKey).next(maybeDoc => {
-            expect(maybeDoc).to.be.null;
+            expect(maybeDoc.isValidDocument()).to.be.false;
           });
         });
       });
@@ -837,7 +837,7 @@ function genericLruGarbageCollectorTests(
         p = p
           .next(() => documentCache.getEntry(txn, key))
           .next(maybeDoc => {
-            expect(maybeDoc).to.be.null;
+            expect(maybeDoc.isValidDocument()).to.be.false;
           });
       });
       expectedRetained.forEach(key => {

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

@@ -269,9 +269,9 @@ function genericRemoteDocumentCacheTests(
     cache = await cachePromise();
   });
 
-  it('returns null for document not in cache', () => {
+  it('returns an invalid document for documents not in cache', () => {
     return cache.getEntry(key(DOC_PATH)).then(doc => {
-      expect(doc).to.equal(null);
+      expect(doc.isValidDocument()).to.be.false;
     });
   });
 
@@ -329,7 +329,7 @@ function genericRemoteDocumentCacheTests(
       .then(read => {
         expectEqual(read.get(key1), docs[0]);
         expectEqual(read.get(key2), docs[1]);
-        expect(read.get(missingKey)).to.be.null;
+        expect(read.get(missingKey)?.isValidDocument()).to.be.false;
       });
   });
 
@@ -343,7 +343,7 @@ function genericRemoteDocumentCacheTests(
         return cache.getEntry(key(DOC_PATH));
       })
       .then(read => {
-        expect(read).to.equal(null);
+        expect(read.isValidDocument()).to.be.false;
       });
   });
 

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

@@ -94,7 +94,7 @@ export class TestRemoteDocumentCache {
     );
   }
 
-  getEntry(documentKey: DocumentKey): Promise<MutableDocument | null> {
+  getEntry(documentKey: DocumentKey): Promise<MutableDocument> {
     return this.persistence.runTransaction('getEntry', 'readonly', txn => {
       return this.cache.getEntry(txn, documentKey);
     });