From eb5964ad0384731c71083425deb2897d25c0634a Mon Sep 17 00:00:00 2001 From: MartinWheelerMT Date: Wed, 6 May 2026 16:23:30 +0100 Subject: [PATCH 1/8] NIAD-3453: Refactor `StorageService` to correctly use DI/IOC * Delete StorageServiceFactory.java and StorageServiceFactoryConfig.java as unnecessary and confusing steps in the DI process. --- .../storage/StorageServiceFactory.java | 37 ------------------- .../storage/StorageServiceFactoryConfig.java | 18 --------- 2 files changed, 55 deletions(-) delete mode 100644 gp2gp-translator/src/main/java/uk/nhs/adaptors/pss/translator/storage/StorageServiceFactory.java delete mode 100644 gp2gp-translator/src/main/java/uk/nhs/adaptors/pss/translator/storage/StorageServiceFactoryConfig.java diff --git a/gp2gp-translator/src/main/java/uk/nhs/adaptors/pss/translator/storage/StorageServiceFactory.java b/gp2gp-translator/src/main/java/uk/nhs/adaptors/pss/translator/storage/StorageServiceFactory.java deleted file mode 100644 index e73508e1d..000000000 --- a/gp2gp-translator/src/main/java/uk/nhs/adaptors/pss/translator/storage/StorageServiceFactory.java +++ /dev/null @@ -1,37 +0,0 @@ -package uk.nhs.adaptors.pss.translator.storage; - -import lombok.Setter; -import org.springframework.beans.factory.FactoryBean; -import org.springframework.beans.factory.annotation.Autowired; -import software.amazon.awssdk.services.s3.S3Client; -import software.amazon.awssdk.services.s3.presigner.S3Presigner; - -@Setter -public class StorageServiceFactory implements FactoryBean { - - @Autowired - private StorageServiceConfiguration configuration; - - public StorageService getObject() { - - // we cannot create a private instance of storageService without triggering - // an EI_EXPOSE_REP via Spotbug tests - StorageService storageService = null; - switch (StorageServiceOptionsEnum.enumOf(configuration.getType())) { - case S3: - storageService = new AWSStorageService(S3Client.builder().build(), configuration, S3Presigner.builder().build()); - break; - case AZURE: - storageService = new AzureStorageService(configuration); - break; - default: - storageService = new LocalStorageService(); - } - - return storageService; - } - - public Class getObjectType() { - return StorageService.class; - } -} \ No newline at end of file diff --git a/gp2gp-translator/src/main/java/uk/nhs/adaptors/pss/translator/storage/StorageServiceFactoryConfig.java b/gp2gp-translator/src/main/java/uk/nhs/adaptors/pss/translator/storage/StorageServiceFactoryConfig.java deleted file mode 100644 index 82c789684..000000000 --- a/gp2gp-translator/src/main/java/uk/nhs/adaptors/pss/translator/storage/StorageServiceFactoryConfig.java +++ /dev/null @@ -1,18 +0,0 @@ -package uk.nhs.adaptors.pss.translator.storage; - -import org.springframework.context.annotation.Bean; -import org.springframework.context.annotation.Configuration; - -@Configuration -public class StorageServiceFactoryConfig { - - @Bean(name = "storage-service") - public StorageServiceFactory storageServiceFactory() { - return new StorageServiceFactory(); - } - - @Bean - public StorageService storageService() { - return storageServiceFactory().getObject(); - } -} \ No newline at end of file From 7604fc57d5a0511c2f849b631f8b97c4e7890804 Mon Sep 17 00:00:00 2001 From: MartinWheelerMT Date: Wed, 6 May 2026 16:25:48 +0100 Subject: [PATCH 2/8] NIAD-3453: Refactor `StorageService` to correctly use DI/IOC * Introduce StorageServiceConfig to conditionally create the required service. * Refactor AzureStorageService to now correctly inject the StorageServiceClient from the IOC container. --- .../storage/AzureStorageService.java | 20 +++---- .../storage/StorageServiceConfig.java | 54 +++++++++++++++++++ 2 files changed, 60 insertions(+), 14 deletions(-) create mode 100644 gp2gp-translator/src/main/java/uk/nhs/adaptors/pss/translator/storage/StorageServiceConfig.java diff --git a/gp2gp-translator/src/main/java/uk/nhs/adaptors/pss/translator/storage/AzureStorageService.java b/gp2gp-translator/src/main/java/uk/nhs/adaptors/pss/translator/storage/AzureStorageService.java index 81e19f2fb..e1f26ef79 100644 --- a/gp2gp-translator/src/main/java/uk/nhs/adaptors/pss/translator/storage/AzureStorageService.java +++ b/gp2gp-translator/src/main/java/uk/nhs/adaptors/pss/translator/storage/AzureStorageService.java @@ -11,25 +11,17 @@ import com.azure.storage.blob.BlobServiceClientBuilder; import com.azure.storage.blob.specialized.BlockBlobClient; import com.azure.storage.common.StorageSharedKeyCredential; +import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; +@SuppressFBWarnings(value = "EI_EXPOSE_REP2", justification = "BlobServiceClient is immutable and thread-safe.") public class AzureStorageService implements StorageService { - // Consistent objects private final BlobServiceClient blobServiceClient; - private String containerName; + private final String containerName; - public AzureStorageService(StorageServiceConfiguration configuration) { - if (!configuration.getAccountReference().isEmpty()) { - - StorageSharedKeyCredential credentials = createAzureCredentials( - configuration.getAccountReference(), configuration.getAccountSecret()); - - String azureEndpoint = createAzureStorageEndpoint(configuration.getAccountReference()); - blobServiceClient = createBlobServiceClient(azureEndpoint, credentials); - containerName = configuration.getContainerName(); - } else { - blobServiceClient = null; - } + public AzureStorageService(BlobServiceClient client, StorageServiceConfiguration config) { + blobServiceClient = client; + containerName = config.getContainerName(); } public void uploadFile(String filename, byte[] fileAsString) throws StorageException { diff --git a/gp2gp-translator/src/main/java/uk/nhs/adaptors/pss/translator/storage/StorageServiceConfig.java b/gp2gp-translator/src/main/java/uk/nhs/adaptors/pss/translator/storage/StorageServiceConfig.java new file mode 100644 index 000000000..ddcfd613d --- /dev/null +++ b/gp2gp-translator/src/main/java/uk/nhs/adaptors/pss/translator/storage/StorageServiceConfig.java @@ -0,0 +1,54 @@ +package uk.nhs.adaptors.pss.translator.storage; + +import com.azure.storage.blob.BlobServiceClient; +import com.azure.storage.blob.BlobServiceClientBuilder; +import com.azure.storage.common.StorageSharedKeyCredential; +import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty; +import org.springframework.context.annotation.Bean; +import org.springframework.context.annotation.Configuration; +import software.amazon.awssdk.services.s3.S3Client; +import software.amazon.awssdk.services.s3.presigner.S3Presigner; + +import java.util.Locale; + +@Configuration +public class StorageServiceConfig { + + @Bean + @ConditionalOnProperty(name = "storage.type", havingValue = "Azure") + public StorageService azureStorageService(StorageServiceConfiguration configuration) { + StorageSharedKeyCredential credentials = new StorageSharedKeyCredential( + configuration.getAccountReference(), + configuration.getAccountSecret() + ); + + String endpoint = String.format( + Locale.ROOT, + "https://%s.blob.core.windows.net", + configuration.getAccountReference() + ); + + BlobServiceClient blobServiceClient = new BlobServiceClientBuilder() + .endpoint(endpoint) + .credential(credentials) + .buildClient(); + + return new AzureStorageService(blobServiceClient, configuration); + } + + @Bean + @ConditionalOnProperty(name = "storage.type", havingValue = "S3") + public StorageService awsStorageService(StorageServiceConfiguration configuration) { + return new AWSStorageService( + S3Client.builder().build(), + configuration, + S3Presigner.builder().build() + ); + } + + @Bean + @ConditionalOnProperty(name = "storage.type", havingValue = "LocalMock", matchIfMissing = true) + public StorageService localStorageService() { + return new LocalStorageService(); + } +} From d1ab12ed98ac72859c45df8abb3de663fcb3db40 Mon Sep 17 00:00:00 2001 From: MartinWheelerMT Date: Wed, 6 May 2026 16:38:13 +0100 Subject: [PATCH 3/8] * Add Integration Test dfor AzureStorageService. * Add `testcontainers` to `build.gradle` to enable use of azurite. --- gp2gp-translator/build.gradle | 1 + .../storage/AzureStorageServiceTest.java | 102 ++++++++++++++++++ 2 files changed, 103 insertions(+) create mode 100644 gp2gp-translator/src/integrationTest/java/uk/nhs/adaptors/pss/translator/storage/AzureStorageServiceTest.java diff --git a/gp2gp-translator/build.gradle b/gp2gp-translator/build.gradle index 6f6e8d5ee..96bf3aabd 100644 --- a/gp2gp-translator/build.gradle +++ b/gp2gp-translator/build.gradle @@ -50,6 +50,7 @@ dependencies { testImplementation 'org.skyscreamer:jsonassert:1.5.3' testImplementation 'org.awaitility:awaitility:4.2.2' testImplementation 'io.findify:s3mock_2.13:0.2.6' + testImplementation 'org.testcontainers:testcontainers-azure:2.0.5' pitest 'com.arcmutate:base:1.3.2' pitest 'com.arcmutate:pitest-git-plugin:2.0.0' diff --git a/gp2gp-translator/src/integrationTest/java/uk/nhs/adaptors/pss/translator/storage/AzureStorageServiceTest.java b/gp2gp-translator/src/integrationTest/java/uk/nhs/adaptors/pss/translator/storage/AzureStorageServiceTest.java new file mode 100644 index 000000000..31305c91d --- /dev/null +++ b/gp2gp-translator/src/integrationTest/java/uk/nhs/adaptors/pss/translator/storage/AzureStorageServiceTest.java @@ -0,0 +1,102 @@ +package uk.nhs.adaptors.pss.translator.storage; + +import com.azure.core.util.BinaryData; +import com.azure.storage.blob.BlobServiceClient; +import com.azure.storage.blob.BlobServiceClientBuilder; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.testcontainers.azure.AzuriteContainer; + +import java.nio.charset.StandardCharsets; + +import static org.junit.jupiter.api.Assertions.assertNotNull; + +public class AzureStorageServiceTest { + + private static final String CONTAINER_NAME = "azurecontainer"; + private static final String FILE_NAME = "testfile.txt"; + + private AzureStorageService azureStorageService; + private AzuriteContainer azuriteContainer; + private BlobServiceClient blobServiceClient; + + @BeforeEach + void setUp() { + azuriteContainer = new AzuriteContainer("mcr.microsoft.com/azure-storage/azurite:3.33.0"); + azuriteContainer.start(); + + blobServiceClient = new BlobServiceClientBuilder() + .connectionString(azuriteContainer.getConnectionString()) + .buildClient(); + blobServiceClient.createBlobContainer(CONTAINER_NAME); + + StorageServiceConfiguration config = new StorageServiceConfiguration(); + config.setContainerName(CONTAINER_NAME); + + azureStorageService = new AzureStorageService(blobServiceClient, config); + } + + @AfterEach + void tearDown() { + azuriteContainer.stop(); + } + + @Test + void uploadToStorageTest() { + String uploadContent = "uploadcontent"; + + azureStorageService.uploadFile(FILE_NAME, uploadContent.getBytes(StandardCharsets.UTF_8)); + + byte[] downloaded = blobServiceClient + .getBlobContainerClient(CONTAINER_NAME) + .getBlobClient(FILE_NAME).downloadContent().toBytes(); + + Assertions.assertEquals(uploadContent, new String(downloaded, StandardCharsets.UTF_8)); + } + + @Test + void downloadFromStorageTest() { + + String fileContent = "dummy-content"; + + blobServiceClient + .getBlobContainerClient(CONTAINER_NAME) + .getBlobClient(FILE_NAME).upload(BinaryData.fromString(fileContent)); + + byte[] response = azureStorageService.downloadFile(FILE_NAME); + String downloadedContent = new String(response, StandardCharsets.UTF_8); + + assertNotNull(response); + Assertions.assertEquals(fileContent, downloadedContent); + } + + @Test + void deleteFileTest() { + + String fileContent = "dummy-content"; + blobServiceClient + .getBlobContainerClient(CONTAINER_NAME) + .getBlobClient(FILE_NAME).upload(BinaryData.fromString(fileContent)); + + azureStorageService.deleteFile(FILE_NAME); + + Exception exception = Assertions.assertThrows(Exception.class, () -> azureStorageService.downloadFile(FILE_NAME)); + + Assertions.assertEquals("Status code 404, BlobNotFound", exception.getMessage()); + } + + @Test + void getFileLocationTest() { + String fileContent = "dummy-content"; + blobServiceClient + .getBlobContainerClient(CONTAINER_NAME) + .getBlobClient(FILE_NAME).upload(BinaryData.fromString(fileContent)); + + String response = azureStorageService.getFileLocation(FILE_NAME); + + assertNotNull(response); + Assertions.assertTrue(response.contains(FILE_NAME)); + } +} \ No newline at end of file From eddf590c5c183eb7b487799c438b4a1b54801eae Mon Sep 17 00:00:00 2001 From: MartinWheelerMT Date: Wed, 6 May 2026 16:51:33 +0100 Subject: [PATCH 4/8] * Remove now unused methods in `AzureStorageService`. --- .../storage/AzureStorageService.java | 19 ------------------- 1 file changed, 19 deletions(-) diff --git a/gp2gp-translator/src/main/java/uk/nhs/adaptors/pss/translator/storage/AzureStorageService.java b/gp2gp-translator/src/main/java/uk/nhs/adaptors/pss/translator/storage/AzureStorageService.java index e1f26ef79..17d4e4615 100644 --- a/gp2gp-translator/src/main/java/uk/nhs/adaptors/pss/translator/storage/AzureStorageService.java +++ b/gp2gp-translator/src/main/java/uk/nhs/adaptors/pss/translator/storage/AzureStorageService.java @@ -4,13 +4,10 @@ import java.io.ByteArrayOutputStream; import java.io.IOException; import java.io.InputStream; -import java.util.Locale; import com.azure.storage.blob.BlobContainerClient; import com.azure.storage.blob.BlobServiceClient; -import com.azure.storage.blob.BlobServiceClientBuilder; import com.azure.storage.blob.specialized.BlockBlobClient; -import com.azure.storage.common.StorageSharedKeyCredential; import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; @SuppressFBWarnings(value = "EI_EXPOSE_REP2", justification = "BlobServiceClient is immutable and thread-safe.") @@ -48,22 +45,6 @@ public String getFileLocation(String filename) { return blobClient.getBlobUrl(); } - private StorageSharedKeyCredential createAzureCredentials(String accountName, String accountKey) { - return new StorageSharedKeyCredential(accountName, accountKey); - } - - private String createAzureStorageEndpoint(String containerName) { - return String.format(Locale.ROOT, "https://%s.blob.core.windows.net", containerName); - } - - private BlobServiceClient createBlobServiceClient(String endpoint, StorageSharedKeyCredential credentials) { - - return new BlobServiceClientBuilder() - .endpoint(endpoint) - .credential(credentials) - .buildClient(); - } - private BlobContainerClient createBlobContainerClient() { return blobServiceClient.getBlobContainerClient(containerName); } From 38af05459b73c3b4a444d21e829032dd6bdb5314 Mon Sep 17 00:00:00 2001 From: MartinWheelerMT Date: Wed, 6 May 2026 16:58:42 +0100 Subject: [PATCH 5/8] * Add unit tests for `AzureStorageService` --- .../storage/AzureStorageServiceTest.java | 105 ++++++++++++++++++ 1 file changed, 105 insertions(+) create mode 100644 gp2gp-translator/src/test/java/uk/nhs/adaptors/pss/translator/storage/AzureStorageServiceTest.java diff --git a/gp2gp-translator/src/test/java/uk/nhs/adaptors/pss/translator/storage/AzureStorageServiceTest.java b/gp2gp-translator/src/test/java/uk/nhs/adaptors/pss/translator/storage/AzureStorageServiceTest.java new file mode 100644 index 000000000..e9fae3827 --- /dev/null +++ b/gp2gp-translator/src/test/java/uk/nhs/adaptors/pss/translator/storage/AzureStorageServiceTest.java @@ -0,0 +1,105 @@ +package uk.nhs.adaptors.pss.translator.storage; + +import com.azure.storage.blob.BlobContainerClient; +import com.azure.storage.blob.BlobServiceClient; +import com.azure.storage.blob.models.BlobProperties; +import com.azure.storage.blob.specialized.BlockBlobClient; +import com.azure.storage.blob.BlobClient; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.Mock; +import org.mockito.junit.jupiter.MockitoExtension; + +import java.io.ByteArrayOutputStream; +import java.io.InputStream; +import java.nio.charset.StandardCharsets; + +import static org.junit.jupiter.api.Assertions.assertArrayEquals; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.doAnswer; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +@ExtendWith(MockitoExtension.class) +public class AzureStorageServiceTest { + + private static final String CONTAINER_NAME = "test-container"; + private static final String FILE_NAME = "testfile.txt"; + private static final byte[] FILE_CONTENT = "mock-content".getBytes(StandardCharsets.UTF_8); + + @Mock + private BlobServiceClient blobServiceClient; + @Mock + private StorageServiceConfiguration configuration; + @Mock + private BlobContainerClient blobContainerClient; + @Mock + private BlobClient blobClient; + @Mock + private BlockBlobClient blockBlobClient; + @Mock + private BlobProperties blobProperties; + + private AzureStorageService azureStorageService; + + @BeforeEach + void setUp() { + when(configuration.getContainerName()).thenReturn(CONTAINER_NAME); + azureStorageService = new AzureStorageService(blobServiceClient, configuration); + } + + private void mockBlobClientChain() { + when(blobServiceClient.getBlobContainerClient(CONTAINER_NAME)).thenReturn(blobContainerClient); + when(blobContainerClient.getBlobClient(FILE_NAME)).thenReturn(blobClient); + when(blobClient.getBlockBlobClient()).thenReturn(blockBlobClient); + } + + @Test + void uploadFile_SuccessfullyUploadsToAzure() throws StorageException { + mockBlobClientChain(); + + azureStorageService.uploadFile(FILE_NAME, FILE_CONTENT); + + verify(blockBlobClient).upload(any(InputStream.class), eq((long) FILE_CONTENT.length)); + } + + @Test + void downloadFile_SuccessfullyDownloadsFromAzure() throws StorageException { + mockBlobClientChain(); + when(blockBlobClient.getProperties()).thenReturn(blobProperties); + when(blobProperties.getBlobSize()).thenReturn((long) FILE_CONTENT.length); + + doAnswer(invocation -> { + ByteArrayOutputStream outputStream = invocation.getArgument(0); + outputStream.write(FILE_CONTENT); + return null; + }).when(blockBlobClient).downloadStream(any(ByteArrayOutputStream.class)); + + byte[] result = azureStorageService.downloadFile(FILE_NAME); + + assertArrayEquals(FILE_CONTENT, result); + } + + @Test + void deleteFile_SuccessfullyDeletesFromAzure() { + mockBlobClientChain(); + + azureStorageService.deleteFile(FILE_NAME); + + verify(blockBlobClient).delete(); + } + + @Test + void getFileLocation_ReturnsCorrectUrl() { + mockBlobClientChain(); + String expectedUrl = "https://azuremock.blob.core.windows.net/test-container/testfile.txt"; + when(blockBlobClient.getBlobUrl()).thenReturn(expectedUrl); + + String result = azureStorageService.getFileLocation(FILE_NAME); + + assertEquals(expectedUrl, result); + } +} \ No newline at end of file From 283e2fd9ccb6d099d40eff4bf461e636e699c057 Mon Sep 17 00:00:00 2001 From: MartinWheelerMT Date: Wed, 6 May 2026 17:15:04 +0100 Subject: [PATCH 6/8] * Add annotations to `StorageServiceConfig` to tell the compiler that a null value will not be returned. * Add unit tests for `StorageServiceConfig` --- .../storage/StorageServiceConfig.java | 7 +- .../storage/StorageServiceConfigTest.java | 75 +++++++++++++++++++ 2 files changed, 79 insertions(+), 3 deletions(-) create mode 100644 gp2gp-translator/src/test/java/uk/nhs/adaptors/pss/translator/storage/StorageServiceConfigTest.java diff --git a/gp2gp-translator/src/main/java/uk/nhs/adaptors/pss/translator/storage/StorageServiceConfig.java b/gp2gp-translator/src/main/java/uk/nhs/adaptors/pss/translator/storage/StorageServiceConfig.java index ddcfd613d..63a14dc0d 100644 --- a/gp2gp-translator/src/main/java/uk/nhs/adaptors/pss/translator/storage/StorageServiceConfig.java +++ b/gp2gp-translator/src/main/java/uk/nhs/adaptors/pss/translator/storage/StorageServiceConfig.java @@ -3,6 +3,7 @@ import com.azure.storage.blob.BlobServiceClient; import com.azure.storage.blob.BlobServiceClientBuilder; import com.azure.storage.common.StorageSharedKeyCredential; +import jakarta.annotation.Nonnull; import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; @@ -16,7 +17,7 @@ public class StorageServiceConfig { @Bean @ConditionalOnProperty(name = "storage.type", havingValue = "Azure") - public StorageService azureStorageService(StorageServiceConfiguration configuration) { + public @Nonnull StorageService azureStorageService(StorageServiceConfiguration configuration) { StorageSharedKeyCredential credentials = new StorageSharedKeyCredential( configuration.getAccountReference(), configuration.getAccountSecret() @@ -38,7 +39,7 @@ public StorageService azureStorageService(StorageServiceConfiguration configurat @Bean @ConditionalOnProperty(name = "storage.type", havingValue = "S3") - public StorageService awsStorageService(StorageServiceConfiguration configuration) { + public @Nonnull StorageService awsStorageService(StorageServiceConfiguration configuration) { return new AWSStorageService( S3Client.builder().build(), configuration, @@ -48,7 +49,7 @@ public StorageService awsStorageService(StorageServiceConfiguration configuratio @Bean @ConditionalOnProperty(name = "storage.type", havingValue = "LocalMock", matchIfMissing = true) - public StorageService localStorageService() { + public @Nonnull StorageService localStorageService() { return new LocalStorageService(); } } diff --git a/gp2gp-translator/src/test/java/uk/nhs/adaptors/pss/translator/storage/StorageServiceConfigTest.java b/gp2gp-translator/src/test/java/uk/nhs/adaptors/pss/translator/storage/StorageServiceConfigTest.java new file mode 100644 index 000000000..299db2a4c --- /dev/null +++ b/gp2gp-translator/src/test/java/uk/nhs/adaptors/pss/translator/storage/StorageServiceConfigTest.java @@ -0,0 +1,75 @@ +package uk.nhs.adaptors.pss.translator.storage; + +import org.junit.jupiter.api.Test; +import org.springframework.boot.test.context.runner.ApplicationContextRunner; + +import java.util.Base64; + +import static org.assertj.core.api.Assertions.assertThat; + +public class StorageServiceConfigTest { + + private final ApplicationContextRunner contextRunner = new ApplicationContextRunner() + .withUserConfiguration(StorageServiceConfig.class); + + @Test + void azureStorageService_IsCreated_WhenTypeIsAzure() { + // Arrange & Act + contextRunner.withPropertyValues("storage.type=Azure") + .withBean(StorageServiceConfiguration.class, () -> { + StorageServiceConfiguration config = new StorageServiceConfiguration(); + config.setAccountReference("account"); + config.setAccountSecret(Base64.getEncoder().encodeToString("secret".getBytes())); + config.setContainerName("container"); + return config; + }) + // Assert + .run(context -> { + assertThat(context).hasSingleBean(StorageService.class); + assertThat(context).hasBean("azureStorageService"); + assertThat(context.getBean(StorageService.class)).isInstanceOf(AzureStorageService.class); + }); + } + + @Test + void awsStorageService_IsCreated_WhenTypeIsS3() { + System.setProperty("aws.region", "eu-west-2"); + + contextRunner.withPropertyValues("storage.type=S3") + .withBean(StorageServiceConfiguration.class, StorageServiceConfiguration::new) + .run(context -> { + assertThat(context).hasSingleBean(StorageService.class); + assertThat(context).hasBean("awsStorageService"); + assertThat(context.getBean(StorageService.class)).isInstanceOf(AWSStorageService.class); + }); + + System.clearProperty("aws.region"); + } + + @Test + void localStorageService_IsCreated_WhenTypeIsLocalMock() { + contextRunner.withPropertyValues("storage.type=LocalMock") + .run(context -> { + assertThat(context).hasSingleBean(StorageService.class); + assertThat(context).hasBean("localStorageService"); + assertThat(context.getBean(StorageService.class)).isInstanceOf(LocalStorageService.class); + }); + } + + @Test + void localStorageService_IsCreated_WhenTypeIsMissing() { + contextRunner + .run(context -> { + assertThat(context).hasSingleBean(StorageService.class); + assertThat(context).hasBean("localStorageService"); + assertThat(context.getBean(StorageService.class)).isInstanceOf(LocalStorageService.class); + }); + } + + @Test + void noStorageService_IsCreated_WhenTypeIsInvalid() { + contextRunner.withPropertyValues("storage.type=INVALID_TYPE") + .withBean(StorageServiceConfiguration.class, StorageServiceConfiguration::new) + .run(context -> assertThat(context).doesNotHaveBean(StorageService.class)); + } +} \ No newline at end of file From a56489ed896e06f1895059777ab08273e3a3fb1f Mon Sep 17 00:00:00 2001 From: MartinWheelerMT Date: Wed, 6 May 2026 17:18:07 +0100 Subject: [PATCH 7/8] * Update container name to remove underscore --- .../pss/translator/storage/AzureStorageServiceTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gp2gp-translator/src/test/java/uk/nhs/adaptors/pss/translator/storage/AzureStorageServiceTest.java b/gp2gp-translator/src/test/java/uk/nhs/adaptors/pss/translator/storage/AzureStorageServiceTest.java index e9fae3827..c08645869 100644 --- a/gp2gp-translator/src/test/java/uk/nhs/adaptors/pss/translator/storage/AzureStorageServiceTest.java +++ b/gp2gp-translator/src/test/java/uk/nhs/adaptors/pss/translator/storage/AzureStorageServiceTest.java @@ -26,7 +26,7 @@ @ExtendWith(MockitoExtension.class) public class AzureStorageServiceTest { - private static final String CONTAINER_NAME = "test-container"; + private static final String CONTAINER_NAME = "container"; private static final String FILE_NAME = "testfile.txt"; private static final byte[] FILE_CONTENT = "mock-content".getBytes(StandardCharsets.UTF_8); From 2895fad1659472bee5967b45f9a9919bbcd86552 Mon Sep 17 00:00:00 2001 From: MartinWheelerMT Date: Wed, 6 May 2026 17:31:38 +0100 Subject: [PATCH 8/8] * Address issues with test names. * Add default coding for for the byte conversion in the tests. --- .../storage/AzureStorageServiceTest.java | 8 ++++---- .../storage/StorageServiceConfigTest.java | 15 +++++++++------ 2 files changed, 13 insertions(+), 10 deletions(-) diff --git a/gp2gp-translator/src/test/java/uk/nhs/adaptors/pss/translator/storage/AzureStorageServiceTest.java b/gp2gp-translator/src/test/java/uk/nhs/adaptors/pss/translator/storage/AzureStorageServiceTest.java index c08645869..2d46d31f1 100644 --- a/gp2gp-translator/src/test/java/uk/nhs/adaptors/pss/translator/storage/AzureStorageServiceTest.java +++ b/gp2gp-translator/src/test/java/uk/nhs/adaptors/pss/translator/storage/AzureStorageServiceTest.java @@ -58,7 +58,7 @@ private void mockBlobClientChain() { } @Test - void uploadFile_SuccessfullyUploadsToAzure() throws StorageException { + void When_UploadFile_Expect_SuccessfullyUploadsToAzure() throws StorageException { mockBlobClientChain(); azureStorageService.uploadFile(FILE_NAME, FILE_CONTENT); @@ -67,7 +67,7 @@ void uploadFile_SuccessfullyUploadsToAzure() throws StorageException { } @Test - void downloadFile_SuccessfullyDownloadsFromAzure() throws StorageException { + void When_DownloadFile_Expect_SuccessfullyDownloadsFromAzure() throws StorageException { mockBlobClientChain(); when(blockBlobClient.getProperties()).thenReturn(blobProperties); when(blobProperties.getBlobSize()).thenReturn((long) FILE_CONTENT.length); @@ -84,7 +84,7 @@ void downloadFile_SuccessfullyDownloadsFromAzure() throws StorageException { } @Test - void deleteFile_SuccessfullyDeletesFromAzure() { + void When_DeleteFile_Expect_SuccessfullyDeletesFromAzure() { mockBlobClientChain(); azureStorageService.deleteFile(FILE_NAME); @@ -93,7 +93,7 @@ void deleteFile_SuccessfullyDeletesFromAzure() { } @Test - void getFileLocation_ReturnsCorrectUrl() { + void When_GetFileLocation_Expect_ReturnsCorrectUrl() { mockBlobClientChain(); String expectedUrl = "https://azuremock.blob.core.windows.net/test-container/testfile.txt"; when(blockBlobClient.getBlobUrl()).thenReturn(expectedUrl); diff --git a/gp2gp-translator/src/test/java/uk/nhs/adaptors/pss/translator/storage/StorageServiceConfigTest.java b/gp2gp-translator/src/test/java/uk/nhs/adaptors/pss/translator/storage/StorageServiceConfigTest.java index 299db2a4c..b6f233539 100644 --- a/gp2gp-translator/src/test/java/uk/nhs/adaptors/pss/translator/storage/StorageServiceConfigTest.java +++ b/gp2gp-translator/src/test/java/uk/nhs/adaptors/pss/translator/storage/StorageServiceConfigTest.java @@ -3,6 +3,7 @@ import org.junit.jupiter.api.Test; import org.springframework.boot.test.context.runner.ApplicationContextRunner; +import java.nio.charset.StandardCharsets; import java.util.Base64; import static org.assertj.core.api.Assertions.assertThat; @@ -13,13 +14,15 @@ public class StorageServiceConfigTest { .withUserConfiguration(StorageServiceConfig.class); @Test - void azureStorageService_IsCreated_WhenTypeIsAzure() { + void When_TypeIsAzure_Expect_AzureStorageServiceIsCreated() { // Arrange & Act contextRunner.withPropertyValues("storage.type=Azure") .withBean(StorageServiceConfiguration.class, () -> { StorageServiceConfiguration config = new StorageServiceConfiguration(); config.setAccountReference("account"); - config.setAccountSecret(Base64.getEncoder().encodeToString("secret".getBytes())); + config.setAccountSecret( + Base64.getEncoder().encodeToString("secret".getBytes(StandardCharsets.UTF_8)) + ); config.setContainerName("container"); return config; }) @@ -32,7 +35,7 @@ void azureStorageService_IsCreated_WhenTypeIsAzure() { } @Test - void awsStorageService_IsCreated_WhenTypeIsS3() { + void When_TypeIsS3_Expect_AwsStorageServiceIsCreated() { System.setProperty("aws.region", "eu-west-2"); contextRunner.withPropertyValues("storage.type=S3") @@ -47,7 +50,7 @@ void awsStorageService_IsCreated_WhenTypeIsS3() { } @Test - void localStorageService_IsCreated_WhenTypeIsLocalMock() { + void When_TypeIsLocalMock_Expect_LocalStorageServiceIsCreated() { contextRunner.withPropertyValues("storage.type=LocalMock") .run(context -> { assertThat(context).hasSingleBean(StorageService.class); @@ -57,7 +60,7 @@ void localStorageService_IsCreated_WhenTypeIsLocalMock() { } @Test - void localStorageService_IsCreated_WhenTypeIsMissing() { + void When_TypeIsMissing_Expect_LocalStorageServiceIsCreated() { contextRunner .run(context -> { assertThat(context).hasSingleBean(StorageService.class); @@ -67,7 +70,7 @@ void localStorageService_IsCreated_WhenTypeIsMissing() { } @Test - void noStorageService_IsCreated_WhenTypeIsInvalid() { + void When_TypeIsInvalid_Expect_StorageServiceIsNotCreated() { contextRunner.withPropertyValues("storage.type=INVALID_TYPE") .withBean(StorageServiceConfiguration.class, StorageServiceConfiguration::new) .run(context -> assertThat(context).doesNotHaveBean(StorageService.class));