diff --git a/hapi-fhir-base/src/main/java/ca/uhn/fhir/util/UrlUtil.java b/hapi-fhir-base/src/main/java/ca/uhn/fhir/util/UrlUtil.java index 1a223809dba..6da04f1a2ba 100644 --- a/hapi-fhir-base/src/main/java/ca/uhn/fhir/util/UrlUtil.java +++ b/hapi-fhir-base/src/main/java/ca/uhn/fhir/util/UrlUtil.java @@ -250,8 +250,16 @@ public class UrlUtil { } public static RuntimeResourceDefinition parseUrlResourceType(FhirContext theCtx, String theUrl) throws DataFormatException { - int paramIndex = theUrl.indexOf('?'); - String resourceName = theUrl.substring(0, paramIndex); + String url = theUrl; + int paramIndex = url.indexOf('?'); + + // Change pattern of "Observation/?param=foo" into "Observation?param=foo" + if (paramIndex > 0 && url.charAt(paramIndex - 1) == '/') { + url = url.substring(0, paramIndex - 1) + url.substring(paramIndex); + paramIndex--; + } + + String resourceName = url.substring(0, paramIndex); if (resourceName.contains("/")) { resourceName = resourceName.substring(resourceName.lastIndexOf('/') + 1); } diff --git a/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/1_6/changes.yaml b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/1_6/changes.yaml index 4ad3a2c19c7..99d452718fa 100644 --- a/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/1_6/changes.yaml +++ b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/1_6/changes.yaml @@ -31,7 +31,7 @@ title: "JSON parser no longer allows the resource ID to be specified in an element called \"_id\" (the correct one is \"id\"). Previously _id was allowed because some early FHIR examples used that form, but this was never actually valid so it is now being removed." - item: type: "add" - title: "JPA server now allows \"forced IDs\" (ids containing non-numeric, client assigned IDs) to use the same logical ID part on different resource types. E.g. A server may now have both Patient/foo and Obervation/foo on the same server.

Note that existing databases will need to modify index \"IDX_FORCEDID\" as it is no longer unique, and perform a reindexing pass." + title: "JPA server now allows \"forced IDs\" (ids containing non-numeric, client assigned IDs) to use the same logical ID part on different resource types. E.g. A server may now have both Patient/foo and Observation/foo on the same server.

Note that existing databases will need to modify index \"IDX_FORCEDID\" as it is no longer unique, and perform a reindexing pass." - item: issue: "350" type: "fix" diff --git a/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/5_5_0/2688-avoid-sql-dupe-check.yaml b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/5_5_0/2688-avoid-sql-dupe-check.yaml new file mode 100644 index 00000000000..74b097d75b7 --- /dev/null +++ b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/5_5_0/2688-avoid-sql-dupe-check.yaml @@ -0,0 +1,5 @@ +--- +type: perf +issue: 2688 +title: "FHIR Transaction duplicate record checks are now performed without any database interactions or SQL statements, + reducing the processing load associated with FHIR transactions by at least a small amount." diff --git a/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/5_5_0/2688-reduce-match-url-limit.yaml b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/5_5_0/2688-reduce-match-url-limit.yaml new file mode 100644 index 00000000000..9192981dfef --- /dev/null +++ b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/5_5_0/2688-reduce-match-url-limit.yaml @@ -0,0 +1,5 @@ +--- +type: perf +issue: 2688 +title: "Conditional URL lookups in the JPA server will now explicitly specify a maximum fetch size of 2, avoiding + fetching more data that won't be used inadvertently in some situations." diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseHapiFhirResourceDao.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseHapiFhirResourceDao.java index 666b5d88d7e..47ae3de2975 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseHapiFhirResourceDao.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseHapiFhirResourceDao.java @@ -1447,7 +1447,12 @@ public abstract class BaseHapiFhirResourceDao extends B @Override public Set searchForIds(SearchParameterMap theParams, RequestDetails theRequest) { return myTransactionService.execute(theRequest, tx -> { - theParams.setLoadSynchronousUpTo(getConfig().getInternalSynchronousSearchSize()); + + if (theParams.getLoadSynchronousUpTo() != null) { + theParams.setLoadSynchronousUpTo(Math.min(getConfig().getInternalSynchronousSearchSize(), theParams.getLoadSynchronousUpTo())); + } else { + theParams.setLoadSynchronousUpTo(getConfig().getInternalSynchronousSearchSize()); + } ISearchBuilder builder = mySearchBuilderFactory.newSearchBuilder(this, getResourceName(), getResourceType()); @@ -1573,7 +1578,14 @@ public abstract class BaseHapiFhirResourceDao extends B entity = myEntityManager.find(ResourceTable.class, pid.getId()); resourceId = entity.getIdDt(); } else { - return create(resource, null, thePerformIndexing, theTransactionDetails, theRequest); + DaoMethodOutcome outcome = create(resource, null, thePerformIndexing, theTransactionDetails, theRequest); + + // Pre-cache the match URL + if (outcome.getPersistentId() != null) { + myMatchResourceUrlService.matchUrlResolved(theMatchUrl, outcome.getPersistentId()); + } + + return outcome; } } else { /* diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseTransactionProcessor.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseTransactionProcessor.java index e27cbd2ad36..b8ae25e13d5 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseTransactionProcessor.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseTransactionProcessor.java @@ -39,7 +39,8 @@ import ca.uhn.fhir.jpa.model.cross.IBasePersistedResource; import ca.uhn.fhir.jpa.model.entity.ModelConfig; import ca.uhn.fhir.jpa.model.entity.ResourceTable; import ca.uhn.fhir.jpa.model.search.StorageProcessingMessage; -import ca.uhn.fhir.rest.server.util.CompositeInterceptorBroadcaster; +import ca.uhn.fhir.jpa.searchparam.extractor.ResourceIndexedSearchParams; +import ca.uhn.fhir.jpa.searchparam.matcher.InMemoryResourceMatcher; import ca.uhn.fhir.model.api.IResource; import ca.uhn.fhir.model.api.ResourceMetadataKeyEnum; import ca.uhn.fhir.parser.DataFormatException; @@ -50,7 +51,6 @@ import ca.uhn.fhir.rest.api.PreferReturnEnum; import ca.uhn.fhir.rest.api.RestOperationTypeEnum; import ca.uhn.fhir.rest.api.server.RequestDetails; import ca.uhn.fhir.rest.api.server.storage.DeferredInterceptorBroadcasts; -import ca.uhn.fhir.rest.api.server.storage.ResourcePersistentId; import ca.uhn.fhir.rest.api.server.storage.TransactionDetails; import ca.uhn.fhir.rest.param.ParameterUtil; import ca.uhn.fhir.rest.server.RestfulServerUtils; @@ -65,6 +65,7 @@ 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.rest.server.servlet.ServletSubRequestDetails; +import ca.uhn.fhir.rest.server.util.CompositeInterceptorBroadcaster; import ca.uhn.fhir.rest.server.util.ServletRequestUtil; import ca.uhn.fhir.util.ElementUtil; import ca.uhn.fhir.util.FhirTerser; @@ -97,6 +98,7 @@ import org.springframework.transaction.support.TransactionTemplate; import javax.annotation.PostConstruct; import java.util.ArrayList; +import java.util.Collection; import java.util.Comparator; import java.util.Date; import java.util.HashMap; @@ -109,6 +111,7 @@ import java.util.Map; import java.util.Optional; import java.util.Set; import java.util.TreeSet; +import java.util.regex.Pattern; import static ca.uhn.fhir.util.StringUtil.toUtf8String; import static org.apache.commons.lang3.StringUtils.defaultString; @@ -118,6 +121,7 @@ import static org.apache.commons.lang3.StringUtils.isNotBlank; public abstract class BaseTransactionProcessor { public static final String URN_PREFIX = "urn:"; + public static final Pattern UNQUALIFIED_MATCH_URL_START = Pattern.compile("^[a-zA-Z0-9_]+="); private static final Logger ourLog = LoggerFactory.getLogger(TransactionProcessor.class); private BaseHapiFhirDao myDao; @Autowired @@ -138,6 +142,8 @@ public abstract class BaseTransactionProcessor { private DaoConfig myDaoConfig; @Autowired private ModelConfig myModelConfig; + @Autowired + private InMemoryResourceMatcher myInMemoryResourceMatcher; @PostConstruct public void start() { @@ -937,7 +943,9 @@ public abstract class BaseTransactionProcessor { if (conditionalRequestUrls.size() > 0) { theTransactionStopWatch.startTask("Check for conflicts in conditional resources"); } - validateNoDuplicates(theRequest, theActionName, conditionalRequestUrls); + if (!myDaoConfig.isMassIngestionMode()) { + validateNoDuplicates(theRequest, theActionName, conditionalRequestUrls, theIdToPersistedOutcome.values()); + } theTransactionStopWatch.endCurrentTask(); for (IIdType next : theAllIds) { @@ -986,15 +994,15 @@ public abstract class BaseTransactionProcessor { * in the database. This is trickier than you'd think because of a couple of possibilities during the * save: * * There may be resources that have not changed (e.g. an update/PUT with a resource body identical - * to what is already in the database) + * to what is already in the database) * * There may be resources with auto-versioned references, meaning we're replacing certain references - * in the resource with a versioned references, referencing the current version at the time of the - * transaction processing + * in the resource with a versioned references, referencing the current version at the time of the + * transaction processing * * There may by auto-versioned references pointing to these unchanged targets - * + *

* If we're not doing any auto-versioned references, we'll just iterate through all resources in the * transaction and save them one at a time. - * + *

* However, if we have any auto-versioned references we do this in 2 passes: First the resources from the * transaction that don't have any auto-versioned references are stored. We do them first since there's * a chance they may be a NOP and we'll need to account for their version number not actually changing. @@ -1162,15 +1170,51 @@ public abstract class BaseTransactionProcessor { } } - private void validateNoDuplicates(RequestDetails theRequest, String theActionName, Map> conditionalRequestUrls) { + private void validateNoDuplicates(RequestDetails theRequest, String theActionName, Map> conditionalRequestUrls, Collection thePersistedOutcomes) { + + IdentityHashMap resourceToIndexedParams = new IdentityHashMap<>(thePersistedOutcomes.size()); + thePersistedOutcomes + .stream() + .filter(t -> !t.isNop()) + .filter(t -> t.getEntity() instanceof ResourceTable) + .filter(t -> t.getEntity().getDeleted() == null) + .filter(t -> t.getResource() != null) + .forEach(t -> resourceToIndexedParams.put(t.getResource(), new ResourceIndexedSearchParams((ResourceTable) t.getEntity()))); + for (Map.Entry> nextEntry : conditionalRequestUrls.entrySet()) { String matchUrl = nextEntry.getKey(); - Class resType = nextEntry.getValue(); if (isNotBlank(matchUrl)) { - Set val = myMatchResourceUrlService.processMatchUrl(matchUrl, resType, theRequest); - if (val.size() > 1) { - throw new InvalidRequestException( - "Unable to process " + theActionName + " - Request would cause multiple resources to match URL: \"" + matchUrl + "\". Does transaction request contain duplicates?"); + if (matchUrl.startsWith("?") || (!matchUrl.contains("?") && UNQUALIFIED_MATCH_URL_START.matcher(matchUrl).find())) { + StringBuilder b = new StringBuilder(); + b.append(myContext.getResourceType(nextEntry.getValue())); + if (!matchUrl.startsWith("?")) { + b.append("?"); + } + b.append(matchUrl); + matchUrl = b.toString(); + } + + if (!myInMemoryResourceMatcher.canBeEvaluatedInMemory(matchUrl).supported()) { + continue; + } + + int counter = 0; + for (Map.Entry entries : resourceToIndexedParams.entrySet()) { + ResourceIndexedSearchParams indexedParams = entries.getValue(); + IBaseResource resource = entries.getKey(); + + String resourceType = myContext.getResourceType(resource); + if (!matchUrl.startsWith(resourceType + "?")) { + continue; + } + + if (myInMemoryResourceMatcher.match(matchUrl, resource, indexedParams).matched()) { + counter++; + if (counter > 1) { + throw new InvalidRequestException( + "Unable to process " + theActionName + " - Request would cause multiple resources to match URL: \"" + matchUrl + "\". Does transaction request contain duplicates?"); + } + } } } } diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/MatchResourceUrlService.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/MatchResourceUrlService.java index 820a03feba0..1fe62763f22 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/MatchResourceUrlService.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/MatchResourceUrlService.java @@ -62,6 +62,9 @@ public class MatchResourceUrlService { @Autowired private MemoryCacheService myMemoryCacheService; + /** + * Note that this will only return a maximum of 2 results!! + */ public Set processMatchUrl(String theMatchUrl, Class theResourceType, RequestDetails theRequest) { if (myDaoConfig.getMatchUrlCache()) { ResourcePersistentId existing = myMemoryCacheService.getIfPresent(MemoryCacheService.CacheEnum.MATCH_URL, theMatchUrl); @@ -75,7 +78,7 @@ public class MatchResourceUrlService { if (paramMap.isEmpty() && paramMap.getLastUpdated() == null) { throw new InvalidRequestException("Invalid match URL[" + theMatchUrl + "] - URL has no search parameters"); } - paramMap.setLoadSynchronous(true); + paramMap.setLoadSynchronousUpTo(2); Set retVal = search(paramMap, theResourceType, theRequest); diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/TransactionProcessorTest.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/TransactionProcessorTest.java index 301f5476e7a..97824dfa84a 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/TransactionProcessorTest.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/TransactionProcessorTest.java @@ -8,6 +8,7 @@ import ca.uhn.fhir.jpa.api.dao.DaoRegistry; import ca.uhn.fhir.jpa.dao.r4.TransactionProcessorVersionAdapterR4; import ca.uhn.fhir.jpa.dao.tx.HapiTransactionService; import ca.uhn.fhir.jpa.model.entity.ModelConfig; +import ca.uhn.fhir.jpa.searchparam.matcher.InMemoryResourceMatcher; import ca.uhn.fhir.rest.server.exceptions.InvalidRequestException; import org.hibernate.Session; import org.hibernate.internal.SessionImpl; @@ -53,6 +54,8 @@ public class TransactionProcessorTest { private HapiTransactionService myHapiTransactionService; @MockBean private ModelConfig myModelConfig; + @MockBean + private InMemoryResourceMatcher myInMemoryResourceMatcher; @MockBean(answer = Answers.RETURNS_DEEP_STUBS) private SessionImpl mySession; diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4QueryCountTest.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4QueryCountTest.java index a594d1e7388..baf324bd080 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4QueryCountTest.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4QueryCountTest.java @@ -45,6 +45,7 @@ import java.util.stream.Collectors; import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.containsInAnyOrder; +import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.empty; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.mockito.ArgumentMatchers.eq; @@ -714,7 +715,7 @@ public class FhirResourceDaoR4QueryCountTest extends BaseJpaR4Test { myCaptureQueriesListener.clear(); mySystemDao.transaction(mySrd, bundleCreator.get()); - assertEquals(2, myCaptureQueriesListener.countSelectQueries()); + assertEquals(1, myCaptureQueriesListener.countSelectQueries()); assertEquals(5, myCaptureQueriesListener.countInsertQueries()); assertEquals(0, myCaptureQueriesListener.countDeleteQueries()); @@ -775,7 +776,8 @@ public class FhirResourceDaoR4QueryCountTest extends BaseJpaR4Test { myCaptureQueriesListener.clear(); mySystemDao.transaction(mySrd, bundleCreator.get()); - assertEquals(2, myCaptureQueriesListener.countSelectQueries()); + myCaptureQueriesListener.logSelectQueries(); + assertEquals(1, myCaptureQueriesListener.countSelectQueries()); assertEquals(5, myCaptureQueriesListener.countInsertQueries()); assertEquals(0, myCaptureQueriesListener.countDeleteQueries()); @@ -793,6 +795,11 @@ public class FhirResourceDaoR4QueryCountTest extends BaseJpaR4Test { assertEquals(3, myCaptureQueriesListener.countInsertQueries()); assertEquals(0, myCaptureQueriesListener.countDeleteQueries()); + // Make sure the match URL query uses a small limit + String matchUrlQuery = myCaptureQueriesListener.getSelectQueries().get(0).getSql(true, true); + assertThat(matchUrlQuery, containsString("t0.HASH_SYS_AND_VALUE = '-4132452001562191669'")); + assertThat(matchUrlQuery, containsString("limit '2'")); + runInTransaction(()->{ List types = myResourceTableDao.findAll().stream().map(t -> t.getResourceType()).collect(Collectors.toList()); assertThat(types, containsInAnyOrder("Patient", "Observation", "Observation")); @@ -1550,7 +1557,7 @@ public class FhirResourceDaoR4QueryCountTest extends BaseJpaR4Test { myCaptureQueriesListener.clear(); mySystemDao.transaction(new SystemRequestDetails(), supplier.get()); // myCaptureQueriesListener.logSelectQueriesForCurrentThread(); - assertEquals(5, myCaptureQueriesListener.countSelectQueriesForCurrentThread()); + assertEquals(3, myCaptureQueriesListener.countSelectQueriesForCurrentThread()); assertEquals(13, myCaptureQueriesListener.countInsertQueriesForCurrentThread()); assertEquals(3, myCaptureQueriesListener.countUpdateQueriesForCurrentThread()); assertEquals(0, myCaptureQueriesListener.countDeleteQueriesForCurrentThread()); 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 d391209848d..ee2ef1e107e 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 @@ -1,7 +1,6 @@ package ca.uhn.fhir.jpa.dao.r4; import ca.uhn.fhir.jpa.api.config.DaoConfig; -import ca.uhn.fhir.jpa.api.model.DaoMethodOutcome; import ca.uhn.fhir.jpa.dao.BaseHapiFhirDao; import ca.uhn.fhir.jpa.model.entity.NormalizedQuantitySearchLevel; import ca.uhn.fhir.jpa.model.entity.ResourceEncodingEnum; @@ -10,7 +9,6 @@ import ca.uhn.fhir.jpa.model.entity.ResourceIndexedSearchParamString; import ca.uhn.fhir.jpa.model.entity.ResourceTable; import ca.uhn.fhir.jpa.model.entity.ResourceTag; import ca.uhn.fhir.jpa.model.entity.TagTypeEnum; -import ca.uhn.fhir.jpa.partition.SystemRequestDetails; import ca.uhn.fhir.jpa.provider.SystemProviderDstu2Test; import ca.uhn.fhir.jpa.searchparam.SearchParameterMap; import ca.uhn.fhir.model.api.ResourceMetadataKeyEnum; @@ -49,7 +47,6 @@ import org.hl7.fhir.r4.model.Communication; import org.hl7.fhir.r4.model.Condition; import org.hl7.fhir.r4.model.DateTimeType; import org.hl7.fhir.r4.model.DiagnosticReport; -import org.hl7.fhir.r4.model.DocumentReference; import org.hl7.fhir.r4.model.Encounter; import org.hl7.fhir.r4.model.EpisodeOfCare; import org.hl7.fhir.r4.model.IdType; @@ -88,12 +85,10 @@ import java.util.List; import java.util.Map; import java.util.Optional; import java.util.Set; -import java.util.function.Supplier; import java.util.stream.Collectors; import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.contains; -import static org.hamcrest.Matchers.containsInAnyOrder; import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.empty; import static org.hamcrest.Matchers.emptyString; @@ -104,7 +99,6 @@ import static org.hamcrest.Matchers.not; import static org.hamcrest.Matchers.startsWith; import static org.junit.jupiter.api.Assertions.assertArrayEquals; import static org.junit.jupiter.api.Assertions.assertEquals; -import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertNotEquals; import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertNull; @@ -833,7 +827,7 @@ public class FhirSystemDaoR4Test extends BaseJpaR4SystemTest { request.addEntry().getRequest().setMethod(HTTPVerb.GET).setUrl("Patient?identifier=foo"); try { - runInTransaction(()->{ + runInTransaction(() -> { mySystemDao.transactionNested(mySrd, request); }); fail(); @@ -886,6 +880,128 @@ public class FhirSystemDaoR4Test extends BaseJpaR4SystemTest { } + @Test + public void testTransactionWithConditionalCreates_IdenticalMatchUrlsDifferentTypes_Unqualified() { + BundleBuilder bb = new BundleBuilder(myFhirCtx); + Patient pt = new Patient(); + pt.addIdentifier().setSystem("foo").setValue("bar"); + bb.addTransactionCreateEntry(pt).conditional("identifier=foo|bar"); + Observation obs = new Observation(); + obs.addIdentifier().setSystem("foo").setValue("bar"); + bb.addTransactionCreateEntry(obs).conditional("identifier=foo|bar"); + + Bundle outcome = mySystemDao.transaction(mySrd, (Bundle) bb.getBundle()); + assertEquals("201 Created", outcome.getEntry().get(0).getResponse().getStatus()); + assertThat(outcome.getEntry().get(0).getResponse().getLocation(), matchesPattern(".*Patient/[0-9]+/_history/1")); + assertEquals("201 Created", outcome.getEntry().get(1).getResponse().getStatus()); + assertThat(outcome.getEntry().get(1).getResponse().getLocation(), matchesPattern(".*Observation/[0-9]+/_history/1")); + + // Take 2 + + bb = new BundleBuilder(myFhirCtx); + pt = new Patient(); + pt.addIdentifier().setSystem("foo").setValue("bar"); + bb.addTransactionCreateEntry(pt).conditional("identifier=foo|bar"); + obs = new Observation(); + obs.addIdentifier().setSystem("foo").setValue("bar"); + bb.addTransactionCreateEntry(obs).conditional("identifier=foo|bar"); + + outcome = mySystemDao.transaction(mySrd, (Bundle) bb.getBundle()); + assertEquals("200 OK", outcome.getEntry().get(0).getResponse().getStatus()); + assertThat(outcome.getEntry().get(0).getResponse().getLocation(), matchesPattern(".*Patient/[0-9]+/_history/1")); + assertEquals("200 OK", outcome.getEntry().get(1).getResponse().getStatus()); + assertThat(outcome.getEntry().get(1).getResponse().getLocation(), matchesPattern(".*Observation/[0-9]+/_history/1")); + + } + + + @Test + public void testTransactionWithConditionalCreates_IdenticalMatchUrlsDifferentTypes_Qualified() { + BundleBuilder bb = new BundleBuilder(myFhirCtx); + Patient pt = new Patient(); + pt.addIdentifier().setSystem("foo").setValue("bar"); + bb.addTransactionCreateEntry(pt).conditional("Patient?identifier=foo|bar"); + Observation obs = new Observation(); + obs.addIdentifier().setSystem("foo").setValue("bar"); + bb.addTransactionCreateEntry(obs).conditional("Observation?identifier=foo|bar"); + + Bundle outcome = mySystemDao.transaction(mySrd, (Bundle) bb.getBundle()); + assertEquals("201 Created", outcome.getEntry().get(0).getResponse().getStatus()); + assertThat(outcome.getEntry().get(0).getResponse().getLocation(), matchesPattern(".*Patient/[0-9]+/_history/1")); + assertEquals("201 Created", outcome.getEntry().get(1).getResponse().getStatus()); + assertThat(outcome.getEntry().get(1).getResponse().getLocation(), matchesPattern(".*Observation/[0-9]+/_history/1")); + + // Take 2 + + bb = new BundleBuilder(myFhirCtx); + pt = new Patient(); + pt.addIdentifier().setSystem("foo").setValue("bar"); + bb.addTransactionCreateEntry(pt).conditional("Patient?identifier=foo|bar"); + obs = new Observation(); + obs.addIdentifier().setSystem("foo").setValue("bar"); + bb.addTransactionCreateEntry(obs).conditional("Observation?identifier=foo|bar"); + + outcome = mySystemDao.transaction(mySrd, (Bundle) bb.getBundle()); + assertEquals("200 OK", outcome.getEntry().get(0).getResponse().getStatus()); + assertThat(outcome.getEntry().get(0).getResponse().getLocation(), matchesPattern(".*Patient/[0-9]+/_history/1")); + assertEquals("200 OK", outcome.getEntry().get(1).getResponse().getStatus()); + assertThat(outcome.getEntry().get(1).getResponse().getLocation(), matchesPattern(".*Observation/[0-9]+/_history/1")); + + } + + + @Test + public void testTransactionWithConditionalCreate_NoResourceTypeInUrl() { + BundleBuilder bb = new BundleBuilder(myFhirCtx); + Patient pt = new Patient(); + pt.setActive(true); + bb.addTransactionCreateEntry(pt).conditional("active=true"); + pt = new Patient(); + pt.setActive(false); + bb.addTransactionCreateEntry(pt).conditional("active=false"); + + Bundle outcome = mySystemDao.transaction(mySrd, (Bundle) bb.getBundle()); + assertEquals("201 Created", outcome.getEntry().get(0).getResponse().getStatus()); + assertEquals("201 Created", outcome.getEntry().get(1).getResponse().getStatus()); + + // Take 2 + + bb = new BundleBuilder(myFhirCtx); + pt = new Patient(); + pt.setActive(true); + bb.addTransactionCreateEntry(pt).conditional("active=true"); + pt = new Patient(); + pt.setActive(false); + bb.addTransactionCreateEntry(pt).conditional("active=false"); + + Bundle outcome2 = mySystemDao.transaction(mySrd, (Bundle) bb.getBundle()); + assertEquals("200 OK", outcome2.getEntry().get(0).getResponse().getStatus()); + assertEquals("200 OK", outcome2.getEntry().get(1).getResponse().getStatus()); + + assertThat(outcome.getEntry().get(0).getResponse().getLocation(), endsWith("/_history/1")); + assertThat(outcome.getEntry().get(1).getResponse().getLocation(), endsWith("/_history/1")); + assertEquals(outcome.getEntry().get(0).getResponse().getLocation(), outcome2.getEntry().get(0).getResponse().getLocation()); + assertEquals(outcome.getEntry().get(1).getResponse().getLocation(), outcome2.getEntry().get(1).getResponse().getLocation()); + + // Take 3 + + bb = new BundleBuilder(myFhirCtx); + pt = new Patient(); + pt.setActive(true); + bb.addTransactionCreateEntry(pt).conditional("?active=true"); + pt = new Patient(); + pt.setActive(false); + bb.addTransactionCreateEntry(pt).conditional("?active=false"); + + Bundle outcome3 = mySystemDao.transaction(mySrd, (Bundle) bb.getBundle()); + assertEquals("200 OK", outcome3.getEntry().get(0).getResponse().getStatus()); + assertEquals("200 OK", outcome3.getEntry().get(1).getResponse().getStatus()); + assertEquals(outcome.getEntry().get(0).getResponse().getLocation(), outcome3.getEntry().get(0).getResponse().getLocation()); + assertEquals(outcome.getEntry().get(1).getResponse().getLocation(), outcome3.getEntry().get(1).getResponse().getLocation()); + + } + + @Test public void testTransactionNoContained() throws IOException { diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/subscription/resthook/RestHookTestR4Test.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/subscription/resthook/RestHookTestR4Test.java index 0700bff5acf..b9b72b9fb8c 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/subscription/resthook/RestHookTestR4Test.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/subscription/resthook/RestHookTestR4Test.java @@ -726,7 +726,7 @@ public class RestHookTestR4Test extends BaseSubscriptionsR4Test { Subscription subscription2 = createSubscription(criteria2, payload); waitForActivatedSubscriptionCount(2); - ourLog.info("** About to send obervation"); + ourLog.info("** About to send observation"); Observation observation1 = sendObservation(code, "SNOMED-CT"); // Should see 1 subscription notification @@ -798,7 +798,7 @@ public class RestHookTestR4Test extends BaseSubscriptionsR4Test { createSubscription(criteria1, payload); waitForActivatedSubscriptionCount(1); - ourLog.info("** About to send obervation"); + ourLog.info("** About to send observation"); Observation observation = new Observation(); observation.addIdentifier().setSystem("foo").setValue("bar1"); @@ -820,7 +820,7 @@ public class RestHookTestR4Test extends BaseSubscriptionsR4Test { .setResource(observation) .setFullUrl(observation.getId()) .getRequest() - .setUrl("Obervation?identifier=foo|bar1") + .setUrl("Observation?identifier=foo|bar1") .setMethod(Bundle.HTTPVerb.PUT); requestBundle.addEntry() .setResource(patient) diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/subscription/resthook/RestHookTestR5Test.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/subscription/resthook/RestHookTestR5Test.java index 4136ce05073..efde0105609 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/subscription/resthook/RestHookTestR5Test.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/subscription/resthook/RestHookTestR5Test.java @@ -593,7 +593,7 @@ public class RestHookTestR5Test extends BaseSubscriptionsR5Test { Subscription subscription2 = createSubscription(criteria2, payload); waitForActivatedSubscriptionCount(2); - ourLog.info("** About to send obervation"); + ourLog.info("** About to send observation"); Observation observation1 = sendObservation(code, "SNOMED-CT"); // Should see 1 subscription notification @@ -668,7 +668,7 @@ public class RestHookTestR5Test extends BaseSubscriptionsR5Test { createSubscription(criteria1, payload); waitForActivatedSubscriptionCount(1); - ourLog.info("** About to send obervation"); + ourLog.info("** About to send observation"); Observation observation = new Observation(); observation.addIdentifier().setSystem("foo").setValue("bar1"); @@ -690,7 +690,7 @@ public class RestHookTestR5Test extends BaseSubscriptionsR5Test { .setResource(observation) .setFullUrl(observation.getId()) .getRequest() - .setUrl("Obervation?identifier=foo|bar1") + .setUrl("Observation?identifier=foo|bar1") .setMethod(Bundle.HTTPVerb.PUT); requestBundle.addEntry() .setResource(patient) diff --git a/hapi-fhir-jpaserver-searchparam/src/main/java/ca/uhn/fhir/jpa/searchparam/matcher/InMemoryResourceMatcher.java b/hapi-fhir-jpaserver-searchparam/src/main/java/ca/uhn/fhir/jpa/searchparam/matcher/InMemoryResourceMatcher.java index 0978dd1f355..f6d0027898f 100644 --- a/hapi-fhir-jpaserver-searchparam/src/main/java/ca/uhn/fhir/jpa/searchparam/matcher/InMemoryResourceMatcher.java +++ b/hapi-fhir-jpaserver-searchparam/src/main/java/ca/uhn/fhir/jpa/searchparam/matcher/InMemoryResourceMatcher.java @@ -39,6 +39,7 @@ import ca.uhn.fhir.rest.param.StringParam; import ca.uhn.fhir.rest.server.exceptions.InvalidRequestException; import ca.uhn.fhir.util.MetaUtil; import ca.uhn.fhir.util.UrlUtil; +import org.apache.commons.lang3.Validate; import org.hl7.fhir.dstu3.model.Location; import org.hl7.fhir.instance.model.api.IAnyResource; import org.hl7.fhir.instance.model.api.IBaseResource; @@ -49,6 +50,7 @@ import javax.annotation.Nonnull; import java.util.List; import java.util.Map; import java.util.Optional; +import java.util.regex.Pattern; public class InMemoryResourceMatcher { @@ -73,6 +75,8 @@ public class InMemoryResourceMatcher { public InMemoryMatchResult match(String theCriteria, IBaseResource theResource, ResourceIndexedSearchParams theSearchParams) { RuntimeResourceDefinition resourceDefinition; if (theResource == null) { + Validate.isTrue(!theCriteria.startsWith("?"), "Invalid match URL format (must match \"[resourceType]?[params]\")"); + Validate.isTrue(theCriteria.contains("?"), "Invalid match URL format (must match \"[resourceType]?[params]\")"); resourceDefinition = UrlUtil.parseUrlResourceType(myFhirContext, theCriteria); } else { resourceDefinition = myFhirContext.getResourceDefinition(theResource); diff --git a/hapi-fhir-structures-r4/src/test/resources/rdf-test-input/diagnosticreport-hla-genetics-results-example.json b/hapi-fhir-structures-r4/src/test/resources/rdf-test-input/diagnosticreport-hla-genetics-results-example.json index 71086433729..9390c992cd4 100644 --- a/hapi-fhir-structures-r4/src/test/resources/rdf-test-input/diagnosticreport-hla-genetics-results-example.json +++ b/hapi-fhir-structures-r4/src/test/resources/rdf-test-input/diagnosticreport-hla-genetics-results-example.json @@ -9,7 +9,7 @@ "resourceType": "DiagnosticReport", "text": { "status": "generated", - "div": "\u003cdiv xmlns\u003d\"http://www.w3.org/1999/xhtml\"\u003e\n\t\t\t\t\t\t\u003ch3\u003eHLA-A,-B,-C genotyping report for Mary Chalmers (MRN:12345)\u003c/h3\u003e\n\t\t\t\t\t\t\u003cpre\u003e\n LOCUS ALLELE 1 ALLELE 2\n HLA-A HLA-A:01:01G HLA-A*01:02\n HLA-B HLA-B*15:01:01G HLA-B*57:01:01G\n HLA-C HLA-C*01:02:01G HLA-C*03:04:01G\n \u003c/pre\u003e\n\t\t\t\t\t\t\u003cp\u003eAllele assignments based on IMGT/HLA 3.23\u003c/p\u003e\n\t\t\t\t\t\t\u003cp\u003eEffective date: 2015-12-15\u003c/p\u003e\n\t\t\t\t\t\t\u003cp\u003eMethod: Sequencing of exons 2 and 3 of HLA Class I genes\u003c/p\u003e\n\t\t\t\t\t\t\u003cp\u003eLab: aTypingLab Inc\u003c/p\u003e\n\t\t\t\t\t\t\u003cp\u003eNote: Please refer the \u003ca href\u003d\"genomics.html#hla\"\u003eimplementation guide \u003c/a\u003e for more explanation on this\n carefully organized bundle example.\u003c/p\u003e\n\t\t\t\t\t\t\u003cpre\u003e\n Bob Milius - NMDP - 2016-12-01\n\n Transaction bundle that creates and links:\n + DiagnosticReport summarizing genotyping for HLA-A,-B,-C typing of patient(donor)\n + Obervations for each genotype\n + Observations for each allele\n + Sequences for exons 2 and 3 for HLA-A,-B, -C\n\n The endpoints of the following resources are hardcoded into this transaction bundle\n because these are presumed to already exist when developing the DiagnosticRequest\n which was to generate this report bundle:\n\n Patient/119 (potential donor)\n Specimen/120 (buccal swab)\n Organization/68 (typing lab)\n ServiceRequest/123 (report is based on this request)\n \u003c/pre\u003e\n\t\t\t\t\t\u003c/div\u003e" + "div": "\u003cdiv xmlns\u003d\"http://www.w3.org/1999/xhtml\"\u003e\n\t\t\t\t\t\t\u003ch3\u003eHLA-A,-B,-C genotyping report for Mary Chalmers (MRN:12345)\u003c/h3\u003e\n\t\t\t\t\t\t\u003cpre\u003e\n LOCUS ALLELE 1 ALLELE 2\n HLA-A HLA-A:01:01G HLA-A*01:02\n HLA-B HLA-B*15:01:01G HLA-B*57:01:01G\n HLA-C HLA-C*01:02:01G HLA-C*03:04:01G\n \u003c/pre\u003e\n\t\t\t\t\t\t\u003cp\u003eAllele assignments based on IMGT/HLA 3.23\u003c/p\u003e\n\t\t\t\t\t\t\u003cp\u003eEffective date: 2015-12-15\u003c/p\u003e\n\t\t\t\t\t\t\u003cp\u003eMethod: Sequencing of exons 2 and 3 of HLA Class I genes\u003c/p\u003e\n\t\t\t\t\t\t\u003cp\u003eLab: aTypingLab Inc\u003c/p\u003e\n\t\t\t\t\t\t\u003cp\u003eNote: Please refer the \u003ca href\u003d\"genomics.html#hla\"\u003eimplementation guide \u003c/a\u003e for more explanation on this\n carefully organized bundle example.\u003c/p\u003e\n\t\t\t\t\t\t\u003cpre\u003e\n Bob Milius - NMDP - 2016-12-01\n\n Transaction bundle that creates and links:\n + DiagnosticReport summarizing genotyping for HLA-A,-B,-C typing of patient(donor)\n + Observations for each genotype\n + Observations for each allele\n + Sequences for exons 2 and 3 for HLA-A,-B, -C\n\n The endpoints of the following resources are hardcoded into this transaction bundle\n because these are presumed to already exist when developing the DiagnosticRequest\n which was to generate this report bundle:\n\n Patient/119 (potential donor)\n Specimen/120 (buccal swab)\n Organization/68 (typing lab)\n ServiceRequest/123 (report is based on this request)\n \u003c/pre\u003e\n\t\t\t\t\t\u003c/div\u003e" }, "extension": [ { @@ -1213,4 +1213,4 @@ } ] } -} \ No newline at end of file +}