diff --git a/packages/firestore/src/local/local_serializer.ts b/packages/firestore/src/local/local_serializer.ts index a40f77cbddb..be58edecbb6 100644 --- a/packages/firestore/src/local/local_serializer.ts +++ b/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, diff --git a/packages/firestore/src/model/document.ts b/packages/firestore/src/model/document.ts index 051af3c0548..747f8bf49ef 100644 --- a/packages/firestore/src/model/document.ts +++ b/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; - 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(); - } - - 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); diff --git a/packages/firestore/src/model/mutation.ts b/packages/firestore/src/model/mutation.ts index fd8ca5beefa..ba018f5f7ce 100644 --- a/packages/firestore/src/model/mutation.ts +++ b/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 { diff --git a/packages/firestore/src/model/proto_values.ts b/packages/firestore/src/model/proto_values.ts index f7478be0422..7d0d381deeb 100644 --- a/packages/firestore/src/model/proto_values.ts +++ b/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)); } diff --git a/packages/firestore/src/remote/serializer.ts b/packages/firestore/src/remote/serializer.ts index 4edcbb8f976..7beadbe9e42 100644 --- a/packages/firestore/src/remote/serializer.ts +++ b/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( diff --git a/packages/firestore/test/unit/local/lru_garbage_collector.test.ts b/packages/firestore/test/unit/local/lru_garbage_collector.test.ts index 074146df902..19178d461ce 100644 --- a/packages/firestore/test/unit/local/lru_garbage_collector.test.ts +++ b/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); diff --git a/packages/firestore/test/unit/model/mutation.test.ts b/packages/firestore/test/unit/model/mutation.test.ts index a5d4748ca39..2b405b41a55 100644 --- a/packages/firestore/test/unit/model/mutation.test.ts +++ b/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); }); diff --git a/packages/firestore/test/util/helpers.ts b/packages/firestore/test/util/helpers.ts index 7d9babd226c..eb64bece9b5 100644 --- a/packages/firestore/test/util/helpers.ts +++ b/packages/firestore/test/util/helpers.ts @@ -125,7 +125,7 @@ export function doc( json: JsonObject, 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(