Fix #696: Corresctly handle negative numbers

This commit is contained in:
James Agnew 2017-07-26 16:59:25 -04:00
parent 6aa58cf887
commit 274e218494
5 changed files with 187 additions and 9 deletions

View File

@ -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<T extends BaseParam> 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<T extends BaseParam> extends BaseParam
}
/**
* @deprecated Use {@link #getPrefix() instead}
* @deprecated Use {@link #getPrefix()} instead
*/
@Deprecated
public QuantityCompararatorEnum getComparator() {
@ -68,7 +73,7 @@ public abstract class BaseParamWithPrefix<T extends BaseParam> extends BaseParam
if (prefix == null) {
return null;
}
return QuantityCompararatorEnum.forCode(prefix.getDstu1Value());
}

View File

@ -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<String> 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();

View File

@ -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();
}
}

View File

@ -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();

View File

@ -188,6 +188,11 @@
instead of the previous
<![CDATA[<code>Bundle.entry.resource</code>]]>
<action>
<action type="fix" issue="696">
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!
</action>
</release>
<release version="2.5" date="2017-06-08">
<action type="fix">