Correctly handle missing search params with new indexing infrastructure

This commit is contained in:
James Agnew 2018-05-23 11:42:38 -04:00
parent 0c5c347db7
commit a3687e446e
7 changed files with 325 additions and 64 deletions

View File

@ -1,6 +1,7 @@
package ca.uhn.fhir.util; package ca.uhn.fhir.util;
import static org.apache.commons.lang3.StringUtils.defaultString; import static org.apache.commons.lang3.StringUtils.defaultString;
import static org.apache.commons.lang3.StringUtils.isBlank;
/* /*
* #%L * #%L
@ -365,6 +366,22 @@ public class FhirTerser {
for (String nextPath : nextParam.getPathsSplit()) { for (String nextPath : nextParam.getPathsSplit()) {
for (IBaseReference nextValue : getValues(theSource, nextPath, IBaseReference.class)) { for (IBaseReference nextValue : getValues(theSource, nextPath, IBaseReference.class)) {
String nextRef = nextValue.getReferenceElement().toUnqualifiedVersionless().getValue(); String nextRef = nextValue.getReferenceElement().toUnqualifiedVersionless().getValue();
/*
* If the reference isn't an explicit resource ID, but instead is just
* a resource object, we'll calculate its ID and treat the target
* as that.
*/
if (isBlank(nextRef) && nextValue.getResource() != null) {
IBaseResource nextTarget = nextValue.getResource();
IIdType nextTargetId = nextTarget.getIdElement().toUnqualifiedVersionless();
if (!nextTargetId.hasResourceType()) {
String resourceType = myContext.getResourceDefinition(nextTarget).getName();
nextTargetId.setParts(null, resourceType, nextTargetId.getIdPart(), null);
}
nextRef = nextTargetId.getValue();
}
if (wantRef.equals(nextRef)) { if (wantRef.equals(nextRef)) {
return true; return true;
} }

View File

