From b224463d35e11e498584d0f2f14e7bc9f95657ce Mon Sep 17 00:00:00 2001 From: Tadgh Date: Fri, 10 Sep 2021 22:46:35 -0400 Subject: [PATCH 01/20] add broken test --- .../fhir/jpa/dao/r4/FhirSystemDaoR4Test.java | 44 +++++++++++++++++++ 1 file changed, 44 insertions(+) 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..388d7696427 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; @@ -1161,6 +1163,48 @@ 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(List.of(patientName)) + .setIdentifier(List.of(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 testTransactionCreateInlineMatchUrlWithOneMatch() { From 94f157a4ac7b1aef76d7cc5723fc7f24274e5e28 Mon Sep 17 00:00:00 2001 From: Tadgh Date: Sat, 11 Sep 2021 15:13:05 -0400 Subject: [PATCH 02/20] Start refactoring of basetransactionprocessor --- hapi-fhir-jpaserver-base/pom.xml | 10 +- .../jpa/dao/BaseTransactionProcessor.java | 143 ++++++++++-------- .../fhir/jpa/dao/r4/FhirSystemDaoR4Test.java | 28 ++++ 3 files changed, 121 insertions(+), 60 deletions(-) diff --git a/hapi-fhir-jpaserver-base/pom.xml b/hapi-fhir-jpaserver-base/pom.xml index de3950d1860..b915ea16c06 100644 --- a/hapi-fhir-jpaserver-base/pom.xml +++ b/hapi-fhir-jpaserver-base/pom.xml @@ -815,7 +815,15 @@ - + + org.apache.maven.plugins + maven-compiler-plugin + + 9 + 9 + + + ${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..ef476ec1d58 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 @@ -93,6 +93,7 @@ import org.hl7.fhir.instance.model.api.IBaseReference; import org.hl7.fhir.instance.model.api.IBaseResource; import org.hl7.fhir.instance.model.api.IIdType; import org.hl7.fhir.instance.model.api.IPrimitiveType; +import org.jetbrains.annotations.NotNull; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.springframework.beans.factory.annotation.Autowired; @@ -481,7 +482,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 +585,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 +614,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 +630,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); + } + + @NotNull + 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 +717,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 +725,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 +735,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 +744,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 +831,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, 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 388d7696427..595716b47fb 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 @@ -1206,6 +1206,34 @@ public class FhirSystemDaoR4Test extends BaseJpaR4SystemTest { assertThat(observationLocation, matchesPattern("Observation/[a-z0-9-]+/_history/1")); } + @Test + public void testConditionalUrlWhichDoesNotMatcHResource() { + 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(List.of(patientName)) + .setIdentifier(List.of(patientIdentifier)); + patient.setId(IdType.newRandomUuid()); + + transactionBundle + .addEntry() + .setFullUrl(patient.getId()) + .setResource(patient) + .getRequest() + .setMethod(Bundle.HTTPVerb.PUT) + .setUrl("/Patient?identifier=" + patientIdentifier.getSystem() + "|" + "zoop"); + + 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")); + } + @Test public void testTransactionCreateInlineMatchUrlWithOneMatch() { String methodName = "testTransactionCreateInlineMatchUrlWithOneMatch"; From 59c8765e6ae9c46559c8d03a3f710a3684571707 Mon Sep 17 00:00:00 2001 From: Tadgh Date: Mon, 13 Sep 2021 13:03:44 -0400 Subject: [PATCH 03/20] Add implementation to validate conditional URLs post-transaction --- .../jpa/dao/BaseTransactionProcessor.java | 43 ++++++++++++++++++- .../fhir/jpa/dao/r4/FhirSystemDaoR4Test.java | 23 ++++++---- .../src/test/resources/logback-test.xml | 2 +- .../matcher/InMemoryResourceMatcher.java | 1 + 4 files changed, 58 insertions(+), 11 deletions(-) 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 ef476ec1d58..f98e01cb0b5 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 @@ -44,7 +44,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 +66,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; @@ -157,6 +160,8 @@ public abstract class BaseTransactionProcessor { private ModelConfig myModelConfig; @Autowired private InMemoryResourceMatcher myInMemoryResourceMatcher; + @Autowired + private SearchParamMatcher mySearchParamMatcher; private TaskExecutor myExecutor ; @@ -852,6 +857,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<>(); @@ -891,6 +897,7 @@ public abstract class BaseTransactionProcessor { String matchUrl = myVersionAdapter.getEntryRequestIfNoneExist(nextReqEntry); matchUrl = performIdSubstitutionsInMatchUrl(theIdSubstitutions, matchUrl); outcome = resourceDao.create(res, matchUrl, false, theTransactionDetails, theRequest); + conditionalUrlToIdMap.put(matchUrl, outcome.getId()); res.setId(outcome.getId()); if (nextResourceId != null) { handleTransactionCreateOrUpdateOutcome(theIdSubstitutions, theIdToPersistedOutcome, nextResourceId, outcome, nextRespEntry, resourceType, res, theRequest); @@ -925,6 +932,7 @@ public abstract class BaseTransactionProcessor { String matchUrl = parts.getResourceType() + '?' + parts.getParams(); matchUrl = performIdSubstitutionsInMatchUrl(theIdSubstitutions, matchUrl); DeleteMethodOutcome deleteOutcome = dao.deleteByUrl(matchUrl, deleteConflicts, theRequest); + conditionalUrlToIdMap.put(matchUrl, deleteOutcome.getId()); List allDeleted = deleteOutcome.getDeletedEntities(); for (ResourceTable deleted : allDeleted) { deletedResources.add(deleted.getIdDt().toUnqualifiedVersionless().getValueAsString()); @@ -967,6 +975,7 @@ public abstract class BaseTransactionProcessor { } matchUrl = performIdSubstitutionsInMatchUrl(theIdSubstitutions, matchUrl); outcome = resourceDao.update(res, matchUrl, false, false, theRequest, theTransactionDetails); + conditionalUrlToIdMap.put(matchUrl, outcome.getId()); if (Boolean.TRUE.equals(outcome.getCreated())) { conditionalRequestUrls.put(matchUrl, res.getClass()); } @@ -1025,6 +1034,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); + conditionalUrlToIdMap.put(matchUrl, outcome.getId()); updatedEntities.add(outcome.getEntity()); if (outcome.getResource() != null) { updatedResources.add(outcome.getResource()); @@ -1081,6 +1091,11 @@ public abstract class BaseTransactionProcessor { if (!myDaoConfig.isMassIngestionMode()) { validateNoDuplicates(theRequest, theActionName, conditionalRequestUrls, theIdToPersistedOutcome.values()); } + + if (!myDaoConfig.isMassIngestionMode()) { + validateAllInsertsMatchTheirConditionalUrls(theIdToPersistedOutcome, conditionalUrlToIdMap, theRequest); + } + theTransactionStopWatch.endCurrentTask(); for (IIdType next : theAllIds) { @@ -1119,6 +1134,32 @@ public abstract class BaseTransactionProcessor { } } + /** + * 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() + .forEach(entry -> { + String matchUrl = entry.getKey(); + IIdType value = entry.getValue(); + DaoMethodOutcome daoMethodOutcome = theIdToPersistedOutcome.get(value); + if (daoMethodOutcome != null && 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 @@ -1409,7 +1450,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/r4/FhirSystemDaoR4Test.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirSystemDaoR4Test.java index 595716b47fb..a0fdc180f78 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 @@ -98,7 +98,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; @@ -1207,12 +1209,14 @@ public class FhirSystemDaoR4Test extends BaseJpaR4SystemTest { } @Test - public void testConditionalUrlWhichDoesNotMatcHResource() { + 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("U1234567890"); + Identifier patientIdentifier = new Identifier().setSystem("http://example.com/mrns").setValue(storedIdentifierValue); Patient patient = new Patient() .setName(List.of(patientName)) .setIdentifier(List.of(patientIdentifier)); @@ -1224,14 +1228,14 @@ public class FhirSystemDaoR4Test extends BaseJpaR4SystemTest { .setResource(patient) .getRequest() .setMethod(Bundle.HTTPVerb.PUT) - .setUrl("/Patient?identifier=" + patientIdentifier.getSystem() + "|" + "zoop"); + .setUrl("/Patient?identifier=" + patientIdentifier.getSystem() + "|" + conditionalUrlIdentifierValue); - 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")); + 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 @@ -1267,6 +1271,7 @@ public class FhirSystemDaoR4Test extends BaseJpaR4SystemTest { } + @Test public void testTransactionUpdateTwoResourcesWithSameId() { Bundle request = new Bundle(); diff --git a/hapi-fhir-jpaserver-base/src/test/resources/logback-test.xml b/hapi-fhir-jpaserver-base/src/test/resources/logback-test.xml index ac75e0d04be..36a83af6599 100644 --- a/hapi-fhir-jpaserver-base/src/test/resources/logback-test.xml +++ b/hapi-fhir-jpaserver-base/src/test/resources/logback-test.xml @@ -42,7 +42,7 @@ - + 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 f6d0027898f..24a0483b823 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; } From 4aa1329e110a17f0b13d7fc82cd8d82354a1c164 Mon Sep 17 00:00:00 2001 From: Tadgh Date: Mon, 13 Sep 2021 13:13:28 -0400 Subject: [PATCH 04/20] Fix up changelog --- .../fhir/changelog/5_6_0/2987-validate-conditional-urls.yaml | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/5_6_0/2987-validate-conditional-urls.yaml 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." From a0cd83398b8e206ffc0dcb5f82179e3536d7edf2 Mon Sep 17 00:00:00 2001 From: Tadgh Date: Mon, 13 Sep 2021 20:52:14 -0400 Subject: [PATCH 05/20] Bump test java to 16 --- hapi-fhir-jpaserver-base/pom.xml | 8 -------- pom.xml | 2 ++ 2 files changed, 2 insertions(+), 8 deletions(-) diff --git a/hapi-fhir-jpaserver-base/pom.xml b/hapi-fhir-jpaserver-base/pom.xml index c37ee56d4e8..428612b17c4 100644 --- a/hapi-fhir-jpaserver-base/pom.xml +++ b/hapi-fhir-jpaserver-base/pom.xml @@ -815,14 +815,6 @@ - - org.apache.maven.plugins - maven-compiler-plugin - - 9 - 9 - - diff --git a/pom.xml b/pom.xml index b808fcf355c..841a4eabbea 100644 --- a/pom.xml +++ b/pom.xml @@ -2001,6 +2001,8 @@ 1.8 1.8 + 16 + 16 true UTF-8 true From b2f881cb76f833613e74dff40373994b425ea29d Mon Sep 17 00:00:00 2001 From: Tadgh Date: Mon, 13 Sep 2021 21:09:48 -0400 Subject: [PATCH 06/20] java 16 --- azure-pipelines.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/azure-pipelines.yml b/azure-pipelines.yml index d8ccce15b76..9d968dfed1d 100644 --- a/azure-pipelines.yml +++ b/azure-pipelines.yml @@ -15,7 +15,7 @@ pool: jobs: - job: Build timeoutInMinutes: 360 - container: maven:3-openjdk-15 + container: maven:3-openjdk-16 steps: - task: DockerInstaller@0 displayName: Docker Installer @@ -32,7 +32,7 @@ jobs: script: mkdir -p $(MAVEN_CACHE_FOLDER); pwd; ls -al $(MAVEN_CACHE_FOLDER) - task: Maven@3 env: - JAVA_HOME_11_X64: /usr/java/openjdk-15 + JAVA_HOME_11_X64: /usr/java/openjdk-16 inputs: goals: 'clean install' # These are Maven CLI options (and show up in the build logs) - "-nsu"=Don't update snapshots. We can remove this when Maven OSS is more healthy From b4e2679872f30f7310b2e416215ea8551c9c9ffd Mon Sep 17 00:00:00 2001 From: Tadgh Date: Mon, 13 Sep 2021 21:15:23 -0400 Subject: [PATCH 07/20] Remove jetbrains --- .../java/ca/uhn/fhir/jpa/dao/BaseTransactionProcessor.java | 3 +-- .../src/test/java/ca/uhn/fhir/jpa/mdm/BaseMdmR4Test.java | 1 - 2 files changed, 1 insertion(+), 3 deletions(-) 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 f98e01cb0b5..8b359bb9229 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 @@ -96,7 +96,6 @@ import org.hl7.fhir.instance.model.api.IBaseReference; import org.hl7.fhir.instance.model.api.IBaseResource; import org.hl7.fhir.instance.model.api.IIdType; import org.hl7.fhir.instance.model.api.IPrimitiveType; -import org.jetbrains.annotations.NotNull; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.springframework.beans.factory.annotation.Autowired; @@ -647,7 +646,7 @@ public abstract class BaseTransactionProcessor { CompositeInterceptorBroadcaster.doCallHooks(myInterceptorBroadcaster, theRequestDetails, thePointcut, params); } - @NotNull + private TransactionWriteOperationsDetails buildWriteOperationsDetails(List theEntries) { TransactionWriteOperationsDetails writeOperationsDetails; List updateRequestUrls = new ArrayList<>(); 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; From 1ca313f8ad9b0498ba6071792681dc05e81ca71f Mon Sep 17 00:00:00 2001 From: Tadgh Date: Mon, 13 Sep 2021 21:27:05 -0400 Subject: [PATCH 08/20] lower logging --- hapi-fhir-jpaserver-base/src/test/resources/logback-test.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hapi-fhir-jpaserver-base/src/test/resources/logback-test.xml b/hapi-fhir-jpaserver-base/src/test/resources/logback-test.xml index 36a83af6599..ac75e0d04be 100644 --- a/hapi-fhir-jpaserver-base/src/test/resources/logback-test.xml +++ b/hapi-fhir-jpaserver-base/src/test/resources/logback-test.xml @@ -42,7 +42,7 @@ - + From b58d63dc7ccfa1fc1287d4f060dbea1502caae8f Mon Sep 17 00:00:00 2001 From: Tadgh Date: Mon, 13 Sep 2021 21:34:14 -0400 Subject: [PATCH 09/20] Remove usage of 16 --- pom.xml | 2 -- 1 file changed, 2 deletions(-) diff --git a/pom.xml b/pom.xml index 841a4eabbea..b808fcf355c 100644 --- a/pom.xml +++ b/pom.xml @@ -2001,8 +2001,6 @@ 1.8 1.8 - 16 - 16 true UTF-8 true From fcf8c434f5ad93277d61baf10e026f6f2debd7f9 Mon Sep 17 00:00:00 2001 From: Tadgh Date: Mon, 13 Sep 2021 21:41:52 -0400 Subject: [PATCH 10/20] jdk --- azure-pipelines.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/azure-pipelines.yml b/azure-pipelines.yml index 9d968dfed1d..d8ccce15b76 100644 --- a/azure-pipelines.yml +++ b/azure-pipelines.yml @@ -15,7 +15,7 @@ pool: jobs: - job: Build timeoutInMinutes: 360 - container: maven:3-openjdk-16 + container: maven:3-openjdk-15 steps: - task: DockerInstaller@0 displayName: Docker Installer @@ -32,7 +32,7 @@ jobs: script: mkdir -p $(MAVEN_CACHE_FOLDER); pwd; ls -al $(MAVEN_CACHE_FOLDER) - task: Maven@3 env: - JAVA_HOME_11_X64: /usr/java/openjdk-16 + JAVA_HOME_11_X64: /usr/java/openjdk-15 inputs: goals: 'clean install' # These are Maven CLI options (and show up in the build logs) - "-nsu"=Don't update snapshots. We can remove this when Maven OSS is more healthy From 49debec36a18b281e69016ef01d74a2debdbb465 Mon Sep 17 00:00:00 2001 From: Tadgh Date: Tue, 14 Sep 2021 10:48:56 -0400 Subject: [PATCH 11/20] Fix problem with null conditionals --- .../fhir/jpa/dao/BaseTransactionProcessor.java | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) 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 8b359bb9229..8e4034f688b 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 @@ -83,6 +83,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; @@ -896,8 +897,8 @@ public abstract class BaseTransactionProcessor { String matchUrl = myVersionAdapter.getEntryRequestIfNoneExist(nextReqEntry); matchUrl = performIdSubstitutionsInMatchUrl(theIdSubstitutions, matchUrl); outcome = resourceDao.create(res, matchUrl, false, theTransactionDetails, theRequest); - conditionalUrlToIdMap.put(matchUrl, outcome.getId()); - 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); } @@ -931,7 +932,7 @@ public abstract class BaseTransactionProcessor { String matchUrl = parts.getResourceType() + '?' + parts.getParams(); matchUrl = performIdSubstitutionsInMatchUrl(theIdSubstitutions, matchUrl); DeleteMethodOutcome deleteOutcome = dao.deleteByUrl(matchUrl, deleteConflicts, theRequest); - conditionalUrlToIdMap.put(matchUrl, deleteOutcome.getId()); + setConditionalUrlToBeValidatedLater(conditionalUrlToIdMap, matchUrl, deleteOutcome.getId()); List allDeleted = deleteOutcome.getDeletedEntities(); for (ResourceTable deleted : allDeleted) { deletedResources.add(deleted.getIdDt().toUnqualifiedVersionless().getValueAsString()); @@ -974,7 +975,7 @@ public abstract class BaseTransactionProcessor { } matchUrl = performIdSubstitutionsInMatchUrl(theIdSubstitutions, matchUrl); outcome = resourceDao.update(res, matchUrl, false, false, theRequest, theTransactionDetails); - conditionalUrlToIdMap.put(matchUrl, outcome.getId()); + setConditionalUrlToBeValidatedLater(conditionalUrlToIdMap, matchUrl, outcome.getId()); if (Boolean.TRUE.equals(outcome.getCreated())) { conditionalRequestUrls.put(matchUrl, res.getClass()); } @@ -1033,7 +1034,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); - conditionalUrlToIdMap.put(matchUrl, outcome.getId()); + setConditionalUrlToBeValidatedLater(conditionalUrlToIdMap, matchUrl, outcome.getId()); updatedEntities.add(outcome.getEntity()); if (outcome.getResource() != null) { updatedResources.add(outcome.getResource()); @@ -1133,6 +1134,12 @@ 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. @@ -1141,6 +1148,7 @@ public abstract class BaseTransactionProcessor { */ 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(); From 9771210553d08ff636ed69534a339056a8836bec Mon Sep 17 00:00:00 2001 From: Tadgh Date: Tue, 14 Sep 2021 13:40:53 -0400 Subject: [PATCH 12/20] Prefer non-lazy outcomes in map --- .../jpa/api/model/LazyDaoMethodOutcome.java | 2 -- .../jpa/dao/BaseTransactionProcessor.java | 26 +++++++++++++++++-- .../fhir/jpa/dao/r4/FhirSystemDaoR4Test.java | 6 ++--- 3 files changed, 27 insertions(+), 7 deletions(-) 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/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 8e4034f688b..6804fcb9848 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; @@ -280,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 { @@ -304,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(); } @@ -1092,10 +1110,14 @@ public abstract class BaseTransactionProcessor { 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) { 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 a0fdc180f78..7e7bd26fbd7 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 @@ -1824,9 +1824,9 @@ public class FhirSystemDaoR4Test extends BaseJpaR4SystemTest { p.addIdentifier().setSystem("urn:system").setValue(methodName); request.addEntry().setResource(p).getRequest().setMethod(HTTPVerb.POST).setIfNoneExist("Patient?identifier=urn%3Asystem%7C" + methodName); - p = new Patient(); - p.addIdentifier().setSystem("urn:system").setValue(methodName); - request.addEntry().setResource(p).getRequest().setMethod(HTTPVerb.POST); +// p = new Patient(); +// p.addIdentifier().setSystem("urn:system").setValue(methodName); +// request.addEntry().setResource(p).getRequest().setMethod(HTTPVerb.POST); try { mySystemDao.transaction(mySrd, request); From b25bdc1169dd8e685cfc6ecdb5a6b16f9471e044 Mon Sep 17 00:00:00 2001 From: Tadgh Date: Tue, 14 Sep 2021 13:48:37 -0400 Subject: [PATCH 13/20] Remove crazy double encoding --- .../ca/uhn/fhir/jpa/dao/r4/FhirSystemDaoR4Test.java | 6 +++--- .../src/test/resources/cdr-bundle.json | 12 ++++++------ 2 files changed, 9 insertions(+), 9 deletions(-) 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 7e7bd26fbd7..a0fdc180f78 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 @@ -1824,9 +1824,9 @@ public class FhirSystemDaoR4Test extends BaseJpaR4SystemTest { p.addIdentifier().setSystem("urn:system").setValue(methodName); request.addEntry().setResource(p).getRequest().setMethod(HTTPVerb.POST).setIfNoneExist("Patient?identifier=urn%3Asystem%7C" + methodName); -// p = new Patient(); -// p.addIdentifier().setSystem("urn:system").setValue(methodName); -// request.addEntry().setResource(p).getRequest().setMethod(HTTPVerb.POST); + p = new Patient(); + p.addIdentifier().setSystem("urn:system").setValue(methodName); + request.addEntry().setResource(p).getRequest().setMethod(HTTPVerb.POST); try { mySystemDao.transaction(mySrd, request); 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 +} From bb5e04e7064c242dd34ff83b6e5c36a1a23fedee Mon Sep 17 00:00:00 2001 From: Tadgh Date: Tue, 14 Sep 2021 17:22:25 -0400 Subject: [PATCH 14/20] Fix broken test --- .../ca/uhn/fhir/jpa/dao/TransactionProcessorTest.java | 3 +++ .../java/ca/uhn/fhir/jpa/dao/r4/FhirSystemDaoR4Test.java | 9 +++++---- 2 files changed, 8 insertions(+), 4 deletions(-) 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/FhirSystemDaoR4Test.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirSystemDaoR4Test.java index a0fdc180f78..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 @@ -85,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; @@ -1173,8 +1174,8 @@ public class FhirSystemDaoR4Test extends BaseJpaR4SystemTest { 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(List.of(patientName)) - .setIdentifier(List.of(patientIdentifier)); + .setName(Arrays.asList(patientName)) + .setIdentifier(Arrays.asList(patientIdentifier)); patient.setId(IdType.newRandomUuid()); transactionBundle @@ -1218,8 +1219,8 @@ public class FhirSystemDaoR4Test extends BaseJpaR4SystemTest { 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(List.of(patientName)) - .setIdentifier(List.of(patientIdentifier)); + .setName(Arrays.asList(patientName)) + .setIdentifier(Arrays.asList(patientIdentifier)); patient.setId(IdType.newRandomUuid()); transactionBundle From 9bb1558b714d5266da478dbb967d18806d2111d5 Mon Sep 17 00:00:00 2001 From: Tadgh Date: Tue, 14 Sep 2021 17:45:09 -0400 Subject: [PATCH 15/20] Update query count ttest --- .../uhn/fhir/jpa/dao/BaseTransactionProcessor.java | 1 + .../dao/r4/FhirResourceDaoR4QueryCountTest.java | 14 +++++++------- 2 files changed, 8 insertions(+), 7 deletions(-) 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 6804fcb9848..d57891839e3 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 @@ -1175,6 +1175,7 @@ public abstract class BaseTransactionProcessor { String matchUrl = entry.getKey(); IIdType value = entry.getValue(); DaoMethodOutcome daoMethodOutcome = theIdToPersistedOutcome.get(value); + //TODO GGG: daoMethodOutcome.getResource() actually executes a DB select query. Any way to avoid this?? if (daoMethodOutcome != null && daoMethodOutcome.getResource() != null) { InMemoryMatchResult match = mySearchParamMatcher.match(matchUrl, daoMethodOutcome.getResource(), theRequest); if (ourLog.isDebugEnabled()) { 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 580cb632e94..06a8110de62 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 @@ -964,7 +964,7 @@ public class FhirResourceDaoR4QueryCountTest extends BaseResourceProviderR4Test outcome = mySystemDao.transaction(mySrd, input.get()); ourLog.info("Resp: {}", myFhirCtx.newJsonParser().setPrettyPrint(true).encodeResourceToString(outcome)); myCaptureQueriesListener.logSelectQueries(); - assertEquals(11, myCaptureQueriesListener.countSelectQueries()); + assertEquals(12, myCaptureQueriesListener.countSelectQueries()); myCaptureQueriesListener.logInsertQueries(); assertEquals(1, myCaptureQueriesListener.countInsertQueries()); myCaptureQueriesListener.logUpdateQueries(); @@ -1043,7 +1043,7 @@ public class FhirResourceDaoR4QueryCountTest extends BaseResourceProviderR4Test myCaptureQueriesListener.clear(); mySystemDao.transaction(mySrd, bundleCreator.get()); myCaptureQueriesListener.logSelectQueries(); - assertEquals(1, myCaptureQueriesListener.countSelectQueries()); + assertEquals(3, myCaptureQueriesListener.countSelectQueries()); assertEquals(3, myCaptureQueriesListener.countInsertQueries()); assertEquals(0, myCaptureQueriesListener.countDeleteQueries()); @@ -1056,7 +1056,7 @@ public class FhirResourceDaoR4QueryCountTest extends BaseResourceProviderR4Test myCaptureQueriesListener.clear(); mySystemDao.transaction(mySrd, bundleCreator.get()); - assertEquals(0, myCaptureQueriesListener.countSelectQueries()); + assertEquals(2, myCaptureQueriesListener.countSelectQueries()); assertEquals(3, myCaptureQueriesListener.countInsertQueries()); assertEquals(0, myCaptureQueriesListener.countDeleteQueries()); @@ -1105,7 +1105,7 @@ public class FhirResourceDaoR4QueryCountTest extends BaseResourceProviderR4Test myCaptureQueriesListener.clear(); mySystemDao.transaction(mySrd, bundleCreator.get()); myCaptureQueriesListener.logSelectQueries(); - assertEquals(2, myCaptureQueriesListener.countSelectQueries()); + assertEquals(4, myCaptureQueriesListener.countSelectQueries()); assertEquals(3, myCaptureQueriesListener.countInsertQueries()); assertEquals(0, myCaptureQueriesListener.countDeleteQueries()); @@ -1123,7 +1123,7 @@ public class FhirResourceDaoR4QueryCountTest extends BaseResourceProviderR4Test myCaptureQueriesListener.clear(); mySystemDao.transaction(mySrd, bundleCreator.get()); - assertEquals(1, myCaptureQueriesListener.countSelectQueries()); + assertEquals(3, myCaptureQueriesListener.countSelectQueries()); assertEquals(3, myCaptureQueriesListener.countInsertQueries()); assertEquals(0, myCaptureQueriesListener.countDeleteQueries()); @@ -1706,7 +1706,7 @@ public class FhirResourceDaoR4QueryCountTest extends BaseResourceProviderR4Test // Lookup the two existing IDs to make sure they are legit myCaptureQueriesListener.logSelectQueriesForCurrentThread(); - assertEquals(3, myCaptureQueriesListener.countSelectQueriesForCurrentThread()); + assertEquals(7, myCaptureQueriesListener.countSelectQueriesForCurrentThread()); assertEquals(3, myCaptureQueriesListener.countInsertQueriesForCurrentThread()); assertEquals(1, myCaptureQueriesListener.countUpdateQueriesForCurrentThread()); assertEquals(0, myCaptureQueriesListener.countDeleteQueriesForCurrentThread()); @@ -1765,7 +1765,7 @@ public class FhirResourceDaoR4QueryCountTest extends BaseResourceProviderR4Test // Lookup the two existing IDs to make sure they are legit myCaptureQueriesListener.logSelectQueriesForCurrentThread(); - assertEquals(1, myCaptureQueriesListener.countSelectQueriesForCurrentThread()); + assertEquals(5, myCaptureQueriesListener.countSelectQueriesForCurrentThread()); assertEquals(3, myCaptureQueriesListener.countInsertQueriesForCurrentThread()); assertEquals(1, myCaptureQueriesListener.countUpdateQueriesForCurrentThread()); assertEquals(0, myCaptureQueriesListener.countDeleteQueriesForCurrentThread()); From 8f0e8b9e51c4b065432d61d4c5b0e06dff095ab8 Mon Sep 17 00:00:00 2001 From: Tadgh Date: Tue, 14 Sep 2021 18:44:55 -0400 Subject: [PATCH 16/20] remove useless DB calls --- .../uhn/fhir/jpa/dao/BaseTransactionProcessor.java | 3 +-- .../dao/r4/FhirResourceDaoR4QueryCountTest.java | 14 +++++++------- 2 files changed, 8 insertions(+), 9 deletions(-) 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 d57891839e3..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 @@ -1175,8 +1175,7 @@ public abstract class BaseTransactionProcessor { String matchUrl = entry.getKey(); IIdType value = entry.getValue(); DaoMethodOutcome daoMethodOutcome = theIdToPersistedOutcome.get(value); - //TODO GGG: daoMethodOutcome.getResource() actually executes a DB select query. Any way to avoid this?? - if (daoMethodOutcome != null && daoMethodOutcome.getResource() != null) { + 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()); 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 06a8110de62..580cb632e94 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 @@ -964,7 +964,7 @@ public class FhirResourceDaoR4QueryCountTest extends BaseResourceProviderR4Test outcome = mySystemDao.transaction(mySrd, input.get()); ourLog.info("Resp: {}", myFhirCtx.newJsonParser().setPrettyPrint(true).encodeResourceToString(outcome)); myCaptureQueriesListener.logSelectQueries(); - assertEquals(12, myCaptureQueriesListener.countSelectQueries()); + assertEquals(11, myCaptureQueriesListener.countSelectQueries()); myCaptureQueriesListener.logInsertQueries(); assertEquals(1, myCaptureQueriesListener.countInsertQueries()); myCaptureQueriesListener.logUpdateQueries(); @@ -1043,7 +1043,7 @@ public class FhirResourceDaoR4QueryCountTest extends BaseResourceProviderR4Test myCaptureQueriesListener.clear(); mySystemDao.transaction(mySrd, bundleCreator.get()); myCaptureQueriesListener.logSelectQueries(); - assertEquals(3, myCaptureQueriesListener.countSelectQueries()); + assertEquals(1, myCaptureQueriesListener.countSelectQueries()); assertEquals(3, myCaptureQueriesListener.countInsertQueries()); assertEquals(0, myCaptureQueriesListener.countDeleteQueries()); @@ -1056,7 +1056,7 @@ public class FhirResourceDaoR4QueryCountTest extends BaseResourceProviderR4Test myCaptureQueriesListener.clear(); mySystemDao.transaction(mySrd, bundleCreator.get()); - assertEquals(2, myCaptureQueriesListener.countSelectQueries()); + assertEquals(0, myCaptureQueriesListener.countSelectQueries()); assertEquals(3, myCaptureQueriesListener.countInsertQueries()); assertEquals(0, myCaptureQueriesListener.countDeleteQueries()); @@ -1105,7 +1105,7 @@ public class FhirResourceDaoR4QueryCountTest extends BaseResourceProviderR4Test myCaptureQueriesListener.clear(); mySystemDao.transaction(mySrd, bundleCreator.get()); myCaptureQueriesListener.logSelectQueries(); - assertEquals(4, myCaptureQueriesListener.countSelectQueries()); + assertEquals(2, myCaptureQueriesListener.countSelectQueries()); assertEquals(3, myCaptureQueriesListener.countInsertQueries()); assertEquals(0, myCaptureQueriesListener.countDeleteQueries()); @@ -1123,7 +1123,7 @@ public class FhirResourceDaoR4QueryCountTest extends BaseResourceProviderR4Test myCaptureQueriesListener.clear(); mySystemDao.transaction(mySrd, bundleCreator.get()); - assertEquals(3, myCaptureQueriesListener.countSelectQueries()); + assertEquals(1, myCaptureQueriesListener.countSelectQueries()); assertEquals(3, myCaptureQueriesListener.countInsertQueries()); assertEquals(0, myCaptureQueriesListener.countDeleteQueries()); @@ -1706,7 +1706,7 @@ public class FhirResourceDaoR4QueryCountTest extends BaseResourceProviderR4Test // Lookup the two existing IDs to make sure they are legit myCaptureQueriesListener.logSelectQueriesForCurrentThread(); - assertEquals(7, myCaptureQueriesListener.countSelectQueriesForCurrentThread()); + assertEquals(3, myCaptureQueriesListener.countSelectQueriesForCurrentThread()); assertEquals(3, myCaptureQueriesListener.countInsertQueriesForCurrentThread()); assertEquals(1, myCaptureQueriesListener.countUpdateQueriesForCurrentThread()); assertEquals(0, myCaptureQueriesListener.countDeleteQueriesForCurrentThread()); @@ -1765,7 +1765,7 @@ public class FhirResourceDaoR4QueryCountTest extends BaseResourceProviderR4Test // Lookup the two existing IDs to make sure they are legit myCaptureQueriesListener.logSelectQueriesForCurrentThread(); - assertEquals(5, myCaptureQueriesListener.countSelectQueriesForCurrentThread()); + assertEquals(1, myCaptureQueriesListener.countSelectQueriesForCurrentThread()); assertEquals(3, myCaptureQueriesListener.countInsertQueriesForCurrentThread()); assertEquals(1, myCaptureQueriesListener.countUpdateQueriesForCurrentThread()); assertEquals(0, myCaptureQueriesListener.countDeleteQueriesForCurrentThread()); From 9070aa357187d6479182dc2b6636dee21816a0ef Mon Sep 17 00:00:00 2001 From: Tadgh Date: Wed, 15 Sep 2021 01:20:46 -0400 Subject: [PATCH 17/20] wip --- .../uhn/fhir/rest/server/interceptor/auth/IAuthRuleBuilder.java | 1 - .../rest/server/interceptor/auth/IAuthRuleBuilderRuleOp.java | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/IAuthRuleBuilder.java b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/IAuthRuleBuilder.java index d9557d86ca3..c2f29fa8c85 100644 --- a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/IAuthRuleBuilder.java +++ b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/IAuthRuleBuilder.java @@ -67,7 +67,6 @@ public interface IAuthRuleBuilder { * and could be shown to the client, but has no semantic meaning within * HAPI FHIR. */ - IAuthRuleBuilderRuleOpClassifierFinished allowAll(String theRuleName); /** * Build the rule list 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 From a0c8cc427937936f572c20785525014e90c2855a Mon Sep 17 00:00:00 2001 From: Tadgh Date: Wed, 15 Sep 2021 01:23:43 -0400 Subject: [PATCH 18/20] fix test --- .../jpa/dao/r4/FhirResourceDaoR4VersionedReferenceTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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..47da8dafd53 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 @@ -467,7 +467,7 @@ public class FhirResourceDaoR4VersionedReferenceTest extends BaseJpaR4Test { Patient patient = new Patient(); patient.setId(IdType.newRandomUuid()); - patient.setActive(true); + patient.setActive(false); builder .addTransactionUpdateEntry(patient) .conditional("Patient?active=false"); From 171f0f8724382a3d5e6e30802cbe27787f7b2ae9 Mon Sep 17 00:00:00 2001 From: Tadgh Date: Wed, 15 Sep 2021 09:04:45 -0400 Subject: [PATCH 19/20] fix rulebuilder --- .../jpa/dao/r4/FhirResourceDaoR4VersionedReferenceTest.java | 3 ++- .../fhir/rest/server/interceptor/auth/IAuthRuleBuilder.java | 1 + 2 files changed, 3 insertions(+), 1 deletion(-) 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 47da8dafd53..423c0dadaa6 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 @@ -467,6 +467,7 @@ public class FhirResourceDaoR4VersionedReferenceTest extends BaseJpaR4Test { Patient patient = new Patient(); patient.setId(IdType.newRandomUuid()); + patient.addName().setFamily("zoop"); patient.setActive(false); builder .addTransactionUpdateEntry(patient) @@ -488,7 +489,7 @@ public class FhirResourceDaoR4VersionedReferenceTest extends BaseJpaR4Test { assertEquals("1", observationId.getVersionIdPart()); // Make sure we're not introducing any extra DB operations - assertEquals(4, myCaptureQueriesListener.logSelectQueries().size()); + assertEquals(5, myCaptureQueriesListener.logSelectQueries().size()); // Read back and verify that reference is now versioned observation = myObservationDao.read(observationId); diff --git a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/IAuthRuleBuilder.java b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/IAuthRuleBuilder.java index c2f29fa8c85..d9557d86ca3 100644 --- a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/IAuthRuleBuilder.java +++ b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/IAuthRuleBuilder.java @@ -67,6 +67,7 @@ public interface IAuthRuleBuilder { * and could be shown to the client, but has no semantic meaning within * HAPI FHIR. */ + IAuthRuleBuilderRuleOpClassifierFinished allowAll(String theRuleName); /** * Build the rule list From 724e4b37dac3a34067f098eaee97098ae1099ca2 Mon Sep 17 00:00:00 2001 From: Tadgh Date: Wed, 15 Sep 2021 11:24:01 -0400 Subject: [PATCH 20/20] Fix up test to actually match --- .../jpa/dao/r4/FhirResourceDaoR4VersionedReferenceTest.java | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) 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 423c0dadaa6..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,7 @@ public class FhirResourceDaoR4VersionedReferenceTest extends BaseJpaR4Test { Patient patient = new Patient(); patient.setId(IdType.newRandomUuid()); - patient.addName().setFamily("zoop"); + patient.setDeceased(new BooleanType(true)); patient.setActive(false); builder .addTransactionUpdateEntry(patient) @@ -489,7 +490,7 @@ public class FhirResourceDaoR4VersionedReferenceTest extends BaseJpaR4Test { assertEquals("1", observationId.getVersionIdPart()); // Make sure we're not introducing any extra DB operations - assertEquals(5, myCaptureQueriesListener.logSelectQueries().size()); + assertEquals(4, myCaptureQueriesListener.logSelectQueries().size()); // Read back and verify that reference is now versioned observation = myObservationDao.read(observationId);