fix(parameters): Correctly check for empty values in AppConfig Parameters Provider.

Author: phipag

powertools-parameters/powertools-parameters-appconfig/src/main/java/software/amazon/lambda/powertools/parameters/appconfig/AppConfigProvider.java

@@ -16,6 +16,8 @@
 
 import java.util.HashMap;
 import java.util.Map;
+
+import software.amazon.awssdk.core.SdkBytes;
 import software.amazon.awssdk.services.appconfigdata.AppConfigDataClient;
 import software.amazon.awssdk.services.appconfigdata.model.GetLatestConfigurationRequest;
 import software.amazon.awssdk.services.appconfigdata.model.GetLatestConfigurationResponse;
@@ -47,7 +49,7 @@ public class AppConfigProvider extends BaseProvider {
     private final HashMap<String, EstablishedSession> establishedSessions = new HashMap<>();
 
     AppConfigProvider(CacheManager cacheManager, TransformationManager transformationManager,
-                      AppConfigDataClient client, String environment, String application) {
+            AppConfigDataClient client, String environment, String application) {
         super(cacheManager, transformationManager);
         this.client = client;
         this.application = application;
@@ -63,7 +65,6 @@ public static AppConfigProviderBuilder builder() {
         return new AppConfigProviderBuilder();
     }
 
-
     /**
      * Retrieve the parameter value from the AppConfig parameter store.<br />
      *
@@ -76,13 +77,12 @@ protected String getValue(String key) {
         // so that we can the initial token. If we already have a session, we can take
         // the next request token from there.
         EstablishedSession establishedSession = establishedSessions.getOrDefault(key, null);
-        String sessionToken = establishedSession != null ?
-                establishedSession.nextSessionToken :
-                client.startConfigurationSession(StartConfigurationSessionRequest.builder()
-                                .applicationIdentifier(this.application)
-                                .environmentIdentifier(this.environment)
-                                .configurationProfileIdentifier(key)
-                                .build())
+        String sessionToken = establishedSession != null ? establishedSession.nextSessionToken
+                : client.startConfigurationSession(StartConfigurationSessionRequest.builder()
+                        .applicationIdentifier(this.application)
+                        .environmentIdentifier(this.environment)
+                        .configurationProfileIdentifier(key)
+                        .build())
                         .initialConfigurationToken();
 
         // Get the configuration using the token
@@ -93,16 +93,18 @@ protected String getValue(String key) {
         // Get the next session token we'll use next time we are asked for this key
         String nextSessionToken = response.nextPollConfigurationToken();
 
-        // Get the value of the key. Note that AppConfig will return null if the value
-        // has not changed since we last asked for it in this session - in this case
-        // we return the value we stashed at last request.
-        String value = response.configuration() != null ?
-                response.configuration().asUtf8String() : // if we have a new value, use it
-                establishedSession != null ?
-                        establishedSession.lastConfigurationValue :
-                        // if we don't but we have a previous value, use that
-                        null; // otherwise we've got no value
-
+        // Get the value of the key. Note that AppConfig will return an empty value if the configuration has not changed
+        // since we last asked for it in this session - in this case we return the value we stashed at last request.
+        // https://docs.aws.amazon.com/appconfig/latest/userguide/appconfig-code-samples-using-API-read-configuration.html
+        SdkBytes configFromApi = response.configuration();
+        String value;
+        if (configFromApi != null && configFromApi.asByteArray().length != 0) {
+            value = configFromApi.asUtf8String();
+        } else if (establishedSession != null) {
+            value = establishedSession.lastConfigurationValue;
+        } else {
+            value = null;
+        }
         // Update the cache so we can get the next value later
         establishedSessions.put(key, new EstablishedSession(nextSessionToken, value));
 

powertools-parameters/powertools-parameters-appconfig/src/test/java/software/amazon/lambda/powertools/parameters/appconfig/AppConfigProviderTest.java

@@ -14,9 +14,9 @@
 
 package software.amazon.lambda.powertools.parameters.appconfig;
 
+import static org.assertj.core.api.Assertions.assertThat;
 import static org.assertj.core.api.Assertions.assertThatIllegalStateException;
 import static org.assertj.core.api.Assertions.assertThatRuntimeException;
-import static org.assertj.core.api.Assertions.assertThat;
 import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
 import static org.mockito.MockitoAnnotations.openMocks;
 import static software.amazon.lambda.powertools.parameters.transform.Transformer.json;
@@ -27,6 +27,7 @@
 import org.mockito.Captor;
 import org.mockito.Mock;
 import org.mockito.Mockito;
+
 import software.amazon.awssdk.core.SdkBytes;
 import software.amazon.awssdk.services.appconfigdata.AppConfigDataClient;
 import software.amazon.awssdk.services.appconfigdata.model.GetLatestConfigurationRequest;
@@ -36,7 +37,7 @@
 import software.amazon.lambda.powertools.parameters.cache.CacheManager;
 import software.amazon.lambda.powertools.parameters.transform.TransformationManager;
 
-public class AppConfigProviderTest {
+class AppConfigProviderTest {
 
     private final String environmentName = "test";
     private final String applicationName = "fakeApp";
@@ -53,7 +54,7 @@ public class AppConfigProviderTest {
     private AppConfigProvider provider;
 
     @BeforeEach
-    public void init() {
+    void init() {
         openMocks(this);
 
         provider = AppConfigProvider.builder()
@@ -65,15 +66,14 @@ public void init() {
                 .build();
     }
 
-
     /**
      * Tests repeated calls to the AppConfigProvider for the same key behave correctly. This is more complicated than
      * it seems, as the service itself will return no-data if the value of a property remains unchanged since the
      * start of a session. This means the provider must cache the result and return it again if it gets no data, but
      * subsequent calls should once again return the new data.
      */
     @Test
-    public void getValueRetrievesValue() {
+    void getValueRetrievesValue() {
         // Arrange
         StartConfigurationSessionResponse firstSession = StartConfigurationSessionResponse.builder()
                 .initialConfigurationToken("token1")
@@ -92,21 +92,29 @@ public void getValueRetrievesValue() {
         GetLatestConfigurationResponse thirdResponse = GetLatestConfigurationResponse.builder()
                 .nextPollConfigurationToken("token4")
                 .build();
+        // Forth response returns empty, which means the provider should yield the previous value again
+        GetLatestConfigurationResponse forthResponse = GetLatestConfigurationResponse.builder()
+                .nextPollConfigurationToken("token5")
+                .configuration(SdkBytes.fromUtf8String(""))
+                .build();
         Mockito.when(client.startConfigurationSession(startSessionRequestCaptor.capture()))
                 .thenReturn(firstSession);
         Mockito.when(client.getLatestConfiguration(getLatestConfigurationRequestCaptor.capture()))
-                .thenReturn(firstResponse, secondResponse, thirdResponse);
+                .thenReturn(firstResponse, secondResponse, thirdResponse, forthResponse);
 
         // Act
         String returnedValue1 = provider.getValue(defaultTestKey);
         String returnedValue2 = provider.getValue(defaultTestKey);
         String returnedValue3 = provider.getValue(defaultTestKey);
+        String returnedValue4 = provider.getValue(defaultTestKey);
 
         // Assert
         assertThat(returnedValue1).isEqualTo(firstResponse.configuration().asUtf8String());
         assertThat(returnedValue2).isEqualTo(secondResponse.configuration().asUtf8String());
         assertThat(returnedValue3).isEqualTo(secondResponse.configuration()
                 .asUtf8String()); // Third response is mocked to return null and should re-use previous value
+        assertThat(returnedValue4).isEqualTo(secondResponse.configuration()
+                .asUtf8String()); // Forth response is mocked to return empty and should re-use previous value
         assertThat(startSessionRequestCaptor.getValue().applicationIdentifier()).isEqualTo(applicationName);
         assertThat(startSessionRequestCaptor.getValue().environmentIdentifier()).isEqualTo(environmentName);
         assertThat(startSessionRequestCaptor.getValue().configurationProfileIdentifier()).isEqualTo(defaultTestKey);
@@ -119,8 +127,7 @@ public void getValueRetrievesValue() {
     }
 
     @Test
-    public void getValueNoValueExists() {
-
+    void getValueNoValueExists() {
         // Arrange
         StartConfigurationSessionResponse session = StartConfigurationSessionResponse.builder()
                 .initialConfigurationToken("token1")
@@ -136,17 +143,16 @@ public void getValueNoValueExists() {
         // Act
         String returnedValue = provider.getValue(defaultTestKey);
 
-
         // Assert
-        assertThat(returnedValue).isEqualTo(null);
+        assertThat(returnedValue).isNull();
     }
 
     /**
      * If we mix requests for different keys together through the same provider, retrieval should
      * work as expected. This means two separate configuration sessions should be established with AppConfig.
      */
     @Test
-    public void multipleKeysRetrievalWorks() {
+    void multipleKeysRetrievalWorks() {
         // Arrange
         String param1Key = "key1";
         StartConfigurationSessionResponse param1Session = StartConfigurationSessionResponse.builder()
@@ -184,49 +190,45 @@ public void multipleKeysRetrievalWorks() {
                 param1Session.initialConfigurationToken());
         assertThat(getLatestConfigurationRequestCaptor.getAllValues().get(1).configurationToken()).isEqualTo(
                 param2Session.initialConfigurationToken());
-
     }
 
     @Test
-    public void getMultipleValuesThrowsException() {
-
+    void getMultipleValuesThrowsException() {
         // Act & Assert
         assertThatRuntimeException().isThrownBy(() -> provider.getMultipleValues("path"))
                 .withMessage("Retrieving multiple parameter values is not supported with the AWS App Config Provider");
     }
 
     @Test
-    public void testAppConfigProviderBuilderMissingEnvironment_throwsException() {
-
+    void testAppConfigProviderBuilderMissingEnvironment_throwsException() {
         // Act & Assert
         assertThatIllegalStateException().isThrownBy(() -> AppConfigProvider.builder()
-                        .withCacheManager(new CacheManager())
-                        .withApplication(applicationName)
-                        .withClient(client)
-                        .build())
+                .withCacheManager(new CacheManager())
+                .withApplication(applicationName)
+                .withClient(client)
+                .build())
                 .withMessage("No environment provided; please provide one");
     }
 
     @Test
-    public void testAppConfigProviderBuilderMissingApplication_throwsException() {
-
+    void testAppConfigProviderBuilderMissingApplication_throwsException() {
         // Act & Assert
         assertThatIllegalStateException().isThrownBy(() -> AppConfigProvider.builder()
-                        .withCacheManager(new CacheManager())
-                        .withEnvironment(environmentName)
-                        .withClient(client)
-                        .build())
+                .withCacheManager(new CacheManager())
+                .withEnvironment(environmentName)
+                .withClient(client)
+                .build())
                 .withMessage("No application provided; please provide one");
     }
-    @Test
-    public void testAppConfigProvider_withoutParameter_shouldHaveDefaultTransformationManager() {
 
+    @Test
+    void testAppConfigProvider_withoutParameter_shouldHaveDefaultTransformationManager() {
         // Act
         AppConfigProvider appConfigProvider = AppConfigProvider.builder()
                 .withEnvironment("test")
                 .withApplication("app")
                 .build();
         // Assert
-        assertDoesNotThrow(()->appConfigProvider.withTransformation(json));
+        assertDoesNotThrow(() -> appConfigProvider.withTransformation(json));
     }
 }