Don't crash on startup if an invalid subscription is in the database

hapi-fhir-base/src/main/java/ca/uhn/fhir/util/SubscriptionUtil.java

@@ -0,0 +1,64 @@
+package ca.uhn.fhir.util;
+
+import ca.uhn.fhir.context.BaseRuntimeChildDefinition;
+import ca.uhn.fhir.context.BaseRuntimeElementDefinition;
+import ca.uhn.fhir.context.FhirContext;
+import ca.uhn.fhir.context.RuntimeResourceDefinition;
+import org.hl7.fhir.instance.model.api.IBase;
+import org.hl7.fhir.instance.model.api.IBaseResource;
+import org.hl7.fhir.instance.model.api.IPrimitiveType;
+import org.thymeleaf.util.Validate;
+
+import java.util.List;
+
+/*
+ * #%L
+ * HAPI FHIR - Core Library
+ * %%
+ * Copyright (C) 2014 - 2018 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%
+ */
+
+/**
+ * Utilities for working with the subscription resource
+ */
+public class SubscriptionUtil {
+
+	private static void populatePrimitiveValue(FhirContext theContext, IBaseResource theSubscription, String theChildName, String theValue) {
+		RuntimeResourceDefinition def = theContext.getResourceDefinition(theSubscription);
+		Validate.isTrue(def.getName().equals("Subscription"), "theResource is not a subscription");
+		BaseRuntimeChildDefinition statusChild = def.getChildByName(theChildName);
+		List<IBase> entries = statusChild.getAccessor().getValues(theSubscription);
+		IPrimitiveType<?> instance;
+		if (entries.size() == 0) {
+			BaseRuntimeElementDefinition<?> statusElement = statusChild.getChildByName(theChildName);
+			instance = (IPrimitiveType<?>) statusElement.newInstance(statusChild.getInstanceConstructorArguments());
+			statusChild.getMutator().addValue(theSubscription, instance);
+		} else {
+			instance = (IPrimitiveType<?>) entries.get(0);
+		}
+
+		instance.setValueAsString(theValue);
+	}
+
+	public static void setReason(FhirContext theContext, IBaseResource theSubscription, String theMessage) {
+		populatePrimitiveValue(theContext, theSubscription, "reason", theMessage);
+	}
+
+	public static void setStatus(FhirContext theContext, IBaseResource theSubscription, String theStatus) {
+		populatePrimitiveValue(theContext, theSubscription, "status", theStatus);
+	}
+
+}

hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseHapiFhirDao.java

@@ -9,9 +9,9 @@ package ca.uhn.fhir.jpa.dao;
  * 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.
@@ -50,6 +50,7 @@ import ca.uhn.fhir.rest.server.interceptor.IServerInterceptor;
 import ca.uhn.fhir.rest.server.interceptor.IServerInterceptor.ActionRequestDetails;
 import ca.uhn.fhir.rest.server.servlet.ServletRequestDetails;
 import ca.uhn.fhir.util.*;
+import com.google.common.annotations.VisibleForTesting;
 import com.google.common.base.Charsets;
 import com.google.common.collect.ArrayListMultimap;
 import com.google.common.collect.Sets;
@@ -75,8 +76,6 @@ import javax.xml.stream.events.Characters;
 import javax.xml.stream.events.XMLEvent;
 import java.io.CharArrayWriter;
 import java.io.UnsupportedEncodingException;
-import java.io.Writer;
-import java.nio.CharBuffer;
 import java.text.Normalizer;
 import java.util.*;
 import java.util.Map.Entry;
