refactor to try and handle duplicated search parameter

This commit is contained in:
leif stawnyczy 2024-08-12 18:22:12 -04:00
parent 675a318263
commit 3a9173c89b
7 changed files with 59 additions and 51 deletions

View File

@ -419,6 +419,17 @@ class ModelScanner {
throw new ConfigurationException(Msg.code(1721) + "Search param " + searchParam.name() throw new ConfigurationException(Msg.code(1721) + "Search param " + searchParam.name()
+ " has an invalid type: " + searchParam.type()); + " 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<String> providesMembershipInCompartments; Set<String> providesMembershipInCompartments;
providesMembershipInCompartments = new HashSet<>(); providesMembershipInCompartments = new HashSet<>();
for (Compartment next : searchParam.providesMembershipIn()) { for (Compartment next : searchParam.providesMembershipIn()) {

View File

@ -1,8 +1,11 @@
--- ---
type: fix type: fix
issue: 6208 issue: 6208
title: "Searching for resources by adding a parameter of `_lastUpdated` to the parameter title: "Tinder generated ResourceProvider files were pulling in a duplicate `_lastUpdated` value.
map (but not using the `_lastUpdated` special use-case parameter) while using This results in a broken signature for the providers, and a broken search.
full text search could sometimes produce null pointers. As a workaround, we will skip search parameters named `_lastUpdated` because
This has been fixed. 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.
" "

View File

@ -270,10 +270,10 @@ public class ExtendedHSearchSearchBuilder {
break; break;
case DATE: case DATE:
List<List<IQueryParameterType>> dateAndOrTerms = boolean isLastUpdate = nextParam.equalsIgnoreCase("_lastupdated");
nextParam.equalsIgnoreCase("_lastupdated") && searchParameterMap.getLastUpdated() != null List<List<IQueryParameterType>> dateAndOrTerms = isLastUpdate && searchParameterMap.getLastUpdated() != null
? getLastUpdatedAndOrList(searchParameterMap) ? getLastUpdatedAndOrList(searchParameterMap)
: searchParameterMap.removeByNameUnmodified(nextParam); : searchParameterMap.removeByNameUnmodified(nextParam);
theBuilder.addDateUnmodifiedSearch(nextParam, dateAndOrTerms); theBuilder.addDateUnmodifiedSearch(nextParam, dateAndOrTerms);
break; break;

View File

@ -59,11 +59,11 @@ public class SearchStrategyFactory {
public boolean isSupportsHSearchDirect( public boolean isSupportsHSearchDirect(
String theResourceType, SearchParameterMap theParams, RequestDetails theRequestDetails) { String theResourceType, SearchParameterMap theParams, RequestDetails theRequestDetails) {
return myFulltextSearchSvc != null return myFulltextSearchSvc != null
&& myStorageSettings.isStoreResourceInHSearchIndex() && myStorageSettings.isStoreResourceInHSearchIndex()
&& myStorageSettings.isAdvancedHSearchIndexing() && myStorageSettings.isAdvancedHSearchIndexing()
&& myFulltextSearchSvc.supportsAllOf(theParams) && myFulltextSearchSvc.supportsAllOf(theParams)
&& theParams.getSummaryMode() == null && theParams.getSummaryMode() == null
&& theParams.getSearchTotalMode() == null; && theParams.getSearchTotalMode() == null;
} }
public ISearchStrategy makeDirectStrategy( public ISearchStrategy makeDirectStrategy(

View File

@ -7,9 +7,6 @@ import ca.uhn.fhir.jpa.search.autocomplete.ValueSetAutocompleteOptions;
import ca.uhn.fhir.jpa.searchparam.SearchParameterMap; import ca.uhn.fhir.jpa.searchparam.SearchParameterMap;
import ca.uhn.fhir.jpa.test.BaseJpaR4Test; import ca.uhn.fhir.jpa.test.BaseJpaR4Test;
import ca.uhn.fhir.rest.api.Constants; 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.QuantityParam;
import ca.uhn.fhir.rest.param.StringAndListParam; import ca.uhn.fhir.rest.param.StringAndListParam;
import ca.uhn.fhir.rest.param.StringOrListParam; 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 jakarta.servlet.http.HttpServletRequest;
import org.assertj.core.api.AssertionsForInterfaceTypes; import org.assertj.core.api.AssertionsForInterfaceTypes;
import org.hl7.fhir.instance.model.api.IIdType; 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.Device;
import org.hl7.fhir.r4.model.Observation; import org.hl7.fhir.r4.model.Observation;
import org.hl7.fhir.r4.model.Observation.ObservationStatus; 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 org.junit.jupiter.api.Test;
import java.util.Arrays; import java.util.Arrays;
import java.util.Date;
import java.util.List; import java.util.List;
import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThat;
import static org.hl7.fhir.r4.model.Observation.SP_VALUE_QUANTITY; 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.junit.jupiter.api.Assertions.fail;
import static org.mockito.Mockito.mock; 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 @Test
public void testResourceQuantitySearch() { public void testResourceQuantitySearch() {
Observation obs1 = new Observation(); Observation obs1 = new Observation();

View File

@ -29,6 +29,7 @@ import ca.uhn.fhir.model.primitive.InstantDt;
import ca.uhn.fhir.model.primitive.UriDt; import ca.uhn.fhir.model.primitive.UriDt;
import ca.uhn.fhir.parser.IParser; import ca.uhn.fhir.parser.IParser;
import ca.uhn.fhir.parser.StrictErrorHandler; 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.Constants;
import ca.uhn.fhir.rest.api.MethodOutcome; import ca.uhn.fhir.rest.api.MethodOutcome;
import ca.uhn.fhir.rest.api.PreferReturnEnum; 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 @Test
public void testCreateConditionalWithPreferRepresentation() { public void testCreateConditionalWithPreferRepresentation() {
Patient p = new Patient(); Patient p = new Patient();

View File

@ -167,6 +167,8 @@ public class ${className}ResourceProvider extends
#if ( $version != 'dstu' ) #if ( $version != 'dstu' )
paramMap.setRevIncludes(theRevIncludes); paramMap.setRevIncludes(theRevIncludes);
paramMap.setLastUpdated(theLastUpdated); 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 #end
paramMap.setIncludes(theIncludes); paramMap.setIncludes(theIncludes);
paramMap.setSort(theSort); paramMap.setSort(theSort);