NIAD-3208: CustomTrustStore refactoring to exclude weird dependencies (#1054)

* changing from autowired field to constructor initizalization

* checkstyle

* suppress false positive alarms sent by spotbugs

* refactoring to relax dependency injection rules

* increasing test coverage

* remove test which is inconsistent between localhost and GitHub Actions

* checkstyle

* addressing PR comments

* throws an exception if s3Client can't be initialized

* removal of an unnecessary bean tagging

* checkstyle

* making log entry more descriptive

Author: ORybak5

service/src/main/java/uk/nhs/adaptors/gp2gp/common/configuration/AppInitializer.java

@@ -7,24 +7,39 @@
 
 import lombok.RequiredArgsConstructor;
 import lombok.extern.slf4j.Slf4j;
+import software.amazon.awssdk.services.s3.S3Client;
 import uk.nhs.adaptors.gp2gp.common.storage.StorageConnectorConfiguration;
 
+import javax.naming.ConfigurationException;
+
 @Component(value = "appInitializer")
 @Slf4j
 @RequiredArgsConstructor(onConstructor = @__(@Autowired))
 public class AppInitializer implements InitializingBean {
+
+    private static final String S3_PREFIX = "s3";
     private final StorageConnectorConfiguration storageConnectorConfiguration;
-    private final CustomTrustStore customTrustStore;
 
     @Override
-    public void afterPropertiesSet() {
+    public void afterPropertiesSet() throws ConfigurationException {
         LOGGER.info("Running app initializer");
         if (StringUtils.isNotBlank(storageConnectorConfiguration.getTrustStoreUrl())) {
             LOGGER.info("Adding custom TrustStore to default one");
+            final CustomTrustStore customTrustStore = new CustomTrustStore(getS3Client());
             customTrustStore.addToDefault(storageConnectorConfiguration.getTrustStoreUrl(),
                 storageConnectorConfiguration.getTrustStorePassword());
         } else {
             LOGGER.warn("Trust store URL is not set. Running service without the trust store.");
         }
     }
+
+    public S3Client getS3Client() throws ConfigurationException {
+        if (StringUtils.isNotBlank(storageConnectorConfiguration.getTrustStoreUrl())
+            && storageConnectorConfiguration.getTrustStoreUrl().startsWith(S3_PREFIX)) {
+            return S3Client.builder().build();
+        }
+
+        throw new ConfigurationException("S3Client cannot be instantiated: "
+                                         + "Trust store URL is either not set or does not start with the 's3://' prefix.");
+    }
 }

service/src/main/java/uk/nhs/adaptors/gp2gp/common/configuration/CustomTrustStore.java

@@ -9,27 +9,26 @@
 import javax.net.ssl.TrustManager;
 import javax.net.ssl.TrustManagerFactory;
 import javax.net.ssl.X509TrustManager;
-
-import org.springframework.beans.factory.annotation.Autowired;
-import org.springframework.stereotype.Component;
 import software.amazon.awssdk.core.ResponseInputStream;
 import software.amazon.awssdk.services.s3.S3Uri;
 import software.amazon.awssdk.services.s3.model.GetObjectResponse;
 import software.amazon.awssdk.services.s3.S3Client;
 import software.amazon.awssdk.services.s3.model.GetObjectRequest;
 
 import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
-import lombok.NoArgsConstructor;
 import lombok.RequiredArgsConstructor;
 import lombok.SneakyThrows;
 import lombok.extern.slf4j.Slf4j;
 
-@Component
 @Slf4j
-@NoArgsConstructor
+@SuppressFBWarnings(value = "EI_EXPOSE_REP2", justification = "S3Client is immutable and thread-safe.")
 public class CustomTrustStore {
-    @Autowired(required = false)
-    private S3Client s3Client;
+
+    private final S3Client s3Client;
+
+    public CustomTrustStore(S3Client s3Client) {
+        this.s3Client = s3Client;
+    }
 
     @SneakyThrows
     public void addToDefault(String trustStorePath, String trustStorePassword) {

service/src/main/java/uk/nhs/adaptors/gp2gp/common/storage/StorageConnectorConfiguration.java

@@ -1,12 +1,7 @@
 package uk.nhs.adaptors.gp2gp.common.storage;
 
-import org.apache.commons.lang3.StringUtils;
 import org.springframework.boot.context.properties.ConfigurationProperties;
-import org.springframework.context.annotation.Bean;
 import org.springframework.context.annotation.Configuration;
-
-import software.amazon.awssdk.services.s3.S3Client;
-
 import lombok.Getter;
 import lombok.Setter;
 
@@ -16,20 +11,10 @@
 @ConfigurationProperties(prefix = "gp2gp.storage")
 public class StorageConnectorConfiguration {
 
-    private static final String S3_PREFIX = "s3";
-
     private String type;
     private String containerName;
     private String azureConnectionString;
     private String trustStoreUrl;
     private String trustStorePassword;
 
-    @Bean
-    public S3Client getS3Client() {
-        if (StringUtils.isNotBlank(trustStoreUrl) && trustStoreUrl.startsWith(S3_PREFIX)) {
-            return S3Client.builder().build();
-        }
-
-        return null;
-    }
 }

service/src/test/java/uk/nhs/adaptors/gp2gp/common/configuration/AppInitializerTest.java

@@ -0,0 +1,43 @@
+package uk.nhs.adaptors.gp2gp.common.configuration;
+
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+import uk.nhs.adaptors.gp2gp.common.storage.StorageConnectorConfiguration;
+import javax.naming.ConfigurationException;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+
+class AppInitializerTest {
+
+    public static final String EXPECTED_ERROR_MESSAGE = "S3Client cannot be instantiated: "
+                                                        + "Trust store URL is either not set or does not start with the 's3://' prefix.";
+    private AppInitializer appInitializer;
+    private StorageConnectorConfiguration storageConnectorConfiguration;
+
+    @BeforeEach
+    void setUp() {
+        storageConnectorConfiguration = new StorageConnectorConfiguration();
+    }
+
+    @Test
+    void getNullWhenTrustStoreUrlDoesNotExists() {
+
+        storageConnectorConfiguration.setTrustStoreUrl(null);
+        appInitializer = new AppInitializer(storageConnectorConfiguration);
+
+        Exception exception = assertThrows(ConfigurationException.class, () -> appInitializer.getS3Client());
+
+        assertEquals(EXPECTED_ERROR_MESSAGE, exception.getMessage());
+    }
+
+    @Test
+    void getNullWhenTrustStoreUrlDoesNotStartWithS3Prefix() {
+
+        storageConnectorConfiguration.setTrustStoreUrl("http://localhost");
+        appInitializer = new AppInitializer(storageConnectorConfiguration);
+
+        Exception exception = assertThrows(ConfigurationException.class, () -> appInitializer.getS3Client());
+
+        assertEquals(EXPECTED_ERROR_MESSAGE, exception.getMessage());
+    }
+}

service/src/test/java/uk/nhs/adaptors/gp2gp/common/configuration/CustomTrustStoreTest.java

@@ -12,7 +12,6 @@
 import software.amazon.awssdk.services.s3.model.CreateBucketRequest;
 import software.amazon.awssdk.services.s3.model.PutObjectRequest;
 import java.io.File;
-import java.lang.reflect.Field;
 import java.net.URI;
 import static org.junit.jupiter.api.Assertions.assertNotNull;
 
@@ -24,8 +23,7 @@ class CustomTrustStoreTest {
     private static final String BUCKET_NAME = "test-bucket";
     private static final String TRUSTSTORE_PATH = "test.jks";
     private static final String TRUSTSTORE_PASSWORD = "password";
-
-    private final CustomTrustStore customTrustStore = new CustomTrustStore();
+    private static CustomTrustStore customTrustStore;
 
     @BeforeAll
     static void setUp() {
@@ -48,18 +46,20 @@ static void setUp() {
                            software.amazon.awssdk.core.sync.RequestBody.fromFile(trustStoreFile));
     }
 
+    @BeforeAll
+    static void setup() {
+        customTrustStore = new CustomTrustStore(s3Client);
+    }
+
     @AfterAll
     static void tearDown() {
         s3Mock.shutdown();
+        customTrustStore = null;
     }
 
     @Test
     void trustManagerLoadsSuccessfullyTest() throws NoSuchFieldException, IllegalAccessException {
 
-        Field s3ClientField = CustomTrustStore.class.getDeclaredField("s3Client");
-        s3ClientField.setAccessible(true);
-        s3ClientField.set(customTrustStore, s3Client);
-
         String s3Uri = "s3://" + BUCKET_NAME + "/" + TRUSTSTORE_PATH;
 
         var trustManager = customTrustStore.getCustomDbTrustManager(s3Client.utilities().parseUri(URI.create(s3Uri)), TRUSTSTORE_PASSWORD);