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

revinclude order (#5009) 

* start building tests

* add test

* merge master

* fix bug

* fix regression

* changelog

* documentation

---------

Co-authored-by: Ken Stevens <ken@smilecdr.com>
This commit is contained in:
Ken Stevens 2023-06-20 21:54:20 +10:00 committed by GitHub
parent ac9eaf9d89
commit 8eec285611
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 86 additions and 19 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

5
hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/6_8_0/5009-revincludes-include-order.yaml Normal file
View File

@ -0,0 +1,5 @@
---
type: add
issue: 5009
title: "Previously, all revincludes would always be evaluated before all includes. Now the system checks if any
revincludes are recursive (i.e. are modified :iterate) and if so, it evaluates all the includes first."

4
hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/docs/server_jpa/search.md
View File

@ -228,3 +228,7 @@ Chained sorting is more than twice as demanding of database performance. They i
In particular, this kind of sorting can be very slow if the search returns a large number of results (e.g. a search for Encounter?sort=patient.name where there is a very large number of Encounter resources and no additional search parameters are limiting the number of included resources). They are safest when used in smaller collections, and as a secondary sort; as a tie-breaker within another sort. E.g. `Encounter?practitioner=practitioner-id&date=2023-02&_sort=location,patient.name`.
In order to improve sorting performance when chained sorts are needed, an [Uplifted Refchain](#uplifted-refchains) can be defined on the SearchParameter. This index will be used for the sorting expression and can improve performance.
# _include and _revinclude order
By default, all _revincludes will be performed first and then all _includes are performed afterwards. However, if any _revinclude parameters are modified with :iterate (or :recurse for earlier versions of FHIR) then all _include parameters will be evaluated first.

56
hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/PersistedJpaBundleProvider.java
View File

@ -46,6 +46,7 @@ import ca.uhn.fhir.jpa.search.cache.SearchCacheStatusEnum;
import ca.uhn.fhir.jpa.searchparam.SearchParameterMap;
import ca.uhn.fhir.jpa.util.MemoryCacheService;
import ca.uhn.fhir.jpa.util.QueryParameterUtils;
import ca.uhn.fhir.model.api.Include;
import ca.uhn.fhir.model.primitive.InstantDt;
import ca.uhn.fhir.rest.api.server.IBundleProvider;
import ca.uhn.fhir.rest.api.server.IPreResourceAccessDetails;
@ -422,18 +423,37 @@ public class PersistedJpaBundleProvider implements IBundleProvider {
if (mySearchEntity.getSearchType() == SearchTypeEnum.SEARCH) {
Integer maxIncludes = myStorageSettings.getMaximumIncludesToLoadPerPage();
// Load _revincludes
Set<JpaPid> includedPids = theSearchBuilder.loadIncludes(myContext, myEntityManager, thePids, mySearchEntity.toRevIncludesList(), true, mySearchEntity.getLastUpdated(), myUuid, myRequest, maxIncludes);
if (maxIncludes != null) {
maxIncludes -= includedPids.size();
}
thePids.addAll(includedPids);
includedPidList.addAll(includedPids);
// Decide whether to perform include or revincludes first based on which one has iterate.
boolean performIncludesBeforeRevincludes = shouldPerformIncludesBeforeRevincudes();
if (performIncludesBeforeRevincludes) {
// Load _includes
Set<JpaPid> includedPids = theSearchBuilder.loadIncludes(myContext, myEntityManager, thePids, mySearchEntity.toIncludesList(), false, mySearchEntity.getLastUpdated(), myUuid, myRequest, maxIncludes);
if (maxIncludes != null) {
maxIncludes -= includedPids.size();
}
thePids.addAll(includedPids);
includedPidList.addAll(includedPids);
// Load _revincludes
Set<JpaPid> revIncludedPids = theSearchBuilder.loadIncludes(myContext, myEntityManager, thePids, mySearchEntity.toRevIncludesList(), true, mySearchEntity.getLastUpdated(), myUuid, myRequest, maxIncludes);
thePids.addAll(revIncludedPids);
includedPidList.addAll(revIncludedPids);
} else {
// Load _revincludes
Set<JpaPid> revIncludedPids = theSearchBuilder.loadIncludes(myContext, myEntityManager, thePids, mySearchEntity.toRevIncludesList(), true, mySearchEntity.getLastUpdated(), myUuid, myRequest, maxIncludes);
if (maxIncludes != null) {
maxIncludes -= revIncludedPids.size();
}
thePids.addAll(revIncludedPids);
includedPidList.addAll(revIncludedPids);
// Load _includes
Set<JpaPid> includedPids = theSearchBuilder.loadIncludes(myContext, myEntityManager, thePids, mySearchEntity.toIncludesList(), false, mySearchEntity.getLastUpdated(), myUuid, myRequest, maxIncludes);
thePids.addAll(includedPids);
includedPidList.addAll(includedPids);
}
// Load _includes
Set<JpaPid> revIncludedPids = theSearchBuilder.loadIncludes(myContext, myEntityManager, thePids, mySearchEntity.toIncludesList(), false, mySearchEntity.getLastUpdated(), myUuid, myRequest, maxIncludes);
thePids.addAll(revIncludedPids);
includedPidList.addAll(revIncludedPids);
}
@ -446,6 +466,20 @@ public class PersistedJpaBundleProvider implements IBundleProvider {
return resources;
}
private boolean shouldPerformIncludesBeforeRevincudes() {
// When revincludes contain a :iterate, we should perform them last so they can iterate through the includes found so far
boolean retval = false;
for (Include nextInclude : mySearchEntity.toRevIncludesList()) {
if (nextInclude.isRecurse()) {
retval = true;
break;
}
}
return retval;
}
public void setInterceptorBroadcaster(IInterceptorBroadcaster theInterceptorBroadcaster) {
myInterceptorBroadcaster = theInterceptorBroadcaster;
}

40
hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/provider/r4/ResourceProviderRevIncludeTest.java
View File

@ -3,33 +3,27 @@ package ca.uhn.fhir.jpa.provider.r4;
import ca.uhn.fhir.interceptor.api.Hook;
import ca.uhn.fhir.interceptor.api.Pointcut;
import ca.uhn.fhir.jpa.provider.BaseResourceProviderR4Test;
import ca.uhn.fhir.jpa.util.SqlQuery;
import ca.uhn.fhir.jpa.util.SqlQueryList;
import ca.uhn.fhir.model.api.Include;
import ca.uhn.fhir.rest.api.CacheControlDirective;
import ca.uhn.fhir.rest.api.server.RequestDetails;
import ca.uhn.fhir.util.BundleUtil;
import org.hl7.fhir.instance.model.api.IBaseResource;
import org.hl7.fhir.instance.model.api.IIdType;
import org.hl7.fhir.r4.model.Bundle;
import org.hl7.fhir.r4.model.CareTeam;
import org.hl7.fhir.r4.model.Condition;
import org.hl7.fhir.r4.model.DetectedIssue;
import org.hl7.fhir.r4.model.Group;
import org.hl7.fhir.r4.model.Patient;
import org.hl7.fhir.r4.model.Practitioner;
import org.hl7.fhir.r4.model.PractitionerRole;
import org.hl7.fhir.r4.model.Reference;
import org.junit.jupiter.api.Test;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.springframework.test.context.jdbc.Sql;
import java.util.ArrayList;
import java.util.List;
import java.util.regex.Pattern;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.hasSize;
import static org.hamcrest.Matchers.matchesPattern;
import static org.junit.jupiter.api.Assertions.assertEquals;
public class ResourceProviderRevIncludeTest extends BaseResourceProviderR4Test {
@ -112,4 +106,34 @@ public class ResourceProviderRevIncludeTest extends BaseResourceProviderR4Test {
myInterceptorRegistry.unregisterInterceptor(sqlCapturingInterceptor);
}
@Test
public void includeRevInclude() {
Practitioner practitioner = new Practitioner();
practitioner.addName().setFamily("testIncludeRevInclude");
IIdType practitionerId = myClient.create().resource(practitioner).execute().getId().toUnqualifiedVersionless();
DetectedIssue detectedIssue = new DetectedIssue();
detectedIssue.getAuthor().setReferenceElement(practitionerId);
IIdType detectedIssueId = myClient.create().resource(detectedIssue).execute().getId().toUnqualifiedVersionless();
PractitionerRole practitionerRole = new PractitionerRole();
practitionerRole.getPractitioner().setReferenceElement(practitionerId);
IIdType practitionerRoleId = myClient.create().resource(practitionerRole).execute().getId().toUnqualifiedVersionless();
// DetectedIssue?_id=123&_include=DetectedIssue:author&_revinclude=PractitionerRole:practitioner
Bundle bundle = myClient.search()
.forResource(DetectedIssue.class)
.where(DetectedIssue.RES_ID.exactly().codes(detectedIssueId.getIdPart()))
.include(new Include("DetectedIssue:author"))
.revInclude(new Include("PractitionerRole:practitioner").setRecurse(true))
.returnBundle(Bundle.class)
.execute();
List<IBaseResource> foundResources = BundleUtil.toListOfResources(myFhirContext, bundle);
assertEquals(3, foundResources.size());
assertEquals(detectedIssueId.getIdPart(), foundResources.get(0).getIdElement().getIdPart());
assertEquals(practitionerId.getIdPart(), foundResources.get(1).getIdElement().getIdPart());
assertEquals(practitionerRoleId.getIdPart(), foundResources.get(2).getIdElement().getIdPart());
}
}