From 43c07077be5220356c0397dde7af26db60c5ec1b Mon Sep 17 00:00:00 2001 From: James Agnew Date: Fri, 22 Feb 2019 12:57:07 -0500 Subject: [PATCH] Automatically remove duplicate conditional creates in JPA --- .../fhir/jpa/dao/TransactionProcessor.java | 49 +++++++++++++-- .../fhir/jpa/dao/r4/FhirSystemDaoR4Test.java | 61 +++++++++++++++++++ src/changes/changes.xml | 6 ++ 3 files changed, 112 insertions(+), 4 deletions(-) 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 ea8b95368d5..512b5e1f452 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 @@ -44,10 +44,7 @@ import ca.uhn.fhir.rest.server.interceptor.IServerInterceptor; import ca.uhn.fhir.rest.server.method.BaseMethodBinding; import ca.uhn.fhir.rest.server.method.BaseResourceReturningMethodBinding; import ca.uhn.fhir.rest.server.servlet.ServletRequestDetails; -import ca.uhn.fhir.util.FhirTerser; -import ca.uhn.fhir.util.ResourceReferenceInfo; -import ca.uhn.fhir.util.StopWatch; -import ca.uhn.fhir.util.UrlUtil; +import ca.uhn.fhir.util.*; import com.google.common.collect.ArrayListMultimap; import org.apache.commons.lang3.Validate; import org.apache.http.NameValuePair; @@ -98,6 +95,14 @@ public class TransactionProcessor { String actionName = "Transaction"; BUNDLE response = processTransactionAsSubRequest((ServletRequestDetails) theRequestDetails, theRequest, actionName); + List entries = myVersionAdapter.getEntries(response); + for (int i = 0; i < entries.size(); i++) { + if (ElementUtil.isEmpty(entries.get(i))) { + entries.remove(i); + i--; + } + } + return response; } @@ -500,6 +505,42 @@ public class TransactionProcessor { Set updatedEntities = new HashSet<>(); Map> conditionalRequestUrls = new HashMap<>(); + /* + * Look for duplicate conditional creates and consolidate them + */ + Map ifNoneExistToUuid = new HashMap<>(); + for (int index = 0, originalIndex = 0; index < theEntries.size(); index++, originalIndex++) { + BUNDLEENTRY nextReqEntry = theEntries.get(index); + String verb = myVersionAdapter.getEntryRequestVerb(nextReqEntry); + String entryUrl = myVersionAdapter.getFullUrl(nextReqEntry); + String requestUrl = myVersionAdapter.getEntryRequestUrl(nextReqEntry); + String ifNoneExist = myVersionAdapter.getEntryRequestIfNoneExist(nextReqEntry); + if ("POST".equals(verb)) { + if (isNotBlank(entryUrl) && isNotBlank(requestUrl) && isNotBlank(ifNoneExist)) { + if (!entryUrl.equals(requestUrl)) { + String key = requestUrl + "|" + ifNoneExist; // just in case the ifNoneExist doesn't include the resource type + if (!ifNoneExistToUuid.containsKey(key)) { + ifNoneExistToUuid.put(key, entryUrl); + } else { + ourLog.info("Discarding transaction bundle entry {} as it contained a duplicate conditional create: {}", originalIndex, ifNoneExist); + theEntries.remove(index); + index--; + String existingUuid = ifNoneExistToUuid.get(key); + for (BUNDLEENTRY nextEntry : theEntries) { + IBaseResource nextResource = myVersionAdapter.getResource(nextEntry); + for (ResourceReferenceInfo nextReference : myContext.newTerser().getAllResourceReferences(nextResource)) { + if (entryUrl.equals(nextReference.getResourceReference().getReferenceElement().getValue())) { + nextReference.getResourceReference().setReference(existingUuid); + } + } + } + } + } + } + } + } + + /* * Loop through the request and process any entries of type * PUT, POST or DELETE diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirSystemDaoR4Test.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirSystemDaoR4Test.java index dea5dcd130d..bd4a7607eab 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirSystemDaoR4Test.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirSystemDaoR4Test.java @@ -38,6 +38,7 @@ import java.io.UnsupportedEncodingException; import java.math.BigDecimal; import java.nio.charset.StandardCharsets; import java.util.*; +import java.util.stream.Collectors; import static org.hamcrest.Matchers.*; import static org.junit.Assert.*; @@ -1044,6 +1045,66 @@ public class FhirSystemDaoR4Test extends BaseJpaR4SystemTest { } + @Test + public void testTransactionWithDuplicateConditionalCreates() { + Bundle request = new Bundle(); + request.setType(BundleType.TRANSACTION); + + Practitioner p = new Practitioner(); + p.setId(IdType.newRandomUuid()); + p.addIdentifier().setSystem("http://foo").setValue("bar"); + request.addEntry() + .setFullUrl(p.getId()) + .setResource(p) + .getRequest() + .setMethod(HTTPVerb.POST) + .setUrl("Practitioner/") + .setIfNoneExist("Practitioner?identifier=http://foo|bar"); + + Observation o = new Observation(); + o.setId(IdType.newRandomUuid()); + o.getPerformerFirstRep().setReference(p.getId()); + request.addEntry() + .setFullUrl(o.getId()) + .setResource(o) + .getRequest() + .setMethod(HTTPVerb.POST) + .setUrl("Observation/"); + + p = new Practitioner(); + p.setId(IdType.newRandomUuid()); + p.addIdentifier().setSystem("http://foo").setValue("bar"); + request.addEntry() + .setFullUrl(p.getId()) + .setResource(p) + .getRequest() + .setMethod(HTTPVerb.POST) + .setUrl("Practitioner/") + .setIfNoneExist("Practitioner?identifier=http://foo|bar"); + + o = new Observation(); + o.setId(IdType.newRandomUuid()); + o.getPerformerFirstRep().setReference(p.getId()); + request.addEntry() + .setFullUrl(o.getId()) + .setResource(o) + .getRequest() + .setMethod(HTTPVerb.POST) + .setUrl("Observation/"); + + Bundle response = mySystemDao.transaction(null, request); + + ourLog.info("Response:\n{}", myFhirCtx.newJsonParser().setPrettyPrint(true).encodeResourceToString(response)); + + List responseTypes = response + .getEntry() + .stream() + .map(t -> new IdType(t.getResponse().getLocation()).getResourceType()) + .collect(Collectors.toList()); + assertThat(responseTypes.toString(), responseTypes, contains("Practitioner", "Observation", "Observation")); + } + + @Test public void testTransactionCreateInlineMatchUrlWithTwoMatches() { String methodName = "testTransactionCreateInlineMatchUrlWithTwoMatches"; diff --git a/src/changes/changes.xml b/src/changes/changes.xml index 6ee1f1fae39..a8940c75ae0 100644 --- a/src/changes/changes.xml +++ b/src/changes/changes.xml @@ -52,6 +52,12 @@ The JPA query builder has been optimized to take better advantage of SQL IN (..) expressions when performing token searches with multiple OR values. + + The JPA server transaction processor will now automatically detect if the request + Bundle contains multiple entries having identical conditional create operations, and + collapse these into a single operation. This is done as a convenience, since many + conversion algorithms can accidentally generate such duplicates. +