Change ServerTelemetryEntity from Class to Type (#6651)

Continuation of the work to convert cache entities from classes to types and moving the related static methods to CacheHelpers namespace
AzureAD · Nov 2, 2023 · 42a5e13 · 42a5e13
1 parent 25472ea
commit 42a5e13
Show file tree

Hide file tree

Showing 11 changed files with 89 additions and 72 deletions.
diff --git a/change/@azure-msal-browser-df07c979-2cd0-4c10-bacf-bd89de87433c.json b/change/@azure-msal-browser-df07c979-2cd0-4c10-bacf-bd89de87433c.json
@@ -0,0 +1,7 @@
+{
+  "type": "minor",
+  "comment": "Convert ServerTelemetryEntity to Type instead of Class #6651",
+  "packageName": "@azure/msal-browser",
+  "email": "[email protected]",
+  "dependentChangeType": "patch"
+}
diff --git a/change/@azure-msal-common-36947fe1-a24c-4fe7-976d-3be088e98af2.json b/change/@azure-msal-common-36947fe1-a24c-4fe7-976d-3be088e98af2.json
@@ -0,0 +1,7 @@
+{
+  "type": "minor",
+  "comment": "Convert ServerTelemetryEntity to Type instead of Class #6651",
+  "packageName": "@azure/msal-common",
+  "email": "[email protected]",
+  "dependentChangeType": "patch"
+}
diff --git a/change/@azure-msal-node-4b69c21b-f2e9-4feb-9dab-fd5ba6fcfb53.json b/change/@azure-msal-node-4b69c21b-f2e9-4feb-9dab-fd5ba6fcfb53.json
@@ -0,0 +1,7 @@
+{
+  "type": "minor",
+  "comment": "Convert ServerTelemetryEntity to Type instead of Class #6651",
+  "packageName": "@azure/msal-node",
+  "email": "[email protected]",
+  "dependentChangeType": "patch"
+}
diff --git a/lib/msal-browser/src/cache/BrowserCacheManager.ts b/lib/msal-browser/src/cache/BrowserCacheManager.ts
@@ -880,12 +880,12 @@ export class BrowserCacheManager extends CacheManager {
             );
             return null;
         }
