honeymoose
/
hapi-fhir
mirror of https://github.com/hapifhir/hapi-fhir.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Merge pull request #1600 from jamesagnew/ks-20191122-fix-include-circular-ref 

fix circular reference include everything
This commit is contained in:
Ken Stevens 2019-11-22 12:55:37 -05:00 committed by GitHub
parent 3173af31ae b36670377f
commit f7f1ec26bc
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 158 additions and 99 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

72
hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/SearchBuilder.java
View File

@ -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<ResourcePersistentId> implements Iterator<ResourcePersistentId> {
private final RequestDetails myRequest;
private Iterator<ResourcePersistentId> myCurrentIterator;
private int myCurrentOffset;
private ArrayList<ResourcePersistentId> myCurrentPids;
private Set<ResourcePersistentId> myCurrentPids;
private ResourcePersistentId myNext;
private int myPageSize = myDaoConfig.getEverythingIncludesFetchPageSize();
IncludesIterator(Set<ResourcePersistentId> 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<Include> includes = Collections.singleton(new Include("*", true));
Set<ResourcePersistentId> newPids = loadIncludes(myContext, myEntityManager, myCurrentPids, includes, false, getParams().getLastUpdated(), mySearchUuid, myRequest);
if (newPids.isEmpty()) {
myNext = NO_MORE;
break;
}
myCurrentOffset = end;
Collection<ResourcePersistentId> pidsToScan = myCurrentPids.subList(start, end);
Set<Include> includes = Collections.singleton(new Include("*", true));
Set<ResourcePersistentId> 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;
}
}

68
hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/SearchBuilderTest.java
View File

@ -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<ResourcePersistentId> 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<ResourceLink> 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());
}
}

117
hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/dstu3/PatientEverythingDstu3Test.java
View File

@ -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<String> 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<String>();
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<String> actual = new TreeSet<String>();
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());