feat(parameters): AppConfigProvider to return the last valid value when the API returns empty value on subsequent calls

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) {

packages/parameters/src/ExpirableValue.ts

@@ -4,10 +4,17 @@ class ExpirableValue implements ExpirableValueInterface {
   public ttl: number;
   public value: string | Uint8Array | Record<string, unknown>;
 
+  /**
+   * 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<string, unknown>, 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 {

packages/parameters/src/appconfig/AppConfigProvider.ts

@@ -13,6 +13,7 @@ import type {
 class AppConfigProvider extends BaseProvider {
   public client: AppConfigDataClient;
   protected configurationTokenStore: Map<string, string> = new Map();
+  protected valueStore: Map<string, Uint8Array> = 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);
   }
 
   /**

packages/parameters/tests/e2e/appConfigProvider.class.test.functionCode.ts

@@ -81,8 +81,16 @@ export const handler = async (_event: unknown, _context: Context): Promise<void>
   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<void>
       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
+    });
+  }
 };

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 () => {

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', () => {