Skip to content

rename shutdown to terminate and publish it #2116

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 10 commits into from
Aug 27, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 22 additions & 0 deletions packages/firebase/index.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6238,6 +6238,28 @@ declare namespace firebase.firestore {
*/
waitForPendingWrites(): Promise<void>;

/**
* Terminates this Firestore instance.
*
* After calling `terminate()` only the `clearPersistence()` method may be used. Any other method
* will throw a `FirestoreError`.
*
* To restart after termination, create a new instance of FirebaseFirestore with
* `firebase.firestore()`.
*
* Termination 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 sending these writes to the server.
*
* Note: Under normal circumstances, calling `terminate()` 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 terminated.
*/
terminate(): Promise<void>;

/**
* @hidden
*/
Expand Down
22 changes: 22 additions & 0 deletions packages/firestore-types/index.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -292,6 +292,28 @@ export class FirebaseFirestore {
*/
waitForPendingWrites(): Promise<void>;

/**
* Terminates this Firestore instance.
*
* After calling `terminate()` only the `clearPersistence()` method may be used. Any other method
* will throw a `FirestoreError`.
*
* To restart after termination, create a new instance of FirebaseFirestore with
* `firebase.firestore()`.
*
* Termination 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 sending these writes to the server.
*
* Note: Under normal circumstances, calling `terminate()` 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 terminated.
*/
terminate(): Promise<void>;

INTERNAL: { delete: () => Promise<void> };
}

Expand Down
4 changes: 4 additions & 0 deletions packages/firestore/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,10 @@
- [feature] Added a `Firestore.waitForPendingWrites()` method that
allows users to wait until all pending writes are acknowledged by the
Firestore backend.
- [feature] Added a `Firestore.terminate()` method which terminates
the instance, releasing any held resources. Once it completes, you can
optionally call `Firestore.clearPersistence()` to wipe persisted Firestore
data from disk.

# 1.4.10
- [changed] Transactions now perform exponential backoff before retrying.
Expand Down
31 changes: 5 additions & 26 deletions packages/firestore/src/api/database.ts
Original file line number Diff line number Diff line change
Expand Up @@ -421,7 +421,7 @@ export class Firestore implements firestore.FirebaseFirestore, FirebaseService {
try {
if (
this._firestoreClient !== undefined &&
!this._firestoreClient.clientShutdown
!this._firestoreClient.clientTerminated
) {
throw new FirestoreError(
Code.FAILED_PRECONDITION,
Expand All @@ -437,35 +437,14 @@ 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<void> {
terminate(): Promise<void> {
(this.app as _FirebaseApp)._removeServiceInstance('firestore');
return this.INTERNAL.delete();
}

get _isShutdown(): boolean {
get _isTerminated(): boolean {
this.ensureClientConfigured();
return this._firestoreClient!.clientShutdown;
return this._firestoreClient!.clientTerminated;
}

waitForPendingWrites(): Promise<void> {
Expand Down Expand Up @@ -567,7 +546,7 @@ export class Firestore implements firestore.FirebaseFirestore, FirebaseService {
// The client must be initalized to ensure that all subsequent API usage
// throws an exception.
this.ensureClientConfigured();
await this._firestoreClient!.shutdown();
await this._firestoreClient!.terminate();
}
};

Expand Down
46 changes: 23 additions & 23 deletions packages/firestore/src/core/firestore_client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,7 @@ export class FirestoreClient {
* unconditionally resolved.
*/
start(persistenceSettings: InternalPersistenceSettings): Promise<void> {
this.verifyNotShutdown();
this.verifyNotTerminated();
// We defer our initialization until we get the current user from
// setChangeListener(). We block the async queue until we got the initial
// user and the initialization is completed. This will prevent any scheduled
Expand Down Expand Up @@ -208,7 +208,7 @@ export class FirestoreClient {

/** Enables the network connection and requeues all pending operations. */
enableNetwork(): Promise<void> {
this.verifyNotShutdown();
this.verifyNotTerminated();
return this.asyncQueue.enqueue(() => {
return this.syncEngine.enableNetwork();
});
Expand Down Expand Up @@ -305,14 +305,14 @@ export class FirestoreClient {
}

/**
* Checks that the client has not been shutdown. Ensures that other methods on
* this class cannot be called after the client is shutdown.
* Checks that the client has not been terminated. Ensures that other methods on
* this class cannot be called after the client is terminated.
*/
private verifyNotShutdown(): void {
private verifyNotTerminated(): void {
if (this.asyncQueue.isShuttingDown) {
throw new FirestoreError(
Code.FAILED_PRECONDITION,
'The client has already been shutdown.'
'The client has already been terminated.'
);
}
}
Expand Down Expand Up @@ -482,10 +482,10 @@ export class FirestoreClient {
}
});

// When a user calls clearPersistence() in one client, all other clientfs
// need to shut down to allow the delete to succeed.
// When a user calls clearPersistence() in one client, all other clients
// need to be terminated to allow the delete to succeed.
await this.persistence.setDatabaseDeletedListener(async () => {
await this.shutdown();
await this.terminate();
});
});
}
Expand All @@ -499,13 +499,13 @@ export class FirestoreClient {

/** Disables the network connection. Pending operations will not complete. */
disableNetwork(): Promise<void> {
this.verifyNotShutdown();
this.verifyNotTerminated();
return this.asyncQueue.enqueue(() => {
return this.syncEngine.disableNetwork();
});
}

shutdown(): Promise<void> {
terminate(): Promise<void> {
return this.asyncQueue.enqueueAndInitiateShutdown(async () => {
// PORTING NOTE: LocalStore does not need an explicit shutdown on web.
if (this.lruScheduler) {
Expand All @@ -528,7 +528,7 @@ export class FirestoreClient {
* or rejection.
*/
waitForPendingWrites(): Promise<void> {
this.verifyNotShutdown();
this.verifyNotTerminated();

const deferred = new Deferred<void>();
this.asyncQueue.enqueueAndForget(() => {
Expand All @@ -542,7 +542,7 @@ export class FirestoreClient {
observer: Observer<ViewSnapshot>,
options: ListenOptions
): QueryListener {
this.verifyNotShutdown();
this.verifyNotTerminated();
const listener = new QueryListener(query, observer, options);
this.asyncQueue.enqueueAndForget(() => {
return this.eventMgr.listen(listener);
Expand All @@ -551,9 +551,9 @@ export class FirestoreClient {
}

unlisten(listener: QueryListener): void {
// Checks for shutdown but does not raise error, allowing unlisten after
// shutdown to be a no-op.
if (this.clientShutdown) {
// Checks for termination but does not raise error, allowing unlisten after
// termination to be a no-op.
if (this.clientTerminated) {
return;
}
this.asyncQueue.enqueueAndForget(() => {
Expand All @@ -562,7 +562,7 @@ export class FirestoreClient {
}

getDocumentFromLocalCache(docKey: DocumentKey): Promise<Document | null> {
this.verifyNotShutdown();
this.verifyNotTerminated();
return this.asyncQueue
.enqueue(() => {
return this.localStore.readDocument(docKey);
Expand All @@ -585,7 +585,7 @@ export class FirestoreClient {
}

getDocumentsFromLocalCache(query: Query): Promise<ViewSnapshot> {
this.verifyNotShutdown();
this.verifyNotTerminated();
return this.asyncQueue
.enqueue(() => {
return this.localStore.executeQuery(query);
Expand All @@ -605,7 +605,7 @@ export class FirestoreClient {
}

write(mutations: Mutation[]): Promise<void> {
this.verifyNotShutdown();
this.verifyNotTerminated();
const deferred = new Deferred<void>();
this.asyncQueue.enqueueAndForget(() =>
this.syncEngine.write(mutations, deferred)
Expand All @@ -617,17 +617,17 @@ export class FirestoreClient {
return this.databaseInfo.databaseId;
}

get clientShutdown(): boolean {
get clientTerminated(): boolean {
// 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.
// related to termination or supposed to be run after termination. It is effectively
// terminated to the eyes of users.
return this.asyncQueue.isShuttingDown;
}

transaction<T>(
updateFunction: (transaction: Transaction) => Promise<T>
): Promise<T> {
this.verifyNotShutdown();
this.verifyNotTerminated();
const deferred = new Deferred<T>();
this.asyncQueue.enqueueAndForget(() => {
this.syncEngine.runTransaction(this.asyncQueue, updateFunction, deferred);
Expand Down
27 changes: 13 additions & 14 deletions packages/firestore/test/integration/api/database.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@ import {
apiDescribe,
arrayContainsAnyOp,
inOp,
shutdownDb,
withTestCollection,
withTestDb,
withTestDbs,
Expand Down Expand Up @@ -956,12 +955,12 @@ apiDescribe('Database', (persistence: boolean) => {
});
});

it('rejects subsequent method calls after shutdown() is called', async () => {
it('rejects subsequent method calls after terminate() is called', async () => {
return withTestDb(persistence, db => {
return db.INTERNAL.delete().then(() => {
expect(() => {
db.disableNetwork();
}).to.throw('The client has already been shutdown.');
}).to.throw('The client has already been terminated.');
});
});
});
Expand Down Expand Up @@ -1076,7 +1075,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 shutdownDb(firestore);
await firestore.terminate();

const newFirestore = firebase.firestore!(firestore.app);
expect(newFirestore).to.not.equal(firestore);
Expand All @@ -1089,47 +1088,47 @@ apiDescribe('Database', (persistence: boolean) => {
});
});

it('app delete leads to instance shutdown', async () => {
it('app delete leads to instance termination', 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;
expect((docRef.firestore as any)._isTerminated).to.be.true;
});
});

it('new operation after shutdown should throw', async () => {
it('new operation after termination should throw', async () => {
await withTestDoc(persistence, async docRef => {
const firestore = docRef.firestore;
await shutdownDb(firestore);
await firestore.terminate();

expect(() => {
firestore.doc(docRef.path).set({ foo: 'bar' });
}).to.throw();
}).to.throw('The client has already been terminated.');
});
});

it('calling shutdown mutiple times should proceed', async () => {
it('calling terminate mutiple times should proceed', async () => {
await withTestDoc(persistence, async docRef => {
const firestore = docRef.firestore;
await shutdownDb(firestore);
await shutdownDb(firestore);
await firestore.terminate();
await firestore.terminate();

expect(() => {
firestore.doc(docRef.path).set({ foo: 'bar' });
}).to.throw();
});
});

it('can unlisten queries after shutdown', async () => {
it('can unlisten queries after termination', async () => {
return withTestDoc(persistence, async docRef => {
const firestore = docRef.firestore;
const accumulator = new EventsAccumulator<firestore.DocumentSnapshot>();
const unsubscribe = docRef.onSnapshot(accumulator.storeEvent);
await accumulator.awaitEvent();
await shutdownDb(firestore);
await firestore.terminate();

// This should proceed without error.
unsubscribe();
Expand Down
5 changes: 0 additions & 5 deletions packages/firestore/test/integration/util/helpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -358,11 +358,6 @@ function wipeDb(db: firestore.FirebaseFirestore): Promise<void> {
return Promise.resolve(undefined);
}

export function shutdownDb(db: firestore.FirebaseFirestore): Promise<void> {
// 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.
Expand Down