From 114de25d1af3fc7441da3242f6bc8cc8354ffa09 Mon Sep 17 00:00:00 2001 From: Sebastian Schmidt Date: Tue, 12 Mar 2019 09:21:55 +0800 Subject: [PATCH] deps/refactor: Removing @google-cloud/projectify (#564) --- dev/conformance/runner.ts | 26 ++-- dev/src/document.ts | 30 ++-- dev/src/index.ts | 226 +++++++++++------------------ dev/src/order.ts | 7 +- dev/src/path.ts | 268 ++++++++++++++++++++++------------- dev/src/reference.ts | 191 ++++++++++++++----------- dev/src/serializer.ts | 8 +- dev/src/timestamp.ts | 4 +- dev/src/validate.ts | 4 +- dev/src/watch.ts | 41 +++--- dev/src/write-batch.ts | 140 +++++++++++------- dev/system-test/firestore.ts | 6 +- dev/test/index.ts | 116 +++++++-------- dev/test/order.ts | 150 ++++++++++---------- dev/test/path.ts | 23 +-- dev/test/query.ts | 11 +- dev/test/transaction.ts | 2 + dev/test/util/helpers.ts | 26 ++-- dev/test/watch.ts | 61 ++++---- package.json | 5 +- 20 files changed, 712 insertions(+), 633 deletions(-) diff --git a/dev/conformance/runner.ts b/dev/conformance/runner.ts index b3eb056a5..890c91ddc 100644 --- a/dev/conformance/runner.ts +++ b/dev/conformance/runner.ts @@ -26,7 +26,7 @@ import * as proto from '../protos/firestore_proto_api'; import {DocumentChange, DocumentSnapshot, FieldPath, FieldValue, Firestore, Query, QueryDocumentSnapshot, QuerySnapshot, Timestamp} from '../src'; import {fieldsFromJson} from '../src/convert'; import {DocumentChangeType} from '../src/document-change'; -import {ResourcePath} from '../src/path'; +import {QualifiedResourcePath} from '../src/path'; import {DocumentData} from '../src/types'; import {isObject} from '../src/util'; import {ApiOverride, createInstance as createInstanceHelper} from '../test/util/helpers'; @@ -68,13 +68,13 @@ const COMMIT_REQUEST_TYPE = let firestore: Firestore; const docRef = (path: string) => { - const relativePath = ResourcePath.fromSlashSeparatedString(path).relativeName; - return firestore.doc(relativePath); + const resourcePath = QualifiedResourcePath.fromSlashSeparatedString(path); + return firestore.doc(resourcePath.relativeName); }; const collRef = (path: string) => { - const relativePath = ResourcePath.fromSlashSeparatedString(path).relativeName; - return firestore.collection(relativePath); + const resourcePath = QualifiedResourcePath.fromSlashSeparatedString(path); + return firestore.collection(resourcePath.relativeName); }; const watchQuery = () => { @@ -287,7 +287,7 @@ function runTest(spec: ConformanceProto) { varargs[0] = convertInput.argument(spec.jsonData); } else { for (let i = 0; i < spec.fieldPaths.length; ++i) { - varargs[2 * i] = new FieldPath(spec.fieldPaths[i].field); + varargs[2 * i] = new FieldPath(...spec.fieldPaths[i].field); } for (let i = 0; i < spec.jsonValues.length; ++i) { varargs[2 * i + 1] = convertInput.argument(spec.jsonValues[i]); @@ -324,15 +324,17 @@ function runTest(spec: ConformanceProto) { } else if (clause.limit) { query = query.limit(clause.limit); } else if (clause.startAt) { - query = query.startAt.apply(query, convertInput.cursor(clause.startAt)); + const cursor = convertInput.cursor(clause.startAt); + query = query.startAt(...cursor); } else if (clause.startAfter) { - query = query.startAfter.apply( - query, convertInput.cursor(clause.startAfter)); + const cursor = convertInput.cursor(clause.startAfter); + query = query.startAfter(...cursor); } else if (clause.endAt) { - query = query.endAt.apply(query, convertInput.cursor(clause.endAt)); + const cursor = convertInput.cursor(clause.endAt); + query = query.endAt(...cursor); } else if (clause.endBefore) { - query = - query.endBefore.apply(query, convertInput.cursor(clause.endBefore)); + const cursor = convertInput.cursor(clause.endBefore); + query = query.endBefore(...cursor); } return query; diff --git a/dev/src/document.ts b/dev/src/document.ts index d1747265f..aba854daf 100644 --- a/dev/src/document.ts +++ b/dev/src/document.ts @@ -153,7 +153,8 @@ export class DocumentSnapshot { * 'target'. */ function merge( - target: ApiMapValue, value: unknown, path: string[], pos: number) { + target: ApiMapValue, value: unknown, path: string[], + pos: number): ApiMapValue|null { const key = path[pos]; const isLast = pos === path.length - 1; @@ -199,10 +200,10 @@ export class DocumentSnapshot { const res: ApiMapValue = {}; - data.forEach((value, key) => { - const components = key.toArray(); - merge(res, value, components, 0); - }); + for (const [key, value] of data) { + const path = key.toArray(); + merge(res, value, path, 0); + } return new DocumentSnapshot(ref, res); } @@ -445,7 +446,7 @@ export class DocumentSnapshot { * @private * @returns The document in the format the API expects. */ - toProto(): api.IWrite|null { + toProto(): api.IWrite { return { update: { name: this._ref.formattedName, @@ -790,8 +791,8 @@ export class DocumentMask { const result = applyDocumentMask(data); if (result.remainingPaths.length !== 0) { - throw new Error(`Input data is missing for field "${ - result.remainingPaths[0].toString()}".`); + throw new Error( + `Input data is missing for field "${result.remainingPaths[0]}".`); } return result.filteredData; @@ -942,15 +943,15 @@ export class DocumentTransform { return null; } - const protoTransforms: Array<{}> = []; - this.transforms.forEach((transform, path) => { - protoTransforms.push(transform.toProto(serializer, path)); - }); + const fieldTransforms: api.DocumentTransform.IFieldTransform[] = []; + for (const [path, transform] of this.transforms) { + fieldTransforms.push(transform.toProto(serializer, path)); + } return { transform: { document: this.ref.formattedName, - fieldTransforms: protoTransforms, + fieldTransforms, }, }; } @@ -998,7 +999,8 @@ export class Precondition { const proto: api.IPrecondition = {}; if (this._lastUpdateTime !== undefined) { - proto.updateTime = this._lastUpdateTime!.toProto().timestampValue; + const valueProto = this._lastUpdateTime!.toProto(); + proto.updateTime = valueProto.timestampValue; } else { proto.exists = this._exists; } diff --git a/dev/src/index.ts b/dev/src/index.ts index 69f6a22ae..bee0157fb 100644 --- a/dev/src/index.ts +++ b/dev/src/index.ts @@ -14,18 +14,15 @@ * limitations under the License. */ -import {replaceProjectIdToken} from '@google-cloud/projectify'; -import * as assert from 'assert'; import * as bun from 'bun'; -import * as extend from 'extend'; +import {CallOptions} from 'google-gax'; import * as through2 from 'through2'; import {google} from '../protos/firestore_proto_api'; import {fieldsFromJson, timestampFromJson} from './convert'; import {DocumentSnapshot, DocumentSnapshotBuilder, QueryDocumentSnapshot} from './document'; import {logger, setLibVersion} from './logger'; -import {FieldPath, validateResourcePath} from './path'; -import {ResourcePath} from './path'; +import {DEFAULT_DATABASE_ID, FieldPath, QualifiedResourcePath, ResourcePath, validateResourcePath} from './path'; import {ClientPool} from './pool'; import {CollectionReference} from './reference'; import {DocumentReference} from './reference'; @@ -207,7 +204,7 @@ export class Firestore { * to work around a connection limit of 100 concurrent requests per client. * @private */ - private _clientPool: ClientPool|null = null; + private _clientPool: ClientPool; /** * The configuration options for the GAPIC client. @@ -223,19 +220,19 @@ export class Firestore { private _settingsFrozen = false; /** - * A Promise that resolves when client initialization completes. Can be - * 'null' if initialization hasn't started yet. + * The serializer to use for the Protobuf transformation. * @private */ - private _clientInitialized: Promise|null = null; + _serializer: Serializer|null = null; /** - * The serializer to use for the Protobuf transformation. + * The project ID for this client. + * + * The project ID is auto-detected during the first request unless a project + * ID is passed to the constructor (or provided via `.settings()`). * @private */ - _serializer: Serializer|null = null; - - private _referencePath: ResourcePath|null = null; + private _projectId: string|undefined = undefined; // GCF currently tears down idle connections after two minutes. Requests // that are issued after this period may fail. On GCF, we therefore issue @@ -311,6 +308,13 @@ export class Firestore { logger('Firestore', null, 'Detected GCF environment'); } + this._clientPool = + new ClientPool(MAX_CONCURRENT_REQUESTS_PER_CLIENT, () => { + const client = new module.exports.v1(this._settings); + logger('Firestore', null, 'Initialized Firestore GAPIC Client'); + return client; + }); + logger('Firestore', null, 'Initialized Firestore'); } @@ -331,16 +335,9 @@ export class Firestore { 'settings.timestampsInSnapshots', settings.timestampsInSnapshots, {optional: true}); - if (this._clientInitialized) { - throw new Error( - 'Firestore has already been started and its settings can no longer ' + - 'be changed. You can only call settings() before calling any other ' + - 'methods on a Firestore object.'); - } - if (this._settingsFrozen) { throw new Error( - 'Firestore.settings() has already be called. You can only call ' + + 'Firestore has already been initialized. You can only call ' + 'settings() once, and only before calling any other methods on a ' + 'Firestore object.'); } @@ -357,11 +354,7 @@ export class Firestore { if (settings && settings.projectId) { validateString('settings.projectId', settings.projectId); - this._referencePath = new ResourcePath(settings.projectId, '(default)'); - } else { - // Initialize a temporary reference path that will be overwritten during - // project ID detection. - this._referencePath = new ResourcePath('{{projectId}}', '(default)'); + this._projectId = settings.projectId; } this._settings = settings; @@ -369,16 +362,27 @@ export class Firestore { } /** - * The root path to the database. + * Returns the Project ID for this Firestore instance. Validates that + * `initializeIfNeeded()` was called before. + * + * @private + */ + get projectId(): string { + if (this._projectId === undefined) { + throw new Error( + 'INTERNAL ERROR: Client is not yet ready to issue requests.'); + } + return this._projectId; + } + + /** + * Returns the root path of the database. Validates that + * `initializeIfNeeded()` was called before. * * @private */ get formattedName(): string { - const components = [ - 'projects', this._referencePath!.projectId, 'databases', - this._referencePath!.databaseId - ]; - return components.join('/'); + return `projects/${this.projectId}/databases/${DEFAULT_DATABASE_ID}`; } /** @@ -396,7 +400,7 @@ export class Firestore { doc(documentPath: string): DocumentReference { validateResourcePath('documentPath', documentPath); - const path = this._referencePath!.append(documentPath); + const path = ResourcePath.EMPTY.append(documentPath); if (!path.isDocument) { throw new Error(`Value for argument "documentPath" must point to a document, but was "${ documentPath}". Your path does not contain an even number of components.`); @@ -424,7 +428,7 @@ export class Firestore { collection(collectionPath: string): CollectionReference { validateResourcePath('collectionPath', collectionPath); - const path = this._referencePath!.append(collectionPath); + const path = ResourcePath.EMPTY.append(collectionPath); if (!path.isCollection) { throw new Error(`Value for argument "collectionPath" must point to a collection, but was "${ collectionPath}". Your path does not contain an odd number of components.`); @@ -515,11 +519,12 @@ export class Firestore { if (typeof documentOrName === 'string') { document.ref = new DocumentReference( - this, ResourcePath.fromSlashSeparatedString(documentOrName)); + this, QualifiedResourcePath.fromSlashSeparatedString(documentOrName)); } else { document.ref = new DocumentReference( this, - ResourcePath.fromSlashSeparatedString(documentOrName.name as string)); + QualifiedResourcePath.fromSlashSeparatedString( + documentOrName.name as string)); document.fieldsProto = documentOrName.fields ? convertFields(documentOrName.fields as ApiMapValue) : {}; @@ -592,7 +597,8 @@ export class Firestore { {optional: true, minValue: 1}); } - return this._runTransaction(updateFunction, transactionOptions); + return this.initializeIfNeeded().then( + () => this._runTransaction(updateFunction, transactionOptions)); } _runTransaction( @@ -667,7 +673,7 @@ export class Firestore { * }); */ listCollections() { - const rootDocument = new DocumentReference(this, this._referencePath!); + const rootDocument = new DocumentReference(this, ResourcePath.EMPTY); return rootDocument.listCollections(); } @@ -712,7 +718,8 @@ export class Firestore { const {documents, fieldMask} = parseGetAllArguments(documentRefsOrReadOptions); - return this.getAll_(documents, fieldMask, requestTag()); + return this.initializeIfNeeded().then( + () => this.getAll_(documents, fieldMask, requestTag())); } /** @@ -813,13 +820,14 @@ export class Firestore { } /** - * Executes a new request using the first available GAPIC client. + * Initializes the client if it is not already initialized. All methods in the + * SDK can be used after this method completes. * * @private + * @return A Promise that resolves when the client is initialized. */ - private _runRequest(op: (client: GapicClient) => Promise): Promise { - // Initialize the client pool if this is the first request. - if (!this._clientInitialized) { + async initializeIfNeeded(): Promise { + if (!this._settingsFrozen) { // Nobody should set timestampsInSnapshots anymore, but the error depends // on whether they set it to true or false... if (this._settings.timestampsInSnapshots === true) { @@ -850,87 +858,39 @@ export class Firestore { Please audit all existing usages of Date when you enable the new behavior.`); } - this._clientInitialized = this._initClientPool().then(clientPool => { - this._clientPool = clientPool; - }); } - return this._clientInitialized!.then(() => this._clientPool!.run(op)); - } - - /** - * Initializes the client pool and invokes Project ID detection. Returns a - * Promise on completion. - * - * @private - */ - private _initClientPool(): Promise> { - assert(!this._clientInitialized, 'Client pool already initialized'); - - const clientPool = - new ClientPool(MAX_CONCURRENT_REQUESTS_PER_CLIENT, () => { - const client = new module.exports.v1(this._settings); - logger('Firestore', null, 'Initialized Firestore GAPIC Client'); - return client; - }); - - const projectIdProvided = - this._referencePath!.projectId !== '{{projectId}}'; + this._settingsFrozen = true; - if (projectIdProvided) { - return Promise.resolve(clientPool); - } else { - return clientPool.run(client => this._detectProjectId(client)) - .then(projectId => { - this._referencePath = - new ResourcePath(projectId, this._referencePath!.databaseId); - return clientPool; + if (this._projectId === undefined) { + this._projectId = await this._clientPool.run(gapicClient => { + return new Promise((resolve, reject) => { + gapicClient.getProjectId((err: Error, projectId: string) => { + if (err) { + logger( + 'Firestore._detectProjectId', null, + 'Failed to detect project ID: %s', err); + reject(err); + } else { + logger( + 'Firestore._detectProjectId', null, 'Detected project ID: %s', + projectId); + resolve(projectId); + } }); - } - } - - /** - * Auto-detects the Firestore Project ID. - * - * @private - * @param gapicClient The Firestore GAPIC client. - * @return A Promise that resolves with the Project ID. - */ - private _detectProjectId(gapicClient: GapicClient): Promise { - return new Promise((resolve, reject) => { - gapicClient.getProjectId((err: Error, projectId: string) => { - if (err) { - logger( - 'Firestore._detectProjectId', null, - 'Failed to detect project ID: %s', err); - reject(err); - } else { - logger( - 'Firestore._detectProjectId', null, 'Detected project ID: %s', - projectId); - resolve(projectId); - } + }); }); - }); + } } /** - * Decorate the request options of an API request. This is used to replace - * any `{{projectId}}` placeholders with the value detected from the user's - * environment, if one wasn't provided manually. - * + * Returns GAX call options that set the cloud resource header. * @private */ - _decorateRequest(request: T): {request: T, gax: {}} { - let decoratedRequest = extend(true, {}, request); - decoratedRequest = - replaceProjectIdToken(decoratedRequest, this._referencePath!.projectId); - const decoratedGax: { - otherArgs: {headers: {[k: string]: string}} - } = {otherArgs: {headers: {}}}; - decoratedGax.otherArgs.headers[CLOUD_RESOURCE_HEADER] = this.formattedName; - - return {request: decoratedRequest, gax: decoratedGax}; + private createCallOptions(): CallOptions { + const gaxHeaders: CallOptions = {otherArgs: {headers: {}}}; + gaxHeaders.otherArgs!.headers[CLOUD_RESOURCE_HEADER] = this.formattedName; + return gaxHeaders; } /** @@ -1132,16 +1092,15 @@ export class Firestore { methodName: string, request: {}, requestTag: string, allowRetries: boolean): Promise { const attempts = allowRetries ? MAX_REQUEST_RETRIES : 1; + const callOptions = this.createCallOptions(); - return this._runRequest(gapicClient => { - const decorated = this._decorateRequest(request); + return this._clientPool.run(gapicClient => { return this._retry(attempts, requestTag, () => { return new Promise((resolve, reject) => { logger( - 'Firestore.request', requestTag, 'Sending request: %j', - decorated.request); + 'Firestore.request', requestTag, 'Sending request: %j', request); gapicClient[methodName]( - decorated.request, decorated.gax, (err: GrpcError, result: T) => { + request, callOptions, (err: GrpcError, result: T) => { if (err) { logger( 'Firestore.request', requestTag, 'Received error:', err); @@ -1178,17 +1137,16 @@ export class Firestore { methodName: string, request: {}, requestTag: string, allowRetries: boolean): Promise { const attempts = allowRetries ? MAX_REQUEST_RETRIES : 1; + const callOptions = this.createCallOptions(); - return this._runRequest(gapicClient => { - const decorated = this._decorateRequest(request); + return this._clientPool.run(gapicClient => { return this._retry(attempts, requestTag, () => { return new Promise((resolve, reject) => { try { logger( 'Firestore.readStream', requestTag, - 'Sending request: %j', decorated.request); - const stream = gapicClient[methodName]( - decorated.request, decorated.gax); + 'Sending request: %j', request); + const stream = gapicClient[methodName](request, callOptions); const logStream = through2.obj(function(this, chunk, enc, callback) { logger( @@ -1229,26 +1187,14 @@ export class Firestore { readWriteStream( methodName: string, request: {}, requestTag: string, allowRetries: boolean): Promise { - const self = this; const attempts = allowRetries ? MAX_REQUEST_RETRIES : 1; + const callOptions = this.createCallOptions(); - return this._runRequest(gapicClient => { - const decorated = this._decorateRequest(request); + return this._clientPool.run(gapicClient => { return this._retry(attempts, requestTag, () => { return Promise.resolve().then(() => { logger('Firestore.readWriteStream', requestTag, 'Opening stream'); - const requestStream = gapicClient[methodName](decorated.gax); - - // The transform stream to assign the project ID. - const transform = through2.obj((chunk, encoding, callback) => { - const decoratedChunk = extend(true, {}, chunk); - replaceProjectIdToken( - decoratedChunk, self._referencePath!.projectId); - logger( - 'Firestore.readWriteStream', requestTag, - 'Streaming request: %j', decoratedChunk); - requestStream.write(decoratedChunk, encoding, callback); - }); + const requestStream = gapicClient[methodName](callOptions); const logStream = through2.obj(function(this, chunk, enc, callback) { logger( @@ -1258,7 +1204,7 @@ export class Firestore { callback(); }); - const resultStream = bun([transform, requestStream, logStream]); + const resultStream = bun([requestStream, logStream]); return this._initializeStream(resultStream, requestTag, request); }); }); diff --git a/dev/src/order.ts b/dev/src/order.ts index 1bffac75b..d338c5431 100644 --- a/dev/src/order.ts +++ b/dev/src/order.ts @@ -16,7 +16,7 @@ import {google} from '../protos/firestore_proto_api'; import {detectValueType} from './convert'; -import {ResourcePath} from './path'; +import {QualifiedResourcePath} from './path'; import {ApiMapValue} from './types'; import api = google.firestore.v1; @@ -151,9 +151,10 @@ function compareBlobs(left: Uint8Array, right: Uint8Array): number { * @private */ function compareReferenceProtos(left: api.IValue, right: api.IValue): number { - const leftPath = ResourcePath.fromSlashSeparatedString(left.referenceValue!); + const leftPath = + QualifiedResourcePath.fromSlashSeparatedString(left.referenceValue!); const rightPath = - ResourcePath.fromSlashSeparatedString(right.referenceValue!); + QualifiedResourcePath.fromSlashSeparatedString(right.referenceValue!); return leftPath.compareTo(rightPath); } diff --git a/dev/src/path.ts b/dev/src/path.ts index bfe9c3944..0172da049 100644 --- a/dev/src/path.ts +++ b/dev/src/path.ts @@ -21,6 +21,12 @@ import {customObjectMessage, invalidArgumentMessage, validateMinNumberOfArgument import api = google.firestore.v1; +/*! + * The default database ID for this Firestore client. We do not yet expose the + * ability to use different databases. + */ +export const DEFAULT_DATABASE_ID = '(default)'; + /*! * A regular expression to verify an absolute Resource Path in Firestore. It * extracts the project ID, the database name and the relative resource path @@ -67,15 +73,6 @@ abstract class Path { */ constructor(protected readonly segments: string[]) {} - /** - * String representation as expected by the proto API. - * - * @private - */ - get formattedName(): string { - return this.canonicalString()!; - } - /** * Returns the number of segments of this field path. * @@ -86,7 +83,6 @@ abstract class Path { } abstract construct(segments: string[]|string): T; - abstract canonicalString(): string; abstract split(relativePath: string): string[]; /** @@ -138,16 +134,6 @@ abstract class Path { return true; } - /** - * Returns a string representation of this path. - * - * @private - * @returns A string representing this path. - */ - toString(): string { - return this.formattedName; - } - /** * Compare the current path against another Path object. * @@ -199,65 +185,39 @@ abstract class Path { } /** - * A slash-separated path for navigating resources (documents and collections) - * within Firestore. + * A slash-separated path for navigating resources within the current Firestore + * instance. * * @private - * @class */ export class ResourcePath extends Path { /** - * The project ID of this path. - */ - readonly projectId: string; - - /** - * The database ID of this path. + * A default instance pointing to the root collection. + * @private */ - readonly databaseId: string; + static EMPTY = new ResourcePath(); /** - * Constructs a Firestore Resource Path. + * Constructs a ResourcePath. * * @private - * @hideconstructor - * - * @param projectId The Firestore project id. - * @param databaseId The Firestore database id. * @param segments Sequence of names of the parts of the path. */ - constructor(projectId: string, databaseId: string, ...segments: string[]) { + constructor(...segments: string[]) { super(segments); - - this.projectId = projectId; - this.databaseId = databaseId; } /** - * String representation of the path relative to the database root. - * + * Indicates whether this path points to a document. * @private - * @type {string} - */ - get relativeName(): string { - return this.segments.join('/'); - } - - /** - * Indicates whether this ResourcePath points to a document. - * - * @private - * @type {boolean} */ get isDocument(): boolean { return this.segments.length > 0 && this.segments.length % 2 === 0; } /** - * Indicates whether this ResourcePath points to a collection. - * + * Indicates whether this path points to a collection. * @private - * @type {boolean} */ get isCollection(): boolean { return this.segments.length % 2 === 1; @@ -265,9 +225,7 @@ export class ResourcePath extends Path { /** * The last component of the path. - * * @private - * @type {string|null} */ get id(): string|null { if (this.segments.length > 0) { @@ -276,48 +234,145 @@ export class ResourcePath extends Path { return null; } + /** + * Returns the location of this path relative to the root of the project's + * database. + * @private + */ + get relativeName() { + return this.segments.join('/'); + } + + /** + * Constructs a new instance of ResourcePath. + * + * @private + * @param segments Sequence of parts of the path. + * @returns The newly created ResourcePath. + */ + construct(segments: string[]): ResourcePath { + return new ResourcePath(...segments); + } + + /** + * Splits a string into path segments, using slashes as separators. + * + * @private + * @param relativePath The path to split. + * @returns The split path segments. + */ + split(relativePath: string): string[] { + // We may have an empty segment at the beginning or end if they had a + // leading or trailing slash (which we allow). + return relativePath.split('/').filter(segment => segment.length > 0); + } + + /** + * Converts this path to a fully qualified ResourcePath. + * + * @private + * @param projectIdIfMissing The project ID of the current Firestore project. + * The project ID is only used if it's not provided as part of this + * ResourcePath. + * @return A fully-qualified resource path pointing to the same element. + */ + toQualifiedResourcePath(projectIdIfMissing: string): QualifiedResourcePath { + return new QualifiedResourcePath( + projectIdIfMissing, DEFAULT_DATABASE_ID, ...this.segments); + } +} + +/** + * A slash-separated path that includes a project and database ID for referring + * to resources in any Firestore project. + * + * @private + */ +export class QualifiedResourcePath extends ResourcePath { + /** + * The project ID of this path. + */ + readonly projectId: string; + + /** + * The database ID of this path. + */ + readonly databaseId: string; + + /** + * Constructs a Firestore Resource Path. + * + * @private + * @param projectId The Firestore project id. + * @param databaseId The Firestore database id. + * @param segments Sequence of names of the parts of the path. + */ + constructor(projectId: string, databaseId: string, ...segments: string[]) { + super(...segments); + + this.projectId = projectId; + this.databaseId = databaseId; + } + + /** + * String representation of the path relative to the database root. + * @private + */ + get relativeName(): string { + return this.segments.join('/'); + } + /** * Creates a resource path from an absolute Firestore path. * * @private - * @param {string} absolutePath A string representation of a Resource Path. - * @returns {ResourcePath} The new ResourcePath. + * @param absolutePath A string representation of a Resource Path. + * @returns The new ResourcePath. */ - static fromSlashSeparatedString(absolutePath: string): ResourcePath { + static fromSlashSeparatedString(absolutePath: string): QualifiedResourcePath { const elements = RESOURCE_PATH_RE.exec(absolutePath); if (elements) { const project = elements[1]; const database = elements[2]; const path = elements[3]; - return new ResourcePath(project, database).append(path); + return new QualifiedResourcePath(project, database).append(path); } throw new Error(`Resource name '${absolutePath}' is not valid.`); } /** - * Splits a string into path segments, using slashes as separators. + * Create a child path beneath the current level. * * @private - * @override - * @param {string} relativePath The path to split. - * @returns {Array.} - The split path segments. + * @param relativePath Relative path to append to the current path. + * @returns The new path. */ - split(relativePath: string): string[] { - // We may have an empty segment at the beginning or end if they had a - // leading or trailing slash (which we allow). - return relativePath.split('/').filter(segment => segment.length > 0); + append(relativePath: ResourcePath|string): QualifiedResourcePath { + // `super.append()` calls `QualifiedResourcePath.construct()` when invoked + // from here and returns a QualifiedResourcePath. + return super.append(relativePath) as QualifiedResourcePath; + } + + + /** + * Create a child path beneath the current level. + * + * @private + * @returns The new path. + */ + parent(): QualifiedResourcePath|null { + return super.parent() as QualifiedResourcePath | null; } /** * String representation of a ResourcePath as expected by the API. * * @private - * @override - * @returns {string} The representation as expected by the API. + * @returns The representation as expected by the API. */ - canonicalString(): string { + get formattedName(): string { const components = [ 'projects', this.projectId, 'databases', this.databaseId, 'documents', ...this.segments @@ -331,43 +386,49 @@ export class ResourcePath extends Path { * methods. * * @private - * @override - * @param {Array.} segments Sequence of names of the parts of the - * path. - * @returns {ResourcePath} The newly created ResourcePath. + * @param segments Sequence of names of the parts of the path. + * @returns The newly created QualifiedResourcePath. */ - construct(segments: string[]): ResourcePath { - return new ResourcePath(this.projectId, this.databaseId, ...segments); + construct(segments: string[]): QualifiedResourcePath { + return new QualifiedResourcePath( + this.projectId, this.databaseId, ...segments); + } + + /** + * Convenience method to match the ResourcePath API. This method always + * returns the current instance. The arguments is ignored. + * + * @param projectIdIfMissing The project ID of the current Firestore project. + * The project ID is only used if it's not provided as part of this + * ResourcePath. + * @private + */ + toQualifiedResourcePath(projectIdIfMissing: string): QualifiedResourcePath { + return this; } /** * Compare the current path against another ResourcePath object. * * @private - * @override - * @param {ResourcePath} other The path to compare to. - * @returns {number} -1 if current < other, 1 if current > other, 0 if equal + * @param other The path to compare to. + * @returns -1 if current < other, 1 if current > other, 0 if equal */ compareTo(other: ResourcePath): number { - // Ignore DocumentReference with {{projectId}} placeholders and assume that - // the resolved IDs match the provided ResourcePath. We could alternatively - // try to resolve the Project ID here, but this is asynchronous as it - // requires Disk I/O. - if (this.projectId !== '{{projectId}}' && - other.projectId !== '{{projectId}}') { + if (other instanceof QualifiedResourcePath) { if (this.projectId < other.projectId) { return -1; } if (this.projectId > other.projectId) { return 1; } - } - if (this.databaseId < other.databaseId) { - return -1; - } - if (this.databaseId > other.databaseId) { - return 1; + if (this.databaseId < other.databaseId) { + return -1; + } + if (this.databaseId > other.databaseId) { + return 1; + } } return super.compareTo(other); @@ -375,7 +436,6 @@ export class ResourcePath extends Path { /** * Converts this ResourcePath to the Firestore Proto representation. - * * @private */ toProto(): api.IValue { @@ -439,12 +499,12 @@ export class FieldPath extends Path { * }); */ constructor(...segments: string[]) { - validateMinNumberOfArguments('FieldPath', arguments, 1); - const elements: string[] = Array.isArray(segments[0]) ? (segments[0] as unknown) as string[] : segments; + validateMinNumberOfArguments('FieldPath', elements, 1); + for (let i = 0; i < elements.length; ++i) { validateString(i, elements[i]); if (elements[i].length === 0) { @@ -489,7 +549,7 @@ export class FieldPath extends Path { * @override * @returns {string} The representation as expected by the API. */ - canonicalString(): string { + get formattedName(): string { return this.segments .map(str => { return UNESCAPED_FIELD_NAME_RE.test(str) ? @@ -499,6 +559,16 @@ export class FieldPath extends Path { .join('.'); } + /** + * Returns a string representation of this path. + * + * @private + * @returns A string representing this path. + */ + toString(): string { + return this.formattedName; + } + /** * Splits a string into path segments, using dots as separators. * @@ -518,8 +588,8 @@ export class FieldPath extends Path { * * @private * @override - * @param {Array.} segments Sequence of field names. - * @returns {ResourcePath} The newly created FieldPath. + * @param segments Sequence of field names. + * @returns The newly created FieldPath. */ construct(segments: string[]) { return new FieldPath(...segments); diff --git a/dev/src/reference.ts b/dev/src/reference.ts index 5e510d43c..d6a9fcd5a 100644 --- a/dev/src/reference.ts +++ b/dev/src/reference.ts @@ -17,7 +17,6 @@ const deepEqual = require('deep-equal'); import * as bun from 'bun'; -import * as extend from 'extend'; import * as through2 from 'through2'; import * as proto from '../protos/firestore_proto_api'; @@ -27,8 +26,8 @@ import {DocumentChange} from './document-change'; import {Firestore} from './index'; import {logger} from './logger'; import {compare} from './order'; -import {FieldPath, ResourcePath, validateFieldPath, validateResourcePath} from './path'; -import {Serializer, validateUserInput} from './serializer'; +import {FieldPath, QualifiedResourcePath, ResourcePath, validateFieldPath, validateResourcePath} from './path'; +import {Serializable, Serializer, validateUserInput} from './serializer'; import {Timestamp} from './timestamp'; import {DocumentData, OrderByDirection, Precondition, SetOptions, UpdateData, WhereFilterOp} from './types'; import {autoId, requestTag} from './util'; @@ -96,7 +95,7 @@ const comparisonOperators: * * @class */ -export class DocumentReference { +export class DocumentReference implements Serializable { /** * @private * @hideconstructor @@ -107,6 +106,7 @@ export class DocumentReference { constructor( private readonly _firestore: Firestore, readonly _path: ResourcePath) {} + /** * The string representation of the DocumentReference's location. * @private @@ -114,7 +114,8 @@ export class DocumentReference { * @name DocumentReference#formattedName */ get formattedName(): string { - return this._path.formattedName; + const projectId = this.firestore.projectId; + return this._path.toQualifiedResourcePath(projectId).formattedName; } /** @@ -255,25 +256,26 @@ export class DocumentReference { * }); */ listCollections(): Promise { - const request = {parent: this._path.formattedName}; - - return this._firestore - .request( - 'listCollectionIds', request, requestTag(), - /* allowRetries= */ true) - .then(collectionIds => { - const collections: CollectionReference[] = []; - - // We can just sort this list using the default comparator since it - // will only contain collection ids. - collectionIds.sort(); - - for (const collectionId of collectionIds) { - collections.push(this.collection(collectionId)); - } - - return collections; - }); + return this.firestore.initializeIfNeeded().then(() => { + const request = {parent: this.formattedName}; + return this._firestore + .request( + 'listCollectionIds', request, requestTag(), + /* allowRetries= */ true) + .then(collectionIds => { + const collections: CollectionReference[] = []; + + // We can just sort this list using the default comparator since it + // will only contain collection ids. + collectionIds.sort(); + + for (const collectionId of collectionIds) { + collections.push(this.collection(collectionId)); + } + + return collections; + }); + }); } /** @@ -859,12 +861,18 @@ docChangesPropertiesToOverride.forEach(property => { {get: () => throwDocChangesMethodError()}); }); +/** Internal representation of a query cursor before serialization. */ +interface QueryCursor { + before?: boolean; + values: unknown[]; +} + /** Internal options to customize the Query class. */ interface QueryOptions { - startAt?: api.ICursor; - startAfter?: api.ICursor; - endAt?: api.ICursor; - endBefore?: api.ICursor; + startAt?: QueryCursor; + startAfter?: QueryCursor; + endAt?: QueryCursor; + endBefore?: QueryCursor; limit?: number; offset?: number; projection?: api.StructuredQuery.IProjection; @@ -948,14 +956,6 @@ export class Query { return fieldValues; } - /** - * The string representation of the Query's location. - * @private - */ - get formattedName(): string { - return this._path.formattedName; - } - /** * The [Firestore]{@link Firestore} instance for the Firestore * database (useful for performing transactions, etc.). @@ -1017,7 +1017,7 @@ export class Query { fieldPath = FieldPath.fromArgument(fieldPath); if (FieldPath.documentId().isEqual(fieldPath)) { - value = this.convertReference(value); + value = this.validateReference(value); } const combinedFilters = this._fieldFilters.concat(new FieldFilter( @@ -1063,7 +1063,7 @@ export class Query { } } - const options = extend(true, {}, this._queryOptions); + const options = Object.assign({}, this._queryOptions); options.projection = {fields}; return new Query( @@ -1134,7 +1134,7 @@ export class Query { limit(limit: number): Query { validateInteger('limit', limit); - const options = extend(true, {}, this._queryOptions); + const options = Object.assign({}, this._queryOptions); options.limit = limit; return new Query( this._firestore, this._path, this._fieldFilters, this._fieldOrders, @@ -1163,7 +1163,7 @@ export class Query { offset(offset: number): Query { validateInteger('offset', offset); - const options = extend(true, {}, this._queryOptions); + const options = Object.assign({}, this._queryOptions); options.offset = offset; return new Query( this._firestore, this._path, this._fieldFilters, this._fieldOrders, @@ -1250,7 +1250,7 @@ export class Query { private createCursor( fieldOrders: FieldOrder[], cursorValuesOrDocumentSnapshot: Array, - before: boolean): api.ICursor { + before: boolean): QueryCursor { let fieldValues; if (Query._isDocumentSnapshot(cursorValuesOrDocumentSnapshot)) { @@ -1266,9 +1266,7 @@ export class Query { 'values must match the orderBy() constraints of the query.'); } - const options: api.ICursor = { - values: [], - }; + const options: QueryCursor = {values: []}; if (before) { options.before = true; @@ -1278,11 +1276,11 @@ export class Query { let fieldValue = fieldValues[i]; if (FieldPath.documentId().isEqual(fieldOrders[i].field)) { - fieldValue = this.convertReference(fieldValue); + fieldValue = this.validateReference(fieldValue); } validateQueryValue(i, fieldValue); - options.values!.push(this._serializer.encodeValue(fieldValue)!); + options.values!.push(fieldValue); } return options; @@ -1294,13 +1292,13 @@ export class Query { * Throws a validation error or returns a DocumentReference that can * directly be used in the Query. * - * @param reference The value to validate. + * @param val The value to validate. * @throws If the value cannot be used for this query. * @return If valid, returns a DocumentReference that can be used with the * query. * @private */ - private convertReference(val: unknown): DocumentReference { + private validateReference(val: unknown): DocumentReference { let reference: DocumentReference; if (typeof val === 'string') { @@ -1350,7 +1348,7 @@ export class Query { Query { validateMinNumberOfArguments('Query.startAt', arguments, 1); - const options = extend(true, {}, this._queryOptions); + const options = Object.assign({}, this._queryOptions); const fieldOrders = this.createImplicitOrderBy(fieldValuesOrDocumentSnapshot); @@ -1385,7 +1383,7 @@ export class Query { Query { validateMinNumberOfArguments('Query.startAfter', arguments, 1); - const options = extend(true, {}, this._queryOptions); + const options = Object.assign({}, this._queryOptions); const fieldOrders = this.createImplicitOrderBy(fieldValuesOrDocumentSnapshot); @@ -1419,7 +1417,7 @@ export class Query { Query { validateMinNumberOfArguments('Query.endBefore', arguments, 1); - const options = extend(true, {}, this._queryOptions); + const options = Object.assign({}, this._queryOptions); const fieldOrders = this.createImplicitOrderBy(fieldValuesOrDocumentSnapshot); @@ -1453,7 +1451,7 @@ export class Query { Query { validateMinNumberOfArguments('Query.endAt', arguments, 1); - const options = extend(true, {}, this._queryOptions); + const options = Object.assign({}, this._queryOptions); const fieldOrders = this.createImplicitOrderBy(fieldValuesOrDocumentSnapshot); @@ -1556,6 +1554,20 @@ export class Query { return bun([responseStream, transform]); } + /** + * Converts a QueryCursor to its proto representation. + * @private + */ + private _toCursor(cursor?: QueryCursor): api.ICursor|undefined { + if (cursor) { + const values = cursor.values.map( + val => this._serializer.encodeValue(val) as api.IValue); + return {before: cursor.before, values}; + } + + return undefined; + } + /** * Internal method for serializing a query to its RunQuery proto * representation with an optional transaction id. @@ -1565,8 +1577,10 @@ export class Query { * @returns Serialized JSON for the query. */ toProto(transactionId?: Uint8Array): api.IRunQueryRequest { + const projectId = this.firestore.projectId; + const parentPath = this._path.parent()!.toQualifiedResourcePath(projectId); const reqOpts: api.IRunQueryRequest = { - parent: this._path.parent()!.formattedName, + parent: parentPath.formattedName, structuredQuery: { from: [ { @@ -1606,8 +1620,8 @@ export class Query { } structuredQuery.offset = this._queryOptions.offset; - structuredQuery.startAt = this._queryOptions.startAt; - structuredQuery.endAt = this._queryOptions.endAt; + structuredQuery.startAt = this._toCursor(this._queryOptions.startAt); + structuredQuery.endAt = this._toCursor(this._queryOptions.endAt); structuredQuery.select = this._queryOptions.projection; reqOpts.transaction = transactionId; @@ -1623,7 +1637,6 @@ export class Query { * @returns A stream of document results. */ _stream(transactionId?: Uint8Array): NodeJS.ReadableStream { - const request = this.toProto(transactionId); const tag = requestTag(); const self = this; @@ -1639,19 +1652,22 @@ export class Query { callback(); }); - this._firestore.readStream('runQuery', request, tag, true) - .then(backendStream => { - backendStream.on('error', err => { - logger( - 'Query._stream', tag, 'Query failed with stream error:', err); + this.firestore.initializeIfNeeded().then(() => { + const request = this.toProto(transactionId); + this._firestore.readStream('runQuery', request, tag, true) + .then(backendStream => { + backendStream.on('error', err => { + logger( + 'Query._stream', tag, 'Query failed with stream error:', err); + stream.destroy(err); + }); + backendStream.resume(); + backendStream.pipe(stream); + }) + .catch(err => { stream.destroy(err); }); - backendStream.resume(); - backendStream.pipe(stream); - }) - .catch(err => { - stream.destroy(err); - }); + }); return stream; } @@ -1832,24 +1848,31 @@ export class CollectionReference extends Query { * }); */ listDocuments(): Promise { - const request: api.IListDocumentsRequest = { - parent: this._path.parent()!.formattedName, - collectionId: this.id, - showMissing: true, - mask: {fieldPaths: []} - }; + return this.firestore.initializeIfNeeded().then(() => { + const resourcePath = + this._path.toQualifiedResourcePath(this.firestore.projectId); + const parentPath = resourcePath.parent()!; + + const request: api.IListDocumentsRequest = { + parent: parentPath.formattedName, + collectionId: this.id, + showMissing: true, + mask: {fieldPaths: []} + }; - return this.firestore - .request( - 'listDocuments', request, requestTag(), /*allowRetries=*/true) - .then(documents => { - // Note that the backend already orders these documents by name, - // so we do not need to manually sort them. - return documents.map(doc => { - const path = ResourcePath.fromSlashSeparatedString(doc.name!); - return this.doc(path.id!); + return this.firestore + .request( + 'listDocuments', request, requestTag(), /*allowRetries=*/true) + .then(documents => { + // Note that the backend already orders these documents by name, + // so we do not need to manually sort them. + return documents.map(doc => { + const path = + QualifiedResourcePath.fromSlashSeparatedString(doc.name!); + return this.doc(path.id!); + }); }); - }); + }); } @@ -2000,7 +2023,7 @@ function validateQueryValue(arg: string|number, value: unknown): void { } /** - * Verifies euqality for an array of objects using the `isEqual` interface. + * Verifies equality for an array of objects using the `isEqual` interface. * * @private * @param left Array of objects supporting `isEqual`. diff --git a/dev/src/serializer.ts b/dev/src/serializer.ts index 707d8cb3d..60ed3ecf9 100644 --- a/dev/src/serializer.ts +++ b/dev/src/serializer.ts @@ -22,7 +22,7 @@ import {FieldTransform} from './field-value'; import {DeleteTransform} from './field-value'; import {GeoPoint} from './geo-point'; import {DocumentReference, Firestore} from './index'; -import {FieldPath, ResourcePath} from './path'; +import {FieldPath, QualifiedResourcePath} from './path'; import {Timestamp} from './timestamp'; import {ApiMapValue, DocumentData, ValidationOptions} from './types'; import {isEmpty, isObject} from './util'; @@ -223,8 +223,8 @@ export class Serializer { return this.timestampsInSnapshots ? timestamp : timestamp.toDate(); } case 'referenceValue': { - const resourcePath = - ResourcePath.fromSlashSeparatedString(proto.referenceValue!); + const resourcePath = QualifiedResourcePath.fromSlashSeparatedString( + proto.referenceValue!); return this.createReference(resourcePath.relativeName); } case 'arrayValue': { @@ -309,7 +309,7 @@ export function validateUserInput( level = level || 0; inArray = inArray || false; - const fieldPathMessage = path ? ` (found in field ${path.toString()})` : ''; + const fieldPathMessage = path ? ` (found in field ${path})` : ''; if (Array.isArray(value)) { for (let i = 0; i < value.length; ++i) { diff --git a/dev/src/timestamp.ts b/dev/src/timestamp.ts index 60f0874a8..50034de76 100644 --- a/dev/src/timestamp.ts +++ b/dev/src/timestamp.ts @@ -18,6 +18,8 @@ import {google} from '../protos/firestore_proto_api'; import {validateInteger} from './validate'; +import api = google.firestore.v1; + /*! * Number of nanoseconds in a millisecond. * @@ -227,7 +229,7 @@ export class Timestamp { * @private * @returns {Object} The `Timestamp` Protobuf object. */ - toProto() { + toProto(): api.IValue { const timestamp: google.protobuf.ITimestamp = {}; if (this.seconds) { diff --git a/dev/src/validate.ts b/dev/src/validate.ts index f5ecf1ba1..ee8e9b994 100644 --- a/dev/src/validate.ts +++ b/dev/src/validate.ts @@ -47,7 +47,7 @@ export interface NumericRangeOptions { */ export function customObjectMessage( arg: string|number, value: unknown, path?: FieldPath): string { - const fieldPathMessage = path ? ` (found in field ${path.toString()})` : ''; + const fieldPathMessage = path ? ` (found in field ${path})` : ''; if (isObject(value)) { const typeName = value.constructor.name; @@ -274,7 +274,7 @@ function formatArgumentName(arg: string|number): string { * @throws if the expectation is not met. */ export function validateMinNumberOfArguments( - funcName: string, args: IArguments, minSize: number): void { + funcName: string, args: IArguments|unknown[], minSize: number): void { if (args.length < minSize) { throw new Error( `Function "${funcName}()" requires at least ` + diff --git a/dev/src/watch.ts b/dev/src/watch.ts index 19dd9825e..aba9389ea 100644 --- a/dev/src/watch.ts +++ b/dev/src/watch.ts @@ -24,7 +24,7 @@ import {DocumentSnapshotBuilder, QueryDocumentSnapshot} from './document'; import {DocumentChange, DocumentChangeType} from './document-change'; import Firestore, {DocumentReference, Query} from './index'; import {logger} from './logger'; -import {ResourcePath} from './path'; +import {QualifiedResourcePath} from './path'; import {Timestamp} from './timestamp'; import {GrpcError, RBTree} from './types'; import {requestTag} from './util'; @@ -238,7 +238,7 @@ abstract class Watch { } /** Returns a 'Target' proto denoting the target to listen on. */ - protected abstract getTarget(resumeToken?: Uint8Array): Promise; + protected abstract getTarget(resumeToken?: Uint8Array): api.ITarget; /** * Returns a comparator for QueryDocumentSnapshots that is used to order the @@ -350,7 +350,7 @@ abstract class Watch { docTree.forEach((snapshot: QueryDocumentSnapshot) => { // Mark each document as deleted. If documents are not deleted, they // will be send again by the server. - changeMap.set(snapshot.ref.formattedName, REMOVED); + changeMap.set(snapshot.ref.path, REMOVED); }); current = false; @@ -421,8 +421,10 @@ abstract class Watch { return; } - request.database = await this._firestore.formattedName; - request.addTarget = await this.getTarget(resumeToken); + await this._firestore.initializeIfNeeded(); + + request.database = this._firestore.formattedName; + request.addTarget = this.getTarget(resumeToken); // Note that we need to call the internal _listen API to pass additional // header values in readWriteStream. @@ -537,7 +539,7 @@ abstract class Watch { * @private */ function addDoc(newDocument: QueryDocumentSnapshot): DocumentChange { - const name = newDocument.ref.formattedName; + const name = newDocument.ref.path; assert(!updatedMap.has(name), 'Document to add already exists'); updatedTree = updatedTree.insert(newDocument, null); const newIndex = updatedTree.find(newDocument).index; @@ -554,7 +556,7 @@ abstract class Watch { */ function modifyDoc(newDocument: QueryDocumentSnapshot): DocumentChange|null { - const name = newDocument.ref.formattedName; + const name = newDocument.ref.path; assert(updatedMap.has(name), 'Document to modify does not exist'); const oldDocument = updatedMap.get(name)!; if (!oldDocument.updateTime.isEqual(newDocument.updateTime)) { @@ -711,25 +713,27 @@ abstract class Watch { const document = proto.documentChange.document!; const name = document.name!; + const relativeName = + QualifiedResourcePath.fromSlashSeparatedString(name) + .relativeName; if (changed) { logger( 'Watch.onSnapshot', this._requestTag, 'Received document change'); const snapshot = new DocumentSnapshotBuilder(); - snapshot.ref = this._firestore.doc( - ResourcePath.fromSlashSeparatedString(name).relativeName); + snapshot.ref = this._firestore.doc(relativeName); snapshot.fieldsProto = document.fields || {}; snapshot.createTime = Timestamp.fromProto(document.createTime!); snapshot.updateTime = Timestamp.fromProto(document.updateTime!); - changeMap.set(name, snapshot); + changeMap.set(relativeName, snapshot); } else if (removed) { logger( 'Watch.onSnapshot', this._requestTag, 'Received document remove'); - changeMap.set(name, REMOVED); + changeMap.set(relativeName, REMOVED); } } else if (proto.documentDelete || proto.documentRemove) { logger( @@ -737,7 +741,10 @@ abstract class Watch { 'Processing remove event'); const name = (proto.documentDelete || proto.documentRemove)!.document!; - changeMap.set(name, REMOVED); + const relativeName = + QualifiedResourcePath.fromSlashSeparatedString(name) + .relativeName; + changeMap.set(relativeName, REMOVED); } else if (proto.filter) { logger( 'Watch.onSnapshot', this._requestTag, @@ -786,9 +793,8 @@ export class DocumentWatch extends Watch { return DOCUMENT_WATCH_COMPARATOR; } - async getTarget(resumeToken?: Uint8Array): - Promise { - const formattedName = await this.ref.formattedName; + getTarget(resumeToken?: Uint8Array): google.firestore.v1.ITarget { + const formattedName = this.ref.formattedName; return { documents: { documents: [formattedName], @@ -816,9 +822,8 @@ export class QueryWatch extends Watch { return this.query.comparator(); } - async getTarget(resumeToken?: Uint8Array): - Promise { - const query = await this.query.toProto(); + getTarget(resumeToken?: Uint8Array): google.firestore.v1.ITarget { + const query = this.query.toProto(); return {query, targetId: WATCH_TARGET_ID, resumeToken}; } } diff --git a/dev/src/write-batch.ts b/dev/src/write-batch.ts index 3a8268084..1b75fb385 100644 --- a/dev/src/write-batch.ts +++ b/dev/src/write-batch.ts @@ -103,9 +103,16 @@ interface WriteOp { export class WriteBatch { private readonly _firestore: Firestore; private readonly _serializer: Serializer; - private readonly _writes: WriteOp[] = []; + + /** + * An array of write operations that are executed as part of the commit. The + * resulting `api.IWrite` will be sent to the backend. + * @private + */ + private readonly _ops: Array<() => WriteOp> = []; private _committed = false; + /** * @private * @hideconstructor @@ -123,7 +130,7 @@ export class WriteBatch { * @private */ get isEmpty(): boolean { - return this._writes.length === 0; + return this._ops.length === 0; } /** @@ -163,16 +170,24 @@ export class WriteBatch { this.verifyNotCommitted(); - const document = DocumentSnapshot.fromObject(documentRef, data); - const precondition = new Precondition({exists: false}); const transform = DocumentTransform.fromObject(documentRef, data); transform.validate(); - this._writes.push({ - write: !document.isEmpty || transform.isEmpty ? document.toProto() : null, - transform: transform.toProto(this._serializer), - precondition: precondition.toProto(), - }); + const precondition = new Precondition({exists: false}); + + const op = () => { + const document = DocumentSnapshot.fromObject(documentRef, data); + const write = + !document.isEmpty || transform.isEmpty ? document.toProto() : null; + + return { + write, + transform: transform.toProto(this._serializer), + precondition: precondition.toProto(), + }; + }; + + this._ops.push(op); return this; } @@ -209,12 +224,16 @@ export class WriteBatch { const conditions = new Precondition(precondition); - this._writes.push({ - write: { - delete: documentRef.formattedName, - }, - precondition: conditions.toProto(), - }); + const op = () => { + return { + write: { + delete: documentRef.formattedName, + }, + precondition: conditions.toProto() + }; + }; + + this._ops.push(op); return this; } @@ -271,28 +290,33 @@ export class WriteBatch { const transform = DocumentTransform.fromObject(documentRef, data); transform.validate(); - const document = DocumentSnapshot.fromObject(documentRef, data); - if (mergePaths) { - documentMask!.removeFields(transform.fields); - } else { - documentMask = DocumentMask.fromObject(data); - } + const op = () => { + const document = DocumentSnapshot.fromObject(documentRef, data); - const hasDocumentData = !document.isEmpty || !documentMask!.isEmpty; + if (mergePaths) { + documentMask!.removeFields(transform.fields); + } else { + documentMask = DocumentMask.fromObject(data); + } - let write; + const hasDocumentData = !document.isEmpty || !documentMask!.isEmpty; - if (!mergePaths && !mergeLeaves) { - write = document.toProto(); - } else if (hasDocumentData || transform.isEmpty) { - write = document.toProto()!; - write.updateMask = documentMask!.toProto(); - } + let write; - this._writes.push({ - write, - transform: transform.toProto(this._serializer), - }); + if (!mergePaths && !mergeLeaves) { + write = document.toProto(); + } else if (hasDocumentData || transform.isEmpty) { + write = document.toProto()!; + write.updateMask = documentMask!.toProto(); + } + + return { + write, + transform: transform.toProto(this._serializer), + }; + }; + + this._ops.push(op); return this; } @@ -406,24 +430,28 @@ export class WriteBatch { validateNoConflictingFields('dataOrField', updateMap); - const document = DocumentSnapshot.fromUpdateMap(documentRef, updateMap); + const transform = DocumentTransform.fromUpdateMap(documentRef, updateMap); + transform.validate(); + const documentMask = DocumentMask.fromUpdateMap(updateMap); - let write: api.IWrite|null = null; + const op = () => { + const document = DocumentSnapshot.fromUpdateMap(documentRef, updateMap); + let write: api.IWrite|null = null; - if (!document.isEmpty || !documentMask.isEmpty) { - write = document.toProto(); - write!.updateMask = documentMask.toProto(); - } + if (!document.isEmpty || !documentMask.isEmpty) { + write = document.toProto(); + write!.updateMask = documentMask.toProto(); + } - const transform = DocumentTransform.fromUpdateMap(documentRef, updateMap); - transform.validate(); + return { + write, + transform: transform.toProto(this._serializer), + precondition: precondition.toProto(), + }; + }; - this._writes.push({ - write, - transform: transform.toProto(this._serializer), - precondition: precondition.toProto(), - }); + this._ops.push(op); return this; } @@ -459,16 +487,19 @@ export class WriteBatch { * this request. * @returns A Promise that resolves when this batch completes. */ - commit_(commitOptions?: {transactionId?: Uint8Array, requestTag?: string}): + async commit_(commitOptions?: + {transactionId?: Uint8Array, requestTag?: string}): Promise { // Note: We don't call `verifyNotCommitted()` to allow for retries. + this._committed = true; + + await this._firestore.initializeIfNeeded(); const explicitTransaction = commitOptions && commitOptions.transactionId; const tag = (commitOptions && commitOptions.requestTag) || requestTag(); - const request: api.ICommitRequest = { - database: this._firestore.formattedName, - }; + const database = this._firestore.formattedName; + const request: api.ICommitRequest = {database}; // On GCF, we periodically force transactional commits to allow for // request retries in case GCF closes our backend connection. @@ -482,9 +513,10 @@ export class WriteBatch { }); } + const writes = this._ops.map(op => op()); request.writes = []; - for (const req of this._writes) { + for (const req of writes) { assert( req.write || req.transform, 'Either a write or transform must be set'); @@ -509,8 +541,6 @@ export class WriteBatch { request.transaction = explicitTransaction; } - this._committed = true; - return this._firestore .request( 'commit', request, tag, /* allowRetries= */ false) @@ -529,8 +559,8 @@ export class WriteBatch { let offset = 0; - for (let i = 0; i < this._writes.length; ++i) { - const writeRequest = this._writes[i]; + for (let i = 0; i < writes.length; ++i) { + const writeRequest = writes[i]; // Don't return two write results for a write that contains a // transform, as the fact that we have to split one write diff --git a/dev/system-test/firestore.ts b/dev/system-test/firestore.ts index cd3b78d7e..630e95974 100644 --- a/dev/system-test/firestore.ts +++ b/dev/system-test/firestore.ts @@ -1165,10 +1165,12 @@ describe('Query class', () => { const snapshot = (id: string, data: DocumentData) => { const ref = randomCol.doc(id); + const fields = ref.firestore._serializer!.encodeFields(data); return randomCol.firestore.snapshot_( { - name: ref.formattedName, - fields: ref.firestore._serializer!.encodeFields(data), + name: 'projects/ignored/databases/(default)/documents/' + + ref._path.relativeName, + fields, createTime: {seconds: 0, nanos: 0}, updateTime: {seconds: 0, nanos: 0}, }, diff --git a/dev/test/index.ts b/dev/test/index.ts index 580020a06..c1b2191be 100644 --- a/dev/test/index.ts +++ b/dev/test/index.ts @@ -22,7 +22,7 @@ import {google} from '../protos/firestore_proto_api'; import * as Firestore from '../src'; import {DocumentSnapshot, FieldPath} from '../src'; -import {ResourcePath} from '../src/path'; +import {QualifiedResourcePath} from '../src/path'; import {GrpcError} from '../src/types'; import {ApiOverride, createInstance, document, DOCUMENT_NAME, found, InvalidApiUsage, missing, stream} from './util/helpers'; @@ -217,8 +217,13 @@ const allSupportedTypesInput = { timestampValue: Firestore.Timestamp.fromDate( new Date('Mar 18, 1985 08:20:00.123 GMT+0100 (CET)')), pathValue: new Firestore.DocumentReference( - {formattedName: DATABASE_ROOT} as any, // tslint:disable-line no-any - new ResourcePath(PROJECT_ID, '(default)', 'collection', 'document')), + { + formattedName: DATABASE_ROOT, + _getProjectId: () => + ({projectId: PROJECT_ID, databaseId: '(default)'}) + } as any, // tslint:disable-line no-any + new QualifiedResourcePath( + PROJECT_ID, '(default)', 'collection', 'document')), arrayValue: ['foo', 42, 'bar'], emptyArray: [], nilValue: null, @@ -241,8 +246,13 @@ const allSupportedTypesOutput = { timestampValue: Firestore.Timestamp.fromDate( new Date('Mar 18, 1985 08:20:00.123 GMT+0100 (CET)')), pathValue: new Firestore.DocumentReference( - {formattedName: DATABASE_ROOT} as any, // tslint:disable-line no-any - new ResourcePath(PROJECT_ID, '(default)', 'collection', 'document')), + { + formattedName: DATABASE_ROOT, + _getProjectId: () => + ({projectId: PROJECT_ID, databaseId: '(default)'}) + } as any, // tslint:disable-line no-any + new QualifiedResourcePath( + PROJECT_ID, '(default)', 'collection', 'document')), arrayValue: ['foo', 42, 'bar'], emptyArray: [], nilValue: null, @@ -270,16 +280,16 @@ describe('instantiation', () => { expect(() => firestore.settings({})) .to.throw( - 'Firestore.settings() has already be called. You can only call settings() once, and only before calling any other methods on a Firestore object.'); + 'Firestore has already been initialized. You can only call settings() once, and only before calling any other methods on a Firestore object.'); }); - it('cannot change settings after client initialized', () => { + it('cannot change settings after client initialized', async () => { const firestore = new Firestore.Firestore(DEFAULT_SETTINGS); - firestore['_runRequest'](() => Promise.resolve()); + await firestore.initializeIfNeeded(); expect(() => firestore.settings({})) .to.throw( - 'Firestore has already been started and its settings can no longer be changed. You can only call settings() before calling any other methods on a Firestore object.'); + 'Firestore has already been initialized. You can only call settings() once, and only before calling any other methods on a Firestore object.'); }); it('validates project ID is string', () => { @@ -322,50 +332,26 @@ describe('instantiation', () => { }); it('uses project id from constructor', () => { - const firestore = new Firestore.Firestore(DEFAULT_SETTINGS); - - return firestore['_runRequest'](() => { - expect(firestore.formattedName) - .to.equal(`projects/${PROJECT_ID}/databases/(default)`); - return Promise.resolve(); - }); - }); - - it('detects project id', () => { - const firestore = new Firestore.Firestore({ - sslCreds: grpc.credentials.createInsecure(), - keyFilename: __dirname + '/fake-certificate.json', - }); - - expect(firestore.formattedName) - .to.equal('projects/{{projectId}}/databases/(default)'); - - firestore['_detectProjectId'] = () => Promise.resolve(PROJECT_ID); - - return firestore['_runRequest'](() => { - expect(firestore.formattedName) - .to.equal(`projects/${PROJECT_ID}/databases/(default)`); - return Promise.resolve(); - }); - }); - - it('uses project id from gapic client', () => { - const firestore = new Firestore.Firestore({ - sslCreds: grpc.credentials.createInsecure(), - keyFilename: './test/fake-certificate.json', - }); - - expect(firestore.formattedName) - .to.equal('projects/{{projectId}}/databases/(default)'); - - const gapicClient = { - getProjectId: (callback: (err: GrpcError|null, resp?: string) => void) => - callback(null, PROJECT_ID) - }; - - return firestore['_detectProjectId'](gapicClient).then(projectId => { - expect(projectId).to.equal(PROJECT_ID); - }); + const firestore = new Firestore.Firestore({projectId: 'foo'}); + + return expect(firestore.formattedName) + .to.equal(`projects/foo/databases/(default)`); + }); + + it('uses project id from gapic client', async () => { + return createInstance( + { + getProjectId: (callback => { + callback(null, 'foo'); + }) + }, + {projectId: undefined}) + .then(async firestore => { + await firestore.initializeIfNeeded(); + expect(firestore.projectId).to.equal('foo'); + expect(firestore.formattedName) + .to.equal(`projects/foo/databases/(default)`); + }); }); it('uses project ID from settings()', () => { @@ -381,19 +367,17 @@ describe('instantiation', () => { }); it('handles error from project ID detection', () => { - const firestore = new Firestore.Firestore({ - sslCreds: grpc.credentials.createInsecure(), - keyFilename: './test/fake-certificate.json', - }); - - const gapicClient = { - getProjectId: (callback: (err: GrpcError|null, resp?: string) => void) => - callback(new Error('Injected Error')) - }; - - return firestore['_detectProjectId'](gapicClient) - .then(() => expect.fail('Error ignored')) - .catch(err => expect('Injected Error').to.equal(err.message)); + return createInstance( + { + getProjectId: (callback => { + callback(new Error('Injected Error'), null); + }) + }, + {projectId: undefined}) + .then(firestore => { + return expect(firestore.collection('foo').add({})) + .to.eventually.be.rejectedWith('Injected Error'); + }); }); it('exports all types', () => { diff --git a/dev/test/order.ts b/dev/test/order.ts index 03def1868..9fd8feec0 100644 --- a/dev/test/order.ts +++ b/dev/test/order.ts @@ -22,7 +22,7 @@ import {Firestore, QueryDocumentSnapshot, setLogFunction, Timestamp} from '../sr import {GeoPoint} from '../src'; import {DocumentReference} from '../src'; import * as order from '../src/order'; -import {ResourcePath} from '../src/path'; +import {QualifiedResourcePath} from '../src/path'; import {createInstance, InvalidApiUsage} from './util/helpers'; import api = google.firestore.v1; @@ -40,22 +40,22 @@ describe('Order', () => { }); /** Converts a value into its proto representation. */ - async function wrap(value: unknown): Promise { - const val = await firestore._serializer!.encodeValue(value); + function wrap(value: unknown): api.IValue { + const val = firestore._serializer!.encodeValue(value); expect(val).to.not.be.null; return val!; } - function blob(data: number[]): Promise { + function blob(data: number[]): api.IValue { return wrap(Buffer.from(data)); } - function resource(pathString: string): Promise { + function resource(pathString: string): api.IValue { return wrap(new DocumentReference( - firestore, ResourcePath.fromSlashSeparatedString(pathString))); + firestore, QualifiedResourcePath.fromSlashSeparatedString(pathString))); } - function geopoint(lat: number, lng: number): Promise { + function geopoint(lat: number, lng: number): api.IValue { return wrap(new GeoPoint(lat, lng)); } @@ -114,98 +114,98 @@ describe('Order', () => { ]); }); - it('is correct', async () => { + it('is correct', () => { const groups = [ // null first - [await wrap(null)], + [wrap(null)], // booleans - [await wrap(false)], - [await wrap(true)], + [wrap(false)], + [wrap(true)], // numbers - [await double(NaN), double(NaN)], - [await double(-Infinity)], - [await double(-Number.MAX_VALUE)], - [await int(Number.MIN_SAFE_INTEGER - 1)], - [await int(Number.MIN_SAFE_INTEGER)], - [await double(-1.1)], + [double(NaN), double(NaN)], + [double(-Infinity)], + [double(-Number.MAX_VALUE)], + [int(Number.MIN_SAFE_INTEGER - 1)], + [int(Number.MIN_SAFE_INTEGER)], + [double(-1.1)], // Integers and Doubles order the same. - [await int(-1), double(-1.0)], - [await double(-Number.MIN_VALUE)], + [int(-1), double(-1.0)], + [double(-Number.MIN_VALUE)], // zeros all compare the same. - [await int(0), double(0.0), double(-0)], - [await double(Number.MIN_VALUE)], - [await int(1), double(1.0)], - [await double(1.1)], - [await int(2)], - [await int(10)], - [await int(Number.MAX_SAFE_INTEGER)], - [await int(Number.MAX_SAFE_INTEGER + 1)], - [await double(Infinity)], + [int(0), double(0.0), double(-0)], + [double(Number.MIN_VALUE)], + [int(1), double(1.0)], + [double(1.1)], + [int(2)], + [int(10)], + [int(Number.MAX_SAFE_INTEGER)], + [int(Number.MAX_SAFE_INTEGER + 1)], + [double(Infinity)], // timestamps - [await wrap(new Date(2016, 5, 20, 10, 20))], - [await wrap(new Date(2016, 10, 21, 15, 32))], + [wrap(new Date(2016, 5, 20, 10, 20))], + [wrap(new Date(2016, 10, 21, 15, 32))], // strings - [await wrap('')], - [await wrap('\u0000\ud7ff\ue000\uffff')], - [await wrap('(╯°□°）╯︵ ┻━┻')], - [await wrap('a')], - [await wrap('abc def')], + [wrap('')], + [wrap('\u0000\ud7ff\ue000\uffff')], + [wrap('(╯°□°）╯︵ ┻━┻')], + [wrap('a')], + [wrap('abc def')], // latin small letter e + combining acute accent + latin small letter b - [await wrap('e\u0301b')], - [await wrap('æ')], + [wrap('e\u0301b')], + [wrap('æ')], // latin small letter e with acute accent + latin small letter a - [await wrap('\u00e9a')], + [wrap('\u00e9a')], // blobs - [await blob([])], - [await blob([0])], - [await blob([0, 1, 2, 3, 4])], - [await blob([0, 1, 2, 4, 3])], - [await blob([255])], + [blob([])], + [blob([0])], + [blob([0, 1, 2, 3, 4])], + [blob([0, 1, 2, 4, 3])], + [blob([255])], // resource names - [await resource('projects/p1/databases/d1/documents/c1/doc1')], - [await resource('projects/p1/databases/d1/documents/c1/doc2')], - [await resource('projects/p1/databases/d1/documents/c1/doc2/c2/doc1')], - [await resource('projects/p1/databases/d1/documents/c1/doc2/c2/doc2')], - [await resource('projects/p1/databases/d1/documents/c10/doc1')], - [await resource('projects/p1/databases/d1/documents/c2/doc1')], - [await resource('projects/p2/databases/d2/documents/c1/doc1')], - [await resource('projects/p2/databases/d2/documents/c1-/doc1')], - [await resource('projects/p2/databases/d3/documents/c1-/doc1')], + [resource('projects/p1/databases/d1/documents/c1/doc1')], + [resource('projects/p1/databases/d1/documents/c1/doc2')], + [resource('projects/p1/databases/d1/documents/c1/doc2/c2/doc1')], + [resource('projects/p1/databases/d1/documents/c1/doc2/c2/doc2')], + [resource('projects/p1/databases/d1/documents/c10/doc1')], + [resource('projects/p1/databases/d1/documents/c2/doc1')], + [resource('projects/p2/databases/d2/documents/c1/doc1')], + [resource('projects/p2/databases/d2/documents/c1-/doc1')], + [resource('projects/p2/databases/d3/documents/c1-/doc1')], // geo points - [await geopoint(-90, -180)], - [await geopoint(-90, 0)], - [await geopoint(-90, 180)], - [await geopoint(0, -180)], - [await geopoint(0, 0)], - [await geopoint(0, 180)], - [await geopoint(1, -180)], - [await geopoint(1, 0)], - [await geopoint(1, 180)], - [await geopoint(90, -180)], - [await geopoint(90, 0)], - [await geopoint(90, 180)], + [geopoint(-90, -180)], + [geopoint(-90, 0)], + [geopoint(-90, 180)], + [geopoint(0, -180)], + [geopoint(0, 0)], + [geopoint(0, 180)], + [geopoint(1, -180)], + [geopoint(1, 0)], + [geopoint(1, 180)], + [geopoint(90, -180)], + [geopoint(90, 0)], + [geopoint(90, 180)], // arrays - [await wrap([])], - [await wrap(['bar'])], - [await wrap(['foo'])], - [await wrap(['foo', 1])], - [await wrap(['foo', 2])], - [await wrap(['foo', '0'])], + [wrap([])], + [wrap(['bar'])], + [wrap(['foo'])], + [wrap(['foo', 1])], + [wrap(['foo', 2])], + [wrap(['foo', '0'])], // objects - [await wrap({bar: 0})], - [await wrap({bar: 0, foo: 1})], - [await wrap({foo: 1})], - [await wrap({foo: 2})], - [await wrap({foo: '0'})], + [wrap({bar: 0})], + [wrap({bar: 0, foo: 1})], + [wrap({foo: 1})], + [wrap({foo: 2})], + [wrap({foo: '0'})], ]; for (let i = 0; i < groups.length; i++) { diff --git a/dev/test/path.ts b/dev/test/path.ts index a2f109c2c..ded27024f 100644 --- a/dev/test/path.ts +++ b/dev/test/path.ts @@ -16,7 +16,7 @@ import {expect} from 'chai'; -import {FieldPath, ResourcePath} from '../src/path'; +import {FieldPath, QualifiedResourcePath} from '../src/path'; import {InvalidApiUsage} from './util/helpers'; const PROJECT_ID = 'test-project'; @@ -24,19 +24,20 @@ const DATABASE_ROOT = `projects/${PROJECT_ID}/databases/(default)`; describe('ResourcePath', () => { it('has id property', () => { - expect(new ResourcePath(PROJECT_ID, '(default)', 'foo').id).to.equal('foo'); - expect(new ResourcePath(PROJECT_ID, '(default)').id).to.be.null; + expect(new QualifiedResourcePath(PROJECT_ID, '(default)', 'foo').id) + .to.equal('foo'); + expect(new QualifiedResourcePath(PROJECT_ID, '(default)').id).to.be.null; }); it('has append() method', () => { - let path = new ResourcePath(PROJECT_ID, '(default)'); + let path = new QualifiedResourcePath(PROJECT_ID, '(default)'); expect(path.formattedName).to.equal(`${DATABASE_ROOT}/documents`); path = path.append('foo'); expect(path.formattedName).to.equal(`${DATABASE_ROOT}/documents/foo`); }); it('has parent() method', () => { - let path = new ResourcePath(PROJECT_ID, '(default)', 'foo'); + let path = new QualifiedResourcePath(PROJECT_ID, '(default)', 'foo'); expect(path.formattedName).to.equal(`${DATABASE_ROOT}/documents/foo`); path = path.parent()!; expect(path.formattedName).to.equal(`${DATABASE_ROOT}/documents`); @@ -44,19 +45,19 @@ describe('ResourcePath', () => { }); it('parses strings', () => { - let path = ResourcePath.fromSlashSeparatedString(DATABASE_ROOT); + let path = QualifiedResourcePath.fromSlashSeparatedString(DATABASE_ROOT); expect(path.formattedName).to.equal(`${DATABASE_ROOT}/documents`); - path = - ResourcePath.fromSlashSeparatedString(`${DATABASE_ROOT}/documents/foo`); + path = QualifiedResourcePath.fromSlashSeparatedString( + `${DATABASE_ROOT}/documents/foo`); expect(path.formattedName).to.equal(`${DATABASE_ROOT}/documents/foo`); expect(() => { - path = - ResourcePath.fromSlashSeparatedString('projects/project/databases'); + path = QualifiedResourcePath.fromSlashSeparatedString( + 'projects/project/databases'); }).to.throw('Resource name \'projects\/project\/databases\' is not valid'); }); it('accepts newlines', () => { - const path = ResourcePath.fromSlashSeparatedString( + const path = QualifiedResourcePath.fromSlashSeparatedString( `${DATABASE_ROOT}/documents/foo\nbar`); expect(path.formattedName).to.equal(`${DATABASE_ROOT}/documents/foo\nbar`); }); diff --git a/dev/test/query.ts b/dev/test/query.ts index e1e3c2cd4..e0e9781b2 100644 --- a/dev/test/query.ts +++ b/dev/test/query.ts @@ -21,7 +21,7 @@ import {google} from '../protos/firestore_proto_api'; import {FieldPath, FieldValue, Firestore, setLogFunction} from '../src'; import {DocumentData, DocumentReference, Query, Timestamp} from '../src'; import {DocumentSnapshot, DocumentSnapshotBuilder} from '../src/document'; -import {ResourcePath} from '../src/path'; +import {QualifiedResourcePath} from '../src/path'; import {ApiOverride, createInstance, document, InvalidApiUsage, stream} from './util/helpers'; import api = google.firestore.v1; @@ -36,7 +36,7 @@ function snapshot( relativePath: string, data: DocumentData): Promise { return createInstance().then(firestore => { const snapshot = new DocumentSnapshotBuilder(); - const path = ResourcePath.fromSlashSeparatedString( + const path = QualifiedResourcePath.fromSlashSeparatedString( `${DATABASE_ROOT}/documents/${relativePath}`); snapshot.ref = new DocumentReference(firestore, path); snapshot.fieldsProto = firestore['_serializer']!.encodeFields(data); @@ -247,6 +247,10 @@ function queryEquals( extend(true, query.structuredQuery, protoComponent); } + // 'extend' removes undefined fields in the request object. The backend + // ignores these fields, but we need to manually strip them before we compare + // the expected and the actual request. + actual = extend(true, {}, actual); expect(actual).to.deep.eq(query); } @@ -340,8 +344,7 @@ describe('query interface', () => { queryEquals( [ query.orderBy('foo').orderBy('__name__').startAt('b', 'c'), - query.orderBy('foo').startAt( - firestore.snapshot_(document('c', 'foo', 'b'), {})), + query.orderBy('foo').orderBy('__name__').startAt('b', 'c') ], []); }); diff --git a/dev/test/transaction.ts b/dev/test/transaction.ts index e9ae0c6a6..fda12554d 100644 --- a/dev/test/transaction.ts +++ b/dev/test/transaction.ts @@ -16,6 +16,7 @@ import {expect, use} from 'chai'; import * as chaiAsPromised from 'chai-as-promised'; +import * as extend from 'extend'; import * as through2 from 'through2'; import * as proto from '../protos/firestore_proto_api'; @@ -283,6 +284,7 @@ function runTransaction( runQuery: (actual) => { const request = expectedRequests.shift()!; expect(request.type).to.equal('query'); + actual = extend(true, {}, actual); // Remove undefined properties expect(actual).to.deep.eq(request.request); return request.stream!; } diff --git a/dev/test/util/helpers.ts b/dev/test/util/helpers.ts index 2c8790c67..94d88e9a8 100644 --- a/dev/test/util/helpers.ts +++ b/dev/test/util/helpers.ts @@ -15,6 +15,7 @@ */ import {expect} from 'chai'; +import * as extend from 'extend'; import {CallOptions, GrpcClient} from 'google-gax'; import * as through2 from 'through2'; @@ -83,6 +84,8 @@ export type ApiOverride = { NodeJS.ReadableStream; runQuery?: (request: api.IRunQueryRequest) => NodeJS.ReadableStream; listen?: () => NodeJS.ReadWriteStream; + getProjectId?: (callback: (err?: Error|null, projectId?: string|null) => + void) => void; }; /** @@ -112,15 +115,18 @@ export function createInstance( const gapicClient: GapicClient = new v1(initializationOptions); if (apiOverrides) { Object.keys(apiOverrides).forEach(override => { - gapicClient._innerApiCalls[override] = - (apiOverrides as {[k: string]: unknown})[override]; + const apiOverride = (apiOverrides as {[k: string]: unknown})[override]; + if (override !== 'getProjectId') { + gapicClient._innerApiCalls[override] = apiOverride; + } else { + gapicClient[override] = apiOverride; + } }); } return gapicClient; }); - // tslint:disable-next-line:no-any - (firestore as any)._initClientPool = () => Promise.resolve(clientPool); + firestore['_clientPool'] = clientPool; return Promise.resolve(firestore); } @@ -312,12 +318,12 @@ export function writeResult(count: number): api.IWriteResponse { } export function requestEquals( - actual: object, expected: {[k: string]: unknown}): void { - const proto = Object.assign( - { - database: DATABASE_ROOT, - }, - expected); + actual: {[k: string]: unknown}, expected: {[k: string]: unknown}): void { + // 'extend' removes undefined fields in the request object. The backend + // ignores these fields, but we need to manually strip them before we compare + // the expected and the actual request. + actual = extend(true, {}, actual); + const proto = Object.assign({database: DATABASE_ROOT}, expected); expect(actual).to.deep.eq(proto); } diff --git a/dev/test/watch.ts b/dev/test/watch.ts index ce6313869..c1f007b58 100644 --- a/dev/test/watch.ts +++ b/dev/test/watch.ts @@ -17,6 +17,7 @@ const duplexify = require('duplexify'); import {expect} from 'chai'; +import * as extend from 'extend'; import {Transform} from 'stream'; import * as through2 from 'through2'; @@ -146,6 +147,12 @@ const modified = (ref: DocumentReference, data: DocumentData) => const removed = (ref: DocumentReference, data: DocumentData) => docChange('removed', ref, data); +function verifyRequest(actual: T, expected: T): void { + // Remove undefined value, as these are ignored by the backend. + actual = extend(true, {}, actual); + expect(actual).to.deep.equal(expected); +} + const EMPTY = { docs: [], docChanges: [] @@ -492,7 +499,7 @@ class WatchHelper { return this.streamHelper.awaitOpen() .then(request => { - expect(request).to.deep.eq(expectedRequest); + verifyRequest(request, expectedRequest); return func(); }) .then(() => this.endWatch()); @@ -508,7 +515,7 @@ class WatchHelper { return this.streamHelper.awaitOpen() .then(request => { - expect(request).to.deep.eq(expectedRequest); + verifyRequest(request, expectedRequest); return func(); }) .then(() => { @@ -755,7 +762,7 @@ describe('Query watch', () => { const unsubscribe = watchHelper.startWatch(); return streamHelper.awaitOpen() .then(request => { - expect(request).to.deep.eq(collQueryJSON()); + verifyRequest(request, collQueryJSON()); watchHelper.sendAddTarget(); watchHelper.sendCurrent(); watchHelper.sendSnapshot(1, Buffer.from([0xabcd])); @@ -915,7 +922,7 @@ describe('Query watch', () => { return streamHelper.awaitReopen(); }) .then(request => { - expect(request).to.deep.eq(resumeTokenQuery(resumeToken)); + verifyRequest(request, resumeTokenQuery(resumeToken)); watchHelper.sendAddTarget(); watchHelper.sendDoc(doc2, {foo: 'b'}); @@ -932,7 +939,7 @@ describe('Query watch', () => { return streamHelper.awaitReopen(); }) .then(request => { - expect(request).to.deep.eq(resumeTokenQuery(resumeToken)); + verifyRequest(request, resumeTokenQuery(resumeToken)); watchHelper.sendAddTarget(); watchHelper.sendDoc(doc3, {foo: 'c'}); watchHelper.sendSnapshot(4, resumeToken); @@ -1042,7 +1049,7 @@ describe('Query watch', () => { return streamHelper.awaitReopen(); }) .then(request => { - expect(request).to.deep.eq(resumeTokenQuery(resumeToken)); + verifyRequest(request, resumeTokenQuery(resumeToken)); expect(streamHelper.streamCount).to.equal(2); }); }); @@ -1610,7 +1617,7 @@ describe('Query watch', () => { .then(request => { expect(streamHelper.streamCount).to.equal(2); expect(oldRequestStream).to.not.equal(streamHelper.writeStream); - expect(collQueryJSON()).to.deep.eq(request); + verifyRequest(request, collQueryJSON()); watchHelper.sendAddTarget(); watchHelper.sendCurrent(); @@ -1823,7 +1830,7 @@ describe('Query watch', () => { watchHelper.sendCurrent(); watchHelper.sendSnapshot(++snapshotVersion); return watchHelper.await('snapshot') - .then(snapshot => watchTest(snapshot as QuerySnapshot)); + .then(snapshot => watchTest(snapshot)); }); } @@ -1969,16 +1976,11 @@ describe('Query watch', () => { return initialSnapshot(snapshot => { return nextSnapshot( - snapshot, - () => { - watchHelper.sendDoc(doc1, {foo: 'a'}); - }) + snapshot, () => watchHelper.sendDoc(doc1, {foo: 'a'})) .then( snapshot => nextSnapshot( snapshot, - () => { - watchHelper.sendDoc(doc2, {foo: 'b'}); - })) + () => watchHelper.sendDoc(doc2, {foo: 'b'}))) .then(snapshot => { firstSnapshot = snapshot; }); @@ -1986,9 +1988,7 @@ describe('Query watch', () => { .then(() => initialSnapshot(snapshot => { return nextSnapshot( snapshot, - () => { - watchHelper.sendDoc(doc1, {foo: 'a'}); - }) + () => watchHelper.sendDoc(doc1, {foo: 'a'})) .then( snapshot => nextSnapshot( snapshot, @@ -2007,18 +2007,19 @@ describe('Query watch', () => { let originalSnapshot: QuerySnapshot; return initialSnapshot(snapshot => { - return nextSnapshot(snapshot, () => { - watchHelper.sendDoc(doc1, {foo: '1'}); - }).then(snapshot => { - originalSnapshot = snapshot; - }); + return nextSnapshot( + snapshot, () => watchHelper.sendDoc(doc1, {foo: '1'})) + .then(snapshot => { + originalSnapshot = snapshot; + }); }) .then(() => initialSnapshot(snapshot => { - return nextSnapshot(snapshot, () => { - watchHelper.sendDoc(doc1, {foo: 1}); - }).then(snapshot => { - expect(snapshot.isEqual(originalSnapshot)).to.be.false; - }); + return nextSnapshot( + snapshot, + () => watchHelper.sendDoc(doc1, {foo: 1})) + .then(snapshot => { + expect(snapshot.isEqual(originalSnapshot)).to.be.false; + }); })); }); @@ -2253,7 +2254,7 @@ describe('DocumentReference watch', () => { streamHelper.write({ documentChange: { document: { - name: doc.parent.formattedName + '/wrong', + name: `projects/${PROJECT_ID}/databases/(default)/col/wrong`, fields: {}, createTime: {seconds: 1, nanos: 2}, updateTime: {seconds: 3, nanos: 4}, @@ -2296,7 +2297,7 @@ describe('DocumentReference watch', () => { return streamHelper.awaitReopen(); }) .then(request => { - expect(request).to.deep.eq(resumeTokenJSON(resumeToken)); + verifyRequest(request, resumeTokenJSON(resumeToken)); // Change the document. watchHelper.sendDoc(doc, {foo: 'b'}); watchHelper.sendSnapshot(3, resumeToken); diff --git a/package.json b/package.json index 1b0653749..341bdb55c 100644 --- a/package.json +++ b/package.json @@ -47,14 +47,11 @@ "predocs-test": "npm run docs" }, "dependencies": { - "@google-cloud/projectify": "^0.3.0", "bun": "^0.0.12", "deep-equal": "^1.0.1", - "extend": "^3.0.1", "functional-red-black-tree": "^1.0.1", "google-gax": "^0.25.0", "lodash.merge": "^4.6.1", - "protobufjs": "^6.8.6", "through2": "^3.0.0" }, "devDependencies": { @@ -71,6 +68,7 @@ "chai-as-promised": "^7.1.1", "codecov": "^3.0.2", "duplexify": "^4.0.0", + "extend": "^3.0.1", "gts": "^0.9.0", "hard-rejection": "^1.0.0", "jsdoc-baseline": "git+https://github.com/hegemonic/jsdoc-baseline.git", @@ -79,6 +77,7 @@ "mocha": "^6.0.0", "nyc": "^13.0.0", "proxyquire": "^2.0.1", + "protobufjs": "^6.8.6", "source-map-support": "^0.5.6", "ts-node": "^8.0.0", "typescript": "~3.3.0",