From 40ecafcd313563c68a504d36c46bff8944d236c8 Mon Sep 17 00:00:00 2001 From: James Agnew Date: Sun, 16 May 2021 21:51:23 -0400 Subject: [PATCH] Don't unescape plus in match URLs (#2658) * Don't unescape plus in match URLs * Perf improvements * Add changelog * Test fix * Work on tests * Fixes * Test fix --- .../main/java/ca/uhn/fhir/util/UrlUtil.java | 24 ++- .../java/ca/uhn/fhir/util/UrlUtilTest.java | 26 ++- .../5_5_0/2653-match-url-with-spaces.yaml | 7 + ...onditonal-create-without-actual-match.yaml | 6 + .../ca/uhn/fhir/jpa/dao/BaseHapiFhirDao.java | 28 +++- .../fhir/jpa/dao/BaseHapiFhirResourceDao.java | 11 +- .../uhn/fhir/jpa/util/MemoryCacheService.java | 13 ++ .../java/ca/uhn/fhir/jpa/dao/BaseJpaTest.java | 156 ++++++++++-------- .../jpa/dao/dstu3/FhirSystemDaoDstu3Test.java | 2 +- .../ca/uhn/fhir/jpa/dao/r4/BaseJpaR4Test.java | 4 - .../dao/r4/FhirResourceDaoR4CreateTest.java | 62 +++++++ ...rResourceDaoR4LegacySearchBuilderTest.java | 6 +- .../r4/FhirResourceDaoR4QueryCountTest.java | 78 +++++++++ .../fhir/jpa/dao/r4/FhirSystemDaoR4Test.java | 4 +- .../fhir/jpa/model/entity/ResourceTable.java | 9 + 15 files changed, 346 insertions(+), 90 deletions(-) create mode 100644 hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/5_5_0/2653-match-url-with-spaces.yaml create mode 100644 hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/5_5_0/2653-reject-conditonal-create-without-actual-match.yaml 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 c5d62978014..1a223809dba 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 @@ -9,6 +9,7 @@ import ca.uhn.fhir.rest.server.exceptions.InvalidRequestException; import com.google.common.escape.Escaper; import com.google.common.net.PercentEscaper; import org.apache.commons.lang3.StringUtils; +import org.apache.commons.text.StringSubstitutor; import org.apache.http.NameValuePair; import org.apache.http.client.utils.URLEncodedUtils; import org.apache.http.message.BasicNameValuePair; @@ -527,11 +528,24 @@ public class UrlUtil { if (questionMarkIndex != -1) { matchUrl = matchUrl.substring(questionMarkIndex + 1); } - matchUrl = matchUrl.replace("|", "%7C"); - matchUrl = matchUrl.replace("=>=", "=%3E%3D"); - matchUrl = matchUrl.replace("=<=", "=%3C%3D"); - matchUrl = matchUrl.replace("=>", "=%3E"); - matchUrl = matchUrl.replace("=<", "=%3C"); + + final String[] searchList = new String[]{ + "+", + "|", + "=>=", + "=<=", + "=>", + "=<" + }; + final String[] replacementList = new String[]{ + "%2B", + "%7C", + "=%3E%3D", + "=%3C%3D", + "=%3E", + "=%3C" + }; + matchUrl = StringUtils.replaceEach(matchUrl, searchList, replacementList); if (matchUrl.contains(" ")) { throw new InvalidRequestException("Failed to parse match URL[" + theMatchUrl + "] - URL is invalid (must not contain spaces)"); } diff --git a/hapi-fhir-base/src/test/java/ca/uhn/fhir/util/UrlUtilTest.java b/hapi-fhir-base/src/test/java/ca/uhn/fhir/util/UrlUtilTest.java index 6045cac1dbf..6c2fd7cc914 100644 --- a/hapi-fhir-base/src/test/java/ca/uhn/fhir/util/UrlUtilTest.java +++ b/hapi-fhir-base/src/test/java/ca/uhn/fhir/util/UrlUtilTest.java @@ -1,8 +1,13 @@ package ca.uhn.fhir.util; +import org.apache.http.message.BasicNameValuePair; import org.junit.jupiter.api.Test; -import static org.junit.jupiter.api.Assertions.*; +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.containsInAnyOrder; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; public class UrlUtilTest { @@ -82,4 +87,23 @@ public class UrlUtilTest { assertEquals(" ", UrlUtil.sanitizeUrlPart(" \0 ")); } + @Test + public void testTranslateMatchUrl_UrlWithSpaces() { + // Real space + assertThat(UrlUtil.translateMatchUrl("Observation?names=homer%20simpson"), + containsInAnyOrder(new BasicNameValuePair("names", "homer simpson"))); + + // Treat + as an actual + and not a space + assertThat(UrlUtil.translateMatchUrl("Observation?names=homer+simpson"), + containsInAnyOrder(new BasicNameValuePair("names", "homer+simpson"))); + + } + + @Test + public void testTranslateMatchUrl_UrlWithPipe() { + // Real space + assertThat(UrlUtil.translateMatchUrl("Observation?names=homer|simpson"), + containsInAnyOrder(new BasicNameValuePair("names", "homer|simpson"))); + } + } diff --git a/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/5_5_0/2653-match-url-with-spaces.yaml b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/5_5_0/2653-match-url-with-spaces.yaml new file mode 100644 index 00000000000..958d8289d02 --- /dev/null +++ b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/5_5_0/2653-match-url-with-spaces.yaml @@ -0,0 +1,7 @@ +--- +type: change +issue: 2653 +title: "When performing a conditional create/update/delete on a JPA server, if the match URL + contained a plus character, this character was interpreted as a space (per legacy URL encoding + rules) even though this has proven to not be the intended behaviour in real life applications. + Plus characters will now be treated literally as a plus character in these URLs." diff --git a/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/5_5_0/2653-reject-conditonal-create-without-actual-match.yaml b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/5_5_0/2653-reject-conditonal-create-without-actual-match.yaml new file mode 100644 index 00000000000..51fc0b7aadf --- /dev/null +++ b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/5_5_0/2653-reject-conditonal-create-without-actual-match.yaml @@ -0,0 +1,6 @@ +--- +type: add +issue: 2653 +title: "When performing a conditional create operation on a JPA server, the system will now + verify that the conditional URL actually matches the data supplied in the resource body, + and aborts the conditional create if it does not." diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseHapiFhirDao.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseHapiFhirDao.java index afc8bfb5cbd..a1f8a5dedf4 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseHapiFhirDao.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseHapiFhirDao.java @@ -53,6 +53,8 @@ import ca.uhn.fhir.jpa.search.PersistedJpaBundleProviderFactory; import ca.uhn.fhir.jpa.search.cache.ISearchCacheSvc; import ca.uhn.fhir.jpa.searchparam.extractor.LogicalReferenceHelper; import ca.uhn.fhir.jpa.searchparam.extractor.ResourceIndexedSearchParams; +import ca.uhn.fhir.jpa.searchparam.matcher.InMemoryMatchResult; +import ca.uhn.fhir.jpa.searchparam.matcher.InMemoryResourceMatcher; import ca.uhn.fhir.jpa.sp.ISearchParamPresenceSvc; import ca.uhn.fhir.jpa.term.api.ITermReadSvc; import ca.uhn.fhir.jpa.util.AddRemoveCount; @@ -77,6 +79,7 @@ import ca.uhn.fhir.rest.api.server.IBundleProvider; import ca.uhn.fhir.rest.api.server.RequestDetails; import ca.uhn.fhir.rest.api.server.storage.TransactionDetails; import ca.uhn.fhir.rest.server.exceptions.InternalErrorException; +import ca.uhn.fhir.rest.server.exceptions.InvalidRequestException; import ca.uhn.fhir.rest.server.exceptions.ResourceNotFoundException; import ca.uhn.fhir.rest.server.exceptions.UnprocessableEntityException; import ca.uhn.fhir.rest.server.interceptor.IServerInterceptor.ActionRequestDetails; @@ -92,7 +95,6 @@ import com.google.common.hash.HashFunction; import com.google.common.hash.Hashing; import org.apache.commons.lang3.NotImplementedException; import org.apache.commons.lang3.Validate; -import org.apache.commons.lang3.tuple.Pair; import org.hl7.fhir.instance.model.api.IAnyResource; import org.hl7.fhir.instance.model.api.IBase; import org.hl7.fhir.instance.model.api.IBaseCoding; @@ -179,10 +181,10 @@ public abstract class BaseHapiFhirDao extends BaseStora public static final String OO_SEVERITY_ERROR = "error"; public static final String OO_SEVERITY_INFO = "information"; public static final String OO_SEVERITY_WARN = "warning"; + public static final String XACT_USERDATA_KEY_RESOLVED_TAG_DEFINITIONS = BaseHapiFhirDao.class.getName() + "_RESOLVED_TAG_DEFINITIONS"; private static final Logger ourLog = LoggerFactory.getLogger(BaseHapiFhirDao.class); private static final Map ourRetrievalContexts = new HashMap<>(); private static final String PROCESSING_SUB_REQUEST = "BaseHapiFhirDao.processingSubRequest"; - public static final String XACT_USERDATA_KEY_RESOLVED_TAG_DEFINITIONS = BaseHapiFhirDao.class.getName() + "_RESOLVED_TAG_DEFINITIONS"; private static boolean ourValidationDisabledForUnitTest; private static boolean ourDisableIncrementOnUpdateForUnitTest = false; @@ -211,6 +213,8 @@ public abstract class BaseHapiFhirDao extends BaseStora @Autowired protected DaoRegistry myDaoRegistry; @Autowired + protected InMemoryResourceMatcher myInMemoryResourceMatcher; + @Autowired ExpungeService myExpungeService; @Autowired private HistoryBuilderFactory myHistoryBuilderFactory; @@ -1082,6 +1086,15 @@ public abstract class BaseHapiFhirDao extends BaseStora myDaoSearchParamSynchronizer = theDaoSearchParamSynchronizer; } + private void verifyMatchUrlForConditionalCreate(IBaseResource theResource, String theIfNoneExist, ResourceTable entity, ResourceIndexedSearchParams theParams) { + // Make sure that the match URL was actually appropriate for the supplied resource + InMemoryMatchResult outcome = myInMemoryResourceMatcher.match(theIfNoneExist, theResource, theParams); + if (outcome.supported() && !outcome.matched()) { + throw new InvalidRequestException("Failed to process conditional create. The supplied resource did not satisfy the conditional URL."); + } + } + + @SuppressWarnings("unchecked") @Override public ResourceTable updateEntity(RequestDetails theRequest, final IBaseResource theResource, IBasePersistedResource @@ -1148,6 +1161,17 @@ public abstract class BaseHapiFhirDao extends BaseStora } if (changed.isChanged()) { + + // Make sure that the match URL was actually appropriate for the supplied + // resource. We only do this for version 1 right now since technically it + // is possible (and legal) for someone to be using a conditional update + // to match a resource and then update it in a way that it no longer + // matches. We could certainly make this configurable though in the + // future. + if (entity.getVersion() <= 1L && entity.getCreatedByMatchUrl() != null) { + verifyMatchUrlForConditionalCreate(theResource, entity.getCreatedByMatchUrl(), entity, newParams); + } + entity.setUpdated(theTransactionDetails.getTransactionDate()); if (theResource instanceof IResource) { entity.setLanguage(((IResource) theResource).getLanguage().getValue()); 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 2287c17b97e..4abd48a72fe 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 @@ -56,6 +56,7 @@ import ca.uhn.fhir.jpa.search.cache.SearchCacheStatusEnum; import ca.uhn.fhir.jpa.search.reindex.IResourceReindexingSvc; import ca.uhn.fhir.jpa.searchparam.MatchUrlService; import ca.uhn.fhir.jpa.searchparam.SearchParameterMap; +import ca.uhn.fhir.jpa.searchparam.extractor.ResourceIndexedSearchParams; import ca.uhn.fhir.jpa.util.JpaInterceptorBroadcaster; import ca.uhn.fhir.jpa.util.MemoryCacheService; import ca.uhn.fhir.model.api.IQueryParameterType; @@ -257,6 +258,7 @@ public abstract class BaseHapiFhirResourceDao extends B ResourceTable entity = new ResourceTable(); entity.setResourceType(toResourceName(theResource)); entity.setPartitionId(theRequestPartitionId); + entity.setCreatedByMatchUrl(theIfNoneExist); if (isNotBlank(theIfNoneExist)) { Set match = myMatchResourceUrlService.processMatchUrl(theIfNoneExist, myResourceType, theRequest); @@ -274,7 +276,7 @@ public abstract class BaseHapiFhirResourceDao extends B }; Supplier idSupplier = () -> { - return myTxTemplate.execute(tx-> { + return myTxTemplate.execute(tx -> { IIdType retVal = myIdHelperService.translatePidIdToForcedId(myFhirContext, myResourceName, pid); if (!retVal.hasVersionIdPart()) { IIdType idWithVersion = myMemoryCacheService.getIfPresent(MemoryCacheService.CacheEnum.RESOURCE_CONDITIONAL_CREATE_VERSION, pid.getIdAsLong()); @@ -356,6 +358,7 @@ public abstract class BaseHapiFhirResourceDao extends B } if (theIfNoneExist != null) { + // Pre-cache the match URL myMatchResourceUrlService.matchUrlResolved(theIfNoneExist, new ResourcePersistentId(entity.getResourceId())); } @@ -659,7 +662,6 @@ public abstract class BaseHapiFhirResourceDao extends B } } - private void doMetaAdd(MT theMetaAdd, BaseHasResource theEntity, RequestDetails theRequestDetails, TransactionDetails theTransactionDetails) { IBaseResource oldVersion = toResource(theEntity, false); @@ -809,7 +811,6 @@ public abstract class BaseHapiFhirResourceDao extends B return myResourceName; } - @Override public Class getResourceType() { return myResourceType; @@ -1810,4 +1811,8 @@ public abstract class BaseHapiFhirResourceDao extends B } + private static ResourceIndexedSearchParams toResourceIndexedSearchParams(ResourceTable theEntity) { + return new ResourceIndexedSearchParams(theEntity); + } + } diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/util/MemoryCacheService.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/util/MemoryCacheService.java index 486cdffeee7..c1d1d8badbd 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/util/MemoryCacheService.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/util/MemoryCacheService.java @@ -68,6 +68,7 @@ public class MemoryCacheService { case CONCEPT_TRANSLATION_REVERSE: timeoutSeconds = myDaoConfig.getTranslationCachesExpireAfterWriteInMinutes() * 1000; break; + case HISTORY_COUNT: case TAG_DEFINITION: case PERSISTENT_ID: case RESOURCE_LOOKUP: @@ -103,6 +104,17 @@ public class MemoryCacheService { getCache(theCache).put(theKey, theValue); } + /** + * This method registers a transaction synchronization that puts an entry in the cache + * if and when the current database transaction successfully commits. If the + * transaction is rolled back, the key+value passed into this method will + * not be added to the cache. + * + * This is useful for situations where you want to store something that has been + * resolved in the DB during the current transaction, but it's not yet guaranteed + * that this item will successfully save to the DB. Use this method in that case + * in order to avoid cache poisoning. + */ public void putAfterCommit(CacheEnum theCache, K theKey, V theValue) { if (TransactionSynchronizationManager.isSynchronizationActive()) { TransactionSynchronizationManager.registerSynchronization(new TransactionSynchronization() { @@ -116,6 +128,7 @@ public class MemoryCacheService { } } + @SuppressWarnings("unchecked") public Map getAllPresent(CacheEnum theCache, Iterable theKeys) { return (Map) getCache(theCache).getAllPresent(theKeys); } diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/BaseJpaTest.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/BaseJpaTest.java index 83c339af0bf..3b149bfba5d 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/BaseJpaTest.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/BaseJpaTest.java @@ -11,6 +11,8 @@ import ca.uhn.fhir.jpa.api.model.ExpungeOptions; import ca.uhn.fhir.jpa.api.svc.ISearchCoordinatorSvc; import ca.uhn.fhir.jpa.bulk.export.api.IBulkDataExportSvc; import ca.uhn.fhir.jpa.config.BaseConfig; +import ca.uhn.fhir.jpa.dao.data.IResourceIndexedSearchParamTokenDao; +import ca.uhn.fhir.jpa.dao.data.IResourceLinkDao; import ca.uhn.fhir.jpa.dao.index.IdHelperService; import ca.uhn.fhir.jpa.entity.TermConcept; import ca.uhn.fhir.jpa.entity.TermValueSet; @@ -25,7 +27,6 @@ import ca.uhn.fhir.jpa.search.PersistedJpaBundleProvider; import ca.uhn.fhir.jpa.search.cache.ISearchCacheSvc; import ca.uhn.fhir.jpa.search.cache.ISearchResultCacheSvc; import ca.uhn.fhir.jpa.search.reindex.IResourceReindexingSvc; -import ca.uhn.fhir.rest.server.util.ISearchParamRegistry; import ca.uhn.fhir.jpa.subscription.match.registry.SubscriptionLoader; import ca.uhn.fhir.jpa.subscription.match.registry.SubscriptionRegistry; import ca.uhn.fhir.jpa.util.CircularQueueCaptureQueriesListener; @@ -37,6 +38,7 @@ import ca.uhn.fhir.rest.api.server.IBundleProvider; import ca.uhn.fhir.rest.server.exceptions.InternalErrorException; import ca.uhn.fhir.rest.server.exceptions.ResourceVersionConflictException; import ca.uhn.fhir.rest.server.servlet.ServletRequestDetails; +import ca.uhn.fhir.rest.server.util.ISearchParamRegistry; import ca.uhn.fhir.test.BaseTest; import ca.uhn.fhir.test.utilities.LoggingExtension; import ca.uhn.fhir.test.utilities.ProxyUtil; @@ -123,16 +125,6 @@ public abstract class BaseJpaTest extends BaseTest { TestUtil.setShouldRandomizeTimezones(false); } - public static Map buildHeapLuceneHibernateSearchProperties() { - Map props = new HashMap<>(); - props.put(BackendSettings.backendKey(BackendSettings.TYPE), "lucene"); - props.put(BackendSettings.backendKey(LuceneBackendSettings.ANALYSIS_CONFIGURER), HapiLuceneAnalysisConfigurer.class.getName()); - props.put(BackendSettings.backendKey(LuceneIndexSettings.DIRECTORY_TYPE), "local-heap"); - props.put(BackendSettings.backendKey(LuceneBackendSettings.LUCENE_VERSION), "LUCENE_CURRENT"); - props.put(HibernateOrmMapperSettings.ENABLED, "true"); - return props; - } - @RegisterExtension public LoggingExtension myLoggingExtension = new LoggingExtension(); @Mock(answer = Answers.RETURNS_DEEP_STUBS) @@ -155,6 +147,10 @@ public abstract class BaseJpaTest extends BaseTest { @Autowired protected SubscriptionLoader mySubscriptionLoader; @Autowired + protected IResourceLinkDao myResourceLinkDao; + @Autowired + protected IResourceIndexedSearchParamTokenDao myResourceIndexedSearchParamTokenDao; + @Autowired private IdHelperService myIdHelperService; @Autowired private MemoryCacheService myMemoryCacheService; @@ -237,6 +233,18 @@ public abstract class BaseJpaTest extends BaseTest { protected abstract PlatformTransactionManager getTxManager(); + protected void logAllResourceLinks() { + runInTransaction(() -> { + ourLog.info("Resource Links:\n * {}", myResourceLinkDao.findAll().stream().map(t -> t.toString()).collect(Collectors.joining("\n * "))); + }); + } + + protected void logAllTokenIndexes() { + runInTransaction(() -> { + ourLog.info("Token indexes:\n * {}", myResourceIndexedSearchParamTokenDao.findAll().stream().map(t -> t.toString()).collect(Collectors.joining("\n * "))); + }); + } + public TransactionTemplate newTxTemplate() { TransactionTemplate retVal = new TransactionTemplate(getTxManager()); retVal.setPropagationBehavior(TransactionDefinition.PROPAGATION_REQUIRES_NEW); @@ -461,6 +469,74 @@ public abstract class BaseJpaTest extends BaseTest { Thread.sleep(500); } + protected TermValueSetConceptDesignation assertTermConceptContainsDesignation(TermValueSetConcept theConcept, String theLanguage, String theUseSystem, String theUseCode, String theUseDisplay, String theDesignationValue) { + Stream stream = theConcept.getDesignations().stream(); + if (theLanguage != null) { + stream = stream.filter(designation -> theLanguage.equalsIgnoreCase(designation.getLanguage())); + } + if (theUseSystem != null) { + stream = stream.filter(designation -> theUseSystem.equalsIgnoreCase(designation.getUseSystem())); + } + if (theUseCode != null) { + stream = stream.filter(designation -> theUseCode.equalsIgnoreCase(designation.getUseCode())); + } + if (theUseDisplay != null) { + stream = stream.filter(designation -> theUseDisplay.equalsIgnoreCase(designation.getUseDisplay())); + } + if (theDesignationValue != null) { + stream = stream.filter(designation -> theDesignationValue.equalsIgnoreCase(designation.getValue())); + } + + Optional first = stream.findFirst(); + if (!first.isPresent()) { + String failureMessage = String.format("Concept %s did not contain designation [%s|%s|%s|%s|%s] ", theConcept.toString(), theLanguage, theUseSystem, theUseCode, theUseDisplay, theDesignationValue); + fail(failureMessage); + return null; + } else { + return first.get(); + } + + } + + protected TermValueSetConcept assertTermValueSetContainsConceptAndIsInDeclaredOrder(TermValueSet theValueSet, String theSystem, String theCode, String theDisplay, Integer theDesignationCount) { + List contains = theValueSet.getConcepts(); + + Stream stream = contains.stream(); + if (theSystem != null) { + stream = stream.filter(concept -> theSystem.equalsIgnoreCase(concept.getSystem())); + } + if (theCode != null) { + stream = stream.filter(concept -> theCode.equalsIgnoreCase(concept.getCode())); + } + if (theDisplay != null) { + stream = stream.filter(concept -> theDisplay.equalsIgnoreCase(concept.getDisplay())); + } + if (theDesignationCount != null) { + stream = stream.filter(concept -> concept.getDesignations().size() == theDesignationCount); + } + + Optional first = stream.findFirst(); + if (!first.isPresent()) { + String failureMessage = String.format("Expanded ValueSet %s did not contain concept [%s|%s|%s] with [%d] designations", theValueSet.getId(), theSystem, theCode, theDisplay, theDesignationCount); + fail(failureMessage); + return null; + } else { + TermValueSetConcept termValueSetConcept = first.get(); + assertEquals(termValueSetConcept.getOrder(), theValueSet.getConcepts().indexOf(termValueSetConcept)); + return termValueSetConcept; + } + } + + public static Map buildHeapLuceneHibernateSearchProperties() { + Map props = new HashMap<>(); + props.put(BackendSettings.backendKey(BackendSettings.TYPE), "lucene"); + props.put(BackendSettings.backendKey(LuceneBackendSettings.ANALYSIS_CONFIGURER), HapiLuceneAnalysisConfigurer.class.getName()); + props.put(BackendSettings.backendKey(LuceneIndexSettings.DIRECTORY_TYPE), "local-heap"); + props.put(BackendSettings.backendKey(LuceneBackendSettings.LUCENE_VERSION), "LUCENE_CURRENT"); + props.put(HibernateOrmMapperSettings.ENABLED, "true"); + return props; + } + @BeforeAll public static void beforeClassRandomizeLocale() { randomizeLocale(); @@ -602,63 +678,5 @@ public abstract class BaseJpaTest extends BaseTest { Thread.sleep(500); } - protected TermValueSetConceptDesignation assertTermConceptContainsDesignation(TermValueSetConcept theConcept, String theLanguage, String theUseSystem, String theUseCode, String theUseDisplay, String theDesignationValue) { - Stream stream = theConcept.getDesignations().stream(); - if (theLanguage != null) { - stream = stream.filter(designation -> theLanguage.equalsIgnoreCase(designation.getLanguage())); - } - if (theUseSystem != null) { - stream = stream.filter(designation -> theUseSystem.equalsIgnoreCase(designation.getUseSystem())); - } - if (theUseCode != null) { - stream = stream.filter(designation -> theUseCode.equalsIgnoreCase(designation.getUseCode())); - } - if (theUseDisplay != null) { - stream = stream.filter(designation -> theUseDisplay.equalsIgnoreCase(designation.getUseDisplay())); - } - if (theDesignationValue != null) { - stream = stream.filter(designation -> theDesignationValue.equalsIgnoreCase(designation.getValue())); - } - - Optional first = stream.findFirst(); - if (!first.isPresent()) { - String failureMessage = String.format("Concept %s did not contain designation [%s|%s|%s|%s|%s] ", theConcept.toString(), theLanguage, theUseSystem, theUseCode, theUseDisplay, theDesignationValue); - fail(failureMessage); - return null; - } else { - return first.get(); - } - - } - - protected TermValueSetConcept assertTermValueSetContainsConceptAndIsInDeclaredOrder(TermValueSet theValueSet, String theSystem, String theCode, String theDisplay, Integer theDesignationCount) { - List contains = theValueSet.getConcepts(); - - Stream stream = contains.stream(); - if (theSystem != null) { - stream = stream.filter(concept -> theSystem.equalsIgnoreCase(concept.getSystem())); - } - if (theCode != null ) { - stream = stream.filter(concept -> theCode.equalsIgnoreCase(concept.getCode())); - } - if (theDisplay != null){ - stream = stream.filter(concept -> theDisplay.equalsIgnoreCase(concept.getDisplay())); - } - if (theDesignationCount != null) { - stream = stream.filter(concept -> concept.getDesignations().size() == theDesignationCount); - } - - Optional first = stream.findFirst(); - if (!first.isPresent()) { - String failureMessage = String.format("Expanded ValueSet %s did not contain concept [%s|%s|%s] with [%d] designations", theValueSet.getId(), theSystem, theCode, theDisplay, theDesignationCount); - fail(failureMessage); - return null; - } else { - TermValueSetConcept termValueSetConcept = first.get(); - assertEquals(termValueSetConcept.getOrder(), theValueSet.getConcepts().indexOf(termValueSetConcept)); - return termValueSetConcept; - } - } - } diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/dstu3/FhirSystemDaoDstu3Test.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/dstu3/FhirSystemDaoDstu3Test.java index dab2c282abd..6778d18624a 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/dstu3/FhirSystemDaoDstu3Test.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/dstu3/FhirSystemDaoDstu3Test.java @@ -2511,7 +2511,7 @@ public class FhirSystemDaoDstu3Test extends BaseJpaDstu3SystemTest { assertEquals("Patient/P1/_history/1", new IdType(resp.getEntry().get(0).getResponse().getLocation()).toUnqualified().getValue()); p = new Patient(); - p.setActive(true); + p.setActive(false); p.addName().setFamily("AAA"); b = new Bundle(); diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/BaseJpaR4Test.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/BaseJpaR4Test.java index 5d2337f14e5..83c949b788f 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/BaseJpaR4Test.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/BaseJpaR4Test.java @@ -216,14 +216,10 @@ public abstract class BaseJpaR4Test extends BaseJpaTest implements ITestDataBuil @Qualifier("myResourceCountsCache") protected ResourceCountCache myResourceCountsCache; @Autowired - protected IResourceLinkDao myResourceLinkDao; - @Autowired protected ISearchParamPresentDao mySearchParamPresentDao; @Autowired protected IResourceIndexedSearchParamStringDao myResourceIndexedSearchParamStringDao; @Autowired - protected IResourceIndexedSearchParamTokenDao myResourceIndexedSearchParamTokenDao; - @Autowired protected IResourceIndexedSearchParamCoordsDao myResourceIndexedSearchParamCoordsDao; @Autowired protected IResourceIndexedSearchParamQuantityDao myResourceIndexedSearchParamQuantityDao; diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4CreateTest.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4CreateTest.java index 0caa0e74b05..27bb3f38b6d 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4CreateTest.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4CreateTest.java @@ -1,19 +1,24 @@ 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.model.entity.NormalizedQuantitySearchLevel; import ca.uhn.fhir.jpa.model.entity.ResourceIndexedSearchParamQuantity; import ca.uhn.fhir.jpa.model.entity.ResourceIndexedSearchParamQuantityNormalized; +import ca.uhn.fhir.jpa.model.entity.ResourceIndexedSearchParamToken; import ca.uhn.fhir.jpa.model.util.UcumServiceUtil; +import ca.uhn.fhir.jpa.partition.SystemRequestDetails; import ca.uhn.fhir.jpa.searchparam.SearchParameterMap; import ca.uhn.fhir.rest.api.server.IBundleProvider; import ca.uhn.fhir.rest.param.QuantityParam; import ca.uhn.fhir.rest.param.ReferenceParam; import ca.uhn.fhir.rest.param.StringParam; import ca.uhn.fhir.rest.param.TokenParam; +import ca.uhn.fhir.rest.server.exceptions.InvalidRequestException; import ca.uhn.fhir.rest.server.exceptions.ResourceNotFoundException; import ca.uhn.fhir.rest.server.exceptions.ResourceVersionConflictException; import ca.uhn.fhir.rest.server.exceptions.UnprocessableEntityException; +import ca.uhn.fhir.util.BundleBuilder; import org.apache.commons.lang3.time.DateUtils; import org.hl7.fhir.instance.model.api.IBaseResource; import org.hl7.fhir.instance.model.api.IIdType; @@ -53,6 +58,7 @@ import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.matchesPattern; import static org.hamcrest.Matchers.is; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertTrue; import static org.junit.jupiter.api.Assertions.fail; @@ -67,6 +73,62 @@ public class FhirResourceDaoR4CreateTest extends BaseJpaR4Test { myModelConfig.setNormalizedQuantitySearchLevel(NormalizedQuantitySearchLevel.NORMALIZED_QUANTITY_SEARCH_NOT_SUPPORTED); } + @Test + public void testConditionalCreateWithPlusInUrl() { + Observation obs = new Observation(); + obs.addIdentifier().setValue("20210427133226.444+0800"); + DaoMethodOutcome outcome = myObservationDao.create(obs, "identifier=20210427133226.444+0800", new SystemRequestDetails()); + assertTrue(outcome.getCreated()); + + logAllTokenIndexes(); + myCaptureQueriesListener.clear(); + obs = new Observation(); + obs.addIdentifier().setValue("20210427133226.444+0800"); + outcome = myObservationDao.create(obs, "identifier=20210427133226.444+0800", new SystemRequestDetails()); + myCaptureQueriesListener.logSelectQueriesForCurrentThread(); + assertFalse(outcome.getCreated()); + } + + /** + * Simulate a client error: Identifier has a "+" but URL has an escaped space character + */ + @Test + public void testConditionalCreateFailsIfMatchUrlDoesntMatch() { + Observation obs = new Observation(); + obs.addIdentifier().setValue("A+B"); + try { + myObservationDao.create(obs, "identifier=A%20B", new SystemRequestDetails()); + fail(); + } catch (InvalidRequestException e) { + assertEquals("Failed to process conditional create. The supplied resource did not satisfy the conditional URL.", e.getMessage()); + } + } + + /** + * Simulate a client error: Identifier has a "+" but URL has an escaped space character + */ + @Test + public void testConditionalCreateFailsIfMatchUrlDoesntMatch_InTransaction() { + BundleBuilder bb = new BundleBuilder(myFhirCtx); + + Patient patient = new Patient(); + patient.setId(IdType.newRandomUuid()); + patient.setActive(true); + bb.addTransactionCreateEntry(patient); + + Observation obs = new Observation(); + obs.getSubject().setReference(patient.getId()); + obs.addIdentifier().setValue("A+B"); + bb.addTransactionCreateEntry(obs).conditional("identifier=A%20B"); + + try { + mySystemDao.transaction(new SystemRequestDetails(), (Bundle) bb.getBundle()); + fail(); + } catch (InvalidRequestException e) { + assertEquals("Failed to process conditional create. The supplied resource did not satisfy the conditional URL.", e.getMessage()); + } + } + @Test public void testCreateResourceWithKoreanText() throws IOException { String input = loadClasspath("/r4/bug832-korean-text.xml"); diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4LegacySearchBuilderTest.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4LegacySearchBuilderTest.java index ef7cbf272af..2b30e6a5d09 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4LegacySearchBuilderTest.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4LegacySearchBuilderTest.java @@ -564,10 +564,8 @@ public class FhirResourceDaoR4LegacySearchBuilderTest extends BaseJpaR4Test { ma.setMedication(new Reference(medId)); IIdType moId = myMedicationAdministrationDao.create(ma).getId().toUnqualified(); - runInTransaction(() -> { - ourLog.info("Resource Links:\n * {}", myResourceLinkDao.findAll().stream().map(t -> t.toString()).collect(Collectors.joining("\n * "))); - ourLog.info("Token indexes:\n * {}", myResourceIndexedSearchParamTokenDao.findAll().stream().map(t -> t.toString()).collect(Collectors.joining("\n * "))); - }); + logAllResourceLinks(); + logAllTokenIndexes(); SearchParameterMap map = SearchParameterMap.newSynchronous(); map.add(MedicationAdministration.SP_MEDICATION, new ReferenceAndListParam().addAnd(new ReferenceOrListParam().add(new ReferenceParam("code", "04823543")))); 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 c414faf3912..94fe0c3e33f 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 @@ -797,6 +797,84 @@ public class FhirResourceDaoR4QueryCountTest extends BaseJpaR4Test { } + @Test + public void testTransactionWithCreateClientAssignedIdAndReference() { + myDaoConfig.setDeleteEnabled(false); + + Bundle input = new Bundle(); + + Patient patient = new Patient(); + patient.setId("Patient/A"); + patient.setActive(true); + input.addEntry() + .setFullUrl(patient.getId()) + .setResource(patient) + .getRequest() + .setMethod(Bundle.HTTPVerb.PUT) + .setUrl("Patient/A"); + + Observation observation = new Observation(); + observation.setId(IdType.newRandomUuid()); + observation.addReferenceRange().setText("A"); + input.addEntry() + .setFullUrl(observation.getId()) + .setResource(observation) + .getRequest() + .setMethod(Bundle.HTTPVerb.POST) + .setUrl("Observation"); + + myCaptureQueriesListener.clear(); + Bundle output = mySystemDao.transaction(mySrd, input); + ourLog.info(myFhirCtx.newJsonParser().setPrettyPrint(true).encodeResourceToString(output)); + + myCaptureQueriesListener.logSelectQueriesForCurrentThread(); + assertEquals(1, myCaptureQueriesListener.countSelectQueriesForCurrentThread()); + myCaptureQueriesListener.logInsertQueriesForCurrentThread(); + assertEquals(5, myCaptureQueriesListener.countInsertQueriesForCurrentThread()); + myCaptureQueriesListener.logUpdateQueriesForCurrentThread(); + assertEquals(2, myCaptureQueriesListener.countUpdateQueriesForCurrentThread()); + assertEquals(0, myCaptureQueriesListener.countDeleteQueriesForCurrentThread()); + + // Pass 2 + + input = new Bundle(); + + patient = new Patient(); + patient.setId("Patient/A"); + patient.setActive(true); + input.addEntry() + .setFullUrl(patient.getId()) + .setResource(patient) + .getRequest() + .setMethod(Bundle.HTTPVerb.PUT) + .setUrl("Patient/A"); + + observation = new Observation(); + observation.setId(IdType.newRandomUuid()); + observation.addReferenceRange().setText("A"); + input.addEntry() + .setFullUrl(observation.getId()) + .setResource(observation) + .getRequest() + .setMethod(Bundle.HTTPVerb.POST) + .setUrl("Observation"); + + myCaptureQueriesListener.clear(); + output = mySystemDao.transaction(mySrd, input); + ourLog.info(myFhirCtx.newJsonParser().setPrettyPrint(true).encodeResourceToString(output)); + + myCaptureQueriesListener.logSelectQueriesForCurrentThread(); + assertEquals(3, myCaptureQueriesListener.countSelectQueriesForCurrentThread()); + myCaptureQueriesListener.logInsertQueriesForCurrentThread(); + assertEquals(2, myCaptureQueriesListener.countInsertQueriesForCurrentThread()); + myCaptureQueriesListener.logUpdateQueriesForCurrentThread(); + assertEquals(1, myCaptureQueriesListener.countUpdateQueriesForCurrentThread()); + assertEquals(0, myCaptureQueriesListener.countDeleteQueriesForCurrentThread()); + + + } + + @Test public void testTransactionWithMultipleReferences() { Bundle input = new Bundle(); 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 2bbea9e63ba..7eb695c5a98 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,6 +1,7 @@ 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; @@ -101,6 +102,7 @@ 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; @@ -3385,7 +3387,7 @@ public class FhirSystemDaoR4Test extends BaseJpaR4SystemTest { assertEquals("Patient/P1/_history/1", new IdType(resp.getEntry().get(0).getResponse().getLocation()).toUnqualified().getValue()); p = new Patient(); - p.setActive(true); + p.setActive(false); p.addName().setFamily("AAA"); b = new Bundle(); diff --git a/hapi-fhir-jpaserver-model/src/main/java/ca/uhn/fhir/jpa/model/entity/ResourceTable.java b/hapi-fhir-jpaserver-model/src/main/java/ca/uhn/fhir/jpa/model/entity/ResourceTable.java index b6371a50478..d6002a726db 100644 --- a/hapi-fhir-jpaserver-model/src/main/java/ca/uhn/fhir/jpa/model/entity/ResourceTable.java +++ b/hapi-fhir-jpaserver-model/src/main/java/ca/uhn/fhir/jpa/model/entity/ResourceTable.java @@ -252,6 +252,9 @@ public class ResourceTable extends BaseHasResource implements Serializable, IBas @OneToOne(optional = true, fetch = FetchType.EAGER, cascade = {}, orphanRemoval = false, mappedBy = "myResource") private ForcedId myForcedId; + @Transient + private volatile String myCreatedByMatchUrl; + /** * Constructor */ @@ -698,5 +701,11 @@ public class ResourceTable extends BaseHasResource implements Serializable, IBas } } + public void setCreatedByMatchUrl(String theCreatedByMatchUrl) { + myCreatedByMatchUrl = theCreatedByMatchUrl; + } + public String getCreatedByMatchUrl() { + return myCreatedByMatchUrl; + } }