@@ -104,6 +103,7 @@ public abstract class BaseHapiFhirDao<T extends IBaseResource> implements IDao {
 	private static final org.slf4j.Logger ourLog = org.slf4j.LoggerFactory.getLogger(BaseHapiFhirDao.class);
 	private static final Map<FhirVersionEnum, FhirContext> ourRetrievalContexts = new HashMap<FhirVersionEnum, FhirContext>();
 	private static final String PROCESSING_SUB_REQUEST = "BaseHapiFhirDao.processingSubRequest";
+	private static boolean ourValidationDisabledForUnitTest;
 
 	static {
 		Map<String, Class<? extends IQueryParameterType>> resourceMetaParams = new HashMap<String, Class<? extends IQueryParameterType>>();
@@ -1213,6 +1213,7 @@ public abstract class BaseHapiFhirDao<T extends IBaseResource> implements IDao {
 		retVal.removeAll(theToRemove);
 		return retVal;
 	}
+
 	private void setUpdatedTime(Collection<? extends BaseResourceIndexedSearchParam> theParams, Date theUpdateTime) {
 		for (BaseResourceIndexedSearchParam nextSearchParam : theParams) {
 			nextSearchParam.setUpdated(theUpdateTime);
@@ -1377,7 +1378,9 @@ public abstract class BaseHapiFhirDao<T extends IBaseResource> implements IDao {
 		 */
 		if (theResource != null) {
 			if (thePerformIndexing) {
-				validateResourceForStorage((T) theResource, theEntity);
+				if (!ourValidationDisabledForUnitTest) {
+					validateResourceForStorage((T) theResource, theEntity);
+				}
 			}
 			String resourceType = myContext.getResourceDefinition(theResource).getName();
 			if (isNotBlank(theEntity.getResourceType()) && !theEntity.getResourceType().equals(resourceType)) {
@@ -2105,6 +2108,14 @@ public abstract class BaseHapiFhirDao<T extends IBaseResource> implements IDao {
 		return b.toString();
 	}
 
+	/**
+	 * Do not call this method outside of unit tests
+	 */
+	@VisibleForTesting
+	public static void setValidationDisabledForUnitTest(boolean theValidationDisabledForUnitTest) {
+		ourValidationDisabledForUnitTest = theValidationDisabledForUnitTest;
+	}
+
 	private static List<BaseCodingDt> toBaseCodingList(List<IBaseCoding> theSecurityLabels) {
 		ArrayList<BaseCodingDt> retVal = new ArrayList<BaseCodingDt>(theSecurityLabels.size());
 		for (IBaseCoding next : theSecurityLabels) {

hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/dstu3/FhirResourceDaoSubscriptionDstu3.java

@@ -29,6 +29,7 @@ import ca.uhn.fhir.jpa.entity.SubscriptionTable;
 import ca.uhn.fhir.parser.DataFormatException;
 import ca.uhn.fhir.rest.api.EncodingEnum;
 import ca.uhn.fhir.rest.server.exceptions.UnprocessableEntityException;
+import org.apache.commons.lang3.ObjectUtils;
 import org.hl7.fhir.dstu3.model.Subscription;
 import org.hl7.fhir.dstu3.model.Subscription.SubscriptionChannelType;
 import org.hl7.fhir.dstu3.model.Subscription.SubscriptionStatus;
@@ -108,6 +109,16 @@ public class FhirResourceDaoSubscriptionDstu3 extends FhirResourceDaoDstu3<Subsc
 	}
 
 	public RuntimeResourceDefinition validateCriteriaAndReturnResourceDefinition(Subscription theResource) {
+		switch (ObjectUtils.defaultIfNull(theResource.getStatus(), SubscriptionStatus.OFF)) {
+			case REQUESTED:
+			case ACTIVE:
+				break;
+			case ERROR:
+			case OFF:
+			case NULL:
+				return null;
+		}
+
 		String query = theResource.getCriteria();
 		if (isBlank(query)) {
 			throw new UnprocessableEntityException("Subscription.criteria must be populated");
@@ -144,6 +155,9 @@ public class FhirResourceDaoSubscriptionDstu3 extends FhirResourceDaoDstu3<Subsc
 		super.validateResourceForStorage(theResource, theEntityToSave);
 
 		RuntimeResourceDefinition resDef = validateCriteriaAndReturnResourceDefinition(theResource);
+		if (resDef == null) {
+			return;
+		}
 
 		IFhirResourceDao<? extends IBaseResource> dao = getDao(resDef.getImplementingClass());
 		if (dao == null) {

hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoSubscriptionR4.java

@@ -29,6 +29,7 @@ import ca.uhn.fhir.jpa.entity.SubscriptionTable;
 import ca.uhn.fhir.parser.DataFormatException;
 import ca.uhn.fhir.rest.api.EncodingEnum;
 import ca.uhn.fhir.rest.server.exceptions.UnprocessableEntityException;
+import org.apache.commons.lang3.ObjectUtils;
 import org.hl7.fhir.instance.model.api.IBaseResource;
 import org.hl7.fhir.instance.model.api.IIdType;
 import org.hl7.fhir.r4.model.Subscription;
@@ -37,20 +38,16 @@ import org.hl7.fhir.r4.model.Subscription.SubscriptionStatus;
 import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.transaction.PlatformTransactionManager;
 
+import javax.annotation.Nullable;
 import java.util.Date;
 
 import static org.apache.commons.lang3.StringUtils.isBlank;
 
 public class FhirResourceDaoSubscriptionR4 extends FhirResourceDaoR4<Subscription> implements IFhirResourceDaoSubscription<Subscription> {
 
-	private static final org.slf4j.Logger ourLog = org.slf4j.LoggerFactory.getLogger(FhirResourceDaoSubscriptionR4.class);
-
 	@Autowired
 	private ISubscriptionTableDao mySubscriptionTableDao;
 
-	@Autowired
-	private PlatformTransactionManager myTxManager;
-
 	private void createSubscriptionTable(ResourceTable theEntity, Subscription theSubscription) {
 		SubscriptionTable subscriptionEntity = new SubscriptionTable();
 		subscriptionEntity.setCreated(new Date());
@@ -108,7 +105,18 @@ public class FhirResourceDaoSubscriptionR4 extends FhirResourceDaoR4<Subscriptio
 		}
 	}
 
+	@Nullable
 	public RuntimeResourceDefinition validateCriteriaAndReturnResourceDefinition(Subscription theResource) {
+		switch (ObjectUtils.defaultIfNull(theResource.getStatus(), Subscription.SubscriptionStatus.OFF)) {
+			case REQUESTED:
+			case ACTIVE:
+				break;
+			case ERROR:
+			case OFF:
+			case NULL:
+				return null;
+		}
+
 		String query = theResource.getCriteria();
 		if (isBlank(query)) {
 			throw new UnprocessableEntityException("Subscription.criteria must be populated");
@@ -145,6 +153,9 @@ public class FhirResourceDaoSubscriptionR4 extends FhirResourceDaoR4<Subscriptio
 		super.validateResourceForStorage(theResource, theEntityToSave);
 
 		RuntimeResourceDefinition resDef = validateCriteriaAndReturnResourceDefinition(theResource);
+		if (resDef == null) {
+			return;
+		}
 
 		IFhirResourceDao<? extends IBaseResource> dao = getDao(resDef.getImplementingClass());
 		if (dao == null) {

hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/subscription/BaseSubscriptionInterceptor.java

@@ -9,9 +9,9 @@ package ca.uhn.fhir.jpa.subscription;
  * 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.

hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/subscription/BaseSubscriptionSubscriber.java

@@ -26,7 +26,6 @@ import org.hl7.fhir.instance.model.api.IBaseResource;
 import org.hl7.fhir.instance.model.api.IPrimitiveType;
 import org.hl7.fhir.r4.model.Subscription;
 import org.springframework.messaging.MessageHandler;
-import org.springframework.messaging.MessagingException;
 
 public abstract class BaseSubscriptionSubscriber implements MessageHandler {
 
@@ -60,14 +59,6 @@ public abstract class BaseSubscriptionSubscriber implements MessageHandler {
 	}
 
 
-	/**
-	 * Does this subscription type (e.g. rest hook, websocket, etc) apply to this interceptor?
-	 */
-	protected boolean subscriptionTypeApplies(IBaseResource theSubscription) {
-		FhirContext ctx = mySubscriptionDao.getContext();
-		return subscriptionTypeApplies(ctx, theSubscription);
-	}
-
 	/**
 	 * Does this subscription type (e.g. rest hook, websocket, etc) apply to this interceptor?
 	 */
@@ -80,10 +71,12 @@ public abstract class BaseSubscriptionSubscriber implements MessageHandler {
 	 * Does this subscription type (e.g. rest hook, websocket, etc) apply to this interceptor?
 	 */
 	static boolean subscriptionTypeApplies(FhirContext theCtx, IBaseResource theSubscription, Subscription.SubscriptionChannelType theChannelType) {
-		IPrimitiveType<?> status = theCtx.newTerser().getSingleValueOrNull(theSubscription, BaseSubscriptionInterceptor.SUBSCRIPTION_TYPE, IPrimitiveType.class);
+		IPrimitiveType<?> subscriptionType = theCtx.newTerser().getSingleValueOrNull(theSubscription, BaseSubscriptionInterceptor.SUBSCRIPTION_TYPE, IPrimitiveType.class);
 		boolean subscriptionTypeApplies = false;
-		if (theChannelType.toCode().equals(status.getValueAsString())) {
+ 		if (subscriptionType != null) {
-			subscriptionTypeApplies = true;
+			if (theChannelType.toCode().equals(subscriptionType.getValueAsString())) {
+				subscriptionTypeApplies = true;
+			}
 		}
 		return subscriptionTypeApplies;
 	}

hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/subscription/SubscriptionActivatingSubscriber.java

@@ -23,6 +23,9 @@ package ca.uhn.fhir.jpa.subscription;
 import ca.uhn.fhir.context.FhirContext;
 import ca.uhn.fhir.jpa.dao.IFhirResourceDao;
 import ca.uhn.fhir.rest.api.RestOperationTypeEnum;
+import ca.uhn.fhir.rest.server.exceptions.UnprocessableEntityException;
+import ca.uhn.fhir.util.SubscriptionUtil;
+import org.apache.commons.lang3.time.DateUtils;
 import org.hl7.fhir.instance.model.api.IBaseResource;
 import org.hl7.fhir.instance.model.api.IIdType;
 import org.hl7.fhir.instance.model.api.IPrimitiveType;
@@ -30,12 +33,14 @@ import org.hl7.fhir.r4.model.Subscription;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 import org.springframework.messaging.MessagingException;
+import org.springframework.scheduling.TaskScheduler;
 import org.springframework.transaction.PlatformTransactionManager;
 import org.springframework.transaction.TransactionStatus;
-import org.springframework.transaction.support.TransactionCallbackWithoutResult;
+import org.springframework.transaction.support.*;
-import org.springframework.transaction.support.TransactionSynchronizationAdapter;
+
-import org.springframework.transaction.support.TransactionSynchronizationManager;
+import java.util.Date;
-import org.springframework.transaction.support.TransactionTemplate;
+import java.util.concurrent.Callable;
+import java.util.concurrent.ExecutorService;
 
 @SuppressWarnings("unchecked")
 public class SubscriptionActivatingSubscriber {
@@ -92,11 +97,18 @@ public class SubscriptionActivatingSubscriber {
 		}
 	}
 
-	private void activateSubscription(IPrimitiveType<?> theStatus, String theActiveStatus, IBaseResource theSubscription, String theRequestedStatus) {
+	private void activateSubscription(IPrimitiveType<?> theStatus, String theActiveStatus, final IBaseResource theSubscription, String theRequestedStatus) {
 		theStatus.setValueAsString(theActiveStatus);
 		ourLog.info("Activating and registering subscription {} from status {} to {}", theSubscription.getIdElement().toUnqualified().getValue(), theRequestedStatus, theActiveStatus);
-		mySubscriptionDao.update(theSubscription);
+		try {
-		mySubscriptionInterceptor.registerSubscription(theSubscription.getIdElement(), theSubscription);
+			mySubscriptionDao.update(theSubscription);
+		} catch (final UnprocessableEntityException e) {
+			ourLog.info("Changing status of {} to ERROR", theSubscription.getIdElement());
+			IBaseResource subscription = mySubscriptionDao.read(theSubscription.getIdElement());
+			SubscriptionUtil.setStatus(myCtx, subscription, "error");
+			SubscriptionUtil.setReason(myCtx, subscription, e.getMessage());
+			mySubscriptionDao.update(subscription);
+		}
 	}
 
 

hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/dstu3/FhirResourceDaoDstu3InvalidSubscriptionTest.java

@@ -0,0 +1,150 @@
+package ca.uhn.fhir.jpa.dao.dstu3;
+
+import ca.uhn.fhir.jpa.dao.BaseHapiFhirDao;
+import ca.uhn.fhir.jpa.dao.DaoConfig;
+import ca.uhn.fhir.jpa.subscription.resthook.SubscriptionRestHookInterceptor;
+import ca.uhn.fhir.rest.server.exceptions.UnprocessableEntityException;
+import ca.uhn.fhir.util.TestUtil;
+import org.hl7.fhir.dstu3.model.Subscription;
+import org.hl7.fhir.instance.model.api.IIdType;
+import org.junit.After;
+import org.junit.AfterClass;
+import org.junit.Test;
+import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.transaction.TransactionStatus;
+import org.springframework.transaction.support.TransactionCallbackWithoutResult;
+import org.springframework.transaction.support.TransactionTemplate;
+
+import javax.persistence.Query;
+
+import static org.junit.Assert.*;
+
+public class FhirResourceDaoDstu3InvalidSubscriptionTest extends BaseJpaDstu3Test {
+
+	@Autowired
+	private SubscriptionRestHookInterceptor myInterceptor;
+
+	@After
+	public void afterResetDao() {
+		myDaoConfig.setResourceServerIdStrategy(new DaoConfig().getResourceServerIdStrategy());
+		BaseHapiFhirDao.setValidationDisabledForUnitTest(false);
+	}
+
+	@Test
+	public void testCreateInvalidSubscriptionOkButCanNotActivate() {
+		Subscription s = new Subscription();
+		s.setStatus(Subscription.SubscriptionStatus.OFF);
+		s.setCriteria("FOO");
+		IIdType id = mySubscriptionDao.create(s).getId().toUnqualified();
+
+		s = mySubscriptionDao.read(id);
+		assertEquals("FOO", s.getCriteria());
+
+		s.setStatus(Subscription.SubscriptionStatus.REQUESTED);
+		try {
+			mySubscriptionDao.update(s);
+			fail();
+		} catch (UnprocessableEntityException e) {
+			assertEquals("Subscription.criteria must be in the form \"{Resource Type}?[params]", e.getMessage());
+		}
+	}
+
+	/**
+	 * Make sure that bad data in the database doesn't prevent startup
+	 */
+	@Test
+	public void testSubscriptionMarkedDeleted() {
+		BaseHapiFhirDao.setValidationDisabledForUnitTest(true);
+
+		Subscription s = new Subscription();
+		s.setStatus(Subscription.SubscriptionStatus.REQUESTED);
+		s.getChannel().setEndpoint("http://foo");
+		s.getChannel().setPayload("application/fhir+json");
+		s.setCriteria("Patient?foo");
+		final IIdType id = mySubscriptionDao.create(s).getId().toUnqualifiedVersionless();
+		assertNotNull(id.getIdPart());
+
+		BaseHapiFhirDao.setValidationDisabledForUnitTest(false);
+
+		new TransactionTemplate(myTransactionMgr).execute(new TransactionCallbackWithoutResult() {
+			@Override
+			protected void doInTransactionWithoutResult(TransactionStatus status) {
+				Query q = myEntityManager.createNativeQuery("UPDATE HFJ_RESOURCE SET RES_DELETED_AT = RES_UPDATED WHERE RES_ID = " + id.getIdPart());
+				q.executeUpdate();
+			}
+		});
+
+		myEntityManager.clear();
+
+		myInterceptor.start();
+	}
+
+	/**
+	 * Make sure that bad data in the database doesn't prevent startup
+	 */
+	@Test
+	public void testSubscriptionWithInvalidCriteria() {
+		BaseHapiFhirDao.setValidationDisabledForUnitTest(true);
+
+		Subscription s = new Subscription();
+		s.setStatus(Subscription.SubscriptionStatus.REQUESTED);
+		s.getChannel().setType(Subscription.SubscriptionChannelType.RESTHOOK);
+		s.getChannel().setEndpoint("http://foo");
+		s.getChannel().setPayload("application/fhir+json");
+		s.setCriteria("BLAH");
+		IIdType id = mySubscriptionDao.create(s).getId().toUnqualifiedVersionless();
+		assertNotNull(id.getIdPart());
+
+		BaseHapiFhirDao.setValidationDisabledForUnitTest(false);
+
+		myInterceptor.start();
+
+	}
+
+	/**
+	 * Make sure that bad data in the database doesn't prevent startup
+	 */
+	@Test
+	public void testSubscriptionWithNoStatus() {
+		BaseHapiFhirDao.setValidationDisabledForUnitTest(true);
+
+		Subscription s = new Subscription();
+		s.getChannel().setType(Subscription.SubscriptionChannelType.RESTHOOK);
+		s.getChannel().setEndpoint("http://foo");
+		s.getChannel().setPayload("application/fhir+json");
+		s.setCriteria("Patient?active=true");
+		IIdType id = mySubscriptionDao.create(s).getId().toUnqualifiedVersionless();
+
+		BaseHapiFhirDao.setValidationDisabledForUnitTest(false);
+
+		myInterceptor.start();
+
+	}
+
+	/**
+	 * Make sure that bad data in the database doesn't prevent startup
+	 */
+	@Test
+	public void testSubscriptionWithNoType() {
+		BaseHapiFhirDao.setValidationDisabledForUnitTest(true);
+
+		Subscription s = new Subscription();
+		s.setStatus(Subscription.SubscriptionStatus.REQUESTED);
+		s.getChannel().setEndpoint("http://foo");
+		s.getChannel().setPayload("application/fhir+json");
+		s.setCriteria("Patient?foo");
+		IIdType id = mySubscriptionDao.create(s).getId().toUnqualifiedVersionless();
+		assertNotNull(id.getIdPart());
+
+		BaseHapiFhirDao.setValidationDisabledForUnitTest(false);
+
+		myInterceptor.start();
+
+	}
+
+	@AfterClass
+	public static void afterClassClearContext() {
+		TestUtil.clearAllStaticFieldsForUnitTest();
+	}
+
+}

hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4InvalidSubscriptionTest.java

@@ -0,0 +1,150 @@
+package ca.uhn.fhir.jpa.dao.r4;
+
+import ca.uhn.fhir.jpa.dao.BaseHapiFhirDao;
+import ca.uhn.fhir.jpa.dao.DaoConfig;
+import ca.uhn.fhir.jpa.subscription.resthook.SubscriptionRestHookInterceptor;
+import ca.uhn.fhir.rest.server.exceptions.UnprocessableEntityException;
+import ca.uhn.fhir.util.TestUtil;
+import org.hl7.fhir.instance.model.api.IIdType;
+import org.hl7.fhir.r4.model.Subscription;
+import org.junit.After;
+import org.junit.AfterClass;
+import org.junit.Test;
+import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.transaction.TransactionStatus;
+import org.springframework.transaction.support.TransactionCallbackWithoutResult;
+import org.springframework.transaction.support.TransactionTemplate;
+
+import javax.persistence.Query;
+
+import static org.junit.Assert.*;
+
+public class FhirResourceDaoR4InvalidSubscriptionTest extends BaseJpaR4Test {
+
+	@Autowired
+	private SubscriptionRestHookInterceptor myInterceptor;
+
+	@After
+	public void afterResetDao() {
+		myDaoConfig.setResourceServerIdStrategy(new DaoConfig().getResourceServerIdStrategy());
+		BaseHapiFhirDao.setValidationDisabledForUnitTest(false);
+	}
+
+	@Test
+	public void testCreateInvalidSubscriptionOkButCanNotActivate() {
+		Subscription s = new Subscription();
+		s.setStatus(Subscription.SubscriptionStatus.OFF);
+		s.setCriteria("FOO");
+		IIdType id = mySubscriptionDao.create(s).getId().toUnqualified();
+
+		s = mySubscriptionDao.read(id);
+		assertEquals("FOO", s.getCriteria());
+
+		s.setStatus(Subscription.SubscriptionStatus.REQUESTED);
+		try {
+			mySubscriptionDao.update(s);
+			fail();
+		} catch (UnprocessableEntityException e) {
+			assertEquals("Subscription.criteria must be in the form \"{Resource Type}?[params]", e.getMessage());
+		}
+	}
+
+	/**
+	 * Make sure that bad data in the database doesn't prevent startup
+	 */
+	@Test
+	public void testSubscriptionMarkedDeleted() {
+		BaseHapiFhirDao.setValidationDisabledForUnitTest(true);
+
+		Subscription s = new Subscription();
+		s.setStatus(Subscription.SubscriptionStatus.REQUESTED);
+		s.getChannel().setEndpoint("http://foo");
+		s.getChannel().setPayload("application/fhir+json");
+		s.setCriteria("Patient?foo");
+		final IIdType id = mySubscriptionDao.create(s).getId().toUnqualifiedVersionless();
+		assertNotNull(id.getIdPart());
+
+		BaseHapiFhirDao.setValidationDisabledForUnitTest(false);
+
+		new TransactionTemplate(myTransactionMgr).execute(new TransactionCallbackWithoutResult() {
+			@Override
+			protected void doInTransactionWithoutResult(TransactionStatus status) {
+				Query q = myEntityManager.createNativeQuery("UPDATE HFJ_RESOURCE SET RES_DELETED_AT = RES_UPDATED WHERE RES_ID = " + id.getIdPart());
+				q.executeUpdate();
+			}
+		});
+
+		myEntityManager.clear();
+
+		myInterceptor.start();
+	}
+
+	/**
+	 * Make sure that bad data in the database doesn't prevent startup
+	 */
+	@Test
+	public void testSubscriptionWithInvalidCriteria() throws InterruptedException {
+		BaseHapiFhirDao.setValidationDisabledForUnitTest(true);
+
+		Subscription s = new Subscription();
+		s.setStatus(Subscription.SubscriptionStatus.REQUESTED);
+		s.getChannel().setType(Subscription.SubscriptionChannelType.RESTHOOK);
+		s.getChannel().setEndpoint("http://foo");
+		s.getChannel().setPayload("application/fhir+json");
+		s.setCriteria("BLAH");
+		IIdType id = mySubscriptionDao.create(s).getId().toUnqualifiedVersionless();
+		assertNotNull(id.getIdPart());
+
+		BaseHapiFhirDao.setValidationDisabledForUnitTest(false);
+
+		myInterceptor.start();
+
+	}
+
+	/**
+	 * Make sure that bad data in the database doesn't prevent startup
+	 */
+	@Test
+	public void testSubscriptionWithNoStatus() {
+		BaseHapiFhirDao.setValidationDisabledForUnitTest(true);
+
+		Subscription s = new Subscription();
+		s.getChannel().setType(Subscription.SubscriptionChannelType.RESTHOOK);
+		s.getChannel().setEndpoint("http://foo");
+		s.getChannel().setPayload("application/fhir+json");
+		s.setCriteria("Patient?active=true");
+		IIdType id = mySubscriptionDao.create(s).getId().toUnqualifiedVersionless();
+
+		BaseHapiFhirDao.setValidationDisabledForUnitTest(false);
+
+		myInterceptor.start();
+
+	}
+
+	/**
+	 * Make sure that bad data in the database doesn't prevent startup
+	 */
+	@Test
+	public void testSubscriptionWithNoType() {
+		BaseHapiFhirDao.setValidationDisabledForUnitTest(true);
+
+		Subscription s = new Subscription();
+		s.setStatus(Subscription.SubscriptionStatus.REQUESTED);
+		s.getChannel().setEndpoint("http://foo");
+		s.getChannel().setPayload("application/fhir+json");
+		s.setCriteria("Patient?foo");
+		IIdType id = mySubscriptionDao.create(s).getId().toUnqualifiedVersionless();
+		assertNotNull(id.getIdPart());
+
+		BaseHapiFhirDao.setValidationDisabledForUnitTest(false);
+
+		myInterceptor.start();
+
+	}
+
+	@AfterClass
+	public static void afterClassClearContext() {
+		TestUtil.clearAllStaticFieldsForUnitTest();
+	}
+
+}

hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/ResponseHighlighterInterceptor.java

@@ -318,7 +318,13 @@ public class ResponseHighlighterInterceptor extends InterceptorAdapter {
 		boolean force = false;
 		String[] formatParams = theRequestDetails.getParameters().get(Constants.PARAM_FORMAT);
 		if (formatParams != null && formatParams.length > 0) {
-			String formatParam = formatParams[0];
+			String formatParam = defaultString(formatParams[0]);
+			int semiColonIdx = formatParam.indexOf(';');
+			if (semiColonIdx != -1) {
+				formatParam = formatParam.substring(0, semiColonIdx);
+			}
+			formatParam = trim(formatParam);
+
 			if (Constants.FORMATS_HTML.contains(formatParam)) { // this is a set
 				force = true;
 			} else if (Constants.FORMATS_HTML_XML.equals(formatParam)) {

hapi-fhir-structures-dstu2/src/test/java/ca/uhn/fhir/rest/server/interceptor/ResponseHighlightingInterceptorTest.java

@@ -1,29 +1,34 @@
 package ca.uhn.fhir.rest.server.interceptor;
 
-import static org.hamcrest.Matchers.containsString;
+import ca.uhn.fhir.context.FhirContext;
-import static org.hamcrest.Matchers.containsStringIgnoringCase;
+import ca.uhn.fhir.context.api.BundleInclusionRule;
-import static org.hamcrest.Matchers.matchesPattern;
+import ca.uhn.fhir.model.api.IResource;
-import static org.hamcrest.Matchers.not;
+import ca.uhn.fhir.model.dstu2.composite.HumanNameDt;
-import static org.hamcrest.Matchers.stringContainsInOrder;
+import ca.uhn.fhir.model.dstu2.composite.IdentifierDt;
-import static org.junit.Assert.assertArrayEquals;
+import ca.uhn.fhir.model.dstu2.resource.Binary;
-import static org.junit.Assert.assertEquals;
+import ca.uhn.fhir.model.dstu2.resource.OperationOutcome;
-import static org.junit.Assert.assertFalse;
+import ca.uhn.fhir.model.dstu2.resource.OperationOutcome.Issue;
-import static org.junit.Assert.assertNull;
+import ca.uhn.fhir.model.dstu2.resource.Organization;
-import static org.junit.Assert.assertThat;
+import ca.uhn.fhir.model.dstu2.resource.Patient;
-import static org.junit.Assert.assertTrue;
+import ca.uhn.fhir.model.dstu2.valueset.IdentifierUseEnum;
-import static org.mockito.Mockito.mock;
+import ca.uhn.fhir.model.primitive.IdDt;
-import static org.mockito.Mockito.when;
+import ca.uhn.fhir.model.primitive.UriDt;
-
+import ca.uhn.fhir.rest.annotation.IdParam;
-import java.io.PrintWriter;
+import ca.uhn.fhir.rest.annotation.Read;
-import java.io.StringWriter;
+import ca.uhn.fhir.rest.annotation.RequiredParam;
-import java.nio.charset.StandardCharsets;
+import ca.uhn.fhir.rest.annotation.Search;
-import java.util.*;
+import ca.uhn.fhir.rest.api.Constants;
-import java.util.concurrent.TimeUnit;
+import ca.uhn.fhir.rest.api.EncodingEnum;
-
+import ca.uhn.fhir.rest.api.RequestTypeEnum;
-import javax.servlet.http.HttpServletRequest;
-import javax.servlet.http.HttpServletResponse;
-
 import ca.uhn.fhir.rest.api.server.ResponseDetails;
+import ca.uhn.fhir.rest.server.IResourceProvider;
+import ca.uhn.fhir.rest.server.RestfulServer;
+import ca.uhn.fhir.rest.server.exceptions.ResourceNotFoundException;
+import ca.uhn.fhir.rest.server.servlet.ServletRequestDetails;
+import ca.uhn.fhir.util.PortUtil;
+import ca.uhn.fhir.util.TestUtil;
+import ca.uhn.fhir.util.UrlUtil;
+import com.phloc.commons.collections.iterate.ArrayEnumeration;
 import org.apache.commons.io.IOUtils;
 import org.apache.http.HttpResponse;
 import org.apache.http.client.methods.CloseableHttpResponse;
@@ -34,30 +39,26 @@ import org.apache.http.impl.conn.PoolingHttpClientConnectionManager;
 import org.eclipse.jetty.server.Server;
 import org.eclipse.jetty.servlet.ServletHandler;
 import org.eclipse.jetty.servlet.ServletHolder;
-import org.junit.*;
+import org.junit.AfterClass;
+import org.junit.Before;
+import org.junit.BeforeClass;
 import org.junit.Test;
 import org.mockito.invocation.InvocationOnMock;
 import org.mockito.stubbing.Answer;
 import org.springframework.web.cors.CorsConfiguration;
 
-import com.phloc.commons.collections.iterate.ArrayEnumeration;
+import javax.servlet.http.HttpServletRequest;
+import javax.servlet.http.HttpServletResponse;
+import java.io.PrintWriter;
+import java.io.StringWriter;
+import java.nio.charset.StandardCharsets;
+import java.util.*;
+import java.util.concurrent.TimeUnit;
 
-import ca.uhn.fhir.context.FhirContext;
+import static org.hamcrest.Matchers.*;
-import ca.uhn.fhir.context.api.BundleInclusionRule;
+import static org.junit.Assert.*;
-import ca.uhn.fhir.model.api.IResource;
+import static org.mockito.Mockito.mock;
-import ca.uhn.fhir.model.dstu2.composite.HumanNameDt;
+import static org.mockito.Mockito.when;
-import ca.uhn.fhir.model.dstu2.composite.IdentifierDt;
-import ca.uhn.fhir.model.dstu2.resource.*;
-import ca.uhn.fhir.model.dstu2.resource.OperationOutcome.Issue;
-import ca.uhn.fhir.model.dstu2.valueset.IdentifierUseEnum;
-import ca.uhn.fhir.model.primitive.IdDt;
-import ca.uhn.fhir.model.primitive.UriDt;
-import ca.uhn.fhir.rest.annotation.*;
-import ca.uhn.fhir.rest.api.*;
-import ca.uhn.fhir.rest.server.*;
-import ca.uhn.fhir.rest.server.exceptions.ResourceNotFoundException;
-import ca.uhn.fhir.rest.server.servlet.ServletRequestDetails;
-import ca.uhn.fhir.util.*;
 
 public class ResponseHighlightingInterceptorTest {
 
@@ -88,7 +89,7 @@ public class ResponseHighlightingInterceptorTest {
 		assertEquals(200, status.getStatusLine().getStatusCode());
 		assertEquals("foo", status.getFirstHeader("content-type").getValue());
 		assertEquals("Attachment;", status.getFirstHeader("Content-Disposition").getValue());
-		assertArrayEquals(new byte[] { 1, 2, 3, 4 }, responseContent);
+		assertArrayEquals(new byte[]{1, 2, 3, 4}, responseContent);
 	}
 
 	@Test
@@ -117,7 +118,7 @@ public class ResponseHighlightingInterceptorTest {
 		assertEquals(200, status.getStatusLine().getStatusCode());
 		assertEquals("foo", status.getFirstHeader("content-type").getValue());
 		assertEquals("Attachment;", status.getFirstHeader("Content-Disposition").getValue());
-		assertArrayEquals(new byte[] { 1, 2, 3, 4 }, responseContent);
+		assertArrayEquals(new byte[]{1, 2, 3, 4}, responseContent);
 
 	}
 
@@ -253,6 +254,23 @@ public class ResponseHighlightingInterceptorTest {
 		ourLog.info(responseContent);
 	}
 
+	@Test
+	public void testForceHtmlJsonWithAdditionalParts() throws Exception {
+		HttpGet httpGet = new HttpGet("http://localhost:" + ourPort + "/Patient/1?_format=" + UrlUtil.escapeUrlParam("html/json; fhirVersion=1.0"));
+		httpGet.addHeader("User-Agent", "Mozilla/5.0 (Windows NT 6.1; WOW64; rv:40.0) Gecko/20100101 Firefox/40.1");
+
+		HttpResponse status = ourClient.execute(httpGet);
+		String responseContent = IOUtils.toString(status.getEntity().getContent(), StandardCharsets.UTF_8);
+		IOUtils.closeQuietly(status.getEntity().getContent());
+		assertEquals(200, status.getStatusLine().getStatusCode());
+		assertEquals("text/html;charset=utf-8", status.getFirstHeader("content-type").getValue().replace(" ", "").toLowerCase());
+		assertThat(responseContent, containsString("html"));
+		assertThat(responseContent, containsString(">{<"));
+		assertThat(responseContent, not(containsString("&lt;")));
+
+		ourLog.info(responseContent);
+	}
+
 	@Test
 	public void testForceHtmlXml() throws Exception {
 		HttpGet httpGet = new HttpGet("http://localhost:" + ourPort + "/Patient/1?_format=html/xml");
@@ -309,7 +327,7 @@ public class ResponseHighlightingInterceptorTest {
 		assertThat(responseContent, stringContainsInOrder("<span class='hlTagName'>OperationOutcome</span>", "Unknown resource type 'Foobar' - Server knows how to handle"));
 
 	}
-	
+
 	@Test
 	public void testGetInvalidResourceNoAcceptHeader() throws Exception {
 		HttpGet httpGet = new HttpGet("http://localhost:" + ourPort + "/Foobar/123");
@@ -403,7 +421,7 @@ public class ResponseHighlightingInterceptorTest {
 		ServletRequestDetails reqDetails = new TestServletRequestDetails();
 		reqDetails.setRequestType(RequestTypeEnum.GET);
 		HashMap<String, String[]> params = new HashMap<String, String[]>();
-		params.put(Constants.PARAM_FORMAT, new String[] { Constants.FORMAT_HTML });
+		params.put(Constants.PARAM_FORMAT, new String[]{Constants.FORMAT_HTML});
 		reqDetails.setParameters(params);
 		reqDetails.setServer(new RestfulServer(ourCtx));
 		reqDetails.setServletRequest(req);
@@ -437,7 +455,7 @@ public class ResponseHighlightingInterceptorTest {
 		ServletRequestDetails reqDetails = new TestServletRequestDetails();
 		reqDetails.setRequestType(RequestTypeEnum.GET);
 		HashMap<String, String[]> params = new HashMap<String, String[]>();
-		params.put(Constants.PARAM_FORMAT, new String[] { Constants.CT_HTML });
+		params.put(Constants.PARAM_FORMAT, new String[]{Constants.CT_HTML});
 		reqDetails.setParameters(params);
 		reqDetails.setServer(new RestfulServer(ourCtx));
 		reqDetails.setServletRequest(req);
@@ -468,9 +486,9 @@ public class ResponseHighlightingInterceptorTest {
 		ServletRequestDetails reqDetails = new TestServletRequestDetails();
 		reqDetails.setRequestType(RequestTypeEnum.GET);
 		HashMap<String, String[]> params = new HashMap<String, String[]>();
-		params.put(Constants.PARAM_PRETTY, new String[] { Constants.PARAM_PRETTY_VALUE_TRUE });
+		params.put(Constants.PARAM_PRETTY, new String[]{Constants.PARAM_PRETTY_VALUE_TRUE});
-		params.put(Constants.PARAM_FORMAT, new String[] { Constants.CT_XML });
+		params.put(Constants.PARAM_FORMAT, new String[]{Constants.CT_XML});
-		params.put(ResponseHighlighterInterceptor.PARAM_RAW, new String[] { ResponseHighlighterInterceptor.PARAM_RAW_TRUE });
+		params.put(ResponseHighlighterInterceptor.PARAM_RAW, new String[]{ResponseHighlighterInterceptor.PARAM_RAW_TRUE});
 		reqDetails.setParameters(params);
 		reqDetails.setServer(new RestfulServer(ourCtx));
 		reqDetails.setServletRequest(req);
@@ -536,7 +554,7 @@ public class ResponseHighlightingInterceptorTest {
 		ServletRequestDetails reqDetails = new TestServletRequestDetails();
 		reqDetails.setRequestType(RequestTypeEnum.GET);
 		HashMap<String, String[]> params = new HashMap<String, String[]>();
-		params.put(Constants.PARAM_PRETTY, new String[] { Constants.PARAM_PRETTY_VALUE_TRUE });
+		params.put(Constants.PARAM_PRETTY, new String[]{Constants.PARAM_PRETTY_VALUE_TRUE});
 		reqDetails.setParameters(params);
 		reqDetails.setServer(new RestfulServer(ourCtx));
 		reqDetails.setServletRequest(req);
@@ -586,7 +604,7 @@ public class ResponseHighlightingInterceptorTest {
 		ourLog.info(output);
 		assertThat(output, containsString("resourceType"));
 	}
-	
+
 	@Test
 	public void testHighlightProducesDefaultJsonWithBrowserRequest2() throws Exception {
 		ResponseHighlighterInterceptor ic = ourInterceptor;
@@ -702,7 +720,7 @@ public class ResponseHighlightingInterceptorTest {
 		assertThat(responseContent, not(containsStringIgnoringCase("Accept")));
 		assertThat(responseContent, not(containsStringIgnoringCase("Content-Type")));
 	}
-	
+
 	@Test
 	public void testShowRequest() throws Exception {
 		ourInterceptor.setShowRequestHeaders(true);
@@ -719,7 +737,7 @@ public class ResponseHighlightingInterceptorTest {
 		assertThat(responseContent, (containsStringIgnoringCase("Accept")));
 		assertThat(responseContent, not(containsStringIgnoringCase("Content-Type")));
 	}
-	
+
 	@Test
 	public void testShowRequestAndResponse() throws Exception {
 		ourInterceptor.setShowRequestHeaders(true);
@@ -768,7 +786,7 @@ public class ResponseHighlightingInterceptorTest {
 
 		ServletHandler proxyHandler = new ServletHandler();
 		ourServlet = new RestfulServer(ourCtx);
-		
+
 		/*
 		 * Enable CORS
 		 */
@@ -783,15 +801,15 @@ public class ResponseHighlightingInterceptorTest {
 		config.addAllowedOrigin("*");
 		config.addExposedHeader("Location");
 		config.addExposedHeader("Content-Location");
-		config.setAllowedMethods(Arrays.asList("GET","POST","PUT","DELETE","OPTIONS"));
+		config.setAllowedMethods(Arrays.asList("GET", "POST", "PUT", "DELETE", "OPTIONS"));
 		ourServlet.registerInterceptor(corsInterceptor);
-		
+
 		ourServlet.registerInterceptor(ourInterceptor);
 		ourServlet.setResourceProviders(patientProvider, new DummyBinaryResourceProvider());
 		ourServlet.setBundleInclusionRule(BundleInclusionRule.BASED_ON_RESOURCE_PRESENCE);
 		ServletHolder servletHolder = new ServletHolder(ourServlet);
 		proxyHandler.addServletWithMapping(servletHolder, "/*");
-		
+
 		ourServer.setHandler(proxyHandler);
 		ourServer.start();
 
@@ -801,7 +819,7 @@ public class ResponseHighlightingInterceptorTest {
 		ourClient = builder.build();
 
 	}
-	
+
 	class TestServletRequestDetails extends ServletRequestDetails {
 		@Override
 		public String getServerBaseForRequest() {
@@ -820,7 +838,7 @@ public class ResponseHighlightingInterceptorTest {
 		public Binary read(@IdParam IdDt theId) {
 			Binary retVal = new Binary();
 			retVal.setId("1");
-			retVal.setContent(new byte[] { 1, 2, 3, 4 });
+			retVal.setContent(new byte[]{1, 2, 3, 4});
 			retVal.setContentType(theId.getIdPart());
 			return retVal;
 		}
@@ -829,7 +847,7 @@ public class ResponseHighlightingInterceptorTest {
 		public List<Binary> search() {
 			Binary retVal = new Binary();
 			retVal.setId("1");
-			retVal.setContent(new byte[] { 1, 2, 3, 4 });
+			retVal.setContent(new byte[]{1, 2, 3, 4});
 			retVal.setContentType("text/plain");
 			return Collections.singletonList(retVal);
 		}
@@ -895,8 +913,7 @@ public class ResponseHighlightingInterceptorTest {
 		/**
 		 * Retrieve the resource by its identifier
 		 *
-		 * @param theId
+		 * @param theId The resource identity
-		 *           The resource identity
 		 * @return The resource
 		 */
 		@Read()
@@ -909,8 +926,7 @@ public class ResponseHighlightingInterceptorTest {
 		/**
 		 * Retrieve the resource by its identifier
 		 *
-		 * @param theId
+		 * @param theId The resource identity
-		 *           The resource identity
 		 * @return The resource
 		 */
 		@Search()

src/changes/changes.xml

@@ -104,6 +104,16 @@
 				caused an exception if the client made a request with
 				no count parameter included. Thanks to Viviana Sanz for reporting!
 			</action>
+			<action type="fix">
+				A bug in the JPA server was fixed where a Subscription incorrectly created 
+				without a status or with invalid criteria would cause a crash during 
+				startup.
+			</action>
+			<action type="add">
+				ResponseHighlightingInterceptor now properly parses _format 
+				parameters that include additional content (e.g.
+				<![CDATA[<code>_format=html/json;fhirVersion=1.0</code>]]>)
+			</action>
 		</release>
 		<release version="3.2.0" date="2018-01-13">
 			<action type="add">