Skip to content

Commit 0bd09ab

Browse files
vitodegiosavitodegiosa
committed
- use computeIfAbsent in ParamManager
- added test for default @param provider - removed ProviderException and added log info
1 parent 64ab03b commit 0bd09ab

5 files changed

Lines changed: 83 additions & 58 deletions

File tree

‎powertools-parameters/pom.xml‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -82,7 +82,10 @@
8282
<artifactId>aspectjrt</artifactId>
8383
<scope>compile</scope>
8484
</dependency>
85-
85+
<dependency>
86+
<groupId>software.amazon.payloadoffloading</groupId>
87+
<artifactId>payloadoffloading-common</artifactId>
88+
</dependency>
8689
<!-- Test dependencies -->
8790
<dependency>
8891
<groupId>org.junit.jupiter</groupId>
@@ -99,6 +102,11 @@
99102
<artifactId>mockito-core</artifactId>
100103
<scope>test</scope>
101104
</dependency>
105+
<dependency>
106+
<groupId>org.mockito</groupId>
107+
<artifactId>mockito-inline</artifactId>
108+
<scope>test</scope>
109+
</dependency>
102110
<dependency>
103111
<groupId>org.apache.commons</groupId>
104112
<artifactId>commons-lang3</artifactId>

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

Lines changed: 30 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -13,10 +13,11 @@
1313
*/
1414
package software.amazon.lambda.powertools.parameters;
1515

16+
import org.apache.commons.logging.Log;
17+
import org.apache.commons.logging.LogFactory;
1618
import software.amazon.awssdk.services.secretsmanager.SecretsManagerClient;
1719
import software.amazon.awssdk.services.ssm.SsmClient;
1820
import software.amazon.lambda.powertools.parameters.cache.CacheManager;
19-
import software.amazon.lambda.powertools.parameters.exception.ProviderException;
2021
import software.amazon.lambda.powertools.parameters.transform.TransformationManager;
2122

