Skip to content

Commit

Permalink
Convert ThrottlingEntity into a type (#6754)
Browse files Browse the repository at this point in the history
Continues the work to convert all our cache entities from static classes
into Types & functions for perf and size
  • Loading branch information
tnorling authored Dec 12, 2023
1 parent 8c1c63d commit 162a8c1
Show file tree
Hide file tree
Showing 9 changed files with 65 additions and 63 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"type": "patch",
"comment": "Convert ThrottlingEntity into a Type",
"packageName": "@azure/msal-browser",
"email": "[email protected]",
"dependentChangeType": "patch"
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"type": "patch",
"comment": "Convert ThrottlingEntity into a Type",
"packageName": "@azure/msal-common",
"email": "[email protected]",
"dependentChangeType": "patch"
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"type": "patch",
"comment": "Convert ThrottlingEntity into a Type",
"packageName": "@azure/msal-node",
"email": "[email protected]",
"dependentChangeType": "patch"
}
7 changes: 2 additions & 5 deletions lib/msal-browser/src/cache/BrowserCacheManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1119,7 +1119,7 @@ export class BrowserCacheManager extends CacheManager {
const parsedThrottlingCache = this.validateAndParseJson(value);
if (
!parsedThrottlingCache ||
!ThrottlingEntity.isThrottlingEntity(
!CacheHelpers.isThrottlingEntity(
throttlingCacheKey,
parsedThrottlingCache
)
Expand All @@ -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;
}

/**
Expand Down
21 changes: 5 additions & 16 deletions lib/msal-browser/test/cache/BrowserCacheManager.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1561,24 +1561,20 @@ 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);

expect(
browserSessionStorage.getThrottlingCache(testKey)
).toEqual(testVal);
expect(
browserSessionStorage.getThrottlingCache(testKey)
).toBeInstanceOf(ThrottlingEntity);

expect(
browserLocalStorage.getThrottlingCache(testKey)
).toEqual(testVal);
expect(
browserLocalStorage.getThrottlingCache(testKey)
).toBeInstanceOf(ThrottlingEntity);
});
});

Expand Down Expand Up @@ -2470,24 +2466,17 @@ 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);

expect(
browserSessionStorage.getThrottlingCache(testKey)
).toEqual(testVal);
expect(
browserSessionStorage.getThrottlingCache(testKey)
).toBeInstanceOf(ThrottlingEntity);
expect(
browserLocalStorage.getThrottlingCache(testKey)
).toEqual(testVal);
expect(
browserLocalStorage.getThrottlingCache(testKey)
).toBeInstanceOf(ThrottlingEntity);
});
});

Expand Down
26 changes: 2 additions & 24 deletions lib/msal-common/src/cache/entities/ThrottlingEntity.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,34 +3,12 @@
* 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
error?: string;
errorCodes?: Array<string>;
errorMessage?: string;
subError?: string;

/**
* validates if a given cache entry is "Throttling", parses <key,value>
* @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;
}
}
};
20 changes: 20 additions & 0 deletions lib/msal-common/src/cache/utils/CacheHelpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import {
CredentialType,
SERVER_TELEM_CONSTANTS,
Separators,
ThrottlingConstants,
} from "../../utils/Constants";
import { TimeUtils } from "../../utils/TimeUtils";
import { AccessTokenEntity } from "../entities/AccessTokenEntity";
Expand Down Expand Up @@ -330,3 +331,22 @@ export function isServerTelemetryEntity(key: string, entity?: object): boolean {

return validateKey && validateEntity;
}

/**
* validates if a given cache entry is "Throttling", parses <key,value>
* @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;
}
28 changes: 14 additions & 14 deletions lib/msal-common/test/cache/entities/ThrottlingEntity.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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";

Expand All @@ -17,39 +17,39 @@ 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", () => {
const throttlingObject = {
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);
});
});
Expand Down
5 changes: 1 addition & 4 deletions lib/msal-node/src/cache/NodeStorage.ts
Original file line number Diff line number Diff line change
Expand Up @@ -421,10 +421,7 @@ export class NodeStorage extends CacheManager {
) as ThrottlingEntity;
if (
throttlingCache &&
ThrottlingEntity.isThrottlingEntity(
throttlingCacheKey,
throttlingCache
)
CacheHelpers.isThrottlingEntity(throttlingCacheKey, throttlingCache)
) {
return throttlingCache;
}
Expand Down

0 comments on commit 162a8c1

Please sign in to comment.