Subscription logic cleanup

hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/SearchBuilder.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.

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

@@ -30,9 +30,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.

hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/entity/Search.java

@@ -19,9 +19,9 @@ import static org.apache.commons.lang3.StringUtils.left;
  * 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/BaseSubscriptionInterceptor.java

@@ -53,7 +53,6 @@ import org.slf4j.LoggerFactory;
 import org.springframework.beans.factory.DisposableBean;
 import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.beans.factory.annotation.Qualifier;
-import org.springframework.context.SmartLifecycle;
 import org.springframework.core.task.AsyncTaskExecutor;
 import org.springframework.messaging.MessageChannel;
 import org.springframework.messaging.MessageHandler;
@@ -378,7 +377,7 @@ public abstract class BaseSubscriptionInterceptor<S extends IBaseResource> exten
 			for (IBaseResource resource : resourceList) {
 				String nextId = resource.getIdElement().getIdPart();
 				allIds.add(nextId);
-				mySubscriptionActivatingSubscriber.activateAndRegisterSubscriptionIfRequired(resource);
+				mySubscriptionActivatingSubscriber.activateOrRegisterSubscriptionIfRequired(resource);
 			}
 
 			unregisterAllSubscriptionsNotInCollection(allIds);

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

@@ -70,8 +70,7 @@ public class SubscriptionActivatingSubscriber {
 		Validate.notNull(theTaskExecutor);
 	}
 
-	public void activateAndRegisterSubscriptionIfRequired(final IBaseResource theSubscription) {
-
+	public synchronized void activateOrRegisterSubscriptionIfRequired(final IBaseResource theSubscription) {
 		// Grab the value for "Subscription.channel.type" so we can see if this
 		// subscriber applies..
 		String subscriptionChannelType = myCtx
@@ -128,21 +127,25 @@ public class SubscriptionActivatingSubscriber {
 		} else if (activeStatus.equals(statusString)) {
 			registerSubscriptionUnlessAlreadyRegistered(theSubscription);
 		} else {
-			if (mySubscriptionInterceptor.hasSubscription(theSubscription.getIdElement())) {
-				ourLog.info("Removing {} subscription {}", statusString, theSubscription.getIdElement().toUnqualified().getValue());
-				mySubscriptionInterceptor.unregisterSubscription(theSubscription.getIdElement());
-			}
+			// Status isn't "active" or "requested"
+			unregisterSubscriptionIfRegistered(theSubscription, statusString);
+		}
+	}
+
+	protected void unregisterSubscriptionIfRegistered(IBaseResource theSubscription, String theStatusString) {
+		if (mySubscriptionInterceptor.hasSubscription(theSubscription.getIdElement())) {
+			ourLog.info("Removing {} subscription {}", theStatusString, theSubscription.getIdElement().toUnqualified().getValue());
+			mySubscriptionInterceptor.unregisterSubscription(theSubscription.getIdElement());
 		}
 	}
 
 	private void activateSubscription(String theActiveStatus, final IBaseResource theSubscription, String theRequestedStatus) {
 		IBaseResource subscription = mySubscriptionDao.read(theSubscription.getIdElement());
 
-		ourLog.info("Activating and registering subscription {} from status {} to {} for channel {}", subscription.getIdElement().toUnqualified().getValue(), theRequestedStatus, theActiveStatus, myChannelType);
+		ourLog.info("Activating and subscription {} from status {} to {} for channel {}", subscription.getIdElement().toUnqualified().getValue(), theRequestedStatus, theActiveStatus, myChannelType);
 		try {
 			SubscriptionUtil.setStatus(myCtx, subscription, theActiveStatus);
 			mySubscriptionDao.update(subscription);
-			registerSubscriptionUnlessAlreadyRegistered(subscription);
 		} catch (final UnprocessableEntityException e) {
 			ourLog.info("Changing status of {} to ERROR", subscription.getIdElement());
 			SubscriptionUtil.setStatus(myCtx, subscription, "error");
@@ -151,6 +154,7 @@ public class SubscriptionActivatingSubscriber {
 		}
 	}
 
+	@SuppressWarnings("EnumSwitchStatementWhichMissesCases")
 	public void handleMessage(RestOperationTypeEnum theOperationType, IIdType theId, final IBaseResource theSubscription) throws MessagingException {
 
 		switch (theOperationType) {
@@ -162,13 +166,7 @@ public class SubscriptionActivatingSubscriber {
 				if (!theId.getResourceType().equals("Subscription")) {
 					return;
 				}
-				TransactionTemplate txTemplate = new TransactionTemplate(myTransactionManager);
-				txTemplate.execute(new TransactionCallbackWithoutResult() {
-					@Override
-					protected void doInTransactionWithoutResult(TransactionStatus status) {
-						activateAndRegisterSubscriptionIfRequired(theSubscription);
-					}
-				});
+				activateAndRegisterSubscriptionIfRequiredInTransaction(theSubscription);
 				break;
 			default:
 				break;
@@ -176,6 +174,16 @@ public class SubscriptionActivatingSubscriber {
 
 	}
 
+	private synchronized void activateAndRegisterSubscriptionIfRequiredInTransaction(IBaseResource theSubscription) {
+		TransactionTemplate txTemplate = new TransactionTemplate(myTransactionManager);
+		txTemplate.execute(new TransactionCallbackWithoutResult() {
+			@Override
+			protected void doInTransactionWithoutResult(TransactionStatus status) {
+				activateOrRegisterSubscriptionIfRequired(theSubscription);
+			}
+		});
+	}
+
 	private void registerSubscriptionUnlessAlreadyRegistered(IBaseResource theSubscription) {
 		if (mySubscriptionInterceptor.hasSubscription(theSubscription.getIdElement())) {
 			ourLog.info("Updating already-registered active subscription {}", theSubscription.getIdElement().toUnqualified().getValue());