Should have realized that transactiondetails is an object reference, so we can't change it effectively

This commit is contained in:
Tadgh 2021-04-13 09:26:05 -04:00
parent 8099b6634a
commit f61d52d199
6 changed files with 84 additions and 27 deletions

View File

@ -1425,7 +1425,8 @@ public enum Pointcut implements IPointcut {
"org.hl7.fhir.instance.model.api.IBaseResource", "org.hl7.fhir.instance.model.api.IBaseResource",
"ca.uhn.fhir.rest.api.server.RequestDetails", "ca.uhn.fhir.rest.api.server.RequestDetails",
"ca.uhn.fhir.rest.server.servlet.ServletRequestDetails", "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", "org.hl7.fhir.instance.model.api.IBaseResource",
"ca.uhn.fhir.rest.api.server.RequestDetails", "ca.uhn.fhir.rest.api.server.RequestDetails",
"ca.uhn.fhir.rest.server.servlet.ServletRequestDetails", "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", "org.hl7.fhir.instance.model.api.IBaseResource",
"ca.uhn.fhir.rest.api.server.RequestDetails", "ca.uhn.fhir.rest.api.server.RequestDetails",
"ca.uhn.fhir.rest.server.servlet.ServletRequestDetails", "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 {
* </p> * </p>
*/ */
STORAGE_TRANSACTION_PROCESSED(void.class, 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.storage.DeferredInterceptorBroadcasts",
"ca.uhn.fhir.rest.api.server.RequestDetails", "ca.uhn.fhir.rest.api.server.RequestDetails",
"ca.uhn.fhir.rest.server.servlet.ServletRequestDetails", "ca.uhn.fhir.rest.server.servlet.ServletRequestDetails",

View File

@ -1364,7 +1364,8 @@ public abstract class BaseHapiFhirDao<T extends IBaseResource> extends BaseStora
.add(IBaseResource.class, theResource) .add(IBaseResource.class, theResource)
.add(RequestDetails.class, theRequestDetails) .add(RequestDetails.class, theRequestDetails)
.addIfMatchesType(ServletRequestDetails.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); doCallHooks(theTransactionDetails, theRequestDetails, Pointcut.STORAGE_PRECOMMIT_RESOURCE_UPDATED, hookParams);
} }

View File

@ -376,7 +376,8 @@ public abstract class BaseHapiFhirResourceDao<T extends IBaseResource> extends B
.add(IBaseResource.class, theResource) .add(IBaseResource.class, theResource)
.add(RequestDetails.class, theRequest) .add(RequestDetails.class, theRequest)
.addIfMatchesType(ServletRequestDetails.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); doCallHooks(theTransactionDetails, theRequest, Pointcut.STORAGE_PRECOMMIT_RESOURCE_CREATED, hookParams);
} }
@ -483,13 +484,11 @@ public abstract class BaseHapiFhirResourceDao<T extends IBaseResource> extends B
.add(IBaseResource.class, resourceToDelete) .add(IBaseResource.class, resourceToDelete)
.add(RequestDetails.class, theRequestDetails) .add(RequestDetails.class, theRequestDetails)
.addIfMatchesType(ServletRequestDetails.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); doCallHooks(theTransactionDetails, theRequestDetails, Pointcut.STORAGE_PRECOMMIT_RESOURCE_DELETED, hookParams);
} else {
doCallHooks(theTransactionDetails, theRequestDetails, Pointcut.STORAGE_PRECOMMIT_RESOURCE_DELETED, hookParams);
}
DaoMethodOutcome outcome = toMethodOutcome(theRequestDetails, savedEntity, resourceToDelete).setCreated(true); DaoMethodOutcome outcome = toMethodOutcome(theRequestDetails, savedEntity, resourceToDelete).setCreated(true);
@ -597,7 +596,8 @@ public abstract class BaseHapiFhirResourceDao<T extends IBaseResource> extends B
.add(IBaseResource.class, resourceToDelete) .add(IBaseResource.class, resourceToDelete)
.add(RequestDetails.class, theRequest) .add(RequestDetails.class, theRequest)
.addIfMatchesType(ServletRequestDetails.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); doCallHooks(transactionDetails, theRequest, Pointcut.STORAGE_PRECOMMIT_RESOURCE_DELETED, hookParams);
} }
}); });
@ -688,7 +688,8 @@ public abstract class BaseHapiFhirResourceDao<T extends IBaseResource> extends B
.add(IBaseResource.class, newVersion) .add(IBaseResource.class, newVersion)
.add(RequestDetails.class, theRequestDetails) .add(RequestDetails.class, theRequestDetails)
.addIfMatchesType(ServletRequestDetails.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_PRESTORAGE_RESOURCE_UPDATED, params);
myInterceptorBroadcaster.callHooks(Pointcut.STORAGE_PRECOMMIT_RESOURCE_UPDATED, params); myInterceptorBroadcaster.callHooks(Pointcut.STORAGE_PRECOMMIT_RESOURCE_UPDATED, params);

View File

