Skip to content

Simplify Document #2685

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 1 addition & 3 deletions packages/firestore/src/local/local_serializer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
92 changes: 6 additions & 86 deletions packages/firestore/src/model/document.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
}

Expand All @@ -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}})`
);
Expand All @@ -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);
Expand Down
66 changes: 18 additions & 48 deletions packages/firestore/src/model/mutation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand All @@ -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 {
Expand Down Expand Up @@ -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(
Expand All @@ -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 {
Expand Down Expand Up @@ -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(
Expand All @@ -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 {
Expand Down
2 changes: 1 addition & 1 deletion packages/firestore/src/model/proto_values.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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));
}
Expand Down
27 changes: 8 additions & 19 deletions packages/firestore/src/remote/serializer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 } {
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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(
Expand Down
14 changes: 10 additions & 4 deletions packages/firestore/test/unit/local/lru_garbage_collector.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -247,8 +247,11 @@ function genericLruGarbageCollectorTests(
return new Document(
key,
SnapshotVersion.fromMicroseconds(1000),
{},
wrapObject({ foo: 3, bar: false })
wrapObject({
foo: 3,
bar: false
}),
{}
);
}

Expand Down Expand Up @@ -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);
Expand Down
11 changes: 3 additions & 8 deletions packages/firestore/test/unit/model/mutation.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
});
Expand Down
2 changes: 1 addition & 1 deletion packages/firestore/test/util/helpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down