From 38d03ea99ac5c38cd96f0afa19d4ccc1d36458b2 Mon Sep 17 00:00:00 2001 From: James Agnew Date: Wed, 23 Jan 2019 21:17:47 -0500 Subject: [PATCH] Invalid ids in subscription queue (#1175) * Start work on this * Work on interceptors * Attempt fix * Avoid environment dependency * Test fixes * One more test fix * One more build tweak * Lots of cleanup * A bit more cleanup * Still more cleanup * Some test fixes * Add legacy methods temporarily * Don't auto-scan interceptor beans * One more test fix * rsolve merge conflicts * Address review comments --- .../ca/uhn/fhir/jpa/dao/BaseHapiFhirDao.java | 34 ++- .../fhir/jpa/dao/BaseHapiFhirResourceDao.java | 67 +++-- .../uhn/fhir/jpa/dao/FhirSystemDaoDstu2.java | 2 +- .../ca/uhn/fhir/jpa/dao/SearchBuilder.java | 1 - .../fhir/jpa/dao/TransactionProcessor.java | 105 ++++--- ...rchParamWithInlineReferencesExtractor.java | 2 +- .../reindex/ResourceReindexingSvcImpl.java | 1 - .../SubscriptionActivatingInterceptor.java | 19 +- .../SubscriptionInterceptorLoader.java | 11 +- .../SubscriptionMatcherInterceptor.java | 21 +- .../ca/uhn/fhir/jpa/dao/r4/BaseJpaR4Test.java | 5 + .../test/InterceptorRegistryTest.java | 159 ----------- .../provider/r4/HookInterceptorR4Test.java | 110 +++++++ .../r4/ResourceProviderInterceptorR4Test.java | 1 + .../subscription/BaseSubscriptionsR4Test.java | 5 +- .../resthook/RestHookTestDstu2Test.java | 6 +- .../resthook/RestHookTestR4Test.java | 34 +++ .../RestHookWithInterceptorR4Test.java | 7 +- .../jpa/model/interceptor/api/HookParams.java | 9 +- .../api/IInterceptorBroadcaster.java | 35 +++ .../interceptor/api/IInterceptorRegistry.java | 47 +-- .../model/interceptor/api/Interceptor.java | 7 + .../jpa/model/interceptor/api/Pointcut.java | 96 ++++++- ...rRegistry.java => InterceptorService.java} | 223 ++++++++++----- .../executor/InterceptorServiceTest.java | 268 ++++++++++++++++++ .../src/test/resources/logback-test.xml | 15 + .../extractor/ResourceLinkExtractor.java | 39 ++- .../fhir/jpa/searchparam/retry/Retrier.java | 1 + .../module/ResourceModifiedMessage.java | 37 ++- .../module/cache/SubscriptionRegistry.java | 8 +- .../matcher/InMemorySubscriptionMatcher.java | 2 +- ...scriptionDeliveringRestHookSubscriber.java | 7 +- .../SubscriptionMatchingSubscriber.java | 10 +- .../config/TestSubscriptionDstu3Config.java | 13 +- .../InMemorySubscriptionMatcherTestR3.java | 27 ++ ...kingQueueSubscribableChannelDstu3Test.java | 4 +- 36 files changed, 1022 insertions(+), 416 deletions(-) delete mode 100644 hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/interceptor/test/InterceptorRegistryTest.java create mode 100644 hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/r4/HookInterceptorR4Test.java create mode 100644 hapi-fhir-jpaserver-model/src/main/java/ca/uhn/fhir/jpa/model/interceptor/api/IInterceptorBroadcaster.java rename hapi-fhir-jpaserver-model/src/main/java/ca/uhn/fhir/jpa/model/interceptor/executor/{InterceptorRegistry.java => InterceptorService.java} (58%) create mode 100644 hapi-fhir-jpaserver-model/src/test/java/ca/uhn/fhir/jpa/model/interceptor/executor/InterceptorServiceTest.java create mode 100644 hapi-fhir-jpaserver-model/src/test/resources/logback-test.xml 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 6bb1e235b7c..b9bd91e2eb6 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 @@ -7,6 +7,9 @@ import ca.uhn.fhir.jpa.dao.index.IdHelperService; import ca.uhn.fhir.jpa.dao.index.SearchParamWithInlineReferencesExtractor; import ca.uhn.fhir.jpa.entity.*; import ca.uhn.fhir.jpa.model.entity.*; +import ca.uhn.fhir.jpa.model.interceptor.api.HookParams; +import ca.uhn.fhir.jpa.model.interceptor.api.IInterceptorBroadcaster; +import ca.uhn.fhir.jpa.model.interceptor.api.Pointcut; import ca.uhn.fhir.jpa.search.ISearchCoordinatorSvc; import ca.uhn.fhir.jpa.search.PersistedJpaBundleProvider; import ca.uhn.fhir.jpa.searchparam.ResourceMetaParams; @@ -69,6 +72,8 @@ import org.springframework.data.domain.SliceImpl; import org.springframework.stereotype.Repository; import org.springframework.transaction.PlatformTransactionManager; import org.springframework.transaction.TransactionDefinition; +import org.springframework.transaction.support.TransactionSynchronizationAdapter; +import org.springframework.transaction.support.TransactionSynchronizationManager; import org.springframework.transaction.support.TransactionTemplate; import javax.persistence.*; @@ -115,7 +120,7 @@ public abstract class BaseHapiFhirDao implements IDao, public static final String OO_SEVERITY_INFO = "information"; public static final String OO_SEVERITY_WARN = "warning"; private static final org.slf4j.Logger ourLog = org.slf4j.LoggerFactory.getLogger(BaseHapiFhirDao.class); - private static final Map ourRetrievalContexts = new HashMap(); + private static final Map ourRetrievalContexts = new HashMap<>(); private static final String PROCESSING_SUB_REQUEST = "BaseHapiFhirDao.processingSubRequest"; private static boolean ourValidationDisabledForUnitTest; private static boolean ourDisableIncrementOnUpdateForUnitTest = false; @@ -125,6 +130,8 @@ public abstract class BaseHapiFhirDao implements IDao, @Autowired protected IdHelperService myIdHelperService; @Autowired + protected IInterceptorBroadcaster myInterceptorBroadcaster; + @Autowired protected IForcedIdDao myForcedIdDao; @Autowired protected ISearchResultDao mySearchResultDao; @@ -1420,9 +1427,8 @@ public abstract class BaseHapiFhirDao implements IDao, return updateEntity(theRequest, theResource, entity, theDeletedTimestampOrNull, true, true, theUpdateTime, false, true); } - public ResourceTable updateInternal(RequestDetails theRequest, T theResource, boolean thePerformIndexing, - boolean theForceUpdateVersion, RequestDetails theRequestDetails, ResourceTable theEntity, IIdType - theResourceId, IBaseResource theOldResource) { + public ResourceTable updateInternal(RequestDetails theRequestDetails, T theResource, boolean thePerformIndexing, boolean theForceUpdateVersion, + ResourceTable theEntity, IIdType theResourceId, IBaseResource theOldResource) { // Notify interceptors ActionRequestDetails requestDetails; if (theRequestDetails != null) { @@ -1439,9 +1445,13 @@ public abstract class BaseHapiFhirDao implements IDao, ((IServerOperationInterceptor) next).resourcePreUpdate(theRequestDetails, theOldResource, theResource); } } + HookParams hookParams = new HookParams() + .add(IBaseResource.class, theOldResource) + .add(IBaseResource.class, theResource); + myInterceptorBroadcaster.callHooks(Pointcut.OP_PRESTORAGE_RESOURCE_UPDATED, hookParams); // Perform update - ResourceTable savedEntity = updateEntity(theRequest, theResource, theEntity, null, thePerformIndexing, thePerformIndexing, new Date(), theForceUpdateVersion, thePerformIndexing); + ResourceTable savedEntity = updateEntity(theRequestDetails, theResource, theEntity, null, thePerformIndexing, thePerformIndexing, new Date(), theForceUpdateVersion, thePerformIndexing); /* * If we aren't indexing (meaning we're probably executing a sub-operation within a transaction), @@ -1470,7 +1480,17 @@ public abstract class BaseHapiFhirDao implements IDao, ((IServerOperationInterceptor) next).resourceUpdated(theRequestDetails, theOldResource, theResource); } } + TransactionSynchronizationManager.registerSynchronization(new TransactionSynchronizationAdapter() { + @Override + public void beforeCommit(boolean readOnly) { + HookParams hookParams = new HookParams() + .add(IBaseResource.class, theOldResource) + .add(IBaseResource.class, theResource); + myInterceptorBroadcaster.callHooks(Pointcut.OP_PRECOMMIT_RESOURCE_UPDATED, hookParams); + } + }); } + return savedEntity; } @@ -1480,6 +1500,10 @@ public abstract class BaseHapiFhirDao implements IDao, id = getContext().getVersion().newIdType().setValue(id.getValue()); } + if (id.hasResourceType() == false) { + id = id.withResourceType(theEntity.getResourceType()); + } + theResource.setId(id); if (theResource instanceof IResource) { ResourceMetadataKeyEnum.VERSION.put((IResource) theResource, id.getVersionIdPart()); 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 9cfd6afec1c..d55055aaa91 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 @@ -26,6 +26,8 @@ import ca.uhn.fhir.context.RuntimeResourceDefinition; import ca.uhn.fhir.context.RuntimeSearchParam; import ca.uhn.fhir.jpa.dao.r4.MatchResourceUrlService; import ca.uhn.fhir.jpa.model.entity.*; +import ca.uhn.fhir.jpa.model.interceptor.api.HookParams; +import ca.uhn.fhir.jpa.model.interceptor.api.Pointcut; import ca.uhn.fhir.jpa.search.DatabaseBackedPagingProvider; import ca.uhn.fhir.jpa.search.PersistedJpaBundleProvider; import ca.uhn.fhir.jpa.search.reindex.IResourceReindexingSvc; @@ -59,6 +61,8 @@ import org.springframework.transaction.PlatformTransactionManager; import org.springframework.transaction.TransactionDefinition; import org.springframework.transaction.annotation.Propagation; import org.springframework.transaction.annotation.Transactional; +import org.springframework.transaction.support.TransactionSynchronizationAdapter; +import org.springframework.transaction.support.TransactionSynchronizationManager; import org.springframework.transaction.support.TransactionTemplate; import javax.annotation.Nonnull; @@ -173,7 +177,7 @@ public abstract class BaseHapiFhirResourceDao extends B } @Override - public DaoMethodOutcome delete(IIdType theId, List theDeleteConflicts, RequestDetails theReques) { + public DaoMethodOutcome delete(IIdType theId, List theDeleteConflicts, RequestDetails theRequest) { if (theId == null || !theId.hasIdPart()) { throw new InvalidRequestException("Can not perform delete, no ID provided"); } @@ -206,12 +210,12 @@ public abstract class BaseHapiFhirResourceDao extends B T resourceToDelete = toResource(myResourceType, entity, null, false); // Notify IServerOperationInterceptors about pre-action call - if (theReques != null) { - theReques.getRequestOperationCallback().resourcePreDelete(resourceToDelete); + if (theRequest != null) { + theRequest.getRequestOperationCallback().resourcePreDelete(resourceToDelete); } for (IServerInterceptor next : getConfig().getInterceptors()) { if (next instanceof IServerOperationInterceptor) { - ((IServerOperationInterceptor) next).resourcePreDelete(theReques, resourceToDelete); + ((IServerOperationInterceptor) next).resourcePreDelete(theRequest, resourceToDelete); } } @@ -220,25 +224,33 @@ public abstract class BaseHapiFhirResourceDao extends B preDelete(resourceToDelete, entity); // Notify interceptors - if (theReques != null) { - ActionRequestDetails requestDetails = new ActionRequestDetails(theReques, getContext(), theId.getResourceType(), theId); + if (theRequest != null) { + ActionRequestDetails requestDetails = new ActionRequestDetails(theRequest, getContext(), theId.getResourceType(), theId); notifyInterceptors(RestOperationTypeEnum.DELETE, requestDetails); } Date updateTime = new Date(); - ResourceTable savedEntity = updateEntity(theReques, null, entity, updateTime, updateTime); + ResourceTable savedEntity = updateEntity(theRequest, null, entity, updateTime, updateTime); resourceToDelete.setId(entity.getIdDt()); // Notify JPA interceptors - if (theReques != null) { - ActionRequestDetails requestDetails = new ActionRequestDetails(theReques, getContext(), theId.getResourceType(), theId); - theReques.getRequestOperationCallback().resourceDeleted(resourceToDelete); + if (theRequest != null) { + ActionRequestDetails requestDetails = new ActionRequestDetails(theRequest, getContext(), theId.getResourceType(), theId); + theRequest.getRequestOperationCallback().resourceDeleted(resourceToDelete); } for (IServerInterceptor next : getConfig().getInterceptors()) { if (next instanceof IServerOperationInterceptor) { - ((IServerOperationInterceptor) next).resourceDeleted(theReques, resourceToDelete); + ((IServerOperationInterceptor) next).resourceDeleted(theRequest, resourceToDelete); } } + TransactionSynchronizationManager.registerSynchronization(new TransactionSynchronizationAdapter() { + @Override + public void beforeCommit(boolean readOnly) { + HookParams hookParams = new HookParams() + .add(IBaseResource.class, resourceToDelete); + myInterceptorBroadcaster.callHooks(Pointcut.OP_PRECOMMIT_RESOURCE_DELETED, hookParams); + } + }); DaoMethodOutcome outcome = toMethodOutcome(savedEntity, resourceToDelete).setCreated(true); @@ -321,6 +333,14 @@ public abstract class BaseHapiFhirResourceDao extends B ((IServerOperationInterceptor) next).resourceDeleted(theRequest, resourceToDelete); } } + TransactionSynchronizationManager.registerSynchronization(new TransactionSynchronizationAdapter() { + @Override + public void beforeCommit(boolean readOnly) { + HookParams hookParams = new HookParams() + .add(IBaseResource.class, resourceToDelete); + myInterceptorBroadcaster.callHooks(Pointcut.OP_PRECOMMIT_RESOURCE_DELETED, hookParams); + } + }); } IBaseOperationOutcome oo; @@ -423,6 +443,9 @@ public abstract class BaseHapiFhirResourceDao extends B ((IServerOperationInterceptor) next).resourcePreCreate(theRequest, theResource); } } + HookParams hookParams = new HookParams() + .add(IBaseResource.class, theResource); + myInterceptorBroadcaster.callHooks(Pointcut.OP_PRESTORAGE_RESOURCE_CREATED, hookParams); // Perform actual DB update ResourceTable updatedEntity = updateEntity(theRequest, theResource, entity, null, thePerformIndexing, thePerformIndexing, theUpdateTime, false, thePerformIndexing); @@ -466,6 +489,14 @@ public abstract class BaseHapiFhirResourceDao extends B } } } + TransactionSynchronizationManager.registerSynchronization(new TransactionSynchronizationAdapter() { + @Override + public void beforeCommit(boolean readOnly) { + HookParams hookParams = new HookParams() + .add(IBaseResource.class, theResource); + myInterceptorBroadcaster.callHooks(Pointcut.OP_PRECOMMIT_RESOURCE_CREATED, hookParams); + } + }); DaoMethodOutcome outcome = toMethodOutcome(entity, theResource).setCreated(true); if (!thePerformIndexing) { @@ -753,7 +784,6 @@ public abstract class BaseHapiFhirResourceDao extends B return retVal; } - @SuppressWarnings("JpaQlInspection") @Override public MT metaGetOperation(Class theType, RequestDetails theRequestDetails) { // Notify interceptors @@ -944,8 +974,7 @@ public abstract class BaseHapiFhirResourceDao extends B if (entity == null) { if (theId.hasVersionIdPart()) { - TypedQuery q = myEntityManager - .createQuery("SELECT t from ResourceHistoryTable t WHERE t.myResourceId = :RID AND t.myResourceType = :RTYP AND t.myResourceVersion = :RVER", ResourceHistoryTable.class); + TypedQuery q = myEntityManager.createQuery("SELECT t from ResourceHistoryTable t WHERE t.myResourceId = :RID AND t.myResourceType = :RTYP AND t.myResourceVersion = :RVER", ResourceHistoryTable.class); q.setParameter("RID", pid); q.setParameter("RTYP", myResourceName); q.setParameter("RVER", theId.getVersionIdPartAsLong()); @@ -1305,7 +1334,7 @@ public abstract class BaseHapiFhirResourceDao extends B /* * Otherwise, we're not in a transaction */ - ResourceTable savedEntity = updateInternal(theRequestDetails, theResource, thePerformIndexing, theForceUpdateVersion, theRequestDetails, entity, resourceId, oldResource); + ResourceTable savedEntity = updateInternal(theRequestDetails, theResource, thePerformIndexing, theForceUpdateVersion, entity, resourceId, oldResource); DaoMethodOutcome outcome = toMethodOutcome(savedEntity, theResource).setCreated(false); if (!thePerformIndexing) { @@ -1320,13 +1349,13 @@ public abstract class BaseHapiFhirResourceDao extends B } @Override - public DaoMethodOutcome update(T theResource, String theMatchUrl, boolean thePerformIndexing, RequestDetails theRequestDetails) { - return update(theResource, theMatchUrl, thePerformIndexing, false, theRequestDetails); + public DaoMethodOutcome update(T theResource, String theMatchUrl, RequestDetails theRequestDetails) { + return update(theResource, theMatchUrl, true, theRequestDetails); } @Override - public DaoMethodOutcome update(T theResource, String theMatchUrl, RequestDetails theRequestDetails) { - return update(theResource, theMatchUrl, true, theRequestDetails); + public DaoMethodOutcome update(T theResource, String theMatchUrl, boolean thePerformIndexing, RequestDetails theRequestDetails) { + return update(theResource, theMatchUrl, thePerformIndexing, false, theRequestDetails); } /** diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/FhirSystemDaoDstu2.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/FhirSystemDaoDstu2.java index 1d4e7456b9e..9db169e2236 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/FhirSystemDaoDstu2.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/FhirSystemDaoDstu2.java @@ -492,7 +492,7 @@ public class FhirSystemDaoDstu2 extends BaseHapiFhirSystemDao { InstantDt deletedInstantOrNull = ResourceMetadataKeyEnum.DELETED_AT.get(nextResource); Date deletedTimestampOrNull = deletedInstantOrNull != null ? deletedInstantOrNull.getValue() : null; if (theUpdatedEntities.contains(nextOutcome.getEntity())) { - updateInternal(theRequestDetails, nextResource, true, false, theRequestDetails, nextOutcome.getEntity(), nextResource.getIdElement(), nextOutcome.getPreviousResource()); + updateInternal(theRequestDetails, nextResource, true, false, nextOutcome.getEntity(), nextResource.getIdElement(), nextOutcome.getPreviousResource()); } else if (!theNonUpdatedEntities.contains(nextOutcome.getEntity())) { updateEntity(theRequestDetails, nextResource, nextOutcome.getEntity(), deletedTimestampOrNull, true, false, theUpdateTime, false, true); } diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/SearchBuilder.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/SearchBuilder.java index cdcb55bb342..e5c0a909037 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/SearchBuilder.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/SearchBuilder.java @@ -100,7 +100,6 @@ import static org.apache.commons.lang3.StringUtils.*; * The SearchBuilder is responsible for actually forming the SQL query that handles * searches for resources */ -@SuppressWarnings("JpaQlInspection") @Component @Scope("prototype") public class SearchBuilder implements ISearchBuilder { diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/TransactionProcessor.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/TransactionProcessor.java index a8fd3445431..ea8b95368d5 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/TransactionProcessor.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/TransactionProcessor.java @@ -89,6 +89,52 @@ public class TransactionProcessor { @Autowired private DaoRegistry myDaoRegistry; + public BUNDLE transaction(RequestDetails theRequestDetails, BUNDLE theRequest) { + if (theRequestDetails != null) { + IServerInterceptor.ActionRequestDetails requestDetails = new IServerInterceptor.ActionRequestDetails(theRequestDetails, theRequest, "Bundle", null); + myDao.notifyInterceptors(RestOperationTypeEnum.TRANSACTION, requestDetails); + } + + String actionName = "Transaction"; + BUNDLE response = processTransactionAsSubRequest((ServletRequestDetails) theRequestDetails, theRequest, actionName); + + return response; + } + + public BUNDLE collection(final RequestDetails theRequestDetails, BUNDLE theRequest) { + String transactionType = myVersionAdapter.getBundleType(theRequest); + + if (!org.hl7.fhir.r4.model.Bundle.BundleType.COLLECTION.toCode().equals(transactionType)) { + throw new InvalidRequestException("Can not process collection Bundle of type: " + transactionType); + } + + ourLog.info("Beginning storing collection with {} resources", myVersionAdapter.getEntries(theRequest).size()); + long start = System.currentTimeMillis(); + + TransactionTemplate txTemplate = new TransactionTemplate(myTxManager); + txTemplate.setPropagationBehavior(TransactionDefinition.PROPAGATION_REQUIRES_NEW); + + BUNDLE resp = myVersionAdapter.createBundle(org.hl7.fhir.r4.model.Bundle.BundleType.BATCHRESPONSE.toCode()); + + List resources = new ArrayList<>(); + for (final BUNDLEENTRY nextRequestEntry : myVersionAdapter.getEntries(theRequest)) { + IBaseResource resource = myVersionAdapter.getResource(nextRequestEntry); + resources.add(resource); + } + + BUNDLE transactionBundle = myVersionAdapter.createBundle("transaction"); + for (IBaseResource next : resources) { + BUNDLEENTRY entry = myVersionAdapter.addEntry(transactionBundle); + myVersionAdapter.setResource(entry, next); + myVersionAdapter.setRequestVerb(entry, "PUT"); + myVersionAdapter.setRequestUrl(entry, next.getIdElement().toUnqualifiedVersionless().getValue()); + } + + transaction(theRequestDetails, transactionBundle); + + return resp; + } + private void populateEntryWithOperationOutcome(BaseServerResponseException caughtEx, BUNDLEENTRY nextEntry) { myVersionAdapter.populateEntryWithOperationOutcome(caughtEx, nextEntry); } @@ -160,16 +206,6 @@ public class TransactionProcessor { myDao = theDao; } - public BUNDLE transaction(RequestDetails theRequestDetails, BUNDLE theRequest) { - if (theRequestDetails != null) { - IServerInterceptor.ActionRequestDetails requestDetails = new IServerInterceptor.ActionRequestDetails(theRequestDetails, theRequest, "Bundle", null); - myDao.notifyInterceptors(RestOperationTypeEnum.TRANSACTION, requestDetails); - } - - String actionName = "Transaction"; - return processTransactionAsSubRequest((ServletRequestDetails) theRequestDetails, theRequest, actionName); - } - private BUNDLE processTransactionAsSubRequest(ServletRequestDetails theRequestDetails, BUNDLE theRequest, String theActionName) { BaseHapiFhirDao.markRequestAsProcessingSubRequest(theRequestDetails); try { @@ -179,40 +215,6 @@ public class TransactionProcessor { } } - public BUNDLE collection(final RequestDetails theRequestDetails, BUNDLE theRequest) { - String transactionType = myVersionAdapter.getBundleType(theRequest); - - if (!org.hl7.fhir.r4.model.Bundle.BundleType.COLLECTION.toCode().equals(transactionType)) { - throw new InvalidRequestException("Can not process collection Bundle of type: " + transactionType); - } - - ourLog.info("Beginning storing collection with {} resources", myVersionAdapter.getEntries(theRequest).size()); - long start = System.currentTimeMillis(); - - TransactionTemplate txTemplate = new TransactionTemplate(myTxManager); - txTemplate.setPropagationBehavior(TransactionDefinition.PROPAGATION_REQUIRES_NEW); - - BUNDLE resp = myVersionAdapter.createBundle(org.hl7.fhir.r4.model.Bundle.BundleType.BATCHRESPONSE.toCode()); - - List resources = new ArrayList<>(); - for (final BUNDLEENTRY nextRequestEntry : myVersionAdapter.getEntries(theRequest)) { - IBaseResource resource = myVersionAdapter.getResource(nextRequestEntry); - resources.add(resource); - } - - BUNDLE transactionBundle = myVersionAdapter.createBundle("transaction"); - for (IBaseResource next : resources) { - BUNDLEENTRY entry = myVersionAdapter.addEntry(transactionBundle); - myVersionAdapter.setResource(entry, next); - myVersionAdapter.setRequestVerb(entry, "PUT"); - myVersionAdapter.setRequestUrl(entry, next.getIdElement().toUnqualifiedVersionless().getValue()); - } - - transaction(theRequestDetails, transactionBundle); - - return resp; - } - private BUNDLE batch(final RequestDetails theRequestDetails, BUNDLE theRequest) { ourLog.info("Beginning batch with {} resources", myVersionAdapter.getEntries(theRequest).size()); long start = System.currentTimeMillis(); @@ -234,8 +236,7 @@ public class TransactionProcessor { BUNDLE subRequestBundle = myVersionAdapter.createBundle(org.hl7.fhir.r4.model.Bundle.BundleType.TRANSACTION.toCode()); myVersionAdapter.addEntry(subRequestBundle, nextRequestEntry); - BUNDLE subResponseBundle = processTransactionAsSubRequest((ServletRequestDetails) theRequestDetails, subRequestBundle, "Batch sub-request"); - return subResponseBundle; + return processTransactionAsSubRequest((ServletRequestDetails) theRequestDetails, subRequestBundle, "Batch sub-request"); }; try { @@ -472,10 +473,6 @@ public class TransactionProcessor { return p.parseResource(theResource.getClass(), p.encodeResourceToString(theResource)); } - public void setEntityManager(EntityManager theEntityManager) { - myEntityManager = theEntityManager; - } - private void validateDependencies() { Validate.notNull(myEntityManager); Validate.notNull(myContext); @@ -526,7 +523,7 @@ public class TransactionProcessor { } } - if (nextResourceId.hasIdPart() && nextResourceId.getIdPart().matches("[a-zA-Z]+\\:.*") && !isPlaceholder(nextResourceId)) { + if (nextResourceId.hasIdPart() && nextResourceId.getIdPart().matches("[a-zA-Z]+:.*") && !isPlaceholder(nextResourceId)) { throw new InvalidRequestException("Invalid placeholder ID found: " + nextResourceId.getIdPart() + " - Must be of the form 'urn:uuid:[uuid]' or 'urn:oid:[oid]'"); } @@ -631,7 +628,7 @@ public class TransactionProcessor { version = ParameterUtil.parseETagValue(myVersionAdapter.getEntryRequestIfMatch(nextReqEntry)); } res.setId(newIdType(parts.getResourceType(), parts.getResourceId(), version)); - outcome = resourceDao.update(res, null, false, theRequestDetails); + outcome = resourceDao.update(res, null, false, false, theRequestDetails); } else { res.setId((String) null); String matchUrl; @@ -641,7 +638,7 @@ public class TransactionProcessor { matchUrl = parts.getResourceType(); } matchUrl = performIdSubstitutionsInMatchUrl(theIdSubstitutions, matchUrl); - outcome = resourceDao.update(res, matchUrl, false, theRequestDetails); + outcome = resourceDao.update(res, matchUrl, false, false, theRequestDetails); if (Boolean.TRUE.equals(outcome.getCreated())) { conditionalRequestUrls.put(matchUrl, res.getClass()); } @@ -727,7 +724,7 @@ public class TransactionProcessor { Date deletedTimestampOrNull = deletedInstantOrNull != null ? deletedInstantOrNull.getValue() : null; if (updatedEntities.contains(nextOutcome.getEntity())) { - myDao.updateInternal(theRequestDetails, nextResource, true, false, theRequestDetails, nextOutcome.getEntity(), nextResource.getIdElement(), nextOutcome.getPreviousResource()); + myDao.updateInternal(theRequestDetails, nextResource, true, false, nextOutcome.getEntity(), nextResource.getIdElement(), nextOutcome.getPreviousResource()); } else if (!nonUpdatedEntities.contains(nextOutcome.getEntity())) { myDao.updateEntity(theRequestDetails, nextResource, nextOutcome.getEntity(), deletedTimestampOrNull, true, false, theUpdateTime, false, true); } diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/index/SearchParamWithInlineReferencesExtractor.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/index/SearchParamWithInlineReferencesExtractor.java index 0990ac67708..00eaf381447 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/index/SearchParamWithInlineReferencesExtractor.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/index/SearchParamWithInlineReferencesExtractor.java @@ -99,7 +99,7 @@ public class SearchParamWithInlineReferencesExtractor { extractInlineReferences(theResource); - myResourceLinkExtractor.extractResourceLinks(theParams, theEntity, theResource, theUpdateTime, myDaoResourceLinkResolver); + myResourceLinkExtractor.extractResourceLinks(theParams, theEntity, theResource, theUpdateTime, myDaoResourceLinkResolver, true); /* * If the existing resource already has links and those match links we still want, use them instead of removing them and re adding them diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/reindex/ResourceReindexingSvcImpl.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/reindex/ResourceReindexingSvcImpl.java index 2ab5fccfcfc..1ba5db9825d 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/reindex/ResourceReindexingSvcImpl.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/reindex/ResourceReindexingSvcImpl.java @@ -391,7 +391,6 @@ public class ResourceReindexingSvcImpl implements IResourceReindexingSvc { }); } - @SuppressWarnings("JpaQlInspection") private void markResourceAsIndexingFailed(final long theId) { TransactionTemplate txTemplate = new TransactionTemplate(myTxManager); txTemplate.setPropagationBehavior(TransactionDefinition.PROPAGATION_REQUIRES_NEW); diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/subscription/SubscriptionActivatingInterceptor.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/subscription/SubscriptionActivatingInterceptor.java index 19b6e3ba1c1..49fb15d6895 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/subscription/SubscriptionActivatingInterceptor.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/subscription/SubscriptionActivatingInterceptor.java @@ -26,6 +26,9 @@ import ca.uhn.fhir.jpa.config.BaseConfig; import ca.uhn.fhir.jpa.dao.DaoConfig; import ca.uhn.fhir.jpa.dao.DaoRegistry; import ca.uhn.fhir.jpa.dao.IFhirResourceDao; +import ca.uhn.fhir.jpa.model.interceptor.api.Hook; +import ca.uhn.fhir.jpa.model.interceptor.api.Interceptor; +import ca.uhn.fhir.jpa.model.interceptor.api.Pointcut; import ca.uhn.fhir.jpa.search.warm.CacheWarmingSvcImpl; import ca.uhn.fhir.jpa.searchparam.MatchUrlService; import ca.uhn.fhir.jpa.subscription.module.CanonicalSubscription; @@ -68,7 +71,8 @@ import java.util.concurrent.TimeUnit; */ @Service @Lazy -public class SubscriptionActivatingInterceptor extends ServerOperationInterceptorAdapter { +@Interceptor(manualRegistration = true) +public class SubscriptionActivatingInterceptor { private Logger ourLog = LoggerFactory.getLogger(SubscriptionActivatingInterceptor.class); private static boolean ourWaitForSubscriptionActivationSynchronouslyForUnitTest; @@ -160,6 +164,7 @@ public class SubscriptionActivatingInterceptor extends ServerOperationIntercepto private boolean activateSubscription(String theActiveStatus, final IBaseResource theSubscription, String theRequestedStatus) { IFhirResourceDao subscriptionDao = myDaoRegistry.getSubscriptionDao(); IBaseResource subscription = subscriptionDao.read(theSubscription.getIdElement()); + subscription.setId(subscription.getIdElement().toVersionless()); ourLog.info("Activating subscription {} from status {} to {}", subscription.getIdElement().toUnqualified().getValue(), theRequestedStatus, theActiveStatus); try { @@ -180,18 +185,18 @@ public class SubscriptionActivatingInterceptor extends ServerOperationIntercepto submitResourceModified(theNewResource, ResourceModifiedMessage.OperationTypeEnum.UPDATE); } - @Override - public void resourceCreated(RequestDetails theRequest, IBaseResource theResource) { + @Hook(Pointcut.OP_PRECOMMIT_RESOURCE_CREATED) + public void resourceCreated(IBaseResource theResource) { submitResourceModified(theResource, ResourceModifiedMessage.OperationTypeEnum.CREATE); } - @Override - public void resourceDeleted(RequestDetails theRequest, IBaseResource theResource) { + @Hook(Pointcut.OP_PRECOMMIT_RESOURCE_DELETED) + public void resourceDeleted(IBaseResource theResource) { submitResourceModified(theResource, ResourceModifiedMessage.OperationTypeEnum.DELETE); } - @Override - public void resourceUpdated(RequestDetails theRequest, IBaseResource theOldResource, IBaseResource theNewResource) { + @Hook(Pointcut.OP_PRECOMMIT_RESOURCE_UPDATED) + public void resourceUpdated(IBaseResource theOldResource, IBaseResource theNewResource) { submitResourceModified(theNewResource, ResourceModifiedMessage.OperationTypeEnum.UPDATE); } diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/subscription/SubscriptionInterceptorLoader.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/subscription/SubscriptionInterceptorLoader.java index feaad036450..5af1faf0591 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/subscription/SubscriptionInterceptorLoader.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/subscription/SubscriptionInterceptorLoader.java @@ -21,6 +21,7 @@ package ca.uhn.fhir.jpa.subscription; */ import ca.uhn.fhir.jpa.dao.DaoConfig; +import ca.uhn.fhir.jpa.model.interceptor.api.IInterceptorRegistry; import ca.uhn.fhir.jpa.subscription.module.cache.SubscriptionLoader; import ca.uhn.fhir.jpa.subscription.module.cache.SubscriptionRegistry; import com.google.common.annotations.VisibleForTesting; @@ -47,6 +48,8 @@ public class SubscriptionInterceptorLoader { private SubscriptionRegistry mySubscriptionRegistry; @Autowired private ApplicationContext myAppicationContext; + @Autowired + private IInterceptorRegistry myInterceptorRegistry; public void registerInterceptors() { Set supportedSubscriptionTypes = myDaoConfig.getSupportedSubscriptionTypes(); @@ -54,12 +57,12 @@ public class SubscriptionInterceptorLoader { if (!supportedSubscriptionTypes.isEmpty()) { loadSubscriptions(); ourLog.info("Registering subscription activating interceptor"); - myDaoConfig.registerInterceptor(mySubscriptionActivatingInterceptor); + myInterceptorRegistry.registerInterceptor(mySubscriptionActivatingInterceptor); } if (myDaoConfig.isSubscriptionMatchingEnabled()) { mySubscriptionMatcherInterceptor.start(); ourLog.info("Registering subscription matcher interceptor"); - myDaoConfig.registerInterceptor(mySubscriptionMatcherInterceptor); + myInterceptorRegistry.registerInterceptor(mySubscriptionMatcherInterceptor); } } @@ -72,7 +75,7 @@ public class SubscriptionInterceptorLoader { @VisibleForTesting void unregisterInterceptorsForUnitTest() { - myDaoConfig.unregisterInterceptor(mySubscriptionActivatingInterceptor); - myDaoConfig.unregisterInterceptor(mySubscriptionMatcherInterceptor); + myInterceptorRegistry.unregisterInterceptor(mySubscriptionActivatingInterceptor); + myInterceptorRegistry.unregisterInterceptor(mySubscriptionMatcherInterceptor); } } diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/subscription/SubscriptionMatcherInterceptor.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/subscription/SubscriptionMatcherInterceptor.java index c6f34c017c0..94d4fc65600 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/subscription/SubscriptionMatcherInterceptor.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/subscription/SubscriptionMatcherInterceptor.java @@ -1,13 +1,14 @@ package ca.uhn.fhir.jpa.subscription; import ca.uhn.fhir.context.FhirContext; +import ca.uhn.fhir.jpa.model.interceptor.api.Hook; +import ca.uhn.fhir.jpa.model.interceptor.api.Interceptor; +import ca.uhn.fhir.jpa.model.interceptor.api.Pointcut; import ca.uhn.fhir.jpa.subscription.module.LinkedBlockingQueueSubscribableChannel; import ca.uhn.fhir.jpa.subscription.module.ResourceModifiedMessage; import ca.uhn.fhir.jpa.subscription.module.cache.SubscriptionChannelFactory; import ca.uhn.fhir.jpa.subscription.module.subscriber.ResourceModifiedJsonMessage; import ca.uhn.fhir.jpa.subscription.module.subscriber.SubscriptionMatchingSubscriber; -import ca.uhn.fhir.rest.api.server.RequestDetails; -import ca.uhn.fhir.rest.server.interceptor.ServerOperationInterceptorAdapter; import com.google.common.annotations.VisibleForTesting; import org.apache.commons.lang3.Validate; import org.hl7.fhir.instance.model.api.IBaseResource; @@ -42,7 +43,8 @@ import javax.annotation.PreDestroy; @Component @Lazy -public class SubscriptionMatcherInterceptor extends ServerOperationInterceptorAdapter implements IResourceModifiedConsumer { +@Interceptor(manualRegistration = true) +public class SubscriptionMatcherInterceptor implements IResourceModifiedConsumer { private Logger ourLog = LoggerFactory.getLogger(SubscriptionMatcherInterceptor.class); private static final String SUBSCRIPTION_MATCHING_CHANNEL_NAME = "subscription-matching"; @@ -82,18 +84,18 @@ public class SubscriptionMatcherInterceptor extends ServerOperationInterceptorAd } } - @Override - public void resourceCreated(RequestDetails theRequest, IBaseResource theResource) { + @Hook(Pointcut.OP_PRECOMMIT_RESOURCE_CREATED) + public void resourceCreated(IBaseResource theResource) { submitResourceModified(theResource, ResourceModifiedMessage.OperationTypeEnum.CREATE); } - @Override - public void resourceDeleted(RequestDetails theRequest, IBaseResource theResource) { + @Hook(Pointcut.OP_PRECOMMIT_RESOURCE_DELETED) + public void resourceDeleted(IBaseResource theResource) { submitResourceModified(theResource, ResourceModifiedMessage.OperationTypeEnum.DELETE); } - @Override - public void resourceUpdated(RequestDetails theRequest, IBaseResource theOldResource, IBaseResource theNewResource) { + @Hook(Pointcut.OP_PRECOMMIT_RESOURCE_UPDATED) + public void resourceUpdated(IBaseResource theOldResource, IBaseResource theNewResource) { submitResourceModified(theNewResource, ResourceModifiedMessage.OperationTypeEnum.UPDATE); } @@ -115,6 +117,7 @@ public class SubscriptionMatcherInterceptor extends ServerOperationInterceptorAd /** * This is an internal API - Use with caution! */ + @Override public void submitResourceModified(final ResourceModifiedMessage theMsg) { sendToProcessingChannel(theMsg); } diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/BaseJpaR4Test.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/BaseJpaR4Test.java index b8ac0fee849..0da4e00e6d0 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/BaseJpaR4Test.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/BaseJpaR4Test.java @@ -8,6 +8,7 @@ import ca.uhn.fhir.jpa.dao.dstu2.FhirResourceDaoDstu2SearchNoFtTest; import ca.uhn.fhir.jpa.model.entity.ModelConfig; import ca.uhn.fhir.jpa.model.entity.ResourceIndexedSearchParamString; import ca.uhn.fhir.jpa.model.entity.ResourceTable; +import ca.uhn.fhir.jpa.model.interceptor.api.IInterceptorRegistry; import ca.uhn.fhir.jpa.provider.r4.JpaSystemProviderR4; import ca.uhn.fhir.jpa.search.DatabaseBackedPagingProvider; import ca.uhn.fhir.jpa.search.ISearchCoordinatorSvc; @@ -150,6 +151,8 @@ public abstract class BaseJpaR4Test extends BaseJpaTest { protected IFhirResourceDao myRiskAssessmentDao; protected IServerInterceptor myInterceptor; @Autowired + protected IInterceptorRegistry myInterceptorRegistry; + @Autowired @Qualifier("myLocationDaoR4") protected IFhirResourceDao myLocationDao; @Autowired @@ -285,6 +288,8 @@ public abstract class BaseJpaR4Test extends BaseJpaTest { myDaoConfig.setReuseCachedSearchResultsForMillis(new DaoConfig().getReuseCachedSearchResultsForMillis()); myDaoConfig.setSuppressUpdatesWithNoChange(new DaoConfig().isSuppressUpdatesWithNoChange()); myDaoConfig.setAllowContainsSearches(new DaoConfig().isAllowContainsSearches()); + + myInterceptorRegistry.clearAnonymousHookForUnitTest(); } @After diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/interceptor/test/InterceptorRegistryTest.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/interceptor/test/InterceptorRegistryTest.java deleted file mode 100644 index de384608297..00000000000 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/interceptor/test/InterceptorRegistryTest.java +++ /dev/null @@ -1,159 +0,0 @@ -package ca.uhn.fhir.jpa.interceptor.test; - -import ca.uhn.fhir.jpa.model.interceptor.executor.InterceptorRegistry; -import ca.uhn.fhir.jpa.model.interceptor.api.HookParams; -import ca.uhn.fhir.jpa.model.interceptor.api.Interceptor; -import ca.uhn.fhir.jpa.model.interceptor.api.Pointcut; -import ca.uhn.fhir.jpa.model.interceptor.api.Hook; -import org.junit.Before; -import org.junit.Test; -import org.junit.runner.RunWith; -import org.springframework.beans.factory.annotation.Autowired; -import org.springframework.context.annotation.Bean; -import org.springframework.context.annotation.ComponentScan; -import org.springframework.context.annotation.Configuration; -import org.springframework.core.annotation.Order; -import org.springframework.test.context.ContextConfiguration; -import org.springframework.test.context.junit4.SpringJUnit4ClassRunner; - -import java.util.ArrayList; -import java.util.List; - -import static org.hamcrest.Matchers.contains; -import static org.junit.Assert.*; - -@RunWith(SpringJUnit4ClassRunner.class) -@ContextConfiguration(classes = {InterceptorRegistryTest.InterceptorRegistryTestCtxConfig.class}) -public class InterceptorRegistryTest { - - private static boolean ourNext_beforeRestHookDelivery_Return1; - private static List ourInvocations = new ArrayList<>(); - private static CanonicalSubscription ourLastCanonicalSubscription; - private static ResourceDeliveryMessage ourLastResourceDeliveryMessage0; - private static ResourceDeliveryMessage ourLastResourceDeliveryMessage1; - - @Autowired - private InterceptorRegistry myInterceptorRegistry; - - @Test - public void testGlobalInterceptorsAreFound() { - List globalInterceptors = myInterceptorRegistry.getGlobalInterceptorsForUnitTest(); - assertEquals(2, globalInterceptors.size()); - assertTrue(globalInterceptors.get(0).getClass().toString(), globalInterceptors.get(0) instanceof MyTestInterceptorOne); - assertTrue(globalInterceptors.get(1).getClass().toString(), globalInterceptors.get(1) instanceof MyTestInterceptorTwo); - } - - @Test - public void testInvokeGlobalInterceptorMethods() { - ResourceDeliveryMessage msg = new ResourceDeliveryMessage(); - CanonicalSubscription subs = new CanonicalSubscription(); - HookParams params = new HookParams(msg, subs); - boolean outcome = myInterceptorRegistry.callHooks(Pointcut.SUBSCRIPTION_BEFORE_REST_HOOK_DELIVERY, params); - assertTrue(outcome); - - assertThat(ourInvocations, contains("MyTestInterceptorOne.beforeRestHookDelivery", "MyTestInterceptorTwo.beforeRestHookDelivery")); - assertSame(msg, ourLastResourceDeliveryMessage0); - assertNull(ourLastResourceDeliveryMessage1); - assertSame(subs, ourLastCanonicalSubscription); - } - - @Test - public void testInvokeGlobalInterceptorMethods_MethodAbortsProcessing() { - ourNext_beforeRestHookDelivery_Return1 = false; - - ResourceDeliveryMessage msg = new ResourceDeliveryMessage(); - CanonicalSubscription subs = new CanonicalSubscription(); - HookParams params = new HookParams(msg, subs); - boolean outcome = myInterceptorRegistry.callHooks(Pointcut.SUBSCRIPTION_BEFORE_REST_HOOK_DELIVERY, params); - assertFalse(outcome); - - assertThat(ourInvocations, contains("MyTestInterceptorOne.beforeRestHookDelivery")); - } - - @Test - public void testCallHooksInvokedWithWrongParameters() { - Integer msg = 123; - CanonicalSubscription subs = new CanonicalSubscription(); - HookParams params = new HookParams(msg, subs); - try { - myInterceptorRegistry.callHooks(Pointcut.SUBSCRIPTION_BEFORE_REST_HOOK_DELIVERY, params); - fail(); - } catch (AssertionError e) { - assertEquals("Wrong hook parameters, wanted [CanonicalSubscription, ResourceDeliveryMessage] and found [CanonicalSubscription, Integer]", e.getMessage()); - } - } - - - @Before - public void before() { - ourNext_beforeRestHookDelivery_Return1 = true; - ourLastCanonicalSubscription = null; - ourLastResourceDeliveryMessage0 = null; - ourLastResourceDeliveryMessage1 = null; - ourInvocations.clear(); - } - - @Configuration - @ComponentScan(basePackages = "ca.uhn.fhir.jpa.model") - static class InterceptorRegistryTestCtxConfig { - - /** - * Note: Orders are deliberately reversed to make sure we get the orders right - * using the @Order annotation - */ - @Bean - public MyTestInterceptorTwo interceptor1() { - return new MyTestInterceptorTwo(); - } - - /** - * Note: Orders are deliberately reversed to make sure we get the orders right - * using the @Order annotation - */ - @Bean - public MyTestInterceptorOne interceptor2() { - return new MyTestInterceptorOne(); - } - - } - - @Interceptor - @Order(100) - public static class MyTestInterceptorOne { - - public MyTestInterceptorOne() { - super(); - } - - @Hook(Pointcut.SUBSCRIPTION_BEFORE_REST_HOOK_DELIVERY) - public boolean beforeRestHookDelivery(CanonicalSubscription theCanonicalSubscription) { - ourLastCanonicalSubscription = theCanonicalSubscription; - ourInvocations.add("MyTestInterceptorOne.beforeRestHookDelivery"); - return ourNext_beforeRestHookDelivery_Return1; - } - - } - - @Interceptor - @Order(200) - public static class MyTestInterceptorTwo { - @Hook(Pointcut.SUBSCRIPTION_BEFORE_REST_HOOK_DELIVERY) - public void beforeRestHookDelivery(ResourceDeliveryMessage theResourceDeliveryMessage0, ResourceDeliveryMessage theResourceDeliveryMessage1) { - ourLastResourceDeliveryMessage0 = theResourceDeliveryMessage0; - ourLastResourceDeliveryMessage1 = theResourceDeliveryMessage1; - ourInvocations.add("MyTestInterceptorTwo.beforeRestHookDelivery"); - } - } - - /** - * Just a make-believe version of this class for the unit test - */ - private static class CanonicalSubscription { - } - - /** - * Just a make-believe version of this class for the unit test - */ - private static class ResourceDeliveryMessage { - } -} diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/r4/HookInterceptorR4Test.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/r4/HookInterceptorR4Test.java new file mode 100644 index 00000000000..74c9f355e40 --- /dev/null +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/r4/HookInterceptorR4Test.java @@ -0,0 +1,110 @@ +package ca.uhn.fhir.jpa.provider.r4; + +import ca.uhn.fhir.jpa.model.interceptor.api.Pointcut; +import ca.uhn.fhir.rest.api.MethodOutcome; +import ca.uhn.fhir.util.TestUtil; +import org.hl7.fhir.instance.model.api.IBaseResource; +import org.hl7.fhir.instance.model.api.IIdType; +import org.hl7.fhir.r4.model.Patient; +import org.junit.AfterClass; +import org.junit.Test; + +import static org.junit.Assert.assertEquals; + +public class HookInterceptorR4Test extends BaseResourceProviderR4Test { + + private static final org.slf4j.Logger ourLog = org.slf4j.LoggerFactory.getLogger(HookInterceptorR4Test.class); + + @Test + public void testOP_PRESTORAGE_RESOURCE_CREATED_ModifyResource() { + myInterceptorRegistry.registerAnonymousHookForUnitTest(Pointcut.OP_PRESTORAGE_RESOURCE_CREATED, t->{ + Patient contents = (Patient) t.get(IBaseResource.class, 0); + contents.getNameFirstRep().setFamily("NEWFAMILY"); + }); + + Patient p = new Patient(); + p.getNameFirstRep().setFamily("OLDFAMILY"); + MethodOutcome outcome = ourClient.create().resource(p).execute(); + + // Response reflects change, stored resource also does + Patient responsePatient = (Patient) outcome.getResource(); + assertEquals("NEWFAMILY", responsePatient.getNameFirstRep().getFamily()); + responsePatient = ourClient.read().resource(Patient.class).withId(outcome.getId()).execute(); + assertEquals("NEWFAMILY", responsePatient.getNameFirstRep().getFamily()); + + } + + @Test + public void testOP_PRECOMMIT_RESOURCE_CREATED_ModifyResource() { + myInterceptorRegistry.registerAnonymousHookForUnitTest(Pointcut.OP_PRECOMMIT_RESOURCE_CREATED, t->{ + Patient contents = (Patient) t.get(IBaseResource.class, 0); + contents.getNameFirstRep().setFamily("NEWFAMILY"); + }); + + Patient p = new Patient(); + p.getNameFirstRep().setFamily("OLDFAMILY"); + MethodOutcome outcome = ourClient.create().resource(p).execute(); + + // Response reflects change, stored resource does not + Patient responsePatient = (Patient) outcome.getResource(); + assertEquals("NEWFAMILY", responsePatient.getNameFirstRep().getFamily()); + responsePatient = ourClient.read().resource(Patient.class).withId(outcome.getId()).execute(); + assertEquals("OLDFAMILY", responsePatient.getNameFirstRep().getFamily()); + + } + + @Test + public void testOP_PRESTORAGE_RESOURCE_UPDATED_ModifyResource() { + Patient p = new Patient(); + p.setActive(true); + IIdType id = ourClient.create().resource(p).execute().getId(); + + myInterceptorRegistry.registerAnonymousHookForUnitTest(Pointcut.OP_PRESTORAGE_RESOURCE_UPDATED, t->{ + Patient contents = (Patient) t.get(IBaseResource.class, 1); + contents.getNameFirstRep().setFamily("NEWFAMILY"); + }); + + p = new Patient(); + p.setId(id); + p.getNameFirstRep().setFamily("OLDFAMILY"); + MethodOutcome outcome = ourClient.update().resource(p).execute(); + + // Response reflects change, stored resource also does + Patient responsePatient = (Patient) outcome.getResource(); + assertEquals("NEWFAMILY", responsePatient.getNameFirstRep().getFamily()); + responsePatient = ourClient.read().resource(Patient.class).withId(outcome.getId()).execute(); + assertEquals("NEWFAMILY", responsePatient.getNameFirstRep().getFamily()); + + } + + @Test + public void testOP_PRECOMMIT_RESOURCE_UPDATED_ModifyResource() { + Patient p = new Patient(); + p.setActive(true); + IIdType id = ourClient.create().resource(p).execute().getId(); + + myInterceptorRegistry.registerAnonymousHookForUnitTest(Pointcut.OP_PRECOMMIT_RESOURCE_UPDATED, t->{ + Patient contents = (Patient) t.get(IBaseResource.class, 1); + contents.getNameFirstRep().setFamily("NEWFAMILY"); + }); + + p = new Patient(); + p.setId(id); + p.getNameFirstRep().setFamily("OLDFAMILY"); + MethodOutcome outcome = ourClient.update().resource(p).execute(); + + // Response reflects change, stored resource does not + Patient responsePatient = (Patient) outcome.getResource(); + assertEquals("NEWFAMILY", responsePatient.getNameFirstRep().getFamily()); + responsePatient = ourClient.read().resource(Patient.class).withId(outcome.getId()).execute(); + assertEquals("OLDFAMILY", responsePatient.getNameFirstRep().getFamily()); + + } + + @AfterClass + public static void afterClassClearContext() { + TestUtil.clearAllStaticFieldsForUnitTest(); + } + + +} diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/r4/ResourceProviderInterceptorR4Test.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/r4/ResourceProviderInterceptorR4Test.java index 4b644ca3458..138a175e572 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/r4/ResourceProviderInterceptorR4Test.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/r4/ResourceProviderInterceptorR4Test.java @@ -366,6 +366,7 @@ public class ResourceProviderInterceptorR4Test extends BaseResourceProviderR4Tes } + private void transaction(Bundle theBundle) throws IOException { String resource = myFhirCtx.newXmlParser().encodeResourceToString(theBundle); HttpPost post = new HttpPost(ourServerBase + "/"); diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/subscription/BaseSubscriptionsR4Test.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/subscription/BaseSubscriptionsR4Test.java index 87d52768498..46f47e283a0 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/subscription/BaseSubscriptionsR4Test.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/subscription/BaseSubscriptionsR4Test.java @@ -153,9 +153,8 @@ public abstract class BaseSubscriptionsR4Test extends BaseResourceProviderR4Test observation.setStatus(Observation.ObservationStatus.FINAL); - MethodOutcome methodOutcome = ourClient.create().resource(observation).execute(); - - observation.setId(methodOutcome.getId()); + IIdType id = myObservationDao.create(observation).getId(); + observation.setId(id); return observation; } diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/subscription/resthook/RestHookTestDstu2Test.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/subscription/resthook/RestHookTestDstu2Test.java index 2a79125a84e..e03232a251b 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/subscription/resthook/RestHookTestDstu2Test.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/subscription/resthook/RestHookTestDstu2Test.java @@ -113,10 +113,8 @@ public class RestHookTestDstu2Test extends BaseResourceProviderDstu2Test { observation.setStatus(ObservationStatusEnum.FINAL); - MethodOutcome methodOutcome = ourClient.create().resource(observation).execute(); - - String observationId = methodOutcome.getId().getIdPart(); - observation.setId(observationId); + IIdType id = myObservationDao.create(observation).getId(); + observation.setId(id); return observation; } diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/subscription/resthook/RestHookTestR4Test.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/subscription/resthook/RestHookTestR4Test.java index 735b051a2ad..6299816a0df 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/subscription/resthook/RestHookTestR4Test.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/subscription/resthook/RestHookTestR4Test.java @@ -23,6 +23,7 @@ import java.util.concurrent.TimeUnit; import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.hasItem; +import static org.hamcrest.Matchers.matchesPattern; import static org.junit.Assert.*; /** @@ -112,6 +113,39 @@ public class RestHookTestR4Test extends BaseSubscriptionsR4Test { } + @Test + public void testPlaceholderReferencesInTransactionAreResolvedCorrectly() throws Exception { + + String payload = "application/fhir+json"; + String code = "1000000050"; + String criteria1 = "Observation?"; + createSubscription(criteria1, payload); + waitForActivatedSubscriptionCount(1); + + // Create a transaction that should match + Bundle bundle = new Bundle(); + bundle.setType(Bundle.BundleType.TRANSACTION); + + Patient patient = new Patient(); + patient.setId(IdType.newRandomUuid()); + patient.getIdentifierFirstRep().setSystem("foo").setValue("AAA"); + bundle.addEntry().setResource(patient).getRequest().setMethod(Bundle.HTTPVerb.POST).setUrl("Patient"); + + Observation observation = new Observation(); + observation.getIdentifierFirstRep().setSystem("foo").setValue("1"); + observation.getCode().addCoding().setCode(code).setSystem("SNOMED-CT"); + observation.setStatus(Observation.ObservationStatus.FINAL); + observation.getSubject().setReference(patient.getId()); + bundle.addEntry().setResource(observation).getRequest().setMethod(Bundle.HTTPVerb.POST).setUrl("Observation"); + + // Send the transaction + mySystemDao.transaction(null, bundle); + + waitForSize(1, ourUpdatedObservations); + + assertThat(ourUpdatedObservations.get(0).getSubject().getReference(), matchesPattern("Patient/[0-9]+")); + } + @Test public void testUpdatesHaveCorrectMetadataUsingTransactions() throws Exception { String payload = "application/fhir+json"; diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/subscription/resthook/RestHookWithInterceptorR4Test.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/subscription/resthook/RestHookWithInterceptorR4Test.java index 4382cf5c80f..441802a9f4a 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/subscription/resthook/RestHookWithInterceptorR4Test.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/subscription/resthook/RestHookWithInterceptorR4Test.java @@ -136,9 +136,14 @@ public class RestHookWithInterceptorR4Test extends BaseSubscriptionsR4Test { @Configuration static class MyTestCtxConfig { + @Autowired + private IInterceptorRegistry myInterceptorRegistry; + @Bean public MyTestInterceptor interceptor() { - return new MyTestInterceptor(); + MyTestInterceptor retVal = new MyTestInterceptor(); + myInterceptorRegistry.registerInterceptor(retVal); + return retVal; } } diff --git a/hapi-fhir-jpaserver-model/src/main/java/ca/uhn/fhir/jpa/model/interceptor/api/HookParams.java b/hapi-fhir-jpaserver-model/src/main/java/ca/uhn/fhir/jpa/model/interceptor/api/HookParams.java index 5156e67c66a..11ec5e61beb 100644 --- a/hapi-fhir-jpaserver-model/src/main/java/ca/uhn/fhir/jpa/model/interceptor/api/HookParams.java +++ b/hapi-fhir-jpaserver-model/src/main/java/ca/uhn/fhir/jpa/model/interceptor/api/HookParams.java @@ -22,11 +22,11 @@ package ca.uhn.fhir.jpa.model.interceptor.api; import com.google.common.collect.ArrayListMultimap; import com.google.common.collect.ListMultimap; +import com.google.common.collect.Multimaps; import java.util.Collection; import java.util.Collections; import java.util.List; -import java.util.stream.Collectors; public class HookParams { @@ -69,10 +69,11 @@ public class HookParams { } /** - * Multivalued parameters will be returned twice in this list + * Returns an unmodifiable multimap of the params, where the + * key is the param type and the value is the actual instance */ - public List getTypesAsSimpleName() { - return myParams.values().stream().map(t -> t.getClass().getSimpleName()).collect(Collectors.toList()); + public ListMultimap, Object> getParamsForType() { + return Multimaps.unmodifiableListMultimap(myParams); } public Collection values() { diff --git a/hapi-fhir-jpaserver-model/src/main/java/ca/uhn/fhir/jpa/model/interceptor/api/IInterceptorBroadcaster.java b/hapi-fhir-jpaserver-model/src/main/java/ca/uhn/fhir/jpa/model/interceptor/api/IInterceptorBroadcaster.java new file mode 100644 index 00000000000..6204991ad93 --- /dev/null +++ b/hapi-fhir-jpaserver-model/src/main/java/ca/uhn/fhir/jpa/model/interceptor/api/IInterceptorBroadcaster.java @@ -0,0 +1,35 @@ +package ca.uhn.fhir.jpa.model.interceptor.api; + +/*- + * #%L + * HAPI FHIR Model + * %% + * Copyright (C) 2014 - 2019 University Health Network + * %% + * 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% + */ + +public interface IInterceptorBroadcaster { + + /** + * Invoke the interceptor methods + */ + boolean callHooks(Pointcut thePointcut, HookParams theParams); + + /** + * Invoke the interceptor methods + */ + boolean callHooks(Pointcut thePointcut, Object... theParams); + +} diff --git a/hapi-fhir-jpaserver-model/src/main/java/ca/uhn/fhir/jpa/model/interceptor/api/IInterceptorRegistry.java b/hapi-fhir-jpaserver-model/src/main/java/ca/uhn/fhir/jpa/model/interceptor/api/IInterceptorRegistry.java index 6661bd90948..f302c316d39 100644 --- a/hapi-fhir-jpaserver-model/src/main/java/ca/uhn/fhir/jpa/model/interceptor/api/IInterceptorRegistry.java +++ b/hapi-fhir-jpaserver-model/src/main/java/ca/uhn/fhir/jpa/model/interceptor/api/IInterceptorRegistry.java @@ -26,6 +26,34 @@ public interface IInterceptorRegistry { int DEFAULT_ORDER = 0; + /** + * Register an interceptor. This method has no effect if the given interceptor is already registered. + * + * @param theInterceptor The interceptor to register + * @return Returns true if at least one valid hook method was found on this interceptor + */ + boolean registerInterceptor(Object theInterceptor); + + /** + * Unregister an interceptor. This method has no effect if the given interceptor is not already registered. + * + * @param theInterceptor The interceptor to unregister + */ + void unregisterInterceptor(Object theInterceptor); + + /** + * @deprecated to be removed + */ + @Deprecated + boolean registerGlobalInterceptor(Object theInterceptor); + + /** + * @deprecated to be removed + */ + @Deprecated + void unregisterGlobalInterceptor(Object theInterceptor); + + @VisibleForTesting void registerAnonymousHookForUnitTest(Pointcut thePointcut, IAnonymousLambdaHook theHook); @@ -34,23 +62,4 @@ public interface IInterceptorRegistry { @VisibleForTesting void clearAnonymousHookForUnitTest(); - - /** - * Register an interceptor - * - * @param theInterceptor The interceptor to register - * @return Returns true if at least one valid hook method was found on this interceptor - */ - boolean registerGlobalInterceptor(Object theInterceptor); - - /** - * Invoke the interceptor methods - */ - boolean callHooks(Pointcut thePointcut, HookParams theParams); - - /** - * Invoke the interceptor methods - */ - boolean callHooks(Pointcut thePointcut, Object... theParams); - } diff --git a/hapi-fhir-jpaserver-model/src/main/java/ca/uhn/fhir/jpa/model/interceptor/api/Interceptor.java b/hapi-fhir-jpaserver-model/src/main/java/ca/uhn/fhir/jpa/model/interceptor/api/Interceptor.java index 669e6833e82..27015c25eae 100644 --- a/hapi-fhir-jpaserver-model/src/main/java/ca/uhn/fhir/jpa/model/interceptor/api/Interceptor.java +++ b/hapi-fhir-jpaserver-model/src/main/java/ca/uhn/fhir/jpa/model/interceptor/api/Interceptor.java @@ -31,4 +31,11 @@ import java.lang.annotation.Target; @Retention(RetentionPolicy.RUNTIME) @Target(ElementType.TYPE) public @interface Interceptor { + + /** + * @return Declares that an interceptor should be manually registered with the registry, + * and should not auto-register using Spring autowiring. + */ + boolean manualRegistration() default false; + } diff --git a/hapi-fhir-jpaserver-model/src/main/java/ca/uhn/fhir/jpa/model/interceptor/api/Pointcut.java b/hapi-fhir-jpaserver-model/src/main/java/ca/uhn/fhir/jpa/model/interceptor/api/Pointcut.java index 979d4331c02..146adafb1e8 100644 --- a/hapi-fhir-jpaserver-model/src/main/java/ca/uhn/fhir/jpa/model/interceptor/api/Pointcut.java +++ b/hapi-fhir-jpaserver-model/src/main/java/ca/uhn/fhir/jpa/model/interceptor/api/Pointcut.java @@ -9,9 +9,9 @@ package ca.uhn.fhir.jpa.model.interceptor.api; * 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. @@ -41,7 +41,7 @@ public enum Pointcut { *
  • ca.uhn.fhir.jpa.subscription.module.subscriber.ResourceDeliveryMessage
    • * */ - SUBSCRIPTION_AFTER_REST_HOOK_DELIVERY("CanonicalSubscription", "ResourceDeliveryMessage"), + SUBSCRIPTION_AFTER_REST_HOOK_DELIVERY("ca.uhn.fhir.jpa.subscription.module.CanonicalSubscription", "ca.uhn.fhir.jpa.subscription.module.subscriber.ResourceDeliveryMessage"), /** * Invoked immediately before the delivery of a REST HOOK subscription. @@ -56,7 +56,7 @@ public enum Pointcut { *
  • ca.uhn.fhir.jpa.subscription.module.subscriber.ResourceDeliveryMessage
    • * */ - SUBSCRIPTION_BEFORE_REST_HOOK_DELIVERY("CanonicalSubscription", "ResourceDeliveryMessage"), + SUBSCRIPTION_BEFORE_REST_HOOK_DELIVERY("ca.uhn.fhir.jpa.subscription.module.CanonicalSubscription", "ca.uhn.fhir.jpa.subscription.module.subscriber.ResourceDeliveryMessage"), /** * Invoked whenever a persisted resource (a resource that has just been stored in the @@ -67,7 +67,7 @@ public enum Pointcut { *
  • ca.uhn.fhir.jpa.subscription.module.ResourceModifiedMessage
    • * */ - SUBSCRIPTION_AFTER_PERSISTED_RESOURCE_CHECKED("ResourceModifiedMessage"), + SUBSCRIPTION_AFTER_PERSISTED_RESOURCE_CHECKED("ca.uhn.fhir.jpa.subscription.module.ResourceModifiedMessage"), /** @@ -83,7 +83,89 @@ public enum Pointcut { *
  • ca.uhn.fhir.jpa.subscription.module.CanonicalSubscription
    • * */ - SUBSCRIPTION_AFTER_ACTIVE_SUBSCRIPTION_REGISTERED("CanonicalSubscription"); + SUBSCRIPTION_AFTER_ACTIVE_SUBSCRIPTION_REGISTERED("ca.uhn.fhir.jpa.subscription.module.CanonicalSubscription"), + + /** + * Invoked before a resource will be updated, immediately before the resource + * is persisted to the database. + *

    + * Hooks will have access to the contents of the resource being created + * and may choose to make modifications to it. These changes will be + * reflected in permanent storage. + *

    + * Hooks may accept the following parameters: + *
      + *
    • org.hl7.fhir.instance.model.api.IBaseResource
      • + *
    + */ + OP_PRESTORAGE_RESOURCE_CREATED("org.hl7.fhir.instance.model.api.IBaseResource"), + + /** + * Invoked before a resource will be created, immediately before the transaction + * is committed (after all validation and other business rules have successfully + * completed, and any other database activity is complete. + *

    + * Hooks will have access to the contents of the resource being created + * but should generally not make any + * changes as storage has already occurred. Changes will not be reflected + * in storage, but may be reflected in the HTTP response. + *

    + * Hooks may accept the following parameters: + *
      + *
    • org.hl7.fhir.instance.model.api.IBaseResource
      • + *
    + */ + OP_PRECOMMIT_RESOURCE_CREATED("org.hl7.fhir.instance.model.api.IBaseResource"), + + /** + * Invoked before a resource will be created + *

    + * 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: + *
      + *
    • org.hl7.fhir.instance.model.api.IBaseResource
      • + *
    + */ + OP_PRECOMMIT_RESOURCE_DELETED("org.hl7.fhir.instance.model.api.IBaseResource"), + + /** + * Invoked before a resource will be updated, immediately before the transaction + * is committed (after all validation and other business rules have successfully + * completed, and any other database activity is complete. + *

    + * Hooks will have access to the contents of the resource being updated + * (both the previous and new contents) but should generally not make any + * changes as storage has already occurred. Changes will not be reflected + * in storage, but may be reflected in the HTTP response. + *

    + * Hooks may accept the following parameters: + *
      + *
    • org.hl7.fhir.instance.model.api.IBaseResource (previous contents)
      • + *
    • org.hl7.fhir.instance.model.api.IBaseResource (new contents)
      • + *
    + */ + OP_PRECOMMIT_RESOURCE_UPDATED("org.hl7.fhir.instance.model.api.IBaseResource", "org.hl7.fhir.instance.model.api.IBaseResource"), + + /** + * Invoked before a resource will be updated, immediately before the resource + * is persisted to the database. + *

    + * Hooks will have access to the contents of the resource being updated + * (both the previous and new contents) and may choose to make modifications + * to the new contents of the resource. These changes will be reflected in + * permanent storage. + *

    + * Hooks may accept the following parameters: + *
      + *
    • org.hl7.fhir.instance.model.api.IBaseResource (previous contents)
      • + *
    • org.hl7.fhir.instance.model.api.IBaseResource (new contents)
      • + *
    + */ + OP_PRESTORAGE_RESOURCE_UPDATED("org.hl7.fhir.instance.model.api.IBaseResource", "org.hl7.fhir.instance.model.api.IBaseResource"), + + ; private final List myParameterTypes; @@ -94,4 +176,4 @@ public enum Pointcut { public List getParameterTypes() { return myParameterTypes; } -} + } diff --git a/hapi-fhir-jpaserver-model/src/main/java/ca/uhn/fhir/jpa/model/interceptor/executor/InterceptorRegistry.java b/hapi-fhir-jpaserver-model/src/main/java/ca/uhn/fhir/jpa/model/interceptor/executor/InterceptorService.java similarity index 58% rename from hapi-fhir-jpaserver-model/src/main/java/ca/uhn/fhir/jpa/model/interceptor/executor/InterceptorRegistry.java rename to hapi-fhir-jpaserver-model/src/main/java/ca/uhn/fhir/jpa/model/interceptor/executor/InterceptorService.java index da86d6212a0..7fc8fd9c4fd 100644 --- a/hapi-fhir-jpaserver-model/src/main/java/ca/uhn/fhir/jpa/model/interceptor/executor/InterceptorRegistry.java +++ b/hapi-fhir-jpaserver-model/src/main/java/ca/uhn/fhir/jpa/model/interceptor/executor/InterceptorService.java @@ -9,9 +9,9 @@ package ca.uhn.fhir.jpa.model.interceptor.executor; * 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. @@ -21,45 +21,43 @@ package ca.uhn.fhir.jpa.model.interceptor.executor; */ import ca.uhn.fhir.jpa.model.interceptor.api.*; +import ca.uhn.fhir.rest.server.exceptions.InternalErrorException; import com.google.common.annotations.VisibleForTesting; import com.google.common.collect.ArrayListMultimap; import com.google.common.collect.ListMultimap; -import com.google.common.collect.Multimaps; import org.apache.commons.collections4.ListUtils; import org.apache.commons.lang3.Validate; import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import org.springframework.beans.BeansException; -import org.springframework.context.ApplicationContext; -import org.springframework.context.ApplicationContextAware; import org.springframework.core.annotation.AnnotationUtils; import org.springframework.core.annotation.Order; import org.springframework.stereotype.Component; import javax.annotation.Nonnull; -import javax.annotation.PostConstruct; +import java.lang.reflect.InvocationTargetException; import java.lang.reflect.Method; import java.util.*; import java.util.concurrent.atomic.AtomicInteger; +import java.util.stream.Collectors; @Component -public class InterceptorRegistry implements IInterceptorRegistry, ApplicationContextAware { - private static final Logger ourLog = LoggerFactory.getLogger(InterceptorRegistry.class); - private ApplicationContext myAppCtx; - private final List myGlobalInterceptors = new ArrayList<>(); +public class InterceptorService implements IInterceptorRegistry, IInterceptorBroadcaster { + private static final Logger ourLog = LoggerFactory.getLogger(InterceptorService.class); + private final List myInterceptors = new ArrayList<>(); private final ListMultimap myInvokers = ArrayListMultimap.create(); - private final ListMultimap myAnonymousInvokers = Multimaps.synchronizedListMultimap(ArrayListMultimap.create()); + private final ListMultimap myAnonymousInvokers = ArrayListMultimap.create(); + private final Object myRegistryMutex = new Object(); /** * Constructor */ - public InterceptorRegistry() { + public InterceptorService() { super(); } @VisibleForTesting - public List getGlobalInterceptorsForUnitTest() { - return myGlobalInterceptors; + List getGlobalInterceptorsForUnitTest() { + return myInterceptors; } @@ -83,34 +81,43 @@ public class InterceptorRegistry implements IInterceptorRegistry, ApplicationCon myAnonymousInvokers.clear(); } - @PostConstruct - public void start() { + @Override + public boolean registerInterceptor(Object theInterceptor) { + synchronized (myRegistryMutex) { - // Grab the global interceptors - String[] globalInterceptorNames = myAppCtx.getBeanNamesForAnnotation(Interceptor.class); - for (String nextName : globalInterceptorNames) { - Object nextInterceptor = myAppCtx.getBean(nextName); - registerGlobalInterceptor(nextInterceptor); + if (isInterceptorAlreadyRegistered(theInterceptor)) { + return false; + } + + Class interceptorClass = theInterceptor.getClass(); + int typeOrder = determineOrder(interceptorClass); + + if (!scanInterceptorForHookMethodsAndAddThem(theInterceptor, typeOrder)) { + return false; + } + + myInterceptors.add(theInterceptor); + + // Make sure we're always sorted according to the order declared in + // @Order + sortByOrderAnnotation(myInterceptors); + for (Pointcut nextPointcut : myInvokers.keys()) { + List nextInvokerList = myInvokers.get(nextPointcut); + nextInvokerList.sort(Comparator.naturalOrder()); + } + + return true; } - } - @Override - public boolean registerGlobalInterceptor(Object theInterceptor) { + private boolean scanInterceptorForHookMethodsAndAddThem(Object theInterceptor, int theTypeOrder) { boolean retVal = false; - - int typeOrder = DEFAULT_ORDER; - Order typeOrderAnnotation = AnnotationUtils.findAnnotation(theInterceptor.getClass(), Order.class); - if (typeOrderAnnotation != null) { - typeOrder = typeOrderAnnotation.value(); - } - for (Method nextMethod : theInterceptor.getClass().getDeclaredMethods()) { Hook hook = AnnotationUtils.findAnnotation(nextMethod, Hook.class); if (hook != null) { - int methodOrder = typeOrder; + int methodOrder = theTypeOrder; Order methodOrderAnnotation = AnnotationUtils.findAnnotation(nextMethod, Order.class); if (methodOrderAnnotation != null) { methodOrder = methodOrderAnnotation.value(); @@ -124,20 +131,45 @@ public class InterceptorRegistry implements IInterceptorRegistry, ApplicationCon retVal = true; } } - - myGlobalInterceptors.add(theInterceptor); - - // Make sure we're always sorted according to the order declared in - // @Order - sortByOrderAnnotation(myGlobalInterceptors); - for (Pointcut nextPointcut : myInvokers.keys()) { - List nextInvokerList = myInvokers.get(nextPointcut); - nextInvokerList.sort(Comparator.naturalOrder()); - } - return retVal; } + private int determineOrder(Class theInterceptorClass) { + int typeOrder = DEFAULT_ORDER; + Order typeOrderAnnotation = AnnotationUtils.findAnnotation(theInterceptorClass, Order.class); + if (typeOrderAnnotation != null) { + typeOrder = typeOrderAnnotation.value(); + } + return typeOrder; + } + + private boolean isInterceptorAlreadyRegistered(Object theInterceptor) { + for (Object next : myInterceptors) { + if (next == theInterceptor) { + return true; + } + } + return false; + } + + @Override + public void unregisterInterceptor(Object theInterceptor) { + synchronized (myRegistryMutex) { + myInterceptors.removeIf(t -> t == theInterceptor); + myInvokers.entries().removeIf(t -> t.getValue().getInterceptor() == theInterceptor); + } + } + + @Override + public boolean registerGlobalInterceptor(Object theInterceptor) { + return registerInterceptor(theInterceptor); + } + + @Override + public void unregisterGlobalInterceptor(Object theInterceptor) { + unregisterInterceptor(theInterceptor); + } + private void sortByOrderAnnotation(List theObjects) { IdentityHashMap interceptorToOrder = new IdentityHashMap<>(); for (Object next : theObjects) { @@ -154,24 +186,15 @@ public class InterceptorRegistry implements IInterceptorRegistry, ApplicationCon } @Override - public void setApplicationContext(@Nonnull ApplicationContext theApplicationContext) throws BeansException { - myAppCtx = theApplicationContext; + public boolean callHooks(Pointcut thePointcut, Object... theParams) { + return callHooks(thePointcut, new HookParams(theParams)); } @Override public boolean callHooks(Pointcut thePointcut, HookParams theParams) { assert haveAppropriateParams(thePointcut, theParams); - List globalInvokers = myInvokers.get(thePointcut); - List anonymousInvokers = myAnonymousInvokers.get(thePointcut); - - List invokers = globalInvokers; - if (anonymousInvokers.isEmpty() == false) { - invokers = ListUtils.union( - anonymousInvokers, - globalInvokers); - invokers.sort(Comparator.naturalOrder()); - } + List invokers = getInvokersForPointcut(thePointcut); /* * Call each hook in order @@ -186,33 +209,68 @@ public class InterceptorRegistry implements IInterceptorRegistry, ApplicationCon return true; } + @VisibleForTesting + List getInterceptorsWithInvokersForPointcut(Pointcut thePointcut) { + return getInvokersForPointcut(thePointcut) + .stream() + .map(BaseInvoker::getInterceptor) + .collect(Collectors.toList()); + } + + /** + * Returns an ordered list of invokers for the given pointcut. Note that + * a new and stable list is returned to.. do whatever you want with it. + */ + private List getInvokersForPointcut(Pointcut thePointcut) { + List invokers; + boolean haveAnonymousInvokers; + synchronized (myRegistryMutex) { + List globalInvokers = myInvokers.get(thePointcut); + List anonymousInvokers = myAnonymousInvokers.get(thePointcut); + invokers = ListUtils.union(anonymousInvokers, globalInvokers); + haveAnonymousInvokers = anonymousInvokers.isEmpty() == false; + } + + if (haveAnonymousInvokers) { + invokers.sort(Comparator.naturalOrder()); + } + return invokers; + } + /** * Only call this when assertions are enabled, it's expensive */ - private boolean haveAppropriateParams(Pointcut thePointcut, HookParams theParams) { - List givenTypes = theParams.getTypesAsSimpleName(); - List wantedTypes = new ArrayList<>(thePointcut.getParameterTypes()); - givenTypes.sort(Comparator.naturalOrder()); - wantedTypes.sort(Comparator.naturalOrder()); - if (!givenTypes.equals(wantedTypes)) { - throw new AssertionError("Wrong hook parameters, wanted " + wantedTypes + " and found " + givenTypes); - } - return true; - } + boolean haveAppropriateParams(Pointcut thePointcut, HookParams theParams) { + Validate.isTrue(theParams.getParamsForType().values().size() == thePointcut.getParameterTypes().size(), "Wrong number of params for pointcut %s - Wanted %s but found %s", thePointcut.name(), toErrorString(thePointcut.getParameterTypes()), theParams.getParamsForType().values().stream().map(t -> t.getClass().getSimpleName()).sorted().collect(Collectors.toList())); - @Override - public boolean callHooks(Pointcut thePointcut, Object... theParams) { - return callHooks(thePointcut, new HookParams(theParams)); + List wantedTypes = new ArrayList<>(thePointcut.getParameterTypes()); + + ListMultimap, Object> givenTypes = theParams.getParamsForType(); + for (Class nextTypeClass : givenTypes.keySet()) { + String nextTypeName = nextTypeClass.getName(); + for (Object nextParamValue : givenTypes.get(nextTypeClass)) { + Validate.isTrue(nextTypeClass.isAssignableFrom(nextParamValue.getClass()), "Invalid params for pointcut %s - %s is not of type %s", thePointcut.name(), nextParamValue.getClass(), nextTypeClass); + Validate.isTrue(wantedTypes.remove(nextTypeName), "Invalid params for pointcut %s - Wanted %s but missing %s", thePointcut.name(), toErrorString(thePointcut.getParameterTypes()), nextTypeName); + } + } + + return true; } private abstract class BaseInvoker implements Comparable { private final int myOrder; + private final Object myInterceptor; - protected BaseInvoker(int theOrder) { + BaseInvoker(Object theInterceptor, int theOrder) { + myInterceptor = theInterceptor; myOrder = theOrder; } + public Object getInterceptor() { + return myInterceptor; + } + abstract boolean invoke(HookParams theParams); @Override @@ -225,7 +283,7 @@ public class InterceptorRegistry implements IInterceptorRegistry, ApplicationCon private final IAnonymousLambdaHook myHook; public AnonymousLambdaInvoker(IAnonymousLambdaHook theHook, int theOrder) { - super(theOrder); + super(theHook, theOrder); myHook = theHook; } @@ -238,7 +296,6 @@ public class InterceptorRegistry implements IInterceptorRegistry, ApplicationCon private class HookInvoker extends BaseInvoker { - private final Object myInterceptor; private final boolean myReturnsBoolean; private final Method myMethod; private final Class[] myParameterTypes; @@ -248,8 +305,7 @@ public class InterceptorRegistry implements IInterceptorRegistry, ApplicationCon * Constructor */ private HookInvoker(Hook theHook, @Nonnull Object theInterceptor, @Nonnull Method theHookMethod, int theOrder) { - super(theOrder); - myInterceptor = theInterceptor; + super(theInterceptor, theOrder); myParameterTypes = theHookMethod.getParameterTypes(); myMethod = theHookMethod; @@ -285,19 +341,32 @@ public class InterceptorRegistry implements IInterceptorRegistry, ApplicationCon // Invoke the method try { - Object returnValue = myMethod.invoke(myInterceptor, args); + Object returnValue = myMethod.invoke(getInterceptor(), args); if (myReturnsBoolean) { return (boolean) returnValue; } else { return true; } + } catch (InvocationTargetException e) { + Throwable targetException = e.getTargetException(); + if (targetException instanceof RuntimeException) { + throw ((RuntimeException) targetException); + } else { + throw new InternalErrorException(targetException); + } } catch (Exception e) { - ourLog.error("Failure executing interceptor method[{}]: {}", myMethod, e.toString(), e); - return true; + throw new InternalErrorException(e); } } } + private static String toErrorString(List theParameterTypes) { + return theParameterTypes + .stream() + .sorted() + .collect(Collectors.joining(",")); + } + } diff --git a/hapi-fhir-jpaserver-model/src/test/java/ca/uhn/fhir/jpa/model/interceptor/executor/InterceptorServiceTest.java b/hapi-fhir-jpaserver-model/src/test/java/ca/uhn/fhir/jpa/model/interceptor/executor/InterceptorServiceTest.java new file mode 100644 index 00000000000..eede79c7d30 --- /dev/null +++ b/hapi-fhir-jpaserver-model/src/test/java/ca/uhn/fhir/jpa/model/interceptor/executor/InterceptorServiceTest.java @@ -0,0 +1,268 @@ +package ca.uhn.fhir.jpa.model.interceptor.executor; + +import ca.uhn.fhir.jpa.model.interceptor.api.*; +import org.hl7.fhir.instance.model.api.IBaseResource; +import org.hl7.fhir.r4.model.Patient; +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.context.annotation.Bean; +import org.springframework.context.annotation.ComponentScan; +import org.springframework.context.annotation.Configuration; +import org.springframework.core.annotation.Order; +import org.springframework.test.context.ContextConfiguration; +import org.springframework.test.context.junit4.SpringJUnit4ClassRunner; + +import java.util.ArrayList; +import java.util.List; + +import static org.hamcrest.Matchers.contains; +import static org.junit.Assert.*; + +@RunWith(SpringJUnit4ClassRunner.class) +@ContextConfiguration(classes = {InterceptorServiceTest.InterceptorRegistryTestCtxConfig.class}) +public class InterceptorServiceTest { + + private static boolean ourNext_beforeRestHookDelivery_Return1; + private static List ourInvocations = new ArrayList<>(); + private static IBaseResource ourLastResourceOne; + private static IBaseResource ourLastResourceTwoA; + private static IBaseResource ourLastResourceTwoB; + + @Autowired + private InterceptorService myInterceptorRegistry; + + @Autowired + private MyTestInterceptorOne myInterceptorOne; + @Autowired + private MyTestInterceptorTwo myInterceptorTwo; + @Autowired + private MyTestInterceptorManual myInterceptorManual; + + @Test + public void testGlobalInterceptorsAreFound() { + List globalInterceptors = myInterceptorRegistry.getGlobalInterceptorsForUnitTest(); + assertEquals(2, globalInterceptors.size()); + assertTrue(globalInterceptors.get(0).getClass().toString(), globalInterceptors.get(0) instanceof MyTestInterceptorOne); + assertTrue(globalInterceptors.get(1).getClass().toString(), globalInterceptors.get(1) instanceof MyTestInterceptorTwo); + } + + @Test + public void testManuallyRegisterGlobalInterceptor() { + + // Register the manual interceptor (has @Order right in the middle) + myInterceptorRegistry.registerInterceptor(myInterceptorManual); + List globalInterceptors = myInterceptorRegistry.getGlobalInterceptorsForUnitTest(); + assertEquals(3, globalInterceptors.size()); + assertTrue(globalInterceptors.get(0).getClass().toString(), globalInterceptors.get(0) instanceof MyTestInterceptorOne); + assertTrue(globalInterceptors.get(1).getClass().toString(), globalInterceptors.get(1) instanceof MyTestInterceptorManual); + assertTrue(globalInterceptors.get(2).getClass().toString(), globalInterceptors.get(2) instanceof MyTestInterceptorTwo); + + // Try to register again (should have no effect + myInterceptorRegistry.registerInterceptor(myInterceptorManual); + globalInterceptors = myInterceptorRegistry.getGlobalInterceptorsForUnitTest(); + assertEquals(3, globalInterceptors.size()); + assertTrue(globalInterceptors.get(0).getClass().toString(), globalInterceptors.get(0) instanceof MyTestInterceptorOne); + assertTrue(globalInterceptors.get(1).getClass().toString(), globalInterceptors.get(1) instanceof MyTestInterceptorManual); + assertTrue(globalInterceptors.get(2).getClass().toString(), globalInterceptors.get(2) instanceof MyTestInterceptorTwo); + + // Make sure we have the right invokers in the right order + List invokers = myInterceptorRegistry.getInterceptorsWithInvokersForPointcut(Pointcut.OP_PRECOMMIT_RESOURCE_CREATED); + assertSame(myInterceptorOne, invokers.get(0)); + assertSame(myInterceptorManual, invokers.get(1)); + assertSame(myInterceptorTwo, invokers.get(2)); + + // Finally, unregister it + myInterceptorRegistry.unregisterInterceptor(myInterceptorManual); + globalInterceptors = myInterceptorRegistry.getGlobalInterceptorsForUnitTest(); + assertEquals(2, globalInterceptors.size()); + assertTrue(globalInterceptors.get(0).getClass().toString(), globalInterceptors.get(0) instanceof MyTestInterceptorOne); + assertTrue(globalInterceptors.get(1).getClass().toString(), globalInterceptors.get(1) instanceof MyTestInterceptorTwo); + + } + + @Test + public void testInvokeGlobalInterceptorMethods() { + Patient patient = new Patient(); + HookParams params = new HookParams() + .add(IBaseResource.class, patient); + boolean outcome = myInterceptorRegistry.callHooks(Pointcut.OP_PRECOMMIT_RESOURCE_CREATED, params); + assertTrue(outcome); + + assertThat(ourInvocations, contains("MyTestInterceptorOne.beforeRestHookDelivery", "MyTestInterceptorTwo.beforeRestHookDelivery")); + assertSame(patient, ourLastResourceTwoA); + assertNull(ourLastResourceTwoB); + assertSame(patient, ourLastResourceOne); + } + + @Test + public void testInvokeGlobalInterceptorMethods_MethodAbortsProcessing() { + ourNext_beforeRestHookDelivery_Return1 = false; + + Patient patient = new Patient(); + HookParams params = new HookParams() + .add(IBaseResource.class, patient); + boolean outcome = myInterceptorRegistry.callHooks(Pointcut.OP_PRECOMMIT_RESOURCE_CREATED, params); + assertFalse(outcome); + + assertThat(ourInvocations, contains("MyTestInterceptorOne.beforeRestHookDelivery")); + } + + @Test + public void testCallHooksInvokedWithWrongParameters() { + Integer msg = 123; + CanonicalSubscription subs = new CanonicalSubscription(); + HookParams params = new HookParams(msg, subs); + try { + myInterceptorRegistry.callHooks(Pointcut.OP_PRECOMMIT_RESOURCE_CREATED, params); + fail(); + } catch (IllegalArgumentException e) { + assertEquals("Wrong number of params for pointcut OP_PRECOMMIT_RESOURCE_CREATED - Wanted org.hl7.fhir.instance.model.api.IBaseResource but found [CanonicalSubscription, Integer]", e.getMessage()); + } + } + + @Test + public void testValidateParamTypes() { + HookParams params = new HookParams(); + params.add(IBaseResource.class, new Patient()); + params.add(IBaseResource.class, new Patient()); + boolean validated = myInterceptorRegistry.haveAppropriateParams(Pointcut.OP_PRECOMMIT_RESOURCE_UPDATED, params); + assertTrue(validated); + } + + @Test + public void testValidateParamTypesMissingParam() { + HookParams params = new HookParams(); + params.add(IBaseResource.class, new Patient()); + try { + myInterceptorRegistry.haveAppropriateParams(Pointcut.OP_PRECOMMIT_RESOURCE_UPDATED, params); + fail(); + } catch (IllegalArgumentException e) { + assertEquals("Wrong number of params for pointcut OP_PRECOMMIT_RESOURCE_UPDATED - Wanted org.hl7.fhir.instance.model.api.IBaseResource,org.hl7.fhir.instance.model.api.IBaseResource but found [Patient]", e.getMessage()); + } + } + + @Test + public void testValidateParamTypesExtraParam() { + HookParams params = new HookParams(); + params.add(IBaseResource.class, new Patient()); + params.add(IBaseResource.class, new Patient()); + params.add(IBaseResource.class, new Patient()); + try { + myInterceptorRegistry.haveAppropriateParams(Pointcut.OP_PRECOMMIT_RESOURCE_UPDATED, params); + fail(); + } catch (IllegalArgumentException e) { + assertEquals("Wrong number of params for pointcut OP_PRECOMMIT_RESOURCE_UPDATED - Wanted org.hl7.fhir.instance.model.api.IBaseResource,org.hl7.fhir.instance.model.api.IBaseResource but found [Patient, Patient, Patient]", e.getMessage()); + } + } + + @SuppressWarnings("unchecked") + @Test + public void testValidateParamTypesWrongParam() { + HookParams params = new HookParams(); + Class clazz = IBaseResource.class; + params.add(clazz, "AAA"); + params.add(clazz, "BBB"); + try { + myInterceptorRegistry.haveAppropriateParams(Pointcut.OP_PRECOMMIT_RESOURCE_UPDATED, params); + fail(); + } catch (IllegalArgumentException e) { + assertEquals("Invalid params for pointcut OP_PRECOMMIT_RESOURCE_UPDATED - class java.lang.String is not of type interface org.hl7.fhir.instance.model.api.IBaseResource", e.getMessage()); + } + } + + @Before + public void before() { + ourNext_beforeRestHookDelivery_Return1 = true; + ourLastResourceOne = null; + ourLastResourceTwoA = null; + ourLastResourceTwoB = null; + ourInvocations.clear(); + } + + @Configuration + @ComponentScan(basePackages = "ca.uhn.fhir.jpa.model") + static class InterceptorRegistryTestCtxConfig { + + @Autowired + private IInterceptorRegistry myInterceptorRegistry; + + /** + * Note: Orders are deliberately reversed to make sure we get the orders right + * using the @Order annotation + */ + @Bean + public MyTestInterceptorTwo interceptor1() { + MyTestInterceptorTwo retVal = new MyTestInterceptorTwo(); + myInterceptorRegistry.registerInterceptor(retVal); + return retVal; + } + + /** + * Note: Orders are deliberately reversed to make sure we get the orders right + * using the @Order annotation + */ + @Bean + public MyTestInterceptorOne interceptor2() { + MyTestInterceptorOne retVal = new MyTestInterceptorOne(); + myInterceptorRegistry.registerInterceptor(retVal); + return retVal; + } + + @Bean + public MyTestInterceptorManual interceptorManual() { + return new MyTestInterceptorManual(); + } + + } + + @Interceptor + @Order(100) + public static class MyTestInterceptorOne { + + public MyTestInterceptorOne() { + super(); + } + + @Hook(Pointcut.OP_PRECOMMIT_RESOURCE_CREATED) + public boolean beforeRestHookDelivery(IBaseResource theResource) { + ourLastResourceOne = theResource; + ourInvocations.add("MyTestInterceptorOne.beforeRestHookDelivery"); + return ourNext_beforeRestHookDelivery_Return1; + } + + } + + @Interceptor + @Order(300) + public static class MyTestInterceptorTwo { + @Hook(Pointcut.OP_PRECOMMIT_RESOURCE_CREATED) + public void beforeRestHookDelivery(IBaseResource theResource0, IBaseResource theResource1) { + ourLastResourceTwoA = theResource0; + ourLastResourceTwoB = theResource1; + ourInvocations.add("MyTestInterceptorTwo.beforeRestHookDelivery"); + } + } + + @Interceptor(manualRegistration = true) + @Order(200) + public static class MyTestInterceptorManual { + @Hook(Pointcut.OP_PRECOMMIT_RESOURCE_CREATED) + public void beforeRestHookDelivery() { + ourInvocations.add("MyTestInterceptorManual.beforeRestHookDelivery"); + } + } + + /** + * Just a make-believe version of this class for the unit test + */ + private static class CanonicalSubscription { + } + + /** + * Just a make-believe version of this class for the unit test + */ + private static class ResourceDeliveryMessage { + } +} diff --git a/hapi-fhir-jpaserver-model/src/test/resources/logback-test.xml b/hapi-fhir-jpaserver-model/src/test/resources/logback-test.xml new file mode 100644 index 00000000000..e40a4e73953 --- /dev/null +++ b/hapi-fhir-jpaserver-model/src/test/resources/logback-test.xml @@ -0,0 +1,15 @@ + + + + INFO + + + %d{yyyy-MM-dd HH:mm:ss.SSS} [%thread] %-5level %logger{36} [%file:%line] %msg%n + + + + + + + + diff --git a/hapi-fhir-jpaserver-searchparam/src/main/java/ca/uhn/fhir/jpa/searchparam/extractor/ResourceLinkExtractor.java b/hapi-fhir-jpaserver-searchparam/src/main/java/ca/uhn/fhir/jpa/searchparam/extractor/ResourceLinkExtractor.java index bb131d7006c..afdc1b7f408 100644 --- a/hapi-fhir-jpaserver-searchparam/src/main/java/ca/uhn/fhir/jpa/searchparam/extractor/ResourceLinkExtractor.java +++ b/hapi-fhir-jpaserver-searchparam/src/main/java/ca/uhn/fhir/jpa/searchparam/extractor/ResourceLinkExtractor.java @@ -9,9 +9,9 @@ package ca.uhn.fhir.jpa.searchparam.extractor; * 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. @@ -58,7 +58,7 @@ public class ResourceLinkExtractor { @Autowired private ISearchParamExtractor mySearchParamExtractor; - public void extractResourceLinks(ResourceIndexedSearchParams theParams, ResourceTable theEntity, IBaseResource theResource, Date theUpdateTime, IResourceLinkResolver theResourceLinkResolver) { + public void extractResourceLinks(ResourceIndexedSearchParams theParams, ResourceTable theEntity, IBaseResource theResource, Date theUpdateTime, IResourceLinkResolver theResourceLinkResolver, boolean theFailOnInvalidReference) { String resourceType = theEntity.getResourceType(); /* @@ -71,13 +71,13 @@ public class ResourceLinkExtractor { Map searchParams = mySearchParamRegistry.getActiveSearchParams(toResourceName(theResource.getClass())); for (RuntimeSearchParam nextSpDef : searchParams.values()) { - extractResourceLinks(theParams, theEntity, theResource, theUpdateTime, theResourceLinkResolver, resourceType, nextSpDef); + extractResourceLinks(theParams, theEntity, theResource, theUpdateTime, theResourceLinkResolver, resourceType, nextSpDef, theFailOnInvalidReference); } theEntity.setHasLinks(theParams.links.size() > 0); } - private void extractResourceLinks(ResourceIndexedSearchParams theParams, ResourceTable theEntity, IBaseResource theResource, Date theUpdateTime, IResourceLinkResolver theResourceLinkResolver, String theResourceType, RuntimeSearchParam nextSpDef) { + private void extractResourceLinks(ResourceIndexedSearchParams theParams, ResourceTable theEntity, IBaseResource theResource, Date theUpdateTime, IResourceLinkResolver theResourceLinkResolver, String theResourceType, RuntimeSearchParam nextSpDef, boolean theFailOnInvalidReference) { if (nextSpDef.getParamType() != RestSearchParameterTypeEnum.REFERENCE) { return; } @@ -94,11 +94,11 @@ public class ResourceLinkExtractor { List refs = mySearchParamExtractor.extractResourceLinks(theResource, nextSpDef); for (PathAndRef nextPathAndRef : refs) { - extractResourceLinks(theParams, theEntity, theUpdateTime, theResourceLinkResolver, theResourceType, nextSpDef, nextPathsUnsplit, multiType, nextPathAndRef); + extractResourceLinks(theParams, theEntity, theUpdateTime, theResourceLinkResolver, theResourceType, nextSpDef, nextPathsUnsplit, multiType, nextPathAndRef, theFailOnInvalidReference); } } - private void extractResourceLinks(ResourceIndexedSearchParams theParams, ResourceTable theEntity, Date theUpdateTime, IResourceLinkResolver theResourceLinkResolver, String theResourceType, RuntimeSearchParam nextSpDef, String theNextPathsUnsplit, boolean theMultiType, PathAndRef nextPathAndRef) { + private void extractResourceLinks(ResourceIndexedSearchParams theParams, ResourceTable theEntity, Date theUpdateTime, IResourceLinkResolver theResourceLinkResolver, String theResourceType, RuntimeSearchParam nextSpDef, String theNextPathsUnsplit, boolean theMultiType, PathAndRef nextPathAndRef, boolean theFailOnInvalidReference) { Object nextObject = nextPathAndRef.getRef(); /* @@ -168,14 +168,25 @@ public class ResourceLinkExtractor { String baseUrl = nextId.getBaseUrl(); String typeString = nextId.getResourceType(); if (isBlank(typeString)) { - throw new InvalidRequestException("Invalid resource reference found at path[" + theNextPathsUnsplit + "] - Does not contain resource type - " + nextId.getValue()); + String msg = "Invalid resource reference found at path[" + theNextPathsUnsplit + "] - Does not contain resource type - " + nextId.getValue(); + if (theFailOnInvalidReference) { + throw new InvalidRequestException(msg); + } else { + ourLog.debug(msg); + return; + } } RuntimeResourceDefinition resourceDefinition; try { resourceDefinition = myContext.getResourceDefinition(typeString); } catch (DataFormatException e) { - throw new InvalidRequestException( - "Invalid resource reference found at path[" + theNextPathsUnsplit + "] - Resource type is unknown or not supported on this server - " + nextId.getValue()); + String msg = "Invalid resource reference found at path[" + theNextPathsUnsplit + "] - Resource type is unknown or not supported on this server - " + nextId.getValue(); + if (theFailOnInvalidReference) { + throw new InvalidRequestException(msg); + } else { + ourLog.debug(msg); + return; + } } if (isNotBlank(baseUrl)) { @@ -194,7 +205,13 @@ public class ResourceLinkExtractor { Class type = resourceDefinition.getImplementingClass(); String id = nextId.getIdPart(); if (StringUtils.isBlank(id)) { - throw new InvalidRequestException("Invalid resource reference found at path[" + theNextPathsUnsplit + "] - Does not contain resource ID - " + nextId.getValue()); + String msg = "Invalid resource reference found at path[" + theNextPathsUnsplit + "] - Does not contain resource ID - " + nextId.getValue(); + if (theFailOnInvalidReference) { + throw new InvalidRequestException(msg); + } else { + ourLog.debug(msg); + return; + } } theResourceLinkResolver.validateTypeOrThrowException(type); diff --git a/hapi-fhir-jpaserver-searchparam/src/main/java/ca/uhn/fhir/jpa/searchparam/retry/Retrier.java b/hapi-fhir-jpaserver-searchparam/src/main/java/ca/uhn/fhir/jpa/searchparam/retry/Retrier.java index cc11e2c8a43..4cb987d3127 100644 --- a/hapi-fhir-jpaserver-searchparam/src/main/java/ca/uhn/fhir/jpa/searchparam/retry/Retrier.java +++ b/hapi-fhir-jpaserver-searchparam/src/main/java/ca/uhn/fhir/jpa/searchparam/retry/Retrier.java @@ -47,6 +47,7 @@ public class Retrier { try { return mySupplier.get(); } catch(RuntimeException e) { + ourLog.trace("Failure during retry: {}", e.getMessage(), e); // with stacktrace if it's ever needed ourLog.info("Failed to {}. Attempt {} / {}: {}", myDescription, retryCount, myMaxRetries, e.getMessage()); lastException = e; try { diff --git a/hapi-fhir-jpaserver-subscription/src/main/java/ca/uhn/fhir/jpa/subscription/module/ResourceModifiedMessage.java b/hapi-fhir-jpaserver-subscription/src/main/java/ca/uhn/fhir/jpa/subscription/module/ResourceModifiedMessage.java index 42a2122b403..4ad69d5647d 100644 --- a/hapi-fhir-jpaserver-subscription/src/main/java/ca/uhn/fhir/jpa/subscription/module/ResourceModifiedMessage.java +++ b/hapi-fhir-jpaserver-subscription/src/main/java/ca/uhn/fhir/jpa/subscription/module/ResourceModifiedMessage.java @@ -9,9 +9,9 @@ package ca.uhn.fhir.jpa.subscription.module; * 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. @@ -22,6 +22,7 @@ package ca.uhn.fhir.jpa.subscription.module; import ca.uhn.fhir.context.FhirContext; import ca.uhn.fhir.jpa.subscription.module.subscriber.IResourceMessage; +import ca.uhn.fhir.util.ResourceReferenceInfo; import com.fasterxml.jackson.annotation.JsonAutoDetect; import com.fasterxml.jackson.annotation.JsonIgnore; import com.fasterxml.jackson.annotation.JsonInclude; @@ -29,6 +30,9 @@ import com.fasterxml.jackson.annotation.JsonProperty; import org.hl7.fhir.instance.model.api.IBaseResource; import org.hl7.fhir.instance.model.api.IIdType; +import java.util.List; + +import static org.apache.commons.lang3.StringUtils.isBlank; import static org.apache.commons.lang3.StringUtils.isNotBlank; @JsonInclude(JsonInclude.Include.NON_NULL) @@ -112,6 +116,15 @@ public class ResourceModifiedMessage implements IResourceMessage { } private void setNewPayload(FhirContext theCtx, IBaseResource theNewPayload) { + /* + * References with placeholders would be invalid by the time we get here, and + * would be caught before we even get here. This check is basically a last-ditch + * effort to make sure nothing has broken in the various safeguards that + * should prevent this from happening (hence it only being an assert as + * opposed to something executed all the time). + */ + assert payloadContainsNoPlaceholderReferences(theCtx, theNewPayload); + /* * Note: Don't set myPayloadDecoded in here- This is a false optimization since * it doesn't actually get used if anyone is doing subscriptions at any @@ -123,7 +136,6 @@ public class ResourceModifiedMessage implements IResourceMessage { myPayloadId = theNewPayload.getIdElement().toUnqualified().getValue(); } - public enum OperationTypeEnum { CREATE, UPDATE, @@ -132,4 +144,23 @@ public class ResourceModifiedMessage implements IResourceMessage { } + private static boolean payloadContainsNoPlaceholderReferences(FhirContext theCtx, IBaseResource theNewPayload) { + List refs = theCtx.newTerser().getAllResourceReferences(theNewPayload); + for (ResourceReferenceInfo next : refs) { + String ref = next.getResourceReference().getReferenceElement().getValue(); + if (isBlank(ref)) { + ref = next.getResourceReference().getResource().getIdElement().getValue(); + } + if (isNotBlank(ref)) { + if (ref.startsWith("#")) { + continue; + } + if (ref.startsWith("urn:uuid:")) { + throw new AssertionError("Reference at " + next.getName() + " is invalid: " + next.getResourceReference()); + } + } + } + return true; + } + } diff --git a/hapi-fhir-jpaserver-subscription/src/main/java/ca/uhn/fhir/jpa/subscription/module/cache/SubscriptionRegistry.java b/hapi-fhir-jpaserver-subscription/src/main/java/ca/uhn/fhir/jpa/subscription/module/cache/SubscriptionRegistry.java index 7c9b9b5da50..65eda9b430a 100644 --- a/hapi-fhir-jpaserver-subscription/src/main/java/ca/uhn/fhir/jpa/subscription/module/cache/SubscriptionRegistry.java +++ b/hapi-fhir-jpaserver-subscription/src/main/java/ca/uhn/fhir/jpa/subscription/module/cache/SubscriptionRegistry.java @@ -21,7 +21,7 @@ package ca.uhn.fhir.jpa.subscription.module.cache; */ import ca.uhn.fhir.jpa.model.entity.ModelConfig; -import ca.uhn.fhir.jpa.model.interceptor.api.IInterceptorRegistry; +import ca.uhn.fhir.jpa.model.interceptor.api.IInterceptorBroadcaster; import ca.uhn.fhir.jpa.model.interceptor.api.Pointcut; import ca.uhn.fhir.jpa.subscription.module.CanonicalSubscription; import com.google.common.annotations.VisibleForTesting; @@ -59,7 +59,7 @@ public class SubscriptionRegistry { @Autowired ModelConfig myModelConfig; @Autowired - private IInterceptorRegistry myInterceptorRegistry; + private IInterceptorBroadcaster myInterceptorBroadcaster; public ActiveSubscription get(String theIdPart) { return myActiveSubscriptionCache.get(theIdPart); @@ -101,7 +101,7 @@ public class SubscriptionRegistry { myActiveSubscriptionCache.put(subscriptionId, activeSubscription); // Interceptor call: SUBSCRIPTION_AFTER_ACTIVE_SUBSCRIPTION_REGISTERED - myInterceptorRegistry.callHooks(Pointcut.SUBSCRIPTION_AFTER_ACTIVE_SUBSCRIPTION_REGISTERED, canonicalized); + myInterceptorBroadcaster.callHooks(Pointcut.SUBSCRIPTION_AFTER_ACTIVE_SUBSCRIPTION_REGISTERED, canonicalized); return canonicalized; } @@ -117,7 +117,7 @@ public class SubscriptionRegistry { unregisterAllSubscriptionsNotInCollection(Collections.emptyList()); } - public void unregisterAllSubscriptionsNotInCollection(Collection theAllIds) { + void unregisterAllSubscriptionsNotInCollection(Collection theAllIds) { myActiveSubscriptionCache.unregisterAllSubscriptionsNotInCollection(theAllIds); } diff --git a/hapi-fhir-jpaserver-subscription/src/main/java/ca/uhn/fhir/jpa/subscription/module/matcher/InMemorySubscriptionMatcher.java b/hapi-fhir-jpaserver-subscription/src/main/java/ca/uhn/fhir/jpa/subscription/module/matcher/InMemorySubscriptionMatcher.java index c7e789b0c37..6418495dcf0 100644 --- a/hapi-fhir-jpaserver-subscription/src/main/java/ca/uhn/fhir/jpa/subscription/module/matcher/InMemorySubscriptionMatcher.java +++ b/hapi-fhir-jpaserver-subscription/src/main/java/ca/uhn/fhir/jpa/subscription/module/matcher/InMemorySubscriptionMatcher.java @@ -59,7 +59,7 @@ public class InMemorySubscriptionMatcher implements ISubscriptionMatcher { entity.setResourceType(resourceType); ResourceIndexedSearchParams searchParams = new ResourceIndexedSearchParams(); mySearchParamExtractorService.extractFromResource(searchParams, entity, resource); - myResourceLinkExtractor.extractResourceLinks(searchParams, entity, resource, resource.getMeta().getLastUpdated(), myInlineResourceLinkResolver); + myResourceLinkExtractor.extractResourceLinks(searchParams, entity, resource, resource.getMeta().getLastUpdated(), myInlineResourceLinkResolver, false); return myCriteriaResourceMatcher.match(criteria, resource, searchParams); } } diff --git a/hapi-fhir-jpaserver-subscription/src/main/java/ca/uhn/fhir/jpa/subscription/module/subscriber/SubscriptionDeliveringRestHookSubscriber.java b/hapi-fhir-jpaserver-subscription/src/main/java/ca/uhn/fhir/jpa/subscription/module/subscriber/SubscriptionDeliveringRestHookSubscriber.java index 0667f7be9f2..a481e678ae1 100644 --- a/hapi-fhir-jpaserver-subscription/src/main/java/ca/uhn/fhir/jpa/subscription/module/subscriber/SubscriptionDeliveringRestHookSubscriber.java +++ b/hapi-fhir-jpaserver-subscription/src/main/java/ca/uhn/fhir/jpa/subscription/module/subscriber/SubscriptionDeliveringRestHookSubscriber.java @@ -20,6 +20,7 @@ package ca.uhn.fhir.jpa.subscription.module.subscriber; * #L% */ +import ca.uhn.fhir.jpa.model.interceptor.api.IInterceptorBroadcaster; import ca.uhn.fhir.jpa.subscription.module.CanonicalSubscription; import ca.uhn.fhir.jpa.model.interceptor.api.Pointcut; import ca.uhn.fhir.jpa.model.interceptor.api.IInterceptorRegistry; @@ -52,7 +53,7 @@ public class SubscriptionDeliveringRestHookSubscriber extends BaseSubscriptionDe IResourceRetriever myResourceRetriever; private Logger ourLog = LoggerFactory.getLogger(SubscriptionDeliveringRestHookSubscriber.class); @Autowired - private IInterceptorRegistry myInterceptorRegistry; + private IInterceptorBroadcaster myInterceptorBroadcaster; protected void deliverPayload(ResourceDeliveryMessage theMsg, CanonicalSubscription theSubscription, EncodingEnum thePayloadType, IGenericClient theClient) { IBaseResource payloadResource = getAndMassagePayload(theMsg, theSubscription); @@ -133,7 +134,7 @@ public class SubscriptionDeliveringRestHookSubscriber extends BaseSubscriptionDe CanonicalSubscription subscription = theMessage.getSubscription(); // Interceptor call: SUBSCRIPTION_BEFORE_REST_HOOK_DELIVERY - if (!myInterceptorRegistry.callHooks(Pointcut.SUBSCRIPTION_BEFORE_REST_HOOK_DELIVERY, theMessage, subscription)) { + if (!myInterceptorBroadcaster.callHooks(Pointcut.SUBSCRIPTION_BEFORE_REST_HOOK_DELIVERY, theMessage, subscription)) { return; } @@ -169,7 +170,7 @@ public class SubscriptionDeliveringRestHookSubscriber extends BaseSubscriptionDe deliverPayload(theMessage, subscription, payloadType, client); // Interceptor call: SUBSCRIPTION_AFTER_REST_HOOK_DELIVERY - if (!myInterceptorRegistry.callHooks(Pointcut.SUBSCRIPTION_AFTER_REST_HOOK_DELIVERY, theMessage, subscription)) { + if (!myInterceptorBroadcaster.callHooks(Pointcut.SUBSCRIPTION_AFTER_REST_HOOK_DELIVERY, theMessage, subscription)) { //noinspection UnnecessaryReturnStatement return; } diff --git a/hapi-fhir-jpaserver-subscription/src/main/java/ca/uhn/fhir/jpa/subscription/module/subscriber/SubscriptionMatchingSubscriber.java b/hapi-fhir-jpaserver-subscription/src/main/java/ca/uhn/fhir/jpa/subscription/module/subscriber/SubscriptionMatchingSubscriber.java index 20a64112d0a..d2aab38685a 100644 --- a/hapi-fhir-jpaserver-subscription/src/main/java/ca/uhn/fhir/jpa/subscription/module/subscriber/SubscriptionMatchingSubscriber.java +++ b/hapi-fhir-jpaserver-subscription/src/main/java/ca/uhn/fhir/jpa/subscription/module/subscriber/SubscriptionMatchingSubscriber.java @@ -1,7 +1,7 @@ package ca.uhn.fhir.jpa.subscription.module.subscriber; import ca.uhn.fhir.context.FhirContext; -import ca.uhn.fhir.jpa.model.interceptor.api.IInterceptorRegistry; +import ca.uhn.fhir.jpa.model.interceptor.api.IInterceptorBroadcaster; import ca.uhn.fhir.jpa.model.interceptor.api.Pointcut; import ca.uhn.fhir.jpa.subscription.module.ResourceModifiedMessage; import ca.uhn.fhir.jpa.subscription.module.cache.ActiveSubscription; @@ -33,9 +33,9 @@ import static org.apache.commons.lang3.StringUtils.isNotBlank; * 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. @@ -55,7 +55,7 @@ public class SubscriptionMatchingSubscriber implements MessageHandler { @Autowired private SubscriptionRegistry mySubscriptionRegistry; @Autowired - private IInterceptorRegistry myInterceptorRegistry; + private IInterceptorBroadcaster myInterceptorBroadcaster; @Override public void handleMessage(Message theMessage) throws MessagingException { @@ -75,7 +75,7 @@ public class SubscriptionMatchingSubscriber implements MessageHandler { try { doMatchActiveSubscriptionsAndDeliver(theMsg); } finally { - myInterceptorRegistry.callHooks(Pointcut.SUBSCRIPTION_AFTER_PERSISTED_RESOURCE_CHECKED, theMsg); + myInterceptorBroadcaster.callHooks(Pointcut.SUBSCRIPTION_AFTER_PERSISTED_RESOURCE_CHECKED, theMsg); } } diff --git a/hapi-fhir-jpaserver-subscription/src/test/java/ca/uhn/fhir/jpa/subscription/module/config/TestSubscriptionDstu3Config.java b/hapi-fhir-jpaserver-subscription/src/test/java/ca/uhn/fhir/jpa/subscription/module/config/TestSubscriptionDstu3Config.java index 3605ae2c30f..bc118b2eaff 100644 --- a/hapi-fhir-jpaserver-subscription/src/test/java/ca/uhn/fhir/jpa/subscription/module/config/TestSubscriptionDstu3Config.java +++ b/hapi-fhir-jpaserver-subscription/src/test/java/ca/uhn/fhir/jpa/subscription/module/config/TestSubscriptionDstu3Config.java @@ -1,16 +1,12 @@ package ca.uhn.fhir.jpa.subscription.module.config; -import ca.uhn.fhir.jpa.model.interceptor.api.IInterceptorRegistry; -import ca.uhn.fhir.jpa.model.interceptor.executor.InterceptorRegistry; import ca.uhn.fhir.jpa.searchparam.registry.ISearchParamProvider; import ca.uhn.fhir.jpa.subscription.module.cache.ISubscriptionProvider; -import org.springframework.context.annotation.Bean; -import org.springframework.context.annotation.Configuration; -import org.springframework.context.annotation.Import; -import org.springframework.context.annotation.Primary; +import org.springframework.context.annotation.*; @Configuration @Import(TestSubscriptionConfig.class) +@ComponentScan(basePackages = {"ca.uhn.fhir.jpa.model.interceptor.executor"}) public class TestSubscriptionDstu3Config extends SubscriptionDstu3Config { @Bean @Primary @@ -24,9 +20,4 @@ public class TestSubscriptionDstu3Config extends SubscriptionDstu3Config { return new MockFhirClientSubscriptionProvider(); } - @Bean - public IInterceptorRegistry interceptorRegistry() { - return new InterceptorRegistry(); - } - } diff --git a/hapi-fhir-jpaserver-subscription/src/test/java/ca/uhn/fhir/jpa/subscription/module/matcher/InMemorySubscriptionMatcherTestR3.java b/hapi-fhir-jpaserver-subscription/src/test/java/ca/uhn/fhir/jpa/subscription/module/matcher/InMemorySubscriptionMatcherTestR3.java index ff6b32d436c..c18641d72f9 100644 --- a/hapi-fhir-jpaserver-subscription/src/test/java/ca/uhn/fhir/jpa/subscription/module/matcher/InMemorySubscriptionMatcherTestR3.java +++ b/hapi-fhir-jpaserver-subscription/src/test/java/ca/uhn/fhir/jpa/subscription/module/matcher/InMemorySubscriptionMatcherTestR3.java @@ -39,6 +39,33 @@ public class InMemorySubscriptionMatcherTestR3 extends BaseSubscriptionDstu3Test assertFalse(result.matched()); } + + /** + * Technically this is an invalid reference in most cases, but this shouldn't choke + * the matcher in the case that it gets used. + */ + @Test + public void testPlaceholderIdInReference() { + + ProcedureRequest pr = new ProcedureRequest(); + pr.setId("ProcedureRequest/123"); + pr.setIntent(ProcedureRequest.ProcedureRequestIntent.ORIGINALORDER); + + pr.setSubject(new Reference("urn:uuid:aaaaaaaaaa")); + assertMatched(pr, "ProcedureRequest?intent=original-order"); + assertNotMatched(pr, "ProcedureRequest?subject=Patient/123"); + + pr.setSubject(new Reference("Foo/123")); + assertMatched(pr, "ProcedureRequest?intent=original-order"); + assertNotMatched(pr, "ProcedureRequest?subject=Patient/123"); + + pr.setSubject(new Reference("Patient/")); + assertMatched(pr, "ProcedureRequest?intent=original-order"); + assertNotMatched(pr, "ProcedureRequest?subject=Patient/123"); + + } + + @Test public void testResourceById() { diff --git a/hapi-fhir-jpaserver-subscription/src/test/java/ca/uhn/fhir/jpa/subscription/module/standalone/BaseBlockingQueueSubscribableChannelDstu3Test.java b/hapi-fhir-jpaserver-subscription/src/test/java/ca/uhn/fhir/jpa/subscription/module/standalone/BaseBlockingQueueSubscribableChannelDstu3Test.java index f2608b12f95..e44c8b2848d 100644 --- a/hapi-fhir-jpaserver-subscription/src/test/java/ca/uhn/fhir/jpa/subscription/module/standalone/BaseBlockingQueueSubscribableChannelDstu3Test.java +++ b/hapi-fhir-jpaserver-subscription/src/test/java/ca/uhn/fhir/jpa/subscription/module/standalone/BaseBlockingQueueSubscribableChannelDstu3Test.java @@ -2,8 +2,8 @@ package ca.uhn.fhir.jpa.subscription.module.standalone; import ca.uhn.fhir.context.FhirContext; import ca.uhn.fhir.jpa.model.interceptor.api.HookParams; +import ca.uhn.fhir.jpa.model.interceptor.api.IInterceptorRegistry; import ca.uhn.fhir.jpa.model.interceptor.api.Pointcut; -import ca.uhn.fhir.jpa.model.interceptor.executor.InterceptorRegistry; import ca.uhn.fhir.jpa.subscription.module.BaseSubscriptionDstu3Test; import ca.uhn.fhir.jpa.subscription.module.PointcutLatch; import ca.uhn.fhir.jpa.subscription.module.ResourceModifiedMessage; @@ -52,7 +52,7 @@ public abstract class BaseBlockingQueueSubscribableChannelDstu3Test extends Base @Autowired SubscriptionChannelFactory mySubscriptionChannelFactory; @Autowired - InterceptorRegistry myInterceptorRegistry; + IInterceptorRegistry myInterceptorRegistry; @Autowired protected SubscriptionRegistry mySubscriptionRegistry;