diff --git a/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/param/BaseParamWithPrefix.java b/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/param/BaseParamWithPrefix.java index 37a5c4b0ef1..e3745f39d17 100644 --- a/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/param/BaseParamWithPrefix.java +++ b/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/param/BaseParamWithPrefix.java @@ -10,7 +10,7 @@ package ca.uhn.fhir.rest.param; * you may not use this file except in compliance with the License. * You may obtain a copy of the License at * - * http://www.apache.org/licenses/LICENSE-2.0 + * http://www.apache.org/licenses/LICENSE-2.0 * * Unless required by applicable law or agreed to in writing, software * distributed under the License is distributed on an "AS IS" BASIS, @@ -44,8 +44,13 @@ public abstract class BaseParamWithPrefix extends BaseParam String extractPrefixAndReturnRest(String theString) { int offset = 0; while (true) { - if (theString.length() == offset || Character.isDigit(theString.charAt(offset))) { + if (theString.length() == offset) { break; + } else { + char nextChar = theString.charAt(offset); + if (nextChar == '-' || Character.isDigit(nextChar)) { + break; + } } offset++; } @@ -60,7 +65,7 @@ public abstract class BaseParamWithPrefix extends BaseParam } /** - * @deprecated Use {@link #getPrefix() instead} + * @deprecated Use {@link #getPrefix()} instead */ @Deprecated public QuantityCompararatorEnum getComparator() { @@ -68,7 +73,7 @@ public abstract class BaseParamWithPrefix extends BaseParam if (prefix == null) { return null; } - + return QuantityCompararatorEnum.forCode(prefix.getDstu1Value()); } diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/dstu3/ResourceProviderDstu3Test.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/dstu3/ResourceProviderDstu3Test.java index 4ae19d1d315..022830a463a 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/dstu3/ResourceProviderDstu3Test.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/dstu3/ResourceProviderDstu3Test.java @@ -11,6 +11,7 @@ import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; import java.io.*; +import java.math.BigDecimal; import java.net.*; import java.nio.charset.StandardCharsets; import java.util.*; @@ -3197,6 +3198,31 @@ public class ResourceProviderDstu3Test extends BaseResourceProviderDstu3Test { } } + @Test() + public void testSearchNegativeNumbers() throws Exception { + Observation o = new Observation(); + o.setValue(new Quantity().setValue(new BigDecimal("-10"))); + String oid1 = myObservationDao.create(o, mySrd).getId().toUnqualifiedVersionless().getValue(); + + Observation o2 = new Observation(); + o2.setValue(new Quantity().setValue(new BigDecimal("-20"))); + String oid2 = myObservationDao.create(o2, mySrd).getId().toUnqualifiedVersionless().getValue(); + + HttpGet get = new HttpGet(ourServerBase + "/Observation?value-quantity=gt-15"); + CloseableHttpResponse resp = ourHttpClient.execute(get); + try { + assertEquals(200, resp.getStatusLine().getStatusCode()); + Bundle bundle = myFhirCtx.newXmlParser().parseResource(Bundle.class, IOUtils.toString(resp.getEntity().getContent(), Constants.CHARSET_UTF8)); + + List ids = toUnqualifiedVersionlessIdValues(bundle); + assertThat(ids, contains(oid1)); + assertThat(ids, not(contains(oid2))); + } finally { + IOUtils.closeQuietly(resp); + } + + } + @Test(expected = InvalidRequestException.class) public void testSearchWithInvalidSort() throws Exception { Observation o = new Observation(); diff --git a/hapi-fhir-structures-dstu3/src/test/java/ca/uhn/fhir/rest/param/NumberParamTest.java b/hapi-fhir-structures-dstu3/src/test/java/ca/uhn/fhir/rest/param/NumberParamTest.java new file mode 100644 index 00000000000..d15fac757e0 --- /dev/null +++ b/hapi-fhir-structures-dstu3/src/test/java/ca/uhn/fhir/rest/param/NumberParamTest.java @@ -0,0 +1,76 @@ +package ca.uhn.fhir.rest.param; + +import static org.junit.Assert.assertEquals; + +import java.math.BigDecimal; + +import org.junit.AfterClass; +import org.junit.Test; + +import ca.uhn.fhir.context.FhirContext; +import ca.uhn.fhir.util.TestUtil; + +public class NumberParamTest { + private static FhirContext ourCtx = FhirContext.forDstu3(); + + @Test + public void testFull() { + NumberParam p = new NumberParam(); + p.setValueAsQueryToken(ourCtx, null, null, "<5.4"); + assertEquals(ParamPrefixEnum.LESSTHAN, p.getPrefix()); + assertEquals("5.4", p.getValue().toPlainString()); + assertEquals("lt5.4", p.getValueAsQueryToken(ourCtx)); + } + + @Test + public void testApproximateLegacy() { + NumberParam p = new NumberParam(); + p.setValueAsQueryToken(ourCtx, null, null, "~5.4"); + assertEquals(null,p.getComparator()); + assertEquals(ParamPrefixEnum.APPROXIMATE, p.getPrefix()); + assertEquals("5.4", p.getValue().toPlainString()); + assertEquals("ap5.4", p.getValueAsQueryToken(ourCtx)); + } + + @Test + public void testApproximate() { + NumberParam p = new NumberParam(); + p.setValueAsQueryToken(ourCtx, null, null, "ap5.4"); + assertEquals(null,p.getComparator()); + assertEquals(ParamPrefixEnum.APPROXIMATE, p.getPrefix()); + assertEquals("5.4", p.getValue().toPlainString()); + assertEquals("ap5.4", p.getValueAsQueryToken(ourCtx)); + } + + @Test + public void testNoQualifier() { + NumberParam p = new NumberParam(); + p.setValueAsQueryToken(ourCtx, null, null, "5.4"); + assertEquals(null, p.getComparator()); + assertEquals(null, p.getPrefix()); + assertEquals("5.4", p.getValue().toPlainString()); + assertEquals("5.4", p.getValueAsQueryToken(ourCtx)); + } + + + /** + * See #696 + */ + @Test + public void testNegativeNumber() { + NumberParam p = new NumberParam(); + p.setValueAsQueryToken(ourCtx, null, null, "-5.4"); + assertEquals(null, p.getComparator()); + assertEquals(null, p.getPrefix()); + assertEquals("-5.4", p.getValue().toPlainString()); + assertEquals(new BigDecimal("-5.4"), p.getValue()); + assertEquals("-5.4", p.getValueAsQueryToken(ourCtx)); + } + + + @AfterClass + public static void afterClassClearContext() { + TestUtil.clearAllStaticFieldsForUnitTest(); + } + +} diff --git a/hapi-fhir-structures-dstu3/src/test/java/ca/uhn/fhir/rest/param/QuantityParamTest.java b/hapi-fhir-structures-dstu3/src/test/java/ca/uhn/fhir/rest/param/QuantityParamTest.java index 0784dc19a0b..853560b722e 100644 --- a/hapi-fhir-structures-dstu3/src/test/java/ca/uhn/fhir/rest/param/QuantityParamTest.java +++ b/hapi-fhir-structures-dstu3/src/test/java/ca/uhn/fhir/rest/param/QuantityParamTest.java @@ -2,6 +2,8 @@ package ca.uhn.fhir.rest.param; import static org.junit.Assert.*; +import java.math.BigDecimal; + import org.junit.AfterClass; import org.junit.Test; @@ -10,37 +12,52 @@ import ca.uhn.fhir.model.dstu.valueset.QuantityCompararatorEnum; import ca.uhn.fhir.util.TestUtil; public class QuantityParamTest { - private static FhirContext ourCtx = FhirContext.forDstu1(); + private static FhirContext ourCtx = FhirContext.forDstu3(); @Test public void testFull() { QuantityParam p = new QuantityParam(); p.setValueAsQueryToken(ourCtx, null, null, "<5.4|http://unitsofmeasure.org|mg"); assertEquals(QuantityCompararatorEnum.LESSTHAN,p.getComparator()); + assertEquals(ParamPrefixEnum.LESSTHAN, p.getPrefix()); assertEquals("5.4", p.getValue().toPlainString()); assertEquals("http://unitsofmeasure.org", p.getSystem()); assertEquals("mg", p.getUnits()); - assertEquals("<5.4|http://unitsofmeasure.org|mg", p.getValueAsQueryToken(ourCtx)); + assertEquals("lt5.4|http://unitsofmeasure.org|mg", p.getValueAsQueryToken(ourCtx)); + } + + @Test + public void testApproximateLegacy() { + QuantityParam p = new QuantityParam(); + p.setValueAsQueryToken(ourCtx, null, null, "~5.4|http://unitsofmeasure.org|mg"); + assertEquals(null,p.getComparator()); + assertEquals(ParamPrefixEnum.APPROXIMATE, p.getPrefix()); + assertEquals(true, p.isApproximate()); + assertEquals("5.4", p.getValue().toPlainString()); + assertEquals("http://unitsofmeasure.org", p.getSystem()); + assertEquals("mg", p.getUnits()); + assertEquals("ap5.4|http://unitsofmeasure.org|mg", p.getValueAsQueryToken(ourCtx)); } @Test public void testApproximate() { QuantityParam p = new QuantityParam(); - p.setValueAsQueryToken(ourCtx, null, null, "~5.4|http://unitsofmeasure.org|mg"); + p.setValueAsQueryToken(ourCtx, null, null, "ap5.4|http://unitsofmeasure.org|mg"); assertEquals(null,p.getComparator()); + assertEquals(ParamPrefixEnum.APPROXIMATE, p.getPrefix()); assertEquals(true, p.isApproximate()); assertEquals("5.4", p.getValue().toPlainString()); assertEquals("http://unitsofmeasure.org", p.getSystem()); assertEquals("mg", p.getUnits()); - assertEquals("~5.4|http://unitsofmeasure.org|mg", p.getValueAsQueryToken(ourCtx)); + assertEquals("ap5.4|http://unitsofmeasure.org|mg", p.getValueAsQueryToken(ourCtx)); } - @Test public void testNoQualifier() { QuantityParam p = new QuantityParam(); p.setValueAsQueryToken(ourCtx, null, null, "5.4|http://unitsofmeasure.org|mg"); assertEquals(null, p.getComparator()); + assertEquals(null, p.getPrefix()); assertEquals("5.4", p.getValue().toPlainString()); assertEquals("http://unitsofmeasure.org", p.getSystem()); assertEquals("mg", p.getUnits()); @@ -53,6 +70,7 @@ public class QuantityParamTest { QuantityParam p = new QuantityParam(); p.setValueAsQueryToken(ourCtx, null, null, "5.4"); assertEquals(null, p.getComparator()); + assertEquals(null, p.getPrefix()); assertEquals("5.4", p.getValue().toPlainString()); assertEquals(null, p.getSystem()); assertEquals(null, p.getUnits()); @@ -84,6 +102,54 @@ public class QuantityParamTest { assertEquals("mg", param.getUnits()); } + /** + * See #696 + */ + @Test + public void testNegativeQuantityWithUnits() { + QuantityParam p = new QuantityParam(); + p.setValueAsQueryToken(ourCtx, null, null, "-5.4|http://unitsofmeasure.org|mg"); + assertEquals(null, p.getComparator()); + assertEquals(null, p.getPrefix()); + assertEquals("-5.4", p.getValue().toPlainString()); + assertEquals(new BigDecimal("-5.4"), p.getValue()); + assertEquals("http://unitsofmeasure.org", p.getSystem()); + assertEquals("mg", p.getUnits()); + assertEquals("-5.4|http://unitsofmeasure.org|mg", p.getValueAsQueryToken(ourCtx)); + } + + /** + * See #696 + */ + @Test + public void testNegativeQuantityWithoutUnits() { + QuantityParam p = new QuantityParam(); + p.setValueAsQueryToken(ourCtx, null, null, "-5.4"); + assertEquals(null, p.getComparator()); + assertEquals(null, p.getPrefix()); + assertEquals("-5.4", p.getValue().toPlainString()); + assertEquals(new BigDecimal("-5.4"), p.getValue()); + assertEquals(null, p.getSystem()); + assertEquals(null, p.getUnits()); + assertEquals("-5.4||", p.getValueAsQueryToken(ourCtx)); + } + + /** + * See #696 + */ + @Test + public void testNegativeQuantityWithoutUnitsWithComparator() { + QuantityParam p = new QuantityParam(); + p.setValueAsQueryToken(ourCtx, null, null, "gt-5.4"); + assertEquals(QuantityCompararatorEnum.GREATERTHAN, p.getComparator()); + assertEquals(ParamPrefixEnum.GREATERTHAN, p.getPrefix()); + assertEquals("-5.4", p.getValue().toPlainString()); + assertEquals(new BigDecimal("-5.4"), p.getValue()); + assertEquals(null, p.getSystem()); + assertEquals(null, p.getUnits()); + assertEquals("gt-5.4||", p.getValueAsQueryToken(ourCtx)); + } + @AfterClass public static void afterClassClearContext() { TestUtil.clearAllStaticFieldsForUnitTest(); diff --git a/src/changes/changes.xml b/src/changes/changes.xml index 87f2b827367..9a9520866c1 100644 --- a/src/changes/changes.xml +++ b/src/changes/changes.xml @@ -188,6 +188,11 @@ instead of the previous Bundle.entry.resource]]> + + An issue was corrected where search parameters containing negative numbers + were sometimes treated as positive numbers when processing the search. Thanks + to Keith Boone for reporting and suggesting a fix! +