From 8dd846dd8de1cb306133ad9c2721782a426be60e Mon Sep 17 00:00:00 2001 From: Hui Wu Date: Wed, 24 Jul 2019 14:45:31 -0400 Subject: [PATCH 01/10] pending firebaseApp change --- packages/firebase/index.d.ts | 5 +- packages/firestore-types/index.d.ts | 5 +- packages/firestore/src/api/database.ts | 235 ++++++++++-------- .../firestore/src/core/firestore_client.ts | 53 ++-- packages/firestore/src/util/async_queue.ts | 64 ++++- .../test/integration/api/database.test.ts | 27 +- .../test/unit/util/async_queue.test.ts | 22 ++ 7 files changed, 275 insertions(+), 136 deletions(-) diff --git a/packages/firebase/index.d.ts b/packages/firebase/index.d.ts index 28b43b2a311..0524ac36de8 100644 --- a/packages/firebase/index.d.ts +++ b/packages/firebase/index.d.ts @@ -6225,7 +6225,10 @@ declare namespace firebase.firestore { /** * @hidden */ - INTERNAL: { delete: () => Promise }; + INTERNAL: { + delete: () => Promise + isShutdown: () => boolean + }; } /** diff --git a/packages/firestore-types/index.d.ts b/packages/firestore-types/index.d.ts index c96fa5925dc..c783db59454 100644 --- a/packages/firestore-types/index.d.ts +++ b/packages/firestore-types/index.d.ts @@ -276,7 +276,10 @@ export class FirebaseFirestore { */ disableNetwork(): Promise; - INTERNAL: { delete: () => Promise }; + INTERNAL: { + delete: () => Promise + isShutdown: () => boolean + }; } /** diff --git a/packages/firestore/src/api/database.ts b/packages/firestore/src/api/database.ts index 161435d773c..4c60c0a9c11 100644 --- a/packages/firestore/src/api/database.ts +++ b/packages/firestore/src/api/database.ts @@ -353,7 +353,7 @@ export class Firestore implements firestore.FirebaseFirestore, FirebaseService { throw new FirestoreError( Code.INVALID_ARGUMENT, '"persistence" is now specified with a separate call to ' + - 'firestore.enablePersistence().' + 'firestore.enablePersistence().' ); } @@ -362,8 +362,8 @@ export class Firestore implements firestore.FirebaseFirestore, FirebaseService { throw new FirestoreError( Code.FAILED_PRECONDITION, '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.' + 'be changed. You can only call settings() before calling any other ' + + 'methods on a Firestore object.' ); } @@ -390,8 +390,8 @@ export class Firestore implements firestore.FirebaseFirestore, FirebaseService { throw new FirestoreError( Code.FAILED_PRECONDITION, 'Firestore has already been started and persistence can no longer ' + - 'be enabled. You can only call enablePersistence() before calling ' + - 'any other methods on a Firestore object.' + 'be enabled. You can only call enablePersistence() before calling ' + + 'any other methods on a Firestore object.' ); } @@ -401,9 +401,9 @@ export class Firestore implements firestore.FirebaseFirestore, FirebaseService { if (settings.experimentalTabSynchronization !== undefined) { log.error( "The 'experimentalTabSynchronization' setting has been renamed to " + - "'synchronizeTabs'. In a future release, the setting will be removed " + - 'and it is recommended that you update your ' + - "firestore.enablePersistence() call to use 'synchronizeTabs'." + "'synchronizeTabs'. In a future release, the setting will be removed " + + 'and it is recommended that you update your ' + + "firestore.enablePersistence() call to use 'synchronizeTabs'." ); } synchronizeTabs = objUtils.defaulted( @@ -427,7 +427,7 @@ export class Firestore implements firestore.FirebaseFirestore, FirebaseService { this.makeDatabaseInfo() ); const deferred = new Deferred(); - this._queue.enqueueAndForget(async () => { + this._queue.enqueueAndForgetEvenAfterShutdown(async () => { try { if ( this._firestoreClient !== undefined && @@ -486,8 +486,8 @@ export class Firestore implements firestore.FirebaseFirestore, FirebaseService { throw new FirestoreError( Code.INVALID_ARGUMENT, 'Document reference is for database ' + - `${otherDb.projectId}/${otherDb.database} but should be ` + - `for database ${thisDb.projectId}/${thisDb.database}` + `${otherDb.projectId}/${otherDb.database} but should be ` + + `for database ${thisDb.projectId}/${thisDb.database}` ); } return new DocumentKeyReference(this._config.databaseId, value._key); @@ -531,18 +531,49 @@ export class Firestore implements firestore.FirebaseFirestore, FirebaseService { throw new FirestoreError( Code.FAILED_PRECONDITION, "Firestore was not initialized using the Firebase SDK. 'app' is " + - 'not available' + 'not available' ); } return this._config.firebaseApp; } + /** + * Shuts down this FirebaseFirestore instance. + * + * After shutdown only the `clearPersistence()` method may be used. Any other method + * will throw an `FirestoreError`. + * + * To restart after shutdown, simply create a new instance of FirebaseFirestore with + * `Firebase.firestore()`. + * + * Shutdown does not cancel any pending writes and any promises that are awaiting a response from + * the server will be resolved with `undefined`. The next time you start this instance, it will resume + * attempting to send these writes to the server. + * + * Note: Under normal circumstances, calling `shutdown()` is not required. This + * method is useful only when you want to force this instance to release all of its resources or + * in combination with `clearPersistence()` to ensure that all local state is destroyed + * between test runs. + * + * @return A promise that is resolved when the instance has been successfully shut down. + */ + // TODO(b/135755126): make this public. + private shutdown(): Promise { + return this.INTERNAL.delete(); + } + INTERNAL = { + // TODO(b/135755126): Make this public delete: async (): Promise => { // The client must be initalized to ensure that all subsequent API usage // throws an exception. this.ensureClientConfigured(); await this._firestoreClient!.shutdown(); + }, + + // TODO(b/135755126): make this public. + isShutdown: (): boolean => { + return this._firestoreClient!.clientShutdown; } }; @@ -572,7 +603,7 @@ export class Firestore implements firestore.FirebaseFirestore, FirebaseService { throw new FirestoreError( Code.INVALID_ARGUMENT, `Invalid collection ID '${collectionId}' passed to function ` + - `Firestore.collectionGroup(). Collection IDs must not contain '/'.` + `Firestore.collectionGroup(). Collection IDs must not contain '/'.` ); } this.ensureClientConfigured(); @@ -648,7 +679,7 @@ export class Transaction implements firestore.Transaction { constructor( private _firestore: Firestore, private _transaction: InternalTransaction - ) {} + ) { } get( documentRef: firestore.DocumentReference @@ -705,10 +736,10 @@ export class Transaction implements firestore.Transaction { const parsed = options.merge || options.mergeFields ? this._firestore._dataConverter.parseMergeData( - 'Transaction.set', - value, - options.mergeFields - ) + 'Transaction.set', + value, + options.mergeFields + ) : this._firestore._dataConverter.parseSetData('Transaction.set', value); this._transaction.set(ref._key, parsed); return this; @@ -782,7 +813,7 @@ export class WriteBatch implements firestore.WriteBatch { private _mutations = [] as Mutation[]; private _committed = false; - constructor(private _firestore: Firestore) {} + constructor(private _firestore: Firestore) { } set( documentRef: firestore.DocumentReference, @@ -800,10 +831,10 @@ export class WriteBatch implements firestore.WriteBatch { const parsed = options.merge || options.mergeFields ? this._firestore._dataConverter.parseMergeData( - 'WriteBatch.set', - value, - options.mergeFields - ) + 'WriteBatch.set', + value, + options.mergeFields + ) : this._firestore._dataConverter.parseSetData('WriteBatch.set', value); this._mutations = this._mutations.concat( parsed.toMutations(ref._key, Precondition.NONE) @@ -894,7 +925,7 @@ export class WriteBatch implements firestore.WriteBatch { throw new FirestoreError( Code.FAILED_PRECONDITION, 'A write batch can no longer be used after commit() ' + - 'has been called.' + 'has been called.' ); } } @@ -915,8 +946,8 @@ export class DocumentReference implements firestore.DocumentReference { throw new FirestoreError( Code.INVALID_ARGUMENT, 'Invalid document reference. Document ' + - 'references must have an even number of segments, but ' + - `${path.canonicalString()} has ${path.length}` + 'references must have an even number of segments, but ' + + `${path.canonicalString()} has ${path.length}` ); } return new DocumentReference(new DocumentKey(path), firestore); @@ -969,14 +1000,14 @@ export class DocumentReference implements firestore.DocumentReference { const parsed = options.merge || options.mergeFields ? this.firestore._dataConverter.parseMergeData( - 'DocumentReference.set', - value, - options.mergeFields - ) + 'DocumentReference.set', + value, + options.mergeFields + ) : this.firestore._dataConverter.parseSetData( - 'DocumentReference.set', - value - ); + 'DocumentReference.set', + value + ); return this._firestoreClient.write( parsed.toMutations(this._key, Precondition.NONE) ); @@ -1020,7 +1051,7 @@ export class DocumentReference implements firestore.DocumentReference { } delete(): Promise { - validateExactNumberOfArgs('DocumentReference.delete', arguments, 0); + validateExactNumberOfArgs('FirebaseFirestore.shutdown', arguments, 0); return this._firestoreClient.write([ new DeleteMutation(this._key, Precondition.NONE) ]); @@ -1220,9 +1251,9 @@ export class DocumentReference implements firestore.DocumentReference { new FirestoreError( Code.UNAVAILABLE, 'Failed to get document from server. (However, this ' + - 'document does exist in the local cache. Run again ' + - 'without setting source to "server" to ' + - 'retrieve the cached document.)' + 'document does exist in the local cache. Run again ' + + 'without setting source to "server" to ' + + 'retrieve the cached document.)' ) ); } else { @@ -1239,7 +1270,7 @@ class SnapshotMetadata implements firestore.SnapshotMetadata { constructor( readonly hasPendingWrites: boolean, readonly fromCache: boolean - ) {} + ) { } isEqual(other: firestore.SnapshotMetadata): boolean { return ( @@ -1253,7 +1284,7 @@ class SnapshotMetadata implements firestore.SnapshotMetadata { * Options interface that can be provided to configure the deserialization of * DocumentSnapshots. */ -export interface SnapshotOptions extends firestore.SnapshotOptions {} +export interface SnapshotOptions extends firestore.SnapshotOptions { } export class DocumentSnapshot implements firestore.DocumentSnapshot { constructor( @@ -1262,7 +1293,7 @@ export class DocumentSnapshot implements firestore.DocumentSnapshot { public _document: Document | null, private _fromCache: boolean, private _hasPendingWrites: boolean - ) {} + ) { } data( options?: firestore.SnapshotOptions @@ -1272,12 +1303,12 @@ export class DocumentSnapshot implements firestore.DocumentSnapshot { return !this._document ? undefined : this.convertObject( - this._document.data, - FieldValueOptions.fromSnapshotOptions( - options, - this._firestore._areTimestampsInSnapshotsEnabled() - ) - ); + this._document.data, + FieldValueOptions.fromSnapshotOptions( + options, + this._firestore._areTimestampsInSnapshotsEnabled() + ) + ); } get( @@ -1356,11 +1387,11 @@ export class DocumentSnapshot implements firestore.DocumentSnapshot { // TODO(b/64130202): Somehow support foreign references. log.error( `Document ${this._key.path} contains a document ` + - `reference within a different database (` + - `${value.databaseId.projectId}/${value.databaseId.database}) which is not ` + - `supported. It will be treated as a reference in the current ` + - `database (${database.projectId}/${database.database}) ` + - `instead.` + `reference within a different database (` + + `${value.databaseId.projectId}/${value.databaseId.database}) which is not ` + + `supported. It will be treated as a reference in the current ` + + `database (${database.projectId}/${database.database}) ` + + `instead.` ); } return new DocumentReference(key, this._firestore); @@ -1392,7 +1423,7 @@ export class QueryDocumentSnapshot extends DocumentSnapshot } export class Query implements firestore.Query { - constructor(public _query: InternalQuery, readonly firestore: Firestore) {} + constructor(public _query: InternalQuery, readonly firestore: Firestore) { } where( field: string | ExternalFieldPath, @@ -1423,7 +1454,7 @@ export class Query implements firestore.Query { throw new FirestoreError( Code.INVALID_ARGUMENT, `Invalid Query. You can't perform '${operator.toString()}' ` + - 'queries on FieldPath.documentId().' + 'queries on FieldPath.documentId().' ); } else if (operator === Operator.IN) { this.validateDisjunctiveFilterElements(value, operator); @@ -1472,21 +1503,21 @@ export class Query implements firestore.Query { throw new FirestoreError( Code.INVALID_ARGUMENT, `Function Query.orderBy() has unknown direction '${directionStr}', ` + - `expected 'asc' or 'desc'.` + `expected 'asc' or 'desc'.` ); } if (this._query.startAt !== null) { throw new FirestoreError( Code.INVALID_ARGUMENT, 'Invalid query. You must not call Query.startAt() or ' + - 'Query.startAfter() before calling Query.orderBy().' + 'Query.startAfter() before calling Query.orderBy().' ); } if (this._query.endAt !== null) { throw new FirestoreError( Code.INVALID_ARGUMENT, 'Invalid query. You must not call Query.endAt() or ' + - 'Query.endBefore() before calling Query.orderBy().' + 'Query.endBefore() before calling Query.orderBy().' ); } const fieldPath = fieldPathFromArgument('Query.orderBy', field); @@ -1502,7 +1533,7 @@ export class Query implements firestore.Query { throw new FirestoreError( Code.INVALID_ARGUMENT, `Invalid Query. Query limit (${n}) is invalid. Limit must be ` + - 'positive.' + 'positive.' ); } return new Query(this._query.withLimit(n), this.firestore); @@ -1593,7 +1624,7 @@ export class Query implements firestore.Query { throw new FirestoreError( Code.NOT_FOUND, `Can't use a DocumentSnapshot that doesn't exist for ` + - `${methodName}().` + `${methodName}().` ); } return this.boundFromDocument(methodName, snap._document!, before); @@ -1637,10 +1668,10 @@ export class Query implements firestore.Query { throw new FirestoreError( Code.INVALID_ARGUMENT, 'Invalid query. You are trying to start or end a query using a ' + - 'document for which the field "' + - orderBy.field + - '" is an uncommitted server timestamp. (Since the value of ' + - 'this field is unknown, you cannot start/end a query with it.)' + 'document for which the field "' + + orderBy.field + + '" is an uncommitted server timestamp. (Since the value of ' + + 'this field is unknown, you cannot start/end a query with it.)' ); } else if (value !== null) { components.push(value); @@ -1649,8 +1680,8 @@ export class Query implements firestore.Query { throw new FirestoreError( Code.INVALID_ARGUMENT, `Invalid query. You are trying to start or end a query using a ` + - `document for which the field '${field}' (used as the ` + - `orderBy) does not exist.` + `document for which the field '${field}' (used as the ` + + `orderBy) does not exist.` ); } } @@ -1672,8 +1703,8 @@ export class Query implements firestore.Query { throw new FirestoreError( Code.INVALID_ARGUMENT, `Too many arguments provided to ${methodName}(). ` + - `The number of arguments must be less than or equal to the ` + - `number of Query.orderBy() clauses` + `The number of arguments must be less than or equal to the ` + + `number of Query.orderBy() clauses` ); } @@ -1686,7 +1717,7 @@ export class Query implements firestore.Query { throw new FirestoreError( Code.INVALID_ARGUMENT, `Invalid query. Expected a string for document ID in ` + - `${methodName}(), but got a ${typeof rawValue}` + `${methodName}(), but got a ${typeof rawValue}` ); } if ( @@ -1696,8 +1727,8 @@ export class Query implements firestore.Query { throw new FirestoreError( Code.INVALID_ARGUMENT, `Invalid query. When querying a collection and ordering by FieldPath.documentId(), ` + - `the value passed to ${methodName}() must be a plain document ID, but ` + - `'${rawValue}' contains a slash.` + `the value passed to ${methodName}() must be a plain document ID, but ` + + `'${rawValue}' contains a slash.` ); } const path = this._query.path.child(ResourcePath.fromString(rawValue)); @@ -1705,9 +1736,9 @@ export class Query implements firestore.Query { throw new FirestoreError( Code.INVALID_ARGUMENT, `Invalid query. When querying a collection group and ordering by ` + - `FieldPath.documentId(), the value passed to ${methodName}() must result in a ` + - `valid document path, but '${path}' is not because it contains an odd number ` + - `of segments.` + `FieldPath.documentId(), the value passed to ${methodName}() must result in a ` + + `valid document path, but '${path}' is not because it contains an odd number ` + + `of segments.` ); } const key = new DocumentKey(path); @@ -1864,9 +1895,9 @@ export class Query implements firestore.Query { new FirestoreError( Code.UNAVAILABLE, 'Failed to get documents from server. (However, these ' + - 'documents may exist in the local cache. Run again ' + - 'without setting source to "server" to ' + - 'retrieve the cached documents.)' + 'documents may exist in the local cache. Run again ' + + 'without setting source to "server" to ' + + 'retrieve the cached documents.)' ) ); } else { @@ -1889,7 +1920,7 @@ export class Query implements firestore.Query { throw new FirestoreError( Code.INVALID_ARGUMENT, 'Invalid query. When querying with FieldPath.documentId(), you ' + - 'must provide a valid document ID, but it was an empty string.' + 'must provide a valid document ID, but it was an empty string.' ); } if ( @@ -1899,8 +1930,8 @@ export class Query implements firestore.Query { throw new FirestoreError( Code.INVALID_ARGUMENT, `Invalid query. When querying a collection by ` + - `FieldPath.documentId(), you must provide a plain document ID, but ` + - `'${documentIdValue}' contains a '/' character.` + `FieldPath.documentId(), you must provide a plain document ID, but ` + + `'${documentIdValue}' contains a '/' character.` ); } const path = this._query.path.child( @@ -1910,8 +1941,8 @@ export class Query implements firestore.Query { throw new FirestoreError( Code.INVALID_ARGUMENT, `Invalid query. When querying a collection group by ` + - `FieldPath.documentId(), the value provided must result in a valid document path, ` + - `but '${path}' is not because it has an odd number of segments (${path.length}).` + `FieldPath.documentId(), the value provided must result in a valid document path, ` + + `but '${path}' is not because it has an odd number of segments (${path.length}).` ); } return new RefValue(this.firestore._databaseId, new DocumentKey(path)); @@ -1922,8 +1953,8 @@ export class Query implements firestore.Query { throw new FirestoreError( Code.INVALID_ARGUMENT, `Invalid query. When querying with FieldPath.documentId(), you must provide a valid ` + - `string or a DocumentReference, but it was: ` + - `${valueDescription(documentIdValue)}.` + `string or a DocumentReference, but it was: ` + + `${valueDescription(documentIdValue)}.` ); } } @@ -1940,28 +1971,28 @@ export class Query implements firestore.Query { throw new FirestoreError( Code.INVALID_ARGUMENT, 'Invalid Query. A non-empty array is required for ' + - `'${operator.toString()}' filters.` + `'${operator.toString()}' filters.` ); } if (value.length > 10) { throw new FirestoreError( Code.INVALID_ARGUMENT, `Invalid Query. '${operator.toString()}' filters support a ` + - 'maximum of 10 elements in the value array.' + 'maximum of 10 elements in the value array.' ); } if (value.indexOf(null) >= 0) { throw new FirestoreError( Code.INVALID_ARGUMENT, `Invalid Query. '${operator.toString()}' filters cannot contain 'null' ` + - 'in the value array.' + 'in the value array.' ); } if (value.filter(element => Number.isNaN(element)).length > 0) { throw new FirestoreError( Code.INVALID_ARGUMENT, `Invalid Query. '${operator.toString()}' filters cannot contain 'NaN' ` + - 'in the value array.' + 'in the value array.' ); } } @@ -1979,9 +2010,9 @@ export class Query implements firestore.Query { throw new FirestoreError( Code.INVALID_ARGUMENT, 'Invalid query. All where filters with an inequality' + - ' (<, <=, >, or >=) must be on the same field. But you have' + - ` inequality filters on '${existingField.toString()}'` + - ` and '${filter.field.toString()}'` + ' (<, <=, >, or >=) must be on the same field. But you have' + + ` inequality filters on '${existingField.toString()}'` + + ` and '${filter.field.toString()}'` ); } @@ -2008,13 +2039,13 @@ export class Query implements firestore.Query { throw new FirestoreError( Code.INVALID_ARGUMENT, 'Invalid query. You cannot use more than one ' + - `'${filter.op.toString()}' filter.` + `'${filter.op.toString()}' filter.` ); } else { throw new FirestoreError( Code.INVALID_ARGUMENT, `Invalid query. You cannot use '${filter.op.toString()}' filters ` + - `with '${conflictingOp.toString()}' filters.` + `with '${conflictingOp.toString()}' filters.` ); } } @@ -2040,10 +2071,10 @@ export class Query implements firestore.Query { throw new FirestoreError( Code.INVALID_ARGUMENT, `Invalid query. You have a where filter with an inequality ` + - `(<, <=, >, or >=) on field '${inequality.toString()}' ` + - `and so you must also use '${inequality.toString()}' ` + - `as your first Query.orderBy(), but your first Query.orderBy() ` + - `is on field '${orderBy.toString()}' instead.` + `(<, <=, >, or >=) on field '${inequality.toString()}' ` + + `and so you must also use '${inequality.toString()}' ` + + `as your first Query.orderBy(), but your first Query.orderBy() ` + + `is on field '${orderBy.toString()}' instead.` ); } } @@ -2118,7 +2149,7 @@ export class QuerySnapshot implements firestore.QuerySnapshot { throw new FirestoreError( Code.INVALID_ARGUMENT, 'To include metadata changes with your document changes, you must ' + - 'also pass { includeMetadataChanges:true } to onSnapshot().' + 'also pass { includeMetadataChanges:true } to onSnapshot().' ); } @@ -2171,8 +2202,8 @@ function throwDocChangesMethodError(): never { throw new FirestoreError( Code.INVALID_ARGUMENT, 'QuerySnapshot.docChanges has been changed from a property into a ' + - 'method, so usages like "querySnapshot.docChanges" should become ' + - '"querySnapshot.docChanges()"' + 'method, so usages like "querySnapshot.docChanges" should become ' + + '"querySnapshot.docChanges()"' ); } @@ -2194,7 +2225,7 @@ docChangesPropertiesToOverride.forEach(property => { Object.defineProperty(QuerySnapshot.prototype.docChanges, property, { get: () => throwDocChangesMethodError() }); - } catch (err) {} // Ignore this failure intentionally + } catch (err) { } // Ignore this failure intentionally }); export class CollectionReference extends Query @@ -2205,8 +2236,8 @@ export class CollectionReference extends Query throw new FirestoreError( Code.INVALID_ARGUMENT, 'Invalid collection reference. Collection ' + - 'references must have an odd number of segments, but ' + - `${path.canonicalString()} has ${path.length}` + 'references must have an odd number of segments, but ' + + `${path.canonicalString()} has ${path.length}` ); } } @@ -2287,7 +2318,7 @@ function validateSetOptions( throw new FirestoreError( Code.INVALID_ARGUMENT, `Invalid options passed to function ${methodName}(): You cannot specify both "merge" ` + - `and "mergeFields".` + `and "mergeFields".` ); } diff --git a/packages/firestore/src/core/firestore_client.ts b/packages/firestore/src/core/firestore_client.ts index 89a818535ce..500593a7bbb 100644 --- a/packages/firestore/src/core/firestore_client.ts +++ b/packages/firestore/src/core/firestore_client.ts @@ -74,14 +74,14 @@ export class IndexedDbPersistenceSettings { constructor( readonly cacheSizeBytes: number, readonly synchronizeTabs: boolean - ) {} + ) { } lruParams(): LruParams { return LruParams.withCacheSize(this.cacheSizeBytes); } } -export class MemoryPersistenceSettings {} +export class MemoryPersistenceSettings { } export type InternalPersistenceSettings = | IndexedDbPersistenceSettings @@ -107,7 +107,6 @@ export class FirestoreClient { private lruScheduler?: LruScheduler; private readonly clientId = AutoId.newId(); - private _clientShutdown = false; // PORTING NOTE: SharedClientState is only used for multi-tab web. private sharedClientState: SharedClientState; @@ -125,7 +124,7 @@ export class FirestoreClient { * an async I/O to complete). */ private asyncQueue: AsyncQueue - ) {} + ) { } /** * Starts up the FirestoreClient, returning only whether or not enabling @@ -254,8 +253,8 @@ export class FirestoreClient { } console.warn( 'Error enabling offline persistence. Falling back to' + - ' persistence disabled: ' + - error + ' persistence disabled: ' + + error ); return this.startMemoryPersistence(); }); @@ -310,7 +309,7 @@ export class FirestoreClient { * this class cannot be called after the client is shutdown. */ private verifyNotShutdown(): void { - if (this._clientShutdown) { + if (this.asyncQueue.isShuttingDown) { throw new FirestoreError( Code.FAILED_PRECONDITION, 'The client has already been shutdown.' @@ -507,22 +506,19 @@ export class FirestoreClient { } shutdown(): Promise { - return this.asyncQueue.enqueue(async () => { - if (!this._clientShutdown) { - // PORTING NOTE: LocalStore does not need an explicit shutdown on web. - if (this.lruScheduler) { - this.lruScheduler.stop(); - } - await this.remoteStore.shutdown(); - await this.sharedClientState.shutdown(); - await this.persistence.shutdown(); - - // `removeChangeListener` must be called after shutting down the - // RemoteStore as it will prevent the RemoteStore from retrieving - // auth tokens. - this.credentials.removeChangeListener(); - this._clientShutdown = true; + return this.asyncQueue.enqueueAndInitilizeShutdown(async () => { + // PORTING NOTE: LocalStore does not need an explicit shutdown on web. + if (this.lruScheduler) { + this.lruScheduler.stop(); } + await this.remoteStore.shutdown(); + await this.sharedClientState.shutdown(); + await this.persistence.shutdown(); + + // `removeChangeListener` must be called after shutting down the + // RemoteStore as it will prevent the RemoteStore from retrieving + // auth tokens. + this.credentials.removeChangeListener(); }); } @@ -561,9 +557,9 @@ export class FirestoreClient { throw new FirestoreError( Code.UNAVAILABLE, 'Failed to get document from cache. (However, this document may ' + - "exist on the server. Run again without setting 'source' in " + - 'the GetOptions to attempt to retrieve the document from the ' + - 'server.)' + "exist on the server. Run again without setting 'source' in " + + 'the GetOptions to attempt to retrieve the document from the ' + + 'server.)' ); } }); @@ -603,7 +599,10 @@ export class FirestoreClient { } get clientShutdown(): boolean { - return this._clientShutdown; + // Technically, the asyncQueue is still running, but only accepting operations + // related to shutdown or supposed to be run after shutdown. It is effectively + // shut down to the eyes of users. + return this.asyncQueue.isShuttingDown; } transaction( @@ -612,7 +611,7 @@ export class FirestoreClient { this.verifyNotShutdown(); // We have to wait for the async queue to be sure syncEngine is initialized. return this.asyncQueue - .enqueue(async () => {}) + .enqueue(async () => { }) .then(() => this.syncEngine.runTransaction(updateFunction)); } } diff --git a/packages/firestore/src/util/async_queue.ts b/packages/firestore/src/util/async_queue.ts index 5483100b893..6c89e2b0920 100644 --- a/packages/firestore/src/util/async_queue.ts +++ b/packages/firestore/src/util/async_queue.ts @@ -86,7 +86,7 @@ class DelayedOperation implements CancelablePromise { // It's normal for the deferred promise to be canceled (due to cancellation) // and so we attach a dummy catch callback to avoid // 'UnhandledPromiseRejectionWarning' log spam. - this.deferred.promise.catch(err => {}); + this.deferred.promise.catch(err => { }); } /** @@ -188,6 +188,10 @@ export class AsyncQueue { // The last promise in the queue. private tail: Promise = Promise.resolve(); + // Is this AsyncQueue being shutting down? Once it is set to true, it will not + // be changed again. + private _isShuttingDown: boolean = false; + // Operations scheduled to be queued in the future. Operations are // automatically removed after they are run or canceled. private delayedOperations: Array> = []; @@ -199,6 +203,10 @@ export class AsyncQueue { // assertion sanity-checks. private operationInProgress = false; + // Is this AsyncQueue being shutting down? If true, this instance will not enqueue + // any new operations, Promises from enqueue requests will not resolve. + get isShuttingDown(): boolean { return this._isShuttingDown; } + /** * Adds a new operation to the queue without waiting for it to complete (i.e. * we ignore the Promise result). @@ -208,12 +216,59 @@ export class AsyncQueue { this.enqueue(op); } + /** + * Regardless if the queue has initialized shutdown, adds a new operation to the + * queue without waiting for it to complete (i.e. we ignore the Promise result). + */ + enqueueAndForgetEvenAfterShutdown(op: () => Promise): void { + this.verifyNotFailed(); + // tslint:disable-next-line:no-floating-promises + this.enqueueInternal(op); + } + + /** + * Regardless if the queue has initialized shutdown, adds a new operation to the + * queue. + */ + private enqueueEvenAfterShutdown(op: () => Promise): Promise { + this.verifyNotFailed(); + // tslint:disable-next-line:no-floating-promises + return this.enqueueInternal(op); + } + + /** + * Adds a new operation to the queue and initialize the shut down of this queue. + * Returns a promise that will be resolved when the promise returned by the new + * operation is (with its value). + * Once this method is called, the only possible way to request running an operation + * is through `enqueueAndForgetEvenAfterShutdown`. + */ + enqueueAndInitilizeShutdown(op: () => Promise): Promise { + this.verifyNotFailed(); + if (this._isShuttingDown) { + // Return a Promise resolves right away if it is already shutdown. + return new Promise((resolve) => resolve(undefined)); + } + + const promise = this.enqueueInternal(op); + this._isShuttingDown = true; + return promise; + } + /** * Adds a new operation to the queue. Returns a promise that will be resolved * when the promise returned by the new operation is (with its value). */ enqueue(op: () => Promise): Promise { this.verifyNotFailed(); + if (this._isShuttingDown) { + // Return a Promise resolves to undefined right away + return new Promise((resolve) => resolve(undefined)); + } + return this.enqueueInternal(op); + } + + private enqueueInternal(op: () => Promise): Promise { const newTail = this.tail.then(() => { this.operationInProgress = true; return op() @@ -286,7 +341,7 @@ export class AsyncQueue { if (this.failure) { fail( 'AsyncQueue is already failed: ' + - (this.failure.stack || this.failure.message) + (this.failure.stack || this.failure.message) ); } } @@ -309,7 +364,8 @@ export class AsyncQueue { * operations are not run. */ drain(): Promise { - return this.enqueue(() => Promise.resolve()); + // It should still be possible to drain the queue after it is shutting down. + return this.enqueueEvenAfterShutdown(() => Promise.resolve()); } /** @@ -338,7 +394,7 @@ export class AsyncQueue { return this.drain().then(() => { assert( lastTimerId === TimerId.All || - this.containsDelayedOperation(lastTimerId), + this.containsDelayedOperation(lastTimerId), `Attempted to drain to missing operation ${lastTimerId}` ); diff --git a/packages/firestore/test/integration/api/database.test.ts b/packages/firestore/test/integration/api/database.test.ts index dd1b8d276a9..fc75e2170bc 100644 --- a/packages/firestore/test/integration/api/database.test.ts +++ b/packages/firestore/test/integration/api/database.test.ts @@ -962,7 +962,7 @@ apiDescribe('Database', (persistence: boolean) => { }); }); - (persistence ? it : it.skip)( + (persistence ? it: it.skip)( 'maintains persistence after restarting app', async () => { await withTestDoc(persistence, async docRef => { @@ -1068,4 +1068,29 @@ apiDescribe('Database', (persistence: boolean) => { await db.enableNetwork(); }); }); + + it('can start a new instance after shut down', async () => { + return withTestDoc(persistence, async docRef => { + const firestore = docRef.firestore; + // TODO(b/135755126): use public `shutdown` once it is availabe. + await firestore.INTERNAL.delete(); + + const newFirestore = firebase.firestore!(firestore.app); + await newFirestore.doc(docRef.path).set({foo: 'bar'}); + const doc = await newFirestore.doc(docRef.path).get(); + expect(doc.data()).to.deep.equal({ foo: 'bar' }); + }); + }); + + it('app delete leads to instance shutdown', async () => { + await withTestDoc(persistence, async docRef => { + await docRef.set({ foo: 'bar' }); + const app = docRef.firestore.app; + await app.delete(); + + expect(docRef.firestore.INTERNAL.isShutdown()).to.be.true; + }); + }); + it('calling shutdown mutiple times should proceed', async () => { + }); }); diff --git a/packages/firestore/test/unit/util/async_queue.test.ts b/packages/firestore/test/unit/util/async_queue.test.ts index 281ff463447..bbf18694eea 100644 --- a/packages/firestore/test/unit/util/async_queue.test.ts +++ b/packages/firestore/test/unit/util/async_queue.test.ts @@ -209,6 +209,28 @@ describe('AsyncQueue', () => { await queue.drain(); expect(completedSteps).to.deep.equal([1, 2]); }); + + it('Schedules operaions with respect to shut down', async () => { + const queue = new AsyncQueue(); + const completedSteps: number[] = []; + const doStep = (n: number): Promise => + defer(() => { + const result = completedSteps.push(n); + return result; + }); + + queue.enqueueAndForget(() => doStep(1)); + + // After this call, only operations requested via + // `enqueueAndForgetEvenAfterShutdown` gets executed. + // tslint:disable-next-line:no-floating-promises + queue.enqueueAndInitilizeShutdown(() => doStep(2)); + queue.enqueueAndForget(() => doStep(3)); + queue.enqueueAndForgetEvenAfterShutdown(() => doStep(4)); + + await queue.drain(); + expect(completedSteps).to.deep.equal([1, 2, 4]); + }); }); function defer(op: () => T): Promise { From 2ab4b34cf27f4fdca4d9282789f0e32501d4aa4a Mon Sep 17 00:00:00 2001 From: Hui Wu Date: Wed, 31 Jul 2019 14:38:26 -0400 Subject: [PATCH 02/10] [AUTOMATED]: Prettier Code Styling --- packages/firebase/index.d.ts | 6 +- packages/firestore-types/index.d.ts | 6 +- packages/firestore/src/api/database.ts | 202 +++++++++--------- .../firestore/src/core/firestore_client.ts | 18 +- packages/firestore/src/util/async_queue.ts | 26 ++- .../test/integration/api/database.test.ts | 9 +- 6 files changed, 137 insertions(+), 130 deletions(-) diff --git a/packages/firebase/index.d.ts b/packages/firebase/index.d.ts index 0524ac36de8..6b96c36a87f 100644 --- a/packages/firebase/index.d.ts +++ b/packages/firebase/index.d.ts @@ -6225,9 +6225,9 @@ declare namespace firebase.firestore { /** * @hidden */ - INTERNAL: { - delete: () => Promise - isShutdown: () => boolean + INTERNAL: { + delete: () => Promise; + isShutdown: () => boolean; }; } diff --git a/packages/firestore-types/index.d.ts b/packages/firestore-types/index.d.ts index c783db59454..efb8d4ee542 100644 --- a/packages/firestore-types/index.d.ts +++ b/packages/firestore-types/index.d.ts @@ -276,9 +276,9 @@ export class FirebaseFirestore { */ disableNetwork(): Promise; - INTERNAL: { - delete: () => Promise - isShutdown: () => boolean + INTERNAL: { + delete: () => Promise; + isShutdown: () => boolean; }; } diff --git a/packages/firestore/src/api/database.ts b/packages/firestore/src/api/database.ts index 4c60c0a9c11..5a56b8a8646 100644 --- a/packages/firestore/src/api/database.ts +++ b/packages/firestore/src/api/database.ts @@ -353,7 +353,7 @@ export class Firestore implements firestore.FirebaseFirestore, FirebaseService { throw new FirestoreError( Code.INVALID_ARGUMENT, '"persistence" is now specified with a separate call to ' + - 'firestore.enablePersistence().' + 'firestore.enablePersistence().' ); } @@ -362,8 +362,8 @@ export class Firestore implements firestore.FirebaseFirestore, FirebaseService { throw new FirestoreError( Code.FAILED_PRECONDITION, '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.' + 'be changed. You can only call settings() before calling any other ' + + 'methods on a Firestore object.' ); } @@ -390,8 +390,8 @@ export class Firestore implements firestore.FirebaseFirestore, FirebaseService { throw new FirestoreError( Code.FAILED_PRECONDITION, 'Firestore has already been started and persistence can no longer ' + - 'be enabled. You can only call enablePersistence() before calling ' + - 'any other methods on a Firestore object.' + 'be enabled. You can only call enablePersistence() before calling ' + + 'any other methods on a Firestore object.' ); } @@ -401,9 +401,9 @@ export class Firestore implements firestore.FirebaseFirestore, FirebaseService { if (settings.experimentalTabSynchronization !== undefined) { log.error( "The 'experimentalTabSynchronization' setting has been renamed to " + - "'synchronizeTabs'. In a future release, the setting will be removed " + - 'and it is recommended that you update your ' + - "firestore.enablePersistence() call to use 'synchronizeTabs'." + "'synchronizeTabs'. In a future release, the setting will be removed " + + 'and it is recommended that you update your ' + + "firestore.enablePersistence() call to use 'synchronizeTabs'." ); } synchronizeTabs = objUtils.defaulted( @@ -486,8 +486,8 @@ export class Firestore implements firestore.FirebaseFirestore, FirebaseService { throw new FirestoreError( Code.INVALID_ARGUMENT, 'Document reference is for database ' + - `${otherDb.projectId}/${otherDb.database} but should be ` + - `for database ${thisDb.projectId}/${thisDb.database}` + `${otherDb.projectId}/${otherDb.database} but should be ` + + `for database ${thisDb.projectId}/${thisDb.database}` ); } return new DocumentKeyReference(this._config.databaseId, value._key); @@ -531,7 +531,7 @@ export class Firestore implements firestore.FirebaseFirestore, FirebaseService { throw new FirestoreError( Code.FAILED_PRECONDITION, "Firestore was not initialized using the Firebase SDK. 'app' is " + - 'not available' + 'not available' ); } return this._config.firebaseApp; @@ -543,7 +543,7 @@ export class Firestore implements firestore.FirebaseFirestore, FirebaseService { * After shutdown only the `clearPersistence()` method may be used. Any other method * will throw an `FirestoreError`. * - * To restart after shutdown, simply create a new instance of FirebaseFirestore with + * To restart after shutdown, simply create a new instance of FirebaseFirestore with * `Firebase.firestore()`. * * Shutdown does not cancel any pending writes and any promises that are awaiting a response from @@ -603,7 +603,7 @@ export class Firestore implements firestore.FirebaseFirestore, FirebaseService { throw new FirestoreError( Code.INVALID_ARGUMENT, `Invalid collection ID '${collectionId}' passed to function ` + - `Firestore.collectionGroup(). Collection IDs must not contain '/'.` + `Firestore.collectionGroup(). Collection IDs must not contain '/'.` ); } this.ensureClientConfigured(); @@ -679,7 +679,7 @@ export class Transaction implements firestore.Transaction { constructor( private _firestore: Firestore, private _transaction: InternalTransaction - ) { } + ) {} get( documentRef: firestore.DocumentReference @@ -736,10 +736,10 @@ export class Transaction implements firestore.Transaction { const parsed = options.merge || options.mergeFields ? this._firestore._dataConverter.parseMergeData( - 'Transaction.set', - value, - options.mergeFields - ) + 'Transaction.set', + value, + options.mergeFields + ) : this._firestore._dataConverter.parseSetData('Transaction.set', value); this._transaction.set(ref._key, parsed); return this; @@ -813,7 +813,7 @@ export class WriteBatch implements firestore.WriteBatch { private _mutations = [] as Mutation[]; private _committed = false; - constructor(private _firestore: Firestore) { } + constructor(private _firestore: Firestore) {} set( documentRef: firestore.DocumentReference, @@ -831,10 +831,10 @@ export class WriteBatch implements firestore.WriteBatch { const parsed = options.merge || options.mergeFields ? this._firestore._dataConverter.parseMergeData( - 'WriteBatch.set', - value, - options.mergeFields - ) + 'WriteBatch.set', + value, + options.mergeFields + ) : this._firestore._dataConverter.parseSetData('WriteBatch.set', value); this._mutations = this._mutations.concat( parsed.toMutations(ref._key, Precondition.NONE) @@ -925,7 +925,7 @@ export class WriteBatch implements firestore.WriteBatch { throw new FirestoreError( Code.FAILED_PRECONDITION, 'A write batch can no longer be used after commit() ' + - 'has been called.' + 'has been called.' ); } } @@ -946,8 +946,8 @@ export class DocumentReference implements firestore.DocumentReference { throw new FirestoreError( Code.INVALID_ARGUMENT, 'Invalid document reference. Document ' + - 'references must have an even number of segments, but ' + - `${path.canonicalString()} has ${path.length}` + 'references must have an even number of segments, but ' + + `${path.canonicalString()} has ${path.length}` ); } return new DocumentReference(new DocumentKey(path), firestore); @@ -1000,14 +1000,14 @@ export class DocumentReference implements firestore.DocumentReference { const parsed = options.merge || options.mergeFields ? this.firestore._dataConverter.parseMergeData( - 'DocumentReference.set', - value, - options.mergeFields - ) + 'DocumentReference.set', + value, + options.mergeFields + ) : this.firestore._dataConverter.parseSetData( - 'DocumentReference.set', - value - ); + 'DocumentReference.set', + value + ); return this._firestoreClient.write( parsed.toMutations(this._key, Precondition.NONE) ); @@ -1251,9 +1251,9 @@ export class DocumentReference implements firestore.DocumentReference { new FirestoreError( Code.UNAVAILABLE, 'Failed to get document from server. (However, this ' + - 'document does exist in the local cache. Run again ' + - 'without setting source to "server" to ' + - 'retrieve the cached document.)' + 'document does exist in the local cache. Run again ' + + 'without setting source to "server" to ' + + 'retrieve the cached document.)' ) ); } else { @@ -1270,7 +1270,7 @@ class SnapshotMetadata implements firestore.SnapshotMetadata { constructor( readonly hasPendingWrites: boolean, readonly fromCache: boolean - ) { } + ) {} isEqual(other: firestore.SnapshotMetadata): boolean { return ( @@ -1284,7 +1284,7 @@ class SnapshotMetadata implements firestore.SnapshotMetadata { * Options interface that can be provided to configure the deserialization of * DocumentSnapshots. */ -export interface SnapshotOptions extends firestore.SnapshotOptions { } +export interface SnapshotOptions extends firestore.SnapshotOptions {} export class DocumentSnapshot implements firestore.DocumentSnapshot { constructor( @@ -1293,7 +1293,7 @@ export class DocumentSnapshot implements firestore.DocumentSnapshot { public _document: Document | null, private _fromCache: boolean, private _hasPendingWrites: boolean - ) { } + ) {} data( options?: firestore.SnapshotOptions @@ -1303,12 +1303,12 @@ export class DocumentSnapshot implements firestore.DocumentSnapshot { return !this._document ? undefined : this.convertObject( - this._document.data, - FieldValueOptions.fromSnapshotOptions( - options, - this._firestore._areTimestampsInSnapshotsEnabled() - ) - ); + this._document.data, + FieldValueOptions.fromSnapshotOptions( + options, + this._firestore._areTimestampsInSnapshotsEnabled() + ) + ); } get( @@ -1387,11 +1387,11 @@ export class DocumentSnapshot implements firestore.DocumentSnapshot { // TODO(b/64130202): Somehow support foreign references. log.error( `Document ${this._key.path} contains a document ` + - `reference within a different database (` + - `${value.databaseId.projectId}/${value.databaseId.database}) which is not ` + - `supported. It will be treated as a reference in the current ` + - `database (${database.projectId}/${database.database}) ` + - `instead.` + `reference within a different database (` + + `${value.databaseId.projectId}/${value.databaseId.database}) which is not ` + + `supported. It will be treated as a reference in the current ` + + `database (${database.projectId}/${database.database}) ` + + `instead.` ); } return new DocumentReference(key, this._firestore); @@ -1423,7 +1423,7 @@ export class QueryDocumentSnapshot extends DocumentSnapshot } export class Query implements firestore.Query { - constructor(public _query: InternalQuery, readonly firestore: Firestore) { } + constructor(public _query: InternalQuery, readonly firestore: Firestore) {} where( field: string | ExternalFieldPath, @@ -1454,7 +1454,7 @@ export class Query implements firestore.Query { throw new FirestoreError( Code.INVALID_ARGUMENT, `Invalid Query. You can't perform '${operator.toString()}' ` + - 'queries on FieldPath.documentId().' + 'queries on FieldPath.documentId().' ); } else if (operator === Operator.IN) { this.validateDisjunctiveFilterElements(value, operator); @@ -1503,21 +1503,21 @@ export class Query implements firestore.Query { throw new FirestoreError( Code.INVALID_ARGUMENT, `Function Query.orderBy() has unknown direction '${directionStr}', ` + - `expected 'asc' or 'desc'.` + `expected 'asc' or 'desc'.` ); } if (this._query.startAt !== null) { throw new FirestoreError( Code.INVALID_ARGUMENT, 'Invalid query. You must not call Query.startAt() or ' + - 'Query.startAfter() before calling Query.orderBy().' + 'Query.startAfter() before calling Query.orderBy().' ); } if (this._query.endAt !== null) { throw new FirestoreError( Code.INVALID_ARGUMENT, 'Invalid query. You must not call Query.endAt() or ' + - 'Query.endBefore() before calling Query.orderBy().' + 'Query.endBefore() before calling Query.orderBy().' ); } const fieldPath = fieldPathFromArgument('Query.orderBy', field); @@ -1533,7 +1533,7 @@ export class Query implements firestore.Query { throw new FirestoreError( Code.INVALID_ARGUMENT, `Invalid Query. Query limit (${n}) is invalid. Limit must be ` + - 'positive.' + 'positive.' ); } return new Query(this._query.withLimit(n), this.firestore); @@ -1624,7 +1624,7 @@ export class Query implements firestore.Query { throw new FirestoreError( Code.NOT_FOUND, `Can't use a DocumentSnapshot that doesn't exist for ` + - `${methodName}().` + `${methodName}().` ); } return this.boundFromDocument(methodName, snap._document!, before); @@ -1668,10 +1668,10 @@ export class Query implements firestore.Query { throw new FirestoreError( Code.INVALID_ARGUMENT, 'Invalid query. You are trying to start or end a query using a ' + - 'document for which the field "' + - orderBy.field + - '" is an uncommitted server timestamp. (Since the value of ' + - 'this field is unknown, you cannot start/end a query with it.)' + 'document for which the field "' + + orderBy.field + + '" is an uncommitted server timestamp. (Since the value of ' + + 'this field is unknown, you cannot start/end a query with it.)' ); } else if (value !== null) { components.push(value); @@ -1680,8 +1680,8 @@ export class Query implements firestore.Query { throw new FirestoreError( Code.INVALID_ARGUMENT, `Invalid query. You are trying to start or end a query using a ` + - `document for which the field '${field}' (used as the ` + - `orderBy) does not exist.` + `document for which the field '${field}' (used as the ` + + `orderBy) does not exist.` ); } } @@ -1703,8 +1703,8 @@ export class Query implements firestore.Query { throw new FirestoreError( Code.INVALID_ARGUMENT, `Too many arguments provided to ${methodName}(). ` + - `The number of arguments must be less than or equal to the ` + - `number of Query.orderBy() clauses` + `The number of arguments must be less than or equal to the ` + + `number of Query.orderBy() clauses` ); } @@ -1717,7 +1717,7 @@ export class Query implements firestore.Query { throw new FirestoreError( Code.INVALID_ARGUMENT, `Invalid query. Expected a string for document ID in ` + - `${methodName}(), but got a ${typeof rawValue}` + `${methodName}(), but got a ${typeof rawValue}` ); } if ( @@ -1727,8 +1727,8 @@ export class Query implements firestore.Query { throw new FirestoreError( Code.INVALID_ARGUMENT, `Invalid query. When querying a collection and ordering by FieldPath.documentId(), ` + - `the value passed to ${methodName}() must be a plain document ID, but ` + - `'${rawValue}' contains a slash.` + `the value passed to ${methodName}() must be a plain document ID, but ` + + `'${rawValue}' contains a slash.` ); } const path = this._query.path.child(ResourcePath.fromString(rawValue)); @@ -1736,9 +1736,9 @@ export class Query implements firestore.Query { throw new FirestoreError( Code.INVALID_ARGUMENT, `Invalid query. When querying a collection group and ordering by ` + - `FieldPath.documentId(), the value passed to ${methodName}() must result in a ` + - `valid document path, but '${path}' is not because it contains an odd number ` + - `of segments.` + `FieldPath.documentId(), the value passed to ${methodName}() must result in a ` + + `valid document path, but '${path}' is not because it contains an odd number ` + + `of segments.` ); } const key = new DocumentKey(path); @@ -1895,9 +1895,9 @@ export class Query implements firestore.Query { new FirestoreError( Code.UNAVAILABLE, 'Failed to get documents from server. (However, these ' + - 'documents may exist in the local cache. Run again ' + - 'without setting source to "server" to ' + - 'retrieve the cached documents.)' + 'documents may exist in the local cache. Run again ' + + 'without setting source to "server" to ' + + 'retrieve the cached documents.)' ) ); } else { @@ -1920,7 +1920,7 @@ export class Query implements firestore.Query { throw new FirestoreError( Code.INVALID_ARGUMENT, 'Invalid query. When querying with FieldPath.documentId(), you ' + - 'must provide a valid document ID, but it was an empty string.' + 'must provide a valid document ID, but it was an empty string.' ); } if ( @@ -1930,8 +1930,8 @@ export class Query implements firestore.Query { throw new FirestoreError( Code.INVALID_ARGUMENT, `Invalid query. When querying a collection by ` + - `FieldPath.documentId(), you must provide a plain document ID, but ` + - `'${documentIdValue}' contains a '/' character.` + `FieldPath.documentId(), you must provide a plain document ID, but ` + + `'${documentIdValue}' contains a '/' character.` ); } const path = this._query.path.child( @@ -1941,8 +1941,8 @@ export class Query implements firestore.Query { throw new FirestoreError( Code.INVALID_ARGUMENT, `Invalid query. When querying a collection group by ` + - `FieldPath.documentId(), the value provided must result in a valid document path, ` + - `but '${path}' is not because it has an odd number of segments (${path.length}).` + `FieldPath.documentId(), the value provided must result in a valid document path, ` + + `but '${path}' is not because it has an odd number of segments (${path.length}).` ); } return new RefValue(this.firestore._databaseId, new DocumentKey(path)); @@ -1953,8 +1953,8 @@ export class Query implements firestore.Query { throw new FirestoreError( Code.INVALID_ARGUMENT, `Invalid query. When querying with FieldPath.documentId(), you must provide a valid ` + - `string or a DocumentReference, but it was: ` + - `${valueDescription(documentIdValue)}.` + `string or a DocumentReference, but it was: ` + + `${valueDescription(documentIdValue)}.` ); } } @@ -1971,28 +1971,28 @@ export class Query implements firestore.Query { throw new FirestoreError( Code.INVALID_ARGUMENT, 'Invalid Query. A non-empty array is required for ' + - `'${operator.toString()}' filters.` + `'${operator.toString()}' filters.` ); } if (value.length > 10) { throw new FirestoreError( Code.INVALID_ARGUMENT, `Invalid Query. '${operator.toString()}' filters support a ` + - 'maximum of 10 elements in the value array.' + 'maximum of 10 elements in the value array.' ); } if (value.indexOf(null) >= 0) { throw new FirestoreError( Code.INVALID_ARGUMENT, `Invalid Query. '${operator.toString()}' filters cannot contain 'null' ` + - 'in the value array.' + 'in the value array.' ); } if (value.filter(element => Number.isNaN(element)).length > 0) { throw new FirestoreError( Code.INVALID_ARGUMENT, `Invalid Query. '${operator.toString()}' filters cannot contain 'NaN' ` + - 'in the value array.' + 'in the value array.' ); } } @@ -2010,9 +2010,9 @@ export class Query implements firestore.Query { throw new FirestoreError( Code.INVALID_ARGUMENT, 'Invalid query. All where filters with an inequality' + - ' (<, <=, >, or >=) must be on the same field. But you have' + - ` inequality filters on '${existingField.toString()}'` + - ` and '${filter.field.toString()}'` + ' (<, <=, >, or >=) must be on the same field. But you have' + + ` inequality filters on '${existingField.toString()}'` + + ` and '${filter.field.toString()}'` ); } @@ -2039,13 +2039,13 @@ export class Query implements firestore.Query { throw new FirestoreError( Code.INVALID_ARGUMENT, 'Invalid query. You cannot use more than one ' + - `'${filter.op.toString()}' filter.` + `'${filter.op.toString()}' filter.` ); } else { throw new FirestoreError( Code.INVALID_ARGUMENT, `Invalid query. You cannot use '${filter.op.toString()}' filters ` + - `with '${conflictingOp.toString()}' filters.` + `with '${conflictingOp.toString()}' filters.` ); } } @@ -2071,10 +2071,10 @@ export class Query implements firestore.Query { throw new FirestoreError( Code.INVALID_ARGUMENT, `Invalid query. You have a where filter with an inequality ` + - `(<, <=, >, or >=) on field '${inequality.toString()}' ` + - `and so you must also use '${inequality.toString()}' ` + - `as your first Query.orderBy(), but your first Query.orderBy() ` + - `is on field '${orderBy.toString()}' instead.` + `(<, <=, >, or >=) on field '${inequality.toString()}' ` + + `and so you must also use '${inequality.toString()}' ` + + `as your first Query.orderBy(), but your first Query.orderBy() ` + + `is on field '${orderBy.toString()}' instead.` ); } } @@ -2149,7 +2149,7 @@ export class QuerySnapshot implements firestore.QuerySnapshot { throw new FirestoreError( Code.INVALID_ARGUMENT, 'To include metadata changes with your document changes, you must ' + - 'also pass { includeMetadataChanges:true } to onSnapshot().' + 'also pass { includeMetadataChanges:true } to onSnapshot().' ); } @@ -2202,8 +2202,8 @@ function throwDocChangesMethodError(): never { throw new FirestoreError( Code.INVALID_ARGUMENT, 'QuerySnapshot.docChanges has been changed from a property into a ' + - 'method, so usages like "querySnapshot.docChanges" should become ' + - '"querySnapshot.docChanges()"' + 'method, so usages like "querySnapshot.docChanges" should become ' + + '"querySnapshot.docChanges()"' ); } @@ -2225,7 +2225,7 @@ docChangesPropertiesToOverride.forEach(property => { Object.defineProperty(QuerySnapshot.prototype.docChanges, property, { get: () => throwDocChangesMethodError() }); - } catch (err) { } // Ignore this failure intentionally + } catch (err) {} // Ignore this failure intentionally }); export class CollectionReference extends Query @@ -2236,8 +2236,8 @@ export class CollectionReference extends Query throw new FirestoreError( Code.INVALID_ARGUMENT, 'Invalid collection reference. Collection ' + - 'references must have an odd number of segments, but ' + - `${path.canonicalString()} has ${path.length}` + 'references must have an odd number of segments, but ' + + `${path.canonicalString()} has ${path.length}` ); } } @@ -2318,7 +2318,7 @@ function validateSetOptions( throw new FirestoreError( Code.INVALID_ARGUMENT, `Invalid options passed to function ${methodName}(): You cannot specify both "merge" ` + - `and "mergeFields".` + `and "mergeFields".` ); } diff --git a/packages/firestore/src/core/firestore_client.ts b/packages/firestore/src/core/firestore_client.ts index 500593a7bbb..f3dbdcd85b6 100644 --- a/packages/firestore/src/core/firestore_client.ts +++ b/packages/firestore/src/core/firestore_client.ts @@ -74,14 +74,14 @@ export class IndexedDbPersistenceSettings { constructor( readonly cacheSizeBytes: number, readonly synchronizeTabs: boolean - ) { } + ) {} lruParams(): LruParams { return LruParams.withCacheSize(this.cacheSizeBytes); } } -export class MemoryPersistenceSettings { } +export class MemoryPersistenceSettings {} export type InternalPersistenceSettings = | IndexedDbPersistenceSettings @@ -124,7 +124,7 @@ export class FirestoreClient { * an async I/O to complete). */ private asyncQueue: AsyncQueue - ) { } + ) {} /** * Starts up the FirestoreClient, returning only whether or not enabling @@ -253,8 +253,8 @@ export class FirestoreClient { } console.warn( 'Error enabling offline persistence. Falling back to' + - ' persistence disabled: ' + - error + ' persistence disabled: ' + + error ); return this.startMemoryPersistence(); }); @@ -557,9 +557,9 @@ export class FirestoreClient { throw new FirestoreError( Code.UNAVAILABLE, 'Failed to get document from cache. (However, this document may ' + - "exist on the server. Run again without setting 'source' in " + - 'the GetOptions to attempt to retrieve the document from the ' + - 'server.)' + "exist on the server. Run again without setting 'source' in " + + 'the GetOptions to attempt to retrieve the document from the ' + + 'server.)' ); } }); @@ -611,7 +611,7 @@ export class FirestoreClient { this.verifyNotShutdown(); // We have to wait for the async queue to be sure syncEngine is initialized. return this.asyncQueue - .enqueue(async () => { }) + .enqueue(async () => {}) .then(() => this.syncEngine.runTransaction(updateFunction)); } } diff --git a/packages/firestore/src/util/async_queue.ts b/packages/firestore/src/util/async_queue.ts index 6c89e2b0920..f99af671a6e 100644 --- a/packages/firestore/src/util/async_queue.ts +++ b/packages/firestore/src/util/async_queue.ts @@ -86,7 +86,7 @@ class DelayedOperation implements CancelablePromise { // It's normal for the deferred promise to be canceled (due to cancellation) // and so we attach a dummy catch callback to avoid // 'UnhandledPromiseRejectionWarning' log spam. - this.deferred.promise.catch(err => { }); + this.deferred.promise.catch(err => {}); } /** @@ -205,7 +205,9 @@ export class AsyncQueue { // Is this AsyncQueue being shutting down? If true, this instance will not enqueue // any new operations, Promises from enqueue requests will not resolve. - get isShuttingDown(): boolean { return this._isShuttingDown; } + get isShuttingDown(): boolean { + return this._isShuttingDown; + } /** * Adds a new operation to the queue without waiting for it to complete (i.e. @@ -220,7 +222,9 @@ export class AsyncQueue { * Regardless if the queue has initialized shutdown, adds a new operation to the * queue without waiting for it to complete (i.e. we ignore the Promise result). */ - enqueueAndForgetEvenAfterShutdown(op: () => Promise): void { + enqueueAndForgetEvenAfterShutdown( + op: () => Promise + ): void { this.verifyNotFailed(); // tslint:disable-next-line:no-floating-promises this.enqueueInternal(op); @@ -230,7 +234,9 @@ export class AsyncQueue { * Regardless if the queue has initialized shutdown, adds a new operation to the * queue. */ - private enqueueEvenAfterShutdown(op: () => Promise): Promise { + private enqueueEvenAfterShutdown( + op: () => Promise + ): Promise { this.verifyNotFailed(); // tslint:disable-next-line:no-floating-promises return this.enqueueInternal(op); @@ -243,11 +249,13 @@ export class AsyncQueue { * Once this method is called, the only possible way to request running an operation * is through `enqueueAndForgetEvenAfterShutdown`. */ - enqueueAndInitilizeShutdown(op: () => Promise): Promise { + enqueueAndInitilizeShutdown( + op: () => Promise + ): Promise { this.verifyNotFailed(); if (this._isShuttingDown) { // Return a Promise resolves right away if it is already shutdown. - return new Promise((resolve) => resolve(undefined)); + return new Promise(resolve => resolve(undefined)); } const promise = this.enqueueInternal(op); @@ -263,7 +271,7 @@ export class AsyncQueue { this.verifyNotFailed(); if (this._isShuttingDown) { // Return a Promise resolves to undefined right away - return new Promise((resolve) => resolve(undefined)); + return new Promise(resolve => resolve(undefined)); } return this.enqueueInternal(op); } @@ -341,7 +349,7 @@ export class AsyncQueue { if (this.failure) { fail( 'AsyncQueue is already failed: ' + - (this.failure.stack || this.failure.message) + (this.failure.stack || this.failure.message) ); } } @@ -394,7 +402,7 @@ export class AsyncQueue { return this.drain().then(() => { assert( lastTimerId === TimerId.All || - this.containsDelayedOperation(lastTimerId), + this.containsDelayedOperation(lastTimerId), `Attempted to drain to missing operation ${lastTimerId}` ); diff --git a/packages/firestore/test/integration/api/database.test.ts b/packages/firestore/test/integration/api/database.test.ts index fc75e2170bc..cc14c5e305a 100644 --- a/packages/firestore/test/integration/api/database.test.ts +++ b/packages/firestore/test/integration/api/database.test.ts @@ -962,7 +962,7 @@ apiDescribe('Database', (persistence: boolean) => { }); }); - (persistence ? it: it.skip)( + (persistence ? it : it.skip)( 'maintains persistence after restarting app', async () => { await withTestDoc(persistence, async docRef => { @@ -1076,7 +1076,7 @@ apiDescribe('Database', (persistence: boolean) => { await firestore.INTERNAL.delete(); const newFirestore = firebase.firestore!(firestore.app); - await newFirestore.doc(docRef.path).set({foo: 'bar'}); + await newFirestore.doc(docRef.path).set({ foo: 'bar' }); const doc = await newFirestore.doc(docRef.path).get(); expect(doc.data()).to.deep.equal({ foo: 'bar' }); }); @@ -1087,10 +1087,9 @@ apiDescribe('Database', (persistence: boolean) => { await docRef.set({ foo: 'bar' }); const app = docRef.firestore.app; await app.delete(); - + expect(docRef.firestore.INTERNAL.isShutdown()).to.be.true; }); }); - it('calling shutdown mutiple times should proceed', async () => { - }); + it('calling shutdown mutiple times should proceed', async () => {}); }); From c9e14954dbeb82c98a552b19decd02dbcdb4c87e Mon Sep 17 00:00:00 2001 From: Hui Wu Date: Wed, 31 Jul 2019 16:46:26 -0400 Subject: [PATCH 03/10] Looks to be working. --- packages/firebase/index.d.ts | 6 +- packages/firestore-types/index.d.ts | 6 +- packages/firestore/src/api/database.ts | 236 ++++++++++-------- .../firestore/src/core/firestore_client.ts | 53 ++-- packages/firestore/src/util/async_queue.ts | 64 ++++- .../test/integration/api/database.test.ts | 47 +++- .../test/unit/util/async_queue.test.ts | 22 ++ 7 files changed, 297 insertions(+), 137 deletions(-) diff --git a/packages/firebase/index.d.ts b/packages/firebase/index.d.ts index 28b43b2a311..73997980051 100644 --- a/packages/firebase/index.d.ts +++ b/packages/firebase/index.d.ts @@ -6225,7 +6225,11 @@ declare namespace firebase.firestore { /** * @hidden */ - INTERNAL: { delete: () => Promise }; + INTERNAL: { + delete: () => Promise + shutdown: () => Promise + isShutdown: () => boolean + }; } /** diff --git a/packages/firestore-types/index.d.ts b/packages/firestore-types/index.d.ts index c96fa5925dc..9cd263ffc18 100644 --- a/packages/firestore-types/index.d.ts +++ b/packages/firestore-types/index.d.ts @@ -276,7 +276,11 @@ export class FirebaseFirestore { */ disableNetwork(): Promise; - INTERNAL: { delete: () => Promise }; + INTERNAL: { + delete: () => Promise + shutdown: () => Promise + isShutdown: () => boolean + }; } /** diff --git a/packages/firestore/src/api/database.ts b/packages/firestore/src/api/database.ts index 161435d773c..a5eec9caa18 100644 --- a/packages/firestore/src/api/database.ts +++ b/packages/firestore/src/api/database.ts @@ -18,7 +18,7 @@ import * as firestore from '@firebase/firestore-types'; import { FirebaseApp } from '@firebase/app-types'; -import { FirebaseService } from '@firebase/app-types/private'; +import { FirebaseService, _FirebaseApp } from '@firebase/app-types/private'; import { DatabaseId, DatabaseInfo } from '../core/database_info'; import { ListenOptions } from '../core/event_manager'; import { @@ -353,7 +353,7 @@ export class Firestore implements firestore.FirebaseFirestore, FirebaseService { throw new FirestoreError( Code.INVALID_ARGUMENT, '"persistence" is now specified with a separate call to ' + - 'firestore.enablePersistence().' + 'firestore.enablePersistence().' ); } @@ -362,8 +362,8 @@ export class Firestore implements firestore.FirebaseFirestore, FirebaseService { throw new FirestoreError( Code.FAILED_PRECONDITION, '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.' + 'be changed. You can only call settings() before calling any other ' + + 'methods on a Firestore object.' ); } @@ -390,8 +390,8 @@ export class Firestore implements firestore.FirebaseFirestore, FirebaseService { throw new FirestoreError( Code.FAILED_PRECONDITION, 'Firestore has already been started and persistence can no longer ' + - 'be enabled. You can only call enablePersistence() before calling ' + - 'any other methods on a Firestore object.' + 'be enabled. You can only call enablePersistence() before calling ' + + 'any other methods on a Firestore object.' ); } @@ -401,9 +401,9 @@ export class Firestore implements firestore.FirebaseFirestore, FirebaseService { if (settings.experimentalTabSynchronization !== undefined) { log.error( "The 'experimentalTabSynchronization' setting has been renamed to " + - "'synchronizeTabs'. In a future release, the setting will be removed " + - 'and it is recommended that you update your ' + - "firestore.enablePersistence() call to use 'synchronizeTabs'." + "'synchronizeTabs'. In a future release, the setting will be removed " + + 'and it is recommended that you update your ' + + "firestore.enablePersistence() call to use 'synchronizeTabs'." ); } synchronizeTabs = objUtils.defaulted( @@ -427,7 +427,7 @@ export class Firestore implements firestore.FirebaseFirestore, FirebaseService { this.makeDatabaseInfo() ); const deferred = new Deferred(); - this._queue.enqueueAndForget(async () => { + this._queue.enqueueAndForgetEvenAfterShutdown(async () => { try { if ( this._firestoreClient !== undefined && @@ -486,8 +486,8 @@ export class Firestore implements firestore.FirebaseFirestore, FirebaseService { throw new FirestoreError( Code.INVALID_ARGUMENT, 'Document reference is for database ' + - `${otherDb.projectId}/${otherDb.database} but should be ` + - `for database ${thisDb.projectId}/${thisDb.database}` + `${otherDb.projectId}/${otherDb.database} but should be ` + + `for database ${thisDb.projectId}/${thisDb.database}` ); } return new DocumentKeyReference(this._config.databaseId, value._key); @@ -531,7 +531,7 @@ export class Firestore implements firestore.FirebaseFirestore, FirebaseService { throw new FirestoreError( Code.FAILED_PRECONDITION, "Firestore was not initialized using the Firebase SDK. 'app' is " + - 'not available' + 'not available' ); } return this._config.firebaseApp; @@ -543,6 +543,36 @@ export class Firestore implements firestore.FirebaseFirestore, FirebaseService { // throws an exception. this.ensureClientConfigured(); await this._firestoreClient!.shutdown(); + }, + + /** + * Shuts down this FirebaseFirestore instance. + * + * After shutdown only the `clearPersistence()` method may be used. Any other method + * will throw an `FirestoreError`. + * + * To restart after shutdown, simply create a new instance of FirebaseFirestore with + * `Firebase.firestore()`. + * + * Shutdown does not cancel any pending writes and any promises that are awaiting a response + * from the server will not be resolved. The next time you start this instance, + * it will resume attempting to send these writes to the server. + * + * Note: Under normal circumstances, calling `shutdown()` is not required. This + * method is useful only when you want to force this instance to release all of its resources or + * in combination with `clearPersistence()` to ensure that all local state is destroyed + * between test runs. + * + * @return A promise that is resolved when the instance has been successfully shut down. + */ + // TODO(b/135755126): make this public. + shutdown: (): Promise => { + (this._config.firebaseApp as _FirebaseApp)._removeServiceInstance('firestore'); + return this.INTERNAL.delete(); + }, + + isShutdown: (): boolean => { + return this._firestoreClient!.clientShutdown; } }; @@ -572,7 +602,7 @@ export class Firestore implements firestore.FirebaseFirestore, FirebaseService { throw new FirestoreError( Code.INVALID_ARGUMENT, `Invalid collection ID '${collectionId}' passed to function ` + - `Firestore.collectionGroup(). Collection IDs must not contain '/'.` + `Firestore.collectionGroup(). Collection IDs must not contain '/'.` ); } this.ensureClientConfigured(); @@ -648,7 +678,7 @@ export class Transaction implements firestore.Transaction { constructor( private _firestore: Firestore, private _transaction: InternalTransaction - ) {} + ) { } get( documentRef: firestore.DocumentReference @@ -705,10 +735,10 @@ export class Transaction implements firestore.Transaction { const parsed = options.merge || options.mergeFields ? this._firestore._dataConverter.parseMergeData( - 'Transaction.set', - value, - options.mergeFields - ) + 'Transaction.set', + value, + options.mergeFields + ) : this._firestore._dataConverter.parseSetData('Transaction.set', value); this._transaction.set(ref._key, parsed); return this; @@ -782,7 +812,7 @@ export class WriteBatch implements firestore.WriteBatch { private _mutations = [] as Mutation[]; private _committed = false; - constructor(private _firestore: Firestore) {} + constructor(private _firestore: Firestore) { } set( documentRef: firestore.DocumentReference, @@ -800,10 +830,10 @@ export class WriteBatch implements firestore.WriteBatch { const parsed = options.merge || options.mergeFields ? this._firestore._dataConverter.parseMergeData( - 'WriteBatch.set', - value, - options.mergeFields - ) + 'WriteBatch.set', + value, + options.mergeFields + ) : this._firestore._dataConverter.parseSetData('WriteBatch.set', value); this._mutations = this._mutations.concat( parsed.toMutations(ref._key, Precondition.NONE) @@ -894,7 +924,7 @@ export class WriteBatch implements firestore.WriteBatch { throw new FirestoreError( Code.FAILED_PRECONDITION, 'A write batch can no longer be used after commit() ' + - 'has been called.' + 'has been called.' ); } } @@ -915,8 +945,8 @@ export class DocumentReference implements firestore.DocumentReference { throw new FirestoreError( Code.INVALID_ARGUMENT, 'Invalid document reference. Document ' + - 'references must have an even number of segments, but ' + - `${path.canonicalString()} has ${path.length}` + 'references must have an even number of segments, but ' + + `${path.canonicalString()} has ${path.length}` ); } return new DocumentReference(new DocumentKey(path), firestore); @@ -969,14 +999,14 @@ export class DocumentReference implements firestore.DocumentReference { const parsed = options.merge || options.mergeFields ? this.firestore._dataConverter.parseMergeData( - 'DocumentReference.set', - value, - options.mergeFields - ) + 'DocumentReference.set', + value, + options.mergeFields + ) : this.firestore._dataConverter.parseSetData( - 'DocumentReference.set', - value - ); + 'DocumentReference.set', + value + ); return this._firestoreClient.write( parsed.toMutations(this._key, Precondition.NONE) ); @@ -1020,7 +1050,7 @@ export class DocumentReference implements firestore.DocumentReference { } delete(): Promise { - validateExactNumberOfArgs('DocumentReference.delete', arguments, 0); + validateExactNumberOfArgs('FirebaseFirestore.shutdown', arguments, 0); return this._firestoreClient.write([ new DeleteMutation(this._key, Precondition.NONE) ]); @@ -1220,9 +1250,9 @@ export class DocumentReference implements firestore.DocumentReference { new FirestoreError( Code.UNAVAILABLE, 'Failed to get document from server. (However, this ' + - 'document does exist in the local cache. Run again ' + - 'without setting source to "server" to ' + - 'retrieve the cached document.)' + 'document does exist in the local cache. Run again ' + + 'without setting source to "server" to ' + + 'retrieve the cached document.)' ) ); } else { @@ -1239,7 +1269,7 @@ class SnapshotMetadata implements firestore.SnapshotMetadata { constructor( readonly hasPendingWrites: boolean, readonly fromCache: boolean - ) {} + ) { } isEqual(other: firestore.SnapshotMetadata): boolean { return ( @@ -1253,7 +1283,7 @@ class SnapshotMetadata implements firestore.SnapshotMetadata { * Options interface that can be provided to configure the deserialization of * DocumentSnapshots. */ -export interface SnapshotOptions extends firestore.SnapshotOptions {} +export interface SnapshotOptions extends firestore.SnapshotOptions { } export class DocumentSnapshot implements firestore.DocumentSnapshot { constructor( @@ -1262,7 +1292,7 @@ export class DocumentSnapshot implements firestore.DocumentSnapshot { public _document: Document | null, private _fromCache: boolean, private _hasPendingWrites: boolean - ) {} + ) { } data( options?: firestore.SnapshotOptions @@ -1272,12 +1302,12 @@ export class DocumentSnapshot implements firestore.DocumentSnapshot { return !this._document ? undefined : this.convertObject( - this._document.data, - FieldValueOptions.fromSnapshotOptions( - options, - this._firestore._areTimestampsInSnapshotsEnabled() - ) - ); + this._document.data, + FieldValueOptions.fromSnapshotOptions( + options, + this._firestore._areTimestampsInSnapshotsEnabled() + ) + ); } get( @@ -1356,11 +1386,11 @@ export class DocumentSnapshot implements firestore.DocumentSnapshot { // TODO(b/64130202): Somehow support foreign references. log.error( `Document ${this._key.path} contains a document ` + - `reference within a different database (` + - `${value.databaseId.projectId}/${value.databaseId.database}) which is not ` + - `supported. It will be treated as a reference in the current ` + - `database (${database.projectId}/${database.database}) ` + - `instead.` + `reference within a different database (` + + `${value.databaseId.projectId}/${value.databaseId.database}) which is not ` + + `supported. It will be treated as a reference in the current ` + + `database (${database.projectId}/${database.database}) ` + + `instead.` ); } return new DocumentReference(key, this._firestore); @@ -1392,7 +1422,7 @@ export class QueryDocumentSnapshot extends DocumentSnapshot } export class Query implements firestore.Query { - constructor(public _query: InternalQuery, readonly firestore: Firestore) {} + constructor(public _query: InternalQuery, readonly firestore: Firestore) { } where( field: string | ExternalFieldPath, @@ -1423,7 +1453,7 @@ export class Query implements firestore.Query { throw new FirestoreError( Code.INVALID_ARGUMENT, `Invalid Query. You can't perform '${operator.toString()}' ` + - 'queries on FieldPath.documentId().' + 'queries on FieldPath.documentId().' ); } else if (operator === Operator.IN) { this.validateDisjunctiveFilterElements(value, operator); @@ -1472,21 +1502,21 @@ export class Query implements firestore.Query { throw new FirestoreError( Code.INVALID_ARGUMENT, `Function Query.orderBy() has unknown direction '${directionStr}', ` + - `expected 'asc' or 'desc'.` + `expected 'asc' or 'desc'.` ); } if (this._query.startAt !== null) { throw new FirestoreError( Code.INVALID_ARGUMENT, 'Invalid query. You must not call Query.startAt() or ' + - 'Query.startAfter() before calling Query.orderBy().' + 'Query.startAfter() before calling Query.orderBy().' ); } if (this._query.endAt !== null) { throw new FirestoreError( Code.INVALID_ARGUMENT, 'Invalid query. You must not call Query.endAt() or ' + - 'Query.endBefore() before calling Query.orderBy().' + 'Query.endBefore() before calling Query.orderBy().' ); } const fieldPath = fieldPathFromArgument('Query.orderBy', field); @@ -1502,7 +1532,7 @@ export class Query implements firestore.Query { throw new FirestoreError( Code.INVALID_ARGUMENT, `Invalid Query. Query limit (${n}) is invalid. Limit must be ` + - 'positive.' + 'positive.' ); } return new Query(this._query.withLimit(n), this.firestore); @@ -1593,7 +1623,7 @@ export class Query implements firestore.Query { throw new FirestoreError( Code.NOT_FOUND, `Can't use a DocumentSnapshot that doesn't exist for ` + - `${methodName}().` + `${methodName}().` ); } return this.boundFromDocument(methodName, snap._document!, before); @@ -1637,10 +1667,10 @@ export class Query implements firestore.Query { throw new FirestoreError( Code.INVALID_ARGUMENT, 'Invalid query. You are trying to start or end a query using a ' + - 'document for which the field "' + - orderBy.field + - '" is an uncommitted server timestamp. (Since the value of ' + - 'this field is unknown, you cannot start/end a query with it.)' + 'document for which the field "' + + orderBy.field + + '" is an uncommitted server timestamp. (Since the value of ' + + 'this field is unknown, you cannot start/end a query with it.)' ); } else if (value !== null) { components.push(value); @@ -1649,8 +1679,8 @@ export class Query implements firestore.Query { throw new FirestoreError( Code.INVALID_ARGUMENT, `Invalid query. You are trying to start or end a query using a ` + - `document for which the field '${field}' (used as the ` + - `orderBy) does not exist.` + `document for which the field '${field}' (used as the ` + + `orderBy) does not exist.` ); } } @@ -1672,8 +1702,8 @@ export class Query implements firestore.Query { throw new FirestoreError( Code.INVALID_ARGUMENT, `Too many arguments provided to ${methodName}(). ` + - `The number of arguments must be less than or equal to the ` + - `number of Query.orderBy() clauses` + `The number of arguments must be less than or equal to the ` + + `number of Query.orderBy() clauses` ); } @@ -1686,7 +1716,7 @@ export class Query implements firestore.Query { throw new FirestoreError( Code.INVALID_ARGUMENT, `Invalid query. Expected a string for document ID in ` + - `${methodName}(), but got a ${typeof rawValue}` + `${methodName}(), but got a ${typeof rawValue}` ); } if ( @@ -1696,8 +1726,8 @@ export class Query implements firestore.Query { throw new FirestoreError( Code.INVALID_ARGUMENT, `Invalid query. When querying a collection and ordering by FieldPath.documentId(), ` + - `the value passed to ${methodName}() must be a plain document ID, but ` + - `'${rawValue}' contains a slash.` + `the value passed to ${methodName}() must be a plain document ID, but ` + + `'${rawValue}' contains a slash.` ); } const path = this._query.path.child(ResourcePath.fromString(rawValue)); @@ -1705,9 +1735,9 @@ export class Query implements firestore.Query { throw new FirestoreError( Code.INVALID_ARGUMENT, `Invalid query. When querying a collection group and ordering by ` + - `FieldPath.documentId(), the value passed to ${methodName}() must result in a ` + - `valid document path, but '${path}' is not because it contains an odd number ` + - `of segments.` + `FieldPath.documentId(), the value passed to ${methodName}() must result in a ` + + `valid document path, but '${path}' is not because it contains an odd number ` + + `of segments.` ); } const key = new DocumentKey(path); @@ -1864,9 +1894,9 @@ export class Query implements firestore.Query { new FirestoreError( Code.UNAVAILABLE, 'Failed to get documents from server. (However, these ' + - 'documents may exist in the local cache. Run again ' + - 'without setting source to "server" to ' + - 'retrieve the cached documents.)' + 'documents may exist in the local cache. Run again ' + + 'without setting source to "server" to ' + + 'retrieve the cached documents.)' ) ); } else { @@ -1889,7 +1919,7 @@ export class Query implements firestore.Query { throw new FirestoreError( Code.INVALID_ARGUMENT, 'Invalid query. When querying with FieldPath.documentId(), you ' + - 'must provide a valid document ID, but it was an empty string.' + 'must provide a valid document ID, but it was an empty string.' ); } if ( @@ -1899,8 +1929,8 @@ export class Query implements firestore.Query { throw new FirestoreError( Code.INVALID_ARGUMENT, `Invalid query. When querying a collection by ` + - `FieldPath.documentId(), you must provide a plain document ID, but ` + - `'${documentIdValue}' contains a '/' character.` + `FieldPath.documentId(), you must provide a plain document ID, but ` + + `'${documentIdValue}' contains a '/' character.` ); } const path = this._query.path.child( @@ -1910,8 +1940,8 @@ export class Query implements firestore.Query { throw new FirestoreError( Code.INVALID_ARGUMENT, `Invalid query. When querying a collection group by ` + - `FieldPath.documentId(), the value provided must result in a valid document path, ` + - `but '${path}' is not because it has an odd number of segments (${path.length}).` + `FieldPath.documentId(), the value provided must result in a valid document path, ` + + `but '${path}' is not because it has an odd number of segments (${path.length}).` ); } return new RefValue(this.firestore._databaseId, new DocumentKey(path)); @@ -1922,8 +1952,8 @@ export class Query implements firestore.Query { throw new FirestoreError( Code.INVALID_ARGUMENT, `Invalid query. When querying with FieldPath.documentId(), you must provide a valid ` + - `string or a DocumentReference, but it was: ` + - `${valueDescription(documentIdValue)}.` + `string or a DocumentReference, but it was: ` + + `${valueDescription(documentIdValue)}.` ); } } @@ -1940,28 +1970,28 @@ export class Query implements firestore.Query { throw new FirestoreError( Code.INVALID_ARGUMENT, 'Invalid Query. A non-empty array is required for ' + - `'${operator.toString()}' filters.` + `'${operator.toString()}' filters.` ); } if (value.length > 10) { throw new FirestoreError( Code.INVALID_ARGUMENT, `Invalid Query. '${operator.toString()}' filters support a ` + - 'maximum of 10 elements in the value array.' + 'maximum of 10 elements in the value array.' ); } if (value.indexOf(null) >= 0) { throw new FirestoreError( Code.INVALID_ARGUMENT, `Invalid Query. '${operator.toString()}' filters cannot contain 'null' ` + - 'in the value array.' + 'in the value array.' ); } if (value.filter(element => Number.isNaN(element)).length > 0) { throw new FirestoreError( Code.INVALID_ARGUMENT, `Invalid Query. '${operator.toString()}' filters cannot contain 'NaN' ` + - 'in the value array.' + 'in the value array.' ); } } @@ -1979,9 +2009,9 @@ export class Query implements firestore.Query { throw new FirestoreError( Code.INVALID_ARGUMENT, 'Invalid query. All where filters with an inequality' + - ' (<, <=, >, or >=) must be on the same field. But you have' + - ` inequality filters on '${existingField.toString()}'` + - ` and '${filter.field.toString()}'` + ' (<, <=, >, or >=) must be on the same field. But you have' + + ` inequality filters on '${existingField.toString()}'` + + ` and '${filter.field.toString()}'` ); } @@ -2008,13 +2038,13 @@ export class Query implements firestore.Query { throw new FirestoreError( Code.INVALID_ARGUMENT, 'Invalid query. You cannot use more than one ' + - `'${filter.op.toString()}' filter.` + `'${filter.op.toString()}' filter.` ); } else { throw new FirestoreError( Code.INVALID_ARGUMENT, `Invalid query. You cannot use '${filter.op.toString()}' filters ` + - `with '${conflictingOp.toString()}' filters.` + `with '${conflictingOp.toString()}' filters.` ); } } @@ -2040,10 +2070,10 @@ export class Query implements firestore.Query { throw new FirestoreError( Code.INVALID_ARGUMENT, `Invalid query. You have a where filter with an inequality ` + - `(<, <=, >, or >=) on field '${inequality.toString()}' ` + - `and so you must also use '${inequality.toString()}' ` + - `as your first Query.orderBy(), but your first Query.orderBy() ` + - `is on field '${orderBy.toString()}' instead.` + `(<, <=, >, or >=) on field '${inequality.toString()}' ` + + `and so you must also use '${inequality.toString()}' ` + + `as your first Query.orderBy(), but your first Query.orderBy() ` + + `is on field '${orderBy.toString()}' instead.` ); } } @@ -2118,7 +2148,7 @@ export class QuerySnapshot implements firestore.QuerySnapshot { throw new FirestoreError( Code.INVALID_ARGUMENT, 'To include metadata changes with your document changes, you must ' + - 'also pass { includeMetadataChanges:true } to onSnapshot().' + 'also pass { includeMetadataChanges:true } to onSnapshot().' ); } @@ -2171,8 +2201,8 @@ function throwDocChangesMethodError(): never { throw new FirestoreError( Code.INVALID_ARGUMENT, 'QuerySnapshot.docChanges has been changed from a property into a ' + - 'method, so usages like "querySnapshot.docChanges" should become ' + - '"querySnapshot.docChanges()"' + 'method, so usages like "querySnapshot.docChanges" should become ' + + '"querySnapshot.docChanges()"' ); } @@ -2194,7 +2224,7 @@ docChangesPropertiesToOverride.forEach(property => { Object.defineProperty(QuerySnapshot.prototype.docChanges, property, { get: () => throwDocChangesMethodError() }); - } catch (err) {} // Ignore this failure intentionally + } catch (err) { } // Ignore this failure intentionally }); export class CollectionReference extends Query @@ -2205,8 +2235,8 @@ export class CollectionReference extends Query throw new FirestoreError( Code.INVALID_ARGUMENT, 'Invalid collection reference. Collection ' + - 'references must have an odd number of segments, but ' + - `${path.canonicalString()} has ${path.length}` + 'references must have an odd number of segments, but ' + + `${path.canonicalString()} has ${path.length}` ); } } @@ -2287,7 +2317,7 @@ function validateSetOptions( throw new FirestoreError( Code.INVALID_ARGUMENT, `Invalid options passed to function ${methodName}(): You cannot specify both "merge" ` + - `and "mergeFields".` + `and "mergeFields".` ); } diff --git a/packages/firestore/src/core/firestore_client.ts b/packages/firestore/src/core/firestore_client.ts index 89a818535ce..500593a7bbb 100644 --- a/packages/firestore/src/core/firestore_client.ts +++ b/packages/firestore/src/core/firestore_client.ts @@ -74,14 +74,14 @@ export class IndexedDbPersistenceSettings { constructor( readonly cacheSizeBytes: number, readonly synchronizeTabs: boolean - ) {} + ) { } lruParams(): LruParams { return LruParams.withCacheSize(this.cacheSizeBytes); } } -export class MemoryPersistenceSettings {} +export class MemoryPersistenceSettings { } export type InternalPersistenceSettings = | IndexedDbPersistenceSettings @@ -107,7 +107,6 @@ export class FirestoreClient { private lruScheduler?: LruScheduler; private readonly clientId = AutoId.newId(); - private _clientShutdown = false; // PORTING NOTE: SharedClientState is only used for multi-tab web. private sharedClientState: SharedClientState; @@ -125,7 +124,7 @@ export class FirestoreClient { * an async I/O to complete). */ private asyncQueue: AsyncQueue - ) {} + ) { } /** * Starts up the FirestoreClient, returning only whether or not enabling @@ -254,8 +253,8 @@ export class FirestoreClient { } console.warn( 'Error enabling offline persistence. Falling back to' + - ' persistence disabled: ' + - error + ' persistence disabled: ' + + error ); return this.startMemoryPersistence(); }); @@ -310,7 +309,7 @@ export class FirestoreClient { * this class cannot be called after the client is shutdown. */ private verifyNotShutdown(): void { - if (this._clientShutdown) { + if (this.asyncQueue.isShuttingDown) { throw new FirestoreError( Code.FAILED_PRECONDITION, 'The client has already been shutdown.' @@ -507,22 +506,19 @@ export class FirestoreClient { } shutdown(): Promise { - return this.asyncQueue.enqueue(async () => { - if (!this._clientShutdown) { - // PORTING NOTE: LocalStore does not need an explicit shutdown on web. - if (this.lruScheduler) { - this.lruScheduler.stop(); - } - await this.remoteStore.shutdown(); - await this.sharedClientState.shutdown(); - await this.persistence.shutdown(); - - // `removeChangeListener` must be called after shutting down the - // RemoteStore as it will prevent the RemoteStore from retrieving - // auth tokens. - this.credentials.removeChangeListener(); - this._clientShutdown = true; + return this.asyncQueue.enqueueAndInitilizeShutdown(async () => { + // PORTING NOTE: LocalStore does not need an explicit shutdown on web. + if (this.lruScheduler) { + this.lruScheduler.stop(); } + await this.remoteStore.shutdown(); + await this.sharedClientState.shutdown(); + await this.persistence.shutdown(); + + // `removeChangeListener` must be called after shutting down the + // RemoteStore as it will prevent the RemoteStore from retrieving + // auth tokens. + this.credentials.removeChangeListener(); }); } @@ -561,9 +557,9 @@ export class FirestoreClient { throw new FirestoreError( Code.UNAVAILABLE, 'Failed to get document from cache. (However, this document may ' + - "exist on the server. Run again without setting 'source' in " + - 'the GetOptions to attempt to retrieve the document from the ' + - 'server.)' + "exist on the server. Run again without setting 'source' in " + + 'the GetOptions to attempt to retrieve the document from the ' + + 'server.)' ); } }); @@ -603,7 +599,10 @@ export class FirestoreClient { } get clientShutdown(): boolean { - return this._clientShutdown; + // Technically, the asyncQueue is still running, but only accepting operations + // related to shutdown or supposed to be run after shutdown. It is effectively + // shut down to the eyes of users. + return this.asyncQueue.isShuttingDown; } transaction( @@ -612,7 +611,7 @@ export class FirestoreClient { this.verifyNotShutdown(); // We have to wait for the async queue to be sure syncEngine is initialized. return this.asyncQueue - .enqueue(async () => {}) + .enqueue(async () => { }) .then(() => this.syncEngine.runTransaction(updateFunction)); } } diff --git a/packages/firestore/src/util/async_queue.ts b/packages/firestore/src/util/async_queue.ts index 5483100b893..b6d45b961c0 100644 --- a/packages/firestore/src/util/async_queue.ts +++ b/packages/firestore/src/util/async_queue.ts @@ -86,7 +86,7 @@ class DelayedOperation implements CancelablePromise { // It's normal for the deferred promise to be canceled (due to cancellation) // and so we attach a dummy catch callback to avoid // 'UnhandledPromiseRejectionWarning' log spam. - this.deferred.promise.catch(err => {}); + this.deferred.promise.catch(err => { }); } /** @@ -188,6 +188,10 @@ export class AsyncQueue { // The last promise in the queue. private tail: Promise = Promise.resolve(); + // Is this AsyncQueue being shutting down? Once it is set to true, it will not + // be changed again. + private _isShuttingDown: boolean = false; + // Operations scheduled to be queued in the future. Operations are // automatically removed after they are run or canceled. private delayedOperations: Array> = []; @@ -199,6 +203,10 @@ export class AsyncQueue { // assertion sanity-checks. private operationInProgress = false; + // Is this AsyncQueue being shutting down? If true, this instance will not enqueue + // any new operations, Promises from enqueue requests will not resolve. + get isShuttingDown(): boolean { return this._isShuttingDown; } + /** * Adds a new operation to the queue without waiting for it to complete (i.e. * we ignore the Promise result). @@ -208,12 +216,59 @@ export class AsyncQueue { this.enqueue(op); } + /** + * Regardless if the queue has initialized shutdown, adds a new operation to the + * queue without waiting for it to complete (i.e. we ignore the Promise result). + */ + enqueueAndForgetEvenAfterShutdown(op: () => Promise): void { + this.verifyNotFailed(); + // tslint:disable-next-line:no-floating-promises + this.enqueueInternal(op); + } + + /** + * Regardless if the queue has initialized shutdown, adds a new operation to the + * queue. + */ + private enqueueEvenAfterShutdown(op: () => Promise): Promise { + this.verifyNotFailed(); + // tslint:disable-next-line:no-floating-promises + return this.enqueueInternal(op); + } + + /** + * Adds a new operation to the queue and initialize the shut down of this queue. + * Returns a promise that will be resolved when the promise returned by the new + * operation is (with its value). + * Once this method is called, the only possible way to request running an operation + * is through `enqueueAndForgetEvenAfterShutdown`. + */ + enqueueAndInitilizeShutdown(op: () => Promise): Promise { + this.verifyNotFailed(); + if (this._isShuttingDown) { + // Return a Promise resolves right away if it is already shutdown. + return new Promise((resolve) => resolve(undefined)); + } + + const promise = this.enqueueInternal(op); + this._isShuttingDown = true; + return promise; + } + /** * Adds a new operation to the queue. Returns a promise that will be resolved * when the promise returned by the new operation is (with its value). */ enqueue(op: () => Promise): Promise { this.verifyNotFailed(); + if (this._isShuttingDown) { + // Return a Promise which never resolves. + return new Promise((resolve) => {}); + } + return this.enqueueInternal(op); + } + + private enqueueInternal(op: () => Promise): Promise { const newTail = this.tail.then(() => { this.operationInProgress = true; return op() @@ -286,7 +341,7 @@ export class AsyncQueue { if (this.failure) { fail( 'AsyncQueue is already failed: ' + - (this.failure.stack || this.failure.message) + (this.failure.stack || this.failure.message) ); } } @@ -309,7 +364,8 @@ export class AsyncQueue { * operations are not run. */ drain(): Promise { - return this.enqueue(() => Promise.resolve()); + // It should still be possible to drain the queue after it is shutting down. + return this.enqueueEvenAfterShutdown(() => Promise.resolve()); } /** @@ -338,7 +394,7 @@ export class AsyncQueue { return this.drain().then(() => { assert( lastTimerId === TimerId.All || - this.containsDelayedOperation(lastTimerId), + this.containsDelayedOperation(lastTimerId), `Attempted to drain to missing operation ${lastTimerId}` ); diff --git a/packages/firestore/test/integration/api/database.test.ts b/packages/firestore/test/integration/api/database.test.ts index dd1b8d276a9..b834d8a5a21 100644 --- a/packages/firestore/test/integration/api/database.test.ts +++ b/packages/firestore/test/integration/api/database.test.ts @@ -962,7 +962,7 @@ apiDescribe('Database', (persistence: boolean) => { }); }); - (persistence ? it : it.skip)( + (persistence ? it: it.skip)( 'maintains persistence after restarting app', async () => { await withTestDoc(persistence, async docRef => { @@ -1068,4 +1068,49 @@ apiDescribe('Database', (persistence: boolean) => { await db.enableNetwork(); }); }); + + it('can start a new instance after shut down', async () => { + return withTestDoc(persistence, async docRef => { + const firestore = docRef.firestore; + await firestore.INTERNAL.shutdown(); + + const newFirestore = firebase.firestore!(firestore.app); + await newFirestore.doc(docRef.path).set({foo: 'bar'}); + const doc = await newFirestore.doc(docRef.path).get(); + expect(doc.data()).to.deep.equal({ foo: 'bar' }); + }); + }); + + it('app delete leads to instance shutdown', async () => { + await withTestDoc(persistence, async docRef => { + await docRef.set({ foo: 'bar' }); + const app = docRef.firestore.app; + await app.delete(); + + expect(docRef.firestore.INTERNAL.isShutdown()).to.be.true; + }); + }); + + it('new operation after shutdown should throw', async () => { + await withTestDoc(persistence, async docRef => { + const firestore = docRef.firestore; + await firestore.INTERNAL.shutdown(); + + expect(() => { + firestore.doc(docRef.path).set({foo: 'bar'}); + }).to.throw(); + }); + }); + + it('calling shutdown mutiple times should proceed', async () => { + await withTestDoc(persistence, async docRef => { + const firestore = docRef.firestore; + await firestore.INTERNAL.shutdown(); + await firestore.INTERNAL.shutdown(); + + expect(() => { + firestore.doc(docRef.path).set({foo: 'bar'}); + }).to.throw(); + }); + }); }); diff --git a/packages/firestore/test/unit/util/async_queue.test.ts b/packages/firestore/test/unit/util/async_queue.test.ts index 281ff463447..bbf18694eea 100644 --- a/packages/firestore/test/unit/util/async_queue.test.ts +++ b/packages/firestore/test/unit/util/async_queue.test.ts @@ -209,6 +209,28 @@ describe('AsyncQueue', () => { await queue.drain(); expect(completedSteps).to.deep.equal([1, 2]); }); + + it('Schedules operaions with respect to shut down', async () => { + const queue = new AsyncQueue(); + const completedSteps: number[] = []; + const doStep = (n: number): Promise => + defer(() => { + const result = completedSteps.push(n); + return result; + }); + + queue.enqueueAndForget(() => doStep(1)); + + // After this call, only operations requested via + // `enqueueAndForgetEvenAfterShutdown` gets executed. + // tslint:disable-next-line:no-floating-promises + queue.enqueueAndInitilizeShutdown(() => doStep(2)); + queue.enqueueAndForget(() => doStep(3)); + queue.enqueueAndForgetEvenAfterShutdown(() => doStep(4)); + + await queue.drain(); + expect(completedSteps).to.deep.equal([1, 2, 4]); + }); }); function defer(op: () => T): Promise { From efffc5eb96796fcf6e3cd5488ec5188938ca650c Mon Sep 17 00:00:00 2001 From: Hui Wu Date: Thu, 1 Aug 2019 11:05:49 -0400 Subject: [PATCH 04/10] browser passes/node fails --- packages/firestore/test/integration/api/database.test.ts | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/packages/firestore/test/integration/api/database.test.ts b/packages/firestore/test/integration/api/database.test.ts index b834d8a5a21..a8d317f90cc 100644 --- a/packages/firestore/test/integration/api/database.test.ts +++ b/packages/firestore/test/integration/api/database.test.ts @@ -21,7 +21,7 @@ import * as firestore from '@firebase/firestore-types'; import { expect, use } from 'chai'; import { SimpleDb } from '../../../src/local/simple_db'; -import { fail } from '../../../src/util/assert'; +import { fail, assert } from '../../../src/util/assert'; import { Code } from '../../../src/util/error'; import { query } from '../../util/api_helpers'; import { Deferred } from '../../util/promise'; @@ -1069,12 +1069,13 @@ apiDescribe('Database', (persistence: boolean) => { }); }); - it('can start a new instance after shut down', async () => { + it.only('can start a new instance after shut down', async () => { return withTestDoc(persistence, async docRef => { const firestore = docRef.firestore; await firestore.INTERNAL.shutdown(); const newFirestore = firebase.firestore!(firestore.app); + expect(newFirestore).to.not.equal(firestore); await newFirestore.doc(docRef.path).set({foo: 'bar'}); const doc = await newFirestore.doc(docRef.path).get(); expect(doc.data()).to.deep.equal({ foo: 'bar' }); From a07588c79f804efb7ce5cb18de5aac624eddf567 Mon Sep 17 00:00:00 2001 From: Hui Wu Date: Thu, 1 Aug 2019 11:08:29 -0400 Subject: [PATCH 05/10] [AUTOMATED]: Prettier Code Styling --- packages/firebase/index.d.ts | 8 +- packages/firestore-types/index.d.ts | 8 +- packages/firestore/src/api/database.ts | 206 +++++++++--------- .../firestore/src/core/firestore_client.ts | 18 +- packages/firestore/src/util/async_queue.ts | 26 ++- .../test/integration/api/database.test.ts | 10 +- 6 files changed, 143 insertions(+), 133 deletions(-) diff --git a/packages/firebase/index.d.ts b/packages/firebase/index.d.ts index 73997980051..79e516c8938 100644 --- a/packages/firebase/index.d.ts +++ b/packages/firebase/index.d.ts @@ -6225,10 +6225,10 @@ declare namespace firebase.firestore { /** * @hidden */ - INTERNAL: { - delete: () => Promise - shutdown: () => Promise - isShutdown: () => boolean + INTERNAL: { + delete: () => Promise; + shutdown: () => Promise; + isShutdown: () => boolean; }; } diff --git a/packages/firestore-types/index.d.ts b/packages/firestore-types/index.d.ts index 9cd263ffc18..eb66cd1830d 100644 --- a/packages/firestore-types/index.d.ts +++ b/packages/firestore-types/index.d.ts @@ -276,10 +276,10 @@ export class FirebaseFirestore { */ disableNetwork(): Promise; - INTERNAL: { - delete: () => Promise - shutdown: () => Promise - isShutdown: () => boolean + INTERNAL: { + delete: () => Promise; + shutdown: () => Promise; + isShutdown: () => boolean; }; } diff --git a/packages/firestore/src/api/database.ts b/packages/firestore/src/api/database.ts index a5eec9caa18..570d34466a8 100644 --- a/packages/firestore/src/api/database.ts +++ b/packages/firestore/src/api/database.ts @@ -353,7 +353,7 @@ export class Firestore implements firestore.FirebaseFirestore, FirebaseService { throw new FirestoreError( Code.INVALID_ARGUMENT, '"persistence" is now specified with a separate call to ' + - 'firestore.enablePersistence().' + 'firestore.enablePersistence().' ); } @@ -362,8 +362,8 @@ export class Firestore implements firestore.FirebaseFirestore, FirebaseService { throw new FirestoreError( Code.FAILED_PRECONDITION, '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.' + 'be changed. You can only call settings() before calling any other ' + + 'methods on a Firestore object.' ); } @@ -390,8 +390,8 @@ export class Firestore implements firestore.FirebaseFirestore, FirebaseService { throw new FirestoreError( Code.FAILED_PRECONDITION, 'Firestore has already been started and persistence can no longer ' + - 'be enabled. You can only call enablePersistence() before calling ' + - 'any other methods on a Firestore object.' + 'be enabled. You can only call enablePersistence() before calling ' + + 'any other methods on a Firestore object.' ); } @@ -401,9 +401,9 @@ export class Firestore implements firestore.FirebaseFirestore, FirebaseService { if (settings.experimentalTabSynchronization !== undefined) { log.error( "The 'experimentalTabSynchronization' setting has been renamed to " + - "'synchronizeTabs'. In a future release, the setting will be removed " + - 'and it is recommended that you update your ' + - "firestore.enablePersistence() call to use 'synchronizeTabs'." + "'synchronizeTabs'. In a future release, the setting will be removed " + + 'and it is recommended that you update your ' + + "firestore.enablePersistence() call to use 'synchronizeTabs'." ); } synchronizeTabs = objUtils.defaulted( @@ -486,8 +486,8 @@ export class Firestore implements firestore.FirebaseFirestore, FirebaseService { throw new FirestoreError( Code.INVALID_ARGUMENT, 'Document reference is for database ' + - `${otherDb.projectId}/${otherDb.database} but should be ` + - `for database ${thisDb.projectId}/${thisDb.database}` + `${otherDb.projectId}/${otherDb.database} but should be ` + + `for database ${thisDb.projectId}/${thisDb.database}` ); } return new DocumentKeyReference(this._config.databaseId, value._key); @@ -531,7 +531,7 @@ export class Firestore implements firestore.FirebaseFirestore, FirebaseService { throw new FirestoreError( Code.FAILED_PRECONDITION, "Firestore was not initialized using the Firebase SDK. 'app' is " + - 'not available' + 'not available' ); } return this._config.firebaseApp; @@ -551,7 +551,7 @@ export class Firestore implements firestore.FirebaseFirestore, FirebaseService { * After shutdown only the `clearPersistence()` method may be used. Any other method * will throw an `FirestoreError`. * - * To restart after shutdown, simply create a new instance of FirebaseFirestore with + * To restart after shutdown, simply create a new instance of FirebaseFirestore with * `Firebase.firestore()`. * * Shutdown does not cancel any pending writes and any promises that are awaiting a response @@ -567,7 +567,9 @@ export class Firestore implements firestore.FirebaseFirestore, FirebaseService { */ // TODO(b/135755126): make this public. shutdown: (): Promise => { - (this._config.firebaseApp as _FirebaseApp)._removeServiceInstance('firestore'); + (this._config.firebaseApp as _FirebaseApp)._removeServiceInstance( + 'firestore' + ); return this.INTERNAL.delete(); }, @@ -602,7 +604,7 @@ export class Firestore implements firestore.FirebaseFirestore, FirebaseService { throw new FirestoreError( Code.INVALID_ARGUMENT, `Invalid collection ID '${collectionId}' passed to function ` + - `Firestore.collectionGroup(). Collection IDs must not contain '/'.` + `Firestore.collectionGroup(). Collection IDs must not contain '/'.` ); } this.ensureClientConfigured(); @@ -678,7 +680,7 @@ export class Transaction implements firestore.Transaction { constructor( private _firestore: Firestore, private _transaction: InternalTransaction - ) { } + ) {} get( documentRef: firestore.DocumentReference @@ -735,10 +737,10 @@ export class Transaction implements firestore.Transaction { const parsed = options.merge || options.mergeFields ? this._firestore._dataConverter.parseMergeData( - 'Transaction.set', - value, - options.mergeFields - ) + 'Transaction.set', + value, + options.mergeFields + ) : this._firestore._dataConverter.parseSetData('Transaction.set', value); this._transaction.set(ref._key, parsed); return this; @@ -812,7 +814,7 @@ export class WriteBatch implements firestore.WriteBatch { private _mutations = [] as Mutation[]; private _committed = false; - constructor(private _firestore: Firestore) { } + constructor(private _firestore: Firestore) {} set( documentRef: firestore.DocumentReference, @@ -830,10 +832,10 @@ export class WriteBatch implements firestore.WriteBatch { const parsed = options.merge || options.mergeFields ? this._firestore._dataConverter.parseMergeData( - 'WriteBatch.set', - value, - options.mergeFields - ) + 'WriteBatch.set', + value, + options.mergeFields + ) : this._firestore._dataConverter.parseSetData('WriteBatch.set', value); this._mutations = this._mutations.concat( parsed.toMutations(ref._key, Precondition.NONE) @@ -924,7 +926,7 @@ export class WriteBatch implements firestore.WriteBatch { throw new FirestoreError( Code.FAILED_PRECONDITION, 'A write batch can no longer be used after commit() ' + - 'has been called.' + 'has been called.' ); } } @@ -945,8 +947,8 @@ export class DocumentReference implements firestore.DocumentReference { throw new FirestoreError( Code.INVALID_ARGUMENT, 'Invalid document reference. Document ' + - 'references must have an even number of segments, but ' + - `${path.canonicalString()} has ${path.length}` + 'references must have an even number of segments, but ' + + `${path.canonicalString()} has ${path.length}` ); } return new DocumentReference(new DocumentKey(path), firestore); @@ -999,14 +1001,14 @@ export class DocumentReference implements firestore.DocumentReference { const parsed = options.merge || options.mergeFields ? this.firestore._dataConverter.parseMergeData( - 'DocumentReference.set', - value, - options.mergeFields - ) + 'DocumentReference.set', + value, + options.mergeFields + ) : this.firestore._dataConverter.parseSetData( - 'DocumentReference.set', - value - ); + 'DocumentReference.set', + value + ); return this._firestoreClient.write( parsed.toMutations(this._key, Precondition.NONE) ); @@ -1250,9 +1252,9 @@ export class DocumentReference implements firestore.DocumentReference { new FirestoreError( Code.UNAVAILABLE, 'Failed to get document from server. (However, this ' + - 'document does exist in the local cache. Run again ' + - 'without setting source to "server" to ' + - 'retrieve the cached document.)' + 'document does exist in the local cache. Run again ' + + 'without setting source to "server" to ' + + 'retrieve the cached document.)' ) ); } else { @@ -1269,7 +1271,7 @@ class SnapshotMetadata implements firestore.SnapshotMetadata { constructor( readonly hasPendingWrites: boolean, readonly fromCache: boolean - ) { } + ) {} isEqual(other: firestore.SnapshotMetadata): boolean { return ( @@ -1283,7 +1285,7 @@ class SnapshotMetadata implements firestore.SnapshotMetadata { * Options interface that can be provided to configure the deserialization of * DocumentSnapshots. */ -export interface SnapshotOptions extends firestore.SnapshotOptions { } +export interface SnapshotOptions extends firestore.SnapshotOptions {} export class DocumentSnapshot implements firestore.DocumentSnapshot { constructor( @@ -1292,7 +1294,7 @@ export class DocumentSnapshot implements firestore.DocumentSnapshot { public _document: Document | null, private _fromCache: boolean, private _hasPendingWrites: boolean - ) { } + ) {} data( options?: firestore.SnapshotOptions @@ -1302,12 +1304,12 @@ export class DocumentSnapshot implements firestore.DocumentSnapshot { return !this._document ? undefined : this.convertObject( - this._document.data, - FieldValueOptions.fromSnapshotOptions( - options, - this._firestore._areTimestampsInSnapshotsEnabled() - ) - ); + this._document.data, + FieldValueOptions.fromSnapshotOptions( + options, + this._firestore._areTimestampsInSnapshotsEnabled() + ) + ); } get( @@ -1386,11 +1388,11 @@ export class DocumentSnapshot implements firestore.DocumentSnapshot { // TODO(b/64130202): Somehow support foreign references. log.error( `Document ${this._key.path} contains a document ` + - `reference within a different database (` + - `${value.databaseId.projectId}/${value.databaseId.database}) which is not ` + - `supported. It will be treated as a reference in the current ` + - `database (${database.projectId}/${database.database}) ` + - `instead.` + `reference within a different database (` + + `${value.databaseId.projectId}/${value.databaseId.database}) which is not ` + + `supported. It will be treated as a reference in the current ` + + `database (${database.projectId}/${database.database}) ` + + `instead.` ); } return new DocumentReference(key, this._firestore); @@ -1422,7 +1424,7 @@ export class QueryDocumentSnapshot extends DocumentSnapshot } export class Query implements firestore.Query { - constructor(public _query: InternalQuery, readonly firestore: Firestore) { } + constructor(public _query: InternalQuery, readonly firestore: Firestore) {} where( field: string | ExternalFieldPath, @@ -1453,7 +1455,7 @@ export class Query implements firestore.Query { throw new FirestoreError( Code.INVALID_ARGUMENT, `Invalid Query. You can't perform '${operator.toString()}' ` + - 'queries on FieldPath.documentId().' + 'queries on FieldPath.documentId().' ); } else if (operator === Operator.IN) { this.validateDisjunctiveFilterElements(value, operator); @@ -1502,21 +1504,21 @@ export class Query implements firestore.Query { throw new FirestoreError( Code.INVALID_ARGUMENT, `Function Query.orderBy() has unknown direction '${directionStr}', ` + - `expected 'asc' or 'desc'.` + `expected 'asc' or 'desc'.` ); } if (this._query.startAt !== null) { throw new FirestoreError( Code.INVALID_ARGUMENT, 'Invalid query. You must not call Query.startAt() or ' + - 'Query.startAfter() before calling Query.orderBy().' + 'Query.startAfter() before calling Query.orderBy().' ); } if (this._query.endAt !== null) { throw new FirestoreError( Code.INVALID_ARGUMENT, 'Invalid query. You must not call Query.endAt() or ' + - 'Query.endBefore() before calling Query.orderBy().' + 'Query.endBefore() before calling Query.orderBy().' ); } const fieldPath = fieldPathFromArgument('Query.orderBy', field); @@ -1532,7 +1534,7 @@ export class Query implements firestore.Query { throw new FirestoreError( Code.INVALID_ARGUMENT, `Invalid Query. Query limit (${n}) is invalid. Limit must be ` + - 'positive.' + 'positive.' ); } return new Query(this._query.withLimit(n), this.firestore); @@ -1623,7 +1625,7 @@ export class Query implements firestore.Query { throw new FirestoreError( Code.NOT_FOUND, `Can't use a DocumentSnapshot that doesn't exist for ` + - `${methodName}().` + `${methodName}().` ); } return this.boundFromDocument(methodName, snap._document!, before); @@ -1667,10 +1669,10 @@ export class Query implements firestore.Query { throw new FirestoreError( Code.INVALID_ARGUMENT, 'Invalid query. You are trying to start or end a query using a ' + - 'document for which the field "' + - orderBy.field + - '" is an uncommitted server timestamp. (Since the value of ' + - 'this field is unknown, you cannot start/end a query with it.)' + 'document for which the field "' + + orderBy.field + + '" is an uncommitted server timestamp. (Since the value of ' + + 'this field is unknown, you cannot start/end a query with it.)' ); } else if (value !== null) { components.push(value); @@ -1679,8 +1681,8 @@ export class Query implements firestore.Query { throw new FirestoreError( Code.INVALID_ARGUMENT, `Invalid query. You are trying to start or end a query using a ` + - `document for which the field '${field}' (used as the ` + - `orderBy) does not exist.` + `document for which the field '${field}' (used as the ` + + `orderBy) does not exist.` ); } } @@ -1702,8 +1704,8 @@ export class Query implements firestore.Query { throw new FirestoreError( Code.INVALID_ARGUMENT, `Too many arguments provided to ${methodName}(). ` + - `The number of arguments must be less than or equal to the ` + - `number of Query.orderBy() clauses` + `The number of arguments must be less than or equal to the ` + + `number of Query.orderBy() clauses` ); } @@ -1716,7 +1718,7 @@ export class Query implements firestore.Query { throw new FirestoreError( Code.INVALID_ARGUMENT, `Invalid query. Expected a string for document ID in ` + - `${methodName}(), but got a ${typeof rawValue}` + `${methodName}(), but got a ${typeof rawValue}` ); } if ( @@ -1726,8 +1728,8 @@ export class Query implements firestore.Query { throw new FirestoreError( Code.INVALID_ARGUMENT, `Invalid query. When querying a collection and ordering by FieldPath.documentId(), ` + - `the value passed to ${methodName}() must be a plain document ID, but ` + - `'${rawValue}' contains a slash.` + `the value passed to ${methodName}() must be a plain document ID, but ` + + `'${rawValue}' contains a slash.` ); } const path = this._query.path.child(ResourcePath.fromString(rawValue)); @@ -1735,9 +1737,9 @@ export class Query implements firestore.Query { throw new FirestoreError( Code.INVALID_ARGUMENT, `Invalid query. When querying a collection group and ordering by ` + - `FieldPath.documentId(), the value passed to ${methodName}() must result in a ` + - `valid document path, but '${path}' is not because it contains an odd number ` + - `of segments.` + `FieldPath.documentId(), the value passed to ${methodName}() must result in a ` + + `valid document path, but '${path}' is not because it contains an odd number ` + + `of segments.` ); } const key = new DocumentKey(path); @@ -1894,9 +1896,9 @@ export class Query implements firestore.Query { new FirestoreError( Code.UNAVAILABLE, 'Failed to get documents from server. (However, these ' + - 'documents may exist in the local cache. Run again ' + - 'without setting source to "server" to ' + - 'retrieve the cached documents.)' + 'documents may exist in the local cache. Run again ' + + 'without setting source to "server" to ' + + 'retrieve the cached documents.)' ) ); } else { @@ -1919,7 +1921,7 @@ export class Query implements firestore.Query { throw new FirestoreError( Code.INVALID_ARGUMENT, 'Invalid query. When querying with FieldPath.documentId(), you ' + - 'must provide a valid document ID, but it was an empty string.' + 'must provide a valid document ID, but it was an empty string.' ); } if ( @@ -1929,8 +1931,8 @@ export class Query implements firestore.Query { throw new FirestoreError( Code.INVALID_ARGUMENT, `Invalid query. When querying a collection by ` + - `FieldPath.documentId(), you must provide a plain document ID, but ` + - `'${documentIdValue}' contains a '/' character.` + `FieldPath.documentId(), you must provide a plain document ID, but ` + + `'${documentIdValue}' contains a '/' character.` ); } const path = this._query.path.child( @@ -1940,8 +1942,8 @@ export class Query implements firestore.Query { throw new FirestoreError( Code.INVALID_ARGUMENT, `Invalid query. When querying a collection group by ` + - `FieldPath.documentId(), the value provided must result in a valid document path, ` + - `but '${path}' is not because it has an odd number of segments (${path.length}).` + `FieldPath.documentId(), the value provided must result in a valid document path, ` + + `but '${path}' is not because it has an odd number of segments (${path.length}).` ); } return new RefValue(this.firestore._databaseId, new DocumentKey(path)); @@ -1952,8 +1954,8 @@ export class Query implements firestore.Query { throw new FirestoreError( Code.INVALID_ARGUMENT, `Invalid query. When querying with FieldPath.documentId(), you must provide a valid ` + - `string or a DocumentReference, but it was: ` + - `${valueDescription(documentIdValue)}.` + `string or a DocumentReference, but it was: ` + + `${valueDescription(documentIdValue)}.` ); } } @@ -1970,28 +1972,28 @@ export class Query implements firestore.Query { throw new FirestoreError( Code.INVALID_ARGUMENT, 'Invalid Query. A non-empty array is required for ' + - `'${operator.toString()}' filters.` + `'${operator.toString()}' filters.` ); } if (value.length > 10) { throw new FirestoreError( Code.INVALID_ARGUMENT, `Invalid Query. '${operator.toString()}' filters support a ` + - 'maximum of 10 elements in the value array.' + 'maximum of 10 elements in the value array.' ); } if (value.indexOf(null) >= 0) { throw new FirestoreError( Code.INVALID_ARGUMENT, `Invalid Query. '${operator.toString()}' filters cannot contain 'null' ` + - 'in the value array.' + 'in the value array.' ); } if (value.filter(element => Number.isNaN(element)).length > 0) { throw new FirestoreError( Code.INVALID_ARGUMENT, `Invalid Query. '${operator.toString()}' filters cannot contain 'NaN' ` + - 'in the value array.' + 'in the value array.' ); } } @@ -2009,9 +2011,9 @@ export class Query implements firestore.Query { throw new FirestoreError( Code.INVALID_ARGUMENT, 'Invalid query. All where filters with an inequality' + - ' (<, <=, >, or >=) must be on the same field. But you have' + - ` inequality filters on '${existingField.toString()}'` + - ` and '${filter.field.toString()}'` + ' (<, <=, >, or >=) must be on the same field. But you have' + + ` inequality filters on '${existingField.toString()}'` + + ` and '${filter.field.toString()}'` ); } @@ -2038,13 +2040,13 @@ export class Query implements firestore.Query { throw new FirestoreError( Code.INVALID_ARGUMENT, 'Invalid query. You cannot use more than one ' + - `'${filter.op.toString()}' filter.` + `'${filter.op.toString()}' filter.` ); } else { throw new FirestoreError( Code.INVALID_ARGUMENT, `Invalid query. You cannot use '${filter.op.toString()}' filters ` + - `with '${conflictingOp.toString()}' filters.` + `with '${conflictingOp.toString()}' filters.` ); } } @@ -2070,10 +2072,10 @@ export class Query implements firestore.Query { throw new FirestoreError( Code.INVALID_ARGUMENT, `Invalid query. You have a where filter with an inequality ` + - `(<, <=, >, or >=) on field '${inequality.toString()}' ` + - `and so you must also use '${inequality.toString()}' ` + - `as your first Query.orderBy(), but your first Query.orderBy() ` + - `is on field '${orderBy.toString()}' instead.` + `(<, <=, >, or >=) on field '${inequality.toString()}' ` + + `and so you must also use '${inequality.toString()}' ` + + `as your first Query.orderBy(), but your first Query.orderBy() ` + + `is on field '${orderBy.toString()}' instead.` ); } } @@ -2148,7 +2150,7 @@ export class QuerySnapshot implements firestore.QuerySnapshot { throw new FirestoreError( Code.INVALID_ARGUMENT, 'To include metadata changes with your document changes, you must ' + - 'also pass { includeMetadataChanges:true } to onSnapshot().' + 'also pass { includeMetadataChanges:true } to onSnapshot().' ); } @@ -2201,8 +2203,8 @@ function throwDocChangesMethodError(): never { throw new FirestoreError( Code.INVALID_ARGUMENT, 'QuerySnapshot.docChanges has been changed from a property into a ' + - 'method, so usages like "querySnapshot.docChanges" should become ' + - '"querySnapshot.docChanges()"' + 'method, so usages like "querySnapshot.docChanges" should become ' + + '"querySnapshot.docChanges()"' ); } @@ -2224,7 +2226,7 @@ docChangesPropertiesToOverride.forEach(property => { Object.defineProperty(QuerySnapshot.prototype.docChanges, property, { get: () => throwDocChangesMethodError() }); - } catch (err) { } // Ignore this failure intentionally + } catch (err) {} // Ignore this failure intentionally }); export class CollectionReference extends Query @@ -2235,8 +2237,8 @@ export class CollectionReference extends Query throw new FirestoreError( Code.INVALID_ARGUMENT, 'Invalid collection reference. Collection ' + - 'references must have an odd number of segments, but ' + - `${path.canonicalString()} has ${path.length}` + 'references must have an odd number of segments, but ' + + `${path.canonicalString()} has ${path.length}` ); } } @@ -2317,7 +2319,7 @@ function validateSetOptions( throw new FirestoreError( Code.INVALID_ARGUMENT, `Invalid options passed to function ${methodName}(): You cannot specify both "merge" ` + - `and "mergeFields".` + `and "mergeFields".` ); } diff --git a/packages/firestore/src/core/firestore_client.ts b/packages/firestore/src/core/firestore_client.ts index 500593a7bbb..f3dbdcd85b6 100644 --- a/packages/firestore/src/core/firestore_client.ts +++ b/packages/firestore/src/core/firestore_client.ts @@ -74,14 +74,14 @@ export class IndexedDbPersistenceSettings { constructor( readonly cacheSizeBytes: number, readonly synchronizeTabs: boolean - ) { } + ) {} lruParams(): LruParams { return LruParams.withCacheSize(this.cacheSizeBytes); } } -export class MemoryPersistenceSettings { } +export class MemoryPersistenceSettings {} export type InternalPersistenceSettings = | IndexedDbPersistenceSettings @@ -124,7 +124,7 @@ export class FirestoreClient { * an async I/O to complete). */ private asyncQueue: AsyncQueue - ) { } + ) {} /** * Starts up the FirestoreClient, returning only whether or not enabling @@ -253,8 +253,8 @@ export class FirestoreClient { } console.warn( 'Error enabling offline persistence. Falling back to' + - ' persistence disabled: ' + - error + ' persistence disabled: ' + + error ); return this.startMemoryPersistence(); }); @@ -557,9 +557,9 @@ export class FirestoreClient { throw new FirestoreError( Code.UNAVAILABLE, 'Failed to get document from cache. (However, this document may ' + - "exist on the server. Run again without setting 'source' in " + - 'the GetOptions to attempt to retrieve the document from the ' + - 'server.)' + "exist on the server. Run again without setting 'source' in " + + 'the GetOptions to attempt to retrieve the document from the ' + + 'server.)' ); } }); @@ -611,7 +611,7 @@ export class FirestoreClient { this.verifyNotShutdown(); // We have to wait for the async queue to be sure syncEngine is initialized. return this.asyncQueue - .enqueue(async () => { }) + .enqueue(async () => {}) .then(() => this.syncEngine.runTransaction(updateFunction)); } } diff --git a/packages/firestore/src/util/async_queue.ts b/packages/firestore/src/util/async_queue.ts index b6d45b961c0..883ff321311 100644 --- a/packages/firestore/src/util/async_queue.ts +++ b/packages/firestore/src/util/async_queue.ts @@ -86,7 +86,7 @@ class DelayedOperation implements CancelablePromise { // It's normal for the deferred promise to be canceled (due to cancellation) // and so we attach a dummy catch callback to avoid // 'UnhandledPromiseRejectionWarning' log spam. - this.deferred.promise.catch(err => { }); + this.deferred.promise.catch(err => {}); } /** @@ -205,7 +205,9 @@ export class AsyncQueue { // Is this AsyncQueue being shutting down? If true, this instance will not enqueue // any new operations, Promises from enqueue requests will not resolve. - get isShuttingDown(): boolean { return this._isShuttingDown; } + get isShuttingDown(): boolean { + return this._isShuttingDown; + } /** * Adds a new operation to the queue without waiting for it to complete (i.e. @@ -220,7 +222,9 @@ export class AsyncQueue { * Regardless if the queue has initialized shutdown, adds a new operation to the * queue without waiting for it to complete (i.e. we ignore the Promise result). */ - enqueueAndForgetEvenAfterShutdown(op: () => Promise): void { + enqueueAndForgetEvenAfterShutdown( + op: () => Promise + ): void { this.verifyNotFailed(); // tslint:disable-next-line:no-floating-promises this.enqueueInternal(op); @@ -230,7 +234,9 @@ export class AsyncQueue { * Regardless if the queue has initialized shutdown, adds a new operation to the * queue. */ - private enqueueEvenAfterShutdown(op: () => Promise): Promise { + private enqueueEvenAfterShutdown( + op: () => Promise + ): Promise { this.verifyNotFailed(); // tslint:disable-next-line:no-floating-promises return this.enqueueInternal(op); @@ -243,11 +249,13 @@ export class AsyncQueue { * Once this method is called, the only possible way to request running an operation * is through `enqueueAndForgetEvenAfterShutdown`. */ - enqueueAndInitilizeShutdown(op: () => Promise): Promise { + enqueueAndInitilizeShutdown( + op: () => Promise + ): Promise { this.verifyNotFailed(); if (this._isShuttingDown) { // Return a Promise resolves right away if it is already shutdown. - return new Promise((resolve) => resolve(undefined)); + return new Promise(resolve => resolve(undefined)); } const promise = this.enqueueInternal(op); @@ -263,7 +271,7 @@ export class AsyncQueue { this.verifyNotFailed(); if (this._isShuttingDown) { // Return a Promise which never resolves. - return new Promise((resolve) => {}); + return new Promise(resolve => {}); } return this.enqueueInternal(op); } @@ -341,7 +349,7 @@ export class AsyncQueue { if (this.failure) { fail( 'AsyncQueue is already failed: ' + - (this.failure.stack || this.failure.message) + (this.failure.stack || this.failure.message) ); } } @@ -394,7 +402,7 @@ export class AsyncQueue { return this.drain().then(() => { assert( lastTimerId === TimerId.All || - this.containsDelayedOperation(lastTimerId), + this.containsDelayedOperation(lastTimerId), `Attempted to drain to missing operation ${lastTimerId}` ); diff --git a/packages/firestore/test/integration/api/database.test.ts b/packages/firestore/test/integration/api/database.test.ts index a8d317f90cc..8f8cf6103b7 100644 --- a/packages/firestore/test/integration/api/database.test.ts +++ b/packages/firestore/test/integration/api/database.test.ts @@ -962,7 +962,7 @@ apiDescribe('Database', (persistence: boolean) => { }); }); - (persistence ? it: it.skip)( + (persistence ? it : it.skip)( 'maintains persistence after restarting app', async () => { await withTestDoc(persistence, async docRef => { @@ -1076,7 +1076,7 @@ apiDescribe('Database', (persistence: boolean) => { const newFirestore = firebase.firestore!(firestore.app); expect(newFirestore).to.not.equal(firestore); - await newFirestore.doc(docRef.path).set({foo: 'bar'}); + await newFirestore.doc(docRef.path).set({ foo: 'bar' }); const doc = await newFirestore.doc(docRef.path).get(); expect(doc.data()).to.deep.equal({ foo: 'bar' }); }); @@ -1087,7 +1087,7 @@ apiDescribe('Database', (persistence: boolean) => { await docRef.set({ foo: 'bar' }); const app = docRef.firestore.app; await app.delete(); - + expect(docRef.firestore.INTERNAL.isShutdown()).to.be.true; }); }); @@ -1098,7 +1098,7 @@ apiDescribe('Database', (persistence: boolean) => { await firestore.INTERNAL.shutdown(); expect(() => { - firestore.doc(docRef.path).set({foo: 'bar'}); + firestore.doc(docRef.path).set({ foo: 'bar' }); }).to.throw(); }); }); @@ -1110,7 +1110,7 @@ apiDescribe('Database', (persistence: boolean) => { await firestore.INTERNAL.shutdown(); expect(() => { - firestore.doc(docRef.path).set({foo: 'bar'}); + firestore.doc(docRef.path).set({ foo: 'bar' }); }).to.throw(); }); }); From 692dfa322c960dfc8e3d938c13ff9886721028f1 Mon Sep 17 00:00:00 2001 From: Hui Wu Date: Thu, 1 Aug 2019 19:39:51 -0400 Subject: [PATCH 06/10] add settings for new instance --- packages/firestore/src/api/database.ts | 27 +------------------ .../test/integration/api/database.test.ts | 10 ++++--- 2 files changed, 8 insertions(+), 29 deletions(-) diff --git a/packages/firestore/src/api/database.ts b/packages/firestore/src/api/database.ts index 1c5e7dcaa81..6f45d6cb0ef 100644 --- a/packages/firestore/src/api/database.ts +++ b/packages/firestore/src/api/database.ts @@ -537,31 +537,6 @@ export class Firestore implements firestore.FirebaseFirestore, FirebaseService { return this._config.firebaseApp; } - /** - * Shuts down this FirebaseFirestore instance. - * - * After shutdown only the `clearPersistence()` method may be used. Any other method - * will throw an `FirestoreError`. - * - * To restart after shutdown, simply create a new instance of FirebaseFirestore with - * `Firebase.firestore()`. - * - * Shutdown does not cancel any pending writes and any promises that are awaiting a response from - * the server will be resolved with `undefined`. The next time you start this instance, it will resume - * attempting to send these writes to the server. - * - * Note: Under normal circumstances, calling `shutdown()` is not required. This - * method is useful only when you want to force this instance to release all of its resources or - * in combination with `clearPersistence()` to ensure that all local state is destroyed - * between test runs. - * - * @return A promise that is resolved when the instance has been successfully shut down. - */ - // TODO(b/135755126): make this public. - private shutdown(): Promise { - return this.INTERNAL.delete(); - } - INTERNAL = { // TODO(b/135755126): Make this public delete: async (): Promise => { @@ -1078,7 +1053,7 @@ export class DocumentReference implements firestore.DocumentReference { } delete(): Promise { - validateExactNumberOfArgs('FirebaseFirestore.shutdown', arguments, 0); + validateExactNumberOfArgs('FirebaseFirestore.delete', arguments, 0); return this._firestoreClient.write([ new DeleteMutation(this._key, Precondition.NONE) ]); diff --git a/packages/firestore/test/integration/api/database.test.ts b/packages/firestore/test/integration/api/database.test.ts index 8f8cf6103b7..9f64447078b 100644 --- a/packages/firestore/test/integration/api/database.test.ts +++ b/packages/firestore/test/integration/api/database.test.ts @@ -21,7 +21,7 @@ import * as firestore from '@firebase/firestore-types'; import { expect, use } from 'chai'; import { SimpleDb } from '../../../src/local/simple_db'; -import { fail, assert } from '../../../src/util/assert'; +import { fail } from '../../../src/util/assert'; import { Code } from '../../../src/util/error'; import { query } from '../../util/api_helpers'; import { Deferred } from '../../util/promise'; @@ -35,7 +35,8 @@ import { withTestDb, withTestDbs, withTestDoc, - withTestDocAndInitialData + withTestDocAndInitialData, + DEFAULT_SETTINGS } from '../util/helpers'; // tslint:disable:no-floating-promises @@ -1069,13 +1070,16 @@ apiDescribe('Database', (persistence: boolean) => { }); }); - it.only('can start a new instance after shut down', async () => { + it('can start a new instance after shut down', async () => { return withTestDoc(persistence, async docRef => { const firestore = docRef.firestore; await firestore.INTERNAL.shutdown(); const newFirestore = firebase.firestore!(firestore.app); expect(newFirestore).to.not.equal(firestore); + + // New instance functions. + newFirestore.settings(DEFAULT_SETTINGS); await newFirestore.doc(docRef.path).set({ foo: 'bar' }); const doc = await newFirestore.doc(docRef.path).get(); expect(doc.data()).to.deep.equal({ foo: 'bar' }); From 1438df067150192d52aafd8950647a14799e9b04 Mon Sep 17 00:00:00 2001 From: Hui Wu Date: Thu, 1 Aug 2019 19:59:32 -0400 Subject: [PATCH 07/10] final self review. --- packages/firestore/src/api/database.ts | 2 +- packages/firestore/src/util/async_queue.ts | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/firestore/src/api/database.ts b/packages/firestore/src/api/database.ts index 6f45d6cb0ef..f825eb42ed4 100644 --- a/packages/firestore/src/api/database.ts +++ b/packages/firestore/src/api/database.ts @@ -1053,7 +1053,7 @@ export class DocumentReference implements firestore.DocumentReference { } delete(): Promise { - validateExactNumberOfArgs('FirebaseFirestore.delete', arguments, 0); + validateExactNumberOfArgs('DocumentReference.delete', arguments, 0); return this._firestoreClient.write([ new DeleteMutation(this._key, Precondition.NONE) ]); diff --git a/packages/firestore/src/util/async_queue.ts b/packages/firestore/src/util/async_queue.ts index 883ff321311..75ed7c8acc0 100644 --- a/packages/firestore/src/util/async_queue.ts +++ b/packages/firestore/src/util/async_queue.ts @@ -188,7 +188,7 @@ export class AsyncQueue { // The last promise in the queue. private tail: Promise = Promise.resolve(); - // Is this AsyncQueue being shutting down? Once it is set to true, it will not + // Is this AsyncQueue being shut down? Once it is set to true, it will not // be changed again. private _isShuttingDown: boolean = false; @@ -203,7 +203,7 @@ export class AsyncQueue { // assertion sanity-checks. private operationInProgress = false; - // Is this AsyncQueue being shutting down? If true, this instance will not enqueue + // Is this AsyncQueue being shut down? If true, this instance will not enqueue // any new operations, Promises from enqueue requests will not resolve. get isShuttingDown(): boolean { return this._isShuttingDown; From eeec3899fb926065450859202392990cb6488158 Mon Sep 17 00:00:00 2001 From: Hui Wu Date: Mon, 5 Aug 2019 21:58:50 -0400 Subject: [PATCH 08/10] addressing comments #1 --- packages/firebase/index.d.ts | 6 +- packages/firestore-types/index.d.ts | 6 +- packages/firestore/src/api/database.ts | 66 +++++++++---------- .../firestore/src/core/firestore_client.ts | 2 +- packages/firestore/src/util/async_queue.ts | 16 ++--- .../test/integration/api/database.test.ts | 10 +-- .../test/unit/util/async_queue.test.ts | 7 +- 7 files changed, 48 insertions(+), 65 deletions(-) diff --git a/packages/firebase/index.d.ts b/packages/firebase/index.d.ts index 79e516c8938..28b43b2a311 100644 --- a/packages/firebase/index.d.ts +++ b/packages/firebase/index.d.ts @@ -6225,11 +6225,7 @@ declare namespace firebase.firestore { /** * @hidden */ - INTERNAL: { - delete: () => Promise; - shutdown: () => Promise; - isShutdown: () => boolean; - }; + INTERNAL: { delete: () => Promise }; } /** diff --git a/packages/firestore-types/index.d.ts b/packages/firestore-types/index.d.ts index eb66cd1830d..c96fa5925dc 100644 --- a/packages/firestore-types/index.d.ts +++ b/packages/firestore-types/index.d.ts @@ -276,11 +276,7 @@ export class FirebaseFirestore { */ disableNetwork(): Promise; - INTERNAL: { - delete: () => Promise; - shutdown: () => Promise; - isShutdown: () => boolean; - }; + INTERNAL: { delete: () => Promise }; } /** diff --git a/packages/firestore/src/api/database.ts b/packages/firestore/src/api/database.ts index f825eb42ed4..dbfe1fe7880 100644 --- a/packages/firestore/src/api/database.ts +++ b/packages/firestore/src/api/database.ts @@ -447,6 +447,39 @@ export class Firestore implements firestore.FirebaseFirestore, FirebaseService { return deferred.promise; } + /** + * Shuts down this Firestore instance. + * + * After shutdown only the `clearPersistence()` method may be used. Any other method + * will throw a `FirestoreError`. + * + * To restart after shutdown, simply create a new instance of FirebaseFirestore with + * `firebase.firestore()`. + * + * Shutdown does not cancel any pending writes and any promises that are awaiting a response + * from the server will not be resolved. If you have persistence enabled, the next time you + * start this instance, it will resume attempting to send these writes to the server. + * + * Note: Under normal circumstances, calling `shutdown()` is not required. This + * method is useful only when you want to force this instance to release all of its resources or + * in combination with `clearPersistence()` to ensure that all local state is destroyed + * between test runs. + * + * @return A promise that is resolved when the instance has been successfully shut down. + */ + // TODO(b/135755126): make this public. + _shutdown(): Promise { + (this.app as _FirebaseApp)._removeServiceInstance( + 'firestore' + ); + return this.INTERNAL.delete(); + } + + get _isShutdown(): boolean { + this.ensureClientConfigured(); + return this._firestoreClient!.clientShutdown; + } + ensureClientConfigured(): FirestoreClient { if (!this._firestoreClient) { // Kick off starting the client but don't actually wait for it. @@ -538,44 +571,11 @@ export class Firestore implements firestore.FirebaseFirestore, FirebaseService { } INTERNAL = { - // TODO(b/135755126): Make this public delete: async (): Promise => { // The client must be initalized to ensure that all subsequent API usage // throws an exception. this.ensureClientConfigured(); await this._firestoreClient!.shutdown(); - }, - - /** - * Shuts down this Firestore instance. - * - * After shutdown only the `clearPersistence()` method may be used. Any other method - * will throw an `FirestoreError`. - * - * To restart after shutdown, simply create a new instance of FirebaseFirestore with - * `Firebase.firestore()`. - * - * Shutdown does not cancel any pending writes and any promises that are awaiting a response - * from the server will not be resolved. The next time you start this instance, - * it will resume attempting to send these writes to the server. - * - * Note: Under normal circumstances, calling `shutdown()` is not required. This - * method is useful only when you want to force this instance to release all of its resources or - * in combination with `clearPersistence()` to ensure that all local state is destroyed - * between test runs. - * - * @return A promise that is resolved when the instance has been successfully shut down. - */ - // TODO(b/135755126): make this public. - shutdown: (): Promise => { - (this._config.firebaseApp as _FirebaseApp)._removeServiceInstance( - 'firestore' - ); - return this.INTERNAL.delete(); - }, - - isShutdown: (): boolean => { - return this._firestoreClient!.clientShutdown; } }; diff --git a/packages/firestore/src/core/firestore_client.ts b/packages/firestore/src/core/firestore_client.ts index f3dbdcd85b6..57df940ab1f 100644 --- a/packages/firestore/src/core/firestore_client.ts +++ b/packages/firestore/src/core/firestore_client.ts @@ -506,7 +506,7 @@ export class FirestoreClient { } shutdown(): Promise { - return this.asyncQueue.enqueueAndInitilizeShutdown(async () => { + return this.asyncQueue.enqueueAndInitiateShutdown(async () => { // PORTING NOTE: LocalStore does not need an explicit shutdown on web. if (this.lruScheduler) { this.lruScheduler.stop(); diff --git a/packages/firestore/src/util/async_queue.ts b/packages/firestore/src/util/async_queue.ts index 75ed7c8acc0..424ca0d9155 100644 --- a/packages/firestore/src/util/async_queue.ts +++ b/packages/firestore/src/util/async_queue.ts @@ -226,7 +226,6 @@ export class AsyncQueue { op: () => Promise ): void { this.verifyNotFailed(); - // tslint:disable-next-line:no-floating-promises this.enqueueInternal(op); } @@ -238,7 +237,6 @@ export class AsyncQueue { op: () => Promise ): Promise { this.verifyNotFailed(); - // tslint:disable-next-line:no-floating-promises return this.enqueueInternal(op); } @@ -249,18 +247,12 @@ export class AsyncQueue { * Once this method is called, the only possible way to request running an operation * is through `enqueueAndForgetEvenAfterShutdown`. */ - enqueueAndInitilizeShutdown( - op: () => Promise - ): Promise { + async enqueueAndInitiateShutdown(op: () => Promise): Promise { this.verifyNotFailed(); - if (this._isShuttingDown) { - // Return a Promise resolves right away if it is already shutdown. - return new Promise(resolve => resolve(undefined)); + if (!this._isShuttingDown) { + this._isShuttingDown = true; + await this.enqueueEvenAfterShutdown(op); } - - const promise = this.enqueueInternal(op); - this._isShuttingDown = true; - return promise; } /** diff --git a/packages/firestore/test/integration/api/database.test.ts b/packages/firestore/test/integration/api/database.test.ts index 9f64447078b..08d7dd4f54c 100644 --- a/packages/firestore/test/integration/api/database.test.ts +++ b/packages/firestore/test/integration/api/database.test.ts @@ -1073,7 +1073,7 @@ apiDescribe('Database', (persistence: boolean) => { it('can start a new instance after shut down', async () => { return withTestDoc(persistence, async docRef => { const firestore = docRef.firestore; - await firestore.INTERNAL.shutdown(); + await (firestore as any)._shutdown(); const newFirestore = firebase.firestore!(firestore.app); expect(newFirestore).to.not.equal(firestore); @@ -1092,14 +1092,14 @@ apiDescribe('Database', (persistence: boolean) => { const app = docRef.firestore.app; await app.delete(); - expect(docRef.firestore.INTERNAL.isShutdown()).to.be.true; + expect((docRef.firestore as any)._isShutdown).to.be.true; }); }); it('new operation after shutdown should throw', async () => { await withTestDoc(persistence, async docRef => { const firestore = docRef.firestore; - await firestore.INTERNAL.shutdown(); + await (firestore as any)._shutdown(); expect(() => { firestore.doc(docRef.path).set({ foo: 'bar' }); @@ -1110,8 +1110,8 @@ apiDescribe('Database', (persistence: boolean) => { it('calling shutdown mutiple times should proceed', async () => { await withTestDoc(persistence, async docRef => { const firestore = docRef.firestore; - await firestore.INTERNAL.shutdown(); - await firestore.INTERNAL.shutdown(); + await (firestore as any)._shutdown(); + await (firestore as any)._shutdown(); expect(() => { firestore.doc(docRef.path).set({ foo: 'bar' }); diff --git a/packages/firestore/test/unit/util/async_queue.test.ts b/packages/firestore/test/unit/util/async_queue.test.ts index bbf18694eea..265d37caa10 100644 --- a/packages/firestore/test/unit/util/async_queue.test.ts +++ b/packages/firestore/test/unit/util/async_queue.test.ts @@ -213,10 +213,9 @@ describe('AsyncQueue', () => { it('Schedules operaions with respect to shut down', async () => { const queue = new AsyncQueue(); const completedSteps: number[] = []; - const doStep = (n: number): Promise => + const doStep = (n: number): Promise => defer(() => { - const result = completedSteps.push(n); - return result; + completedSteps.push(n); }); queue.enqueueAndForget(() => doStep(1)); @@ -224,7 +223,7 @@ describe('AsyncQueue', () => { // After this call, only operations requested via // `enqueueAndForgetEvenAfterShutdown` gets executed. // tslint:disable-next-line:no-floating-promises - queue.enqueueAndInitilizeShutdown(() => doStep(2)); + queue.enqueueAndInitiateShutdown(() => doStep(2)); queue.enqueueAndForget(() => doStep(3)); queue.enqueueAndForgetEvenAfterShutdown(() => doStep(4)); From 45c65a11ece180247e6ad999045069f2b4ba315c Mon Sep 17 00:00:00 2001 From: Hui Wu Date: Mon, 5 Aug 2019 21:58:59 -0400 Subject: [PATCH 09/10] [AUTOMATED]: Prettier Code Styling --- packages/firestore/src/api/database.ts | 58 +++++++++++++------------- 1 file changed, 28 insertions(+), 30 deletions(-) diff --git a/packages/firestore/src/api/database.ts b/packages/firestore/src/api/database.ts index dbfe1fe7880..fde7d73e6cc 100644 --- a/packages/firestore/src/api/database.ts +++ b/packages/firestore/src/api/database.ts @@ -448,37 +448,35 @@ export class Firestore implements firestore.FirebaseFirestore, FirebaseService { } /** - * Shuts down this Firestore instance. - * - * After shutdown only the `clearPersistence()` method may be used. Any other method - * will throw a `FirestoreError`. - * - * To restart after shutdown, simply create a new instance of FirebaseFirestore with - * `firebase.firestore()`. - * - * Shutdown does not cancel any pending writes and any promises that are awaiting a response - * from the server will not be resolved. If you have persistence enabled, the next time you - * start this instance, it will resume attempting to send these writes to the server. - * - * Note: Under normal circumstances, calling `shutdown()` is not required. This - * method is useful only when you want to force this instance to release all of its resources or - * in combination with `clearPersistence()` to ensure that all local state is destroyed - * between test runs. - * - * @return A promise that is resolved when the instance has been successfully shut down. - */ - // TODO(b/135755126): make this public. - _shutdown(): Promise { - (this.app as _FirebaseApp)._removeServiceInstance( - 'firestore' - ); - return this.INTERNAL.delete(); - } + * Shuts down this Firestore instance. + * + * After shutdown only the `clearPersistence()` method may be used. Any other method + * will throw a `FirestoreError`. + * + * To restart after shutdown, simply create a new instance of FirebaseFirestore with + * `firebase.firestore()`. + * + * Shutdown does not cancel any pending writes and any promises that are awaiting a response + * from the server will not be resolved. If you have persistence enabled, the next time you + * start this instance, it will resume attempting to send these writes to the server. + * + * Note: Under normal circumstances, calling `shutdown()` is not required. This + * method is useful only when you want to force this instance to release all of its resources or + * in combination with `clearPersistence()` to ensure that all local state is destroyed + * between test runs. + * + * @return A promise that is resolved when the instance has been successfully shut down. + */ + // TODO(b/135755126): make this public. + _shutdown(): Promise { + (this.app as _FirebaseApp)._removeServiceInstance('firestore'); + return this.INTERNAL.delete(); + } - get _isShutdown(): boolean { - this.ensureClientConfigured(); - return this._firestoreClient!.clientShutdown; - } + get _isShutdown(): boolean { + this.ensureClientConfigured(); + return this._firestoreClient!.clientShutdown; + } ensureClientConfigured(): FirestoreClient { if (!this._firestoreClient) { From 59ea6b47133fca57cca7526e8ac0db4f4f15b3e4 Mon Sep 17 00:00:00 2001 From: Hui Wu Date: Tue, 6 Aug 2019 15:20:33 -0400 Subject: [PATCH 10/10] address comments #2 --- packages/firestore/src/util/async_queue.ts | 1 + .../firestore/test/integration/api/database.test.ts | 10 ++++++---- packages/firestore/test/integration/util/helpers.ts | 5 +++++ 3 files changed, 12 insertions(+), 4 deletions(-) diff --git a/packages/firestore/src/util/async_queue.ts b/packages/firestore/src/util/async_queue.ts index 424ca0d9155..680807e58be 100644 --- a/packages/firestore/src/util/async_queue.ts +++ b/packages/firestore/src/util/async_queue.ts @@ -226,6 +226,7 @@ export class AsyncQueue { op: () => Promise ): void { this.verifyNotFailed(); + // tslint:disable-next-line:no-floating-promises this.enqueueInternal(op); } diff --git a/packages/firestore/test/integration/api/database.test.ts b/packages/firestore/test/integration/api/database.test.ts index 08d7dd4f54c..651514d7cdb 100644 --- a/packages/firestore/test/integration/api/database.test.ts +++ b/packages/firestore/test/integration/api/database.test.ts @@ -31,6 +31,7 @@ import { apiDescribe, arrayContainsAnyOp, inOp, + shutdownDb, withTestCollection, withTestDb, withTestDbs, @@ -1073,7 +1074,7 @@ apiDescribe('Database', (persistence: boolean) => { it('can start a new instance after shut down', async () => { return withTestDoc(persistence, async docRef => { const firestore = docRef.firestore; - await (firestore as any)._shutdown(); + await shutdownDb(firestore); const newFirestore = firebase.firestore!(firestore.app); expect(newFirestore).to.not.equal(firestore); @@ -1092,6 +1093,7 @@ apiDescribe('Database', (persistence: boolean) => { const app = docRef.firestore.app; await app.delete(); + // eslint-disable-next-line @typescript-eslint/no-explicit-any expect((docRef.firestore as any)._isShutdown).to.be.true; }); }); @@ -1099,7 +1101,7 @@ apiDescribe('Database', (persistence: boolean) => { it('new operation after shutdown should throw', async () => { await withTestDoc(persistence, async docRef => { const firestore = docRef.firestore; - await (firestore as any)._shutdown(); + await shutdownDb(firestore); expect(() => { firestore.doc(docRef.path).set({ foo: 'bar' }); @@ -1110,8 +1112,8 @@ apiDescribe('Database', (persistence: boolean) => { it('calling shutdown mutiple times should proceed', async () => { await withTestDoc(persistence, async docRef => { const firestore = docRef.firestore; - await (firestore as any)._shutdown(); - await (firestore as any)._shutdown(); + await shutdownDb(firestore); + await shutdownDb(firestore); expect(() => { firestore.doc(docRef.path).set({ foo: 'bar' }); diff --git a/packages/firestore/test/integration/util/helpers.ts b/packages/firestore/test/integration/util/helpers.ts index dab836d6b28..786412e57d4 100644 --- a/packages/firestore/test/integration/util/helpers.ts +++ b/packages/firestore/test/integration/util/helpers.ts @@ -303,6 +303,11 @@ function wipeDb(db: firestore.FirebaseFirestore): Promise { return Promise.resolve(undefined); } +export function shutdownDb(db: firestore.FirebaseFirestore): Promise { + // eslint-disable-next-line @typescript-eslint/no-explicit-any + return (db as any)._shutdown(); +} + // TODO(in-queries): This exists just so we don't have to do the cast // repeatedly. Once we expose 'array-contains-any' publicly we can remove it and // just use 'array-contains-any' in all the tests.