PRMT-4112 Addressing SonarCloud code smells

src/integration/java/uk/nhs/prm/repo/ehrtransferservice/parsers/ParserBrokerIntegrationTest.java

@@ -73,13 +73,13 @@ public class ParserBrokerIntegrationTest {
     @Autowired
     private TransferStore transferStore;
 
-    private final String conversationIdForSmallEhr="ff27abc3-9730-40f7-ba82-382152e6b90a";
-    private final String conversationIdForCopc="ff1457fb-4f58-4870-8d90-24d9c3ef8b91";
+    private static final String CONVERSATION_ID_FOR_SMALL_EHR = "ff27abc3-9730-40f7-ba82-382152e6b90a";
+    private static final String CONVERSATION_ID_FOR_COPC = "ff1457fb-4f58-4870-8d90-24d9c3ef8b91";
 
     @BeforeEach
     public void setup(){
-        RepoIncomingEvent repoIncomingEventForSmallEhr= new RepoIncomingEvent("NHS_number_12312","gp_4823","NemsId_48309","dest_gp_2484","2023-01-05",conversationIdForSmallEhr);
-        RepoIncomingEvent repoIncomingEventForCopc= new RepoIncomingEvent("NHS_number_12312","gp_4823","NemsId_48309","dest_gp_2484","2023-01-05",conversationIdForCopc);
+        RepoIncomingEvent repoIncomingEventForSmallEhr = new RepoIncomingEvent("NHS_number_12312","gp_4823","NemsId_48309","dest_gp_2484","2023-01-05", CONVERSATION_ID_FOR_SMALL_EHR);
+        RepoIncomingEvent repoIncomingEventForCopc = new RepoIncomingEvent("NHS_number_12312","gp_4823","NemsId_48309","dest_gp_2484","2023-01-05", CONVERSATION_ID_FOR_COPC);
         transferStore.createEhrTransfer(repoIncomingEventForSmallEhr,"ACTION:EHR_REQUEST_SENT");
         transferStore.createEhrTransfer(repoIncomingEventForCopc,"ACTION:EHR_REQUEST_SENT");
     }
@@ -95,10 +95,10 @@ public void tearDown() {
 
 
         Map<String, AttributeValue> key = new HashMap<>();
-        key.put("conversation_id", AttributeValue.builder().s(conversationIdForSmallEhr).build());
+        key.put("conversation_id", AttributeValue.builder().s(CONVERSATION_ID_FOR_SMALL_EHR).build());
         dbClient.deleteItem(DeleteItemRequest.builder().tableName(transferTrackerDbTableName).key(key).build());
         key.clear();
-        key.put("conversation_id", AttributeValue.builder().s(conversationIdForCopc).build());
+        key.put("conversation_id", AttributeValue.builder().s(CONVERSATION_ID_FOR_COPC).build());
         dbClient.deleteItem(DeleteItemRequest.builder().tableName(transferTrackerDbTableName).key(key).build());
     }
 

src/main/java/uk/nhs/prm/repo/ehrtransferservice/config/ActiveMQConfig.java

@@ -7,7 +7,6 @@
 import org.springframework.context.annotation.Bean;
 import org.springframework.context.annotation.Configuration;
 import org.springframework.jms.config.DefaultJmsListenerContainerFactory;
-import org.springframework.jms.config.JmsListenerContainerFactory;
 
 import javax.jms.ConnectionFactory;
 

src/main/java/uk/nhs/prm/repo/ehrtransferservice/config/DynamoDBClientSpringConfiguration.java

@@ -4,7 +4,6 @@
 import org.springframework.beans.factory.annotation.Value;
 import org.springframework.context.annotation.Bean;
 import org.springframework.context.annotation.Configuration;
-import software.amazon.awssdk.auth.credentials.AwsBasicCredentials;
 import software.amazon.awssdk.auth.credentials.AwsCredentialsProvider;
 import software.amazon.awssdk.regions.Region;
 import software.amazon.awssdk.services.dynamodb.DynamoDbClient;

src/test/java/uk/nhs/prm/repo/ehrtransferservice/gp2gp_message_models/EhrExtractMessageWrapperTest.java

@@ -9,7 +9,7 @@
 import static org.hamcrest.Matchers.equalTo;
 
 @Tag("unit")
-public class EhrExtractMessageWrapperTest {
+class EhrExtractMessageWrapperTest {
     @Test
     void shouldRetrieveNHSNumberFromEHRExtract() throws JsonProcessingException {
         String extract = "<?xml version=\"1.0\" encoding=\"UTF-8\"?>\n" +

src/test/java/uk/nhs/prm/repo/ehrtransferservice/gp2gp_message_models/EhrRequestMessageWrapperTest.java

@@ -9,7 +9,7 @@
 import static org.hamcrest.Matchers.equalTo;
 
 @Tag("unit")
-public class EhrRequestMessageWrapperTest {
+class EhrRequestMessageWrapperTest {
     @Test
     void shouldRetrieveNHSNumberFromEhrRequest() throws JsonProcessingException {
         String extract = "<?xml version=\"1.0\" encoding=\"UTF-8\"?>\n" +

src/test/java/uk/nhs/prm/repo/ehrtransferservice/gp2gp_message_models/ParsedMessageTest.java

@@ -11,7 +11,7 @@
 import static org.hamcrest.Matchers.equalTo;
 
 @Tag("unit")
-public class ParsedMessageTest {
+class ParsedMessageTest {
     private final Reference mid;
     private final Reference othermid;
     private final Reference cid;

src/test/java/uk/nhs/prm/repo/ehrtransferservice/handlers/LargeMessageFragmentHandlerTest.java

@@ -60,7 +60,7 @@ void shouldCallEhrRepoServiceToStoreTheMessage() throws Exception {
         when(ehrRepoService.storeMessage(largeMessageFragmentsArgumentCaptor.capture())).thenReturn(new StoreMessageResult(new StoreMessageResponseBody("complete")));
         largeMessageFragmentHandler.handleMessage(largeSqsMessage);
         verify(ehrRepoService).storeMessage(largeMessageFragmentsArgumentCaptor.capture());
-        assertThat(largeMessageFragmentsArgumentCaptor.getValue().getNhsNumber()).isEqualTo("");
+        assertThat(largeMessageFragmentsArgumentCaptor.getValue().getNhsNumber()).isEmpty();
     }
 
     @Test

src/test/java/uk/nhs/prm/repo/ehrtransferservice/handlers/NegativeAcknowledgementHandlerTest.java

@@ -24,7 +24,7 @@
 import static org.mockito.Mockito.*;
 
 @ExtendWith(MockitoExtension.class)
-public class NegativeAcknowledgementHandlerTest {
+class NegativeAcknowledgementHandlerTest {
 
     @Mock
     TransferStore transferStore;

src/test/java/uk/nhs/prm/repo/ehrtransferservice/models/ack/AcknowledgementTest.java

@@ -1,45 +1,33 @@
 package uk.nhs.prm.repo.ehrtransferservice.models.ack;
 
-import org.junit.jupiter.api.BeforeEach;
-import org.junit.jupiter.api.Test;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.Arguments;
+import org.junit.jupiter.params.provider.MethodSource;
 import uk.nhs.prm.repo.ehrtransferservice.gp2gp_message_models.AcknowledgementMessageWrapper;
 
+import java.util.stream.Stream;
+
 import static org.assertj.core.api.Assertions.assertThat;
 
 class AcknowledgementTest {
-    private AcknowledgementMessageWrapper acknowledgementMessageWrapper;
-
-    @BeforeEach
-    void setUp() {
-        acknowledgementMessageWrapper = new AcknowledgementMessageWrapper();
+    @ParameterizedTest
+    @MethodSource("provideArgumentsForAcknowledgementType")
+    void acknowledgementTypeCodeShouldBeNegativeAcknowledgement(String typeCode, boolean expected) {
+        // given
+        AcknowledgementMessageWrapper acknowledgementMessageWrapper = new AcknowledgementMessageWrapper();
         acknowledgementMessageWrapper.acknowledgement = new AcknowledgementMessageWrapper.Acknowledgement();
-    }
-
-    @Test
-    public void shouldBeAPositiveAcknowledgementIfTypeIsAA() {
-        acknowledgementMessageWrapper.acknowledgement.typeCode = "AA";
-        var ack = new Acknowledgement(null, acknowledgementMessageWrapper, "bob");
-        assertThat(ack.isNegativeAcknowledgement()).isFalse();
-    }
-
-    @Test
-    public void shouldBeANegativeAcknowledgementIfTypeIsAE() {
-        acknowledgementMessageWrapper.acknowledgement.typeCode = "AE";
+        acknowledgementMessageWrapper.acknowledgement.typeCode = typeCode;
         var ack = new Acknowledgement(null, acknowledgementMessageWrapper, "bob");
-        assertThat(ack.isNegativeAcknowledgement()).isTrue();
-    }
 
-    @Test
-    public void shouldBeANegativeAcknowledgementIfTypeIsAR() {
-        acknowledgementMessageWrapper.acknowledgement.typeCode = "AR";
-        var ack = new Acknowledgement(null, acknowledgementMessageWrapper, "bob");
-        assertThat(ack.isNegativeAcknowledgement()).isTrue();
+        // then
+        assertThat(ack.isNegativeAcknowledgement()).isEqualTo(expected);
     }
 
-    @Test
-    public void shouldBeANegativeAcknowledgementIfTypeIsUnknown() {
-        acknowledgementMessageWrapper.acknowledgement.typeCode = "something that should never be set";
-        var ack = new Acknowledgement(null, acknowledgementMessageWrapper, "bob");
-        assertThat(ack.isNegativeAcknowledgement()).isTrue();
+    private static Stream<Arguments> provideArgumentsForAcknowledgementType() {
+        return Stream.of(
+                Arguments.of("AA", false),
+                Arguments.of("AE", true),
+                Arguments.of("AR", true),
+                Arguments.of("something that should never be set", true));
     }
 }

src/test/java/uk/nhs/prm/repo/ehrtransferservice/parsers/MCCIUK13AcknowledgementParsingTest.java

@@ -12,27 +12,27 @@
 import static org.assertj.core.api.Assertions.assertThat;
 
 @Tag("unit")
-public class MCCIUK13AcknowledgementParsingTest {
+class MCCIUK13AcknowledgementParsingTest {
 
     private final Parser parser = new Parser();
     private final ReadableTestDataHandler readableReader = new ReadableTestDataHandler();
 
     @Test
-    public void shouldNotFailToParseWhenFailedToExtractFailureDetailsFromNegativeAcknowledgement() throws IOException {
+    void shouldNotFailToParseWhenFailedToExtractFailureDetailsFromNegativeAcknowledgement() throws IOException {
         String messageAsString = readableReader.readMessage("MCCI_IN010000UK13", "EmptyFailure");
         var parsedAcknowledgement = (Acknowledgement) parser.parse(messageAsString);
 
         assertThat(parsedAcknowledgement.getFailureDetails()).isEmpty();
     }
 
     @Test
-    public void shouldExtractFailureDetailsFromTheReasonsFor_AE_TypeFailure() throws IOException {
+    void shouldExtractFailureDetailsFromTheReasonsFor_AE_TypeFailure() throws IOException {
         String messageAsString = readableReader.readMessage("MCCI_IN010000UK13", "AE_TypeFailure");
         var parsedAcknowledgement = (Acknowledgement) parser.parse(messageAsString);
 
         assertThat(parsedAcknowledgement.getTypeCode()).isEqualTo(AcknowledgementTypeCode.AE);
 
-        assertThat(parsedAcknowledgement.getFailureDetails().size()).isEqualTo(2);
+        assertThat(parsedAcknowledgement.getFailureDetails()).hasSize(2);
         var firstReasonFailure = parsedAcknowledgement.getFailureDetails().get(0);
         assertThat(firstReasonFailure.displayName()).isEqualTo("Update Failed");
         assertThat(firstReasonFailure.code()).isEqualTo("15");
@@ -46,21 +46,21 @@ public void shouldExtractFailureDetailsFromTheReasonsFor_AE_TypeFailure() throws
     }
 
     @Test
-    public void shouldNotFailToParseWhenReasonQualifierHasSomeExtraFieldsOnIt() throws IOException {
+    void shouldNotFailToParseWhenReasonQualifierHasSomeExtraFieldsOnIt() throws IOException {
         String messageAsString = readableReader.readMessage("MCCI_IN010000UK13", "AE_TypeFailure_ExtraQualifierFields");
         var parsed = (Acknowledgement) parser.parse(messageAsString);
 
         assertThat(parsed.getFailureDetails().get(0).level()).isEqualTo(FailureLevel.WARNING);
     }
 
     @Test
-    public void shouldExtractFailureDetailsFromTheReasonsFor_AR_TypeFailure() throws IOException {
+    void shouldExtractFailureDetailsFromTheReasonsFor_AR_TypeFailure() throws IOException {
         String messageAsString = readableReader.readMessage("MCCI_IN010000UK13", "AR_TypeFailure");
         var parsedAcknowledgement = (Acknowledgement) parser.parse(messageAsString);
 
         assertThat(parsedAcknowledgement.getTypeCode()).isEqualTo(AcknowledgementTypeCode.AR);
 
-        assertThat(parsedAcknowledgement.getFailureDetails().size()).isEqualTo(2);
+        assertThat(parsedAcknowledgement.getFailureDetails()).hasSize(2);
         var firstReasonFailure = parsedAcknowledgement.getFailureDetails().get(0);
         assertThat(firstReasonFailure.displayName()).isEqualTo("Access denied");
         assertThat(firstReasonFailure.code()).isEqualTo("1");

src/test/java/uk/nhs/prm/repo/ehrtransferservice/services/PresignedUrlTest.java

@@ -17,7 +17,7 @@
 import static org.junit.jupiter.api.Assertions.assertThrows;
 
 @Tag("unit")
-public class PresignedUrlTest {
+class PresignedUrlTest {
 
     @RegisterExtension
     WireMockExtension wireMock = new WireMockExtension();

src/test/java/uk/nhs/prm/repo/ehrtransferservice/services/ehr_repo/EhrRepoServiceTest.java

@@ -21,7 +21,7 @@
 
 @Tag("unit")
 @ExtendWith(MockitoExtension.class)
-public class EhrRepoServiceTest {
+class EhrRepoServiceTest {
     EhrRepoService ehrRepoService;
     @Mock
     EhrRepoClient mockEhrRepoClient;

src/test/java/uk/nhs/prm/repo/ehrtransferservice/services/ehr_repo/StoreMessageResultTest.java

@@ -1,33 +1,27 @@
 package uk.nhs.prm.repo.ehrtransferservice.services.ehr_repo;
 
-import org.junit.jupiter.api.Test;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.Arguments;
+import org.junit.jupiter.params.provider.MethodSource;
 import uk.nhs.prm.repo.ehrtransferservice.models.confirmmessagestored.StoreMessageResponseBody;
 
+import java.util.stream.Stream;
+
 import static org.assertj.core.api.Assertions.assertThat;
 
 class StoreMessageResultTest {
-
-    @Test
-    public void ehrIsCompleteIfHealthRecordStatusIsComplete() {
-        var result = new StoreMessageResult(new StoreMessageResponseBody("complete"));
-        assertThat(result.isEhrComplete()).isTrue();
-    }
-
-    @Test
-    public void ehrIsNotCompleteIfHealthRecordStatusIsAnythingOtherThanComplete() {
-        var result = new StoreMessageResult(new StoreMessageResponseBody("not complete"));
-        assertThat(result.isEhrComplete()).isFalse();
-    }
-
-    @Test
-    public void ehrIsNotCompleteIfHealthRecordStatusIsMissing() {
-        var result = new StoreMessageResult(new StoreMessageResponseBody(null));
-        assertThat(result.isEhrComplete()).isFalse();
+    @ParameterizedTest
+    @MethodSource("provideArgumentsForHealthRecordStatus")
+    void ehrIsCompleteIfHealthRecordStatusIsComplete(String healthRecordStatus, boolean expected) {
+        var result = new StoreMessageResult(new StoreMessageResponseBody(healthRecordStatus));
+        assertThat(result.isEhrComplete()).isEqualTo(expected);
     }
 
-    @Test
-    public void ehrIsNotCompleteIfHealthRecordStatusIsEmpty() {
-        var result = new StoreMessageResult(new StoreMessageResponseBody(""));
-        assertThat(result.isEhrComplete()).isFalse();
+    private static Stream<Arguments> provideArgumentsForHealthRecordStatus() {
+        return Stream.of(
+                Arguments.of("complete", true),
+                Arguments.of("not complete", false),
+                Arguments.of("", false),
+                Arguments.of(null, false));
     }
 }

src/test/java/uk/nhs/prm/repo/ehrtransferservice/services/gp2gp_messenger/Gp2gpMessengerServiceTest.java

@@ -26,7 +26,7 @@
 import static org.springframework.test.util.ReflectionTestUtils.setField;
 
 @ExtendWith(MockitoExtension.class)
-public class Gp2gpMessengerServiceTest {
+class Gp2gpMessengerServiceTest {
     @Mock
     Gp2gpMessengerClient gp2gpMessengerClient;
     @Mock