honeymoose
/
hapi-fhir
mirror of https://github.com/hapifhir/hapi-fhir.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Fix range calculation for number or quantity search parameters (#3871) 

* Fix range calculation for number or quantity search parameters

* Unify clause building for prefixed numeric parameters.
Fix range tests for quantity.

Co-authored-by: juan.marchionatto <juan.marchionatto@smilecdr.com>
This commit is contained in:
jmarchionatto 2022-08-03 15:44:10 -04:00 committed by GitHub
parent 71a3fa949c
commit a6c2e58c2c
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 85 additions and 126 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

118
hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/search/ExtendedHSearchClauseBuilder.java
View File

@ -48,6 +48,7 @@ import org.apache.commons.lang3.tuple.Pair;
import org.hibernate.search.engine.search.common.BooleanOperator;
import org.hibernate.search.engine.search.predicate.dsl.BooleanPredicateClausesStep;
import org.hibernate.search.engine.search.predicate.dsl.PredicateFinalStep;
import org.hibernate.search.engine.search.predicate.dsl.RangePredicateOptionsStep;
import org.hibernate.search.engine.search.predicate.dsl.SearchPredicateFactory;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@ -83,7 +84,6 @@ public class ExtendedHSearchClauseBuilder {
private static final Logger ourLog = LoggerFactory.getLogger(ExtendedHSearchClauseBuilder.class);
private static final double QTY_APPROX_TOLERANCE_PERCENT = .10;
private static final double QTY_TOLERANCE_PERCENT = .05;
final FhirContext myFhirContext;
public final SearchPredicateFactory myPredicateFactory;
@ -535,7 +535,7 @@ public class ExtendedHSearchClauseBuilder {
QuantityParam canonicalQty = UcumServiceUtil.toCanonicalQuantityOrNull(qtyParam);
if (canonicalQty != null) {
String valueFieldPath = fieldPath + "." + QTY_VALUE_NORM;
setPrefixedQuantityPredicate(theQuantityTerms, activePrefix, canonicalQty, valueFieldPath);
setPrefixedNumericPredicate(theQuantityTerms, activePrefix, canonicalQty.getValue(), valueFieldPath, true);
theQuantityTerms.must(myPredicateFactory.match()
.field(fieldPath + "." + QTY_CODE_NORM)
.matching(canonicalQty.getUnits()));
@ -545,7 +545,7 @@ public class ExtendedHSearchClauseBuilder {
// not NORMALIZED_QUANTITY_SEARCH_SUPPORTED or non-canonicalizable parameter
String valueFieldPath = fieldPath + "." + QTY_VALUE;
setPrefixedQuantityPredicate(theQuantityTerms, activePrefix, qtyParam, valueFieldPath);
setPrefixedNumericPredicate(theQuantityTerms, activePrefix, qtyParam.getValue(), valueFieldPath, true);
if ( isNotBlank(qtyParam.getSystem()) ) {
theQuantityTerms.must(
@ -561,74 +561,75 @@ public class ExtendedHSearchClauseBuilder {
}
private void setPrefixedQuantityPredicate(BooleanPredicateClausesStep<?> theQuantityTerms,
ParamPrefixEnum thePrefix, QuantityParam theQuantity, String valueFieldPath) {
private void setPrefixedNumericPredicate(BooleanPredicateClausesStep<?> theQuantityTerms,
ParamPrefixEnum thePrefix, BigDecimal theNumberValue, String valueFieldPath, boolean theIsMust) {
double value = theQuantity.getValue().doubleValue();
double value = theNumberValue.doubleValue();
Pair<BigDecimal, BigDecimal> range = NumericParamRangeUtil.getRange(theNumberValue);
double approxTolerance = value * QTY_APPROX_TOLERANCE_PERCENT;
double defaultTolerance = value * QTY_TOLERANCE_PERCENT;
switch (thePrefix) {
ParamPrefixEnum activePrefix = thePrefix == null ? ParamPrefixEnum.EQUAL : thePrefix;
switch (activePrefix) {
// searches for resource quantity between passed param value +/- 10%
case APPROXIMATE:
theQuantityTerms.must(
myPredicateFactory.range()
.field(valueFieldPath)
.between(value-approxTolerance, value+approxTolerance));
var predApp = myPredicateFactory.range().field(valueFieldPath)
.between(value-approxTolerance, value+approxTolerance);
addMustOrShouldPredicate(theQuantityTerms, predApp, theIsMust);
break;
// searches for resource quantity between passed param value +/- 5%
case EQUAL:
theQuantityTerms.must(
myPredicateFactory.range()
.field(valueFieldPath)
.between(value-defaultTolerance, value+defaultTolerance));
var predEq = myPredicateFactory.range().field(valueFieldPath)
.between(range.getLeft().doubleValue(), range.getRight().doubleValue());
addMustOrShouldPredicate(theQuantityTerms, predEq, theIsMust);
break;
// searches for resource quantity > param value
case GREATERTHAN:
case STARTS_AFTER: // treated as GREATERTHAN because search doesn't handle ranges
theQuantityTerms.must(
myPredicateFactory.range()
.field(valueFieldPath)
.greaterThan(value));
var predGt = myPredicateFactory.range().field(valueFieldPath).greaterThan(value);
addMustOrShouldPredicate(theQuantityTerms, predGt, theIsMust);
break;
// searches for resource quantity not < param value
case GREATERTHAN_OR_EQUALS:
theQuantityTerms.must(
myPredicateFactory.range()
.field(valueFieldPath)
.atLeast(value));
theQuantityTerms.must(myPredicateFactory.range().field(valueFieldPath).atLeast(value));
var predGe = myPredicateFactory.range().field(valueFieldPath).atLeast(value);
addMustOrShouldPredicate(theQuantityTerms, predGe, theIsMust);
break;
// searches for resource quantity < param value
case LESSTHAN:
case ENDS_BEFORE: // treated as LESSTHAN because search doesn't handle ranges
theQuantityTerms.must(
myPredicateFactory.range()
.field(valueFieldPath)
.lessThan(value));
var predLt = myPredicateFactory.range().field(valueFieldPath).lessThan(value);
addMustOrShouldPredicate(theQuantityTerms, predLt, theIsMust);
break;
// searches for resource quantity not > param value
// searches for resource quantity not > param value
case LESSTHAN_OR_EQUALS:
theQuantityTerms.must(
myPredicateFactory.range()
.field(valueFieldPath)
.atMost(value));
var predLe = myPredicateFactory.range().field(valueFieldPath).atMost(value);
addMustOrShouldPredicate(theQuantityTerms, predLe, theIsMust);
break;
// NOT_EQUAL: searches for resource quantity not between passed param value +/- 5%
// NOT_EQUAL: searches for resource quantity not between passed param value +/- 5%
case NOT_EQUAL:
theQuantityTerms.mustNot(
myPredicateFactory.range()
.field(valueFieldPath)
.between(value-defaultTolerance, value+defaultTolerance));
theQuantityTerms.mustNot(myPredicateFactory.range()
.field(valueFieldPath).between(range.getLeft().doubleValue(), range.getRight().doubleValue()));
break;
}
}
private void addMustOrShouldPredicate(BooleanPredicateClausesStep<?> theQuantityTerms,
RangePredicateOptionsStep<?> thePredicateToAdd, boolean theIsMust) {
if (theIsMust) {
theQuantityTerms.must(thePredicateToAdd);
} else {
theQuantityTerms.should(thePredicateToAdd);
}
}
public void addUriUnmodifiedSearch(String theParamName, List<List<IQueryParameterType>> theUriUnmodifiedAndOrTerms) {
for (List<IQueryParameterType> nextAnd : theUriUnmodifiedAndOrTerms) {
@ -653,46 +654,7 @@ public class ExtendedHSearchClauseBuilder {
numberPredicateStep.minimumShouldMatchNumber(1);
for (NumberParam orTerm : orTerms) {
double value = orTerm.getValue().doubleValue();
double approxTolerance = value * QTY_APPROX_TOLERANCE_PERCENT;
Pair<BigDecimal, BigDecimal> range = NumericParamRangeUtil.getRange(orTerm.getValue());
ParamPrefixEnum activePrefix = orTerm.getPrefix() == null ? ParamPrefixEnum.EQUAL : orTerm.getPrefix();
switch (activePrefix) {
case APPROXIMATE:
numberPredicateStep.should(myPredicateFactory.range()
.field(fieldPath).between(value - approxTolerance, value + approxTolerance));
break;
case EQUAL:
numberPredicateStep.should(myPredicateFactory.range()
.field(fieldPath).between(range.getLeft().doubleValue(), range.getRight().doubleValue()));
break;
case STARTS_AFTER:
case GREATERTHAN:
numberPredicateStep.should(myPredicateFactory.range().field(fieldPath).greaterThan(value));
break;
case GREATERTHAN_OR_EQUALS:
numberPredicateStep.should(myPredicateFactory.range().field(fieldPath).atLeast(value));
break;
case ENDS_BEFORE:
case LESSTHAN:
numberPredicateStep.should(myPredicateFactory.range().field(fieldPath).lessThan(value));
break;
case LESSTHAN_OR_EQUALS:
numberPredicateStep.should(myPredicateFactory.range().field(fieldPath).atMost(value));
break;
case NOT_EQUAL:
numberPredicateStep.mustNot(myPredicateFactory.range()
.field(fieldPath).between(range.getLeft().doubleValue(), range.getRight().doubleValue()));
break;
}
setPrefixedNumericPredicate(numberPredicateStep, orTerm.getPrefix(), orTerm.getValue(), fieldPath, false);
}
myRootClause.must(numberPredicateStep);

93
hapi-fhir-jpaserver-elastic-test-utilities/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4SearchWithElasticSearchIT.java
View File

@ -1096,9 +1096,6 @@ public class FhirResourceDaoR4SearchWithElasticSearchIT extends BaseJpaTest impl
assertNotFind("when gt unitless", "/Observation?value-quantity=0.7");
assertNotFind("when gt", "/Observation?value-quantity=0.7||mmHg");
assertFind("when a little gt - default is approx", "/Observation?value-quantity=0.599");
assertFind("when a little lt - default is approx", "/Observation?value-quantity=0.601");
assertFind("when eq unitless", "/Observation?value-quantity=0.6");
assertFind("when eq with units", "/Observation?value-quantity=0.6||mm[Hg]");
}
@ -1161,7 +1158,6 @@ public class FhirResourceDaoR4SearchWithElasticSearchIT extends BaseJpaTest impl
}
}
@Nested
public class CombinedQueries {
@ -1288,8 +1284,8 @@ public class FhirResourceDaoR4SearchWithElasticSearchIT extends BaseJpaTest impl
IIdType obs1Id = myObservationDao.create(obs1, mySrd).getId().toUnqualifiedVersionless();
Observation obs2 = getObservation();
addComponentWithCodeAndQuantity(obs2, "8480-6",307);
addComponentWithCodeAndQuantity(obs2, "8462-4",260);
addComponentWithCodeAndQuantity(obs2, "8480-6", 307);
addComponentWithCodeAndQuantity(obs2, "8462-4", 260);
myObservationDao.create(obs2, mySrd).getId().toUnqualifiedVersionless();
@ -1368,6 +1364,41 @@ public class FhirResourceDaoR4SearchWithElasticSearchIT extends BaseJpaTest impl
}
}
@Nested
public class SpecTestCases {
@Test
void specCase1() {
String id1 = withObservationWithValueQuantity(5.34).getIdPart();
String id2 = withObservationWithValueQuantity(5.36).getIdPart();
String id3 = withObservationWithValueQuantity(5.40).getIdPart();
String id4 = withObservationWithValueQuantity(5.44).getIdPart();
String id5 = withObservationWithValueQuantity(5.46).getIdPart();
// GET [base]/Observation?value-quantity=5.4 :: 5.4(+/-0.05)
assertFindIds("when le", Set.of(id2, id3, id4), "/Observation?value-quantity=5.4");
}
@Test
void specCase2() {
String id1 = withObservationWithValueQuantity(0.005394).getIdPart();
String id2 = withObservationWithValueQuantity(0.005395).getIdPart();
String id3 = withObservationWithValueQuantity(0.0054).getIdPart();
String id4 = withObservationWithValueQuantity(0.005404).getIdPart();
String id5 = withObservationWithValueQuantity(0.005406).getIdPart();
// GET [base]/Observation?value-quantity=5.40e-3 :: 0.0054(+/-0.000005)
assertFindIds("when le", Set.of(id2, id3, id4), "/Observation?value-quantity=5.40e-3");
}
@Test
void specCase6() {
String id1 = withObservationWithValueQuantity(4.85).getIdPart();
String id2 = withObservationWithValueQuantity(4.86).getIdPart();
String id3 = withObservationWithValueQuantity(5.94).getIdPart();
String id4 = withObservationWithValueQuantity(5.95).getIdPart();
// GET [base]/Observation?value-quantity=ap5.4 :: 5.4(+/- 10%) :: [4.86 ... 5.94]
assertFindIds("when le", Set.of(id2, id3), "/Observation?value-quantity=ap5.4");
}
}
}
@ -1398,8 +1429,6 @@ public class FhirResourceDaoR4SearchWithElasticSearchIT extends BaseJpaTest impl
assertFind("when eq UCUM 10*3/L ", "/Observation?value-quantity=60|" + UCUM_CODESYSTEM_URL + "|10*3/L");
assertFind("when eq UCUM 10*9/L", "/Observation?value-quantity=0.000060|" + UCUM_CODESYSTEM_URL + "|10*9/L");
assertFind("when gt UCUM 10*3/L", "/Observation?value-quantity=eq58|" + UCUM_CODESYSTEM_URL + "|10*3/L");
assertFind("when le UCUM 10*3/L", "/Observation?value-quantity=eq63|" + UCUM_CODESYSTEM_URL + "|10*3/L");
assertNotFind("when ne UCUM 10*3/L", "/Observation?value-quantity=80|" + UCUM_CODESYSTEM_URL + "|10*3/L");
assertNotFind("when gt UCUM 10*3/L", "/Observation?value-quantity=50|" + UCUM_CODESYSTEM_URL + "|10*3/L");
@ -2348,7 +2377,7 @@ public class FhirResourceDaoR4SearchWithElasticSearchIT extends BaseJpaTest impl
public class TestSpecCasesUsingSignificantFigures {
@Test
void NumberSpecCase1() {
void specCase1() {
String raId1 = createRiskAssessmentWithPredictionProbability(99.4).getIdPart();
String raId2 = createRiskAssessmentWithPredictionProbability(99.6).getIdPart();
String raId3 = createRiskAssessmentWithPredictionProbability(100.4).getIdPart();
@ -2358,7 +2387,7 @@ public class FhirResourceDaoR4SearchWithElasticSearchIT extends BaseJpaTest impl
}
@Test
void NumberSpecCase2() {
void specCase2() {
String raId1 = createRiskAssessmentWithPredictionProbability(99.994).getIdPart();
String raId2 = createRiskAssessmentWithPredictionProbability(99.996).getIdPart();
String raId3 = createRiskAssessmentWithPredictionProbability(100.004).getIdPart();
@ -2368,7 +2397,7 @@ public class FhirResourceDaoR4SearchWithElasticSearchIT extends BaseJpaTest impl
}
@Test
void NumberSpecCase3() {
void specCase3() {
String raId1 = createRiskAssessmentWithPredictionProbability(40).getIdPart();
String raId2 = createRiskAssessmentWithPredictionProbability(55).getIdPart();
String raId3 = createRiskAssessmentWithPredictionProbability(140).getIdPart();
@ -2379,7 +2408,7 @@ public class FhirResourceDaoR4SearchWithElasticSearchIT extends BaseJpaTest impl
}
@Test
void NumberSpecCase4() {
void specCase4() {
String raId1 = createRiskAssessmentWithPredictionProbability(99).getIdPart();
String raId2 = createRiskAssessmentWithPredictionProbability(100).getIdPart();
// [parameter]=lt100 Values that are less than exactly 100
@ -2387,7 +2416,7 @@ public class FhirResourceDaoR4SearchWithElasticSearchIT extends BaseJpaTest impl
}
@Test
void NumberSpecCase5() {
void specCase5() {
String raId1 = createRiskAssessmentWithPredictionProbability(99).getIdPart();
String raId2 = createRiskAssessmentWithPredictionProbability(100).getIdPart();
String raId3 = createRiskAssessmentWithPredictionProbability(101).getIdPart();
@ -2396,7 +2425,7 @@ public class FhirResourceDaoR4SearchWithElasticSearchIT extends BaseJpaTest impl
}
@Test
void NumberSpecCase6() {
void specCase6() {
String raId1 = createRiskAssessmentWithPredictionProbability(100).getIdPart();
String raId2 = createRiskAssessmentWithPredictionProbability(101).getIdPart();
// [parameter]=gt100 Values that are greater than exactly 100
@ -2404,7 +2433,7 @@ public class FhirResourceDaoR4SearchWithElasticSearchIT extends BaseJpaTest impl
}
@Test
void NumberSpecCase7() {
void specCase7() {
String raId1 = createRiskAssessmentWithPredictionProbability(99).getIdPart();
String raId2 = createRiskAssessmentWithPredictionProbability(100).getIdPart();
String raId3 = createRiskAssessmentWithPredictionProbability(101).getIdPart();
@ -2413,7 +2442,7 @@ public class FhirResourceDaoR4SearchWithElasticSearchIT extends BaseJpaTest impl
}
@Test
void NumberSpecCase8() {
void specCase8() {
String raId1 = createRiskAssessmentWithPredictionProbability(99.4).getIdPart();
String raId2 = createRiskAssessmentWithPredictionProbability(99.6).getIdPart();
String raId3 = createRiskAssessmentWithPredictionProbability(100).getIdPart();
@ -2423,38 +2452,6 @@ public class FhirResourceDaoR4SearchWithElasticSearchIT extends BaseJpaTest impl
assertFindIds("when le", Set.of(raId1, raId5), "/RiskAssessment?probability=ne100");
}
@Test
void QuantitySpecCase1() {
String raId1 = createRiskAssessmentWithPredictionProbability(5.34).getIdPart();
String raId2 = createRiskAssessmentWithPredictionProbability(5.36).getIdPart();
String raId3 = createRiskAssessmentWithPredictionProbability(5.40).getIdPart();
String raId4 = createRiskAssessmentWithPredictionProbability(5.44).getIdPart();
String raId5 = createRiskAssessmentWithPredictionProbability(5.46).getIdPart();
// GET [base]/Observation?value-quantity=5.4 :: 5.4(+/-0.05)
assertFindIds("when le", Set.of(raId2, raId3, raId4), "/RiskAssessment?probability=5.4");
}
@Test
void QuantitySpecCase2() {
String raId1 = createRiskAssessmentWithPredictionProbability(0.005394).getIdPart();
String raId2 = createRiskAssessmentWithPredictionProbability(0.005395).getIdPart();
String raId3 = createRiskAssessmentWithPredictionProbability(0.0054).getIdPart();
String raId4 = createRiskAssessmentWithPredictionProbability(0.005404).getIdPart();
String raId5 = createRiskAssessmentWithPredictionProbability(0.005406).getIdPart();
// GET [base]/Observation?value-quantity=5.40e-3 :: 0.0054(+/-0.000005)
assertFindIds("when le", Set.of(raId2, raId3, raId4), "/RiskAssessment?probability=5.40e-3");
}
@Test
void QuantitySpecCase6() {
String raId1 = createRiskAssessmentWithPredictionProbability(4.85).getIdPart();
String raId2 = createRiskAssessmentWithPredictionProbability(4.86).getIdPart();
String raId3 = createRiskAssessmentWithPredictionProbability(5.94).getIdPart();
String raId4 = createRiskAssessmentWithPredictionProbability(5.95).getIdPart();
// GET [base]/Observation?value-quantity=ap5.4 :: 5.4(+/- 10%) :: [4.86 ... 5.94]
assertFindIds("when le", Set.of(raId2, raId3), "/RiskAssessment?probability=ap5.4");
}
}
@Test