diff --git a/packages/firestore/src/api/database.ts b/packages/firestore/src/api/database.ts index 161435d773c..fde7d73e6cc 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 { @@ -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 && @@ -447,6 +447,37 @@ 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. diff --git a/packages/firestore/src/core/firestore_client.ts b/packages/firestore/src/core/firestore_client.ts index 89a818535ce..57df940ab1f 100644 --- a/packages/firestore/src/core/firestore_client.ts +++ b/packages/firestore/src/core/firestore_client.ts @@ -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; @@ -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.enqueueAndInitiateShutdown(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(); }); } @@ -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( diff --git a/packages/firestore/src/util/async_queue.ts b/packages/firestore/src/util/async_queue.ts index 5483100b893..680807e58be 100644 --- a/packages/firestore/src/util/async_queue.ts +++ b/packages/firestore/src/util/async_queue.ts @@ -188,6 +188,10 @@ export class AsyncQueue { // The last promise in the queue. private tail: Promise = Promise.resolve(); + // Is this AsyncQueue being shut 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,12 @@ export class AsyncQueue { // assertion sanity-checks. private operationInProgress = false; + // 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; + } + /** * Adds a new operation to the queue without waiting for it to complete (i.e. * we ignore the Promise result). @@ -208,12 +218,58 @@ 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(); + 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`. + */ + async enqueueAndInitiateShutdown(op: () => Promise): Promise { + this.verifyNotFailed(); + if (!this._isShuttingDown) { + this._isShuttingDown = true; + await this.enqueueEvenAfterShutdown(op); + } + } + /** * 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() @@ -309,7 +365,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()); } /** diff --git a/packages/firestore/test/integration/api/database.test.ts b/packages/firestore/test/integration/api/database.test.ts index dd1b8d276a9..651514d7cdb 100644 --- a/packages/firestore/test/integration/api/database.test.ts +++ b/packages/firestore/test/integration/api/database.test.ts @@ -31,11 +31,13 @@ import { apiDescribe, arrayContainsAnyOp, inOp, + shutdownDb, withTestCollection, withTestDb, withTestDbs, withTestDoc, - withTestDocAndInitialData + withTestDocAndInitialData, + DEFAULT_SETTINGS } from '../util/helpers'; // tslint:disable:no-floating-promises @@ -1068,4 +1070,54 @@ 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 shutdownDb(firestore); + + 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' }); + }); + }); + + 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(); + + // eslint-disable-next-line @typescript-eslint/no-explicit-any + 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 shutdownDb(firestore); + + 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 shutdownDb(firestore); + await shutdownDb(firestore); + + expect(() => { + firestore.doc(docRef.path).set({ foo: 'bar' }); + }).to.throw(); + }); + }); }); 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. diff --git a/packages/firestore/test/unit/util/async_queue.test.ts b/packages/firestore/test/unit/util/async_queue.test.ts index 281ff463447..265d37caa10 100644 --- a/packages/firestore/test/unit/util/async_queue.test.ts +++ b/packages/firestore/test/unit/util/async_queue.test.ts @@ -209,6 +209,27 @@ 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(() => { + completedSteps.push(n); + }); + + queue.enqueueAndForget(() => doStep(1)); + + // After this call, only operations requested via + // `enqueueAndForgetEvenAfterShutdown` gets executed. + // tslint:disable-next-line:no-floating-promises + queue.enqueueAndInitiateShutdown(() => 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 {