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 7c3b306e392..1f3ef92ea71 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 @@ -20,15 +20,7 @@ package ca.uhn.fhir.jpa.dao; * #L% */ -import ca.uhn.fhir.context.BaseRuntimeChildDefinition; -import ca.uhn.fhir.context.BaseRuntimeDeclaredChildDefinition; -import ca.uhn.fhir.context.BaseRuntimeElementDefinition; -import ca.uhn.fhir.context.ConfigurationException; -import ca.uhn.fhir.context.FhirContext; -import ca.uhn.fhir.context.RuntimeChildChoiceDefinition; -import ca.uhn.fhir.context.RuntimeChildResourceDefinition; -import ca.uhn.fhir.context.RuntimeResourceDefinition; -import ca.uhn.fhir.context.RuntimeSearchParam; +import ca.uhn.fhir.context.*; import ca.uhn.fhir.interceptor.api.HookParams; import ca.uhn.fhir.interceptor.api.IInterceptorBroadcaster; import ca.uhn.fhir.interceptor.api.Pointcut; @@ -51,18 +43,8 @@ import ca.uhn.fhir.jpa.searchparam.registry.ISearchParamRegistry; import ca.uhn.fhir.jpa.searchparam.util.SourceParam; import ca.uhn.fhir.jpa.term.VersionIndependentConcept; import ca.uhn.fhir.jpa.term.api.ITermReadSvc; -import ca.uhn.fhir.jpa.util.BaseIterator; -import ca.uhn.fhir.jpa.util.CurrentThreadCaptureQueriesListener; -import ca.uhn.fhir.jpa.util.JpaInterceptorBroadcaster; -import ca.uhn.fhir.jpa.util.ScrollableResultsIterator; -import ca.uhn.fhir.jpa.util.SqlQueryList; -import ca.uhn.fhir.model.api.IPrimitiveDatatype; -import ca.uhn.fhir.model.api.IQueryParameterAnd; -import ca.uhn.fhir.model.api.IQueryParameterOr; -import ca.uhn.fhir.model.api.IQueryParameterType; -import ca.uhn.fhir.model.api.IResource; -import ca.uhn.fhir.model.api.Include; -import ca.uhn.fhir.model.api.ResourceMetadataKeyEnum; +import ca.uhn.fhir.jpa.util.*; +import ca.uhn.fhir.model.api.*; import ca.uhn.fhir.model.base.composite.BaseCodingDt; import ca.uhn.fhir.model.base.composite.BaseIdentifierDt; import ca.uhn.fhir.model.base.composite.BaseQuantityDt; @@ -70,11 +52,7 @@ import ca.uhn.fhir.model.primitive.IdDt; import ca.uhn.fhir.model.primitive.InstantDt; import ca.uhn.fhir.model.valueset.BundleEntrySearchModeEnum; import ca.uhn.fhir.parser.DataFormatException; -import ca.uhn.fhir.rest.api.Constants; -import ca.uhn.fhir.rest.api.QualifiedParamList; -import ca.uhn.fhir.rest.api.RestSearchParameterTypeEnum; -import ca.uhn.fhir.rest.api.SortOrderEnum; -import ca.uhn.fhir.rest.api.SortSpec; +import ca.uhn.fhir.rest.api.*; import ca.uhn.fhir.rest.api.server.IPreResourceAccessDetails; import ca.uhn.fhir.rest.api.server.RequestDetails; import ca.uhn.fhir.rest.param.*; @@ -85,6 +63,7 @@ import ca.uhn.fhir.rest.server.exceptions.ResourceNotFoundException; import ca.uhn.fhir.rest.server.servlet.ServletRequestDetails; import ca.uhn.fhir.util.StopWatch; import ca.uhn.fhir.util.UrlUtil; +import com.google.common.annotations.VisibleForTesting; import com.google.common.collect.Lists; import com.google.common.collect.Maps; import com.google.common.collect.Sets; @@ -119,11 +98,7 @@ import java.util.Map.Entry; import java.util.stream.Collectors; import static org.apache.commons.lang3.ObjectUtils.defaultIfNull; -import static org.apache.commons.lang3.StringUtils.defaultIfBlank; -import static org.apache.commons.lang3.StringUtils.defaultString; -import static org.apache.commons.lang3.StringUtils.isBlank; -import static org.apache.commons.lang3.StringUtils.isNotBlank; -import static org.apache.commons.lang3.StringUtils.trim; +import static org.apache.commons.lang3.StringUtils.*; /** * The SearchBuilder is responsible for actually forming the SQL query that handles @@ -2595,6 +2570,8 @@ public class SearchBuilder implements ISearchBuilder { nextRoundMatches = pidsToInclude; } while (includes.size() > 0 && nextRoundMatches.size() > 0 && addedSomeThisRound); + allAdded.removeAll(original); + ourLog.info("Loaded {} {} in {} rounds and {} ms for search {}", allAdded.size(), theReverseMode ? "_revincludes" : "_includes", roundCounts, w.getMillisAndRestart(), theSearchIdOrDescription); // Interceptor call: STORAGE_PREACCESS_RESOURCES @@ -3109,19 +3086,26 @@ public class SearchBuilder implements ISearchBuilder { } + @VisibleForTesting + void setParamsForUnitTest(SearchParameterMap theParams) { + myParams = theParams; + } + + SearchParameterMap getParams() { + return myParams; + } + public class IncludesIterator extends BaseIterator implements Iterator { private final RequestDetails myRequest; private Iterator myCurrentIterator; - private int myCurrentOffset; - private ArrayList myCurrentPids; + private Set myCurrentPids; private ResourcePersistentId myNext; private int myPageSize = myDaoConfig.getEverythingIncludesFetchPageSize(); IncludesIterator(Set thePidSet, RequestDetails theRequest) { - myCurrentPids = new ArrayList<>(thePidSet); + myCurrentPids = new HashSet<>(thePidSet); myCurrentIterator = EMPTY_LONG_LIST.iterator(); - myCurrentOffset = 0; myRequest = theRequest; } @@ -3133,21 +3117,13 @@ public class SearchBuilder implements ISearchBuilder { break; } - int start = myCurrentOffset; - int end = myCurrentOffset + myPageSize; - if (end > myCurrentPids.size()) { - end = myCurrentPids.size(); - } - if (end - start <= 0) { + Set includes = Collections.singleton(new Include("*", true)); + Set newPids = loadIncludes(myContext, myEntityManager, myCurrentPids, includes, false, getParams().getLastUpdated(), mySearchUuid, myRequest); + if (newPids.isEmpty()) { myNext = NO_MORE; break; } - myCurrentOffset = end; - Collection pidsToScan = myCurrentPids.subList(start, end); - Set includes = Collections.singleton(new Include("*", true)); - Set newPids = loadIncludes(myContext, myEntityManager, pidsToScan, includes, false, myParams.getLastUpdated(), mySearchUuid, myRequest); myCurrentIterator = newPids.iterator(); - } } @@ -3484,4 +3460,8 @@ public class SearchBuilder implements ISearchBuilder { return thePredicates.toArray(new Predicate[0]); } + @VisibleForTesting + void setEntityManagerForUnitTest(EntityManager theEntityManager) { + myEntityManager = theEntityManager; + } } diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/SearchBuilderTest.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/SearchBuilderTest.java index 11761730577..10708c8a82d 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/SearchBuilderTest.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/SearchBuilderTest.java @@ -1,25 +1,34 @@ package ca.uhn.fhir.jpa.dao; +import ca.uhn.fhir.jpa.model.cross.ResourcePersistentId; +import ca.uhn.fhir.jpa.model.entity.ResourceLink; +import ca.uhn.fhir.jpa.model.entity.ResourceTable; +import ca.uhn.fhir.jpa.searchparam.SearchParameterMap; +import ca.uhn.fhir.rest.param.ParamPrefixEnum; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.junit.MockitoJUnitRunner; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import javax.persistence.EntityManager; +import javax.persistence.TypedQuery; +import java.math.BigDecimal; +import java.math.MathContext; +import java.util.ArrayList; +import java.util.HashSet; +import java.util.List; +import java.util.Set; + import static org.hamcrest.Matchers.startsWith; import static org.junit.Assert.*; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; -import java.math.BigDecimal; -import java.math.MathContext; - -import org.junit.AfterClass; -import org.junit.Test; - -import ca.uhn.fhir.rest.param.ParamPrefixEnum; -import ca.uhn.fhir.util.TestUtil; - +@RunWith(MockitoJUnitRunner.class) public class SearchBuilderTest { - private static final org.slf4j.Logger ourLog = org.slf4j.LoggerFactory.getLogger(SearchBuilderTest.class); - - @AfterClass - public static void afterClassClearContext() { - TestUtil.clearAllStaticFieldsForUnitTest(); - } - + private static final Logger ourLog = LoggerFactory.getLogger(SearchBuilderTest.class); @Test public void testCalculateMultiplierEqualNoDecimal() { @@ -97,5 +106,32 @@ public class SearchBuilderTest { assertThat(out.toPlainString(), startsWith("20.000")); } + @Test + public void testIncludeIterator() { + BaseHapiFhirDao mockDao = mock(BaseHapiFhirDao.class); + when(mockDao.getConfig()).thenReturn(new DaoConfig()); + SearchBuilder searchBuilder = new SearchBuilder(mockDao); + searchBuilder.setParamsForUnitTest(new SearchParameterMap()); + EntityManager mockEntityManager = mock(EntityManager.class); + searchBuilder.setEntityManagerForUnitTest(mockEntityManager); + + Set pidSet = new HashSet<>(); + pidSet.add(new ResourcePersistentId(1L)); + pidSet.add(new ResourcePersistentId(2L)); + + TypedQuery mockQuery = mock(TypedQuery.class); + when(mockEntityManager.createQuery(any(), any())).thenReturn(mockQuery); + List resultList = new ArrayList<>(); + ResourceLink link = new ResourceLink(); + ResourceTable target = new ResourceTable(); + target.setId(1L); + link.setTargetResource(target); + resultList.add(link); + when(mockQuery.getResultList()).thenReturn(resultList); + + SearchBuilder.IncludesIterator includesIterator = searchBuilder.new IncludesIterator(pidSet, null); + // hasNext() should return false if the pid added was already on our list going in. + assertFalse(includesIterator.hasNext()); + } } 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 index ac27342da30..354a3ee3412 100644 --- 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 @@ -1,11 +1,10 @@ package ca.uhn.fhir.jpa.provider.dstu3; -import static org.hamcrest.Matchers.*; -import static org.junit.Assert.*; - -import java.io.IOException; -import java.util.*; - +import ca.uhn.fhir.jpa.dao.DaoConfig; +import ca.uhn.fhir.parser.StrictErrorHandler; +import ca.uhn.fhir.rest.api.EncodingEnum; +import ca.uhn.fhir.util.TestUtil; +import com.google.common.base.Charsets; import org.apache.commons.io.IOUtils; import org.apache.http.client.ClientProtocolException; import org.apache.http.client.methods.CloseableHttpResponse; @@ -14,25 +13,31 @@ import org.hl7.fhir.dstu3.model.*; import org.hl7.fhir.dstu3.model.Bundle.BundleEntryComponent; import org.hl7.fhir.dstu3.model.Encounter.EncounterStatus; import org.hl7.fhir.dstu3.model.Observation.ObservationStatus; -import org.junit.*; +import org.junit.After; +import org.junit.AfterClass; +import org.junit.Before; +import org.junit.Test; -import com.google.common.base.Charsets; +import java.io.IOException; +import java.util.ArrayList; +import java.util.Set; +import java.util.TreeSet; -import ca.uhn.fhir.jpa.dao.DaoConfig; -import ca.uhn.fhir.parser.StrictErrorHandler; -import ca.uhn.fhir.rest.api.EncodingEnum; -import ca.uhn.fhir.util.TestUtil; +import static org.hamcrest.Matchers.*; +import static org.junit.Assert.*; 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 myOrgId; + private String myPatientId; private String encId1; private String encId2; private ArrayList myObsIds; private String myWrongPatId; private String myWrongEnc1; + private Organization myOrg; + private Patient myPatient; @Before public void beforeDisableResultReuse() { @@ -54,42 +59,44 @@ public class PatientEverythingDstu3Test extends BaseResourceProviderDstu3Test { myFhirCtx.setParserErrorHandler(new StrictErrorHandler()); myDaoConfig.setAllowMultipleDelete(true); - - Organization org = new Organization(); - org.setName("an org"); - orgId = ourClient.create().resource(org).execute().getId().toUnqualifiedVersionless().getValue(); - ourLog.info("OrgId: {}", orgId); - Patient patient = new Patient(); - patient.getManagingOrganization().setReference(orgId); - patId = ourClient.create().resource(patient).execute().getId().toUnqualifiedVersionless().getValue(); + myOrg = new Organization(); + myOrg.setName("an org"); + myOrgId = ourClient.create().resource(myOrg).execute().getId().toUnqualifiedVersionless().getValue(); + myOrg.setId(myOrgId); + ourLog.info("OrgId: {}", myOrgId); + + myPatient = new Patient(); + myPatient.getManagingOrganization().setReference(myOrgId); + myPatientId = ourClient.create().resource(myPatient).execute().getId().toUnqualifiedVersionless().getValue(); + myPatient.setId(myPatientId); Patient patient2 = new Patient(); - patient2.getManagingOrganization().setReference(orgId); + patient2.getManagingOrganization().setReference(myOrgId); myWrongPatId = ourClient.create().resource(patient2).execute().getId().toUnqualifiedVersionless().getValue(); Encounter enc1 = new Encounter(); enc1.setStatus(EncounterStatus.CANCELLED); - enc1.getSubject().setReference(patId); - enc1.getServiceProvider().setReference(orgId); + enc1.getSubject().setReference(myPatientId); + enc1.getServiceProvider().setReference(myOrgId); encId1 = ourClient.create().resource(enc1).execute().getId().toUnqualifiedVersionless().getValue(); Encounter enc2 = new Encounter(); enc2.setStatus(EncounterStatus.ARRIVED); - enc2.getSubject().setReference(patId); - enc2.getServiceProvider().setReference(orgId); + enc2.getSubject().setReference(myPatientId); + enc2.getServiceProvider().setReference(myOrgId); encId2 = ourClient.create().resource(enc2).execute().getId().toUnqualifiedVersionless().getValue(); Encounter wrongEnc1 = new Encounter(); wrongEnc1.setStatus(EncounterStatus.ARRIVED); wrongEnc1.getSubject().setReference(myWrongPatId); - wrongEnc1.getServiceProvider().setReference(orgId); + wrongEnc1.getServiceProvider().setReference(myOrgId); myWrongEnc1 = ourClient.create().resource(wrongEnc1).execute().getId().toUnqualifiedVersionless().getValue(); myObsIds = new ArrayList(); for (int i = 0; i < 20; i++) { Observation obs = new Observation(); - obs.getSubject().setReference(patId); + obs.getSubject().setReference(myPatientId); obs.setStatus(ObservationStatus.FINAL); String obsId = ourClient.create().resource(obs).execute().getId().toUnqualifiedVersionless().getValue(); myObsIds.add(obsId); @@ -103,7 +110,7 @@ public class PatientEverythingDstu3Test extends BaseResourceProviderDstu3Test { @Test public void testEverythingReturnsCorrectResources() throws Exception { - Bundle bundle = fetchBundle(ourServerBase + "/" + patId + "/$everything?_format=json&_count=100", EncodingEnum.JSON); + Bundle bundle = fetchBundle(ourServerBase + "/" + myPatientId + "/$everything?_format=json&_count=100", EncodingEnum.JSON); assertNull(bundle.getLink("next")); @@ -114,15 +121,51 @@ public class PatientEverythingDstu3Test extends BaseResourceProviderDstu3Test { ourLog.info("Found IDs: {}", actual); - assertThat(actual, hasItem(patId)); + assertThat(actual, hasItem(myPatientId)); assertThat(actual, hasItem(encId1)); assertThat(actual, hasItem(encId2)); - assertThat(actual, hasItem(orgId)); + assertThat(actual, hasItem(myOrgId)); assertThat(actual, hasItems(myObsIds.toArray(new String[0]))); assertThat(actual, not(hasItem(myWrongPatId))); assertThat(actual, not(hasItem(myWrongEnc1))); } + @Test + public void testEverythingHandlesCircularReferences() throws Exception { + Patient linkedPatient1 = new Patient(); + linkedPatient1.addLink().setOther(new Reference(myPatientId)); + String linkedPatient1Id = ourClient.create().resource(linkedPatient1).execute().getId().toUnqualifiedVersionless().getValue(); + + Patient linkedPatient2 = new Patient(); + linkedPatient2.addLink().setOther(new Reference(linkedPatient1Id)); + String linkedPatient2Id = ourClient.create().resource(linkedPatient2).execute().getId().toUnqualifiedVersionless().getValue(); + + myPatient.addLink().setOther(new Reference(linkedPatient2Id)); + ourClient.update().resource(myPatient).execute(); + + Bundle bundle = fetchBundle(ourServerBase + "/" + myPatientId + "/$everything?_format=json&_count=100", EncodingEnum.JSON); + + assertNull(bundle.getLink("next")); + + Set actual = new TreeSet(); + for (BundleEntryComponent nextEntry : bundle.getEntry()) { + actual.add(nextEntry.getResource().getIdElement().toUnqualifiedVersionless().getValue()); + } + + ourLog.info("Found IDs: {}", actual); + + assertThat(actual, hasItem(myPatientId)); + assertThat(actual, hasItem(linkedPatient1Id)); + assertThat(actual, hasItem(linkedPatient2Id)); + assertThat(actual, hasItem(encId1)); + assertThat(actual, hasItem(encId2)); + assertThat(actual, hasItem(myOrgId)); + assertThat(actual, hasItems(myObsIds.toArray(new String[0]))); + assertThat(actual, not(hasItem(myWrongPatId))); + assertThat(actual, not(hasItem(myWrongEnc1))); + + } + /** * See #674 */ @@ -130,7 +173,7 @@ public class PatientEverythingDstu3Test extends BaseResourceProviderDstu3Test { public void testEverythingReturnsCorrectResourcesSmallPage() throws Exception { myDaoConfig.setEverythingIncludesFetchPageSize(1); - Bundle bundle = fetchBundle(ourServerBase + "/" + patId + "/$everything?_format=json&_count=100", EncodingEnum.JSON); + Bundle bundle = fetchBundle(ourServerBase + "/" + myPatientId + "/$everything?_format=json&_count=100", EncodingEnum.JSON); assertNull(bundle.getLink("next")); @@ -141,10 +184,10 @@ public class PatientEverythingDstu3Test extends BaseResourceProviderDstu3Test { ourLog.info("Found IDs: {}", actual); - assertThat(actual, hasItem(patId)); + assertThat(actual, hasItem(myPatientId)); assertThat(actual, hasItem(encId1)); assertThat(actual, hasItem(encId2)); - assertThat(actual, hasItem(orgId)); + assertThat(actual, hasItem(myOrgId)); assertThat(actual, hasItems(myObsIds.toArray(new String[0]))); assertThat(actual, not(hasItem(myWrongPatId))); assertThat(actual, not(hasItem(myWrongEnc1))); @@ -156,7 +199,7 @@ public class PatientEverythingDstu3Test extends BaseResourceProviderDstu3Test { @Test public void testEverythingPagesWithCorrectEncodingJson() throws Exception { - Bundle bundle = fetchBundle(ourServerBase + "/" + patId + "/$everything?_format=json&_count=1", EncodingEnum.JSON); + Bundle bundle = fetchBundle(ourServerBase + "/" + myPatientId + "/$everything?_format=json&_count=1", EncodingEnum.JSON); assertNotNull(bundle.getLink("next").getUrl()); assertThat(bundle.getLink("next").getUrl(), containsString("_format=json")); @@ -173,7 +216,7 @@ public class PatientEverythingDstu3Test extends BaseResourceProviderDstu3Test { @Test public void testEverythingPagesWithCorrectEncodingXml() throws Exception { - Bundle bundle = fetchBundle(ourServerBase + "/" + patId + "/$everything?_format=xml&_count=1", EncodingEnum.XML); + Bundle bundle = fetchBundle(ourServerBase + "/" + myPatientId + "/$everything?_format=xml&_count=1", EncodingEnum.XML); assertNotNull(bundle.getLink("next").getUrl()); ourLog.info("Next link: {}", bundle.getLink("next").getUrl());