From 62c630c546e985db73e37fc6dc75ba7a7a8182f9 Mon Sep 17 00:00:00 2001 From: karneet1212 <112980019+karneet1212@users.noreply.github.com> Date: Mon, 21 Nov 2022 10:08:10 -0500 Subject: [PATCH] bundle not throwing an error when multiple matches were found (#4283) * Added test + condition to throw error * changelog * changes made based on comments --- ...tches-are-found-for-a-conditional-url.yaml | 6 ++++ .../fhir/jpa/dao/r4/FhirSystemDaoR4Test.java | 35 +++++++++++++++++++ .../server/storage/TransactionDetails.java | 12 +++++++ 3 files changed, 53 insertions(+) create mode 100644 hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/6_3_0/4281-bundle-does-not-throw-an-error-when-multiple-matches-are-found-for-a-conditional-url.yaml diff --git a/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/6_3_0/4281-bundle-does-not-throw-an-error-when-multiple-matches-are-found-for-a-conditional-url.yaml b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/6_3_0/4281-bundle-does-not-throw-an-error-when-multiple-matches-are-found-for-a-conditional-url.yaml new file mode 100644 index 00000000000..3374876bfee --- /dev/null +++ b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/6_3_0/4281-bundle-does-not-throw-an-error-when-multiple-matches-are-found-for-a-conditional-url.yaml @@ -0,0 +1,6 @@ +--- +type: fix +issue: 4281 +title: "Previously, when creating a bundle, if multiple resources matched the conditional URL provided + in the request, it would process the request using the last resource found as a match. + An error is now thrown when multiple resources are found" diff --git a/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirSystemDaoR4Test.java b/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirSystemDaoR4Test.java index a4a4bd874b1..96d3146eafa 100644 --- a/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirSystemDaoR4Test.java +++ b/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirSystemDaoR4Test.java @@ -1881,6 +1881,41 @@ public class FhirSystemDaoR4Test extends BaseJpaR4SystemTest { } } + @Test + public void testTransactionCreateMatchUrlWithTwoMatch2() { + String methodName = "testTransactionCreateMatchUrlWithTwoMatch"; + + Patient p = new Patient(); + p.addIdentifier().setSystem("urn:system").setValue(methodName); + IIdType id = myPatientDao.create(p, mySrd).getId(); + ourLog.info("Created patient, got it: {}", id); + + p = new Patient(); + p.addIdentifier().setSystem("urn:system").setValue(methodName); + id = myPatientDao.create(p, mySrd).getId(); + ourLog.info("Created patient, got it: {}", id); + + Bundle request = new Bundle(); + p = new Patient(); + p.addIdentifier().setSystem("urn:system").setValue(methodName); + p.addName().setFamily("Hello"); + p.setId("Patient/" + methodName); + request.addEntry().setResource(p).getRequest().setMethod(HTTPVerb.POST).setIfNoneExist("Patient?identifier=urn%3Asystem%7C" + methodName); + + Observation o = new Observation(); + o.addIdentifier().setSystem("urn:system").setValue(methodName); + o.getCode().setText("Some Observation"); + o.getSubject().setReference("Patient/" + methodName); + request.addEntry().setResource(o).getRequest().setMethod(HTTPVerb.POST).setIfNoneExist("Observation?identifier=urn%3Asystem%7C" + methodName); + + try { + mySystemDao.transaction(mySrd, request); + fail(); + } catch (PreconditionFailedException e) { + assertThat(e.getMessage(), containsString("Multiple resources match this search")); + } + } + @Test public void testTransactionCreateMatchUrlWithZeroMatch() { String methodName = "testTransactionCreateMatchUrlWithZeroMatch"; diff --git a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/api/server/storage/TransactionDetails.java b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/api/server/storage/TransactionDetails.java index bda664b1f34..12b39e4c71e 100644 --- a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/api/server/storage/TransactionDetails.java +++ b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/api/server/storage/TransactionDetails.java @@ -20,9 +20,11 @@ package ca.uhn.fhir.rest.api.server.storage; * #L% */ +import ca.uhn.fhir.i18n.Msg; import ca.uhn.fhir.interceptor.api.HookParams; import ca.uhn.fhir.interceptor.api.Pointcut; import ca.uhn.fhir.rest.api.InterceptorInvocationTimingEnum; +import ca.uhn.fhir.rest.server.exceptions.PreconditionFailedException; import com.google.common.collect.ArrayListMultimap; import com.google.common.collect.ListMultimap; import org.apache.commons.lang3.Validate; @@ -162,10 +164,20 @@ public class TransactionDetails { if (myResolvedMatchUrls.isEmpty()) { myResolvedMatchUrls = new HashMap<>(); + } else if (matchUrlWithDiffIdExists(theConditionalUrl, thePersistentId)) { + String msg = "Invalid match URL " + theConditionalUrl + " - Multiple resources match this search"; + throw new PreconditionFailedException(Msg.code(2207) + msg); } myResolvedMatchUrls.put(theConditionalUrl, thePersistentId); } + private boolean matchUrlWithDiffIdExists(String theConditionalUrl, @Nonnull ResourcePersistentId thePersistentId) { + if (myResolvedMatchUrls.containsKey(theConditionalUrl) && myResolvedMatchUrls.get(theConditionalUrl) != NOT_FOUND) { + return myResolvedMatchUrls.get(theConditionalUrl).getId() != thePersistentId.getId(); + } + return false; + } + /** * This is the wall-clock time that a given transaction started. */