Skip to content

Commit a13f5a0

Browse files
Defer check whether auth is available (#4845)
1 parent c34ac7a commit a13f5a0

9 files changed

+114
-82
lines changed

packages/firestore/.idea/runConfigurations/All_Tests__Emulator_.xml

+1-1
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

packages/firestore/.idea/runConfigurations/All_Tests__Emulator_w__Mock_Persistence_.xml

+1-1
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

packages/firestore/.idea/runConfigurations/Integration_Tests__Emulator_.xml

+1-1
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

packages/firestore/.idea/runConfigurations/Integration_Tests__Emulator_w__Mock_Persistence_.xml

+1-1
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

packages/firestore/.idea/runConfigurations/Unit_Tests.xml

+2-2
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

packages/firestore/.idea/runConfigurations/Unit_Tests__w__Mock_Persistence_.xml

+1-1
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

packages/firestore/src/api/credentials.ts

+87-47
Original file line numberDiff line numberDiff line change
@@ -23,8 +23,10 @@ import { Provider } from '@firebase/component';
2323

2424
import { User } from '../auth/user';
2525
import { hardAssert, debugAssert } from '../util/assert';
26+
import { AsyncQueue } from '../util/async_queue';
2627
import { Code, FirestoreError } from '../util/error';
2728
import { logDebug } from '../util/log';
29+
import { Deferred } from '../util/promise';
2830

2931
// TODO(mikelehen): This should be split into multiple files and probably
3032
// moved to an auth/ folder to match other platforms.
@@ -78,7 +80,7 @@ export class OAuthToken implements Token {
7880
* token and may need to invalidate other state if the current user has also
7981
* changed.
8082
*/
81-
export type CredentialChangeListener = (user: User) => void;
83+
export type CredentialChangeListener = (user: User) => Promise<void>;
8284

8385
/**
8486
* Provides methods for getting the uid and token for the current user and
@@ -98,8 +100,13 @@ export interface CredentialsProvider {
98100
* Specifies a listener to be notified of credential changes
99101
* (sign-in / sign-out, token changes). It is immediately called once with the
100102
* initial user.
103+
*
104+
* The change listener is invoked on the provided AsyncQueue.
101105
*/
102-
setChangeListener(changeListener: CredentialChangeListener): void;
106+
setChangeListener(
107+
asyncQueue: AsyncQueue,
108+
changeListener: CredentialChangeListener
109+
): void;
103110

104111
/** Removes the previously-set change listener. */
105112
removeChangeListener(): void;
@@ -120,14 +127,17 @@ export class EmptyCredentialsProvider implements CredentialsProvider {
120127

121128
invalidateToken(): void {}
122129

123-
setChangeListener(changeListener: CredentialChangeListener): void {
130+
setChangeListener(
131+
asyncQueue: AsyncQueue,
132+
changeListener: CredentialChangeListener
133+
): void {
124134
debugAssert(
125135
!this.changeListener,
126136
'Can only call setChangeListener() once.'
127137
);
128138
this.changeListener = changeListener;
129139
// Fire with initial user.
130-
changeListener(User.UNAUTHENTICATED);
140+
asyncQueue.enqueueRetryable(() => changeListener(User.UNAUTHENTICATED));
131141
}
132142

133143
removeChangeListener(): void {
@@ -155,14 +165,17 @@ export class EmulatorCredentialsProvider implements CredentialsProvider {
155165

156166
invalidateToken(): void {}
157167

158-
setChangeListener(changeListener: CredentialChangeListener): void {
168+
setChangeListener(
169+
asyncQueue: AsyncQueue,
170+
changeListener: CredentialChangeListener
171+
): void {
159172
debugAssert(
160173
!this.changeListener,
161174
'Can only call setChangeListener() once.'
162175
);
163176
this.changeListener = changeListener;
164177
// Fire with initial user.
165-
changeListener(this.token.user);
178+
asyncQueue.enqueueRetryable(() => changeListener(this.token.user));
166179
}
167180

168181
removeChangeListener(): void {
@@ -175,11 +188,13 @@ export class FirebaseCredentialsProvider implements CredentialsProvider {
175188
* The auth token listener registered with FirebaseApp, retained here so we
176189
* can unregister it.
177190
*/
178-
private tokenListener: ((token: string | null) => void) | null = null;
191+
private tokenListener: () => void;
179192

180193
/** Tracks the current User. */
181194
private currentUser: User = User.UNAUTHENTICATED;
182-
private receivedInitialUser: boolean = false;
195+
196+
/** Promise that allows blocking on the first `tokenListener` event. */
197+
private receivedInitialUser = new Deferred();
183198

184199
/**
185200
* Counter used to detect if the token changed while a getToken request was
@@ -188,44 +203,55 @@ export class FirebaseCredentialsProvider implements CredentialsProvider {
188203
private tokenCounter = 0;
189204

190205
/** The listener registered with setChangeListener(). */
191-
private changeListener: CredentialChangeListener | null = null;
206+
private changeListener: CredentialChangeListener = () => Promise.resolve();
207+
208+
private invokeChangeListener = false;
192209

193210
private forceRefresh = false;
194211

195-
private auth: FirebaseAuthInternal | null;
212+
private auth: FirebaseAuthInternal | null = null;
213+
214+
private asyncQueue: AsyncQueue | null = null;
196215

197216
constructor(authProvider: Provider<FirebaseAuthInternalName>) {
198217
this.tokenListener = () => {
199218
this.tokenCounter++;
200219
this.currentUser = this.getUser();
201-
this.receivedInitialUser = true;
202-
if (this.changeListener) {
203-
this.changeListener(this.currentUser);
220+
this.receivedInitialUser.resolve();
221+
if (this.invokeChangeListener) {
222+
this.asyncQueue!.enqueueRetryable(() =>
223+
this.changeListener(this.currentUser)
224+
);
204225
}
205226
};
206227

207-
this.tokenCounter = 0;
208-
209-
this.auth = authProvider.getImmediate({ optional: true });
228+
const registerAuth = (auth: FirebaseAuthInternal): void => {
229+
logDebug('FirebaseCredentialsProvider', 'Auth detected');
230+
this.auth = auth;
231+
this.awaitTokenAndRaiseInitialEvent();
232+
this.auth.addAuthTokenListener(this.tokenListener);
233+
};
210234

211-
if (this.auth) {
212-
this.auth.addAuthTokenListener(this.tokenListener!);
213-
} else {
214-
// if auth is not available, invoke tokenListener once with null token
215-
this.tokenListener(null);
216-
authProvider.get().then(
217-
auth => {
218-
this.auth = auth;
219-
if (this.tokenListener) {
220-
// tokenListener can be removed by removeChangeListener()
221-
this.auth.addAuthTokenListener(this.tokenListener);
222-
}
223-
},
224-
() => {
225-
/* this.authProvider.get() never rejects */
235+
authProvider.onInit(auth => registerAuth(auth));
236+
237+
// Our users can initialize Auth right after Firestore, so we give it
238+
// a chance to register itself with the component framework before we
239+
// determine whether to start up in unauthenticated mode.
240+
setTimeout(() => {
241+
if (!this.auth) {
242+
const auth = authProvider.getImmediate({ optional: true });
243+
if (auth) {
244+
registerAuth(auth);
245+
} else if (this.invokeChangeListener) {
246+
// If auth is still not available, invoke the change listener once
247+
// with null token
248+
logDebug('FirebaseCredentialsProvider', 'Auth not yet detected');
249+
this.asyncQueue!.enqueueRetryable(() =>
250+
this.changeListener(this.currentUser)
251+
);
226252
}
227-
);
228-
}
253+
}
254+
}, 0);
229255
}
230256

231257
getToken(): Promise<Token | null> {
@@ -273,25 +299,21 @@ export class FirebaseCredentialsProvider implements CredentialsProvider {
273299
this.forceRefresh = true;
274300
}
275301

276-
setChangeListener(changeListener: CredentialChangeListener): void {
277-
debugAssert(
278-
!this.changeListener,
279-
'Can only call setChangeListener() once.'
280-
);
302+
setChangeListener(
303+
asyncQueue: AsyncQueue,
304+
changeListener: CredentialChangeListener
305+
): void {
306+
debugAssert(!this.asyncQueue, 'Can only call setChangeListener() once.');
307+
this.invokeChangeListener = true;
308+
this.asyncQueue = asyncQueue;
281309
this.changeListener = changeListener;
282-
283-
// Fire the initial event
284-
if (this.receivedInitialUser) {
285-
changeListener(this.currentUser);
286-
}
287310
}
288311

289312
removeChangeListener(): void {
290313
if (this.auth) {
291314
this.auth.removeAuthTokenListener(this.tokenListener!);
292315
}
293-
this.tokenListener = null;
294-
this.changeListener = null;
316+
this.changeListener = () => Promise.resolve();
295317
}
296318

297319
// Auth.getUid() can return null even with a user logged in. It is because
@@ -306,6 +328,21 @@ export class FirebaseCredentialsProvider implements CredentialsProvider {
306328
);
307329
return new User(currentUid);
308330
}
331+
332+
/**
333+
* Blocks the AsyncQueue until the next user is available. This function also
334+
* invokes `this.changeListener` immediately once the token is available.
335+
*/
336+
private awaitTokenAndRaiseInitialEvent(): void {
337+
if (this.invokeChangeListener) {
338+
this.invokeChangeListener = false; // Prevent double-firing of the listener
339+
this.asyncQueue!.enqueueRetryable(async () => {
340+
await this.receivedInitialUser.promise;
341+
await this.changeListener(this.currentUser);
342+
this.invokeChangeListener = true;
343+
});
344+
}
345+
}
309346
}
310347

311348
// Manual type definition for the subset of Gapi we use.
@@ -368,9 +405,12 @@ export class FirstPartyCredentialsProvider implements CredentialsProvider {
368405
);
369406
}
370407

371-
setChangeListener(changeListener: CredentialChangeListener): void {
408+
setChangeListener(
409+
asyncQueue: AsyncQueue,
410+
changeListener: CredentialChangeListener
411+
): void {
372412
// Fire with initial uid.
373-
changeListener(User.FIRST_PARTY);
413+
asyncQueue.enqueueRetryable(() => changeListener(User.FIRST_PARTY));
374414
}
375415

376416
removeChangeListener(): void {}

packages/firestore/src/core/firestore_client.ts

+11-25
Original file line numberDiff line numberDiff line change
@@ -97,8 +97,8 @@ export const MAX_CONCURRENT_LIMBO_RESOLUTIONS = 100;
9797
export class FirestoreClient {
9898
private user = User.UNAUTHENTICATED;
9999
private readonly clientId = AutoId.newId();
100-
private credentialListener: CredentialChangeListener = () => {};
101-
private readonly receivedInitialUser = new Deferred<void>();
100+
private credentialListener: CredentialChangeListener = () =>
101+
Promise.resolve();
102102

103103
offlineComponents?: OfflineComponentProvider;
104104
onlineComponents?: OnlineComponentProvider;
@@ -116,17 +116,14 @@ export class FirestoreClient {
116116
public asyncQueue: AsyncQueue,
117117
private databaseInfo: DatabaseInfo
118118
) {
119-
this.credentials.setChangeListener(user => {
119+
this.credentials.setChangeListener(asyncQueue, async user => {
120120
logDebug(LOG_TAG, 'Received user=', user.uid);
121+
await this.credentialListener(user);
121122
this.user = user;
122-
this.credentialListener(user);
123-
this.receivedInitialUser.resolve();
124123
});
125124
}
126125

127126
async getConfiguration(): Promise<ComponentConfiguration> {
128-
await this.receivedInitialUser.promise;
129-
130127
return {
131128
asyncQueue: this.asyncQueue,
132129
databaseInfo: this.databaseInfo,
@@ -137,12 +134,8 @@ export class FirestoreClient {
137134
};
138135
}
139136

140-
setCredentialChangeListener(listener: (user: User) => void): void {
137+
setCredentialChangeListener(listener: (user: User) => Promise<void>): void {
141138
this.credentialListener = listener;
142-
// eslint-disable-next-line @typescript-eslint/no-floating-promises
143-
this.receivedInitialUser.promise.then(() =>
144-
this.credentialListener(this.user)
145-
);
146139
}
147140

148141
/**
@@ -198,15 +191,13 @@ export async function setOfflineComponentProvider(
198191
await offlineComponentProvider.initialize(configuration);
199192

200193
let currentUser = configuration.initialUser;
201-
client.setCredentialChangeListener(user => {
194+
client.setCredentialChangeListener(async user => {
202195
if (!currentUser.isEqual(user)) {
196+
await localStoreHandleUserChange(
197+
offlineComponentProvider.localStore,
198+
user
199+
);
203200
currentUser = user;
204-
client.asyncQueue.enqueueRetryable(async () => {
205-
await localStoreHandleUserChange(
206-
offlineComponentProvider.localStore,
207-
user
208-
);
209-
});
210201
}
211202
});
212203

@@ -236,12 +227,7 @@ export async function setOnlineComponentProvider(
236227
// The CredentialChangeListener of the online component provider takes
237228
// precedence over the offline component provider.
238229
client.setCredentialChangeListener(user =>
239-
client.asyncQueue.enqueueRetryable(() =>
240-
remoteStoreHandleCredentialChange(
241-
onlineComponentProvider.remoteStore,
242-
user
243-
)
244-
)
230+
remoteStoreHandleCredentialChange(onlineComponentProvider.remoteStore, user)
245231
);
246232
client.onlineComponents = onlineComponentProvider;
247233
}

0 commit comments

Comments
 (0)