Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
4 changes: 3 additions & 1 deletion packages/commons/tests/utils/e2eUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -37,6 +37,7 @@ export type StackWithLambdaFunctionOptions = {
runtime: string
bundling?: NodejsFunctionProps['bundling']
layers?: NodejsFunctionProps['layers']
timeout?: Duration
};

type FunctionPayload = {[key: string]: string | boolean | number};
Expand All @@ -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) {
Expand Down
20 changes: 18 additions & 2 deletions packages/parameters/src/appconfig/AppConfigProvider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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();
dreamorosi marked this conversation as resolved.
Show resolved Hide resolved
private application?: string;
private environment: string;

Expand Down Expand Up @@ -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)) {
Expand Down Expand Up @@ -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);
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand All @@ -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();
Expand All @@ -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
});
}
};
29 changes: 28 additions & 1 deletion packages/parameters/tests/e2e/appConfigProvider.class.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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: {
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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',
}
});
Expand Down Expand Up @@ -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 () => {
Expand Down
93 changes: 93 additions & 0 deletions packages/parameters/tests/unit/AppConfigProvider.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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
Expand Down Expand Up @@ -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', () => {
Expand All @@ -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();
});
});
});