Skip to content

Commit 2907439

Browse files
authored
Allow user to unlisten to queries after shutdown. (#2083)
* Can unlisten * explicitly check for shut down * [AUTOMATED]: Prettier Code Styling * better comment. * [AUTOMATED]: Prettier Code Styling
1 parent cacb48e commit 2907439

File tree

2 files changed

+46
-25
lines changed

2 files changed

+46
-25
lines changed

packages/firestore/src/core/firestore_client.ts

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -551,7 +551,11 @@ export class FirestoreClient {
551551
}
552552

553553
unlisten(listener: QueryListener): void {
554-
this.verifyNotShutdown();
554+
// Checks for shutdown but does not raise error, allowing unlisten after
555+
// shutdown to be a no-op.
556+
if (this.clientShutdown) {
557+
return;
558+
}
555559
this.asyncQueue.enqueueAndForget(() => {
556560
return this.eventMgr.unlisten(listener);
557561
});

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

Lines changed: 41 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -746,7 +746,7 @@ apiDescribe('Database', (persistence: boolean) => {
746746
});
747747
});
748748
});
749-
return Promise.all([deferred1.promise, deferred2.promise]).then(() => { });
749+
return Promise.all([deferred1.promise, deferred2.promise]).then(() => {});
750750
});
751751
});
752752

@@ -785,7 +785,7 @@ apiDescribe('Database', (persistence: boolean) => {
785785
it('will reject listens', () => {
786786
const deferred = new Deferred();
787787
queryForRejection.onSnapshot(
788-
() => { },
788+
() => {},
789789
(err: Error) => {
790790
expect(err.name).to.exist;
791791
expect(err.message).to.exist;
@@ -798,12 +798,12 @@ apiDescribe('Database', (persistence: boolean) => {
798798
it('will reject same listens twice in a row', () => {
799799
const deferred = new Deferred();
800800
queryForRejection.onSnapshot(
801-
() => { },
801+
() => {},
802802
(err: Error) => {
803803
expect(err.name).to.exist;
804804
expect(err.message).to.exist;
805805
queryForRejection.onSnapshot(
806-
() => { },
806+
() => {},
807807
(err2: Error) => {
808808
expect(err2.name).to.exist;
809809
expect(err2.message).to.exist;
@@ -1124,6 +1124,21 @@ apiDescribe('Database', (persistence: boolean) => {
11241124
});
11251125
});
11261126

1127+
it('can unlisten queries after shutdown', async () => {
1128+
return withTestDoc(persistence, async docRef => {
1129+
const firestore = docRef.firestore;
1130+
const accumulator = new EventsAccumulator<firestore.DocumentSnapshot>();
1131+
const unsubscribe = docRef.onSnapshot(accumulator.storeEvent);
1132+
await accumulator.awaitEvent();
1133+
await shutdownDb(firestore);
1134+
1135+
// This should proceed without error.
1136+
unsubscribe();
1137+
// Multiple calls should proceed as well.
1138+
unsubscribe();
1139+
});
1140+
});
1141+
11271142
it('can wait for pending writes', async () => {
11281143
await withTestDoc(persistence, async docRef => {
11291144
const firestore = docRef.firestore;
@@ -1141,30 +1156,32 @@ apiDescribe('Database', (persistence: boolean) => {
11411156
});
11421157

11431158
it('waiting for pending writes should fail when user changes', async () => {
1144-
await withMockCredentialProviderTestDb(persistence, async (db, mockCredentialsProvider) => {
1145-
// Prevent pending writes receiving acknowledgement.
1146-
await db.disableNetwork();
1147-
db.doc('abc/123').set({ foo: 'bar' });
1148-
const awaitPendingWrite = waitForPendingWrites(db);
1159+
await withMockCredentialProviderTestDb(
1160+
persistence,
1161+
async (db, mockCredentialsProvider) => {
1162+
// Prevent pending writes receiving acknowledgement.
1163+
await db.disableNetwork();
1164+
db.doc('abc/123').set({ foo: 'bar' });
1165+
const awaitPendingWrite = waitForPendingWrites(db);
11491166

1150-
mockCredentialsProvider.triggerUserChange(new User('user_1'));
1167+
mockCredentialsProvider.triggerUserChange(new User('user_1'));
11511168

1152-
await expect(awaitPendingWrite).to.be.eventually.rejectedWith(
1153-
"'waitForPendingWrites' promise is rejected due to a user change."
1154-
);
1155-
});
1169+
await expect(awaitPendingWrite).to.be.eventually.rejectedWith(
1170+
"'waitForPendingWrites' promise is rejected due to a user change."
1171+
);
1172+
}
1173+
);
11561174
});
11571175

1158-
it('waiting for pending writes resolves immediately when offline and no pending writes',
1159-
async () => {
1160-
await withTestDoc(persistence, async docRef => {
1161-
const firestore = docRef.firestore;
1162-
// Prevent pending writes receiving acknowledgement.
1163-
await firestore.disableNetwork();
1176+
it('waiting for pending writes resolves immediately when offline and no pending writes', async () => {
1177+
await withTestDoc(persistence, async docRef => {
1178+
const firestore = docRef.firestore;
1179+
// Prevent pending writes receiving acknowledgement.
1180+
await firestore.disableNetwork();
11641181

1165-
// `awaitsPendingWrites` is created when there is no pending writes, it will resolve
1166-
// immediately even if we are offline.
1167-
await waitForPendingWrites(firestore);
1168-
});
1182+
// `awaitsPendingWrites` is created when there is no pending writes, it will resolve
1183+
// immediately even if we are offline.
1184+
await waitForPendingWrites(firestore);
11691185
});
1186+
});
11701187
});

0 commit comments

Comments
 (0)