Include includes on revincludes (#2902)

* Include includes on revincludes

hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/5_6_0/2902-includes-includes-on-revincludes.yaml

@@ -0,0 +1,6 @@
+---
+type: fix
+issue: 2902
+jira: SMILE-3000
+backport: cust_fmc_5_3
+title: "Fixed a bug wherein includes were not being included on revincludes."

hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseHapiFhirResourceDao.java

@@ -886,7 +886,7 @@ public abstract class BaseHapiFhirResourceDao<T extends IBaseResource> extends B
 		}
 		IRestfulServerDefaults server = theRequestDetails.getServer();
 		IPagingProvider pagingProvider = server.getPagingProvider();
-		return pagingProvider instanceof DatabaseBackedPagingProvider;
+		return pagingProvider != null;
 	}
 
 	protected void markResourcesMatchingExpressionAsNeedingReindexing(Boolean theCurrentlyReindexing, String theExpression) {

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

@@ -41,6 +41,7 @@ import ca.uhn.fhir.jpa.partition.RequestPartitionHelperSvc;
 import ca.uhn.fhir.jpa.search.cache.ISearchCacheSvc;
 import ca.uhn.fhir.jpa.search.cache.SearchCacheStatusEnum;
 import ca.uhn.fhir.jpa.util.InterceptorUtil;
+import ca.uhn.fhir.model.api.Include;
 import ca.uhn.fhir.rest.server.util.CompositeInterceptorBroadcaster;
 import ca.uhn.fhir.jpa.util.MemoryCacheService;
 import ca.uhn.fhir.model.primitive.InstantDt;
@@ -74,6 +75,7 @@ import java.util.List;
 import java.util.Optional;
 import java.util.Set;
 import java.util.function.Function;
+import java.util.stream.Collectors;
 
 public class PersistedJpaBundleProvider implements IBundleProvider {
 
@@ -409,19 +411,25 @@ public class PersistedJpaBundleProvider implements IBundleProvider {
 	// Note: Leave as protected, HSPC depends on this
 	@SuppressWarnings("WeakerAccess")
 	protected List<IBaseResource> toResourceList(ISearchBuilder theSearchBuilder, List<ResourcePersistentId> thePids) {
-		Set<ResourcePersistentId> includedPids = new HashSet<>();
 
+		List<ResourcePersistentId> includedPidList = new ArrayList<>();
 		if (mySearchEntity.getSearchType() == SearchTypeEnum.SEARCH) {
 			Integer maxIncludes = myDaoConfig.getMaximumIncludesToLoadPerPage();
-			includedPids.addAll(theSearchBuilder.loadIncludes(myContext, myEntityManager, thePids, mySearchEntity.toRevIncludesList(), true, mySearchEntity.getLastUpdated(), myUuid, myRequest, maxIncludes));
+
+			// Load _revincludes
+			Set<ResourcePersistentId> includedPids = theSearchBuilder.loadIncludes(myContext, myEntityManager, thePids, mySearchEntity.toRevIncludesList(), true, mySearchEntity.getLastUpdated(), myUuid, myRequest, maxIncludes);
 			if (maxIncludes != null) {
 				maxIncludes -= includedPids.size();
 			}
-			includedPids.addAll(theSearchBuilder.loadIncludes(myContext, myEntityManager, thePids, mySearchEntity.toIncludesList(), false, mySearchEntity.getLastUpdated(), myUuid, myRequest, maxIncludes));
+			thePids.addAll(includedPids);
-		}
+			includedPidList.addAll(includedPids);
 
-		List<ResourcePersistentId> includedPidList = new ArrayList<>(includedPids);
+			// Load _includes
-		thePids.addAll(includedPidList);
+			Set<ResourcePersistentId> revIncludedPids = theSearchBuilder.loadIncludes(myContext, myEntityManager, thePids, mySearchEntity.toIncludesList(), false, mySearchEntity.getLastUpdated(), myUuid, myRequest, maxIncludes);
+			thePids.addAll(revIncludedPids);
+			includedPidList.addAll(revIncludedPids);
+
+		}
 
 		// Execute the query and make sure we return distinct results
 		List<IBaseResource> resources = new ArrayList<>();

hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/SearchCoordinatorSvcImpl.java

@@ -536,19 +536,22 @@ public class SearchCoordinatorSvcImpl implements ISearchCoordinatorSvc {
 			 * individually for pages as we return them to clients
 			 */
 
+			// _includes
 			Integer maxIncludes = myDaoConfig.getMaximumIncludesToLoadPerPage();
 			final Set<ResourcePersistentId> includedPids = theSb.loadIncludes(myContext, myEntityManager, pids, theParams.getRevIncludes(), true, theParams.getLastUpdated(), "(synchronous)", theRequestDetails, maxIncludes);
-
 			if (maxIncludes != null) {
 				maxIncludes -= includedPids.size();
 			}
-
+			pids.addAll(includedPids);
-			if (theParams.getEverythingMode() == null && (maxIncludes == null || maxIncludes > 0)) {
-				includedPids.addAll(theSb.loadIncludes(myContext, myEntityManager, pids, theParams.getIncludes(), false, theParams.getLastUpdated(), "(synchronous)", theRequestDetails, maxIncludes));
-			}
-
 			List<ResourcePersistentId> includedPidsList = new ArrayList<>(includedPids);
-			pids.addAll(includedPidsList);
+
+			// _revincludes
+			if (theParams.getEverythingMode() == null && (maxIncludes == null || maxIncludes > 0)) {
+				Set<ResourcePersistentId> revIncludedPids = theSb.loadIncludes(myContext, myEntityManager, pids, theParams.getIncludes(), false, theParams.getLastUpdated(), "(synchronous)", theRequestDetails, maxIncludes);
+				includedPids.addAll(revIncludedPids);
+				pids.addAll(revIncludedPids);
+				includedPidsList.addAll(revIncludedPids);
+			}
 
 			List<IBaseResource> resources = new ArrayList<>();
 			theSb.loadResourcesByPid(pids, includedPidsList, resources, false, theRequestDetails);

hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/BaseJpaR4Test.java

@@ -103,6 +103,7 @@ import org.hl7.fhir.r4.model.AllergyIntolerance;
 import org.hl7.fhir.r4.model.Appointment;
 import org.hl7.fhir.r4.model.AuditEvent;
 import org.hl7.fhir.r4.model.Binary;
+import org.hl7.fhir.r4.model.BodyStructure;
 import org.hl7.fhir.r4.model.Bundle;
 import org.hl7.fhir.r4.model.CapabilityStatement;
 import org.hl7.fhir.r4.model.CarePlan;
@@ -371,6 +372,9 @@ public abstract class BaseJpaR4Test extends BaseJpaTest implements ITestDataBuil
 	@Qualifier("myBinaryDaoR4")
 	protected IFhirResourceDao<Binary> myBinaryDao;
 	@Autowired
+	@Qualifier("myBodyStructureDaoR4")
+	protected IFhirResourceDao<BodyStructure> myBodyStructureDao;
+	@Autowired
 	@Qualifier("myDocumentReferenceDaoR4")
 	protected IFhirResourceDao<DocumentReference> myDocumentReferenceDao;
 	@Autowired

hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4LegacySearchBuilderTest.java

@@ -929,7 +929,7 @@ public class FhirResourceDaoR4LegacySearchBuilderTest extends BaseJpaR4Test {
 		List<IIdType> actual = toUnqualifiedVersionlessIds(resp);
 		myCaptureQueriesListener.logSelectQueriesForCurrentThread();
 		assertThat(actual, containsInAnyOrder(orgId, medId, patId, moId, patId2));
-		assertEquals(6, myCaptureQueriesListener.getSelectQueriesForCurrentThread().size());
+		assertEquals(1, myCaptureQueriesListener.getSelectQueriesForCurrentThread().size());
 
 		// Specific patient ID with linked stuff
 		request = mock(HttpServletRequest.class);

hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4SearchIncludeTest.java

@@ -1,17 +1,25 @@
 package ca.uhn.fhir.jpa.dao.r4;
 
 import ca.uhn.fhir.jpa.api.config.DaoConfig;
+import ca.uhn.fhir.jpa.search.PersistedJpaBundleProvider;
+import ca.uhn.fhir.jpa.search.PersistedJpaSearchFirstPageBundleProvider;
 import ca.uhn.fhir.jpa.searchparam.SearchParameterMap;
 import ca.uhn.fhir.model.api.Include;
 import ca.uhn.fhir.rest.api.server.IBundleProvider;
 import ca.uhn.fhir.rest.param.TokenParam;
+import ca.uhn.fhir.rest.server.SimpleBundleProvider;
 import org.hamcrest.Matcher;
+import org.hamcrest.Matchers;
 import org.hamcrest.collection.IsIterableContainingInAnyOrder;
+import org.hl7.fhir.r4.model.BodyStructure;
 import org.hl7.fhir.r4.model.CarePlan;
+import org.hl7.fhir.r4.model.Enumerations;
 import org.hl7.fhir.r4.model.EpisodeOfCare;
 import org.hl7.fhir.r4.model.Organization;
 import org.hl7.fhir.r4.model.Patient;
+import org.hl7.fhir.r4.model.Procedure;
 import org.hl7.fhir.r4.model.Reference;
+import org.hl7.fhir.r4.model.SearchParameter;
 import org.junit.jupiter.api.AfterEach;
 import org.junit.jupiter.api.Test;
 
@@ -23,7 +31,10 @@ import java.util.stream.IntStream;
 import static org.hamcrest.MatcherAssert.assertThat;
 import static org.hamcrest.Matchers.containsInAnyOrder;
 import static org.hamcrest.Matchers.equalTo;
+import static org.hl7.fhir.r4.model.ResourceType.BodyStructure;
 import static org.hl7.fhir.r4.model.ResourceType.Patient;
+import static org.hl7.fhir.r4.model.ResourceType.Procedure;
+import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.fail;
 
 @SuppressWarnings({"unchecked", "Duplicates"})
@@ -114,6 +125,61 @@ public class FhirResourceDaoR4SearchIncludeTest extends BaseJpaR4Test {
 		}
 	}
 
+
+	@Test
+	public void testRevIncludeOnIncludedResource() {
+		SearchParameter sp = new SearchParameter();
+		sp.addBase("Procedure");
+		sp.setStatus(Enumerations.PublicationStatus.ACTIVE);
+		sp.setCode("focalAccess");
+		sp.setType(Enumerations.SearchParamType.REFERENCE);
+		sp.setExpression("Procedure.extension('http://fkcfhir.org/fhir/cs/CS1MachNumber')");
+		sp.addTarget("BodyStructure");
+		mySearchParameterDao.create(sp, mySrd);
+		mySearchParamRegistry.forceRefresh();
+
+		BodyStructure bs = new BodyStructure();
+		bs.setId("B51936689");
+		bs.setDescription("Foo");
+		myBodyStructureDao.update(bs, mySrd);
+
+		Procedure p = new Procedure();
+		p.setId("PRA8780542726");
+		p.setStatus(org.hl7.fhir.r4.model.Procedure.ProcedureStatus.COMPLETED);
+		myProcedureDao.update(p, mySrd);
+
+		p = new Procedure();
+		p.setId("PRA8780542785");
+		p.addPartOf().setReference("Procedure/PRA8780542726");
+		p.setStatus(org.hl7.fhir.r4.model.Procedure.ProcedureStatus.COMPLETED);
+		p.addExtension("http://fkcfhir.org/fhir/cs/CS1MachNumber", new Reference("BodyStructure/B51936689"));
+		myProcedureDao.update(p, mySrd);
+
+		logAllResources();
+		logAllResourceLinks();
+
+		// Non synchronous
+		SearchParameterMap map = new SearchParameterMap();
+		map.add("_id", new TokenParam("PRA8780542726"));
+		map.addRevInclude(new Include("Procedure:part-of"));
+		map.addInclude(new Include("Procedure:focalAccess").asRecursive());
+		IBundleProvider outcome = myProcedureDao.search(map, mySrd);
+		assertEquals(PersistedJpaSearchFirstPageBundleProvider.class, outcome.getClass());
+		List<String> ids = toUnqualifiedVersionlessIdValues(outcome);
+		assertThat(ids.toString(), ids, Matchers.containsInAnyOrder("Procedure/PRA8780542726", "Procedure/PRA8780542785", "BodyStructure/B51936689"));
+
+		// Synchronous
+		map = new SearchParameterMap().setLoadSynchronous(true);
+		map.add("_id", new TokenParam("PRA8780542726"));
+		map.addRevInclude(new Include("Procedure:part-of"));
+		map.addInclude(new Include("Procedure:focalAccess").asRecursive());
+		outcome = myProcedureDao.search(map, mySrd);
+		assertEquals(SimpleBundleProvider.class, outcome.getClass());
+		ids = toUnqualifiedVersionlessIdValues(outcome);
+		assertThat(ids.toString(), ids, Matchers.containsInAnyOrder("Procedure/PRA8780542726", "Procedure/PRA8780542785", "BodyStructure/B51936689"));
+	}
+
+
 	@Test
 	public void testRevIncludesPaged_AsyncSearch() {
 		int eocCount = 10;

hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4SearchLastNIT.java

@@ -33,6 +33,7 @@ public class FhirResourceDaoR4SearchLastNIT extends BaseR4SearchLastN {
 
 		// Set up search parameters that will return 75 Observations.
 		SearchParameterMap params = new SearchParameterMap();
+		params.setLoadSynchronous(true);
 		ReferenceParam subjectParam1 = new ReferenceParam("Patient", "", patient0Id.getValue());
 		ReferenceParam subjectParam2 = new ReferenceParam("Patient", "", patient1Id.getValue());
 		ReferenceParam subjectParam3 = new ReferenceParam("Patient", "", patient2Id.getValue());

hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4SearchNoFtTest.java

@@ -778,7 +778,7 @@ public class FhirResourceDaoR4SearchNoFtTest extends BaseJpaR4Test {
 		List<IIdType> actual = toUnqualifiedVersionlessIds(resp);
 		myCaptureQueriesListener.logSelectQueriesForCurrentThread();
 		assertThat(actual, containsInAnyOrder(orgId, medId, patId, moId, patId2));
-		assertEquals(5, myCaptureQueriesListener.getSelectQueriesForCurrentThread().size());
+		assertEquals(1, myCaptureQueriesListener.getSelectQueriesForCurrentThread().size());
 
 		// Specific patient ID with linked stuff
 		request = mock(HttpServletRequest.class);

hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/subscription/email/EmailSubscriptionDstu2Test.java

@@ -41,7 +41,7 @@ public class EmailSubscriptionDstu2Test extends BaseResourceProviderDstu2Test {
 	private static final Logger ourLog = LoggerFactory.getLogger(EmailSubscriptionDstu2Test.class);
 
 	@RegisterExtension
-	static GreenMailExtension ourGreenMail = new GreenMailExtension(ServerSetupTest.SMTP);
+	static GreenMailExtension ourGreenMail = new GreenMailExtension(ServerSetupTest.SMTP.withPort(0));
 
 	private List<IIdType> mySubscriptionIds = new ArrayList<>();
 
@@ -148,7 +148,7 @@ public class EmailSubscriptionDstu2Test extends BaseResourceProviderDstu2Test {
 	private MailConfig withMailConfig() {
 		return new MailConfig()
 			.setSmtpHostname(ServerSetupTest.SMTP.getBindAddress())
-			.setSmtpPort(ServerSetupTest.SMTP.getPort());
+			.setSmtpPort(ourGreenMail.getSmtp().getPort());
 	}
 
 }

hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/subscription/email/EmailSubscriptionDstu3Test.java

@@ -42,7 +42,7 @@ public class EmailSubscriptionDstu3Test extends BaseResourceProviderDstu3Test {
 	private static final org.slf4j.Logger ourLog = org.slf4j.LoggerFactory.getLogger(EmailSubscriptionDstu3Test.class);
 
 	@RegisterExtension
-	static GreenMailExtension ourGreenMail = new GreenMailExtension(ServerSetupTest.SMTP);
+	static GreenMailExtension ourGreenMail = new GreenMailExtension(ServerSetupTest.SMTP.withPort(0));
 
 	@Autowired
 	private SubscriptionTestUtil mySubscriptionTestUtil;
@@ -261,7 +261,7 @@ public class EmailSubscriptionDstu3Test extends BaseResourceProviderDstu3Test {
 	private MailConfig withMailConfig() {
 		return new MailConfig()
 			.setSmtpHostname(ServerSetupTest.SMTP.getBindAddress())
-			.setSmtpPort(ServerSetupTest.SMTP.getPort());
+			.setSmtpPort(ourGreenMail.getSmtp().getPort());
 	}