From 0f6ae50105b3ba2e7c6e57f276b9400d5c1a46d9 Mon Sep 17 00:00:00 2001 From: Tadgh Date: Tue, 31 Aug 2021 10:47:23 -0400 Subject: [PATCH] Fix bug in hashToSearchMap --- .../fhir/jpa/dao/TransactionProcessor.java | 22 ++++-- .../r4/FhirResourceDaoR4SearchNoFtTest.java | 74 ++++--------------- .../duplicate-conditional-create.json | 66 +++++++++++++++++ 3 files changed, 94 insertions(+), 68 deletions(-) create mode 100644 hapi-fhir-jpaserver-base/src/test/resources/duplicate-conditional-create.json diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/TransactionProcessor.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/TransactionProcessor.java index bc73ea4a623..c8e25d517e3 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/TransactionProcessor.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/TransactionProcessor.java @@ -244,17 +244,21 @@ public class TransactionProcessor extends BaseTransactionProcessor { if (orPredicates.size() > 1) { cq.where(cb.or(orPredicates.toArray(EMPTY_PREDICATE_ARRAY))); - Map hashToSearchMap = buildHashToSearchMap(searchParameterMapsToResolve); + Map> hashToSearchMap = buildHashToSearchMap(searchParameterMapsToResolve); TypedQuery query = myEntityManager.createQuery(cq); List results = query.getResultList(); for (ResourceIndexedSearchParamToken nextResult : results) { - Optional matchedSearch = Optional.ofNullable(hashToSearchMap.get(nextResult.getHashSystemAndValue())); + Optional> matchedSearch = Optional.ofNullable(hashToSearchMap.get(nextResult.getHashSystemAndValue())); if (!matchedSearch.isPresent()) { matchedSearch = Optional.ofNullable(hashToSearchMap.get(nextResult.getHashValue())); } - matchedSearch.ifPresent(matchUrlToResolve -> setSearchToResolvedAndPrefetchFoundResourcePid(theTransactionDetails, idsToPreFetch, nextResult, matchUrlToResolve)); + matchedSearch.ifPresent(matchUrlsToResolve -> { + matchUrlsToResolve.forEach(matchUrl -> { + setSearchToResolvedAndPrefetchFoundResourcePid(theTransactionDetails, idsToPreFetch, nextResult, matchUrl); + }); + }); } //For each SP Map which did not return a result, tag it as not found. searchParameterMapsToResolve.stream() @@ -322,15 +326,19 @@ public class TransactionProcessor extends BaseTransactionProcessor { return hashPredicate; } - private Map buildHashToSearchMap(List searchParameterMapsToResolve) { - Map hashToSearch = new HashMap<>(); + private Map> buildHashToSearchMap(List searchParameterMapsToResolve) { + Map> hashToSearch = new HashMap<>(); //Build a lookup map so we don't have to iterate over the searches repeatedly. for (MatchUrlToResolve nextSearchParameterMap : searchParameterMapsToResolve) { if (nextSearchParameterMap.myHashSystemAndValue != null) { - hashToSearch.put(nextSearchParameterMap.myHashSystemAndValue, nextSearchParameterMap); + List matchUrlsToResolve = hashToSearch.getOrDefault(nextSearchParameterMap.myHashSystemAndValue, new ArrayList<>()); + matchUrlsToResolve.add(nextSearchParameterMap); + hashToSearch.put(nextSearchParameterMap.myHashSystemAndValue, matchUrlsToResolve); } if (nextSearchParameterMap.myHashValue!= null) { - hashToSearch.put(nextSearchParameterMap.myHashValue, nextSearchParameterMap); + List matchUrlsToResolve = hashToSearch.getOrDefault(nextSearchParameterMap.myHashValue, new ArrayList<>()); + matchUrlsToResolve.add(nextSearchParameterMap); + hashToSearch.put(nextSearchParameterMap.myHashValue, matchUrlsToResolve); } } return hashToSearch; diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4SearchNoFtTest.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4SearchNoFtTest.java index c7782f16389..71b3e7bcdc2 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4SearchNoFtTest.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4SearchNoFtTest.java @@ -127,6 +127,7 @@ import org.hl7.fhir.r4.model.ValueSet; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Disabled; +import org.junit.jupiter.api.DisplayName; import org.junit.jupiter.api.Test; import org.mockito.ArgumentCaptor; import org.mockito.ArgumentMatchers; @@ -1360,74 +1361,25 @@ public class FhirResourceDaoR4SearchNoFtTest extends BaseJpaR4Test { } @Test - public void testDuplicateConditionalCreatesOnToken() { - String bundle = "{\n" + - " \"resourceType\": \"Bundle\",\n" + - " \"type\": \"transaction\",\n" + - " \"entry\": [ {\n" + - " \"fullUrl\": \"urn:uuid:33b76421-1c91-471f-ae1c-e7486e804f18\",\n" + - " \"resource\": {\n" + - " \"resourceType\": \"Organization\",\n" + - " \"identifier\": [ {\n" + - " \"system\": \"https://fhir.infoway-inforoute.ca/NamingSystem/ca-on-health-care-facility-id\",\n" + - " \"value\": \"3972\"\n" + - " } ]\n" + - " },\n" + - " \"request\": {\n" + - " \"method\": \"POST\",\n" + - " \"url\": \"/Organization\",\n" + - " \"ifNoneExist\": \"Organization?identifier=https%3A%2F%2Ffhir.infoway-inforoute.ca%2FNamingSystem%2Fca-on-health-care-facility-id|3972\"\n" + - " }\n" + - " }, {\n" + - " \"fullUrl\": \"urn:uuid:65d2bf18-543e-4d05-b66b-07cee541172f\",\n" + - " \"resource\": {\n" + - " \"resourceType\": \"Organization\",\n" + - " \"identifier\": [ {\n" + - " \"system\": \"https://fhir.infoway-inforoute.ca/NamingSystem/ca-on-health-care-facility-id\",\n" + - " \"value\": \"3972\"\n" + - " } ]\n" + - " },\n" + - " \"request\": {\n" + - " \"method\": \"POST\",\n" + - " \"url\": \"/Organization\",\n" + - " \"ifNoneExist\": \"Organization?identifier=https%3A%2F%2Ffhir.infoway-inforoute.ca%2FNamingSystem%2Fca-on-health-care-facility-id|3972\"\n" + - " }\n" + - " }, {\n" + - " \"fullUrl\": \"urn:uuid:2a4635e2-e678-4ed7-9a92-901d67787434\",\n" + - " \"resource\": {\n" + - " \"resourceType\": \"ServiceRequest\",\n" + - " \"identifier\": [ {\n" + - " \"system\": \"https://corhealth-ontario.ca/NamingSystem/service-request-id\",\n" + - " \"value\": \"1\"\n" + - " } ],\n" + - " \"performer\": [ {\n" + - " \"reference\": \"urn:uuid:65d2bf18-543e-4d05-b66b-07cee541172f\",\n" + - " \"type\": \"Organization\"\n" + - " } ]\n" + - " },\n" + - " \"request\": {\n" + - " \"method\": \"PUT\",\n" + - " \"url\": \"/ServiceRequest?identifier=https%3A%2F%2Fcorhealth-ontario.ca%2FNamingSystem%2Fservice-request-id|1\"\n" + - " }\n" + - " } ]\n" + - "}"; + @DisplayName("Duplicate Conditional Creates all resolve to the same match") + public void testDuplicateConditionalCreatesOnToken() throws IOException { + String inputString = IOUtils.toString(getClass().getResourceAsStream("/duplicate-conditional-create.json"), StandardCharsets.UTF_8); + Bundle firstBundle = myFhirCtx.newJsonParser().parseResource(Bundle.class, inputString); - Bundle bundle1 = (Bundle) myFhirCtx.newJsonParser().parseResource(bundle); - Bundle duplicateBundle = (Bundle) myFhirCtx.newJsonParser().parseResource(bundle); - ourLog.error("TRANS 1"); - Bundle bundleResponse = mySystemDao.transaction(new SystemRequestDetails(), bundle1); - bundleResponse.getEntry().stream() - .forEach( entry -> { - assertThat(entry.getResponse().getStatus(), is(equalTo("201 Created"))); - }); + //Before you ask, yes, this has to be separately parsed. The reason for this is that the parameters passed to mySystemDao.transaction are _not_ immutable, so we cannot + //simply reuse the original bundle object. + Bundle duplicateBundle = myFhirCtx.newJsonParser().parseResource(Bundle.class, inputString); + + Bundle bundleResponse = mySystemDao.transaction(new SystemRequestDetails(), firstBundle); + bundleResponse.getEntry() + .forEach( entry -> assertThat(entry.getResponse().getStatus(), is(equalTo("201 Created")))); IBundleProvider search = myOrganizationDao.search(new SearchParameterMap().setLoadSynchronous(true)); assertEquals(1, search.getAllResources().size()); //Running the bundle again should just result in 0 new resources created, as the org should already exist, and there is no update to the SR. - ourLog.error("TRANS 2"); bundleResponse= mySystemDao.transaction(new SystemRequestDetails(), duplicateBundle); - bundleResponse.getEntry().stream() + bundleResponse.getEntry() .forEach( entry -> { assertThat(entry.getResponse().getStatus(), is(equalTo("200 OK"))); }); diff --git a/hapi-fhir-jpaserver-base/src/test/resources/duplicate-conditional-create.json b/hapi-fhir-jpaserver-base/src/test/resources/duplicate-conditional-create.json new file mode 100644 index 00000000000..26ea0369f1f --- /dev/null +++ b/hapi-fhir-jpaserver-base/src/test/resources/duplicate-conditional-create.json @@ -0,0 +1,66 @@ +{ + "resourceType": "Bundle", + "type": "transaction", + "entry": [ + { + "fullUrl": "urn:uuid:4cd35592-5d4d-462b-8483-e404c023d316", + "resource": { + "resourceType": "Organization", + "identifier": [ + { + "system": "https://fhir.tester.ca/NamingSystem/ca-on-health-care-facility-id", + "value": "3972" + } + ] + }, + "request": { + "method": "POST", + "url": "/Organization", + "ifNoneExist": "Organization?identifier=https://fhir.tester.ca/NamingSystem/ca-on-health-care-facility-id|3972" + } + }, + { + "fullUrl": "urn:uuid:02643c1d-94d1-4991-a063-036fa0f57ec2", + "resource": { + "resourceType": "Organization", + "identifier": [ + { + "system": "https://fhir.tester.ca/NamingSystem/ca-on-health-care-facility-id", + "value": "3972" + } + ] + }, + "request": { + "method": "POST", + "url": "/Organization", + "ifNoneExist": "Organization?identifier=https://fhir.tester.ca/NamingSystem/ca-on-health-care-facility-id|3972" + } + }, + { + "fullUrl": "urn:uuid:8271e94f-e08b-498e-ad6d-751928c3ff99", + "resource": { + "resourceType": "ServiceRequest", + "identifier": [ + { + "system": "https://fhir-tester.ca/NamingSystem/service-request-id", + "value": "1" + } + ], + "performer": [ + { + "reference": "urn:uuid:4cd35592-5d4d-462b-8483-e404c023d316", + "type": "Organization" + }, + { + "reference": "urn:uuid:02643c1d-94d1-4991-a063-036fa0f57ec2", + "type": "Organization" + } + ] + }, + "request": { + "method": "PUT", + "url": "/ServiceRequest?identifier=https://fhir-tester.ca/NamingSystem/service-request-id|1" + } + } + ] +}