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/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/e2e/appConfigProvider.class.test.functionCode.ts b/packages/parameters/tests/e2e/appConfigProvider.class.test.functionCode.ts index 11d8feb01f..3633b7d5d6 100644 --- a/packages/parameters/tests/e2e/appConfigProvider.class.test.functionCode.ts +++ b/packages/parameters/tests/e2e/appConfigProvider.class.test.functionCode.ts @@ -73,10 +73,10 @@ 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(); @@ -94,7 +94,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(); @@ -111,4 +111,35 @@ export const handler = async (_event: unknown, _context: Context): Promise error: err.message }); } + // Test 7 + // 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' + } + ); + logger.log({ + test: 'get-expired', + value: { + counter: middleware.counter, // should be 2 + result1: expiredResult1, + result2: expiredResult2 + }, + }); + } 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..26b6210ea2 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: { @@ -111,6 +112,12 @@ 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 + * + * 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 * * 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 @@ -191,7 +198,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', } }); @@ -314,6 +321,26 @@ describe(`parameters E2E tests (appConfigProvider) for runtime ${runtime}`, () = }, TEST_CASE_TIMEOUT); + // 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 () => { + + const logs = invocationLogs[0].getFunctionLogs(); + const testLog = InvocationLogs.parseFunctionLog(logs[6]); + const result = freeFormBase64PlainTextValue; + + expect(testLog).toStrictEqual({ + test: 'get-expired', + value: { + counter: 2, + result1: result, + result2: result + } + }); + + }, TEST_CASE_TIMEOUT); + }); afterAll(async () => { diff --git a/packages/parameters/tests/unit/AppConfigProvider.test.ts b/packages/parameters/tests/unit/AppConfigProvider.test.ts index 56365d882c..5bc16e3191 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, @@ -21,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 @@ -225,6 +230,49 @@ 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 () => { + + // 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', () => { @@ -243,3 +291,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