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 readMetadataForMessageLinking so it doesn't return itself #1043

Merged
merged 1 commit into from
Apr 25, 2024
Merged

Refactor readMetadataForMessageLinking so it doesn't return itself #1043

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
4 changes: 0 additions & 4 deletions etor/src/main/java/gov/hhs/cdc/trustedintermediary/etor/EtorDomainRegistration.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -231,10 +231,6 @@ DomainResponse handleMetadata(DomainRequest request) {
Set<String> messageIdsToLink =
partnerMetadataOrchestrator.findMessagesIdsToLink(metadataId);

// Remove the metadataId from the set of messageIdsToLink to avoid showing it in the
// partner metadata's linked ids
messageIdsToLink.remove(metadataId);

FhirMetadata<?> responseObject =
partnerMetadataConverter.extractPublicMetadataToOperationOutcome(
metadata.get(), metadataId, messageIdsToLink);
Expand Down
4 changes: 4 additions & 0 deletions etor/src/main/java/gov/hhs/cdc/trustedintermediary/etor/messages/SendMessageHelper.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,10 @@ public void linkMessage(String receivedSubmissionId) {
if (messageIdsToLink == null || messageIdsToLink.isEmpty()) {
return;
}

// Add receivedSubmissionId to complete the list of messageIds to link
messageIdsToLink.add(receivedSubmissionId);

Comment on lines 70 to +76
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this work correctly now?

Did partnerMetadataOrchestrator.findMessagesIdsToLink(receivedSubmissionId) return receivedSubmissionId in the returned set, but now it doesn't? Is it possible that the if statement now always returns an empty set if this is the first one?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correct, findMessagesIdsToLink was returning receivedSubmissionId in the set. The problem with the previous logic and query is that it will return a record even if there's no pair to match, as it will always return itself. In this case, if there's a pair to match, it will return that so if won't be empty. If there's no pair to match, we want to return an empty set

logger.logInfo(
"Found messages to link for receivedSubmissionId {}: {}",
receivedSubmissionId,
Expand Down
1 change: 1 addition & 0 deletions ...ava/gov/hhs/cdc/trustedintermediary/external/database/DatabasePartnerMetadataStorage.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,7 @@ public Set<PartnerMetadata> readMetadataForMessageLinking(String submissionId)
ON m1.placer_order_number = m2.placer_order_number
AND (m1.sending_facility_details = m2.sending_facility_details
OR m1.sending_facility_details = m2.receiving_facility_details)
AND m1.received_message_id <> m2.received_message_id
WHERE m1.received_message_id = ?;
""");
statement.setString(1, submissionId);
Expand Down
4 changes: 3 additions & 1 deletion ...n/java/gov/hhs/cdc/trustedintermediary/external/localfile/FilePartnerMetadataStorage.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,9 @@ public Set<PartnerMetadata> readMetadataForMessageLinking(String receivedSubmiss
|| metadata.sendingFacilityDetails()
.equals(
match
.receivingFacilityDetails())))
.receivingFacilityDetails()))
&& !metadata.receivedSubmissionId()
.equals(receivedSubmissionId))
.collect(Collectors.toSet());
} catch (Exception e) {
throw new PartnerMetadataException(
Expand Down
5 changes: 2 additions & 3 deletions etor/src/test/groovy/gov/hhs/cdc/trustedintermediary/etor/EtorDomainRegistrationTest.groovy
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -279,8 +279,7 @@ class EtorDomainRegistrationTest extends Specification {
def metadata = new PartnerMetadata("receivedSubmissionId", "sender", Instant.now(), null,
"hash", PartnerMetadataStatus.DELIVERED, PartnerMetadataMessageType.ORDER,
sendingApp, sendingFacility, receivingApp, receivingFacility, "placer_order_number")
def linkedMessageIds = new HashSet<>(Set.of(receivedSubmissionId, "Test1", "Test2"))
def relevantMessageIds = linkedMessageIds.findAll { it != receivedSubmissionId }
def linkedMessageIds = Set.of(receivedSubmissionId, "Test1", "Test2")

def connector = new EtorDomainRegistration()
TestApplicationContext.register(EtorDomainRegistration, connector)
Expand Down Expand Up @@ -311,7 +310,7 @@ class EtorDomainRegistrationTest extends Specification {
actualStatusCode == expectedStatusCode
1 * mockPartnerMetadataOrchestrator.getMetadata(receivedSubmissionId) >> Optional.ofNullable(metadata)
1 * mockPartnerMetadataOrchestrator.findMessagesIdsToLink(receivedSubmissionId) >> linkedMessageIds
1 * mockPartnerMetadataConverter.extractPublicMetadataToOperationOutcome(_ as PartnerMetadata, _ as String, relevantMessageIds) >> Mock(FhirMetadata)
1 * mockPartnerMetadataConverter.extractPublicMetadataToOperationOutcome(_ as PartnerMetadata, _ as String, linkedMessageIds) >> Mock(FhirMetadata)
1 * mockResponseHelper.constructOkResponseFromString(_ as String) >> new DomainResponse(expectedStatusCode)
}

Expand Down
14 changes: 2 additions & 12 deletions etor/src/test/groovy/gov/hhs/cdc/trustedintermediary/etor/orders/SendOrderUseCaseTest.groovy
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,22 +36,12 @@ class SendOrderUseCaseTest extends Specification {
given:
def receivedSubmissionId = "receivedId"
def sentSubmissionId = "sentId"
def messagesIdsToLink = Set.of("messageId1", "messageId2")
def messagesIdsToLink = new HashSet<>(Set.of("messageId1", "messageId2"))

def sendOrder = SendOrderUseCase.getInstance()
def mockOrder = new OrderMock(null, null, null, null, null, null, null, null)
def mockOmlOrder = Mock(Order)

def partnerMetadata = new PartnerMetadata(receivedSubmissionId,
_ as String,
PartnerMetadataMessageType.ORDER,
mockOrder.getSendingApplicationDetails(),
mockOrder.getSendingFacilityDetails(),
mockOrder.getReceivingApplicationDetails(),
mockOrder.getReceivingFacilityDetails(),
mockOrder.getPlacerOrderNumber()
)

TestApplicationContext.injectRegisteredImplementations()

when:
Expand All @@ -68,7 +58,7 @@ class SendOrderUseCaseTest extends Specification {
1 * mockOrchestrator.updateMetadataForReceivedMessage(_ as PartnerMetadata)
1 * mockOrchestrator.updateMetadataForSentMessage(receivedSubmissionId, sentSubmissionId)
1 * mockOrchestrator.findMessagesIdsToLink(receivedSubmissionId) >> messagesIdsToLink
1 * mockOrchestrator.linkMessages(messagesIdsToLink)
1 * mockOrchestrator.linkMessages(messagesIdsToLink + receivedSubmissionId)
}

def "send fails to send"() {
Expand Down
6 changes: 4 additions & 2 deletions .../gov/hhs/cdc/trustedintermediary/external/localfile/FilePartnerMetadataStorageTest.groovy
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,8 @@ class FilePartnerMetadataStorageTest extends Specification {
def metadataSetWithMatchingSendingFacilityDetails = FilePartnerMetadataStorage.getInstance().readMetadataForMessageLinking(receivedSubmissionId1)

then:
metadataSetWithMatchingSendingFacilityDetails.containsAll(Set.of(matchingSendingFacilityDetailsMetadata1, otherMatchingSendingFacilityDetailsMetadata1))
metadataSetWithMatchingSendingFacilityDetails.contains(otherMatchingSendingFacilityDetailsMetadata1)
!metadataSetWithMatchingSendingFacilityDetails.contains(matchingSendingFacilityDetailsMetadata1)

when:
def receivedSubmissionId2 = "receivedSubmissionId2"
Expand All @@ -158,7 +159,8 @@ class FilePartnerMetadataStorageTest extends Specification {
def metadataSetWithMatchingSendingAndReceivingFacilityDetails = FilePartnerMetadataStorage.getInstance().readMetadataForMessageLinking(receivedSubmissionId2)

then:
metadataSetWithMatchingSendingAndReceivingFacilityDetails.containsAll(Set.of(matchingSendingFacilityDetailsMetadata2, matchingReceivingFacilityDetailsMetadata2))
metadataSetWithMatchingSendingAndReceivingFacilityDetails.contains(matchingReceivingFacilityDetailsMetadata2)
!metadataSetWithMatchingSendingAndReceivingFacilityDetails.contains(matchingSendingFacilityDetailsMetadata2)
}

def "readMetadataForMessageLinking returns an empty set when no metadata is found"() {
Expand Down
Loading