Add tests

This commit is contained in:
James Agnew 2024-07-03 16:57:22 -04:00
parent 6ed919de22
commit 8a5e2679d5
2 changed files with 160 additions and 11 deletions

View File

@ -1912,13 +1912,12 @@ public class SearchBuilder implements ISearchBuilder<JpaPid> {
if (comboParam != null) {
Collections.sort(comboParamNames);
if (!validateParamValuesAreValidForComboParam(theRequest, theParams, comboParamNames)) {
return;
// Since we're going to remove elements below
theParams.values().forEach(this::ensureSubListsAreWritable);
while (validateParamValuesAreValidForComboParam(theRequest, theParams, comboParamNames)) {
applyComboSearchParam(theQueryStack, theParams, theRequest, comboParamNames, comboParam);
}
// FIXME: abort if too many permutations
applyComboSearchParam(theQueryStack, theParams, theRequest, comboParamNames, comboParam);
}
}
@ -1928,8 +1927,6 @@ public class SearchBuilder implements ISearchBuilder<JpaPid> {
RequestDetails theRequest,
List<String> theComboParamNames,
RuntimeSearchParam theComboParam) {
// Since we're going to remove elements below
theParams.values().forEach(this::ensureSubListsAreWritable);
List<List<IQueryParameterType>> inputs = new ArrayList<>();
for (String nextParamName : theComboParamNames) {
@ -2016,15 +2013,19 @@ public class SearchBuilder implements ISearchBuilder<JpaPid> {
private boolean validateParamValuesAreValidForComboParam(
RequestDetails theRequest, @Nonnull SearchParameterMap theParams, List<String> theComboParamNames) {
boolean paramValuesAreValidForCombo = true;
List<List<IQueryParameterType>> paramOrValues = new ArrayList<>(theComboParamNames.size());
for (String nextParamName : theComboParamNames) {
List<List<IQueryParameterType>> nextValues = theParams.get(nextParamName);
if (nextValues.isEmpty()) {
if (nextValues == null || nextValues.isEmpty()) {
paramValuesAreValidForCombo = false;
break;
}
for (List<IQueryParameterType> nextAndValue : nextValues) {
List<IQueryParameterType> nextAndValue = nextValues.get(0);
paramOrValues.add(nextAndValue);
for (IQueryParameterType nextOrValue : nextAndValue) {
if (nextOrValue instanceof DateParam) {
DateParam dateParam = (DateParam) nextOrValue;
@ -2058,7 +2059,7 @@ public class SearchBuilder implements ISearchBuilder<JpaPid> {
break;
}
}
}
// Reference params are only eligible for using a composite index if they
// are qualified
@ -2073,6 +2074,13 @@ public class SearchBuilder implements ISearchBuilder<JpaPid> {
}
}
}
if (PermutationBuilder.calculatePermutationCount(paramOrValues) > 500) {
ourLog.debug(
"Search is not a candidate for unique combo searching - Too many OR values would result in too many permutations");
paramValuesAreValidForCombo = false;
}
return paramValuesAreValidForCombo;
}

View File

@ -7,10 +7,12 @@ import ca.uhn.fhir.jpa.searchparam.SearchParameterMap;
import ca.uhn.fhir.jpa.searchparam.submit.interceptor.SearchParamValidatingInterceptor;
import ca.uhn.fhir.jpa.util.SqlQuery;
import ca.uhn.fhir.rest.api.server.IBundleProvider;
import ca.uhn.fhir.rest.param.DateOrListParam;
import ca.uhn.fhir.rest.param.DateParam;
import ca.uhn.fhir.rest.param.ParamPrefixEnum;
import ca.uhn.fhir.rest.param.ReferenceParam;
import ca.uhn.fhir.rest.param.StringAndListParam;
import ca.uhn.fhir.rest.param.StringOrListParam;
import ca.uhn.fhir.rest.param.StringParam;
import ca.uhn.fhir.rest.param.TokenAndListParam;
import ca.uhn.fhir.rest.param.TokenOrListParam;
@ -430,6 +432,77 @@ public class FhirResourceDaoR4ComboNonUniqueParamTest extends BaseComboParamsR4T
assertThat(myMessages.get(0)).contains("This search uses an unqualified resource");
}
/**
* If there are two parameters as a part of a combo param, and we have
* multiple AND repetitions for both, then we can just join on the
* combo index twice.
*/
@Test
public void testMultipleAndCombinations_EqualNumbers() {
createStringAndStringCombo_FamilyAndGiven();
createPatient(
withId("A"),
withFamily("SIMPSON"), withGiven("HOMER"),
withFamily("jones"), withGiven("frank")
);
createPatient(
withId("B"),
withFamily("SIMPSON"), withGiven("MARGE")
);
SearchParameterMap params = SearchParameterMap.newSynchronous();
params.add("family", new StringAndListParam().addAnd(new StringParam("simpson")).addAnd(new StringParam("JONES")));
params.add("given", new StringAndListParam().addAnd(new StringParam("homer")).addAnd(new StringParam("frank")));
myCaptureQueriesListener.clear();
IBundleProvider results = myPatientDao.search(params, mySrd);
List<String> actual = toUnqualifiedVersionlessIdValues(results);
myCaptureQueriesListener.logSelectQueries();
assertThat(actual).contains("Patient/A");
String expected = "SELECT t0.RES_ID FROM HFJ_IDX_CMB_TOK_NU t0 INNER JOIN HFJ_IDX_CMB_TOK_NU t1 ON (t0.RES_ID = t1.RES_ID) WHERE ((t0.HASH_COMPLETE = '822090206952728926') AND (t1.HASH_COMPLETE = '-8088946700286918311'))";
assertEquals(expected, myCaptureQueriesListener.getSelectQueriesForCurrentThread().get(0).getSql(true, false));
}
/**
* If there are two parameters as a part of a combo param, and we have
* multiple AND repetitions for one but not the other, than we'll use
* the combo index for the first pair, but we don't create a second pair.
* We could probably optimize this to use the combo index for both, but
* it's not clear that this would actually help and this is probably
* a pretty contrived use case.
*/
@Test
public void testMultipleAndCombinations_NonEqualNumbers() {
createStringAndStringCombo_FamilyAndGiven();
createPatient(
withId("A"),
withFamily("SIMPSON"), withGiven("HOMER"),
withFamily("jones"), withGiven("frank")
);
createPatient(
withId("B"),
withFamily("SIMPSON"), withGiven("MARGE")
);
SearchParameterMap params = SearchParameterMap.newSynchronous();
params.add("family", new StringAndListParam().addAnd(new StringParam("simpson")).addAnd(new StringParam("JONES")));
params.add("given", new StringAndListParam().addAnd(new StringParam("homer")));
myCaptureQueriesListener.clear();
IBundleProvider results = myPatientDao.search(params, mySrd);
List<String> actual = toUnqualifiedVersionlessIdValues(results);
myCaptureQueriesListener.logSelectQueries();
assertThat(actual).contains("Patient/A");
String expected = "SELECT t0.RES_ID FROM HFJ_IDX_CMB_TOK_NU t0 INNER JOIN HFJ_SPIDX_STRING t1 ON (t0.RES_ID = t1.RES_ID) WHERE ((t0.HASH_COMPLETE = '822090206952728926') AND ((t1.HASH_NORM_PREFIX = '-3664262414674370905') AND (t1.SP_VALUE_NORMALIZED LIKE 'JONES%')))";
assertEquals(expected, myCaptureQueriesListener.getSelectQueriesForCurrentThread().get(0).getSql(true, false));
}
@Test
public void testOrQuery() {
createTokenAndReferenceCombo_FamilyAndOrganization();
@ -504,6 +577,46 @@ public class FhirResourceDaoR4ComboNonUniqueParamTest extends BaseComboParamsR4T
myMessages.clear();
}
private void createStringAndStringCombo_FamilyAndGiven() {
SearchParameter sp = new SearchParameter();
sp.setId("SearchParameter/patient-family");
sp.setType(Enumerations.SearchParamType.STRING);
sp.setCode("family");
sp.setExpression("Patient.name.family");
sp.setStatus(PublicationStatus.ACTIVE);
sp.addBase("Patient");
mySearchParameterDao.update(sp, mySrd);
sp = new SearchParameter();
sp.setId("SearchParameter/patient-given");
sp.setType(Enumerations.SearchParamType.STRING);
sp.setCode("given");
sp.setExpression("Patient.name.given");
sp.setStatus(PublicationStatus.ACTIVE);
sp.addBase("Patient");
mySearchParameterDao.update(sp, mySrd);
sp = new SearchParameter();
sp.setId("SearchParameter/patient-names");
sp.setType(Enumerations.SearchParamType.COMPOSITE);
sp.setStatus(PublicationStatus.ACTIVE);
sp.addBase("Patient");
sp.addComponent()
.setExpression("Patient")
.setDefinition("SearchParameter/patient-family");
sp.addComponent()
.setExpression("Patient")
.setDefinition("SearchParameter/patient-given");
sp.addExtension()
.setUrl(HapiExtensions.EXT_SP_UNIQUE)
.setValue(new BooleanType(false));
mySearchParameterDao.update(sp, mySrd);
mySearchParamRegistry.forceRefresh();
myMessages.clear();
}
private void createStringAndTokenCombo_NameAndGender() {
SearchParameter sp = new SearchParameter();
sp.setId("SearchParameter/patient-family");
@ -701,6 +814,34 @@ public class FhirResourceDaoR4ComboNonUniqueParamTest extends BaseComboParamsR4T
myMessages.clear();
}
@Test
public void testTooManyPermutations() {
// Test
StringOrListParam noteTextParam = new StringOrListParam();
DateOrListParam dateParam = new DateOrListParam();
for (int i = 0; i < 30; i++) {
noteTextParam.add(new StringParam("A" + i));
noteTextParam.add(new StringParam("B" + i));
noteTextParam.add(new StringParam("C" + i));
noteTextParam.add(new StringParam("D" + i));
dateParam.add(new DateParam("2020-01-" + String.format("%02d", i+1)));
}
SearchParameterMap params = SearchParameterMap
.newSynchronous()
.add("note-text", noteTextParam)
.add("date", dateParam);
myCaptureQueriesListener.clear();
IBundleProvider results = myObservationDao.search(params, mySrd);
assertThat(toUnqualifiedIdValues(results)).isEmpty();
// Verify
myCaptureQueriesListener.logSelectQueries();
String formatted = myCaptureQueriesListener.getSelectQueries().get(0).getSql(true, true);
assertThat(formatted).doesNotContain("HFJ_IDX_CMB_TOK_NU");
}
/**
* Can't create or search for combo params with dateTimes that don't have DAY precision
*/