Skip to content

Commit

Permalink
Don't switch network status to UNKNOWN during user change (#3700)
Browse files Browse the repository at this point in the history
  • Loading branch information
schmidt-sebastian committed Aug 28, 2020
1 parent 0dfd516 commit 249d40c
Show file tree
Hide file tree
Showing 4 changed files with 34 additions and 8 deletions.
5 changes: 5 additions & 0 deletions .changeset/cold-impalas-drop.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@firebase/firestore": patch
---

Fixes a bug that caused the client to not raise snapshots from cache if a user change happened while the network connection was disabled.
1 change: 1 addition & 0 deletions packages/firestore/exp/src/api/components.ts
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,7 @@ export async function setOnlineComponentProvider(
// The CredentialChangeListener of the online component provider takes
// precedence over the offline component provider.
firestore._setCredentialChangeListener(user =>
// TODO(firestoreexp): This should be enqueueRetryable.
firestore._queue.enqueueAndForget(() =>
onlineComponentProvider.remoteStore.handleCredentialChange(user)
)
Expand Down
19 changes: 11 additions & 8 deletions packages/firestore/src/remote/remote_store.ts
Original file line number Diff line number Diff line change
Expand Up @@ -791,21 +791,24 @@ export class RemoteStore implements TargetMetadataProvider {

async handleCredentialChange(user: User): Promise<void> {
this.asyncQueue.verifyOperationInProgress();
debugAssert(
!!this.remoteSyncer.handleCredentialChange,
'handleCredentialChange() not set'
);

logDebug(LOG_TAG, 'RemoteStore received new credentials');
const canUseNetwork = this.canUseNetwork();

// Tear down and re-create our network streams. This will ensure we get a
// fresh auth token for the new user and re-fill the write pipeline with
// new mutations from the LocalStore (since mutations are per-user).
logDebug(LOG_TAG, 'RemoteStore received new credentials');
this.offlineCauses.add(OfflineCause.CredentialChange);

await this.disableNetworkInternal();
this.onlineStateTracker.set(OnlineState.Unknown);
debugAssert(
!!this.remoteSyncer.handleCredentialChange,
'handleCredentialChange() not set'
);
if (canUseNetwork) {
// Don't set the network status to Unknown if we are offline.
this.onlineStateTracker.set(OnlineState.Unknown);
}
await this.remoteSyncer.handleCredentialChange(user);

this.offlineCauses.delete(OfflineCause.CredentialChange);
await this.enableNetworkInternal();
}
Expand Down
17 changes: 17 additions & 0 deletions packages/firestore/test/unit/specs/offline_spec.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -239,4 +239,21 @@ describeSpec('Offline:', [], () => {
);
}
);

specTest('Client stays offline during credential change', [], () => {
// Reproduces a bug that caused the client to switch to OnlineState
// `Unknown` during a credential change.

const query1 = query('collection');
return (
spec()
.disableNetwork()
.changeUser('user1')
.userListens(query1)
// Client is still offline and we raise a `fromCache` event immediately
.expectEvents(query1, {
fromCache: true
})
);
});
});

0 comments on commit 249d40c

Please sign in to comment.