Skip to content

Commit 80d0846

Browse files
author
Hui-Wu
authored
port public shutdown to web sdk. (#2045)
* pending firebaseApp change * [AUTOMATED]: Prettier Code Styling * Looks to be working. * browser passes/node fails * [AUTOMATED]: Prettier Code Styling * add settings for new instance * final self review. * addressing comments #1 * [AUTOMATED]: Prettier Code Styling * address comments #2
1 parent 7a2403a commit 80d0846

File tree

6 files changed

+187
-22
lines changed

6 files changed

+187
-22
lines changed

packages/firestore/src/api/database.ts

Lines changed: 33 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@
1818
import * as firestore from '@firebase/firestore-types';
1919

2020
import { FirebaseApp } from '@firebase/app-types';
21-
import { FirebaseService } from '@firebase/app-types/private';
21+
import { FirebaseService, _FirebaseApp } from '@firebase/app-types/private';
2222
import { DatabaseId, DatabaseInfo } from '../core/database_info';
2323
import { ListenOptions } from '../core/event_manager';
2424
import {
@@ -427,7 +427,7 @@ export class Firestore implements firestore.FirebaseFirestore, FirebaseService {
427427
this.makeDatabaseInfo()
428428
);
429429
const deferred = new Deferred<void>();
430-
this._queue.enqueueAndForget(async () => {
430+
this._queue.enqueueAndForgetEvenAfterShutdown(async () => {
431431
try {
432432
if (
433433
this._firestoreClient !== undefined &&
@@ -447,6 +447,37 @@ export class Firestore implements firestore.FirebaseFirestore, FirebaseService {
447447
return deferred.promise;
448448
}
449449

450+
/**
451+
* Shuts down this Firestore instance.
452+
*
453+
* After shutdown only the `clearPersistence()` method may be used. Any other method
454+
* will throw a `FirestoreError`.
455+
*
456+
* To restart after shutdown, simply create a new instance of FirebaseFirestore with
457+
* `firebase.firestore()`.
458+
*
459+
* Shutdown does not cancel any pending writes and any promises that are awaiting a response
460+
* from the server will not be resolved. If you have persistence enabled, the next time you
461+
* start this instance, it will resume attempting to send these writes to the server.
462+
*
463+
* Note: Under normal circumstances, calling `shutdown()` is not required. This
464+
* method is useful only when you want to force this instance to release all of its resources or
465+
* in combination with `clearPersistence()` to ensure that all local state is destroyed
466+
* between test runs.
467+
*
468+
* @return A promise that is resolved when the instance has been successfully shut down.
469+
*/
470+
// TODO(b/135755126): make this public.
471+
_shutdown(): Promise<void> {
472+
(this.app as _FirebaseApp)._removeServiceInstance('firestore');
473+
return this.INTERNAL.delete();
474+
}
475+
476+
get _isShutdown(): boolean {
477+
this.ensureClientConfigured();
478+
return this._firestoreClient!.clientShutdown;
479+
}
480+
450481
ensureClientConfigured(): FirestoreClient {
451482
if (!this._firestoreClient) {
452483
// Kick off starting the client but don't actually wait for it.

packages/firestore/src/core/firestore_client.ts

Lines changed: 17 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -107,7 +107,6 @@ export class FirestoreClient {
107107
private lruScheduler?: LruScheduler;
108108

109109
private readonly clientId = AutoId.newId();
110-
private _clientShutdown = false;
111110

112111
// PORTING NOTE: SharedClientState is only used for multi-tab web.
113112
private sharedClientState: SharedClientState;
@@ -310,7 +309,7 @@ export class FirestoreClient {
310309
* this class cannot be called after the client is shutdown.
311310
*/
312311
private verifyNotShutdown(): void {
313-
if (this._clientShutdown) {
312+
if (this.asyncQueue.isShuttingDown) {
314313
throw new FirestoreError(
315314
Code.FAILED_PRECONDITION,
316315
'The client has already been shutdown.'
@@ -507,22 +506,19 @@ export class FirestoreClient {
507506
}
508507

509508
shutdown(): Promise<void> {
510-
return this.asyncQueue.enqueue(async () => {
511-
if (!this._clientShutdown) {
512-
// PORTING NOTE: LocalStore does not need an explicit shutdown on web.
513-
if (this.lruScheduler) {
514-
this.lruScheduler.stop();
515-
}
516-
await this.remoteStore.shutdown();
517-
await this.sharedClientState.shutdown();
518-
await this.persistence.shutdown();
519-
520-
// `removeChangeListener` must be called after shutting down the
521-
// RemoteStore as it will prevent the RemoteStore from retrieving
522-
// auth tokens.
523-
this.credentials.removeChangeListener();
524-
this._clientShutdown = true;
509+
return this.asyncQueue.enqueueAndInitiateShutdown(async () => {
510+
// PORTING NOTE: LocalStore does not need an explicit shutdown on web.
511+
if (this.lruScheduler) {
512+
this.lruScheduler.stop();
525513
}
514+
await this.remoteStore.shutdown();
515+
await this.sharedClientState.shutdown();
516+
await this.persistence.shutdown();
517+
518+
// `removeChangeListener` must be called after shutting down the
519+
// RemoteStore as it will prevent the RemoteStore from retrieving
520+
// auth tokens.
521+
this.credentials.removeChangeListener();
526522
});
527523
}
528524

@@ -603,7 +599,10 @@ export class FirestoreClient {
603599
}
604600

605601
get clientShutdown(): boolean {
606-
return this._clientShutdown;
602+
// Technically, the asyncQueue is still running, but only accepting operations
603+
// related to shutdown or supposed to be run after shutdown. It is effectively
604+
// shut down to the eyes of users.
605+
return this.asyncQueue.isShuttingDown;
607606
}
608607

609608
transaction<T>(

packages/firestore/src/util/async_queue.ts

Lines changed: 58 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -188,6 +188,10 @@ export class AsyncQueue {
188188
// The last promise in the queue.
189189
private tail: Promise<unknown> = Promise.resolve();
190190

191+
// Is this AsyncQueue being shut down? Once it is set to true, it will not
192+
// be changed again.
193+
private _isShuttingDown: boolean = false;
194+
191195
// Operations scheduled to be queued in the future. Operations are
192196
// automatically removed after they are run or canceled.
193197
private delayedOperations: Array<DelayedOperation<unknown>> = [];
@@ -199,6 +203,12 @@ export class AsyncQueue {
199203
// assertion sanity-checks.
200204
private operationInProgress = false;
201205

206+
// Is this AsyncQueue being shut down? If true, this instance will not enqueue
207+
// any new operations, Promises from enqueue requests will not resolve.
208+
get isShuttingDown(): boolean {
209+
return this._isShuttingDown;
210+
}
211+
202212
/**
203213
* Adds a new operation to the queue without waiting for it to complete (i.e.
204214
* we ignore the Promise result).
@@ -208,12 +218,58 @@ export class AsyncQueue {
208218
this.enqueue(op);
209219
}
210220

221+
/**
222+
* Regardless if the queue has initialized shutdown, adds a new operation to the
223+
* queue without waiting for it to complete (i.e. we ignore the Promise result).
224+
*/
225+
enqueueAndForgetEvenAfterShutdown<T extends unknown>(
226+
op: () => Promise<T>
227+
): void {
228+
this.verifyNotFailed();
229+
// tslint:disable-next-line:no-floating-promises
230+
this.enqueueInternal(op);
231+
}
232+
233+
/**
234+
* Regardless if the queue has initialized shutdown, adds a new operation to the
235+
* queue.
236+
*/
237+
private enqueueEvenAfterShutdown<T extends unknown>(
238+
op: () => Promise<T>
239+
): Promise<T> {
240+
this.verifyNotFailed();
241+
return this.enqueueInternal(op);
242+
}
243+
244+
/**
245+
* Adds a new operation to the queue and initialize the shut down of this queue.
246+
* Returns a promise that will be resolved when the promise returned by the new
247+
* operation is (with its value).
248+
* Once this method is called, the only possible way to request running an operation
249+
* is through `enqueueAndForgetEvenAfterShutdown`.
250+
*/
251+
async enqueueAndInitiateShutdown(op: () => Promise<void>): Promise<void> {
252+
this.verifyNotFailed();
253+
if (!this._isShuttingDown) {
254+
this._isShuttingDown = true;
255+
await this.enqueueEvenAfterShutdown(op);
256+
}
257+
}
258+
211259
/**
212260
* Adds a new operation to the queue. Returns a promise that will be resolved
213261
* when the promise returned by the new operation is (with its value).
214262
*/
215263
enqueue<T extends unknown>(op: () => Promise<T>): Promise<T> {
216264
this.verifyNotFailed();
265+
if (this._isShuttingDown) {
266+
// Return a Promise which never resolves.
267+
return new Promise<T>(resolve => {});
268+
}
269+
return this.enqueueInternal(op);
270+
}
271+
272+
private enqueueInternal<T extends unknown>(op: () => Promise<T>): Promise<T> {
217273
const newTail = this.tail.then(() => {
218274
this.operationInProgress = true;
219275
return op()
@@ -309,7 +365,8 @@ export class AsyncQueue {
309365
* operations are not run.
310366
*/
311367
drain(): Promise<void> {
312-
return this.enqueue(() => Promise.resolve());
368+
// It should still be possible to drain the queue after it is shutting down.
369+
return this.enqueueEvenAfterShutdown(() => Promise.resolve());
313370
}
314371

315372
/**

packages/firestore/test/integration/api/database.test.ts

Lines changed: 53 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,11 +31,13 @@ import {
3131
apiDescribe,
3232
arrayContainsAnyOp,
3333
inOp,
34+
shutdownDb,
3435
withTestCollection,
3536
withTestDb,
3637
withTestDbs,
3738
withTestDoc,
38-
withTestDocAndInitialData
39+
withTestDocAndInitialData,
40+
DEFAULT_SETTINGS
3941
} from '../util/helpers';
4042

4143
// tslint:disable:no-floating-promises
@@ -1068,4 +1070,54 @@ apiDescribe('Database', (persistence: boolean) => {
10681070
await db.enableNetwork();
10691071
});
10701072
});
1073+
1074+
it('can start a new instance after shut down', async () => {
1075+
return withTestDoc(persistence, async docRef => {
1076+
const firestore = docRef.firestore;
1077+
await shutdownDb(firestore);
1078+
1079+
const newFirestore = firebase.firestore!(firestore.app);
1080+
expect(newFirestore).to.not.equal(firestore);
1081+
1082+
// New instance functions.
1083+
newFirestore.settings(DEFAULT_SETTINGS);
1084+
await newFirestore.doc(docRef.path).set({ foo: 'bar' });
1085+
const doc = await newFirestore.doc(docRef.path).get();
1086+
expect(doc.data()).to.deep.equal({ foo: 'bar' });
1087+
});
1088+
});
1089+
1090+
it('app delete leads to instance shutdown', async () => {
1091+
await withTestDoc(persistence, async docRef => {
1092+
await docRef.set({ foo: 'bar' });
1093+
const app = docRef.firestore.app;
1094+
await app.delete();
1095+
1096+
// eslint-disable-next-line @typescript-eslint/no-explicit-any
1097+
expect((docRef.firestore as any)._isShutdown).to.be.true;
1098+
});
1099+
});
1100+
1101+
it('new operation after shutdown should throw', async () => {
1102+
await withTestDoc(persistence, async docRef => {
1103+
const firestore = docRef.firestore;
1104+
await shutdownDb(firestore);
1105+
1106+
expect(() => {
1107+
firestore.doc(docRef.path).set({ foo: 'bar' });
1108+
}).to.throw();
1109+
});
1110+
});
1111+
1112+
it('calling shutdown mutiple times should proceed', async () => {
1113+
await withTestDoc(persistence, async docRef => {
1114+
const firestore = docRef.firestore;
1115+
await shutdownDb(firestore);
1116+
await shutdownDb(firestore);
1117+
1118+
expect(() => {
1119+
firestore.doc(docRef.path).set({ foo: 'bar' });
1120+
}).to.throw();
1121+
});
1122+
});
10711123
});

packages/firestore/test/integration/util/helpers.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -303,6 +303,11 @@ function wipeDb(db: firestore.FirebaseFirestore): Promise<void> {
303303
return Promise.resolve(undefined);
304304
}
305305

306+
export function shutdownDb(db: firestore.FirebaseFirestore): Promise<void> {
307+
// eslint-disable-next-line @typescript-eslint/no-explicit-any
308+
return (db as any)._shutdown();
309+
}
310+
306311
// TODO(in-queries): This exists just so we don't have to do the cast
307312
// repeatedly. Once we expose 'array-contains-any' publicly we can remove it and
308313
// just use 'array-contains-any' in all the tests.

packages/firestore/test/unit/util/async_queue.test.ts

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -209,6 +209,27 @@ describe('AsyncQueue', () => {
209209
await queue.drain();
210210
expect(completedSteps).to.deep.equal([1, 2]);
211211
});
212+
213+
it('Schedules operaions with respect to shut down', async () => {
214+
const queue = new AsyncQueue();
215+
const completedSteps: number[] = [];
216+
const doStep = (n: number): Promise<void> =>
217+
defer(() => {
218+
completedSteps.push(n);
219+
});
220+
221+
queue.enqueueAndForget(() => doStep(1));
222+
223+
// After this call, only operations requested via
224+
// `enqueueAndForgetEvenAfterShutdown` gets executed.
225+
// tslint:disable-next-line:no-floating-promises
226+
queue.enqueueAndInitiateShutdown(() => doStep(2));
227+
queue.enqueueAndForget(() => doStep(3));
228+
queue.enqueueAndForgetEvenAfterShutdown(() => doStep(4));
229+
230+
await queue.drain();
231+
expect(completedSteps).to.deep.equal([1, 2, 4]);
232+
});
212233
});
213234

214235
function defer<T>(op: () => T): Promise<T> {

0 commit comments

Comments
 (0)