Simplify Document

packages/firestore/src/local/local_serializer.ts

@@ -79,9 +79,7 @@ export class LocalSerializer {
     const dbReadTime = this.toDbTimestampKey(readTime);
     const parentPath = maybeDoc.key.path.popLast().toArray();
     if (maybeDoc instanceof Document) {
-      const doc = maybeDoc.proto
-        ? maybeDoc.proto
-        : this.remoteSerializer.toDocument(maybeDoc);
+      const doc = this.remoteSerializer.toDocument(maybeDoc);
       const hasCommittedMutations = maybeDoc.hasCommittedMutations;
       return new DbRemoteDocument(
         /* unknownDocument= */ null,

packages/firestore/src/model/document.ts

@@ -16,15 +16,12 @@
  */
 
 import { SnapshotVersion } from '../core/snapshot_version';
-import { assert, fail } from '../util/assert';
+import { fail } from '../util/assert';
 
 import { DocumentKey } from './document_key';
 import { FieldValue, JsonObject, ObjectValue } from './field_value';
 import { FieldPath } from './path';
 
-import * as api from '../protos/firestore_proto_api';
-import * as obj from '../util/obj';
-
 export interface DocumentOptions {
   hasLocalMutations?: boolean;
   hasCommittedMutations?: boolean;
@@ -60,76 +57,22 @@ export class Document extends MaybeDocument {
   readonly hasLocalMutations: boolean;
   readonly hasCommittedMutations: boolean;
 
-  /**
-   * A cache of canonicalized FieldPaths to FieldValues that have already been
-   * deserialized in `getField()`.
-   */
-  private fieldValueCache?: Map<string, FieldValue | null>;
-
   constructor(
     key: DocumentKey,
     version: SnapshotVersion,
-    options: DocumentOptions,
-    private objectValue?: ObjectValue,
-    readonly proto?: api.Document,
-    private readonly converter?: (value: api.Value) => FieldValue
+    private readonly objectValue: ObjectValue,
+    options: DocumentOptions
   ) {
     super(key, version);
-    assert(
-      this.objectValue !== undefined ||
-        (this.proto !== undefined && this.converter !== undefined),
-      'If objectValue is not defined, proto and converter need to be set.'
-    );
-
     this.hasLocalMutations = !!options.hasLocalMutations;
     this.hasCommittedMutations = !!options.hasCommittedMutations;
   }
 
   field(path: FieldPath): FieldValue | null {
-    if (this.objectValue) {
-      return this.objectValue.field(path);
-    } else {
-      if (!this.fieldValueCache) {
-        // TODO(b/136090445): Remove the cache when `getField` is no longer
-        // called during Query ordering.
-        this.fieldValueCache = new Map<string, FieldValue>();
-      }
-
-      const canonicalPath = path.canonicalString();
-
-      let fieldValue = this.fieldValueCache.get(canonicalPath);
-
-      if (fieldValue === undefined) {
-        // Instead of deserializing the full Document proto, we only
-        // deserialize the value at the requested field path. This speeds up
-        // Query execution as query filters can discard documents based on a
-        // single field.
-        const protoValue = this.getProtoField(path);
-        if (protoValue === undefined) {
-          fieldValue = null;
-        } else {
-          fieldValue = this.converter!(protoValue);
-        }
-        this.fieldValueCache.set(canonicalPath, fieldValue);
-      }
-
-      return fieldValue!;
-    }
+    return this.objectValue.field(path);
   }
 
   data(): ObjectValue {
-    if (!this.objectValue) {
-      const result = ObjectValue.newBuilder();
-      obj.forEach(this.proto!.fields || {}, (key: string, value: api.Value) => {
-        result.set(new FieldPath([key]), this.converter!(value));
-      });
-      this.objectValue = result.build();
-
-      // Once objectValue is computed, values inside the fieldValueCache are no
-      // longer accessed.
-      this.fieldValueCache = undefined;
-    }
-
     return this.objectValue;
   }
 
@@ -144,13 +87,13 @@ export class Document extends MaybeDocument {
       this.version.isEqual(other.version) &&
       this.hasLocalMutations === other.hasLocalMutations &&
       this.hasCommittedMutations === other.hasCommittedMutations &&
-      this.data().isEqual(other.data())
+      this.objectValue.isEqual(other.objectValue)
     );
   }
 
   toString(): string {
     return (
-      `Document(${this.key}, ${this.version}, ${this.data().toString()}, ` +
+      `Document(${this.key}, ${this.version}, ${this.objectValue.toString()}, ` +
       `{hasLocalMutations: ${this.hasLocalMutations}}), ` +
       `{hasCommittedMutations: ${this.hasCommittedMutations}})`
     );
@@ -160,29 +103,6 @@ export class Document extends MaybeDocument {
     return this.hasLocalMutations || this.hasCommittedMutations;
   }
 
-  /**
-   * Returns the nested Protobuf value for 'path`. Can only be called if
-   * `proto` was provided at construction time.
-   */
-  private getProtoField(path: FieldPath): api.Value | undefined {
-    assert(
-      this.proto !== undefined,
-      'Can only call getProtoField() when proto is defined'
-    );
-
-    let protoValue: api.Value | undefined = this.proto!.fields
-      ? this.proto!.fields[path.firstSegment()]
-      : undefined;
-    for (let i = 1; i < path.length; ++i) {
-      if (!protoValue || !protoValue.mapValue || !protoValue.mapValue.fields) {
-        return undefined;
-      }
-      protoValue = protoValue.mapValue.fields[path.get(i)];
-    }
-
-    return protoValue;
-  }
-
   static compareByField(field: FieldPath, d1: Document, d2: Document): number {
     const v1 = d1.field(field);
     const v2 = d2.field(field);

packages/firestore/src/model/field_value.ts

@@ -111,7 +111,7 @@ export class FieldValueOptions {
  * Potential types returned by FieldValue.value(). This could be stricter
  * (instead of using {}), but there's little benefit.
  *
- * Note that currently we use AnyJs (which is identical except includes
+ * Note that currently we use `unknown` (which is identical except includes
  * undefined) for incoming user data as a convenience to the calling code (but
  * we'll throw if the data contains undefined). This should probably be changed
  * to use FieldType, but all consuming code will have to be updated to
@@ -367,6 +367,9 @@ export class TimestampValue extends FieldValue {
  *   localWriteTime.
  */
 export class ServerTimestampValue extends FieldValue {
+  // TODO(mrschmidt): Represent ServerTimestamps as a PrimitiveType with a
+  //  Map containing a private `__type__` field (or similar).
+
   typeOrder = TypeOrder.TimestampValue;
 
   constructor(
@@ -402,7 +405,7 @@ export class ServerTimestampValue extends FieldValue {
   compareTo(other: FieldValue): number {
     if (other instanceof ServerTimestampValue) {
       return this.localWriteTime._compareTo(other.localWriteTime);
-    } else if (other instanceof TimestampValue) {
+    } else if (other.typeOrder === TypeOrder.TimestampValue) {
       // Server timestamps come after all concrete timestamps.
       return 1;
     } else {

packages/firestore/src/model/mutation.ts

@@ -354,14 +354,9 @@ export class SetMutation extends Mutation {
     // have held.
 
     const version = mutationResult.version;
-    return new Document(
-      this.key,
-      version,
-      {
-        hasCommittedMutations: true
-      },
-      this.value
-    );
+    return new Document(this.key, version, this.value, {
+      hasCommittedMutations: true
+    });
   }
 
   applyToLocalView(
@@ -376,14 +371,9 @@ export class SetMutation extends Mutation {
     }
 
     const version = Mutation.getPostMutationVersion(maybeDoc);
-    return new Document(
-      this.key,
-      version,
-      {
-        hasLocalMutations: true
-      },
-      this.value
-    );
+    return new Document(this.key, version, this.value, {
+      hasLocalMutations: true
+    });
   }
 
   extractBaseValue(maybeDoc: MaybeDocument | null): null {
@@ -445,14 +435,9 @@ export class PatchMutation extends Mutation {
     }
 
     const newData = this.patchDocument(maybeDoc);
-    return new Document(
-      this.key,
-      mutationResult.version,
-      {
-        hasCommittedMutations: true
-      },
-      newData
-    );
+    return new Document(this.key, mutationResult.version, newData, {
+      hasCommittedMutations: true
+    });
   }
 
   applyToLocalView(
@@ -468,14 +453,9 @@ export class PatchMutation extends Mutation {
 
     const version = Mutation.getPostMutationVersion(maybeDoc);
     const newData = this.patchDocument(maybeDoc);
-    return new Document(
-      this.key,
-      version,
-      {
-        hasLocalMutations: true
-      },
-      newData
-    );
+    return new Document(this.key, version, newData, {
+      hasLocalMutations: true
+    });
   }
 
   extractBaseValue(maybeDoc: MaybeDocument | null): null {
@@ -573,14 +553,9 @@ export class TransformMutation extends Mutation {
 
     const version = mutationResult.version;
     const newData = this.transformObject(doc.data(), transformResults);
-    return new Document(
-      this.key,
-      version,
-      {
-        hasCommittedMutations: true
-      },
-      newData
-    );
+    return new Document(this.key, version, newData, {
+      hasCommittedMutations: true
+    });
   }
 
   applyToLocalView(
@@ -601,14 +576,9 @@ export class TransformMutation extends Mutation {
       baseDoc
     );
     const newData = this.transformObject(doc.data(), transformResults);
-    return new Document(
-      this.key,
-      doc.version,
-      {
-        hasLocalMutations: true
-      },
-      newData
-    );
+    return new Document(this.key, doc.version, newData, {
+      hasLocalMutations: true
+    });
   }
 
   extractBaseValue(maybeDoc: MaybeDocument | null): ObjectValue | null {