From d2c0b21773460229152b223cfe83e58fb0a5be5f Mon Sep 17 00:00:00 2001 From: Anthony Sute Date: Thu, 14 Nov 2019 15:05:23 -0500 Subject: [PATCH] (1) Changes to equal and not-equal operator logic for numerics to use exact values per discussions with Grahame and James (2) Additional unit tests (3) Changes to tests affected by (1) above --- .../ca/uhn/fhir/jpa/dao/SearchBuilder.java | 45 +--- .../dao/dstu3/FhirResourceDaoDstu3Test.java | 8 +- .../dao/r4/FhirResourceDaoR4FilterTest.java | 226 +++++++++++++++++- .../jpa/dao/r4/FhirResourceDaoR4Test.java | 8 +- 4 files changed, 242 insertions(+), 45 deletions(-) diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/SearchBuilder.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/SearchBuilder.java index 49ae341a506..409946652da 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/SearchBuilder.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/SearchBuilder.java @@ -1467,29 +1467,9 @@ public class SearchBuilder implements ISearchBuilder { BigDecimal theValue, final Expression thePath, String invalidMessageName) { - return createPredicateNumeric(theResourceName, - theParamName, - theFrom, - builder, - theParam, - thePrefix, - theValue, - thePath, - invalidMessageName, - null); - } - - private Predicate createPredicateNumeric(String theResourceName, - String theParamName, - From theFrom, - CriteriaBuilder builder, - IQueryParameterType theParam, - ParamPrefixEnum thePrefix, - BigDecimal theValue, - final Expression thePath, - String invalidMessageName, - SearchFilterParser.CompareOperation operation) { Predicate num; + // Per discussions with Grahame Grieve and James Agnew on 11/13/19, modified logic for EQUAL and NOT_EQUAL operators below so as to + // use exact value matching. The "fuzz amount" matching is still used with the APPROXIMATE operator. switch (thePrefix) { case GREATERTHAN: num = builder.gt(thePath, theValue); @@ -1503,25 +1483,22 @@ public class SearchBuilder implements ISearchBuilder { case LESSTHAN_OR_EQUALS: num = builder.le(thePath, theValue); break; - case APPROXIMATE: case EQUAL: + num = builder.equal(thePath, theValue); + break; case NOT_EQUAL: + num = builder.notEqual(thePath, theValue); + break; + case APPROXIMATE: BigDecimal mul = calculateFuzzAmount(thePrefix, theValue); BigDecimal low = theValue.subtract(mul, MathContext.DECIMAL64); BigDecimal high = theValue.add(mul, MathContext.DECIMAL64); Predicate lowPred; Predicate highPred; - if (thePrefix != ParamPrefixEnum.NOT_EQUAL) { - lowPred = builder.ge(thePath.as(BigDecimal.class), low); - highPred = builder.le(thePath.as(BigDecimal.class), high); - num = builder.and(lowPred, highPred); - ourLog.trace("Searching for {} <= val <= {}", low, high); - } else { - // Prefix was "ne", so reverse it! - lowPred = builder.lt(thePath.as(BigDecimal.class), low); - highPred = builder.gt(thePath.as(BigDecimal.class), high); - num = builder.or(lowPred, highPred); - } + lowPred = builder.ge(thePath.as(BigDecimal.class), low); + highPred = builder.le(thePath.as(BigDecimal.class), high); + num = builder.and(lowPred, highPred); + ourLog.trace("Searching for {} <= val <= {}", low, high); break; case ENDS_BEFORE: case STARTS_AFTER: diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/dstu3/FhirResourceDaoDstu3Test.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/dstu3/FhirResourceDaoDstu3Test.java index a4e00e8c9c2..bbfe07992da 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/dstu3/FhirResourceDaoDstu3Test.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/dstu3/FhirResourceDaoDstu3Test.java @@ -359,12 +359,12 @@ public class FhirResourceDaoDstu3Test extends BaseJpaDstu3Test { { IBundleProvider found = myObservationDao.search(new SearchParameterMap(Observation.SP_VALUE_QUANTITY, new QuantityParam("123", "foo", "bar")).setLoadSynchronous(true)); List list = toUnqualifiedVersionlessIds(found); - assertThat(list, containsInAnyOrder(id3)); + assertThat(list, Matchers.empty()); } { IBundleProvider found = myObservationDao.search(new SearchParameterMap(Observation.SP_VALUE_QUANTITY, new QuantityParam("123.0", "foo", "bar")).setLoadSynchronous(true)); List list = toUnqualifiedVersionlessIds(found); - assertThat(list, containsInAnyOrder(id3)); + assertThat(list, Matchers.empty()); } { IBundleProvider found = myObservationDao.search(new SearchParameterMap(Observation.SP_VALUE_QUANTITY, new QuantityParam("123.01", "foo", "bar")).setLoadSynchronous(true)); @@ -379,12 +379,12 @@ public class FhirResourceDaoDstu3Test extends BaseJpaDstu3Test { { IBundleProvider found = myObservationDao.search(new SearchParameterMap(Observation.SP_VALUE_QUANTITY, new QuantityParam("123.02", "foo", "bar")).setLoadSynchronous(true)); List list = toUnqualifiedVersionlessIds(found); - assertThat(list, containsInAnyOrder()); + assertThat(list, Matchers.empty()); } { IBundleProvider found = myObservationDao.search(new SearchParameterMap(Observation.SP_VALUE_QUANTITY, new QuantityParam("123.001", "foo", "bar")).setLoadSynchronous(true)); List list = toUnqualifiedVersionlessIds(found); - assertThat(list, containsInAnyOrder()); + assertThat(list, Matchers.empty()); } } diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4FilterTest.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4FilterTest.java index 622dd21be39..52d0fca99b3 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4FilterTest.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4FilterTest.java @@ -8,9 +8,7 @@ import ca.uhn.fhir.rest.param.StringParam; import ca.uhn.fhir.rest.server.exceptions.InvalidRequestException; import org.hamcrest.Matchers; import org.hl7.fhir.instance.model.api.IIdType; -import org.hl7.fhir.r4.model.CarePlan; -import org.hl7.fhir.r4.model.DateType; -import org.hl7.fhir.r4.model.Patient; +import org.hl7.fhir.r4.model.*; import org.junit.After; import org.junit.AfterClass; import org.junit.Before; @@ -723,6 +721,228 @@ public class FhirResourceDaoR4FilterTest extends BaseJpaR4Test { } + @Test + public void testNumericComparatorEq() { + + RiskAssessment ra1 = new RiskAssessment(); + RiskAssessment ra2 = new RiskAssessment(); + + RiskAssessment.RiskAssessmentPredictionComponent component = new RiskAssessment.RiskAssessmentPredictionComponent(); + DecimalType doseNumber = new DecimalType(0.25); + component.setProbability(doseNumber); + ra1.addPrediction(component); + String raId1 = myRiskAssessmentDao.create(ra1).getId().toUnqualifiedVersionless().getValue(); + + component = new RiskAssessment.RiskAssessmentPredictionComponent(); + doseNumber = new DecimalType(0.3); + component.setProbability(doseNumber); + ra2.addPrediction(component); + String raId2 = myRiskAssessmentDao.create(ra2).getId().toUnqualifiedVersionless().getValue(); + + SearchParameterMap map; + List found; + + map = new SearchParameterMap(); + map.setLoadSynchronous(true); + map.add(Constants.PARAM_FILTER, new StringParam("probability eq 0.25")); + found = toUnqualifiedVersionlessIdValues(myRiskAssessmentDao.search(map)); + assertThat(found, containsInAnyOrder(raId1)); + + map = new SearchParameterMap(); + map.setLoadSynchronous(true); + map.add(Constants.PARAM_FILTER, new StringParam("probability eq 0.3")); + found = toUnqualifiedVersionlessIdValues(myRiskAssessmentDao.search(map)); + assertThat(found, containsInAnyOrder(raId2)); + + map = new SearchParameterMap(); + map.setLoadSynchronous(true); + map.add(Constants.PARAM_FILTER, new StringParam("probability eq 0.1")); + found = toUnqualifiedVersionlessIdValues(myRiskAssessmentDao.search(map)); + assertThat(found, Matchers.empty()); + + } + + @Test + public void testNumericComparatorNe() { + + RiskAssessment ra1 = new RiskAssessment(); + RiskAssessment ra2 = new RiskAssessment(); + + RiskAssessment.RiskAssessmentPredictionComponent component = new RiskAssessment.RiskAssessmentPredictionComponent(); + DecimalType doseNumber = new DecimalType(0.25); + component.setProbability(doseNumber); + ra1.addPrediction(component); + String raId1 = myRiskAssessmentDao.create(ra1).getId().toUnqualifiedVersionless().getValue(); + + component = new RiskAssessment.RiskAssessmentPredictionComponent(); + doseNumber = new DecimalType(0.3); + component.setProbability(doseNumber); + ra2.addPrediction(component); + String raId2 = myRiskAssessmentDao.create(ra2).getId().toUnqualifiedVersionless().getValue(); + + SearchParameterMap map; + List found; + + map = new SearchParameterMap(); + map.setLoadSynchronous(true); + map.add(Constants.PARAM_FILTER, new StringParam("probability ne 0.25")); + found = toUnqualifiedVersionlessIdValues(myRiskAssessmentDao.search(map)); + assertThat(found, containsInAnyOrder(raId2)); + + map = new SearchParameterMap(); + map.setLoadSynchronous(true); + map.add(Constants.PARAM_FILTER, new StringParam("probability ne 0.3")); + found = toUnqualifiedVersionlessIdValues(myRiskAssessmentDao.search(map)); + assertThat(found, containsInAnyOrder(raId1)); + + map = new SearchParameterMap(); + map.setLoadSynchronous(true); + map.add(Constants.PARAM_FILTER, new StringParam("probability ne 0.1")); + found = toUnqualifiedVersionlessIdValues(myRiskAssessmentDao.search(map)); + assertThat(found, containsInAnyOrder(raId1, raId2)); + + } + + @Test + public void testNumericComparatorGt() { + + RiskAssessment ra1 = new RiskAssessment(); + RiskAssessment ra2 = new RiskAssessment(); + + RiskAssessment.RiskAssessmentPredictionComponent component = new RiskAssessment.RiskAssessmentPredictionComponent(); + DecimalType doseNumber = new DecimalType(0.25); + component.setProbability(doseNumber); + ra1.addPrediction(component); + String raId1 = myRiskAssessmentDao.create(ra1).getId().toUnqualifiedVersionless().getValue(); + + component = new RiskAssessment.RiskAssessmentPredictionComponent(); + doseNumber = new DecimalType(0.3); + component.setProbability(doseNumber); + ra2.addPrediction(component); + String raId2 = myRiskAssessmentDao.create(ra2).getId().toUnqualifiedVersionless().getValue(); + + SearchParameterMap map; + List found; + + map = new SearchParameterMap(); + map.setLoadSynchronous(true); + map.add(Constants.PARAM_FILTER, new StringParam("probability gt 0.25")); + found = toUnqualifiedVersionlessIdValues(myRiskAssessmentDao.search(map)); + assertThat(found, containsInAnyOrder(raId2)); + + map = new SearchParameterMap(); + map.setLoadSynchronous(true); + map.add(Constants.PARAM_FILTER, new StringParam("probability gt 0.3")); + found = toUnqualifiedVersionlessIdValues(myRiskAssessmentDao.search(map)); + assertThat(found, Matchers.empty()); + + } + + @Test + public void testNumericComparatorLt() { + + RiskAssessment ra1 = new RiskAssessment(); + RiskAssessment ra2 = new RiskAssessment(); + + RiskAssessment.RiskAssessmentPredictionComponent component = new RiskAssessment.RiskAssessmentPredictionComponent(); + DecimalType doseNumber = new DecimalType(0.25); + component.setProbability(doseNumber); + ra1.addPrediction(component); + String raId1 = myRiskAssessmentDao.create(ra1).getId().toUnqualifiedVersionless().getValue(); + + component = new RiskAssessment.RiskAssessmentPredictionComponent(); + doseNumber = new DecimalType(0.3); + component.setProbability(doseNumber); + ra2.addPrediction(component); + String raId2 = myRiskAssessmentDao.create(ra2).getId().toUnqualifiedVersionless().getValue(); + + SearchParameterMap map; + List found; + + map = new SearchParameterMap(); + map.setLoadSynchronous(true); + map.add(Constants.PARAM_FILTER, new StringParam("probability lt 0.3")); + found = toUnqualifiedVersionlessIdValues(myRiskAssessmentDao.search(map)); + assertThat(found, containsInAnyOrder(raId1)); + + map = new SearchParameterMap(); + map.setLoadSynchronous(true); + map.add(Constants.PARAM_FILTER, new StringParam("probability lt 0.25")); + found = toUnqualifiedVersionlessIdValues(myRiskAssessmentDao.search(map)); + assertThat(found, Matchers.empty()); + + } + + @Test + public void testNumericComparatorGe() { + + RiskAssessment ra1 = new RiskAssessment(); + RiskAssessment ra2 = new RiskAssessment(); + + RiskAssessment.RiskAssessmentPredictionComponent component = new RiskAssessment.RiskAssessmentPredictionComponent(); + DecimalType doseNumber = new DecimalType(0.25); + component.setProbability(doseNumber); + ra1.addPrediction(component); + String raId1 = myRiskAssessmentDao.create(ra1).getId().toUnqualifiedVersionless().getValue(); + + component = new RiskAssessment.RiskAssessmentPredictionComponent(); + doseNumber = new DecimalType(0.3); + component.setProbability(doseNumber); + ra2.addPrediction(component); + String raId2 = myRiskAssessmentDao.create(ra2).getId().toUnqualifiedVersionless().getValue(); + + SearchParameterMap map; + List found; + + map = new SearchParameterMap(); + map.setLoadSynchronous(true); + map.add(Constants.PARAM_FILTER, new StringParam("probability ge 0.25")); + found = toUnqualifiedVersionlessIdValues(myRiskAssessmentDao.search(map)); + assertThat(found, containsInAnyOrder(raId1, raId2)); + + map = new SearchParameterMap(); + map.setLoadSynchronous(true); + map.add(Constants.PARAM_FILTER, new StringParam("probability ge 0.3")); + found = toUnqualifiedVersionlessIdValues(myRiskAssessmentDao.search(map)); + assertThat(found, containsInAnyOrder(raId2)); + + } + + @Test + public void testNumericComparatorLe() { + + RiskAssessment ra1 = new RiskAssessment(); + RiskAssessment ra2 = new RiskAssessment(); + + RiskAssessment.RiskAssessmentPredictionComponent component = new RiskAssessment.RiskAssessmentPredictionComponent(); + DecimalType doseNumber = new DecimalType(0.25); + component.setProbability(doseNumber); + ra1.addPrediction(component); + String raId1 = myRiskAssessmentDao.create(ra1).getId().toUnqualifiedVersionless().getValue(); + + component = new RiskAssessment.RiskAssessmentPredictionComponent(); + doseNumber = new DecimalType(0.3); + component.setProbability(doseNumber); + ra2.addPrediction(component); + String raId2 = myRiskAssessmentDao.create(ra2).getId().toUnqualifiedVersionless().getValue(); + + SearchParameterMap map; + List found; + + map = new SearchParameterMap(); + map.setLoadSynchronous(true); + map.add(Constants.PARAM_FILTER, new StringParam("probability le 0.25")); + found = toUnqualifiedVersionlessIdValues(myRiskAssessmentDao.search(map)); + assertThat(found, containsInAnyOrder(raId1)); + + map = new SearchParameterMap(); + map.setLoadSynchronous(true); + map.add(Constants.PARAM_FILTER, new StringParam("probability le 0.3")); + found = toUnqualifiedVersionlessIdValues(myRiskAssessmentDao.search(map)); + assertThat(found, containsInAnyOrder(raId1, raId2)); + + } + @AfterClass public static void afterClassClearContext() { TestUtil.clearAllStaticFieldsForUnitTest(); diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4Test.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4Test.java index 09beea31823..aed0d8c80d3 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4Test.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4Test.java @@ -462,12 +462,12 @@ public class FhirResourceDaoR4Test extends BaseJpaR4Test { { IBundleProvider found = myObservationDao.search(new SearchParameterMap(Observation.SP_VALUE_QUANTITY, new QuantityParam("123", "foo", "bar")).setLoadSynchronous(true)); List list = toUnqualifiedVersionlessIds(found); - assertThat(list, containsInAnyOrder(id3)); + assertThat(list, Matchers.empty()); } { IBundleProvider found = myObservationDao.search(new SearchParameterMap(Observation.SP_VALUE_QUANTITY, new QuantityParam("123.0", "foo", "bar")).setLoadSynchronous(true)); List list = toUnqualifiedVersionlessIds(found); - assertThat(list, containsInAnyOrder(id3)); + assertThat(list, Matchers.empty()); } { IBundleProvider found = myObservationDao.search(new SearchParameterMap(Observation.SP_VALUE_QUANTITY, new QuantityParam("123.01", "foo", "bar")).setLoadSynchronous(true)); @@ -482,12 +482,12 @@ public class FhirResourceDaoR4Test extends BaseJpaR4Test { { IBundleProvider found = myObservationDao.search(new SearchParameterMap(Observation.SP_VALUE_QUANTITY, new QuantityParam("123.02", "foo", "bar")).setLoadSynchronous(true)); List list = toUnqualifiedVersionlessIds(found); - assertThat(list, containsInAnyOrder()); + assertThat(list, Matchers.empty()); } { IBundleProvider found = myObservationDao.search(new SearchParameterMap(Observation.SP_VALUE_QUANTITY, new QuantityParam("123.001", "foo", "bar")).setLoadSynchronous(true)); List list = toUnqualifiedVersionlessIds(found); - assertThat(list, containsInAnyOrder()); + assertThat(list, Matchers.empty()); } }