From 964919d880b54f1420420cf79c3dbfb0ca1d4c11 Mon Sep 17 00:00:00 2001 From: James Date: Mon, 19 Dec 2016 14:03:11 -0500 Subject: [PATCH] Improve parsing --- .../uhn/fhir/parser/ErrorHandlerAdapter.java | 3 +- .../uhn/fhir/parser/IParserErrorHandler.java | 10 +- .../java/ca/uhn/fhir/parser/JsonParser.java | 31 +++-- .../uhn/fhir/parser/LenientErrorHandler.java | 47 +++++-- .../java/ca/uhn/fhir/parser/ParserState.java | 124 ++++++++++-------- .../uhn/fhir/parser/StrictErrorHandler.java | 6 +- .../ca/uhn/fhir/parser/JsonParserTest.java | 9 ++ .../ca/uhn/fhir/parser/ErrorHandlerTest.java | 18 ++- .../ResourceValidatorDstu2Test.java | 12 +- .../ca/uhn/fhir/parser/ErrorHandlerTest.java | 6 +- .../uhn/fhir/parser/JsonParserDstu3Test.java | 87 +++++++++++- .../ResourceValidatorDstu3Test.java | 51 +++++-- .../validation/FhirInstanceValidatorTest.java | 30 ++++- src/changes/changes.xml | 4 + 14 files changed, 334 insertions(+), 104 deletions(-) diff --git a/hapi-fhir-base/src/main/java/ca/uhn/fhir/parser/ErrorHandlerAdapter.java b/hapi-fhir-base/src/main/java/ca/uhn/fhir/parser/ErrorHandlerAdapter.java index cf3ff095b6d..68facd0083d 100644 --- a/hapi-fhir-base/src/main/java/ca/uhn/fhir/parser/ErrorHandlerAdapter.java +++ b/hapi-fhir-base/src/main/java/ca/uhn/fhir/parser/ErrorHandlerAdapter.java @@ -1,5 +1,6 @@ package ca.uhn.fhir.parser; +import ca.uhn.fhir.parser.json.JsonLikeValue.ScalarType; import ca.uhn.fhir.parser.json.JsonLikeValue.ValueType; /* @@ -33,7 +34,7 @@ public class ErrorHandlerAdapter implements IParserErrorHandler { } @Override - public void incorrectJsonType(IParseLocation theLocation, String theElementName, ValueType theExpected, ValueType theFound) { + public void incorrectJsonType(IParseLocation theLocation, String theElementName, ValueType theExpected, ScalarType theExpectedScalarType, ValueType theFound, ScalarType theFoundScalarType) { // NOP } diff --git a/hapi-fhir-base/src/main/java/ca/uhn/fhir/parser/IParserErrorHandler.java b/hapi-fhir-base/src/main/java/ca/uhn/fhir/parser/IParserErrorHandler.java index 23d5f83dafb..8675b110ca4 100644 --- a/hapi-fhir-base/src/main/java/ca/uhn/fhir/parser/IParserErrorHandler.java +++ b/hapi-fhir-base/src/main/java/ca/uhn/fhir/parser/IParserErrorHandler.java @@ -1,5 +1,6 @@ package ca.uhn.fhir.parser; +import ca.uhn.fhir.parser.json.JsonLikeValue.ScalarType; import ca.uhn.fhir.parser.json.JsonLikeValue.ValueType; /* @@ -44,11 +45,14 @@ public interface IParserErrorHandler { * The location in the document. Note that this may be null as the ParseLocation feature is experimental. Use with caution, as the API may change. * @param theElementName * The name of the element that was found. - * @param theFound The datatype that was found at this location - * @param theExpected The datatype that was expected at this location + * @param theExpectedValueType The datatype that was expected at this location + * @param theExpectedScalarType If theExpectedValueType is {@link ValueType#SCALAR}, this is the specific scalar type expected. Otherwise this parameter will be null. + * @param theFoundValueType The datatype that was found at this location + * @param theFoundScalarType If theFoundValueType is {@link ValueType#SCALAR}, this is the specific scalar type found. Otherwise this parameter will be null. + * * @since 2.2 */ - void incorrectJsonType(IParseLocation theLocation, String theElementName, ValueType theExpected, ValueType theFound); + void incorrectJsonType(IParseLocation theLocation, String theElementName, ValueType theExpectedValueType, ScalarType theExpectedScalarType, ValueType theFoundValueType, ScalarType theFoundScalarType); /** * The parser detected an atttribute value that was invalid (such as: empty "" values are not permitted) diff --git a/hapi-fhir-base/src/main/java/ca/uhn/fhir/parser/JsonParser.java b/hapi-fhir-base/src/main/java/ca/uhn/fhir/parser/JsonParser.java index 6ea2d268e7f..2fd9470f13b 100644 --- a/hapi-fhir-base/src/main/java/ca/uhn/fhir/parser/JsonParser.java +++ b/hapi-fhir-base/src/main/java/ca/uhn/fhir/parser/JsonParser.java @@ -54,6 +54,7 @@ import ca.uhn.fhir.parser.json.JsonLikeObject; import ca.uhn.fhir.parser.json.JsonLikeStructure; import ca.uhn.fhir.parser.json.JsonLikeValue; import ca.uhn.fhir.parser.json.JsonLikeWriter; +import ca.uhn.fhir.parser.json.JsonLikeValue.ScalarType; import ca.uhn.fhir.parser.json.JsonLikeValue.ValueType; import ca.uhn.fhir.rest.server.EncodingEnum; import ca.uhn.fhir.util.ElementUtil; @@ -1106,7 +1107,7 @@ public class JsonParser extends BaseParser implements IJsonLikeParser { JsonLikeValue alternateVal = theAlternateVal; if (alternateVal != null && alternateVal.isObject() == false) { - getErrorHandler().incorrectJsonType(null, theAlternateName, ValueType.OBJECT, alternateVal.getJsonType()); + getErrorHandler().incorrectJsonType(null, theAlternateName, ValueType.OBJECT, null, alternateVal.getJsonType(), null); return; } @@ -1122,8 +1123,10 @@ public class JsonParser extends BaseParser implements IJsonLikeParser { JsonLikeArray array = nextVal.getAsArray(); parseExtension(theState, array, isModifier); } else if ("id".equals(nextKey)) { - if (nextVal.isString() || nextVal.isNumber()) { + if (nextVal.isString()) { theState.attributeValue("id", nextVal.getAsString()); + } else { + getErrorHandler().incorrectJsonType(null, "id", ValueType.SCALAR, ScalarType.STRING, nextVal.getJsonType(), nextVal.getDataType()); } } else if ("fhir_comments".equals(nextKey)) { parseFhirComments(nextVal, theState); @@ -1249,7 +1252,7 @@ public class JsonParser extends BaseParser implements IJsonLikeParser { } JsonLikeValue nextVal = theObject.get(nextName); - parseChildren(theState, nextName, nextVal, null, null); + parseChildren(theState, nextName, nextVal, null, null, false); } } @@ -1286,7 +1289,7 @@ public class JsonParser extends BaseParser implements IJsonLikeParser { handledUnderscoreNames++; } - parseChildren(theState, nextName, nextVal, alternateVal, alternateName); + parseChildren(theState, nextName, nextVal, alternateVal, alternateName, false); } @@ -1318,7 +1321,7 @@ public class JsonParser extends BaseParser implements IJsonLikeParser { theState.endingElement(); } } else { - getErrorHandler().incorrectJsonType(null, alternateName, ValueType.OBJECT, nextValue.getJsonType()); + getErrorHandler().incorrectJsonType(null, alternateName, ValueType.OBJECT, null, nextValue.getJsonType(), null); } } } @@ -1327,13 +1330,19 @@ public class JsonParser extends BaseParser implements IJsonLikeParser { } - private void parseChildren(ParserState theState, String theName, JsonLikeValue theJsonVal, JsonLikeValue theAlternateVal, String theAlternateName) { + private void parseChildren(ParserState theState, String theName, JsonLikeValue theJsonVal, JsonLikeValue theAlternateVal, String theAlternateName, boolean theInArray) { + if (theName.equals("id")) { + if (!theJsonVal.isString()) { + getErrorHandler().incorrectJsonType(null, "id", ValueType.SCALAR, ScalarType.STRING, theJsonVal.getJsonType(), theJsonVal.getDataType()); + } + } + if (theJsonVal.isArray()) { JsonLikeArray nextArray = theJsonVal.getAsArray(); JsonLikeValue alternateVal = theAlternateVal; if (alternateVal != null && alternateVal.isArray() == false) { - getErrorHandler().incorrectJsonType(null, theAlternateName, ValueType.ARRAY, alternateVal.getJsonType()); + getErrorHandler().incorrectJsonType(null, theAlternateName, ValueType.ARRAY, null, alternateVal.getJsonType(), null); alternateVal = null; } @@ -1344,9 +1353,13 @@ public class JsonParser extends BaseParser implements IJsonLikeParser { if (nextAlternateArray != null) { nextAlternate = nextAlternateArray.get(i); } - parseChildren(theState, theName, nextObject, nextAlternate, theAlternateName); + parseChildren(theState, theName, nextObject, nextAlternate, theAlternateName, true); } } else if (theJsonVal.isObject()) { + if (!theInArray && theState.elementIsRepeating(theName)) { + getErrorHandler().incorrectJsonType(null, theName, ValueType.ARRAY, null, ValueType.OBJECT, null); + } + theState.enteringNewElement(null, theName); parseAlternates(theAlternateVal, theState, theAlternateName, theAlternateName); JsonLikeObject nextObject = theJsonVal.getAsObject(); @@ -1406,7 +1419,7 @@ public class JsonParser extends BaseParser implements IJsonLikeParser { parseExtension(theState, jsonVal, true); } else { JsonLikeValue jsonVal = nextExtObj.get(next); - parseChildren(theState, next, jsonVal, null, null); + parseChildren(theState, next, jsonVal, null, null, false); } } theState.endingElement(); 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 fced37beb77..e0b1e0414d5 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,9 +1,9 @@ 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.ScalarType; import ca.uhn.fhir.parser.json.JsonLikeValue.ValueType; /* @@ -16,7 +16,7 @@ import ca.uhn.fhir.parser.json.JsonLikeValue.ValueType; * 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, @@ -29,7 +29,7 @@ import ca.uhn.fhir.parser.json.JsonLikeValue.ValueType; /** * 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)} + * 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. @@ -55,7 +55,8 @@ public class LenientErrorHandler implements IParserErrorHandler { /** * Constructor * - * @param theLogErrors Should errors be logged? + * @param theLogErrors + * Should errors be logged? * @since 1.2 */ public LenientErrorHandler(boolean theLogErrors) { @@ -70,9 +71,12 @@ public class LenientErrorHandler implements IParserErrorHandler { } @Override - public void incorrectJsonType(IParseLocation theLocation, String theElementName, ValueType theExpected, ValueType theFound) { + public void incorrectJsonType(IParseLocation theLocation, String theElementName, ValueType theExpected, ScalarType theExpectedScalarType, ValueType theFound, ScalarType theFoundScalarType) { if (myLogErrors) { - ourLog.warn("Found incorrect type for element {} - Expected {} and found {}", new Object[]{theElementName, theExpected.name(), theFound.name()}); + if (ourLog.isWarnEnabled()) { + String message = createIncorrectJsonTypeMessage(theElementName, theExpected, theExpectedScalarType, theFound, theFoundScalarType); + ourLog.warn(message); + } } } @@ -88,11 +92,11 @@ public class LenientErrorHandler implements IParserErrorHandler { } /** - * If set to false (default is true) invalid values will be logged. By + * 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 + * 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) *

* @@ -110,13 +114,14 @@ public class LenientErrorHandler implements IParserErrorHandler { } /** - * If set to false (default is true) invalid values will be logged. By + * 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 + * 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() */ @@ -153,4 +158,26 @@ public class LenientErrorHandler implements IParserErrorHandler { } } + public static String createIncorrectJsonTypeMessage(String theElementName, ValueType theExpected, ScalarType theExpectedScalarType, ValueType theFound, ScalarType theFoundScalarType) { + StringBuilder b = new StringBuilder(); + b.append("Found incorrect type for element "); + b.append(theElementName); + b.append(" - Expected "); + b.append(theExpected.name()); + if (theExpectedScalarType != null) { + b.append(" ("); + b.append(theExpectedScalarType.name()); + b.append(")"); + } + b.append(" and found "); + b.append(theFound.name()); + if (theFoundScalarType != null) { + b.append(" ("); + b.append(theFoundScalarType.name()); + b.append(")"); + } + String message = b.toString(); + return message; + } + } 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 bc3f927af0e..2691d379dd7 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 @@ -36,6 +36,7 @@ import org.hl7.fhir.instance.model.api.*; import ca.uhn.fhir.context.*; import ca.uhn.fhir.context.BaseRuntimeChildDefinition.IMutator; import ca.uhn.fhir.model.api.*; +import ca.uhn.fhir.model.api.annotation.Child; import ca.uhn.fhir.model.base.composite.BaseResourceReferenceDt; import ca.uhn.fhir.model.base.resource.ResourceMetadataMap; import ca.uhn.fhir.model.primitive.IdDt; @@ -85,6 +86,10 @@ class ParserState { } } + public boolean elementIsRepeating(String theChildName) { + return myState.elementIsRepeating(theChildName); + } + public void endingElement() throws DataFormatException { myState.endingElement(); } @@ -747,7 +752,8 @@ class ParserState { } } - + + private abstract class BaseState { private PreResourceState myPreResourceState; @@ -766,6 +772,10 @@ class ParserState { myErrorHandler.unknownAttribute(null, theName); } + public boolean elementIsRepeating(@SuppressWarnings("unused") String theChildName) { + return false; + } + public void endingElement() throws DataFormatException { // ignore by default } @@ -1524,11 +1534,21 @@ class ParserState { } } + @Override + public boolean elementIsRepeating(String theChildName) { + try { + BaseRuntimeChildDefinition child = myDefinition.getChildByNameOrThrowDataFormatException(theChildName); + return child.getMax() > 1 || child.getMax() == Child.MAX_UNLIMITED; + } catch (DataFormatException e) { + return false; + } + } + @Override public void endingElement() { pop(); } - + @Override public void enteringNewElement(String theNamespace, String theChildName) throws DataFormatException { BaseRuntimeChildDefinition child; @@ -1951,45 +1971,6 @@ class ParserState { pop(); } - protected void weaveContainedResources() { - FhirTerser terser = myContext.newTerser(); - terser.visit(myInstance, new IModelVisitor() { - - @Override - public void acceptElement(IBaseResource theResource, IBase theElement, List thePathToElement, BaseRuntimeChildDefinition theChildDefinition, BaseRuntimeElementDefinition theDefinition) { - if (theElement instanceof BaseResourceReferenceDt) { - BaseResourceReferenceDt nextRef = (BaseResourceReferenceDt) theElement; - String ref = nextRef.getReference().getValue(); - if (isNotBlank(ref)) { - if (ref.startsWith("#")) { - IResource target = (IResource) myContainedResources.get(ref); - if (target != null) { - ourLog.debug("Resource contains local ref {} in field {}", ref, thePathToElement); - nextRef.setResource(target); - } else { - myErrorHandler.unknownReference(null, ref); - } - } - } - } else if (theElement instanceof IBaseReference) { - IBaseReference nextRef = (IBaseReference) theElement; - String ref = nextRef.getReferenceElement().getValue(); - if (isNotBlank(ref)) { - if (ref.startsWith("#")) { - IBaseResource target = myContainedResources.get(ref); - if (target != null) { - ourLog.debug("Resource contains local ref {} in field {}", ref, thePathToElement); - nextRef.setResource(target); - } else { - myErrorHandler.unknownReference(null, ref); - } - } - } - } - } - }); - } - @Override public void enteringNewElement(String theNamespaceUri, String theLocalPart) throws DataFormatException { BaseRuntimeElementDefinition definition; @@ -2037,7 +2018,7 @@ class ParserState { push(new ResourceStateHl7Org(getRootPreResourceState(), def, myInstance)); } } - + public Map getContainedResources() { return myContainedResources; } @@ -2062,16 +2043,6 @@ class ParserState { protected abstract void populateTarget(); - public ParserState.PreResourceState setRequireResourceType(boolean theRequireResourceType) { - myRequireResourceType = theRequireResourceType; - return this; - } - - @Override - public void wereBack() { - postProcess(); - } - private void postProcess() { if (myContext.hasDefaultTypeForProfile()) { IBaseMetaType meta = myInstance.getMeta(); @@ -2110,6 +2081,11 @@ class ParserState { populateTarget(); } + public ParserState.PreResourceState setRequireResourceType(boolean theRequireResourceType) { + myRequireResourceType = theRequireResourceType; + return this; + } + private void stitchBundleCrossReferences() { final boolean bundle = "Bundle".equals(myContext.getResourceDefinition(myInstance).getName()); if (bundle) { @@ -2155,6 +2131,50 @@ class ParserState { } } + protected void weaveContainedResources() { + FhirTerser terser = myContext.newTerser(); + terser.visit(myInstance, new IModelVisitor() { + + @Override + public void acceptElement(IBaseResource theResource, IBase theElement, List thePathToElement, BaseRuntimeChildDefinition theChildDefinition, BaseRuntimeElementDefinition theDefinition) { + if (theElement instanceof BaseResourceReferenceDt) { + BaseResourceReferenceDt nextRef = (BaseResourceReferenceDt) theElement; + String ref = nextRef.getReference().getValue(); + if (isNotBlank(ref)) { + if (ref.startsWith("#")) { + IResource target = (IResource) myContainedResources.get(ref); + if (target != null) { + ourLog.debug("Resource contains local ref {} in field {}", ref, thePathToElement); + nextRef.setResource(target); + } else { + myErrorHandler.unknownReference(null, ref); + } + } + } + } else if (theElement instanceof IBaseReference) { + IBaseReference nextRef = (IBaseReference) theElement; + String ref = nextRef.getReferenceElement().getValue(); + if (isNotBlank(ref)) { + if (ref.startsWith("#")) { + IBaseResource target = myContainedResources.get(ref); + if (target != null) { + ourLog.debug("Resource contains local ref {} in field {}", ref, thePathToElement); + nextRef.setResource(target); + } else { + myErrorHandler.unknownReference(null, ref); + } + } + } + } + } + }); + } + + @Override + public void wereBack() { + postProcess(); + } + } private class PreResourceStateHapi extends PreResourceState { diff --git a/hapi-fhir-base/src/main/java/ca/uhn/fhir/parser/StrictErrorHandler.java b/hapi-fhir-base/src/main/java/ca/uhn/fhir/parser/StrictErrorHandler.java index a54dbaa60ee..559848473cb 100644 --- a/hapi-fhir-base/src/main/java/ca/uhn/fhir/parser/StrictErrorHandler.java +++ b/hapi-fhir-base/src/main/java/ca/uhn/fhir/parser/StrictErrorHandler.java @@ -1,6 +1,7 @@ package ca.uhn.fhir.parser; import ca.uhn.fhir.context.FhirContext; +import ca.uhn.fhir.parser.json.JsonLikeValue.ScalarType; import ca.uhn.fhir.parser.json.JsonLikeValue.ValueType; /* @@ -38,8 +39,9 @@ public class StrictErrorHandler implements IParserErrorHandler { } @Override - public void incorrectJsonType(IParseLocation theLocation, String theElementName, ValueType theExpected, ValueType theFound) { - throw new DataFormatException("Found incorrect type for element " + theElementName + " - Expected " + theExpected.name() + " and found " + theFound.name()); + public void incorrectJsonType(IParseLocation theLocation, String theElementName, ValueType theExpected, ScalarType theExpectedScalarType, ValueType theFound, ScalarType theFoundScalarType) { + String message = LenientErrorHandler.createIncorrectJsonTypeMessage(theElementName, theExpected, theExpectedScalarType, theFound, theFoundScalarType); + throw new DataFormatException(message); } @Override diff --git a/hapi-fhir-structures-dstu/src/test/java/ca/uhn/fhir/parser/JsonParserTest.java b/hapi-fhir-structures-dstu/src/test/java/ca/uhn/fhir/parser/JsonParserTest.java index 14adfbe9bc6..00968104c63 100644 --- a/hapi-fhir-structures-dstu/src/test/java/ca/uhn/fhir/parser/JsonParserTest.java +++ b/hapi-fhir-structures-dstu/src/test/java/ca/uhn/fhir/parser/JsonParserTest.java @@ -10,6 +10,10 @@ import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertNull; import static org.junit.Assert.assertThat; import static org.junit.Assert.fail; +import static org.mockito.Matchers.any; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; import java.io.*; import java.nio.charset.Charset; @@ -27,6 +31,7 @@ import org.junit.Before; import org.junit.BeforeClass; import org.junit.Ignore; import org.junit.Test; +import org.mockito.ArgumentCaptor; import ca.uhn.fhir.context.ConfigurationException; import ca.uhn.fhir.context.FhirContext; @@ -71,6 +76,10 @@ import ca.uhn.fhir.model.primitive.InstantDt; import ca.uhn.fhir.model.primitive.StringDt; import ca.uhn.fhir.model.primitive.XhtmlDt; import ca.uhn.fhir.narrative.INarrativeGenerator; +import ca.uhn.fhir.parser.IParserErrorHandler.IParseLocation; +import ca.uhn.fhir.parser.json.JsonLikeValue; +import ca.uhn.fhir.parser.json.JsonLikeValue.ScalarType; +import ca.uhn.fhir.parser.json.JsonLikeValue.ValueType; import ca.uhn.fhir.util.TestUtil; import net.sf.json.JSON; import net.sf.json.JSONSerializer; diff --git a/hapi-fhir-structures-dstu2.1/src/test/java/ca/uhn/fhir/parser/ErrorHandlerTest.java b/hapi-fhir-structures-dstu2.1/src/test/java/ca/uhn/fhir/parser/ErrorHandlerTest.java index a8752e147e7..b9a1773cd2d 100644 --- a/hapi-fhir-structures-dstu2.1/src/test/java/ca/uhn/fhir/parser/ErrorHandlerTest.java +++ b/hapi-fhir-structures-dstu2.1/src/test/java/ca/uhn/fhir/parser/ErrorHandlerTest.java @@ -4,6 +4,7 @@ import static org.junit.Assert.fail; import org.junit.Test; +import ca.uhn.fhir.parser.json.JsonLikeValue.ScalarType; import ca.uhn.fhir.parser.json.JsonLikeValue.ValueType; /* @@ -36,7 +37,7 @@ public class ErrorHandlerTest { new ErrorHandlerAdapter().containedResourceWithNoId(null); new ErrorHandlerAdapter().unknownReference(null, null); new ErrorHandlerAdapter().missingRequiredElement(null, null); - new ErrorHandlerAdapter().incorrectJsonType(null, null, null, null); + new ErrorHandlerAdapter().incorrectJsonType(null, null, null, null, null, null); new ErrorHandlerAdapter().invalidValue(null, null, null); } @@ -47,7 +48,7 @@ public class ErrorHandlerTest { new LenientErrorHandler().unknownElement(null, null); new LenientErrorHandler().containedResourceWithNoId(null); new LenientErrorHandler().unknownReference(null, null); - new LenientErrorHandler().incorrectJsonType(null, null, ValueType.ARRAY, ValueType.SCALAR); + new LenientErrorHandler().incorrectJsonType(null, null, ValueType.ARRAY, null, ValueType.SCALAR, null); new LenientErrorHandler().setErrorOnInvalidValue(false).invalidValue(null, "FOO", ""); new LenientErrorHandler().invalidValue(null, null, ""); try { @@ -85,7 +86,12 @@ public class ErrorHandlerTest { @Test(expected = DataFormatException.class) public void testStrictMethods6() { - new StrictErrorHandler().incorrectJsonType(null, null, ValueType.ARRAY, ValueType.SCALAR); + new StrictErrorHandler().incorrectJsonType(null, null, ValueType.ARRAY, null, ValueType.SCALAR, null); + } + + @Test(expected = DataFormatException.class) + public void testStrictMethods8() { + new StrictErrorHandler().incorrectJsonType(null, null, ValueType.SCALAR, ScalarType.BOOLEAN, ValueType.SCALAR, ScalarType.STRING); } @Test(expected = DataFormatException.class) @@ -93,4 +99,10 @@ public class ErrorHandlerTest { new StrictErrorHandler().invalidValue(null, null, null); } + + @Test() + public void testLenientMethods8() { + new LenientErrorHandler().incorrectJsonType(null, null, ValueType.SCALAR, ScalarType.BOOLEAN, ValueType.SCALAR, ScalarType.STRING); + } + } 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 415b2eed102..25489b13322 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 @@ -11,8 +11,7 @@ import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; import java.io.IOException; -import java.text.DateFormat; -import java.text.SimpleDateFormat; +import java.nio.charset.StandardCharsets; import java.util.ArrayList; import java.util.Date; import java.util.List; @@ -30,7 +29,6 @@ import ca.uhn.fhir.model.api.TemporalPrecisionEnum; import ca.uhn.fhir.model.dstu2.composite.CodeableConceptDt; import ca.uhn.fhir.model.dstu2.composite.ResourceReferenceDt; import ca.uhn.fhir.model.dstu2.composite.TimingDt; -import ca.uhn.fhir.model.dstu2.resource.AllergyIntolerance; import ca.uhn.fhir.model.dstu2.resource.Condition; import ca.uhn.fhir.model.dstu2.resource.MedicationOrder; import ca.uhn.fhir.model.dstu2.resource.OperationOutcome; @@ -40,7 +38,6 @@ 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; @@ -69,7 +66,6 @@ public class ResourceValidatorDstu2Test { return encoded; } - /** * See issue #50 */ @@ -130,7 +126,7 @@ public class ResourceValidatorDstu2Test { @Test public void testSchemaBundleValidatorFails() throws IOException { - String res = IOUtils.toString(getClass().getClassLoader().getResourceAsStream("bundle-example.json")); + String res = IOUtils.toString(getClass().getClassLoader().getResourceAsStream("bundle-example.json"), StandardCharsets.UTF_8); Bundle b = ourCtx.newJsonParser().parseBundle(res); FhirValidator val = createFhirValidator(); @@ -153,7 +149,7 @@ public class ResourceValidatorDstu2Test { @Test public void testSchemaBundleValidatorIsSuccessful() throws IOException { - String res = IOUtils.toString(getClass().getClassLoader().getResourceAsStream("bundle-example.json")); + String res = IOUtils.toString(getClass().getClassLoader().getResourceAsStream("bundle-example.json"), StandardCharsets.UTF_8); Bundle b = ourCtx.newJsonParser().parseBundle(res); ourLog.info(ourCtx.newXmlParser().setPrettyPrint(true).encodeBundleToString(b)); @@ -221,7 +217,7 @@ public class ResourceValidatorDstu2Test { @Test public void testSchematronResourceValidator() throws IOException { - String res = IOUtils.toString(getClass().getClassLoader().getResourceAsStream("patient-example-dicom.json")); + String res = IOUtils.toString(getClass().getClassLoader().getResourceAsStream("patient-example-dicom.json"), StandardCharsets.UTF_8); Patient p = ourCtx.newJsonParser().parseResource(Patient.class, res); FhirValidator val = ourCtx.newValidator(); 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 3e6e4f31137..1ef7a852380 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 @@ -36,7 +36,7 @@ public class ErrorHandlerTest { new ErrorHandlerAdapter().containedResourceWithNoId(null); new ErrorHandlerAdapter().unknownReference(null, null); new ErrorHandlerAdapter().missingRequiredElement(null, null); - new ErrorHandlerAdapter().incorrectJsonType(null, null, null, null); + new ErrorHandlerAdapter().incorrectJsonType(null, null, null, null, null, null); new ErrorHandlerAdapter().invalidValue(null, null, null); } @@ -47,7 +47,7 @@ public class ErrorHandlerTest { new LenientErrorHandler().unknownElement(null, null); new LenientErrorHandler().containedResourceWithNoId(null); new LenientErrorHandler().unknownReference(null, null); - new LenientErrorHandler().incorrectJsonType(null, null, ValueType.ARRAY, ValueType.SCALAR); + new LenientErrorHandler().incorrectJsonType(null, null, ValueType.ARRAY, null, ValueType.SCALAR, null); new LenientErrorHandler().setErrorOnInvalidValue(false).invalidValue(null, "FOO", ""); new LenientErrorHandler().invalidValue(null, null, ""); try { @@ -85,7 +85,7 @@ public class ErrorHandlerTest { @Test(expected = DataFormatException.class) public void testStrictMethods6() { - new StrictErrorHandler().incorrectJsonType(null, null, ValueType.ARRAY, ValueType.SCALAR); + new StrictErrorHandler().incorrectJsonType(null, null, ValueType.ARRAY, null, ValueType.SCALAR, null); } @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 38ec6fb73f5..bc5ac21e26f 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,6 +13,7 @@ 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.any; import static org.mockito.Matchers.eq; import static org.mockito.Matchers.isNull; import static org.mockito.Mockito.atLeastOnce; @@ -95,6 +96,7 @@ import ca.uhn.fhir.narrative.DefaultThymeleafNarrativeGenerator; import ca.uhn.fhir.parser.IParserErrorHandler.IParseLocation; import ca.uhn.fhir.parser.PatientWithExtendedContactDstu3.CustomContactComponent; import ca.uhn.fhir.parser.XmlParserDstu3Test.TestPatientFor327; +import ca.uhn.fhir.parser.json.JsonLikeValue.ScalarType; import ca.uhn.fhir.parser.json.JsonLikeValue.ValueType; import ca.uhn.fhir.rest.server.Constants; import ca.uhn.fhir.util.TestUtil; @@ -113,6 +115,79 @@ public class JsonParserDstu3Test { ourCtx.setNarrativeGenerator(null); } + @Test + public void testIncorrectJsonTypesIdAndArray() { + + // ID should be a String and communication should be an Array + String input = "{\"resourceType\": \"Patient\",\n" + + " \"id\": 123,\n" + + " \"communication\": {\n" + + " \"language\": {\n" + + " \"text\": \"Hindi\"\n" + + " },\n" + + " \"preferred\": true\n" + + " }\n" + + "}"; + + IParser p = ourCtx.newJsonParser(); + + IParserErrorHandler errorHandler = mock(IParserErrorHandler.class); + p.setParserErrorHandler(errorHandler); + Patient patient = (Patient) p.parseResource(input); + + ArgumentCaptor elementName = ArgumentCaptor.forClass(String.class); + ArgumentCaptor found = ArgumentCaptor.forClass(ValueType.class); + ArgumentCaptor expected = ArgumentCaptor.forClass(ValueType.class); + ArgumentCaptor expectedScalarType = ArgumentCaptor.forClass(ScalarType.class); + ArgumentCaptor foundScalarType = ArgumentCaptor.forClass(ScalarType.class); + verify(errorHandler, times(2)).incorrectJsonType(any(IParseLocation.class), elementName.capture(), expected.capture(), expectedScalarType.capture(), found.capture(), foundScalarType.capture()); + + assertEquals(ValueType.SCALAR, found.getAllValues().get(0)); + assertEquals(ValueType.SCALAR, expected.getAllValues().get(0)); + assertEquals(ScalarType.NUMBER, foundScalarType.getAllValues().get(0)); + assertEquals(ScalarType.STRING, expectedScalarType.getAllValues().get(0)); + + assertEquals(ValueType.OBJECT, found.getAllValues().get(1)); + assertEquals(ValueType.ARRAY, expected.getAllValues().get(1)); + assertEquals(null, foundScalarType.getAllValues().get(1)); + assertEquals(null, expectedScalarType.getAllValues().get(1)); + + assertEquals("123", patient.getIdElement().getIdPart()); + assertEquals("Hindi", patient.getCommunicationFirstRep().getLanguage().getText()); + } + + @Test + public void testIncorrectJsonTypesNone() { + + // ID should be a String and communication should be an Array + String input = "{\"resourceType\": \"Patient\",\n" + + " \"id\": \"123\",\n" + + " \"communication\": [{\n" + + " \"language\": {\n" + + " \"text\": \"Hindi\"\n" + + " },\n" + + " \"preferred\": true\n" + + " }]\n" + + "}"; + + IParser p = ourCtx.newJsonParser(); + + IParserErrorHandler errorHandler = mock(IParserErrorHandler.class); + p.setParserErrorHandler(errorHandler); + Patient patient = (Patient) p.parseResource(input); + + ArgumentCaptor elementName = ArgumentCaptor.forClass(String.class); + ArgumentCaptor found = ArgumentCaptor.forClass(ValueType.class); + ArgumentCaptor expected = ArgumentCaptor.forClass(ValueType.class); + ArgumentCaptor expectedScalarType = ArgumentCaptor.forClass(ScalarType.class); + ArgumentCaptor foundScalarType = ArgumentCaptor.forClass(ScalarType.class); + verify(errorHandler, times(0)).incorrectJsonType(any(IParseLocation.class), elementName.capture(), expected.capture(), expectedScalarType.capture(), found.capture(), foundScalarType.capture()); + + assertEquals("123", patient.getIdElement().getIdPart()); + assertEquals("Hindi", patient.getCommunicationFirstRep().getLanguage().getText()); + } + + /** * See #276 */ @@ -200,14 +275,18 @@ public class JsonParserDstu3Test { ArgumentCaptor elementName = ArgumentCaptor.forClass(String.class); ArgumentCaptor expected = ArgumentCaptor.forClass(ValueType.class); ArgumentCaptor actual = ArgumentCaptor.forClass(ValueType.class); - verify(errorHandler, atLeastOnce()).incorrectJsonType(Mockito.any(IParseLocation.class), elementName.capture(), expected.capture(), actual.capture()); - verify(errorHandler, atLeastOnce()).incorrectJsonType(Mockito.any(IParseLocation.class), Mockito.eq("_id"), Mockito.eq(ValueType.OBJECT), Mockito.eq(ValueType.SCALAR)); - verify(errorHandler, atLeastOnce()).incorrectJsonType(Mockito.any(IParseLocation.class), Mockito.eq("__v"), Mockito.eq(ValueType.OBJECT), Mockito.eq(ValueType.SCALAR)); - verify(errorHandler, atLeastOnce()).incorrectJsonType(Mockito.any(IParseLocation.class), Mockito.eq("_status"), Mockito.eq(ValueType.OBJECT), Mockito.eq(ValueType.SCALAR)); + ArgumentCaptor expectedScalar = ArgumentCaptor.forClass(ScalarType.class); + ArgumentCaptor actualScalar = ArgumentCaptor.forClass(ScalarType.class); + verify(errorHandler, atLeastOnce()).incorrectJsonType(Mockito.any(IParseLocation.class), elementName.capture(), expected.capture(), expectedScalar.capture(), actual.capture(), actualScalar.capture()); + verify(errorHandler, atLeastOnce()).incorrectJsonType(Mockito.any(IParseLocation.class), Mockito.eq("_id"), Mockito.eq(ValueType.OBJECT), expectedScalar.capture(), Mockito.eq(ValueType.SCALAR), actualScalar.capture()); + verify(errorHandler, atLeastOnce()).incorrectJsonType(Mockito.any(IParseLocation.class), Mockito.eq("__v"), Mockito.eq(ValueType.OBJECT), expectedScalar.capture(), Mockito.eq(ValueType.SCALAR), actualScalar.capture()); + verify(errorHandler, atLeastOnce()).incorrectJsonType(Mockito.any(IParseLocation.class), Mockito.eq("_status"), Mockito.eq(ValueType.OBJECT), expectedScalar.capture(), Mockito.eq(ValueType.SCALAR), actualScalar.capture()); assertEquals("_id", elementName.getAllValues().get(0)); assertEquals(ValueType.OBJECT, expected.getAllValues().get(0)); assertEquals(ValueType.SCALAR, actual.getAllValues().get(0)); + assertEquals(null, expectedScalar.getAllValues().get(0)); + assertEquals(null, actualScalar.getAllValues().get(0)); } @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 1ac4ee5c064..35be7b58a4b 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 @@ -9,6 +9,7 @@ import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; import java.io.IOException; +import java.nio.charset.StandardCharsets; import java.util.ArrayList; import java.util.Date; import java.util.List; @@ -41,6 +42,7 @@ import ca.uhn.fhir.parser.XmlParserDstu3Test; import ca.uhn.fhir.util.TestUtil; import ca.uhn.fhir.validation.FhirValidator; import ca.uhn.fhir.validation.SchemaBaseValidator; +import ca.uhn.fhir.validation.SingleValidationMessage; import ca.uhn.fhir.validation.ValidationResult; import ca.uhn.fhir.validation.schematron.SchematronBaseValidator; @@ -65,7 +67,7 @@ public class ResourceValidatorDstu3Test { // 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); @@ -75,9 +77,9 @@ public class ResourceValidatorDstu3Test { String resultString = parser.setPrettyPrint(true).encodeResourceToString(result.toOperationOutcome()); ourLog.info(resultString); - assertEquals(2, ((OperationOutcome)result.toOperationOutcome()).getIssue().size()); + assertEquals(2, ((OperationOutcome) result.toOperationOutcome()).getIssue().size()); assertThat(resultString, StringContains.containsString("cvc-pattern-valid")); - + try { parser.parseResource(encoded); fail(); @@ -86,7 +88,6 @@ public class ResourceValidatorDstu3Test { } } - /** * Make sure that the elements that appear in all resources (meta, language, extension, etc) all appear in the correct order */ @@ -189,7 +190,7 @@ public class ResourceValidatorDstu3Test { @Test @Ignore public void testValidateQuestionnaire() throws IOException { - String input = IOUtils.toString(getClass().getResourceAsStream("/questionnaire_jon_z_20160506.xml")); + String input = IOUtils.toString(getClass().getResourceAsStream("/questionnaire_jon_z_20160506.xml"), StandardCharsets.UTF_8); FhirValidator val = ourCtx.newValidator(); val.registerValidatorModule(new FhirInstanceValidator()); @@ -203,6 +204,41 @@ public class ResourceValidatorDstu3Test { assertTrue(result.isSuccessful()); } + @Test + public void testValidateJsonNumericId() { + String input = "{\"resourceType\": \"Patient\",\n" + + " \"id\": 123,\n" + + " \"meta\": {\n" + + " \"versionId\": \"29\",\n" + + " \"lastUpdated\": \"2015-12-22T19:53:11.000Z\"\n" + + " },\n" + + " \"communication\": {\n" + + " \"language\": {\n" + + " \"coding\": [\n" + + " {\n" + + " \"system\": \"urn:ietf:bcp:47\",\n" + + " \"code\": \"hi\",\n" + + " \"display\": \"Hindi\",\n" + + " \"userSelected\": false\n" + + " }],\n" + + " \"text\": \"Hindi\"\n" + + " },\n" + + " \"preferred\": true\n" + + " }\n" + + "}"; + + FhirValidator val = ourCtx.newValidator(); + val.registerValidatorModule(new FhirInstanceValidator()); + ValidationResult output = val.validateWithResult(input); + + OperationOutcome operationOutcome = (OperationOutcome) output.toOperationOutcome(); + String encoded = ourCtx.newJsonParser().setPrettyPrint(true).encodeResourceToString(operationOutcome); + ourLog.info(encoded); + + assertThat(encoded, containsString("Error parsing JSON: the primitive value must be a string")); + + } + /** * See https://groups.google.com/d/msgid/hapi-fhir/a266083f-6454-4cf0-a431-c6500f052bea%40googlegroups.com?utm_medium= email&utm_source=footer */ @@ -259,9 +295,9 @@ public class ResourceValidatorDstu3Test { BenefitComponent benComp = fhirObj.addInsurance().addBenefitBalance().addFinancial(); // Test between .benefit[x] and benefitUsed[x] benComp.setBenefitUsed(new UnsignedIntType(2)); - + String input = ourCtx.newXmlParser().encodeResourceToString(fhirObj); - + FhirValidator validator = ourCtx.newValidator(); validator.registerValidatorModule(new FhirInstanceValidator()); @@ -269,6 +305,5 @@ public class ResourceValidatorDstu3Test { // we should get some results, not an exception assertEquals(4, result.getMessages().size()); } - } diff --git a/hapi-fhir-structures-hl7org-dstu2/src/test/java/ca/uhn/fhir/validation/FhirInstanceValidatorTest.java b/hapi-fhir-structures-hl7org-dstu2/src/test/java/ca/uhn/fhir/validation/FhirInstanceValidatorTest.java index c3800106b72..db8ed1369f4 100644 --- a/hapi-fhir-structures-hl7org-dstu2/src/test/java/ca/uhn/fhir/validation/FhirInstanceValidatorTest.java +++ b/hapi-fhir-structures-hl7org-dstu2/src/test/java/ca/uhn/fhir/validation/FhirInstanceValidatorTest.java @@ -371,7 +371,35 @@ public class FhirInstanceValidatorTest { } - + @Test + public void testValidateJsonNumericId() { + String input="{\"resourceType\": \"Patient\",\n" + + " \"id\": 123,\n" + + " \"meta\": {\n" + + " \"versionId\": \"29\",\n" + + " \"lastUpdated\": \"2015-12-22T19:53:11.000Z\"\n" + + " },\n" + + " \"communication\": {\n" + + " \"language\": {\n" + + " \"coding\": [\n" + + " {\n" + + " \"system\": \"urn:ietf:bcp:47\",\n" + + " \"code\": \"hi\",\n" + + " \"display\": \"Hindi\",\n" + + " \"userSelected\": false\n" + + " }],\n" + + " \"text\": \"Hindi\"\n" + + " },\n" + + " \"preferred\": true\n" + + " }\n" + + "}"; + + ValidationResult output = myVal.validateWithResult(input); + List all = logResultsAndReturnNonInformationalOnes(output); + assertEquals(0, all.size()); + + } + @Test public void testValidateResourceWithValuesetExpansion() { diff --git a/src/changes/changes.xml b/src/changes/changes.xml index f0e82d69a33..47362fab794 100644 --- a/src/changes/changes.xml +++ b/src/changes/changes.xml @@ -161,6 +161,10 @@ Correct a typo in client IHttpRequest]]> class: "bufferEntitity" should be "bufferEntity". + + ErrorHandler is now called (resulting in a warning by default, but can also be an exception) when arsing JSON if + the resource ID is not a JSON string, or an object is found where an array is expected (e.g. repeating field) +