Jr 20211021 chained references 3 (#3107)

* Create index entries for outbound references of contained resources

* build query for chained reference

* fix case where the contained reference is an explicit id rather than a continued chain

* fix contained index to use path names not search param names

* make qualified search work

* cleanup and changelog

* recurse while creating indexes on contained resources

* double link both contained

* longer contained subchains

* adding some failing test cases to illustrate the limitations of qualified searches

* clean up merge cruft

* changelog

* create recursive resource links

* add test coverage for a more complicated case

* changelog

* remove unnecessary check for _contained flag

* fix broken tests

hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/5_6_0/3106-chained-search-multi-contained-resource-to-outbound-ref.yaml

@@ -0,0 +1,5 @@
+---
+type: add
+issue: 3106
+jira: SMILE-3151
+title: "Further enhances the features added by issue 3100 to allow chained searches across any combination of discrete and contained references."

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

@@ -1139,13 +1139,7 @@ public class QueryStack {
 					break;
 				case REFERENCE:
 					for (List<? extends IQueryParameterType> nextAnd : theAndOrParams) {
-						if (theSearchContainedMode.equals(SearchContainedModeEnum.TRUE)) {
+						if (isEligibleForContainedResourceSearch(nextAnd)) {
-							// TODO: The _contained parameter is not intended to control search chain interpretation like this.
-							//   See SMILE-2898 for details.
-							//   For now, leave the incorrect implementation alone, just in case someone is relying on it,
-							//   until the complete fix is available.
-							andPredicates.add(createPredicateReferenceForContainedResource(null, theResourceName, theParamName, new ArrayList<>(), nextParamDef, nextAnd, null, theRequest, theRequestPartitionId));
-						} else if (isEligibleForContainedResourceSearch(nextAnd)) {
 							andPredicates.add(toOrPredicate(
 								createPredicateReference(theSourceJoinColumn, theResourceName, theParamName, new ArrayList<>(), nextAnd, null, theRequest, theRequestPartitionId),
 								createPredicateReferenceForContainedResource(theSourceJoinColumn, theResourceName, theParamName, new ArrayList<>(), nextParamDef, nextAnd, null, theRequest, theRequestPartitionId)

hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/builder/predicate/ResourceLinkPredicateBuilder.java

@@ -61,6 +61,7 @@ import ca.uhn.fhir.rest.param.SpecialParam;
 import ca.uhn.fhir.rest.param.StringParam;
 import ca.uhn.fhir.rest.param.TokenParam;
 import ca.uhn.fhir.rest.param.TokenParamModifier;
+import ca.uhn.fhir.rest.param.UriParam;
 import ca.uhn.fhir.rest.server.exceptions.InternalErrorException;
 import ca.uhn.fhir.rest.server.exceptions.InvalidRequestException;
 import ca.uhn.fhir.rest.server.servlet.ServletRequestDetails;
@@ -656,6 +657,8 @@ public class ResourceLinkPredicateBuilder extends BaseJoiningPredicateBuilder {
 				}
 				throw new InternalErrorException("Don't know how to convert param type: " + theParam.getParamType());
 			case URI:
+				qp = new UriParam();
+				break;
 			case HAS:
 			default:
 				throw new InternalErrorException("Don't know how to convert param type: " + theParam.getParamType());

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

@@ -769,6 +769,48 @@ public class ChainingR4SearchTest extends BaseJpaR4Test {
 		assertThat(oids, contains(oid1.getIdPart()));
 	}
 
+	@Test
+	public void testShouldResolveAFourLinkChainWhereTheFirstTwoReferencesAreContained() throws Exception {
+
+		// setup
+		myModelConfig.setIndexOnContainedResourcesRecursively(true);
+		IIdType oid1;
+
+		{
+			Organization org = new Organization();
+			org.setId(IdType.newRandomUuid());
+			org.setName("HealthCo");
+			myOrganizationDao.create(org, mySrd);
+
+			Organization partOfOrg = new Organization();
+			partOfOrg.setId("child");
+			partOfOrg.getPartOf().setReference(org.getId());
+
+			Patient p = new Patient();
+			p.setId("pat");
+			p.addName().setFamily("Smith").addGiven("John");
+			p.getManagingOrganization().setReference("#child");
+
+			Observation obs = new Observation();
+			obs.getContained().add(org);
+			obs.getContained().add(partOfOrg);
+			obs.getContained().add(p);
+			obs.getCode().setText("Observation 1");
+			obs.getSubject().setReference("#pat");
+
+			oid1 = myObservationDao.create(obs, mySrd).getId().toUnqualifiedVersionless();
+		}
+
+		String url = "/Observation?subject.organization.partof.name=HealthCo";
+
+		// execute
+		List<String> oids = searchAndReturnUnqualifiedVersionlessIdValues(url);
+
+		// validate
+		assertEquals(1L, oids.size());
+		assertThat(oids, contains(oid1.getIdPart()));
+	}
+
 	@Test
 	public void testShouldResolveAFourLinkChainWhereAllReferencesAreContained() throws Exception {
 

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

@@ -77,7 +77,6 @@ public class FhirResourceDaoR4ContainedTest extends BaseJpaR4Test {
 
 		map = new SearchParameterMap();
 		map.add("subject", new ReferenceParam("name", "Smith"));
-		map.setSearchContainedMode(SearchContainedModeEnum.TRUE);
 		assertThat(toUnqualifiedVersionlessIdValues(myObservationDao.search(map)), containsInAnyOrder(toValues(id)));
 	}
 	
@@ -112,7 +111,6 @@ public class FhirResourceDaoR4ContainedTest extends BaseJpaR4Test {
 
 		map = new SearchParameterMap();
 		map.add("subject", new ReferenceParam("name", "Smith"));
-		map.setSearchContainedMode(SearchContainedModeEnum.TRUE);
 		map.setLoadSynchronous(true);
 
 		assertThat(toUnqualifiedVersionlessIdValues(myObservationDao.search(map)), containsInAnyOrder(toValues(id)));
@@ -183,7 +181,6 @@ public class FhirResourceDaoR4ContainedTest extends BaseJpaR4Test {
 
 		map = new SearchParameterMap();
 		map.add("general-practitioner", new ReferenceParam("family", "Smith"));
-		map.setSearchContainedMode(SearchContainedModeEnum.TRUE);
 
 		assertThat(toUnqualifiedVersionlessIdValues(myPatientDao.search(map)), containsInAnyOrder(toValues(id)));
 	}
@@ -268,30 +265,10 @@ public class FhirResourceDaoR4ContainedTest extends BaseJpaR4Test {
 
 		map = new SearchParameterMap();
 		map.add("based-on", new ReferenceParam("authored", "2021-02-23"));
-		map.setSearchContainedMode(SearchContainedModeEnum.TRUE);
 
 		assertThat(toUnqualifiedVersionlessIdValues(myEncounterDao.search(map)), containsInAnyOrder(toValues(id)));
 	}
 	
-	@Test
-	public void testSearchWithNotSupportedSearchType() {
-
-		SearchParameterMap map;
-
-		map = new SearchParameterMap();
-		map.add("subject", new ReferenceParam("near", "toronto"));
-		map.setSearchContainedMode(SearchContainedModeEnum.TRUE);
-		
-		try {
-			IBundleProvider outcome = myObservationDao.search(map);
-			outcome.getResources(0, 1).get(0);
-			fail();
-		} catch (InvalidRequestException e) {
-			assertEquals(e.getMessage(), "The search type: SPECIAL is not supported.");
-		}
-		
-	}
-	
 	@Test
 	public void testSearchWithNotSupportedSearchParameter() {
 
@@ -299,14 +276,13 @@ public class FhirResourceDaoR4ContainedTest extends BaseJpaR4Test {
 
 		map = new SearchParameterMap();
 		map.add("subject", new ReferenceParam("marital-status", "M"));
-		map.setSearchContainedMode(SearchContainedModeEnum.TRUE);
 
 		try {
 			IBundleProvider outcome = myObservationDao.search(map);
 			outcome.getResources(0, 1).get(0);
 			fail();
 		} catch (InvalidRequestException e) {
-			assertEquals(e.getMessage(), "Unknown search parameter name: subject.marital-status.");
+			assertEquals("Invalid parameter chain: subject.marital-status", e.getMessage());
 		}
 		
 	}

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

@@ -135,7 +135,7 @@ public class FhirResourceDaoR4CreateTest extends BaseJpaR4Test {
 		p.getNameFirstRep().setFamily("Smith");
 
 		Observation containedObs = new Observation();
-		containedObs.setId("#obs");
+		containedObs.setId("obs");
 		containedObs.setSubject(new Reference("#pat"));
 
 		Encounter enc = new Encounter();
@@ -172,7 +172,7 @@ public class FhirResourceDaoR4CreateTest extends BaseJpaR4Test {
 		org2.setPartOf(new Reference("#org1"));
 
 		Observation containedObs = new Observation();
-		containedObs.setId("#obs");
+		containedObs.setId("obs");
 		containedObs.addPerformer(new Reference("#org1"));
 
 		Encounter enc = new Encounter();
@@ -206,6 +206,90 @@ public class FhirResourceDaoR4CreateTest extends BaseJpaR4Test {
 		});
 	}
 
+	@Test
+	public void testCreateLinkCreatesAppropriatePaths_ContainedResourceRecursive_ToOutboundReference() {
+		myModelConfig.setIndexOnContainedResources(true);
+		myModelConfig.setIndexOnContainedResourcesRecursively(true);
+
+		Organization org = new Organization();
+		org.setId("Organization/ABC");
+		myOrganizationDao.update(org);
+
+		Patient p = new Patient();
+		p.setId("pat");
+		p.setActive(true);
+		p.setManagingOrganization(new Reference("Organization/ABC"));
+
+		Observation containedObs = new Observation();
+		containedObs.setId("#cont");
+		containedObs.setSubject(new Reference("#pat"));
+
+		Encounter enc = new Encounter();
+		enc.getContained().add(p);
+		enc.getContained().add(containedObs);
+		enc.addReasonReference(new Reference("#cont"));
+		myEncounterDao.create(enc, mySrd);
+
+		runInTransaction(() ->{
+			List<ResourceLink> allLinks = myResourceLinkDao.findAll();
+			Optional<ResourceLink> link = allLinks
+				.stream()
+				.filter(t -> "Encounter.reasonReference.subject.managingOrganization".equals(t.getSourcePath()))
+				.findFirst();
+			assertTrue(link.isPresent());
+			assertEquals("Organization", link.get().getTargetResourceType());
+			assertEquals("ABC", link.get().getTargetResourceId());
+		});
+	}
+
+	@Test
+	public void testCreateLinkCreatesAppropriatePaths_ContainedResourceRecursive_ToOutboundReference_NoLoops() {
+		myModelConfig.setIndexOnContainedResources(true);
+		myModelConfig.setIndexOnContainedResourcesRecursively(true);
+
+		Organization org = new Organization();
+		org.setId("Organization/ABC");
+		myOrganizationDao.update(org);
+
+		Patient p = new Patient();
+		p.setId("pat");
+		p.setActive(true);
+		p.setManagingOrganization(new Reference("Organization/ABC"));
+
+		Observation obs1 = new Observation();
+		obs1.setId("obs1");
+		obs1.setSubject(new Reference("#pat"));
+		obs1.addPartOf(new Reference("#obs2"));
+
+		Observation obs2 = new Observation();
+		obs2.setId("obs2");
+		obs2.addPartOf(new Reference("#obs1"));
+
+		Encounter enc = new Encounter();
+		enc.getContained().add(p);
+		enc.getContained().add(obs1);
+		enc.getContained().add(obs2);
+		enc.addReasonReference(new Reference("#obs2"));
+		myEncounterDao.create(enc, mySrd);
+
+		runInTransaction(() ->{
+			List<ResourceLink> allLinks = myResourceLinkDao.findAll();
+			Optional<ResourceLink> link = allLinks
+				.stream()
+				.filter(t -> "Encounter.reasonReference.partOf.subject.managingOrganization".equals(t.getSourcePath()))
+				.findFirst();
+			assertTrue(link.isPresent());
+			assertEquals("Organization", link.get().getTargetResourceType());
+			assertEquals("ABC", link.get().getTargetResourceId());
+
+			Optional<ResourceLink> noLink = allLinks
+				.stream()
+				.filter(t -> "Encounter.reasonReference.partOf.partOf.partOf.subject.managingOrganization".equals(t.getSourcePath()))
+				.findFirst();
+			assertFalse(noLink.isPresent());
+		});
+	}
+
 	@Test
 	public void testConditionalCreateWithPlusInUrl() {
 		Observation obs = new Observation();

hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/r4/ResourceProviderR4SearchContainedTest.java

@@ -823,20 +823,20 @@ public class ResourceProviderR4SearchContainedTest extends BaseResourceProviderR
 		}
 
 		//-- Search by uri
-		String uri = ourServerBase + "/Observation?based-on.instantiates-uri=http://www.hl7.com&_contained=true";
+		String uri = ourServerBase + "/Observation?based-on.instantiates-uri=http://www.hl7.com";
 		List<String> oids = searchAndReturnUnqualifiedVersionlessIdValues(uri);
 
 		assertEquals(1L, oids.size());
 		assertThat(oids, contains(oid1.getValue()));
 
 		//-- Search by uri more than 1 results
-		uri = ourServerBase + "/Observation?based-on.instantiates-uri=http://www2.hl7.com&_contained=true";
+		uri = ourServerBase + "/Observation?based-on.instantiates-uri=http://www2.hl7.com";
 		oids = searchAndReturnUnqualifiedVersionlessIdValues(uri);
 
 		assertEquals(2L, oids.size());
 
 		//-- Search by uri with 'or'
-		uri = ourServerBase + "/Observation?based-on.instantiates-uri=http://www.hl7.com,http://www2.hl7.com&_contained=true";
+		uri = ourServerBase + "/Observation?based-on.instantiates-uri=http://www.hl7.com,http://www2.hl7.com";
 		oids = searchAndReturnUnqualifiedVersionlessIdValues(uri);
 
 		assertEquals(3L, oids.size());

hapi-fhir-jpaserver-searchparam/src/main/java/ca/uhn/fhir/jpa/searchparam/extractor/SearchParamExtractorService.java

@@ -137,7 +137,7 @@ public class SearchParamExtractorService {
 		extractSearchIndexParametersForContainedResources(theRequestDetails, theParams, theResource, theEntity, containedResources, new HashSet<>());
 	}
 
-	private void extractSearchIndexParametersForContainedResources(RequestDetails theRequestDetails, ResourceIndexedSearchParams theParams, IBaseResource theResource, ResourceTable theEntity, Collection<IBaseResource> containedResources, Collection<IBaseResource> theAlreadySeenResources) {
+	private void extractSearchIndexParametersForContainedResources(RequestDetails theRequestDetails, ResourceIndexedSearchParams theParams, IBaseResource theResource, ResourceTable theEntity, Collection<IBaseResource> theContainedResources, Collection<IBaseResource> theAlreadySeenResources) {
 		// 2. Find referenced search parameters
 		ISearchParamExtractor.SearchParamSet<PathAndRef> referencedSearchParamSet = mySearchParamExtractor.extractResourceLinks(theResource, true);
 
@@ -153,7 +153,7 @@ public class SearchParamExtractorService {
 				continue;
 
 			// 3.2 find the contained resource
-			IBaseResource containedResource = findContainedResource(containedResources, nextPathAndRef.getRef());
+			IBaseResource containedResource = findContainedResource(theContainedResources, nextPathAndRef.getRef());
 			if (containedResource == null)
 				continue;
 
@@ -171,7 +171,7 @@ public class SearchParamExtractorService {
 			if (myModelConfig.isIndexOnContainedResourcesRecursively()) {
 				HashSet<IBaseResource> nextAlreadySeenResources = new HashSet<>(theAlreadySeenResources);
 				nextAlreadySeenResources.add(containedResource);
-				extractSearchIndexParametersForContainedResources(theRequestDetails, currParams, containedResource, theEntity, containedResources, nextAlreadySeenResources);
+				extractSearchIndexParametersForContainedResources(theRequestDetails, currParams, containedResource, theEntity, theContainedResources, nextAlreadySeenResources);
 			}
 
 			// 3.5 added reference name as a prefix for the contained resource if any
@@ -430,6 +430,11 @@ public class SearchParamExtractorService {
 		// 1. get all contained resources
 		Collection<IBaseResource> containedResources = terser.getAllEmbeddedResources(theResource, false);
 
+		extractResourceLinksForContainedResources(theRequestPartitionId, theParams, theEntity, theResource, theTransactionDetails, theFailOnInvalidReference, theRequest, containedResources, new HashSet<>());
+	}
+
+	private void extractResourceLinksForContainedResources(RequestPartitionId theRequestPartitionId, ResourceIndexedSearchParams theParams, ResourceTable theEntity, IBaseResource theResource, TransactionDetails theTransactionDetails, boolean theFailOnInvalidReference, RequestDetails theRequest, Collection<IBaseResource> theContainedResources, Collection<IBaseResource> theAlreadySeenResources) {
+
 		// 2. Find referenced search parameters
 		ISearchParamExtractor.SearchParamSet<PathAndRef> referencedSearchParamSet = mySearchParamExtractor.extractResourceLinks(theResource, true);
 
@@ -445,15 +450,27 @@ public class SearchParamExtractorService {
 				continue;
 
 			// 3.2 find the contained resource
-			IBaseResource containedResource = findContainedResource(containedResources, nextPathAndRef.getRef());
+			IBaseResource containedResource = findContainedResource(theContainedResources, nextPathAndRef.getRef());
 			if (containedResource == null)
 				continue;
 
+			// 3.2.1 if we've already processed this resource upstream, do not process it again, to prevent infinite loops
+			if (theAlreadySeenResources.contains(containedResource)) {
+				continue;
+			}
+
 			currParams = new ResourceIndexedSearchParams();
 
 			// 3.3 create indexes for the current contained resource
 			extractResourceLinks(theRequestPartitionId, currParams, theEntity, containedResource, theTransactionDetails, theFailOnInvalidReference, theRequest);
 
+			// 3.4 recurse to process any other contained resources referenced by this one
+			if (myModelConfig.isIndexOnContainedResourcesRecursively()) {
+				HashSet<IBaseResource> nextAlreadySeenResources = new HashSet<>(theAlreadySeenResources);
+				nextAlreadySeenResources.add(containedResource);
+				extractResourceLinksForContainedResources(theRequestPartitionId, currParams, theEntity, containedResource, theTransactionDetails, theFailOnInvalidReference, theRequest, theContainedResources, nextAlreadySeenResources);
+			}
+
 			// 3.4 added reference name as a prefix for the contained resource if any
 			// e.g. for Observation.subject contained reference
 			// the SP_NAME = subject.family