fixed null subscription id (#1170)

* fixed null subscription id

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

@@ -21,6 +21,7 @@ package ca.uhn.fhir.jpa.subscription.dbmatcher;
  */
 
 import ca.uhn.fhir.jpa.dao.DaoConfig;
+import ca.uhn.fhir.jpa.subscription.module.CanonicalSubscription;
 import ca.uhn.fhir.jpa.subscription.module.ResourceModifiedMessage;
 import ca.uhn.fhir.jpa.subscription.module.matcher.ISubscriptionMatcher;
 import ca.uhn.fhir.jpa.subscription.module.matcher.SubscriptionMatchResult;
@@ -43,16 +44,16 @@ public class CompositeInMemoryDaoSubscriptionMatcher implements ISubscriptionMat
 	}
 
 	@Override
-	public SubscriptionMatchResult match(String criteria, ResourceModifiedMessage msg) {
+	public SubscriptionMatchResult match(CanonicalSubscription theSubscription, ResourceModifiedMessage theMsg) {
 		SubscriptionMatchResult result;
 		if (myDaoConfig.isEnableInMemorySubscriptionMatching()) {
-			result = myInMemorySubscriptionMatcher.match(criteria, msg);
+			result = myInMemorySubscriptionMatcher.match(theSubscription, theMsg);
 			if (!result.supported()) {
-				ourLog.info("Criteria {} not supported by InMemoryMatcher: {}.  Reverting to DatabaseMatcher", criteria, result.getUnsupportedReason());
+				ourLog.info("Criteria {} for Subscription {} not supported by InMemoryMatcher: {}.  Reverting to DatabaseMatcher", theSubscription.getCriteriaString(), theSubscription.getIdElementString(), result.getUnsupportedReason());
-				result = myDaoSubscriptionMatcher.match(criteria, msg);
+				result = myDaoSubscriptionMatcher.match(theSubscription, theMsg);
 			}
 		} else {
-			result = myDaoSubscriptionMatcher.match(criteria, msg);
+			result = myDaoSubscriptionMatcher.match(theSubscription, theMsg);
 		}
 		return result;
 	}

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

@@ -27,6 +27,7 @@ import ca.uhn.fhir.jpa.dao.IFhirResourceDao;
 import ca.uhn.fhir.jpa.provider.ServletSubRequestDetails;
 import ca.uhn.fhir.jpa.searchparam.MatchUrlService;
 import ca.uhn.fhir.jpa.searchparam.SearchParameterMap;
+import ca.uhn.fhir.jpa.subscription.module.CanonicalSubscription;
 import ca.uhn.fhir.jpa.subscription.module.ResourceModifiedMessage;
 import ca.uhn.fhir.jpa.subscription.module.matcher.ISubscriptionMatcher;
 import ca.uhn.fhir.jpa.subscription.module.matcher.SubscriptionMatchResult;
@@ -49,10 +50,11 @@ public class DaoSubscriptionMatcher implements ISubscriptionMatcher {
 	MatchUrlService myMatchUrlService;
 
 	@Override
-	public SubscriptionMatchResult match(String criteria, ResourceModifiedMessage msg) {
+	public SubscriptionMatchResult match(CanonicalSubscription theSubscription, ResourceModifiedMessage theMsg) {
-		IIdType id = msg.getId(myCtx);
+		IIdType id = theMsg.getId(myCtx);
 		String resourceType = id.getResourceType();
 		String resourceId = id.getIdPart();
+		String criteria = theSubscription.getCriteriaString();
 
 		// run the subscriptions query and look for matches, add the id as part of the criteria to avoid getting matches of previous resources rather than the recent resource
 		criteria += "&_id=" + resourceType + "/" + resourceId;

hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/subscription/module/matcher/InMemorySubscriptionMatcherTestR4.java

@@ -4,6 +4,7 @@ import ca.uhn.fhir.context.FhirContext;
 import ca.uhn.fhir.jpa.config.TestR4Config;
 import ca.uhn.fhir.jpa.model.entity.ResourceIndexedSearchParamString;
 import ca.uhn.fhir.jpa.searchparam.SearchParameterMap;
+import ca.uhn.fhir.jpa.subscription.module.CanonicalSubscription;
 import ca.uhn.fhir.jpa.subscription.module.ResourceModifiedMessage;
 import ca.uhn.fhir.model.api.TemporalPrecisionEnum;
 import ca.uhn.fhir.rest.api.server.IBundleProvider;
@@ -380,10 +381,13 @@ public class InMemorySubscriptionMatcherTestR4 {
 		params.add(Patient.SP_FAMILY, new StringParam("testSearchNameParam01Fam"));
 		try {
 			String criteria = params.toNormalizedQueryString(myContext);
+			CanonicalSubscription subscription = new CanonicalSubscription();
+			subscription.setCriteriaString(criteria);
+			subscription.setIdElement(new IdType("Subscription", 123L));
 			ResourceModifiedMessage msg = new ResourceModifiedMessage(myContext, patient, ResourceModifiedMessage.OperationTypeEnum.CREATE);
 			msg.setSubscriptionId("Subscription/123");
 			msg.setId(new IdType("Patient/ABC"));
-			SubscriptionMatchResult result = myInMemorySubscriptionMatcher.match(criteria, msg);
+			SubscriptionMatchResult result = myInMemorySubscriptionMatcher.match(subscription, msg);
 			fail();
 		} catch (InternalErrorException e){
 			assertEquals("Failure processing resource ID[Patient/ABC] for subscription ID[Subscription/123]: Invalid resource reference found at path[Patient.managingOrganization] - Does not contain resource type - urn:uuid:13720262-b392-465f-913e-54fb198ff954", e.getMessage());

hapi-fhir-jpaserver-subscription/src/main/java/ca/uhn/fhir/jpa/subscription/module/matcher/CriteriaResourceMatcher.java

@@ -57,7 +57,7 @@ public class CriteriaResourceMatcher {
 		}
 		searchParameterMap.clean();
 		if (searchParameterMap.getLastUpdated() != null) {
-			return new SubscriptionMatchResult(Constants.PARAM_LASTUPDATED, "Qualifiers not supported");
+			return new SubscriptionMatchResult(Constants.PARAM_LASTUPDATED, "Standard Parameters not supported");
 		}
 
 		for (Map.Entry<String, List<List<? extends IQueryParameterType>>> entry : searchParameterMap.entrySet()) {
@@ -79,7 +79,7 @@ public class CriteriaResourceMatcher {
 
 		if (hasQualifiers(theAndOrParams)) {
 
-			return new SubscriptionMatchResult(theParamName, "Qualifiers not supported.");
+			return new SubscriptionMatchResult(theParamName, "Standard Parameters not supported.");
 
 		}
 		if (hasPrefixes(theAndOrParams)) {

hapi-fhir-jpaserver-subscription/src/main/java/ca/uhn/fhir/jpa/subscription/module/matcher/ISubscriptionMatcher.java

@@ -20,8 +20,9 @@ package ca.uhn.fhir.jpa.subscription.module.matcher;
  * #L%
  */
 
+import ca.uhn.fhir.jpa.subscription.module.CanonicalSubscription;
 import ca.uhn.fhir.jpa.subscription.module.ResourceModifiedMessage;
 
 public interface ISubscriptionMatcher {
-	SubscriptionMatchResult match(String criteria, ResourceModifiedMessage msg);
+	SubscriptionMatchResult match(CanonicalSubscription subscription, ResourceModifiedMessage msg);
 }

hapi-fhir-jpaserver-subscription/src/main/java/ca/uhn/fhir/jpa/subscription/module/matcher/InMemorySubscriptionMatcher.java

@@ -26,6 +26,7 @@ import ca.uhn.fhir.jpa.model.entity.ResourceTable;
 import ca.uhn.fhir.jpa.searchparam.extractor.ResourceIndexedSearchParams;
 import ca.uhn.fhir.jpa.searchparam.extractor.ResourceLinkExtractor;
 import ca.uhn.fhir.jpa.searchparam.extractor.SearchParamExtractorService;
+import ca.uhn.fhir.jpa.subscription.module.CanonicalSubscription;
 import ca.uhn.fhir.jpa.subscription.module.ResourceModifiedMessage;
 import ca.uhn.fhir.rest.server.exceptions.InternalErrorException;
 import org.hl7.fhir.instance.model.api.IBaseResource;
@@ -45,11 +46,11 @@ public class InMemorySubscriptionMatcher implements ISubscriptionMatcher {
 	private InlineResourceLinkResolver myInlineResourceLinkResolver;
 
 	@Override
-	public SubscriptionMatchResult match(String criteria, ResourceModifiedMessage msg) {
+	public SubscriptionMatchResult match(CanonicalSubscription theSubscription, ResourceModifiedMessage theMsg) {
 		try {
-			return match(criteria, msg.getNewPayload(myContext));
+			return match(theSubscription.getCriteriaString(), theMsg.getNewPayload(myContext));
 		} catch (Exception e) {
-			throw new InternalErrorException("Failure processing resource ID[" + msg.getId(myContext) + "] for subscription ID[" + msg.getSubscriptionId() + "]: " + e.getMessage(), e);
+			throw new InternalErrorException("Failure processing resource ID[" + theMsg.getId(myContext) + "] for subscription ID[" + theSubscription.getIdElementString() + "]: " + e.getMessage(), e);
 		}
 	}
 

hapi-fhir-jpaserver-subscription/src/main/java/ca/uhn/fhir/jpa/subscription/module/matcher/SubscriptionMatchResult.java

@@ -43,14 +43,6 @@ public class SubscriptionMatchResult {
 		this.myMatcherShortName = theMatcherShortName;
 	}
 
-	public SubscriptionMatchResult(String theUnsupportedParameter, String theUnsupportedReason, String theMatcherShortName) {
-		this.myMatch = false;
-		this.mySupported = false;
-		this.myUnsupportedParameter = theUnsupportedParameter;
-		this.myUnsupportedReason = theUnsupportedReason;
-		this.myMatcherShortName = theMatcherShortName;
-	}
-
 	public boolean supported() {
 		return mySupported;
 	}

hapi-fhir-jpaserver-subscription/src/main/java/ca/uhn/fhir/jpa/subscription/module/subscriber/SubscriptionMatchingSubscriber.java

@@ -78,8 +78,7 @@ public class SubscriptionMatchingSubscriber implements MessageHandler {
 				return;
 		}
 
-		IIdType id = theMsg.getId(myFhirContext);
+		IIdType resourceId = theMsg.getId(myFhirContext);
-		String resourceType = id.getResourceType();
 
 		Collection<ActiveSubscription> subscriptions = mySubscriptionRegistry.getAll();
 
@@ -87,8 +86,7 @@ public class SubscriptionMatchingSubscriber implements MessageHandler {
 
 		for (ActiveSubscription nextActiveSubscription : subscriptions) {
 
-			String nextSubscriptionId = nextActiveSubscription.getIdElement(myFhirContext).toUnqualifiedVersionless().getValue();
+			String nextSubscriptionId = getId(nextActiveSubscription);
-			String nextCriteriaString = nextActiveSubscription.getCriteriaString();
 
 			if (isNotBlank(theMsg.getSubscriptionId())) {
 				if (!theMsg.getSubscriptionId().equals(nextSubscriptionId)) {
@@ -97,29 +95,16 @@ public class SubscriptionMatchingSubscriber implements MessageHandler {
 				}
 			}
 
-			if (StringUtils.isBlank(nextCriteriaString)) {
+			if (!validCriteria(nextActiveSubscription, resourceId)) {
 				continue;
 			}
 
-			// see if the criteria matches the created object
+			SubscriptionMatchResult matchResult = mySubscriptionMatcher.match(nextActiveSubscription.getSubscription(), theMsg);
-			ourLog.trace("Checking subscription {} for {} with criteria {}", nextSubscriptionId, resourceType, nextCriteriaString);
-			String criteriaResource = nextCriteriaString;
-			int index = criteriaResource.indexOf("?");
-			if (index != -1) {
-				criteriaResource = criteriaResource.substring(0, criteriaResource.indexOf("?"));
-			}
-
-			if (resourceType != null && nextCriteriaString != null && !criteriaResource.equals(resourceType)) {
-				ourLog.trace("Skipping subscription search for {} because it does not match the criteria {}", resourceType, nextCriteriaString);
-				continue;
-			}
-
-			SubscriptionMatchResult matchResult = mySubscriptionMatcher.match(nextCriteriaString, theMsg);
 			if (!matchResult.matched()) {
 				continue;
 			}
 
-			ourLog.info("Subscription {} was matched by resource {} using matcher {}", nextActiveSubscription.getSubscription().getIdElement(myFhirContext).getValue(), id.toUnqualifiedVersionless().getValue(), matchResult.matcherShortName());
+			ourLog.info("Subscription {} was matched by resource {} using matcher {}", nextActiveSubscription.getSubscription().getIdElement(myFhirContext).getValue(), resourceId.toUnqualifiedVersionless().getValue(), matchResult.matcherShortName());
 
 			ResourceDeliveryMessage deliveryMsg = new ResourceDeliveryMessage();
 			deliveryMsg.setPayload(myFhirContext, theMsg.getNewPayload(myFhirContext));
@@ -136,4 +121,33 @@ public class SubscriptionMatchingSubscriber implements MessageHandler {
 			}
 		}
 	}
+
+	private String getId(ActiveSubscription theActiveSubscription) {
+		return theActiveSubscription.getIdElement(myFhirContext).toUnqualifiedVersionless().getValue();
+	}
+
+	private boolean validCriteria(ActiveSubscription theActiveSubscription, IIdType theResourceId) {
+		String criteriaString = theActiveSubscription.getCriteriaString();
+		String subscriptionId = getId(theActiveSubscription);
+		String resourceType = theResourceId.getResourceType();
+
+		if (StringUtils.isBlank(criteriaString)) {
+			return false;
+		}
+
+		// see if the criteria matches the created object
+		ourLog.trace("Checking subscription {} for {} with criteria {}", subscriptionId, resourceType, criteriaString);
+		String criteriaResource = criteriaString;
+		int index = criteriaResource.indexOf("?");
+		if (index != -1) {
+			criteriaResource = criteriaResource.substring(0, criteriaResource.indexOf("?"));
+		}
+
+		if (resourceType != null && criteriaString != null && !criteriaResource.equals(resourceType)) {
+			ourLog.trace("Skipping subscription search for {} because it does not match the criteria {}", resourceType, criteriaString);
+			return false;
+		}
+
+		return true;
+	}
 }