-        const parsedMetadata = this.validateAndParseJson(value);
+        const parsedEntity = this.validateAndParseJson(value);
         if (
-            !parsedMetadata ||
-            !ServerTelemetryEntity.isServerTelemetryEntity(
+            !parsedEntity ||
+            !CacheHelpers.isServerTelemetryEntity(
                 serverTelemetryKey,
-                parsedMetadata
+                parsedEntity
             )
         ) {
             this.logger.trace(
@@ -895,10 +895,7 @@ export class BrowserCacheManager extends CacheManager {
         }
 
         this.logger.trace("BrowserCacheManager.getServerTelemetry: cache hit");
-        return CacheManager.toObject(
-            new ServerTelemetryEntity(),
-            parsedMetadata
-        );
+        return parsedEntity as ServerTelemetryEntity;
     }
 
     /**

diff --git a/lib/msal-browser/test/cache/BrowserCacheManager.spec.ts b/lib/msal-browser/test/cache/BrowserCacheManager.spec.ts
@@ -1384,23 +1384,21 @@ describe("BrowserCacheManager tests", () => {
 
                 it("getServerTelemetry returns ServerTelemetryEntity", () => {
                     const testKey = "server-telemetry-clientId";
-                    const testVal = new ServerTelemetryEntity();
+                    const testVal = {
+                        failedRequests: ["61|test-correlationId"],
+                        errors: ["test_error"],
+                        cacheHits: 2,
+                    };
 
                     browserLocalStorage.setServerTelemetry(testKey, testVal);
                     browserSessionStorage.setServerTelemetry(testKey, testVal);
 
                     expect(
                         browserSessionStorage.getServerTelemetry(testKey)
                     ).toEqual(testVal);
-                    expect(
-                        browserSessionStorage.getServerTelemetry(testKey)
-                    ).toBeInstanceOf(ServerTelemetryEntity);
                     expect(
                         browserLocalStorage.getServerTelemetry(testKey)
                     ).toEqual(testVal);
-                    expect(
-                        browserLocalStorage.getServerTelemetry(testKey)
-                    ).toBeInstanceOf(ServerTelemetryEntity);
                 });
             });
 
@@ -2294,23 +2292,21 @@ describe("BrowserCacheManager tests", () => {
 
                 it("getServerTelemetry returns ServerTelemetryEntity", () => {
                     const testKey = "server-telemetry-clientId";
-                    const testVal = new ServerTelemetryEntity();
+                    const testVal = {
+                        failedRequests: ["61|test-correlationId"],
+                        errors: ["test_error"],
+                        cacheHits: 2,
+                    };
 
                     browserLocalStorage.setServerTelemetry(testKey, testVal);
                     browserSessionStorage.setServerTelemetry(testKey, testVal);
 
                     expect(
                         browserSessionStorage.getServerTelemetry(testKey)
                     ).toEqual(testVal);
-                    expect(
-                        browserSessionStorage.getServerTelemetry(testKey)
-                    ).toBeInstanceOf(ServerTelemetryEntity);
                     expect(
                         browserLocalStorage.getServerTelemetry(testKey)
                     ).toEqual(testVal);
-                    expect(
-                        browserLocalStorage.getServerTelemetry(testKey)
-                    ).toBeInstanceOf(ServerTelemetryEntity);
                 });
             });
 

diff --git a/lib/msal-common/src/cache/entities/ServerTelemetryEntity.ts b/lib/msal-common/src/cache/entities/ServerTelemetryEntity.ts
@@ -3,36 +3,8 @@
  * Licensed under the MIT License.
  */
 
-import { SERVER_TELEM_CONSTANTS } from "../../utils/Constants";
-
-export class ServerTelemetryEntity {
+export type ServerTelemetryEntity = {
     failedRequests: Array<string | number>;
     errors: string[];
     cacheHits: number;
-
-    constructor() {
-        this.failedRequests = [];
-        this.errors = [];
-        this.cacheHits = 0;
-    }
-
-    /**
-     * validates if a given cache entry is "Telemetry", parses <key,value>
-     * @param key
-     * @param entity
-     */
-    static isServerTelemetryEntity(key: string, entity?: object): boolean {
-        const validateKey: boolean =
-            key.indexOf(SERVER_TELEM_CONSTANTS.CACHE_KEY) === 0;
-        let validateEntity: boolean = true;
-
-        if (entity) {
-            validateEntity =
-                entity.hasOwnProperty("failedRequests") &&
-                entity.hasOwnProperty("errors") &&
-                entity.hasOwnProperty("cacheHits");
-        }
-
-        return validateKey && validateEntity;
-    }
-}
+};
diff --git a/lib/msal-common/src/cache/utils/CacheHelpers.ts b/lib/msal-common/src/cache/utils/CacheHelpers.ts
@@ -12,6 +12,7 @@ import {
 import {
     AuthenticationScheme,
     CredentialType,
+    SERVER_TELEM_CONSTANTS,
     Separators,
 } from "../../utils/Constants";
 import { TimeUtils } from "../../utils/TimeUtils";
@@ -304,3 +305,23 @@ function generateScheme(credentialEntity: CredentialEntity): string {
         ? credentialEntity.tokenType.toLowerCase()
         : "";
 }
+
+/**
+ * validates if a given cache entry is "Telemetry", parses <key,value>
+ * @param key
+ * @param entity
+ */
+export function isServerTelemetryEntity(key: string, entity?: object): boolean {
+    const validateKey: boolean =
+        key.indexOf(SERVER_TELEM_CONSTANTS.CACHE_KEY) === 0;
+    let validateEntity: boolean = true;
+
+    if (entity) {
+        validateEntity =
+            entity.hasOwnProperty("failedRequests") &&
+            entity.hasOwnProperty("errors") &&
+            entity.hasOwnProperty("cacheHits");
+    }
+
+    return validateKey && validateEntity;
+}
diff --git a/lib/msal-common/src/telemetry/server/ServerTelemetryManager.ts b/lib/msal-common/src/telemetry/server/ServerTelemetryManager.ts
@@ -160,7 +160,11 @@ export class ServerTelemetryManager {
      * Get the server telemetry entity from cache or initialize a new one
      */
     getLastRequests(): ServerTelemetryEntity {
-        const initialValue: ServerTelemetryEntity = new ServerTelemetryEntity();
+        const initialValue: ServerTelemetryEntity = {
+            failedRequests: [],
+            errors: [],
+            cacheHits: 0,
+        };
         const lastRequests = this.cacheManager.getServerTelemetry(
             this.telemetryCacheKey
         ) as ServerTelemetryEntity;
@@ -181,11 +185,13 @@ export class ServerTelemetryManager {
             this.cacheManager.removeItem(this.telemetryCacheKey);
         } else {
             // Partial data was flushed to server, construct a new telemetry cache item with errors that were not flushed
-            const serverTelemEntity = new ServerTelemetryEntity();
-            serverTelemEntity.failedRequests =
-                lastRequests.failedRequests.slice(numErrorsFlushed * 2); // failedRequests contains 2 items for each error
-            serverTelemEntity.errors =
-                lastRequests.errors.slice(numErrorsFlushed);
+            const serverTelemEntity: ServerTelemetryEntity = {
+                failedRequests: lastRequests.failedRequests.slice(
+                    numErrorsFlushed * 2
+                ), // failedRequests contains 2 items for each error
+                errors: lastRequests.errors.slice(numErrorsFlushed),
+                cacheHits: 0,
+            };
 
             this.cacheManager.setServerTelemetry(
                 this.telemetryCacheKey,

diff --git a/lib/msal-common/test/cache/entities/ServerTelemetryEntity.spec.ts b/lib/msal-common/test/cache/entities/ServerTelemetryEntity.spec.ts
@@ -1,9 +1,9 @@
-import { ServerTelemetryEntity } from "../../../src/cache/entities/ServerTelemetryEntity";
 import {
     SERVER_TELEM_CONSTANTS,
     Separators,
 } from "../../../src/utils/Constants";
 import { TEST_CONFIG } from "../../test_kit/StringConstants";
+import * as CacheHelpers from "../../../src/cache/utils/CacheHelpers";
 
 describe("ServerTelemetryEntity.ts Unit Tests", () => {
     const ServerTelemetryKey =
@@ -19,7 +19,7 @@ describe("ServerTelemetryEntity.ts Unit Tests", () => {
         };
 
         expect(
-            ServerTelemetryEntity.isServerTelemetryEntity(
+            CacheHelpers.isServerTelemetryEntity(
                 ServerTelemetryKey,
                 serverTelemetryObject
             )
@@ -34,7 +34,7 @@ describe("ServerTelemetryEntity.ts Unit Tests", () => {
         };
 
         expect(
-            ServerTelemetryEntity.isServerTelemetryEntity(
+            CacheHelpers.isServerTelemetryEntity(
                 ServerTelemetryKey,
                 missingCacheHits
             )
@@ -46,7 +46,7 @@ describe("ServerTelemetryEntity.ts Unit Tests", () => {
             cacheHits: 0,
         };
         expect(
-            ServerTelemetryEntity.isServerTelemetryEntity(
+            CacheHelpers.isServerTelemetryEntity(
                 ServerTelemetryKey,
                 missingErrors
             )
@@ -58,7 +58,7 @@ describe("ServerTelemetryEntity.ts Unit Tests", () => {
             cacheHits: 0,
         };
         expect(
-            ServerTelemetryEntity.isServerTelemetryEntity(
+            CacheHelpers.isServerTelemetryEntity(
                 ServerTelemetryKey,
                 missingFailedRequests
             )
@@ -68,26 +68,26 @@ describe("ServerTelemetryEntity.ts Unit Tests", () => {
             testParam: "test",
         };
         expect(
-            ServerTelemetryEntity.isServerTelemetryEntity(
+            CacheHelpers.isServerTelemetryEntity(
                 ServerTelemetryKey,
                 noCommonValues
             )
         ).toBe(false);
     });
 
     it("Verify an object is a ServerTelemetry Entity only when cache key is passed", () => {
-        expect(
-            ServerTelemetryEntity.isServerTelemetryEntity(ServerTelemetryKey)
-        ).toBe(true);
+        expect(CacheHelpers.isServerTelemetryEntity(ServerTelemetryKey)).toBe(
+            true
+        );
     });
 
     it("Verify an object is not a ServerTelemetry Entity only when cache key is passed", () => {
         const ServerTelemetryKey =
             Separators.CACHE_KEY_SEPARATOR +
             SERVER_TELEM_CONSTANTS.CACHE_KEY +
             TEST_CONFIG.MSAL_CLIENT_ID;
-        expect(
-            ServerTelemetryEntity.isServerTelemetryEntity(ServerTelemetryKey)
-        ).toBe(false);
+        expect(CacheHelpers.isServerTelemetryEntity(ServerTelemetryKey)).toBe(
+            false
+        );
     });
 });
diff --git a/lib/msal-common/test/telemetry/ServerTelemetryManager.spec.ts b/lib/msal-common/test/telemetry/ServerTelemetryManager.spec.ts
@@ -361,7 +361,11 @@ describe("ServerTelemetryManager.ts", () => {
     });
 
     it("incrementCacheHits", () => {
-        const initialCacheValue = new ServerTelemetryEntity();
+        const initialCacheValue: ServerTelemetryEntity = {
+            failedRequests: [],
+            errors: [],
+            cacheHits: 0,
+        };
         initialCacheValue.cacheHits = 1;
         testCacheManager.setServerTelemetry(cacheKey, initialCacheValue);
         const telemetryManager = new ServerTelemetryManager(

diff --git a/lib/msal-node/src/cache/NodeStorage.ts b/lib/msal-node/src/cache/NodeStorage.ts
@@ -339,7 +339,7 @@ export class NodeStorage extends CacheManager {
         ) as ServerTelemetryEntity;
         if (
             serverTelemetryEntity &&
-            ServerTelemetryEntity.isServerTelemetryEntity(
+            CacheHelpers.isServerTelemetryEntity(
                 serverTelemetrykey,
                 serverTelemetryEntity
             )