From 85517c47e226eaa5d4e133825223b1a751f290b5 Mon Sep 17 00:00:00 2001 From: James Agnew Date: Thu, 19 Aug 2021 12:31:29 -0400 Subject: [PATCH] Include includes on revincludes (#2902) * Include includes on revincludes --- ...2902-includes-includes-on-revincludes.yaml | 6 ++ .../fhir/jpa/dao/BaseHapiFhirResourceDao.java | 2 +- .../search/PersistedJpaBundleProvider.java | 20 ++++-- .../jpa/search/SearchCoordinatorSvcImpl.java | 17 +++-- .../ca/uhn/fhir/jpa/dao/r4/BaseJpaR4Test.java | 4 ++ ...rResourceDaoR4LegacySearchBuilderTest.java | 2 +- .../FhirResourceDaoR4SearchIncludeTest.java | 66 +++++++++++++++++++ .../r4/FhirResourceDaoR4SearchLastNIT.java | 1 + .../r4/FhirResourceDaoR4SearchNoFtTest.java | 2 +- .../email/EmailSubscriptionDstu2Test.java | 4 +- .../email/EmailSubscriptionDstu3Test.java | 4 +- 11 files changed, 108 insertions(+), 20 deletions(-) create mode 100644 hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/5_6_0/2902-includes-includes-on-revincludes.yaml diff --git a/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/5_6_0/2902-includes-includes-on-revincludes.yaml b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/5_6_0/2902-includes-includes-on-revincludes.yaml new file mode 100644 index 00000000000..3085e6b801f --- /dev/null +++ b/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." diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseHapiFhirResourceDao.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseHapiFhirResourceDao.java index 1efdf533534..2d2911b9f92 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseHapiFhirResourceDao.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseHapiFhirResourceDao.java @@ -886,7 +886,7 @@ public abstract class BaseHapiFhirResourceDao extends B } IRestfulServerDefaults server = theRequestDetails.getServer(); IPagingProvider pagingProvider = server.getPagingProvider(); - return pagingProvider instanceof DatabaseBackedPagingProvider; + return pagingProvider != null; } protected void markResourcesMatchingExpressionAsNeedingReindexing(Boolean theCurrentlyReindexing, String theExpression) { 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 5fdec9ec884..38ddcb8077b 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 @@ -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 toResourceList(ISearchBuilder theSearchBuilder, List thePids) { - Set includedPids = new HashSet<>(); + List 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 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 includedPidList = new ArrayList<>(includedPids); - thePids.addAll(includedPidList); + // Load _includes + Set 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 resources = new ArrayList<>(); diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/SearchCoordinatorSvcImpl.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/SearchCoordinatorSvcImpl.java index dba0366e468..4d266271bd9 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/SearchCoordinatorSvcImpl.java +++ b/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 includedPids = theSb.loadIncludes(myContext, myEntityManager, pids, theParams.getRevIncludes(), true, theParams.getLastUpdated(), "(synchronous)", theRequestDetails, maxIncludes); - if (maxIncludes != null) { maxIncludes -= includedPids.size(); } - - if (theParams.getEverythingMode() == null && (maxIncludes == null || maxIncludes > 0)) { - includedPids.addAll(theSb.loadIncludes(myContext, myEntityManager, pids, theParams.getIncludes(), false, theParams.getLastUpdated(), "(synchronous)", theRequestDetails, maxIncludes)); - } - + pids.addAll(includedPids); List includedPidsList = new ArrayList<>(includedPids); - pids.addAll(includedPidsList); + + // _revincludes + if (theParams.getEverythingMode() == null && (maxIncludes == null || maxIncludes > 0)) { + Set revIncludedPids = theSb.loadIncludes(myContext, myEntityManager, pids, theParams.getIncludes(), false, theParams.getLastUpdated(), "(synchronous)", theRequestDetails, maxIncludes); + includedPids.addAll(revIncludedPids); + pids.addAll(revIncludedPids); + includedPidsList.addAll(revIncludedPids); + } List resources = new ArrayList<>(); theSb.loadResourcesByPid(pids, includedPidsList, resources, false, theRequestDetails); diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/BaseJpaR4Test.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/BaseJpaR4Test.java index 716db77adef..95a9732fed5 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/BaseJpaR4Test.java +++ b/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 myBinaryDao; @Autowired + @Qualifier("myBodyStructureDaoR4") + protected IFhirResourceDao myBodyStructureDao; + @Autowired @Qualifier("myDocumentReferenceDaoR4") protected IFhirResourceDao myDocumentReferenceDao; @Autowired diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4LegacySearchBuilderTest.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4LegacySearchBuilderTest.java index 75ba865d268..7a18b4ed391 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4LegacySearchBuilderTest.java +++ b/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 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); diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4SearchIncludeTest.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4SearchIncludeTest.java index 575b71648ab..a95130ace11 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4SearchIncludeTest.java +++ b/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 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; diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4SearchLastNIT.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4SearchLastNIT.java index 19076e16b97..6e1adee1f66 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4SearchLastNIT.java +++ b/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()); diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4SearchNoFtTest.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4SearchNoFtTest.java index c5cfa742aa0..a34682ddfd8 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4SearchNoFtTest.java +++ b/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 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); diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/subscription/email/EmailSubscriptionDstu2Test.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/subscription/email/EmailSubscriptionDstu2Test.java index f56ce7090ae..91b55bf142e 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/subscription/email/EmailSubscriptionDstu2Test.java +++ b/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 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()); } } diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/subscription/email/EmailSubscriptionDstu3Test.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/subscription/email/EmailSubscriptionDstu3Test.java index 9390df0679d..29b060e7abd 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/subscription/email/EmailSubscriptionDstu3Test.java +++ b/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()); }