From b3b9273ca74a7993bfef362710727c6aa2c05756 Mon Sep 17 00:00:00 2001 From: James Agnew Date: Mon, 19 Jun 2017 06:16:27 -0400 Subject: [PATCH] Work on everything fixes for #674 --- .../ca/uhn/fhir/jpa/dao/SearchBuilder.java | 82 ++++++-- .../search/PersistedJpaBundleProvider.java | 2 +- .../dstu3/PatientEverythingDstu3Test.java | 195 ++++++++++++++++++ .../dstu3/ResourceProviderDstu3Test.java | 1 + 4 files changed, 263 insertions(+), 17 deletions(-) create mode 100644 hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/dstu3/PatientEverythingDstu3Test.java diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/SearchBuilder.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/SearchBuilder.java index e2d6223fa51..e5f8f21e1e4 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/SearchBuilder.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/SearchBuilder.java @@ -44,6 +44,7 @@ import org.hl7.fhir.instance.model.api.*; import com.google.common.collect.*; import ca.uhn.fhir.context.*; +import ca.uhn.fhir.jpa.dao.SearchBuilder.IncludesIterator; import ca.uhn.fhir.jpa.dao.data.IForcedIdDao; import ca.uhn.fhir.jpa.dao.data.IResourceIndexedSearchParamUriDao; import ca.uhn.fhir.jpa.entity.*; @@ -71,6 +72,7 @@ import ca.uhn.fhir.util.UrlUtil; */ public class SearchBuilder implements ISearchBuilder { private static Long NO_MORE = Long.valueOf(-1); + private static final org.slf4j.Logger ourLog = org.slf4j.LoggerFactory.getLogger(SearchBuilder.class); private List myAlsoIncludePids; private CriteriaBuilder myBuilder; @@ -90,7 +92,6 @@ public class SearchBuilder implements ISearchBuilder { private ISearchParamRegistry mySearchParamRegistry; private String mySearchUuid; private IHapiTerminologySvc myTerminologySvc; - /** * Constructor */ @@ -1048,7 +1049,7 @@ public class SearchBuilder implements ISearchBuilder { if (!isBlank(unitsValue)) { code = theBuilder.equal(theFrom.get("myUnits"), unitsValue); } - + cmpValue = ObjectUtils.defaultIfNull(cmpValue, ParamPrefixEnum.EQUAL); final Expression path = theFrom.get("myValue"); String invalidMessageName = "invalidQuantityPrefix"; @@ -1357,11 +1358,11 @@ public class SearchBuilder implements ISearchBuilder { * Now perform the search */ final TypedQuery query = myEntityManager.createQuery(outerQuery); - + if (theMaximumResults != null) { query.setMaxResults(theMaximumResults); } - + return query; } @@ -1629,12 +1630,6 @@ public class SearchBuilder implements ISearchBuilder { List results = q.getResultList(); for (ResourceLink resourceLink : results) { if (theReverseMode) { - // if (theEverythingModeEnum.isEncounter()) { - // if (resourceLink.getSourcePath().equals("Encounter.subject") || - // resourceLink.getSourcePath().equals("Encounter.patient")) { - // nextRoundOmit.add(resourceLink.getSourceResourcePid()); - // } - // } pidsToInclude.add(resourceLink.getSourceResourcePid()); } else { pidsToInclude.add(resourceLink.getTargetResourcePid()); @@ -1745,10 +1740,10 @@ public class SearchBuilder implements ISearchBuilder { } private void searchForIdsWithAndOr(String theResourceName, String theParamName, List> theAndOrParams) { - + for (int andListIdx = 0; andListIdx < theAndOrParams.size(); andListIdx++) { List nextOrList = theAndOrParams.get(andListIdx); - + for (int orListIdx = 0; orListIdx < nextOrList.size(); orListIdx++) { IQueryParameterType nextOr = nextOrList.get(orListIdx); boolean hasNoValue = false; @@ -1760,14 +1755,14 @@ public class SearchBuilder implements ISearchBuilder { hasNoValue = true; } } - + if (hasNoValue) { ourLog.debug("Ignoring empty parameter: {}", theParamName); nextOrList.remove(orListIdx); orListIdx--; } } - + if (nextOrList.isEmpty()) { theAndOrParams.remove(andListIdx); andListIdx--; @@ -1777,7 +1772,7 @@ public class SearchBuilder implements ISearchBuilder { if (theAndOrParams.isEmpty()) { return; } - + if (theParamName.equals(BaseResource.SP_RES_ID)) { addPredicateResourceId(theAndOrParams); @@ -1973,6 +1968,34 @@ public class SearchBuilder implements ISearchBuilder { return thePredicates.toArray(new Predicate[thePredicates.size()]); } + public class IncludesIterator implements Iterator{ + + private Long myNext; + + @Override + public boolean hasNext() { + fetchNext(); + return myNext != NO_MORE; + } + + @Override + public Long next() { + fetchNext(); + Long retVal = myNext; + myNext = null; + return retVal; + } + + private void fetchNext() { + if (myNext == null) { + + + + } + } + + } + private enum JoinEnum { DATE, NUMBER, QUANTITY, REFERENCE, STRING, TOKEN, URI @@ -2007,16 +2030,24 @@ public class SearchBuilder implements ISearchBuilder { } private final class QueryIterator implements Iterator { + private boolean myFirst = true; + private IncludesIterator myIncludesIterator; private Long myNext; private final Set myPidSet = new HashSet(); private Iterator myPreResultsIterator; private Iterator myResultsIterator; private SortSpec mySort; + private boolean myStillNeedToFetchIncludes; private StopWatch myStopwatch = null; private QueryIterator() { mySort = myParams.getSort(); + + // Includes are processed inline for $everything query + if (myParams.getEverythingMode() != null) { + myStillNeedToFetchIncludes = true; + } } private void fetchNext() { @@ -2061,7 +2092,24 @@ public class SearchBuilder implements ISearchBuilder { } if (myNext == null) { - myNext = NO_MORE; + if (myStillNeedToFetchIncludes) { + myIncludesIterator = new IncludesIterator(); + myStillNeedToFetchIncludes = false; + } + if (myIncludesIterator != null) { + while (myIncludesIterator.hasNext()) { + Long next = myIncludesIterator.next(); + if (next != null && myPidSet.add(next)) { + myNext = next; + break; + } + } + if (myNext == null) { + myNext = NO_MORE; + } + } else { + myNext = NO_MORE; + } } } // if we need to fetch the next result @@ -2070,9 +2118,11 @@ public class SearchBuilder implements ISearchBuilder { ourLog.info("Initial query result returned in {}ms for query {}", myStopwatch.getMillis(), mySearchUuid); myFirst = false; } + if (myNext == NO_MORE) { ourLog.info("Query found {} matches in {}ms for query {}", myPidSet.size(), myStopwatch.getMillis(), mySearchUuid); } + } @Override diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/PersistedJpaBundleProvider.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/PersistedJpaBundleProvider.java index 083d76d38a2..d077dea033a 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/PersistedJpaBundleProvider.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/PersistedJpaBundleProvider.java @@ -241,8 +241,8 @@ public class PersistedJpaBundleProvider implements IBundleProvider { Set includedPids = new HashSet(); if (mySearchEntity.getSearchType() == SearchTypeEnum.SEARCH) { includedPids.addAll(sb.loadReverseIncludes(myDao, myContext, myEntityManager, pidsSubList, mySearchEntity.toRevIncludesList(), true, mySearchEntity.getLastUpdated())); + includedPids.addAll(sb.loadReverseIncludes(myDao, myContext, myEntityManager, pidsSubList, mySearchEntity.toIncludesList(), false, mySearchEntity.getLastUpdated())); } - includedPids.addAll(sb.loadReverseIncludes(myDao, myContext, myEntityManager, pidsSubList, mySearchEntity.toIncludesList(), false, mySearchEntity.getLastUpdated())); // Execute the query and make sure we return distinct results List resources = new ArrayList(); diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/dstu3/PatientEverythingDstu3Test.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/dstu3/PatientEverythingDstu3Test.java new file mode 100644 index 00000000000..14a85ab76cb --- /dev/null +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/dstu3/PatientEverythingDstu3Test.java @@ -0,0 +1,195 @@ +package ca.uhn.fhir.jpa.provider.dstu3; + +import static org.apache.commons.lang3.StringUtils.isNotBlank; +import static org.hamcrest.Matchers.contains; +import static org.hamcrest.Matchers.containsInAnyOrder; +import static org.hamcrest.Matchers.containsInRelativeOrder; +import static org.hamcrest.Matchers.containsString; +import static org.hamcrest.Matchers.empty; +import static org.hamcrest.Matchers.greaterThan; +import static org.hamcrest.Matchers.hasItem; +import static org.hamcrest.Matchers.hasItems; +import static org.hamcrest.Matchers.hasSize; +import static org.hamcrest.Matchers.lessThan; +import static org.hamcrest.Matchers.lessThanOrEqualTo; +import static org.hamcrest.Matchers.not; +import static org.hamcrest.Matchers.startsWith; +import static org.hamcrest.Matchers.stringContainsInOrder; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNotEquals; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertThat; +import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; + +import java.io.*; +import java.net.*; +import java.nio.charset.StandardCharsets; +import java.util.*; + +import org.apache.commons.io.IOUtils; +import org.apache.commons.lang3.StringUtils; +import org.apache.commons.lang3.Validate; +import org.apache.http.NameValuePair; +import org.apache.http.client.ClientProtocolException; +import org.apache.http.client.entity.UrlEncodedFormEntity; +import org.apache.http.client.methods.*; +import org.apache.http.entity.*; +import org.apache.http.message.BasicNameValuePair; +import org.hl7.fhir.dstu3.model.*; +import org.hl7.fhir.dstu3.model.Bundle.*; +import org.hl7.fhir.dstu3.model.Encounter.EncounterLocationComponent; +import org.hl7.fhir.dstu3.model.Encounter.EncounterStatus; +import org.hl7.fhir.dstu3.model.Enumerations.AdministrativeGender; +import org.hl7.fhir.dstu3.model.Narrative.NarrativeStatus; +import org.hl7.fhir.dstu3.model.Observation.ObservationStatus; +import org.hl7.fhir.dstu3.model.Questionnaire.QuestionnaireItemType; +import org.hl7.fhir.dstu3.model.Subscription.SubscriptionChannelType; +import org.hl7.fhir.dstu3.model.Subscription.SubscriptionStatus; +import org.hl7.fhir.instance.model.Encounter.EncounterState; +import org.hl7.fhir.instance.model.api.*; +import org.junit.*; +import org.springframework.transaction.TransactionStatus; +import org.springframework.transaction.support.TransactionCallback; + +import com.google.common.base.Charsets; +import com.google.common.collect.Lists; + +import ca.uhn.fhir.jpa.dao.DaoConfig; +import ca.uhn.fhir.jpa.entity.Search; +import ca.uhn.fhir.model.api.TemporalPrecisionEnum; +import ca.uhn.fhir.model.primitive.InstantDt; +import ca.uhn.fhir.model.primitive.UriDt; +import ca.uhn.fhir.parser.IParser; +import ca.uhn.fhir.parser.StrictErrorHandler; +import ca.uhn.fhir.rest.api.MethodOutcome; +import ca.uhn.fhir.rest.api.SummaryEnum; +import ca.uhn.fhir.rest.client.IGenericClient; +import ca.uhn.fhir.rest.gclient.StringClientParam; +import ca.uhn.fhir.rest.param.*; +import ca.uhn.fhir.rest.server.Constants; +import ca.uhn.fhir.rest.server.EncodingEnum; +import ca.uhn.fhir.rest.server.exceptions.*; +import ca.uhn.fhir.rest.server.interceptor.RequestValidatingInterceptor; +import ca.uhn.fhir.util.TestUtil; +import ca.uhn.fhir.util.UrlUtil; + +public class PatientEverythingDstu3Test extends BaseResourceProviderDstu3Test { + + private static final org.slf4j.Logger ourLog = org.slf4j.LoggerFactory.getLogger(PatientEverythingDstu3Test.class); + private String orgId; + private String patId; + private String encId1; + private String encId2; + private ArrayList myObsIds; + + @Before + public void beforeDisableResultReuse() { + myDaoConfig.setReuseCachedSearchResultsForMillis(null); + } + + @Override + @After + public void after() throws Exception { + super.after(); + + myDaoConfig.setReuseCachedSearchResultsForMillis(new DaoConfig().getReuseCachedSearchResultsForMillis()); + } + + @Override + public void before() throws Exception { + super.before(); + myFhirCtx.setParserErrorHandler(new StrictErrorHandler()); + + myDaoConfig.setAllowMultipleDelete(true); + + Organization org = new Organization(); + org.setName("an org"); + orgId = ourClient.create().resource(org).execute().getId().toUnqualifiedVersionless().getValue(); + + Patient patient = new Patient(); + patient.getManagingOrganization().setReference(orgId); + patId = ourClient.create().resource(patient).execute().getId().toUnqualifiedVersionless().getValue(); + + Patient patient2 = new Patient(); + patient2.getManagingOrganization().setReference(orgId); + ourClient.create().resource(patient2).execute().getId().toUnqualifiedVersionless().getValue(); + + Encounter enc1 = new Encounter(); + enc1.setStatus(EncounterStatus.CANCELLED); + enc1.getServiceProvider().setReference(orgId); + encId1 = ourClient.create().resource(enc1).execute().getId().toUnqualifiedVersionless().getValue(); + + Encounter enc2 = new Encounter(); + enc2.setStatus(EncounterStatus.ARRIVED); + enc2.getServiceProvider().setReference(orgId); + encId2 = ourClient.create().resource(enc2).execute().getId().toUnqualifiedVersionless().getValue(); + + myObsIds = new ArrayList(); + for (int i = 0; i < 20; i++) { + Observation obs = new Observation(); + obs.getSubject().setReference(patId); + obs.setStatus(ObservationStatus.FINAL); + String obsId = ourClient.create().resource(obs).execute().getId().toUnqualifiedVersionless().getValue(); + myObsIds.add(obsId); + } + + } + + /** + * See #674 + */ + @Test + public void testEverythingPagesWithCorrectEncodingJson() throws Exception { + + Bundle bundle = fetchBundle(ourServerBase + "/" + patId + "/$everything?_format=json&_count=1", EncodingEnum.JSON); + + assertNotNull(bundle.getLink("next").getUrl()); + assertThat(bundle.getLink("next").getUrl(), containsString("_format=json")); + bundle = fetchBundle(bundle.getLink("next").getUrl(), EncodingEnum.JSON); + + assertNotNull(bundle.getLink("next").getUrl()); + assertThat(bundle.getLink("next").getUrl(), containsString("_format=json")); + bundle = fetchBundle(bundle.getLink("next").getUrl(), EncodingEnum.JSON); + } + + /** + * See #674 + */ + @Test + public void testEverythingPagesWithCorrectEncodingXml() throws Exception { + + Bundle bundle = fetchBundle(ourServerBase + "/" + patId + "/$everything?_format=xml&_count=1", EncodingEnum.XML); + + assertNotNull(bundle.getLink("next").getUrl()); + ourLog.info("Next link: {}", bundle.getLink("next").getUrl()); + assertThat(bundle.getLink("next").getUrl(), containsString("_format=xml")); + bundle = fetchBundle(bundle.getLink("next").getUrl(), EncodingEnum.XML); + + assertNotNull(bundle.getLink("next").getUrl()); + ourLog.info("Next link: {}", bundle.getLink("next").getUrl()); + assertThat(bundle.getLink("next").getUrl(), containsString("_format=xml")); + bundle = fetchBundle(bundle.getLink("next").getUrl(), EncodingEnum.XML); + } + + private Bundle fetchBundle(String theUrl, EncodingEnum theEncoding) throws IOException, ClientProtocolException { + Bundle bundle; + HttpGet get = new HttpGet(theUrl); + CloseableHttpResponse resp = ourHttpClient.execute(get); + try { + assertEquals(theEncoding.getResourceContentTypeNonLegacy(), resp.getFirstHeader(Constants.HEADER_CONTENT_TYPE).getValue().replaceAll(";.*", "")); + bundle = theEncoding.newParser(myFhirCtx).parseResource(Bundle.class, IOUtils.toString(resp.getEntity().getContent(), Charsets.UTF_8)); + } finally { + IOUtils.closeQuietly(resp); + } + + return bundle; + } + + @AfterClass + public static void afterClassClearContext() { + TestUtil.clearAllStaticFieldsForUnitTest(); + } + +} diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/dstu3/ResourceProviderDstu3Test.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/dstu3/ResourceProviderDstu3Test.java index ac20a5c6607..9a8460e140a 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/dstu3/ResourceProviderDstu3Test.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/dstu3/ResourceProviderDstu3Test.java @@ -47,6 +47,7 @@ import org.hl7.fhir.dstu3.model.Observation.ObservationStatus; import org.hl7.fhir.dstu3.model.Questionnaire.QuestionnaireItemType; import org.hl7.fhir.dstu3.model.Subscription.SubscriptionChannelType; import org.hl7.fhir.dstu3.model.Subscription.SubscriptionStatus; +import org.hl7.fhir.instance.model.Encounter.EncounterState; import org.hl7.fhir.instance.model.api.*; import org.junit.*; import org.springframework.transaction.TransactionStatus;