Skip to content

Commit 286a2d0

Browse files
phipagphipag
authored
improv(parameters): Make parameters top-level provider thread-safe fo… (#2284)
* improv(parameters): Make parameters top-level provider thread-safe for cache and transformer management. * Use countdown latch instead of Thread.sleep for transformation manager tests. * Make SSMProvider and AppConfigProvider thread-safe. * Make all tests package-private. * Fix sonar finding AZp41z3q7Kei0U644EOS. * Fix sonar finding AZp41z3q7Kei0U644EOS for real now. * Fix PMD findings. * Fix PMD findings. * Fix PMD findings.
1 parent 464490d commit 286a2d0

File tree

21 files changed

+535
-191
lines changed

21 files changed

+535
-191
lines changed

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,8 +14,8 @@
1414

1515
package software.amazon.lambda.powertools.parameters.appconfig;
1616

17-
import java.util.HashMap;
1817
import java.util.Map;
18+
import java.util.concurrent.ConcurrentHashMap;
1919

2020
import software.amazon.awssdk.core.SdkBytes;
2121
import software.amazon.awssdk.services.appconfigdata.AppConfigDataClient;
@@ -46,7 +46,7 @@ public class AppConfigProvider extends BaseProvider {
4646
private final AppConfigDataClient client;
4747
private final String application;
4848
private final String environment;
49-
private final Map<String, EstablishedSession> establishedSessions = new HashMap<>();
49+
private final Map<String, EstablishedSession> establishedSessions = new ConcurrentHashMap<>();
5050

5151
AppConfigProvider(CacheManager cacheManager, TransformationManager transformationManager,
5252
AppConfigDataClient client, String environment, String application) {

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,10 +22,10 @@
2222
import org.junit.jupiter.api.Test;
2323
import org.mockito.Mockito;
2424

25-
public class AppConfigParamAspectTest {
25+
class AppConfigParamAspectTest {
2626

2727
@Test
28-
public void parameterInjectedByProvider() throws Exception {
28+
void parameterInjectedByProvider() throws Exception {
2929
// Setup our aspect to return a mocked AppConfigProvider
3030
String environment = "myEnvironment";
3131
String appName = "myApp";

‎powertools-parameters/powertools-parameters-dynamodb/src/test/java/software/amazon/lambda/powertools/parameters/dynamodb/DynamoDbParamAspectTest.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,10 +21,10 @@
2121
import org.junit.jupiter.api.Test;
2222
import org.mockito.Mockito;
2323

24-
public class DynamoDbParamAspectTest {
24+
class DynamoDbParamAspectTest {
2525

2626
@Test
27-
public void parameterInjectedByProvider() throws Exception {
27+
void parameterInjectedByProvider() throws Exception {
2828
// Setup our aspect to return a mocked DynamoDbProvider
2929
String tableName = "my-test-tablename";
3030
String key = "myKey";

‎powertools-parameters/powertools-parameters-dynamodb/src/test/java/software/amazon/lambda/powertools/parameters/dynamodb/DynamoDbProviderE2ETest.java‎

Lines changed: 13 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -36,10 +36,10 @@
3636
* will move this across.
3737
*/
3838
@Disabled
39-
public class DynamoDbProviderE2ETest {
39+
class DynamoDbProviderE2ETest {
4040

41-
final String ParamsTestTable = "ddb-params-test";
42-
final String MultiparamsTestTable = "ddb-multiparams-test";
41+
private static final String PARAMS_TEST_TABLE = "ddb-params-test";
42+
private static final String MULTI_PARAMS_TEST_TABLE = "ddb-multiparams-test";
4343
private final DynamoDbClient ddbClient;
4444

4545
public DynamoDbProviderE2ETest() {
@@ -52,49 +52,49 @@ public DynamoDbProviderE2ETest() {
5252
}
5353

5454
@Test
55-
public void TestGetValue() {
55+
void TestGetValue() {
5656

5757
// Arrange
58-
HashMap<String, AttributeValue> testItem = new HashMap<String, AttributeValue>();
58+
Map<String, AttributeValue> testItem = new HashMap<>();
5959
testItem.put("id", AttributeValue.fromS("test_param"));
6060
testItem.put("value", AttributeValue.fromS("the_value_is_hello!"));
6161
ddbClient.putItem(PutItemRequest.builder()
62-
.tableName(ParamsTestTable)
62+
.tableName(PARAMS_TEST_TABLE)
6363
.item(testItem)
6464
.build());
6565

6666
// Act
67-
DynamoDbProvider provider = makeProvider(ParamsTestTable);
67+
DynamoDbProvider provider = makeProvider(PARAMS_TEST_TABLE);
6868
String value = provider.getValue("test_param");
6969

7070
// Assert
7171
assertThat(value).isEqualTo("the_value_is_hello!");
7272
}
7373

7474
@Test
75-
public void TestGetValues() {
75+
void TestGetValues() {
7676

7777
// Arrange
78-
HashMap<String, AttributeValue> testItem = new HashMap<String, AttributeValue>();
78+
Map<String, AttributeValue> testItem = new HashMap<>();
7979
testItem.put("id", AttributeValue.fromS("test_param"));
8080
testItem.put("sk", AttributeValue.fromS("test_param_part_1"));
8181
testItem.put("value", AttributeValue.fromS("the_value_is_hello!"));
8282
ddbClient.putItem(PutItemRequest.builder()
83-
.tableName(MultiparamsTestTable)
83+
.tableName(MULTI_PARAMS_TEST_TABLE)
8484
.item(testItem)
8585
.build());
8686

87-
HashMap<String, AttributeValue> testItem2 = new HashMap<String, AttributeValue>();
87+
Map<String, AttributeValue> testItem2 = new HashMap<>();
8888
testItem2.put("id", AttributeValue.fromS("test_param"));
8989
testItem2.put("sk", AttributeValue.fromS("test_param_part_2"));
9090
testItem2.put("value", AttributeValue.fromS("the_value_is_still_hello!"));
9191
ddbClient.putItem(PutItemRequest.builder()
92-
.tableName(MultiparamsTestTable)
92+
.tableName(MULTI_PARAMS_TEST_TABLE)
9393
.item(testItem2)
9494
.build());
9595

9696
// Act
97-
DynamoDbProvider provider = makeProvider(MultiparamsTestTable);
97+
DynamoDbProvider provider = makeProvider(MULTI_PARAMS_TEST_TABLE);
9898
Map<String, String> values = provider.getMultipleValues("test_param");
9999

100100
// Assert

‎powertools-parameters/powertools-parameters-dynamodb/src/test/java/software/amazon/lambda/powertools/parameters/dynamodb/DynamoDbProviderTest.java‎

Lines changed: 22 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -45,9 +45,9 @@
4545
import software.amazon.lambda.powertools.parameters.transform.TransformationManager;
4646

4747
@ExtendWith(MockitoExtension.class)
48-
public class DynamoDbProviderTest {
48+
class DynamoDbProviderTest {
4949

50-
private final String tableName = "ddb-test-table";
50+
private static final String TABLE_NAME = "ddb-test-table";
5151

5252
@Mock
5353
DynamoDbClient client;
@@ -64,19 +64,19 @@ public class DynamoDbProviderTest {
6464
private DynamoDbProvider provider;
6565

6666
@BeforeEach
67-
public void init() {
67+
void init() {
6868
openMocks(this);
6969
CacheManager cacheManager = new CacheManager();
70-
provider = new DynamoDbProvider(cacheManager, transformationManager, client, tableName);
70+
provider = new DynamoDbProvider(cacheManager, transformationManager, client, TABLE_NAME);
7171
}
7272

7373
@Test
74-
public void getValue() {
74+
void getValue() {
7575

7676
// Arrange
7777
String key = "Key1";
7878
String expectedValue = "Value1";
79-
HashMap<String, AttributeValue> responseData = new HashMap<String, AttributeValue>();
79+
Map<String, AttributeValue> responseData = new HashMap<>();
8080
responseData.put("id", AttributeValue.fromS(key));
8181
responseData.put("value", AttributeValue.fromS(expectedValue));
8282
GetItemResponse response = GetItemResponse.builder()
@@ -89,12 +89,12 @@ public void getValue() {
8989

9090
// Assert
9191
assertThat(value).isEqualTo(expectedValue);
92-
assertThat(getItemValueCaptor.getValue().tableName()).isEqualTo(tableName);
92+
assertThat(getItemValueCaptor.getValue().tableName()).isEqualTo(TABLE_NAME);
9393
assertThat(getItemValueCaptor.getValue().key().get("id").s()).isEqualTo(key);
9494
}
9595

9696
@Test
97-
public void getValueWithNullResultsReturnsNull() {
97+
void getValueWithNullResultsReturnsNull() {
9898
// Arrange
9999
Mockito.when(client.getItem(getItemValueCaptor.capture())).thenReturn(GetItemResponse.builder()
100100
.item(null)
@@ -108,7 +108,7 @@ public void getValueWithNullResultsReturnsNull() {
108108
}
109109

110110
@Test
111-
public void getValueWithoutResultsReturnsNull() {
111+
void getValueWithoutResultsReturnsNull() {
112112
// Arrange
113113
Mockito.when(client.getItem(getItemValueCaptor.capture())).thenReturn(GetItemResponse.builder()
114114
.item(new HashMap<>())
@@ -122,10 +122,10 @@ public void getValueWithoutResultsReturnsNull() {
122122
}
123123

124124
@Test
125-
public void getValueWithMalformedRowThrows() {
125+
void getValueWithMalformedRowThrows() {
126126
// Arrange
127127
String key = "Key1";
128-
HashMap<String, AttributeValue> responseData = new HashMap<String, AttributeValue>();
128+
Map<String, AttributeValue> responseData = new HashMap<>();
129129
responseData.put("id", AttributeValue.fromS(key));
130130
responseData.put("not-value", AttributeValue.fromS("something"));
131131
Mockito.when(client.getItem(getItemValueCaptor.capture())).thenReturn(GetItemResponse.builder()
@@ -138,19 +138,19 @@ public void getValueWithMalformedRowThrows() {
138138
}
139139

140140
@Test
141-
public void getValues() {
141+
void getValues() {
142142

143143
// Arrange
144144
String key = "Key1";
145145
String subkey1 = "Subkey1";
146146
String val1 = "Val1";
147147
String subkey2 = "Subkey2";
148148
String val2 = "Val2";
149-
HashMap<String, AttributeValue> item1 = new HashMap<String, AttributeValue>();
149+
Map<String, AttributeValue> item1 = new HashMap<>();
150150
item1.put("id", AttributeValue.fromS(key));
151151
item1.put("sk", AttributeValue.fromS(subkey1));
152152
item1.put("value", AttributeValue.fromS(val1));
153-
HashMap<String, AttributeValue> item2 = new HashMap<String, AttributeValue>();
153+
Map<String, AttributeValue> item2 = new HashMap<>();
154154
item2.put("id", AttributeValue.fromS(key));
155155
item2.put("sk", AttributeValue.fromS(subkey2));
156156
item2.put("value", AttributeValue.fromS(val2));
@@ -166,13 +166,13 @@ public void getValues() {
166166
assertThat(values.size()).isEqualTo(2);
167167
assertThat(values.get(subkey1)).isEqualTo(val1);
168168
assertThat(values.get(subkey2)).isEqualTo(val2);
169-
assertThat(queryRequestCaptor.getValue().tableName()).isEqualTo(tableName);
169+
assertThat(queryRequestCaptor.getValue().tableName()).isEqualTo(TABLE_NAME);
170170
assertThat(queryRequestCaptor.getValue().keyConditionExpression()).isEqualTo("id = :v_id");
171171
assertThat(queryRequestCaptor.getValue().expressionAttributeValues().get(":v_id").s()).isEqualTo(key);
172172
}
173173

174174
@Test
175-
public void getValuesWithoutResultsReturnsNull() {
175+
void getValuesWithoutResultsReturnsNull() {
176176
// Arrange
177177
Mockito.when(client.query(queryRequestCaptor.capture())).thenReturn(
178178
QueryResponse.builder().items().build());
@@ -185,10 +185,10 @@ public void getValuesWithoutResultsReturnsNull() {
185185
}
186186

187187
@Test
188-
public void getMultipleValuesMissingSortKey_throwsException() {
188+
void getMultipleValuesMissingSortKey_throwsException() {
189189
// Arrange
190190
String key = "Key1";
191-
HashMap<String, AttributeValue> item = new HashMap<String, AttributeValue>();
191+
Map<String, AttributeValue> item = new HashMap<>();
192192
item.put("id", AttributeValue.fromS(key));
193193
item.put("value", AttributeValue.fromS("somevalue"));
194194
QueryResponse response = QueryResponse.builder()
@@ -204,10 +204,10 @@ public void getMultipleValuesMissingSortKey_throwsException() {
204204
}
205205

206206
@Test
207-
public void getValuesWithMalformedRowThrows() {
207+
void getValuesWithMalformedRowThrows() {
208208
// Arrange
209209
String key = "Key1";
210-
HashMap<String, AttributeValue> item1 = new HashMap<String, AttributeValue>();
210+
Map<String, AttributeValue> item1 = new HashMap<>();
211211
item1.put("id", AttributeValue.fromS(key));
212212
item1.put("sk", AttributeValue.fromS("some-subkey"));
213213
item1.put("not-value", AttributeValue.fromS("somevalue"));
@@ -224,7 +224,7 @@ public void getValuesWithMalformedRowThrows() {
224224
}
225225

226226
@Test
227-
public void testDynamoDBBuilderMissingTable_throwsException() {
227+
void testDynamoDBBuilderMissingTable_throwsException() {
228228

229229
// Act & Assert
230230
assertThatIllegalStateException().isThrownBy(() -> DynamoDbProvider.builder()
@@ -233,7 +233,7 @@ public void testDynamoDBBuilderMissingTable_throwsException() {
233233
}
234234

235235
@Test
236-
public void testDynamoDBBuilder_withoutParameter_shouldHaveDefaultTransformationManager() {
236+
void testDynamoDBBuilder_withoutParameter_shouldHaveDefaultTransformationManager() {
237237

238238
// Act
239239
DynamoDbProvider dynamoDbProvider = DynamoDbProvider.builder().withTable("test-table")

‎powertools-parameters/powertools-parameters-secrets/src/test/java/software/amazon/lambda/powertools/parameters/secrets/SecretsParamAspectTest.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,10 +21,10 @@
2121
import org.junit.jupiter.api.Test;
2222
import org.mockito.Mockito;
2323

24-
public class SecretsParamAspectTest {
24+
class SecretsParamAspectTest {
2525

2626
@Test
27-
public void parameterInjectedByProvider() throws Exception {
27+
void parameterInjectedByProvider() throws Exception {
2828
// Setup our aspect to return a mocked SecretsProvider
2929
String key = "myKey";
3030
String value = "mySecretValue";

‎powertools-parameters/powertools-parameters-secrets/src/test/java/software/amazon/lambda/powertools/parameters/secrets/SecretsProviderTest.java‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@
4040
import software.amazon.lambda.powertools.parameters.transform.TransformationManager;
4141

4242
@ExtendWith(MockitoExtension.class)
43-
public class SecretsProviderTest {
43+
class SecretsProviderTest {
4444

4545
@Mock
4646
SecretsManagerClient client;
@@ -56,13 +56,13 @@ public class SecretsProviderTest {
5656
SecretsProvider provider;
5757

5858
@BeforeEach
59-
public void init() {
59+
void init() {
6060
cacheManager = new CacheManager();
6161
provider = new SecretsProvider(cacheManager, transformationManager, client);
6262
}
6363

6464
@Test
65-
public void getValue() {
65+
void getValue() {
6666
String key = "Key1";
6767
String expectedValue = "Value1";
6868
GetSecretValueResponse response = GetSecretValueResponse.builder().secretString(expectedValue).build();
@@ -76,7 +76,7 @@ public void getValue() {
7676
}
7777

7878
@Test
79-
public void getValueBase64() {
79+
void getValueBase64() {
8080
String key = "Key2";
8181
String expectedValue = "Value2";
8282
byte[] valueb64 = Base64.getEncoder().encode(expectedValue.getBytes());
@@ -91,14 +91,14 @@ public void getValueBase64() {
9191
}
9292

9393
@Test
94-
public void getMultipleValuesThrowsException() {
94+
void getMultipleValuesThrowsException() {
9595
// Act & Assert
9696
assertThatRuntimeException().isThrownBy(() -> provider.getMultipleValues("path"))
9797
.withMessage("Impossible to get multiple values from AWS Secrets Manager");
9898
}
9999

100100
@Test
101-
public void testGetSecretsProvider_withoutParameter_shouldCreateDefaultClient() {
101+
void testGetSecretsProvider_withoutParameter_shouldCreateDefaultClient() {
102102
// Act
103103
SecretsProvider secretsProvider = SecretsProvider.builder()
104104
.build();
@@ -109,7 +109,7 @@ public void testGetSecretsProvider_withoutParameter_shouldCreateDefaultClient()
109109
}
110110

111111
@Test
112-
public void testGetSecretsProvider_withoutParameter_shouldHaveDefaultTransformationManager() {
112+
void testGetSecretsProvider_withoutParameter_shouldHaveDefaultTransformationManager() {
113113
// Act
114114
SecretsProvider secretsProvider = SecretsProvider.builder()
115115
.build();

0 commit comments

Comments
 (0)