Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor: Clarify purpose of AttachmentReferenceUpdaterService #660

Merged
merged 1 commit into from
Jun 28, 2024
Merged

Refactor: Clarify purpose of AttachmentReferenceUpdaterService #660

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 12 additions & 10 deletions ...c/main/java/uk/nhs/adaptors/pss/translator/service/AttachmentReferenceUpdaterService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,18 +32,21 @@ public class AttachmentReferenceUpdaterService {

private final StorageManagerService storageManagerService;

public String updateReferenceToAttachment(List<InboundMessage.Attachment> attachments, String conversationId, String payloadStr)
public String replaceOriginalFilenameWithStorageFilenameInEhrExtract(
List<InboundMessage.Attachment> attachments,
String conversationId,
String ehrExtract)
throws ValidationException, AttachmentNotFoundException, InlineAttachmentProcessingException {

if (conversationId == null || conversationId.isEmpty()) {
throw new ValidationException("ConversationId cannot be null or empty");
}

if (attachments == null) {
return payloadStr;
return ehrExtract;
}

String resultPayload = payloadStr;
String resultPayload = ehrExtract;

Set<String> expectedFilenames = attachments.stream()
.filter(attachment -> !XmlParseUtilService.parseIsSkeleton(attachment.getDescription()))
Expand All @@ -52,28 +55,27 @@ public String updateReferenceToAttachment(List<InboundMessage.Attachment> attach
.collect(Collectors.toCollection(HashSet::new));

Pattern pattern = Pattern.compile(wrapWithReferenceElement("file://localhost/([^\"]+)"));
Matcher matcher = pattern.matcher(payloadStr);
Matcher originalFilenameMatch = pattern.matcher(ehrExtract);

while (matcher.find()) {
String decodedFilename = UriUtils.decode(matcher.group(1), StandardCharsets.UTF_8);
while (originalFilenameMatch.find()) {
String decodedFilename = UriUtils.decode(originalFilenameMatch.group(1), StandardCharsets.UTF_8);

if (expectedFilenames.contains(decodedFilename)) {

String fileLocation = storageManagerService.getFileLocation(decodedFilename, conversationId);
String referenceElement = wrapWithReferenceElement(xmlEscape(fileLocation));
String updatedReferenceElement = wrapWithReferenceElement(xmlEscape(fileLocation));

resultPayload = resultPayload.replace(matcher.group(0), referenceElement);
resultPayload = resultPayload.replace(originalFilenameMatch.group(0), updatedReferenceElement);

expectedFilenames.remove(decodedFilename);
}
}

if (!expectedFilenames.isEmpty()) {
throw new AttachmentNotFoundException("Unable to find attachment(s): " + expectedFilenames);
throw new AttachmentNotFoundException("Unable to find attachment(s) in EhrExtract: " + expectedFilenames);
}

return resultPayload;

}

private String xmlEscape(String str) {
Expand Down
2 changes: 1 addition & 1 deletion ...or/src/main/java/uk/nhs/adaptors/pss/translator/service/InboundMessageMergingService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ public void mergeAndBundleMessage(String conversationId) throws JAXBException, J
Arrays.asList(bypassPayloadLoadingArray),
conversationId
);
var newPayloadStr = attachmentReferenceUpdaterService.updateReferenceToAttachment(
var newPayloadStr = attachmentReferenceUpdaterService.replaceOriginalFilenameWithStorageFilenameInEhrExtract(
messageAttachments,
conversationId,
inboundMessage.getPayload()
Expand Down
2 changes: 1 addition & 1 deletion ...ranslator/src/main/java/uk/nhs/adaptors/pss/translator/task/EhrExtractMessageHandler.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -181,7 +181,7 @@ private void processAndCompleteEHRMessage(InboundMessage inboundMessage,
}

// upload all references to files in the inbound message
var fileUpdatedPayload = attachmentReferenceUpdaterService.updateReferenceToAttachment(
var fileUpdatedPayload = attachmentReferenceUpdaterService.replaceOriginalFilenameWithStorageFilenameInEhrExtract(
inboundMessage.getAttachments(),
conversationId,
inboundMessage.getPayload()
Expand Down
34 changes: 25 additions & 9 deletions ...t/java/uk/nhs/adaptors/pss/translator/service/AttachmentReferenceUpdaterServiceTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,9 @@ public void When_NoAttachmentsGiven_Expect_NoChanges()

var content = getFileContent(PAYLOAD_XML);

var result = attachmentReferenceUpdaterService.updateReferenceToAttachment(null, CONVERSATION_ID, content);
var result = attachmentReferenceUpdaterService.replaceOriginalFilenameWithStorageFilenameInEhrExtract(
null, CONVERSATION_ID, content
);

assertEquals(content, result);
}
Expand All @@ -153,7 +155,9 @@ public void When_AttachmentGiven_Expect_GetFileLocationHitOnce()

when(storageManagerService.getFileLocation(any(), any())).thenReturn("https://location.com");

attachmentReferenceUpdaterService.updateReferenceToAttachment(mockAttachment, CONVERSATION_ID, content);
attachmentReferenceUpdaterService.replaceOriginalFilenameWithStorageFilenameInEhrExtract(
mockAttachment, CONVERSATION_ID, content
);

verify(storageManagerService, times(1)).getFileLocation(any(), any());
}
Expand All @@ -165,7 +169,9 @@ public void When_AttachmentGiven_Expect_PayloadXmlChanged()
var content = getFileContent(PAYLOAD_XML);
when(storageManagerService.getFileLocation(any(), any())).thenReturn("https://location.com");

var result = attachmentReferenceUpdaterService.updateReferenceToAttachment(mockAttachment, CONVERSATION_ID, content);
var result = attachmentReferenceUpdaterService.replaceOriginalFilenameWithStorageFilenameInEhrExtract(
mockAttachment, CONVERSATION_ID, content
);

assertNotEquals(content, result);
assertTrue(result.contains("https://location.com"));
Expand All @@ -180,7 +186,9 @@ public void When_AttachmentGiven_Expect_ValidXml()
var content = getFileContent(FLAT_PAYLOAD_XML);
when(storageManagerService.getFileLocation(any(), any())).thenReturn("https://location.com");

var result = attachmentReferenceUpdaterService.updateReferenceToAttachment(mockAttachment, CONVERSATION_ID, content);
var result = attachmentReferenceUpdaterService.replaceOriginalFilenameWithStorageFilenameInEhrExtract(
mockAttachment, CONVERSATION_ID, content
);

assertNotEquals(content, result);
assertTrue(result.contains("https://location.com"));
Expand All @@ -199,7 +207,9 @@ public void When_MultipleAttachmentsGiven_Expect_GetFileLocationHitMultipleTimes
var content = getFileContent(PAYLOAD_XML);
when(storageManagerService.getFileLocation(any(), any())).thenReturn("https://location.com");

var result = attachmentReferenceUpdaterService.updateReferenceToAttachment(mockThreeAttachments, CONVERSATION_ID, content);
var result = attachmentReferenceUpdaterService.replaceOriginalFilenameWithStorageFilenameInEhrExtract(
mockThreeAttachments, CONVERSATION_ID, content
);

verify(storageManagerService, times(mockThreeAttachments.size())).getFileLocation(any(), any());
assertTrue(result.contains("https://location.com"));
Expand All @@ -215,7 +225,9 @@ public void When_TppStyleAttachmentGiven_Expect_PayloadXmlChanged()
var content = getFileContent(FLAT_PAYLOAD_XML);
when(storageManagerService.getFileLocation(any(), any())).thenReturn("https://location.com");

var result = attachmentReferenceUpdaterService.updateReferenceToAttachment(mockTppStyleAttachment, CONVERSATION_ID, content);
var result = attachmentReferenceUpdaterService.replaceOriginalFilenameWithStorageFilenameInEhrExtract(
mockTppStyleAttachment, CONVERSATION_ID, content
);

verify(storageManagerService, times(mockTppStyleAttachment.size())).getFileLocation(any(), any());
assertTrue(result.contains("https://location.com"));
Expand All @@ -236,7 +248,9 @@ public void When_EncodedUrlsPresent_Expect_PayloadXmlChanged()
when(storageManagerService.getFileLocation(eq("8681AF4F-E577-4C8D-A2CE-43CABE3D5FB4_sample_mpeg4.mp4"), any()))
.thenReturn("https://location.com/sampleMpeg4.mp4");

var result = attachmentReferenceUpdaterService.updateReferenceToAttachment(mockEncodedAttachments, CONVERSATION_ID, content);
var result = attachmentReferenceUpdaterService.replaceOriginalFilenameWithStorageFilenameInEhrExtract(
mockEncodedAttachments, CONVERSATION_ID, content
);

verify(storageManagerService, times(mockEncodedAttachments.size())).getFileLocation(any(), any());

Expand All @@ -248,10 +262,12 @@ public void When_MissingAttachmentGiven_Expect_AttachmentNotFoundException() {
var content = getFileContent(PAYLOAD_XML);

assertThatThrownBy(
() -> attachmentReferenceUpdaterService.updateReferenceToAttachment(mockMissingAttachment, CONVERSATION_ID, content)
() -> attachmentReferenceUpdaterService.replaceOriginalFilenameWithStorageFilenameInEhrExtract(
mockMissingAttachment, CONVERSATION_ID, content
)
)
.isInstanceOf(AttachmentNotFoundException.class)
.hasMessageContaining("Unable to find attachment(s): [missing_attachment.txt]");
.hasMessageContaining("Unable to find attachment(s) in EhrExtract: [missing_attachment.txt]");
}

private String getFileContent(String filename) {
Expand Down
21 changes: 13 additions & 8 deletions ...c/test/java/uk/nhs/adaptors/pss/translator/service/InboundMessageMergingServiceTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,8 @@ private void prepareMocks(InboundMessage inboundMessage, ArrayList<PatientAttach
when(patientAttachmentLogService.findAttachmentLogs(CONVERSATION_ID)).thenReturn(attachments);
when(migrationRequestDao.getMigrationRequest(any())).thenReturn(patientMigrationRequest);
when(objectMapper.readValue(inboundMessageAsString, InboundMessage.class)).thenReturn(inboundMessage);
when(attachmentReferenceUpdaterService.updateReferenceToAttachment(any(), any(), any())).thenReturn(inboundMessage.getPayload());
when(attachmentReferenceUpdaterService.replaceOriginalFilenameWithStorageFilenameInEhrExtract(any(), any(), any()))
.thenReturn(inboundMessage.getPayload());

}

Expand Down Expand Up @@ -231,7 +232,8 @@ public void When_MergeAndBundleMessageThrowsValidationException_Expect_SendNack(
when(objectMapper.readValue(inboundMessageAsString, InboundMessage.class)).thenReturn(inboundMessage);
when(skeletonProcessingService.updateInboundMessageWithSkeleton(any(), any(), any())).thenReturn(inboundMessage);

doThrow(ValidationException.class).when(attachmentReferenceUpdaterService).updateReferenceToAttachment(any(), any(), any());
doThrow(ValidationException.class).when(attachmentReferenceUpdaterService)
.replaceOriginalFilenameWithStorageFilenameInEhrExtract(any(), any(), any());

inboundMessageMergingService.mergeAndBundleMessage(CONVERSATION_ID);
verify(nackAckPreparationService, times(1)).sendNackMessage(any(NACKReason.class), any(RCMRIN030000UK06Message.class), any());
Expand Down Expand Up @@ -260,7 +262,7 @@ public void When_MergeAndBundleMessageThrowsInlineAttachmentProcessingException_

doThrow(InlineAttachmentProcessingException.class)
.when(attachmentReferenceUpdaterService)
.updateReferenceToAttachment(any(), any(), any());
.replaceOriginalFilenameWithStorageFilenameInEhrExtract(any(), any(), any());

inboundMessageMergingService.mergeAndBundleMessage(CONVERSATION_ID);
verify(nackAckPreparationService, times(1))
Expand Down Expand Up @@ -288,7 +290,8 @@ public void When_MergeAndBundleMessageThrowsAttachmentNotFoundException_Expect_S
when(objectMapper.readValue(inboundMessageAsString, InboundMessage.class)).thenReturn(inboundMessage);
when(skeletonProcessingService.updateInboundMessageWithSkeleton(any(), any(), any())).thenReturn(inboundMessage);

doThrow(AttachmentNotFoundException.class).when(attachmentReferenceUpdaterService).updateReferenceToAttachment(any(), any(), any());
doThrow(AttachmentNotFoundException.class).when(attachmentReferenceUpdaterService)
.replaceOriginalFilenameWithStorageFilenameInEhrExtract(any(), any(), any());

inboundMessageMergingService.mergeAndBundleMessage(CONVERSATION_ID);
verify(nackAckPreparationService, times(1))
Expand Down Expand Up @@ -343,7 +346,7 @@ public void When_MergeAndBundleMessageThrowsJAXBException_Expect_SendNack() thro
when(patientAttachmentLogService.findAttachmentLogs(CONVERSATION_ID)).thenReturn(attachments);
when(migrationRequestDao.getMigrationRequest(any())).thenReturn(patientMigrationRequest);
when(objectMapper.readValue(inboundMessageAsString, InboundMessage.class)).thenReturn(inboundMessage);
when(attachmentReferenceUpdaterService.updateReferenceToAttachment(any(), any(), any())).thenReturn("");
when(attachmentReferenceUpdaterService.replaceOriginalFilenameWithStorageFilenameInEhrExtract(any(), any(), any())).thenReturn("");
when(skeletonProcessingService.updateInboundMessageWithSkeleton(any(), any(), any())).thenReturn(inboundMessage);

inboundMessageMergingService.mergeAndBundleMessage(CONVERSATION_ID);
Expand All @@ -370,7 +373,8 @@ public void When_MergeAndBundleMessageThrowsBundleMappingException_Expect_SendNa
when(patientAttachmentLogService.findAttachmentLogs(CONVERSATION_ID)).thenReturn(attachments);
when(migrationRequestDao.getMigrationRequest(any())).thenReturn(patientMigrationRequest);
when(objectMapper.readValue(inboundMessageAsString, InboundMessage.class)).thenReturn(inboundMessage);
when(attachmentReferenceUpdaterService.updateReferenceToAttachment(any(), any(), any())).thenReturn(inboundMessage.getPayload());
when(attachmentReferenceUpdaterService.replaceOriginalFilenameWithStorageFilenameInEhrExtract(any(), any(), any()))
.thenReturn(inboundMessage.getPayload());
when(skeletonProcessingService.updateInboundMessageWithSkeleton(any(), any(), any())).thenReturn(inboundMessage);

doThrow(BundleMappingException.class).when(bundleMapperService).mapToBundle(any(RCMRIN030000UK06Message.class), any(), anyList());
Expand Down Expand Up @@ -399,7 +403,8 @@ public void When_MergeAndBundleMessageThrowsJsonProcessingException_Expect_SendN
when(patientAttachmentLogService.findAttachmentLogs(CONVERSATION_ID)).thenReturn(attachments);
when(migrationRequestDao.getMigrationRequest(any())).thenReturn(patientMigrationRequest);
when(objectMapper.readValue(inboundMessageAsString, InboundMessage.class)).thenReturn(inboundMessage);
when(attachmentReferenceUpdaterService.updateReferenceToAttachment(any(), any(), any())).thenReturn(inboundMessage.getPayload());
when(attachmentReferenceUpdaterService.replaceOriginalFilenameWithStorageFilenameInEhrExtract(any(), any(), any()))
.thenReturn(inboundMessage.getPayload());
when(skeletonProcessingService.updateInboundMessageWithSkeleton(any(), any(), any())).thenReturn(inboundMessage);

doThrow(JsonProcessingException.class).when(objectMapper).writeValueAsString(any(InboundMessage.class));
Expand Down Expand Up @@ -459,7 +464,7 @@ public void When_AttachmentsPresent_Expect_AttachmentReferenceUpdated()

inboundMessageMergingService.mergeAndBundleMessage(CONVERSATION_ID);

verify(attachmentReferenceUpdaterService).updateReferenceToAttachment(any(), any(), any());
verify(attachmentReferenceUpdaterService).replaceOriginalFilenameWithStorageFilenameInEhrExtract(any(), any(), any());
verify(nackAckPreparationService, never()).sendNackMessage(any(NACKReason.class), any(RCMRIN030000UK06Message.class), any());
}

Expand Down
22 changes: 11 additions & 11 deletions ...lator/src/test/java/uk/nhs/adaptors/pss/translator/task/EhrExtractMessageHandlerTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -235,7 +235,7 @@ public void When_HandleMessageWithValidDataIsCalled_Expect_CallsAttachmentRefere

ehrExtractMessageHandler.handleMessage(inboundMessage, CONVERSATION_ID, RCMRIN030000UK06Message.class);

verify(attachmentReferenceUpdaterService).updateReferenceToAttachment(
verify(attachmentReferenceUpdaterService).replaceOriginalFilenameWithStorageFilenameInEhrExtract(
inboundMessage.getAttachments(), CONVERSATION_ID, inboundMessage.getPayload());
}

Expand Down Expand Up @@ -378,9 +378,9 @@ public void When_HandleMessage_WithMapToBundleThrows_Expect_BundleMappingExcepti
.build();

when(migrationRequestDao.getMigrationRequest(CONVERSATION_ID)).thenReturn(migrationRequest);
when(attachmentReferenceUpdaterService
.updateReferenceToAttachment(inboundMessage.getAttachments(), CONVERSATION_ID, inboundMessage.getPayload()))
.thenReturn(inboundMessage.getPayload());
when(attachmentReferenceUpdaterService.replaceOriginalFilenameWithStorageFilenameInEhrExtract(
inboundMessage.getAttachments(), CONVERSATION_ID, inboundMessage.getPayload()
)).thenReturn(inboundMessage.getPayload());

doThrow(new BundleMappingException("Test Exception"))
.when(bundleMapperService).mapToBundle(any(RCMRIN030000UK06Message.class), any(), any());
Expand All @@ -407,9 +407,9 @@ public void When_HandleMessage_WithEncodeToJsonThrows_Expect_DataFormatException

when(bundleMapperService.mapToBundle(any(RCMRIN030000UK06Message.class), eq(LOSING_ODE_CODE), any())).thenReturn(bundle);
when(migrationRequestDao.getMigrationRequest(CONVERSATION_ID)).thenReturn(migrationRequest);
when(attachmentReferenceUpdaterService
.updateReferenceToAttachment(inboundMessage.getAttachments(), CONVERSATION_ID, inboundMessage.getPayload()))
.thenReturn(inboundMessage.getPayload());
when(attachmentReferenceUpdaterService.replaceOriginalFilenameWithStorageFilenameInEhrExtract(
inboundMessage.getAttachments(), CONVERSATION_ID, inboundMessage.getPayload()
)).thenReturn(inboundMessage.getPayload());

doThrow(new DataFormatException()).when(fhirParser).encodeToJson(bundle);

Expand Down Expand Up @@ -440,7 +440,7 @@ public void When_HandleSingleMessageWithValidDataIsCalled_Expect_NotToCallSendCo
prepareMigrationRequestAndMigrationStatusMocks();

when(attachmentReferenceUpdaterService
.updateReferenceToAttachment(
.replaceOriginalFilenameWithStorageFilenameInEhrExtract(
inboundMessage.getAttachments(), CONVERSATION_ID, inboundMessage.getPayload()
)).thenReturn(inboundMessage.getPayload());

Expand Down Expand Up @@ -489,7 +489,7 @@ public void When_HandleSingleMessageWithSkeletonAttachmentAndValidDataIsCalled_E
when(xPathService.getNodeValue(any(), any())).thenReturn("MESSAGE-ID");
when(skeletonProcessingService.updateInboundMessageWithSkeleton(any(), any(), any())).thenReturn(inboundMessage);
when(attachmentReferenceUpdaterService
.updateReferenceToAttachment(
.replaceOriginalFilenameWithStorageFilenameInEhrExtract(
inboundMessage.getAttachments(), CONVERSATION_ID, inboundMessage.getPayload()
)).thenReturn(inboundMessage.getPayload());

Expand Down Expand Up @@ -521,7 +521,7 @@ public void When_HandleSingleMessageWithAFilenameDescription_Expect_PatientAttac

when(xPathService.getNodeValue(any(), any())).thenReturn("MESSAGE-ID");
when(attachmentReferenceUpdaterService
.updateReferenceToAttachment(
.replaceOriginalFilenameWithStorageFilenameInEhrExtract(
inboundMessage.getAttachments(), CONVERSATION_ID, inboundMessage.getPayload()
)).thenReturn(inboundMessage.getPayload());

Expand Down Expand Up @@ -803,7 +803,7 @@ private void prepareMocks(InboundMessage inboundMessage) throws BundleMappingExc
when(objectMapper.writeValueAsString(inboundMessage)).thenReturn(INBOUND_MESSAGE_STRING);
when(bundleMapperService.mapToBundle(any(RCMRIN030000UK06Message.class), eq(LOSING_ODE_CODE), any())).thenReturn(bundle);
when(attachmentReferenceUpdaterService
.updateReferenceToAttachment(
.replaceOriginalFilenameWithStorageFilenameInEhrExtract(
inboundMessage.getAttachments(), CONVERSATION_ID, inboundMessage.getPayload()
)).thenReturn(inboundMessage.getPayload());
}
Expand Down