@ -66,6 +66,7 @@ public class ResourceIndexedSearchParamDate extends BaseResourceIndexedSearchPar
* Constructor * Constructor
*/ */
public ResourceIndexedSearchParamDate() { public ResourceIndexedSearchParamDate() {
super();
} }
/** /**

View File

@ -200,10 +200,14 @@ public class ResourceIndexedSearchParamQuantity extends BaseResourceIndexedSearc
b.append("system", getSystem()); b.append("system", getSystem());
b.append("units", getUnits()); b.append("units", getUnits());
b.append("value", getValue()); b.append("value", getValue());
b.append("missing", isMissing());
return b.build(); return b.build();
} }
private static String toTruncatedString(BigDecimal theValue) { private static String toTruncatedString(BigDecimal theValue) {
if (theValue == null) {
return null;
}
return theValue.setScale(0, RoundingMode.FLOOR).toPlainString(); return theValue.setScale(0, RoundingMode.FLOOR).toPlainString();
} }

View File

@ -0,0 +1,164 @@
package ca.uhn.fhir.jpa.provider.r4;
import ca.uhn.fhir.context.FhirContext;
import ca.uhn.fhir.jpa.dao.DaoConfig;
import ca.uhn.fhir.jpa.dao.r4.BaseJpaR4Test;
import ca.uhn.fhir.jpa.provider.SystemProviderDstu2Test;
import ca.uhn.fhir.jpa.rp.r4.ObservationResourceProvider;
import ca.uhn.fhir.jpa.rp.r4.OrganizationResourceProvider;
import ca.uhn.fhir.jpa.rp.r4.PatientResourceProvider;
import ca.uhn.fhir.jpa.testutil.RandomServerPortProvider;
import ca.uhn.fhir.rest.api.Constants;
import ca.uhn.fhir.rest.api.EncodingEnum;
import ca.uhn.fhir.rest.client.api.IGenericClient;
import ca.uhn.fhir.rest.client.interceptor.SimpleRequestHeaderInterceptor;
import ca.uhn.fhir.rest.server.FifoMemoryPagingProvider;
import ca.uhn.fhir.rest.server.RestfulServer;
import ca.uhn.fhir.rest.server.exceptions.InvalidRequestException;
import ca.uhn.fhir.rest.server.exceptions.ResourceGoneException;
import ca.uhn.fhir.rest.server.interceptor.RequestValidatingInterceptor;
import ca.uhn.fhir.rest.server.interceptor.ResponseHighlighterInterceptor;
import ca.uhn.fhir.util.TestUtil;
import ca.uhn.fhir.validation.ResultSeverityEnum;
import org.apache.commons.io.IOUtils;
import org.apache.http.Header;
import org.apache.http.client.methods.CloseableHttpResponse;
import org.apache.http.client.methods.HttpGet;
import org.apache.http.client.methods.HttpPost;
import org.apache.http.entity.ContentType;
import org.apache.http.entity.StringEntity;
import org.apache.http.impl.client.CloseableHttpClient;
import org.apache.http.impl.client.HttpClientBuilder;
import org.apache.http.impl.conn.PoolingHttpClientConnectionManager;
import org.eclipse.jetty.server.Server;
import org.eclipse.jetty.servlet.ServletContextHandler;
import org.eclipse.jetty.servlet.ServletHolder;
import org.hl7.fhir.instance.model.api.IIdType;
import org.hl7.fhir.r4.hapi.validation.FhirInstanceValidator;
import org.hl7.fhir.r4.model.*;
import org.hl7.fhir.r4.model.Bundle.BundleType;
import org.hl7.fhir.r4.model.Bundle.HTTPVerb;
import org.hl7.fhir.r4.model.Enumerations.AdministrativeGender;
import org.junit.*;
import org.springframework.transaction.annotation.Propagation;
import org.springframework.transaction.annotation.Transactional;
import java.io.InputStream;
import java.nio.charset.StandardCharsets;
import java.util.concurrent.TimeUnit;
import static org.hamcrest.Matchers.*;
import static org.junit.Assert.*;
public class EmptyIndexesR4Test extends BaseJpaR4Test {
private static RestfulServer myRestServer;
private static IGenericClient ourClient;
private static FhirContext ourCtx;
private static CloseableHttpClient ourHttpClient;
private static Server ourServer;
private static String ourServerBase;
private SimpleRequestHeaderInterceptor mySimpleHeaderInterceptor;
@SuppressWarnings("deprecation")
@After
public void after() {
myRestServer.setUseBrowserFriendlyContentTypes(true);
ourClient.unregisterInterceptor(mySimpleHeaderInterceptor);
myDaoConfig.setIndexMissingFields(new DaoConfig().getIndexMissingFields());
}
@Before
public void before() {
mySimpleHeaderInterceptor = new SimpleRequestHeaderInterceptor();
ourClient.registerInterceptor(mySimpleHeaderInterceptor);
myDaoConfig.setIndexMissingFields(DaoConfig.IndexEnabledEnum.DISABLED);
}
@Before
public void beforeStartServer() throws Exception {
if (myRestServer == null) {
PatientResourceProvider patientRp = new PatientResourceProvider();
patientRp.setDao(myPatientDao);
QuestionnaireResourceProviderR4 questionnaireRp = new QuestionnaireResourceProviderR4();
questionnaireRp.setDao(myQuestionnaireDao);
ObservationResourceProvider observationRp = new ObservationResourceProvider();
observationRp.setDao(myObservationDao);
OrganizationResourceProvider organizationRp = new OrganizationResourceProvider();
organizationRp.setDao(myOrganizationDao);
RestfulServer restServer = new RestfulServer(ourCtx);
restServer.setResourceProviders(patientRp, questionnaireRp, observationRp, organizationRp);
restServer.setPlainProviders(mySystemProvider);
int myPort = RandomServerPortProvider.findFreePort();
ourServer = new Server(myPort);
ServletContextHandler proxyHandler = new ServletContextHandler();
proxyHandler.setContextPath("/");
ourServerBase = "http://localhost:" + myPort + "/fhir/context";
ServletHolder servletHolder = new ServletHolder();
servletHolder.setServlet(restServer);
proxyHandler.addServlet(servletHolder, "/fhir/context/*");
ourCtx = FhirContext.forR4();
restServer.setFhirContext(ourCtx);
ourServer.setHandler(proxyHandler);
ourServer.start();
PoolingHttpClientConnectionManager connectionManager = new PoolingHttpClientConnectionManager(5000, TimeUnit.MILLISECONDS);
HttpClientBuilder builder = HttpClientBuilder.create();
builder.setConnectionManager(connectionManager);
ourHttpClient = builder.build();
ourCtx.getRestfulClientFactory().setSocketTimeout(600 * 1000);
ourClient = ourCtx.newRestfulGenericClient(ourServerBase);
ourClient.setLogRequestAndResponse(true);
myRestServer = restServer;
}
myRestServer.setDefaultResponseEncoding(EncodingEnum.XML);
myRestServer.setPagingProvider(myPagingProvider);
}
@Test
public void testDontCreateNullIndexesWithOnlyString() {
Observation obs = new Observation();
obs.getCode().setText("ZXCVBNM ASDFGHJKL QWERTYUIOPASDFGHJKL");
myObservationDao.create(obs, mySrd).getId().toUnqualifiedVersionless();
runInTransaction(()->{
assertThat(myResourceIndexedSearchParamQuantityDao.findAll(), empty());
assertThat(myResourceIndexedSearchParamTokenDao.findAll(), empty());
});
}
@Test
public void testDontCreateNullIndexesWithOnlyToken() {
Observation obs = new Observation();
obs.getCode().addCoding().setSystem("FOO").setCode("BAR");
myObservationDao.create(obs, mySrd).getId().toUnqualifiedVersionless();
runInTransaction(()->{
assertThat(myResourceIndexedSearchParamQuantityDao.findAll(), empty());
assertThat(myResourceIndexedSearchParamStringDao.findAll(), empty());
// code and combo-code
assertThat(myResourceIndexedSearchParamTokenDao.findAll().toString(), myResourceIndexedSearchParamTokenDao.findAll(), hasSize(2));
});
}
@AfterClass
public static void afterClassClearContext() throws Exception {
ourServer.stop();
TestUtil.clearAllStaticFieldsForUnitTest();
}
}

View File

@ -1,37 +1,11 @@
package ca.uhn.fhir.jpa.provider.r4; package ca.uhn.fhir.jpa.provider.r4;
import static org.hamcrest.Matchers.*;
import static org.junit.Assert.*;
import java.io.InputStream;
import java.nio.charset.StandardCharsets;
import java.util.concurrent.TimeUnit;
import org.apache.commons.io.IOUtils;
import org.apache.http.Header;
import org.apache.http.client.methods.*;
import org.apache.http.entity.ContentType;
import org.apache.http.entity.StringEntity;
import org.apache.http.impl.client.CloseableHttpClient;
import org.apache.http.impl.client.HttpClientBuilder;
import org.apache.http.impl.conn.PoolingHttpClientConnectionManager;
import org.eclipse.jetty.server.Server;
import org.eclipse.jetty.servlet.ServletContextHandler;
import org.eclipse.jetty.servlet.ServletHolder;
import org.hl7.fhir.r4.hapi.validation.FhirInstanceValidator;
import org.hl7.fhir.r4.model.*;
import org.hl7.fhir.r4.model.Bundle.BundleType;
import org.hl7.fhir.r4.model.Bundle.HTTPVerb;
import org.hl7.fhir.r4.model.Enumerations.AdministrativeGender;
import org.hl7.fhir.instance.model.api.IIdType;
import org.junit.*;
import org.springframework.transaction.annotation.Propagation;
import org.springframework.transaction.annotation.Transactional;
import ca.uhn.fhir.context.FhirContext; import ca.uhn.fhir.context.FhirContext;
import ca.uhn.fhir.jpa.dao.r4.BaseJpaR4Test; import ca.uhn.fhir.jpa.dao.r4.BaseJpaR4Test;
import ca.uhn.fhir.jpa.provider.SystemProviderDstu2Test; import ca.uhn.fhir.jpa.provider.SystemProviderDstu2Test;
import ca.uhn.fhir.jpa.rp.r4.*; import ca.uhn.fhir.jpa.rp.r4.ObservationResourceProvider;
import ca.uhn.fhir.jpa.rp.r4.OrganizationResourceProvider;
import ca.uhn.fhir.jpa.rp.r4.PatientResourceProvider;
import ca.uhn.fhir.jpa.testutil.RandomServerPortProvider; import ca.uhn.fhir.jpa.testutil.RandomServerPortProvider;
import ca.uhn.fhir.rest.api.Constants; import ca.uhn.fhir.rest.api.Constants;
import ca.uhn.fhir.rest.api.EncodingEnum; import ca.uhn.fhir.rest.api.EncodingEnum;
@ -45,6 +19,35 @@ import ca.uhn.fhir.rest.server.interceptor.RequestValidatingInterceptor;
import ca.uhn.fhir.rest.server.interceptor.ResponseHighlighterInterceptor; import ca.uhn.fhir.rest.server.interceptor.ResponseHighlighterInterceptor;
import ca.uhn.fhir.util.TestUtil; import ca.uhn.fhir.util.TestUtil;
import ca.uhn.fhir.validation.ResultSeverityEnum; import ca.uhn.fhir.validation.ResultSeverityEnum;
import org.apache.commons.io.IOUtils;
import org.apache.http.Header;
import org.apache.http.client.methods.CloseableHttpResponse;
import org.apache.http.client.methods.HttpGet;
import org.apache.http.client.methods.HttpPost;
import org.apache.http.entity.ContentType;
import org.apache.http.entity.StringEntity;
import org.apache.http.impl.client.CloseableHttpClient;
import org.apache.http.impl.client.HttpClientBuilder;
import org.apache.http.impl.conn.PoolingHttpClientConnectionManager;
import org.eclipse.jetty.server.Server;
import org.eclipse.jetty.servlet.ServletContextHandler;
import org.eclipse.jetty.servlet.ServletHolder;
import org.hl7.fhir.instance.model.api.IIdType;
import org.hl7.fhir.r4.hapi.validation.FhirInstanceValidator;
import org.hl7.fhir.r4.model.*;
import org.hl7.fhir.r4.model.Bundle.BundleType;
import org.hl7.fhir.r4.model.Bundle.HTTPVerb;
import org.hl7.fhir.r4.model.Enumerations.AdministrativeGender;
import org.junit.*;
import org.springframework.transaction.annotation.Propagation;
import org.springframework.transaction.annotation.Transactional;
import java.io.InputStream;
import java.nio.charset.StandardCharsets;
import java.util.concurrent.TimeUnit;
import static org.hamcrest.Matchers.*;
import static org.junit.Assert.*;
public class SystemProviderR4Test extends BaseJpaR4Test { public class SystemProviderR4Test extends BaseJpaR4Test {
@ -210,7 +213,8 @@ public class SystemProviderR4Test extends BaseJpaR4Test {
ourLog.info(output); ourLog.info(output);
assertEquals(200, http.getStatusLine().getStatusCode()); assertEquals(200, http.getStatusLine().getStatusCode());
} finally { } finally {
IOUtils.closeQuietly(http);; IOUtils.closeQuietly(http);
;
} }
get = new HttpGet(ourServerBase + "/$perform-reindexing-pass"); get = new HttpGet(ourServerBase + "/$perform-reindexing-pass");
@ -220,7 +224,8 @@ public class SystemProviderR4Test extends BaseJpaR4Test {
ourLog.info(output); ourLog.info(output);
assertEquals(200, http.getStatusLine().getStatusCode()); assertEquals(200, http.getStatusLine().getStatusCode());
} finally { } finally {
IOUtils.closeQuietly(http);; IOUtils.closeQuietly(http);
;
} }
} }
@ -237,6 +242,7 @@ public class SystemProviderR4Test extends BaseJpaR4Test {
assertThat(http.getFirstHeader("Content-Type").getValue(), containsString("application/fhir+json")); assertThat(http.getFirstHeader("Content-Type").getValue(), containsString("application/fhir+json"));
} }
@Transactional(propagation = Propagation.NEVER) @Transactional(propagation = Propagation.NEVER)
@Test @Test
public void testSuggestKeywords() throws Exception { public void testSuggestKeywords() throws Exception {
@ -577,33 +583,33 @@ public class SystemProviderR4Test extends BaseJpaR4Test {
//@formatter:off //@formatter:off
String input = "<Bundle xmlns=\"http://hl7.org/fhir\">\n" + String input = "<Bundle xmlns=\"http://hl7.org/fhir\">\n" +
" <id value=\"20160113160203\"/>\n" + " <id value=\"20160113160203\"/>\n" +
" <type value=\"transaction\"/>\n" + " <type value=\"transaction\"/>\n" +
" <entry>\n" + " <entry>\n" +
" <fullUrl value=\"urn:uuid:c72aa430-2ddc-456e-7a09-dea8264671d8\"/>\n" + " <fullUrl value=\"urn:uuid:c72aa430-2ddc-456e-7a09-dea8264671d8\"/>\n" +
" <resource>\n" + " <resource>\n" +
" <Encounter>\n" + " <Encounter>\n" +
" <identifier>\n" + " <identifier>\n" +
" <use value=\"official\"/>\n" + " <use value=\"official\"/>\n" +
" <system value=\"http://healthcare.example.org/identifiers/encounter\"/>\n" + " <system value=\"http://healthcare.example.org/identifiers/encounter\"/>\n" +
" <value value=\"845962.8975469\"/>\n" + " <value value=\"845962.8975469\"/>\n" +
" </identifier>\n" + " </identifier>\n" +
" <status value=\"in-progress\"/>\n" + " <status value=\"in-progress\"/>\n" +
" <class value=\"inpatient\"/>\n" + " <class value=\"inpatient\"/>\n" +
" <patient>\n" + " <patient>\n" +
" <reference value=\"Patient?family=van%20de%20Heuvelcx85ioqWJbI&amp;given=Pietercx85ioqWJbI\"/>\n" + " <reference value=\"Patient?family=van%20de%20Heuvelcx85ioqWJbI&amp;given=Pietercx85ioqWJbI\"/>\n" +
" </patient>\n" + " </patient>\n" +
" <serviceProvider>\n" + " <serviceProvider>\n" +
" <reference value=\"Organization?identifier=urn:oid:2.16.840.1.113883.2.4.6.1|07-8975469\"/>\n" + " <reference value=\"Organization?identifier=urn:oid:2.16.840.1.113883.2.4.6.1|07-8975469\"/>\n" +
" </serviceProvider>\n" + " </serviceProvider>\n" +
" </Encounter>\n" + " </Encounter>\n" +
" </resource>\n" + " </resource>\n" +
" <request>\n" + " <request>\n" +
" <method value=\"POST\"/>\n" + " <method value=\"POST\"/>\n" +
" <url value=\"Encounter\"/>\n" + " <url value=\"Encounter\"/>\n" +
" </request>\n" + " </request>\n" +
" </entry>\n" + " </entry>\n" +
"</Bundle>"; "</Bundle>";
//@formatter:off //@formatter:off
HttpPost req = new HttpPost(ourServerBase); HttpPost req = new HttpPost(ourServerBase);

View File

@ -390,6 +390,69 @@ public class AuthorizationInterceptorR4Test {
assertTrue(ourHitMethod); assertTrue(ourHitMethod);
} }
@Test
public void testAllowByCompartmentUsingUnqualifiedIds() throws Exception {
ourServlet.registerInterceptor(new AuthorizationInterceptor(PolicyEnum.DENY) {
@Override
public List<IAuthRule> buildRuleList(RequestDetails theRequestDetails) {
return new RuleBuilder().allow().read().resourcesOfType(CarePlan.class).inCompartment("Patient", new IdType("Patient/123")).andThen().denyAll()
.build();
}
});
HttpGet httpGet;
HttpResponse status;
Patient patient;
CarePlan carePlan;
// Unqualified
patient = new Patient();
patient.setId("123");
carePlan = new CarePlan();
carePlan.setStatus(CarePlan.CarePlanStatus.ACTIVE);
carePlan.getSubject().setResource(patient);
ourHitMethod = false;
ourReturn = Collections.singletonList(carePlan);
httpGet = new HttpGet("http://localhost:" + ourPort + "/CarePlan/135154");
status = ourClient.execute(httpGet);
extractResponseAndClose(status);
assertEquals(200, status.getStatusLine().getStatusCode());
assertTrue(ourHitMethod);
// Qualified
patient = new Patient();
patient.setId("Patient/123");
carePlan = new CarePlan();
carePlan.setStatus(CarePlan.CarePlanStatus.ACTIVE);
carePlan.getSubject().setResource(patient);
ourHitMethod = false;
ourReturn = Collections.singletonList(carePlan);
httpGet = new HttpGet("http://localhost:" + ourPort + "/CarePlan/135154");
status = ourClient.execute(httpGet);
extractResponseAndClose(status);
assertEquals(200, status.getStatusLine().getStatusCode());
assertTrue(ourHitMethod);
// Wrong one
patient = new Patient();
patient.setId("456");
carePlan = new CarePlan();
carePlan.setStatus(CarePlan.CarePlanStatus.ACTIVE);
carePlan.getSubject().setResource(patient);
ourHitMethod = false;
ourReturn = Collections.singletonList(carePlan);
httpGet = new HttpGet("http://localhost:" + ourPort + "/CarePlan/135154");
status = ourClient.execute(httpGet);
extractResponseAndClose(status);
assertEquals(403, status.getStatusLine().getStatusCode());
assertTrue(ourHitMethod);
}
@Test @Test
public void testBatchWhenOnlyTransactionAllowed() throws Exception { public void testBatchWhenOnlyTransactionAllowed() throws Exception {
ourServlet.registerInterceptor(new AuthorizationInterceptor(PolicyEnum.DENY) { ourServlet.registerInterceptor(new AuthorizationInterceptor(PolicyEnum.DENY) {

View File

@ -248,6 +248,12 @@
the number of threads allocated to reindexing may now be adjusted via a the number of threads allocated to reindexing may now be adjusted via a
setting in the DaoConfig. setting in the DaoConfig.
</action> </action>
<action type="fix">
AuthorizationInterceptor did not correctly grant access to resources
by compartment when the reference on the target resource that pointed
to the compartment owner was defined using a resource object (ResourceReference#setResource)
instead of a reference (ResourceReference#setReference).
</action>
</release> </release>
<release version="3.3.0" date="2018-03-29"> <release version="3.3.0" date="2018-03-29">
<action type="add"> <action type="add">