Skip to content
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

chore(auth): debounce refreshAuthTokens #12845

Merged
merged 14 commits into from
Jan 18, 2024
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,15 @@ import { AuthConfig } from '@aws-amplify/core';
import {
assertTokenProviderConfig,
decodeJWT,
deDupCallback,
} from '@aws-amplify/core/internals/utils';
import { initiateAuth } from '../utils/clients/CognitoIdentityProvider';
import { getRegion } from '../utils/clients/CognitoIdentityProvider/utils';
import { assertAuthTokensWithRefreshToken } from '../utils/types';
import { AuthError } from '../../../errors/AuthError';
import { getUserContextData } from './userContextData';

export const refreshAuthTokens: TokenRefresher = async ({
const refreshAuthTokensCallback: TokenRefresher = async ({
israx marked this conversation as resolved.
Show resolved Hide resolved
tokens,
authConfig,
username,
Expand Down Expand Up @@ -72,3 +73,5 @@ export const refreshAuthTokens: TokenRefresher = async ({
username,
};
};

export const refreshAuthTokens = deDupCallback(refreshAuthTokensCallback);
14 changes: 7 additions & 7 deletions packages/aws-amplify/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -300,13 +300,13 @@
"name": "[Analytics] record (Kinesis)",
"path": "./dist/esm/analytics/kinesis/index.mjs",
"import": "{ record }",
"limit": "44.47 kB"
"limit": "44.48 kB"
},
{
"name": "[Analytics] record (Kinesis Firehose)",
"path": "./dist/esm/analytics/kinesis-firehose/index.mjs",
"import": "{ record }",
"limit": "41.50 kB"
"limit": "41.54 kB"
},
{
"name": "[Analytics] record (Personalize)",
Expand Down Expand Up @@ -396,19 +396,19 @@
"name": "[Auth] fetchMFAPreference (Cognito)",
"path": "./dist/esm/auth/index.mjs",
"import": "{ fetchMFAPreference }",
"limit": "8.30 kB"
"limit": "8.35 kB"
},
{
"name": "[Auth] verifyTOTPSetup (Cognito)",
"path": "./dist/esm/auth/index.mjs",
"import": "{ verifyTOTPSetup }",
"limit": "9.13 kB"
"limit": "9.18 kB"
},
{
"name": "[Auth] updatePassword (Cognito)",
"path": "./dist/esm/auth/index.mjs",
"import": "{ updatePassword }",
"limit": "9.14 kB"
"limit": "9.19 kB"
},
{
"name": "[Auth] setUpTOTP (Cognito)",
Expand All @@ -420,7 +420,7 @@
"name": "[Auth] updateUserAttributes (Cognito)",
"path": "./dist/esm/auth/index.mjs",
"import": "{ updateUserAttributes }",
"limit": "8.41 kB"
"limit": "8.46 kB"
},
{
"name": "[Auth] getCurrentUser (Cognito)",
Expand All @@ -432,7 +432,7 @@
"name": "[Auth] confirmUserAttribute (Cognito)",
"path": "./dist/esm/auth/index.mjs",
"import": "{ confirmUserAttribute }",
"limit": "9.15 kB"
"limit": "9.19 kB"
},
{
"name": "[Auth] signInWithRedirect (Cognito)",
Expand Down
46 changes: 46 additions & 0 deletions packages/core/__tests__/utils/debounceCallback.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
import { debounceCallback } from '../../src/utils/debounceCallback';
israx marked this conversation as resolved.
Show resolved Hide resolved

describe('test debounce callback', () => {
const numberOfConcurrentCalls = 10;
const mockServiceCallback = jest.fn();

afterEach(() => {
mockServiceCallback.mockClear();
});

it('should allow to invoke the callback when there is no concurrent calls', async () => {
const debouncedCallback = debounceCallback(mockServiceCallback);

debouncedCallback();
expect(mockServiceCallback).toHaveBeenCalledTimes(1);
});

it('should invoke the callback one time during concurrent sync calls', () => {
const debouncedCallback = debounceCallback(mockServiceCallback);
for (let i = 0; i < numberOfConcurrentCalls; i++) {
debouncedCallback();
}
expect(mockServiceCallback).toHaveBeenCalledTimes(1);
});

it('should allow to invoke the callback again after the promise has being resolved', async () => {
const debouncedCallback = debounceCallback(mockServiceCallback);
for (let i = 0; i < numberOfConcurrentCalls; i++) {
debouncedCallback();
}

await debouncedCallback();

debouncedCallback();
expect(mockServiceCallback).toHaveBeenCalledTimes(2);
});
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test may not be accurate - the mockServiceCallback called times should increase after the promised returned by it resolves, and when debouncedCallback gets called again, but in this test, the mockServiceCallback doesn't even return a promise.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the mockServiceCallback doesn't return a promise but the deDupe function does because it creates a promise on top of the callback passed. What I'm trying to do is to call the deDuped function concurrently and then calling it again when it was able to resolve.


it('should return a value once the callback is resolved', async () => {
const mockReturnValue = { id: 1 };
mockServiceCallback.mockImplementation(() => mockReturnValue);
const debouncedCallback = debounceCallback(mockServiceCallback);
const result = await debouncedCallback();
expect(result).toEqual(mockReturnValue);
expect(mockServiceCallback).toHaveBeenCalledTimes(1);
});
});
1 change: 1 addition & 0 deletions packages/core/src/libraryUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ export {
retry,
urlSafeDecode,
urlSafeEncode,
deDupCallback,
} from './utils';
export { parseAWSExports } from './parseAWSExports';
export { LegacyConfig } from './singleton/types';
Expand Down
6 changes: 2 additions & 4 deletions packages/core/src/singleton/apis/fetchAuthSession.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,8 @@

import { fetchAuthSession as fetchAuthSessionInternal } from './internal/fetchAuthSession';
import { Amplify } from '../Amplify';
import { AuthSession, FetchAuthSessionOptions } from '../Auth/types';
import { FetchAuthSessionOptions } from '../Auth/types';

export const fetchAuthSession = (
options?: FetchAuthSessionOptions
): Promise<AuthSession> => {
export const fetchAuthSession = (options?: FetchAuthSessionOptions) => {
return fetchAuthSessionInternal(Amplify, options);
};
34 changes: 34 additions & 0 deletions packages/core/src/utils/deDupCallback.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
// SPDX-License-Identifier: Apache-2.0

/**
* returns inflight promise if there hasn't been resolved yet
*
* @param callback - callback to be deDup.
* @returns - the return type of the callback
*/
export const deDupCallback = <F extends (...args: any[]) => any>(
israx marked this conversation as resolved.
Show resolved Hide resolved
callback: F
) => {
let inflightPromise: Promise<Awaited<ReturnType<F>>> | undefined;
return async (...args: Parameters<F>): Promise<Awaited<ReturnType<F>>> => {
if (inflightPromise) return inflightPromise;

if (!inflightPromise) {
inflightPromise = new Promise(async (resolve, reject) => {
try {
const result = await callback(...args);
resolve(result);
} catch (error) {
reject(error);
}
});
}

try {
return await inflightPromise;
} finally {
inflightPromise = undefined;
}
};
};
israx marked this conversation as resolved.
Show resolved Hide resolved
1 change: 1 addition & 0 deletions packages/core/src/utils/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,3 +15,4 @@ export {
export { urlSafeDecode } from './urlSafeDecode';
export { urlSafeEncode } from './urlSafeEncode';
export { deepFreeze } from './deepFreeze';
export { deDupCallback } from './deDupCallback';
2 changes: 1 addition & 1 deletion packages/datastore/src/sync/processors/subscription.ts
Original file line number Diff line number Diff line change
Expand Up @@ -279,7 +279,7 @@ class SubscriptionProcessor {

try {
// retrieving current token info from Cognito UserPools
const session = await await fetchAuthSession();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

😬

const session = await fetchAuthSession();
oidcTokenPayload = session.tokens?.idToken?.payload;
} catch (err) {
// best effort to get jwt from Cognito
Expand Down
Loading