diff --git a/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/5_6_0/2987-validate-conditional-urls.yaml b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/5_6_0/2987-validate-conditional-urls.yaml new file mode 100644 index 00000000000..ce1048e8ece --- /dev/null +++ b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/5_6_0/2987-validate-conditional-urls.yaml @@ -0,0 +1,5 @@ +--- +type: change +jira: SMILE-2927 +title: "During transactions, any resources that were PUT or POSTed with a conditional URL now receive extra validation. There is now a final +storage step which ensures that the stored resource actually matches the conditional URL." diff --git a/hapi-fhir-jpaserver-api/src/main/java/ca/uhn/fhir/jpa/api/model/LazyDaoMethodOutcome.java b/hapi-fhir-jpaserver-api/src/main/java/ca/uhn/fhir/jpa/api/model/LazyDaoMethodOutcome.java index 0ed9808a84e..07caf13ad19 100644 --- a/hapi-fhir-jpaserver-api/src/main/java/ca/uhn/fhir/jpa/api/model/LazyDaoMethodOutcome.java +++ b/hapi-fhir-jpaserver-api/src/main/java/ca/uhn/fhir/jpa/api/model/LazyDaoMethodOutcome.java @@ -52,13 +52,11 @@ public class LazyDaoMethodOutcome extends DaoMethodOutcome { private void tryToRunSupplier() { if (myEntitySupplier != null) { - EntityAndResource entityAndResource = myEntitySupplier.get(); setEntity(entityAndResource.getEntity()); setResource(entityAndResource.getResource()); setId(entityAndResource.getResource().getIdElement()); myEntitySupplierUseCallback.run(); - } } diff --git a/hapi-fhir-jpaserver-base/pom.xml b/hapi-fhir-jpaserver-base/pom.xml index 1c6c860a492..428612b17c4 100644 --- a/hapi-fhir-jpaserver-base/pom.xml +++ b/hapi-fhir-jpaserver-base/pom.xml @@ -815,7 +815,7 @@ - + ${project.basedir}/src/main/resources 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 d40d555a8de..a5830be5107 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 @@ -35,6 +35,7 @@ import ca.uhn.fhir.jpa.api.model.DaoMethodOutcome; import ca.uhn.fhir.jpa.api.model.DeleteConflict; import ca.uhn.fhir.jpa.api.model.DeleteConflictList; import ca.uhn.fhir.jpa.api.model.DeleteMethodOutcome; +import ca.uhn.fhir.jpa.api.model.LazyDaoMethodOutcome; import ca.uhn.fhir.jpa.cache.IResourceVersionSvc; import ca.uhn.fhir.jpa.cache.ResourcePersistentIdMap; import ca.uhn.fhir.jpa.dao.tx.HapiTransactionService; @@ -44,7 +45,9 @@ 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.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.searchparam.matcher.SearchParamMatcher; import ca.uhn.fhir.model.api.IResource; import ca.uhn.fhir.model.api.ResourceMetadataKeyEnum; import ca.uhn.fhir.parser.DataFormatException; @@ -64,6 +67,7 @@ import ca.uhn.fhir.rest.server.exceptions.InvalidRequestException; import ca.uhn.fhir.rest.server.exceptions.MethodNotAllowedException; import ca.uhn.fhir.rest.server.exceptions.NotModifiedException; import ca.uhn.fhir.rest.server.exceptions.PayloadTooLargeException; +import ca.uhn.fhir.rest.server.exceptions.PreconditionFailedException; import ca.uhn.fhir.rest.server.interceptor.IServerInterceptor; import ca.uhn.fhir.rest.server.method.BaseMethodBinding; import ca.uhn.fhir.rest.server.method.BaseResourceReturningMethodBinding; @@ -80,6 +84,7 @@ import ca.uhn.fhir.util.UrlUtil; import com.google.common.annotations.VisibleForTesting; import com.google.common.collect.ArrayListMultimap; import com.google.common.collect.ListMultimap; +import org.apache.commons.lang3.StringUtils; import org.apache.commons.lang3.Validate; import org.hl7.fhir.dstu3.model.Bundle; import org.hl7.fhir.exceptions.FHIRException; @@ -156,6 +161,8 @@ public abstract class BaseTransactionProcessor { private ModelConfig myModelConfig; @Autowired private InMemoryResourceMatcher myInMemoryResourceMatcher; + @Autowired + private SearchParamMatcher mySearchParamMatcher; private TaskExecutor myExecutor ; @@ -274,7 +281,9 @@ public abstract class BaseTransactionProcessor { idSubstitutions.put(id, newId); } } - idToPersistedOutcome.put(newId, outcome); + + populateIdToPersistedOutcomeMap(idToPersistedOutcome, newId, outcome); + if (outcome.getCreated()) { myVersionAdapter.setResponseStatus(newEntry, toStatusString(Constants.STATUS_HTTP_201_CREATED)); } else { @@ -298,6 +307,21 @@ public abstract class BaseTransactionProcessor { } + /** Method which populates entry in idToPersistedOutcome. + * Will store whatever outcome is sent, unless the key already exists, then we only replace an instance if we find that the instance + * we are replacing with is non-lazy. This allows us to evaluate later more easily, as we _know_ we need access to these. + */ + private void populateIdToPersistedOutcomeMap(Map idToPersistedOutcome, IIdType newId, DaoMethodOutcome outcome) { + //Prefer real method outcomes over lazy ones. + if (idToPersistedOutcome.containsKey(newId)) { + if (!(outcome instanceof LazyDaoMethodOutcome)) { + idToPersistedOutcome.put(newId, outcome); + } + } else { + idToPersistedOutcome.put(newId, outcome); + } + } + private Date getLastModified(IBaseResource theRes) { return theRes.getMeta().getLastUpdated(); } @@ -481,7 +505,7 @@ public abstract class BaseTransactionProcessor { entries.sort(new TransactionSorter(placeholderIds)); // perform all writes - doTransactionWriteOperations(theRequestDetails, theActionName, + prepareThenExecuteTransactionWriteOperations(theRequestDetails, theActionName, transactionDetails, transactionStopWatch, response, originalRequestOrder, entries); @@ -584,38 +608,15 @@ public abstract class BaseTransactionProcessor { * heavy load with lots of concurrent transactions using all available * database connections. */ - private void doTransactionWriteOperations(RequestDetails theRequestDetails, String theActionName, - TransactionDetails theTransactionDetails, StopWatch theTransactionStopWatch, - IBaseBundle theResponse, IdentityHashMap theOriginalRequestOrder, - List theEntries) { + private void prepareThenExecuteTransactionWriteOperations(RequestDetails theRequestDetails, String theActionName, + TransactionDetails theTransactionDetails, StopWatch theTransactionStopWatch, + IBaseBundle theResponse, IdentityHashMap theOriginalRequestOrder, + List theEntries) { + TransactionWriteOperationsDetails writeOperationsDetails = null; - if (CompositeInterceptorBroadcaster.hasHooks(Pointcut.STORAGE_TRANSACTION_WRITE_OPERATIONS_PRE, myInterceptorBroadcaster, theRequestDetails) || - CompositeInterceptorBroadcaster.hasHooks(Pointcut.STORAGE_TRANSACTION_WRITE_OPERATIONS_POST, myInterceptorBroadcaster, theRequestDetails)) { - - List updateRequestUrls = new ArrayList<>(); - List conditionalCreateRequestUrls = new ArrayList<>(); - for (IBase nextEntry : theEntries) { - String method = myVersionAdapter.getEntryRequestVerb(myContext, nextEntry); - if ("PUT".equals(method)) { - String requestUrl = myVersionAdapter.getEntryRequestUrl(nextEntry); - if (isNotBlank(requestUrl)) { - updateRequestUrls.add(requestUrl); - } - } else if ("POST".equals(method)) { - String requestUrl = myVersionAdapter.getEntryRequestIfNoneExist(nextEntry); - if (isNotBlank(requestUrl) && requestUrl.contains("?")) { - conditionalCreateRequestUrls.add(requestUrl); - } - } - } - - writeOperationsDetails = new TransactionWriteOperationsDetails(); - writeOperationsDetails.setUpdateRequestUrls(updateRequestUrls); - writeOperationsDetails.setConditionalCreateRequestUrls(conditionalCreateRequestUrls); - HookParams params = new HookParams() - .add(TransactionDetails.class, theTransactionDetails) - .add(TransactionWriteOperationsDetails.class, writeOperationsDetails); - CompositeInterceptorBroadcaster.doCallHooks(myInterceptorBroadcaster, theRequestDetails, Pointcut.STORAGE_TRANSACTION_WRITE_OPERATIONS_PRE, params); + if (haveWriteOperationsHooks(theRequestDetails)) { + writeOperationsDetails = buildWriteOperationsDetails(theEntries); + callWriteOperationsHook(Pointcut.STORAGE_TRANSACTION_WRITE_OPERATIONS_PRE, theRequestDetails, theTransactionDetails, writeOperationsDetails); } TransactionCallback> txCallback = status -> { @@ -636,13 +637,9 @@ public abstract class BaseTransactionProcessor { try { entriesToProcess = myHapiTransactionService.execute(theRequestDetails, theTransactionDetails, txCallback); - } - finally { - if (writeOperationsDetails != null) { - HookParams params = new HookParams() - .add(TransactionDetails.class, theTransactionDetails) - .add(TransactionWriteOperationsDetails.class, writeOperationsDetails); - CompositeInterceptorBroadcaster.doCallHooks(myInterceptorBroadcaster, theRequestDetails, Pointcut.STORAGE_TRANSACTION_WRITE_OPERATIONS_POST, params); + } finally { + if (haveWriteOperationsHooks(theRequestDetails)) { + callWriteOperationsHook(Pointcut.STORAGE_TRANSACTION_WRITE_OPERATIONS_POST, theRequestDetails, theTransactionDetails, writeOperationsDetails); } } @@ -656,6 +653,45 @@ public abstract class BaseTransactionProcessor { } } + private boolean haveWriteOperationsHooks(RequestDetails theRequestDetails) { + return CompositeInterceptorBroadcaster.hasHooks(Pointcut.STORAGE_TRANSACTION_WRITE_OPERATIONS_PRE, myInterceptorBroadcaster, theRequestDetails) || + CompositeInterceptorBroadcaster.hasHooks(Pointcut.STORAGE_TRANSACTION_WRITE_OPERATIONS_POST, myInterceptorBroadcaster, theRequestDetails); + } + + private void callWriteOperationsHook(Pointcut thePointcut, RequestDetails theRequestDetails, TransactionDetails theTransactionDetails, TransactionWriteOperationsDetails theWriteOperationsDetails) { + HookParams params = new HookParams() + .add(TransactionDetails.class, theTransactionDetails) + .add(TransactionWriteOperationsDetails.class, theWriteOperationsDetails); + CompositeInterceptorBroadcaster.doCallHooks(myInterceptorBroadcaster, theRequestDetails, thePointcut, params); + } + + + private TransactionWriteOperationsDetails buildWriteOperationsDetails(List theEntries) { + TransactionWriteOperationsDetails writeOperationsDetails; + List updateRequestUrls = new ArrayList<>(); + List conditionalCreateRequestUrls = new ArrayList<>(); + //Extract + for (IBase nextEntry : theEntries) { + String method = myVersionAdapter.getEntryRequestVerb(myContext, nextEntry); + if ("PUT".equals(method)) { + String requestUrl = myVersionAdapter.getEntryRequestUrl(nextEntry); + if (isNotBlank(requestUrl)) { + updateRequestUrls.add(requestUrl); + } + } else if ("POST".equals(method)) { + String requestUrl = myVersionAdapter.getEntryRequestIfNoneExist(nextEntry); + if (isNotBlank(requestUrl) && requestUrl.contains("?")) { + conditionalCreateRequestUrls.add(requestUrl); + } + } + } + + writeOperationsDetails = new TransactionWriteOperationsDetails(); + writeOperationsDetails.setUpdateRequestUrls(updateRequestUrls); + writeOperationsDetails.setConditionalCreateRequestUrls(conditionalCreateRequestUrls); + return writeOperationsDetails; + } + private boolean isValidVerb(String theVerb) { try { return org.hl7.fhir.r4.model.Bundle.HTTPVerb.fromCode(theVerb) != null; @@ -704,7 +740,7 @@ public abstract class BaseTransactionProcessor { IBaseResource resource = myVersionAdapter.getResource(nextReqEntry); if (resource != null) { String verb = myVersionAdapter.getEntryRequestVerb(myContext, nextReqEntry); - String entryUrl = myVersionAdapter.getFullUrl(nextReqEntry); + String entryFullUrl = myVersionAdapter.getFullUrl(nextReqEntry); String requestUrl = myVersionAdapter.getEntryRequestUrl(nextReqEntry); String ifNoneExist = myVersionAdapter.getEntryRequestIfNoneExist(nextReqEntry); String key = verb + "|" + requestUrl + "|" + ifNoneExist; @@ -712,7 +748,7 @@ public abstract class BaseTransactionProcessor { // Conditional UPDATE boolean consolidateEntry = false; if ("PUT".equals(verb)) { - if (isNotBlank(entryUrl) && isNotBlank(requestUrl)) { + if (isNotBlank(entryFullUrl) && isNotBlank(requestUrl)) { int questionMarkIndex = requestUrl.indexOf('?'); if (questionMarkIndex >= 0 && requestUrl.length() > (questionMarkIndex + 1)) { consolidateEntry = true; @@ -722,8 +758,8 @@ public abstract class BaseTransactionProcessor { // Conditional CREATE if ("POST".equals(verb)) { - if (isNotBlank(entryUrl) && isNotBlank(requestUrl) && isNotBlank(ifNoneExist)) { - if (!entryUrl.equals(requestUrl)) { + if (isNotBlank(entryFullUrl) && isNotBlank(requestUrl) && isNotBlank(ifNoneExist)) { + if (!entryFullUrl.equals(requestUrl)) { consolidateEntry = true; } } @@ -731,33 +767,41 @@ public abstract class BaseTransactionProcessor { if (consolidateEntry) { if (!keyToUuid.containsKey(key)) { - keyToUuid.put(key, entryUrl); + keyToUuid.put(key, entryFullUrl); } else { ourLog.info("Discarding transaction bundle entry {} as it contained a duplicate conditional {}", originalIndex, verb); theEntries.remove(index); index--; String existingUuid = keyToUuid.get(key); - for (IBase nextEntry : theEntries) { - IBaseResource nextResource = myVersionAdapter.getResource(nextEntry); - for (IBaseReference nextReference : myContext.newTerser().getAllPopulatedChildElementsOfType(nextResource, IBaseReference.class)) { - // We're interested in any references directly to the placeholder ID, but also - // references that have a resource target that has the placeholder ID. - String nextReferenceId = nextReference.getReferenceElement().getValue(); - if (isBlank(nextReferenceId) && nextReference.getResource() != null) { - nextReferenceId = nextReference.getResource().getIdElement().getValue(); - } - if (entryUrl.equals(nextReferenceId)) { - nextReference.setReference(existingUuid); - nextReference.setResource(null); - } - } - } + replaceReferencesInEntriesWithConsolidatedUUID(theEntries, entryFullUrl, existingUuid); } } } } } + /** + * Iterates over all entries, and if it finds any which have references which match the fullUrl of the entry that was consolidated out + * replace them with our new consolidated UUID + */ + private void replaceReferencesInEntriesWithConsolidatedUUID(List theEntries, String theEntryFullUrl, String existingUuid) { + for (IBase nextEntry : theEntries) { + IBaseResource nextResource = myVersionAdapter.getResource(nextEntry); + for (IBaseReference nextReference : myContext.newTerser().getAllPopulatedChildElementsOfType(nextResource, IBaseReference.class)) { + // We're interested in any references directly to the placeholder ID, but also + // references that have a resource target that has the placeholder ID. + String nextReferenceId = nextReference.getReferenceElement().getValue(); + if (isBlank(nextReferenceId) && nextReference.getResource() != null) { + nextReferenceId = nextReference.getResource().getIdElement().getValue(); + } + if (theEntryFullUrl.equals(nextReferenceId)) { + nextReference.setReference(existingUuid); + nextReference.setResource(null); + } + } + } + } + /** * Retrieves the next resource id (IIdType) from the base resource and next request entry. * @param theBaseResource - base resource @@ -810,12 +854,16 @@ public abstract class BaseTransactionProcessor { return nextResourceId; } + /** After pre-hooks have been called + * + */ protected Map doTransactionWriteOperations(final RequestDetails theRequest, String theActionName, TransactionDetails theTransactionDetails, Set theAllIds, Map theIdSubstitutions, Map theIdToPersistedOutcome, IBaseBundle theResponse, IdentityHashMap theOriginalRequestOrder, List theEntries, StopWatch theTransactionStopWatch) { + // During a transaction, we don't execute hooks, instead, we execute them all post-transaction. theTransactionDetails.beginAcceptingDeferredInterceptorBroadcasts( Pointcut.STORAGE_PRECOMMIT_RESOURCE_CREATED, Pointcut.STORAGE_PRECOMMIT_RESOURCE_UPDATED, @@ -827,6 +875,7 @@ public abstract class BaseTransactionProcessor { Map entriesToProcess = new IdentityHashMap<>(); Set nonUpdatedEntities = new HashSet<>(); Set updatedEntities = new HashSet<>(); + Map conditionalUrlToIdMap = new HashMap<>(); List updatedResources = new ArrayList<>(); Map> conditionalRequestUrls = new HashMap<>(); @@ -866,7 +915,8 @@ public abstract class BaseTransactionProcessor { String matchUrl = myVersionAdapter.getEntryRequestIfNoneExist(nextReqEntry); matchUrl = performIdSubstitutionsInMatchUrl(theIdSubstitutions, matchUrl); outcome = resourceDao.create(res, matchUrl, false, theTransactionDetails, theRequest); - res.setId(outcome.getId()); + setConditionalUrlToBeValidatedLater(conditionalUrlToIdMap, matchUrl, outcome.getId()); + res.setId(outcome.getId()); if (nextResourceId != null) { handleTransactionCreateOrUpdateOutcome(theIdSubstitutions, theIdToPersistedOutcome, nextResourceId, outcome, nextRespEntry, resourceType, res, theRequest); } @@ -900,6 +950,7 @@ public abstract class BaseTransactionProcessor { String matchUrl = parts.getResourceType() + '?' + parts.getParams(); matchUrl = performIdSubstitutionsInMatchUrl(theIdSubstitutions, matchUrl); DeleteMethodOutcome deleteOutcome = dao.deleteByUrl(matchUrl, deleteConflicts, theRequest); + setConditionalUrlToBeValidatedLater(conditionalUrlToIdMap, matchUrl, deleteOutcome.getId()); List allDeleted = deleteOutcome.getDeletedEntities(); for (ResourceTable deleted : allDeleted) { deletedResources.add(deleted.getIdDt().toUnqualifiedVersionless().getValueAsString()); @@ -942,6 +993,7 @@ public abstract class BaseTransactionProcessor { } matchUrl = performIdSubstitutionsInMatchUrl(theIdSubstitutions, matchUrl); outcome = resourceDao.update(res, matchUrl, false, false, theRequest, theTransactionDetails); + setConditionalUrlToBeValidatedLater(conditionalUrlToIdMap, matchUrl, outcome.getId()); if (Boolean.TRUE.equals(outcome.getCreated())) { conditionalRequestUrls.put(matchUrl, res.getClass()); } @@ -1000,6 +1052,7 @@ public abstract class BaseTransactionProcessor { IFhirResourceDao dao = toDao(parts, verb, url); IIdType patchId = myContext.getVersion().newIdType().setValue(parts.getResourceId()); DaoMethodOutcome outcome = dao.patch(patchId, matchUrl, patchType, patchBody, patchBodyParameters, theRequest); + setConditionalUrlToBeValidatedLater(conditionalUrlToIdMap, matchUrl, outcome.getId()); updatedEntities.add(outcome.getEntity()); if (outcome.getResource() != null) { updatedResources.add(outcome.getResource()); @@ -1056,6 +1109,15 @@ public abstract class BaseTransactionProcessor { if (!myDaoConfig.isMassIngestionMode()) { validateNoDuplicates(theRequest, theActionName, conditionalRequestUrls, theIdToPersistedOutcome.values()); } + + theTransactionStopWatch.endCurrentTask(); + if (conditionalUrlToIdMap.size() > 0) { + theTransactionStopWatch.startTask("Check that all conditionally created/updated entities actually match their conditionals."); + } + + if (!myDaoConfig.isMassIngestionMode()) { + validateAllInsertsMatchTheirConditionalUrls(theIdToPersistedOutcome, conditionalUrlToIdMap, theRequest); + } theTransactionStopWatch.endCurrentTask(); for (IIdType next : theAllIds) { @@ -1094,6 +1156,39 @@ public abstract class BaseTransactionProcessor { } } + private void setConditionalUrlToBeValidatedLater(Map theConditionalUrlToIdMap, String theMatchUrl, IIdType theId) { + if (!StringUtils.isBlank(theMatchUrl)) { + theConditionalUrlToIdMap.put(theMatchUrl, theId); + } + } + + /** + * After transaction processing and resolution of indexes and references, we want to validate that the resources that were stored _actually_ + * match the conditional URLs that they were brought in on. + * @param theIdToPersistedOutcome + * @param conditionalUrlToIdMap + */ + private void validateAllInsertsMatchTheirConditionalUrls(Map theIdToPersistedOutcome, Map conditionalUrlToIdMap, RequestDetails theRequest) { + conditionalUrlToIdMap.entrySet().stream() + .filter(entry -> entry.getKey() != null) + .forEach(entry -> { + String matchUrl = entry.getKey(); + IIdType value = entry.getValue(); + DaoMethodOutcome daoMethodOutcome = theIdToPersistedOutcome.get(value); + if (daoMethodOutcome != null && !daoMethodOutcome.isNop() && daoMethodOutcome.getResource() != null) { + InMemoryMatchResult match = mySearchParamMatcher.match(matchUrl, daoMethodOutcome.getResource(), theRequest); + if (ourLog.isDebugEnabled()) { + ourLog.debug("Checking conditional URL [{}] against resource with ID [{}]: Supported?:[{}], Matched?:[{}]", matchUrl, value, match.supported(), match.matched()); + } + if (match.supported()) { + if (!match.matched()) { + throw new PreconditionFailedException("Invalid conditional URL \"" + matchUrl + "\". The given resource is not matched by this URL."); + }; + } + } + }); + } + /** * Checks for any delete conflicts. * @param theDeleteConflicts - set of delete conflicts @@ -1384,7 +1479,7 @@ public abstract class BaseTransactionProcessor { thePersistedOutcomes .stream() .filter(t -> !t.isNop()) - .filter(t -> t.getEntity() instanceof ResourceTable) + .filter(t -> t.getEntity() instanceof ResourceTable)//N.B. GGG: This validation never occurs for mongo, as nothing is a ResourceTable. .filter(t -> t.getEntity().getDeleted() == null) .filter(t -> t.getResource() != null) .forEach(t -> resourceToIndexedParams.put(t.getResource(), new ResourceIndexedSearchParams((ResourceTable) t.getEntity()))); 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 f48b4ede91f..02a797f60f6 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 @@ -13,6 +13,7 @@ import ca.uhn.fhir.jpa.model.entity.ModelConfig; import ca.uhn.fhir.jpa.partition.IRequestPartitionHelperSvc; import ca.uhn.fhir.jpa.searchparam.MatchUrlService; import ca.uhn.fhir.jpa.searchparam.matcher.InMemoryResourceMatcher; +import ca.uhn.fhir.jpa.searchparam.matcher.SearchParamMatcher; import ca.uhn.fhir.rest.server.exceptions.InvalidRequestException; import org.hibernate.Session; import org.hibernate.internal.SessionImpl; @@ -72,6 +73,8 @@ public class TransactionProcessorTest { private IRequestPartitionHelperSvc myRequestPartitionHelperSvc; @MockBean private IResourceVersionSvc myResourceVersionSvc; + @MockBean + private SearchParamMatcher mySearchParamMatcher; @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/FhirResourceDaoR4VersionedReferenceTest.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4VersionedReferenceTest.java index 0c85f54f0a0..0975df2548e 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4VersionedReferenceTest.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4VersionedReferenceTest.java @@ -11,6 +11,7 @@ import ca.uhn.fhir.rest.param.TokenParam; import ca.uhn.fhir.util.BundleBuilder; import org.hl7.fhir.instance.model.api.IBaseResource; import org.hl7.fhir.instance.model.api.IIdType; +import org.hl7.fhir.r4.model.BooleanType; import org.hl7.fhir.r4.model.Bundle; import org.hl7.fhir.r4.model.Condition; import org.hl7.fhir.r4.model.Encounter; @@ -467,7 +468,8 @@ public class FhirResourceDaoR4VersionedReferenceTest extends BaseJpaR4Test { Patient patient = new Patient(); patient.setId(IdType.newRandomUuid()); - patient.setActive(true); + patient.setDeceased(new BooleanType(true)); + patient.setActive(false); builder .addTransactionUpdateEntry(patient) .conditional("Patient?active=false"); 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 88a0987be86..fd513807f31 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 @@ -51,7 +51,9 @@ import org.hl7.fhir.r4.model.DateTimeType; import org.hl7.fhir.r4.model.DiagnosticReport; import org.hl7.fhir.r4.model.Encounter; import org.hl7.fhir.r4.model.EpisodeOfCare; +import org.hl7.fhir.r4.model.HumanName; import org.hl7.fhir.r4.model.IdType; +import org.hl7.fhir.r4.model.Identifier; import org.hl7.fhir.r4.model.Medication; import org.hl7.fhir.r4.model.MedicationRequest; import org.hl7.fhir.r4.model.Meta; @@ -83,6 +85,7 @@ import java.io.InputStream; import java.io.UnsupportedEncodingException; import java.math.BigDecimal; import java.nio.charset.StandardCharsets; +import java.util.Arrays; import java.util.HashSet; import java.util.List; import java.util.Map; @@ -96,7 +99,9 @@ import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.empty; import static org.hamcrest.Matchers.emptyString; import static org.hamcrest.Matchers.endsWith; +import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.greaterThan; +import static org.hamcrest.Matchers.is; import static org.hamcrest.Matchers.matchesPattern; import static org.hamcrest.Matchers.not; import static org.hamcrest.Matchers.startsWith; @@ -1161,6 +1166,78 @@ public class FhirSystemDaoR4Test extends BaseJpaR4SystemTest { validate(outcome); } + @Test + public void testConditionalUpdate_forObservationWithNonExistentPatientSubject_shouldCreateLinkedResources() { + Bundle transactionBundle = new Bundle().setType(BundleType.TRANSACTION); + + // Patient + HumanName patientName = new HumanName().setFamily("TEST_LAST_NAME").addGiven("TEST_FIRST_NAME"); + Identifier patientIdentifier = new Identifier().setSystem("http://example.com/mrns").setValue("U1234567890"); + Patient patient = new Patient() + .setName(Arrays.asList(patientName)) + .setIdentifier(Arrays.asList(patientIdentifier)); + patient.setId(IdType.newRandomUuid()); + + transactionBundle + .addEntry() + .setFullUrl(patient.getId()) + .setResource(patient) + .getRequest() + .setMethod(Bundle.HTTPVerb.PUT) + .setUrl("/Patient?identifier=" + patientIdentifier.getSystem() + "|" + patientIdentifier.getValue()); + + // Observation + Observation observation = new Observation(); + observation.setId(IdType.newRandomUuid()); + observation.getSubject().setReference(patient.getIdElement().toUnqualifiedVersionless().toString()); + + transactionBundle + .addEntry() + .setFullUrl(observation.getId()) + .setResource(observation) + .getRequest() + .setMethod(Bundle.HTTPVerb.PUT) + .setUrl("/Observation?subject=" + patient.getIdElement().toUnqualifiedVersionless().toString()); + + ourLog.info("Patient TEMP UUID: {}", patient.getId()); + String s = myFhirCtx.newJsonParser().setPrettyPrint(true).encodeResourceToString(transactionBundle); + System.out.println(s); + Bundle outcome= mySystemDao.transaction(null, transactionBundle); + String patientLocation = outcome.getEntry().get(0).getResponse().getLocation(); + assertThat(patientLocation, matchesPattern("Patient/[a-z0-9-]+/_history/1")); + String observationLocation = outcome.getEntry().get(1).getResponse().getLocation(); + assertThat(observationLocation, matchesPattern("Observation/[a-z0-9-]+/_history/1")); + } + + @Test + public void testConditionalUrlWhichDoesNotMatchResource() { + Bundle transactionBundle = new Bundle().setType(BundleType.TRANSACTION); + + String storedIdentifierValue = "woop"; + String conditionalUrlIdentifierValue = "zoop"; + // Patient + HumanName patientName = new HumanName().setFamily("TEST_LAST_NAME").addGiven("TEST_FIRST_NAME"); + Identifier patientIdentifier = new Identifier().setSystem("http://example.com/mrns").setValue(storedIdentifierValue); + Patient patient = new Patient() + .setName(Arrays.asList(patientName)) + .setIdentifier(Arrays.asList(patientIdentifier)); + patient.setId(IdType.newRandomUuid()); + + transactionBundle + .addEntry() + .setFullUrl(patient.getId()) + .setResource(patient) + .getRequest() + .setMethod(Bundle.HTTPVerb.PUT) + .setUrl("/Patient?identifier=" + patientIdentifier.getSystem() + "|" + conditionalUrlIdentifierValue); + + try { + mySystemDao.transaction(null, transactionBundle); + fail(); + } catch (PreconditionFailedException e) { + assertThat(e.getMessage(), is(equalTo("Invalid conditional URL \"Patient?identifier=http://example.com/mrns|" + conditionalUrlIdentifierValue +"\". The given resource is not matched by this URL."))); + } + } @Test public void testTransactionCreateInlineMatchUrlWithOneMatch() { @@ -1195,6 +1272,7 @@ public class FhirSystemDaoR4Test extends BaseJpaR4SystemTest { } + @Test public void testTransactionUpdateTwoResourcesWithSameId() { Bundle request = new Bundle(); diff --git a/hapi-fhir-jpaserver-base/src/test/resources/cdr-bundle.json b/hapi-fhir-jpaserver-base/src/test/resources/cdr-bundle.json index 2e8644593a5..9afb12a7cc8 100644 --- a/hapi-fhir-jpaserver-base/src/test/resources/cdr-bundle.json +++ b/hapi-fhir-jpaserver-base/src/test/resources/cdr-bundle.json @@ -44,7 +44,7 @@ }, "request": { "method": "PUT", - "url": "/Practitioner?identifier=http%253A%252F%252Facme.org%252Fclinicians%257C777" + "url": "/Practitioner?identifier=http%3A%2F%2Facme.org%2Fclinicians%7C777" } }, { @@ -125,7 +125,7 @@ }, "request": { "method": "PUT", - "url": "/Patient?identifier=http%253A%252F%252Facme.org%252Fmrns%257C7000135" + "url": "/Patient?identifier=http%3A%2F%2Facme.org%2Fmrns%7C7000135" } }, { @@ -203,7 +203,7 @@ }, "request": { "method": "PUT", - "url": "/Encounter?identifier=http%253A%252F%252Facme.org%252FvisitNumbers%257C4736455" + "url": "/Encounter?identifier=http%3A%2F%2Facme.org%2FvisitNumbers%7C4736455" } }, { @@ -267,7 +267,7 @@ }, "request": { "method": "PUT", - "url": "/Practitioner?identifier=http%253A%252F%252Facme.org%252Fclinicians%257C3622" + "url": "/Practitioner?identifier=http%3A%2F%2Facme.org%2Fclinicians%7C3622" } }, { @@ -312,8 +312,8 @@ }, "request": { "method": "PUT", - "url": "/Practitioner?identifier=http%253A%252F%252Facme.org%252Fclinicians%257C7452" + "url": "/Practitioner?identifier=http%3A%2F%2Facme.org%2Fclinicians%7C7452" } } ] -} \ No newline at end of file +} diff --git a/hapi-fhir-jpaserver-mdm/src/test/java/ca/uhn/fhir/jpa/mdm/BaseMdmR4Test.java b/hapi-fhir-jpaserver-mdm/src/test/java/ca/uhn/fhir/jpa/mdm/BaseMdmR4Test.java index 96115a95bf2..055f12a25b7 100644 --- a/hapi-fhir-jpaserver-mdm/src/test/java/ca/uhn/fhir/jpa/mdm/BaseMdmR4Test.java +++ b/hapi-fhir-jpaserver-mdm/src/test/java/ca/uhn/fhir/jpa/mdm/BaseMdmR4Test.java @@ -61,7 +61,6 @@ import org.hl7.fhir.r4.model.Organization; import org.hl7.fhir.r4.model.Patient; import org.hl7.fhir.r4.model.Practitioner; import org.hl7.fhir.r4.model.Reference; -import org.jetbrains.annotations.NotNull; import javax.annotation.Nullable; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; 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 86dbdd3047b..19c8cb40b60 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 @@ -156,6 +156,7 @@ public class InMemoryResourceMatcher { String resourceName = theResourceDefinition.getName(); RuntimeSearchParam paramDef = mySearchParamRegistry.getActiveSearchParam(resourceName, theParamName); InMemoryMatchResult checkUnsupportedResult = checkUnsupportedPrefixes(theParamName, paramDef, theAndOrParams); + if (!checkUnsupportedResult.supported()) { return checkUnsupportedResult; } diff --git a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/IAuthRuleBuilderRuleOp.java b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/IAuthRuleBuilderRuleOp.java index 94427a2d8fc..c21e9990842 100644 --- a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/IAuthRuleBuilderRuleOp.java +++ b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/IAuthRuleBuilderRuleOp.java @@ -53,7 +53,7 @@ public interface IAuthRuleBuilderRuleOp extends IAuthRuleBuilderAppliesToPatient/123 - Any Patient resource with the ID "123" will be matched *
  • 123 - Any resource of any type with the ID "123" will be matched
    • * - * + >* * @param theId The ID of the resource to apply (e.g. Patient/123) * @throws IllegalArgumentException If theId does not contain an ID with at least an ID part * @throws NullPointerException If theId is null