@ -1030,13 +1030,13 @@ public abstract class BaseTransactionProcessor {
HookParams params = new HookParams() HookParams params = new HookParams()
.add(RequestDetails.class, theRequest) .add(RequestDetails.class, theRequest)
.addIfMatchesType(ServletRequestDetails.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); JpaInterceptorBroadcaster.doCallHooks(myInterceptorBroadcaster, theRequest, Pointcut.STORAGE_TRANSACTION_PROCESSED, params);
theTransactionDetails.deferredBroadcastProcessingFinished(); theTransactionDetails.deferredBroadcastProcessingFinished();
//finishedCallingDeferredInterceptorBroadcasts //finishedCallingDeferredInterceptorBroadcasts
return entriesToProcess; return entriesToProcess;

View File

@ -1,23 +1,34 @@
package ca.uhn.fhir.jpa.dao.r4; 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.IInterceptorService;
import ca.uhn.fhir.interceptor.api.Pointcut; 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.ResourceGoneException;
import ca.uhn.fhir.rest.server.exceptions.ResourceVersionConflictException; import ca.uhn.fhir.rest.server.exceptions.ResourceVersionConflictException;
import ca.uhn.test.concurrency.PointcutLatch; 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.instance.model.api.IIdType;
import org.hl7.fhir.r4.model.Bundle; import org.hl7.fhir.r4.model.Bundle;
import org.hl7.fhir.r4.model.DiagnosticReport; import org.hl7.fhir.r4.model.DiagnosticReport;
import org.hl7.fhir.r4.model.Observation; import org.hl7.fhir.r4.model.Observation;
import org.hl7.fhir.r4.model.Patient;
import org.hl7.fhir.r4.model.Reference; import org.hl7.fhir.r4.model.Reference;
import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test; import org.junit.jupiter.api.Test;
import org.springframework.beans.factory.annotation.Autowired; import org.springframework.beans.factory.annotation.Autowired;
import java.util.List;
import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.empty; import static org.hamcrest.Matchers.empty;
import static org.hamcrest.Matchers.hasSize;
import static org.hamcrest.Matchers.matchesPattern; import static org.hamcrest.Matchers.matchesPattern;
import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.junit.jupiter.api.Assertions.fail; import static org.junit.jupiter.api.Assertions.fail;
public class TransactionHookTest extends BaseJpaR4SystemTest { public class TransactionHookTest extends BaseJpaR4SystemTest {
@ -34,26 +45,62 @@ public class TransactionHookTest extends BaseJpaR4SystemTest {
@BeforeEach @BeforeEach
public void beforeEach() { public void beforeEach() {
myInterceptorService.registerAnonymousInterceptor(Pointcut.STORAGE_TRANSACTION_PROCESSED, myPointcutLatch); 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 @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(); final Observation obs1 = new Observation();
obs1.setStatus(Observation.ObservationStatus.FINAL); obs1.setStatus(Observation.ObservationStatus.FINAL);
obs1.setSubject(new Reference(urnReference));
Bundle b = new Bundle(); Bundle b = new Bundle();
Bundle.BundleEntryComponent bundleEntryComponent = b.addEntry(); Bundle.BundleEntryComponent bundleEntryComponent = b.addEntry();
bundleEntryComponent.setResource(obs1); 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 { //Delete an observation
mySystemDao.transaction(mySrd, b); b.addEntry().getRequest().setMethod(Bundle.HTTPVerb.DELETE).setUrl(daoMethodOutcome.getId().toUnqualifiedVersionless().getValue());
fail();
} catch (ResourceVersionConflictException e) {
// good, transaction should not succeed because DiagnosticReport has a reference to the obs2 myPointcutLatch.setExpectedCount(4);
} mySystemDao.transaction(mySrd, b);
List<HookParams> hookParams = myPointcutLatch.awaitExpected();
IBaseBundle bundleResponseParam = hookParams.get(3).get(IBaseBundle.class);
DeferredInterceptorBroadcasts broadcastsParam = hookParams.get(0).get(DeferredInterceptorBroadcasts.class);
ListMultimap<Pointcut, HookParams> deferredInterceptorBroadcasts = broadcastsParam.getDeferredInterceptorBroadcasts();
assertThat(deferredInterceptorBroadcasts.entries(), hasSize(3));
List<HookParams> 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 @Test

View File

@ -32,6 +32,7 @@ import java.util.Collections;
import java.util.Date; import java.util.Date;
import java.util.EnumSet; import java.util.EnumSet;
import java.util.HashMap; import java.util.HashMap;
import java.util.List;
import java.util.Map; import java.util.Map;
import java.util.function.Supplier; import java.util.function.Supplier;
@ -194,8 +195,12 @@ public class TransactionDetails {
myDeferredInterceptorBroadcasts.put(thePointcut, theHookParams); myDeferredInterceptorBroadcasts.put(thePointcut, theHookParams);
} }
public boolean isPointcutDeferred() { public Boolean isPointcutDeferred(Pointcut thePointcut) {
return myIsPointcutDeferred; if (myDeferredInterceptorBroadcasts == null) {
return false;
}
List<HookParams> hookParams = myDeferredInterceptorBroadcasts.get(thePointcut);
return hookParams != null;
} }
public void deferredBroadcastProcessingFinished() { public void deferredBroadcastProcessingFinished() {