From 3a9173c89bb84091317731e0aa7b64a25a31556a Mon Sep 17 00:00:00 2001 From: leif stawnyczy Date: Mon, 12 Aug 2024 18:22:12 -0400 Subject: [PATCH] refactor to try and handle duplicated search parameter --- .../ca/uhn/fhir/context/ModelScanner.java | 11 ++++++ ...t-update-can-sometimes-produce-errors.yaml | 11 ++++-- .../search/ExtendedHSearchSearchBuilder.java | 8 ++-- .../jpa/search/SearchStrategyFactory.java | 10 ++--- .../dao/r4/FhirResourceDaoR4SearchFtTest.java | 38 ------------------- .../provider/r4/ResourceProviderR4Test.java | 30 +++++++++++++++ .../resources/vm/jpa_resource_provider.vm | 2 + 7 files changed, 59 insertions(+), 51 deletions(-) diff --git a/hapi-fhir-base/src/main/java/ca/uhn/fhir/context/ModelScanner.java b/hapi-fhir-base/src/main/java/ca/uhn/fhir/context/ModelScanner.java index fdcad88f10c..aa291cd52ab 100644 --- a/hapi-fhir-base/src/main/java/ca/uhn/fhir/context/ModelScanner.java +++ b/hapi-fhir-base/src/main/java/ca/uhn/fhir/context/ModelScanner.java @@ -419,6 +419,17 @@ class ModelScanner { throw new ConfigurationException(Msg.code(1721) + "Search param " + searchParam.name() + " has an invalid type: " + searchParam.type()); } + /* + * This is a HACK! + * tinder generator jpa_resource_provider.vm already has _lastUpdated + * manually added, so we're just going to manually skip this value + * so we don't corrupt the PatientResourceProvider.java. + * + */ + if (searchParam.name().equalsIgnoreCase("_lastUpdated")) { + continue; + } + Set providesMembershipInCompartments; providesMembershipInCompartments = new HashSet<>(); for (Compartment next : searchParam.providesMembershipIn()) { diff --git a/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/7_4_0/6208-searches-on-last-update-can-sometimes-produce-errors.yaml b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/7_4_0/6208-searches-on-last-update-can-sometimes-produce-errors.yaml index 1c7ac7d367b..53dc6859a6b 100644 --- a/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/7_4_0/6208-searches-on-last-update-can-sometimes-produce-errors.yaml +++ b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/7_4_0/6208-searches-on-last-update-can-sometimes-produce-errors.yaml @@ -1,8 +1,11 @@ --- type: fix issue: 6208 -title: "Searching for resources by adding a parameter of `_lastUpdated` to the parameter - map (but not using the `_lastUpdated` special use-case parameter) while using - full text search could sometimes produce null pointers. - This has been fixed. +title: "Tinder generated ResourceProvider files were pulling in a duplicate `_lastUpdated` value. + This results in a broken signature for the providers, and a broken search. + As a workaround, we will skip search parameters named `_lastUpdated` because + these are already manually added to the search parameter map in jpa_resource_provider.vm. + + THIS IS ONLY TO FIX A REGRESSION FOR RELEASE! + A REAL FIX IS FORTHCOMING IN A FUTHRE VERSION. " diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/search/ExtendedHSearchSearchBuilder.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/search/ExtendedHSearchSearchBuilder.java index 23ddf4a1424..eb32b167f15 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/search/ExtendedHSearchSearchBuilder.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/search/ExtendedHSearchSearchBuilder.java @@ -270,10 +270,10 @@ public class ExtendedHSearchSearchBuilder { break; case DATE: - List> dateAndOrTerms = - nextParam.equalsIgnoreCase("_lastupdated") && searchParameterMap.getLastUpdated() != null - ? getLastUpdatedAndOrList(searchParameterMap) - : searchParameterMap.removeByNameUnmodified(nextParam); + boolean isLastUpdate = nextParam.equalsIgnoreCase("_lastupdated"); + List> dateAndOrTerms = isLastUpdate && searchParameterMap.getLastUpdated() != null + ? getLastUpdatedAndOrList(searchParameterMap) + : searchParameterMap.removeByNameUnmodified(nextParam); theBuilder.addDateUnmodifiedSearch(nextParam, dateAndOrTerms); break; diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/SearchStrategyFactory.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/SearchStrategyFactory.java index 5c251948a07..307c9af58ff 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/SearchStrategyFactory.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/SearchStrategyFactory.java @@ -59,11 +59,11 @@ public class SearchStrategyFactory { public boolean isSupportsHSearchDirect( String theResourceType, SearchParameterMap theParams, RequestDetails theRequestDetails) { return myFulltextSearchSvc != null - && myStorageSettings.isStoreResourceInHSearchIndex() - && myStorageSettings.isAdvancedHSearchIndexing() - && myFulltextSearchSvc.supportsAllOf(theParams) - && theParams.getSummaryMode() == null - && theParams.getSearchTotalMode() == null; + && myStorageSettings.isStoreResourceInHSearchIndex() + && myStorageSettings.isAdvancedHSearchIndexing() + && myFulltextSearchSvc.supportsAllOf(theParams) + && theParams.getSummaryMode() == null + && theParams.getSearchTotalMode() == null; } public ISearchStrategy makeDirectStrategy( diff --git a/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4SearchFtTest.java b/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4SearchFtTest.java index 7843693ce44..1daec9318e5 100644 --- a/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4SearchFtTest.java +++ b/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4SearchFtTest.java @@ -7,9 +7,6 @@ import ca.uhn.fhir.jpa.search.autocomplete.ValueSetAutocompleteOptions; import ca.uhn.fhir.jpa.searchparam.SearchParameterMap; import ca.uhn.fhir.jpa.test.BaseJpaR4Test; import ca.uhn.fhir.rest.api.Constants; -import ca.uhn.fhir.rest.api.server.IBundleProvider; -import ca.uhn.fhir.rest.api.server.SystemRequestDetails; -import ca.uhn.fhir.rest.param.DateParam; import ca.uhn.fhir.rest.param.QuantityParam; import ca.uhn.fhir.rest.param.StringAndListParam; import ca.uhn.fhir.rest.param.StringOrListParam; @@ -20,7 +17,6 @@ import ca.uhn.fhir.rest.server.servlet.ServletRequestDetails; import jakarta.servlet.http.HttpServletRequest; import org.assertj.core.api.AssertionsForInterfaceTypes; import org.hl7.fhir.instance.model.api.IIdType; -import org.hl7.fhir.r4.model.DateType; import org.hl7.fhir.r4.model.Device; import org.hl7.fhir.r4.model.Observation; import org.hl7.fhir.r4.model.Observation.ObservationStatus; @@ -31,12 +27,10 @@ import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import java.util.Arrays; -import java.util.Date; import java.util.List; import static org.assertj.core.api.Assertions.assertThat; import static org.hl7.fhir.r4.model.Observation.SP_VALUE_QUANTITY; -import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.fail; import static org.mockito.Mockito.mock; @@ -506,38 +500,6 @@ public class FhirResourceDaoR4SearchFtTest extends BaseJpaR4Test { } } - @Test - public void testSearchByUrl() { - // setup - DateType dt = new DateType(new Date()); - String nowStr = dt.getValueAsString(); - - SearchParameterMap map = new SearchParameterMap(); - map.setLoadSynchronous(true); - // we are explicitly not setting the _lastUpdated parameter - // but using a _lastUpdated parameter key - map.add("_lastUpdated", new DateParam(nowStr)); - - boolean storeResourceInHSearch = myStorageSettings.isStoreResourceInHSearchIndex(); - boolean advancedHSearch = myStorageSettings.isAdvancedHSearchIndexing(); - - try { - // use full text search - myStorageSettings.setStoreResourceInHSearchIndex(true); - myStorageSettings.setAdvancedHSearchIndexing(true); - - // test - IBundleProvider result = myPatientDao.search(map, new SystemRequestDetails()); - - // we just expect something - ie, no errors - assertNotNull(result); - } finally { - // reset back to previous - myStorageSettings.setAdvancedHSearchIndexing(advancedHSearch); - myStorageSettings.setStoreResourceInHSearchIndex(storeResourceInHSearch); - } - } - @Test public void testResourceQuantitySearch() { Observation obs1 = new Observation(); diff --git a/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/provider/r4/ResourceProviderR4Test.java b/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/provider/r4/ResourceProviderR4Test.java index c90a55b210c..87884305ccf 100644 --- a/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/provider/r4/ResourceProviderR4Test.java +++ b/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/provider/r4/ResourceProviderR4Test.java @@ -29,6 +29,7 @@ import ca.uhn.fhir.model.primitive.InstantDt; import ca.uhn.fhir.model.primitive.UriDt; import ca.uhn.fhir.parser.IParser; import ca.uhn.fhir.parser.StrictErrorHandler; +import ca.uhn.fhir.rest.api.CacheControlDirective; import ca.uhn.fhir.rest.api.Constants; import ca.uhn.fhir.rest.api.MethodOutcome; import ca.uhn.fhir.rest.api.PreferReturnEnum; @@ -1112,6 +1113,35 @@ public class ResourceProviderR4Test extends BaseResourceProviderR4Test { } + @Test + public void testSearchByUrl() { + // setup + DateType dt = new DateType(new Date()); + String nowStr = dt.getValueAsString(); + + boolean storeResourceInHSearch = myStorageSettings.isStoreResourceInHSearchIndex(); + boolean advancedHSearch = myStorageSettings.isAdvancedHSearchIndexing(); + + try { + // use full text search + myStorageSettings.setStoreResourceInHSearchIndex(true); + myStorageSettings.setAdvancedHSearchIndexing(true); + + // test + Bundle b = myClient.search() + .byUrl("Patient?_lastUpdated=" + nowStr) + .returnBundle(Bundle.class) + .cacheControl(CacheControlDirective.noCache()) + .execute(); + + assertNotNull(b); + } finally { + // reset back to previous + myStorageSettings.setAdvancedHSearchIndexing(advancedHSearch); + myStorageSettings.setStoreResourceInHSearchIndex(storeResourceInHSearch); + } + } + @Test public void testCreateConditionalWithPreferRepresentation() { Patient p = new Patient(); diff --git a/hapi-tinder-plugin/src/main/resources/vm/jpa_resource_provider.vm b/hapi-tinder-plugin/src/main/resources/vm/jpa_resource_provider.vm index 421c2a663d9..a8470f0e940 100644 --- a/hapi-tinder-plugin/src/main/resources/vm/jpa_resource_provider.vm +++ b/hapi-tinder-plugin/src/main/resources/vm/jpa_resource_provider.vm @@ -167,6 +167,8 @@ public class ${className}ResourceProvider extends #if ( $version != 'dstu' ) paramMap.setRevIncludes(theRevIncludes); paramMap.setLastUpdated(theLastUpdated); + // we will add this manually here because ModelScanner ignores it + paramMap.add(ca.uhn.fhir.rest.api.Constants.PARAM_LASTUPDATED, theLastUpdated); #end paramMap.setIncludes(theIncludes); paramMap.setSort(theSort);