From eb7c9fd0704069a61f3b2597871326c3e87067e9 Mon Sep 17 00:00:00 2001 From: ORybak5 <12736698+ORybak5@users.noreply.github.com> Date: Fri, 22 May 2026 15:31:37 +0100 Subject: [PATCH] test refactoring --- .../uk/nhs/adaptors/gp2gp/ResourceHelper.java | 14 +- .../adaptors/gp2gp/ResourceHelperTest.java | 40 +++++ .../gp2gp/common/task/TaskConsumerTest.java | 84 +++++++--- .../ehr/SendDocumentTaskExecutorTest.java | 89 +++++----- .../ehr/request/EhrExtractAckHandlerTest.java | 88 ++-------- .../request/EhrExtractRequestHandlerTest.java | 90 +++++----- .../EhrExtractStatusValidatorTest.java | 156 +++++++++--------- .../gp2gp/mhs/InboundMessageConsumerTest.java | 78 ++++++--- .../gp2gp/mhs/InboundMessageHandlerTest.java | 34 +++- 9 files changed, 380 insertions(+), 293 deletions(-) create mode 100644 service/src/test/java/uk/nhs/adaptors/gp2gp/ResourceHelperTest.java diff --git a/service/src/test/java/uk/nhs/adaptors/gp2gp/ResourceHelper.java b/service/src/test/java/uk/nhs/adaptors/gp2gp/ResourceHelper.java index b1aff2756f..fd4722eaad 100644 --- a/service/src/test/java/uk/nhs/adaptors/gp2gp/ResourceHelper.java +++ b/service/src/test/java/uk/nhs/adaptors/gp2gp/ResourceHelper.java @@ -9,8 +9,20 @@ import uk.nhs.adaptors.gp2gp.common.service.XPathService; public class ResourceHelper { + public static String loadClasspathResourceAsString(String path) { - return new Scanner(ResourceHelper.class.getResourceAsStream(path), StandardCharsets.UTF_8).useDelimiter("\\A").next(); + if (path == null || path.isBlank()) { + throw new IllegalArgumentException("Classpath resource path must be provided"); + } + + var resourceStream = ResourceHelper.class.getResourceAsStream(path); + if (resourceStream == null) { + throw new IllegalArgumentException("Classpath resource not found: " + path); + } + + try (var scanner = new Scanner(resourceStream, StandardCharsets.UTF_8).useDelimiter("\\A")) { + return scanner.hasNext() ? scanner.next() : ""; + } } @SneakyThrows diff --git a/service/src/test/java/uk/nhs/adaptors/gp2gp/ResourceHelperTest.java b/service/src/test/java/uk/nhs/adaptors/gp2gp/ResourceHelperTest.java new file mode 100644 index 0000000000..a3ee4d565b --- /dev/null +++ b/service/src/test/java/uk/nhs/adaptors/gp2gp/ResourceHelperTest.java @@ -0,0 +1,40 @@ +package uk.nhs.adaptors.gp2gp; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; + +import org.junit.jupiter.api.Test; + +class ResourceHelperTest { + @Test + void When_ValidClasspathResourceProvided_Expect_ResourceLoadedAsString() { + var actual = ResourceHelper.loadClasspathResourceAsString("/ehr/request/RCMR_IN010000UK05_header.xml"); + + assertThat(actual) + .contains("DFF5321C-C6EA-468E-BBC2-B0E48000E071"); + } + + @Test + void When_ValidXmlClasspathResourceProvided_Expect_ResourceParsedAsXmlDocument() { + var actual = ResourceHelper.loadClasspathResourceAsXml("/ehr/request/RCMR_IN010000UK05_header.xml"); + + assertThat(actual.getDocumentElement().getNodeName()).isEqualTo("soap:Envelope"); + } + + @Test + void When_ResourcePathIsBlank_Expect_ClearException() { + assertThatThrownBy(() -> ResourceHelper.loadClasspathResourceAsString(" ")) + .isInstanceOf(IllegalArgumentException.class) + .hasMessage("Classpath resource path must be provided"); + } + + @Test + void When_ResourceDoesNotExist_Expect_ClearException() { + assertThatThrownBy(() -> ResourceHelper.loadClasspathResourceAsString("/does/not/exist.xml")) + .isInstanceOf(IllegalArgumentException.class) + .hasMessage("Classpath resource not found: /does/not/exist.xml"); + } +} + + diff --git a/service/src/test/java/uk/nhs/adaptors/gp2gp/common/task/TaskConsumerTest.java b/service/src/test/java/uk/nhs/adaptors/gp2gp/common/task/TaskConsumerTest.java index 9527714684..d0580b8255 100644 --- a/service/src/test/java/uk/nhs/adaptors/gp2gp/common/task/TaskConsumerTest.java +++ b/service/src/test/java/uk/nhs/adaptors/gp2gp/common/task/TaskConsumerTest.java @@ -3,6 +3,7 @@ import static org.assertj.core.api.AssertionsForClassTypes.assertThatExceptionOfType; import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.doThrow; +import static org.mockito.Mockito.never; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; @@ -42,79 +43,110 @@ class TaskConsumerTest { @Test @SneakyThrows void When_TaskHandlerReturnsTrue_Expect_MessageAcknowledged() { - when(taskHandler.handle(any())).thenReturn(true); + stubHandleResult(true); - taskConsumer.receive(message, session); + callReceive(); - verify(taskHandler).handle(message); - verify(message).acknowledge(); + verifyTaskHandled(); + verify(message, times(1)).acknowledge(); + verifyMdcReset(); } @Test @SneakyThrows void When_TaskHandlerReturnsFalse_Expect_MessageNotAcknowledged() { - when(taskHandler.handle(any())).thenReturn(false); + stubHandleResult(false); - taskConsumer.receive(message, session); + callReceive(); - verify(taskHandler).handle(message); - verify(message, times(0)).acknowledge(); + verifyTaskHandled(); + verify(message, never()).acknowledge(); + verifyMdcReset(); } @Test @SneakyThrows void When_TaskHandlerReturnsFalse_Expect_SessionRolledBack() { - when(taskHandler.handle(any())).thenReturn(false); + stubHandleResult(false); - taskConsumer.receive(message, session); + callReceive(); - verify(taskHandler).handle(message); + verifyTaskHandled(); verify(session, times(1)).rollback(); + verifyMdcReset(); } @Test @SneakyThrows void When_TaskHandlerThrowsException_Expect_MessageNotAcknowledged() { - doThrow(RuntimeException.class).when(taskHandler).handle(message); + stubHandleThrows(RuntimeException.class); - taskConsumer.receive(message, session); + callReceive(); - verify(taskHandler).handle(message); - verify(message, times(0)).acknowledge(); + verifyTaskHandled(); + verify(message, never()).acknowledge(); + verifyMdcReset(); } @Test @SneakyThrows void When_TaskHandlerThrowsException_Expect_SessionRolledBack() { - doThrow(RuntimeException.class).when(taskHandler).handle(message); + stubHandleThrows(RuntimeException.class); - taskConsumer.receive(message, session); + callReceive(); - verify(taskHandler).handle(message); + verifyTaskHandled(); verify(session, times(1)).rollback(); + verifyMdcReset(); } @Test @SneakyThrows void When_TaskHandlerThrowsDataResourceAccessFailureException_Expect_ExceptionIsThrown() { - doThrow(DataAccessResourceFailureException.class).when(taskHandler).handle(message); + stubHandleThrows(DataAccessResourceFailureException.class); assertThatExceptionOfType(DataAccessResourceFailureException.class) - .isThrownBy(() -> taskConsumer.receive(message, session)); + .isThrownBy(this::callReceive); - verify(session, times(0)).rollback(); - verify(message, times(0)).acknowledge(); + verify(session, never()).rollback(); + verify(message, never()).acknowledge(); + verifyMdcReset(); } @Test @SneakyThrows void When_TaskHandlerThrowsMhsConnectionException_Expect_ExceptionIsThrown() { - doThrow(MhsConnectionException.class).when(taskHandler).handle(message); + stubHandleThrows(MhsConnectionException.class); assertThatExceptionOfType(MhsConnectionException.class) - .isThrownBy(() -> taskConsumer.receive(message, session)); + .isThrownBy(this::callReceive); + + verify(session, never()).rollback(); + verify(message, never()).acknowledge(); + verifyMdcReset(); + } + + @SneakyThrows + private void callReceive() { + taskConsumer.receive(message, session); + } + + @SneakyThrows + private void stubHandleResult(boolean result) { + when(taskHandler.handle(any())).thenReturn(result); + } + + @SneakyThrows + private void stubHandleThrows(Class exceptionType) { + doThrow(exceptionType).when(taskHandler).handle(message); + } + + @SneakyThrows + private void verifyTaskHandled() { + verify(taskHandler).handle(message); + } - verify(session, times(0)).rollback(); - verify(message, times(0)).acknowledge(); + private void verifyMdcReset() { + verify(mdcService).resetAllMdcKeys(); } } diff --git a/service/src/test/java/uk/nhs/adaptors/gp2gp/ehr/SendDocumentTaskExecutorTest.java b/service/src/test/java/uk/nhs/adaptors/gp2gp/ehr/SendDocumentTaskExecutorTest.java index 2897aa4b07..087e2ac8fb 100644 --- a/service/src/test/java/uk/nhs/adaptors/gp2gp/ehr/SendDocumentTaskExecutorTest.java +++ b/service/src/test/java/uk/nhs/adaptors/gp2gp/ehr/SendDocumentTaskExecutorTest.java @@ -43,7 +43,16 @@ @ExtendWith(MockitoExtension.class) class SendDocumentTaskExecutorTest { + private static final int SIZE_THRESHOLD_FOUR = 4; + private static final int SIZE_OF_EACH_CHUNK = 1; + private static final int NUMBER_OF_CHUNKS = 5; + private static final String STORAGE_FILE_NAME = "large_file_which_will_be_split.txt"; + private static final String STORAGE_CONTENT_TYPE_UNUSED = "should-not-be-used"; + private static final String RANDOM_ID = "RANDOM-ID"; + private static final String RANDOM_ODS = "RANDOM-ODS"; + private static final String MESSAGE_ID = "88"; + private static final String TASK_CONTENT_TYPE = "should-be-used"; @Mock private EhrExtractStatusService ehrExtractStatusService; @Mock private DetectDocumentsSentService detectDocumentsSentService; @Mock private MhsClient mhsClient; @@ -62,68 +71,56 @@ void When_ChunkingString_Expect_StringIsProperlySplit(String input, int sizeThre @SneakyThrows @Test void When_DocumentNeedsToBeSplitIntoFiveChunks_Expect_FiveMhsRequestsWithAttachmentsOfContentTypeOctetStream() { - final int SIZE_OF_EACH_CHUNK = 1; - final int NUMBER_OF_CHUNKS = 5; - final String storageFileName = "large_file_which_will_be_split.txt"; - // Arrange this.gp2gpConfiguration.setLargeAttachmentThreshold(SIZE_OF_EACH_CHUNK); - uploadDocumentToStorageWrapperWithPayloadSize(SIZE_OF_EACH_CHUNK * NUMBER_OF_CHUNKS, storageFileName, "should-not-be-used"); + uploadFiveChunkDocumentToStorage(); // Act - this.sendDocumentTaskExecutor.execute( - SendDocumentTaskDefinition.builder() - .documentName(storageFileName) - .messageId("88") - .fromOdsCode("RANDOM-ODS") - .conversationId("RANDOM-ID") - .documentContentType("should-be-used") - .build() - ); + this.sendDocumentTaskExecutor.execute(createTaskDefinition()); // Assert verify(mhsRequestBuilder, times(NUMBER_OF_CHUNKS)).buildSendEhrExtractCommonRequest( - argThat(mhsRequestBodyContainsAttachmentWithContentType(MimeTypes.OCTET_STREAM)), - eq("RANDOM-ID"), - eq("RANDOM-ODS"), + argThat(mhsRequestBodyContainsOctetStreamAttachment()), + eq(RANDOM_ID), + eq(RANDOM_ODS), any() ); + verify(mhsClient, times(NUMBER_OF_CHUNKS + 1)).sendMessageToMHS(any()); } @DisplayName("When_DocumentNeedsToBeSplitInto5Chunks_Expect_" + "MhsMessageWith5ExternalAttachmentWithDescriptionContentTypeHeaderFromTaskDefinition") @Test void When_DocumentNeedsToBeSplitInto5Chunks_Expect_MhsMessageWith5ExternalAttachmentCorrectlySet() { - final int SIZE_OF_EACH_CHUNK = 1; - final int NUMBER_OF_CHUNKS = 5; - final String storageFileName = "large_file_which_will_be_split.txt"; - // Arrange this.gp2gpConfiguration.setLargeAttachmentThreshold(SIZE_OF_EACH_CHUNK); - uploadDocumentToStorageWrapperWithPayloadSize(SIZE_OF_EACH_CHUNK * NUMBER_OF_CHUNKS, storageFileName, "should-not-be-used"); + uploadFiveChunkDocumentToStorage(); // Act - this.sendDocumentTaskExecutor.execute( - SendDocumentTaskDefinition.builder() - .documentName(storageFileName) - .messageId("88") - .fromOdsCode("RANDOM-ODS") - .conversationId("RANDOM-ID") - .documentContentType("should-be-used") - .build() - ); + this.sendDocumentTaskExecutor.execute(createTaskDefinition()); // Assert verify(mhsRequestBuilder, times(1)).buildSendEhrExtractCommonRequest( - argThat(mhsRequestBodyWithAnExternalAttachmentForEachChunkWithContentType(NUMBER_OF_CHUNKS, "should-be-used")), - eq("RANDOM-ID"), - eq("RANDOM-ODS"), + argThat(mhsRequestBodyHasExternalAttachmentForEachChunkWithTaskContentType()), + eq(RANDOM_ID), + eq(RANDOM_ODS), any() ); + verify(mhsClient, times(NUMBER_OF_CHUNKS + 1)).sendMessageToMHS(any()); + } + + private SendDocumentTaskDefinition createTaskDefinition() { + return SendDocumentTaskDefinition.builder() + .documentName(STORAGE_FILE_NAME) + .messageId(MESSAGE_ID) + .fromOdsCode(RANDOM_ODS) + .conversationId(RANDOM_ID) + .documentContentType(TASK_CONTENT_TYPE) + .build(); } @NotNull - private static ArgumentMatcher mhsRequestBodyContainsAttachmentWithContentType(String contentType) { + private static ArgumentMatcher mhsRequestBodyContainsOctetStreamAttachment() { return mhsRequestBody -> { ObjectMapper objectMapper = new ObjectMapper(); OutboundMessage outboundMessage; @@ -135,13 +132,13 @@ private static ArgumentMatcher mhsRequestBodyContainsAttachmentWithConte if (outboundMessage.getAttachments().isEmpty()) { return false; } - return Objects.equals(outboundMessage.getAttachments().get(0).getContentType(), contentType); + return Objects.equals(outboundMessage.getAttachments().getFirst().getContentType(), MimeTypes.OCTET_STREAM); }; } @NotNull private static ArgumentMatcher - mhsRequestBodyWithAnExternalAttachmentForEachChunkWithContentType(int numberOfChunks, String contentType) { + mhsRequestBodyHasExternalAttachmentForEachChunkWithTaskContentType() { return mhsRequestBody -> { ObjectMapper objectMapper = new ObjectMapper(); @@ -151,28 +148,28 @@ private static ArgumentMatcher mhsRequestBodyContainsAttachmentWithConte } catch (JsonProcessingException e) { return false; } - if (outboundMessage.getExternalAttachments() == null || outboundMessage.getExternalAttachments().size() != numberOfChunks) { + if (outboundMessage.getExternalAttachments() == null + || outboundMessage.getExternalAttachments().size() != NUMBER_OF_CHUNKS) { return false; } return outboundMessage.getExternalAttachments().stream().allMatch( - externalAttachment -> externalAttachment.getDescription().contains("ContentType=" + contentType) + externalAttachment -> externalAttachment.getDescription().contains("ContentType=" + TASK_CONTENT_TYPE) ); }; } - private void uploadDocumentToStorageWrapperWithPayloadSize(int payloadSize, String storageFileName, String contentType) { - byte[] storageDataWrapper = generateStorageDataWrapper(payloadSize, contentType); + private void uploadFiveChunkDocumentToStorage() { + byte[] storageDataWrapper = generateStorageDataWrapper(); this.storageConnector.uploadToStorage( new ByteArrayInputStream(storageDataWrapper), storageDataWrapper.length, - storageFileName + STORAGE_FILE_NAME ); } - @NotNull - private static byte[] generateStorageDataWrapper(int sizeOfPayload, String contentType) { - String payload = "a".repeat(sizeOfPayload); - String attachment = "{\"content_type\":\"" + contentType + "\",\"is_base64\":false" + private static byte[] generateStorageDataWrapper() { + String payload = "a".repeat(SIZE_OF_EACH_CHUNK * NUMBER_OF_CHUNKS); + String attachment = "{\"content_type\":\"" + STORAGE_CONTENT_TYPE_UNUSED + "\",\"is_base64\":false" + ",\"description\":\"\",\"payload\":\"" + payload + "\"}"; String outboundMessage = "{\"payload\": \"\", \"attachments\": [" + attachment + "], \"external_attachments\": []}"; String encodedData = outboundMessage.replace("\"", "\\\""); // Poor persons JSON encode diff --git a/service/src/test/java/uk/nhs/adaptors/gp2gp/ehr/request/EhrExtractAckHandlerTest.java b/service/src/test/java/uk/nhs/adaptors/gp2gp/ehr/request/EhrExtractAckHandlerTest.java index 9476b3bfb0..9e2bf5638c 100644 --- a/service/src/test/java/uk/nhs/adaptors/gp2gp/ehr/request/EhrExtractAckHandlerTest.java +++ b/service/src/test/java/uk/nhs/adaptors/gp2gp/ehr/request/EhrExtractAckHandlerTest.java @@ -86,10 +86,7 @@ void When_HandleUnsupportedAckTypeCode_Expect_ExceptionThrown() { @Test void When_Handle_WithAckAndMessageRefEqualsEhrExtractMessageId_Expect_ConversationIsClosed() { - - when(xPathService.getNodeValue(any(), eq(ACK_TYPE_CODE_XPATH))).thenReturn(ACK_OK_CODE); - when(xPathService.getNodeValue(any(), eq(MESSAGE_REF_XPATH))).thenReturn(EHR_MESSAGE_REF); - when(ehrExtractStatusService.fetchEhrExtractMessageId(CONVERSATION_ID)).thenReturn(Optional.of(EHR_MESSAGE_REF)); + stubAckAndMessageReference(ACK_OK_CODE, EHR_MESSAGE_REF, EHR_MESSAGE_REF); ehrExtractAckHandler.handle(CONVERSATION_ID, document); @@ -102,10 +99,7 @@ void When_Handle_WithAckAndMessageRefEqualsEhrExtractMessageId_Expect_Conversati @Test void When_Handle_WithAckAndMessageRefEqualsEhrExtractMessageId_Expect_AckSaved() { - - when(xPathService.getNodeValue(any(), eq(ACK_TYPE_CODE_XPATH))).thenReturn(ACK_OK_CODE); - when(xPathService.getNodeValue(any(), eq(MESSAGE_REF_XPATH))).thenReturn(EHR_MESSAGE_REF); - when(ehrExtractStatusService.fetchEhrExtractMessageId(CONVERSATION_ID)).thenReturn(Optional.of(EHR_MESSAGE_REF)); + stubAckAndMessageReference(ACK_OK_CODE, EHR_MESSAGE_REF, EHR_MESSAGE_REF); ehrExtractAckHandler.handle(CONVERSATION_ID, document); @@ -114,10 +108,7 @@ void When_Handle_WithAckAndMessageRefEqualsEhrExtractMessageId_Expect_AckSaved() @Test void When_Handle_WithAckDoesNotReferenceEhrExtract_Expect_ReceivedAckFieldNotUpdated() { - - when(xPathService.getNodeValue(any(), eq(ACK_TYPE_CODE_XPATH))).thenReturn(ACK_OK_CODE); - when(xPathService.getNodeValue(any(), eq(MESSAGE_REF_XPATH))).thenReturn(RANDOM_MESSAGE_REF); - when(ehrExtractStatusService.fetchEhrExtractMessageId(CONVERSATION_ID)).thenReturn(Optional.of(EHR_MESSAGE_REF)); + stubAckAndMessageReference(ACK_OK_CODE, RANDOM_MESSAGE_REF, EHR_MESSAGE_REF); ehrExtractAckHandler.handle(CONVERSATION_ID, document); @@ -126,10 +117,7 @@ void When_Handle_WithAckDoesNotReferenceEhrExtract_Expect_ReceivedAckFieldNotUpd @Test void When_Handle_WithAckDoesNotReferenceEhrExtract_Expect_AckSaved() { - - when(xPathService.getNodeValue(any(), eq(ACK_TYPE_CODE_XPATH))).thenReturn(ACK_OK_CODE); - when(xPathService.getNodeValue(any(), eq(MESSAGE_REF_XPATH))).thenReturn(RANDOM_MESSAGE_REF); - when(ehrExtractStatusService.fetchEhrExtractMessageId(CONVERSATION_ID)).thenReturn(Optional.of(EHR_MESSAGE_REF)); + stubAckAndMessageReference(ACK_OK_CODE, RANDOM_MESSAGE_REF, EHR_MESSAGE_REF); ehrExtractAckHandler.handle(CONVERSATION_ID, document); @@ -138,10 +126,8 @@ void When_Handle_WithAckDoesNotReferenceEhrExtract_Expect_AckSaved() { @Test void When_Handle_WithAckAndNoEhrExtractMessageIdForConversation_Expect_EhrExtractException() { - when(xPathService.getNodeValue(any(), eq(ACK_TYPE_CODE_XPATH))).thenReturn(ACK_OK_CODE); when(xPathService.getNodeValue(any(), eq(MESSAGE_REF_XPATH))).thenReturn(EHR_MESSAGE_REF); - when(ehrExtractStatusService.fetchEhrExtractMessageId(CONVERSATION_ID)).thenReturn(Optional.empty()); assertThatExceptionOfType(EhrExtractException.class) @@ -151,14 +137,9 @@ void When_Handle_WithAckAndNoEhrExtractMessageIdForConversation_Expect_EhrExtrac @Test void When_Handle_WithNackReferencesEhrExtract_Expect_ConversationClosed() throws XPathExpressionException, ParserConfigurationException, IOException, SAXException { - NodeList codeNodeList = codeElementToNodeList(ERROR_CODE_ELEMENT_99); - var document = mock(Document.class); - - when(xPathService.getNodeValue(any(), eq(ACK_TYPE_CODE_XPATH))).thenReturn(ACK_BUSINESS_ERROR_CODE); - when(xPathService.getNodeValue(any(), eq(MESSAGE_REF_XPATH))).thenReturn(EHR_MESSAGE_REF); + stubAckAndMessageReference(ACK_BUSINESS_ERROR_CODE, EHR_MESSAGE_REF, EHR_MESSAGE_REF); when(xPathService.getNodes(any(), eq(ERROR_CODE_XPATH))).thenReturn(codeNodeList); - when(ehrExtractStatusService.fetchEhrExtractMessageId(CONVERSATION_ID)).thenReturn(Optional.of(EHR_MESSAGE_REF)); ehrExtractAckHandler.handle(CONVERSATION_ID, document); @@ -170,14 +151,9 @@ void When_Handle_WithNackReferencesEhrExtract_Expect_ConversationClosed() throws @Test void When_Handle_WithNackReferencesEhrExtract_Expect_AckSaved() throws XPathExpressionException, ParserConfigurationException, IOException, SAXException { - NodeList codeNodeList = codeElementToNodeList(ERROR_CODE_ELEMENT_99); - var document = mock(Document.class); - - when(xPathService.getNodeValue(any(), eq(ACK_TYPE_CODE_XPATH))).thenReturn(ACK_BUSINESS_ERROR_CODE); - when(xPathService.getNodeValue(any(), eq(MESSAGE_REF_XPATH))).thenReturn(EHR_MESSAGE_REF); + stubAckAndMessageReference(ACK_BUSINESS_ERROR_CODE, EHR_MESSAGE_REF, EHR_MESSAGE_REF); when(xPathService.getNodes(any(), eq(ERROR_CODE_XPATH))).thenReturn(codeNodeList); - when(ehrExtractStatusService.fetchEhrExtractMessageId(CONVERSATION_ID)).thenReturn(Optional.of(EHR_MESSAGE_REF)); ehrExtractAckHandler.handle(CONVERSATION_ID, document); @@ -187,15 +163,9 @@ void When_Handle_WithNackReferencesEhrExtract_Expect_AckSaved() throws XPathExpr @Test void When_Handle_WithNackDoesNotReferenceExtract_Expect_ReceivedAckFieldNotUpdated() throws XPathExpressionException, ParserConfigurationException, IOException, SAXException { - NodeList codeNodeList = codeElementToNodeList(ERROR_CODE_ELEMENT_99); - - var document = mock(Document.class); - - when(xPathService.getNodeValue(any(), eq(ACK_TYPE_CODE_XPATH))).thenReturn(ACK_BUSINESS_ERROR_CODE); - when(xPathService.getNodeValue(any(), eq(MESSAGE_REF_XPATH))).thenReturn(EHR_MESSAGE_REF); + stubAckAndMessageReference(ACK_BUSINESS_ERROR_CODE, EHR_MESSAGE_REF, RANDOM_MESSAGE_REF); when(xPathService.getNodes(any(), eq(ERROR_CODE_XPATH))).thenReturn(codeNodeList); - when(ehrExtractStatusService.fetchEhrExtractMessageId(CONVERSATION_ID)).thenReturn(Optional.of(RANDOM_MESSAGE_REF)); ehrExtractAckHandler.handle(CONVERSATION_ID, document); @@ -205,15 +175,9 @@ void When_Handle_WithNackDoesNotReferenceExtract_Expect_ReceivedAckFieldNotUpdat @Test void When_Handle_WithNackDoesNotReferenceExtract_Expect_AckSaved() throws XPathExpressionException, ParserConfigurationException, IOException, SAXException { - NodeList codeNodeList = codeElementToNodeList(ERROR_CODE_ELEMENT_99); - - var document = mock(Document.class); - - when(xPathService.getNodeValue(any(), eq(ACK_TYPE_CODE_XPATH))).thenReturn(ACK_BUSINESS_ERROR_CODE); - when(xPathService.getNodeValue(any(), eq(MESSAGE_REF_XPATH))).thenReturn(EHR_MESSAGE_REF); + stubAckAndMessageReference(ACK_BUSINESS_ERROR_CODE, EHR_MESSAGE_REF, RANDOM_MESSAGE_REF); when(xPathService.getNodes(any(), eq(ERROR_CODE_XPATH))).thenReturn(codeNodeList); - when(ehrExtractStatusService.fetchEhrExtractMessageId(CONVERSATION_ID)).thenReturn(Optional.of(RANDOM_MESSAGE_REF)); ehrExtractAckHandler.handle(CONVERSATION_ID, document); @@ -223,15 +187,9 @@ void When_Handle_WithNackDoesNotReferenceExtract_Expect_AckSaved() throws XPathE @Test void When_Handle_WithRejectedReferencesEhrExtract_Expect_ConversationClosed() throws XPathExpressionException, ParserConfigurationException, IOException, SAXException { - NodeList codeNodeList = codeElementToNodeList(ERROR_CODE_ELEMENT_18); - - var document = mock(Document.class); - - when(xPathService.getNodeValue(any(), eq(ACK_TYPE_CODE_XPATH))).thenReturn(ACK_REJECTED_CODE); - when(xPathService.getNodeValue(any(), eq(MESSAGE_REF_XPATH))).thenReturn(EHR_MESSAGE_REF); + stubAckAndMessageReference(ACK_REJECTED_CODE, EHR_MESSAGE_REF, EHR_MESSAGE_REF); when(xPathService.getNodes(any(), eq(ACK_DETAILS_XPATH))).thenReturn(codeNodeList); - when(ehrExtractStatusService.fetchEhrExtractMessageId(CONVERSATION_ID)).thenReturn(Optional.of(EHR_MESSAGE_REF)); ehrExtractAckHandler.handle(CONVERSATION_ID, document); @@ -243,14 +201,9 @@ void When_Handle_WithRejectedReferencesEhrExtract_Expect_ConversationClosed() th @Test void When_Handle_WithRejectedDoesNotReferenceExtract_Expect_ReceivedAckFieldNotUpdated() throws XPathExpressionException, ParserConfigurationException, IOException, SAXException { - NodeList codeNodeList = codeElementToNodeList(ERROR_CODE_ELEMENT_18); - var document = mock(Document.class); - - when(xPathService.getNodeValue(any(), eq(ACK_TYPE_CODE_XPATH))).thenReturn(ACK_REJECTED_CODE); - when(xPathService.getNodeValue(any(), eq(MESSAGE_REF_XPATH))).thenReturn(EHR_MESSAGE_REF); + stubAckAndMessageReference(ACK_REJECTED_CODE, EHR_MESSAGE_REF, RANDOM_MESSAGE_REF); when(xPathService.getNodes(any(), eq(ACK_DETAILS_XPATH))).thenReturn(codeNodeList); - when(ehrExtractStatusService.fetchEhrExtractMessageId(CONVERSATION_ID)).thenReturn(Optional.of(RANDOM_MESSAGE_REF)); ehrExtractAckHandler.handle(CONVERSATION_ID, document); @@ -260,15 +213,9 @@ void When_Handle_WithRejectedDoesNotReferenceExtract_Expect_ReceivedAckFieldNotU @Test void When_Handle_WithRejectedReferencesEhrExtract_Expect_AckSaved() throws XPathExpressionException, ParserConfigurationException, IOException, SAXException { - NodeList codeNodeList = codeElementToNodeList(ERROR_CODE_ELEMENT_18); - - var document = mock(Document.class); - - when(xPathService.getNodeValue(any(), eq(ACK_TYPE_CODE_XPATH))).thenReturn(ACK_REJECTED_CODE); - when(xPathService.getNodeValue(any(), eq(MESSAGE_REF_XPATH))).thenReturn(EHR_MESSAGE_REF); + stubAckAndMessageReference(ACK_REJECTED_CODE, EHR_MESSAGE_REF, EHR_MESSAGE_REF); when(xPathService.getNodes(any(), eq(ACK_DETAILS_XPATH))).thenReturn(codeNodeList); - when(ehrExtractStatusService.fetchEhrExtractMessageId(CONVERSATION_ID)).thenReturn(Optional.of(EHR_MESSAGE_REF)); ehrExtractAckHandler.handle(CONVERSATION_ID, document); @@ -278,14 +225,9 @@ void When_Handle_WithRejectedReferencesEhrExtract_Expect_AckSaved() throws XPath @Test void When_Handle_WithRejectedDoesNotReferenceExtract_Expect_AckSaved() throws XPathExpressionException, ParserConfigurationException, IOException, SAXException { - NodeList codeNodeList = codeElementToNodeList(ERROR_CODE_ELEMENT_18); - var document = mock(Document.class); - - when(xPathService.getNodeValue(any(), eq(ACK_TYPE_CODE_XPATH))).thenReturn(ACK_REJECTED_CODE); - when(xPathService.getNodeValue(any(), eq(MESSAGE_REF_XPATH))).thenReturn(EHR_MESSAGE_REF); + stubAckAndMessageReference(ACK_REJECTED_CODE, EHR_MESSAGE_REF, RANDOM_MESSAGE_REF); when(xPathService.getNodes(any(), eq(ACK_DETAILS_XPATH))).thenReturn(codeNodeList); - when(ehrExtractStatusService.fetchEhrExtractMessageId(CONVERSATION_ID)).thenReturn(Optional.of(RANDOM_MESSAGE_REF)); ehrExtractAckHandler.handle(CONVERSATION_ID, document); @@ -305,4 +247,10 @@ private NodeList codeElementToNodeList(String element) throws XPathExpressionExc return (NodeList) xPathExpression.evaluate(document, NODESET); } + + private void stubAckAndMessageReference(String ackTypeCode, String messageRef, String storedEhrMessageRef) { + when(xPathService.getNodeValue(any(), eq(ACK_TYPE_CODE_XPATH))).thenReturn(ackTypeCode); + when(xPathService.getNodeValue(any(), eq(MESSAGE_REF_XPATH))).thenReturn(messageRef); + when(ehrExtractStatusService.fetchEhrExtractMessageId(CONVERSATION_ID)).thenReturn(Optional.of(storedEhrMessageRef)); + } } diff --git a/service/src/test/java/uk/nhs/adaptors/gp2gp/ehr/request/EhrExtractRequestHandlerTest.java b/service/src/test/java/uk/nhs/adaptors/gp2gp/ehr/request/EhrExtractRequestHandlerTest.java index b2da7df5d0..d39fb93f61 100644 --- a/service/src/test/java/uk/nhs/adaptors/gp2gp/ehr/request/EhrExtractRequestHandlerTest.java +++ b/service/src/test/java/uk/nhs/adaptors/gp2gp/ehr/request/EhrExtractRequestHandlerTest.java @@ -43,6 +43,9 @@ @ExtendWith(MockitoExtension.class) class EhrExtractRequestHandlerTest { + + private static final String REQUEST_HEADER_XML = "/ehr/request/RCMR_IN010000UK05_header.xml"; + private static final String REQUEST_BODY_XML = "/ehr/request/RCMR_IN010000UK05_body.xml"; private static final String REQUEST_ID = "041CA2AE-3EC6-4AC9-942F-0F6621CC0BFC"; private static final String CONVERSATION_ID = "DFF5321C-C6EA-468E-BBC2-B0E48000E071"; private static final String NHS_NUMBER = "9692294935"; @@ -60,6 +63,7 @@ class EhrExtractRequestHandlerTest { @Spy private XPathService xPathService; + @Mock private TimestampService timestampService; @@ -74,14 +78,13 @@ class EhrExtractRequestHandlerTest { @Test void When_ValidEhrRequestReceived_Expect_EhrExtractStatusIsCreated() { - Document soapHeader = ResourceHelper.loadClasspathResourceAsXml("/ehr/request/RCMR_IN010000UK05_header.xml"); - Document soapBody = ResourceHelper.loadClasspathResourceAsXml("/ehr/request/RCMR_IN010000UK05_body.xml"); + Document soapHeader = loadRequestHeader(); + Document soapBody = loadRequestBody(); Instant now = Instant.now(); when(timestampService.now()).thenReturn(now); when(randomIdGeneratorService.createNewId()).thenReturn(TASK_ID); - when(ehrExtractStatusRepository.save(any())) - .thenAnswer((Answer) invocation -> (EhrExtractStatus) invocation.getArguments()[0]); + stubRepositorySaveReturnsInput(); ehrExtractRequestHandler.handleStart(soapHeader, soapBody, MESSAGE_TIMESTAMP); @@ -94,8 +97,8 @@ void When_ValidEhrRequestReceived_Expect_EhrExtractStatusIsCreated() { @Test @SneakyThrows void When_DuplicateEhrExtractRequestReceived_Expect_NoTasksAreCreated() { - Document soapHeader = ResourceHelper.loadClasspathResourceAsXml("/ehr/request/RCMR_IN010000UK05_header.xml"); - Document soapBody = ResourceHelper.loadClasspathResourceAsXml("/ehr/request/RCMR_IN010000UK05_body.xml"); + Document soapHeader = loadRequestHeader(); + Document soapBody = loadRequestBody(); Instant now = Instant.now(); when(timestampService.now()).thenReturn(now); when(ehrExtractStatusRepository.save(any())).thenThrow(mock(DuplicateKeyException.class)); @@ -156,53 +159,42 @@ private static List pathsToBodyValuesNoEhrExtract() { } @SneakyThrows - @ParameterizedTest + @ParameterizedTest(name = "[{index}] missing body element at {0}") @MethodSource("pathsToBodyValues") void When_RequiredElementMissingFromBody_Expect_HandlerThrowsException(String xpath) { - Document header = ResourceHelper.loadClasspathResourceAsXml("/ehr/request/RCMR_IN010000UK05_header.xml"); - Document body = ResourceHelper.loadClasspathResourceAsXml("/ehr/request/RCMR_IN010000UK05_body.xml"); + Document header = loadRequestHeader(); + Document body = loadRequestBody(); - when(ehrExtractStatusRepository.save(any())) - .thenAnswer((Answer) invocation -> (EhrExtractStatus) invocation.getArguments()[0]); + stubRepositorySaveReturnsInput(); removeAttributeElement(xpath, body); - assertThatExceptionOfType(MissingValueException.class) - .isThrownBy(() -> ehrExtractRequestHandler.handleStart(header, body, MESSAGE_TIMESTAMP)) - .withMessageContaining(xpath) - .withMessageContaining(SpineInteraction.EHR_EXTRACT_REQUEST.getInteractionId()); + assertMissingValueExceptionForPath(header, body, xpath); } @SneakyThrows - @ParameterizedTest + @ParameterizedTest(name = "[{index}] blank body value at {0}") @MethodSource("pathsToBodyValues") void When_RequiredValueIsBlankInBody_Expect_HandlerThrowsException(String xpath) { - Document header = ResourceHelper.loadClasspathResourceAsXml("/ehr/request/RCMR_IN010000UK05_header.xml"); - Document body = ResourceHelper.loadClasspathResourceAsXml("/ehr/request/RCMR_IN010000UK05_body.xml"); - when(ehrExtractStatusRepository.save(any())) - .thenAnswer((Answer) invocation -> (EhrExtractStatus) invocation.getArguments()[0]); + Document header = loadRequestHeader(); + Document body = loadRequestBody(); + stubRepositorySaveReturnsInput(); clearAttribute(xpath, body); - assertThatExceptionOfType(MissingValueException.class) - .isThrownBy(() -> ehrExtractRequestHandler.handleStart(header, body, MESSAGE_TIMESTAMP)) - .withMessageContaining(xpath) - .withMessageContaining(SpineInteraction.EHR_EXTRACT_REQUEST.getInteractionId()); + assertMissingValueExceptionForPath(header, body, xpath); } @SneakyThrows - @ParameterizedTest + @ParameterizedTest(name = "[{index}] blank body value (no ehr extract path) at {0}") @MethodSource("pathsToBodyValuesNoEhrExtract") void When_RequiredValueIsBlankInBodyAndNoNeedForEhrExtract_Expect_HandlerThrowsException(String xpath) { - Document header = ResourceHelper.loadClasspathResourceAsXml("/ehr/request/RCMR_IN010000UK05_header.xml"); - Document body = ResourceHelper.loadClasspathResourceAsXml("/ehr/request/RCMR_IN010000UK05_body.xml"); + Document header = loadRequestHeader(); + Document body = loadRequestBody(); clearAttribute(xpath, body); - assertThatExceptionOfType(MissingValueException.class) - .isThrownBy(() -> ehrExtractRequestHandler.handleStart(header, body, MESSAGE_TIMESTAMP)) - .withMessageContaining(xpath) - .withMessageContaining(SpineInteraction.EHR_EXTRACT_REQUEST.getInteractionId()); + assertMissingValueExceptionForPath(header, body, xpath); } private static List pathsToHeaderValues() { @@ -213,35 +205,47 @@ private static List pathsToHeaderValues() { } @SneakyThrows - @ParameterizedTest + @ParameterizedTest(name = "[{index}] missing header element at {0}") @MethodSource("pathsToHeaderValues") void When_RequiredElementMissingFromHeader_Expect_HandlerThrowsException(String xpath) { - Document header = ResourceHelper.loadClasspathResourceAsXml("/ehr/request/RCMR_IN010000UK05_header.xml"); - Document body = ResourceHelper.loadClasspathResourceAsXml("/ehr/request/RCMR_IN010000UK05_body.xml"); + Document header = loadRequestHeader(); + Document body = loadRequestBody(); - when(ehrExtractStatusRepository.save(any())) - .thenAnswer((Answer) invocation -> (EhrExtractStatus) invocation.getArguments()[0]); + stubRepositorySaveReturnsInput(); removeElement(xpath, header); - assertThatExceptionOfType(MissingValueException.class) - .isThrownBy(() -> ehrExtractRequestHandler.handleStart(header, body, MESSAGE_TIMESTAMP)) - .withMessageContaining(xpath) - .withMessageContaining(SpineInteraction.EHR_EXTRACT_REQUEST.getInteractionId()); + assertMissingValueExceptionForPath(header, body, xpath); } @SneakyThrows - @ParameterizedTest + @ParameterizedTest(name = "[{index}] blank header value at {0}") @MethodSource("pathsToHeaderValues") void When_RequiredValueIsBlankInHeader_Expect_HandlerThrowsException(String xpath) { - Document header = ResourceHelper.loadClasspathResourceAsXml("/ehr/request/RCMR_IN010000UK05_header.xml"); - Document body = ResourceHelper.loadClasspathResourceAsXml("/ehr/request/RCMR_IN010000UK05_body.xml"); + Document header = loadRequestHeader(); + Document body = loadRequestBody(); clearElement(xpath, header); + stubRepositorySaveReturnsInput(); + + assertMissingValueExceptionForPath(header, body, xpath); + } + + private void stubRepositorySaveReturnsInput() { when(ehrExtractStatusRepository.save(any())) .thenAnswer((Answer) invocation -> (EhrExtractStatus) invocation.getArguments()[0]); + } + + private Document loadRequestHeader() { + return ResourceHelper.loadClasspathResourceAsXml(REQUEST_HEADER_XML); + } + + private Document loadRequestBody() { + return ResourceHelper.loadClasspathResourceAsXml(REQUEST_BODY_XML); + } + private void assertMissingValueExceptionForPath(Document header, Document body, String xpath) { assertThatExceptionOfType(MissingValueException.class) .isThrownBy(() -> ehrExtractRequestHandler.handleStart(header, body, MESSAGE_TIMESTAMP)) .withMessageContaining(xpath) diff --git a/service/src/test/java/uk/nhs/adaptors/gp2gp/ehr/request/EhrExtractStatusValidatorTest.java b/service/src/test/java/uk/nhs/adaptors/gp2gp/ehr/request/EhrExtractStatusValidatorTest.java index 53a8ddc8d8..f700104de5 100644 --- a/service/src/test/java/uk/nhs/adaptors/gp2gp/ehr/request/EhrExtractStatusValidatorTest.java +++ b/service/src/test/java/uk/nhs/adaptors/gp2gp/ehr/request/EhrExtractStatusValidatorTest.java @@ -2,9 +2,6 @@ import static org.assertj.core.api.Assertions.assertThat; -import java.util.ArrayList; -import java.util.Arrays; -import java.util.Collections; import java.util.List; import org.junit.jupiter.api.Test; @@ -19,15 +16,12 @@ class EhrExtractStatusValidatorTest { @Test void When_AllPreparingDataStepsAreFinished_Expect_ReturnTrue() { - EhrExtractStatus ehrExtractStatus = new EhrExtractStatus(); - - EhrExtractStatus.GpcAccessDocument gpcAccessDocument = new EhrExtractStatus.GpcAccessDocument( - Arrays.asList(getFinishedGpcDocument(), getFinishedGpcDocument()), PATIENT_ID - ); - ehrExtractStatus.setGpcAccessDocument(gpcAccessDocument); - ehrExtractStatus.setGpcAccessStructured(getFinishedGpcAccessStructured()); + var ehrExtractStatus = buildEhrExtractStatus(List.of(getFinishedGpcDocument(), getFinishedGpcDocument()), + getFinishedGpcAccessStructured()); - assertThat(EhrExtractStatusValidator.isPreparingDataFinished(ehrExtractStatus)).isTrue(); + assertThat(EhrExtractStatusValidator.isPreparingDataFinished(ehrExtractStatus)) + .as("all document and structured access steps are complete") + .isTrue(); } private EhrExtractStatus.GpcDocument getFinishedGpcDocument() { @@ -52,28 +46,21 @@ private EhrExtractStatus.GpcAccessStructured getGpcAccessStructured(String objec @Test void When_AllPreparingDataStepsAreFinishedAndDocumentsListIsEmpty_Expect_ReturnTrue() { - EhrExtractStatus ehrExtractStatus = new EhrExtractStatus(); - - EhrExtractStatus.GpcAccessDocument gpcAccessDocument = new EhrExtractStatus.GpcAccessDocument( - Collections.emptyList(), PATIENT_ID - ); - ehrExtractStatus.setGpcAccessDocument(gpcAccessDocument); - ehrExtractStatus.setGpcAccessStructured(getFinishedGpcAccessStructured()); + var ehrExtractStatus = buildEhrExtractStatus(List.of(), getFinishedGpcAccessStructured()); - assertThat(EhrExtractStatusValidator.isPreparingDataFinished(ehrExtractStatus)).isTrue(); + assertThat(EhrExtractStatusValidator.isPreparingDataFinished(ehrExtractStatus)) + .as("empty documents list is valid when structured access is complete") + .isTrue(); } @Test void When_AllPreparingDataStepsNotFinished_Expect_ReturnFalse() { - EhrExtractStatus ehrExtractStatus = new EhrExtractStatus(); - - EhrExtractStatus.GpcAccessDocument gpcAccessDocument = new EhrExtractStatus.GpcAccessDocument( - Arrays.asList(getUnfinishedGpcDocument(), getUnfinishedGpcDocument()), PATIENT_ID - ); - ehrExtractStatus.setGpcAccessDocument(gpcAccessDocument); - ehrExtractStatus.setGpcAccessStructured(getUnfinishedGpcAccessStructured()); + var ehrExtractStatus = buildEhrExtractStatus(List.of(getUnfinishedGpcDocument(), getUnfinishedGpcDocument()), + getUnfinishedGpcAccessStructured()); - assertThat(EhrExtractStatusValidator.isPreparingDataFinished(ehrExtractStatus)).isFalse(); + assertThat(EhrExtractStatusValidator.isPreparingDataFinished(ehrExtractStatus)) + .as("both document and structured access steps are incomplete") + .isFalse(); } private EhrExtractStatus.GpcDocument getUnfinishedGpcDocument() { @@ -86,77 +73,68 @@ private EhrExtractStatus.GpcAccessStructured getUnfinishedGpcAccessStructured() @Test void When_DocumentAccessStepIsNotFinished_Expect_ReturnFalse() { - EhrExtractStatus ehrExtractStatus = new EhrExtractStatus(); + var ehrExtractStatus = buildEhrExtractStatus(List.of(getUnfinishedGpcDocument(), getUnfinishedGpcDocument()), + getFinishedGpcAccessStructured()); - EhrExtractStatus.GpcAccessDocument gpcAccessDocument = new EhrExtractStatus.GpcAccessDocument( - Arrays.asList(getUnfinishedGpcDocument(), getUnfinishedGpcDocument()), PATIENT_ID - ); - ehrExtractStatus.setGpcAccessDocument(gpcAccessDocument); - ehrExtractStatus.setGpcAccessStructured(getFinishedGpcAccessStructured()); - - assertThat(EhrExtractStatusValidator.isPreparingDataFinished(ehrExtractStatus)).isFalse(); + assertThat(EhrExtractStatusValidator.isPreparingDataFinished(ehrExtractStatus)) + .as("document access is incomplete") + .isFalse(); } @Test void When_OnlyOneDocumentAccessStepIsFinished_Expect_ReturnFalse() { - EhrExtractStatus ehrExtractStatus = new EhrExtractStatus(); + var ehrExtractStatus = buildEhrExtractStatus(List.of(getUnfinishedGpcDocument(), getFinishedGpcDocument()), + getFinishedGpcAccessStructured()); - EhrExtractStatus.GpcAccessDocument gpcAccessDocument = new EhrExtractStatus.GpcAccessDocument( - Arrays.asList(getUnfinishedGpcDocument(), getFinishedGpcDocument()), PATIENT_ID - ); - ehrExtractStatus.setGpcAccessDocument(gpcAccessDocument); - ehrExtractStatus.setGpcAccessStructured(getFinishedGpcAccessStructured()); - - assertThat(EhrExtractStatusValidator.isPreparingDataFinished(ehrExtractStatus)).isFalse(); + assertThat(EhrExtractStatusValidator.isPreparingDataFinished(ehrExtractStatus)) + .as("all documents must be ready") + .isFalse(); } @Test void When_AccessStructuredStepIsNotFinished_Expect_ReturnFalse() { - EhrExtractStatus ehrExtractStatus = new EhrExtractStatus(); + var ehrExtractStatus = buildEhrExtractStatus(List.of(getFinishedGpcDocument(), getFinishedGpcDocument()), + getUnfinishedGpcAccessStructured()); - EhrExtractStatus.GpcAccessDocument gpcAccessDocument = new EhrExtractStatus.GpcAccessDocument( - Arrays.asList(getFinishedGpcDocument(), getFinishedGpcDocument()), PATIENT_ID - ); - ehrExtractStatus.setGpcAccessDocument(gpcAccessDocument); - ehrExtractStatus.setGpcAccessStructured(getUnfinishedGpcAccessStructured()); - - assertThat(EhrExtractStatusValidator.isPreparingDataFinished(ehrExtractStatus)).isFalse(); + assertThat(EhrExtractStatusValidator.isPreparingDataFinished(ehrExtractStatus)) + .as("structured access is incomplete") + .isFalse(); } @Test void When_AllPreparingDataStepsWereNotStarted_Expect_ReturnFalse() { EhrExtractStatus ehrExtractStatus = new EhrExtractStatus(); - assertThat(EhrExtractStatusValidator.isPreparingDataFinished(ehrExtractStatus)).isFalse(); + assertThat(EhrExtractStatusValidator.isPreparingDataFinished(ehrExtractStatus)) + .as("no steps started") + .isFalse(); } @Test void When_AccessStructuredStepIsNotStarted_Expect_ReturnFalse() { - EhrExtractStatus ehrExtractStatus = new EhrExtractStatus(); - - EhrExtractStatus.GpcAccessDocument gpcAccessDocument = new EhrExtractStatus.GpcAccessDocument( - Arrays.asList(getFinishedGpcDocument(), getFinishedGpcDocument()), PATIENT_ID - ); - ehrExtractStatus.setGpcAccessDocument(gpcAccessDocument); + var ehrExtractStatus = buildEhrExtractStatusWithDocumentsOnly(List.of(getFinishedGpcDocument(), getFinishedGpcDocument())); - assertThat(EhrExtractStatusValidator.isPreparingDataFinished(ehrExtractStatus)).isFalse(); + assertThat(EhrExtractStatusValidator.isPreparingDataFinished(ehrExtractStatus)) + .as("structured step must be present") + .isFalse(); } @Test void When_AccessDocumentStepIsNotStarted_Expect_ReturnFalse() { - EhrExtractStatus ehrExtractStatus = new EhrExtractStatus(); - ehrExtractStatus.setGpcAccessStructured(getFinishedGpcAccessStructured()); + var ehrExtractStatus = buildEhrExtractStatusWithStructuredOnly(getFinishedGpcAccessStructured()); - assertThat(EhrExtractStatusValidator.isPreparingDataFinished(ehrExtractStatus)).isFalse(); + assertThat(EhrExtractStatusValidator.isPreparingDataFinished(ehrExtractStatus)) + .as("document access step must be present") + .isFalse(); } @Test void When_AllDocumentsInEhrExtractStatusAreSent_Expect_True() { - EhrExtractStatus ehrExtractStatus = new EhrExtractStatus(); - List documentList = new ArrayList<>(); - documentList.add(getFinishedGpcDocumentSent()); - ehrExtractStatus.setGpcAccessDocument(new EhrExtractStatus.GpcAccessDocument(documentList, "123")); - assertThat(EhrExtractStatusValidator.areAllDocumentsSent(ehrExtractStatus)).isTrue(); + var ehrExtractStatus = buildEhrExtractStatusWithDocumentsOnly(List.of(getFinishedGpcDocumentSent())); + + assertThat(EhrExtractStatusValidator.areAllDocumentsSent(ehrExtractStatus)) + .as("single document was sent to MHS") + .isTrue(); } private EhrExtractStatus.GpcDocument getFinishedGpcDocumentSent() { @@ -167,20 +145,40 @@ private EhrExtractStatus.GpcDocument getFinishedGpcDocumentSent() { @Test void When_OneDocumentIsSentAndOneDocumentNotSent_Expect_False() { - EhrExtractStatus ehrExtractStatus = new EhrExtractStatus(); - List documentList = new ArrayList<>(); - documentList.add(getFinishedGpcDocumentSent()); - documentList.add(getFinishedGpcDocument()); - ehrExtractStatus.setGpcAccessDocument(new EhrExtractStatus.GpcAccessDocument(documentList, "123")); - assertThat(EhrExtractStatusValidator.areAllDocumentsSent(ehrExtractStatus)).isFalse(); + var ehrExtractStatus = buildEhrExtractStatusWithDocumentsOnly( + List.of(getFinishedGpcDocumentSent(), getFinishedGpcDocument())); + + assertThat(EhrExtractStatusValidator.areAllDocumentsSent(ehrExtractStatus)) + .as("all documents must be sent") + .isFalse(); } @Test - void When_NoDocumentsInEhrExtractStatus_Expect_False() { - EhrExtractStatus ehrExtractStatus = new EhrExtractStatus(); - List documentList = new ArrayList<>(); - documentList.add(getFinishedGpcDocument()); - ehrExtractStatus.setGpcAccessDocument(new EhrExtractStatus.GpcAccessDocument(documentList, "123")); - assertThat(EhrExtractStatusValidator.areAllDocumentsSent(ehrExtractStatus)).isFalse(); + void When_DocumentsExistButNoneAreSent_Expect_False() { + var ehrExtractStatus = buildEhrExtractStatusWithDocumentsOnly(List.of(getFinishedGpcDocument())); + + assertThat(EhrExtractStatusValidator.areAllDocumentsSent(ehrExtractStatus)) + .as("documents without sent-to-MHS marker are not considered sent") + .isFalse(); + } + + private EhrExtractStatus buildEhrExtractStatus(List documents, + EhrExtractStatus.GpcAccessStructured gpcAccessStructured) { + var ehrExtractStatus = new EhrExtractStatus(); + ehrExtractStatus.setGpcAccessDocument(new EhrExtractStatus.GpcAccessDocument(documents, PATIENT_ID)); + ehrExtractStatus.setGpcAccessStructured(gpcAccessStructured); + return ehrExtractStatus; + } + + private EhrExtractStatus buildEhrExtractStatusWithDocumentsOnly(List documents) { + var ehrExtractStatus = new EhrExtractStatus(); + ehrExtractStatus.setGpcAccessDocument(new EhrExtractStatus.GpcAccessDocument(documents, PATIENT_ID)); + return ehrExtractStatus; + } + + private EhrExtractStatus buildEhrExtractStatusWithStructuredOnly(EhrExtractStatus.GpcAccessStructured gpcAccessStructured) { + var ehrExtractStatus = new EhrExtractStatus(); + ehrExtractStatus.setGpcAccessStructured(gpcAccessStructured); + return ehrExtractStatus; } } diff --git a/service/src/test/java/uk/nhs/adaptors/gp2gp/mhs/InboundMessageConsumerTest.java b/service/src/test/java/uk/nhs/adaptors/gp2gp/mhs/InboundMessageConsumerTest.java index 1f6fa6e3e2..2dfbca978b 100644 --- a/service/src/test/java/uk/nhs/adaptors/gp2gp/mhs/InboundMessageConsumerTest.java +++ b/service/src/test/java/uk/nhs/adaptors/gp2gp/mhs/InboundMessageConsumerTest.java @@ -3,6 +3,7 @@ import static org.assertj.core.api.AssertionsForClassTypes.assertThatExceptionOfType; import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.doThrow; +import static org.mockito.Mockito.never; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; @@ -23,6 +24,7 @@ @ExtendWith(MockitoExtension.class) class InboundMessageConsumerTest { + @Mock private InboundMessageHandler inboundMessageHandler; @Mock @@ -37,65 +39,97 @@ class InboundMessageConsumerTest { @Test @SneakyThrows void When_MessageHandlerReturnsTrue_Expect_MessageIsAcknowledged() { - when(inboundMessageHandler.handle(any())).thenReturn(true); - inboundMessageConsumer.receive(mockMessage, mockSession); + stubHandleResult(true); - verify(inboundMessageHandler).handle(mockMessage); - verify(mockMessage).acknowledge(); + callReceive(); + + verifyMessageHandled(); + verify(mockMessage, times(1)).acknowledge(); + verifyMdcReset(); } @Test @SneakyThrows void When_MessageHandlerReturnsFalse_Expect_MessageIsNotAcknowledged() { - when(inboundMessageHandler.handle(any())).thenReturn(false); + stubHandleResult(false); - inboundMessageConsumer.receive(mockMessage, mockSession); + callReceive(); - verify(inboundMessageHandler).handle(mockMessage); - verify(mockMessage, times(0)).acknowledge(); + verifyMessageHandled(); + verify(mockMessage, never()).acknowledge(); + verifyMdcReset(); } @Test @SneakyThrows void When_MessageHandlerReturnsFalse_Expect_SessionToBeRolledBack() { - when(inboundMessageHandler.handle(any())).thenReturn(false); + stubHandleResult(false); - inboundMessageConsumer.receive(mockMessage, mockSession); - verify(inboundMessageHandler).handle(mockMessage); + callReceive(); + + verifyMessageHandled(); verify(mockSession, times(1)).rollback(); + verifyMdcReset(); } @Test @SneakyThrows void When_MessageHandlerThrowsException_Expect_MessageIsNotAcknowledged() { - doThrow(new RuntimeException()).when(inboundMessageHandler).handle(mockMessage); + stubHandleThrows(new RuntimeException()); - inboundMessageConsumer.receive(mockMessage, mockSession); + callReceive(); - verify(inboundMessageHandler).handle(mockMessage); - verify(mockMessage, times(0)).acknowledge(); + verifyMessageHandled(); + verify(mockMessage, never()).acknowledge(); + verifyMdcReset(); } @Test @SneakyThrows void When_MessageHandlerThrowsJMSException_Expect_SessionIsRolledBack() { - doThrow(new JMSRuntimeException("Test")).when(inboundMessageHandler).handle(mockMessage); + stubHandleThrows(new JMSRuntimeException("Test")); - inboundMessageConsumer.receive(mockMessage, mockSession); + callReceive(); - verify(inboundMessageHandler).handle(mockMessage); + verifyMessageHandled(); verify(mockSession, times(1)).rollback(); + verifyMdcReset(); } @Test @SneakyThrows void When_MessageHandlerThrowsDataAccessResourceFailure_Expect_ExceptionIsThrown() { - doThrow(new DataAccessResourceFailureException("Test exception")).when(inboundMessageHandler).handle(mockMessage); + stubHandleThrows(new DataAccessResourceFailureException("Test exception")); assertThatExceptionOfType(DataAccessResourceFailureException.class) - .isThrownBy(() -> inboundMessageConsumer.receive(mockMessage, mockSession)); + .isThrownBy(this::callReceive); + + verify(mockSession, never()).rollback(); + verify(mockMessage, never()).acknowledge(); + verifyMdcReset(); + } + + @SneakyThrows + private void callReceive() { + inboundMessageConsumer.receive(mockMessage, mockSession); + } + + @SneakyThrows + private void stubHandleResult(boolean shouldAcknowledge) { + when(inboundMessageHandler.handle(any())).thenReturn(shouldAcknowledge); + } + + @SneakyThrows + private void stubHandleThrows(RuntimeException exception) { + doThrow(exception).when(inboundMessageHandler).handle(mockMessage); + } + + @SneakyThrows + private void verifyMessageHandled() { + verify(inboundMessageHandler).handle(mockMessage); + } - verify(mockSession, times(0)).rollback(); - verify(mockMessage, times(0)).acknowledge(); + private void verifyMdcReset() { + verify(mdcService).resetAllMdcKeys(); } } diff --git a/service/src/test/java/uk/nhs/adaptors/gp2gp/mhs/InboundMessageHandlerTest.java b/service/src/test/java/uk/nhs/adaptors/gp2gp/mhs/InboundMessageHandlerTest.java index 5d3512886a..e342022a14 100644 --- a/service/src/test/java/uk/nhs/adaptors/gp2gp/mhs/InboundMessageHandlerTest.java +++ b/service/src/test/java/uk/nhs/adaptors/gp2gp/mhs/InboundMessageHandlerTest.java @@ -2,7 +2,6 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatThrownBy; -import static org.junit.Assert.assertFalse; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyString; import static org.mockito.ArgumentMatchers.eq; @@ -10,7 +9,9 @@ import static org.mockito.Mockito.doReturn; import static org.mockito.Mockito.doThrow; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyNoMoreInteractions; import static org.mockito.Mockito.verifyNoInteractions; import static org.mockito.Mockito.when; @@ -104,7 +105,7 @@ void When_ExceptionIsThrownWhenParsingTheMessage_Expect_MessageProcessingToBeAbo @SneakyThrows void When_MessageIsEhrExtractRequest_Expect_RequestHandlerCalled() { setUpEhrExtract(EBXML_CONTENT); - Document header = ResourceHelper.loadClasspathResourceAsXml("/ehr/request/RCMR_IN010000UK05_header.xml"); + Document header = loadHeader("/ehr/request/RCMR_IN010000UK05_header.xml"); Document payload = mock(Document.class); doReturn(header).when(xPathService).parseDocumentFromXml(EBXML_CONTENT); doReturn(payload).when(xPathService).parseDocumentFromXml(PAYLOAD_CONTENT); @@ -112,6 +113,8 @@ void When_MessageIsEhrExtractRequest_Expect_RequestHandlerCalled() { var result = inboundMessageHandler.handle(message); assertThat(result).isTrue(); + verify(processFailureHandlingService).hasProcessFailed(CONVERSATION_ID); + verify(mdcService).applyConversationId(CONVERSATION_ID); verify(ehrExtractRequestHandler).handleStart(eq(header), eq(payload), any()); } @@ -119,7 +122,7 @@ void When_MessageIsEhrExtractRequest_Expect_RequestHandlerCalled() { @SneakyThrows void When_MessageIsEhrExtractRequestAck_Expect_RequestAckHandlerCalled() { setUpEhrExtract(EBXML_CONTENT); - Document header = ResourceHelper.loadClasspathResourceAsXml("/ehr/request/MCCI_IN010000UK13_header.xml"); + Document header = loadHeader("/ehr/request/MCCI_IN010000UK13_header.xml"); Document payload = mock(Document.class); doReturn(header).when(xPathService).parseDocumentFromXml(EBXML_CONTENT); doReturn(payload).when(xPathService).parseDocumentFromXml(PAYLOAD_CONTENT); @@ -127,6 +130,8 @@ void When_MessageIsEhrExtractRequestAck_Expect_RequestAckHandlerCalled() { var result = inboundMessageHandler.handle(message); assertThat(result).isTrue(); + verify(processFailureHandlingService).hasProcessFailed("75049C80-5271-11EA-9384-E83935108FD5"); + verify(mdcService).applyConversationId("75049C80-5271-11EA-9384-E83935108FD5"); verify(ehrExtractRequestHandler).handleAcknowledgement("75049C80-5271-11EA-9384-E83935108FD5", payload); } @@ -140,8 +145,10 @@ void When_MessageIsForUnknownInteraction_Expect_ExceptionIsThrown() { doReturn(payload).when(xPathService).parseDocumentFromXml(PAYLOAD_CONTENT); doReturn(UNKNOWN_INTERACTION_ID).when(xPathService).getNodeValue(eq(header), anyString()); - assertFalse(inboundMessageHandler.handle(message)); + assertThat(inboundMessageHandler.handle(message)).isFalse(); + verify(processFailureHandlingService).hasProcessFailed(any()); + verify(mdcService).applyConversationId(any()); verifyNoInteractions(ehrExtractRequestHandler); } @@ -153,6 +160,7 @@ void When_MessageHeaderCannotBeParsed_Expect_FalseToBeReturned() { var result = inboundMessageHandler.handle(message); assertThat(result).isFalse(); + verifyNoInteractions(ehrExtractRequestHandler, processFailureHandlingService, mdcService); } @Test @@ -165,6 +173,7 @@ void When_MessagePayloadCannotBeParsed_Expect_FalseToBeReturned() { var result = inboundMessageHandler.handle(message); assertThat(result).isFalse(); + verifyNoInteractions(ehrExtractRequestHandler, processFailureHandlingService, mdcService); } @Test @@ -197,6 +206,13 @@ void When_MessageProcessingFails_Expect_ResultFromFailureHandlerToBeReturned() { assertThat(inboundMessageHandler.handle(message)).isTrue(); assertThat(inboundMessageHandler.handle(message)).isFalse(); + + verify(processFailureHandlingService, times(2)).failProcess( + CONVERSATION_ID, + NACK_ERROR_CODE, + "There has been an error when processing the message", + "InboundMessageHandler" + ); } @Test @@ -222,6 +238,7 @@ void When_DatabaseAccessFailure_Expect_ExceptionThrown() { doThrow(testException).when(processFailureHandlingService).hasProcessFailed(any()); assertThatThrownBy(() -> inboundMessageHandler.handle(message)).isSameAs(testException); + verifyNoInteractions(ehrExtractRequestHandler, mdcService); } @@ -236,16 +253,21 @@ void When_ProcessHasAlreadyFailed_Expect_TaskNotExecuted() { assertThat(result).isTrue(); verify(processFailureHandlingService).hasProcessFailed(CONVERSATION_ID); - verifyNoInteractions(ehrExtractRequestHandler); + verifyNoInteractions(ehrExtractRequestHandler, mdcService); + verifyNoMoreInteractions(processFailureHandlingService); } private void setupValidMessage() throws JMSException, JsonProcessingException, SAXException { setUpEhrExtract(EBXML_CONTENT); - Document header = ResourceHelper.loadClasspathResourceAsXml("/ehr/request/RCMR_IN010000UK05_header.xml"); + Document header = loadHeader("/ehr/request/RCMR_IN010000UK05_header.xml"); doReturn(header).when(xPathService).parseDocumentFromXml(EBXML_CONTENT); doReturn(mock(Document.class)).when(xPathService).parseDocumentFromXml(PAYLOAD_CONTENT); } + private Document loadHeader(String resourcePath) { + return ResourceHelper.loadClasspathResourceAsXml(resourcePath); + } + private void setUpEhrExtract(String ebxmlContent) throws JMSException, JsonProcessingException { when(message.getBody(String.class)).thenReturn(INBOUND_MESSAGE_CONTENT); var inboundMessage = new InboundMessage();