fix(util-user-agent-node): should memoize app id (#2223)

* fix(util-user-agent-node): should only load app id once * fix(util-user-agent-node): address feedbacks Co-authored-by: Tyson Andre <tyson.andre@uwaterloo.ca>
aws · Apr 9, 2021 · fd40114 · fd40114
1 parent 5ae46f6
commit fd40114
Show file tree

Hide file tree

Showing 2 changed files with 49 additions and 16 deletions.
diff --git a/packages/util-user-agent-node/src/index.spec.ts b/packages/util-user-agent-node/src/index.spec.ts
@@ -16,8 +16,17 @@ jest.mock("@aws-sdk/node-config-provider", () => ({
   loadConfig: () => mockAppIdLoader,
 }));
 
+import { UserAgent } from "@aws-sdk/types";
+
 import { defaultUserAgent } from ".";
 
+const validateUserAgent = (userAgent: UserAgent, expected: UserAgent) => {
+  expect(userAgent.length).toBe(expected.length);
+  for (const pair of expected) {
+    expect(userAgent).toContainEqual(pair);
+  }
+};
+
 describe("defaultUserAgent", () => {
   beforeEach(() => {
     jest.resetAllMocks();
@@ -27,31 +36,54 @@ describe("defaultUserAgent", () => {
     jest.clearAllMocks();
   });
 
+  const basicUserAgent: UserAgent = [
+    ["aws-sdk-js", "0.1.0"],
+    ["api/s3", "0.1.0"],
+    ["os/darwin", "19.6.0"],
+    ["lang/js"],
+    ["md/nodejs", "14.13.1"],
+  ];
+
   it("should response basic node default user agent", async () => {
     const userAgent = await defaultUserAgent({ serviceId: "s3", clientVersion: "0.1.0" })();
-    expect(userAgent).toContainEqual(["aws-sdk-js", "0.1.0"]);
-    expect(userAgent).toContainEqual(["api/s3", "0.1.0"]);
-    expect(userAgent).toContainEqual(["os/darwin", "19.6.0"]);
-    expect(userAgent).toContainEqual(["lang/js"]);
+    validateUserAgent(userAgent, basicUserAgent);
   });
 
   it("should skip api version if service id is not supplied", async () => {
     const userAgent = await defaultUserAgent({ serviceId: undefined, clientVersion: "0.1.0" })();
-    expect(userAgent).not.toContainEqual(["api/s3", "0.1.0"]);
+    validateUserAgent(
+      userAgent,
+      basicUserAgent.filter((pair) => pair[0] !== "api/s3")
+    );
   });
 
   it("should add AWS_EXECUTION_ENV", async () => {
     //@ts-ignore mock environmental variables
     mockEnv.AWS_EXECUTION_ENV = "lambda";
     const userAgent = await defaultUserAgent({ serviceId: "s3", clientVersion: "0.1.0" })();
-    expect(userAgent).toContainEqual(["exec-env/lambda"]);
+    const expectedUserAgent: UserAgent = [...basicUserAgent, ["exec-env/lambda"]];
+    validateUserAgent(userAgent, expectedUserAgent);
+    //@ts-ignore mock environmental variables
+    delete mockEnv.AWS_EXECUTION_ENV;
   });
 
   it("should load app id if available", async () => {
     mockAppIdLoader.mockClear();
     const appId = "appId12345";
     mockAppIdLoader.mockResolvedValue(appId);
     const userAgent = await defaultUserAgent({ serviceId: "s3", clientVersion: "0.1.0" })();
-    expect(userAgent).toContainEqual([`app/${appId}`]);
+    const expectedUserAgent: UserAgent = [...basicUserAgent, [`app/${appId}`]];
+    validateUserAgent(userAgent, expectedUserAgent);
+  });
+
+  it("should memoize app id", async () => {
+    mockAppIdLoader.mockClear();
+    const appId = "appId12345";
+    mockAppIdLoader.mockResolvedValue(appId);
+    const userAgentProvider = defaultUserAgent({ serviceId: "s3", clientVersion: "0.1.0" });
+    const expectedUserAgent: UserAgent = [...basicUserAgent, [`app/${appId}`]];
+    validateUserAgent(await userAgentProvider(), expectedUserAgent);
+    validateUserAgent(await userAgentProvider(), expectedUserAgent);
+    expect(mockAppIdLoader).toBeCalledTimes(1);
   });
 });
diff --git a/packages/util-user-agent-node/src/index.ts b/packages/util-user-agent-node/src/index.ts
@@ -14,10 +14,7 @@ interface DefaultUserAgentOptions {
 /**
  * Collect metrics from runtime to put into user agent.
  */
-export const defaultUserAgent = ({
-  serviceId,
-  clientVersion,
-}: DefaultUserAgentOptions): Provider<UserAgent> => async () => {
+export const defaultUserAgent = ({ serviceId, clientVersion }: DefaultUserAgentOptions): Provider<UserAgent> => {
   const sections: UserAgent = [
     // sdk-metadata
     ["aws-sdk-js", clientVersion],
@@ -40,14 +37,18 @@ export const defaultUserAgent = ({
     sections.push([`exec-env/${env.AWS_EXECUTION_ENV}`]);
   }
 
-  const appId = await loadConfig<string | undefined>({
+  const appIdPromise = loadConfig<string | undefined>({
     environmentVariableSelector: (env) => env[UA_APP_ID_ENV_NAME],
     configFileSelector: (profile) => profile[UA_APP_ID_INI_NAME],
     default: undefined,
   })();
-  if (appId) {
-    sections.push([`app/${appId}`]);
-  }
 
-  return sections;
+  let resolvedUserAgent: UserAgent | undefined = undefined;
+  return async () => {
+    if (!resolvedUserAgent) {
+      const appId = await appIdPromise;
+      resolvedUserAgent = appId ? [...sections, [`app/${appId}`]] : [...sections];
+    }
+    return resolvedUserAgent;
+  };
 };