From 162a8c1e2eaf7bc1992cb9ba11100818cf788391 Mon Sep 17 00:00:00 2001 From: Thomas Norling Date: Tue, 12 Dec 2023 11:27:57 -0800 Subject: [PATCH] Convert ThrottlingEntity into a type (#6754) Continues the work to convert all our cache entities from static classes into Types & functions for perf and size --- ...-db45337f-959c-40db-b8a7-6e906fdc913b.json | 7 +++++ ...-8a857037-5571-48eb-9211-8efca31765b2.json | 7 +++++ ...-5000935d-29a7-4c4c-a80c-602d7f5f9b05.json | 7 +++++ .../src/cache/BrowserCacheManager.ts | 7 ++--- .../test/cache/BrowserCacheManager.spec.ts | 21 ++++---------- .../src/cache/entities/ThrottlingEntity.ts | 26 ++--------------- .../src/cache/utils/CacheHelpers.ts | 20 +++++++++++++ .../cache/entities/ThrottlingEntity.spec.ts | 28 +++++++++---------- lib/msal-node/src/cache/NodeStorage.ts | 5 +--- 9 files changed, 65 insertions(+), 63 deletions(-) create mode 100644 change/@azure-msal-browser-db45337f-959c-40db-b8a7-6e906fdc913b.json create mode 100644 change/@azure-msal-common-8a857037-5571-48eb-9211-8efca31765b2.json create mode 100644 change/@azure-msal-node-5000935d-29a7-4c4c-a80c-602d7f5f9b05.json diff --git a/change/@azure-msal-browser-db45337f-959c-40db-b8a7-6e906fdc913b.json b/change/@azure-msal-browser-db45337f-959c-40db-b8a7-6e906fdc913b.json new file mode 100644 index 0000000000..50290ce56b --- /dev/null +++ b/change/@azure-msal-browser-db45337f-959c-40db-b8a7-6e906fdc913b.json @@ -0,0 +1,7 @@ +{ + "type": "patch", + "comment": "Convert ThrottlingEntity into a Type", + "packageName": "@azure/msal-browser", + "email": "thomas.norling@microsoft.com", + "dependentChangeType": "patch" +} diff --git a/change/@azure-msal-common-8a857037-5571-48eb-9211-8efca31765b2.json b/change/@azure-msal-common-8a857037-5571-48eb-9211-8efca31765b2.json new file mode 100644 index 0000000000..6f035ac242 --- /dev/null +++ b/change/@azure-msal-common-8a857037-5571-48eb-9211-8efca31765b2.json @@ -0,0 +1,7 @@ +{ + "type": "patch", + "comment": "Convert ThrottlingEntity into a Type", + "packageName": "@azure/msal-common", + "email": "thomas.norling@microsoft.com", + "dependentChangeType": "patch" +} diff --git a/change/@azure-msal-node-5000935d-29a7-4c4c-a80c-602d7f5f9b05.json b/change/@azure-msal-node-5000935d-29a7-4c4c-a80c-602d7f5f9b05.json new file mode 100644 index 0000000000..2250a6e0c1 --- /dev/null +++ b/change/@azure-msal-node-5000935d-29a7-4c4c-a80c-602d7f5f9b05.json @@ -0,0 +1,7 @@ +{ + "type": "patch", + "comment": "Convert ThrottlingEntity into a Type", + "packageName": "@azure/msal-node", + "email": "thomas.norling@microsoft.com", + "dependentChangeType": "patch" +} diff --git a/lib/msal-browser/src/cache/BrowserCacheManager.ts b/lib/msal-browser/src/cache/BrowserCacheManager.ts index a6c6c75440..ab9c3e56c1 100644 --- a/lib/msal-browser/src/cache/BrowserCacheManager.ts +++ b/lib/msal-browser/src/cache/BrowserCacheManager.ts @@ -1119,7 +1119,7 @@ export class BrowserCacheManager extends CacheManager { const parsedThrottlingCache = this.validateAndParseJson(value); if ( !parsedThrottlingCache || - !ThrottlingEntity.isThrottlingEntity( + !CacheHelpers.isThrottlingEntity( throttlingCacheKey, parsedThrottlingCache ) @@ -1131,10 +1131,7 @@ export class BrowserCacheManager extends CacheManager { } this.logger.trace("BrowserCacheManager.getThrottlingCache: cache hit"); - return CacheManager.toObject( - new ThrottlingEntity(), - parsedThrottlingCache - ); + return parsedThrottlingCache as ThrottlingEntity; } /** diff --git a/lib/msal-browser/test/cache/BrowserCacheManager.spec.ts b/lib/msal-browser/test/cache/BrowserCacheManager.spec.ts index 7d3d1e7365..40d284e611 100644 --- a/lib/msal-browser/test/cache/BrowserCacheManager.spec.ts +++ b/lib/msal-browser/test/cache/BrowserCacheManager.spec.ts @@ -1561,8 +1561,9 @@ describe("BrowserCacheManager tests", () => { it("getThrottlingCache returns ThrottlingEntity", () => { const testKey = "throttling"; - const testVal = new ThrottlingEntity(); - testVal.throttleTime = 60; + const testVal = { + throttleTime: 60, + }; browserLocalStorage.setThrottlingCache(testKey, testVal); browserSessionStorage.setThrottlingCache(testKey, testVal); @@ -1570,15 +1571,10 @@ describe("BrowserCacheManager tests", () => { expect( browserSessionStorage.getThrottlingCache(testKey) ).toEqual(testVal); - expect( - browserSessionStorage.getThrottlingCache(testKey) - ).toBeInstanceOf(ThrottlingEntity); + expect( browserLocalStorage.getThrottlingCache(testKey) ).toEqual(testVal); - expect( - browserLocalStorage.getThrottlingCache(testKey) - ).toBeInstanceOf(ThrottlingEntity); }); }); @@ -2470,8 +2466,7 @@ describe("BrowserCacheManager tests", () => { it("getThrottlingCache returns ThrottlingEntity", () => { const testKey = "throttling"; - const testVal = new ThrottlingEntity(); - testVal.throttleTime = 60; + const testVal = { throttleTime: 60 }; browserLocalStorage.setThrottlingCache(testKey, testVal); browserSessionStorage.setThrottlingCache(testKey, testVal); @@ -2479,15 +2474,9 @@ describe("BrowserCacheManager tests", () => { expect( browserSessionStorage.getThrottlingCache(testKey) ).toEqual(testVal); - expect( - browserSessionStorage.getThrottlingCache(testKey) - ).toBeInstanceOf(ThrottlingEntity); expect( browserLocalStorage.getThrottlingCache(testKey) ).toEqual(testVal); - expect( - browserLocalStorage.getThrottlingCache(testKey) - ).toBeInstanceOf(ThrottlingEntity); }); }); diff --git a/lib/msal-common/src/cache/entities/ThrottlingEntity.ts b/lib/msal-common/src/cache/entities/ThrottlingEntity.ts index 5bafe8de15..fe231986a0 100644 --- a/lib/msal-common/src/cache/entities/ThrottlingEntity.ts +++ b/lib/msal-common/src/cache/entities/ThrottlingEntity.ts @@ -3,9 +3,7 @@ * Licensed under the MIT License. */ -import { ThrottlingConstants } from "../../utils/Constants"; - -export class ThrottlingEntity { +export type ThrottlingEntity = { // Unix-time value representing the expiration of the throttle throttleTime: number; // Information provided by the server @@ -13,24 +11,4 @@ export class ThrottlingEntity { errorCodes?: Array; errorMessage?: string; subError?: string; - - /** - * validates if a given cache entry is "Throttling", parses - * @param key - * @param entity - */ - static isThrottlingEntity(key: string, entity?: object): boolean { - let validateKey: boolean = false; - if (key) { - validateKey = - key.indexOf(ThrottlingConstants.THROTTLING_PREFIX) === 0; - } - - let validateEntity: boolean = true; - if (entity) { - validateEntity = entity.hasOwnProperty("throttleTime"); - } - - return validateKey && validateEntity; - } -} +}; diff --git a/lib/msal-common/src/cache/utils/CacheHelpers.ts b/lib/msal-common/src/cache/utils/CacheHelpers.ts index 86aa55eafe..83c9029373 100644 --- a/lib/msal-common/src/cache/utils/CacheHelpers.ts +++ b/lib/msal-common/src/cache/utils/CacheHelpers.ts @@ -14,6 +14,7 @@ import { CredentialType, SERVER_TELEM_CONSTANTS, Separators, + ThrottlingConstants, } from "../../utils/Constants"; import { TimeUtils } from "../../utils/TimeUtils"; import { AccessTokenEntity } from "../entities/AccessTokenEntity"; @@ -330,3 +331,22 @@ export function isServerTelemetryEntity(key: string, entity?: object): boolean { return validateKey && validateEntity; } + +/** + * validates if a given cache entry is "Throttling", parses + * @param key + * @param entity + */ +export function isThrottlingEntity(key: string, entity?: object): boolean { + let validateKey: boolean = false; + if (key) { + validateKey = key.indexOf(ThrottlingConstants.THROTTLING_PREFIX) === 0; + } + + let validateEntity: boolean = true; + if (entity) { + validateEntity = entity.hasOwnProperty("throttleTime"); + } + + return validateKey && validateEntity; +} diff --git a/lib/msal-common/test/cache/entities/ThrottlingEntity.spec.ts b/lib/msal-common/test/cache/entities/ThrottlingEntity.spec.ts index 66c0463579..df2f48abe6 100644 --- a/lib/msal-common/test/cache/entities/ThrottlingEntity.spec.ts +++ b/lib/msal-common/test/cache/entities/ThrottlingEntity.spec.ts @@ -3,7 +3,7 @@ * Licensed under the MIT License. */ -import { ThrottlingEntity } from "../../../src/cache/entities/ThrottlingEntity"; +import { CacheHelpers } from "../../../src"; import { ThrottlingConstants, Separators } from "../../../src/utils/Constants"; import { TEST_CONFIG } from "../../test_kit/StringConstants"; @@ -17,24 +17,24 @@ describe("ThrottlingEntity", () => { const throttlingObject = { throttleTime: 0, }; - expect( - ThrottlingEntity.isThrottlingEntity(key, throttlingObject) - ).toBe(true); + expect(CacheHelpers.isThrottlingEntity(key, throttlingObject)).toBe( + true + ); }); it("Verifies if an object is a ThrottlingEntity when no object is given", () => { - expect(ThrottlingEntity.isThrottlingEntity(key)).toBe(true); + expect(CacheHelpers.isThrottlingEntity(key)).toBe(true); // @ts-ignore - expect(ThrottlingEntity.isThrottlingEntity(key, null)).toBe(true); + expect(CacheHelpers.isThrottlingEntity(key, null)).toBe(true); }); it("Verifies if an object is not a ThrottlingEntity based on field", () => { const throttlingObject = { test: 0, }; - expect( - ThrottlingEntity.isThrottlingEntity(key, throttlingObject) - ).toBe(false); + expect(CacheHelpers.isThrottlingEntity(key, throttlingObject)).toBe( + false + ); }); it("Verifies if an object is not a ThrottlingEntity based on key", () => { @@ -42,14 +42,14 @@ describe("ThrottlingEntity", () => { throttleTime: 0, }; expect( - ThrottlingEntity.isThrottlingEntity("asd", throttlingObject) - ).toBe(false); - expect( - ThrottlingEntity.isThrottlingEntity("", throttlingObject) + CacheHelpers.isThrottlingEntity("asd", throttlingObject) ).toBe(false); + expect(CacheHelpers.isThrottlingEntity("", throttlingObject)).toBe( + false + ); expect( // @ts-ignore - ThrottlingEntity.isThrottlingEntity(null, throttlingObject) + CacheHelpers.isThrottlingEntity(null, throttlingObject) ).toBe(false); }); }); diff --git a/lib/msal-node/src/cache/NodeStorage.ts b/lib/msal-node/src/cache/NodeStorage.ts index 93ab126567..d06ebbb048 100644 --- a/lib/msal-node/src/cache/NodeStorage.ts +++ b/lib/msal-node/src/cache/NodeStorage.ts @@ -421,10 +421,7 @@ export class NodeStorage extends CacheManager { ) as ThrottlingEntity; if ( throttlingCache && - ThrottlingEntity.isThrottlingEntity( - throttlingCacheKey, - throttlingCache - ) + CacheHelpers.isThrottlingEntity(throttlingCacheKey, throttlingCache) ) { return throttlingCache; }