Skip to content

Commit

Permalink
Skip MDM candidate search if parameters are missing (#5658)
Browse files Browse the repository at this point in the history
* Issue #5657

Skip an MDM candidate search if any search parameter is missing (as per documentation,
and to avoid non-performant searches), rather than only when all of them are missing.

* Address the unit test failure.

* Add credit for #5658

---------

Co-authored-by: James Agnew <[email protected]>
  • Loading branch information
adriennesox and jamesagnew committed Jun 12, 2024
1 parent a8d21aa commit 024d848
Show file tree
Hide file tree
Showing 5 changed files with 51 additions and 7 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
type: perf
issue: 5658
title: "The MDM candidate search will now be skipped if data for required
search parameters is not present. Thanks to Adrienne Sox for the
pull request!"
Original file line number Diff line number Diff line change
Expand Up @@ -55,18 +55,17 @@ public Optional<String> buildResourceQueryString(

// If there are candidate search params, then make use of them, otherwise, search with only the filters.
if (resourceSearchParam != null) {
resourceSearchParam.iterator().forEachRemaining(searchParam -> {
for (String searchParam : resourceSearchParam) {
// to compare it to all known GOLDEN_RESOURCE objects, using the overlapping search parameters that they
// have.
List<String> valuesFromResourceForSearchParam =
myMdmSearchParamSvc.getValueFromResourceForSearchParam(theResource, searchParam);
if (!valuesFromResourceForSearchParam.isEmpty()) {
criteria.add(buildResourceMatchQuery(searchParam, valuesFromResourceForSearchParam));
if (valuesFromResourceForSearchParam.isEmpty()) {
// Don't search if some of the search parameters aren't present in the resource
return Optional.empty();
}
});
if (criteria.isEmpty()) {
// TODO GGG/KHS, re-evaluate whether we should early drop here.
return Optional.empty();

criteria.add(buildResourceMatchQuery(searchParam, valuesFromResourceForSearchParam));
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,27 @@ public void after() throws IOException {
super.after();
}

protected void clearMdmLinks() {
// Override of super class.
//
// Each resource that needs to be cleared produces a separate work chunk in batch processing, owing to a loop
// in MdmGenerateRangeChunksStep. There are three resource types used in this test, so that means there are
// three work chunks created for finding GoldenResourceIds and deleting them.
//
// TestR4Config proscribes (via JpaBatch2Config/BaseBatch2Config) that there are 4 threads processing chunks.
// However, TestR4Config also proscribes that there is somewhere between 3 and 8 database connections available.
//
// Consequently, if we clear all resource types at once, we can end up with 3 threads processing a chunk each.
// Which would be fine, except LoadGoldenIdsStep requires 2 database connections - one to read data,
// and the second to create the clear step chunks based on that data. If TestR4Config rolls low and there
// are only three connections available, we risk a deadlock due to database connection exhaustion if we
// process all 3 resource types at once. (With 4 connections or more, one of the three chunks is guaranteed to
// be able to finish, thereby freeing resources for the other chunks to finish.)
clearMdmLinks("Medication");
clearMdmLinks("Practitioner");
clearMdmLinks("Patient");
}

@ParameterizedTest
@MethodSource("requestTypes")
public void testBatchRunOnAllMedications(ServletRequestDetails theSyncOrAsyncRequest) throws InterruptedException {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -97,4 +97,17 @@ public void testEmptyCandidateSearchParamsWorksInConjunctionWithFilterParams() {
assertEquals(true, result.isPresent());
assertThat(result).contains("Patient?active=true");
}

@Test
public void testNoSearchIfASearchParamIsMissing() {
Patient patient = new Patient();
HumanName humanName = patient.addName();
humanName.addGiven("Jose");
humanName.addGiven("Martin");
MdmResourceSearchParamJson searchParamJson = new MdmResourceSearchParamJson();
searchParamJson.addSearchParam("given");
searchParamJson.addSearchParam("family");
Optional<String> result = myMdmCandidateSearchCriteriaBuilderSvc.buildResourceQueryString("Patient", patient, Collections.emptyList(), searchParamJson);
assertFalse(result.isPresent());
}
}
5 changes: 5 additions & 0 deletions pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -916,6 +916,11 @@
<name>Stefan Lindström</name>
<organization>Softhouse AB</organization>
</developer>
<developer>
<id>adriennesox</id>
<name>Adrienne Sox</name>
<organization>Galileo, Inc.</organization>
</developer>
</developers>

<licenses>
Expand Down

0 comments on commit 024d848

Please sign in to comment.