From a6d1cc56c7ee7962c8cfdfb1a79d90ec15a47344 Mon Sep 17 00:00:00 2001 From: Ken Stevens Date: Mon, 21 Jan 2019 16:58:14 -0500 Subject: [PATCH 1/5] added maxRetriex extension to canonical subscription --- .../module/CanonicalSubscription.java | 29 ++++++++++++++----- .../cache/SubscriptionCanonicalizer.java | 17 +++++++++-- .../module/cache/SubscriptionConstants.java | 6 ++++ ...scriptionDeliveringRestHookSubscriber.java | 3 +- 4 files changed, 43 insertions(+), 12 deletions(-) diff --git a/hapi-fhir-jpaserver-subscription/src/main/java/ca/uhn/fhir/jpa/subscription/module/CanonicalSubscription.java b/hapi-fhir-jpaserver-subscription/src/main/java/ca/uhn/fhir/jpa/subscription/module/CanonicalSubscription.java index f240b810453e..707a75aabc48 100644 --- a/hapi-fhir-jpaserver-subscription/src/main/java/ca/uhn/fhir/jpa/subscription/module/CanonicalSubscription.java +++ b/hapi-fhir-jpaserver-subscription/src/main/java/ca/uhn/fhir/jpa/subscription/module/CanonicalSubscription.java @@ -295,6 +295,8 @@ public static class RestHookDetails { private boolean myStripVersionId; @JsonProperty("deliverLatestVersion") private boolean myDeliverLatestVersion; + @JsonProperty("maxRetries") + private int myMaxRetries; /** * Constructor @@ -311,6 +313,23 @@ public void setDeliverLatestVersion(boolean theDeliverLatestVersion) { myDeliverLatestVersion = theDeliverLatestVersion; } + + public boolean isStripVersionId() { + return myStripVersionId; + } + + public void setStripVersionId(boolean theStripVersionId) { + myStripVersionId = theStripVersionId; + } + + public int getMaxRetries() { + return myMaxRetries; + } + + public void setMaxRetries(int theMaxRetries) { + myMaxRetries = theMaxRetries; + } + @Override public boolean equals(Object theO) { if (this == theO) return true; @@ -322,6 +341,7 @@ public boolean equals(Object theO) { return new EqualsBuilder() .append(myStripVersionId, that.myStripVersionId) .append(myDeliverLatestVersion, that.myDeliverLatestVersion) + .append(myMaxRetries, that.myMaxRetries) .isEquals(); } @@ -330,17 +350,10 @@ public int hashCode() { return new HashCodeBuilder(17, 37) .append(myStripVersionId) .append(myDeliverLatestVersion) + .append(myMaxRetries) .toHashCode(); } - public boolean isStripVersionId() { - return myStripVersionId; - } - - public void setStripVersionId(boolean theStripVersionId) { - myStripVersionId = theStripVersionId; - } - } @JsonInclude(JsonInclude.Include.NON_NULL) diff --git a/hapi-fhir-jpaserver-subscription/src/main/java/ca/uhn/fhir/jpa/subscription/module/cache/SubscriptionCanonicalizer.java b/hapi-fhir-jpaserver-subscription/src/main/java/ca/uhn/fhir/jpa/subscription/module/cache/SubscriptionCanonicalizer.java index b7a3f09ce7c3..ed25780a4160 100644 --- a/hapi-fhir-jpaserver-subscription/src/main/java/ca/uhn/fhir/jpa/subscription/module/cache/SubscriptionCanonicalizer.java +++ b/hapi-fhir-jpaserver-subscription/src/main/java/ca/uhn/fhir/jpa/subscription/module/cache/SubscriptionCanonicalizer.java @@ -28,6 +28,7 @@ import ca.uhn.fhir.model.api.IPrimitiveDatatype; import ca.uhn.fhir.rest.server.exceptions.InternalErrorException; import ca.uhn.fhir.rest.server.exceptions.PreconditionFailedException; +import org.hl7.fhir.dstu3.model.Subscription; import org.hl7.fhir.exceptions.FHIRException; import org.hl7.fhir.instance.model.api.IBaseReference; import org.hl7.fhir.instance.model.api.IBaseResource; @@ -96,10 +97,10 @@ protected CanonicalSubscription canonicalizeDstu3(IBaseResource theSubscription) retVal.setIdElement(subscription.getIdElement()); retVal.setPayloadString(subscription.getChannel().getPayload()); - if (retVal.getChannelType() == CanonicalSubscriptionChannelType.EMAIL) { + if (retVal.getChannelType() == CanonicalSubscriptionChannelType.EMAIL) { String from; String subjectTemplate; - String bodyTemplate; + try { from = subscription.getChannel().getExtensionString(SubscriptionConstants.EXT_SUBSCRIPTION_EMAIL_FROM); subjectTemplate = subscription.getChannel().getExtensionString(SubscriptionConstants.EXT_SUBSCRIPTION_SUBJECT_TEMPLATE); @@ -111,16 +112,22 @@ protected CanonicalSubscription canonicalizeDstu3(IBaseResource theSubscription) } if (retVal.getChannelType() == CanonicalSubscriptionChannelType.RESTHOOK) { + String stripVersionIds; String deliverLatestVersion; + String maxRetries; try { stripVersionIds = subscription.getChannel().getExtensionString(SubscriptionConstants.EXT_SUBSCRIPTION_RESTHOOK_STRIP_VERSION_IDS); deliverLatestVersion = subscription.getChannel().getExtensionString(SubscriptionConstants.EXT_SUBSCRIPTION_RESTHOOK_DELIVER_LATEST_VERSION); + maxRetries = subscription.getChannel().getExtensionString(SubscriptionConstants.EXT_SUBSCRIPTION_MAX_RETRIES); } catch (FHIRException theE) { throw new ConfigurationException("Failed to extract subscription extension(s): " + theE.getMessage(), theE); } retVal.getRestHookDetails().setStripVersionId(Boolean.parseBoolean(stripVersionIds)); retVal.getRestHookDetails().setDeliverLatestVersion(Boolean.parseBoolean(deliverLatestVersion)); + if (isNotBlank(maxRetries)) { + retVal.getRestHookDetails().setMaxRetries(Integer.parseInt(maxRetries)); + } } } catch (FHIRException theE) { @@ -239,14 +246,20 @@ protected CanonicalSubscription canonicalizeR4(IBaseResource theSubscription) { if (retVal.getChannelType() == CanonicalSubscriptionChannelType.RESTHOOK) { String stripVersionIds; String deliverLatestVersion; + String maxRetries; try { stripVersionIds = subscription.getChannel().getExtensionString(SubscriptionConstants.EXT_SUBSCRIPTION_RESTHOOK_STRIP_VERSION_IDS); deliverLatestVersion = subscription.getChannel().getExtensionString(SubscriptionConstants.EXT_SUBSCRIPTION_RESTHOOK_DELIVER_LATEST_VERSION); + maxRetries = subscription.getChannel().getExtensionString(SubscriptionConstants.EXT_SUBSCRIPTION_MAX_RETRIES); + } catch (FHIRException theE) { throw new ConfigurationException("Failed to extract subscription extension(s): " + theE.getMessage(), theE); } retVal.getRestHookDetails().setStripVersionId(Boolean.parseBoolean(stripVersionIds)); retVal.getRestHookDetails().setDeliverLatestVersion(Boolean.parseBoolean(deliverLatestVersion)); + if (isNotBlank(maxRetries)) { + retVal.getRestHookDetails().setMaxRetries(Integer.parseInt(maxRetries)); + } } List topicExts = subscription.getExtensionsByUrl("http://hl7.org/fhir/subscription/topics"); diff --git a/hapi-fhir-jpaserver-subscription/src/main/java/ca/uhn/fhir/jpa/subscription/module/cache/SubscriptionConstants.java b/hapi-fhir-jpaserver-subscription/src/main/java/ca/uhn/fhir/jpa/subscription/module/cache/SubscriptionConstants.java index 5d9f3f1ef20f..e785464508d3 100644 --- a/hapi-fhir-jpaserver-subscription/src/main/java/ca/uhn/fhir/jpa/subscription/module/cache/SubscriptionConstants.java +++ b/hapi-fhir-jpaserver-subscription/src/main/java/ca/uhn/fhir/jpa/subscription/module/cache/SubscriptionConstants.java @@ -67,6 +67,12 @@ public class SubscriptionConstants { */ public static final String EXT_SUBSCRIPTION_RESTHOOK_DELIVER_LATEST_VERSION = "http://hapifhir.io/fhir/StructureDefinition/subscription-resthook-deliver-latest-version"; + /** + * This extension URL indicates the maximum number of delivery retries that will be attempted before the subscription delivery is considered to have failed. + */ + + public static final String EXT_SUBSCRIPTION_MAX_RETRIES = "http://hapifhir.io/fhir/StructureDefinition/subscription-max-retries"; + /** * The number of threads used in subscription channel processing */ 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 3358e8ab0cdf..cefaf35eb798 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 @@ -101,8 +101,7 @@ protected void doDelivery(ResourceDeliveryMessage theMsg, CanonicalSubscription try { operation.execute(); } catch (ResourceNotFoundException e) { - ourLog.error("Cannot reach " + theMsg.getSubscription().getEndpointUrl()); - e.printStackTrace(); + ourLog.error("Cannot reach " + theMsg.getSubscription().getEndpointUrl(), e); throw e; } } From 80dd11d9bf1f006a9d37e4317aa4c8fde4deced4 Mon Sep 17 00:00:00 2001 From: Ken Stevens Date: Tue, 22 Jan 2019 09:56:49 -0500 Subject: [PATCH 2/5] added support for _id in in-memory matcher --- .../matcher/CriteriaResourceMatcher.java | 54 +++++++++++++------ .../matcher/InMemorySubscriptionMatcher.java | 3 +- .../InMemorySubscriptionMatcherTestR3.java | 21 ++++---- src/changes/changes.xml | 3 ++ 4 files changed, 52 insertions(+), 29 deletions(-) diff --git a/hapi-fhir-jpaserver-subscription/src/main/java/ca/uhn/fhir/jpa/subscription/module/matcher/CriteriaResourceMatcher.java b/hapi-fhir-jpaserver-subscription/src/main/java/ca/uhn/fhir/jpa/subscription/module/matcher/CriteriaResourceMatcher.java index 3e8fabb9aba3..34f1793fede0 100644 --- a/hapi-fhir-jpaserver-subscription/src/main/java/ca/uhn/fhir/jpa/subscription/module/matcher/CriteriaResourceMatcher.java +++ b/hapi-fhir-jpaserver-subscription/src/main/java/ca/uhn/fhir/jpa/subscription/module/matcher/CriteriaResourceMatcher.java @@ -20,6 +20,7 @@ * #L% */ +import ca.uhn.fhir.context.FhirContext; import ca.uhn.fhir.context.RuntimeResourceDefinition; import ca.uhn.fhir.context.RuntimeSearchParam; import ca.uhn.fhir.jpa.searchparam.MatchUrlService; @@ -30,8 +31,11 @@ import ca.uhn.fhir.rest.api.Constants; import ca.uhn.fhir.rest.param.BaseParamWithPrefix; import ca.uhn.fhir.rest.param.ReferenceParam; +import ca.uhn.fhir.rest.param.StringParam; import ca.uhn.fhir.rest.server.exceptions.InvalidRequestException; import org.hl7.fhir.instance.model.api.IAnyResource; +import org.hl7.fhir.instance.model.api.IBaseResource; +import org.hl7.fhir.instance.model.api.IIdType; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.stereotype.Service; @@ -42,16 +46,18 @@ @Service public class CriteriaResourceMatcher { - public static final String CRITERIA = "CRITERIA"; + private static final String CRITERIA = "CRITERIA"; @Autowired private MatchUrlService myMatchUrlService; @Autowired ISearchParamRegistry mySearchParamRegistry; + @Autowired + FhirContext myFhirContext; - public SubscriptionMatchResult match(String theCriteria, RuntimeResourceDefinition theResourceDefinition, ResourceIndexedSearchParams theSearchParams) { + public SubscriptionMatchResult match(String theCriteria, IBaseResource theResource, ResourceIndexedSearchParams theSearchParams) { SearchParameterMap searchParameterMap; try { - searchParameterMap = myMatchUrlService.translateMatchUrl(theCriteria, theResourceDefinition); + searchParameterMap = myMatchUrlService.translateMatchUrl(theCriteria, myFhirContext.getResourceDefinition(theResource)); } catch (UnsupportedOperationException e) { return new SubscriptionMatchResult(theCriteria, CRITERIA); } @@ -63,7 +69,7 @@ public SubscriptionMatchResult match(String theCriteria, RuntimeResourceDefiniti for (Map.Entry>> entry : searchParameterMap.entrySet()) { String theParamName = entry.getKey(); List> theAndOrParams = entry.getValue(); - SubscriptionMatchResult result = matchIdsWithAndOr(theParamName, theAndOrParams, theResourceDefinition, theSearchParams); + SubscriptionMatchResult result = matchIdsWithAndOr(theParamName, theAndOrParams, theResource, theSearchParams); if (!result.matched()){ return result; } @@ -72,7 +78,7 @@ public SubscriptionMatchResult match(String theCriteria, RuntimeResourceDefiniti } // This method is modelled from SearchBuilder.searchForIdsWithAndOr() - private SubscriptionMatchResult matchIdsWithAndOr(String theParamName, List> theAndOrParams, RuntimeResourceDefinition theResourceDefinition, ResourceIndexedSearchParams theSearchParams) { + private SubscriptionMatchResult matchIdsWithAndOr(String theParamName, List> theAndOrParams, IBaseResource theResource, ResourceIndexedSearchParams theSearchParams) { if (theAndOrParams.isEmpty()) { return new SubscriptionMatchResult(true, CRITERIA); } @@ -90,30 +96,44 @@ private SubscriptionMatchResult matchIdsWithAndOr(String theParamName, List> theAndOrParams, IBaseResource theResource) { + return theAndOrParams.stream().allMatch(nextAnd -> matchIdsOr(nextAnd, theResource)); + } + private boolean matchIdsOr(List theOrParams, IBaseResource theResource) { + return theOrParams.stream().anyMatch(param -> param instanceof StringParam && matchId(((StringParam)param).getValue(), theResource.getIdElement())); + } + + private boolean matchId(String theValue, IIdType theId) { + return theValue.equals(theId.getValue()) || theValue.equals(theId.getIdPart()); + } + private SubscriptionMatchResult matchResourceParam(String theParamName, List> theAndOrParams, ResourceIndexedSearchParams theSearchParams, String theResourceName, RuntimeSearchParam theParamDef) { if (theParamDef != null) { switch (theParamDef.getParamType()) { 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 414cac544b01..52c70f0f773c 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 @@ -61,7 +61,6 @@ SubscriptionMatchResult match(String criteria, IBaseResource resource) { ResourceIndexedSearchParams searchParams = new ResourceIndexedSearchParams(); mySearchParamExtractorService.extractFromResource(searchParams, entity, resource); myResourceLinkExtractor.extractResourceLinks(searchParams, entity, resource, resource.getMeta().getLastUpdated(), myInlineResourceLinkResolver); - RuntimeResourceDefinition resourceDefinition = myContext.getResourceDefinition(resource); - return myCriteriaResourceMatcher.match(criteria, resourceDefinition, searchParams); + return myCriteriaResourceMatcher.match(criteria, resource, searchParams); } } 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 eb0706b20012..7d37ee2c71ea 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 @@ -12,6 +12,7 @@ import org.springframework.beans.factory.annotation.Autowired; import java.util.Arrays; +import java.util.Collections; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; @@ -38,8 +39,8 @@ private void assertNotMatched(IBaseResource resource, String criteria) { assertFalse(result.matched()); } + // FIXME KHS @Test - @Ignore public void testResourceById() { ProcedureRequest pr = new ProcedureRequest(); @@ -47,12 +48,12 @@ public void testResourceById() { pr.setIntent(ProcedureRequest.ProcedureRequestIntent.ORIGINALORDER); assertMatched(pr, "ProcedureRequest?_id=123"); - assertMatched(pr, "ProcedureRequest?_id=Patient/123"); - assertMatched(pr, "ProcedureRequest?_id=Patient/123,Patient/999"); - assertMatched(pr, "ProcedureRequest?_id=Patient/123&_id=Patient/123"); - assertNotMatched(pr, "ProcedureRequest?_id=Patient/888"); - assertNotMatched(pr, "ProcedureRequest?_id=Patient/888,Patient/999"); - assertNotMatched(pr, "ProcedureRequest?_id=Patient/123&_id=Patient/888"); + assertMatched(pr, "ProcedureRequest?_id=ProcedureRequest/123"); + assertMatched(pr, "ProcedureRequest?_id=ProcedureRequest/123,ProcedureRequest/999"); + assertMatched(pr, "ProcedureRequest?_id=ProcedureRequest/123&_id=ProcedureRequest/123"); + assertNotMatched(pr, "ProcedureRequest?_id=ProcedureRequest/888"); + assertNotMatched(pr, "ProcedureRequest?_id=ProcedureRequest/888,ProcedureRequest/999"); + assertNotMatched(pr, "ProcedureRequest?_id=ProcedureRequest/123&_id=ProcedureRequest/888"); } @@ -301,7 +302,7 @@ public void testProvenance() { sp.setXpathUsage(SearchParameter.XPathUsageType.NORMAL); sp.setStatus(Enumerations.PublicationStatus.ACTIVE); - IBundleProvider bundle = new SimpleBundleProvider(Arrays.asList(sp), "uuid"); + IBundleProvider bundle = new SimpleBundleProvider(Collections.singletonList(sp), "uuid"); initSearchParamRegistry(bundle); { @@ -333,7 +334,7 @@ public void testBodySite() { sp.setXpathUsage(SearchParameter.XPathUsageType.NORMAL); sp.setStatus(Enumerations.PublicationStatus.ACTIVE); - IBundleProvider bundle = new SimpleBundleProvider(Arrays.asList(sp), "uuid"); + IBundleProvider bundle = new SimpleBundleProvider(Collections.singletonList(sp), "uuid"); initSearchParamRegistry(bundle); { @@ -425,7 +426,7 @@ public void testProcedureRequestCategory() { sp.setXpathUsage(SearchParameter.XPathUsageType.NORMAL); sp.setStatus(Enumerations.PublicationStatus.ACTIVE); - IBundleProvider bundle = new SimpleBundleProvider(Arrays.asList(sp), "uuid"); + IBundleProvider bundle = new SimpleBundleProvider(Collections.singletonList(sp), "uuid"); initSearchParamRegistry(bundle); { diff --git a/src/changes/changes.xml b/src/changes/changes.xml index 6646dde15baa..39e4af8db4d4 100644 --- a/src/changes/changes.xml +++ b/src/changes/changes.xml @@ -320,6 +320,9 @@ result in the 10th result being returned). This will now result in an empty response Bundle as would be expected. + + Added support for _id in in-memory matcher + From c7f4cd483e44c1d172a6b0260d7a5ba633a3ae08 Mon Sep 17 00:00:00 2001 From: Ken Stevens Date: Tue, 22 Jan 2019 16:39:00 -0500 Subject: [PATCH 3/5] Backing out subscription retry mechanism --- .../module/CanonicalSubscription.java | 29 +++++-------------- .../cache/SubscriptionCanonicalizer.java | 22 ++++---------- .../module/cache/SubscriptionConstants.java | 6 ---- .../InMemorySubscriptionMatcherTestR3.java | 1 - 4 files changed, 13 insertions(+), 45 deletions(-) diff --git a/hapi-fhir-jpaserver-subscription/src/main/java/ca/uhn/fhir/jpa/subscription/module/CanonicalSubscription.java b/hapi-fhir-jpaserver-subscription/src/main/java/ca/uhn/fhir/jpa/subscription/module/CanonicalSubscription.java index 707a75aabc48..f240b810453e 100644 --- a/hapi-fhir-jpaserver-subscription/src/main/java/ca/uhn/fhir/jpa/subscription/module/CanonicalSubscription.java +++ b/hapi-fhir-jpaserver-subscription/src/main/java/ca/uhn/fhir/jpa/subscription/module/CanonicalSubscription.java @@ -295,8 +295,6 @@ public static class RestHookDetails { private boolean myStripVersionId; @JsonProperty("deliverLatestVersion") private boolean myDeliverLatestVersion; - @JsonProperty("maxRetries") - private int myMaxRetries; /** * Constructor @@ -313,23 +311,6 @@ public void setDeliverLatestVersion(boolean theDeliverLatestVersion) { myDeliverLatestVersion = theDeliverLatestVersion; } - - public boolean isStripVersionId() { - return myStripVersionId; - } - - public void setStripVersionId(boolean theStripVersionId) { - myStripVersionId = theStripVersionId; - } - - public int getMaxRetries() { - return myMaxRetries; - } - - public void setMaxRetries(int theMaxRetries) { - myMaxRetries = theMaxRetries; - } - @Override public boolean equals(Object theO) { if (this == theO) return true; @@ -341,7 +322,6 @@ public boolean equals(Object theO) { return new EqualsBuilder() .append(myStripVersionId, that.myStripVersionId) .append(myDeliverLatestVersion, that.myDeliverLatestVersion) - .append(myMaxRetries, that.myMaxRetries) .isEquals(); } @@ -350,10 +330,17 @@ public int hashCode() { return new HashCodeBuilder(17, 37) .append(myStripVersionId) .append(myDeliverLatestVersion) - .append(myMaxRetries) .toHashCode(); } + public boolean isStripVersionId() { + return myStripVersionId; + } + + public void setStripVersionId(boolean theStripVersionId) { + myStripVersionId = theStripVersionId; + } + } @JsonInclude(JsonInclude.Include.NON_NULL) diff --git a/hapi-fhir-jpaserver-subscription/src/main/java/ca/uhn/fhir/jpa/subscription/module/cache/SubscriptionCanonicalizer.java b/hapi-fhir-jpaserver-subscription/src/main/java/ca/uhn/fhir/jpa/subscription/module/cache/SubscriptionCanonicalizer.java index ed25780a4160..265c6b911644 100644 --- a/hapi-fhir-jpaserver-subscription/src/main/java/ca/uhn/fhir/jpa/subscription/module/cache/SubscriptionCanonicalizer.java +++ b/hapi-fhir-jpaserver-subscription/src/main/java/ca/uhn/fhir/jpa/subscription/module/cache/SubscriptionCanonicalizer.java @@ -64,7 +64,7 @@ public CanonicalSubscription canonicalize(S theSubscription) { } } - protected CanonicalSubscription canonicalizeDstu2(IBaseResource theSubscription) { + private CanonicalSubscription canonicalizeDstu2(IBaseResource theSubscription) { ca.uhn.fhir.model.dstu2.resource.Subscription subscription = (ca.uhn.fhir.model.dstu2.resource.Subscription) theSubscription; CanonicalSubscription retVal = new CanonicalSubscription(); @@ -83,7 +83,7 @@ protected CanonicalSubscription canonicalizeDstu2(IBaseResource theSubscription) return retVal; } - protected CanonicalSubscription canonicalizeDstu3(IBaseResource theSubscription) { + private CanonicalSubscription canonicalizeDstu3(IBaseResource theSubscription) { org.hl7.fhir.dstu3.model.Subscription subscription = (org.hl7.fhir.dstu3.model.Subscription) theSubscription; CanonicalSubscription retVal = new CanonicalSubscription(); @@ -97,10 +97,10 @@ protected CanonicalSubscription canonicalizeDstu3(IBaseResource theSubscription) retVal.setIdElement(subscription.getIdElement()); retVal.setPayloadString(subscription.getChannel().getPayload()); - if (retVal.getChannelType() == CanonicalSubscriptionChannelType.EMAIL) { + if (retVal.getChannelType() == CanonicalSubscriptionChannelType.EMAIL) { String from; String subjectTemplate; - + String bodyTemplate; try { from = subscription.getChannel().getExtensionString(SubscriptionConstants.EXT_SUBSCRIPTION_EMAIL_FROM); subjectTemplate = subscription.getChannel().getExtensionString(SubscriptionConstants.EXT_SUBSCRIPTION_SUBJECT_TEMPLATE); @@ -112,22 +112,16 @@ protected CanonicalSubscription canonicalizeDstu3(IBaseResource theSubscription) } if (retVal.getChannelType() == CanonicalSubscriptionChannelType.RESTHOOK) { - String stripVersionIds; String deliverLatestVersion; - String maxRetries; try { stripVersionIds = subscription.getChannel().getExtensionString(SubscriptionConstants.EXT_SUBSCRIPTION_RESTHOOK_STRIP_VERSION_IDS); deliverLatestVersion = subscription.getChannel().getExtensionString(SubscriptionConstants.EXT_SUBSCRIPTION_RESTHOOK_DELIVER_LATEST_VERSION); - maxRetries = subscription.getChannel().getExtensionString(SubscriptionConstants.EXT_SUBSCRIPTION_MAX_RETRIES); } catch (FHIRException theE) { throw new ConfigurationException("Failed to extract subscription extension(s): " + theE.getMessage(), theE); } retVal.getRestHookDetails().setStripVersionId(Boolean.parseBoolean(stripVersionIds)); retVal.getRestHookDetails().setDeliverLatestVersion(Boolean.parseBoolean(deliverLatestVersion)); - if (isNotBlank(maxRetries)) { - retVal.getRestHookDetails().setMaxRetries(Integer.parseInt(maxRetries)); - } } } catch (FHIRException theE) { @@ -217,7 +211,7 @@ private String extractExtension(IBaseResource theSubscription, String theUrl) { return null; } - protected CanonicalSubscription canonicalizeR4(IBaseResource theSubscription) { + private CanonicalSubscription canonicalizeR4(IBaseResource theSubscription) { org.hl7.fhir.r4.model.Subscription subscription = (org.hl7.fhir.r4.model.Subscription) theSubscription; CanonicalSubscription retVal = new CanonicalSubscription(); @@ -246,20 +240,14 @@ protected CanonicalSubscription canonicalizeR4(IBaseResource theSubscription) { if (retVal.getChannelType() == CanonicalSubscriptionChannelType.RESTHOOK) { String stripVersionIds; String deliverLatestVersion; - String maxRetries; try { stripVersionIds = subscription.getChannel().getExtensionString(SubscriptionConstants.EXT_SUBSCRIPTION_RESTHOOK_STRIP_VERSION_IDS); deliverLatestVersion = subscription.getChannel().getExtensionString(SubscriptionConstants.EXT_SUBSCRIPTION_RESTHOOK_DELIVER_LATEST_VERSION); - maxRetries = subscription.getChannel().getExtensionString(SubscriptionConstants.EXT_SUBSCRIPTION_MAX_RETRIES); - } catch (FHIRException theE) { throw new ConfigurationException("Failed to extract subscription extension(s): " + theE.getMessage(), theE); } retVal.getRestHookDetails().setStripVersionId(Boolean.parseBoolean(stripVersionIds)); retVal.getRestHookDetails().setDeliverLatestVersion(Boolean.parseBoolean(deliverLatestVersion)); - if (isNotBlank(maxRetries)) { - retVal.getRestHookDetails().setMaxRetries(Integer.parseInt(maxRetries)); - } } List topicExts = subscription.getExtensionsByUrl("http://hl7.org/fhir/subscription/topics"); diff --git a/hapi-fhir-jpaserver-subscription/src/main/java/ca/uhn/fhir/jpa/subscription/module/cache/SubscriptionConstants.java b/hapi-fhir-jpaserver-subscription/src/main/java/ca/uhn/fhir/jpa/subscription/module/cache/SubscriptionConstants.java index e785464508d3..5d9f3f1ef20f 100644 --- a/hapi-fhir-jpaserver-subscription/src/main/java/ca/uhn/fhir/jpa/subscription/module/cache/SubscriptionConstants.java +++ b/hapi-fhir-jpaserver-subscription/src/main/java/ca/uhn/fhir/jpa/subscription/module/cache/SubscriptionConstants.java @@ -67,12 +67,6 @@ public class SubscriptionConstants { */ public static final String EXT_SUBSCRIPTION_RESTHOOK_DELIVER_LATEST_VERSION = "http://hapifhir.io/fhir/StructureDefinition/subscription-resthook-deliver-latest-version"; - /** - * This extension URL indicates the maximum number of delivery retries that will be attempted before the subscription delivery is considered to have failed. - */ - - public static final String EXT_SUBSCRIPTION_MAX_RETRIES = "http://hapifhir.io/fhir/StructureDefinition/subscription-max-retries"; - /** * The number of threads used in subscription channel processing */ 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 7d37ee2c71ea..ff6b32d436c0 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,7 +39,6 @@ private void assertNotMatched(IBaseResource resource, String criteria) { assertFalse(result.matched()); } - // FIXME KHS @Test public void testResourceById() { From 3577cb998944f362e33342b5c054768ba0be10ed Mon Sep 17 00:00:00 2001 From: Ken Stevens Date: Tue, 22 Jan 2019 16:40:32 -0500 Subject: [PATCH 4/5] final cleanup --- .../jpa/subscription/module/cache/SubscriptionCanonicalizer.java | 1 - .../subscription/module/matcher/InMemorySubscriptionMatcher.java | 1 - 2 files changed, 2 deletions(-) diff --git a/hapi-fhir-jpaserver-subscription/src/main/java/ca/uhn/fhir/jpa/subscription/module/cache/SubscriptionCanonicalizer.java b/hapi-fhir-jpaserver-subscription/src/main/java/ca/uhn/fhir/jpa/subscription/module/cache/SubscriptionCanonicalizer.java index 265c6b911644..4199054d76b3 100644 --- a/hapi-fhir-jpaserver-subscription/src/main/java/ca/uhn/fhir/jpa/subscription/module/cache/SubscriptionCanonicalizer.java +++ b/hapi-fhir-jpaserver-subscription/src/main/java/ca/uhn/fhir/jpa/subscription/module/cache/SubscriptionCanonicalizer.java @@ -28,7 +28,6 @@ import ca.uhn.fhir.model.api.IPrimitiveDatatype; import ca.uhn.fhir.rest.server.exceptions.InternalErrorException; import ca.uhn.fhir.rest.server.exceptions.PreconditionFailedException; -import org.hl7.fhir.dstu3.model.Subscription; import org.hl7.fhir.exceptions.FHIRException; import org.hl7.fhir.instance.model.api.IBaseReference; import org.hl7.fhir.instance.model.api.IBaseResource; 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 52c70f0f773c..c7e789b0c372 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 @@ -21,7 +21,6 @@ */ import ca.uhn.fhir.context.FhirContext; -import ca.uhn.fhir.context.RuntimeResourceDefinition; import ca.uhn.fhir.jpa.model.entity.ResourceTable; import ca.uhn.fhir.jpa.searchparam.extractor.ResourceIndexedSearchParams; import ca.uhn.fhir.jpa.searchparam.extractor.ResourceLinkExtractor; From 7b5a8d9c6f4225d6843b41931cfae957986ec023 Mon Sep 17 00:00:00 2001 From: Ken Stevens Date: Wed, 23 Jan 2019 13:17:29 -0500 Subject: [PATCH 5/5] James' feedback --- .../subscriber/SubscriptionDeliveringRestHookSubscriber.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) 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 cefaf35eb798..0667f7be9f2c 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 @@ -101,7 +101,8 @@ protected void doDelivery(ResourceDeliveryMessage theMsg, CanonicalSubscription try { operation.execute(); } catch (ResourceNotFoundException e) { - ourLog.error("Cannot reach " + theMsg.getSubscription().getEndpointUrl(), e); + ourLog.error("Cannot reach {} ", theMsg.getSubscription().getEndpointUrl()); + ourLog.error("Exception: ", e); throw e; } }