Apply fix from #3270 (#3712)

* Apply fix from #3270

* Rework compare method
This commit is contained in:
James Agnew 2022-06-16 15:46:14 -04:00 committed by GitHub
parent 93b89b2bd7
commit ac8f4f1e56
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 88 additions and 66 deletions

View File

@ -0,0 +1,5 @@
---
type: add
issue: 3269
title: "The SearchParameterMap query normalizer did not support `_include` and `_revinclude` parameters
with the `:recurse` qualifier. This has been corrected. Thanks to Augustas Vilčinskas for the PR!"

View File

@ -21,6 +21,7 @@ import ca.uhn.fhir.util.UrlUtil;
import com.fasterxml.jackson.annotation.JsonIgnore;
import org.apache.commons.lang3.StringUtils;
import org.apache.commons.lang3.Validate;
import org.apache.commons.lang3.builder.CompareToBuilder;
import org.apache.commons.lang3.builder.ToStringBuilder;
import org.apache.commons.lang3.builder.ToStringStyle;
@ -38,7 +39,9 @@ import java.util.Map;
import java.util.Set;
import java.util.stream.Collectors;
import static ca.uhn.fhir.rest.param.ParamPrefixEnum.*;
import static ca.uhn.fhir.rest.param.ParamPrefixEnum.GREATERTHAN_OR_EQUALS;
import static ca.uhn.fhir.rest.param.ParamPrefixEnum.LESSTHAN_OR_EQUALS;
import static ca.uhn.fhir.rest.param.ParamPrefixEnum.NOT_EQUAL;
import static org.apache.commons.lang3.StringUtils.isBlank;
import static org.apache.commons.lang3.StringUtils.isNotBlank;
@ -91,6 +94,13 @@ public class SearchParameterMap implements Serializable {
super();
}
/**
* Constructor
*/
public SearchParameterMap(String theName, IQueryParameterType theParam) {
add(theName, theParam);
}
/**
* Creates and returns a copy of this map
*/
@ -118,7 +128,7 @@ public class SearchParameterMap implements Serializable {
for (Map.Entry<String, List<List<IQueryParameterType>>> entry : mySearchParameterMap.entrySet()) {
List<List<IQueryParameterType>> andParams = entry.getValue();
List<List<IQueryParameterType>> newAndParams = new ArrayList<>();
for(List<IQueryParameterType> orParams: andParams) {
for (List<IQueryParameterType> orParams : andParams) {
List<IQueryParameterType> newOrParams = new ArrayList<>(orParams);
newAndParams.add(newOrParams);
}
@ -126,17 +136,9 @@ public class SearchParameterMap implements Serializable {
}
return map;
}
/**
* Constructor
*/
public SearchParameterMap(String theName, IQueryParameterType theParam) {
add(theName, theParam);
}
public SummaryEnum getSummaryMode() {
return mySummaryMode;
}
@ -234,6 +236,9 @@ public class SearchParameterMap implements Serializable {
for (Include nextInclude : list) {
addUrlParamSeparator(b);
b.append(paramName);
if (nextInclude.isRecurse()) {
b.append(Constants.PARAM_INCLUDE_QUALIFIER_RECURSE);
}
b.append('=');
b.append(UrlUtil.escapeUrlParam(nextInclude.getParamType()));
b.append(':');
@ -689,9 +694,8 @@ public class SearchParameterMap implements Serializable {
* Given a search parameter name and modifier (e.g. :text),
* get and remove all Search Parameters matching this name and modifier
*
* @param theName the query parameter key
* @param theName the query parameter key
* @param theModifier the qualifier you want to remove - nullable for unmodified params.
*
* @return an And/Or List of Query Parameters matching the qualifier.
*/
public List<List<IQueryParameterType>> removeByNameAndModifier(String theName, String theModifier) {
@ -706,7 +710,7 @@ public class SearchParameterMap implements Serializable {
for (List<IQueryParameterType> orList : andList) {
if (!orList.isEmpty() &&
StringUtils.defaultString(orList.get(0).getQueryParameterQualifier(), "")
.equals(theModifier)) {
.equals(theModifier)) {
matchingParameters.add(orList);
} else {
remainderParameters.add(orList);
@ -729,14 +733,14 @@ public class SearchParameterMap implements Serializable {
/**
* For each search parameter in the map, extract any which have the given qualifier.
* e.g. Take the url: Observation?code:text=abc&code=123&code:text=def&reason:text=somereason
*
* <p>
* If we call this function with `:text`, it will return a map that looks like:
*
* <p>
* code -> [[code:text=abc], [code:text=def]]
* reason -> [[reason:text=somereason]]
*
* <p>
* and the remaining search parameters in the map will be:
*
* <p>
* code -> [[code=123]]
*
* @param theQualifier
@ -819,6 +823,30 @@ public class SearchParameterMap implements Serializable {
}
}
static int compare(FhirContext theCtx, IQueryParameterType theO1, IQueryParameterType theO2) {
CompareToBuilder b = new CompareToBuilder();
b.append(theO1.getMissing(), theO2.getMissing());
b.append(theO1.getQueryParameterQualifier(), theO2.getQueryParameterQualifier());
if (b.toComparison() == 0) {
b.append(theO1.getValueAsQueryToken(theCtx), theO2.getValueAsQueryToken(theCtx));
}
return b.toComparison();
}
public static SearchParameterMap newSynchronous() {
SearchParameterMap retVal = new SearchParameterMap();
retVal.setLoadSynchronous(true);
return retVal;
}
public static SearchParameterMap newSynchronous(String theName, IQueryParameterType theParam) {
SearchParameterMap retVal = new SearchParameterMap();
retVal.setLoadSynchronous(true);
retVal.add(theName, theParam);
return retVal;
}
public static class IncludeComparator implements Comparator<Include> {
@Override
@ -865,50 +893,5 @@ public class SearchParameterMap implements Serializable {
}
private static int compare(FhirContext theCtx, IQueryParameterType theO1, IQueryParameterType theO2) {
int retVal;
if (theO1.getMissing() == null && theO2.getMissing() == null) {
retVal = 0;
} else if (theO1.getMissing() == null) {
retVal = -1;
} else if (theO2.getMissing() == null) {
retVal = 1;
} else if (ObjectUtil.equals(theO1.getMissing(), theO2.getMissing())) {
retVal = 0;
} else {
if (theO1.getMissing()) {
retVal = 1;
} else {
retVal = -1;
}
}
if (retVal == 0) {
String q1 = theO1.getQueryParameterQualifier();
String q2 = theO2.getQueryParameterQualifier();
retVal = StringUtils.compare(q1, q2);
}
if (retVal == 0) {
String v1 = theO1.getValueAsQueryToken(theCtx);
String v2 = theO2.getValueAsQueryToken(theCtx);
retVal = StringUtils.compare(v1, v2);
}
return retVal;
}
public static SearchParameterMap newSynchronous() {
SearchParameterMap retVal = new SearchParameterMap();
retVal.setLoadSynchronous(true);
return retVal;
}
public static SearchParameterMap newSynchronous(String theName, IQueryParameterType theParam) {
SearchParameterMap retVal = new SearchParameterMap();
retVal.setLoadSynchronous(true);
retVal.add(theName, theParam);
return retVal;
}
}

View File

@ -19,6 +19,7 @@ import java.time.Instant;
import java.util.HashSet;
import java.util.List;
import static ca.uhn.fhir.jpa.searchparam.SearchParameterMap.compare;
import static ca.uhn.fhir.rest.param.TokenParamModifier.TEXT;
import static org.hamcrest.CoreMatchers.is;
import static org.hamcrest.CoreMatchers.nullValue;
@ -180,4 +181,32 @@ class SearchParameterMapTest {
Assertions.assertEquals(orig.get("datetime"), clone.get("datetime"));
Assertions.assertEquals(orig.get("int"), clone.get("int"));
}
@Test
public void testCompareParameters() {
// Missing
assertEquals(0, compare(ourFhirContext, new StringParam().setMissing(true), new StringParam().setMissing(true)));
assertEquals(-1, compare(ourFhirContext, new StringParam("A"), new StringParam().setMissing(true)));
assertEquals(1, compare(ourFhirContext, new StringParam().setMissing(true), new StringParam("A")));
// Qualifier
assertEquals(0, compare(ourFhirContext, new StringParam("A").setContains(true), new StringParam("A").setContains(true)));
assertEquals(1, compare(ourFhirContext, new StringParam("A").setContains(true), new StringParam("A")));
assertEquals(-1, compare(ourFhirContext, new StringParam("A"), new StringParam("A").setContains(true)));
// Value
assertEquals(0, compare(ourFhirContext, new StringParam("A"), new StringParam("A")));
assertEquals(-1, compare(ourFhirContext, new StringParam("A"), new StringParam("B")));
assertEquals(1, compare(ourFhirContext, new StringParam("B"), new StringParam("A")));
// Value + Comparator (value should have no effect if comparator is changed)
assertEquals(1, compare(ourFhirContext, new StringParam("B").setContains(true), new StringParam("A")));
assertEquals(1, compare(ourFhirContext, new StringParam("A").setContains(true), new StringParam("B")));
}
}

View File

@ -90,8 +90,8 @@ public class SearchParameterMapTest {
String queryString = map.toNormalizedQueryString(ourCtx);
ourLog.info(queryString);
ourLog.info(UrlUtil.unescape(queryString));
assertEquals("?birthdate=ap2011&_include=Patient:aartvark&_include=Patient:aartvark:a&_include=Patient:aartvark:z&_include=Patient:subject", queryString);
assertEquals("?birthdate=ap2011&_include=Patient:aartvark&_include=Patient:aartvark:a&_include=Patient:aartvark:z&_include=Patient:subject", UrlUtil.unescape(queryString));
assertEquals("?birthdate=ap2011&_include:recurse=Patient:aartvark&_include=Patient:aartvark:a&_include=Patient:aartvark:z&_include=Patient:subject", queryString);
assertEquals("?birthdate=ap2011&_include:recurse=Patient:aartvark&_include=Patient:aartvark:a&_include=Patient:aartvark:z&_include=Patient:subject", UrlUtil.unescape(queryString));
}
@Test
@ -108,8 +108,8 @@ public class SearchParameterMapTest {
String queryString = map.toNormalizedQueryString(ourCtx);
ourLog.info(queryString);
ourLog.info(UrlUtil.unescape(queryString));
assertEquals("?birthdate=ap2011&_revinclude=Patient:aartvark&_revinclude=Patient:aartvark:a&_revinclude=Patient:aartvark:z&_revinclude=Patient:subject", queryString);
assertEquals("?birthdate=ap2011&_revinclude=Patient:aartvark&_revinclude=Patient:aartvark:a&_revinclude=Patient:aartvark:z&_revinclude=Patient:subject", UrlUtil.unescape(queryString));
assertEquals("?birthdate=ap2011&_revinclude:recurse=Patient:aartvark&_revinclude=Patient:aartvark:a&_revinclude=Patient:aartvark:z&_revinclude=Patient:subject", queryString);
assertEquals("?birthdate=ap2011&_revinclude:recurse=Patient:aartvark&_revinclude=Patient:aartvark:a&_revinclude=Patient:aartvark:z&_revinclude=Patient:subject", UrlUtil.unescape(queryString));
}
@Test

View File

@ -747,6 +747,11 @@
<name>Joe Shook</name>
<organization>Surescripts LLC</organization>
</developer>
<developer>
<id>vilaug</id>
<name>Augustas Vilčinskas</name>
<organization>Ivido</organization>
</developer>
</developers>
<licenses>