From 0b909409e0a9f649b8d8efe1e6c362bd91e47b9e Mon Sep 17 00:00:00 2001 From: Brian Krygsman Date: Thu, 9 Mar 2023 20:08:58 -0500 Subject: [PATCH 01/13] Closes 1363 Add a local cache to AppConfigProvider to handle the scenario where the maxAge is expired, but the AppConfig session still has the latest configuration. In this case, AppConfig returns an empty value. We don't want to return empty values to our callers. This uses the same data structure we have in place to track NextPollConfigurationToken. This approach roughly the Python library's behavior. --- .../src/appconfig/AppConfigProvider.ts | 20 ++++++++- .../tests/unit/AppConfigProvider.test.ts | 45 +++++++++++++++++++ 2 files changed, 63 insertions(+), 2 deletions(-) diff --git a/packages/parameters/src/appconfig/AppConfigProvider.ts b/packages/parameters/src/appconfig/AppConfigProvider.ts index 9e9e2b0152..ce42545055 100644 --- a/packages/parameters/src/appconfig/AppConfigProvider.ts +++ b/packages/parameters/src/appconfig/AppConfigProvider.ts @@ -13,6 +13,7 @@ import type { class AppConfigProvider extends BaseProvider { public client: AppConfigDataClient; protected configurationTokenStore: Map = new Map(); + protected valueStore: Map = new Map(); private application?: string; private environment: string; @@ -79,6 +80,10 @@ class AppConfigProvider extends BaseProvider { * The new AppConfig APIs require two API calls to return the configuration * First we start the session and after that we retrieve the configuration * We need to store { name: token } pairs to use in the next execution + * We also need to store { name : value } pairs because AppConfig returns + * an empty value if the session already has the latest configuration + * but, we don't want to return an empty value to our callers. + * {@link https://docs.aws.amazon.com/appconfig/latest/userguide/appconfig-retrieving-the-configuration.html} **/ if (!this.configurationTokenStore.has(name)) { @@ -106,14 +111,25 @@ class AppConfigProvider extends BaseProvider { }); const response = await this.client.send(getConfigurationCommand); - + if (response.NextPollConfigurationToken) { this.configurationTokenStore.set(name, response.NextPollConfigurationToken); } else { this.configurationTokenStore.delete(name); } - return response.Configuration; + /** When the response is not empty, stash the result locally before returning + * See AppConfig docs: + * {@link https://docs.aws.amazon.com/appconfig/latest/userguide/appconfig-retrieving-the-configuration.html} + **/ + if (response.Configuration !== undefined && response.Configuration?.length > 0 ) { + this.valueStore.set(name, response.Configuration); + + return response.Configuration; + } + + // Otherwise, use a stashed value + return this.valueStore.get(name); } /** diff --git a/packages/parameters/tests/unit/AppConfigProvider.test.ts b/packages/parameters/tests/unit/AppConfigProvider.test.ts index 56365d882c..b907d283a8 100644 --- a/packages/parameters/tests/unit/AppConfigProvider.test.ts +++ b/packages/parameters/tests/unit/AppConfigProvider.test.ts @@ -225,6 +225,51 @@ describe('Class: AppConfigProvider', () => { // Act & Assess await expect(provider.get(name)).rejects.toThrow(); }); + + test('when session returns an empty configuration on the second call, it returns the last value', async () => { + + client.reset(); + + // Prepare + const options: AppConfigProviderOptions = { + application: 'MyApp', + environment: 'MyAppProdEnv', + }; + const provider = new AppConfigProvider(options); + const name = 'MyAppFeatureFlag'; + + const fakeInitialToken = 'aW5pdGlhbFRva2Vu'; + const fakeNextToken1 = 'bmV4dFRva2Vu'; + const fakeNextToken2 = 'bmV4dFRva2Vq'; + const mockData = encoder.encode('myAppConfiguration'); + + client + .on(StartConfigurationSessionCommand) + .resolves({ + InitialConfigurationToken: fakeInitialToken, + }) + .on(GetLatestConfigurationCommand) + .resolvesOnce({ + Configuration: mockData, + NextPollConfigurationToken: fakeNextToken1, + }) + .resolvesOnce({ + Configuration: undefined, + NextPollConfigurationToken: fakeNextToken2, + }); + + // Act + + // Load local cache + const result1 = await provider.get(name, { forceFetch: true }); + + // Read from local cache, given empty response from service + const result2 = await provider.get(name, { forceFetch: true }); + + // Assess + expect(result1).toBe(mockData); + expect(result2).toBe(mockData); + }); }); describe('Method: _getMultiple', () => { From 2d80baaeb42a64b892f689c50c9b7838928cbb69 Mon Sep 17 00:00:00 2001 From: Brian Krygsman Date: Thu, 9 Mar 2023 22:27:48 -0500 Subject: [PATCH 02/13] Add E2E test coverage checking return values pre- and post-expiration Includes update to e2e lambda config type that allows test writers to specify a test function duration. Needed because to wait for parameter expiration, we needed a longer-than-default timeout. --- packages/commons/tests/utils/e2eUtils.ts | 4 +- ...pConfigProvider.class.test.functionCode.ts | 42 ++++++++++++++++++- .../tests/e2e/appConfigProvider.class.test.ts | 33 +++++++++++++-- 3 files changed, 73 insertions(+), 6 deletions(-) diff --git a/packages/commons/tests/utils/e2eUtils.ts b/packages/commons/tests/utils/e2eUtils.ts index 5d2e75f4a5..ae5c1441ae 100644 --- a/packages/commons/tests/utils/e2eUtils.ts +++ b/packages/commons/tests/utils/e2eUtils.ts @@ -5,7 +5,7 @@ * E2E utils is used by e2e tests. They are helper function that calls either CDK or SDK * to interact with services. */ -import { App, CfnOutput, Stack } from 'aws-cdk-lib'; +import { App, CfnOutput, Stack, Duration } from 'aws-cdk-lib'; import { NodejsFunction, NodejsFunctionProps @@ -37,6 +37,7 @@ export type StackWithLambdaFunctionOptions = { runtime: string bundling?: NodejsFunctionProps['bundling'] layers?: NodejsFunctionProps['layers'] + timeout?: Duration }; type FunctionPayload = {[key: string]: string | boolean | number}; @@ -55,6 +56,7 @@ export const createStackWithLambdaFunction = (params: StackWithLambdaFunctionOpt bundling: params.bundling, layers: params.layers, logRetention: RetentionDays.ONE_DAY, + timeout: params.timeout }); if (params.logGroupOutputKey) { diff --git a/packages/parameters/tests/e2e/appConfigProvider.class.test.functionCode.ts b/packages/parameters/tests/e2e/appConfigProvider.class.test.functionCode.ts index 11d8feb01f..87bb3a9aeb 100644 --- a/packages/parameters/tests/e2e/appConfigProvider.class.test.functionCode.ts +++ b/packages/parameters/tests/e2e/appConfigProvider.class.test.functionCode.ts @@ -81,8 +81,16 @@ export const handler = async (_event: unknown, _context: Context): Promise try { providerWithMiddleware.clearCache(); middleware.counter = 0; - await providerWithMiddleware.get(freeFormBase64encodedPlainText); - await providerWithMiddleware.get(freeFormBase64encodedPlainText); + const result1 = await providerWithMiddleware.get(freeFormBase64encodedPlainText); + const result2 = await providerWithMiddleware.get(freeFormBase64encodedPlainText); + logger.log({ + test: 'get-cached-result1', + value: result1 + }); + logger.log({ + test: 'get-cached-result2', + value: result2 + }); logger.log({ test: 'get-cached', value: middleware.counter // should be 1 @@ -111,4 +119,34 @@ export const handler = async (_event: unknown, _context: Context): Promise error: err.message }); } + + // Test 8 + // get parameter twice, but wait long enough that cache expires count SDK calls and return values + try { + providerWithMiddleware.clearCache(); + middleware.counter = 0; + const expiredResult1 = await providerWithMiddleware.get(freeFormBase64encodedPlainText, { maxAge: 5 }); + + // Wait + await new Promise(resolve => setTimeout(resolve, 6000)); + + const expiredResult2 = await providerWithMiddleware.get(freeFormBase64encodedPlainText, { maxAge: 5 }); + logger.log({ + test: 'get-expired-result1', + value: expiredResult1 + }); + logger.log({ + test: 'get-expired-result2', + value: expiredResult2 + }); + logger.log({ + test: 'get-expired', + value: middleware.counter // should be 2 + }); + } catch (err) { + logger.log({ + test: 'get-expired', + error: err.message + }); + } }; \ No newline at end of file diff --git a/packages/parameters/tests/e2e/appConfigProvider.class.test.ts b/packages/parameters/tests/e2e/appConfigProvider.class.test.ts index 02b2de3291..261ec0ca5c 100644 --- a/packages/parameters/tests/e2e/appConfigProvider.class.test.ts +++ b/packages/parameters/tests/e2e/appConfigProvider.class.test.ts @@ -4,7 +4,7 @@ * @group e2e/parameters/appconfig/class */ import path from 'path'; -import { App, Stack, Aspects } from 'aws-cdk-lib'; +import { App, Stack, Aspects, Duration } from 'aws-cdk-lib'; import { toBase64 } from '@aws-sdk/util-base64-node'; import { v4 } from 'uuid'; import { @@ -111,6 +111,7 @@ let stack: Stack; * Test 6 * get parameter twice, but force fetch 2nd time, we count number of SDK requests and * check that we made two API calls + * check that we got matching results * * Note: To avoid race conditions, we add a dependency between each pair of configuration profiles. * This allows us to influence the order of creation and ensure that each configuration profile @@ -141,6 +142,7 @@ describe(`parameters E2E tests (appConfigProvider) for runtime ${runtime}`, () = FEATURE_FLAG_NAME: featureFlagName, }, runtime, + timeout: Duration.seconds(10), }); // Create the base resources for an AppConfig application. @@ -291,8 +293,12 @@ describe(`parameters E2E tests (appConfigProvider) for runtime ${runtime}`, () = it('should retrieve single parameter cached', () => { const logs = invocationLogs[0].getFunctionLogs(); - const testLog = InvocationLogs.parseFunctionLog(logs[4]); + const resultLog1 = InvocationLogs.parseFunctionLog(logs[4]); + const resultLog2 = InvocationLogs.parseFunctionLog(logs[5]); + expect(resultLog1.value).toStrictEqual(resultLog2.value); + + const testLog = InvocationLogs.parseFunctionLog(logs[6]); expect(testLog).toStrictEqual({ test: 'get-cached', value: 1 @@ -305,7 +311,7 @@ describe(`parameters E2E tests (appConfigProvider) for runtime ${runtime}`, () = it('should retrieve single parameter twice without caching', async () => { const logs = invocationLogs[0].getFunctionLogs(); - const testLog = InvocationLogs.parseFunctionLog(logs[5]); + const testLog = InvocationLogs.parseFunctionLog(logs[7]); expect(testLog).toStrictEqual({ test: 'get-forced', @@ -314,6 +320,27 @@ describe(`parameters E2E tests (appConfigProvider) for runtime ${runtime}`, () = }, TEST_CASE_TIMEOUT); + // Test 7 - get parameter twice, wait for expiration betweenn + // we count number of SDK requests and check that we made two API calls + // and check that the values match + it('should retrieve single parameter twice, with expiration between and matching values', async () => { + + const logs = invocationLogs[0].getFunctionLogs(); + + const resultLog1 = InvocationLogs.parseFunctionLog(logs[8]); + const resultLog2 = InvocationLogs.parseFunctionLog(logs[9]); + expect(resultLog1.test).toBe('get-expired-result1'); + expect(resultLog2.test).toBe('get-expired-result2'); + expect(resultLog1.value).toStrictEqual(resultLog2.value); + + const testLog = InvocationLogs.parseFunctionLog(logs[10]); + expect(testLog).toStrictEqual({ + test: 'get-expired', + value: 2 + }); + + }, TEST_CASE_TIMEOUT); + }); afterAll(async () => { From 3fea0d7e826a6d184b04f6adc8efc62bc1751069 Mon Sep 17 00:00:00 2001 From: Brian Krygsman Date: Thu, 9 Mar 2023 22:32:17 -0500 Subject: [PATCH 03/13] Use timestamp/millisecond based math for checking expiration. --- packages/parameters/src/ExpirableValue.ts | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/packages/parameters/src/ExpirableValue.ts b/packages/parameters/src/ExpirableValue.ts index 7dcc3a2473..746688fc47 100644 --- a/packages/parameters/src/ExpirableValue.ts +++ b/packages/parameters/src/ExpirableValue.ts @@ -4,10 +4,17 @@ class ExpirableValue implements ExpirableValueInterface { public ttl: number; public value: string | Uint8Array | Record; + /** + * Creates a new cached value which will expire automatically + * @param value Parameter value to be cached + * @param maxAge Maximum number of seconds to cache the value for + */ public constructor(value: string | Uint8Array | Record, maxAge: number) { this.value = value; - const timeNow = new Date(); - this.ttl = timeNow.setSeconds(timeNow.getSeconds() + maxAge); + + const maxAgeInMilliseconds = maxAge * 1000; + const nowTimestamp = Date.now(); + this.ttl = nowTimestamp + maxAgeInMilliseconds; } public isExpired(): boolean { From 838c3415c5445b6d6b556bee999516b2ac3d300e Mon Sep 17 00:00:00 2001 From: Brian Krygsman Date: Fri, 10 Mar 2023 12:21:55 -0500 Subject: [PATCH 04/13] Revert "Use timestamp/millisecond based math for checking expiration." This reverts commit 3fea0d7e826a6d184b04f6adc8efc62bc1751069. Further testing shows that the original algorithm was correct. --- packages/parameters/src/ExpirableValue.ts | 11 ++--------- 1 file changed, 2 insertions(+), 9 deletions(-) diff --git a/packages/parameters/src/ExpirableValue.ts b/packages/parameters/src/ExpirableValue.ts index 746688fc47..7dcc3a2473 100644 --- a/packages/parameters/src/ExpirableValue.ts +++ b/packages/parameters/src/ExpirableValue.ts @@ -4,17 +4,10 @@ class ExpirableValue implements ExpirableValueInterface { public ttl: number; public value: string | Uint8Array | Record; - /** - * Creates a new cached value which will expire automatically - * @param value Parameter value to be cached - * @param maxAge Maximum number of seconds to cache the value for - */ public constructor(value: string | Uint8Array | Record, maxAge: number) { this.value = value; - - const maxAgeInMilliseconds = maxAge * 1000; - const nowTimestamp = Date.now(); - this.ttl = nowTimestamp + maxAgeInMilliseconds; + const timeNow = new Date(); + this.ttl = timeNow.setSeconds(timeNow.getSeconds() + maxAge); } public isExpired(): boolean { From f16d3a57bae627fb9de7b295f74d5d6cfd58ca50 Mon Sep 17 00:00:00 2001 From: Brian Krygsman Date: Sun, 12 Mar 2023 16:58:24 -0400 Subject: [PATCH 05/13] Update appConfig integration tests to use single log line. Drop wait, use maxAge instead. Add comment for Test case 7. Fix test case naming (test '4' was skipped on one of the files). --- ...pConfigProvider.class.test.functionCode.ts | 40 ++++++------------- .../tests/e2e/appConfigProvider.class.test.ts | 34 ++++++++-------- 2 files changed, 30 insertions(+), 44 deletions(-) diff --git a/packages/parameters/tests/e2e/appConfigProvider.class.test.functionCode.ts b/packages/parameters/tests/e2e/appConfigProvider.class.test.functionCode.ts index 87bb3a9aeb..4df7c7a410 100644 --- a/packages/parameters/tests/e2e/appConfigProvider.class.test.functionCode.ts +++ b/packages/parameters/tests/e2e/appConfigProvider.class.test.functionCode.ts @@ -73,27 +73,21 @@ export const handler = async (_event: unknown, _context: Context): Promise // Test 3 - get a free-form base64-encoded plain text and apply binary transformation (should return a decoded string) await _call_get(freeFormBase64encodedPlainText, 'get-freeform-base64-plaintext-binary', { transform: 'binary' }); - // Test 5 - get a feature flag and apply json transformation (should return an object) + // Test 4 - get a feature flag and apply json transformation (should return an object) await _call_get(featureFlagName, 'get-feature-flag-binary', { transform: 'json' }); - // Test 6 + // Test 5 // get parameter twice with middleware, which counts the number of requests, we check later if we only called AppConfig API once try { providerWithMiddleware.clearCache(); middleware.counter = 0; const result1 = await providerWithMiddleware.get(freeFormBase64encodedPlainText); const result2 = await providerWithMiddleware.get(freeFormBase64encodedPlainText); - logger.log({ - test: 'get-cached-result1', - value: result1 - }); - logger.log({ - test: 'get-cached-result2', - value: result2 - }); logger.log({ test: 'get-cached', - value: middleware.counter // should be 1 + value: middleware.counter, // should be 1 + result1: result1, + result2: result2 }); } catch (err) { logger.log({ @@ -102,7 +96,7 @@ export const handler = async (_event: unknown, _context: Context): Promise }); } - // Test 7 + // Test 6 // get parameter twice, but force fetch 2nd time, we count number of SDK requests and check that we made two API calls try { providerWithMiddleware.clearCache(); @@ -120,28 +114,18 @@ export const handler = async (_event: unknown, _context: Context): Promise }); } - // Test 8 + // Test 7 // get parameter twice, but wait long enough that cache expires count SDK calls and return values try { providerWithMiddleware.clearCache(); middleware.counter = 0; - const expiredResult1 = await providerWithMiddleware.get(freeFormBase64encodedPlainText, { maxAge: 5 }); - - // Wait - await new Promise(resolve => setTimeout(resolve, 6000)); - - const expiredResult2 = await providerWithMiddleware.get(freeFormBase64encodedPlainText, { maxAge: 5 }); - logger.log({ - test: 'get-expired-result1', - value: expiredResult1 - }); - logger.log({ - test: 'get-expired-result2', - value: expiredResult2 - }); + const expiredResult1 = await providerWithMiddleware.get(freeFormBase64encodedPlainText, { maxAge: 0 }); + const expiredResult2 = await providerWithMiddleware.get(freeFormBase64encodedPlainText, { maxAge: 0 }); logger.log({ test: 'get-expired', - value: middleware.counter // should be 2 + value: middleware.counter, // should be 2 + result1: expiredResult1, + result2: expiredResult2 }); } catch (err) { logger.log({ diff --git a/packages/parameters/tests/e2e/appConfigProvider.class.test.ts b/packages/parameters/tests/e2e/appConfigProvider.class.test.ts index 261ec0ca5c..a728636f90 100644 --- a/packages/parameters/tests/e2e/appConfigProvider.class.test.ts +++ b/packages/parameters/tests/e2e/appConfigProvider.class.test.ts @@ -54,6 +54,7 @@ const freeFormJsonValue = { const freeFormYamlValue = `foo: bar `; const freeFormPlainTextValue = 'foo'; +const freeFormBase64PlainTextValue = toBase64(new TextEncoder().encode(freeFormPlainTextValue)); const featureFlagValue = { version: '1', flags: { @@ -113,6 +114,11 @@ let stack: Stack; * check that we made two API calls * check that we got matching results * + * Test 7 + * get parameter twice, wait for expiration betweenn + * we count number of SDK requests and check that we made two API calls + * and check that the values match + * * Note: To avoid race conditions, we add a dependency between each pair of configuration profiles. * This allows us to influence the order of creation and ensure that each configuration profile * is created after the previous one. This is necessary because we share the same AppConfig @@ -193,7 +199,7 @@ describe(`parameters E2E tests (appConfigProvider) for runtime ${runtime}`, () = name: freeFormBase64PlainTextName, type: 'AWS.Freeform', content: { - content: toBase64(new TextEncoder().encode(freeFormPlainTextValue)), + content: freeFormBase64PlainTextValue, contentType: 'text/plain', } }); @@ -293,15 +299,14 @@ describe(`parameters E2E tests (appConfigProvider) for runtime ${runtime}`, () = it('should retrieve single parameter cached', () => { const logs = invocationLogs[0].getFunctionLogs(); + const testLog = InvocationLogs.parseFunctionLog(logs[4]); + const result = freeFormBase64PlainTextValue; - const resultLog1 = InvocationLogs.parseFunctionLog(logs[4]); - const resultLog2 = InvocationLogs.parseFunctionLog(logs[5]); - expect(resultLog1.value).toStrictEqual(resultLog2.value); - - const testLog = InvocationLogs.parseFunctionLog(logs[6]); expect(testLog).toStrictEqual({ test: 'get-cached', - value: 1 + value: 1, + result1: result, + result2: result }); }, TEST_CASE_TIMEOUT); @@ -311,7 +316,7 @@ describe(`parameters E2E tests (appConfigProvider) for runtime ${runtime}`, () = it('should retrieve single parameter twice without caching', async () => { const logs = invocationLogs[0].getFunctionLogs(); - const testLog = InvocationLogs.parseFunctionLog(logs[7]); + const testLog = InvocationLogs.parseFunctionLog(logs[5]); expect(testLog).toStrictEqual({ test: 'get-forced', @@ -326,17 +331,14 @@ describe(`parameters E2E tests (appConfigProvider) for runtime ${runtime}`, () = it('should retrieve single parameter twice, with expiration between and matching values', async () => { const logs = invocationLogs[0].getFunctionLogs(); + const testLog = InvocationLogs.parseFunctionLog(logs[6]); + const result = freeFormBase64PlainTextValue; - const resultLog1 = InvocationLogs.parseFunctionLog(logs[8]); - const resultLog2 = InvocationLogs.parseFunctionLog(logs[9]); - expect(resultLog1.test).toBe('get-expired-result1'); - expect(resultLog2.test).toBe('get-expired-result2'); - expect(resultLog1.value).toStrictEqual(resultLog2.value); - - const testLog = InvocationLogs.parseFunctionLog(logs[10]); expect(testLog).toStrictEqual({ test: 'get-expired', - value: 2 + value: 2, + result1: result, + result2: result }); }, TEST_CASE_TIMEOUT); From 3aeacb819c7b22d1799cf0b6ac1eadd68231ed65 Mon Sep 17 00:00:00 2001 From: Brian Krygsman Date: Sun, 12 Mar 2023 17:03:48 -0400 Subject: [PATCH 06/13] Revert increase of Lambda function duration back to default. Not needed anymore since we're not waiting for Test 7. --- .../parameters/tests/e2e/appConfigProvider.class.test.ts | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/packages/parameters/tests/e2e/appConfigProvider.class.test.ts b/packages/parameters/tests/e2e/appConfigProvider.class.test.ts index a728636f90..a8bafed89d 100644 --- a/packages/parameters/tests/e2e/appConfigProvider.class.test.ts +++ b/packages/parameters/tests/e2e/appConfigProvider.class.test.ts @@ -4,7 +4,7 @@ * @group e2e/parameters/appconfig/class */ import path from 'path'; -import { App, Stack, Aspects, Duration } from 'aws-cdk-lib'; +import { App, Stack, Aspects } from 'aws-cdk-lib'; import { toBase64 } from '@aws-sdk/util-base64-node'; import { v4 } from 'uuid'; import { @@ -147,8 +147,7 @@ describe(`parameters E2E tests (appConfigProvider) for runtime ${runtime}`, () = FREEFORM_BASE64_ENCODED_PLAIN_TEXT_NAME: freeFormBase64PlainTextName, FEATURE_FLAG_NAME: featureFlagName, }, - runtime, - timeout: Duration.seconds(10), + runtime }); // Create the base resources for an AppConfig application. From d246306552d931f56850229cff1bc3f851bdb445 Mon Sep 17 00:00:00 2001 From: Brian Krygsman Date: Sun, 12 Mar 2023 17:51:59 -0400 Subject: [PATCH 07/13] Focus value test integration tests on Test 7 --- .../e2e/appConfigProvider.class.test.functionCode.ts | 12 +++++------- .../tests/e2e/appConfigProvider.class.test.ts | 5 +---- 2 files changed, 6 insertions(+), 11 deletions(-) diff --git a/packages/parameters/tests/e2e/appConfigProvider.class.test.functionCode.ts b/packages/parameters/tests/e2e/appConfigProvider.class.test.functionCode.ts index 4df7c7a410..d90b0c9e39 100644 --- a/packages/parameters/tests/e2e/appConfigProvider.class.test.functionCode.ts +++ b/packages/parameters/tests/e2e/appConfigProvider.class.test.functionCode.ts @@ -81,13 +81,11 @@ export const handler = async (_event: unknown, _context: Context): Promise try { providerWithMiddleware.clearCache(); middleware.counter = 0; - const result1 = await providerWithMiddleware.get(freeFormBase64encodedPlainText); - const result2 = await providerWithMiddleware.get(freeFormBase64encodedPlainText); + await providerWithMiddleware.get(freeFormBase64encodedPlainText); + await providerWithMiddleware.get(freeFormBase64encodedPlainText); logger.log({ test: 'get-cached', - value: middleware.counter, // should be 1 - result1: result1, - result2: result2 + value: middleware.counter // should be 1 }); } catch (err) { logger.log({ @@ -119,8 +117,8 @@ export const handler = async (_event: unknown, _context: Context): Promise try { providerWithMiddleware.clearCache(); middleware.counter = 0; - const expiredResult1 = await providerWithMiddleware.get(freeFormBase64encodedPlainText, { maxAge: 0 }); - const expiredResult2 = await providerWithMiddleware.get(freeFormBase64encodedPlainText, { maxAge: 0 }); + const expiredResult1 = await providerWithMiddleware.get(freeFormBase64encodedPlainText, { maxAge: 0, transform: 'base64' }); + const expiredResult2 = await providerWithMiddleware.get(freeFormBase64encodedPlainText, { maxAge: 0, transform: 'base64' }); logger.log({ test: 'get-expired', value: middleware.counter, // should be 2 diff --git a/packages/parameters/tests/e2e/appConfigProvider.class.test.ts b/packages/parameters/tests/e2e/appConfigProvider.class.test.ts index a8bafed89d..2ff226e2aa 100644 --- a/packages/parameters/tests/e2e/appConfigProvider.class.test.ts +++ b/packages/parameters/tests/e2e/appConfigProvider.class.test.ts @@ -299,13 +299,10 @@ describe(`parameters E2E tests (appConfigProvider) for runtime ${runtime}`, () = const logs = invocationLogs[0].getFunctionLogs(); const testLog = InvocationLogs.parseFunctionLog(logs[4]); - const result = freeFormBase64PlainTextValue; expect(testLog).toStrictEqual({ test: 'get-cached', - value: 1, - result1: result, - result2: result + value: 1 }); }, TEST_CASE_TIMEOUT); From 3bde5084177473f99e3713f2557c02f3bfdfcb5e Mon Sep 17 00:00:00 2001 From: Brian Krygsman Date: Sun, 12 Mar 2023 17:54:52 -0400 Subject: [PATCH 08/13] Comma formatting --- packages/parameters/tests/e2e/appConfigProvider.class.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/parameters/tests/e2e/appConfigProvider.class.test.ts b/packages/parameters/tests/e2e/appConfigProvider.class.test.ts index 2ff226e2aa..ab6207506b 100644 --- a/packages/parameters/tests/e2e/appConfigProvider.class.test.ts +++ b/packages/parameters/tests/e2e/appConfigProvider.class.test.ts @@ -147,7 +147,7 @@ describe(`parameters E2E tests (appConfigProvider) for runtime ${runtime}`, () = FREEFORM_BASE64_ENCODED_PLAIN_TEXT_NAME: freeFormBase64PlainTextName, FEATURE_FLAG_NAME: featureFlagName, }, - runtime + runtime, }); // Create the base resources for an AppConfig application. From 6e15a09dc9cdb731907032565638a23e3f8e170c Mon Sep 17 00:00:00 2001 From: Brian Krygsman Date: Sun, 12 Mar 2023 18:10:41 -0400 Subject: [PATCH 09/13] Add unit tests for ExpirableValue Doesn't refactor ExpirableValue to take an injection of Date.now, just implements tests based on what we can do with the existing interface. --- .../tests/unit/AppConfigProvider.test.ts | 46 +++++++++++++++++++ 1 file changed, 46 insertions(+) diff --git a/packages/parameters/tests/unit/AppConfigProvider.test.ts b/packages/parameters/tests/unit/AppConfigProvider.test.ts index b907d283a8..4a88379a23 100644 --- a/packages/parameters/tests/unit/AppConfigProvider.test.ts +++ b/packages/parameters/tests/unit/AppConfigProvider.test.ts @@ -4,6 +4,7 @@ * @group unit/parameters/AppConfigProvider/class */ import { AppConfigProvider } from '../../src/appconfig/index'; +import { ExpirableValue } from '../../src/ExpirableValue'; import { AppConfigProviderOptions } from '../../src/types/AppConfigProvider'; import { AppConfigDataClient, @@ -288,3 +289,48 @@ describe('Class: AppConfigProvider', () => { }); }); }); + +describe('Class: ExpirableValue', () => { + beforeEach(() => { + jest.clearAllMocks(); + }); + + describe('Method: constructor', () => { + test('when created, it has ttl set to at least maxAge seconds from test start', () => { + // Prepare + const seconds = 10; + const nowTimestamp = Date.now(); + const futureTimestampSeconds = nowTimestamp/1000+(seconds); + + // Act + const expirableValue = new ExpirableValue('foo', seconds); + + // Assess + expect(expirableValue.ttl).toBeGreaterThan(futureTimestampSeconds); + }); + }); + + describe('Method: isExpired', () => { + test('when called, it returns true when maxAge is in the future', () => { + // Prepare + const seconds = 60; + + // Act + const expirableValue = new ExpirableValue('foo', seconds); + + // Assess + expect(expirableValue.isExpired()).toBeFalsy(); + }); + + test('when called, it returns false when maxAge is in the past', () => { + // Prepare + const seconds = -60; + + // Act + const expirableValue = new ExpirableValue('foo', seconds); + + // Assess + expect(expirableValue.isExpired()).toBeTruthy(); + }); + }); +}); \ No newline at end of file From ba6e8db08f35e9ccb7e740baba5e787fc0411470 Mon Sep 17 00:00:00 2001 From: Brian Krygsman Date: Mon, 13 Mar 2023 13:56:55 -0400 Subject: [PATCH 10/13] Address formatting feedback and updates comments to indicate we're no longer waiting during Test 7 runs. --- ...appConfigProvider.class.test.functionCode.ts | 17 +++++++++++++---- .../tests/e2e/appConfigProvider.class.test.ts | 4 ++-- 2 files changed, 15 insertions(+), 6 deletions(-) diff --git a/packages/parameters/tests/e2e/appConfigProvider.class.test.functionCode.ts b/packages/parameters/tests/e2e/appConfigProvider.class.test.functionCode.ts index d90b0c9e39..09e75bf126 100644 --- a/packages/parameters/tests/e2e/appConfigProvider.class.test.functionCode.ts +++ b/packages/parameters/tests/e2e/appConfigProvider.class.test.functionCode.ts @@ -111,14 +111,23 @@ export const handler = async (_event: unknown, _context: Context): Promise error: err.message }); } - // Test 7 - // get parameter twice, but wait long enough that cache expires count SDK calls and return values + // get parameter twice, using maxAge to avoid primary cache, count SDK calls and return values try { providerWithMiddleware.clearCache(); middleware.counter = 0; - const expiredResult1 = await providerWithMiddleware.get(freeFormBase64encodedPlainText, { maxAge: 0, transform: 'base64' }); - const expiredResult2 = await providerWithMiddleware.get(freeFormBase64encodedPlainText, { maxAge: 0, transform: 'base64' }); + const expiredResult1 = await providerWithMiddleware.get( + freeFormBase64encodedPlainText, { + maxAge: 0, + transform: 'base64' + } + ); + const expiredResult2 = await providerWithMiddleware.get( + freeFormBase64encodedPlainText, { + maxAge: 0, + transform: 'base64' + } + ); logger.log({ test: 'get-expired', value: middleware.counter, // should be 2 diff --git a/packages/parameters/tests/e2e/appConfigProvider.class.test.ts b/packages/parameters/tests/e2e/appConfigProvider.class.test.ts index ab6207506b..8e2b2944cd 100644 --- a/packages/parameters/tests/e2e/appConfigProvider.class.test.ts +++ b/packages/parameters/tests/e2e/appConfigProvider.class.test.ts @@ -115,7 +115,7 @@ let stack: Stack; * check that we got matching results * * Test 7 - * get parameter twice, wait for expiration betweenn + * get parameter twice, using maxAge to avoid primary cache * we count number of SDK requests and check that we made two API calls * and check that the values match * @@ -321,7 +321,7 @@ describe(`parameters E2E tests (appConfigProvider) for runtime ${runtime}`, () = }, TEST_CASE_TIMEOUT); - // Test 7 - get parameter twice, wait for expiration betweenn + // Test 7 - get parameter twice, using maxAge to avoid primary cache // we count number of SDK requests and check that we made two API calls // and check that the values match it('should retrieve single parameter twice, with expiration between and matching values', async () => { From 61f64765d430d080a54780a0b5c745364c1ffbed Mon Sep 17 00:00:00 2001 From: Brian Krygsman Date: Mon, 13 Mar 2023 14:40:26 -0400 Subject: [PATCH 11/13] Adjust log message structure to match pattern Specfically test/value as top-level properties. --- .../e2e/appConfigProvider.class.test.functionCode.ts | 8 +++++--- .../parameters/tests/e2e/appConfigProvider.class.test.ts | 8 +++++--- 2 files changed, 10 insertions(+), 6 deletions(-) diff --git a/packages/parameters/tests/e2e/appConfigProvider.class.test.functionCode.ts b/packages/parameters/tests/e2e/appConfigProvider.class.test.functionCode.ts index 09e75bf126..3633b7d5d6 100644 --- a/packages/parameters/tests/e2e/appConfigProvider.class.test.functionCode.ts +++ b/packages/parameters/tests/e2e/appConfigProvider.class.test.functionCode.ts @@ -130,9 +130,11 @@ export const handler = async (_event: unknown, _context: Context): Promise ); logger.log({ test: 'get-expired', - value: middleware.counter, // should be 2 - result1: expiredResult1, - result2: expiredResult2 + value: { + counter: middleware.counter, // should be 2 + result1: expiredResult1, + result2: expiredResult2 + }, }); } catch (err) { logger.log({ diff --git a/packages/parameters/tests/e2e/appConfigProvider.class.test.ts b/packages/parameters/tests/e2e/appConfigProvider.class.test.ts index 8e2b2944cd..26b6210ea2 100644 --- a/packages/parameters/tests/e2e/appConfigProvider.class.test.ts +++ b/packages/parameters/tests/e2e/appConfigProvider.class.test.ts @@ -332,9 +332,11 @@ describe(`parameters E2E tests (appConfigProvider) for runtime ${runtime}`, () = expect(testLog).toStrictEqual({ test: 'get-expired', - value: 2, - result1: result, - result2: result + value: { + counter: 2, + result1: result, + result2: result + } }); }, TEST_CASE_TIMEOUT); From 3c46d0c05131c65df3d899d682632133e6959f98 Mon Sep 17 00:00:00 2001 From: Brian Krygsman Date: Mon, 13 Mar 2023 14:45:03 -0400 Subject: [PATCH 12/13] Move client reset to afterEach for consistency. --- packages/parameters/tests/unit/AppConfigProvider.test.ts | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/packages/parameters/tests/unit/AppConfigProvider.test.ts b/packages/parameters/tests/unit/AppConfigProvider.test.ts index 4a88379a23..c87eeb4948 100644 --- a/packages/parameters/tests/unit/AppConfigProvider.test.ts +++ b/packages/parameters/tests/unit/AppConfigProvider.test.ts @@ -22,6 +22,10 @@ describe('Class: AppConfigProvider', () => { jest.clearAllMocks(); }); + afterEach(() => { + client.reset(); + }); + describe('Method: constructor', () => { test('when the class instantiates without SDK client and client config it has default options', async () => { // Prepare From cead3f5d23b1c8bfa3963ad17bf0a92b432c0ce5 Mon Sep 17 00:00:00 2001 From: Brian Krygsman Date: Mon, 13 Mar 2023 14:50:02 -0400 Subject: [PATCH 13/13] Drop old reset --- packages/parameters/tests/unit/AppConfigProvider.test.ts | 2 -- 1 file changed, 2 deletions(-) diff --git a/packages/parameters/tests/unit/AppConfigProvider.test.ts b/packages/parameters/tests/unit/AppConfigProvider.test.ts index c87eeb4948..5bc16e3191 100644 --- a/packages/parameters/tests/unit/AppConfigProvider.test.ts +++ b/packages/parameters/tests/unit/AppConfigProvider.test.ts @@ -233,8 +233,6 @@ describe('Class: AppConfigProvider', () => { test('when session returns an empty configuration on the second call, it returns the last value', async () => { - client.reset(); - // Prepare const options: AppConfigProviderOptions = { application: 'MyApp',