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

Correct invalid decimal numbers stored in DB (#2003) 

* Correct invalid decimal numbers stored in DB

* Add changelog
This commit is contained in:
James Agnew 2020-07-27 13:33:27 -04:00 committed by GitHub
parent 06fd306898
commit 3a7ac0debc
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 112 additions and 13 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

9
hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/5_1_0/2003-correct-invalid-decimals-in-db.yaml Normal file
View File

@ -0,0 +1,9 @@
---
type: fix
issue: 2003
title: "In HAPI FHIR 4.2.0 and before, due to the lenient Gson parser it was possible to store data in the JPA server
that contained invalid decimal numbers with no leading digits, e.g. `.123` and `-.123`. When we moved to Jackson as a JSON
parser, these values could no longer be parsed due to Jackson's more strict (and correct) interpretation of the JSON
specification. Unfortunately this led to data previously stored in the database being unusable. A fix has been implemented that
automatically adds a leading zero to any decimals that were previously saved in invalid state. New data will still be blocked from
being added if it contains invalid JSON numbers."

2
hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseHapiFhirDao.java
View File

@ -919,7 +919,7 @@ public abstract class BaseHapiFhirDao<T extends IBaseResource> extends BaseStora
if (resourceEncoding != ResourceEncodingEnum.DEL) {
LenientErrorHandler errorHandler = new LenientErrorHandler(false).setErrorOnInvalidValue(false);
IParser parser = new TolerantJsonParser(getContext(theEntity.getFhirVersion()), errorHandler);
IParser parser = new TolerantJsonParser(getContext(theEntity.getFhirVersion()), errorHandler, theEntity.getId());
try {
retVal = parser.parseResource(resourceType, resourceText);

49
hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/TolerantJsonParser.java
View File

@ -20,26 +20,42 @@ package ca.uhn.fhir.jpa.dao;
* #L%
*/
import ca.uhn.fhir.context.BaseRuntimeChildDefinition;
import ca.uhn.fhir.context.BaseRuntimeElementDefinition;
import ca.uhn.fhir.context.FhirContext;
import ca.uhn.fhir.parser.DataFormatException;
import ca.uhn.fhir.parser.IParserErrorHandler;
import ca.uhn.fhir.parser.JsonParser;
import ca.uhn.fhir.util.IModelVisitor2;
import com.google.gson.Gson;
import com.google.gson.JsonArray;
import com.google.gson.JsonElement;
import com.google.gson.JsonObject;
import com.google.gson.JsonPrimitive;
import org.hl7.fhir.instance.model.api.IBase;
import org.hl7.fhir.instance.model.api.IBaseResource;
import org.hl7.fhir.instance.model.api.IPrimitiveType;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import java.util.Iterator;
import java.util.Map;
import java.math.BigDecimal;
import java.util.List;
import static org.apache.commons.lang3.StringUtils.defaultString;
class TolerantJsonParser extends JsonParser {
TolerantJsonParser(FhirContext theContext, IParserErrorHandler theParserErrorHandler) {
private static final Logger ourLog = LoggerFactory.getLogger(TolerantJsonParser.class);
private final FhirContext myContext;
private final Long myResourcePid;
/**
* Constructor
*
* @param theResourcePid The ID of the resource that will be parsed with this parser. It would be ok to change the
* datatype for this param if we ever need to since it's only used for logging.
*/
TolerantJsonParser(FhirContext theContext, IParserErrorHandler theParserErrorHandler, Long theResourcePid) {
super(theContext, theParserErrorHandler);
myContext = theContext;
myResourcePid = theResourcePid;
}
@Override
@ -71,11 +87,28 @@ class TolerantJsonParser extends JsonParser {
JsonObject object = gson.fromJson(theMessageString, JsonObject.class);
String corrected = gson.toJson(object);
return super.parseResource(theResourceType, corrected);
T parsed = super.parseResource(theResourceType, corrected);
myContext.newTerser().visit(parsed, new IModelVisitor2() {
@Override
public boolean acceptElement(IBase theElement, List<IBase> theContainingElementPath, List<BaseRuntimeChildDefinition> theChildDefinitionPath, List<BaseRuntimeElementDefinition<?>> theElementDefinitionPath) {
BaseRuntimeElementDefinition<?> def = theElementDefinitionPath.get(theElementDefinitionPath.size() - 1);
if (def.getName().equals("decimal")) {
IPrimitiveType<BigDecimal> decimal = (IPrimitiveType<BigDecimal>) theElement;
String newPlainString = decimal.getValue().toPlainString();
ourLog.warn("Correcting invalid previously saved decimal number for Resource[pid={}] - Was {} and now is {}", myResourcePid, decimal.getValueAsString(), newPlainString);
decimal.setValueAsString(newPlainString);
}
return true;
}
});
return parsed;
}
throw e;
}
}
}

8
hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/TolerantJsonParserR4Test.java
View File

@ -25,7 +25,7 @@ public class TolerantJsonParserR4Test {
"}";
TolerantJsonParser parser = new TolerantJsonParser(myFhirContext, new LenientErrorHandler());
TolerantJsonParser parser = new TolerantJsonParser(myFhirContext, new LenientErrorHandler(), 123L);
Observation obs = parser.parseResource(Observation.class, input);
assertEquals("0.5", obs.getValueQuantity().getValueElement().getValueAsString());
@ -41,7 +41,7 @@ public class TolerantJsonParserR4Test {
"}";
TolerantJsonParser parser = new TolerantJsonParser(myFhirContext, new LenientErrorHandler());
TolerantJsonParser parser = new TolerantJsonParser(myFhirContext, new LenientErrorHandler(), 123L);
Observation obs = parser.parseResource(Observation.class, input);
assertEquals("0.5", obs.getValueQuantity().getValueElement().getValueAsString());
@ -57,7 +57,7 @@ public class TolerantJsonParserR4Test {
"}";
TolerantJsonParser parser = new TolerantJsonParser(myFhirContext, new LenientErrorHandler());
TolerantJsonParser parser = new TolerantJsonParser(myFhirContext, new LenientErrorHandler(), 123L);
Observation obs = parser.parseResource(Observation.class, input);
assertEquals("0", obs.getValueQuantity().getValueElement().getValueAsString());
@ -73,7 +73,7 @@ public class TolerantJsonParserR4Test {
"}";
TolerantJsonParser parser = new TolerantJsonParser(myFhirContext, new LenientErrorHandler());
TolerantJsonParser parser = new TolerantJsonParser(myFhirContext, new LenientErrorHandler(), 123L);
try {
parser.parseResource(Observation.class, input);
} catch (DataFormatException e) {

57
hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/r4/ResourceProviderInvalidDataR4Test.java Normal file
View File

@ -0,0 +1,57 @@
package ca.uhn.fhir.jpa.provider.r4;
import ca.uhn.fhir.jpa.dao.GZipUtil;
import ca.uhn.fhir.jpa.model.entity.ResourceHistoryTable;
import org.apache.commons.io.IOUtils;
import org.apache.http.client.methods.CloseableHttpResponse;
import org.apache.http.client.methods.HttpGet;
import org.hl7.fhir.r4.model.Observation;
import org.hl7.fhir.r4.model.Quantity;
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.Test;
import java.io.IOException;
import java.nio.charset.StandardCharsets;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.containsString;
public class ResourceProviderInvalidDataR4Test extends BaseResourceProviderR4Test {
private static final org.slf4j.Logger ourLog = org.slf4j.LoggerFactory.getLogger(ResourceProviderInvalidDataR4Test.class);
@Override
@AfterEach
public void after() throws Exception {
super.after();
ourRestServer.getInterceptorService().unregisterAllInterceptors();
}
@Test
public void testRetrieveDataSavedWithInvalidDecimal() throws IOException {
Observation obs = new Observation();
obs.setStatus(Observation.ObservationStatus.FINAL);
obs.setValue(new Quantity().setValue(100).setCode("km"));
Long id = myObservationDao.create(obs).getId().getIdPartAsLong();
// Manually set the value to be an invalid decimal number
runInTransaction(() -> {
ResourceHistoryTable resVer = myResourceHistoryTableDao.findForIdAndVersionAndFetchProvenance(id, 1);
byte[] bytesCompressed = resVer.getResource();
String resourceText = GZipUtil.decompress(bytesCompressed);
resourceText = resourceText.replace("100", "-.100");
bytesCompressed = GZipUtil.compress(resourceText);
resVer.setResource(bytesCompressed);
myResourceHistoryTableDao.save(resVer);
});
HttpGet httpGet = new HttpGet(ourServerBase + "/Observation/" + id);
httpGet.addHeader("Accept", "application/fhir+json");
try (CloseableHttpResponse status = ourHttpClient.execute(httpGet)) {
String responseContent = IOUtils.toString(status.getEntity().getContent(), StandardCharsets.UTF_8);
ourLog.info("Response content: " + responseContent);
assertThat(responseContent, containsString("\"value\":-0.100"));
}
}
}