fix handling of common search parameters

hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/5_7_0/3374-chained-common-search-params.yaml

@@ -0,0 +1,4 @@
+---
+type: fix
+issue: 3374
+title: "Chained searches will handle common search parameters correctly when the `Index Contained Resources` configuration parameter is enabled."

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

@@ -731,18 +731,35 @@ public class QueryStack {
 	private class ChainElement {
 		private final String myResourceType;
 		private final RuntimeSearchParam mySearchParam;
+		private final String myPath;
 
 		public ChainElement(String theResourceType, RuntimeSearchParam theSearchParam) {
 			this.myResourceType = theResourceType;
 			this.mySearchParam = theSearchParam;
+			this.myPath = extractPath(theResourceType, theSearchParam);
 		}
 
 		public String getResourceType() {
 			return myResourceType;
 		}
 
-		public RuntimeSearchParam getSearchParam() {
+		public String getPath() { return myPath; }
-			return mySearchParam;
+		
+		public String getSearchParameterName() { return mySearchParam.getName(); }
+
+		private String extractPath(String theResourceType, RuntimeSearchParam theSearchParam) {
+			List<String> pathsForType = theSearchParam.getPathsSplit().stream()
+				.map(String::trim)
+				.filter(t -> t.startsWith(theResourceType))
+				.collect(Collectors.toList());
+			if (pathsForType.isEmpty()) {
+				ourLog.warn("Search parameter {} does not have a path for resource type {}.", theSearchParam.getName(), theResourceType);
+				return "";
+			} else if (pathsForType.size() > 1) {
+				ourLog.warn("Search parameter {} has multiple paths for resource type {}. Selecting {}", theSearchParam.getName(), theResourceType, pathsForType.get(0));
+			}
+
+			return pathsForType.get(0);
 		}
 
 		@Override
@@ -988,65 +1005,65 @@ public class QueryStack {
 		// Note: the first element in each chain is assumed to be discrete. This may need to change when we add proper support for `_contained`
 		if (nextChain.size() == 1) {
 			// discrete -> discrete
-			updateMapOfReferenceLinks(referenceLinks, Lists.newArrayList(nextChain.get(0).getSearchParam().getPath()), leafNodes);
+			updateMapOfReferenceLinks(referenceLinks, Lists.newArrayList(nextChain.get(0).getPath()), leafNodes);
 			// discrete -> contained
 			updateMapOfReferenceLinks(referenceLinks, Lists.newArrayList(),
 				leafNodes
 					.stream()
-					.map(t -> t.withPathPrefix(nextChain.get(0).getResourceType(), nextChain.get(0).getSearchParam().getName()))
+					.map(t -> t.withPathPrefix(nextChain.get(0).getResourceType(), nextChain.get(0).getSearchParameterName()))
 					.collect(Collectors.toSet()));
 		} else if (nextChain.size() == 2) {
 			// discrete -> discrete -> discrete
-			updateMapOfReferenceLinks(referenceLinks, Lists.newArrayList(nextChain.get(0).getSearchParam().getPath(), nextChain.get(1).getSearchParam().getPath()), leafNodes);
+			updateMapOfReferenceLinks(referenceLinks, Lists.newArrayList(nextChain.get(0).getPath(), nextChain.get(1).getPath()), leafNodes);
 			// discrete -> discrete -> contained
-			updateMapOfReferenceLinks(referenceLinks, Lists.newArrayList(nextChain.get(0).getSearchParam().getPath()),
+			updateMapOfReferenceLinks(referenceLinks, Lists.newArrayList(nextChain.get(0).getPath()),
 				leafNodes
 					.stream()
-					.map(t -> t.withPathPrefix(nextChain.get(1).getResourceType(), nextChain.get(1).getSearchParam().getName()))
+					.map(t -> t.withPathPrefix(nextChain.get(1).getResourceType(), nextChain.get(1).getSearchParameterName()))
 					.collect(Collectors.toSet()));
 			// discrete -> contained -> discrete
-			updateMapOfReferenceLinks(referenceLinks, Lists.newArrayList(mergePaths(nextChain.get(0).getSearchParam().getPath(), nextChain.get(1).getSearchParam().getPath())), leafNodes);
+			updateMapOfReferenceLinks(referenceLinks, Lists.newArrayList(mergePaths(nextChain.get(0).getPath(), nextChain.get(1).getPath())), leafNodes);
 			if (myModelConfig.isIndexOnContainedResourcesRecursively()) {
 				// discrete -> contained -> contained
 				updateMapOfReferenceLinks(referenceLinks, Lists.newArrayList(),
 					leafNodes
 						.stream()
-						.map(t -> t.withPathPrefix(nextChain.get(0).getResourceType(), nextChain.get(0).getSearchParam().getName() + "." + nextChain.get(1).getSearchParam().getName()))
+						.map(t -> t.withPathPrefix(nextChain.get(0).getResourceType(), nextChain.get(0).getSearchParameterName() + "." + nextChain.get(1).getSearchParameterName()))
 						.collect(Collectors.toSet()));
 			}
 		} else if (nextChain.size() == 3) {
 			// discrete -> discrete -> discrete -> discrete
-			updateMapOfReferenceLinks(referenceLinks, Lists.newArrayList(nextChain.get(0).getSearchParam().getPath(), nextChain.get(1).getSearchParam().getPath(), nextChain.get(2).getSearchParam().getPath()), leafNodes);
+			updateMapOfReferenceLinks(referenceLinks, Lists.newArrayList(nextChain.get(0).getPath(), nextChain.get(1).getPath(), nextChain.get(2).getPath()), leafNodes);
 			// discrete -> discrete -> discrete -> contained
-			updateMapOfReferenceLinks(referenceLinks, Lists.newArrayList(nextChain.get(0).getSearchParam().getPath(), nextChain.get(1).getSearchParam().getPath()),
+			updateMapOfReferenceLinks(referenceLinks, Lists.newArrayList(nextChain.get(0).getPath(), nextChain.get(1).getPath()),
 				leafNodes
 					.stream()
-					.map(t -> t.withPathPrefix(nextChain.get(2).getResourceType(), nextChain.get(2).getSearchParam().getName()))
+					.map(t -> t.withPathPrefix(nextChain.get(2).getResourceType(), nextChain.get(2).getSearchParameterName()))
 					.collect(Collectors.toSet()));
 			// discrete -> discrete -> contained -> discrete
-			updateMapOfReferenceLinks(referenceLinks, Lists.newArrayList(nextChain.get(0).getSearchParam().getPath(), mergePaths(nextChain.get(1).getSearchParam().getPath(), nextChain.get(2).getSearchParam().getPath())), leafNodes);
+			updateMapOfReferenceLinks(referenceLinks, Lists.newArrayList(nextChain.get(0).getPath(), mergePaths(nextChain.get(1).getPath(), nextChain.get(2).getPath())), leafNodes);
 			// discrete -> contained -> discrete -> discrete
-			updateMapOfReferenceLinks(referenceLinks, Lists.newArrayList(mergePaths(nextChain.get(0).getSearchParam().getPath(), nextChain.get(1).getSearchParam().getPath()), nextChain.get(2).getSearchParam().getPath()), leafNodes);
+			updateMapOfReferenceLinks(referenceLinks, Lists.newArrayList(mergePaths(nextChain.get(0).getPath(), nextChain.get(1).getPath()), nextChain.get(2).getPath()), leafNodes);
 			// discrete -> contained -> discrete -> contained
-			updateMapOfReferenceLinks(referenceLinks, Lists.newArrayList(mergePaths(nextChain.get(0).getSearchParam().getPath(), nextChain.get(1).getSearchParam().getPath())),
+			updateMapOfReferenceLinks(referenceLinks, Lists.newArrayList(mergePaths(nextChain.get(0).getPath(), nextChain.get(1).getPath())),
 				leafNodes
 					.stream()
-					.map(t -> t.withPathPrefix(nextChain.get(2).getResourceType(), nextChain.get(2).getSearchParam().getName()))
+					.map(t -> t.withPathPrefix(nextChain.get(2).getResourceType(), nextChain.get(2).getSearchParameterName()))
 					.collect(Collectors.toSet()));
 			if (myModelConfig.isIndexOnContainedResourcesRecursively()) {
 				// discrete -> contained -> contained -> discrete
-				updateMapOfReferenceLinks(referenceLinks, Lists.newArrayList(mergePaths(nextChain.get(0).getSearchParam().getPath(), nextChain.get(1).getSearchParam().getPath(), nextChain.get(2).getSearchParam().getPath())), leafNodes);
+				updateMapOfReferenceLinks(referenceLinks, Lists.newArrayList(mergePaths(nextChain.get(0).getPath(), nextChain.get(1).getPath(), nextChain.get(2).getPath())), leafNodes);
 				// discrete -> discrete -> contained -> contained
-				updateMapOfReferenceLinks(referenceLinks, Lists.newArrayList(nextChain.get(0).getSearchParam().getPath()),
+				updateMapOfReferenceLinks(referenceLinks, Lists.newArrayList(nextChain.get(0).getPath()),
 					leafNodes
 						.stream()
-						.map(t -> t.withPathPrefix(nextChain.get(1).getResourceType(), nextChain.get(1).getSearchParam().getName() + "." + nextChain.get(2).getSearchParam().getName()))
+						.map(t -> t.withPathPrefix(nextChain.get(1).getResourceType(), nextChain.get(1).getSearchParameterName() + "." + nextChain.get(2).getSearchParameterName()))
 						.collect(Collectors.toSet()));
 				// discrete -> contained -> contained -> contained
 				updateMapOfReferenceLinks(referenceLinks, Lists.newArrayList(),
 					leafNodes
 						.stream()
-						.map(t -> t.withPathPrefix(nextChain.get(0).getResourceType(), nextChain.get(0).getSearchParam().getName() + "." + nextChain.get(1).getSearchParam().getName() + "." + nextChain.get(2).getSearchParam().getName()))
+						.map(t -> t.withPathPrefix(nextChain.get(0).getResourceType(), nextChain.get(0).getSearchParameterName() + "." + nextChain.get(1).getSearchParameterName() + "." + nextChain.get(2).getSearchParameterName()))
 						.collect(Collectors.toSet()));
 			}
 		} else {

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

@@ -12,11 +12,14 @@ import ca.uhn.fhir.rest.api.server.IBundleProvider;
 import ca.uhn.fhir.rest.server.exceptions.InvalidRequestException;
 import org.hl7.fhir.instance.model.api.IIdType;
 import org.hl7.fhir.r4.model.Device;
+import org.hl7.fhir.r4.model.Encounter;
 import org.hl7.fhir.r4.model.IdType;
 import org.hl7.fhir.r4.model.Location;
 import org.hl7.fhir.r4.model.Observation;
 import org.hl7.fhir.r4.model.Organization;
 import org.hl7.fhir.r4.model.Patient;
+import org.hl7.fhir.r4.model.Quantity;
+import org.hl7.fhir.r4.model.Reference;
 import org.hl7.fhir.r4.model.StringType;
 import org.junit.jupiter.api.AfterEach;
 import org.junit.jupiter.api.BeforeEach;
@@ -130,6 +133,50 @@ public class ChainingR4SearchTest extends BaseJpaR4Test {
 		assertThat(oids, contains(oid1.getIdPart()));
 	}
 
+	@Test
+	public void testShouldResolveATwoLinkChainWithStandAloneResources_CommonReference() throws Exception {
+
+		// setup
+		myModelConfig.setIndexOnContainedResources(true);
+
+		IIdType oid1;
+
+		{
+			Patient p = new Patient();
+			p.setId(IdType.newRandomUuid());
+			p.addName().setFamily("Smith").addGiven("John");
+			myPatientDao.create(p, mySrd);
+
+			Encounter encounter = new Encounter();
+			encounter.setId(IdType.newRandomUuid());
+			encounter.addIdentifier().setSystem("foo").setValue("bar");
+			myEncounterDao.create(encounter, mySrd);
+
+			Observation obs = new Observation();
+			obs.getCode().setText("Body Weight");
+			obs.getCode().addCoding().setCode("obs2").setSystem("Some System").setDisplay("Body weight as measured by me");
+			obs.setStatus(Observation.ObservationStatus.FINAL);
+			obs.setValue(new Quantity(81));
+			obs.setSubject(new Reference(p.getId()));
+			obs.setEncounter(new Reference(encounter.getId()));
+			oid1 = myObservationDao.create(obs, mySrd).getId().toUnqualifiedVersionless();
+
+			// Create a dummy record so that an unconstrained query doesn't pass the test due to returning the only record
+			myObservationDao.create(new Observation(), mySrd);
+		}
+
+		String url = "/Observation?encounter.identifier=foo|bar";
+
+		// execute
+		myCaptureQueriesListener.clear();
+		List<String> oids = searchAndReturnUnqualifiedVersionlessIdValues(url);
+		myCaptureQueriesListener.logSelectQueries();
+
+		// validate
+		assertEquals(1L, oids.size());
+		assertThat(oids, contains(oid1.getIdPart()));
+	}
+
 	@Test
 	public void testShouldResolveATwoLinkChainWithAContainedResource() throws Exception {
 		// setup
@@ -330,6 +377,46 @@ public class ChainingR4SearchTest extends BaseJpaR4Test {
 		assertThat(oids, contains(oid1.getIdPart()));
 	}
 
+	@Test
+	public void testShouldResolveATwoLinkChainWithAContainedResource_CommonReference() throws Exception {
+
+		// setup
+		myModelConfig.setIndexOnContainedResources(true);
+
+		IIdType oid1;
+
+		{
+			Encounter encounter = new Encounter();
+			encounter.setId("enc");
+			encounter.addIdentifier().setSystem("foo").setValue("bar");
+
+			Observation obs = new Observation();
+			obs.getCode().setText("Body Weight");
+			obs.getCode().addCoding().setCode("obs2").setSystem("Some System").setDisplay("Body weight as measured by me");
+			obs.setStatus(Observation.ObservationStatus.FINAL);
+			obs.setValue(new Quantity(81));
+
+			obs.addContained(encounter);
+			obs.setEncounter(new Reference("#enc"));
+
+			oid1 = myObservationDao.create(obs, mySrd).getId().toUnqualifiedVersionless();
+
+			// Create a dummy record so that an unconstrained query doesn't pass the test due to returning the only record
+			myObservationDao.create(new Observation(), mySrd);
+		}
+
+		String url = "/Observation?encounter.identifier=foo|bar";
+
+		// execute
+		myCaptureQueriesListener.clear();
+		List<String> oids = searchAndReturnUnqualifiedVersionlessIdValues(url);
+		myCaptureQueriesListener.logSelectQueries();
+
+		// validate
+		assertEquals(1L, oids.size());
+		assertThat(oids, contains(oid1.getIdPart()));
+	}
+
 	@Test
 	public void testShouldResolveAThreeLinkChainWhereAllResourcesStandAloneWithoutContainedResourceIndexing() throws Exception {
 
@@ -477,6 +564,51 @@ public class ChainingR4SearchTest extends BaseJpaR4Test {
 		assertThat(oids, contains(oid1.getIdPart()));
 	}
 
+	@Test
+	public void testShouldResolveAThreeLinkChainWithAContainedResourceAtTheEndOfTheChain_CommonReference() throws Exception {
+
+		// setup
+		myModelConfig.setIndexOnContainedResources(true);
+
+		IIdType oid1;
+
+		{
+			Patient p = new Patient();
+			p.setId("pat");
+			p.addName().setFamily("Smith").addGiven("John");
+
+			Encounter encounter = new Encounter();
+			encounter.addContained(p);
+			encounter.setId(IdType.newRandomUuid());
+			encounter.addIdentifier().setSystem("foo").setValue("bar");
+			encounter.setSubject(new Reference("#pat"));
+			myEncounterDao.create(encounter, mySrd);
+
+			Observation obs = new Observation();
+			obs.getCode().setText("Body Weight");
+			obs.getCode().addCoding().setCode("obs2").setSystem("Some System").setDisplay("Body weight as measured by me");
+			obs.setStatus(Observation.ObservationStatus.FINAL);
+			obs.setValue(new Quantity(81));
+			obs.setSubject(new Reference(p.getId()));
+			obs.setEncounter(new Reference(encounter.getId()));
+			oid1 = myObservationDao.create(obs, mySrd).getId().toUnqualifiedVersionless();
+
+			// Create a dummy record so that an unconstrained query doesn't pass the test due to returning the only record
+			myObservationDao.create(new Observation(), mySrd);
+		}
+
+		String url = "/Observation?encounter.patient.name=Smith";
+
+		// execute
+		myCaptureQueriesListener.clear();
+		List<String> oids = searchAndReturnUnqualifiedVersionlessIdValues(url);
+		myCaptureQueriesListener.logSelectQueries();
+
+		// validate
+		assertEquals(1L, oids.size());
+		assertThat(oids, contains(oid1.getIdPart()));
+	}
+
 	@Test
 	public void testShouldResolveAThreeLinkChainWithAContainedResourceAtTheBeginningOfTheChain() throws Exception {
 		// Adding support for this case in SMILE-3151
@@ -518,6 +650,50 @@ public class ChainingR4SearchTest extends BaseJpaR4Test {
 		assertThat(oids, contains(oid1.getIdPart()));
 	}
 
+	@Test
+	public void testShouldResolveAThreeLinkChainWithAContainedResourceAtTheBeginningOfTheChain_CommonReference() throws Exception {
+
+		// setup
+		myModelConfig.setIndexOnContainedResources(true);
+
+		IIdType oid1;
+
+		{
+			Patient p = new Patient();
+			p.setId(IdType.newRandomUuid());
+			p.addName().setFamily("Smith").addGiven("John");
+			myPatientDao.create(p, mySrd);
+
+			Encounter encounter = new Encounter();
+			encounter.setId("enc");
+			encounter.addIdentifier().setSystem("foo").setValue("bar");
+			encounter.setSubject(new Reference(p.getId()));
+
+			Observation obs = new Observation();
+			obs.addContained(encounter);
+			obs.getCode().setText("Body Weight");
+			obs.getCode().addCoding().setCode("obs2").setSystem("Some System").setDisplay("Body weight as measured by me");
+			obs.setStatus(Observation.ObservationStatus.FINAL);
+			obs.setValue(new Quantity(81));
+			obs.setEncounter(new Reference("#enc"));
+			oid1 = myObservationDao.create(obs, mySrd).getId().toUnqualifiedVersionless();
+
+			// Create a dummy record so that an unconstrained query doesn't pass the test due to returning the only record
+			myObservationDao.create(new Observation(), mySrd);
+		}
+
+		String url = "/Observation?encounter.identifier=foo|bar";
+
+		// execute
+		myCaptureQueriesListener.clear();
+		List<String> oids = searchAndReturnUnqualifiedVersionlessIdValues(url);
+		myCaptureQueriesListener.logSelectQueries();
+
+		// validate
+		assertEquals(1L, oids.size());
+		assertThat(oids, contains(oid1.getIdPart()));
+	}
+
 	@Test
 	public void testShouldNotResolveAThreeLinkChainWithAllContainedResourcesWhenRecursiveContainedIndexesAreDisabled() throws Exception {