From 8a276f2d36436fd7e3683e9b72e84b871b3f8e61 Mon Sep 17 00:00:00 2001 From: Sebastian Schmidt Date: Mon, 26 Aug 2019 16:21:58 -0700 Subject: [PATCH 1/6] Don't deserialize full Document proto for Query execution --- packages/firestore/CHANGELOG.md | 3 +- packages/firestore/src/api/database.ts | 8 +- packages/firestore/src/core/query.ts | 2 +- packages/firestore/src/core/view.ts | 2 +- packages/firestore/src/model/document.ts | 91 ++++++++++++++++--- packages/firestore/src/model/mutation.ts | 72 ++++++++++----- packages/firestore/src/remote/serializer.ts | 29 +++--- .../unit/local/lru_garbage_collector.test.ts | 8 +- .../test/unit/model/mutation.test.ts | 11 ++- .../test/unit/remote/node/serializer.test.ts | 2 +- .../test/unit/specs/query_spec.test.ts | 6 +- .../firestore/test/unit/specs/spec_builder.ts | 2 +- packages/firestore/test/util/helpers.ts | 2 +- 13 files changed, 173 insertions(+), 65 deletions(-) diff --git a/packages/firestore/CHANGELOG.md b/packages/firestore/CHANGELOG.md index 697d582dc11..64000281efe 100644 --- a/packages/firestore/CHANGELOG.md +++ b/packages/firestore/CHANGELOG.md @@ -1,5 +1,6 @@ - # Unreleased +- [changed] Improved performance for queries with filters that only return a + small subset of the documents in a collection. - [fixed] Fixed an internal assertion that was triggered when an update with a `FieldValue.serverTimestamp()` and an update with a `FieldValue.increment()` were pending for the same document. diff --git a/packages/firestore/src/api/database.ts b/packages/firestore/src/api/database.ts index 90b9d923c6b..5ca59cc8e28 100644 --- a/packages/firestore/src/api/database.ts +++ b/packages/firestore/src/api/database.ts @@ -1311,7 +1311,7 @@ export class DocumentSnapshot implements firestore.DocumentSnapshot { return !this._document ? undefined : this.convertObject( - this._document.data, + this._document.data(), FieldValueOptions.fromSnapshotOptions( options, this._firestore._areTimestampsInSnapshotsEnabled() @@ -1326,9 +1326,9 @@ export class DocumentSnapshot implements firestore.DocumentSnapshot { 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 !== null) { return this.convertValue( value, diff --git a/packages/firestore/src/core/query.ts b/packages/firestore/src/core/query.ts index 295551aa0b7..37187efcaf0 100644 --- a/packages/firestore/src/core/query.ts +++ b/packages/firestore/src/core/query.ts @@ -759,7 +759,7 @@ export class Bound { } else { const docValue = doc.field(orderByComponent.field); assert( - docValue !== undefined, + docValue !== null, 'Field should exist since document matched the orderBy already.' ); comparison = component.compareTo(docValue!); diff --git a/packages/firestore/src/core/view.ts b/packages/firestore/src/core/view.ts index c57b2231341..1a3df0457d3 100644 --- a/packages/firestore/src/core/view.ts +++ b/packages/firestore/src/core/view.ts @@ -169,7 +169,7 @@ export class View { // Calculate change if (oldDoc && newDoc) { - const docsEqual = oldDoc.data.isEqual(newDoc.data); + const docsEqual = oldDoc.data().isEqual(newDoc.data()); if (!docsEqual) { if (!this.shouldWaitForSyncedDocument(oldDoc, newDoc)) { changeSet.track({ diff --git a/packages/firestore/src/model/document.ts b/packages/firestore/src/model/document.ts index 822002b246a..78136176602 100644 --- a/packages/firestore/src/model/document.ts +++ b/packages/firestore/src/model/document.ts @@ -16,13 +16,14 @@ */ import { SnapshotVersion } from '../core/snapshot_version'; -import { fail } from '../util/assert'; +import { assert, 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; @@ -59,16 +60,19 @@ 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, - readonly data: ObjectValue, options: DocumentOptions, - /** - * Memoized serialized form of the document for optimization purposes (avoids repeated - * serialization). Might be undefined. - */ - readonly proto?: api.Document + private objectValue?: ObjectValue, + readonly proto?: api.Document, + private readonly converter?: (value: api.Value) => FieldValue ) { super(key, version); this.hasLocalMutations = !!options.hasLocalMutations; @@ -76,7 +80,51 @@ export class Document extends MaybeDocument { } field(path: FieldPath): FieldValue | null { - return this.data.field(path); + if (this.objectValue) { + return this.objectValue.field(path); + } else { + assert( + this.proto !== undefined && this.converter !== undefined, + 'Expected proto and converter to be defined.' + ); + + 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. + let protoValue: api.Value | undefined = this.proto!.fields[ + path.firstSegment() + ]; + for (let i = 1; protoValue !== undefined && i < path.length; ++i) { + if (protoValue.mapValue) { + protoValue = protoValue.mapValue.fields[path.get(i)]; + } else { + protoValue = undefined; + } + } + + if (protoValue === undefined) { + fieldValue = null; + } else { + fieldValue = this.converter!(protoValue); + } + + this.fieldValueCache.set(canonicalPath, fieldValue); + } + + return fieldValue!; + } } fieldValue(path: FieldPath): unknown { @@ -84,8 +132,29 @@ export class Document extends MaybeDocument { return field ? field.value() : undefined; } + data(): ObjectValue { + if (!this.objectValue) { + assert( + this.proto !== undefined && this.converter !== undefined, + 'Expected proto and converter to be defined.' + ); + + let result = ObjectValue.EMPTY; + obj.forEach(this.proto!.fields, (key: string, value: api.Value) => { + result = result.set(new FieldPath([key]), this.converter!(value)); + }); + this.objectValue = result; + + // Once objectValue is computed, values inside the fieldValueCache are no + // longer accessed. + this.fieldValueCache = undefined; + } + + return this.objectValue; + } + value(): JsonObject { - return this.data.value(); + return this.data().value(); } isEqual(other: MaybeDocument | null | undefined): boolean { @@ -93,9 +162,9 @@ export class Document extends MaybeDocument { other instanceof Document && this.key.isEqual(other.key) && this.version.isEqual(other.version) && - this.data.isEqual(other.data) && this.hasLocalMutations === other.hasLocalMutations && - this.hasCommittedMutations === other.hasCommittedMutations + this.hasCommittedMutations === other.hasCommittedMutations && + this.data().isEqual(other.data()) ); } diff --git a/packages/firestore/src/model/mutation.ts b/packages/firestore/src/model/mutation.ts index e0aedb3b334..92a752cf45c 100644 --- a/packages/firestore/src/model/mutation.ts +++ b/packages/firestore/src/model/mutation.ts @@ -353,9 +353,14 @@ export class SetMutation extends Mutation { // have held. const version = mutationResult.version; - return new Document(this.key, version, this.value, { - hasCommittedMutations: true - }); + return new Document( + this.key, + version, + { + hasCommittedMutations: true + }, + this.value + ); } applyToLocalView( @@ -370,9 +375,14 @@ export class SetMutation extends Mutation { } const version = Mutation.getPostMutationVersion(maybeDoc); - return new Document(this.key, version, this.value, { - hasLocalMutations: true - }); + return new Document( + this.key, + version, + { + hasLocalMutations: true + }, + this.value + ); } extractBaseValue(maybeDoc: MaybeDocument | null): null { @@ -434,9 +444,14 @@ export class PatchMutation extends Mutation { } const newData = this.patchDocument(maybeDoc); - return new Document(this.key, mutationResult.version, newData, { - hasCommittedMutations: true - }); + return new Document( + this.key, + mutationResult.version, + { + hasCommittedMutations: true + }, + newData + ); } applyToLocalView( @@ -452,9 +467,14 @@ export class PatchMutation extends Mutation { const version = Mutation.getPostMutationVersion(maybeDoc); const newData = this.patchDocument(maybeDoc); - return new Document(this.key, version, newData, { - hasLocalMutations: true - }); + return new Document( + this.key, + version, + { + hasLocalMutations: true + }, + newData + ); } extractBaseValue(maybeDoc: MaybeDocument | null): null { @@ -478,7 +498,7 @@ export class PatchMutation extends Mutation { private patchDocument(maybeDoc: MaybeDocument | null): ObjectValue { let data: ObjectValue; if (maybeDoc instanceof Document) { - data = maybeDoc.data; + data = maybeDoc.data(); } else { data = ObjectValue.EMPTY; } @@ -550,10 +570,15 @@ export class TransformMutation extends Mutation { ); const version = mutationResult.version; - const newData = this.transformObject(doc.data, transformResults); - return new Document(this.key, version, newData, { - hasCommittedMutations: true - }); + const newData = this.transformObject(doc.data(), transformResults); + return new Document( + this.key, + version, + { + hasCommittedMutations: true + }, + newData + ); } applyToLocalView( @@ -573,10 +598,15 @@ export class TransformMutation extends Mutation { maybeDoc, baseDoc ); - const newData = this.transformObject(doc.data, transformResults); - return new Document(this.key, doc.version, newData, { - hasLocalMutations: true - }); + const newData = this.transformObject(doc.data(), transformResults); + return new Document( + this.key, + doc.version, + { + hasLocalMutations: true + }, + newData + ); } extractBaseValue(maybeDoc: MaybeDocument | null): ObjectValue | null { diff --git a/packages/firestore/src/remote/serializer.ts b/packages/firestore/src/remote/serializer.ts index 0d5aa0e89b7..0018c65cd24 100644 --- a/packages/firestore/src/remote/serializer.ts +++ b/packages/firestore/src/remote/serializer.ts @@ -519,7 +519,7 @@ export class JsonProtoSerializer { ); return { name: this.toName(document.key), - fields: this.toFields(document.data), + fields: this.toFields(document.data()), updateTime: this.toTimestamp(document.version.toTimestamp()) }; } @@ -528,11 +528,15 @@ export class JsonProtoSerializer { document: api.Document, hasCommittedMutations?: boolean ): Document { + const key = this.fromName(document.name!); + const version = this.fromVersion(document.updateTime!); return new Document( - this.fromName(document.name!), - this.fromVersion(document.updateTime!), - this.fromFields(document.fields || {}), - { hasCommittedMutations: !!hasCommittedMutations } + key, + version, + { hasCommittedMutations: !!hasCommittedMutations }, + undefined, + document, + v => this.fromValue(v) ); } @@ -577,8 +581,9 @@ export class JsonProtoSerializer { assertPresent(doc.found!.updateTime, 'doc.found.updateTime'); const key = this.fromName(doc.found!.name!); const version = this.fromVersion(doc.found!.updateTime!); - const fields = this.fromFields(doc.found!.fields || {}); - return new Document(key, version, fields, {}, doc.found!); + return new Document(key, version, {}, undefined, doc.found!, v => + this.fromValue(v) + ); } private fromMissing(result: api.BatchGetDocumentsResponse): NoDocument { @@ -639,7 +644,7 @@ export class JsonProtoSerializer { documentChange: { document: { name: this.toName(doc.key), - fields: this.toFields(doc.data), + fields: this.toFields(doc.data()), updateTime: this.toVersion(doc.version) }, targetIds: watchChange.updatedTargetIds, @@ -718,15 +723,13 @@ export class JsonProtoSerializer { const entityChange = change.documentChange!; const key = this.fromName(entityChange.document!.name!); const version = this.fromVersion(entityChange.document!.updateTime!); - const fields = this.fromFields(entityChange.document!.fields || {}); - // The document may soon be re-serialized back to protos in order to store it in local - // persistence. Memoize the encoded form to avoid encoding it again. const doc = new Document( key, version, - fields, {}, - entityChange.document! + undefined, + entityChange.document!, + v => this.fromValue(v) ); const updatedTargetIds = entityChange.targetIds || []; const removedTargetIds = entityChange.removedTargetIds || []; 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 693956f5444..51305abfc87 100644 --- a/packages/firestore/test/unit/local/lru_garbage_collector.test.ts +++ b/packages/firestore/test/unit/local/lru_garbage_collector.test.ts @@ -243,8 +243,8 @@ function genericLruGarbageCollectorTests( return new Document( key, SnapshotVersion.fromMicroseconds(1000), - wrapObject({ foo: 3, bar: false }), - {} + {}, + wrapObject({ foo: 3, bar: false }) ); } @@ -774,8 +774,8 @@ 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 657d7dd2ba6..e601e226212 100644 --- a/packages/firestore/test/unit/model/mutation.test.ts +++ b/packages/firestore/test/unit/model/mutation.test.ts @@ -199,9 +199,14 @@ describe('Mutation', () => { foo: { bar: '' }, baz: 'baz-value' }).set(field('foo.bar'), new ServerTimestampValue(timestamp, null)); - const expectedDoc = new Document(key('collection/key'), version(0), data, { - hasLocalMutations: true - }); + const expectedDoc = new Document( + key('collection/key'), + version(0), + { + hasLocalMutations: true + }, + data + ); expect(transformedDoc).to.deep.equal(expectedDoc); }); diff --git a/packages/firestore/test/unit/remote/node/serializer.test.ts b/packages/firestore/test/unit/remote/node/serializer.test.ts index 75cbc6cec52..d8b13dc4dd2 100644 --- a/packages/firestore/test/unit/remote/node/serializer.test.ts +++ b/packages/firestore/test/unit/remote/node/serializer.test.ts @@ -675,7 +675,7 @@ describe('Serializer', () => { const d = doc('foo/bar', 42, { a: 5, b: 'b' }); const proto = { name: s.toName(d.key), - fields: s.toFields(d.data), + fields: s.toFields(d.data()), updateTime: s.toVersion(d.version) }; const serialized = s.toDocument(d); diff --git a/packages/firestore/test/unit/specs/query_spec.test.ts b/packages/firestore/test/unit/specs/query_spec.test.ts index 53f05c65425..5722a916a33 100644 --- a/packages/firestore/test/unit/specs/query_spec.test.ts +++ b/packages/firestore/test/unit/specs/query_spec.test.ts @@ -82,9 +82,9 @@ describeSpec('Queries:', [], () => { ); return specWithCachedDocs(...cachedDocs) - .userSets(toWrite1.key.toString(), toWrite1.data.value()) - .userSets(toWrite2.key.toString(), toWrite2.data.value()) - .userSets(toWrite3.key.toString(), toWrite3.data.value()) + .userSets(toWrite1.key.toString(), toWrite1.data().value()) + .userSets(toWrite2.key.toString(), toWrite2.data().value()) + .userSets(toWrite3.key.toString(), toWrite3.data().value()) .userListens(cgQuery) .expectEvents(cgQuery, { added: [cachedDocs[0], toWrite1, toWrite2], diff --git a/packages/firestore/test/unit/specs/spec_builder.ts b/packages/firestore/test/unit/specs/spec_builder.ts index 7773920c666..fc3d55c58de 100644 --- a/packages/firestore/test/unit/specs/spec_builder.ts +++ b/packages/firestore/test/unit/specs/spec_builder.ts @@ -882,7 +882,7 @@ export class SpecBuilder { return { key: SpecBuilder.keyToSpec(doc.key), version: doc.version.toMicroseconds(), - value: doc.data.value(), + value: doc.data().value(), options: { hasLocalMutations: doc.hasLocalMutations, hasCommittedMutations: doc.hasCommittedMutations diff --git a/packages/firestore/test/util/helpers.ts b/packages/firestore/test/util/helpers.ts index 68d65a50475..f76554d4ff9 100644 --- a/packages/firestore/test/util/helpers.ts +++ b/packages/firestore/test/util/helpers.ts @@ -122,7 +122,7 @@ export function doc( json: JsonObject, options: DocumentOptions = {} ): Document { - return new Document(key(keyStr), version(ver), wrapObject(json), options); + return new Document(key(keyStr), version(ver), options, wrapObject(json)); } export function deletedDoc( From f318dcd67dbaa636954ef719fae2c0b8a157b34d Mon Sep 17 00:00:00 2001 From: Sebastian Schmidt Date: Wed, 28 Aug 2019 16:10:12 -0700 Subject: [PATCH 2/6] Address feeback --- packages/firestore/src/model/document.ts | 55 ++++++++++--------- .../test/unit/model/document.test.ts | 4 +- 2 files changed, 30 insertions(+), 29 deletions(-) diff --git a/packages/firestore/src/model/document.ts b/packages/firestore/src/model/document.ts index 78136176602..9fcd2e6bbf8 100644 --- a/packages/firestore/src/model/document.ts +++ b/packages/firestore/src/model/document.ts @@ -75,6 +75,12 @@ export class Document extends MaybeDocument { private readonly converter?: (value: api.Value) => FieldValue ) { 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; } @@ -83,11 +89,6 @@ export class Document extends MaybeDocument { if (this.objectValue) { return this.objectValue.field(path); } else { - assert( - this.proto !== undefined && this.converter !== undefined, - 'Expected proto and converter to be defined.' - ); - if (!this.fieldValueCache) { // TODO(b/136090445): Remove the cache when `getField` is no longer // called during Query ordering. @@ -103,23 +104,12 @@ export class Document extends MaybeDocument { // deserialize the value at the requested field path. This speeds up // Query execution as query filters can discard documents based on a // single field. - let protoValue: api.Value | undefined = this.proto!.fields[ - path.firstSegment() - ]; - for (let i = 1; protoValue !== undefined && i < path.length; ++i) { - if (protoValue.mapValue) { - protoValue = protoValue.mapValue.fields[path.get(i)]; - } else { - protoValue = undefined; - } - } - + const protoValue = this.getProtoField(path); if (protoValue === undefined) { fieldValue = null; } else { fieldValue = this.converter!(protoValue); } - this.fieldValueCache.set(canonicalPath, fieldValue); } @@ -127,18 +117,8 @@ export class Document extends MaybeDocument { } } - fieldValue(path: FieldPath): unknown { - const field = this.field(path); - return field ? field.value() : undefined; - } - data(): ObjectValue { if (!this.objectValue) { - assert( - this.proto !== undefined && this.converter !== undefined, - 'Expected proto and converter to be defined.' - ); - let result = ObjectValue.EMPTY; obj.forEach(this.proto!.fields, (key: string, value: api.Value) => { result = result.set(new FieldPath([key]), this.converter!(value)); @@ -180,6 +160,27 @@ export class Document extends MaybeDocument { return this.hasLocalMutations || this.hasCommittedMutations; } + /** + * Returns a 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[ + path.firstSegment() + ]; + for (let i = 1; i < path.length; ++i) { + if (!protoValue || !protoValue.mapValue) { + 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/test/unit/model/document.test.ts b/packages/firestore/test/unit/model/document.test.ts index 967925d6202..f42b74a33fb 100644 --- a/packages/firestore/test/unit/model/document.test.ts +++ b/packages/firestore/test/unit/model/document.test.ts @@ -42,10 +42,10 @@ describe('Document', () => { }; const document = doc('rooms/Eros', 1, data, { hasLocalMutations: true }); - expect(document.fieldValue(field('desc'))).to.deep.equal( + expect(document.field(field('desc'))!.value()).to.deep.equal( 'Discuss all the project related stuff' ); - expect(document.fieldValue(field('owner.title'))).to.deep.equal( + expect(document.field(field('owner.title'))!.value()).to.deep.equal( 'scallywag' ); expect(document.hasLocalMutations).to.equal(true); From 91c228d7594dce8658f14df08bf6a040f42491ea Mon Sep 17 00:00:00 2001 From: Sebastian Schmidt Date: Wed, 28 Aug 2019 16:13:00 -0700 Subject: [PATCH 3/6] [AUTOMATED]: Prettier Code Styling --- packages/firestore/src/model/document.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/firestore/src/model/document.ts b/packages/firestore/src/model/document.ts index 9fcd2e6bbf8..10f8949b688 100644 --- a/packages/firestore/src/model/document.ts +++ b/packages/firestore/src/model/document.ts @@ -167,7 +167,7 @@ export class Document extends MaybeDocument { private getProtoField(path: FieldPath): api.Value | undefined { assert( this.proto !== undefined, - "Can only call getProtoField() when proto is defined" + 'Can only call getProtoField() when proto is defined' ); let protoValue: api.Value | undefined = this.proto!.fields[ path.firstSegment() From 69e402b8a552423fc9dea88ba92284e272c6865f Mon Sep 17 00:00:00 2001 From: Sebastian Schmidt Date: Wed, 28 Aug 2019 17:26:23 -0700 Subject: [PATCH 4/6] Comments --- packages/firestore/src/model/document.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/packages/firestore/src/model/document.ts b/packages/firestore/src/model/document.ts index 10f8949b688..af2ae9e91a7 100644 --- a/packages/firestore/src/model/document.ts +++ b/packages/firestore/src/model/document.ts @@ -161,7 +161,7 @@ export class Document extends MaybeDocument { } /** - * Returns a the nested Protobuf value for 'path`. Can only be called if + * 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 { @@ -169,6 +169,7 @@ export class Document extends MaybeDocument { this.proto !== undefined, 'Can only call getProtoField() when proto is defined' ); + let protoValue: api.Value | undefined = this.proto!.fields[ path.firstSegment() ]; @@ -178,6 +179,7 @@ export class Document extends MaybeDocument { } protoValue = protoValue.mapValue.fields[path.get(i)]; } + return protoValue; } From 27cb4fe8acd27365ec9940b9ceb1b302e2628ba9 Mon Sep 17 00:00:00 2001 From: Sebastian Schmidt Date: Wed, 28 Aug 2019 17:26:30 -0700 Subject: [PATCH 5/6] [AUTOMATED]: Prettier Code Styling --- packages/firestore/src/model/document.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/firestore/src/model/document.ts b/packages/firestore/src/model/document.ts index af2ae9e91a7..f04759ddb6e 100644 --- a/packages/firestore/src/model/document.ts +++ b/packages/firestore/src/model/document.ts @@ -179,7 +179,7 @@ export class Document extends MaybeDocument { } protoValue = protoValue.mapValue.fields[path.get(i)]; } - + return protoValue; } From 1ded5b14ef4cffdf3d9bca9ecb601c3b759b7e1e Mon Sep 17 00:00:00 2001 From: Sebastian Schmidt Date: Wed, 28 Aug 2019 18:00:06 -0700 Subject: [PATCH 6/6] Fix test --- packages/firestore/test/integration/remote/remote.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/firestore/test/integration/remote/remote.test.ts b/packages/firestore/test/integration/remote/remote.test.ts index f4581bec536..a305f8cb4bd 100644 --- a/packages/firestore/test/integration/remote/remote.test.ts +++ b/packages/firestore/test/integration/remote/remote.test.ts @@ -59,7 +59,7 @@ describe('Remote Storage', () => { const doc = docs[0]; expect(doc).to.be.an.instanceof(Document); if (doc instanceof Document) { - expect(doc.data).to.deep.equal(mutation.value); + expect(doc.data()).to.deep.equal(mutation.value); expect(doc.key).to.deep.equal(k); expect(SnapshotVersion.MIN.compareTo(doc.version)).to.be.lessThan( 0