diff --git a/packages/firestore/src/model/proto_values.ts b/packages/firestore/src/model/proto_values.ts index 46e57c3f413..c62bf8aa291 100644 --- a/packages/firestore/src/model/proto_values.ts +++ b/packages/firestore/src/model/proto_values.ts @@ -32,11 +32,6 @@ const ISO_TIMESTAMP_REG_EXP = new RegExp( /^\d{4}-\d\d-\d\dT\d\d:\d\d:\d\d(?:\.(\d+))?Z$/ ); -// Denotes the possible representations for timestamps in the Value type. -type ProtoTimestampValue = - | string - | { seconds?: string | number; nanos?: number }; - /** Extracts the backend's type order for the provided value. */ export function typeOrder(value: api.Value): TypeOrder { if ('nullValue' in value) { @@ -235,10 +230,7 @@ function compareNumbers(left: api.Value, right: api.Value): number { return numericComparator(leftNumber, rightNumber); } -function compareTimestamps( - left: ProtoTimestampValue, - right: ProtoTimestampValue -): number { +function compareTimestamps(left: api.Timestamp, right: api.Timestamp): number { 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. @@ -376,7 +368,7 @@ function canonifyByteString(byteString: string | Uint8Array): string { return normalizeByteString(byteString).toBase64(); } -function canonifyTimestamp(timestamp: ProtoTimestampValue): string { +function canonifyTimestamp(timestamp: api.Timestamp): string { const normalizedTimestamp = normalizeTimestamp(timestamp); return `time(${normalizedTimestamp.seconds},${normalizedTimestamp.nanos})`; } @@ -478,7 +470,7 @@ function estimateArrayByteSize(arrayValue: api.ArrayValue): number { * nanos" representation. */ export function normalizeTimestamp( - date: ProtoTimestampValue + date: api.Timestamp ): { seconds: number; nanos: number } { assert(!!date, 'Cannot normalize null or undefined timestamp.'); diff --git a/packages/firestore/src/protos/firestore_proto_api.d.ts b/packages/firestore/src/protos/firestore_proto_api.d.ts index 341d285acdb..04b8c553e84 100644 --- a/packages/firestore/src/protos/firestore_proto_api.d.ts +++ b/packages/firestore/src/protos/firestore_proto_api.d.ts @@ -26,6 +26,9 @@ export declare type PromiseRequestService = any; export interface ApiClientObjectMap { [k: string]: T; } +export declare type Timestamp = + | string + | { seconds?: string | number; nanos?: number }; export declare type CompositeFilterOp = 'OPERATOR_UNSPECIFIED' | 'AND'; export interface ICompositeFilterOpEnum { @@ -180,7 +183,7 @@ export declare namespace firestoreV1ApiClientInterfaces { name?: string; fields?: ApiClientObjectMap; createTime?: string; - updateTime?: string; + updateTime?: Timestamp; } interface DocumentChange { document?: Document; @@ -190,7 +193,7 @@ export declare namespace firestoreV1ApiClientInterfaces { interface DocumentDelete { document?: string; removedTargetIds?: number[]; - readTime?: string; + readTime?: Timestamp; } interface DocumentMask { fieldPaths?: string[]; @@ -290,7 +293,7 @@ export declare namespace firestoreV1ApiClientInterfaces { } interface Precondition { exists?: boolean; - updateTime?: string; + updateTime?: Timestamp; } interface Projection { fields?: FieldReference[]; @@ -333,12 +336,12 @@ export declare namespace firestoreV1ApiClientInterfaces { startAt?: Cursor; endAt?: Cursor; offset?: number; - limit?: number; + limit?: number | { value: number }; } interface Target { query?: QueryTarget; documents?: DocumentsTarget; - resumeToken?: string; + resumeToken?: string | Uint8Array; readTime?: string; targetId?: number; once?: boolean; @@ -347,8 +350,8 @@ export declare namespace firestoreV1ApiClientInterfaces { targetChangeType?: TargetChangeTargetChangeType; targetIds?: number[]; cause?: Status; - resumeToken?: string; - readTime?: string; + resumeToken?: string | Uint8Array; + readTime?: Timestamp; } interface TransactionOptions { readOnly?: ReadOnly; @@ -363,7 +366,7 @@ export declare namespace firestoreV1ApiClientInterfaces { booleanValue?: boolean; integerValue?: string | number; doubleValue?: string | number; - timestampValue?: string | { seconds?: string | number; nanos?: number }; + timestampValue?: Timestamp; stringValue?: string; bytesValue?: string | Uint8Array; referenceValue?: string; @@ -382,17 +385,17 @@ export declare namespace firestoreV1ApiClientInterfaces { interface WriteRequest { streamId?: string; writes?: Write[]; - streamToken?: string; + streamToken?: string | Uint8Array; labels?: ApiClientObjectMap; } interface WriteResponse { streamId?: string; streamToken?: string; writeResults?: WriteResult[]; - commitTime?: string; + commitTime?: Timestamp; } interface WriteResult { - updateTime?: string; + updateTime?: Timestamp; transformResults?: Value[]; } } diff --git a/packages/firestore/src/remote/serializer.ts b/packages/firestore/src/remote/serializer.ts index c76837e3497..3c915eef4e1 100644 --- a/packages/firestore/src/remote/serializer.ts +++ b/packages/firestore/src/remote/serializer.ts @@ -54,7 +54,7 @@ import { assert, fail } from '../util/assert'; import { Code, FirestoreError } from '../util/error'; import * as obj from '../util/obj'; import { ByteString } from '../util/byte_string'; -import * as typeUtils from '../util/types'; +import { isNegativeZero, isNullOrUndefined } from '../util/types'; import { ArrayRemoveTransformOperation, @@ -99,14 +99,7 @@ const OPERATORS = (() => { })(); function assertPresent(value: unknown, description: string): asserts value { - assert(!typeUtils.isNullOrUndefined(value), description + ' is missing'); -} - -// This is a supplement to the generated proto interfaces, which fail to account -// for the fact that a timestamp may be encoded as either a string OR this. -interface TimestampProto { - seconds?: string | number; - nanos?: number; + assert(!isNullOrUndefined(value), description + ' is missing'); } export interface SerializerOptions { @@ -148,46 +141,33 @@ export class JsonProtoSerializer { * our generated proto interfaces say Int32Value must be. But GRPC actually * expects a { value: } struct. */ - private toInt32Value(val: number | null): number | null { - if (this.options.useProto3Json || typeUtils.isNullOrUndefined(val)) { + private toInt32Proto(val: number | null): number | { value: number } | null { + if (this.options.useProto3Json || isNullOrUndefined(val)) { return val; } else { - // ProtobufJS requires that we wrap Int32Values. - // Use any because we need to match generated Proto types. - // eslint-disable-next-line @typescript-eslint/no-explicit-any - return { value: val } as any; + return { value: val }; } } /** * Returns a number (or null) from a google.protobuf.Int32Value proto. - * DO NOT USE THIS FOR ANYTHING ELSE. - * This method cheats. It's typed as accepting "number" because that's what - * our generated proto interfaces say Int32Value must be, but it actually - * accepts { value: number } to match our serialization in toInt32Value(). */ - private fromInt32Value(val: number | undefined): number | null { + private fromInt32Proto( + val: number | { value: number } | undefined + ): number | null { let result; if (typeof val === 'object') { - // Use any because we need to match generated Proto types. - // eslint-disable-next-line @typescript-eslint/no-explicit-any - result = (val as any).value; + result = val.value; } else { - // We accept raw numbers (without the {value: ... } wrapper) for - // compatibility with legacy persisted data. result = val; } - return typeUtils.isNullOrUndefined(result) ? null : result; + return isNullOrUndefined(result) ? null : result; } /** * Returns a value for a Date that's appropriate to put into a proto. - * DO NOT USE THIS FOR ANYTHING ELSE. - * This method cheats. It's typed as returning "string" because that's what - * our generated proto interfaces say dates must be. But it's easier and safer - * to actually return a Timestamp proto. */ - private toTimestamp(timestamp: Timestamp): string { + private toTimestamp(timestamp: Timestamp): api.Timestamp { if (this.options.useProto3Json) { // Serialize to ISO-8601 date format, but with full nano resolution. // Since JS Date has only millis, let's only use it for the seconds and @@ -208,25 +188,21 @@ export class JsonProtoSerializer { } } - private fromTimestamp(date: string | TimestampProto): Timestamp { + private fromTimestamp(date: api.Timestamp): Timestamp { const timestamp = normalizeTimestamp(date); return new Timestamp(timestamp.seconds, timestamp.nanos); } /** * Returns a value for bytes that's appropriate to put in a proto. - * DO NOT USE THIS FOR ANYTHING ELSE. - * This method cheats. It's typed as returning "string" because that's what - * our generated proto interfaces say bytes must be. But it should return - * an Uint8Array in Node. * * Visible for testing. */ - toBytes(bytes: Blob | ByteString): string { + toBytes(bytes: Blob | ByteString): string | Uint8Array { if (this.options.useProto3Json) { return bytes.toBase64(); } else { - return (bytes.toUint8Array() as unknown) as string; + return bytes.toUint8Array(); } } @@ -249,11 +225,11 @@ export class JsonProtoSerializer { } } - toVersion(version: SnapshotVersion): string { + toVersion(version: SnapshotVersion): api.Timestamp { return this.toTimestamp(version.toTimestamp()); } - fromVersion(version: string): SnapshotVersion { + fromVersion(version: api.Timestamp): SnapshotVersion { assert(!!version, "Trying to deserialize version that isn't set"); return SnapshotVersion.fromTimestamp(this.fromTimestamp(version)); } @@ -374,7 +350,7 @@ export class JsonProtoSerializer { return { doubleValue: 'Infinity' } as {}; } else if (doubleValue === -Infinity) { return { doubleValue: '-Infinity' } as {}; - } else if (typeUtils.isNegativeZero(doubleValue)) { + } else if (isNegativeZero(doubleValue)) { return { doubleValue: '-0' } as {}; } } @@ -846,7 +822,7 @@ export class JsonProtoSerializer { private fromWriteResult( proto: api.WriteResult, - commitTime: string + commitTime: api.Timestamp ): MutationResult { // NOTE: Deletes don't have an updateTime. let version = proto.updateTime @@ -873,7 +849,7 @@ export class JsonProtoSerializer { fromWriteResults( protos: api.WriteResult[] | undefined, - commitTime?: string + commitTime?: api.Timestamp ): MutationResult[] { if (protos && protos.length > 0) { assert( @@ -1000,7 +976,7 @@ export class JsonProtoSerializer { result.structuredQuery!.orderBy = orderBy; } - const limit = this.toInt32Value(target.limit); + const limit = this.toInt32Proto(target.limit); if (limit !== null) { result.structuredQuery!.limit = limit; } @@ -1046,7 +1022,7 @@ export class JsonProtoSerializer { let limit: number | null = null; if (query.limit) { - limit = this.fromInt32Value(query.limit); + limit = this.fromInt32Proto(query.limit); } let startAt: Bound | null = null; diff --git a/packages/firestore/src/util/types.ts b/packages/firestore/src/util/types.ts index 3457371ae11..c1939d7739c 100644 --- a/packages/firestore/src/util/types.ts +++ b/packages/firestore/src/util/types.ts @@ -23,12 +23,12 @@ export interface StringMap { /** * Returns whether a variable is either undefined or null. */ -export function isNullOrUndefined(value: unknown): boolean { +export function isNullOrUndefined(value: unknown): value is null | undefined { return value === null || value === undefined; } /** Returns whether the value represents -0. */ -export function isNegativeZero(value: number) : boolean { +export function isNegativeZero(value: number): boolean { // Detect if the value is -0.0. Based on polyfill from // https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Object/is return value === -0 && 1 / value === 1 / -0; diff --git a/packages/firestore/test/unit/specs/spec_test_runner.ts b/packages/firestore/test/unit/specs/spec_test_runner.ts index 29c7df3a7c6..6ffdb0969e6 100644 --- a/packages/firestore/test/unit/specs/spec_test_runner.ts +++ b/packages/firestore/test/unit/specs/spec_test_runner.ts @@ -196,7 +196,10 @@ class MockConnection implements Connection { return this.watchOpen.promise; } - ackWrite(commitTime?: string, mutationResults?: api.WriteResult[]): void { + ackWrite( + commitTime?: api.Timestamp, + mutationResults?: api.WriteResult[] + ): void { this.writeStream!.callOnMessage({ // Convert to base64 string so it can later be parsed into ByteString. streamToken: PlatformSupport.getPlatform().btoa( @@ -1151,7 +1154,7 @@ abstract class TestRunner { expect(actualTarget.query).to.deep.equal(expectedTarget.query); expect(actualTarget.targetId).to.equal(expectedTarget.targetId); expect(actualTarget.readTime).to.equal(expectedTarget.readTime); - expect(actualTarget.resumeToken || '').to.equal(expectedTarget.resumeToken || ''); + expect(actualTarget.resumeToken).to.equal(expectedTarget.resumeToken); delete actualTargets[targetId]; }); expect(obj.size(actualTargets)).to.equal(