2223
import java.lang.reflect.Constructor;
@@ -29,6 +30,8 @@
2930
*/
3031
public final class ParamManager {
3132

33+
private static final Log LOG = LogFactory.getLog(ParamManager.class);
34+
3235
private static final CacheManager cacheManager = new CacheManager();
3336
private static final TransformationManager transformationManager = new TransformationManager();
3437

@@ -42,20 +45,9 @@ public final class ParamManager {
4245
*/
4346
public static <T extends BaseProvider> T getProvider(Class<T> providerClass) {
4447
if (providerClass == null) {
45-
throw new IllegalStateException("You cannot provide a null provider class.");
46-
}
47-
try {
48-
49-
if(!providers.containsKey(providerClass)) {
50-
Constructor<T> constructor = providerClass.getDeclaredConstructor(CacheManager.class);
51-
T provider = constructor.newInstance(cacheManager);
52-
provider.setTransformationManager(transformationManager);
53-
providers.putIfAbsent(providerClass, provider);
54-
}
55-
return (T) providers.get(providerClass);
56-
} catch (InstantiationException | IllegalAccessException | NoSuchMethodException | InvocationTargetException e) {
57-
throw new ProviderException(e);
48+
throw new IllegalStateException("providerClass cannot be null.");
5849
}
50+
return (T) providers.computeIfAbsent(providerClass, (k) -> createProvider(k));
5951
}
6052

6153
/**
@@ -82,14 +74,11 @@ public static SSMProvider getSsmProvider() {
8274
* @return a {@link SecretsProvider}
8375
*/
8476
public static SecretsProvider getSecretsProvider(SecretsManagerClient client) {
85-
if (!providers.containsKey(SecretsProvider.class)) {
86-
providers.putIfAbsent(SecretsProvider.class, SecretsProvider.builder()
87-
.withClient(client)
88-
.withCacheManager(cacheManager)
89-
.withTransformationManager(transformationManager)
90-
.build());
91-
}
92-
return (SecretsProvider) providers.get(SecretsProvider.class);
77+
return (SecretsProvider) providers.computeIfAbsent(SecretsProvider.class, (k) -> SecretsProvider.builder()
78+
.withClient(client)
79+
.withCacheManager(cacheManager)
80+
.withTransformationManager(transformationManager)
81+
.build());
9382
}
9483

9584
/**
@@ -98,14 +87,11 @@ public static SecretsProvider getSecretsProvider(SecretsManagerClient client) {
9887
* @return a {@link SSMProvider}
9988
*/
10089
public static SSMProvider getSsmProvider(SsmClient client) {
101-
if (!providers.containsKey(SSMProvider.class)) {
102-
providers.putIfAbsent(SSMProvider.class, SSMProvider.builder()
103-
.withClient(client)
104-
.withCacheManager(cacheManager)
105-
.withTransformationManager(transformationManager)
106-
.build());
107-
}
108-
return (SSMProvider) providers.get(SSMProvider.class);
90+
return (SSMProvider) providers.computeIfAbsent(SSMProvider.class, (k) -> SSMProvider.builder()
91+
.withClient(client)
92+
.withCacheManager(cacheManager)
93+
.withTransformationManager(transformationManager)
94+
.build());
10995
}
11096

11197
public static CacheManager getCacheManager() {
@@ -115,4 +101,18 @@ public static CacheManager getCacheManager() {
115101
public static TransformationManager getTransformationManager() {
116102
return transformationManager;
117103
}
104+
105+
private static <T extends BaseProvider> T createProvider(Class<T> providerClass) {
106+
try {
107+
Constructor<T> constructor = providerClass.getDeclaredConstructor(CacheManager.class);
108+
T provider = constructor.newInstance(cacheManager);
109+
provider.setTransformationManager(transformationManager);
110+
return provider;
111+
} catch (NoSuchMethodException | IllegalAccessException | InstantiationException | InvocationTargetException e) {
112+
LOG.error("Failed creating provider instance", e);
113+
throw new RuntimeException("Unexpected error occurred. Please raise issue at " +
114+
"https://github.com/awslabs/aws-lambda-powertools-java/issues", e);
115+
}
116+
}
117+
118118
}

‎powertools-parameters/src/main/java/software/amazon/lambda/powertools/parameters/exception/ProviderException.java‎

Lines changed: 0 additions & 12 deletions
This file was deleted.

‎powertools-parameters/src/main/java/software/amazon/lambda/powertools/parameters/internal/LambdaParametersAspect.java‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -16,10 +16,11 @@ public void getParam(Param paramAnnotation) {
1616

1717
@Around("getParam(paramAnnotation)")
1818
public Object injectParam(final ProceedingJoinPoint joinPoint, final Param paramAnnotation) {
19-
BaseProvider provider = ParamManager.getProvider(paramAnnotation.provider());
20-
if(null == provider) {
21-
throw new IllegalArgumentException(String.format("ParamProvider %s not supported.", paramAnnotation.provider().getName()));
19+
if(null == paramAnnotation.provider()) {
20+
throw new IllegalArgumentException("provider for Param annotation cannot be null!");
2221
}
22+
BaseProvider provider = ParamManager.getProvider(paramAnnotation.provider());
23+
2324
if(paramAnnotation.transformer().isInterface()) {
2425
// No transformation
2526
return provider.get(paramAnnotation.key());

‎powertools-parameters/src/test/java/software/amazon/lambda/powertools/parameters/internal/LambdaParametersAspectTest.java‎

Lines changed: 40 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,17 +1,30 @@
11
package software.amazon.lambda.powertools.parameters.internal;
22

3+
import org.junit.jupiter.api.BeforeEach;
34
import org.junit.jupiter.api.Test;
5+
import org.mockito.Mock;
6+
import org.mockito.MockedStatic;
47
import software.amazon.lambda.powertools.parameters.Param;
8+
import software.amazon.lambda.powertools.parameters.ParamManager;
9+
import software.amazon.lambda.powertools.parameters.SSMProvider;
510
import software.amazon.lambda.powertools.parameters.exception.TransformationException;
611
import software.amazon.lambda.powertools.parameters.transform.Base64Transformer;
712
import software.amazon.lambda.powertools.parameters.transform.JsonTransformer;
813
import software.amazon.lambda.powertools.parameters.transform.ObjectToDeserialize;
914

1015
import static org.assertj.core.api.Assertions.assertThat;
1116
import static org.assertj.core.api.Assertions.assertThatExceptionOfType;
17+
import static org.mockito.Mockito.*;
18+
import static org.mockito.MockitoAnnotations.openMocks;
1219

1320
public class LambdaParametersAspectTest {
1421

22+
@Mock
23+
private SSMProvider defaultProvider;
24+
25+
@Param(key = "/default")
26+
private String defaultValue;
27+
1528
@Param(key = "/simple", provider = CustomProvider.class)
1629
private String param;
1730

@@ -24,28 +37,43 @@ public class LambdaParametersAspectTest {
2437
@Param(key = "/json", provider = CustomProvider.class, transformer = JsonTransformer.class)
2538
private AnotherObject wrongTransform;
2639

40+
@BeforeEach
41+
public void init() {
42+
openMocks(this);
43+
}
44+
2745
@Test
28-
public void testSimple() {
29-
String paramValue = param;
46+
public void testDefault_ShouldUseSSMProvider() {
47+
try (MockedStatic<ParamManager> mocked = mockStatic(ParamManager.class)) {
48+
mocked.when(() -> ParamManager.getProvider(SSMProvider.class)).thenReturn(defaultProvider);
49+
when(defaultProvider.get("/default")).thenReturn("value");
3050

31-
assertThat(paramValue).isEqualTo("value");
51+
assertThat(defaultValue).isEqualTo("value");
52+
mocked.verify(times(1), () -> ParamManager.getProvider(SSMProvider.class));
53+
verify(defaultProvider, times(1)).get("/default");
54+
55+
mocked.reset();
56+
}
57+
}
58+
59+
@Test
60+
public void testSimple() {
61+
assertThat(param).isEqualTo("value");
3262
}
3363

3464
@Test
3565
public void testWithBasicTransform() {
36-
String paramValue = basicTransform;
37-
assertThat(paramValue).isEqualTo("value");
66+
assertThat(basicTransform).isEqualTo("value");
3867
}
3968

4069
@Test
4170
public void testWithComplexTransform() {
42-
ObjectToDeserialize paramValue = complexTransform;
43-
assertThat(paramValue).isNotNull();
44-
assertThat(paramValue).isInstanceOf(ObjectToDeserialize.class);
45-
assertThat(paramValue).matches(
46-
o -> o.getFoo().equals("Foo") &&
47-
o.getBar() == 42 &&
48-
o.getBaz() == 123456789);
71+
assertThat(complexTransform)
72+
.isInstanceOf(ObjectToDeserialize.class)
73+
.matches(
74+
o -> o.getFoo().equals("Foo") &&
75+
o.getBar() == 42 &&
76+
o.getBaz() == 123456789);
4977
}
5078

5179
@Test

0 commit comments

Comments
 (0)