Simplify Document (#2685)

Author: schmidt-sebastian

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/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 {

packages/firestore/src/model/proto_values.ts

@@ -242,7 +242,7 @@ function compareTimestamps(
   if (typeof left === 'string' && typeof right === 'string') {
     // Use string ordering for ISO 8601 timestamps, but strip the timezone
     // suffix to ensure proper ordering for timestamps of different precision.
-    // The only supported timezone is UTC (i.e. 'Z') based on 
+    // The only supported timezone is UTC (i.e. 'Z') based on
     // ISO_TIMESTAMP_REG_EXP.
     return primitiveComparator(left.slice(0, -1), right.slice(0, -1));
   }

packages/firestore/src/remote/serializer.ts

@@ -483,14 +483,10 @@ export class JsonProtoSerializer {
   ): Document {
     const key = this.fromName(document.name!);
     const version = this.fromVersion(document.updateTime!);
-    return new Document(
-      key,
-      version,
-      { hasCommittedMutations: !!hasCommittedMutations },
-      undefined,
-      document,
-      v => this.fromValue(v)
-    );
+    const data = this.fromFields(document.fields!);
+    return new Document(key, version, data, {
+      hasCommittedMutations: !!hasCommittedMutations
+    });
   }
 
   toFields(fields: fieldValue.ObjectValue): { [key: string]: api.Value } {
@@ -534,9 +530,8 @@ export class JsonProtoSerializer {
     assertPresent(doc.found.updateTime, 'doc.found.updateTime');
     const key = this.fromName(doc.found.name);
     const version = this.fromVersion(doc.found.updateTime);
-    return new Document(key, version, {}, undefined, doc.found, v =>
-      this.fromValue(v)
-    );
+    const data = this.fromFields(doc.found.fields!);
+    return new Document(key, version, data, {});
   }
 
   private fromMissing(result: api.BatchGetDocumentsResponse): NoDocument {
@@ -673,14 +668,8 @@ export class JsonProtoSerializer {
       );
       const key = this.fromName(entityChange.document.name);
       const version = this.fromVersion(entityChange.document.updateTime);
-      const doc = new Document(
-        key,
-        version,
-        {},
-        undefined,
-        entityChange.document!,
-        v => this.fromValue(v)
-      );
+      const data = this.fromFields(entityChange.document.fields!);
+      const doc = new Document(key, version, data, {});
       const updatedTargetIds = entityChange.targetIds || [];
       const removedTargetIds = entityChange.removedTargetIds || [];
       watchChange = new DocumentWatchChange(

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

@@ -247,8 +247,11 @@ function genericLruGarbageCollectorTests(
     return new Document(
       key,
       SnapshotVersion.fromMicroseconds(1000),
-      {},
-      wrapObject({ foo: 3, bar: false })
+      wrapObject({
+        foo: 3,
+        bar: false
+      }),
+      {}
     );
   }
 
@@ -782,8 +785,11 @@ function genericLruGarbageCollectorTests(
         const doc = new Document(
           middleDocToUpdate,
           SnapshotVersion.fromMicroseconds(2000),
-          {},
-          wrapObject({ foo: 4, bar: true })
+          wrapObject({
+            foo: 4,
+            bar: true
+          }),
+          {}
         );
         return saveDocument(txn, doc).next(() => {
           return updateTargetInTransaction(txn, middleTarget);

packages/firestore/test/unit/model/mutation.test.ts

@@ -202,14 +202,9 @@ describe('Mutation', () => {
       .toBuilder()
       .set(field('foo.bar'), new ServerTimestampValue(timestamp, null))
       .build();
-    const expectedDoc = new Document(
-      key('collection/key'),
-      version(0),
-      {
-        hasLocalMutations: true
-      },
-      data
-    );
+    const expectedDoc = new Document(key('collection/key'), version(0), data, {
+      hasLocalMutations: true
+    });
 
     expect(transformedDoc).to.deep.equal(expectedDoc);
   });

packages/firestore/test/util/helpers.ts

@@ -125,7 +125,7 @@ export function doc(
   json: JsonObject<unknown>,
   options: DocumentOptions = {}
 ): Document {
-  return new Document(key(keyStr), version(ver), options, wrapObject(json));
+  return new Document(key(keyStr), version(ver), wrapObject(json), options);
 }
 
 export function deletedDoc(