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

* 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.

* 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.

* Use timestamp/millisecond based math for checking expiration.

* Revert "Use timestamp/millisecond based math for checking expiration."

This reverts commit 3fea0d7.

Further testing shows that the original algorithm was correct.

* 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).

* Revert increase of Lambda function duration back to default.

Not needed anymore since we're not waiting for Test 7.

* Focus value test integration tests on Test 7

* Comma formatting

* 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.

* Address formatting feedback and updates comments to indicate we're no longer waiting during Test 7 runs.

* Adjust log message structure to match pattern

Specfically test/value as top-level properties.

* Move client reset to afterEach for consistency.

* Drop old reset

Author: brnkrygs

Diff for: 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 for: 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);
   }
 
   /**

Diff for: packages/parameters/tests/e2e/appConfigProvider.class.test.functionCode.ts

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

Diff for: 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 for: 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();
+    }); 
+  });
+});