fixed bug in in-memory matching for reference id when searchparam has two components

This commit is contained in:
Ken Stevens 2019-02-08 16:37:08 -05:00
parent 52c62884fb
commit f40614a1aa
4 changed files with 83 additions and 37 deletions

View File

@ -7,6 +7,7 @@ import ca.uhn.fhir.jpa.provider.dstu3.BaseResourceProviderDstu3Test;
import ca.uhn.fhir.jpa.subscription.NotificationServlet; import ca.uhn.fhir.jpa.subscription.NotificationServlet;
import ca.uhn.fhir.jpa.subscription.SubscriptionTestUtil; import ca.uhn.fhir.jpa.subscription.SubscriptionTestUtil;
import ca.uhn.fhir.jpa.subscription.module.cache.SubscriptionConstants; import ca.uhn.fhir.jpa.subscription.module.cache.SubscriptionConstants;
import ca.uhn.fhir.jpa.subscription.module.interceptor.SubscriptionDebugLogInterceptor;
import ca.uhn.fhir.jpa.subscription.module.matcher.SubscriptionMatchingStrategy; import ca.uhn.fhir.jpa.subscription.module.matcher.SubscriptionMatchingStrategy;
import ca.uhn.fhir.rest.annotation.Create; import ca.uhn.fhir.rest.annotation.Create;
import ca.uhn.fhir.rest.annotation.ResourceParam; import ca.uhn.fhir.rest.annotation.ResourceParam;
@ -31,9 +32,10 @@ import javax.servlet.http.HttpServletRequest;
import java.util.ArrayList; import java.util.ArrayList;
import java.util.Collections; import java.util.Collections;
import java.util.List; import java.util.List;
import java.util.concurrent.CountDownLatch;
import java.util.concurrent.TimeUnit;
import static org.junit.Assert.assertEquals; import static org.junit.Assert.*;
import static org.junit.Assert.fail;
/** /**
* Test the rest-hook subscriptions * Test the rest-hook subscriptions
@ -54,12 +56,13 @@ public class RestHookTestDstu3Test extends BaseResourceProviderDstu3Test {
private SubscriptionTestUtil mySubscriptionTestUtil; private SubscriptionTestUtil mySubscriptionTestUtil;
private static NotificationServlet ourNotificationServlet; private static NotificationServlet ourNotificationServlet;
private static String ourNotificationListenerServer; private static String ourNotificationListenerServer;
private static CountDownLatch communicationRequestListenerLatch;
@After @After
public void afterUnregisterRestHookListener() { public void afterUnregisterRestHookListener() {
ourLog.info("**** Starting @After *****"); ourLog.info("**** Starting @After *****");
for (IIdType next : mySubscriptionIds){ for (IIdType next : mySubscriptionIds) {
ourClient.delete().resourceById(next).execute(); ourClient.delete().resourceById(next).execute();
} }
mySubscriptionIds.clear(); mySubscriptionIds.clear();
@ -72,11 +75,14 @@ public class RestHookTestDstu3Test extends BaseResourceProviderDstu3Test {
myDaoConfig.setAllowMultipleDelete(new DaoConfig().isAllowMultipleDelete()); myDaoConfig.setAllowMultipleDelete(new DaoConfig().isAllowMultipleDelete());
mySubscriptionTestUtil.unregisterSubscriptionInterceptor(); mySubscriptionTestUtil.unregisterSubscriptionInterceptor();
myInterceptorRegistry.clearAnonymousHookForUnitTest();
} }
@Before @Before
public void beforeRegisterRestHookListener() { public void beforeRegisterRestHookListener() {
mySubscriptionTestUtil.registerRestHookInterceptor(); mySubscriptionTestUtil.registerRestHookInterceptor();
SubscriptionDebugLogInterceptor interceptor = new SubscriptionDebugLogInterceptor();
myInterceptorRegistry.registerInterceptor(interceptor);
} }
@Before @Before
@ -112,7 +118,7 @@ public class RestHookTestDstu3Test extends BaseResourceProviderDstu3Test {
waitForQueueToDrain(); waitForQueueToDrain();
return (Subscription)methodOutcome.getResource(); return (Subscription) methodOutcome.getResource();
} }
private Observation sendObservation(String code, String system) { private Observation sendObservation(String code, String system) {
@ -201,7 +207,7 @@ public class RestHookTestDstu3Test extends BaseResourceProviderDstu3Test {
waitForSize(1, ourUpdatedObservations); waitForSize(1, ourUpdatedObservations);
assertEquals(Constants.CT_FHIR_JSON_NEW, ourContentTypes.get(0)); assertEquals(Constants.CT_FHIR_JSON_NEW, ourContentTypes.get(0));
} }
@Test @Test
public void testRestHookSubscriptionApplicationJson() throws Exception { public void testRestHookSubscriptionApplicationJson() throws Exception {
String payload = "application/json"; String payload = "application/json";
@ -435,7 +441,7 @@ public class RestHookTestDstu3Test extends BaseResourceProviderDstu3Test {
Subscription subscriptionActivated = ourClient.read().resource(Subscription.class).withId(subscriptionId.toUnqualifiedVersionless()).execute(); Subscription subscriptionActivated = ourClient.read().resource(Subscription.class).withId(subscriptionId.toUnqualifiedVersionless()).execute();
assertEquals(Subscription.SubscriptionStatus.ACTIVE, subscriptionActivated.getStatus()); assertEquals(Subscription.SubscriptionStatus.ACTIVE, subscriptionActivated.getStatus());
tags = subscriptionActivated.getMeta().getTag(); tags = subscriptionActivated.getMeta().getTag();
assertEquals(1, tags.size()); assertEquals(1, tags.size());
tag = tags.get(0); tag = tags.get(0);
assertEquals(SubscriptionConstants.EXT_SUBSCRIPTION_MATCHING_STRATEGY, tag.getSystem()); assertEquals(SubscriptionConstants.EXT_SUBSCRIPTION_MATCHING_STRATEGY, tag.getSystem());
@ -459,14 +465,32 @@ public class RestHookTestDstu3Test extends BaseResourceProviderDstu3Test {
Subscription subscription = ourClient.read().resource(Subscription.class).withId(subscriptionId.toUnqualifiedVersionless()).execute(); Subscription subscription = ourClient.read().resource(Subscription.class).withId(subscriptionId.toUnqualifiedVersionless()).execute();
assertEquals(Subscription.SubscriptionStatus.ACTIVE, subscription.getStatus()); assertEquals(Subscription.SubscriptionStatus.ACTIVE, subscription.getStatus());
tags = subscription.getMeta().getTag(); tags = subscription.getMeta().getTag();
assertEquals(1, tags.size()); assertEquals(1, tags.size());
tag = tags.get(0); tag = tags.get(0);
assertEquals(SubscriptionConstants.EXT_SUBSCRIPTION_MATCHING_STRATEGY, tag.getSystem()); assertEquals(SubscriptionConstants.EXT_SUBSCRIPTION_MATCHING_STRATEGY, tag.getSystem());
assertEquals(SubscriptionMatchingStrategy.DATABASE.toString(), tag.getCode()); assertEquals(SubscriptionMatchingStrategy.DATABASE.toString(), tag.getCode());
assertEquals("Database", tag.getDisplay()); assertEquals("Database", tag.getDisplay());
} }
@Test
public void testCommunicationRequestWithRef() throws InterruptedException {
Organization org = new Organization();
MethodOutcome methodOutcome = ourClient.create().resource(org).execute();
String orgId = methodOutcome.getId().getIdPart();
String criteria = "CommunicationRequest?requester=1276," + orgId + "&occurrence=ge2019-02-08T00:00:00-05:00&occurrence=le2019-02-09T00:00:00-05:00";
String payload = "application/fhir+xml";
createSubscription(criteria, payload, ourListenerServerBase);
CommunicationRequest cr = new CommunicationRequest();
cr.getRequester().getAgent().setReference("Organization/" + orgId);
cr.setOccurrence(new DateTimeType("2019-02-08T00:01:00-05:00"));
communicationRequestListenerLatch = new CountDownLatch(1);
ourClient.create().resource(cr).execute();
assertTrue("Timed out waiting for subscription to match", communicationRequestListenerLatch.await(10, TimeUnit.SECONDS));
}
@BeforeClass @BeforeClass
public static void startListenerServer() throws Exception { public static void startListenerServer() throws Exception {
ourListenerPort = PortUtil.findFreePort(); ourListenerPort = PortUtil.findFreePort();
@ -475,7 +499,8 @@ public class RestHookTestDstu3Test extends BaseResourceProviderDstu3Test {
ourNotificationListenerServer = "http://localhost:" + ourListenerPort + "/fhir/subscription"; ourNotificationListenerServer = "http://localhost:" + ourListenerPort + "/fhir/subscription";
ObservationListener obsListener = new ObservationListener(); ObservationListener obsListener = new ObservationListener();
ourListenerRestServer.setResourceProviders(obsListener); CommunicationRequestListener crListener = new CommunicationRequestListener();
ourListenerRestServer.setResourceProviders(obsListener, crListener);
ourListenerServer = new Server(ourListenerPort); ourListenerServer = new Server(ourListenerPort);
ourNotificationServlet = new NotificationServlet(); ourNotificationServlet = new NotificationServlet();
@ -505,7 +530,7 @@ public class RestHookTestDstu3Test extends BaseResourceProviderDstu3Test {
public MethodOutcome create(@ResourceParam Observation theObservation, HttpServletRequest theRequest) { public MethodOutcome create(@ResourceParam Observation theObservation, HttpServletRequest theRequest) {
ourLog.info("Received Listener Create"); ourLog.info("Received Listener Create");
ourContentTypes.add(theRequest.getHeader(Constants.HEADER_CONTENT_TYPE).replaceAll(";.*", "")); ourContentTypes.add(theRequest.getHeader(Constants.HEADER_CONTENT_TYPE).replaceAll(";.*", ""));
ourCreatedObservations.add(theObservation); ourCreatedObservations.add((Observation) theObservation);
return new MethodOutcome(new IdType("Observation/1"), true); return new MethodOutcome(new IdType("Observation/1"), true);
} }
@ -521,7 +546,27 @@ public class RestHookTestDstu3Test extends BaseResourceProviderDstu3Test {
ourLog.info("Received Listener Update (now have {} updates)", ourUpdatedObservations.size()); ourLog.info("Received Listener Update (now have {} updates)", ourUpdatedObservations.size());
return new MethodOutcome(new IdType("Observation/1"), false); return new MethodOutcome(new IdType("Observation/1"), false);
} }
} }
public static class CommunicationRequestListener implements IResourceProvider {
@Create
public MethodOutcome create(@ResourceParam CommunicationRequest theResource, HttpServletRequest theRequest) {
ourLog.info("Received CommunicationRequestListener Create");
communicationRequestListenerLatch.countDown();
return new MethodOutcome(new IdType("CommunicationRequest/1"), true);
}
@Override
public Class<? extends IBaseResource> getResourceType() {
return CommunicationRequest.class;
}
@Update
public MethodOutcome update(@ResourceParam CommunicationRequest theResource, HttpServletRequest theRequest) {
ourLog.info("Received CommunicationRequestListener Update");
communicationRequestListenerLatch.countDown();
return new MethodOutcome(new IdType("CommunicationRequest/1"), false);
}
}
} }

View File

@ -3,37 +3,26 @@ package ca.uhn.fhir.jpa.subscription.resthook;
import ca.uhn.fhir.jpa.config.StoppableSubscriptionDeliveringRestHookSubscriber; import ca.uhn.fhir.jpa.config.StoppableSubscriptionDeliveringRestHookSubscriber;
import ca.uhn.fhir.jpa.subscription.BaseSubscriptionsR4Test; import ca.uhn.fhir.jpa.subscription.BaseSubscriptionsR4Test;
import ca.uhn.fhir.jpa.subscription.module.cache.SubscriptionConstants; import ca.uhn.fhir.jpa.subscription.module.cache.SubscriptionConstants;
import ca.uhn.fhir.jpa.subscription.module.interceptor.SubscriptionDebugLogInterceptor;
import ca.uhn.fhir.rest.api.CacheControlDirective; import ca.uhn.fhir.rest.api.CacheControlDirective;
import ca.uhn.fhir.rest.api.Constants; import ca.uhn.fhir.rest.api.Constants;
import ca.uhn.fhir.rest.api.MethodOutcome; import ca.uhn.fhir.rest.api.MethodOutcome;
import ca.uhn.fhir.rest.server.exceptions.InvalidRequestException;
import ca.uhn.fhir.rest.server.exceptions.UnprocessableEntityException; import ca.uhn.fhir.rest.server.exceptions.UnprocessableEntityException;
import org.hl7.fhir.instance.model.api.IBaseBundle; import org.hl7.fhir.instance.model.api.IBaseBundle;
import org.hl7.fhir.r4.model.*; import org.hl7.fhir.r4.model.*;
import org.junit.After; import org.junit.After;
import org.junit.Assert; import org.junit.Assert;
import org.junit.Ignore;
import org.junit.Test; import org.junit.Test;
import org.mockito.ArgumentMatchers;
import org.slf4j.Logger; import org.slf4j.Logger;
import org.slf4j.LoggerFactory; import org.slf4j.LoggerFactory;
import org.slf4j.event.Level;
import org.slf4j.helpers.MessageFormatter;
import org.springframework.beans.factory.annotation.Autowired; import org.springframework.beans.factory.annotation.Autowired;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.List;
import java.util.concurrent.CountDownLatch; import java.util.concurrent.CountDownLatch;
import java.util.concurrent.TimeUnit; import java.util.concurrent.TimeUnit;
import java.util.stream.Collectors;
import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.hasItem; import static org.hamcrest.Matchers.hasItem;
import static org.hamcrest.Matchers.matchesPattern; import static org.hamcrest.Matchers.matchesPattern;
import static org.junit.Assert.*; import static org.junit.Assert.*;
import static org.mockito.Mockito.*;
/** /**
* Test the rest-hook subscriptions * Test the rest-hook subscriptions
@ -773,9 +762,7 @@ public class RestHookTestR4Test extends BaseSubscriptionsR4Test {
assertEquals(Constants.CT_FHIR_XML_NEW, ourContentTypes.get(0)); assertEquals(Constants.CT_FHIR_XML_NEW, ourContentTypes.get(0));
} }
// TODO: reenable
@Test @Test
@Ignore
public void testRestHookSubscriptionInvalidCriteria() throws Exception { public void testRestHookSubscriptionInvalidCriteria() throws Exception {
String payload = "application/xml"; String payload = "application/xml";
@ -784,8 +771,8 @@ public class RestHookTestR4Test extends BaseSubscriptionsR4Test {
try { try {
createSubscription(criteria1, payload); createSubscription(criteria1, payload);
fail(); fail();
} catch (InvalidRequestException e) { } catch (UnprocessableEntityException e) {
assertEquals("HTTP 400 Bad Request: Invalid criteria: Failed to parse match URL[Observation?codeeeee=SNOMED-CT] - Resource type Observation does not have a parameter with name: codeeeee", e.getMessage()); assertEquals("HTTP 422 Unprocessable Entity: Invalid subscription criteria submitted: Observation?codeeeee=SNOMED-CT Failed to parse match URL[Observation?codeeeee=SNOMED-CT] - Resource type Observation does not have a parameter with name: codeeeee", e.getMessage());
} }
} }

View File

@ -218,12 +218,12 @@ public final class ResourceIndexedSearchParams {
return populatedResourceLinkParameters; return populatedResourceLinkParameters;
} }
public boolean matchParam(String theResourceName, String theParamName, RuntimeSearchParam paramDef, IQueryParameterType theParam) { public boolean matchParam(String theResourceName, String theParamName, RuntimeSearchParam theParamDef, IQueryParameterType theParam) {
if (paramDef == null) { if (theParamDef == null) {
return false; return false;
} }
Collection<? extends BaseResourceIndexedSearchParam> resourceParams; Collection<? extends BaseResourceIndexedSearchParam> resourceParams;
switch (paramDef.getParamType()) { switch (theParamDef.getParamType()) {
case TOKEN: case TOKEN:
resourceParams = tokenParams; resourceParams = tokenParams;
break; break;
@ -243,7 +243,7 @@ public final class ResourceIndexedSearchParams {
resourceParams = dateParams; resourceParams = dateParams;
break; break;
case REFERENCE: case REFERENCE:
return matchResourceLinks(theResourceName, theParamName, theParam); return matchResourceLinks(theResourceName, theParamName, theParam, theParamDef.getPath());
case COMPOSITE: case COMPOSITE:
case HAS: case HAS:
case SPECIAL: case SPECIAL:
@ -260,11 +260,11 @@ public final class ResourceIndexedSearchParams {
return resourceParams.stream().anyMatch(namedParamPredicate); return resourceParams.stream().anyMatch(namedParamPredicate);
} }
private boolean matchResourceLinks(String theResourceName, String theParamName, IQueryParameterType theParam) { private boolean matchResourceLinks(String theResourceName, String theParamName, IQueryParameterType theParam, String theParamPath) {
ReferenceParam reference = (ReferenceParam)theParam; ReferenceParam reference = (ReferenceParam)theParam;
Predicate<ResourceLink> namedParamPredicate = resourceLink -> Predicate<ResourceLink> namedParamPredicate = resourceLink ->
resourceLinkMatches(theResourceName, resourceLink, theParamName) resourceLinkMatches(theResourceName, resourceLink, theParamName, theParamPath)
&& resourceIdMatches(resourceLink, reference); && resourceIdMatches(resourceLink, reference);
return links.stream().anyMatch(namedParamPredicate); return links.stream().anyMatch(namedParamPredicate);
@ -285,9 +285,9 @@ public final class ResourceIndexedSearchParams {
} }
} }
private boolean resourceLinkMatches(String theResourceName, ResourceLink theResourceLink, String theParamName) { private boolean resourceLinkMatches(String theResourceName, ResourceLink theResourceLink, String theParamName, String theParamPath) {
return theResourceLink.getTargetResource().getResourceType().equalsIgnoreCase(theParamName) || return theResourceLink.getTargetResource().getResourceType().equalsIgnoreCase(theParamName) ||
theResourceLink.getSourcePath().equalsIgnoreCase(theResourceName+"."+theParamName); theResourceLink.getSourcePath().equalsIgnoreCase(theParamPath);
} }
@Override @Override

View File

@ -541,7 +541,21 @@ public class InMemorySubscriptionMatcherTestR3 extends BaseSubscriptionDstu3Test
} }
} }
// These last two are covered by other tests above @Test
// String criteria = "ProcedureRequest?intent=original-order&category=Laboratory,Ancillary%20Orders,Hemodialysis&status=suspended,entered-in-error,cancelled"; public void testCommunicationRequestWithRefAndDate() {
// String criteria = "Observation?code=70965-9&context.type=IHD"; String criteria = "CommunicationRequest?requester=O1271,O1276&occurrence=ge2019-02-08T00:00:00-05:00&occurrence=le2019-02-09T00:00:00-05:00";
CommunicationRequest cr = new CommunicationRequest();
cr.getRequester().getAgent().setReference("Organization/O1276");
cr.setOccurrence(new DateTimeType("2019-02-08T00:01:00-05:00"));
assertUnsupported(cr, criteria);
}
@Test
public void testCommunicationRequestWithRef() {
String criteria = "CommunicationRequest?requester=O1271,O1276";
CommunicationRequest cr = new CommunicationRequest();
cr.getRequester().getAgent().setReference("Organization/O1276");
assertMatched(cr, criteria);
}
} }