Make Documents mutable (#4548)

Author: schmidt-sebastian

packages/firestore/src/core/bundle.ts

@@ -17,8 +17,8 @@
 
 import { LoadBundleTaskProgress } from '@firebase/firestore-types';
 
-import { MaybeDocumentMap } from '../model/collections';
-import { MaybeDocument } from '../model/document';
+import { DocumentMap } from '../model/collections';
+import { MutableDocument } from '../model/document';
 import { DocumentKey } from '../model/document_key';
 import { BundledDocumentMetadata as ProtoBundledDocumentMetadata } from '../protos/firestore_bundle_proto';
 import {
@@ -46,7 +46,7 @@ export type BundledDocuments = BundledDocument[];
 export class BundleLoadResult {
   constructor(
     readonly progress: LoadBundleTaskProgress,
-    readonly changedDocs: MaybeDocumentMap
+    readonly changedDocs: DocumentMap
   ) {}
 }
 
@@ -89,9 +89,9 @@ export interface BundleConverter {
   toDocumentKey(name: string): DocumentKey;
 
   /**
-   * Converts a BundleDocument to a MaybeDocument.
+   * Converts a BundleDocument to a MutableDocument.
    */
-  toMaybeDocument(bundledDoc: BundledDocument): MaybeDocument;
+  toMutableDocument(bundledDoc: BundledDocument): MutableDocument;
 
   toSnapshotVersion(time: ApiTimestamp): SnapshotVersion;
 }

packages/firestore/src/core/bundle_impl.ts

@@ -23,7 +23,7 @@ import {
   localStoreSaveNamedQuery
 } from '../local/local_store_impl';
 import { documentKeySet, DocumentKeySet } from '../model/collections';
-import { MaybeDocument, NoDocument } from '../model/document';
+import { MutableDocument } from '../model/document';
 import { DocumentKey } from '../model/document_key';
 import {
   BundleMetadata as ProtoBundleMetadata,
@@ -58,17 +58,17 @@ export class BundleConverterImpl implements BundleConverter {
   }
 
   /**
-   * Converts a BundleDocument to a MaybeDocument.
+   * Converts a BundleDocument to a MutableDocument.
    */
-  toMaybeDocument(bundledDoc: BundledDocument): MaybeDocument {
+  toMutableDocument(bundledDoc: BundledDocument): MutableDocument {
     if (bundledDoc.metadata.exists) {
       debugAssert(
         !!bundledDoc.document,
         'Document is undefined when metadata.exist is true.'
       );
       return fromDocument(this.serializer, bundledDoc.document!, false);
     } else {
-      return new NoDocument(
+      return MutableDocument.newNoDocument(
         this.toDocumentKey(bundledDoc.metadata.name!),
         this.toSnapshotVersion(bundledDoc.metadata.readTime!)
       );

packages/firestore/src/core/firestore_client.ts

@@ -31,7 +31,7 @@ import {
   localStoreReadDocument
 } from '../local/local_store_impl';
 import { Persistence } from '../local/persistence';
-import { Document, NoDocument } from '../model/document';
+import { Document } from '../model/document';
 import { DocumentKey } from '../model/document_key';
 import { Mutation } from '../model/mutation';
 import { toByteStreamReader } from '../platform/byte_stream_reader';
@@ -498,10 +498,10 @@ async function readDocumentFromCache(
   result: Deferred<Document | null>
 ): Promise<void> {
   try {
-    const maybeDoc = await localStoreReadDocument(localStore, docKey);
-    if (maybeDoc instanceof Document) {
-      result.resolve(maybeDoc);
-    } else if (maybeDoc instanceof NoDocument) {
+    const document = await localStoreReadDocument(localStore, docKey);
+    if (document.isFoundDocument()) {
+      result.resolve(document);
+    } else if (document.isNoDocument()) {
       result.resolve(null);
     } else {
       result.reject(

packages/firestore/src/core/query.ts

@@ -456,6 +456,7 @@ export function stringifyQuery(query: Query): string {
 /** Returns whether `doc` matches the constraints of `query`. */
 export function queryMatches(query: Query, doc: Document): boolean {
   return (
+    doc.isFoundDocument() &&
     queryMatchesPathAndCollectionGroup(query, doc) &&
     queryMatchesOrderBy(query, doc) &&
     queryMatchesFilters(query, doc) &&
@@ -491,7 +492,7 @@ function queryMatchesPathAndCollectionGroup(
 function queryMatchesOrderBy(query: Query, doc: Document): boolean {
   for (const orderBy of query.explicitOrderBy) {
     // order by key always matches
-    if (!orderBy.field.isKeyField() && doc.field(orderBy.field) === null) {
+    if (!orderBy.field.isKeyField() && doc.data.field(orderBy.field) === null) {
       return false;
     }
   }

packages/firestore/src/core/sync_engine_impl.ts

@@ -45,9 +45,9 @@ import { TargetData, TargetPurpose } from '../local/target_data';
 import {
   DocumentKeySet,
   documentKeySet,
-  MaybeDocumentMap
+  DocumentMap
 } from '../model/collections';
-import { MaybeDocument, NoDocument } from '../model/document';
+import { MutableDocument } from '../model/document';
 import { DocumentKey } from '../model/document_key';
 import { Mutation } from '../model/mutation';
 import { MutationBatchResult } from '../model/mutation_batch';
@@ -161,7 +161,7 @@ class LimboResolution {
  */
 type ApplyDocChangesHandler = (
   queryView: QueryView,
-  changes: MaybeDocumentMap,
+  changes: DocumentMap,
   remoteEvent?: RemoteEvent
 ) => Promise<ViewSnapshot | undefined>;
 
@@ -618,12 +618,12 @@ export async function syncEngineRejectListen(
     // This is kind of a hack. Ideally, we would have a method in the local
     // store to purge a document. However, it would be tricky to keep all of
     // the local store's invariants with another method.
-    let documentUpdates = new SortedMap<DocumentKey, MaybeDocument>(
+    let documentUpdates = new SortedMap<DocumentKey, MutableDocument>(
       DocumentKey.comparator
     );
     documentUpdates = documentUpdates.insert(
       limboKey,
-      new NoDocument(limboKey, SnapshotVersion.min())
+      MutableDocument.newNoDocument(limboKey, SnapshotVersion.min())
     );
     const resolvedLimboDocuments = documentKeySet().add(limboKey);
     const event = new RemoteEvent(
@@ -1001,7 +1001,7 @@ export function syncEngineGetEnqueuedLimboDocumentResolutions(
 
 export async function syncEngineEmitNewSnapsAndNotifyLocalStore(
   syncEngine: SyncEngine,
-  changes: MaybeDocumentMap,
+  changes: DocumentMap,
   remoteEvent?: RemoteEvent
 ): Promise<void> {
   const syncEngineImpl = debugCast(syncEngine, SyncEngineImpl);
@@ -1052,7 +1052,7 @@ export async function syncEngineEmitNewSnapsAndNotifyLocalStore(
 async function applyDocChanges(
   syncEngineImpl: SyncEngineImpl,
   queryView: QueryView,
-  changes: MaybeDocumentMap,
+  changes: DocumentMap,
   remoteEvent?: RemoteEvent
 ): Promise<ViewSnapshot | undefined> {
   let viewDocChanges = queryView.view.computeDocChanges(changes);

packages/firestore/src/core/target.ts

@@ -298,7 +298,7 @@ export class FieldFilter extends Filter {
   }
 
   matches(doc: Document): boolean {
-    const other = doc.field(this.field);
+    const other = doc.data.field(this.field);
     // Types do not have to match in NOT_EQUAL filters.
     if (this.op === Operator.NOT_EQUAL) {
       return (
@@ -459,7 +459,7 @@ export class ArrayContainsFilter extends FieldFilter {
   }
 
   matches(doc: Document): boolean {
-    const other = doc.field(this.field);
+    const other = doc.data.field(this.field);
     return isArray(other) && arrayValueContains(other.arrayValue, this.value);
   }
 }
@@ -472,7 +472,7 @@ export class InFilter extends FieldFilter {
   }
 
   matches(doc: Document): boolean {
-    const other = doc.field(this.field);
+    const other = doc.data.field(this.field);
     return other !== null && arrayValueContains(this.value.arrayValue!, other);
   }
 }
@@ -490,7 +490,7 @@ export class NotInFilter extends FieldFilter {
     ) {
       return false;
     }
-    const other = doc.field(this.field);
+    const other = doc.data.field(this.field);
     return other !== null && !arrayValueContains(this.value.arrayValue!, other);
   }
 }
@@ -503,7 +503,7 @@ export class ArrayContainsAnyFilter extends FieldFilter {
   }
 
   matches(doc: Document): boolean {
-    const other = doc.field(this.field);
+    const other = doc.data.field(this.field);
     if (!isArray(other) || !other.arrayValue.values) {
       return false;
     }
@@ -588,7 +588,7 @@ export function sortsBeforeDocument(
         doc.key
       );
     } else {
-      const docValue = doc.field(orderByComponent.field);
+      const docValue = doc.data.field(orderByComponent.field);
       debugAssert(
         docValue !== null,
         'Field should exist since document matched the orderBy already.'

packages/firestore/src/core/transaction.ts

@@ -16,7 +16,7 @@
  */
 
 import { ParsedSetData, ParsedUpdateData } from '../lite/user_data_reader';
-import { Document, MaybeDocument, NoDocument } from '../model/document';
+import { Document } from '../model/document';
 import { DocumentKey } from '../model/document_key';
 import {
   DeleteMutation,
@@ -60,7 +60,7 @@ export class Transaction {
 
   constructor(private datastore: Datastore) {}
 
-  async lookup(keys: DocumentKey[]): Promise<MaybeDocument[]> {
+  async lookup(keys: DocumentKey[]): Promise<Document[]> {
     this.ensureCommitNotCalled();
 
     if (this.mutations.length > 0) {
@@ -70,13 +70,7 @@ export class Transaction {
       );
     }
     const docs = await invokeBatchGetDocumentsRpc(this.datastore, keys);
-    docs.forEach(doc => {
-      if (doc instanceof NoDocument || doc instanceof Document) {
-        this.recordVersion(doc);
-      } else {
-        fail('Document in a transaction was a ' + doc.constructor.name);
-      }
-    });
+    docs.forEach(doc => this.recordVersion(doc));
     return docs;
   }
 
@@ -120,12 +114,12 @@ export class Transaction {
     this.committed = true;
   }
 
-  private recordVersion(doc: MaybeDocument): void {
+  private recordVersion(doc: Document): void {
     let docVersion: SnapshotVersion;
 
-    if (doc instanceof Document) {
+    if (doc.isFoundDocument()) {
       docVersion = doc.version;
-    } else if (doc instanceof NoDocument) {
+    } else if (doc.isNoDocument()) {
       // For deleted docs, we must use baseVersion 0 when we overwrite them.
       docVersion = SnapshotVersion.min();
     } else {