From 875b224ac6b5a84a51afb6f3cdda04696e89a653 Mon Sep 17 00:00:00 2001 From: TipzCM Date: Mon, 15 Jul 2024 13:29:00 -0400 Subject: [PATCH] fixing canonical search (#6105) --- .../7_4_0/6094-fixing-canonical-search.yaml | 6 +++ .../extractor/BaseSearchParamExtractor.java | 20 ++++++++- .../FhirResourceDaoR4SearchIncludeTest.java | 8 +++- ...rResourceDaoR4StandardQueriesNoFTTest.java | 43 +++++++++++++++++++ 4 files changed, 74 insertions(+), 3 deletions(-) create mode 100644 hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/7_4_0/6094-fixing-canonical-search.yaml diff --git a/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/7_4_0/6094-fixing-canonical-search.yaml b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/7_4_0/6094-fixing-canonical-search.yaml new file mode 100644 index 00000000000..e094bf997ac --- /dev/null +++ b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/7_4_0/6094-fixing-canonical-search.yaml @@ -0,0 +1,6 @@ +--- +type: fix +issue: 6094 +title: "Fix for regression of searches for canonical uris using a version + (eg: http://example.com|1.2.3). +" 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 6ce98f6c290..a35dc11d554 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 @@ -2171,14 +2171,32 @@ public abstract class BaseSearchParamExtractor implements ISearchParamExtractor if (parsed.isAbsolute()) { String refValue = fakeReference.getReferenceElement().getValue(); + + myPathAndRef = new PathAndRef(theSearchParam.getName(), thePath, fakeReference, true); + theParams.add(myPathAndRef); + + /* + * If we have a versioned canonical uri, + * we will index both the version and unversioned uri + * (ie: uri|version and uri) + * This will allow searching to work on both versioned and non-versioned. + * + * HOWEVER + * This doesn't actually fix chained searching (MeasureReport?measure.identifier=...) + */ if (refValue.contains("|")) { + // extract the non-versioned AND the versioned above so both searches work. + fakeReference = (IBaseReference) myContext + .getElementDefinition("Reference") + .newInstance(); fakeReference.setReference(refValue.substring(0, refValue.indexOf('|'))); } myPathAndRef = new PathAndRef(theSearchParam.getName(), thePath, fakeReference, true); theParams.add(myPathAndRef); - break; } + + break; } theParams.addWarning("Ignoring canonical reference (indexing canonical is not yet supported)"); diff --git a/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4SearchIncludeTest.java b/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4SearchIncludeTest.java index a51f5423add..0e558751e7c 100644 --- a/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4SearchIncludeTest.java +++ b/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4SearchIncludeTest.java @@ -48,6 +48,7 @@ import java.util.stream.IntStream; import static org.assertj.core.api.Assertions.assertThat; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertTrue; import static org.junit.jupiter.api.Assertions.fail; import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.never; @@ -121,7 +122,6 @@ public class FhirResourceDaoR4SearchIncludeTest extends BaseJpaR4Test { qrIrrelevant.setQuestionnaire("http://fooIrrelevant"); myQuestionnaireResponseDao.update(qrIrrelevant, mySrd); - IBundleProvider outcome; IFhirResourceDao dao; SearchParameterMap map; @@ -182,7 +182,11 @@ public class FhirResourceDaoR4SearchIncludeTest extends BaseJpaR4Test { // (or somehow made things worse) and the search for the canonical target is no longer the 4th // SQL query assertThat(searchForCanonicalReferencesQuery.getSql(true, false)).contains("rispu1_0.HASH_IDENTITY in ('-600769180185160063')"); - assertThat(searchForCanonicalReferencesQuery.getSql(true, false)).contains("rispu1_0.SP_URI in ('http://foo')"); + assertTrue( + searchForCanonicalReferencesQuery.getSql(true, false).contains("rispu1_0.SP_URI in ('http://foo')") + || searchForCanonicalReferencesQuery.getSql(true, false).contains("rispu1_0.SP_URI in ('http://foo','http://foo|1.0')"), + searchForCanonicalReferencesQuery.getSql(true, false) + ); } } diff --git a/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4StandardQueriesNoFTTest.java b/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4StandardQueriesNoFTTest.java index f5121fdaf86..210146c4912 100644 --- a/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4StandardQueriesNoFTTest.java +++ b/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4StandardQueriesNoFTTest.java @@ -515,6 +515,49 @@ public class FhirResourceDaoR4StandardQueriesNoFTTest extends BaseJpaTest { myTestDaoSearch.assertSearchFindsOnly("search by server assigned id", "Patient?family=smith&_pid=" + id, id); } + @Nested + public class CanonicalReferences { + + @Test + void testCanonicalReferenceSearchNoVersion() { + // given + IIdType reportId = myDataBuilder.createResourceFromJson(""" + { + "resourceType": "MeasureReport", + "measure": "http://StructureDefinition.com" + } + """); + + myTestDaoSearch.assertSearchNotFound("unversioned search finds MeasureReport by canonical reference", + "MeasureReport?measure=http://StructureDefinition.com|1.2.3", reportId); + myTestDaoSearch.assertSearchFinds("versioned search finds MeasureReport by canonical reference with right version", + "MeasureReport?measure=http://StructureDefinition.com", reportId); + } + + /** + * Hapi bug - https://github.com/hapifhir/hapi-fhir/issues/6094 + */ + @Test + void testCanonicalReferenceSearch() { + // given + IIdType reportId = myDataBuilder.createResourceFromJson(""" + { + "resourceType": "MeasureReport", + "measure": "http://StructureDefinition.com|1.2.3" + } + """); + + // when + myTestDaoSearch.assertSearchFinds("unversioned search finds MeasureReport by canonical reference", + "MeasureReport?measure=http://StructureDefinition.com", reportId); + myTestDaoSearch.assertSearchFinds("versioned search finds MeasureReport by canonical reference with right version", + "MeasureReport?measure=http://StructureDefinition.com|1.2.3", reportId); + myTestDaoSearch.assertSearchNotFound("versioned search does not find MeasureReport by canonical reference with wrong version", + "MeasureReport?measure=http://StructureDefinition.com|2.0", reportId); + } + + } + @Test void testSortByPid() {