From 2e2048ce5284992059bff4d1baccaede3483c29d Mon Sep 17 00:00:00 2001 From: Jason Roberts Date: Wed, 1 Sep 2021 13:37:45 -0400 Subject: [PATCH] Add more test cases, not passing --- .../fhir/jpa/search/builder/QueryStack.java | 9 +- .../dao/r4/ChainedContainedR4SearchTest.java | 67 ++++++- .../r4/ResourceProviderR4SearchTest.java | 187 ++++++++++++++++++ 3 files changed, 258 insertions(+), 5 deletions(-) create mode 100644 hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/r4/ResourceProviderR4SearchTest.java diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/builder/QueryStack.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/builder/QueryStack.java index 253b07e33b6..73f6efacf81 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/builder/QueryStack.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/builder/QueryStack.java @@ -1133,10 +1133,15 @@ public class QueryStack { break; case REFERENCE: for (List nextAnd : theAndOrParams) { - if (theSearchContainedMode.equals(SearchContainedModeEnum.TRUE)) + if (theSearchContainedMode.equals(SearchContainedModeEnum.TRUE)) { + // TODO: The _contained parameter is not intended to control search chain interpretation like this. + // See SMILE-2898 for details. + // For now, leave the incorrect implementation alone, just in case someone is relying on it, + // until the complete fix is available. andPredicates.add(createPredicateReferenceForContainedResource(theSourceJoinColumn, theResourceName, theParamName, nextParamDef, nextAnd, null, theRequest, theRequestPartitionId)); - else + } else { andPredicates.add(createPredicateReference(theSourceJoinColumn, theResourceName, theParamName, nextAnd, null, theRequest, theRequestPartitionId)); + } } break; case STRING: diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/ChainedContainedR4SearchTest.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/ChainedContainedR4SearchTest.java index 0c73197bdcd..8efb1df3087 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/ChainedContainedR4SearchTest.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/ChainedContainedR4SearchTest.java @@ -59,7 +59,68 @@ public class ChainedContainedR4SearchTest extends BaseJpaR4Test { } @Test - public void testAllResourcesStandAlone() throws Exception { + public void testShouldResolveATwoLinkChainWithStandAloneResources() throws Exception { + + // setup + IIdType oid1; + + { + Patient p = new Patient(); + p.setId(IdType.newRandomUuid()); + p.addName().setFamily("Smith").addGiven("John"); + myPatientDao.create(p, mySrd); + + Observation obs = new Observation(); + obs.getCode().setText("Observation 1"); + obs.getSubject().setReference(p.getId()); + + oid1 = myObservationDao.create(obs, mySrd).getId().toUnqualifiedVersionless(); + + ourLog.info("Input: {}", myFhirCtx.newJsonParser().setPrettyPrint(true).encodeResourceToString(obs)); + } + + String url = "/Observation?subject.name=Smith"; + + // execute + myCaptureQueriesListener.clear(); + List oids = searchAndReturnUnqualifiedVersionlessIdValues(url); + ourLog.info(">>> " + myCaptureQueriesListener.getSelectQueriesForCurrentThread()); + + // validate + assertEquals(1L, oids.size()); + assertThat(oids, contains(oid1.getIdPart())); + } + + @Test + public void testShouldResolveATwoLinkChainWithAContainedResource() throws Exception { + IIdType oid1; + + { + Patient p = new Patient(); + p.setId("pat"); + p.addName().setFamily("Smith").addGiven("John"); + + Observation obs = new Observation(); + obs.getContained().add(p); + obs.getCode().setText("Observation 1"); + obs.getSubject().setReference("#pat"); + + oid1 = myObservationDao.create(obs, mySrd).getId().toUnqualifiedVersionless(); + + ourLog.info("Input: {}", myFhirCtx.newJsonParser().setPrettyPrint(true).encodeResourceToString(obs)); + } + + String url = "/Observation?subject.name=Smith"; + myCaptureQueriesListener.clear(); + List oids = searchAndReturnUnqualifiedVersionlessIdValues(url); + ourLog.info(">>> " + myCaptureQueriesListener.getSelectQueriesForCurrentThread()); + + assertEquals(1L, oids.size()); + assertThat(oids, contains(oid1.getIdPart())); + } + + @Test + public void testShouldResolveAThreeLinkChainWhereAllResourcesStandAlone() throws Exception { // setup IIdType oid1; @@ -98,7 +159,7 @@ public class ChainedContainedR4SearchTest extends BaseJpaR4Test { } @Test - public void testContainedResourceAtTheEndOfTheChain() throws Exception { + public void testShouldResolveAThreeLinkChainWithAContainedResourceAtTheEndOfTheChain() throws Exception { // This is the case that is most relevant to SMILE-2899 IIdType oid1; @@ -133,7 +194,7 @@ public class ChainedContainedR4SearchTest extends BaseJpaR4Test { } @Test - public void testContainedResourceAtTheBeginningOfTheChain() throws Exception { + public void testShouldResolveAThreeLinkChainWithAContainedResourceAtTheBeginningOfTheChain() throws Exception { // This case seems like it would be less frequent in production, but we don't want to // paint ourselves into a corner where we require the contained link to be the last // one in the chain diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/r4/ResourceProviderR4SearchTest.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/r4/ResourceProviderR4SearchTest.java new file mode 100644 index 00000000000..d38c44a5097 --- /dev/null +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/r4/ResourceProviderR4SearchTest.java @@ -0,0 +1,187 @@ +package ca.uhn.fhir.jpa.provider.r4; + +import ca.uhn.fhir.jpa.api.config.DaoConfig; +import ca.uhn.fhir.jpa.api.dao.IFhirResourceDao; +import ca.uhn.fhir.jpa.model.entity.ModelConfig; +import ca.uhn.fhir.parser.StrictErrorHandler; +import ca.uhn.fhir.rest.client.interceptor.CapturingInterceptor; +import org.apache.commons.io.IOUtils; +import org.apache.http.client.methods.CloseableHttpResponse; +import org.apache.http.client.methods.HttpGet; +import org.hl7.fhir.instance.model.api.IIdType; +import org.hl7.fhir.r4.model.Bundle; +import org.hl7.fhir.r4.model.ClinicalImpression; +import org.hl7.fhir.r4.model.IdType; +import org.hl7.fhir.r4.model.Observation; +import org.hl7.fhir.r4.model.Organization; +import org.hl7.fhir.r4.model.Patient; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.beans.factory.annotation.Qualifier; + +import java.io.IOException; +import java.nio.charset.StandardCharsets; +import java.util.List; + +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.contains; +import static org.junit.jupiter.api.Assertions.assertEquals; + + +public class ResourceProviderR4SearchTest extends BaseResourceProviderR4Test { + + private static final org.slf4j.Logger ourLog = org.slf4j.LoggerFactory.getLogger(ResourceProviderR4SearchTest.class); + @Autowired + @Qualifier("myClinicalImpressionDaoR4") + protected IFhirResourceDao myClinicalImpressionDao; + private CapturingInterceptor myCapturingInterceptor = new CapturingInterceptor(); + + @Override + @AfterEach + public void after() throws Exception { + super.after(); + + myDaoConfig.setAllowMultipleDelete(new DaoConfig().isAllowMultipleDelete()); + myDaoConfig.setAllowExternalReferences(new DaoConfig().isAllowExternalReferences()); + myDaoConfig.setReuseCachedSearchResultsForMillis(new DaoConfig().getReuseCachedSearchResultsForMillis()); + myDaoConfig.setCountSearchResultsUpTo(new DaoConfig().getCountSearchResultsUpTo()); + myDaoConfig.setSearchPreFetchThresholds(new DaoConfig().getSearchPreFetchThresholds()); + myDaoConfig.setAllowContainsSearches(new DaoConfig().isAllowContainsSearches()); + myDaoConfig.setIndexMissingFields(new DaoConfig().getIndexMissingFields()); + + myClient.unregisterInterceptor(myCapturingInterceptor); + myModelConfig.setIndexOnContainedResources(false); + myModelConfig.setIndexOnContainedResources(new ModelConfig().isIndexOnContainedResources()); + } + + @BeforeEach + @Override + public void before() throws Exception { + super.before(); + myFhirCtx.setParserErrorHandler(new StrictErrorHandler()); + + myDaoConfig.setAllowMultipleDelete(true); + myClient.registerInterceptor(myCapturingInterceptor); + myDaoConfig.setSearchPreFetchThresholds(new DaoConfig().getSearchPreFetchThresholds()); + myModelConfig.setIndexOnContainedResources(true); + myDaoConfig.setReuseCachedSearchResultsForMillis(null); + } + + @Test + public void testAllResourcesStandAlone() throws Exception { + + IIdType oid1; + + { + Organization org = new Organization(); + org.setId(IdType.newRandomUuid()); + org.setName("HealthCo"); + myOrganizationDao.create(org, mySrd); + + Patient p = new Patient(); + p.setId(IdType.newRandomUuid()); + p.addName().setFamily("Smith").addGiven("John"); + p.getManagingOrganization().setReference(org.getId()); + myPatientDao.create(p, mySrd); + + Observation obs = new Observation(); + obs.getCode().setText("Observation 1"); + obs.getSubject().setReference(p.getId()); + + oid1 = myObservationDao.create(obs, mySrd).getId().toUnqualifiedVersionless(); + + ourLog.info("Input: {}", myFhirCtx.newJsonParser().setPrettyPrint(true).encodeResourceToString(obs)); + } + + String uri = ourServerBase + "/Observation?subject.organization.name=HealthCo"; + List oids = searchAndReturnUnqualifiedVersionlessIdValues(uri); + + assertEquals(1L, oids.size()); + assertThat(oids, contains(oid1.getValue())); + } + + @Test + public void testContainedResourceAtTheEndOfTheChain() throws Exception { + // This is the case that is most relevant to SMILE-2899 + IIdType oid1; + + { + Organization org = new Organization(); + org.setId("org"); + org.setName("HealthCo"); + + Patient p = new Patient(); + p.setId(IdType.newRandomUuid()); + p.getContained().add(org); + p.addName().setFamily("Smith").addGiven("John"); + p.getManagingOrganization().setReference("#org"); + myPatientDao.create(p, mySrd); + + Observation obs = new Observation(); + obs.getCode().setText("Observation 1"); + obs.getSubject().setReference(p.getId()); + + oid1 = myObservationDao.create(obs, mySrd).getId().toUnqualifiedVersionless(); + + ourLog.info("Input: {}", myFhirCtx.newJsonParser().setPrettyPrint(true).encodeResourceToString(obs)); + } + + String uri = ourServerBase + "/Observation?subject.organization.name=HealthCo"; + List oids = searchAndReturnUnqualifiedVersionlessIdValues(uri); + + assertEquals(1L, oids.size()); + assertThat(oids, contains(oid1.getValue())); + } + + @Test + public void testContainedResourceAtTheBeginningOfTheChain() throws Exception { + // This case seems like it would be less frequent in production, but we don't want to + // paint ourselves into a corner where we require the contained link to be the last + // one in the chain + + IIdType oid1; + + { + Organization org = new Organization(); + org.setId(IdType.newRandomUuid()); + org.setName("HealthCo"); + myOrganizationDao.create(org, mySrd); + + Patient p = new Patient(); + p.setId("pat"); + p.addName().setFamily("Smith").addGiven("John"); + p.getManagingOrganization().setReference(org.getId()); + + Observation obs = new Observation(); + obs.getContained().add(p); + obs.getCode().setText("Observation 1"); + obs.getSubject().setReference("#pat"); + + oid1 = myObservationDao.create(obs, mySrd).getId().toUnqualifiedVersionless(); + + ourLog.info("Input: {}", myFhirCtx.newJsonParser().setPrettyPrint(true).encodeResourceToString(obs)); + } + + String uri = ourServerBase + "/Observation?subject.organization.name=HealthCo"; + List oids = searchAndReturnUnqualifiedVersionlessIdValues(uri); + + assertEquals(1L, oids.size()); + assertThat(oids, contains(oid1.getValue())); + } + + private List searchAndReturnUnqualifiedVersionlessIdValues(String uri) throws IOException { + List ids; + HttpGet get = new HttpGet(uri); + + try (CloseableHttpResponse response = ourHttpClient.execute(get)) { + String resp = IOUtils.toString(response.getEntity().getContent(), StandardCharsets.UTF_8); + ourLog.info(resp); + Bundle bundle = myFhirCtx.newXmlParser().parseResource(Bundle.class, resp); + ids = toUnqualifiedVersionlessIdValues(bundle); + } + return ids; + } + +}