From d597cf2763f55c55f21d9dab857f67619d5aca05 Mon Sep 17 00:00:00 2001 From: James Agnew Date: Fri, 1 Dec 2023 15:19:51 -0500 Subject: [PATCH] Allow chained searches in Bundles where the fullUrl is fully qualified (#5529) * Allow chained searches in Bundles where the fullUrl is fully qualified * Add changelog * Spotless --- ...-in-bundle-where-fullurl-is-qualified.yaml | 4 ++ .../extractor/BaseSearchParamExtractor.java | 19 +++++- .../r4/FhirResourceDaoR4SearchNoFtTest.java | 65 ++++++++++++------- 3 files changed, 62 insertions(+), 26 deletions(-) create mode 100644 hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/7_0_0/5529-allow-chained-search-in-bundle-where-fullurl-is-qualified.yaml diff --git a/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/7_0_0/5529-allow-chained-search-in-bundle-where-fullurl-is-qualified.yaml b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/7_0_0/5529-allow-chained-search-in-bundle-where-fullurl-is-qualified.yaml new file mode 100644 index 00000000000..2fdbb4cb53d --- /dev/null +++ b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/7_0_0/5529-allow-chained-search-in-bundle-where-fullurl-is-qualified.yaml @@ -0,0 +1,4 @@ +--- +type: fix +issue: 5529 +title: "When using a chained SearchParameter to search within a Bundle as [described here](https://smilecdr.com/docs/fhir_storage_relational/chained_searches_and_sorts.html#document-and-message-search-parameters), if the `Bundle.entry.fullUrl` was fully qualified but the reference was not, the search did not work. This has been corrected." diff --git a/hapi-fhir-jpaserver-searchparam/src/main/java/ca/uhn/fhir/jpa/searchparam/extractor/BaseSearchParamExtractor.java b/hapi-fhir-jpaserver-searchparam/src/main/java/ca/uhn/fhir/jpa/searchparam/extractor/BaseSearchParamExtractor.java index a4a4425071c..8ed714eb385 100644 --- a/hapi-fhir-jpaserver-searchparam/src/main/java/ca/uhn/fhir/jpa/searchparam/extractor/BaseSearchParamExtractor.java +++ b/hapi-fhir-jpaserver-searchparam/src/main/java/ca/uhn/fhir/jpa/searchparam/extractor/BaseSearchParamExtractor.java @@ -63,6 +63,7 @@ import org.hl7.fhir.instance.model.api.IBaseReference; import org.hl7.fhir.instance.model.api.IBaseResource; import org.hl7.fhir.instance.model.api.IIdType; import org.hl7.fhir.instance.model.api.IPrimitiveType; +import org.hl7.fhir.r4.model.IdType; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.context.ApplicationContext; @@ -2010,17 +2011,31 @@ public abstract class BaseSearchParamExtractor implements ISearchParamExtractor * references within a Bundle */ if (theAppContext instanceof IBaseBundle && isNotBlank(theUrl) && !theUrl.startsWith("#")) { + String unqualifiedVersionlessReference; + boolean isPlaceholderReference; + if (theUrl.startsWith("urn:")) { + isPlaceholderReference = true; + unqualifiedVersionlessReference = null; + } else { + isPlaceholderReference = false; + unqualifiedVersionlessReference = + new IdType(theUrl).toUnqualifiedVersionless().getValue(); + } + List entries = BundleUtil.toListOfEntries(getContext(), (IBaseBundle) theAppContext); for (BundleEntryParts next : entries) { if (next.getResource() != null) { - if (theUrl.startsWith("urn:uuid:")) { + if (isPlaceholderReference) { if (theUrl.equals(next.getUrl()) || theUrl.equals( next.getResource().getIdElement().getValue())) { return (T) next.getResource(); } } else { - if (theUrl.equals(next.getResource().getIdElement().getValue())) { + if (unqualifiedVersionlessReference.equals(next.getResource() + .getIdElement() + .toUnqualifiedVersionless() + .getValue())) { return (T) next.getResource(); } } diff --git a/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4SearchNoFtTest.java b/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4SearchNoFtTest.java index 779cd067177..826f6ffad95 100644 --- a/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4SearchNoFtTest.java +++ b/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4SearchNoFtTest.java @@ -5793,8 +5793,15 @@ public class FhirResourceDaoR4SearchNoFtTest extends BaseJpaR4Test { * [base]/Bundle?composition.patient.identifier=foo */ @ParameterizedTest - @CsvSource({"urn:uuid:5c34dc2c-9b5d-4ec1-b30b-3e2d4371508b", "Patient/ABC"}) - public void testCreateAndSearchForFullyChainedSearchParameter(String thePatientId) { + @CsvSource({ + "true , urn:uuid:5c34dc2c-9b5d-4ec1-b30b-3e2d4371508b , urn:uuid:5c34dc2c-9b5d-4ec1-b30b-3e2d4371508b", + "false, urn:uuid:5c34dc2c-9b5d-4ec1-b30b-3e2d4371508b , urn:uuid:5c34dc2c-9b5d-4ec1-b30b-3e2d4371508b", + "true , Patient/ABC , Patient/ABC ", + "false, Patient/ABC , Patient/ABC ", + "true , Patient/ABC , http://example.com/fhir/Patient/ABC ", + "false, Patient/ABC , http://example.com/fhir/Patient/ABC ", + }) + public void testCreateAndSearchForFullyChainedSearchParameter(boolean theUseFullChainInName, String thePatientId, String theFullUrl) { // Setup 1 myStorageSettings.setIndexMissingFields(JpaStorageSettings.IndexEnabledEnum.DISABLED); @@ -5819,13 +5826,18 @@ public class FhirResourceDaoR4SearchNoFtTest extends BaseJpaR4Test { composition.setSubject(new Reference(thePatientId)); Patient patient = new Patient(); - patient.setId(new IdType(thePatientId)); + patient.setId(new IdType(theFullUrl)); patient.addIdentifier().setSystem("http://foo").setValue("bar"); Bundle bundle = new Bundle(); bundle.setType(Bundle.BundleType.DOCUMENT); - bundle.addEntry().setResource(composition); - bundle.addEntry().setResource(patient); + bundle + .addEntry() + .setResource(composition); + bundle + .addEntry() + .setFullUrl(theFullUrl) + .setResource(patient); myBundleDao.create(bundle, mySrd); @@ -5833,35 +5845,40 @@ public class FhirResourceDaoR4SearchNoFtTest extends BaseJpaR4Test { bundle2.setType(Bundle.BundleType.DOCUMENT); myBundleDao.create(bundle2, mySrd); - // Verify 1 - runInTransaction(() -> { + // Test + + SearchParameterMap map; + if (theUseFullChainInName) { + map = SearchParameterMap.newSynchronous("composition.patient.identifier", new TokenParam("http://foo", "bar")); + } else { + map = SearchParameterMap.newSynchronous("composition", new ReferenceParam("patient.identifier", "http://foo|bar")); + } + IBundleProvider outcome = myBundleDao.search(map, mySrd); + + // Verify + + List params = extractAllTokenIndexes(); + assertThat(params.toString(), params, containsInAnyOrder( + "composition.patient.identifier http://foo|bar" + )); + assertEquals(1, outcome.size()); + } + + private List extractAllTokenIndexes() { + List params = runInTransaction(() -> { logAllTokenIndexes(); - List params = myResourceIndexedSearchParamTokenDao + return myResourceIndexedSearchParamTokenDao .findAll() .stream() .filter(t -> t.getParamName().contains(".")) .map(t -> t.getParamName() + " " + t.getSystem() + "|" + t.getValue()) .toList(); - assertThat(params.toString(), params, containsInAnyOrder( - "composition.patient.identifier http://foo|bar" - )); }); - - // Test 2 - IBundleProvider outcome; - - SearchParameterMap map = SearchParameterMap - .newSynchronous("composition.patient.identifier", new TokenParam("http://foo", "bar")); - outcome = myBundleDao.search(map, mySrd); - assertEquals(1, outcome.size()); - - map = SearchParameterMap - .newSynchronous("composition", new ReferenceParam("patient.identifier", "http://foo|bar")); - outcome = myBundleDao.search(map, mySrd); - assertEquals(1, outcome.size()); + return params; } + @Nested public class TagBelowTests {