Skip to content

Commit

Permalink
fix: getToken() returns existing promise to a token if one exists i…
Browse files Browse the repository at this point in the history
…nstead of a new token. (#2648)

* fix:  returns existing promise to a token if one exists instead of a new token.

* fix: lint
  • Loading branch information
jonathanedey authored Aug 1, 2024
1 parent d09e3bd commit 745c22e
Show file tree
Hide file tree
Showing 3 changed files with 23 additions and 6 deletions.
17 changes: 12 additions & 5 deletions src/app/firebase-app.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,26 +39,29 @@ export interface FirebaseAccessToken {
*/
export class FirebaseAppInternals {
private cachedToken_: FirebaseAccessToken;
private promiseToCachedToken_: Promise<FirebaseAccessToken>;
private tokenListeners_: Array<(token: string) => void>;
private isRefreshing: boolean;

// eslint-disable-next-line @typescript-eslint/naming-convention
constructor(private credential_: Credential) {
this.tokenListeners_ = [];
this.isRefreshing = false;
}

public getToken(forceRefresh = false): Promise<FirebaseAccessToken> {
if (forceRefresh || this.shouldRefresh()) {
return this.refreshToken();
this.promiseToCachedToken_ = this.refreshToken();
}

return Promise.resolve(this.cachedToken_);
return this.promiseToCachedToken_
}

public getCachedToken(): FirebaseAccessToken | null {
return this.cachedToken_ || null;
}

private refreshToken(): Promise<FirebaseAccessToken> {
this.isRefreshing = true;
return Promise.resolve(this.credential_.getAccessToken())
.then((result) => {
// Since the developer can provide the credential implementation, we want to weakly verify
Expand Down Expand Up @@ -108,11 +111,15 @@ export class FirebaseAppInternals {
}

throw new FirebaseAppError(AppErrorCodes.INVALID_CREDENTIAL, errorMessage);
});
})
.finally(() => {
this.isRefreshing = false;
})
}

private shouldRefresh(): boolean {
return !this.cachedToken_ || (this.cachedToken_.expirationTime - Date.now()) <= TOKEN_EXPIRY_THRESHOLD_MILLIS;
return (!this.cachedToken_ || (this.cachedToken_.expirationTime - Date.now()) <= TOKEN_EXPIRY_THRESHOLD_MILLIS)
&& !this.isRefreshing;
}

/**
Expand Down
2 changes: 1 addition & 1 deletion test/integration/messaging.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -278,7 +278,7 @@ describe('admin.messaging', () => {
});
});

xit('sendToDeviceGroup() returns a response with success count', () => {
it('sendToDeviceGroup() returns a response with success count', () => {
return getMessaging().sendToDeviceGroup(notificationKey, payload, options)
.then((response) => {
expect(typeof response.successCount).to.equal('number');
Expand Down
10 changes: 10 additions & 0 deletions test/unit/app/firebase-app.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -808,6 +808,16 @@ describe('FirebaseApp', () => {
});
});

it('only refreshes the token once for concurrent calls', () => {
const promise1 = mockApp.INTERNAL.getToken();
const promise2 = mockApp.INTERNAL.getToken();
expect(getTokenStub).to.have.been.calledOnce;
return Promise.all([promise1, promise2]).then((tokens) => {
expect(tokens[0]).to.equal(tokens[1]);
expect(getTokenStub).to.have.been.calledOnce;
})
});

it('Includes the original error in exception', () => {
getTokenStub.restore();
const mockError = new FirebaseAppError(
Expand Down

0 comments on commit 745c22e

Please sign in to comment.