From 8846477b7b276d9788e2106525ebb82181f51bd3 Mon Sep 17 00:00:00 2001 From: Michael Lawley Date: Wed, 30 Nov 2016 12:03:02 +1000 Subject: [PATCH 1/5] Ensure that older DSTU3 versions (ie Baltimore / May2016 / 1.4.0) use the legacy mime types --- .../fhir/rest/server/RestfulServerUtils.java | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/server/RestfulServerUtils.java b/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/server/RestfulServerUtils.java index bf3b99d2c03..2b86df6f05e 100644 --- a/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/server/RestfulServerUtils.java +++ b/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/server/RestfulServerUtils.java @@ -77,6 +77,15 @@ public class RestfulServerUtils { private static final HashSet TEXT_ENCODE_ELEMENTS = new HashSet(Arrays.asList("Bundle", "*.text", "*.(mandatory)")); + private static String actualDstu3FhirVersion = FhirVersionEnum.DSTU3.getFhirVersionString(); + static { + try { + Class c = Class.forName("org.hl7.fhir.dstu3.model.Constants"); + actualDstu3FhirVersion = (String) c.getDeclaredField("VERSION").get(null); + } catch (Exception e) { + } + } + public static void configureResponseParser(RequestDetails theRequestDetails, IParser parser) { // Pretty print boolean prettyPrint = RestfulServerUtils.prettyPrintResponse(theRequestDetails.getServer(), theRequestDetails); @@ -757,12 +766,16 @@ public class RestfulServerUtils { myEncoding = theEncoding; if (theContentType != null) { if (theContentType.equals(EncodingEnum.JSON_PLAIN_STRING) || theContentType.equals(EncodingEnum.XML_PLAIN_STRING)) { - myNonLegacy = !theCtx.getVersion().getVersion().isOlderThan(FhirVersionEnum.DSTU3); + FhirVersionEnum ctxtEnum = theCtx.getVersion().getVersion(); + myNonLegacy = ctxtEnum.isNewerThan(FhirVersionEnum.DSTU3) + || (ctxtEnum.isEquivalentTo(FhirVersionEnum.DSTU3) && !"1.4.0".equals(actualDstu3FhirVersion)); } else { myNonLegacy = EncodingEnum.isNonLegacy(theContentType); } } else { - if (theCtx.getVersion().getVersion().isOlderThan(FhirVersionEnum.DSTU3)) { + FhirVersionEnum ctxtEnum = theCtx.getVersion().getVersion(); + if (ctxtEnum.isOlderThan(FhirVersionEnum.DSTU3) + || (ctxtEnum.isEquivalentTo(FhirVersionEnum.DSTU3) && "1.4.0".equals(actualDstu3FhirVersion))) { myNonLegacy = null; } else { myNonLegacy = Boolean.TRUE; From a9a350754467d5de70c49cde7d2819b09d80af50 Mon Sep 17 00:00:00 2001 From: Michael Lawley Date: Tue, 6 Dec 2016 10:32:23 +1000 Subject: [PATCH 2/5] #516 Report invalid values for enums through parser error handler --- .../main/java/ca/uhn/fhir/parser/ParserState.java | 6 +++++- .../ca/uhn/fhir/parser/JsonParserDstu3Test.java | 13 +++++++++++++ 2 files changed, 18 insertions(+), 1 deletion(-) diff --git a/hapi-fhir-base/src/main/java/ca/uhn/fhir/parser/ParserState.java b/hapi-fhir-base/src/main/java/ca/uhn/fhir/parser/ParserState.java index 73b139a2cff..5539ffba97a 100644 --- a/hapi-fhir-base/src/main/java/ca/uhn/fhir/parser/ParserState.java +++ b/hapi-fhir-base/src/main/java/ca/uhn/fhir/parser/ParserState.java @@ -2309,7 +2309,11 @@ class ParserState { if ("".equals(theValue)) { myErrorHandler.invalidValue(null, theValue, "Attribute values must not be empty (\"\")"); } else { - myInstance.setValueAsString(theValue); + try { + myInstance.setValueAsString(theValue); + } catch (IllegalArgumentException e) { + myErrorHandler.invalidValue(null, theValue, e.getMessage()); + } } } else if ("id".equals(theName)) { if (myInstance instanceof IIdentifiableElement) { diff --git a/hapi-fhir-structures-dstu3/src/test/java/ca/uhn/fhir/parser/JsonParserDstu3Test.java b/hapi-fhir-structures-dstu3/src/test/java/ca/uhn/fhir/parser/JsonParserDstu3Test.java index b70bcf2ce9e..5f3798ef146 100644 --- a/hapi-fhir-structures-dstu3/src/test/java/ca/uhn/fhir/parser/JsonParserDstu3Test.java +++ b/hapi-fhir-structures-dstu3/src/test/java/ca/uhn/fhir/parser/JsonParserDstu3Test.java @@ -49,6 +49,7 @@ import com.google.common.collect.Sets; import ca.uhn.fhir.context.FhirContext; import ca.uhn.fhir.narrative.DefaultThymeleafNarrativeGenerator; +import ca.uhn.fhir.parser.DataFormatException; import ca.uhn.fhir.parser.IParserErrorHandler.IParseLocation; import ca.uhn.fhir.parser.PatientWithExtendedContactDstu3.CustomContactComponent; import ca.uhn.fhir.parser.XmlParserDstu3Test.TestPatientFor327; @@ -1193,6 +1194,18 @@ public class JsonParserDstu3Test { assertEquals("{\"resourceType\":\"Observation\",\"valueQuantity\":{\"value\":0.0000000000000001}}", str); } + /** + * #516 + */ + @Test(expected=DataFormatException.class) + public void testInvalidEnumValue() { + String res = "{ \"resourceType\": \"ValueSet\", \"url\": \"http://sample/ValueSet/education-levels\", \"version\": \"1\", \"name\": \"Education Levels\", \"status\": \"draft\", \"compose\": { \"include\": [ { \"filter\": [ { \"property\": \"n\", \"op\": \"n\", \"value\": \"365460000\" } ], \"system\": \"http://snomed.info/sct\" } ], \"exclude\": [ { \"concept\": [ { \"code\": \"224298008\" }, { \"code\": \"365460000\" }, { \"code\": \"473462005\" }, { \"code\": \"424587006\" } ], \"system\": \"http://snomed.info/sct\" } ] }, \"description\": \"A selection of Education Levels\", \"text\": { \"status\": \"generated\", \"div\": \"

Education Levels

http://csiro.au/ValueSet/education-levels

A selection of Education Levels

\" }, \"experimental\": true, \"date\": \"2016-07-26\" }"; + IParser parser = ourCtx.newJsonParser(); + parser.setParserErrorHandler(new StrictErrorHandler()); + ValueSet parsed = parser.parseResource(ValueSet.class, res); + fail("DataFormat Invalid attribute exception should be thrown"); + } + /** * #65 */ From 2f55fdee2e3c8b050c44c3bca809248828171c5a Mon Sep 17 00:00:00 2001 From: James Date: Sat, 10 Dec 2016 07:38:23 -0500 Subject: [PATCH 3/5] Credit for #525 --- src/changes/changes.xml | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/changes/changes.xml b/src/changes/changes.xml index 67e90e373a4..359fadd2801 100644 --- a/src/changes/changes.xml +++ b/src/changes/changes.xml @@ -93,6 +93,12 @@ to using Thymeleaf 3. Thanks to GitHub user @gsureshkumar for reporting! + + When parsing invalid enum values in STU3, + report errors through the parserErrorHandler, + not by throwing an exception. Thanks to + Michael Lawley for the pull request! + From 1fba4ff265fb65f5912249adcfc61a5aaf8aab1d Mon Sep 17 00:00:00 2001 From: jamesagnew Date: Sat, 10 Dec 2016 08:44:00 -0500 Subject: [PATCH 4/5] Test refactor --- .../main/java/ca/uhn/fhir/util/TestUtil.java | 46 ++++++++++--------- 1 file changed, 25 insertions(+), 21 deletions(-) diff --git a/hapi-fhir-base/src/main/java/ca/uhn/fhir/util/TestUtil.java b/hapi-fhir-base/src/main/java/ca/uhn/fhir/util/TestUtil.java index eab1e317d52..20837333fd7 100644 --- a/hapi-fhir-base/src/main/java/ca/uhn/fhir/util/TestUtil.java +++ b/hapi-fhir-base/src/main/java/ca/uhn/fhir/util/TestUtil.java @@ -79,27 +79,7 @@ public class TestUtil { } - /* - * Set some system properties randomly after each test.. this is kind of hackish, - * but it helps us make sure we don't have any tests that depend on a particular - * environment - */ - Locale[] availableLocales = { Locale.CANADA, Locale.GERMANY, Locale.TAIWAN }; - Locale.setDefault(availableLocales[(int) (Math.random() * availableLocales.length)]); - ourLog.info("Tests are running in locale: " + Locale.getDefault().getDisplayName()); - if (Math.random() < 0.5) { - ourLog.info("Tests are using WINDOWS line endings and ISO-8851-1"); - System.setProperty("file.encoding", "ISO-8859-1"); - System.setProperty("line.separator", "\r\n"); - } else { - ourLog.info("Tests are using UNIX line endings and UTF-8"); - System.setProperty("file.encoding", "UTF-8"); - System.setProperty("line.separator", "\n"); - } - String availableTimeZones[] = { "GMT+08:00", "GMT-05:00", "GMT+00:00", "GMT+03:30" }; - String timeZone = availableTimeZones[(int) (Math.random() * availableTimeZones.length)]; - TimeZone.setDefault(TimeZone.getTimeZone(timeZone)); - ourLog.info("Tests are using time zone: {}", TimeZone.getDefault().getID()); + randomizeLocale(); /* * If we're running a CI build, set all loggers to TRACE level to ensure coverage @@ -116,4 +96,28 @@ public class TestUtil { } } + /** + * Set some system properties randomly after each test.. this is kind of hackish, + * but it helps us make sure we don't have any tests that depend on a particular + * environment + */ + public static void randomizeLocale() { + Locale[] availableLocales = { Locale.CANADA, Locale.GERMANY, Locale.TAIWAN }; + Locale.setDefault(availableLocales[(int) (Math.random() * availableLocales.length)]); + ourLog.info("Tests are running in locale: " + Locale.getDefault().getDisplayName()); + if (Math.random() < 0.5) { + ourLog.info("Tests are using WINDOWS line endings and ISO-8851-1"); + System.setProperty("file.encoding", "ISO-8859-1"); + System.setProperty("line.separator", "\r\n"); + } else { + ourLog.info("Tests are using UNIX line endings and UTF-8"); + System.setProperty("file.encoding", "UTF-8"); + System.setProperty("line.separator", "\n"); + } + String availableTimeZones[] = { "GMT+08:00", "GMT-05:00", "GMT+00:00", "GMT+03:30" }; + String timeZone = availableTimeZones[(int) (Math.random() * availableTimeZones.length)]; + TimeZone.setDefault(TimeZone.getTimeZone(timeZone)); + ourLog.info("Tests are using time zone: {}", TimeZone.getDefault().getID()); + } + } From ee63bbea745ea25a1bf199a4153bb35b3ccf808e Mon Sep 17 00:00:00 2001 From: James Date: Sat, 10 Dec 2016 14:14:22 -0500 Subject: [PATCH 5/5] Fix #516 - Handle STU3 invalid enum values with an appropriate exception --- .../java/example/PagingPatientProvider.java | 95 ++++++----- .../uhn/fhir/parser/LenientErrorHandler.java | 52 +++++- .../java/ca/uhn/fhir/parser/ParserState.java | 2 + .../fhir/validation/ValidationContext.java | 8 +- .../ResourceProviderDstu3ValueSetTest.java | 31 ++++ .../resources/bug_516_invalid_expansion.json | 47 ++++++ .../src/test/resources/dstu1_bundle.xml | 2 - .../ca/uhn/fhir/jpa/demo/JpaServerDemo.java | 8 +- .../validation/ResourceValidatorTest.java | 21 ++- .../uhn/fhir/parser/JsonParserDstu2Test.java | 5 +- .../uhn/fhir/parser/XmlParserDstu2Test.java | 5 +- .../ResourceValidatorDstu2Test.java | 31 ++-- .../fhir/dstu3/model/BaseDateTimeType.java | 13 +- .../hl7/fhir/dstu3/model/PrimitiveType.java | 2 +- .../ca/uhn/fhir/parser/ErrorHandlerTest.java | 11 +- .../uhn/fhir/parser/JsonParserDstu3Test.java | 153 +++++++++++++++--- .../uhn/fhir/parser/XmlParserDstu3Test.java | 16 +- .../ResourceValidatorDstu3Test.java | 47 +++++- src/changes/changes.xml | 15 ++ src/site/xdoc/doc_validation.xml | 133 +++++++-------- 20 files changed, 524 insertions(+), 173 deletions(-) create mode 100644 hapi-fhir-jpaserver-base/src/test/resources/bug_516_invalid_expansion.json diff --git a/examples/src/main/java/example/PagingPatientProvider.java b/examples/src/main/java/example/PagingPatientProvider.java index 8112b4677eb..ab398a3679a 100644 --- a/examples/src/main/java/example/PagingPatientProvider.java +++ b/examples/src/main/java/example/PagingPatientProvider.java @@ -13,69 +13,68 @@ import ca.uhn.fhir.rest.param.StringParam; import ca.uhn.fhir.rest.server.IBundleProvider; import ca.uhn.fhir.rest.server.IResourceProvider; - @SuppressWarnings("null") -//START SNIPPET: provider +// START SNIPPET: provider public class PagingPatientProvider implements IResourceProvider { - /** - * Search for Patient resources matching a given family name - */ - @Search - public IBundleProvider search(@RequiredParam(name = Patient.SP_FAMILY) StringParam theFamily) { - final InstantDt searchTime = InstantDt.withCurrentTime(); + /** + * Search for Patient resources matching a given family name + */ + @Search + public IBundleProvider search(@RequiredParam(name = Patient.SP_FAMILY) StringParam theFamily) { + final InstantDt searchTime = InstantDt.withCurrentTime(); - /* - * First, we'll search the database for a set of database row IDs that - * match the given search criteria. That way we can keep just the - * row IDs around, and load the actual resources on demand later - * as the client pages through them. - */ - final List matchingResourceIds = null; // <-- implement this + /** + * First, we'll search the database for a set of database row IDs that + * match the given search criteria. That way we can keep just the row IDs + * around, and load the actual resources on demand later as the client + * pages through them. + */ + final List matchingResourceIds = null; // <-- implement this - /* - * Return a bundle provider which can page through the IDs and - * return the resources that go with them. - */ - return new IBundleProvider() { + /** + * Return a bundle provider which can page through the IDs and return the + * resources that go with them. + */ + return new IBundleProvider() { - @Override - public int size() { - return matchingResourceIds.size(); - } + @Override + public int size() { + return matchingResourceIds.size(); + } - @Override - public List getResources(int theFromIndex, int theToIndex) { - int end = Math.max(theToIndex, matchingResourceIds.size() - 1); - List idsToReturn = matchingResourceIds.subList(theFromIndex, end); - return loadResourcesByIds(idsToReturn); - } + @Override + public List getResources(int theFromIndex, int theToIndex) { + int end = Math.max(theToIndex, matchingResourceIds.size() - 1); + List idsToReturn = matchingResourceIds.subList(theFromIndex, end); + return loadResourcesByIds(idsToReturn); + } - @Override - public InstantDt getPublished() { - return searchTime; - } + @Override + public InstantDt getPublished() { + return searchTime; + } @Override public Integer preferredPageSize() { // Typically this method just returns null return null; } - }; - } + }; + } - /** - * Load a list of patient resources given their IDs - */ - private List loadResourcesByIds(List theIdsToReturn) { - // .. implement this search against the database .. - return null; - } + /** + * Load a list of patient resources given their IDs + */ + private List loadResourcesByIds(List theIdsToReturn) { + // .. implement this search against the database .. + return null; + } - @Override - public Class getResourceType() { - return Patient.class; - } + @Override + public Class getResourceType() { + return Patient.class; + } } -//END SNIPPET: provider +// END SNIPPET: provider diff --git a/hapi-fhir-base/src/main/java/ca/uhn/fhir/parser/LenientErrorHandler.java b/hapi-fhir-base/src/main/java/ca/uhn/fhir/parser/LenientErrorHandler.java index d45af3dbc4b..fced37beb77 100644 --- a/hapi-fhir-base/src/main/java/ca/uhn/fhir/parser/LenientErrorHandler.java +++ b/hapi-fhir-base/src/main/java/ca/uhn/fhir/parser/LenientErrorHandler.java @@ -1,5 +1,8 @@ package ca.uhn.fhir.parser; +import static org.apache.commons.lang3.StringUtils.isBlank; +import static org.apache.commons.lang3.StringUtils.isNotBlank; + import ca.uhn.fhir.context.FhirContext; import ca.uhn.fhir.parser.json.JsonLikeValue.ValueType; @@ -24,7 +27,13 @@ import ca.uhn.fhir.parser.json.JsonLikeValue.ValueType; */ /** - * The default error handler, which logs issues but does not abort parsing + * The default error handler, which logs issues but does not abort parsing, with only one exception: + *

+ * The {@link #invalidValue(ca.uhn.fhir.parser.IParserErrorHandler.IParseLocation, String, String)} + * method will throw a {@link DataFormatException} by default since ignoring this type of error + * can lead to data loss (since invalid values are silently ignored). See + * {@link #setErrorOnInvalidValue(boolean)} for information on this. + *

* * @see IParser#setParserErrorHandler(IParserErrorHandler) * @see FhirContext#setParserErrorHandler(IParserErrorHandler) @@ -32,6 +41,8 @@ import ca.uhn.fhir.parser.json.JsonLikeValue.ValueType; public class LenientErrorHandler implements IParserErrorHandler { private static final org.slf4j.Logger ourLog = org.slf4j.LoggerFactory.getLogger(LenientErrorHandler.class); + private static final StrictErrorHandler STRICT_ERROR_HANDLER = new StrictErrorHandler(); + private boolean myErrorOnInvalidValue = true; private boolean myLogErrors; /** @@ -67,11 +78,30 @@ public class LenientErrorHandler implements IParserErrorHandler { @Override public void invalidValue(IParseLocation theLocation, String theValue, String theError) { - if (myLogErrors) { - ourLog.warn("Invalid attribute value \"{}\": {}", theValue, theError); + if (isBlank(theValue) || myErrorOnInvalidValue == false) { + if (myLogErrors) { + ourLog.warn("Invalid attribute value \"{}\": {}", theValue, theError); + } + } else { + STRICT_ERROR_HANDLER.invalidValue(theLocation, theValue, theError); } } + /** + * If set to false (default is true) invalid values will be logged. By + * default invalid attribute values cause this error handler to throw a {@link DataFormatException} (unlike + * other methods in this class which default to simply logging errors). + *

+ * Note that empty values (e.g. "") will not lead to an error when this is set to + * true, only invalid values (e.g. a gender code of foo) + *

+ * + * @see #setErrorOnInvalidValue(boolean) + */ + public boolean isErrorOnInvalidValue() { + return myErrorOnInvalidValue; + } + @Override public void missingRequiredElement(IParseLocation theLocation, String theElementName) { if (myLogErrors) { @@ -79,6 +109,22 @@ public class LenientErrorHandler implements IParserErrorHandler { } } + /** + * If set to false (default is true) invalid values will be logged. By + * default invalid attribute values cause this error handler to throw a {@link DataFormatException} (unlike + * other methods in this class which default to simply logging errors). + *

+ * Note that empty values (e.g. "") will not lead to an error when this is set to + * true, only invalid values (e.g. a gender code of foo) + *

+ * @return Returns a reference to this for easy method chaining + * @see #isErrorOnInvalidValue() + */ + public LenientErrorHandler setErrorOnInvalidValue(boolean theErrorOnInvalidValue) { + myErrorOnInvalidValue = theErrorOnInvalidValue; + return this; + } + @Override public void unexpectedRepeatingElement(IParseLocation theLocation, String theElementName) { if (myLogErrors) { diff --git a/hapi-fhir-base/src/main/java/ca/uhn/fhir/parser/ParserState.java b/hapi-fhir-base/src/main/java/ca/uhn/fhir/parser/ParserState.java index 5539ffba97a..bc3f927af0e 100644 --- a/hapi-fhir-base/src/main/java/ca/uhn/fhir/parser/ParserState.java +++ b/hapi-fhir-base/src/main/java/ca/uhn/fhir/parser/ParserState.java @@ -2311,6 +2311,8 @@ class ParserState { } else { try { myInstance.setValueAsString(theValue); + } catch (DataFormatException e) { + myErrorHandler.invalidValue(null, theValue, e.getMessage()); } catch (IllegalArgumentException e) { myErrorHandler.invalidValue(null, theValue, e.getMessage()); } diff --git a/hapi-fhir-base/src/main/java/ca/uhn/fhir/validation/ValidationContext.java b/hapi-fhir-base/src/main/java/ca/uhn/fhir/validation/ValidationContext.java index fd898b378d3..ef9c9432111 100644 --- a/hapi-fhir-base/src/main/java/ca/uhn/fhir/validation/ValidationContext.java +++ b/hapi-fhir-base/src/main/java/ca/uhn/fhir/validation/ValidationContext.java @@ -25,6 +25,8 @@ import org.hl7.fhir.instance.model.api.IBaseResource; import ca.uhn.fhir.context.FhirContext; import ca.uhn.fhir.model.api.Bundle; import ca.uhn.fhir.model.api.IResource; +import ca.uhn.fhir.parser.IParser; +import ca.uhn.fhir.parser.LenientErrorHandler; import ca.uhn.fhir.rest.method.MethodUtil; import ca.uhn.fhir.rest.server.EncodingEnum; import ca.uhn.fhir.rest.server.exceptions.InvalidRequestException; @@ -146,7 +148,11 @@ public class ValidationContext extends BaseValidationContext implements IV @Override public IBaseResource getResource() { if (myParsed == null) { - myParsed = getResourceAsStringEncoding().newParser(getFhirContext()).parseResource(getResourceAsString()); + IParser parser = getResourceAsStringEncoding().newParser(getFhirContext()); + LenientErrorHandler errorHandler = new LenientErrorHandler(); + errorHandler.setErrorOnInvalidValue(false); + parser.setParserErrorHandler(errorHandler); + myParsed = parser.parseResource(getResourceAsString()); } return myParsed; } diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/dstu3/ResourceProviderDstu3ValueSetTest.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/dstu3/ResourceProviderDstu3ValueSetTest.java index 5bd28142308..7f57c782ee2 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/dstu3/ResourceProviderDstu3ValueSetTest.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/dstu3/ResourceProviderDstu3ValueSetTest.java @@ -11,7 +11,13 @@ import static org.junit.Assert.assertThat; import static org.junit.Assert.fail; import java.io.IOException; +import java.nio.charset.StandardCharsets; +import org.apache.commons.io.IOUtils; +import org.apache.http.client.methods.CloseableHttpResponse; +import org.apache.http.client.methods.HttpPost; +import org.apache.http.entity.ContentType; +import org.apache.http.entity.StringEntity; import org.hl7.fhir.dstu3.model.BooleanType; import org.hl7.fhir.dstu3.model.CodeSystem; import org.hl7.fhir.dstu3.model.CodeType; @@ -34,6 +40,7 @@ import ca.uhn.fhir.jpa.entity.ResourceTable; import ca.uhn.fhir.jpa.entity.TermCodeSystemVersion; import ca.uhn.fhir.jpa.entity.TermConcept; import ca.uhn.fhir.jpa.entity.TermConceptParentChildLink.RelationshipTypeEnum; +import ca.uhn.fhir.rest.server.Constants; import ca.uhn.fhir.rest.server.exceptions.InvalidRequestException; import ca.uhn.fhir.util.TestUtil; @@ -54,6 +61,30 @@ public class ResourceProviderDstu3ValueSetTest extends BaseResourceProviderDstu3 myExtensionalVsId = myValueSetDao.create(upload, mySrd).getId().toUnqualifiedVersionless(); } + /** + * #516 + */ + @Test + public void testInvalidFilter() throws Exception { + String string = IOUtils.toString(getClass().getResourceAsStream("/bug_516_invalid_expansion.json"), StandardCharsets.UTF_8); + HttpPost post = new HttpPost(ourServerBase+"/ValueSet/%24expand"); + post.setEntity(new StringEntity(string, ContentType.parse(Constants.CT_FHIR_JSON_NEW))); + + CloseableHttpResponse resp = ourHttpClient.execute(post); + try { + + String respString = IOUtils.toString(resp.getEntity().getContent(), StandardCharsets.UTF_8); + ourLog.info(respString); + + ourLog.info(resp.toString()); + + assertEquals(400, resp.getStatusLine().getStatusCode()); + assertThat(respString, containsString("Unknown FilterOperator code 'n'")); + + }finally { + IOUtils.closeQuietly(resp); + } + } private CodeSystem createExternalCs() { CodeSystem codeSystem = new CodeSystem(); diff --git a/hapi-fhir-jpaserver-base/src/test/resources/bug_516_invalid_expansion.json b/hapi-fhir-jpaserver-base/src/test/resources/bug_516_invalid_expansion.json new file mode 100644 index 00000000000..93ccab739e6 --- /dev/null +++ b/hapi-fhir-jpaserver-base/src/test/resources/bug_516_invalid_expansion.json @@ -0,0 +1,47 @@ +{ + "resourceType": "ValueSet", + "url": "http://sample/ValueSet/education-levels", + "version": "1", + "name": "Education Levels", + "status": "draft", + "compose": { + "include": [ + { + "filter": [ + { + "property": "n", + "op": "n", + "value": "365460000" + } + ], + "system": "http://snomed.info/sct" + } + ], + "exclude": [ + { + "concept": [ + { + "code": "224298008" + }, + { + "code": "365460000" + }, + { + "code": "473462005" + }, + { + "code": "424587006" + } + ], + "system": "http://snomed.info/sct" + } + ] + }, + "description": "A\nselection of Education Levels", + "text": { + "status": "generated", + "div": "

Education\nLevels

http://csiro.au/ValueSet/education-levels

A selection of\nEducation Levels

" + }, + "experimental": true, + "date": "2016-07-26" +} diff --git a/hapi-fhir-jpaserver-base/src/test/resources/dstu1_bundle.xml b/hapi-fhir-jpaserver-base/src/test/resources/dstu1_bundle.xml index 5f9e5fc10fe..79c6b5bdba3 100644 --- a/hapi-fhir-jpaserver-base/src/test/resources/dstu1_bundle.xml +++ b/hapi-fhir-jpaserver-base/src/test/resources/dstu1_bundle.xml @@ -1019,14 +1019,12 @@ - - diff --git a/hapi-fhir-jpaserver-example/src/main/java/ca/uhn/fhir/jpa/demo/JpaServerDemo.java b/hapi-fhir-jpaserver-example/src/main/java/ca/uhn/fhir/jpa/demo/JpaServerDemo.java index a5d5dda1101..316ab330624 100644 --- a/hapi-fhir-jpaserver-example/src/main/java/ca/uhn/fhir/jpa/demo/JpaServerDemo.java +++ b/hapi-fhir-jpaserver-example/src/main/java/ca/uhn/fhir/jpa/demo/JpaServerDemo.java @@ -27,6 +27,7 @@ import ca.uhn.fhir.model.api.IResource; import ca.uhn.fhir.model.dstu2.composite.MetaDt; import ca.uhn.fhir.model.dstu2.resource.Bundle; import ca.uhn.fhir.narrative.DefaultThymeleafNarrativeGenerator; +import ca.uhn.fhir.parser.StrictErrorHandler; import ca.uhn.fhir.rest.server.ETagSupportEnum; import ca.uhn.fhir.rest.server.EncodingEnum; import ca.uhn.fhir.rest.server.IResourceProvider; @@ -49,10 +50,13 @@ public class JpaServerDemo extends RestfulServer { * We want to support FHIR DSTU2 format. This means that the server * will use the DSTU2 bundle format and other DSTU2 encoding changes. * - * If you want to use DSTU1 instead, change the following line, and change the 2 occurrences of dstu2 in web.xml to dstu1 + * If you want to use DSTU1 instead, change the following line, and + * change the 2 occurrences of dstu2 in web.xml to dstu1 */ FhirVersionEnum fhirVersion = FhirVersionEnum.DSTU2; - setFhirContext(new FhirContext(fhirVersion)); + FhirContext context = new FhirContext(fhirVersion); + + setFhirContext(context); // Get the spring context from the web container (it's declared in web.xml) myAppCtx = ContextLoaderListener.getCurrentWebApplicationContext(); diff --git a/hapi-fhir-structures-dstu/src/test/java/ca/uhn/fhir/validation/ResourceValidatorTest.java b/hapi-fhir-structures-dstu/src/test/java/ca/uhn/fhir/validation/ResourceValidatorTest.java index b49a9a943a5..364ab4c9ba5 100644 --- a/hapi-fhir-structures-dstu/src/test/java/ca/uhn/fhir/validation/ResourceValidatorTest.java +++ b/hapi-fhir-structures-dstu/src/test/java/ca/uhn/fhir/validation/ResourceValidatorTest.java @@ -20,6 +20,8 @@ import ca.uhn.fhir.model.dstu.resource.Patient; import ca.uhn.fhir.model.dstu.valueset.ContactSystemEnum; import ca.uhn.fhir.model.primitive.DateTimeDt; import ca.uhn.fhir.parser.DataFormatException; +import ca.uhn.fhir.parser.IParser; +import ca.uhn.fhir.parser.StrictErrorHandler; import ca.uhn.fhir.util.TestUtil; public class ResourceValidatorTest { @@ -72,23 +74,34 @@ public class ResourceValidatorTest { /** * See issue #50 */ - @Test(expected=DataFormatException.class) + @Test() public void testOutOfBoundsDate() { Patient p = new Patient(); p.setBirthDate(new DateTimeDt("2000-12-31")); // Put in an invalid date - String encoded = ourCtx.newXmlParser().setPrettyPrint(true).encodeResourceToString(p).replace("2000-12-31", "2000-15-31"); + IParser parser = ourCtx.newXmlParser(); + parser.setParserErrorHandler(new StrictErrorHandler()); + + String encoded = parser.setPrettyPrint(true).encodeResourceToString(p).replace("2000-12-31", "2000-15-31"); ourLog.info(encoded); assertThat(encoded, StringContains.containsString("2000-15-31")); - ValidationResult result = ourCtx.newValidator().validateWithResult(encoded); - String resultString = ourCtx.newXmlParser().setPrettyPrint(true).encodeResourceToString(result.toOperationOutcome()); + FhirValidator validator = ourCtx.newValidator(); + ValidationResult result = validator.validateWithResult(encoded); + String resultString = parser.setPrettyPrint(true).encodeResourceToString(result.toOperationOutcome()); ourLog.info(resultString); assertEquals(2, ((OperationOutcome)result.toOperationOutcome()).getIssue().size()); assertThat(resultString, StringContains.containsString("cvc-datatype-valid.1.2.3")); + + try { + parser.parseResource(encoded); + fail(); + } catch (DataFormatException e) { + assertEquals("DataFormatException at [[row,col {unknown-source}]: [2,4]]: Invalid attribute value \"2000-15-31\": Invalid date/time format: \"2000-15-31\"", e.getMessage()); + } } @Test diff --git a/hapi-fhir-structures-dstu2/src/test/java/ca/uhn/fhir/parser/JsonParserDstu2Test.java b/hapi-fhir-structures-dstu2/src/test/java/ca/uhn/fhir/parser/JsonParserDstu2Test.java index 97d3d8b904b..7cd5019b17f 100644 --- a/hapi-fhir-structures-dstu2/src/test/java/ca/uhn/fhir/parser/JsonParserDstu2Test.java +++ b/hapi-fhir-structures-dstu2/src/test/java/ca/uhn/fhir/parser/JsonParserDstu2Test.java @@ -170,7 +170,10 @@ public class JsonParserDstu2Test { @Test public void testParseEmptyValue() { String input = "{\"resourceType\":\"QuestionnaireResponse\",\"id\":\"123\",\"authored\":\"\",\"group\":{\"linkId\":\"\"}}"; - QuestionnaireResponse qr = ourCtx.newJsonParser().parseResource(QuestionnaireResponse.class, input); + IParser parser = ourCtx.newJsonParser(); + + parser.setParserErrorHandler(new LenientErrorHandler().setErrorOnInvalidValue(false)); + QuestionnaireResponse qr = parser.parseResource(QuestionnaireResponse.class, input); assertEquals("QuestionnaireResponse/123", qr.getIdElement().getValue()); assertEquals(null, qr.getAuthored()); diff --git a/hapi-fhir-structures-dstu2/src/test/java/ca/uhn/fhir/parser/XmlParserDstu2Test.java b/hapi-fhir-structures-dstu2/src/test/java/ca/uhn/fhir/parser/XmlParserDstu2Test.java index 4e0cdfb7c30..441acc63334 100644 --- a/hapi-fhir-structures-dstu2/src/test/java/ca/uhn/fhir/parser/XmlParserDstu2Test.java +++ b/hapi-fhir-structures-dstu2/src/test/java/ca/uhn/fhir/parser/XmlParserDstu2Test.java @@ -2524,7 +2524,10 @@ public class XmlParserDstu2Test { ""; //@formatter:on - ourCtx.newXmlParser().parseResource(resource); + + IParser parser = ourCtx.newXmlParser(); + parser.setParserErrorHandler(new StrictErrorHandler()); + parser.parseResource(resource); } @Test diff --git a/hapi-fhir-structures-dstu2/src/test/java/ca/uhn/fhir/validation/ResourceValidatorDstu2Test.java b/hapi-fhir-structures-dstu2/src/test/java/ca/uhn/fhir/validation/ResourceValidatorDstu2Test.java index bf384296410..415b2eed102 100644 --- a/hapi-fhir-structures-dstu2/src/test/java/ca/uhn/fhir/validation/ResourceValidatorDstu2Test.java +++ b/hapi-fhir-structures-dstu2/src/test/java/ca/uhn/fhir/validation/ResourceValidatorDstu2Test.java @@ -40,10 +40,12 @@ import ca.uhn.fhir.model.dstu2.valueset.ContactPointSystemEnum; import ca.uhn.fhir.model.dstu2.valueset.NarrativeStatusEnum; import ca.uhn.fhir.model.dstu2.valueset.UnitsOfTimeEnum; import ca.uhn.fhir.model.primitive.DateDt; +import ca.uhn.fhir.model.primitive.DateTimeDt; import ca.uhn.fhir.model.primitive.InstantDt; import ca.uhn.fhir.model.primitive.StringDt; import ca.uhn.fhir.parser.DataFormatException; import ca.uhn.fhir.parser.IParser; +import ca.uhn.fhir.parser.StrictErrorHandler; import ca.uhn.fhir.parser.XmlParserDstu2Test.TestPatientFor327; import ca.uhn.fhir.util.TestUtil; import ca.uhn.fhir.validation.schematron.SchematronBaseValidator; @@ -71,26 +73,33 @@ public class ResourceValidatorDstu2Test { /** * See issue #50 */ - @Test(expected=DataFormatException.class) + @Test() public void testOutOfBoundsDate() { Patient p = new Patient(); - p.setBirthDate(new DateDt("2000-15-31")); + p.setBirthDate(new DateDt("2000-12-31")); - String encoded = ourCtx.newXmlParser().setPrettyPrint(true).encodeResourceToString(p); + // Put in an invalid date + IParser parser = ourCtx.newXmlParser(); + parser.setParserErrorHandler(new StrictErrorHandler()); + + String encoded = parser.setPrettyPrint(true).encodeResourceToString(p).replace("2000-12-31", "2000-15-31"); ourLog.info(encoded); assertThat(encoded, StringContains.containsString("2000-15-31")); - p = ourCtx.newXmlParser().parseResource(Patient.class, encoded); - assertEquals("2000-15-31", p.getBirthDateElement().getValueAsString()); - assertEquals("2001-03-31", new SimpleDateFormat("yyyy-MM-dd").format(p.getBirthDate())); - - ValidationResult result = ourCtx.newValidator().validateWithResult(p); - String resultString = ourCtx.newXmlParser().setPrettyPrint(true).encodeResourceToString(result.toOperationOutcome()); + ValidationResult result = ourCtx.newValidator().validateWithResult(encoded); + String resultString = parser.setPrettyPrint(true).encodeResourceToString(result.toOperationOutcome()); ourLog.info(resultString); - assertEquals(2, ((OperationOutcome) result.toOperationOutcome()).getIssue().size()); - assertThat(resultString, StringContains.containsString("2000-15-31")); + assertEquals(2, ((OperationOutcome)result.toOperationOutcome()).getIssue().size()); + assertThat(resultString, StringContains.containsString("cvc-pattern-valid")); + + try { + parser.parseResource(encoded); + fail(); + } catch (DataFormatException e) { + assertEquals("DataFormatException at [[row,col {unknown-source}]: [2,4]]: Invalid attribute value \"2000-15-31\": Invalid date/time format: \"2000-15-31\"", e.getMessage()); + } } @SuppressWarnings("deprecation") diff --git a/hapi-fhir-structures-dstu3/src/main/java/org/hl7/fhir/dstu3/model/BaseDateTimeType.java b/hapi-fhir-structures-dstu3/src/main/java/org/hl7/fhir/dstu3/model/BaseDateTimeType.java index ecc194d87f4..c91d54658f5 100644 --- a/hapi-fhir-structures-dstu3/src/main/java/org/hl7/fhir/dstu3/model/BaseDateTimeType.java +++ b/hapi-fhir-structures-dstu3/src/main/java/org/hl7/fhir/dstu3/model/BaseDateTimeType.java @@ -1,6 +1,7 @@ package org.hl7.fhir.dstu3.model; import static org.apache.commons.lang3.StringUtils.isBlank; +import static org.hamcrest.Matchers.emptyOrNullString; import java.util.Calendar; import java.util.Date; @@ -334,7 +335,7 @@ public abstract class BaseDateTimeType extends PrimitiveType { myFractionalSeconds = ""; } - setPrecision(precision); + myPrecision = precision; return cal.getTime(); } @@ -372,8 +373,8 @@ public abstract class BaseDateTimeType extends PrimitiveType { if (isBlank(theValue)) { throwBadDateFormat(theWholeValue); } else if (theValue.charAt(0) == 'Z') { - clearTimeZone(); - setTimeZoneZulu(true); + myTimeZone = null; + myTimeZoneZulu = true; } else if (theValue.length() != 6) { throwBadDateFormat(theWholeValue, "Timezone offset must be in the form \"Z\", \"-HH:mm\", or \"+HH:mm\""); } else if (theValue.charAt(3) != ':' || !(theValue.charAt(0) == '+' || theValue.charAt(0) == '-')) { @@ -381,8 +382,8 @@ public abstract class BaseDateTimeType extends PrimitiveType { } else { parseInt(theWholeValue, theValue.substring(1, 3), 0, 23); parseInt(theWholeValue, theValue.substring(4, 6), 0, 59); - clearTimeZone(); - setTimeZone(TimeZone.getTimeZone("GMT" + theValue)); + myTimeZoneZulu = false; + myTimeZone = TimeZone.getTimeZone("GMT" + theValue); } return this; @@ -390,12 +391,14 @@ public abstract class BaseDateTimeType extends PrimitiveType { public BaseDateTimeType setTimeZone(TimeZone theTimeZone) { myTimeZone = theTimeZone; + myTimeZoneZulu = false; updateStringValue(); return this; } public BaseDateTimeType setTimeZoneZulu(boolean theTimeZoneZulu) { myTimeZoneZulu = theTimeZoneZulu; + myTimeZone = null; updateStringValue(); return this; } diff --git a/hapi-fhir-structures-dstu3/src/main/java/org/hl7/fhir/dstu3/model/PrimitiveType.java b/hapi-fhir-structures-dstu3/src/main/java/org/hl7/fhir/dstu3/model/PrimitiveType.java index ec9cd59c732..f9e3d4d6037 100644 --- a/hapi-fhir-structures-dstu3/src/main/java/org/hl7/fhir/dstu3/model/PrimitiveType.java +++ b/hapi-fhir-structures-dstu3/src/main/java/org/hl7/fhir/dstu3/model/PrimitiveType.java @@ -70,13 +70,13 @@ public abstract class PrimitiveType extends Type implements IPrimitiveType } public void fromStringValue(String theValue) { + myStringValue = theValue; if (theValue == null) { myCoercedValue = null; } else { // NB this might be null myCoercedValue = parse(theValue); } - myStringValue = theValue; } public T getValue() { diff --git a/hapi-fhir-structures-dstu3/src/test/java/ca/uhn/fhir/parser/ErrorHandlerTest.java b/hapi-fhir-structures-dstu3/src/test/java/ca/uhn/fhir/parser/ErrorHandlerTest.java index 42d721ebff6..3e6e4f31137 100644 --- a/hapi-fhir-structures-dstu3/src/test/java/ca/uhn/fhir/parser/ErrorHandlerTest.java +++ b/hapi-fhir-structures-dstu3/src/test/java/ca/uhn/fhir/parser/ErrorHandlerTest.java @@ -1,5 +1,7 @@ package ca.uhn.fhir.parser; +import static org.junit.Assert.*; + import org.junit.Test; import ca.uhn.fhir.parser.json.JsonLikeValue.ValueType; @@ -46,7 +48,14 @@ public class ErrorHandlerTest { new LenientErrorHandler().containedResourceWithNoId(null); new LenientErrorHandler().unknownReference(null, null); new LenientErrorHandler().incorrectJsonType(null, null, ValueType.ARRAY, ValueType.SCALAR); - new LenientErrorHandler().invalidValue(null, null, null); + new LenientErrorHandler().setErrorOnInvalidValue(false).invalidValue(null, "FOO", ""); + new LenientErrorHandler().invalidValue(null, null, ""); + try { + new LenientErrorHandler().invalidValue(null, "FOO", ""); + fail(); + } catch (DataFormatException e) { + // good, this one method defaults to causing an error + } } @Test(expected = DataFormatException.class) diff --git a/hapi-fhir-structures-dstu3/src/test/java/ca/uhn/fhir/parser/JsonParserDstu3Test.java b/hapi-fhir-structures-dstu3/src/test/java/ca/uhn/fhir/parser/JsonParserDstu3Test.java index 5f3798ef146..f9072a0788b 100644 --- a/hapi-fhir-structures-dstu3/src/test/java/ca/uhn/fhir/parser/JsonParserDstu3Test.java +++ b/hapi-fhir-structures-dstu3/src/test/java/ca/uhn/fhir/parser/JsonParserDstu3Test.java @@ -13,35 +13,78 @@ import static org.junit.Assert.assertSame; import static org.junit.Assert.assertThat; import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; +import static org.mockito.Matchers.eq; +import static org.mockito.Matchers.isNull; import static org.mockito.Mockito.atLeastOnce; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; -import java.io.ByteArrayInputStream; import java.io.IOException; -import java.io.ObjectInputStream; import java.math.BigDecimal; import java.nio.charset.StandardCharsets; -import java.util.*; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Date; +import java.util.HashSet; +import java.util.List; +import java.util.UUID; import org.apache.commons.io.IOUtils; import org.hamcrest.Matchers; import org.hamcrest.core.StringContains; -import org.hl7.fhir.dstu3.model.*; import org.hl7.fhir.dstu3.model.Address.AddressUse; import org.hl7.fhir.dstu3.model.Address.AddressUseEnumFactory; +import org.hl7.fhir.dstu3.model.Attachment; +import org.hl7.fhir.dstu3.model.AuditEvent; +import org.hl7.fhir.dstu3.model.Basic; +import org.hl7.fhir.dstu3.model.Binary; +import org.hl7.fhir.dstu3.model.Bundle; import org.hl7.fhir.dstu3.model.Bundle.BundleEntryComponent; import org.hl7.fhir.dstu3.model.Bundle.BundleType; +import org.hl7.fhir.dstu3.model.Claim; +import org.hl7.fhir.dstu3.model.Coding; +import org.hl7.fhir.dstu3.model.Communication; +import org.hl7.fhir.dstu3.model.Condition; import org.hl7.fhir.dstu3.model.Condition.ConditionVerificationStatus; +import org.hl7.fhir.dstu3.model.Conformance; import org.hl7.fhir.dstu3.model.Conformance.UnknownContentCode; +import org.hl7.fhir.dstu3.model.Coverage; +import org.hl7.fhir.dstu3.model.DateTimeType; +import org.hl7.fhir.dstu3.model.DateType; +import org.hl7.fhir.dstu3.model.DecimalType; +import org.hl7.fhir.dstu3.model.DiagnosticReport; +import org.hl7.fhir.dstu3.model.EnumFactory; import org.hl7.fhir.dstu3.model.Enumeration; import org.hl7.fhir.dstu3.model.Enumerations.AdministrativeGender; +import org.hl7.fhir.dstu3.model.ExplanationOfBenefit; +import org.hl7.fhir.dstu3.model.Extension; +import org.hl7.fhir.dstu3.model.HumanName; +import org.hl7.fhir.dstu3.model.IdType; import org.hl7.fhir.dstu3.model.Identifier.IdentifierUse; +import org.hl7.fhir.dstu3.model.Linkage; +import org.hl7.fhir.dstu3.model.Medication; +import org.hl7.fhir.dstu3.model.MedicationRequest; +import org.hl7.fhir.dstu3.model.Observation; import org.hl7.fhir.dstu3.model.Observation.ObservationStatus; -import org.hl7.fhir.instance.model.api.IBaseResource; +import org.hl7.fhir.dstu3.model.Parameters; +import org.hl7.fhir.dstu3.model.Patient; +import org.hl7.fhir.dstu3.model.PrimitiveType; +import org.hl7.fhir.dstu3.model.Quantity; +import org.hl7.fhir.dstu3.model.QuestionnaireResponse; +import org.hl7.fhir.dstu3.model.Reference; +import org.hl7.fhir.dstu3.model.RelatedPerson; +import org.hl7.fhir.dstu3.model.SampledData; +import org.hl7.fhir.dstu3.model.SimpleQuantity; +import org.hl7.fhir.dstu3.model.StringType; +import org.hl7.fhir.dstu3.model.UriType; +import org.hl7.fhir.dstu3.model.ValueSet; import org.hl7.fhir.utilities.xhtml.XhtmlNode; -import org.junit.*; +import org.junit.After; +import org.junit.AfterClass; +import org.junit.Assert; +import org.junit.Ignore; +import org.junit.Test; import org.mockito.ArgumentCaptor; import org.mockito.Mockito; @@ -49,7 +92,6 @@ import com.google.common.collect.Sets; import ca.uhn.fhir.context.FhirContext; import ca.uhn.fhir.narrative.DefaultThymeleafNarrativeGenerator; -import ca.uhn.fhir.parser.DataFormatException; import ca.uhn.fhir.parser.IParserErrorHandler.IParseLocation; import ca.uhn.fhir.parser.PatientWithExtendedContactDstu3.CustomContactComponent; import ca.uhn.fhir.parser.XmlParserDstu3Test.TestPatientFor327; @@ -93,7 +135,7 @@ public class JsonParserDstu3Test { IParser p = ourCtx.newJsonParser().setPrettyPrint(true); String encoded = p.encodeResourceToString(resource); ourLog.info(encoded); - + assertEquals(3, countMatches(encoded, "resourceType")); } @@ -122,15 +164,18 @@ public class JsonParserDstu3Test { */ @Test public void testParseEmptyValue() { - String input = "{\"resourceType\":\"QuestionnaireResponse\",\"id\":\"123\",\"authored\":\"\",\"item\":[{\"item\":[{\"linkId\":\"\"}]}]}"; - QuestionnaireResponse qr = ourCtx.newJsonParser().parseResource(QuestionnaireResponse.class, input); - + String input = "{\"resourceType\":\"QuestionnaireResponse\",\"id\":\"123\",\"authored\":\"\",\"group\":{\"linkId\":\"\"}}"; + IParser parser = ourCtx.newJsonParser(); + + parser.setParserErrorHandler(new LenientErrorHandler().setErrorOnInvalidValue(false)); + QuestionnaireResponse qr = parser.parseResource(QuestionnaireResponse.class, input); + assertEquals("QuestionnaireResponse/123", qr.getIdElement().getValue()); assertEquals(null, qr.getAuthored()); assertEquals(null, qr.getAuthoredElement().getValue()); assertEquals(null, qr.getAuthoredElement().getValueAsString()); - assertEquals(null, qr.getItemFirstRep().getItemFirstRep().getLinkId()); - assertEquals(null, qr.getItemFirstRep().getItemFirstRep().getLinkIdElement().getValue()); + assertEquals(null, qr.getItemFirstRep().getLinkId()); + assertEquals(null, qr.getItemFirstRep().getLinkIdElement().getValue()); } /** @@ -1194,17 +1239,77 @@ public class JsonParserDstu3Test { assertEquals("{\"resourceType\":\"Observation\",\"valueQuantity\":{\"value\":0.0000000000000001}}", str); } - /** - * #516 - */ - @Test(expected=DataFormatException.class) - public void testInvalidEnumValue() { - String res = "{ \"resourceType\": \"ValueSet\", \"url\": \"http://sample/ValueSet/education-levels\", \"version\": \"1\", \"name\": \"Education Levels\", \"status\": \"draft\", \"compose\": { \"include\": [ { \"filter\": [ { \"property\": \"n\", \"op\": \"n\", \"value\": \"365460000\" } ], \"system\": \"http://snomed.info/sct\" } ], \"exclude\": [ { \"concept\": [ { \"code\": \"224298008\" }, { \"code\": \"365460000\" }, { \"code\": \"473462005\" }, { \"code\": \"424587006\" } ], \"system\": \"http://snomed.info/sct\" } ] }, \"description\": \"A selection of Education Levels\", \"text\": { \"status\": \"generated\", \"div\": \"

Education Levels

http://csiro.au/ValueSet/education-levels

A selection of Education Levels

\" }, \"experimental\": true, \"date\": \"2016-07-26\" }"; - IParser parser = ourCtx.newJsonParser(); - parser.setParserErrorHandler(new StrictErrorHandler()); - ValueSet parsed = parser.parseResource(ValueSet.class, res); - fail("DataFormat Invalid attribute exception should be thrown"); - } + /** + * #516 + */ + @Test(expected = DataFormatException.class) + public void testInvalidEnumValue() { + String res = "{ \"resourceType\": \"ValueSet\", \"url\": \"http://sample/ValueSet/education-levels\", \"version\": \"1\", \"name\": \"Education Levels\", \"status\": \"draft\", \"compose\": { \"include\": [ { \"filter\": [ { \"property\": \"n\", \"op\": \"n\", \"value\": \"365460000\" } ], \"system\": \"http://snomed.info/sct\" } ], \"exclude\": [ { \"concept\": [ { \"code\": \"224298008\" }, { \"code\": \"365460000\" }, { \"code\": \"473462005\" }, { \"code\": \"424587006\" } ], \"system\": \"http://snomed.info/sct\" } ] }, \"description\": \"A selection of Education Levels\", \"text\": { \"status\": \"generated\", \"div\": \"

Education Levels

http://csiro.au/ValueSet/education-levels

A selection of Education Levels

\" }, \"experimental\": true, \"date\": \"2016-07-26\" }"; + IParser parser = ourCtx.newJsonParser(); + parser.setParserErrorHandler(new StrictErrorHandler()); + ValueSet parsed = parser.parseResource(ValueSet.class, res); + fail("DataFormat Invalid attribute exception should be thrown"); + } + + @Test + public void testInvalidEnumValueBlank() { + IParserErrorHandler errorHandler = mock(IParserErrorHandler.class); + + String res = "{ \"resourceType\": \"Patient\", \"gender\": \"\" }"; + IParser parser = ourCtx.newJsonParser(); + parser.setParserErrorHandler(errorHandler); + Patient parsed = parser.parseResource(Patient.class, res); + + assertEquals(null, parsed.getGenderElement().getValue()); + assertEquals(null, parsed.getGenderElement().getValueAsString()); + + ArgumentCaptor msgCaptor = ArgumentCaptor.forClass(String.class); + verify(errorHandler, times(1)).invalidValue(isNull(IParseLocation.class), eq(""), msgCaptor.capture()); + assertEquals("Attribute values must not be empty (\"\")", msgCaptor.getValue()); + + String encoded = ourCtx.newJsonParser().encodeResourceToString(parsed); + assertEquals("{\"resourceType\":\"Patient\"}", encoded); + } + + @Test + public void testInvalidEnumValueInvalid() { + IParserErrorHandler errorHandler = mock(IParserErrorHandler.class); + + String res = "{ \"resourceType\": \"Patient\", \"gender\": \"foo\" }"; + IParser parser = ourCtx.newJsonParser(); + parser.setParserErrorHandler(errorHandler); + Patient parsed = parser.parseResource(Patient.class, res); + + assertEquals(null, parsed.getGenderElement().getValue()); + assertEquals("foo", parsed.getGenderElement().getValueAsString()); + + ArgumentCaptor msgCaptor = ArgumentCaptor.forClass(String.class); + verify(errorHandler, times(1)).invalidValue(isNull(IParseLocation.class), eq("foo"), msgCaptor.capture()); + assertEquals("Unknown AdministrativeGender code 'foo'", msgCaptor.getValue()); + + String encoded = ourCtx.newJsonParser().encodeResourceToString(parsed); + assertEquals("{\"resourceType\":\"Patient\",\"gender\":\"foo\"}", encoded); + } + + @Test + public void testInvalidDateTimeValueInvalid() throws Exception { + IParserErrorHandler errorHandler = mock(IParserErrorHandler.class); + + String res = "{ \"resourceType\": \"Observation\", \"valueDateTime\": \"foo\" }"; + IParser parser = ourCtx.newJsonParser(); + parser.setParserErrorHandler(errorHandler); + Observation parsed = parser.parseResource(Observation.class, res); + + assertEquals(null, parsed.getValueDateTimeType().getValue()); + assertEquals("foo", parsed.getValueDateTimeType().getValueAsString()); + + ArgumentCaptor msgCaptor = ArgumentCaptor.forClass(String.class); + verify(errorHandler, times(1)).invalidValue(isNull(IParseLocation.class), eq("foo"), msgCaptor.capture()); + assertEquals("Invalid date/time format: \"foo\"", msgCaptor.getValue()); + + String encoded = ourCtx.newJsonParser().encodeResourceToString(parsed); + assertEquals("{\"resourceType\":\"Observation\",\"valueDateTime\":\"foo\"}", encoded); + } /** * #65 diff --git a/hapi-fhir-structures-dstu3/src/test/java/ca/uhn/fhir/parser/XmlParserDstu3Test.java b/hapi-fhir-structures-dstu3/src/test/java/ca/uhn/fhir/parser/XmlParserDstu3Test.java index 8150d592e10..3ac7d05a430 100644 --- a/hapi-fhir-structures-dstu3/src/test/java/ca/uhn/fhir/parser/XmlParserDstu3Test.java +++ b/hapi-fhir-structures-dstu3/src/test/java/ca/uhn/fhir/parser/XmlParserDstu3Test.java @@ -2855,7 +2855,7 @@ public class XmlParserDstu3Test { /** * See #366 */ - @Test(expected = DataFormatException.class) + @Test() public void testParseInvalidBoolean() { //@formatter:off String resource = "\n" + @@ -2863,7 +2863,19 @@ public class XmlParserDstu3Test { ""; //@formatter:on - ourCtx.newXmlParser().parseResource(resource); + IParser p = ourCtx.newXmlParser(); + + try { + p.parseResource(resource); + fail(); + } catch (DataFormatException e) { + assertEquals("DataFormatException at [[row,col {unknown-source}]: [2,4]]: Invalid attribute value \"1\": Invalid boolean string: '1'", e.getMessage()); + } + + LenientErrorHandler errorHandler = new LenientErrorHandler(); + assertEquals(true, errorHandler.isErrorOnInvalidValue()); + errorHandler.setErrorOnInvalidValue(false); + p.setParserErrorHandler(errorHandler); } @Test diff --git a/hapi-fhir-structures-dstu3/src/test/java/org/hl7/fhir/dstu3/hapi/validation/ResourceValidatorDstu3Test.java b/hapi-fhir-structures-dstu3/src/test/java/org/hl7/fhir/dstu3/hapi/validation/ResourceValidatorDstu3Test.java index 952f3c79771..8dcf1745855 100644 --- a/hapi-fhir-structures-dstu3/src/test/java/org/hl7/fhir/dstu3/hapi/validation/ResourceValidatorDstu3Test.java +++ b/hapi-fhir-structures-dstu3/src/test/java/org/hl7/fhir/dstu3/hapi/validation/ResourceValidatorDstu3Test.java @@ -3,8 +3,10 @@ package org.hl7.fhir.dstu3.hapi.validation; import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.not; import static org.hamcrest.Matchers.stringContainsInOrder; +import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertThat; import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; import java.io.IOException; import java.util.ArrayList; @@ -12,14 +14,24 @@ import java.util.Date; import java.util.List; import org.apache.commons.io.IOUtils; -import org.hl7.fhir.dstu3.model.*; +import org.hamcrest.core.StringContains; +import org.hl7.fhir.dstu3.model.CodeableConcept; +import org.hl7.fhir.dstu3.model.Coding; +import org.hl7.fhir.dstu3.model.Condition; import org.hl7.fhir.dstu3.model.Condition.ConditionVerificationStatus; +import org.hl7.fhir.dstu3.model.DateType; import org.hl7.fhir.dstu3.model.Narrative.NarrativeStatus; +import org.hl7.fhir.dstu3.model.OperationOutcome; +import org.hl7.fhir.dstu3.model.Patient; +import org.hl7.fhir.dstu3.model.Reference; +import org.hl7.fhir.dstu3.model.StringType; import org.junit.AfterClass; import org.junit.Test; import ca.uhn.fhir.context.FhirContext; +import ca.uhn.fhir.parser.DataFormatException; import ca.uhn.fhir.parser.IParser; +import ca.uhn.fhir.parser.StrictErrorHandler; import ca.uhn.fhir.parser.XmlParserDstu3Test; import ca.uhn.fhir.util.TestUtil; import ca.uhn.fhir.validation.FhirValidator; @@ -37,6 +49,39 @@ public class ResourceValidatorDstu3Test { TestUtil.clearAllStaticFieldsForUnitTest(); } + /** + * See issue #50 + */ + @Test() + public void testOutOfBoundsDate() { + Patient p = new Patient(); + p.setBirthDateElement(new DateType("2000-12-31")); + + // Put in an invalid date + IParser parser = ourCtx.newXmlParser(); + parser.setParserErrorHandler(new StrictErrorHandler()); + + String encoded = parser.setPrettyPrint(true).encodeResourceToString(p).replace("2000-12-31", "2000-15-31"); + ourLog.info(encoded); + + assertThat(encoded, StringContains.containsString("2000-15-31")); + + ValidationResult result = ourCtx.newValidator().validateWithResult(encoded); + String resultString = parser.setPrettyPrint(true).encodeResourceToString(result.toOperationOutcome()); + ourLog.info(resultString); + + assertEquals(2, ((OperationOutcome)result.toOperationOutcome()).getIssue().size()); + assertThat(resultString, StringContains.containsString("cvc-pattern-valid")); + + try { + parser.parseResource(encoded); + fail(); + } catch (DataFormatException e) { + assertEquals("DataFormatException at [[row,col {unknown-source}]: [2,4]]: Invalid attribute value \"2000-15-31\": Invalid date/time format: \"2000-15-31\"", e.getMessage()); + } + } + + /** * Make sure that the elements that appear in all resources (meta, language, extension, etc) all appear in the correct order */ diff --git a/src/changes/changes.xml b/src/changes/changes.xml index 359fadd2801..9a1ae9fa9f8 100644 --- a/src/changes/changes.xml +++ b/src/changes/changes.xml @@ -99,6 +99,21 @@ not by throwing an exception. Thanks to Michael Lawley for the pull request! + + When parsing DSTU3 resources with enumerated + types that contain invalid values, the parser will now + invoke the parserErrorHandler. For example, when parsing + {"resourceType":"Patient", "gender":"foo"} + ]]> + the previous behaviour was to throw an InvalidArgumentException. + Now, the parserErrorHandler is invoked. In addition, thw + LenientErrorHandler has been modified so that this one case + will result in a DataFormatException. This has the effect + that servers which receive an invalid enum velue will return + an HTTP 400 instead of an HTTP 500. Thanks to Jim + Steel for reporting! + diff --git a/src/site/xdoc/doc_validation.xml b/src/site/xdoc/doc_validation.xml index 8f7c50503a4..67524c7c656 100644 --- a/src/site/xdoc/doc_validation.xml +++ b/src/site/xdoc/doc_validation.xml @@ -14,12 +14,6 @@ sections below.

    -
  • - Resource Validation - is validation of the raw or parsed resource against - the official FHIR validation rules (e.g. Schema/Schematron/Profile/StructureDefinition/ValueSet) - as well as against custom profiles which have been developed. -
  • Parser Validation is validation at runtime during the parsing @@ -31,10 +25,77 @@ that no data is being lost during parsing, but is less comprehensive than resource validation against raw text data.
    • +
  • + Resource Validation + is validation of the raw or parsed resource against + the official FHIR validation rules (e.g. Schema/Schematron/Profile/StructureDefinition/ValueSet) + as well as against custom profiles which have been developed. +
+ +
+ +

+ Parser validation is controlled by calling + setParserErrorHandler(IParserErrorHandler) + on + either the FhirContext or on individual parser instances. This method + takes an + IParserErrorHandler + , which is a callback that + will be invoked any time a parse issue is detected. +

+

+ There are two implementations of + IParserErrorHandler + worth + mentioning. You can also supply your own implementation if you want. +

+
    +
  • + LenientErrorHandler + logs any errors but does not abort parsing. By default this handler is used, and it + logs errors at "warning" level. It can also be configured to silently + ignore issues. +
    • +
  • + StrictErrorHandler + throws a + DataFormatException + if any errors are detected. +
    • +
+ +

+ The following example shows how to configure a parser to use strict validation. +

+ + + + + +

+ You can also configure the error handler at the FhirContext level, which is useful + for clients. +

+ + + + + +

+ FhirContext level validators can also be useful on servers. +

+ + + + + +
+
@@ -223,66 +284,6 @@
-
- -

- Parser validation is controlled by calling - setParserErrorHandler(IParserErrorHandler) - on - either the FhirContext or on individual parser instances. This method - takes an - IParserErrorHandler - , which is a callback that - will be invoked any time a parse issue is detected. -

-

- There are two implementations of - IParserErrorHandler - worth - mentioning. You can also supply your own implementation if you want. -

-
    -
  • - LenientErrorHandler - logs any errors but does not abort parsing. By default this handler is used, and it - logs errors at "warning" level. It can also be configured to silently - ignore issues. -
    • -
  • - StrictErrorHandler - throws a - DataFormatException - if any errors are detected. -
    • -
- -

- The following example shows how to configure a parser to use strict validation. -

- - - - - -

- You can also configure the error handler at the FhirContext level, which is useful - for clients. -

- - - - - -

- FhirContext level validators can also be useful on servers. -

- - - - - -
-