From 1e07fcd2b39dd295de876e41d3387967c23f40ee Mon Sep 17 00:00:00 2001 From: James Agnew Date: Thu, 15 Aug 2019 08:35:50 -0400 Subject: [PATCH] Fix an unintended regression in #1357 (#1429) * Start working on a tweak to #1357 - Not yet complete * Tweaks to avoid an unintended regression from #1357 --- .../ca/uhn/fhir/rest/api/EncodingEnum.java | 19 ++++++++++++++--- .../uhn/fhir/rest/api/EncodingEnumTest.java | 17 +++++++++++++++ .../module/subscriber/IResourceRetriever.java | 2 +- .../subscriber/ResourceDeliveryMessage.java | 11 +++------- ...scriptionDeliveringRestHookSubscriber.java | 4 ---- .../SubscriptionMatchingSubscriber.java | 21 +++++++------------ ...SubscriptionDeliveringEmailSubscriber.java | 14 ++++++++++++- 7 files changed, 57 insertions(+), 31 deletions(-) create mode 100644 hapi-fhir-base/src/test/java/ca/uhn/fhir/rest/api/EncodingEnumTest.java diff --git a/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/api/EncodingEnum.java b/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/api/EncodingEnum.java index 65060416a2e..85d51952f6a 100644 --- a/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/api/EncodingEnum.java +++ b/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/api/EncodingEnum.java @@ -197,12 +197,25 @@ public enum EncodingEnum { } } - private static String getTypeWithoutCharset(final String theContentType) { + static String getTypeWithoutCharset(final String theContentType) { if (theContentType == null) { return null; } else { - String[] contentTypeSplitted = theContentType.split(";"); - return contentTypeSplitted[0]; + + int start = 0; + for (; start < theContentType.length(); start++) { + if (theContentType.charAt(start) != ' ') { + break; + } + } + int end = start; + for (; end < theContentType.length(); end++) { + if (theContentType.charAt(end) == ' ' || theContentType.charAt(end) == ';') { + break; + } + } + + return theContentType.substring(start, end); } } diff --git a/hapi-fhir-base/src/test/java/ca/uhn/fhir/rest/api/EncodingEnumTest.java b/hapi-fhir-base/src/test/java/ca/uhn/fhir/rest/api/EncodingEnumTest.java new file mode 100644 index 00000000000..f8848a3d53e --- /dev/null +++ b/hapi-fhir-base/src/test/java/ca/uhn/fhir/rest/api/EncodingEnumTest.java @@ -0,0 +1,17 @@ +package ca.uhn.fhir.rest.api; + +import org.junit.Test; + +import static org.junit.Assert.*; + +public class EncodingEnumTest { + + @Test + public void getTypeWithoutCharset() { + assertEquals("text/plain", EncodingEnum.getTypeWithoutCharset("text/plain")); + assertEquals("text/plain", EncodingEnum.getTypeWithoutCharset(" text/plain")); + assertEquals("text/plain", EncodingEnum.getTypeWithoutCharset(" text/plain; charset=utf-8")); + assertEquals("text/plain", EncodingEnum.getTypeWithoutCharset(" text/plain ; charset=utf-8")); + } + +} diff --git a/hapi-fhir-jpaserver-subscription/src/main/java/ca/uhn/fhir/jpa/subscription/module/subscriber/IResourceRetriever.java b/hapi-fhir-jpaserver-subscription/src/main/java/ca/uhn/fhir/jpa/subscription/module/subscriber/IResourceRetriever.java index 2302973111f..f56b61981b4 100644 --- a/hapi-fhir-jpaserver-subscription/src/main/java/ca/uhn/fhir/jpa/subscription/module/subscriber/IResourceRetriever.java +++ b/hapi-fhir-jpaserver-subscription/src/main/java/ca/uhn/fhir/jpa/subscription/module/subscriber/IResourceRetriever.java @@ -24,5 +24,5 @@ import org.hl7.fhir.instance.model.api.IBaseResource; import org.hl7.fhir.instance.model.api.IIdType; public interface IResourceRetriever { - IBaseResource getResource(IIdType id); + IBaseResource getResource(IIdType theId); } diff --git a/hapi-fhir-jpaserver-subscription/src/main/java/ca/uhn/fhir/jpa/subscription/module/subscriber/ResourceDeliveryMessage.java b/hapi-fhir-jpaserver-subscription/src/main/java/ca/uhn/fhir/jpa/subscription/module/subscriber/ResourceDeliveryMessage.java index 565fdb45523..7f42c840536 100644 --- a/hapi-fhir-jpaserver-subscription/src/main/java/ca/uhn/fhir/jpa/subscription/module/subscriber/ResourceDeliveryMessage.java +++ b/hapi-fhir-jpaserver-subscription/src/main/java/ca/uhn/fhir/jpa/subscription/module/subscriber/ResourceDeliveryMessage.java @@ -23,6 +23,7 @@ package ca.uhn.fhir.jpa.subscription.module.subscriber; import ca.uhn.fhir.context.FhirContext; import ca.uhn.fhir.jpa.subscription.module.CanonicalSubscription; import ca.uhn.fhir.jpa.subscription.module.ResourceModifiedMessage; +import ca.uhn.fhir.rest.api.EncodingEnum; import com.fasterxml.jackson.annotation.JsonAutoDetect; import com.fasterxml.jackson.annotation.JsonIgnore; import com.fasterxml.jackson.annotation.JsonInclude; @@ -97,15 +98,9 @@ public class ResourceDeliveryMessage extends BaseResourceMessage implements IRes mySubscription = theSubscription; } - public void setPayload(FhirContext theCtx, IBaseResource thePayload, Boolean theXml) { + public void setPayload(FhirContext theCtx, IBaseResource thePayload, EncodingEnum theEncoding) { myPayload = thePayload; - - if (theXml) { - myPayloadString = theCtx.newXmlParser().encodeResourceToString(thePayload); - } else { - myPayloadString = theCtx.newJsonParser().encodeResourceToString(thePayload); - } - + myPayloadString = theEncoding.newParser(theCtx).encodeResourceToString(thePayload); myPayloadId = thePayload.getIdElement().toUnqualified().getValue(); } 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 c0b736fcfd6..26b0c355265 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 @@ -155,10 +155,6 @@ public class SubscriptionDeliveringRestHookSubscriber extends BaseSubscriptionDe String payloadString = subscription.getPayloadString(); EncodingEnum payloadType = null; if (payloadString != null) { - if (payloadString.contains(";")) { - payloadString = payloadString.substring(0, payloadString.indexOf(';')); - } - payloadString = payloadString.trim(); payloadType = EncodingEnum.forContentType(payloadString); } 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 f1e92053820..8f2df1f4d77 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 @@ -11,6 +11,7 @@ import ca.uhn.fhir.jpa.subscription.module.cache.ActiveSubscription; import ca.uhn.fhir.jpa.subscription.module.cache.SubscriptionRegistry; import ca.uhn.fhir.jpa.subscription.module.matcher.ISubscriptionMatcher; import ca.uhn.fhir.rest.api.Constants; +import ca.uhn.fhir.rest.api.EncodingEnum; import org.apache.commons.lang3.StringUtils; import org.hl7.fhir.instance.model.api.IBaseResource; import org.hl7.fhir.instance.model.api.IIdType; @@ -25,6 +26,7 @@ import org.springframework.stereotype.Service; import java.util.Collection; +import static org.apache.commons.lang3.ObjectUtils.defaultIfNull; import static org.apache.commons.lang3.StringUtils.isNotBlank; /*- @@ -104,7 +106,7 @@ public class SubscriptionMatchingSubscriber implements MessageHandler { private void doMatchActiveSubscriptionsAndDeliver(ResourceModifiedMessage theMsg) { IIdType resourceId = theMsg.getId(myFhirContext); - Boolean isXml = false, isJson = false, isText = false; + Boolean isText = false; Collection subscriptions = mySubscriptionRegistry.getAll(); @@ -138,28 +140,19 @@ public class SubscriptionMatchingSubscriber implements MessageHandler { IBaseResource payload = theMsg.getNewPayload(myFhirContext); CanonicalSubscription subscription = nextActiveSubscription.getSubscription(); + EncodingEnum encoding = null; if (subscription.getPayloadString() != null && !subscription.getPayloadString().isEmpty()) { - isXml = subscription.getPayloadString().equals(Constants.CT_XML) || subscription.getPayloadString().equals(Constants.CT_FHIR_XML_NEW); - isJson = subscription.getPayloadString().equals(Constants.CT_JSON) || subscription.getPayloadString().equals(Constants.CT_FHIR_JSON_NEW); + encoding = EncodingEnum.forContentType(subscription.getPayloadString()); isText = subscription.getPayloadString().equals(Constants.CT_TEXT); } + encoding = defaultIfNull(encoding, EncodingEnum.JSON); ResourceDeliveryMessage deliveryMsg = new ResourceDeliveryMessage(); - // Only include the payload if either XML or JSON was specified in the subscription's payload property - // See http://hl7.org/fhir/subscription-definitions.html#Subscription.channel.payload - if (isXml || isJson) { - deliveryMsg.setPayload(myFhirContext, payload, isXml); - } else if (isText) { - // TODO: Handle payload mimetype of text/plain (for just the .text representation of the resource being updated?) - } - + deliveryMsg.setPayload(myFhirContext, payload, encoding); deliveryMsg.setSubscription(subscription); deliveryMsg.setOperationType(theMsg.getOperationType()); deliveryMsg.copyAdditionalPropertiesFrom(theMsg); - if (payload == null) { - deliveryMsg.setPayloadId(theMsg.getId(myFhirContext)); - } // Interceptor call: SUBSCRIPTION_RESOURCE_MATCHED HookParams params = new HookParams() diff --git a/hapi-fhir-jpaserver-subscription/src/main/java/ca/uhn/fhir/jpa/subscription/module/subscriber/email/SubscriptionDeliveringEmailSubscriber.java b/hapi-fhir-jpaserver-subscription/src/main/java/ca/uhn/fhir/jpa/subscription/module/subscriber/email/SubscriptionDeliveringEmailSubscriber.java index f2238d3b249..765e849d814 100644 --- a/hapi-fhir-jpaserver-subscription/src/main/java/ca/uhn/fhir/jpa/subscription/module/subscriber/email/SubscriptionDeliveringEmailSubscriber.java +++ b/hapi-fhir-jpaserver-subscription/src/main/java/ca/uhn/fhir/jpa/subscription/module/subscriber/email/SubscriptionDeliveringEmailSubscriber.java @@ -20,10 +20,12 @@ package ca.uhn.fhir.jpa.subscription.module.subscriber.email; * #L% */ +import ca.uhn.fhir.context.FhirContext; import ca.uhn.fhir.jpa.model.entity.ModelConfig; import ca.uhn.fhir.jpa.subscription.module.CanonicalSubscription; import ca.uhn.fhir.jpa.subscription.module.subscriber.BaseSubscriptionDeliverySubscriber; import ca.uhn.fhir.jpa.subscription.module.subscriber.ResourceDeliveryMessage; +import ca.uhn.fhir.rest.api.EncodingEnum; import org.apache.commons.lang3.StringUtils; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -43,6 +45,8 @@ public class SubscriptionDeliveringEmailSubscriber extends BaseSubscriptionDeliv @Autowired private ModelConfig myModelConfig; + @Autowired + private FhirContext myCtx; private IEmailSender myEmailSender; @@ -66,13 +70,21 @@ public class SubscriptionDeliveringEmailSubscriber extends BaseSubscriptionDeliv } } + String payload = ""; + if (isNotBlank(subscription.getPayloadString())) { + EncodingEnum encoding = EncodingEnum.forContentType(subscription.getPayloadString()); + if (encoding != null) { + payload = theMessage.getPayloadString(); + } + } + String from = processEmailAddressUri(defaultString(subscription.getEmailDetails().getFrom(), myModelConfig.getEmailFromAddress())); String subjectTemplate = defaultString(subscription.getEmailDetails().getSubjectTemplate(), provideDefaultSubjectTemplate()); EmailDetails details = new EmailDetails(); details.setTo(destinationAddresses); details.setFrom(from); - details.setBodyTemplate(theMessage.getPayloadString()); + details.setBodyTemplate(payload); details.setSubjectTemplate(subjectTemplate); details.setSubscription(subscription.getIdElement(myFhirContext));