Overhaul Documents

Remove different internal types for MaybeDocument, Document, NoDocument,
and UnknownDocument. These are all now just Documents.

Also make Documents non-nullable nearly everywhere.

The change introduces a DocumentType enum and maps the old classes like
so:

  null =>             DocumentType.UNKNOWN
  UnknownDocument =>  DocumentType.CONTENTS_UNKNOWN
  NoDocument =>       DocumentType.MISSING
  Document =>         DocumentType.EXISTING

Part of why this change works is that most of the system only cares
whether or not a document exists. We have several refinements of
non-existing documents for the purposes of caching but everywhere else
we can ignore it and just check Document.exists. The instanceof checks
were usually overconstraining things.

The motivation for this change is in two parts:

  * It's always bugged me that we had so many instanceof checks around
  * In C++ we don't have instanceof anyway, so we have to do something
    else.

Also, this definitely addresses b/32275378.

Author: wilhuff

packages/firestore/src/api/database.ts

@@ -40,7 +40,8 @@ import { Transaction as InternalTransaction } from '../core/transaction';
 import { ChangeType, ViewSnapshot } from '../core/view_snapshot';
 import { IndexedDbPersistence } from '../local/indexeddb_persistence';
 import { LruParams } from '../local/lru_garbage_collector';
-import { Document, MaybeDocument, NoDocument } from '../model/document';
+import { getDocument } from '../model/collections';
+import { Document } from '../model/document';
 import { DocumentKey } from '../model/document_key';
 import {
   ArrayValue,
@@ -79,7 +80,7 @@ import { LogLevel } from '../util/log';
 import { AutoId } from '../util/misc';
 import * as objUtils from '../util/obj';
 import { Rejecter, Resolver } from '../util/promise';
-import { Deferred } from './../util/promise';
+import { Deferred } from '../util/promise';
 import { FieldPath as ExternalFieldPath } from './field_path';
 
 import {
@@ -661,37 +662,18 @@ export class Transaction implements firestore.Transaction {
       documentRef,
       this._firestore
     );
-    return this._transaction
-      .lookup([ref._key])
-      .then((docs: MaybeDocument[]) => {
-        if (!docs || docs.length !== 1) {
-          return fail('Mismatch in docs returned from document lookup.');
-        }
-        const doc = docs[0];
-        if (doc instanceof NoDocument) {
-          return new DocumentSnapshot(
-            this._firestore,
-            ref._key,
-            null,
-            /* fromCache= */ false,
-            /* hasPendingWrites= */ false
-          );
-        } else if (doc instanceof Document) {
-          return new DocumentSnapshot(
-            this._firestore,
-            ref._key,
-            doc,
-            /* fromCache= */ false,
-            /* hasPendingWrites= */ false
-          );
-        } else {
-          throw fail(
-            `BatchGetDocumentsRequest returned unexpected document type: ${
-              doc.constructor.name
-            }`
-          );
-        }
-      });
+    return this._transaction.lookup([ref._key]).then((docs: Document[]) => {
+      if (!docs || docs.length !== 1) {
+        return fail('Mismatch in docs returned from document lookup.');
+      }
+      const doc = docs[0];
+      return new DocumentSnapshot(
+        this._firestore,
+        doc,
+        /* fromCache= */ false,
+        /* hasPendingWrites= */ false
+      );
+    });
   }
 
   set(
@@ -1130,12 +1112,11 @@ export class DocumentReference implements firestore.DocumentReference {
             snapshot.docs.size <= 1,
             'Too many documents returned on a document query'
           );
-          const doc = snapshot.docs.get(this._key);
+          const doc = getDocument(this._key, snapshot.docs);
 
           observer.next(
             new DocumentSnapshot(
               this.firestore,
-              this._key,
               doc,
               snapshot.fromCache,
               snapshot.hasPendingWrites
@@ -1170,10 +1151,9 @@ export class DocumentReference implements firestore.DocumentReference {
               resolve(
                 new DocumentSnapshot(
                   this.firestore,
-                  this._key,
                   doc,
                   /*fromCache=*/ true,
-                  doc instanceof Document ? doc.hasLocalMutations : false
+                  doc.hasLocalMutations
                 )
               );
             }, reject);
@@ -1262,8 +1242,7 @@ export interface SnapshotOptions extends firestore.SnapshotOptions {}
 export class DocumentSnapshot implements firestore.DocumentSnapshot {
   constructor(
     private _firestore: Firestore,
-    private _key: DocumentKey,
-    public _document: Document | null,
+    public _document: Document,
     private _fromCache: boolean,
     private _hasPendingWrites: boolean
   ) {}
@@ -1273,7 +1252,7 @@ export class DocumentSnapshot implements firestore.DocumentSnapshot {
   ): firestore.DocumentData | undefined {
     validateBetweenNumberOfArgs('DocumentSnapshot.data', arguments, 0, 1);
     options = validateSnapshotOptions('DocumentSnapshot.data', options);
-    return !this._document
+    return !this._document.exists
       ? undefined
       : this.convertObject(
           this._document.data,
@@ -1290,33 +1269,31 @@ export class DocumentSnapshot implements firestore.DocumentSnapshot {
   ): unknown {
     validateBetweenNumberOfArgs('DocumentSnapshot.get', arguments, 1, 2);
     options = validateSnapshotOptions('DocumentSnapshot.get', options);
-    if (this._document) {
-      const value = this._document.data.field(
-        fieldPathFromArgument('DocumentSnapshot.get', fieldPath)
+    const value = this._document.data.field(
+      fieldPathFromArgument('DocumentSnapshot.get', fieldPath)
+    );
+    if (value !== undefined) {
+      return this.convertValue(
+        value,
+        FieldValueOptions.fromSnapshotOptions(
+          options,
+          this._firestore._areTimestampsInSnapshotsEnabled()
+        )
       );
-      if (value !== undefined) {
-        return this.convertValue(
-          value,
-          FieldValueOptions.fromSnapshotOptions(
-            options,
-            this._firestore._areTimestampsInSnapshotsEnabled()
-          )
-        );
-      }
     }
     return undefined;
   }
 
   get id(): string {
-    return this._key.path.lastSegment();
+    return this._document.key.path.lastSegment();
   }
 
   get ref(): firestore.DocumentReference {
-    return new DocumentReference(this._key, this._firestore);
+    return new DocumentReference(this._document.key, this._firestore);
   }
 
   get exists(): boolean {
-    return this._document !== null;
+    return this._document.exists;
   }
 
   get metadata(): firestore.SnapshotMetadata {
@@ -1330,10 +1307,7 @@ export class DocumentSnapshot implements firestore.DocumentSnapshot {
     return (
       this._firestore === other._firestore &&
       this._fromCache === other._fromCache &&
-      this._key.isEqual(other._key) &&
-      (this._document === null
-        ? other._document === null
-        : this._document.isEqual(other._document))
+      this._document.isEqual(other._document)
     );
   }
 
@@ -1359,7 +1333,7 @@ export class DocumentSnapshot implements firestore.DocumentSnapshot {
       if (!value.databaseId.isEqual(database)) {
         // TODO(b/64130202): Somehow support foreign references.
         log.error(
-          `Document ${this._key.path} contains a document ` +
+          `Document ${this._document.key.path} contains a document ` +
             `reference within a different database (` +
             `${value.databaseId.projectId}/${
               value.databaseId.database
@@ -1389,12 +1363,11 @@ export class QueryDocumentSnapshot extends DocumentSnapshot
   implements firestore.QueryDocumentSnapshot {
   constructor(
     firestore: Firestore,
-    key: DocumentKey,
     document: Document,
     fromCache: boolean,
     hasPendingWrites: boolean
   ) {
-    super(firestore, key, document, fromCache, hasPendingWrites);
+    super(firestore, document, fromCache, hasPendingWrites);
   }
 
   data(options?: SnapshotOptions): firestore.DocumentData {
@@ -2136,7 +2109,6 @@ export class QuerySnapshot implements firestore.QuerySnapshot {
   private convertToDocumentImpl(doc: Document): QueryDocumentSnapshot {
     return new QueryDocumentSnapshot(
       this._firestore,
-      doc.key,
       doc,
       this.metadata.fromCache,
       this._snapshot.mutatedKeys.has(doc.key)
@@ -2348,7 +2320,6 @@ export function changesFromSnapshot(
     return snapshot.docChanges.map(change => {
       const doc = new QueryDocumentSnapshot(
         firestore,
-        change.doc.key,
         change.doc,
         snapshot.fromCache,
         snapshot.mutatedKeys.has(change.doc.key)
@@ -2380,7 +2351,6 @@ export function changesFromSnapshot(
       .map(change => {
         const doc = new QueryDocumentSnapshot(
           firestore,
-          change.doc.key,
           change.doc,
           snapshot.fromCache,
           snapshot.mutatedKeys.has(change.doc.key)

packages/firestore/src/core/firestore_client.ts

@@ -26,7 +26,7 @@ import {
   documentKeySet,
   DocumentMap
 } from '../model/collections';
-import { Document, MaybeDocument, NoDocument } from '../model/document';
+import { Document } from '../model/document';
 import { DocumentKey } from '../model/document_key';
 import { Mutation } from '../model/mutation';
 import { Platform } from '../platform/platform';
@@ -228,6 +228,7 @@ export class FirestoreClient {
    *     result of enabling offline persistence. This method will reject this if
    *     IndexedDB fails to start for any reason. If usePersistence is false
    *     this is unconditionally resolved.
+   * @param user The identity of the user that's currently active.
    * @returns a Promise indicating whether or not initialization should
    *     continue, i.e. that one of the persistence implementations actually
    *     succeeded.
@@ -480,7 +481,7 @@ export class FirestoreClient {
           }
         });
 
-        // When a user calls clearPersistence() in one client, all other clientfs
+        // When a user calls clearPersistence() in one client, all other clients
         // need to shut down to allow the delete to succeed.
         await this.persistence.setDatabaseDeletedListener(async () => {
           await this.shutdown();
@@ -543,17 +544,15 @@ export class FirestoreClient {
     });
   }
 
-  getDocumentFromLocalCache(docKey: DocumentKey): Promise<Document | null> {
+  getDocumentFromLocalCache(docKey: DocumentKey): Promise<Document> {
     this.verifyNotShutdown();
     return this.asyncQueue
       .enqueue(() => {
         return this.localStore.readDocument(docKey);
       })
-      .then((maybeDoc: MaybeDocument | null) => {
-        if (maybeDoc instanceof Document) {
+      .then((maybeDoc: Document) => {
+        if (maybeDoc.definite) {
           return maybeDoc;
-        } else if (maybeDoc instanceof NoDocument) {
-          return null;
         } else {
           throw new FirestoreError(
             Code.UNAVAILABLE,

packages/firestore/src/core/query.ts

@@ -323,6 +323,7 @@ export class Query {
 
   matches(doc: Document): boolean {
     return (
+      doc.exists &&
       this.matchesPathAndCollectionGroup(doc) &&
       this.matchesOrderBy(doc) &&
       this.matchesFilters(doc) &&

packages/firestore/src/core/snapshot_version.ts

@@ -36,7 +36,7 @@ export class SnapshotVersion {
     return new SnapshotVersion(value);
   }
 
-  static forDeletedDoc(): SnapshotVersion {
+  static forMissingDoc(): SnapshotVersion {
     return SnapshotVersion.MIN;
   }
 

packages/firestore/src/core/sync_engine.ts

@@ -23,9 +23,9 @@ import { ReferenceSet } from '../local/reference_set';
 import {
   documentKeySet,
   DocumentKeySet,
-  MaybeDocumentMap
+  DocumentMap
 } from '../model/collections';
-import { MaybeDocument, NoDocument } from '../model/document';
+import { Document } from '../model/document';
 import { DocumentKey } from '../model/document_key';
 import { Mutation } from '../model/mutation';
 import { MutationBatchResult } from '../model/mutation_batch';
@@ -526,12 +526,12 @@ export class SyncEngine implements RemoteSyncer, SharedClientStateSyncer {
       // 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, Document>(
         DocumentKey.comparator
       );
       documentUpdates = documentUpdates.insert(
         limboKey,
-        new NoDocument(limboKey, SnapshotVersion.forDeletedDoc())
+        Document.missing(limboKey, SnapshotVersion.forMissingDoc())
       );
       const resolvedLimboDocuments = documentKeySet().add(limboKey);
       const event = new RemoteEvent(
@@ -758,7 +758,7 @@ export class SyncEngine implements RemoteSyncer, SharedClientStateSyncer {
   }
 
   private async emitNewSnapsAndNotifyLocalStore(
-    changes: MaybeDocumentMap,
+    changes: DocumentMap,
     remoteEvent?: RemoteEvent
   ): Promise<void> {
     const newSnaps: ViewSnapshot[] = [];

packages/firestore/src/core/transaction.ts

@@ -17,8 +17,7 @@
 
 import { ParsedSetData, ParsedUpdateData } from '../api/user_data_converter';
 import { documentVersionMap } from '../model/collections';
-import { Document, NoDocument } from '../model/document';
-import { MaybeDocument } from '../model/document';
+import { Document } from '../model/document';
 import { DocumentKey } from '../model/document_key';
 import { DeleteMutation, Mutation, Precondition } from '../model/mutation';
 import { Datastore } from '../remote/datastore';
@@ -38,16 +37,14 @@ export class Transaction {
 
   constructor(private datastore: Datastore) {}
 
-  private recordVersion(doc: MaybeDocument): void {
+  private recordVersion(doc: Document): void {
     let docVersion: SnapshotVersion;
 
-    if (doc instanceof Document) {
+    if (doc.exists) {
       docVersion = doc.version;
-    } else if (doc instanceof NoDocument) {
-      // For deleted docs, we must use baseVersion 0 when we overwrite them.
-      docVersion = SnapshotVersion.forDeletedDoc();
     } else {
-      throw fail('Document in a transaction was a ' + doc.constructor.name);
+      // For deleted docs, we must use baseVersion 0 when we overwrite them.
+      docVersion = SnapshotVersion.forMissingDoc();
     }
 
     const existingVersion = this.readVersions.get(doc.key);
@@ -64,23 +61,21 @@ export class Transaction {
     }
   }
 
-  lookup(keys: DocumentKey[]): Promise<MaybeDocument[]> {
+  lookup(keys: DocumentKey[]): Promise<Document[]> {
     if (this.committed) {
-      return Promise.reject<MaybeDocument[]>(
-        'Transaction has already completed.'
-      );
+      return Promise.reject<Document[]>('Transaction has already completed.');
     }
     if (this.mutations.length > 0) {
-      return Promise.reject<MaybeDocument[]>(
+      return Promise.reject<Document[]>(
         'Transactions lookups are invalid after writes.'
       );
     }
     return this.datastore.lookup(keys).then(docs => {
       docs.forEach(doc => {
-        if (doc instanceof NoDocument || doc instanceof Document) {
+        if (doc.definite) {
           this.recordVersion(doc);
         } else {
-          fail('Document in a transaction was a ' + doc.constructor.name);
+          fail('Document in a transaction was a ' + doc.toString());
         }
       });
       return docs;
@@ -115,7 +110,7 @@ export class Transaction {
    */
   private preconditionForUpdate(key: DocumentKey): Precondition {
     const version = this.readVersions.get(key);
-    if (version && version.isEqual(SnapshotVersion.forDeletedDoc())) {
+    if (version && version.isEqual(SnapshotVersion.forMissingDoc())) {
       // The document doesn't exist, so fail the transaction.
       throw new FirestoreError(
         Code.FAILED_PRECONDITION,
@@ -145,7 +140,7 @@ export class Transaction {
     // ensure that the precondition for the next write will be exists: false.
     this.readVersions = this.readVersions.insert(
       key,
-      SnapshotVersion.forDeletedDoc()
+      SnapshotVersion.forMissingDoc()
     );
   }