From 8f30f7b0f2d671a7e3cf874646918922c4bb2a01 Mon Sep 17 00:00:00 2001 From: Tadgh Date: Fri, 9 Apr 2021 11:08:47 -0400 Subject: [PATCH 01/30] Vague skeleton of pointcut --- .../ca/uhn/fhir/interceptor/api/Pointcut.java | 40 +++++++++++++++++++ .../jpa/dao/BaseTransactionProcessor.java | 40 ++++++++++++------- .../DeferredInterceptorBroadcasts.java | 8 ++++ .../server/storage/TransactionDetails.java | 10 +++++ 4 files changed, 84 insertions(+), 14 deletions(-) create mode 100644 hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/api/server/storage/DeferredInterceptorBroadcasts.java diff --git a/hapi-fhir-base/src/main/java/ca/uhn/fhir/interceptor/api/Pointcut.java b/hapi-fhir-base/src/main/java/ca/uhn/fhir/interceptor/api/Pointcut.java index b36bc90a43f..1463d5f1369 100644 --- a/hapi-fhir-base/src/main/java/ca/uhn/fhir/interceptor/api/Pointcut.java +++ b/hapi-fhir-base/src/main/java/ca/uhn/fhir/interceptor/api/Pointcut.java @@ -1511,6 +1511,46 @@ public enum Pointcut implements IPointcut { "ca.uhn.fhir.rest.api.server.storage.TransactionDetails" ), + /** + * Storage Hook: + * Invoked before a resource will be deleted + *

+ * Hooks will have access to the contents of the resource being deleted + * but should not make any changes as storage has already occurred + *

+ * Hooks may accept the following parameters: + * + *

+ * Hooks should return void. + *

+ */ + STORAGE_TRANSACTION_PROCESSED(void.class, + "org.hl7.fhir.instance.model.api.IBaseResource", + "ca.uhn.fhir.rest.api.server.storage.DeferredInterceptorBroadcasts", + "ca.uhn.fhir.rest.api.server.RequestDetails", + "ca.uhn.fhir.rest.server.servlet.ServletRequestDetails", + "ca.uhn.fhir.rest.api.server.storage.TransactionDetails" + ), + + /** * Storage Hook: * Invoked when a resource delete operation is about to fail due to referential integrity checks. Intended for use with {@literal ca.uhn.fhir.jpa.interceptor.CascadingDeleteInterceptor}. 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 379664a141a..1e25f7bb007 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 @@ -995,25 +995,16 @@ public abstract class BaseTransactionProcessor { flushSession(theIdToPersistedOutcome); theTransactionStopWatch.endCurrentTask(); - if (conditionalRequestUrls.size() > 0) { - theTransactionStopWatch.startTask("Check for conflicts in conditional resources"); - } + + /* * Double check we didn't allow any duplicates we shouldn't have */ - for (Map.Entry> nextEntry : conditionalRequestUrls.entrySet()) { - String matchUrl = nextEntry.getKey(); - Class resType = nextEntry.getValue(); - if (isNotBlank(matchUrl)) { - Set val = myMatchResourceUrlService.processMatchUrl(matchUrl, resType, theRequest); - if (val.size() > 1) { - throw new InvalidRequestException( - "Unable to process " + theActionName + " - Request would cause multiple resources to match URL: \"" + matchUrl + "\". Does transaction request contain duplicates?"); - } - } + if (conditionalRequestUrls.size() > 0) { + theTransactionStopWatch.startTask("Check for conflicts in conditional resources"); } - + validateNoDuplicates(theRequest, theActionName, conditionalRequestUrls); theTransactionStopWatch.endCurrentTask(); for (IIdType next : theAllIds) { @@ -1034,6 +1025,13 @@ public abstract class BaseTransactionProcessor { JpaInterceptorBroadcaster.doCallHooks(myInterceptorBroadcaster, theRequest, nextPointcut, nextParams); } + + theTransactionDetails.deferredBroadcastProcessingFinished(); + + + + //finishedCallingDeferredInterceptorBroadcasts + return entriesToProcess; } finally { @@ -1043,6 +1041,20 @@ public abstract class BaseTransactionProcessor { } } + private void validateNoDuplicates(RequestDetails theRequest, String theActionName, Map> conditionalRequestUrls) { + for (Map.Entry> nextEntry : conditionalRequestUrls.entrySet()) { + String matchUrl = nextEntry.getKey(); + Class resType = nextEntry.getValue(); + if (isNotBlank(matchUrl)) { + Set val = myMatchResourceUrlService.processMatchUrl(matchUrl, resType, theRequest); + if (val.size() > 1) { + throw new InvalidRequestException( + "Unable to process " + theActionName + " - Request would cause multiple resources to match URL: \"" + matchUrl + "\". Does transaction request contain duplicates?"); + } + } + } + } + protected abstract void flushSession(Map theIdToPersistedOutcome); private void validateResourcePresent(IBaseResource theResource, Integer theOrder, String theVerb) { diff --git a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/api/server/storage/DeferredInterceptorBroadcasts.java b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/api/server/storage/DeferredInterceptorBroadcasts.java new file mode 100644 index 00000000000..f13e6e41407 --- /dev/null +++ b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/api/server/storage/DeferredInterceptorBroadcasts.java @@ -0,0 +1,8 @@ +package ca.uhn.fhir.rest.api.server.storage; + +import com.google.common.collect.ListMultimap; + +public class DeferredInterceptorBroadcasts { + + public DeferredInterceptorBroadcasts(ListMultimap) +} diff --git a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/api/server/storage/TransactionDetails.java b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/api/server/storage/TransactionDetails.java index 57520bc2570..76a28163313 100644 --- a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/api/server/storage/TransactionDetails.java +++ b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/api/server/storage/TransactionDetails.java @@ -53,6 +53,7 @@ public class TransactionDetails { private Map myUserData; private ListMultimap myDeferredInterceptorBroadcasts; private EnumSet myDeferredInterceptorBroadcastPointcuts; + private boolean myIsPointcutDeferred; /** * Constructor @@ -189,7 +190,16 @@ public class TransactionDetails { */ public void addDeferredInterceptorBroadcast(Pointcut thePointcut, HookParams theHookParams) { Validate.isTrue(isAcceptingDeferredInterceptorBroadcasts(thePointcut)); + myIsPointcutDeferred = true; myDeferredInterceptorBroadcasts.put(thePointcut, theHookParams); } + + public boolean isPointcutDeferred() { + return myIsPointcutDeferred; + } + + public void deferredBroadcastProcessingFinished() { + myIsPointcutDeferred = false; + } } From 28c917b7a22d9625a7eaa6ddc4683eb582872df1 Mon Sep 17 00:00:00 2001 From: Tadgh Date: Fri, 9 Apr 2021 11:28:47 -0400 Subject: [PATCH 02/30] Flesh out holder object --- .../storage/DeferredInterceptorBroadcasts.java | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/api/server/storage/DeferredInterceptorBroadcasts.java b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/api/server/storage/DeferredInterceptorBroadcasts.java index f13e6e41407..c8bcc355d4e 100644 --- a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/api/server/storage/DeferredInterceptorBroadcasts.java +++ b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/api/server/storage/DeferredInterceptorBroadcasts.java @@ -1,8 +1,18 @@ package ca.uhn.fhir.rest.api.server.storage; +import ca.uhn.fhir.interceptor.api.HookParams; +import ca.uhn.fhir.interceptor.api.Pointcut; import com.google.common.collect.ListMultimap; public class DeferredInterceptorBroadcasts { - public DeferredInterceptorBroadcasts(ListMultimap) + ListMultimap myDeferredInterceptorBroadcasts; + + public DeferredInterceptorBroadcasts(ListMultimap theDeferredInterceptorBroadcasts) { + myDeferredInterceptorBroadcasts = theDeferredInterceptorBroadcasts; + } + + public ListMultimap getDeferredInterceptorBroadcasts() { + return myDeferredInterceptorBroadcasts; + } } From ac8e012892a92cb23f3241b0448523350962a6eb Mon Sep 17 00:00:00 2001 From: Tadgh Date: Fri, 9 Apr 2021 13:07:15 -0400 Subject: [PATCH 03/30] wip --- .../ca/uhn/fhir/interceptor/api/Pointcut.java | 9 +- .../jpa/dao/BaseTransactionProcessor.java | 7 + .../fhir/jpa/dao/r4/TransactionHookTest.java | 213 ++++++++++++++++++ 3 files changed, 226 insertions(+), 3 deletions(-) create mode 100644 hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/TransactionHookTest.java diff --git a/hapi-fhir-base/src/main/java/ca/uhn/fhir/interceptor/api/Pointcut.java b/hapi-fhir-base/src/main/java/ca/uhn/fhir/interceptor/api/Pointcut.java index 1463d5f1369..ed1b6e7bad2 100644 --- a/hapi-fhir-base/src/main/java/ca/uhn/fhir/interceptor/api/Pointcut.java +++ b/hapi-fhir-base/src/main/java/ca/uhn/fhir/interceptor/api/Pointcut.java @@ -1513,10 +1513,10 @@ public enum Pointcut implements IPointcut { /** * Storage Hook: - * Invoked before a resource will be deleted + * Invoked after all entries in a transaction bundle have been executed *

- * Hooks will have access to the contents of the resource being deleted - * but should not make any changes as storage has already occurred + * Hooks will have access to the original bundle, as well as all the deferred interceptor broadcasts related to the + * processing of the transaction bundle *

* Hooks may accept the following parameters: *
    @@ -1537,6 +1537,9 @@ public enum Pointcut implements IPointcut { *
  • * ca.uhn.fhir.rest.api.server.storage.TransactionDetails - The outer transaction details object (since 5.0.0) *
    • + *
  • + * ca.uhn.fhir.rest.api.server.storage.DeferredInterceptorBroadcasts- A collection of pointcut invocations and their parameters which were deferred. + *
    • *
*

* Hooks should return void. 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 1e25f7bb007..332cfe0b60b 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 @@ -48,6 +48,7 @@ import ca.uhn.fhir.rest.api.PatchTypeEnum; import ca.uhn.fhir.rest.api.PreferReturnEnum; import ca.uhn.fhir.rest.api.RestOperationTypeEnum; import ca.uhn.fhir.rest.api.server.RequestDetails; +import ca.uhn.fhir.rest.api.server.storage.DeferredInterceptorBroadcasts; import ca.uhn.fhir.rest.api.server.storage.ResourcePersistentId; import ca.uhn.fhir.rest.api.server.storage.TransactionDetails; import ca.uhn.fhir.rest.param.ParameterUtil; @@ -1025,6 +1026,12 @@ public abstract class BaseTransactionProcessor { JpaInterceptorBroadcaster.doCallHooks(myInterceptorBroadcaster, theRequest, nextPointcut, nextParams); } + DeferredInterceptorBroadcasts deferredInterceptorBroadcasts = new DeferredInterceptorBroadcasts(deferredBroadcastEvents); + HookParams params = new HookParams() + .add(RequestDetails.class, theRequest) + .addIfMatchesType(ServletRequestDetails.class, theRequest) + .add(DeferredInterceptorBroadcasts.class, deferredInterceptorBroadcasts); + JpaInterceptorBroadcaster.doCallHooks(myInterceptorBroadcaster, theRequest, Pointcut.STORAGE_TRANSACTION_PROCESSED, params); theTransactionDetails.deferredBroadcastProcessingFinished(); diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/TransactionHookTest.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/TransactionHookTest.java new file mode 100644 index 00000000000..8cb39681e7a --- /dev/null +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/TransactionHookTest.java @@ -0,0 +1,213 @@ +package ca.uhn.fhir.jpa.dao.r4; + +import ca.uhn.fhir.interceptor.api.IInterceptorService; +import ca.uhn.fhir.interceptor.api.Pointcut; +import ca.uhn.fhir.rest.server.exceptions.ResourceGoneException; +import ca.uhn.fhir.rest.server.exceptions.ResourceVersionConflictException; +import ca.uhn.test.concurrency.PointcutLatch; +import org.hl7.fhir.instance.model.api.IIdType; +import org.hl7.fhir.r4.model.Bundle; +import org.hl7.fhir.r4.model.DiagnosticReport; +import org.hl7.fhir.r4.model.Observation; +import org.hl7.fhir.r4.model.Reference; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.springframework.beans.factory.annotation.Autowired; + +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.empty; +import static org.hamcrest.Matchers.matchesPattern; +import static org.junit.jupiter.api.Assertions.fail; + +public class TransactionHookTest extends BaseJpaR4SystemTest { + + @AfterEach + public void after() { + myDaoConfig.setEnforceReferentialIntegrityOnDelete(true); + } + + PointcutLatch myPointcutLatch = new PointcutLatch(Pointcut.STORAGE_TRANSACTION_PROCESSED); + @Autowired + private IInterceptorService myInterceptorService; + + @BeforeEach + public void beforeEach() { + myInterceptorService.registerAnonymousInterceptor(Pointcut.STORAGE_TRANSACTION_PROCESSED, myPointcutLatch); + } + + @Test + public void testHookShouldContainParamsForAllCreateUpdateDeleteInvocations() { + final Observation obs1 = new Observation(); + obs1.setStatus(Observation.ObservationStatus.FINAL); + IIdType obs1id = myObservationDao.create(obs1).getId().toUnqualifiedVersionless(); + + final Observation obs2 = new Observation(); + obs2.setStatus(Observation.ObservationStatus.FINAL); + IIdType obs2id = myObservationDao.create(obs2).getId().toUnqualifiedVersionless(); + + final DiagnosticReport rpt = new DiagnosticReport(); + rpt.addResult(new Reference(obs2id)); + IIdType rptId = myDiagnosticReportDao.create(rpt).getId().toUnqualifiedVersionless(); + + myObservationDao.read(obs1id); + myObservationDao.read(obs2id); + myDiagnosticReportDao.read(rptId); + + Bundle b = new Bundle(); + b.addEntry().getRequest().setMethod(Bundle.HTTPVerb.DELETE).setUrl(obs2id.getValue()); + + try { + mySystemDao.transaction(mySrd, b); + fail(); + } catch (ResourceVersionConflictException e) { + // good, transaction should not succeed because DiagnosticReport has a reference to the obs2 + } + } + + @Test + public void testDeleteInTransactionShouldSucceedWhenReferencesAreAlsoRemoved() { + final Observation obs1 = new Observation(); + obs1.setStatus(Observation.ObservationStatus.FINAL); + IIdType obs1id = myObservationDao.create(obs1).getId().toUnqualifiedVersionless(); + + final Observation obs2 = new Observation(); + obs2.setStatus(Observation.ObservationStatus.FINAL); + IIdType obs2id = myObservationDao.create(obs2).getId().toUnqualifiedVersionless(); + + final DiagnosticReport rpt = new DiagnosticReport(); + rpt.addResult(new Reference(obs2id)); + IIdType rptId = myDiagnosticReportDao.create(rpt).getId().toUnqualifiedVersionless(); + + myObservationDao.read(obs1id); + myObservationDao.read(obs2id); + myDiagnosticReportDao.read(rptId); + + Bundle b = new Bundle(); + b.addEntry().getRequest().setMethod(Bundle.HTTPVerb.DELETE).setUrl(rptId.getValue()); + b.addEntry().getRequest().setMethod(Bundle.HTTPVerb.DELETE).setUrl(obs2id.getValue()); + + try { + // transaction should succeed because the DiagnosticReport which references obs2 is also deleted + mySystemDao.transaction(mySrd, b); + } catch (ResourceVersionConflictException e) { + fail(); + } + } + + + @Test + public void testDeleteWithHas_SourceModifiedToNoLongerIncludeReference() { + + Observation obs1 = new Observation(); + obs1.setStatus(Observation.ObservationStatus.FINAL); + IIdType obs1id = myObservationDao.create(obs1).getId().toUnqualifiedVersionless(); + + Observation obs2 = new Observation(); + obs2.setStatus(Observation.ObservationStatus.FINAL); + IIdType obs2id = myObservationDao.create(obs2).getId().toUnqualifiedVersionless(); + + DiagnosticReport rpt = new DiagnosticReport(); + rpt.addIdentifier().setSystem("foo").setValue("IDENTIFIER"); + rpt.addResult(new Reference(obs2id)); + IIdType rptId = myDiagnosticReportDao.create(rpt).getId().toUnqualifiedVersionless(); + + myObservationDao.read(obs1id); + myObservationDao.read(obs2id); + + rpt = new DiagnosticReport(); + rpt.addIdentifier().setSystem("foo").setValue("IDENTIFIER"); + + Bundle b = new Bundle(); + b.addEntry().getRequest().setMethod(Bundle.HTTPVerb.DELETE).setUrl("Observation?_has:DiagnosticReport:result:identifier=foo|IDENTIFIER"); + b.addEntry().setResource(rpt).getRequest().setMethod(Bundle.HTTPVerb.PUT).setUrl("DiagnosticReport?identifier=foo|IDENTIFIER"); + mySystemDao.transaction(mySrd, b); + + myObservationDao.read(obs1id); + try { + myObservationDao.read(obs2id); + fail(); + } catch (ResourceGoneException e) { + // good + } + + rpt = myDiagnosticReportDao.read(rptId); + assertThat(rpt.getResult(), empty()); + } + + @Test + public void testDeleteWithId_SourceModifiedToNoLongerIncludeReference() { + + Observation obs1 = new Observation(); + obs1.setStatus(Observation.ObservationStatus.FINAL); + IIdType obs1id = myObservationDao.create(obs1).getId().toUnqualifiedVersionless(); + + Observation obs2 = new Observation(); + obs2.setStatus(Observation.ObservationStatus.FINAL); + IIdType obs2id = myObservationDao.create(obs2).getId().toUnqualifiedVersionless(); + + DiagnosticReport rpt = new DiagnosticReport(); + rpt.addResult(new Reference(obs1id)); + IIdType rptId = myDiagnosticReportDao.create(rpt).getId().toUnqualifiedVersionless(); + + myObservationDao.read(obs1id); + myObservationDao.read(obs2id); + + rpt = new DiagnosticReport(); + rpt.addResult(new Reference(obs2id)); + + Bundle b = new Bundle(); + b.addEntry().getRequest().setMethod(Bundle.HTTPVerb.DELETE).setUrl(obs1id.getValue()); + b.addEntry().setResource(rpt).getRequest().setMethod(Bundle.HTTPVerb.PUT).setUrl(rptId.getValue()); + mySystemDao.transaction(mySrd, b); + + myObservationDao.read(obs2id); + myDiagnosticReportDao.read(rptId); + try { + myObservationDao.read(obs1id); + fail(); + } catch (ResourceGoneException e) { + // good + } + + } + + + @Test + public void testDeleteWithHas_SourceModifiedToStillIncludeReference() { + + Observation obs1 = new Observation(); + obs1.setStatus(Observation.ObservationStatus.FINAL); + IIdType obs1id = myObservationDao.create(obs1).getId().toUnqualifiedVersionless(); + + Observation obs2 = new Observation(); + obs2.setStatus(Observation.ObservationStatus.FINAL); + IIdType obs2id = myObservationDao.create(obs2).getId().toUnqualifiedVersionless(); + + DiagnosticReport rpt = new DiagnosticReport(); + rpt.addIdentifier().setSystem("foo").setValue("IDENTIFIER"); + rpt.addResult(new Reference(obs2id)); + IIdType rptId = myDiagnosticReportDao.create(rpt).getId().toUnqualifiedVersionless(); + + myObservationDao.read(obs1id); + myObservationDao.read(obs2id); + + rpt = new DiagnosticReport(); + rpt.addIdentifier().setSystem("foo").setValue("IDENTIFIER"); + rpt.addResult(new Reference(obs2id)); + + Bundle b = new Bundle(); + b.addEntry().getRequest().setMethod(Bundle.HTTPVerb.DELETE).setUrl("Observation?_has:DiagnosticReport:result:identifier=foo|IDENTIFIER"); + b.addEntry().setResource(rpt).getRequest().setMethod(Bundle.HTTPVerb.PUT).setUrl("DiagnosticReport?identifier=foo|IDENTIFIER"); + try { + mySystemDao.transaction(mySrd, b); + fail(); + } catch (ResourceVersionConflictException e ) { + assertThat(e.getMessage(), matchesPattern("Unable to delete Observation/[0-9]+ because at least one resource has a reference to this resource. First reference found was resource DiagnosticReport/[0-9]+ in path DiagnosticReport.result")); + } + + myObservationDao.read(obs1id); + myObservationDao.read(obs2id); + myDiagnosticReportDao.read(rptId); + } +} From 8099b6634a5d8e90832beda0f55126b04ad0bf67 Mon Sep 17 00:00:00 2001 From: Tadgh Date: Fri, 9 Apr 2021 18:21:15 -0400 Subject: [PATCH 04/30] wip --- .../fhir/jpa/dao/r4/TransactionHookTest.java | 19 +++++-------------- 1 file changed, 5 insertions(+), 14 deletions(-) diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/TransactionHookTest.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/TransactionHookTest.java index 8cb39681e7a..c686ef636c2 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/TransactionHookTest.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/TransactionHookTest.java @@ -40,22 +40,13 @@ public class TransactionHookTest extends BaseJpaR4SystemTest { public void testHookShouldContainParamsForAllCreateUpdateDeleteInvocations() { final Observation obs1 = new Observation(); obs1.setStatus(Observation.ObservationStatus.FINAL); - IIdType obs1id = myObservationDao.create(obs1).getId().toUnqualifiedVersionless(); - - final Observation obs2 = new Observation(); - obs2.setStatus(Observation.ObservationStatus.FINAL); - IIdType obs2id = myObservationDao.create(obs2).getId().toUnqualifiedVersionless(); - - final DiagnosticReport rpt = new DiagnosticReport(); - rpt.addResult(new Reference(obs2id)); - IIdType rptId = myDiagnosticReportDao.create(rpt).getId().toUnqualifiedVersionless(); - - myObservationDao.read(obs1id); - myObservationDao.read(obs2id); - myDiagnosticReportDao.read(rptId); Bundle b = new Bundle(); - b.addEntry().getRequest().setMethod(Bundle.HTTPVerb.DELETE).setUrl(obs2id.getValue()); + Bundle.BundleEntryComponent bundleEntryComponent = b.addEntry(); + bundleEntryComponent.setResource(obs1); + bundleEntryComponent.getRequest().setMethod(Bundle.HTTPVerb.POST); + + try { mySystemDao.transaction(mySrd, b); From f61d52d199639bf1b7a91b9ef071317015bf8d9f Mon Sep 17 00:00:00 2001 From: Tadgh Date: Tue, 13 Apr 2021 09:26:05 -0400 Subject: [PATCH 05/30] Should have realized that transactiondetails is an object reference, so we can't change it effectively --- .../ca/uhn/fhir/interceptor/api/Pointcut.java | 11 ++-- .../ca/uhn/fhir/jpa/dao/BaseHapiFhirDao.java | 3 +- .../fhir/jpa/dao/BaseHapiFhirResourceDao.java | 19 +++--- .../jpa/dao/BaseTransactionProcessor.java | 6 +- .../fhir/jpa/dao/r4/TransactionHookTest.java | 63 ++++++++++++++++--- .../server/storage/TransactionDetails.java | 9 ++- 6 files changed, 84 insertions(+), 27 deletions(-) diff --git a/hapi-fhir-base/src/main/java/ca/uhn/fhir/interceptor/api/Pointcut.java b/hapi-fhir-base/src/main/java/ca/uhn/fhir/interceptor/api/Pointcut.java index ed1b6e7bad2..9551243f5ee 100644 --- a/hapi-fhir-base/src/main/java/ca/uhn/fhir/interceptor/api/Pointcut.java +++ b/hapi-fhir-base/src/main/java/ca/uhn/fhir/interceptor/api/Pointcut.java @@ -1425,7 +1425,8 @@ public enum Pointcut implements IPointcut { "org.hl7.fhir.instance.model.api.IBaseResource", "ca.uhn.fhir.rest.api.server.RequestDetails", "ca.uhn.fhir.rest.server.servlet.ServletRequestDetails", - "ca.uhn.fhir.rest.api.server.storage.TransactionDetails" + "ca.uhn.fhir.rest.api.server.storage.TransactionDetails", + Boolean.class.getName() ), /** @@ -1469,7 +1470,8 @@ public enum Pointcut implements IPointcut { "org.hl7.fhir.instance.model.api.IBaseResource", "ca.uhn.fhir.rest.api.server.RequestDetails", "ca.uhn.fhir.rest.server.servlet.ServletRequestDetails", - "ca.uhn.fhir.rest.api.server.storage.TransactionDetails" + "ca.uhn.fhir.rest.api.server.storage.TransactionDetails", + Boolean.class.getName() ), @@ -1508,7 +1510,8 @@ public enum Pointcut implements IPointcut { "org.hl7.fhir.instance.model.api.IBaseResource", "ca.uhn.fhir.rest.api.server.RequestDetails", "ca.uhn.fhir.rest.server.servlet.ServletRequestDetails", - "ca.uhn.fhir.rest.api.server.storage.TransactionDetails" + "ca.uhn.fhir.rest.api.server.storage.TransactionDetails", + Boolean.class.getName() ), /** @@ -1546,7 +1549,7 @@ public enum Pointcut implements IPointcut { *

*/ STORAGE_TRANSACTION_PROCESSED(void.class, - "org.hl7.fhir.instance.model.api.IBaseResource", + "org.hl7.fhir.instance.model.api.IBaseBundle", "ca.uhn.fhir.rest.api.server.storage.DeferredInterceptorBroadcasts", "ca.uhn.fhir.rest.api.server.RequestDetails", "ca.uhn.fhir.rest.server.servlet.ServletRequestDetails", diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseHapiFhirDao.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseHapiFhirDao.java index 02ef443c876..9bdb1a5466f 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseHapiFhirDao.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseHapiFhirDao.java @@ -1364,7 +1364,8 @@ public abstract class BaseHapiFhirDao extends BaseStora .add(IBaseResource.class, theResource) .add(RequestDetails.class, theRequestDetails) .addIfMatchesType(ServletRequestDetails.class, theRequestDetails) - .add(TransactionDetails.class, theTransactionDetails); + .add(TransactionDetails.class, theTransactionDetails) + .add(Boolean.class, theTransactionDetails.isPointcutDeferred(Pointcut.STORAGE_PRECOMMIT_RESOURCE_CREATED)); doCallHooks(theTransactionDetails, theRequestDetails, Pointcut.STORAGE_PRECOMMIT_RESOURCE_UPDATED, hookParams); } diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseHapiFhirResourceDao.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseHapiFhirResourceDao.java index 2d4661baf01..ab4ccbf268c 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseHapiFhirResourceDao.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseHapiFhirResourceDao.java @@ -376,7 +376,8 @@ public abstract class BaseHapiFhirResourceDao extends B .add(IBaseResource.class, theResource) .add(RequestDetails.class, theRequest) .addIfMatchesType(ServletRequestDetails.class, theRequest) - .add(TransactionDetails.class, theTransactionDetails); + .add(TransactionDetails.class, theTransactionDetails) + .add(Boolean.class, theTransactionDetails.isPointcutDeferred(Pointcut.STORAGE_PRECOMMIT_RESOURCE_CREATED)); doCallHooks(theTransactionDetails, theRequest, Pointcut.STORAGE_PRECOMMIT_RESOURCE_CREATED, hookParams); } @@ -483,13 +484,11 @@ public abstract class BaseHapiFhirResourceDao extends B .add(IBaseResource.class, resourceToDelete) .add(RequestDetails.class, theRequestDetails) .addIfMatchesType(ServletRequestDetails.class, theRequestDetails) - .add(TransactionDetails.class, theTransactionDetails); + .add(TransactionDetails.class, theTransactionDetails) + .add(Boolean.class, theTransactionDetails.isPointcutDeferred(Pointcut.STORAGE_PRECOMMIT_RESOURCE_DELETED)); - if (theTransactionDetails.isAcceptingDeferredInterceptorBroadcasts()) { - theTransactionDetails.addDeferredInterceptorBroadcast(Pointcut.STORAGE_PRECOMMIT_RESOURCE_DELETED, hookParams); - } else { - doCallHooks(theTransactionDetails, theRequestDetails, Pointcut.STORAGE_PRECOMMIT_RESOURCE_DELETED, hookParams); - } + + doCallHooks(theTransactionDetails, theRequestDetails, Pointcut.STORAGE_PRECOMMIT_RESOURCE_DELETED, hookParams); DaoMethodOutcome outcome = toMethodOutcome(theRequestDetails, savedEntity, resourceToDelete).setCreated(true); @@ -597,7 +596,8 @@ public abstract class BaseHapiFhirResourceDao extends B .add(IBaseResource.class, resourceToDelete) .add(RequestDetails.class, theRequest) .addIfMatchesType(ServletRequestDetails.class, theRequest) - .add(TransactionDetails.class, transactionDetails); + .add(TransactionDetails.class, transactionDetails) + .add(Boolean.class, transactionDetails.isPointcutDeferred(Pointcut.STORAGE_PRECOMMIT_RESOURCE_DELETED)); doCallHooks(transactionDetails, theRequest, Pointcut.STORAGE_PRECOMMIT_RESOURCE_DELETED, hookParams); } }); @@ -688,7 +688,8 @@ public abstract class BaseHapiFhirResourceDao extends B .add(IBaseResource.class, newVersion) .add(RequestDetails.class, theRequestDetails) .addIfMatchesType(ServletRequestDetails.class, theRequestDetails) - .add(TransactionDetails.class, theTransactionDetails); + .add(TransactionDetails.class, theTransactionDetails) + .add(Boolean.class, theTransactionDetails.isPointcutDeferred(Pointcut.STORAGE_PRECOMMIT_RESOURCE_UPDATED)); myInterceptorBroadcaster.callHooks(Pointcut.STORAGE_PRESTORAGE_RESOURCE_UPDATED, params); myInterceptorBroadcaster.callHooks(Pointcut.STORAGE_PRECOMMIT_RESOURCE_UPDATED, params); 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 332cfe0b60b..2fbb28bcaa8 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 @@ -1030,13 +1030,13 @@ public abstract class BaseTransactionProcessor { HookParams params = new HookParams() .add(RequestDetails.class, theRequest) .addIfMatchesType(ServletRequestDetails.class, theRequest) - .add(DeferredInterceptorBroadcasts.class, deferredInterceptorBroadcasts); + .add(DeferredInterceptorBroadcasts.class, deferredInterceptorBroadcasts) + .add(TransactionDetails.class, theTransactionDetails) + .add(IBaseBundle.class, theResponse); JpaInterceptorBroadcaster.doCallHooks(myInterceptorBroadcaster, theRequest, Pointcut.STORAGE_TRANSACTION_PROCESSED, params); theTransactionDetails.deferredBroadcastProcessingFinished(); - - //finishedCallingDeferredInterceptorBroadcasts return entriesToProcess; diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/TransactionHookTest.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/TransactionHookTest.java index c686ef636c2..e52cfbbb9c4 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/TransactionHookTest.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/TransactionHookTest.java @@ -1,23 +1,34 @@ package ca.uhn.fhir.jpa.dao.r4; +import ca.uhn.fhir.interceptor.api.HookParams; import ca.uhn.fhir.interceptor.api.IInterceptorService; import ca.uhn.fhir.interceptor.api.Pointcut; +import ca.uhn.fhir.jpa.api.model.DaoMethodOutcome; +import ca.uhn.fhir.rest.api.server.storage.DeferredInterceptorBroadcasts; import ca.uhn.fhir.rest.server.exceptions.ResourceGoneException; import ca.uhn.fhir.rest.server.exceptions.ResourceVersionConflictException; import ca.uhn.test.concurrency.PointcutLatch; +import com.google.common.collect.ListMultimap; +import org.hl7.fhir.instance.model.api.IBaseBundle; +import org.hl7.fhir.instance.model.api.IBaseResource; import org.hl7.fhir.instance.model.api.IIdType; import org.hl7.fhir.r4.model.Bundle; import org.hl7.fhir.r4.model.DiagnosticReport; import org.hl7.fhir.r4.model.Observation; +import org.hl7.fhir.r4.model.Patient; import org.hl7.fhir.r4.model.Reference; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.springframework.beans.factory.annotation.Autowired; +import java.util.List; + import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.empty; +import static org.hamcrest.Matchers.hasSize; import static org.hamcrest.Matchers.matchesPattern; +import static org.junit.jupiter.api.Assertions.assertTrue; import static org.junit.jupiter.api.Assertions.fail; public class TransactionHookTest extends BaseJpaR4SystemTest { @@ -34,26 +45,62 @@ public class TransactionHookTest extends BaseJpaR4SystemTest { @BeforeEach public void beforeEach() { myInterceptorService.registerAnonymousInterceptor(Pointcut.STORAGE_TRANSACTION_PROCESSED, myPointcutLatch); + myInterceptorService.registerAnonymousInterceptor(Pointcut.STORAGE_PRECOMMIT_RESOURCE_CREATED, myPointcutLatch); + myInterceptorService.registerAnonymousInterceptor(Pointcut.STORAGE_PRECOMMIT_RESOURCE_UPDATED, myPointcutLatch); + myInterceptorService.registerAnonymousInterceptor(Pointcut.STORAGE_PRECOMMIT_RESOURCE_DELETED, myPointcutLatch); } @Test - public void testHookShouldContainParamsForAllCreateUpdateDeleteInvocations() { + public void testHookShouldContainParamsForAllCreateUpdateDeleteInvocations() throws InterruptedException { + + String urnReference = "urn:uuid:3bc44de3-069d-442d-829b-f3ef68cae371"; + + final Observation obsToDelete = new Observation(); + obsToDelete.setStatus(Observation.ObservationStatus.FINAL); + DaoMethodOutcome daoMethodOutcome = myObservationDao.create(obsToDelete); + + Patient pat1 = new Patient(); + final Observation obs1 = new Observation(); obs1.setStatus(Observation.ObservationStatus.FINAL); + obs1.setSubject(new Reference(urnReference)); Bundle b = new Bundle(); Bundle.BundleEntryComponent bundleEntryComponent = b.addEntry(); + bundleEntryComponent.setResource(obs1); - bundleEntryComponent.getRequest().setMethod(Bundle.HTTPVerb.POST); + bundleEntryComponent.getRequest().setMethod(Bundle.HTTPVerb.POST).setUrl("Observation"); + + Bundle.BundleEntryComponent patientComponent = b.addEntry(); + patientComponent.setFullUrl(urnReference); + patientComponent.setResource(pat1); + patientComponent.getRequest().setMethod(Bundle.HTTPVerb.POST).setUrl("Patient"); - try { - mySystemDao.transaction(mySrd, b); - fail(); - } catch (ResourceVersionConflictException e) { - // good, transaction should not succeed because DiagnosticReport has a reference to the obs2 - } + //Delete an observation + b.addEntry().getRequest().setMethod(Bundle.HTTPVerb.DELETE).setUrl(daoMethodOutcome.getId().toUnqualifiedVersionless().getValue()); + + + myPointcutLatch.setExpectedCount(4); + mySystemDao.transaction(mySrd, b); + List hookParams = myPointcutLatch.awaitExpected(); + + IBaseBundle bundleResponseParam = hookParams.get(3).get(IBaseBundle.class); + DeferredInterceptorBroadcasts broadcastsParam = hookParams.get(0).get(DeferredInterceptorBroadcasts.class); + ListMultimap deferredInterceptorBroadcasts = broadcastsParam.getDeferredInterceptorBroadcasts(); + assertThat(deferredInterceptorBroadcasts.entries(), hasSize(3)); + + List createPointcutInvocations = deferredInterceptorBroadcasts.get(Pointcut.STORAGE_PRECOMMIT_RESOURCE_CREATED); + assertThat(createPointcutInvocations, hasSize(2)); + + IBaseResource firstCreatedResource = createPointcutInvocations.get(0).get(IBaseResource.class); + assertTrue(firstCreatedResource instanceof Observation); + + IBaseResource secondCreatedResource = createPointcutInvocations.get(1).get(IBaseResource.class); + assertTrue(secondCreatedResource instanceof Patient); + + assertThat(deferredInterceptorBroadcasts.get(Pointcut.STORAGE_PRECOMMIT_RESOURCE_DELETED), hasSize(1)); } @Test diff --git a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/api/server/storage/TransactionDetails.java b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/api/server/storage/TransactionDetails.java index 76a28163313..ac8794b7e00 100644 --- a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/api/server/storage/TransactionDetails.java +++ b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/api/server/storage/TransactionDetails.java @@ -32,6 +32,7 @@ import java.util.Collections; import java.util.Date; import java.util.EnumSet; import java.util.HashMap; +import java.util.List; import java.util.Map; import java.util.function.Supplier; @@ -194,8 +195,12 @@ public class TransactionDetails { myDeferredInterceptorBroadcasts.put(thePointcut, theHookParams); } - public boolean isPointcutDeferred() { - return myIsPointcutDeferred; + public Boolean isPointcutDeferred(Pointcut thePointcut) { + if (myDeferredInterceptorBroadcasts == null) { + return false; + } + List hookParams = myDeferredInterceptorBroadcasts.get(thePointcut); + return hookParams != null; } public void deferredBroadcastProcessingFinished() { From d695e35e59a0e331425054b589a77cfe969d64e5 Mon Sep 17 00:00:00 2001 From: Tadgh Date: Tue, 13 Apr 2021 09:31:45 -0400 Subject: [PATCH 06/30] Update test --- .../java/ca/uhn/fhir/jpa/dao/r4/TransactionHookTest.java | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/TransactionHookTest.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/TransactionHookTest.java index e52cfbbb9c4..9b31ae1bce3 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/TransactionHookTest.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/TransactionHookTest.java @@ -9,7 +9,6 @@ import ca.uhn.fhir.rest.server.exceptions.ResourceGoneException; import ca.uhn.fhir.rest.server.exceptions.ResourceVersionConflictException; import ca.uhn.test.concurrency.PointcutLatch; import com.google.common.collect.ListMultimap; -import org.hl7.fhir.instance.model.api.IBaseBundle; import org.hl7.fhir.instance.model.api.IBaseResource; import org.hl7.fhir.instance.model.api.IIdType; import org.hl7.fhir.r4.model.Bundle; @@ -77,7 +76,6 @@ public class TransactionHookTest extends BaseJpaR4SystemTest { patientComponent.getRequest().setMethod(Bundle.HTTPVerb.POST).setUrl("Patient"); - //Delete an observation b.addEntry().getRequest().setMethod(Bundle.HTTPVerb.DELETE).setUrl(daoMethodOutcome.getId().toUnqualifiedVersionless().getValue()); @@ -86,8 +84,7 @@ public class TransactionHookTest extends BaseJpaR4SystemTest { mySystemDao.transaction(mySrd, b); List hookParams = myPointcutLatch.awaitExpected(); - IBaseBundle bundleResponseParam = hookParams.get(3).get(IBaseBundle.class); - DeferredInterceptorBroadcasts broadcastsParam = hookParams.get(0).get(DeferredInterceptorBroadcasts.class); + DeferredInterceptorBroadcasts broadcastsParam = hookParams.get(3).get(DeferredInterceptorBroadcasts.class); ListMultimap deferredInterceptorBroadcasts = broadcastsParam.getDeferredInterceptorBroadcasts(); assertThat(deferredInterceptorBroadcasts.entries(), hasSize(3)); @@ -95,10 +92,14 @@ public class TransactionHookTest extends BaseJpaR4SystemTest { assertThat(createPointcutInvocations, hasSize(2)); IBaseResource firstCreatedResource = createPointcutInvocations.get(0).get(IBaseResource.class); + boolean wasDeferred = createPointcutInvocations.get(0).get(Boolean.class); assertTrue(firstCreatedResource instanceof Observation); + assertTrue(wasDeferred); IBaseResource secondCreatedResource = createPointcutInvocations.get(1).get(IBaseResource.class); + wasDeferred = createPointcutInvocations.get(1).get(Boolean.class); assertTrue(secondCreatedResource instanceof Patient); + assertTrue(wasDeferred); assertThat(deferredInterceptorBroadcasts.get(Pointcut.STORAGE_PRECOMMIT_RESOURCE_DELETED), hasSize(1)); } From a0c11d7c667b264f5eb42254e5750420121ec5ac Mon Sep 17 00:00:00 2001 From: Tadgh Date: Tue, 13 Apr 2021 10:04:45 -0400 Subject: [PATCH 07/30] Update docs --- .../main/java/ca/uhn/fhir/interceptor/api/Pointcut.java | 9 +++++++++ .../uhn/hapi/fhir/changelog/5_4_0/2534-new-pointcut.yaml | 4 ++++ 2 files changed, 13 insertions(+) create mode 100644 hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/5_4_0/2534-new-pointcut.yaml diff --git a/hapi-fhir-base/src/main/java/ca/uhn/fhir/interceptor/api/Pointcut.java b/hapi-fhir-base/src/main/java/ca/uhn/fhir/interceptor/api/Pointcut.java index 9551243f5ee..cd459aef3cc 100644 --- a/hapi-fhir-base/src/main/java/ca/uhn/fhir/interceptor/api/Pointcut.java +++ b/hapi-fhir-base/src/main/java/ca/uhn/fhir/interceptor/api/Pointcut.java @@ -1416,6 +1416,9 @@ public enum Pointcut implements IPointcut { *
  • * ca.uhn.fhir.rest.api.server.storage.TransactionDetails - The outer transaction details object (since 5.0.0) *
    • + *
  • + * Boolean - Whether this pointcut invocation was deferred or not(since 5.4.0) + *
    • * *

    * Hooks should return void. @@ -1460,6 +1463,9 @@ public enum Pointcut implements IPointcut { *

  • * ca.uhn.fhir.rest.api.server.storage.TransactionDetails - The outer transaction details object (since 5.0.0) *
    • + *
  • + * Boolean - Whether this pointcut invocation was deferred or not(since 5.4.0) + *
    • * *

    * Hooks should return void. @@ -1501,6 +1507,9 @@ public enum Pointcut implements IPointcut { *

  • * ca.uhn.fhir.rest.api.server.storage.TransactionDetails - The outer transaction details object (since 5.0.0) *
    • + *
  • + * Boolean - Whether this pointcut invocation was deferred or not(since 5.4.0) + *
    • * *

    * Hooks should return void. diff --git a/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/5_4_0/2534-new-pointcut.yaml b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/5_4_0/2534-new-pointcut.yaml new file mode 100644 index 00000000000..cc4e0776fe5 --- /dev/null +++ b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/5_4_0/2534-new-pointcut.yaml @@ -0,0 +1,4 @@ +--- +type: add +issue: 2534 +title: "Add new pointcut `STORAGE_TRANSACTION_PROCESSED`, which fires after all operations in a transaction have executed." From 346c9d783cfc9f391c8ecfcb7e0057a30f8375bb Mon Sep 17 00:00:00 2001 From: Tadgh Date: Tue, 13 Apr 2021 10:36:17 -0400 Subject: [PATCH 08/30] Update tests to reflect new reality --- .../uhn/fhir/interceptor/executor/InterceptorServiceTest.java | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/hapi-fhir-base/src/test/java/ca/uhn/fhir/interceptor/executor/InterceptorServiceTest.java b/hapi-fhir-base/src/test/java/ca/uhn/fhir/interceptor/executor/InterceptorServiceTest.java index 8799cd1608b..fd180b9ab75 100644 --- a/hapi-fhir-base/src/test/java/ca/uhn/fhir/interceptor/executor/InterceptorServiceTest.java +++ b/hapi-fhir-base/src/test/java/ca/uhn/fhir/interceptor/executor/InterceptorServiceTest.java @@ -455,11 +455,12 @@ public class InterceptorServiceTest { params.add(String.class, "D"); params.add(String.class, "E"); params.add(String.class, "F"); + params.add(String.class, "G"); try { svc.haveAppropriateParams(Pointcut.STORAGE_PRECOMMIT_RESOURCE_UPDATED, params); fail(); } catch (IllegalArgumentException e) { - assertEquals("Wrong number of params for pointcut STORAGE_PRECOMMIT_RESOURCE_UPDATED - Wanted ca.uhn.fhir.rest.api.server.RequestDetails,ca.uhn.fhir.rest.api.server.storage.TransactionDetails,ca.uhn.fhir.rest.server.servlet.ServletRequestDetails,org.hl7.fhir.instance.model.api.IBaseResource,org.hl7.fhir.instance.model.api.IBaseResource but found [String, String, String, String, String, String]", e.getMessage()); + assertEquals("Wrong number of params for pointcut STORAGE_PRECOMMIT_RESOURCE_UPDATED - Wanted ca.uhn.fhir.rest.api.server.RequestDetails,ca.uhn.fhir.rest.api.server.storage.TransactionDetails,ca.uhn.fhir.rest.server.servlet.ServletRequestDetails,java.lang.Boolean,org.hl7.fhir.instance.model.api.IBaseResource,org.hl7.fhir.instance.model.api.IBaseResource but found [String, String, String, String, String, String, String]", e.getMessage()); } } @@ -474,6 +475,7 @@ public class InterceptorServiceTest { params.add((Class) String.class, 3); params.add((Class) String.class, 4); params.add((Class) String.class, 5); + params.add((Class) String.class, 6); try { svc.haveAppropriateParams(Pointcut.STORAGE_PRECOMMIT_RESOURCE_UPDATED, params); fail(); From 63c9e892b606f48dd274d0a6dccd324a46214a5f Mon Sep 17 00:00:00 2001 From: Tadgh Date: Tue, 13 Apr 2021 11:12:26 -0400 Subject: [PATCH 09/30] Fix up the hashmap provider --- .../provider/HashMapResourceProvider.java | 29 +++++++++++++++---- 1 file changed, 23 insertions(+), 6 deletions(-) diff --git a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/provider/HashMapResourceProvider.java b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/provider/HashMapResourceProvider.java index 23223aa09df..421210bf5db 100644 --- a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/provider/HashMapResourceProvider.java +++ b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/provider/HashMapResourceProvider.java @@ -387,25 +387,42 @@ public class HashMapResourceProvider implements IResour if (!myIdToHistory.containsKey(theIdPart)) { // Interceptor call: STORAGE_PRESTORAGE_RESOURCE_CREATED - HookParams params = new HookParams() + HookParams preStorageParams = new HookParams() .add(RequestDetails.class, theRequestDetails) .addIfMatchesType(ServletRequestDetails.class, theRequestDetails) .add(IBaseResource.class, theResource) .add(TransactionDetails.class, theTransactionDetails); - interceptorBroadcaster.callHooks(Pointcut.STORAGE_PRESTORAGE_RESOURCE_CREATED, params); - interceptorBroadcaster.callHooks(Pointcut.STORAGE_PRECOMMIT_RESOURCE_CREATED, params); + interceptorBroadcaster.callHooks(Pointcut.STORAGE_PRESTORAGE_RESOURCE_CREATED, preStorageParams); + + // Interceptor call: STORAGE_PRECOMMIT_RESOURCE_CREATED + HookParams preCommitParams = new HookParams() + .add(RequestDetails.class, theRequestDetails) + .addIfMatchesType(ServletRequestDetails.class, theRequestDetails) + .add(IBaseResource.class, theResource) + .add(TransactionDetails.class, theTransactionDetails) + .add(Boolean.class, theTransactionDetails.isPointcutDeferred(Pointcut.STORAGE_PRECOMMIT_RESOURCE_CREATED)); + interceptorBroadcaster.callHooks(Pointcut.STORAGE_PRECOMMIT_RESOURCE_CREATED, preCommitParams); } else { // Interceptor call: STORAGE_PRESTORAGE_RESOURCE_UPDATED - HookParams params = new HookParams() + HookParams preStorageParams = new HookParams() .add(RequestDetails.class, theRequestDetails) .addIfMatchesType(ServletRequestDetails.class, theRequestDetails) .add(IBaseResource.class, myIdToHistory.get(theIdPart).getFirst()) .add(IBaseResource.class, theResource) .add(TransactionDetails.class, theTransactionDetails); - interceptorBroadcaster.callHooks(Pointcut.STORAGE_PRESTORAGE_RESOURCE_UPDATED, params); - interceptorBroadcaster.callHooks(Pointcut.STORAGE_PRECOMMIT_RESOURCE_UPDATED, params); + interceptorBroadcaster.callHooks(Pointcut.STORAGE_PRESTORAGE_RESOURCE_UPDATED, preStorageParams); + + // Interceptor call: STORAGE_PRECOMMIT_RESOURCE_UPDATED + HookParams preCommitParams = new HookParams() + .add(RequestDetails.class, theRequestDetails) + .addIfMatchesType(ServletRequestDetails.class, theRequestDetails) + .add(IBaseResource.class, myIdToHistory.get(theIdPart).getFirst()) + .add(IBaseResource.class, theResource) + .add(TransactionDetails.class, theTransactionDetails) + .add(Boolean.class, theTransactionDetails.isPointcutDeferred(Pointcut.STORAGE_PRECOMMIT_RESOURCE_CREATED)); + interceptorBroadcaster.callHooks(Pointcut.STORAGE_PRECOMMIT_RESOURCE_UPDATED, preCommitParams); } } From de3d6cd36a4e98bc95813eb78d829f90116f3393 Mon Sep 17 00:00:00 2001 From: Tadgh Date: Tue, 13 Apr 2021 13:49:50 -0400 Subject: [PATCH 10/30] Update pointcut invocations --- .../uhn/fhir/jpa/dao/BaseHapiFhirResourceDao.java | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseHapiFhirResourceDao.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseHapiFhirResourceDao.java index ab4ccbf268c..53f95cbbde6 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseHapiFhirResourceDao.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseHapiFhirResourceDao.java @@ -721,14 +721,23 @@ public abstract class BaseHapiFhirResourceDao extends B // Interceptor call: STORAGE_PRECOMMIT_RESOURCE_UPDATED IBaseResource newVersion = toResource(theEntity, false); - HookParams params = new HookParams() + HookParams preStorageParams = new HookParams() .add(IBaseResource.class, oldVersion) .add(IBaseResource.class, newVersion) .add(RequestDetails.class, theRequestDetails) .addIfMatchesType(ServletRequestDetails.class, theRequestDetails) .add(TransactionDetails.class, theTransactionDetails); - myInterceptorBroadcaster.callHooks(Pointcut.STORAGE_PRESTORAGE_RESOURCE_UPDATED, params); - myInterceptorBroadcaster.callHooks(Pointcut.STORAGE_PRECOMMIT_RESOURCE_UPDATED, params); + myInterceptorBroadcaster.callHooks(Pointcut.STORAGE_PRESTORAGE_RESOURCE_UPDATED, preStorageParams); + + HookParams preCommitParams = new HookParams() + .add(IBaseResource.class, oldVersion) + .add(IBaseResource.class, newVersion) + .add(RequestDetails.class, theRequestDetails) + .addIfMatchesType(ServletRequestDetails.class, theRequestDetails) + .add(TransactionDetails.class, theTransactionDetails) + .add(Boolean.class, theTransactionDetails.isPointcutDeferred(Pointcut.STORAGE_PRECOMMIT_RESOURCE_UPDATED)); + + myInterceptorBroadcaster.callHooks(Pointcut.STORAGE_PRECOMMIT_RESOURCE_UPDATED, preCommitParams); } From 3732698e01a6b167245d30e1b4014c2ab0120fe9 Mon Sep 17 00:00:00 2001 From: Tadgh Date: Tue, 13 Apr 2021 13:50:53 -0400 Subject: [PATCH 11/30] Update a few more pointcuts --- .../uhn/fhir/jpa/dao/BaseHapiFhirResourceDao.java | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseHapiFhirResourceDao.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseHapiFhirResourceDao.java index 53f95cbbde6..8c49bccb759 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseHapiFhirResourceDao.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseHapiFhirResourceDao.java @@ -681,17 +681,24 @@ public abstract class BaseHapiFhirResourceDao extends B myEntityManager.merge(theEntity); // Interceptor call: STORAGE_PRECOMMIT_RESOURCE_UPDATED - // Interceptor call: STORAGE_PRESTORAGE_RESOURCE_UPDATED IBaseResource newVersion = toResource(theEntity, false); - HookParams params = new HookParams() + HookParams preStorageParams = new HookParams() + .add(IBaseResource.class, oldVersion) + .add(IBaseResource.class, newVersion) + .add(RequestDetails.class, theRequestDetails) + .addIfMatchesType(ServletRequestDetails.class, theRequestDetails) + .add(TransactionDetails.class, theTransactionDetails); + myInterceptorBroadcaster.callHooks(Pointcut.STORAGE_PRESTORAGE_RESOURCE_UPDATED, preStorageParams); + + // Interceptor call: STORAGE_PRECOMMIT_RESOURCE_UPDATED + HookParams preCommitParams = new HookParams() .add(IBaseResource.class, oldVersion) .add(IBaseResource.class, newVersion) .add(RequestDetails.class, theRequestDetails) .addIfMatchesType(ServletRequestDetails.class, theRequestDetails) .add(TransactionDetails.class, theTransactionDetails) .add(Boolean.class, theTransactionDetails.isPointcutDeferred(Pointcut.STORAGE_PRECOMMIT_RESOURCE_UPDATED)); - myInterceptorBroadcaster.callHooks(Pointcut.STORAGE_PRESTORAGE_RESOURCE_UPDATED, params); - myInterceptorBroadcaster.callHooks(Pointcut.STORAGE_PRECOMMIT_RESOURCE_UPDATED, params); + myInterceptorBroadcaster.callHooks(Pointcut.STORAGE_PRECOMMIT_RESOURCE_UPDATED, preCommitParams); } From fef972602642def2d41fe31745a63fbab8264041 Mon Sep 17 00:00:00 2001 From: Tadgh Date: Wed, 14 Apr 2021 12:52:30 -0400 Subject: [PATCH 12/30] Fix typo in docs --- .../src/main/java/ca/uhn/fhir/interceptor/api/Pointcut.java | 2 +- .../src/main/java/ca/uhn/fhir/util/BundleBuilder.java | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/hapi-fhir-base/src/main/java/ca/uhn/fhir/interceptor/api/Pointcut.java b/hapi-fhir-base/src/main/java/ca/uhn/fhir/interceptor/api/Pointcut.java index f14b338f78f..a475de9b98a 100644 --- a/hapi-fhir-base/src/main/java/ca/uhn/fhir/interceptor/api/Pointcut.java +++ b/hapi-fhir-base/src/main/java/ca/uhn/fhir/interceptor/api/Pointcut.java @@ -1450,7 +1450,7 @@ public enum Pointcut implements IPointcut { * Hooks may accept the following parameters: *

      *
    • org.hl7.fhir.instance.model.api.IBaseResource - The previous contents of the resource
      • - *
    • org.hl7.fhir.instance.model.api.IBaseResource - The proposed new new contents of the resource
      • + *
    • org.hl7.fhir.instance.model.api.IBaseResource - The proposed new contents of the resource
      • *
    • * ca.uhn.fhir.rest.api.server.RequestDetails - A bean containing details about the request that is about to be processed, including details such as the * resource type and logical ID (if any) and other FHIR-specific aspects of the request which have been diff --git a/hapi-fhir-base/src/main/java/ca/uhn/fhir/util/BundleBuilder.java b/hapi-fhir-base/src/main/java/ca/uhn/fhir/util/BundleBuilder.java index ca79ffb009b..1a20f1edbc7 100644 --- a/hapi-fhir-base/src/main/java/ca/uhn/fhir/util/BundleBuilder.java +++ b/hapi-fhir-base/src/main/java/ca/uhn/fhir/util/BundleBuilder.java @@ -352,7 +352,6 @@ public class BundleBuilder { public void conditional(String theConditionalUrl) { myUrl.setValueAsString(theConditionalUrl); } - } public class CreateBuilder { From 0bc61b72f7be625f0951c10891a7ff2f540a50f2 Mon Sep 17 00:00:00 2001 From: Tadgh Date: Wed, 14 Apr 2021 14:04:00 -0400 Subject: [PATCH 13/30] Add new operation type for transcations --- .../ca/uhn/fhir/rest/server/messaging/BaseResourceMessage.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/messaging/BaseResourceMessage.java b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/messaging/BaseResourceMessage.java index f1ce661c719..dd7f266afb2 100644 --- a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/messaging/BaseResourceMessage.java +++ b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/messaging/BaseResourceMessage.java @@ -160,6 +160,7 @@ public abstract class BaseResourceMessage implements IResourceMessage, IModelJso CREATE, UPDATE, DELETE, - MANUALLY_TRIGGERED + MANUALLY_TRIGGERED, + TRANSACTION } } From 1670ed720249bf54c126808005a380b47ed33a9f Mon Sep 17 00:00:00 2001 From: Tadgh Date: Mon, 19 Apr 2021 20:01:02 -0400 Subject: [PATCH 14/30] Begin topological sort implementation --- .../java/ca/uhn/fhir/util/BundleUtil.java | 35 ++++++++++++++++ .../fhir/jpa/dao/r4/TransactionHookTest.java | 42 +++++++++++++++++++ 2 files changed, 77 insertions(+) diff --git a/hapi-fhir-base/src/main/java/ca/uhn/fhir/util/BundleUtil.java b/hapi-fhir-base/src/main/java/ca/uhn/fhir/util/BundleUtil.java index d48969ae5ca..1d48b6a8fa9 100644 --- a/hapi-fhir-base/src/main/java/ca/uhn/fhir/util/BundleUtil.java +++ b/hapi-fhir-base/src/main/java/ca/uhn/fhir/util/BundleUtil.java @@ -12,6 +12,8 @@ import org.apache.commons.lang3.tuple.Pair; import org.hl7.fhir.instance.model.api.*; import java.util.ArrayList; +import java.util.Arrays; +import java.util.HashMap; import java.util.List; import java.util.Objects; import java.util.function.Consumer; @@ -170,6 +172,39 @@ public class BundleUtil { return entryListAccumulator.getList(); } + static int WHITE = 1; + static int GRAY = 2; + static int BLACK = 3; + + public static IBaseBundle topologicalSort(FhirContext theContext, IBaseBundle theBundle) { + boolean isPossible = true; + HashMap color = new HashMap(); + HashMap> adjList = new HashMap<>(); + List topologicalOrder = new ArrayList<>(); + + List> prerequisites = new ArrayList<>(); + + List bundleEntryParts = toListOfEntries(theContext, theBundle); + for (BundleEntryParts bundleEntryPart : bundleEntryParts) { + IBaseResource resource = bundleEntryPart.getResource(); + String resourceId = resource.getIdElement().toString(); + if (resourceId == null) { + if (bundleEntryPart.getFullUrl() != null) { + resourceId = bundleEntryPart.getFullUrl(); + } + } + List allResourceReferences = theContext.newTerser().getAllResourceReferences(resource); + String finalResourceId = resourceId; + allResourceReferences + .forEach(refInfo -> { + prerequisites.add(Arrays.asList(finalResourceId, refInfo.getResourceReference().getReferenceElement().getValue())); + }); + + } + System.out.println("zoop!!"); + return null; + } + public static void processEntries(FhirContext theContext, IBaseBundle theBundle, Consumer theProcessor) { RuntimeResourceDefinition bundleDef = theContext.getResourceDefinition(theBundle); diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/TransactionHookTest.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/TransactionHookTest.java index 9b31ae1bce3..57e4ae32ec6 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/TransactionHookTest.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/TransactionHookTest.java @@ -7,6 +7,7 @@ import ca.uhn.fhir.jpa.api.model.DaoMethodOutcome; import ca.uhn.fhir.rest.api.server.storage.DeferredInterceptorBroadcasts; import ca.uhn.fhir.rest.server.exceptions.ResourceGoneException; import ca.uhn.fhir.rest.server.exceptions.ResourceVersionConflictException; +import ca.uhn.fhir.util.BundleUtil; import ca.uhn.test.concurrency.PointcutLatch; import com.google.common.collect.ListMultimap; import org.hl7.fhir.instance.model.api.IBaseResource; @@ -14,7 +15,9 @@ import org.hl7.fhir.instance.model.api.IIdType; import org.hl7.fhir.r4.model.Bundle; import org.hl7.fhir.r4.model.DiagnosticReport; import org.hl7.fhir.r4.model.Observation; +import org.hl7.fhir.r4.model.Organization; import org.hl7.fhir.r4.model.Patient; +import org.hl7.fhir.r4.model.Quantity; import org.hl7.fhir.r4.model.Reference; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; @@ -41,6 +44,7 @@ public class TransactionHookTest extends BaseJpaR4SystemTest { @Autowired private IInterceptorService myInterceptorService; + @BeforeEach public void beforeEach() { myInterceptorService.registerAnonymousInterceptor(Pointcut.STORAGE_TRANSACTION_PROCESSED, myPointcutLatch); @@ -49,6 +53,44 @@ public class TransactionHookTest extends BaseJpaR4SystemTest { myInterceptorService.registerAnonymousInterceptor(Pointcut.STORAGE_PRECOMMIT_RESOURCE_DELETED, myPointcutLatch); } + @Test + public void testTopologicalTransactionSorting() { + + Bundle b = new Bundle(); + Bundle.BundleEntryComponent bundleEntryComponent = b.addEntry(); + final Observation obs1 = new Observation(); + obs1.setStatus(Observation.ObservationStatus.FINAL); + obs1.setSubject(new Reference("Patient/P1")); + obs1.setValue(new Quantity(4)); + obs1.setId("Observation/O1"); + bundleEntryComponent.setResource(obs1); + bundleEntryComponent.getRequest().setMethod(Bundle.HTTPVerb.POST).setUrl("Observation"); + + bundleEntryComponent = b.addEntry(); + final Observation obs2 = new Observation(); + obs2.setStatus(Observation.ObservationStatus.FINAL); + obs2.setValue(new Quantity(4)); + obs2.setId("Observation/O2"); + bundleEntryComponent.setResource(obs2); + bundleEntryComponent.getRequest().setMethod(Bundle.HTTPVerb.POST).setUrl("Observation"); + + Bundle.BundleEntryComponent patientComponent = b.addEntry(); + Patient pat1 = new Patient(); + pat1.setId("Patient/P1"); + pat1.setManagingOrganization(new Reference("Organization/Org1")); + patientComponent.setResource(pat1); + patientComponent.getRequest().setMethod(Bundle.HTTPVerb.POST).setUrl("Patient"); + + Bundle.BundleEntryComponent organizationComponent = b.addEntry(); + Organization org1 = new Organization(); + org1.setId("Organization/Org1"); + organizationComponent.setResource(org1); + organizationComponent.getRequest().setMethod(Bundle.HTTPVerb.POST).setUrl("Patient"); + + BundleUtil.topologicalSort(myFhirCtx, b); + + } + @Test public void testHookShouldContainParamsForAllCreateUpdateDeleteInvocations() throws InterruptedException { From ff2690b74e4db4d593134755e4e9a1dc8c8eb2b2 Mon Sep 17 00:00:00 2001 From: Tadgh Date: Tue, 20 Apr 2021 14:12:25 -0400 Subject: [PATCH 15/30] Add tests for transaction sorting --- .../java/ca/uhn/fhir/util/BundleUtil.java | 115 ++++++++++++++++-- .../fhir/util/bundle/BundleEntryParts.java | 1 - .../fhir/jpa/dao/r4/TransactionHookTest.java | 100 ++++++++++++++- 3 files changed, 200 insertions(+), 16 deletions(-) diff --git a/hapi-fhir-base/src/main/java/ca/uhn/fhir/util/BundleUtil.java b/hapi-fhir-base/src/main/java/ca/uhn/fhir/util/BundleUtil.java index 1d48b6a8fa9..471c317b8da 100644 --- a/hapi-fhir-base/src/main/java/ca/uhn/fhir/util/BundleUtil.java +++ b/hapi-fhir-base/src/main/java/ca/uhn/fhir/util/BundleUtil.java @@ -1,6 +1,10 @@ package ca.uhn.fhir.util; -import ca.uhn.fhir.context.*; +import ca.uhn.fhir.context.BaseRuntimeChildDefinition; +import ca.uhn.fhir.context.BaseRuntimeElementCompositeDefinition; +import ca.uhn.fhir.context.BaseRuntimeElementDefinition; +import ca.uhn.fhir.context.FhirContext; +import ca.uhn.fhir.context.RuntimeResourceDefinition; import ca.uhn.fhir.rest.api.PatchTypeEnum; import ca.uhn.fhir.rest.api.RequestTypeEnum; import ca.uhn.fhir.rest.server.exceptions.InvalidRequestException; @@ -9,17 +13,21 @@ import ca.uhn.fhir.util.bundle.BundleEntryParts; import ca.uhn.fhir.util.bundle.EntryListAccumulator; import ca.uhn.fhir.util.bundle.ModifiableBundleEntry; import org.apache.commons.lang3.tuple.Pair; -import org.hl7.fhir.instance.model.api.*; +import org.hl7.fhir.instance.model.api.IBase; +import org.hl7.fhir.instance.model.api.IBaseBinary; +import org.hl7.fhir.instance.model.api.IBaseBundle; +import org.hl7.fhir.instance.model.api.IBaseResource; +import org.hl7.fhir.instance.model.api.IPrimitiveType; import java.util.ArrayList; -import java.util.Arrays; +import java.util.Collections; import java.util.HashMap; import java.util.List; +import java.util.Map; import java.util.Objects; import java.util.function.Consumer; import static org.apache.commons.lang3.StringUtils.isNotBlank; - /* * #%L * HAPI FHIR - Core Library @@ -44,6 +52,8 @@ import static org.apache.commons.lang3.StringUtils.isNotBlank; * Fetch resources from a bundle */ public class BundleUtil { + private static final org.slf4j.Logger ourLog = org.slf4j.LoggerFactory.getLogger(BundleUtil.class); + /** * @return Returns null if the link isn't found or has no value @@ -176,18 +186,31 @@ public class BundleUtil { static int GRAY = 2; static int BLACK = 3; - public static IBaseBundle topologicalSort(FhirContext theContext, IBaseBundle theBundle) { - boolean isPossible = true; + public static List topologicalSort(FhirContext theContext, IBaseBundle theBundle, RequestTypeEnum theRequestTypeEnum) { + SortLegality legality = new SortLegality(); HashMap color = new HashMap(); HashMap> adjList = new HashMap<>(); List topologicalOrder = new ArrayList<>(); - - List> prerequisites = new ArrayList<>(); - List bundleEntryParts = toListOfEntries(theContext, theBundle); + bundleEntryParts.removeIf(bep -> !bep.getRequestType().equals(theRequestTypeEnum)); + HashMap resourceIdToBundleEntryMap = new HashMap<>(); + for (BundleEntryParts bundleEntryPart : bundleEntryParts) { IBaseResource resource = bundleEntryPart.getResource(); String resourceId = resource.getIdElement().toString(); + resourceIdToBundleEntryMap.put(resourceId, bundleEntryPart); + if (resourceId == null) { + if (bundleEntryPart.getFullUrl() != null) { + resourceId = bundleEntryPart.getFullUrl(); + } + } + color.put(resourceId, WHITE); + } + + for (BundleEntryParts bundleEntryPart : bundleEntryParts) { + IBaseResource resource = bundleEntryPart.getResource(); + String resourceId = resource.getIdElement().toString(); + resourceIdToBundleEntryMap.put(resourceId, bundleEntryPart); if (resourceId == null) { if (bundleEntryPart.getFullUrl() != null) { resourceId = bundleEntryPart.getFullUrl(); @@ -197,14 +220,80 @@ public class BundleUtil { String finalResourceId = resourceId; allResourceReferences .forEach(refInfo -> { - prerequisites.add(Arrays.asList(finalResourceId, refInfo.getResourceReference().getReferenceElement().getValue())); + String referencedResourceId = refInfo.getResourceReference().getReferenceElement().getValue(); + if (color.containsKey(referencedResourceId)) { + if (!adjList.containsKey(finalResourceId)) { + adjList.put(finalResourceId, new ArrayList<>()); + } + adjList.get(finalResourceId).add(refInfo.getResourceReference().getReferenceElement().getValue()); + } }); - } - System.out.println("zoop!!"); - return null; + //All nodes are now white + //Adjacency List has been built. + + for (Map.Entry entry:color.entrySet()) { + if (entry.getValue() == WHITE) { + depthFirstSearch(entry.getKey(), color, adjList, topologicalOrder, legality); + } + } + if (legality.isLegal()) { + if (ourLog.isDebugEnabled()) { + ourLog.debug("Topological order is: {}", String.join(",", topologicalOrder)); + } + List beps = new ArrayList<>(); + + for (int i = 0;i < topologicalOrder.size(); i++) { + BundleEntryParts bep = resourceIdToBundleEntryMap.get(topologicalOrder.get(i)); + beps.add(bep); + } + + //In case of delete, we want to delete child elements LAST. + if (theRequestTypeEnum.equals(RequestTypeEnum.DELETE)) { + Collections.reverse(beps); + } + return beps; + } else { + return null; + } } + private static class SortLegality { + private boolean myIsLegal; + + SortLegality() { + this.myIsLegal = true; + } + private void setLegal(boolean theLegal) { + myIsLegal = theLegal; + } + + public boolean isLegal() { + return myIsLegal; + } + } + private static void depthFirstSearch(String theResourceId, HashMap theResourceIdToColor, HashMap> theAdjList, List theTopologicalOrder, SortLegality theLegality) { + System.out.println("RECURSING ON " + theResourceId); + if (!theLegality.isLegal()) { + System.out.println("IMPOSSIBLE!"); + return; + } + + //We are currently recursing over this node (gray) + theResourceIdToColor.put(theResourceId, GRAY); + + for (String neighbourResourceId: theAdjList.getOrDefault(theResourceId, new ArrayList<>())) { + if (theResourceIdToColor.get(neighbourResourceId) == WHITE) { + depthFirstSearch(neighbourResourceId, theResourceIdToColor, theAdjList, theTopologicalOrder, theLegality); + } else if (theResourceIdToColor.get(neighbourResourceId) == GRAY) { + theLegality.setLegal(false); + return; + } + } + //Mark the node as black + theResourceIdToColor.put(theResourceId, BLACK); + theTopologicalOrder.add(theResourceId); + } public static void processEntries(FhirContext theContext, IBaseBundle theBundle, Consumer theProcessor) { RuntimeResourceDefinition bundleDef = theContext.getResourceDefinition(theBundle); diff --git a/hapi-fhir-base/src/main/java/ca/uhn/fhir/util/bundle/BundleEntryParts.java b/hapi-fhir-base/src/main/java/ca/uhn/fhir/util/bundle/BundleEntryParts.java index 5d698cec7b3..7bc9ecfa456 100644 --- a/hapi-fhir-base/src/main/java/ca/uhn/fhir/util/bundle/BundleEntryParts.java +++ b/hapi-fhir-base/src/main/java/ca/uhn/fhir/util/bundle/BundleEntryParts.java @@ -58,5 +58,4 @@ public class BundleEntryParts { public String getUrl() { return myUrl; } - } diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/TransactionHookTest.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/TransactionHookTest.java index 57e4ae32ec6..0d046d782cb 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/TransactionHookTest.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/TransactionHookTest.java @@ -4,10 +4,12 @@ import ca.uhn.fhir.interceptor.api.HookParams; import ca.uhn.fhir.interceptor.api.IInterceptorService; import ca.uhn.fhir.interceptor.api.Pointcut; import ca.uhn.fhir.jpa.api.model.DaoMethodOutcome; +import ca.uhn.fhir.rest.api.RequestTypeEnum; import ca.uhn.fhir.rest.api.server.storage.DeferredInterceptorBroadcasts; import ca.uhn.fhir.rest.server.exceptions.ResourceGoneException; import ca.uhn.fhir.rest.server.exceptions.ResourceVersionConflictException; import ca.uhn.fhir.util.BundleUtil; +import ca.uhn.fhir.util.bundle.BundleEntryParts; import ca.uhn.test.concurrency.PointcutLatch; import com.google.common.collect.ListMultimap; import org.hl7.fhir.instance.model.api.IBaseResource; @@ -24,14 +26,17 @@ import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.springframework.beans.factory.annotation.Autowired; +import java.util.Collections; import java.util.List; import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.empty; import static org.hamcrest.Matchers.hasSize; import static org.hamcrest.Matchers.matchesPattern; +import static org.hamcrest.Matchers.nullValue; import static org.junit.jupiter.api.Assertions.assertTrue; import static org.junit.jupiter.api.Assertions.fail; +import static org.hamcrest.Matchers.is; public class TransactionHookTest extends BaseJpaR4SystemTest { @@ -54,7 +59,7 @@ public class TransactionHookTest extends BaseJpaR4SystemTest { } @Test - public void testTopologicalTransactionSorting() { + public void testTopologicalTransactionSortForCreates() { Bundle b = new Bundle(); Bundle.BundleEntryComponent bundleEntryComponent = b.addEntry(); @@ -87,8 +92,99 @@ public class TransactionHookTest extends BaseJpaR4SystemTest { organizationComponent.setResource(org1); organizationComponent.getRequest().setMethod(Bundle.HTTPVerb.POST).setUrl("Patient"); - BundleUtil.topologicalSort(myFhirCtx, b); + List postBundleEntries = BundleUtil.topologicalSort(myFhirCtx, b, RequestTypeEnum.POST); + assertThat(postBundleEntries, hasSize(4)); + + int observationIndex = getIndexOfEntryWithId("Observation/O1", postBundleEntries); + int patientIndex = getIndexOfEntryWithId("Patient/P1", postBundleEntries); + int organizationIndex = getIndexOfEntryWithId("Organization/Org1", postBundleEntries); + + assertTrue(organizationIndex < patientIndex); + assertTrue(patientIndex < observationIndex); + } + + @Test + public void testTransactionSorterFailsOnCyclicReference() { + Bundle b = new Bundle(); + Bundle.BundleEntryComponent bundleEntryComponent = b.addEntry(); + final Observation obs1 = new Observation(); + obs1.setStatus(Observation.ObservationStatus.FINAL); + obs1.setSubject(new Reference("Patient/P1")); + obs1.setValue(new Quantity(4)); + obs1.setId("Observation/O1"); + obs1.setHasMember(Collections.singletonList(new Reference("Observation/O2"))); + bundleEntryComponent.setResource(obs1); + bundleEntryComponent.getRequest().setMethod(Bundle.HTTPVerb.POST).setUrl("Observation"); + + bundleEntryComponent = b.addEntry(); + final Observation obs2 = new Observation(); + obs2.setStatus(Observation.ObservationStatus.FINAL); + obs2.setValue(new Quantity(4)); + obs2.setId("Observation/O2"); + obs2.setHasMember(Collections.singletonList(new Reference("Observation/O1"))); + bundleEntryComponent.setResource(obs2); + bundleEntryComponent.getRequest().setMethod(Bundle.HTTPVerb.POST).setUrl("Observation"); + List postBundleEntries = BundleUtil.topologicalSort(myFhirCtx, b, RequestTypeEnum.POST); + + //Null value indicates that we hit a cycle, and could not process the deletions in order + assertThat(postBundleEntries, is(nullValue())); + } + + @Test + public void testTransactionSorterReturnsDeletesInCorrectProcessingOrder() { + Bundle b = new Bundle(); + Bundle.BundleEntryComponent bundleEntryComponent = b.addEntry(); + final Observation obs1 = new Observation(); + obs1.setStatus(Observation.ObservationStatus.FINAL); + obs1.setSubject(new Reference("Patient/P1")); + obs1.setValue(new Quantity(4)); + obs1.setId("Observation/O1"); + bundleEntryComponent.setResource(obs1); + bundleEntryComponent.getRequest().setMethod(Bundle.HTTPVerb.DELETE).setUrl("Observation"); + + bundleEntryComponent = b.addEntry(); + final Observation obs2 = new Observation(); + obs2.setStatus(Observation.ObservationStatus.FINAL); + obs2.setValue(new Quantity(4)); + obs2.setId("Observation/O2"); + bundleEntryComponent.setResource(obs2); + bundleEntryComponent.getRequest().setMethod(Bundle.HTTPVerb.DELETE).setUrl("Observation"); + + Bundle.BundleEntryComponent patientComponent = b.addEntry(); + Patient pat1 = new Patient(); + pat1.setId("Patient/P1"); + pat1.setManagingOrganization(new Reference("Organization/Org1")); + patientComponent.setResource(pat1); + patientComponent.getRequest().setMethod(Bundle.HTTPVerb.DELETE).setUrl("Patient"); + + Bundle.BundleEntryComponent organizationComponent = b.addEntry(); + Organization org1 = new Organization(); + org1.setId("Organization/Org1"); + organizationComponent.setResource(org1); + organizationComponent.getRequest().setMethod(Bundle.HTTPVerb.DELETE).setUrl("Organization"); + + List postBundleEntries = BundleUtil.topologicalSort(myFhirCtx, b, RequestTypeEnum.DELETE); + + assertThat(postBundleEntries, hasSize(4)); + + int observationIndex = getIndexOfEntryWithId("Observation/O1", postBundleEntries); + int patientIndex = getIndexOfEntryWithId("Patient/P1", postBundleEntries); + int organizationIndex = getIndexOfEntryWithId("Organization/Org1", postBundleEntries); + + assertTrue(patientIndex < organizationIndex); + assertTrue(observationIndex < patientIndex); + } + + private int getIndexOfEntryWithId(String theResourceId, List theBundleEntryParts) { + for (int i = 0; i < theBundleEntryParts.size(); i++) { + String id = theBundleEntryParts.get(i).getResource().getIdElement().toUnqualifiedVersionless().toString(); + if (id.equals(theResourceId)) { + return i; + } + } + fail("Didn't find resource with ID " + theResourceId); + return -1; } @Test From 3dee0c8ab6185bd9a6d6da57898fb0fa9bc0aff5 Mon Sep 17 00:00:00 2001 From: Tadgh Date: Tue, 20 Apr 2021 17:48:55 -0400 Subject: [PATCH 16/30] Working in-place bundle sort --- .../java/ca/uhn/fhir/util/BundleUtil.java | 189 ++++++++++++------ .../fhir/jpa/dao/r4/TransactionHookTest.java | 40 ++-- 2 files changed, 149 insertions(+), 80 deletions(-) diff --git a/hapi-fhir-base/src/main/java/ca/uhn/fhir/util/BundleUtil.java b/hapi-fhir-base/src/main/java/ca/uhn/fhir/util/BundleUtil.java index 471c317b8da..73341065a66 100644 --- a/hapi-fhir-base/src/main/java/ca/uhn/fhir/util/BundleUtil.java +++ b/hapi-fhir-base/src/main/java/ca/uhn/fhir/util/BundleUtil.java @@ -20,12 +20,14 @@ import org.hl7.fhir.instance.model.api.IBaseResource; import org.hl7.fhir.instance.model.api.IPrimitiveType; import java.util.ArrayList; -import java.util.Collections; import java.util.HashMap; +import java.util.LinkedHashSet; import java.util.List; import java.util.Map; import java.util.Objects; +import java.util.Set; import java.util.function.Consumer; +import java.util.stream.Collectors; import static org.apache.commons.lang3.StringUtils.isNotBlank; /* @@ -186,13 +188,47 @@ public class BundleUtil { static int GRAY = 2; static int BLACK = 3; - public static List topologicalSort(FhirContext theContext, IBaseBundle theBundle, RequestTypeEnum theRequestTypeEnum) { + public static void sortEntriesIntoProcessingOrder(FhirContext theContext, IBaseBundle theBundle) { + Map partsToIBaseMap = getPartsToIBaseMap(theContext, theBundle); + LinkedHashSet retVal = new LinkedHashSet<>(); + + //Get all deletions. + LinkedHashSet deleteParts = sortEntriesIntoProcessingOrder(theContext, theBundle, RequestTypeEnum.DELETE, partsToIBaseMap); + if (deleteParts == null) { + throw new IllegalStateException("Cycle!"); + } else { + retVal.addAll(deleteParts); + } + + //Get all Creations + LinkedHashSet createParts= sortEntriesIntoProcessingOrder(theContext, theBundle, RequestTypeEnum.POST, partsToIBaseMap); + if (createParts== null) { + throw new IllegalStateException("Cycle!"); + } else { + retVal.addAll(createParts); + } + + // Get all Updates + LinkedHashSet updateParts= sortEntriesIntoProcessingOrder(theContext, theBundle, RequestTypeEnum.PUT, partsToIBaseMap); + if (updateParts == null) { + throw new IllegalStateException("Cycle!"); + } else { + retVal.addAll(updateParts); + } + //Once we are done adding all DELETE, POST, PUT operations, add everything else. + retVal.addAll(partsToIBaseMap.values()); + + //Blow away the entries and reset them in the right order. + TerserUtil.clearField(theContext, "entry", theBundle); + TerserUtil.setField(theContext, "entry", theBundle, retVal.toArray(new IBase[0])); + } + + public static LinkedHashSet sortEntriesIntoProcessingOrder(FhirContext theContext, IBaseBundle theBundle, RequestTypeEnum theRequestTypeEnum, Map thePartsToIBaseMap) { SortLegality legality = new SortLegality(); - HashMap color = new HashMap(); + HashMap color = new HashMap<>(); HashMap> adjList = new HashMap<>(); List topologicalOrder = new ArrayList<>(); - List bundleEntryParts = toListOfEntries(theContext, theBundle); - bundleEntryParts.removeIf(bep -> !bep.getRequestType().equals(theRequestTypeEnum)); + Set bundleEntryParts = thePartsToIBaseMap.keySet().stream().filter(part -> part.getRequestType().equals(theRequestTypeEnum)).collect(Collectors.toSet()); HashMap resourceIdToBundleEntryMap = new HashMap<>(); for (BundleEntryParts bundleEntryPart : bundleEntryParts) { @@ -204,7 +240,9 @@ public class BundleUtil { resourceId = bundleEntryPart.getFullUrl(); } } + color.put(resourceId, WHITE); + } for (BundleEntryParts bundleEntryPart : bundleEntryParts) { @@ -229,30 +267,33 @@ public class BundleUtil { } }); } - //All nodes are now white - //Adjacency List has been built. for (Map.Entry entry:color.entrySet()) { if (entry.getValue() == WHITE) { depthFirstSearch(entry.getKey(), color, adjList, topologicalOrder, legality); } } + if (legality.isLegal()) { if (ourLog.isDebugEnabled()) { ourLog.debug("Topological order is: {}", String.join(",", topologicalOrder)); } - List beps = new ArrayList<>(); - for (int i = 0;i < topologicalOrder.size(); i++) { - BundleEntryParts bep = resourceIdToBundleEntryMap.get(topologicalOrder.get(i)); - beps.add(bep); + LinkedHashSet orderedEntries = new LinkedHashSet<>(); + for (int i = 0; i < topologicalOrder.size(); i++) { + BundleEntryParts bep; + if (theRequestTypeEnum.equals(RequestTypeEnum.DELETE)) { + int index = topologicalOrder.size() - i - 1; + bep = resourceIdToBundleEntryMap.get(topologicalOrder.get(index)); + } else { + bep = resourceIdToBundleEntryMap.get(topologicalOrder.get(i)); + } + IBase base = thePartsToIBaseMap.get(bep); + orderedEntries.add(base); } - //In case of delete, we want to delete child elements LAST. - if (theRequestTypeEnum.equals(RequestTypeEnum.DELETE)) { - Collections.reverse(beps); - } - return beps; + return orderedEntries; + } else { return null; } @@ -295,15 +336,38 @@ public class BundleUtil { theTopologicalOrder.add(theResourceId); } + private static Map getPartsToIBaseMap(FhirContext theContext, IBaseBundle theBundle) { + RuntimeResourceDefinition bundleDef = theContext.getResourceDefinition(theBundle); + BaseRuntimeChildDefinition entryChildDef = bundleDef.getChildByName("entry"); + List entries = entryChildDef.getAccessor().getValues(theBundle); + + BaseRuntimeElementCompositeDefinition entryChildContentsDef = (BaseRuntimeElementCompositeDefinition) entryChildDef.getChildByName("entry"); + BaseRuntimeChildDefinition fullUrlChildDef = entryChildContentsDef.getChildByName("fullUrl"); + BaseRuntimeChildDefinition resourceChildDef = entryChildContentsDef.getChildByName("resource"); + BaseRuntimeChildDefinition requestChildDef = entryChildContentsDef.getChildByName("request"); + BaseRuntimeElementCompositeDefinition requestChildContentsDef = (BaseRuntimeElementCompositeDefinition) requestChildDef.getChildByName("request"); + BaseRuntimeChildDefinition requestUrlChildDef = requestChildContentsDef.getChildByName("url"); + BaseRuntimeChildDefinition requestIfNoneExistChildDef = requestChildContentsDef.getChildByName("ifNoneExist"); + BaseRuntimeChildDefinition methodChildDef = requestChildContentsDef.getChildByName("method"); + Map map = new HashMap<>(); + for (IBase nextEntry : entries) { + BundleEntryParts parts = getBundleEntryParts(fullUrlChildDef, resourceChildDef, requestChildDef, requestUrlChildDef, requestIfNoneExistChildDef, methodChildDef, nextEntry); + /* + * All 3 might be null - That's ok because we still want to know the + * order in the original bundle. + */ + map.put(parts, nextEntry); + } + return map; + + } public static void processEntries(FhirContext theContext, IBaseBundle theBundle, Consumer theProcessor) { RuntimeResourceDefinition bundleDef = theContext.getResourceDefinition(theBundle); BaseRuntimeChildDefinition entryChildDef = bundleDef.getChildByName("entry"); List entries = entryChildDef.getAccessor().getValues(theBundle); BaseRuntimeElementCompositeDefinition entryChildContentsDef = (BaseRuntimeElementCompositeDefinition) entryChildDef.getChildByName("entry"); - BaseRuntimeChildDefinition fullUrlChildDef = entryChildContentsDef.getChildByName("fullUrl"); - BaseRuntimeChildDefinition resourceChildDef = entryChildContentsDef.getChildByName("resource"); BaseRuntimeChildDefinition requestChildDef = entryChildContentsDef.getChildByName("request"); BaseRuntimeElementCompositeDefinition requestChildContentsDef = (BaseRuntimeElementCompositeDefinition) requestChildDef.getChildByName("request"); @@ -312,57 +376,62 @@ public class BundleUtil { BaseRuntimeChildDefinition methodChildDef = requestChildContentsDef.getChildByName("method"); for (IBase nextEntry : entries) { - IBaseResource resource = null; - String url = null; - RequestTypeEnum requestType = null; - String conditionalUrl = null; - String fullUrl = fullUrlChildDef - .getAccessor() - .getFirstValueOrNull(nextEntry) - .map(t->((IPrimitiveType)t).getValueAsString()) - .orElse(null); - - for (IBase nextResource : resourceChildDef.getAccessor().getValues(nextEntry)) { - resource = (IBaseResource) nextResource; - } - for (IBase nextRequest : requestChildDef.getAccessor().getValues(nextEntry)) { - for (IBase nextUrl : requestUrlChildDef.getAccessor().getValues(nextRequest)) { - url = ((IPrimitiveType) nextUrl).getValueAsString(); - } - for (IBase nextMethod : methodChildDef.getAccessor().getValues(nextRequest)) { - String methodString = ((IPrimitiveType) nextMethod).getValueAsString(); - if (isNotBlank(methodString)) { - requestType = RequestTypeEnum.valueOf(methodString); - } - } - - if (requestType != null) { - //noinspection EnumSwitchStatementWhichMissesCases - switch (requestType) { - case PUT: - conditionalUrl = url != null && url.contains("?") ? url : null; - break; - case POST: - List ifNoneExistReps = requestIfNoneExistChildDef.getAccessor().getValues(nextRequest); - if (ifNoneExistReps.size() > 0) { - IPrimitiveType ifNoneExist = (IPrimitiveType) ifNoneExistReps.get(0); - conditionalUrl = ifNoneExist.getValueAsString(); - } - break; - } - } - } - + BundleEntryParts parts = getBundleEntryParts(fullUrlChildDef, resourceChildDef, requestChildDef, requestUrlChildDef, requestIfNoneExistChildDef, methodChildDef, nextEntry); /* * All 3 might be null - That's ok because we still want to know the * order in the original bundle. */ BundleEntryMutator mutator = new BundleEntryMutator(theContext, nextEntry, requestChildDef, requestChildContentsDef, entryChildContentsDef); - ModifiableBundleEntry entry = new ModifiableBundleEntry(new BundleEntryParts(fullUrl, requestType, url, resource, conditionalUrl), mutator); + ModifiableBundleEntry entry = new ModifiableBundleEntry(parts, mutator); theProcessor.accept(entry); } } + private static BundleEntryParts getBundleEntryParts(BaseRuntimeChildDefinition fullUrlChildDef, BaseRuntimeChildDefinition resourceChildDef, BaseRuntimeChildDefinition requestChildDef, BaseRuntimeChildDefinition requestUrlChildDef, BaseRuntimeChildDefinition requestIfNoneExistChildDef, BaseRuntimeChildDefinition methodChildDef, IBase nextEntry) { + IBaseResource resource = null; + String url = null; + RequestTypeEnum requestType = null; + String conditionalUrl = null; + String fullUrl = fullUrlChildDef + .getAccessor() + .getFirstValueOrNull(nextEntry) + .map(t->((IPrimitiveType)t).getValueAsString()) + .orElse(null); + + for (IBase nextResource : resourceChildDef.getAccessor().getValues(nextEntry)) { + resource = (IBaseResource) nextResource; + } + for (IBase nextRequest : requestChildDef.getAccessor().getValues(nextEntry)) { + for (IBase nextUrl : requestUrlChildDef.getAccessor().getValues(nextRequest)) { + url = ((IPrimitiveType) nextUrl).getValueAsString(); + } + for (IBase nextMethod : methodChildDef.getAccessor().getValues(nextRequest)) { + String methodString = ((IPrimitiveType) nextMethod).getValueAsString(); + if (isNotBlank(methodString)) { + requestType = RequestTypeEnum.valueOf(methodString); + } + } + + if (requestType != null) { + //noinspection EnumSwitchStatementWhichMissesCases + switch (requestType) { + case PUT: + conditionalUrl = url != null && url.contains("?") ? url : null; + break; + case POST: + List ifNoneExistReps = requestIfNoneExistChildDef.getAccessor().getValues(nextRequest); + if (ifNoneExistReps.size() > 0) { + IPrimitiveType ifNoneExist = (IPrimitiveType) ifNoneExistReps.get(0); + conditionalUrl = ifNoneExist.getValueAsString(); + } + break; + } + } + } + BundleEntryParts parts = new BundleEntryParts(fullUrl, requestType, url, resource, conditionalUrl); + return parts; + } + /** * Extract all of the resources from a given bundle */ diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/TransactionHookTest.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/TransactionHookTest.java index 0d046d782cb..e6cc81e04f3 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/TransactionHookTest.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/TransactionHookTest.java @@ -4,12 +4,10 @@ import ca.uhn.fhir.interceptor.api.HookParams; import ca.uhn.fhir.interceptor.api.IInterceptorService; import ca.uhn.fhir.interceptor.api.Pointcut; import ca.uhn.fhir.jpa.api.model.DaoMethodOutcome; -import ca.uhn.fhir.rest.api.RequestTypeEnum; import ca.uhn.fhir.rest.api.server.storage.DeferredInterceptorBroadcasts; import ca.uhn.fhir.rest.server.exceptions.ResourceGoneException; import ca.uhn.fhir.rest.server.exceptions.ResourceVersionConflictException; import ca.uhn.fhir.util.BundleUtil; -import ca.uhn.fhir.util.bundle.BundleEntryParts; import ca.uhn.test.concurrency.PointcutLatch; import com.google.common.collect.ListMultimap; import org.hl7.fhir.instance.model.api.IBaseResource; @@ -33,10 +31,8 @@ import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.empty; import static org.hamcrest.Matchers.hasSize; import static org.hamcrest.Matchers.matchesPattern; -import static org.hamcrest.Matchers.nullValue; import static org.junit.jupiter.api.Assertions.assertTrue; import static org.junit.jupiter.api.Assertions.fail; -import static org.hamcrest.Matchers.is; public class TransactionHookTest extends BaseJpaR4SystemTest { @@ -92,13 +88,14 @@ public class TransactionHookTest extends BaseJpaR4SystemTest { organizationComponent.setResource(org1); organizationComponent.getRequest().setMethod(Bundle.HTTPVerb.POST).setUrl("Patient"); - List postBundleEntries = BundleUtil.topologicalSort(myFhirCtx, b, RequestTypeEnum.POST); + BundleUtil.sortEntriesIntoProcessingOrder(myFhirCtx, b); - assertThat(postBundleEntries, hasSize(4)); + assertThat(b.getEntry(), hasSize(4)); - int observationIndex = getIndexOfEntryWithId("Observation/O1", postBundleEntries); - int patientIndex = getIndexOfEntryWithId("Patient/P1", postBundleEntries); - int organizationIndex = getIndexOfEntryWithId("Organization/Org1", postBundleEntries); + List entry = b.getEntry(); + int observationIndex = getIndexOfEntryWithId("Observation/O1", b); + int patientIndex = getIndexOfEntryWithId("Patient/P1", b); + int organizationIndex = getIndexOfEntryWithId("Organization/Org1", b); assertTrue(organizationIndex < patientIndex); assertTrue(patientIndex < observationIndex); @@ -125,10 +122,12 @@ public class TransactionHookTest extends BaseJpaR4SystemTest { obs2.setHasMember(Collections.singletonList(new Reference("Observation/O1"))); bundleEntryComponent.setResource(obs2); bundleEntryComponent.getRequest().setMethod(Bundle.HTTPVerb.POST).setUrl("Observation"); - List postBundleEntries = BundleUtil.topologicalSort(myFhirCtx, b, RequestTypeEnum.POST); + try { + BundleUtil.sortEntriesIntoProcessingOrder(myFhirCtx, b); + fail(); + } catch (IllegalStateException e ) { - //Null value indicates that we hit a cycle, and could not process the deletions in order - assertThat(postBundleEntries, is(nullValue())); + } } @Test @@ -164,21 +163,22 @@ public class TransactionHookTest extends BaseJpaR4SystemTest { organizationComponent.setResource(org1); organizationComponent.getRequest().setMethod(Bundle.HTTPVerb.DELETE).setUrl("Organization"); - List postBundleEntries = BundleUtil.topologicalSort(myFhirCtx, b, RequestTypeEnum.DELETE); + BundleUtil.sortEntriesIntoProcessingOrder(myFhirCtx, b); - assertThat(postBundleEntries, hasSize(4)); + assertThat(b.getEntry(), hasSize(4)); - int observationIndex = getIndexOfEntryWithId("Observation/O1", postBundleEntries); - int patientIndex = getIndexOfEntryWithId("Patient/P1", postBundleEntries); - int organizationIndex = getIndexOfEntryWithId("Organization/Org1", postBundleEntries); + int observationIndex = getIndexOfEntryWithId("Observation/O1", b); + int patientIndex = getIndexOfEntryWithId("Patient/P1", b); + int organizationIndex = getIndexOfEntryWithId("Organization/Org1", b); assertTrue(patientIndex < organizationIndex); assertTrue(observationIndex < patientIndex); } - private int getIndexOfEntryWithId(String theResourceId, List theBundleEntryParts) { - for (int i = 0; i < theBundleEntryParts.size(); i++) { - String id = theBundleEntryParts.get(i).getResource().getIdElement().toUnqualifiedVersionless().toString(); + private int getIndexOfEntryWithId(String theResourceId, Bundle theBundle) { + List entries = theBundle.getEntry(); + for (int i = 0; i < entries.size(); i++) { + String id = entries.get(i).getResource().getIdElement().toUnqualifiedVersionless().toString(); if (id.equals(theResourceId)) { return i; } From e997d49d42c2db2b27c1092d6501ca776b0bfe11 Mon Sep 17 00:00:00 2001 From: Tadgh Date: Tue, 20 Apr 2021 18:25:45 -0400 Subject: [PATCH 17/30] Additional Tests --- .../fhir/jpa/dao/r4/TransactionHookTest.java | 125 +++++++++++++++--- 1 file changed, 104 insertions(+), 21 deletions(-) diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/TransactionHookTest.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/TransactionHookTest.java index e6cc81e04f3..8413a6d8118 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/TransactionHookTest.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/TransactionHookTest.java @@ -14,6 +14,8 @@ import org.hl7.fhir.instance.model.api.IBaseResource; import org.hl7.fhir.instance.model.api.IIdType; import org.hl7.fhir.r4.model.Bundle; import org.hl7.fhir.r4.model.DiagnosticReport; +import org.hl7.fhir.r4.model.ExplanationOfBenefit; +import org.hl7.fhir.r4.model.Medication; import org.hl7.fhir.r4.model.Observation; import org.hl7.fhir.r4.model.Organization; import org.hl7.fhir.r4.model.Patient; @@ -27,10 +29,16 @@ import org.springframework.beans.factory.annotation.Autowired; import java.util.Collections; import java.util.List; +import static org.hamcrest.Matchers.is; import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.empty; +import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.hasSize; import static org.hamcrest.Matchers.matchesPattern; +import static org.hl7.fhir.r4.model.Bundle.HTTPVerb.DELETE; +import static org.hl7.fhir.r4.model.Bundle.HTTPVerb.GET; +import static org.hl7.fhir.r4.model.Bundle.HTTPVerb.POST; +import static org.hl7.fhir.r4.model.Bundle.HTTPVerb.PUT; import static org.junit.jupiter.api.Assertions.assertTrue; import static org.junit.jupiter.api.Assertions.fail; @@ -65,7 +73,7 @@ public class TransactionHookTest extends BaseJpaR4SystemTest { obs1.setValue(new Quantity(4)); obs1.setId("Observation/O1"); bundleEntryComponent.setResource(obs1); - bundleEntryComponent.getRequest().setMethod(Bundle.HTTPVerb.POST).setUrl("Observation"); + bundleEntryComponent.getRequest().setMethod(POST).setUrl("Observation"); bundleEntryComponent = b.addEntry(); final Observation obs2 = new Observation(); @@ -73,20 +81,20 @@ public class TransactionHookTest extends BaseJpaR4SystemTest { obs2.setValue(new Quantity(4)); obs2.setId("Observation/O2"); bundleEntryComponent.setResource(obs2); - bundleEntryComponent.getRequest().setMethod(Bundle.HTTPVerb.POST).setUrl("Observation"); + bundleEntryComponent.getRequest().setMethod(POST).setUrl("Observation"); Bundle.BundleEntryComponent patientComponent = b.addEntry(); Patient pat1 = new Patient(); pat1.setId("Patient/P1"); pat1.setManagingOrganization(new Reference("Organization/Org1")); patientComponent.setResource(pat1); - patientComponent.getRequest().setMethod(Bundle.HTTPVerb.POST).setUrl("Patient"); + patientComponent.getRequest().setMethod(POST).setUrl("Patient"); Bundle.BundleEntryComponent organizationComponent = b.addEntry(); Organization org1 = new Organization(); org1.setId("Organization/Org1"); organizationComponent.setResource(org1); - organizationComponent.getRequest().setMethod(Bundle.HTTPVerb.POST).setUrl("Patient"); + organizationComponent.getRequest().setMethod(POST).setUrl("Patient"); BundleUtil.sortEntriesIntoProcessingOrder(myFhirCtx, b); @@ -112,7 +120,7 @@ public class TransactionHookTest extends BaseJpaR4SystemTest { obs1.setId("Observation/O1"); obs1.setHasMember(Collections.singletonList(new Reference("Observation/O2"))); bundleEntryComponent.setResource(obs1); - bundleEntryComponent.getRequest().setMethod(Bundle.HTTPVerb.POST).setUrl("Observation"); + bundleEntryComponent.getRequest().setMethod(POST).setUrl("Observation"); bundleEntryComponent = b.addEntry(); final Observation obs2 = new Observation(); @@ -121,7 +129,7 @@ public class TransactionHookTest extends BaseJpaR4SystemTest { obs2.setId("Observation/O2"); obs2.setHasMember(Collections.singletonList(new Reference("Observation/O1"))); bundleEntryComponent.setResource(obs2); - bundleEntryComponent.getRequest().setMethod(Bundle.HTTPVerb.POST).setUrl("Observation"); + bundleEntryComponent.getRequest().setMethod(POST).setUrl("Observation"); try { BundleUtil.sortEntriesIntoProcessingOrder(myFhirCtx, b); fail(); @@ -130,6 +138,81 @@ public class TransactionHookTest extends BaseJpaR4SystemTest { } } + @Test + public void testTransactionSortingReturnsOperationsInCorrectOrder() { + + Bundle b = new Bundle(); + + //UPDATE patient + Bundle.BundleEntryComponent patientUpdateComponent= b.addEntry(); + final Patient p2 = new Patient(); + p2.setId("Patient/P2"); + p2.getNameFirstRep().setFamily("Test!"); + patientUpdateComponent.setResource(p2); + patientUpdateComponent.getRequest().setMethod(PUT).setUrl("Patient/P2"); + + //CREATE observation + Bundle.BundleEntryComponent bundleEntryComponent = b.addEntry(); + final Observation obs1 = new Observation(); + obs1.setStatus(Observation.ObservationStatus.FINAL); + obs1.setSubject(new Reference("Patient/P1")); + obs1.setValue(new Quantity(4)); + obs1.setId("Observation/O1"); + bundleEntryComponent.setResource(obs1); + bundleEntryComponent.getRequest().setMethod(POST).setUrl("Observation"); + + //DELETE medication + Bundle.BundleEntryComponent medicationComponent= b.addEntry(); + final Medication med1 = new Medication(); + med1.setId("Medication/M1"); + medicationComponent.setResource(med1); + medicationComponent.getRequest().setMethod(DELETE).setUrl("Medication"); + + //GET medication + Bundle.BundleEntryComponent searchComponent = b.addEntry(); + searchComponent.getRequest().setMethod(GET).setUrl("Medication?code=123"); + + //CREATE patient + Bundle.BundleEntryComponent patientComponent = b.addEntry(); + Patient pat1 = new Patient(); + pat1.setId("Patient/P1"); + pat1.setManagingOrganization(new Reference("Organization/Org1")); + patientComponent.setResource(pat1); + patientComponent.getRequest().setMethod(POST).setUrl("Patient"); + + //CREATE organization + Bundle.BundleEntryComponent organizationComponent = b.addEntry(); + Organization org1 = new Organization(); + org1.setId("Organization/Org1"); + organizationComponent.setResource(org1); + organizationComponent.getRequest().setMethod(POST).setUrl("Organization"); + + //DELETE ExplanationOfBenefit + Bundle.BundleEntryComponent explanationOfBenefitComponent= b.addEntry(); + final ExplanationOfBenefit eob1 = new ExplanationOfBenefit(); + eob1.setId("ExplanationOfBenefit/E1"); + explanationOfBenefitComponent.setResource(eob1); + explanationOfBenefitComponent.getRequest().setMethod(DELETE).setUrl("ExplanationOfBenefit"); + + BundleUtil.sortEntriesIntoProcessingOrder(myFhirCtx, b); + + assertThat(b.getEntry(), hasSize(7)); + + List entry = b.getEntry(); + + // DELETEs first + assertThat(entry.get(0).getRequest().getMethod(), is(equalTo(DELETE))); + assertThat(entry.get(1).getRequest().getMethod(), is(equalTo(DELETE))); + // Then POSTs + assertThat(entry.get(2).getRequest().getMethod(), is(equalTo(POST))); + assertThat(entry.get(3).getRequest().getMethod(), is(equalTo(POST))); + assertThat(entry.get(4).getRequest().getMethod(), is(equalTo(POST))); + // Then PUTs + assertThat(entry.get(5).getRequest().getMethod(), is(equalTo(PUT))); + // Then GETs + assertThat(entry.get(6).getRequest().getMethod(), is(equalTo(GET))); + } + @Test public void testTransactionSorterReturnsDeletesInCorrectProcessingOrder() { Bundle b = new Bundle(); @@ -140,7 +223,7 @@ public class TransactionHookTest extends BaseJpaR4SystemTest { obs1.setValue(new Quantity(4)); obs1.setId("Observation/O1"); bundleEntryComponent.setResource(obs1); - bundleEntryComponent.getRequest().setMethod(Bundle.HTTPVerb.DELETE).setUrl("Observation"); + bundleEntryComponent.getRequest().setMethod(DELETE).setUrl("Observation"); bundleEntryComponent = b.addEntry(); final Observation obs2 = new Observation(); @@ -148,20 +231,20 @@ public class TransactionHookTest extends BaseJpaR4SystemTest { obs2.setValue(new Quantity(4)); obs2.setId("Observation/O2"); bundleEntryComponent.setResource(obs2); - bundleEntryComponent.getRequest().setMethod(Bundle.HTTPVerb.DELETE).setUrl("Observation"); + bundleEntryComponent.getRequest().setMethod(DELETE).setUrl("Observation"); Bundle.BundleEntryComponent patientComponent = b.addEntry(); Patient pat1 = new Patient(); pat1.setId("Patient/P1"); pat1.setManagingOrganization(new Reference("Organization/Org1")); patientComponent.setResource(pat1); - patientComponent.getRequest().setMethod(Bundle.HTTPVerb.DELETE).setUrl("Patient"); + patientComponent.getRequest().setMethod(DELETE).setUrl("Patient"); Bundle.BundleEntryComponent organizationComponent = b.addEntry(); Organization org1 = new Organization(); org1.setId("Organization/Org1"); organizationComponent.setResource(org1); - organizationComponent.getRequest().setMethod(Bundle.HTTPVerb.DELETE).setUrl("Organization"); + organizationComponent.getRequest().setMethod(DELETE).setUrl("Organization"); BundleUtil.sortEntriesIntoProcessingOrder(myFhirCtx, b); @@ -206,16 +289,16 @@ public class TransactionHookTest extends BaseJpaR4SystemTest { Bundle.BundleEntryComponent bundleEntryComponent = b.addEntry(); bundleEntryComponent.setResource(obs1); - bundleEntryComponent.getRequest().setMethod(Bundle.HTTPVerb.POST).setUrl("Observation"); + bundleEntryComponent.getRequest().setMethod(POST).setUrl("Observation"); Bundle.BundleEntryComponent patientComponent = b.addEntry(); patientComponent.setFullUrl(urnReference); patientComponent.setResource(pat1); - patientComponent.getRequest().setMethod(Bundle.HTTPVerb.POST).setUrl("Patient"); + patientComponent.getRequest().setMethod(POST).setUrl("Patient"); //Delete an observation - b.addEntry().getRequest().setMethod(Bundle.HTTPVerb.DELETE).setUrl(daoMethodOutcome.getId().toUnqualifiedVersionless().getValue()); + b.addEntry().getRequest().setMethod(DELETE).setUrl(daoMethodOutcome.getId().toUnqualifiedVersionless().getValue()); myPointcutLatch.setExpectedCount(4); @@ -261,8 +344,8 @@ public class TransactionHookTest extends BaseJpaR4SystemTest { myDiagnosticReportDao.read(rptId); Bundle b = new Bundle(); - b.addEntry().getRequest().setMethod(Bundle.HTTPVerb.DELETE).setUrl(rptId.getValue()); - b.addEntry().getRequest().setMethod(Bundle.HTTPVerb.DELETE).setUrl(obs2id.getValue()); + b.addEntry().getRequest().setMethod(DELETE).setUrl(rptId.getValue()); + b.addEntry().getRequest().setMethod(DELETE).setUrl(obs2id.getValue()); try { // transaction should succeed because the DiagnosticReport which references obs2 is also deleted @@ -296,8 +379,8 @@ public class TransactionHookTest extends BaseJpaR4SystemTest { rpt.addIdentifier().setSystem("foo").setValue("IDENTIFIER"); Bundle b = new Bundle(); - b.addEntry().getRequest().setMethod(Bundle.HTTPVerb.DELETE).setUrl("Observation?_has:DiagnosticReport:result:identifier=foo|IDENTIFIER"); - b.addEntry().setResource(rpt).getRequest().setMethod(Bundle.HTTPVerb.PUT).setUrl("DiagnosticReport?identifier=foo|IDENTIFIER"); + b.addEntry().getRequest().setMethod(DELETE).setUrl("Observation?_has:DiagnosticReport:result:identifier=foo|IDENTIFIER"); + b.addEntry().setResource(rpt).getRequest().setMethod(PUT).setUrl("DiagnosticReport?identifier=foo|IDENTIFIER"); mySystemDao.transaction(mySrd, b); myObservationDao.read(obs1id); @@ -334,8 +417,8 @@ public class TransactionHookTest extends BaseJpaR4SystemTest { rpt.addResult(new Reference(obs2id)); Bundle b = new Bundle(); - b.addEntry().getRequest().setMethod(Bundle.HTTPVerb.DELETE).setUrl(obs1id.getValue()); - b.addEntry().setResource(rpt).getRequest().setMethod(Bundle.HTTPVerb.PUT).setUrl(rptId.getValue()); + b.addEntry().getRequest().setMethod(DELETE).setUrl(obs1id.getValue()); + b.addEntry().setResource(rpt).getRequest().setMethod(PUT).setUrl(rptId.getValue()); mySystemDao.transaction(mySrd, b); myObservationDao.read(obs2id); @@ -374,8 +457,8 @@ public class TransactionHookTest extends BaseJpaR4SystemTest { rpt.addResult(new Reference(obs2id)); Bundle b = new Bundle(); - b.addEntry().getRequest().setMethod(Bundle.HTTPVerb.DELETE).setUrl("Observation?_has:DiagnosticReport:result:identifier=foo|IDENTIFIER"); - b.addEntry().setResource(rpt).getRequest().setMethod(Bundle.HTTPVerb.PUT).setUrl("DiagnosticReport?identifier=foo|IDENTIFIER"); + b.addEntry().getRequest().setMethod(DELETE).setUrl("Observation?_has:DiagnosticReport:result:identifier=foo|IDENTIFIER"); + b.addEntry().setResource(rpt).getRequest().setMethod(PUT).setUrl("DiagnosticReport?identifier=foo|IDENTIFIER"); try { mySystemDao.transaction(mySrd, b); fail(); From ad8921ac4e852e06376e710fc17d281dd4657e04 Mon Sep 17 00:00:00 2001 From: Tadgh Date: Wed, 21 Apr 2021 10:39:23 -0400 Subject: [PATCH 18/30] Add docs, enum, remove prints --- .../ca/uhn/fhir/interceptor/api/Pointcut.java | 13 +++-- .../api/InterceptorInvocationTimingEnum.java | 10 ++++ .../java/ca/uhn/fhir/util/BundleUtil.java | 54 ++++++++++++------- .../ca/uhn/fhir/jpa/dao/BaseHapiFhirDao.java | 3 +- .../fhir/jpa/dao/BaseHapiFhirResourceDao.java | 11 ++-- .../fhir/jpa/dao/r4/TransactionHookTest.java | 9 ++-- .../server/storage/TransactionDetails.java | 7 +-- .../provider/HashMapResourceProvider.java | 5 +- 8 files changed, 72 insertions(+), 40 deletions(-) create mode 100644 hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/api/InterceptorInvocationTimingEnum.java diff --git a/hapi-fhir-base/src/main/java/ca/uhn/fhir/interceptor/api/Pointcut.java b/hapi-fhir-base/src/main/java/ca/uhn/fhir/interceptor/api/Pointcut.java index a475de9b98a..733caacdae1 100644 --- a/hapi-fhir-base/src/main/java/ca/uhn/fhir/interceptor/api/Pointcut.java +++ b/hapi-fhir-base/src/main/java/ca/uhn/fhir/interceptor/api/Pointcut.java @@ -1423,6 +1423,9 @@ public enum Pointcut implements IPointcut { *
    • * Boolean - Whether this pointcut invocation was deferred or not(since 5.4.0) *
      • + *
    • + * ca.uhn.fhir.rest.api.InterceptorInvocationTimingEnum - The timing at which the invocation of the interceptor took place. Options are ACTIVE and DEFERRED. + *
      • *
    *

    * Hooks should return void. @@ -1433,7 +1436,7 @@ public enum Pointcut implements IPointcut { "ca.uhn.fhir.rest.api.server.RequestDetails", "ca.uhn.fhir.rest.server.servlet.ServletRequestDetails", "ca.uhn.fhir.rest.api.server.storage.TransactionDetails", - Boolean.class.getName() + "ca.uhn.fhir.rest.api.InterceptorInvocationTimingEnum" ), /** @@ -1468,7 +1471,7 @@ public enum Pointcut implements IPointcut { * ca.uhn.fhir.rest.api.server.storage.TransactionDetails - The outer transaction details object (since 5.0.0) * *

  • - * Boolean - Whether this pointcut invocation was deferred or not(since 5.4.0) + * ca.uhn.fhir.rest.api.InterceptorInvocationTimingEnum - The timing at which the invocation of the interceptor took place. Options are ACTIVE and DEFERRED. *
    • * *

    @@ -1481,7 +1484,7 @@ public enum Pointcut implements IPointcut { "ca.uhn.fhir.rest.api.server.RequestDetails", "ca.uhn.fhir.rest.server.servlet.ServletRequestDetails", "ca.uhn.fhir.rest.api.server.storage.TransactionDetails", - Boolean.class.getName() + "ca.uhn.fhir.rest.api.InterceptorInvocationTimingEnum" ), @@ -1512,7 +1515,7 @@ public enum Pointcut implements IPointcut { * ca.uhn.fhir.rest.api.server.storage.TransactionDetails - The outer transaction details object (since 5.0.0) * *

  • - * Boolean - Whether this pointcut invocation was deferred or not(since 5.4.0) + * ca.uhn.fhir.rest.api.InterceptorInvocationTimingEnum - The timing at which the invocation of the interceptor took place. Options are ACTIVE and DEFERRED. *
    • * *

    @@ -1524,7 +1527,7 @@ public enum Pointcut implements IPointcut { "ca.uhn.fhir.rest.api.server.RequestDetails", "ca.uhn.fhir.rest.server.servlet.ServletRequestDetails", "ca.uhn.fhir.rest.api.server.storage.TransactionDetails", - Boolean.class.getName() + "ca.uhn.fhir.rest.api.InterceptorInvocationTimingEnum" ), /** diff --git a/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/api/InterceptorInvocationTimingEnum.java b/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/api/InterceptorInvocationTimingEnum.java new file mode 100644 index 00000000000..173774a9133 --- /dev/null +++ b/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/api/InterceptorInvocationTimingEnum.java @@ -0,0 +1,10 @@ +package ca.uhn.fhir.rest.api; + +/** + * during invocation of certain pointcuts, it is important to know whether they are being executed in + * active or deferred fashion. This enum allows the pointcuts to see how they were invoked. + */ +public enum InterceptorInvocationTimingEnum { + ACTIVE, + DEFERRED +} diff --git a/hapi-fhir-base/src/main/java/ca/uhn/fhir/util/BundleUtil.java b/hapi-fhir-base/src/main/java/ca/uhn/fhir/util/BundleUtil.java index 73341065a66..174be06e559 100644 --- a/hapi-fhir-base/src/main/java/ca/uhn/fhir/util/BundleUtil.java +++ b/hapi-fhir-base/src/main/java/ca/uhn/fhir/util/BundleUtil.java @@ -188,12 +188,17 @@ public class BundleUtil { static int GRAY = 2; static int BLACK = 3; + /** + * + * @param theContext + * @param theBundle + */ public static void sortEntriesIntoProcessingOrder(FhirContext theContext, IBaseBundle theBundle) { Map partsToIBaseMap = getPartsToIBaseMap(theContext, theBundle); LinkedHashSet retVal = new LinkedHashSet<>(); //Get all deletions. - LinkedHashSet deleteParts = sortEntriesIntoProcessingOrder(theContext, theBundle, RequestTypeEnum.DELETE, partsToIBaseMap); + LinkedHashSet deleteParts = sortEntriesOfTypeIntoProcessingOrder(theContext, RequestTypeEnum.DELETE, partsToIBaseMap); if (deleteParts == null) { throw new IllegalStateException("Cycle!"); } else { @@ -201,7 +206,7 @@ public class BundleUtil { } //Get all Creations - LinkedHashSet createParts= sortEntriesIntoProcessingOrder(theContext, theBundle, RequestTypeEnum.POST, partsToIBaseMap); + LinkedHashSet createParts= sortEntriesOfTypeIntoProcessingOrder(theContext, RequestTypeEnum.POST, partsToIBaseMap); if (createParts== null) { throw new IllegalStateException("Cycle!"); } else { @@ -209,7 +214,7 @@ public class BundleUtil { } // Get all Updates - LinkedHashSet updateParts= sortEntriesIntoProcessingOrder(theContext, theBundle, RequestTypeEnum.PUT, partsToIBaseMap); + LinkedHashSet updateParts= sortEntriesOfTypeIntoProcessingOrder(theContext, RequestTypeEnum.PUT, partsToIBaseMap); if (updateParts == null) { throw new IllegalStateException("Cycle!"); } else { @@ -223,7 +228,7 @@ public class BundleUtil { TerserUtil.setField(theContext, "entry", theBundle, retVal.toArray(new IBase[0])); } - public static LinkedHashSet sortEntriesIntoProcessingOrder(FhirContext theContext, IBaseBundle theBundle, RequestTypeEnum theRequestTypeEnum, Map thePartsToIBaseMap) { + private static LinkedHashSet sortEntriesOfTypeIntoProcessingOrder(FhirContext theContext, RequestTypeEnum theRequestTypeEnum, Map thePartsToIBaseMap) { SortLegality legality = new SortLegality(); HashMap color = new HashMap<>(); HashMap> adjList = new HashMap<>(); @@ -299,24 +304,10 @@ public class BundleUtil { } } - private static class SortLegality { - private boolean myIsLegal; - - SortLegality() { - this.myIsLegal = true; - } - private void setLegal(boolean theLegal) { - myIsLegal = theLegal; - } - - public boolean isLegal() { - return myIsLegal; - } - } private static void depthFirstSearch(String theResourceId, HashMap theResourceIdToColor, HashMap> theAdjList, List theTopologicalOrder, SortLegality theLegality) { - System.out.println("RECURSING ON " + theResourceId); + if (!theLegality.isLegal()) { - System.out.println("IMPOSSIBLE!"); + ourLog.debug("Found a cycle while trying to sort bundle entries. This bundle is not sortable."); return; } @@ -361,6 +352,13 @@ public class BundleUtil { return map; } + + /** + * Given a bundle, and a consumer, apply the consumer to each entry in the bundle. + * @param theContext The FHIR Context + * @param theBundle The bundle to have its entries processed. + * @param theProcessor a {@link Consumer} which will operate on all the entries of a bundle. + */ public static void processEntries(FhirContext theContext, IBaseBundle theBundle, Consumer theProcessor) { RuntimeResourceDefinition bundleDef = theContext.getResourceDefinition(theBundle); BaseRuntimeChildDefinition entryChildDef = bundleDef.getChildByName("entry"); @@ -483,4 +481,20 @@ public class BundleUtil { } return isPatch; } + + private static class SortLegality { + private boolean myIsLegal; + + SortLegality() { + this.myIsLegal = true; + } + private void setLegal(boolean theLegal) { + myIsLegal = theLegal; + } + + public boolean isLegal() { + return myIsLegal; + } + } + } diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseHapiFhirDao.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseHapiFhirDao.java index 9bdb1a5466f..005320e8e67 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseHapiFhirDao.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseHapiFhirDao.java @@ -70,6 +70,7 @@ import ca.uhn.fhir.parser.DataFormatException; import ca.uhn.fhir.parser.IParser; import ca.uhn.fhir.parser.LenientErrorHandler; import ca.uhn.fhir.rest.api.Constants; +import ca.uhn.fhir.rest.api.InterceptorInvocationTimingEnum; import ca.uhn.fhir.rest.api.RestOperationTypeEnum; import ca.uhn.fhir.rest.api.RestSearchParameterTypeEnum; import ca.uhn.fhir.rest.api.server.IBundleProvider; @@ -1365,7 +1366,7 @@ public abstract class BaseHapiFhirDao extends BaseStora .add(RequestDetails.class, theRequestDetails) .addIfMatchesType(ServletRequestDetails.class, theRequestDetails) .add(TransactionDetails.class, theTransactionDetails) - .add(Boolean.class, theTransactionDetails.isPointcutDeferred(Pointcut.STORAGE_PRECOMMIT_RESOURCE_CREATED)); + .add(InterceptorInvocationTimingEnum.class, theTransactionDetails.getInvocationTiming(Pointcut.STORAGE_PRECOMMIT_RESOURCE_CREATED)); doCallHooks(theTransactionDetails, theRequestDetails, Pointcut.STORAGE_PRECOMMIT_RESOURCE_UPDATED, hookParams); } diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseHapiFhirResourceDao.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseHapiFhirResourceDao.java index 8c49bccb759..cf768c557d7 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseHapiFhirResourceDao.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseHapiFhirResourceDao.java @@ -54,6 +54,7 @@ import ca.uhn.fhir.jpa.search.PersistedJpaBundleProvider; import ca.uhn.fhir.jpa.search.cache.SearchCacheStatusEnum; import ca.uhn.fhir.jpa.search.reindex.IResourceReindexingSvc; import ca.uhn.fhir.jpa.searchparam.MatchUrlService; +import ca.uhn.fhir.rest.api.InterceptorInvocationTimingEnum; import ca.uhn.fhir.rest.api.SearchContainedModeEnum; import ca.uhn.fhir.jpa.searchparam.SearchParameterMap; import ca.uhn.fhir.jpa.util.JpaInterceptorBroadcaster; @@ -377,7 +378,7 @@ public abstract class BaseHapiFhirResourceDao extends B .add(RequestDetails.class, theRequest) .addIfMatchesType(ServletRequestDetails.class, theRequest) .add(TransactionDetails.class, theTransactionDetails) - .add(Boolean.class, theTransactionDetails.isPointcutDeferred(Pointcut.STORAGE_PRECOMMIT_RESOURCE_CREATED)); + .add(InterceptorInvocationTimingEnum.class, theTransactionDetails.getInvocationTiming(Pointcut.STORAGE_PRECOMMIT_RESOURCE_CREATED)); doCallHooks(theTransactionDetails, theRequest, Pointcut.STORAGE_PRECOMMIT_RESOURCE_CREATED, hookParams); } @@ -485,7 +486,7 @@ public abstract class BaseHapiFhirResourceDao extends B .add(RequestDetails.class, theRequestDetails) .addIfMatchesType(ServletRequestDetails.class, theRequestDetails) .add(TransactionDetails.class, theTransactionDetails) - .add(Boolean.class, theTransactionDetails.isPointcutDeferred(Pointcut.STORAGE_PRECOMMIT_RESOURCE_DELETED)); + .add(InterceptorInvocationTimingEnum.class, theTransactionDetails.getInvocationTiming(Pointcut.STORAGE_PRECOMMIT_RESOURCE_DELETED)); doCallHooks(theTransactionDetails, theRequestDetails, Pointcut.STORAGE_PRECOMMIT_RESOURCE_DELETED, hookParams); @@ -597,7 +598,7 @@ public abstract class BaseHapiFhirResourceDao extends B .add(RequestDetails.class, theRequest) .addIfMatchesType(ServletRequestDetails.class, theRequest) .add(TransactionDetails.class, transactionDetails) - .add(Boolean.class, transactionDetails.isPointcutDeferred(Pointcut.STORAGE_PRECOMMIT_RESOURCE_DELETED)); + .add(InterceptorInvocationTimingEnum.class, transactionDetails.getInvocationTiming(Pointcut.STORAGE_PRECOMMIT_RESOURCE_DELETED)); doCallHooks(transactionDetails, theRequest, Pointcut.STORAGE_PRECOMMIT_RESOURCE_DELETED, hookParams); } }); @@ -697,7 +698,7 @@ public abstract class BaseHapiFhirResourceDao extends B .add(RequestDetails.class, theRequestDetails) .addIfMatchesType(ServletRequestDetails.class, theRequestDetails) .add(TransactionDetails.class, theTransactionDetails) - .add(Boolean.class, theTransactionDetails.isPointcutDeferred(Pointcut.STORAGE_PRECOMMIT_RESOURCE_UPDATED)); + .add(InterceptorInvocationTimingEnum.class, theTransactionDetails.getInvocationTiming(Pointcut.STORAGE_PRECOMMIT_RESOURCE_UPDATED)); myInterceptorBroadcaster.callHooks(Pointcut.STORAGE_PRECOMMIT_RESOURCE_UPDATED, preCommitParams); } @@ -742,7 +743,7 @@ public abstract class BaseHapiFhirResourceDao extends B .add(RequestDetails.class, theRequestDetails) .addIfMatchesType(ServletRequestDetails.class, theRequestDetails) .add(TransactionDetails.class, theTransactionDetails) - .add(Boolean.class, theTransactionDetails.isPointcutDeferred(Pointcut.STORAGE_PRECOMMIT_RESOURCE_UPDATED)); + .add(InterceptorInvocationTimingEnum.class, theTransactionDetails.getInvocationTiming(Pointcut.STORAGE_PRECOMMIT_RESOURCE_UPDATED)); myInterceptorBroadcaster.callHooks(Pointcut.STORAGE_PRECOMMIT_RESOURCE_UPDATED, preCommitParams); diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/TransactionHookTest.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/TransactionHookTest.java index 8413a6d8118..bcb255187b9 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/TransactionHookTest.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/TransactionHookTest.java @@ -4,6 +4,7 @@ import ca.uhn.fhir.interceptor.api.HookParams; import ca.uhn.fhir.interceptor.api.IInterceptorService; import ca.uhn.fhir.interceptor.api.Pointcut; import ca.uhn.fhir.jpa.api.model.DaoMethodOutcome; +import ca.uhn.fhir.rest.api.InterceptorInvocationTimingEnum; import ca.uhn.fhir.rest.api.server.storage.DeferredInterceptorBroadcasts; import ca.uhn.fhir.rest.server.exceptions.ResourceGoneException; import ca.uhn.fhir.rest.server.exceptions.ResourceVersionConflictException; @@ -313,14 +314,14 @@ public class TransactionHookTest extends BaseJpaR4SystemTest { assertThat(createPointcutInvocations, hasSize(2)); IBaseResource firstCreatedResource = createPointcutInvocations.get(0).get(IBaseResource.class); - boolean wasDeferred = createPointcutInvocations.get(0).get(Boolean.class); + InterceptorInvocationTimingEnum timing = createPointcutInvocations.get(0).get(InterceptorInvocationTimingEnum.class); assertTrue(firstCreatedResource instanceof Observation); - assertTrue(wasDeferred); + assertTrue(timing.equals(InterceptorInvocationTimingEnum.DEFERRED)); IBaseResource secondCreatedResource = createPointcutInvocations.get(1).get(IBaseResource.class); - wasDeferred = createPointcutInvocations.get(1).get(Boolean.class); + timing = createPointcutInvocations.get(1).get(InterceptorInvocationTimingEnum.class); assertTrue(secondCreatedResource instanceof Patient); - assertTrue(wasDeferred); + assertTrue(timing.equals(InterceptorInvocationTimingEnum.DEFERRED)); assertThat(deferredInterceptorBroadcasts.get(Pointcut.STORAGE_PRECOMMIT_RESOURCE_DELETED), hasSize(1)); } diff --git a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/api/server/storage/TransactionDetails.java b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/api/server/storage/TransactionDetails.java index ac8794b7e00..80af36bfbfe 100644 --- a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/api/server/storage/TransactionDetails.java +++ b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/api/server/storage/TransactionDetails.java @@ -22,6 +22,7 @@ package ca.uhn.fhir.rest.api.server.storage; import ca.uhn.fhir.interceptor.api.HookParams; import ca.uhn.fhir.interceptor.api.Pointcut; +import ca.uhn.fhir.rest.api.InterceptorInvocationTimingEnum; import com.google.common.collect.ArrayListMultimap; import com.google.common.collect.ListMultimap; import org.apache.commons.lang3.Validate; @@ -195,12 +196,12 @@ public class TransactionDetails { myDeferredInterceptorBroadcasts.put(thePointcut, theHookParams); } - public Boolean isPointcutDeferred(Pointcut thePointcut) { + public InterceptorInvocationTimingEnum getInvocationTiming(Pointcut thePointcut) { if (myDeferredInterceptorBroadcasts == null) { - return false; + return InterceptorInvocationTimingEnum.ACTIVE; } List hookParams = myDeferredInterceptorBroadcasts.get(thePointcut); - return hookParams != null; + return hookParams == null ? InterceptorInvocationTimingEnum.ACTIVE : InterceptorInvocationTimingEnum.DEFERRED; } public void deferredBroadcastProcessingFinished() { diff --git a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/provider/HashMapResourceProvider.java b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/provider/HashMapResourceProvider.java index 421210bf5db..34e5da5c78a 100644 --- a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/provider/HashMapResourceProvider.java +++ b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/provider/HashMapResourceProvider.java @@ -39,6 +39,7 @@ import ca.uhn.fhir.rest.annotation.RequiredParam; import ca.uhn.fhir.rest.annotation.ResourceParam; import ca.uhn.fhir.rest.annotation.Search; import ca.uhn.fhir.rest.annotation.Update; +import ca.uhn.fhir.rest.api.InterceptorInvocationTimingEnum; import ca.uhn.fhir.rest.api.MethodOutcome; import ca.uhn.fhir.rest.api.server.IPreResourceAccessDetails; import ca.uhn.fhir.rest.api.server.IPreResourceShowDetails; @@ -400,7 +401,7 @@ public class HashMapResourceProvider implements IResour .addIfMatchesType(ServletRequestDetails.class, theRequestDetails) .add(IBaseResource.class, theResource) .add(TransactionDetails.class, theTransactionDetails) - .add(Boolean.class, theTransactionDetails.isPointcutDeferred(Pointcut.STORAGE_PRECOMMIT_RESOURCE_CREATED)); + .add(InterceptorInvocationTimingEnum.class, theTransactionDetails.getInvocationTiming(Pointcut.STORAGE_PRECOMMIT_RESOURCE_CREATED)); interceptorBroadcaster.callHooks(Pointcut.STORAGE_PRECOMMIT_RESOURCE_CREATED, preCommitParams); } else { @@ -421,7 +422,7 @@ public class HashMapResourceProvider implements IResour .add(IBaseResource.class, myIdToHistory.get(theIdPart).getFirst()) .add(IBaseResource.class, theResource) .add(TransactionDetails.class, theTransactionDetails) - .add(Boolean.class, theTransactionDetails.isPointcutDeferred(Pointcut.STORAGE_PRECOMMIT_RESOURCE_CREATED)); + .add(InterceptorInvocationTimingEnum.class, theTransactionDetails.getInvocationTiming(Pointcut.STORAGE_PRECOMMIT_RESOURCE_CREATED)); interceptorBroadcaster.callHooks(Pointcut.STORAGE_PRECOMMIT_RESOURCE_UPDATED, preCommitParams); } From c80d18aca2e347def678a931c6f3d2ffbe9e631e Mon Sep 17 00:00:00 2001 From: Tadgh Date: Wed, 21 Apr 2021 11:16:10 -0400 Subject: [PATCH 19/30] Fix test for pointcut --- hapi-fhir-base/src/main/java/ca/uhn/fhir/util/BundleUtil.java | 1 - .../uhn/fhir/interceptor/executor/InterceptorServiceTest.java | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/hapi-fhir-base/src/main/java/ca/uhn/fhir/util/BundleUtil.java b/hapi-fhir-base/src/main/java/ca/uhn/fhir/util/BundleUtil.java index 174be06e559..5b4c732f85b 100644 --- a/hapi-fhir-base/src/main/java/ca/uhn/fhir/util/BundleUtil.java +++ b/hapi-fhir-base/src/main/java/ca/uhn/fhir/util/BundleUtil.java @@ -350,7 +350,6 @@ public class BundleUtil { map.put(parts, nextEntry); } return map; - } /** diff --git a/hapi-fhir-base/src/test/java/ca/uhn/fhir/interceptor/executor/InterceptorServiceTest.java b/hapi-fhir-base/src/test/java/ca/uhn/fhir/interceptor/executor/InterceptorServiceTest.java index fd180b9ab75..4eb9de9fafe 100644 --- a/hapi-fhir-base/src/test/java/ca/uhn/fhir/interceptor/executor/InterceptorServiceTest.java +++ b/hapi-fhir-base/src/test/java/ca/uhn/fhir/interceptor/executor/InterceptorServiceTest.java @@ -460,7 +460,7 @@ public class InterceptorServiceTest { svc.haveAppropriateParams(Pointcut.STORAGE_PRECOMMIT_RESOURCE_UPDATED, params); fail(); } catch (IllegalArgumentException e) { - assertEquals("Wrong number of params for pointcut STORAGE_PRECOMMIT_RESOURCE_UPDATED - Wanted ca.uhn.fhir.rest.api.server.RequestDetails,ca.uhn.fhir.rest.api.server.storage.TransactionDetails,ca.uhn.fhir.rest.server.servlet.ServletRequestDetails,java.lang.Boolean,org.hl7.fhir.instance.model.api.IBaseResource,org.hl7.fhir.instance.model.api.IBaseResource but found [String, String, String, String, String, String, String]", e.getMessage()); + assertEquals("Wrong number of params for pointcut STORAGE_PRECOMMIT_RESOURCE_UPDATED - Wanted ca.uhn.fhir.rest.api.InterceptorInvocationTimingEnum,ca.uhn.fhir.rest.api.server.RequestDetails,ca.uhn.fhir.rest.api.server.storage.TransactionDetails,ca.uhn.fhir.rest.server.servlet.ServletRequestDetails,org.hl7.fhir.instance.model.api.IBaseResource,org.hl7.fhir.instance.model.api.IBaseResource but found [String, String, String, String, String, String, String]", e.getMessage()); } } From 596196b3e1044cf21214515239edcb9d2ac89110 Mon Sep 17 00:00:00 2001 From: Tadgh Date: Fri, 23 Apr 2021 09:08:41 -0400 Subject: [PATCH 20/30] Add licenses --- .../api/InterceptorInvocationTimingEnum.java | 20 +++++++++++++++++++ .../DeferredInterceptorBroadcasts.java | 20 +++++++++++++++++++ 2 files changed, 40 insertions(+) diff --git a/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/api/InterceptorInvocationTimingEnum.java b/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/api/InterceptorInvocationTimingEnum.java index 173774a9133..044f65656bd 100644 --- a/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/api/InterceptorInvocationTimingEnum.java +++ b/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/api/InterceptorInvocationTimingEnum.java @@ -1,5 +1,25 @@ package ca.uhn.fhir.rest.api; +/*- + * #%L + * HAPI FHIR - Core Library + * %% + * Copyright (C) 2014 - 2021 Smile CDR, Inc. + * %% + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * #L% + */ + /** * during invocation of certain pointcuts, it is important to know whether they are being executed in * active or deferred fashion. This enum allows the pointcuts to see how they were invoked. diff --git a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/api/server/storage/DeferredInterceptorBroadcasts.java b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/api/server/storage/DeferredInterceptorBroadcasts.java index c8bcc355d4e..beb54f96137 100644 --- a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/api/server/storage/DeferredInterceptorBroadcasts.java +++ b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/api/server/storage/DeferredInterceptorBroadcasts.java @@ -1,5 +1,25 @@ package ca.uhn.fhir.rest.api.server.storage; +/*- + * #%L + * HAPI FHIR - Server Framework + * %% + * Copyright (C) 2014 - 2021 Smile CDR, Inc. + * %% + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * #L% + */ + import ca.uhn.fhir.interceptor.api.HookParams; import ca.uhn.fhir.interceptor.api.Pointcut; import com.google.common.collect.ListMultimap; From 0b8132b1a2fa2ae5bf2678dc977bfd94d117da08 Mon Sep 17 00:00:00 2001 From: Tadgh Date: Mon, 26 Apr 2021 11:04:39 -0400 Subject: [PATCH 21/30] Bump elastic version --- pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pom.xml b/pom.xml index 38b66b8cffa..0fdbee1c23f 100644 --- a/pom.xml +++ b/pom.xml @@ -1531,7 +1531,7 @@ org.elasticsearch.client elasticsearch-rest-high-level-client - 7.10.2 + 7.12.0 org.hibernate.search From 9ecb390dae232e9b46974d22f791327f9dbc7b79 Mon Sep 17 00:00:00 2001 From: Tadgh Date: Mon, 26 Apr 2021 13:40:11 -0400 Subject: [PATCH 22/30] Move tests to proper class. Fix issue with version referencing --- .../java/ca/uhn/fhir/util/BundleUtil.java | 8 +- .../fhir/jpa/dao/r4/TransactionHookTest.java | 217 ----------------- .../uhn/fhir/util/bundle/BundleUtilTest.java | 225 ++++++++++++++++++ 3 files changed, 229 insertions(+), 221 deletions(-) diff --git a/hapi-fhir-base/src/main/java/ca/uhn/fhir/util/BundleUtil.java b/hapi-fhir-base/src/main/java/ca/uhn/fhir/util/BundleUtil.java index 5b4c732f85b..f49c9398170 100644 --- a/hapi-fhir-base/src/main/java/ca/uhn/fhir/util/BundleUtil.java +++ b/hapi-fhir-base/src/main/java/ca/uhn/fhir/util/BundleUtil.java @@ -238,7 +238,7 @@ public class BundleUtil { for (BundleEntryParts bundleEntryPart : bundleEntryParts) { IBaseResource resource = bundleEntryPart.getResource(); - String resourceId = resource.getIdElement().toString(); + String resourceId = resource.getIdElement().toVersionless().toString(); resourceIdToBundleEntryMap.put(resourceId, bundleEntryPart); if (resourceId == null) { if (bundleEntryPart.getFullUrl() != null) { @@ -252,7 +252,7 @@ public class BundleUtil { for (BundleEntryParts bundleEntryPart : bundleEntryParts) { IBaseResource resource = bundleEntryPart.getResource(); - String resourceId = resource.getIdElement().toString(); + String resourceId = resource.getIdElement().toVersionless().toString(); resourceIdToBundleEntryMap.put(resourceId, bundleEntryPart); if (resourceId == null) { if (bundleEntryPart.getFullUrl() != null) { @@ -263,12 +263,12 @@ public class BundleUtil { String finalResourceId = resourceId; allResourceReferences .forEach(refInfo -> { - String referencedResourceId = refInfo.getResourceReference().getReferenceElement().getValue(); + String referencedResourceId = refInfo.getResourceReference().getReferenceElement().toVersionless().getValue(); if (color.containsKey(referencedResourceId)) { if (!adjList.containsKey(finalResourceId)) { adjList.put(finalResourceId, new ArrayList<>()); } - adjList.get(finalResourceId).add(refInfo.getResourceReference().getReferenceElement().getValue()); + adjList.get(finalResourceId).add(referencedResourceId); } }); } diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/TransactionHookTest.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/TransactionHookTest.java index bcb255187b9..4a63a512a64 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/TransactionHookTest.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/TransactionHookTest.java @@ -8,36 +8,27 @@ import ca.uhn.fhir.rest.api.InterceptorInvocationTimingEnum; import ca.uhn.fhir.rest.api.server.storage.DeferredInterceptorBroadcasts; import ca.uhn.fhir.rest.server.exceptions.ResourceGoneException; import ca.uhn.fhir.rest.server.exceptions.ResourceVersionConflictException; -import ca.uhn.fhir.util.BundleUtil; import ca.uhn.test.concurrency.PointcutLatch; import com.google.common.collect.ListMultimap; import org.hl7.fhir.instance.model.api.IBaseResource; import org.hl7.fhir.instance.model.api.IIdType; import org.hl7.fhir.r4.model.Bundle; import org.hl7.fhir.r4.model.DiagnosticReport; -import org.hl7.fhir.r4.model.ExplanationOfBenefit; -import org.hl7.fhir.r4.model.Medication; import org.hl7.fhir.r4.model.Observation; -import org.hl7.fhir.r4.model.Organization; import org.hl7.fhir.r4.model.Patient; -import org.hl7.fhir.r4.model.Quantity; import org.hl7.fhir.r4.model.Reference; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.springframework.beans.factory.annotation.Autowired; -import java.util.Collections; import java.util.List; -import static org.hamcrest.Matchers.is; import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.empty; -import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.hasSize; import static org.hamcrest.Matchers.matchesPattern; import static org.hl7.fhir.r4.model.Bundle.HTTPVerb.DELETE; -import static org.hl7.fhir.r4.model.Bundle.HTTPVerb.GET; import static org.hl7.fhir.r4.model.Bundle.HTTPVerb.POST; import static org.hl7.fhir.r4.model.Bundle.HTTPVerb.PUT; import static org.junit.jupiter.api.Assertions.assertTrue; @@ -63,214 +54,6 @@ public class TransactionHookTest extends BaseJpaR4SystemTest { myInterceptorService.registerAnonymousInterceptor(Pointcut.STORAGE_PRECOMMIT_RESOURCE_DELETED, myPointcutLatch); } - @Test - public void testTopologicalTransactionSortForCreates() { - - Bundle b = new Bundle(); - Bundle.BundleEntryComponent bundleEntryComponent = b.addEntry(); - final Observation obs1 = new Observation(); - obs1.setStatus(Observation.ObservationStatus.FINAL); - obs1.setSubject(new Reference("Patient/P1")); - obs1.setValue(new Quantity(4)); - obs1.setId("Observation/O1"); - bundleEntryComponent.setResource(obs1); - bundleEntryComponent.getRequest().setMethod(POST).setUrl("Observation"); - - bundleEntryComponent = b.addEntry(); - final Observation obs2 = new Observation(); - obs2.setStatus(Observation.ObservationStatus.FINAL); - obs2.setValue(new Quantity(4)); - obs2.setId("Observation/O2"); - bundleEntryComponent.setResource(obs2); - bundleEntryComponent.getRequest().setMethod(POST).setUrl("Observation"); - - Bundle.BundleEntryComponent patientComponent = b.addEntry(); - Patient pat1 = new Patient(); - pat1.setId("Patient/P1"); - pat1.setManagingOrganization(new Reference("Organization/Org1")); - patientComponent.setResource(pat1); - patientComponent.getRequest().setMethod(POST).setUrl("Patient"); - - Bundle.BundleEntryComponent organizationComponent = b.addEntry(); - Organization org1 = new Organization(); - org1.setId("Organization/Org1"); - organizationComponent.setResource(org1); - organizationComponent.getRequest().setMethod(POST).setUrl("Patient"); - - BundleUtil.sortEntriesIntoProcessingOrder(myFhirCtx, b); - - assertThat(b.getEntry(), hasSize(4)); - - List entry = b.getEntry(); - int observationIndex = getIndexOfEntryWithId("Observation/O1", b); - int patientIndex = getIndexOfEntryWithId("Patient/P1", b); - int organizationIndex = getIndexOfEntryWithId("Organization/Org1", b); - - assertTrue(organizationIndex < patientIndex); - assertTrue(patientIndex < observationIndex); - } - - @Test - public void testTransactionSorterFailsOnCyclicReference() { - Bundle b = new Bundle(); - Bundle.BundleEntryComponent bundleEntryComponent = b.addEntry(); - final Observation obs1 = new Observation(); - obs1.setStatus(Observation.ObservationStatus.FINAL); - obs1.setSubject(new Reference("Patient/P1")); - obs1.setValue(new Quantity(4)); - obs1.setId("Observation/O1"); - obs1.setHasMember(Collections.singletonList(new Reference("Observation/O2"))); - bundleEntryComponent.setResource(obs1); - bundleEntryComponent.getRequest().setMethod(POST).setUrl("Observation"); - - bundleEntryComponent = b.addEntry(); - final Observation obs2 = new Observation(); - obs2.setStatus(Observation.ObservationStatus.FINAL); - obs2.setValue(new Quantity(4)); - obs2.setId("Observation/O2"); - obs2.setHasMember(Collections.singletonList(new Reference("Observation/O1"))); - bundleEntryComponent.setResource(obs2); - bundleEntryComponent.getRequest().setMethod(POST).setUrl("Observation"); - try { - BundleUtil.sortEntriesIntoProcessingOrder(myFhirCtx, b); - fail(); - } catch (IllegalStateException e ) { - - } - } - - @Test - public void testTransactionSortingReturnsOperationsInCorrectOrder() { - - Bundle b = new Bundle(); - - //UPDATE patient - Bundle.BundleEntryComponent patientUpdateComponent= b.addEntry(); - final Patient p2 = new Patient(); - p2.setId("Patient/P2"); - p2.getNameFirstRep().setFamily("Test!"); - patientUpdateComponent.setResource(p2); - patientUpdateComponent.getRequest().setMethod(PUT).setUrl("Patient/P2"); - - //CREATE observation - Bundle.BundleEntryComponent bundleEntryComponent = b.addEntry(); - final Observation obs1 = new Observation(); - obs1.setStatus(Observation.ObservationStatus.FINAL); - obs1.setSubject(new Reference("Patient/P1")); - obs1.setValue(new Quantity(4)); - obs1.setId("Observation/O1"); - bundleEntryComponent.setResource(obs1); - bundleEntryComponent.getRequest().setMethod(POST).setUrl("Observation"); - - //DELETE medication - Bundle.BundleEntryComponent medicationComponent= b.addEntry(); - final Medication med1 = new Medication(); - med1.setId("Medication/M1"); - medicationComponent.setResource(med1); - medicationComponent.getRequest().setMethod(DELETE).setUrl("Medication"); - - //GET medication - Bundle.BundleEntryComponent searchComponent = b.addEntry(); - searchComponent.getRequest().setMethod(GET).setUrl("Medication?code=123"); - - //CREATE patient - Bundle.BundleEntryComponent patientComponent = b.addEntry(); - Patient pat1 = new Patient(); - pat1.setId("Patient/P1"); - pat1.setManagingOrganization(new Reference("Organization/Org1")); - patientComponent.setResource(pat1); - patientComponent.getRequest().setMethod(POST).setUrl("Patient"); - - //CREATE organization - Bundle.BundleEntryComponent organizationComponent = b.addEntry(); - Organization org1 = new Organization(); - org1.setId("Organization/Org1"); - organizationComponent.setResource(org1); - organizationComponent.getRequest().setMethod(POST).setUrl("Organization"); - - //DELETE ExplanationOfBenefit - Bundle.BundleEntryComponent explanationOfBenefitComponent= b.addEntry(); - final ExplanationOfBenefit eob1 = new ExplanationOfBenefit(); - eob1.setId("ExplanationOfBenefit/E1"); - explanationOfBenefitComponent.setResource(eob1); - explanationOfBenefitComponent.getRequest().setMethod(DELETE).setUrl("ExplanationOfBenefit"); - - BundleUtil.sortEntriesIntoProcessingOrder(myFhirCtx, b); - - assertThat(b.getEntry(), hasSize(7)); - - List entry = b.getEntry(); - - // DELETEs first - assertThat(entry.get(0).getRequest().getMethod(), is(equalTo(DELETE))); - assertThat(entry.get(1).getRequest().getMethod(), is(equalTo(DELETE))); - // Then POSTs - assertThat(entry.get(2).getRequest().getMethod(), is(equalTo(POST))); - assertThat(entry.get(3).getRequest().getMethod(), is(equalTo(POST))); - assertThat(entry.get(4).getRequest().getMethod(), is(equalTo(POST))); - // Then PUTs - assertThat(entry.get(5).getRequest().getMethod(), is(equalTo(PUT))); - // Then GETs - assertThat(entry.get(6).getRequest().getMethod(), is(equalTo(GET))); - } - - @Test - public void testTransactionSorterReturnsDeletesInCorrectProcessingOrder() { - Bundle b = new Bundle(); - Bundle.BundleEntryComponent bundleEntryComponent = b.addEntry(); - final Observation obs1 = new Observation(); - obs1.setStatus(Observation.ObservationStatus.FINAL); - obs1.setSubject(new Reference("Patient/P1")); - obs1.setValue(new Quantity(4)); - obs1.setId("Observation/O1"); - bundleEntryComponent.setResource(obs1); - bundleEntryComponent.getRequest().setMethod(DELETE).setUrl("Observation"); - - bundleEntryComponent = b.addEntry(); - final Observation obs2 = new Observation(); - obs2.setStatus(Observation.ObservationStatus.FINAL); - obs2.setValue(new Quantity(4)); - obs2.setId("Observation/O2"); - bundleEntryComponent.setResource(obs2); - bundleEntryComponent.getRequest().setMethod(DELETE).setUrl("Observation"); - - Bundle.BundleEntryComponent patientComponent = b.addEntry(); - Patient pat1 = new Patient(); - pat1.setId("Patient/P1"); - pat1.setManagingOrganization(new Reference("Organization/Org1")); - patientComponent.setResource(pat1); - patientComponent.getRequest().setMethod(DELETE).setUrl("Patient"); - - Bundle.BundleEntryComponent organizationComponent = b.addEntry(); - Organization org1 = new Organization(); - org1.setId("Organization/Org1"); - organizationComponent.setResource(org1); - organizationComponent.getRequest().setMethod(DELETE).setUrl("Organization"); - - BundleUtil.sortEntriesIntoProcessingOrder(myFhirCtx, b); - - assertThat(b.getEntry(), hasSize(4)); - - int observationIndex = getIndexOfEntryWithId("Observation/O1", b); - int patientIndex = getIndexOfEntryWithId("Patient/P1", b); - int organizationIndex = getIndexOfEntryWithId("Organization/Org1", b); - - assertTrue(patientIndex < organizationIndex); - assertTrue(observationIndex < patientIndex); - } - - private int getIndexOfEntryWithId(String theResourceId, Bundle theBundle) { - List entries = theBundle.getEntry(); - for (int i = 0; i < entries.size(); i++) { - String id = entries.get(i).getResource().getIdElement().toUnqualifiedVersionless().toString(); - if (id.equals(theResourceId)) { - return i; - } - } - fail("Didn't find resource with ID " + theResourceId); - return -1; - } - @Test public void testHookShouldContainParamsForAllCreateUpdateDeleteInvocations() throws InterruptedException { diff --git a/hapi-fhir-structures-r4/src/test/java/ca/uhn/fhir/util/bundle/BundleUtilTest.java b/hapi-fhir-structures-r4/src/test/java/ca/uhn/fhir/util/bundle/BundleUtilTest.java index b27b77aec2c..faf4c6f5ba9 100644 --- a/hapi-fhir-structures-r4/src/test/java/ca/uhn/fhir/util/bundle/BundleUtilTest.java +++ b/hapi-fhir-structures-r4/src/test/java/ca/uhn/fhir/util/bundle/BundleUtilTest.java @@ -4,14 +4,31 @@ import ca.uhn.fhir.context.FhirContext; import ca.uhn.fhir.util.BundleUtil; import ca.uhn.fhir.util.TestUtil; import org.hl7.fhir.r4.model.Bundle; +import org.hl7.fhir.r4.model.ExplanationOfBenefit; +import org.hl7.fhir.r4.model.Medication; +import org.hl7.fhir.r4.model.Observation; +import org.hl7.fhir.r4.model.Organization; import org.hl7.fhir.r4.model.Patient; +import org.hl7.fhir.r4.model.Quantity; +import org.hl7.fhir.r4.model.Reference; import org.junit.jupiter.api.AfterAll; import org.junit.jupiter.api.Test; +import java.util.Collections; import java.util.List; import java.util.function.Consumer; +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.hasSize; +import static org.hamcrest.Matchers.is; +import static org.hl7.fhir.r4.model.Bundle.HTTPVerb.DELETE; +import static org.hl7.fhir.r4.model.Bundle.HTTPVerb.GET; +import static org.hl7.fhir.r4.model.Bundle.HTTPVerb.POST; +import static org.hl7.fhir.r4.model.Bundle.HTTPVerb.PUT; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.junit.jupiter.api.Assertions.fail; public class BundleUtilTest { @@ -87,6 +104,214 @@ public class BundleUtilTest { assertEquals("Observation?foo=bar", bundle.getEntryFirstRep().getRequest().getUrl()); } + @Test + public void testTopologicalTransactionSortForCreates() { + + Bundle b = new Bundle(); + Bundle.BundleEntryComponent bundleEntryComponent = b.addEntry(); + final Observation obs1 = new Observation(); + obs1.setStatus(Observation.ObservationStatus.FINAL); + obs1.setSubject(new Reference("Patient/P1")); + obs1.setValue(new Quantity(4)); + obs1.setId("Observation/O1"); + bundleEntryComponent.setResource(obs1); + bundleEntryComponent.getRequest().setMethod(POST).setUrl("Observation"); + + bundleEntryComponent = b.addEntry(); + final Observation obs2 = new Observation(); + obs2.setStatus(Observation.ObservationStatus.FINAL); + obs2.setValue(new Quantity(4)); + obs2.setId("Observation/O2"); + bundleEntryComponent.setResource(obs2); + bundleEntryComponent.getRequest().setMethod(POST).setUrl("Observation"); + + Bundle.BundleEntryComponent patientComponent = b.addEntry(); + Patient pat1 = new Patient(); + pat1.setId("Patient/P1"); + pat1.setManagingOrganization(new Reference("Organization/Org1")); + patientComponent.setResource(pat1); + patientComponent.getRequest().setMethod(POST).setUrl("Patient"); + + Bundle.BundleEntryComponent organizationComponent = b.addEntry(); + Organization org1 = new Organization(); + org1.setId("Organization/Org1"); + organizationComponent.setResource(org1); + organizationComponent.getRequest().setMethod(POST).setUrl("Patient"); + + BundleUtil.sortEntriesIntoProcessingOrder(ourCtx, b); + + assertThat(b.getEntry(), hasSize(4)); + + List entry = b.getEntry(); + int observationIndex = getIndexOfEntryWithId("Observation/O1", b); + int patientIndex = getIndexOfEntryWithId("Patient/P1", b); + int organizationIndex = getIndexOfEntryWithId("Organization/Org1", b); + + assertTrue(organizationIndex < patientIndex); + assertTrue(patientIndex < observationIndex); + } + + @Test + public void testTransactionSorterFailsOnCyclicReference() { + Bundle b = new Bundle(); + Bundle.BundleEntryComponent bundleEntryComponent = b.addEntry(); + final Observation obs1 = new Observation(); + obs1.setStatus(Observation.ObservationStatus.FINAL); + obs1.setSubject(new Reference("Patient/P1")); + obs1.setValue(new Quantity(4)); + obs1.setId("Observation/O1/_history/1"); + obs1.setHasMember(Collections.singletonList(new Reference("Observation/O2"))); + bundleEntryComponent.setResource(obs1); + bundleEntryComponent.getRequest().setMethod(POST).setUrl("Observation"); + + bundleEntryComponent = b.addEntry(); + final Observation obs2 = new Observation(); + obs2.setStatus(Observation.ObservationStatus.FINAL); + obs2.setValue(new Quantity(4)); + obs2.setId("Observation/O2/_history/1"); + obs2.setHasMember(Collections.singletonList(new Reference("Observation/O1/_history/3"))); + bundleEntryComponent.setResource(obs2); + bundleEntryComponent.getRequest().setMethod(POST).setUrl("Observation"); + try { + BundleUtil.sortEntriesIntoProcessingOrder(ourCtx, b); + fail(); + } catch (IllegalStateException e ) { + + } + } + + @Test + public void testTransactionSortingReturnsOperationsInCorrectOrder() { + + Bundle b = new Bundle(); + + //UPDATE patient + Bundle.BundleEntryComponent patientUpdateComponent= b.addEntry(); + final Patient p2 = new Patient(); + p2.setId("Patient/P2"); + p2.getNameFirstRep().setFamily("Test!"); + patientUpdateComponent.setResource(p2); + patientUpdateComponent.getRequest().setMethod(PUT).setUrl("Patient/P2"); + + //CREATE observation + Bundle.BundleEntryComponent bundleEntryComponent = b.addEntry(); + final Observation obs1 = new Observation(); + obs1.setStatus(Observation.ObservationStatus.FINAL); + obs1.setSubject(new Reference("Patient/P1")); + obs1.setValue(new Quantity(4)); + obs1.setId("Observation/O1"); + bundleEntryComponent.setResource(obs1); + bundleEntryComponent.getRequest().setMethod(POST).setUrl("Observation"); + + //DELETE medication + Bundle.BundleEntryComponent medicationComponent= b.addEntry(); + final Medication med1 = new Medication(); + med1.setId("Medication/M1"); + medicationComponent.setResource(med1); + medicationComponent.getRequest().setMethod(DELETE).setUrl("Medication"); + + //GET medication + Bundle.BundleEntryComponent searchComponent = b.addEntry(); + searchComponent.getRequest().setMethod(GET).setUrl("Medication?code=123"); + + //CREATE patient + Bundle.BundleEntryComponent patientComponent = b.addEntry(); + Patient pat1 = new Patient(); + pat1.setId("Patient/P1"); + pat1.setManagingOrganization(new Reference("Organization/Org1")); + patientComponent.setResource(pat1); + patientComponent.getRequest().setMethod(POST).setUrl("Patient"); + + //CREATE organization + Bundle.BundleEntryComponent organizationComponent = b.addEntry(); + Organization org1 = new Organization(); + org1.setId("Organization/Org1"); + organizationComponent.setResource(org1); + organizationComponent.getRequest().setMethod(POST).setUrl("Organization"); + + //DELETE ExplanationOfBenefit + Bundle.BundleEntryComponent explanationOfBenefitComponent= b.addEntry(); + final ExplanationOfBenefit eob1 = new ExplanationOfBenefit(); + eob1.setId("ExplanationOfBenefit/E1"); + explanationOfBenefitComponent.setResource(eob1); + explanationOfBenefitComponent.getRequest().setMethod(DELETE).setUrl("ExplanationOfBenefit"); + + BundleUtil.sortEntriesIntoProcessingOrder(ourCtx, b); + + assertThat(b.getEntry(), hasSize(7)); + + List entry = b.getEntry(); + + // DELETEs first + assertThat(entry.get(0).getRequest().getMethod(), is(equalTo(DELETE))); + assertThat(entry.get(1).getRequest().getMethod(), is(equalTo(DELETE))); + // Then POSTs + assertThat(entry.get(2).getRequest().getMethod(), is(equalTo(POST))); + assertThat(entry.get(3).getRequest().getMethod(), is(equalTo(POST))); + assertThat(entry.get(4).getRequest().getMethod(), is(equalTo(POST))); + // Then PUTs + assertThat(entry.get(5).getRequest().getMethod(), is(equalTo(PUT))); + // Then GETs + assertThat(entry.get(6).getRequest().getMethod(), is(equalTo(GET))); + } + + @Test + public void testTransactionSorterReturnsDeletesInCorrectProcessingOrder() { + Bundle b = new Bundle(); + Bundle.BundleEntryComponent bundleEntryComponent = b.addEntry(); + final Observation obs1 = new Observation(); + obs1.setStatus(Observation.ObservationStatus.FINAL); + obs1.setSubject(new Reference("Patient/P1")); + obs1.setValue(new Quantity(4)); + obs1.setId("Observation/O1"); + bundleEntryComponent.setResource(obs1); + bundleEntryComponent.getRequest().setMethod(DELETE).setUrl("Observation"); + + bundleEntryComponent = b.addEntry(); + final Observation obs2 = new Observation(); + obs2.setStatus(Observation.ObservationStatus.FINAL); + obs2.setValue(new Quantity(4)); + obs2.setId("Observation/O2"); + bundleEntryComponent.setResource(obs2); + bundleEntryComponent.getRequest().setMethod(DELETE).setUrl("Observation"); + + Bundle.BundleEntryComponent patientComponent = b.addEntry(); + Patient pat1 = new Patient(); + pat1.setId("Patient/P1"); + pat1.setManagingOrganization(new Reference("Organization/Org1")); + patientComponent.setResource(pat1); + patientComponent.getRequest().setMethod(DELETE).setUrl("Patient"); + + Bundle.BundleEntryComponent organizationComponent = b.addEntry(); + Organization org1 = new Organization(); + org1.setId("Organization/Org1"); + organizationComponent.setResource(org1); + organizationComponent.getRequest().setMethod(DELETE).setUrl("Organization"); + + BundleUtil.sortEntriesIntoProcessingOrder(ourCtx, b); + + assertThat(b.getEntry(), hasSize(4)); + + int observationIndex = getIndexOfEntryWithId("Observation/O1", b); + int patientIndex = getIndexOfEntryWithId("Patient/P1", b); + int organizationIndex = getIndexOfEntryWithId("Organization/Org1", b); + + assertTrue(patientIndex < organizationIndex); + assertTrue(observationIndex < patientIndex); + } + + private int getIndexOfEntryWithId(String theResourceId, Bundle theBundle) { + List entries = theBundle.getEntry(); + for (int i = 0; i < entries.size(); i++) { + String id = entries.get(i).getResource().getIdElement().toUnqualifiedVersionless().toString(); + if (id.equals(theResourceId)) { + return i; + } + } + fail("Didn't find resource with ID " + theResourceId); + return -1; + } + @AfterAll public static void afterClassClearContext() { TestUtil.clearAllStaticFieldsForUnitTest(); From b007fecb1dafef7a95695a0990947f81cbe04190 Mon Sep 17 00:00:00 2001 From: Tadgh Date: Mon, 26 Apr 2021 13:40:40 -0400 Subject: [PATCH 23/30] Add comment --- .../src/test/java/ca/uhn/fhir/util/bundle/BundleUtilTest.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/hapi-fhir-structures-r4/src/test/java/ca/uhn/fhir/util/bundle/BundleUtilTest.java b/hapi-fhir-structures-r4/src/test/java/ca/uhn/fhir/util/bundle/BundleUtilTest.java index faf4c6f5ba9..c7ddf26b077 100644 --- a/hapi-fhir-structures-r4/src/test/java/ca/uhn/fhir/util/bundle/BundleUtilTest.java +++ b/hapi-fhir-structures-r4/src/test/java/ca/uhn/fhir/util/bundle/BundleUtilTest.java @@ -169,7 +169,8 @@ public class BundleUtilTest { obs2.setStatus(Observation.ObservationStatus.FINAL); obs2.setValue(new Quantity(4)); obs2.setId("Observation/O2/_history/1"); - obs2.setHasMember(Collections.singletonList(new Reference("Observation/O1/_history/3"))); + //We use a random history version here to ensure cycles are counted without versions. + obs2.setHasMember(Collections.singletonList(new Reference("Observation/O1/_history/300"))); bundleEntryComponent.setResource(obs2); bundleEntryComponent.getRequest().setMethod(POST).setUrl("Observation"); try { From 3adc25934814fbf13a3979533afdf29d67374337 Mon Sep 17 00:00:00 2001 From: Tadgh Date: Mon, 26 Apr 2021 14:08:20 -0400 Subject: [PATCH 24/30] Productioninizg --- .../java/ca/uhn/fhir/util/BundleUtil.java | 46 +++++++++++-------- 1 file changed, 28 insertions(+), 18 deletions(-) diff --git a/hapi-fhir-base/src/main/java/ca/uhn/fhir/util/BundleUtil.java b/hapi-fhir-base/src/main/java/ca/uhn/fhir/util/BundleUtil.java index f49c9398170..26119e9b0fd 100644 --- a/hapi-fhir-base/src/main/java/ca/uhn/fhir/util/BundleUtil.java +++ b/hapi-fhir-base/src/main/java/ca/uhn/fhir/util/BundleUtil.java @@ -189,38 +189,42 @@ public class BundleUtil { static int BLACK = 3; /** + * Function which will do an in-place sort of a bundles' entries, to the correct processing order, which is: + * 1. Deletes + * 2. Creates + * 3. Updates * - * @param theContext - * @param theBundle + * Furthermore, within these operation types, the entries will be sorted based on the order in which they should be processed + * e.g. if you have 2 CREATEs, one for a Patient, and one for an Observation which has this Patient as its Subject, + * the patient will come first, then the observation. + * + * In cases of there being a cyclic dependency (e.g. Organization/1 is partOf Organization/2 and Organization/2 is partOf Organization/1) + * this function will throw an IllegalStateException. + * + * @param theContext The FhirContext. + * @param theBundle The {@link IBaseBundle} which contains the entries you would like sorted into processing order. */ - public static void sortEntriesIntoProcessingOrder(FhirContext theContext, IBaseBundle theBundle) { + public static void sortEntriesIntoProcessingOrder(FhirContext theContext, IBaseBundle theBundle) throws IllegalStateException { Map partsToIBaseMap = getPartsToIBaseMap(theContext, theBundle); LinkedHashSet retVal = new LinkedHashSet<>(); //Get all deletions. LinkedHashSet deleteParts = sortEntriesOfTypeIntoProcessingOrder(theContext, RequestTypeEnum.DELETE, partsToIBaseMap); - if (deleteParts == null) { - throw new IllegalStateException("Cycle!"); - } else { - retVal.addAll(deleteParts); - } + validatePartsNotNull(deleteParts); + retVal.addAll(deleteParts); //Get all Creations LinkedHashSet createParts= sortEntriesOfTypeIntoProcessingOrder(theContext, RequestTypeEnum.POST, partsToIBaseMap); - if (createParts== null) { - throw new IllegalStateException("Cycle!"); - } else { - retVal.addAll(createParts); - } + validatePartsNotNull(createParts); + retVal.addAll(createParts); // Get all Updates LinkedHashSet updateParts= sortEntriesOfTypeIntoProcessingOrder(theContext, RequestTypeEnum.PUT, partsToIBaseMap); - if (updateParts == null) { - throw new IllegalStateException("Cycle!"); - } else { - retVal.addAll(updateParts); - } + validatePartsNotNull(updateParts); + retVal.addAll(updateParts); + //Once we are done adding all DELETE, POST, PUT operations, add everything else. + //Since this is a set, it will just fail to add already-added operations. retVal.addAll(partsToIBaseMap.values()); //Blow away the entries and reset them in the right order. @@ -228,6 +232,12 @@ public class BundleUtil { TerserUtil.setField(theContext, "entry", theBundle, retVal.toArray(new IBase[0])); } + private static void validatePartsNotNull(LinkedHashSet theDeleteParts) { + if (theDeleteParts == null) { + throw new IllegalStateException("This transaction contains a cycle, so it cannot be sorted."); + } + } + private static LinkedHashSet sortEntriesOfTypeIntoProcessingOrder(FhirContext theContext, RequestTypeEnum theRequestTypeEnum, Map thePartsToIBaseMap) { SortLegality legality = new SortLegality(); HashMap color = new HashMap<>(); From bb9c7a43e7a0b11f7254ff314a3091ceeee37248 Mon Sep 17 00:00:00 2001 From: Kevin Hartmann Date: Mon, 26 Apr 2021 14:32:58 -0400 Subject: [PATCH 25/30] Added test to to verify operation section was added to Conformance Statement; Jame's changes to the Conformance Statement code have already made it pass, so no more code changes are necessary. :-) --- hapi-fhir-validation/pom.xml | 8 ++- ...rverCapabilityStatementProviderR4Test.java | 68 +++++++++++++++++++ 2 files changed, 75 insertions(+), 1 deletion(-) diff --git a/hapi-fhir-validation/pom.xml b/hapi-fhir-validation/pom.xml index b180c3de905..fe38263e439 100644 --- a/hapi-fhir-validation/pom.xml +++ b/hapi-fhir-validation/pom.xml @@ -297,8 +297,14 @@ + + ca.uhn.hapi.fhir + hapi-fhir-jpaserver-model + 5.4.0-PRE5-SNAPSHOT + test + - + diff --git a/hapi-fhir-validation/src/test/java/ca/uhn/fhir/rest/server/ServerCapabilityStatementProviderR4Test.java b/hapi-fhir-validation/src/test/java/ca/uhn/fhir/rest/server/ServerCapabilityStatementProviderR4Test.java index f151466e3aa..7b9ed78406a 100644 --- a/hapi-fhir-validation/src/test/java/ca/uhn/fhir/rest/server/ServerCapabilityStatementProviderR4Test.java +++ b/hapi-fhir-validation/src/test/java/ca/uhn/fhir/rest/server/ServerCapabilityStatementProviderR4Test.java @@ -2,6 +2,7 @@ package ca.uhn.fhir.rest.server; import ca.uhn.fhir.context.FhirContext; import ca.uhn.fhir.context.FhirVersionEnum; +import ca.uhn.fhir.jpa.model.util.JpaConstants; import ca.uhn.fhir.model.api.Include; import ca.uhn.fhir.model.api.annotation.Description; import ca.uhn.fhir.model.api.annotation.ResourceDef; @@ -46,6 +47,7 @@ import ca.uhn.fhir.validation.ValidationResult; import com.google.common.collect.Lists; import org.hl7.fhir.common.hapi.validation.validator.FhirInstanceValidator; 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.hl7.fhir.r4.model.Bundle; import org.hl7.fhir.r4.model.CapabilityStatement; @@ -60,6 +62,7 @@ import org.hl7.fhir.r4.model.DateType; import org.hl7.fhir.r4.model.DiagnosticReport; import org.hl7.fhir.r4.model.Encounter; import org.hl7.fhir.r4.model.Enumerations.PublicationStatus; +import org.hl7.fhir.r4.model.Group; import org.hl7.fhir.r4.model.IdType; import org.hl7.fhir.r4.model.Observation; import org.hl7.fhir.r4.model.OperationDefinition; @@ -76,9 +79,11 @@ import org.junit.jupiter.api.Test; import javax.servlet.ServletConfig; import javax.servlet.ServletException; import javax.servlet.http.HttpServletRequest; +import java.io.IOException; import java.util.ArrayList; import java.util.Collection; import java.util.Collections; +import java.util.Date; import java.util.HashSet; import java.util.List; import java.util.Set; @@ -1034,6 +1039,27 @@ public class ServerCapabilityStatementProviderR4Test { assertThat(toStrings(patientResource.getSearchRevInclude()), containsInAnyOrder("Observation:subject")); } + @Test + public void testBulkDataExport() throws ServletException { + RestfulServer rs = new RestfulServer(myCtx); + rs.setResourceProviders(new BulkDataExportProvider()); + rs.setServerAddressStrategy(new HardcodedServerAddressStrategy("http://localhost/baseR4")); + + ServerCapabilityStatementProvider sc = new ServerCapabilityStatementProvider(rs); + rs.setServerConformanceProvider(sc); + rs.init(createServletConfig()); + + CapabilityStatement conformance = (CapabilityStatement) sc.getServerConformance(createHttpServletRequest(), createRequestDetails(rs)); + ourLog.info(myCtx.newJsonParser().setPrettyPrint(true).encodeResourceToString(conformance)); + + List resources = conformance.getRestFirstRep().getResource(); + CapabilityStatementRestResourceComponent groupResource = resources.stream() + .filter(resource -> "Group".equals(resource.getType())) + .findFirst().get(); + ourLog.info("---"); + ourLog.info(groupResource.toString()); + } + private List toOperationIdParts(List theOperation) { ArrayList retVal = Lists.newArrayList(); for (CapabilityStatementRestResourceOperationComponent next : theOperation) { @@ -1070,6 +1096,48 @@ public class ServerCapabilityStatementProviderR4Test { return myCtx.newXmlParser().setPrettyPrint(false).encodeResourceToString(theResource); } + public static class BulkDataExportProvider implements IResourceProvider { + @Operation(name = JpaConstants.OPERATION_EXPORT, global = false /* set to true once we can handle this */, manualResponse = true, idempotent = true) + public void export( + @OperationParam(name = JpaConstants.PARAM_EXPORT_OUTPUT_FORMAT, min = 0, max = 1, typeName = "string") IPrimitiveType theOutputFormat, + @OperationParam(name = JpaConstants.PARAM_EXPORT_TYPE, min = 0, max = 1, typeName = "string") IPrimitiveType theType, + @OperationParam(name = JpaConstants.PARAM_EXPORT_SINCE, min = 0, max = 1, typeName = "instant") IPrimitiveType theSince, + @OperationParam(name = JpaConstants.PARAM_EXPORT_TYPE_FILTER, min = 0, max = 1, typeName = "string") IPrimitiveType theTypeFilter, + ServletRequestDetails theRequestDetails + ) { } + + @Operation(name = JpaConstants.OPERATION_EXPORT, manualResponse = true, idempotent = true, typeName = "Group") + public void groupExport( + @IdParam IIdType theIdParam, + @OperationParam(name = JpaConstants.PARAM_EXPORT_OUTPUT_FORMAT, min = 0, max = 1, typeName = "string") IPrimitiveType theOutputFormat, + @OperationParam(name = JpaConstants.PARAM_EXPORT_TYPE, min = 0, max = 1, typeName = "string") IPrimitiveType theType, + @OperationParam(name = JpaConstants.PARAM_EXPORT_SINCE, min = 0, max = 1, typeName = "instant") IPrimitiveType theSince, + @OperationParam(name = JpaConstants.PARAM_EXPORT_TYPE_FILTER, min = 0, max = 1, typeName = "string") IPrimitiveType theTypeFilter, + @OperationParam(name = JpaConstants.PARAM_EXPORT_MDM, min = 0, max = 1, typeName = "boolean") IPrimitiveType theMdm, + ServletRequestDetails theRequestDetails + ) {} + + @Operation(name = JpaConstants.OPERATION_EXPORT, manualResponse = true, idempotent = true, typeName = "Patient") + public void patientExport( + @OperationParam(name = JpaConstants.PARAM_EXPORT_OUTPUT_FORMAT, min = 0, max = 1, typeName = "string") IPrimitiveType theOutputFormat, + @OperationParam(name = JpaConstants.PARAM_EXPORT_TYPE, min = 0, max = 1, typeName = "string") IPrimitiveType theType, + @OperationParam(name = JpaConstants.PARAM_EXPORT_SINCE, min = 0, max = 1, typeName = "instant") IPrimitiveType theSince, + @OperationParam(name = JpaConstants.PARAM_EXPORT_TYPE_FILTER, min = 0, max = 1, typeName = "string") IPrimitiveType theTypeFilter, + ServletRequestDetails theRequestDetails + ) {} + + @Operation(name = JpaConstants.OPERATION_EXPORT_POLL_STATUS, manualResponse = true, idempotent = true) + public void exportPollStatus( + @OperationParam(name = JpaConstants.PARAM_EXPORT_POLL_STATUS_JOB_ID, typeName = "string", min = 0, max = 1) IPrimitiveType theJobId, + ServletRequestDetails theRequestDetails + ) throws IOException {} + + @Override + public Class getResourceType() { + return Group.class; + } + } + @SuppressWarnings("unused") public static class ConditionalProvider implements IResourceProvider { From 3a901a6671fc40d4fcc3649868fb6dd92fb9670c Mon Sep 17 00:00:00 2001 From: Kevin Hartmann Date: Mon, 26 Apr 2021 16:57:13 -0400 Subject: [PATCH 26/30] Cleaned up extra logging. --- .../rest/server/ServerCapabilityStatementProviderR4Test.java | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/hapi-fhir-validation/src/test/java/ca/uhn/fhir/rest/server/ServerCapabilityStatementProviderR4Test.java b/hapi-fhir-validation/src/test/java/ca/uhn/fhir/rest/server/ServerCapabilityStatementProviderR4Test.java index f56b7ac7855..9f789227954 100644 --- a/hapi-fhir-validation/src/test/java/ca/uhn/fhir/rest/server/ServerCapabilityStatementProviderR4Test.java +++ b/hapi-fhir-validation/src/test/java/ca/uhn/fhir/rest/server/ServerCapabilityStatementProviderR4Test.java @@ -1247,7 +1247,7 @@ public class ServerCapabilityStatementProviderR4Test { } @Search - public List search(@OptionalParam(name = "subject") ReferenceParam theSubject) { + public List search(@OptionalParam(name = "subject") ReferenceParam theSubject) { return Collections.emptyList(); } @@ -1289,8 +1289,6 @@ public class ServerCapabilityStatementProviderR4Test { CapabilityStatementRestResourceComponent groupResource = resources.stream() .filter(resource -> "Group".equals(resource.getType())) .findFirst().get(); - ourLog.info("---"); - ourLog.info(groupResource.toString()); } private List toOperationIdParts(List theOperation) { From a5e734fec8a69d0e9bd9b6e35fc431ed18c9c5b3 Mon Sep 17 00:00:00 2001 From: Tadgh Date: Mon, 26 Apr 2021 22:24:47 -0400 Subject: [PATCH 27/30] Handle sorting entries which contain no resources --- .../java/ca/uhn/fhir/i18n/HapiLocalizer.java | 2 + .../java/ca/uhn/fhir/util/BundleUtil.java | 55 ++++++++++--------- .../uhn/fhir/util/bundle/BundleUtilTest.java | 10 ++++ 3 files changed, 41 insertions(+), 26 deletions(-) diff --git a/hapi-fhir-base/src/main/java/ca/uhn/fhir/i18n/HapiLocalizer.java b/hapi-fhir-base/src/main/java/ca/uhn/fhir/i18n/HapiLocalizer.java index 403ea58cc21..a7fd6d24edf 100644 --- a/hapi-fhir-base/src/main/java/ca/uhn/fhir/i18n/HapiLocalizer.java +++ b/hapi-fhir-base/src/main/java/ca/uhn/fhir/i18n/HapiLocalizer.java @@ -10,6 +10,8 @@ import java.util.concurrent.ConcurrentHashMap; import static org.apache.commons.lang3.StringUtils.*; + + /* * #%L * HAPI FHIR - Core Library diff --git a/hapi-fhir-base/src/main/java/ca/uhn/fhir/util/BundleUtil.java b/hapi-fhir-base/src/main/java/ca/uhn/fhir/util/BundleUtil.java index 26119e9b0fd..f53af2cd4ca 100644 --- a/hapi-fhir-base/src/main/java/ca/uhn/fhir/util/BundleUtil.java +++ b/hapi-fhir-base/src/main/java/ca/uhn/fhir/util/BundleUtil.java @@ -248,39 +248,42 @@ public class BundleUtil { for (BundleEntryParts bundleEntryPart : bundleEntryParts) { IBaseResource resource = bundleEntryPart.getResource(); - String resourceId = resource.getIdElement().toVersionless().toString(); - resourceIdToBundleEntryMap.put(resourceId, bundleEntryPart); - if (resourceId == null) { - if (bundleEntryPart.getFullUrl() != null) { - resourceId = bundleEntryPart.getFullUrl(); + if (resource != null) { + String resourceId = resource.getIdElement().toVersionless().toString(); + resourceIdToBundleEntryMap.put(resourceId, bundleEntryPart); + if (resourceId == null) { + if (bundleEntryPart.getFullUrl() != null) { + resourceId = bundleEntryPart.getFullUrl(); + } } + + color.put(resourceId, WHITE); } - - color.put(resourceId, WHITE); - } for (BundleEntryParts bundleEntryPart : bundleEntryParts) { IBaseResource resource = bundleEntryPart.getResource(); - String resourceId = resource.getIdElement().toVersionless().toString(); - resourceIdToBundleEntryMap.put(resourceId, bundleEntryPart); - if (resourceId == null) { - if (bundleEntryPart.getFullUrl() != null) { - resourceId = bundleEntryPart.getFullUrl(); - } - } - List allResourceReferences = theContext.newTerser().getAllResourceReferences(resource); - String finalResourceId = resourceId; - allResourceReferences - .forEach(refInfo -> { - String referencedResourceId = refInfo.getResourceReference().getReferenceElement().toVersionless().getValue(); - if (color.containsKey(referencedResourceId)) { - if (!adjList.containsKey(finalResourceId)) { - adjList.put(finalResourceId, new ArrayList<>()); - } - adjList.get(finalResourceId).add(referencedResourceId); + if (resource != null) { + String resourceId = resource.getIdElement().toVersionless().toString(); + resourceIdToBundleEntryMap.put(resourceId, bundleEntryPart); + if (resourceId == null) { + if (bundleEntryPart.getFullUrl() != null) { + resourceId = bundleEntryPart.getFullUrl(); } - }); + } + List allResourceReferences = theContext.newTerser().getAllResourceReferences(resource); + String finalResourceId = resourceId; + allResourceReferences + .forEach(refInfo -> { + String referencedResourceId = refInfo.getResourceReference().getReferenceElement().toVersionless().getValue(); + if (color.containsKey(referencedResourceId)) { + if (!adjList.containsKey(finalResourceId)) { + adjList.put(finalResourceId, new ArrayList<>()); + } + adjList.get(finalResourceId).add(referencedResourceId); + } + }); + } } for (Map.Entry entry:color.entrySet()) { diff --git a/hapi-fhir-structures-r4/src/test/java/ca/uhn/fhir/util/bundle/BundleUtilTest.java b/hapi-fhir-structures-r4/src/test/java/ca/uhn/fhir/util/bundle/BundleUtilTest.java index c7ddf26b077..2fe10217652 100644 --- a/hapi-fhir-structures-r4/src/test/java/ca/uhn/fhir/util/bundle/BundleUtilTest.java +++ b/hapi-fhir-structures-r4/src/test/java/ca/uhn/fhir/util/bundle/BundleUtilTest.java @@ -1,6 +1,7 @@ package ca.uhn.fhir.util.bundle; import ca.uhn.fhir.context.FhirContext; +import ca.uhn.fhir.util.BundleBuilder; import ca.uhn.fhir.util.BundleUtil; import ca.uhn.fhir.util.TestUtil; import org.hl7.fhir.r4.model.Bundle; @@ -256,6 +257,15 @@ public class BundleUtilTest { assertThat(entry.get(6).getRequest().getMethod(), is(equalTo(GET))); } + @Test + public void testBundleSortsCanHandlesDeletesThatContainNoResources() { + Patient p = new Patient(); + p.setId("Patient/123"); + BundleBuilder builder = new BundleBuilder(ourCtx); + builder.addTransactionDeleteEntry(p); + BundleUtil.sortEntriesIntoProcessingOrder(ourCtx, builder.getBundle()); + } + @Test public void testTransactionSorterReturnsDeletesInCorrectProcessingOrder() { Bundle b = new Bundle(); From 4a5b62616cb59d6414b2c5e5180e1d81fed6df08 Mon Sep 17 00:00:00 2001 From: Kevin Hartmann Date: Tue, 27 Apr 2021 15:01:03 -0400 Subject: [PATCH 28/30] Improved assertions for testBulkDataExport() --- .../rest/server/ServerCapabilityStatementProviderR4Test.java | 2 ++ 1 file changed, 2 insertions(+) diff --git a/hapi-fhir-validation/src/test/java/ca/uhn/fhir/rest/server/ServerCapabilityStatementProviderR4Test.java b/hapi-fhir-validation/src/test/java/ca/uhn/fhir/rest/server/ServerCapabilityStatementProviderR4Test.java index 9f789227954..ca366cab38a 100644 --- a/hapi-fhir-validation/src/test/java/ca/uhn/fhir/rest/server/ServerCapabilityStatementProviderR4Test.java +++ b/hapi-fhir-validation/src/test/java/ca/uhn/fhir/rest/server/ServerCapabilityStatementProviderR4Test.java @@ -1289,6 +1289,8 @@ public class ServerCapabilityStatementProviderR4Test { CapabilityStatementRestResourceComponent groupResource = resources.stream() .filter(resource -> "Group".equals(resource.getType())) .findFirst().get(); + + assertThat(toOperationNames(groupResource.getOperation()),containsInAnyOrder("export", "export-poll-status")); } private List toOperationIdParts(List theOperation) { From 10b83a69d0ea49ce75950c5f14aa5f5b17d7319c Mon Sep 17 00:00:00 2001 From: Tadgh Date: Tue, 27 Apr 2021 15:58:48 -0400 Subject: [PATCH 29/30] Remove elastic bump as HS relies on it --- pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pom.xml b/pom.xml index 0fdbee1c23f..38b66b8cffa 100644 --- a/pom.xml +++ b/pom.xml @@ -1531,7 +1531,7 @@ org.elasticsearch.client elasticsearch-rest-high-level-client - 7.12.0 + 7.10.2 org.hibernate.search From b9151a4a758b2c80cdaea06ba45303fd6eab8d3c Mon Sep 17 00:00:00 2001 From: Kevin Hartmann Date: Tue, 27 Apr 2021 16:34:24 -0400 Subject: [PATCH 30/30] Added Changelog. --- .../2526-add-operation-section-to-conformance-statement.yaml | 4 ++++ 1 file changed, 4 insertions(+) create mode 100644 hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/5_4_0/2526-add-operation-section-to-conformance-statement.yaml diff --git a/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/5_4_0/2526-add-operation-section-to-conformance-statement.yaml b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/5_4_0/2526-add-operation-section-to-conformance-statement.yaml new file mode 100644 index 00000000000..9f720fdca60 --- /dev/null +++ b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/5_4_0/2526-add-operation-section-to-conformance-statement.yaml @@ -0,0 +1,4 @@ +--- +type: fix +issue: 2526 +title: "Added Operation section to Conformance Statement."