From 2a43aa3b6ccc39cab1ecbd4a1fd98f95ec70d965 Mon Sep 17 00:00:00 2001 From: jamesagnew Date: Thu, 9 Apr 2020 09:53:13 -0400 Subject: [PATCH 1/5] Improve error message reporting for invalid json - Fix #1793 --- .../ca/uhn/fhir/parser/BaseErrorHandler.java | 14 +++++++++ .../java/ca/uhn/fhir/parser/JsonParser.java | 20 ++++++------ .../uhn/fhir/parser/LenientErrorHandler.java | 14 ++++----- .../ca/uhn/fhir/parser/ParseLocation.java | 9 +++++- .../java/ca/uhn/fhir/parser/ParserState.java | 31 +++++++++++-------- .../uhn/fhir/parser/StrictErrorHandler.java | 15 ++++----- ...report-attr-name-in-json-parse-errors.yaml | 7 +++++ .../ca/uhn/fhir/parser/JsonParserR4Test.java | 26 ++++++++++++++++ 8 files changed, 98 insertions(+), 38 deletions(-) create mode 100644 hapi-fhir-base/src/main/java/ca/uhn/fhir/parser/BaseErrorHandler.java create mode 100644 hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/4_3_0/1793-report-attr-name-in-json-parse-errors.yaml diff --git a/hapi-fhir-base/src/main/java/ca/uhn/fhir/parser/BaseErrorHandler.java b/hapi-fhir-base/src/main/java/ca/uhn/fhir/parser/BaseErrorHandler.java new file mode 100644 index 00000000000..34a657d0a83 --- /dev/null +++ b/hapi-fhir-base/src/main/java/ca/uhn/fhir/parser/BaseErrorHandler.java @@ -0,0 +1,14 @@ +package ca.uhn.fhir.parser; + +class BaseErrorHandler { + + String describeLocation(IParserErrorHandler.IParseLocation theLocation) { + if (theLocation == null) { + return ""; + } else { + return theLocation.toString() + " "; + } + } + +} + 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 4dea1260aca..6a5917853ed 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 @@ -174,7 +174,7 @@ public class JsonParser extends BaseParser implements IJsonLikeParser { theEventWriter.init(); RuntimeResourceDefinition resDef = myContext.getResourceDefinition(theResource); - encodeResourceToJsonStreamWriter(resDef, theResource, theEventWriter, null, false, theEncodeContext); + encodeResourceToJsonStreamWriter(resDef, theResource, theEventWriter, null, false, theEncodeContext); theEventWriter.flush(); } @@ -258,9 +258,9 @@ public class JsonParser extends BaseParser implements IJsonLikeParser { break; } else if (valueObj instanceof Long) { if (theChildName != null) { - theEventWriter.write(theChildName, (long)valueObj); + theEventWriter.write(theChildName, (long) valueObj); } else { - theEventWriter.write((long)valueObj); + theEventWriter.write((long) valueObj); } break; } @@ -604,7 +604,7 @@ public class JsonParser extends BaseParser implements IJsonLikeParser { return maxCardinality > 1 || maxCardinality == Child.MAX_UNLIMITED; } - private void encodeCompositeElementToStreamWriter(RuntimeResourceDefinition theResDef, IBaseResource theResource, IBase theNextValue, JsonLikeWriter theEventWriter, boolean theContainedResource, CompositeChildElement theParent, EncodeContext theEncodeContext) throws IOException, DataFormatException { + private void encodeCompositeElementToStreamWriter(RuntimeResourceDefinition theResDef, IBaseResource theResource, IBase theNextValue, JsonLikeWriter theEventWriter, boolean theContainedResource, CompositeChildElement theParent, EncodeContext theEncodeContext) throws IOException, DataFormatException { writeCommentsPreAndPost(theNextValue, theEventWriter); encodeCompositeElementChildrenToStreamWriter(theResDef, theResource, theNextValue, theEventWriter, theContainedResource, theParent, theEncodeContext); @@ -1387,10 +1387,6 @@ public class JsonParser extends BaseParser implements IJsonLikeParser { } } - private static void write(JsonLikeWriter theWriter, String theName, String theValue) throws IOException { - theWriter.write(theName, theValue); - } - private class HeldExtension implements Comparable { private CompositeChildElement myChildElem; @@ -1485,7 +1481,7 @@ public class JsonParser extends BaseParser implements IJsonLikeParser { extractAndWriteExtensionsAsDirectChild(myValue, theEventWriter, def, theResDef, theResource, myChildElem, null, theEncodeContext, theContainedResource); } else { String childName = myDef.getChildNameByDatatype(myValue.getClass()); - encodeChildElementToStreamWriter(theResDef, theResource, theEventWriter, myValue, def, childName, false, myParent, false, theEncodeContext); + encodeChildElementToStreamWriter(theResDef, theResource, theEventWriter, myValue, def, childName, false, myParent, false, theEncodeContext); managePrimitiveExtension(myValue, theResDef, theResource, theEventWriter, def, childName, theEncodeContext, theContainedResource); } @@ -1562,7 +1558,7 @@ public class JsonParser extends BaseParser implements IJsonLikeParser { if (childDef == null) { throw new ConfigurationException("Unable to encode extension, unrecognized child element type: " + value.getClass().getCanonicalName()); } - encodeChildElementToStreamWriter(theResDef, theResource, theEventWriter, value, childDef, childName, false, myParent,false, theEncodeContext); + encodeChildElementToStreamWriter(theResDef, theResource, theEventWriter, value, childDef, childName, false, myParent, false, theEncodeContext); managePrimitiveExtension(value, theResDef, theResource, theEventWriter, childDef, childName, theEncodeContext, theContainedResource); theEncodeContext.popPath(); @@ -1574,4 +1570,8 @@ public class JsonParser extends BaseParser implements IJsonLikeParser { theEventWriter.endObject(); } } + + private static void write(JsonLikeWriter theWriter, String theName, String theValue) throws IOException { + theWriter.write(theName, theValue); + } } 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 a43a429508c..5082d3cdad7 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 @@ -38,7 +38,7 @@ import ca.uhn.fhir.parser.json.JsonLikeValue.ValueType; * @see IParser#setParserErrorHandler(IParserErrorHandler) * @see FhirContext#setParserErrorHandler(IParserErrorHandler) */ -public class LenientErrorHandler implements IParserErrorHandler { +public class LenientErrorHandler extends BaseErrorHandler implements IParserErrorHandler { private static final org.slf4j.Logger ourLog = org.slf4j.LoggerFactory.getLogger(LenientErrorHandler.class); private static final StrictErrorHandler STRICT_ERROR_HANDLER = new StrictErrorHandler(); @@ -84,7 +84,7 @@ public class LenientErrorHandler implements IParserErrorHandler { public void invalidValue(IParseLocation theLocation, String theValue, String theError) { if (isBlank(theValue) || myErrorOnInvalidValue == false) { if (myLogErrors) { - ourLog.warn("Invalid attribute value \"{}\": {}", theValue, theError); + ourLog.warn("{}Invalid attribute value \"{}\": {}", describeLocation(theLocation), theValue, theError); } } else { STRICT_ERROR_HANDLER.invalidValue(theLocation, theValue, theError); @@ -133,35 +133,35 @@ public class LenientErrorHandler implements IParserErrorHandler { @Override public void unexpectedRepeatingElement(IParseLocation theLocation, String theElementName) { if (myLogErrors) { - ourLog.warn("Multiple repetitions of non-repeatable element '{}' found while parsing", theElementName); + ourLog.warn("{}Multiple repetitions of non-repeatable element '{}' found while parsing", describeLocation(theLocation), theElementName); } } @Override public void unknownAttribute(IParseLocation theLocation, String theElementName) { if (myLogErrors) { - ourLog.warn("Unknown attribute '{}' found while parsing", theElementName); + ourLog.warn("{}Unknown attribute '{}' found while parsing",describeLocation(theLocation), theElementName); } } @Override public void unknownElement(IParseLocation theLocation, String theElementName) { if (myLogErrors) { - ourLog.warn("Unknown element '{}' found while parsing", theElementName); + ourLog.warn("{}Unknown element '{}' found while parsing", describeLocation(theLocation), theElementName); } } @Override public void unknownReference(IParseLocation theLocation, String theReference) { if (myLogErrors) { - ourLog.warn("Resource has invalid reference: {}", theReference); + ourLog.warn("{}Resource has invalid reference: {}", describeLocation(theLocation), theReference); } } @Override public void extensionContainsValueAndNestedExtensions(IParseLocation theLocation) { if (myLogErrors) { - ourLog.warn("Extension contains both a value and nested extensions: {}", theLocation); + ourLog.warn("Extension contains both a value and nested extensions: {}", describeLocation(theLocation)); } } diff --git a/hapi-fhir-base/src/main/java/ca/uhn/fhir/parser/ParseLocation.java b/hapi-fhir-base/src/main/java/ca/uhn/fhir/parser/ParseLocation.java index f8451143806..fc150cdf2fc 100644 --- a/hapi-fhir-base/src/main/java/ca/uhn/fhir/parser/ParseLocation.java +++ b/hapi-fhir-base/src/main/java/ca/uhn/fhir/parser/ParseLocation.java @@ -54,6 +54,13 @@ class ParseLocation implements IParseLocation { @Override public String toString() { - return defaultString(myParentElementName); + return "[element=\"" + defaultString(myParentElementName) + "\"]"; + } + + /** + * Factory method + */ + static ParseLocation fromElementName(String theChildName) { + return new ParseLocation(theChildName); } } 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 d3f6f7193f6..34a2b765a0c 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 @@ -191,7 +191,7 @@ class ParserState { return thePrimitiveTarget.newInstance(theDefinition.getInstanceConstructorArguments()); } - public IPrimitiveType getPrimitiveInstance(BaseRuntimeChildDefinition theChild, RuntimePrimitiveDatatypeDefinition thePrimitiveTarget) { + public IPrimitiveType getPrimitiveInstance(BaseRuntimeChildDefinition theChild, RuntimePrimitiveDatatypeDefinition thePrimitiveTarget, String theChildName) { return thePrimitiveTarget.newInstance(theChild.getInstanceConstructorArguments()); } @@ -456,7 +456,7 @@ class ParserState { RuntimePrimitiveDatatypeDefinition primitiveTarget = (RuntimePrimitiveDatatypeDefinition) target; IPrimitiveType newChildInstance = newPrimitiveInstance(myDefinition, primitiveTarget); myDefinition.getMutator().addValue(myParentInstance, newChildInstance); - PrimitiveState newState = new PrimitiveState(getPreResourceState(), newChildInstance); + PrimitiveState newState = new PrimitiveState(getPreResourceState(), newChildInstance, theLocalPart); push(newState); return; } @@ -583,9 +583,9 @@ class ParserState { case PRIMITIVE_DATATYPE: { RuntimePrimitiveDatatypeDefinition primitiveTarget = (RuntimePrimitiveDatatypeDefinition) target; IPrimitiveType newChildInstance; - newChildInstance = getPrimitiveInstance(child, primitiveTarget); + newChildInstance = getPrimitiveInstance(child, primitiveTarget, theChildName); child.getMutator().addValue(myInstance, newChildInstance); - PrimitiveState newState = new PrimitiveState(getPreResourceState(), newChildInstance); + PrimitiveState newState = new PrimitiveState(getPreResourceState(), newChildInstance, theChildName); push(newState); return; } @@ -752,7 +752,7 @@ class ParserState { RuntimePrimitiveDatatypeDefinition primitiveTarget = (RuntimePrimitiveDatatypeDefinition) target; IPrimitiveType newChildInstance = newInstance(primitiveTarget); myExtension.setValue(newChildInstance); - PrimitiveState newState = new PrimitiveState(getPreResourceState(), newChildInstance); + PrimitiveState newState = new PrimitiveState(getPreResourceState(), newChildInstance, theLocalPart); push(newState); return; } @@ -824,7 +824,7 @@ class ParserState { break; case "lastUpdated": InstantDt updated = new InstantDt(); - push(new PrimitiveState(getPreResourceState(), updated)); + push(new PrimitiveState(getPreResourceState(), updated, theLocalPart)); myMap.put(ResourceMetadataKeyEnum.UPDATED, updated); break; case "security": @@ -850,7 +850,7 @@ class ParserState { newProfiles = new ArrayList<>(1); } IdDt profile = new IdDt(); - push(new PrimitiveState(getPreResourceState(), profile)); + push(new PrimitiveState(getPreResourceState(), profile, theLocalPart)); newProfiles.add(profile); myMap.put(ResourceMetadataKeyEnum.PROFILES, Collections.unmodifiableList(newProfiles)); break; @@ -891,7 +891,7 @@ class ParserState { private class MetaVersionElementState extends BaseState { - private ResourceMetadataMap myMap; + private final ResourceMetadataMap myMap; MetaVersionElementState(ParserState.PreResourceState thePreResourceState, ResourceMetadataMap theMap) { super(thePreResourceState); @@ -1268,23 +1268,27 @@ class ParserState { } private class PrimitiveState extends BaseState { + private final String myChildName; private IPrimitiveType myInstance; - PrimitiveState(PreResourceState thePreResourceState, IPrimitiveType theInstance) { + PrimitiveState(PreResourceState thePreResourceState, IPrimitiveType theInstance, String theChildName) { super(thePreResourceState); myInstance = theInstance; + myChildName = theChildName; } @Override public void attributeValue(String theName, String theValue) throws DataFormatException { if ("value".equals(theName)) { if ("".equals(theValue)) { - myErrorHandler.invalidValue(null, theValue, "Attribute values must not be empty (\"\")"); + ParseLocation location = ParseLocation.fromElementName(myChildName); + myErrorHandler.invalidValue(location, theValue, "Attribute value for element must not be empty (\"\")"); } else { try { myInstance.setValueAsString(theValue); } catch (DataFormatException | IllegalArgumentException e) { - myErrorHandler.invalidValue(null, theValue, e.getMessage()); + ParseLocation location = ParseLocation.fromElementName(myChildName); + myErrorHandler.invalidValue(location, theValue, e.getMessage()); } } } else if ("id".equals(theName)) { @@ -1295,7 +1299,8 @@ class ParserState { } else if (myInstance instanceof IBaseResource) { new IdDt(theValue).applyTo((org.hl7.fhir.instance.model.api.IBaseResource) myInstance); } else { - myErrorHandler.unknownAttribute(null, theName); + ParseLocation location = ParseLocation.fromElementName(myChildName); + myErrorHandler.unknownAttribute(location, theName); } } else { super.attributeValue(theName, theValue); @@ -1343,7 +1348,7 @@ class ParserState { @Override public void enteringNewElement(String theNamespace, String theChildName) throws DataFormatException { if ("id".equals(theChildName)) { - push(new PrimitiveState(getPreResourceState(), myInstance.getId())); + push(new PrimitiveState(getPreResourceState(), myInstance.getId(), theChildName)); } else if ("meta".equals(theChildName)) { push(new MetaElementState(getPreResourceState(), myInstance.getResourceMetadata())); } else { 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 433b3f0cbf5..7c35006b255 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 @@ -3,6 +3,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; +import ca.uhn.fhir.util.UrlUtil; /* * #%L @@ -31,7 +32,7 @@ import ca.uhn.fhir.parser.json.JsonLikeValue.ValueType; * @see IParser#setParserErrorHandler(IParserErrorHandler) * @see FhirContext#setParserErrorHandler(IParserErrorHandler) */ -public class StrictErrorHandler implements IParserErrorHandler { +public class StrictErrorHandler extends BaseErrorHandler implements IParserErrorHandler { @Override public void containedResourceWithNoId(IParseLocation theLocation) { @@ -46,7 +47,7 @@ public class StrictErrorHandler implements IParserErrorHandler { @Override public void invalidValue(IParseLocation theLocation, String theValue, String theError) { - throw new DataFormatException("Invalid attribute value \"" + theValue + "\": " + theError); + throw new DataFormatException(describeLocation(theLocation) + "Invalid attribute value \"" + UrlUtil.sanitizeUrlPart(theValue) + "\": " + theError); } @Override @@ -65,27 +66,27 @@ public class StrictErrorHandler implements IParserErrorHandler { @Override public void unexpectedRepeatingElement(IParseLocation theLocation, String theElementName) { - throw new DataFormatException("Multiple repetitions of non-repeatable element '" + theElementName + "' found during parse"); + throw new DataFormatException(describeLocation(theLocation) + "Multiple repetitions of non-repeatable element '" + theElementName + "' found during parse"); } @Override public void unknownAttribute(IParseLocation theLocation, String theAttributeName) { - throw new DataFormatException("Unknown attribute '" + theAttributeName + "' found during parse"); + throw new DataFormatException(describeLocation(theLocation) + "Unknown attribute '" + theAttributeName + "' found during parse"); } @Override public void unknownElement(IParseLocation theLocation, String theElementName) { - throw new DataFormatException("Unknown element '" + theElementName + "' found during parse"); + throw new DataFormatException(describeLocation(theLocation) + "Unknown element '" + theElementName + "' found during parse"); } @Override public void unknownReference(IParseLocation theLocation, String theReference) { - throw new DataFormatException("Resource has invalid reference: " + theReference); + throw new DataFormatException(describeLocation(theLocation) + "Resource has invalid reference: " + theReference); } @Override public void extensionContainsValueAndNestedExtensions(IParseLocation theLocation) { - throw new DataFormatException("Extension contains both a value and nested extensions: " + theLocation); + throw new DataFormatException(describeLocation(theLocation) + "Extension contains both a value and nested extensions: " + theLocation); } } diff --git a/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/4_3_0/1793-report-attr-name-in-json-parse-errors.yaml b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/4_3_0/1793-report-attr-name-in-json-parse-errors.yaml new file mode 100644 index 00000000000..ad2b30fa92a --- /dev/null +++ b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/4_3_0/1793-report-attr-name-in-json-parse-errors.yaml @@ -0,0 +1,7 @@ +--- +type: add +issue: 1793 +title: "When parsing JSON resources, if an element contains an invalid value, the Parser Error Handler + did not have access to the actual name of the element being parsed. This meant that errors lacked useful + detail in order to diagnose the issue. This has been corrected. Thanks to GitHub user + @jwalter for reporting!" diff --git a/hapi-fhir-structures-r4/src/test/java/ca/uhn/fhir/parser/JsonParserR4Test.java b/hapi-fhir-structures-r4/src/test/java/ca/uhn/fhir/parser/JsonParserR4Test.java index 350746625aa..b3363a6a8b6 100644 --- a/hapi-fhir-structures-r4/src/test/java/ca/uhn/fhir/parser/JsonParserR4Test.java +++ b/hapi-fhir-structures-r4/src/test/java/ca/uhn/fhir/parser/JsonParserR4Test.java @@ -569,6 +569,32 @@ public class JsonParserR4Test extends BaseTest { } + /** + * See #1793 + */ + @Test + public void testParseEmptyAttribute() { + String input = "{\n" + + " \"resourceType\": \"Patient\",\n" + + " \"identifier\": [\n" + + " {\n" + + " \"system\": \"https://example.com\",\n" + + " \"value\": \"\"\n" + + " }\n" + + " ]\n" + + "}"; + + IParser jsonParser = ourCtx.newJsonParser(); + jsonParser.setParserErrorHandler(new StrictErrorHandler()); + try { + jsonParser.parseResource(Patient.class, input); + fail(); + } catch (DataFormatException e) { + assertEquals("[element=\"value\"] Invalid attribute value \"\": Attribute value for element must not be empty (\"\")", e.getMessage()); + } + + } + @Test public void testParseExtensionOnPrimitive() throws IOException { String input = IOUtils.toString(JsonParserR4Test.class.getResourceAsStream("/extension-on-line.txt"), Constants.CHARSET_UTF8); From 879fbb0bf8041e2d0310dcd657eec2d39ecb8126 Mon Sep 17 00:00:00 2001 From: jamesagnew Date: Thu, 9 Apr 2020 10:34:18 -0400 Subject: [PATCH 2/5] Test fixes --- .../java/ca/uhn/fhir/parser/LenientErrorHandler.java | 2 +- .../src/main/java/ca/uhn/fhir/parser/ParserState.java | 2 +- .../main/java/ca/uhn/fhir/parser/StrictErrorHandler.java | 2 +- .../java/ca/uhn/fhir/parser/JsonParserDstu2_1Test.java | 9 +++++---- .../java/ca/uhn/fhir/parser/XmlParserDstu2_1Test.java | 2 +- .../uhn/fhir/validation/ResourceValidatorDstu2Test.java | 2 +- .../java/ca/uhn/fhir/parser/JsonParserDstu3Test.java | 8 ++++---- .../test/java/ca/uhn/fhir/parser/XmlParserDstu3Test.java | 4 ++-- .../test/java/ca/uhn/fhir/parser/JsonParserR4Test.java | 4 ++-- 9 files changed, 18 insertions(+), 17 deletions(-) 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 5082d3cdad7..49f62bb8553 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 @@ -161,7 +161,7 @@ public class LenientErrorHandler extends BaseErrorHandler implements IParserErro @Override public void extensionContainsValueAndNestedExtensions(IParseLocation theLocation) { if (myLogErrors) { - ourLog.warn("Extension contains both a value and nested extensions: {}", describeLocation(theLocation)); + ourLog.warn("{}Extension contains both a value and nested extensions", describeLocation(theLocation)); } } 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 34a2b765a0c..095997a4903 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 @@ -1282,7 +1282,7 @@ class ParserState { if ("value".equals(theName)) { if ("".equals(theValue)) { ParseLocation location = ParseLocation.fromElementName(myChildName); - myErrorHandler.invalidValue(location, theValue, "Attribute value for element must not be empty (\"\")"); + myErrorHandler.invalidValue(location, theValue, "Attribute value must not be empty (\"\")"); } else { try { myInstance.setValueAsString(theValue); 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 7c35006b255..4030b54c11c 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 @@ -86,7 +86,7 @@ public class StrictErrorHandler extends BaseErrorHandler implements IParserError @Override public void extensionContainsValueAndNestedExtensions(IParseLocation theLocation) { - throw new DataFormatException(describeLocation(theLocation) + "Extension contains both a value and nested extensions: " + theLocation); + throw new DataFormatException(describeLocation(theLocation) + "Extension contains both a value and nested extensions"); } } diff --git a/hapi-fhir-structures-dstu2.1/src/test/java/ca/uhn/fhir/parser/JsonParserDstu2_1Test.java b/hapi-fhir-structures-dstu2.1/src/test/java/ca/uhn/fhir/parser/JsonParserDstu2_1Test.java index f8a232a60a4..9d9c5531fab 100644 --- a/hapi-fhir-structures-dstu2.1/src/test/java/ca/uhn/fhir/parser/JsonParserDstu2_1Test.java +++ b/hapi-fhir-structures-dstu2.1/src/test/java/ca/uhn/fhir/parser/JsonParserDstu2_1Test.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.ArgumentMatchers.any; import static org.mockito.Matchers.eq; import static org.mockito.Matchers.isNull; import static org.mockito.Mockito.mock; @@ -1098,8 +1099,8 @@ public class JsonParserDstu2_1Test { 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()); + verify(errorHandler, times(1)).invalidValue(any(IParseLocation.class), eq(""), msgCaptor.capture()); + assertEquals("Attribute value must not be empty (\"\")", msgCaptor.getValue()); String encoded = ourCtx.newJsonParser().encodeResourceToString(parsed); assertEquals("{\"resourceType\":\"Patient\"}", encoded); @@ -1118,7 +1119,7 @@ public class JsonParserDstu2_1Test { assertEquals("foo", parsed.getGenderElement().getValueAsString()); ArgumentCaptor msgCaptor = ArgumentCaptor.forClass(String.class); - verify(errorHandler, times(1)).invalidValue(isNull(IParseLocation.class), eq("foo"), msgCaptor.capture()); + verify(errorHandler, times(1)).invalidValue(any(IParseLocation.class), eq("foo"), msgCaptor.capture()); assertEquals("Unknown AdministrativeGender code 'foo'", msgCaptor.getValue()); String encoded = ourCtx.newJsonParser().encodeResourceToString(parsed); @@ -1138,7 +1139,7 @@ public class JsonParserDstu2_1Test { assertEquals("foo", parsed.getValueDateTimeType().getValueAsString()); ArgumentCaptor msgCaptor = ArgumentCaptor.forClass(String.class); - verify(errorHandler, times(1)).invalidValue(isNull(IParseLocation.class), eq("foo"), msgCaptor.capture()); + verify(errorHandler, times(1)).invalidValue(any(IParseLocation.class), eq("foo"), msgCaptor.capture()); assertEquals("Invalid date/time format: \"foo\"", msgCaptor.getValue()); String encoded = ourCtx.newJsonParser().encodeResourceToString(parsed); diff --git a/hapi-fhir-structures-dstu2.1/src/test/java/ca/uhn/fhir/parser/XmlParserDstu2_1Test.java b/hapi-fhir-structures-dstu2.1/src/test/java/ca/uhn/fhir/parser/XmlParserDstu2_1Test.java index 2e810637a11..66fb4cd9723 100644 --- a/hapi-fhir-structures-dstu2.1/src/test/java/ca/uhn/fhir/parser/XmlParserDstu2_1Test.java +++ b/hapi-fhir-structures-dstu2.1/src/test/java/ca/uhn/fhir/parser/XmlParserDstu2_1Test.java @@ -2379,7 +2379,7 @@ public class XmlParserDstu2_1Test { 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()); + assertEquals("DataFormatException at [[row,col {unknown-source}]: [2,4]]: [element=\"active\"] Invalid attribute value \"1\": Invalid boolean string: '1'", e.getMessage()); } LenientErrorHandler errorHandler = new LenientErrorHandler(); 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 0c7e76468fe..ce1afde22af 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 @@ -82,7 +82,7 @@ public class ResourceValidatorDstu2Test { 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()); + assertEquals("DataFormatException at [[row,col {unknown-source}]: [2,4]]: [element=\"birthDate\"] Invalid attribute value \"2000-15-31\": Invalid date/time format: \"2000-15-31\"", e.getMessage()); } } 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 b87a3408961..21926260b05 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 @@ -1504,7 +1504,7 @@ public class JsonParserDstu3Test { assertEquals("foo", parsed.getValueDateTimeType().getValueAsString()); ArgumentCaptor msgCaptor = ArgumentCaptor.forClass(String.class); - verify(errorHandler, times(1)).invalidValue(isNull(), eq("foo"), msgCaptor.capture()); + verify(errorHandler, times(1)).invalidValue(any(), eq("foo"), msgCaptor.capture()); assertEquals("Invalid date/time format: \"foo\"", msgCaptor.getValue()); String encoded = ourCtx.newJsonParser().encodeResourceToString(parsed); @@ -1536,8 +1536,8 @@ public class JsonParserDstu3Test { assertEquals(null, parsed.getGenderElement().getValueAsString()); ArgumentCaptor msgCaptor = ArgumentCaptor.forClass(String.class); - verify(errorHandler, times(1)).invalidValue(isNull(), eq(""), msgCaptor.capture()); - assertEquals("Attribute values must not be empty (\"\")", msgCaptor.getValue()); + verify(errorHandler, times(1)).invalidValue(any(), eq(""), msgCaptor.capture()); + assertEquals("Attribute value must not be empty (\"\")", msgCaptor.getValue()); String encoded = ourCtx.newJsonParser().encodeResourceToString(parsed); assertEquals("{\"resourceType\":\"Patient\"}", encoded); @@ -1556,7 +1556,7 @@ public class JsonParserDstu3Test { assertEquals("foo", parsed.getGenderElement().getValueAsString()); ArgumentCaptor msgCaptor = ArgumentCaptor.forClass(String.class); - verify(errorHandler, times(1)).invalidValue(isNull(), eq("foo"), msgCaptor.capture()); + verify(errorHandler, times(1)).invalidValue(any(), eq("foo"), msgCaptor.capture()); assertEquals("Unknown AdministrativeGender code 'foo'", msgCaptor.getValue()); String encoded = ourCtx.newJsonParser().encodeResourceToString(parsed); 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 98fdae848b5..281139b3c49 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 @@ -1542,7 +1542,7 @@ public class XmlParserDstu3Test { parser.encodeResourceToString(p); fail(); } catch (DataFormatException e) { - assertEquals("Extension contains both a value and nested extensions: Patient(res).extension", e.getMessage()); + assertEquals("[element=\"Patient(res).extension\"] Extension contains both a value and nested extensions: [element=\"Patient(res).extension\"]", e.getMessage()); } } @@ -3119,7 +3119,7 @@ public class XmlParserDstu3Test { 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()); + assertEquals("DataFormatException at [[row,col {unknown-source}]: [2,4]]: [element=\"active\"] Invalid attribute value \"1\": Invalid boolean string: '1'", e.getMessage()); } LenientErrorHandler errorHandler = new LenientErrorHandler(); diff --git a/hapi-fhir-structures-r4/src/test/java/ca/uhn/fhir/parser/JsonParserR4Test.java b/hapi-fhir-structures-r4/src/test/java/ca/uhn/fhir/parser/JsonParserR4Test.java index b3363a6a8b6..2756f643cb1 100644 --- a/hapi-fhir-structures-r4/src/test/java/ca/uhn/fhir/parser/JsonParserR4Test.java +++ b/hapi-fhir-structures-r4/src/test/java/ca/uhn/fhir/parser/JsonParserR4Test.java @@ -350,7 +350,7 @@ public class JsonParserR4Test extends BaseTest { parser.encodeResourceToString(p); fail(); } catch (DataFormatException e) { - assertEquals("Extension contains both a value and nested extensions: Patient(res).extension", e.getMessage()); + assertEquals("[element=\"Patient(res).extension\"] Extension contains both a value and nested extensions", e.getMessage()); } } @@ -590,7 +590,7 @@ public class JsonParserR4Test extends BaseTest { jsonParser.parseResource(Patient.class, input); fail(); } catch (DataFormatException e) { - assertEquals("[element=\"value\"] Invalid attribute value \"\": Attribute value for element must not be empty (\"\")", e.getMessage()); + assertEquals("[element=\"value\"] Invalid attribute value \"\": Attribute value must not be empty (\"\")", e.getMessage()); } } From dce4205adb9e98941e0396291fb4dfec8277bc35 Mon Sep 17 00:00:00 2001 From: jamesagnew Date: Thu, 9 Apr 2020 11:38:49 -0400 Subject: [PATCH 3/5] Test fix --- .../src/test/java/ca/uhn/fhir/parser/XmlParserDstu3Test.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 281139b3c49..6fdf299757a 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 @@ -1542,7 +1542,7 @@ public class XmlParserDstu3Test { parser.encodeResourceToString(p); fail(); } catch (DataFormatException e) { - assertEquals("[element=\"Patient(res).extension\"] Extension contains both a value and nested extensions: [element=\"Patient(res).extension\"]", e.getMessage()); + assertEquals("[element=\"Patient(res).extension\"] Extension contains both a value and nested extensions", e.getMessage()); } } From c1d682ed02e00abce48ccf48c1fd42c7d0d20378 Mon Sep 17 00:00:00 2001 From: jamesagnew Date: Thu, 9 Apr 2020 12:36:33 -0400 Subject: [PATCH 4/5] Test fixes --- ...ClientServerValidationTestHl7OrgDstu2.java | 4 ++-- .../ResponseSizeCapturingInterceptorTest.java | 19 ++++++++++++++++--- 2 files changed, 18 insertions(+), 5 deletions(-) diff --git a/hapi-fhir-structures-hl7org-dstu2/src/test/java/ca/uhn/fhir/rest/client/ClientServerValidationTestHl7OrgDstu2.java b/hapi-fhir-structures-hl7org-dstu2/src/test/java/ca/uhn/fhir/rest/client/ClientServerValidationTestHl7OrgDstu2.java index 9a24569f3ab..392538e5343 100644 --- a/hapi-fhir-structures-hl7org-dstu2/src/test/java/ca/uhn/fhir/rest/client/ClientServerValidationTestHl7OrgDstu2.java +++ b/hapi-fhir-structures-hl7org-dstu2/src/test/java/ca/uhn/fhir/rest/client/ClientServerValidationTestHl7OrgDstu2.java @@ -100,7 +100,7 @@ public class ClientServerValidationTestHl7OrgDstu2 { @Test public void testServerReturnsWrongVersionForDstu2() throws Exception { - String wrongFhirVersion = "3.0.1"; + String wrongFhirVersion = "3.0.2"; assertThat(wrongFhirVersion, is(FhirVersionEnum.DSTU3.getFhirVersionString())); // asserting that what we assume to be the DSTU3 FHIR version is still correct Conformance conf = new Conformance(); conf.setFhirVersion(wrongFhirVersion); @@ -119,7 +119,7 @@ public class ClientServerValidationTestHl7OrgDstu2 { myCtx.newRestfulGenericClient("http://foo").read(new UriDt("http://foo/Patient/1")); fail(); } catch (FhirClientInappropriateForServerException e) { - assertThat(e.toString(), containsString("The server at base URL \"http://foo/metadata\" returned a conformance statement indicating that it supports FHIR version \"3.0.1\" which corresponds to DSTU3, but this client is configured to use DSTU2_HL7ORG (via the FhirContext)")); + assertThat(e.toString(), containsString("The server at base URL \"http://foo/metadata\" returned a conformance statement indicating that it supports FHIR version \"3.0.2\" which corresponds to DSTU3, but this client is configured to use DSTU2_HL7ORG (via the FhirContext)")); } } diff --git a/hapi-fhir-structures-r4/src/test/java/ca/uhn/fhir/rest/server/interceptor/ResponseSizeCapturingInterceptorTest.java b/hapi-fhir-structures-r4/src/test/java/ca/uhn/fhir/rest/server/interceptor/ResponseSizeCapturingInterceptorTest.java index 2f3d5046d6a..e87a80303e5 100644 --- a/hapi-fhir-structures-r4/src/test/java/ca/uhn/fhir/rest/server/interceptor/ResponseSizeCapturingInterceptorTest.java +++ b/hapi-fhir-structures-r4/src/test/java/ca/uhn/fhir/rest/server/interceptor/ResponseSizeCapturingInterceptorTest.java @@ -1,8 +1,10 @@ package ca.uhn.fhir.rest.server.interceptor; import ca.uhn.fhir.context.FhirContext; +import ca.uhn.fhir.interceptor.api.Pointcut; import ca.uhn.fhir.test.utilities.server.HashMapResourceProviderRule; import ca.uhn.fhir.test.utilities.server.RestfulServerRule; +import ca.uhn.test.concurrency.PointcutLatch; import org.apache.commons.lang3.exception.ExceptionUtils; import org.hl7.fhir.instance.model.api.IIdType; import org.hl7.fhir.r4.model.Patient; @@ -28,13 +30,17 @@ import java.util.regex.Pattern; import java.util.stream.Collectors; import static org.awaitility.Awaitility.await; +import static org.awaitility.Awaitility.waitAtMost; import static org.hamcrest.CoreMatchers.not; import static org.hamcrest.Matchers.hasSize; import static org.hamcrest.Matchers.matchesPattern; import static org.junit.Assert.assertEquals; import static org.junit.Assert.fail; import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.atMost; import static org.mockito.Mockito.doAnswer; +import static org.mockito.Mockito.timeout; +import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; @RunWith(MockitoJUnitRunner.class) @@ -63,11 +69,18 @@ public class ResponseSizeCapturingInterceptorTest { } @Test - public void testReadResource() { + public void testReadResource() throws InterruptedException { + PointcutLatch createLatch = new PointcutLatch(Pointcut.SERVER_PROCESSING_COMPLETED); + createLatch.setExpectedCount(1); + ourServerRule.getRestfulServer().getInterceptorService().registerAnonymousInterceptor(Pointcut.SERVER_PROCESSING_COMPLETED, createLatch); + Patient resource = new Patient(); resource.setActive(true); IIdType id = ourServerRule.getFhirClient().create().resource(resource).execute().getId().toUnqualifiedVersionless(); + createLatch.awaitExpected(); + ourServerRule.getRestfulServer().getInterceptorService().unregisterInterceptor(createLatch); + myInterceptor.registerConsumer(myConsumer); List stacks = Collections.synchronizedList(new ArrayList<>()); @@ -84,8 +97,8 @@ public class ResponseSizeCapturingInterceptorTest { resource = ourServerRule.getFhirClient().read().resource(Patient.class).withId(id).execute(); assertEquals(true, resource.getActive()); - await().until(()->stacks.size() > 0); - await().until(()->stacks.stream().collect(Collectors.joining("\n")), not(matchesPattern(Pattern.compile(".*INVOCATION.*INVOCATION.*", Pattern.MULTILINE | Pattern.DOTALL)))); + verify(myConsumer, timeout(Duration.ofSeconds(10)).times(1)).accept(myResultCaptor.capture()); + assertEquals(100, myResultCaptor.getValue().getWrittenChars()); } From ac29073b727849754587233441f5f29d62e9232a Mon Sep 17 00:00:00 2001 From: jamesagnew Date: Thu, 9 Apr 2020 14:22:04 -0400 Subject: [PATCH 5/5] Test fixes --- .../hapi/validation/ResourceValidatorDstu2_1Test.java | 2 +- .../fhir/dstu3/hapi/validation/ResourceValidatorDstu3Test.java | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/hapi-fhir-validation/src/test/java/org/hl7/fhir/dstu2016may/hapi/validation/ResourceValidatorDstu2_1Test.java b/hapi-fhir-validation/src/test/java/org/hl7/fhir/dstu2016may/hapi/validation/ResourceValidatorDstu2_1Test.java index a412f5f0112..5e0c0ee864b 100644 --- a/hapi-fhir-validation/src/test/java/org/hl7/fhir/dstu2016may/hapi/validation/ResourceValidatorDstu2_1Test.java +++ b/hapi-fhir-validation/src/test/java/org/hl7/fhir/dstu2016may/hapi/validation/ResourceValidatorDstu2_1Test.java @@ -72,7 +72,7 @@ public class ResourceValidatorDstu2_1Test { 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()); + assertEquals("DataFormatException at [[row,col {unknown-source}]: [2,4]]: [element=\"birthDate\"] Invalid attribute value \"2000-15-31\": Invalid date/time format: \"2000-15-31\"", e.getMessage()); } } diff --git a/hapi-fhir-validation/src/test/java/org/hl7/fhir/dstu3/hapi/validation/ResourceValidatorDstu3Test.java b/hapi-fhir-validation/src/test/java/org/hl7/fhir/dstu3/hapi/validation/ResourceValidatorDstu3Test.java index 7da1192219d..967315b17bf 100644 --- a/hapi-fhir-validation/src/test/java/org/hl7/fhir/dstu3/hapi/validation/ResourceValidatorDstu3Test.java +++ b/hapi-fhir-validation/src/test/java/org/hl7/fhir/dstu3/hapi/validation/ResourceValidatorDstu3Test.java @@ -98,7 +98,7 @@ public class ResourceValidatorDstu3Test { 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()); + assertEquals("DataFormatException at [[row,col {unknown-source}]: [2,4]]: [element=\"birthDate\"] Invalid attribute value \"2000-15-31\": Invalid date/time format: \"2000-15-31\"", e.getMessage()); } }