From 2aacebf280c8502df5e9e6e10f1af30949371fa9 Mon Sep 17 00:00:00 2001 From: AllanZhengYP Date: Thu, 8 Apr 2021 19:27:21 +0000 Subject: [PATCH 1/3] fix(util-user-agent-node): should only load app id once --- .../util-user-agent-node/src/index.spec.ts | 10 ++++++++++ packages/util-user-agent-node/src/index.ts | 18 +++++++++--------- 2 files changed, 19 insertions(+), 9 deletions(-) diff --git a/packages/util-user-agent-node/src/index.spec.ts b/packages/util-user-agent-node/src/index.spec.ts index bcb9ff3e48d8..64741c3a1bbc 100644 --- a/packages/util-user-agent-node/src/index.spec.ts +++ b/packages/util-user-agent-node/src/index.spec.ts @@ -54,4 +54,14 @@ describe("defaultUserAgent", () => { const userAgent = await defaultUserAgent({ serviceId: "s3", clientVersion: "0.1.0" })(); expect(userAgent).toContainEqual([`app/${appId}`]); }); + + it("should memoize app id", async () => { + mockAppIdLoader.mockClear(); + const appId = "appId12345"; + mockAppIdLoader.mockResolvedValue(appId); + const userAgnetProvider = defaultUserAgent({ serviceId: "s3", clientVersion: "0.1.0" }); + await userAgnetProvider(); + await userAgnetProvider(); + expect(mockAppIdLoader).toBeCalledTimes(1); + }); }); diff --git a/packages/util-user-agent-node/src/index.ts b/packages/util-user-agent-node/src/index.ts index 8b08c2f906c1..1dd0658ec0d2 100644 --- 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 => async () => { +export const defaultUserAgent = ({ serviceId, clientVersion }: DefaultUserAgentOptions): Provider => { const sections: UserAgent = [ // sdk-metadata ["aws-sdk-js", clientVersion], @@ -40,14 +37,17 @@ export const defaultUserAgent = ({ sections.push([`exec-env/${env.AWS_EXECUTION_ENV}`]); } - const appId = await loadConfig({ + const appIdPromise = loadConfig({ 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; + return async () => { + const appId = await appIdPromise; + if (appId) { + sections.push([`app/${appId}`]); + } + return sections; + }; }; From db3f5c3675db6974310c16a8504113683ece8c05 Mon Sep 17 00:00:00 2001 From: AllanZhengYP Date: Thu, 8 Apr 2021 21:08:32 +0000 Subject: [PATCH 2/3] fix(util-user-agent-node): address feedbacks Co-authored-by: Tyson Andre --- packages/util-user-agent-node/src/index.spec.ts | 5 ++--- packages/util-user-agent-node/src/index.ts | 8 ++++---- 2 files changed, 6 insertions(+), 7 deletions(-) diff --git a/packages/util-user-agent-node/src/index.spec.ts b/packages/util-user-agent-node/src/index.spec.ts index 64741c3a1bbc..81cfa44cfd38 100644 --- a/packages/util-user-agent-node/src/index.spec.ts +++ b/packages/util-user-agent-node/src/index.spec.ts @@ -59,9 +59,8 @@ describe("defaultUserAgent", () => { mockAppIdLoader.mockClear(); const appId = "appId12345"; mockAppIdLoader.mockResolvedValue(appId); - const userAgnetProvider = defaultUserAgent({ serviceId: "s3", clientVersion: "0.1.0" }); - await userAgnetProvider(); - await userAgnetProvider(); + const userAgentProvider = defaultUserAgent({ serviceId: "s3", clientVersion: "0.1.0" }); + expect(await userAgentProvider()).toEqual(await userAgentProvider()); expect(mockAppIdLoader).toBeCalledTimes(1); }); }); diff --git a/packages/util-user-agent-node/src/index.ts b/packages/util-user-agent-node/src/index.ts index 1dd0658ec0d2..7e618b1c136e 100644 --- a/packages/util-user-agent-node/src/index.ts +++ b/packages/util-user-agent-node/src/index.ts @@ -43,11 +43,11 @@ export const defaultUserAgent = ({ serviceId, clientVersion }: DefaultUserAgentO default: undefined, })(); + let resolvedUserAgent: UserAgent | undefined = undefined; return async () => { - const appId = await appIdPromise; - if (appId) { - sections.push([`app/${appId}`]); + if (!resolvedUserAgent) { + resolvedUserAgent = [...sections, [`app/${await appIdPromise}`]]; } - return sections; + return resolvedUserAgent; }; }; From 82128816bea8e85ac28fd5e15e426b7539fecc73 Mon Sep 17 00:00:00 2001 From: AllanZhengYP Date: Thu, 8 Apr 2021 22:00:59 +0000 Subject: [PATCH 3/3] test(util-user-agent-node): refactor unit test --- .../util-user-agent-node/src/index.spec.ts | 39 +++++++++++++++---- packages/util-user-agent-node/src/index.ts | 3 +- 2 files changed, 33 insertions(+), 9 deletions(-) diff --git a/packages/util-user-agent-node/src/index.spec.ts b/packages/util-user-agent-node/src/index.spec.ts index 81cfa44cfd38..0ed407393ed8 100644 --- 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,24 +36,35 @@ 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 () => { @@ -52,7 +72,8 @@ describe("defaultUserAgent", () => { 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 () => { @@ -60,7 +81,9 @@ describe("defaultUserAgent", () => { const appId = "appId12345"; mockAppIdLoader.mockResolvedValue(appId); const userAgentProvider = defaultUserAgent({ serviceId: "s3", clientVersion: "0.1.0" }); - expect(await userAgentProvider()).toEqual(await userAgentProvider()); + 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 index 7e618b1c136e..aadbdce08bb9 100644 --- a/packages/util-user-agent-node/src/index.ts +++ b/packages/util-user-agent-node/src/index.ts @@ -46,7 +46,8 @@ export const defaultUserAgent = ({ serviceId, clientVersion }: DefaultUserAgentO let resolvedUserAgent: UserAgent | undefined = undefined; return async () => { if (!resolvedUserAgent) { - resolvedUserAgent = [...sections, [`app/${await appIdPromise}`]]; + const appId = await appIdPromise; + resolvedUserAgent = appId ? [...sections, [`app/${appId}`]] : [...sections]; } return resolvedUserAgent; };