Add line numbers to validation failures

hapi-fhir-base/src/main/java/ca/uhn/fhir/util/OperationOutcomeUtil.java

@@ -19,22 +19,17 @@ package ca.uhn.fhir.util;
  * limitations under the License.
  * #L%
  */
-import static org.apache.commons.lang3.StringUtils.isNotBlank;
-
-import java.util.List;
 
+import ca.uhn.fhir.context.*;
+import ca.uhn.fhir.rest.server.exceptions.InternalErrorException;
 import org.hl7.fhir.instance.model.api.IBase;
 import org.hl7.fhir.instance.model.api.IBaseOperationOutcome;
 import org.hl7.fhir.instance.model.api.IBaseResource;
 import org.hl7.fhir.instance.model.api.IPrimitiveType;
 
-import ca.uhn.fhir.context.BaseRuntimeChildDefinition;
+import java.util.List;
-import ca.uhn.fhir.context.BaseRuntimeElementCompositeDefinition;
+
-import ca.uhn.fhir.context.BaseRuntimeElementDefinition;
+import static org.apache.commons.lang3.StringUtils.isNotBlank;
-import ca.uhn.fhir.context.FhirContext;
-import ca.uhn.fhir.context.FhirVersionEnum;
-import ca.uhn.fhir.context.RuntimeResourceDefinition;
-import ca.uhn.fhir.rest.server.exceptions.InternalErrorException;
 
 /**
  * Utilities for dealing with OperationOutcome resources across various model versions
@@ -43,20 +38,17 @@ public class OperationOutcomeUtil {
 
 	/**
 	 * Add an issue to an OperationOutcome
-	 * 
+	 *  @param theCtx              The fhir context
-	 * @param theCtx
+	 * @param theOperationOutcome The OO resource to add to
-	 *           The fhir context
+	 * @param theSeverity         The severity (fatal | error | warning | information)
-	 * @param theOperationOutcome
+	 * @param theDetails          The details string
-	 *           The OO resource to add to
-	 * @param theSeverity
-	 *           The severity (fatal | error | warning | information)
-	 * @param theDetails
-	 *           The details string
 	 * @param theCode
+	 * @return Returns the newly added issue
 	 */
-	public static void addIssue(FhirContext theCtx, IBaseOperationOutcome theOperationOutcome, String theSeverity, String theDetails, String theLocation, String theCode) {
+	public static IBase addIssue(FhirContext theCtx, IBaseOperationOutcome theOperationOutcome, String theSeverity, String theDetails, String theLocation, String theCode) {
 		IBase issue = createIssue(theCtx, theOperationOutcome);
 		populateDetails(theCtx, issue, theSeverity, theDetails, theLocation, theCode);
+		return issue;
 	}
 
 	private static IBase createIssue(FhirContext theCtx, IBaseResource theOutcome) {
@@ -140,7 +132,6 @@ public class OperationOutcomeUtil {
 
 		BaseRuntimeElementDefinition<?> stringDef = detailsChild.getChildByName(detailsChild.getElementName());
 		BaseRuntimeChildDefinition severityChild = issueElement.getChildByName("severity");
-		BaseRuntimeChildDefinition locationChild = issueElement.getChildByName("location");
 
 		IPrimitiveType<?> severityElem = (IPrimitiveType<?>) severityChild.getChildByName("severity").newInstance(severityChild.getInstanceConstructorArguments());
 		severityElem.setValueAsString(theSeverity);
@@ -150,7 +141,13 @@ public class OperationOutcomeUtil {
 		string.setValueAsString(theDetails);
 		detailsChild.getMutator().setValue(theIssue, string);
 
+		addLocationToIssue(theCtx, theIssue, theLocation);
+	}
+
+	public static void addLocationToIssue(FhirContext theContext, IBase theIssue, String theLocation) {
 		if (isNotBlank(theLocation)) {
+			BaseRuntimeElementCompositeDefinition<?> issueElement = (BaseRuntimeElementCompositeDefinition<?>) theContext.getElementDefinition(theIssue.getClass());
+			BaseRuntimeChildDefinition locationChild = issueElement.getChildByName("location");
 			IPrimitiveType<?> locationElem = (IPrimitiveType<?>) locationChild.getChildByName("location").newInstance(locationChild.getInstanceConstructorArguments());
 			locationElem.setValueAsString(theLocation);
 			locationChild.getMutator().addValue(theIssue, locationElem);

hapi-fhir-base/src/main/java/ca/uhn/fhir/validation/IInstanceValidatorModule.java

@@ -0,0 +1,12 @@
+package ca.uhn.fhir.validation;
+
+/**
+ * This interface marks a {@link IValidatorModule validator module} that uses the FHIR
+ * FhirInstanceValidator as the underlying engine (i.e. it performs
+ * profile validation)
+ */
+public interface IInstanceValidatorModule extends IValidatorModule {
+
+	// nothing extra yet
+
+}

hapi-fhir-base/src/main/java/ca/uhn/fhir/validation/ValidationResult.java

@@ -25,6 +25,7 @@ import static org.apache.commons.lang3.StringUtils.isNotBlank;
 import java.util.Collections;
 import java.util.List;
 
+import org.hl7.fhir.instance.model.api.IBase;
 import org.hl7.fhir.instance.model.api.IBaseOperationOutcome;
 
 import ca.uhn.fhir.context.FhirContext;
@@ -120,7 +121,19 @@ public class ValidationResult {
 				location = null;
 			}
 			String severity = next.getSeverity() != null ? next.getSeverity().getCode() : null;
-			OperationOutcomeUtil.addIssue(myCtx, theOperationOutcome, severity, next.getMessage(), location, Constants.OO_INFOSTATUS_PROCESSING);
+			IBase issue = OperationOutcomeUtil.addIssue(myCtx, theOperationOutcome, severity, next.getMessage(), location, Constants.OO_INFOSTATUS_PROCESSING);
+			
+			if (next.getLocationLine() != null || next.getLocationCol() != null) {
+				String line = "(unknown)";
+				if (next.getLocationLine() != null) {
+					line = next.getLocationLine().toString();
+				}
+				String col = "(unknown)";
+				if (next.getLocationCol() != null) {
+					col = next.getLocationCol().toString();
+				}
+				OperationOutcomeUtil.addLocationToIssue(myCtx, issue, "Line " + line + ", Col " + col);
+			}
 		}
 
 		if (myMessages.isEmpty()) {

hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/JpaResourceDao.java

@@ -0,0 +1,9 @@
+package ca.uhn.fhir.jpa.dao;
+
+import org.hl7.fhir.instance.model.api.IBaseResource;
+
+public class JpaResourceDao<T extends IBaseResource> extends BaseHapiFhirResourceDao<T> {
+
+	// nothing yet
+
+}

hapi-fhir-structures-r4/src/test/java/ca/uhn/fhir/util/OperationOutcomeUtilTest.java

@@ -0,0 +1,37 @@
+package ca.uhn.fhir.util;
+
+import ca.uhn.fhir.context.FhirContext;
+import org.hl7.fhir.instance.model.api.IBase;
+import org.hl7.fhir.r4.model.OperationOutcome;
+import org.junit.Test;
+
+import static org.junit.Assert.*;
+
+public class OperationOutcomeUtilTest {
+
+	private FhirContext myCtx = FhirContext.forR4();
+
+	@Test
+	public void testHasIssueTrue() {
+		OperationOutcome oo =new OperationOutcome();
+		oo.addIssue().setDiagnostics("foo");
+		assertTrue(OperationOutcomeUtil.hasIssues(myCtx, oo));
+	}
+
+	@Test
+	public void testHasIssueFalse() {
+		OperationOutcome oo =new OperationOutcome();
+		assertFalse(OperationOutcomeUtil.hasIssues(myCtx, oo));
+	}
+
+	@Test
+	public void testAddIssue() {
+		OperationOutcome oo = (OperationOutcome) OperationOutcomeUtil.newInstance(myCtx);
+		IBase issue = OperationOutcomeUtil.addIssue(myCtx, oo, "error", "Help i'm a bug", "/Patient", "throttled");
+		OperationOutcomeUtil.addLocationToIssue(myCtx, issue, null);
+		OperationOutcomeUtil.addLocationToIssue(myCtx, issue, "");
+		OperationOutcomeUtil.addLocationToIssue(myCtx, issue, "line 3");
+		assertEquals("{\"resourceType\":\"OperationOutcome\",\"issue\":[{\"severity\":\"error\",\"code\":\"throttled\",\"diagnostics\":\"Help i'm a bug\",\"location\":[\"/Patient\",\"line 3\"]}]}", myCtx.newJsonParser().encodeResourceToString(oo));
+	}
+
+}

hapi-fhir-test-utilities/src/main/java/ca/uhn/fhir/test/BaseTest.java

@@ -1,19 +1,49 @@
 package ca.uhn.fhir.test;
 
+import ca.uhn.fhir.context.FhirContext;
+import ca.uhn.fhir.rest.api.EncodingEnum;
+import ca.uhn.fhir.rest.server.exceptions.InternalErrorException;
 import com.google.common.base.Charsets;
 import org.apache.commons.io.IOUtils;
+import org.hl7.fhir.instance.model.api.IBaseResource;
 
 import java.io.IOException;
 import java.io.InputStream;
+import java.util.function.Function;
+import java.util.zip.GZIPInputStream;
 
 public class BaseTest {
 
 	protected String loadResource(String theClasspath) throws IOException {
-		InputStream stream = BaseTest.class.getResourceAsStream(theClasspath);
+		Function<InputStream, InputStream> streamTransform = t->t;
-		if (stream==null) {
+		return loadResource(theClasspath, streamTransform);
-			throw new IllegalArgumentException("Unable to find resource: " + theClasspath);
-		}
-		return IOUtils.toString(stream, Charsets.UTF_8);
 	}
 
+	private String loadResource(String theClasspath, Function<InputStream, InputStream> theStreamTransform) throws IOException {
+		try (InputStream stream = BaseTest.class.getResourceAsStream(theClasspath)) {
+			if (stream == null) {
+				throw new IllegalArgumentException("Unable to find resource: " + theClasspath);
+			}
+
+			InputStream newStream = theStreamTransform.apply(stream);
+
+			return IOUtils.toString(newStream, Charsets.UTF_8);
+		}
+	}
+
+	protected String loadCompressedResource(String theClasspath) throws IOException {
+		Function<InputStream, InputStream> streamTransform = t-> {
+			try {
+				return new GZIPInputStream(t);
+			} catch (IOException e) {
+				throw new InternalErrorException(e);
+			}
+		};
+		return loadResource(theClasspath, streamTransform);
+	}
+
+	protected <T extends IBaseResource> T loadResource(FhirContext theCtx, Class<T> theType, String theClasspath) throws IOException {
+		String raw = loadResource(theClasspath);
+		EncodingEnum.detectEncodingNoDefault(raw).newParser(theCtx).parseResource(theType, raw);
+	}
 }

hapi-fhir-validation/src/test/java/org/hl7/fhir/r4/validation/FhirInstanceValidatorR4Test.java

@@ -2,6 +2,7 @@ package org.hl7.fhir.r4.validation;
 
 import ca.uhn.fhir.context.FhirContext;
 import ca.uhn.fhir.rest.api.Constants;
+import ca.uhn.fhir.test.BaseTest;
 import ca.uhn.fhir.util.TestUtil;
 import ca.uhn.fhir.validation.FhirValidator;
 import ca.uhn.fhir.validation.ResultSeverityEnum;
@@ -54,7 +55,7 @@ import static org.mockito.ArgumentMatchers.nullable;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.when;
 
-public class FhirInstanceValidatorR4Test {
+public class FhirInstanceValidatorR4Test extends BaseTest {
 
 	private static final org.slf4j.Logger ourLog = org.slf4j.LoggerFactory.getLogger(FhirInstanceValidatorR4Test.class);
 	private static DefaultProfileValidationSupport myDefaultValidationSupport = new DefaultProfileValidationSupport();
@@ -71,7 +72,7 @@ public class FhirInstanceValidatorR4Test {
 	private Map<String, ValueSetExpansionComponent> mySupportedCodeSystemsForExpansion;
 	private FhirValidator myVal;
 	private ArrayList<String> myValidConcepts;
-	private Set<String> myValidSystems = new HashSet<String>();
+	private Set<String> myValidSystems = new HashSet<>();
 
 	private void addValidConcept(String theSystem, String theCode) {
 		myValidSystems.add(theSystem);
@@ -111,21 +112,18 @@ public class FhirInstanceValidatorR4Test {
 
 		myValidConcepts = new ArrayList<>();
 
-		when(myMockSupport.expandValueSet(nullable(FhirContext.class), nullable(ConceptSetComponent.class))).thenAnswer(new Answer<ValueSetExpander.ValueSetExpansionOutcome>() {
+		when(myMockSupport.expandValueSet(nullable(FhirContext.class), nullable(ConceptSetComponent.class))).thenAnswer(t -> {
-			@Override
+			ConceptSetComponent arg = (ConceptSetComponent) t.getArguments()[1];
-			public ValueSetExpander.ValueSetExpansionOutcome answer(InvocationOnMock theInvocation) throws Throwable {
+			ValueSetExpansionComponent retVal = mySupportedCodeSystemsForExpansion.get(arg.getSystem());
-				ConceptSetComponent arg = (ConceptSetComponent) theInvocation.getArguments()[1];
+			if (retVal == null) {
-				ValueSetExpansionComponent retVal = mySupportedCodeSystemsForExpansion.get(arg.getSystem());
+				ValueSetExpander.ValueSetExpansionOutcome outcome = myDefaultValidationSupport.expandValueSet(ourCtx, arg);
-				if (retVal == null) {
+				return outcome;
-					ValueSetExpander.ValueSetExpansionOutcome outcome = myDefaultValidationSupport.expandValueSet(ourCtx, arg);
-					return outcome;
-				}
-				ourLog.debug("expandValueSet({}) : {}", new Object[]{theInvocation.getArguments()[0], retVal});
-
-				ValueSet valueset = new ValueSet();
-				valueset.setExpansion(retVal);
-				return new ValueSetExpander.ValueSetExpansionOutcome(valueset);
 			}
+			ourLog.debug("expandValueSet({}) : {}", new Object[]{t.getArguments()[0], retVal});
+
+			ValueSet valueset = new ValueSet();
+			valueset.setExpansion(retVal);
+			return new ValueSetExpander.ValueSetExpansionOutcome(valueset);
 		});
 		when(myMockSupport.isCodeSystemSupported(nullable(FhirContext.class), nullable(String.class))).thenAnswer(new Answer<Boolean>() {
 			@Override
@@ -217,7 +215,7 @@ public class FhirInstanceValidatorR4Test {
 		int index = 0;
 		for (SingleValidationMessage next : theOutput.getMessages()) {
 			ourLog.info("Result {}: {} - {}:{} {} - {}",
-				new Object[]{index, next.getSeverity(), defaultString(next.getLocationLine()), defaultString(next.getLocationCol()), next.getLocationString(), next.getMessage()});
+				index, next.getSeverity(), defaultString(next.getLocationLine()), defaultString(next.getLocationCol()), next.getLocationString(), next.getMessage());
 			index++;
 
 			retVal.add(next);
@@ -304,11 +302,11 @@ public class FhirInstanceValidatorR4Test {
 
 
 	private List<SingleValidationMessage> logResultsAndReturnNonInformationalOnes(ValidationResult theOutput) {
-		List<SingleValidationMessage> retVal = new ArrayList<SingleValidationMessage>();
+		List<SingleValidationMessage> retVal = new ArrayList<>();
 
 		int index = 0;
 		for (SingleValidationMessage next : theOutput.getMessages()) {
-			ourLog.info("Result {}: {} - {} - {}", new Object[]{index, next.getSeverity(), next.getLocationString(), next.getMessage()});
+			ourLog.info("Result {}: {} - {} - {}", index, next.getSeverity(), next.getLocationString(), next.getMessage());
 			index++;
 
 			if (next.getSeverity() != ResultSeverityEnum.INFORMATION) {
@@ -320,11 +318,11 @@ public class FhirInstanceValidatorR4Test {
 	}
 
 	private List<SingleValidationMessage> logResultsAndReturnErrorOnes(ValidationResult theOutput) {
-		List<SingleValidationMessage> retVal = new ArrayList<SingleValidationMessage>();
+		List<SingleValidationMessage> retVal = new ArrayList<>();
 
 		int index = 0;
 		for (SingleValidationMessage next : theOutput.getMessages()) {
-			ourLog.info("Result {}: {} - {} - {}", new Object[]{index, next.getSeverity(), next.getLocationString(), next.getMessage()});
+			ourLog.info("Result {}: {} - {} - {}", index, next.getSeverity(), next.getLocationString(), next.getMessage());
 			index++;
 
 			if (next.getSeverity().ordinal() > ResultSeverityEnum.WARNING.ordinal()) {
@@ -359,7 +357,7 @@ public class FhirInstanceValidatorR4Test {
 
 	@Test
 	public void testValidateBundleWithNoFullUrl() throws IOException {
-		String encoded = IOUtils.toString(FhirInstanceValidatorR4Test.class.getResourceAsStream("/r4/r4-caredove-bundle.json"));
+		String encoded = loadResource("/r4/r4-caredove-bundle.json");
 
 		ValidationResult output = myVal.validateWithResult(encoded);
 		List<SingleValidationMessage> errors = logResultsAndReturnNonInformationalOnes(output);
@@ -459,9 +457,7 @@ public class FhirInstanceValidatorR4Test {
 	@Test
 	@Ignore
 	public void testValidateBigRawJsonResource() throws Exception {
-		InputStream stream = FhirInstanceValidatorR4Test.class.getResourceAsStream("/conformance.json.gz");
+		String input = super.loadCompressedResource("/conformance.json.gz");
-		stream = new GZIPInputStream(stream);
-		String input = IOUtils.toString(stream);
 
 		long start = System.currentTimeMillis();
 		ValidationResult output = null;
@@ -485,8 +481,7 @@ public class FhirInstanceValidatorR4Test {
 		org.hl7.fhir.r4.model.Bundle bundle;
 		String name = "profiles-resources";
 		ourLog.info("Uploading " + name);
-		String vsContents;
+		String vsContents = loadResource("/org/hl7/fhir/r4/model/profile/" + name + ".xml");
-		vsContents = IOUtils.toString(FhirInstanceValidatorR4Test.class.getResourceAsStream("/org/hl7/fhir/r4/model/profile/" + name + ".xml"), "UTF-8");
 
 		TreeSet<String> ids = new TreeSet<>();
 
@@ -532,7 +527,7 @@ public class FhirInstanceValidatorR4Test {
 
 	@Test
 	public void testValidateBundleWithNoType() throws Exception {
-		String vsContents = IOUtils.toString(FhirInstanceValidatorR4Test.class.getResourceAsStream("/r4/bundle-with-no-type.json"), "UTF-8");
+		String vsContents = loadResource("/r4/bundle-with-no-type.json");
 
 		ValidationResult output = myVal.validateWithResult(vsContents);
 		logResultsAndReturnNonInformationalOnes(output);
@@ -562,7 +557,7 @@ public class FhirInstanceValidatorR4Test {
 		String name = "profiles-resources";
 		ourLog.info("Uploading " + name);
 		String inputString;
-		inputString = IOUtils.toString(FhirInstanceValidatorR4Test.class.getResourceAsStream("/brian_reinhold_bundle.json"), "UTF-8");
+		inputString = loadResource("/brian_reinhold_bundle.json");
 		Bundle bundle = ourCtx.newJsonParser().parseResource(Bundle.class, inputString);
 
 		FHIRPathEngine fp = new FHIRPathEngine(new HapiWorkerContext(ourCtx, myDefaultValidationSupport));
@@ -588,7 +583,7 @@ public class FhirInstanceValidatorR4Test {
 	@Test
 	@Ignore
 	public void testValidateDocument() throws Exception {
-		String vsContents = IOUtils.toString(FhirInstanceValidatorR4Test.class.getResourceAsStream("/sample-document.xml"), "UTF-8");
+		String vsContents = loadResource("/sample-document.xml");
 
 		ValidationResult output = myVal.validateWithResult(vsContents);
 		logResultsAndReturnNonInformationalOnes(output);
@@ -608,7 +603,7 @@ public class FhirInstanceValidatorR4Test {
 		FhirValidator val = ourCtx.newValidator();
 		val.registerValidatorModule(new FhirInstanceValidator(support));
 
-		Consent input = ourCtx.newJsonParser().parseResource(Consent.class, IOUtils.toString(ResourceValidatorDstu3Test.class.getResourceAsStream("/r4/myconsent-resource.json")));
+		Consent input = super.loadResource(ourCtx, Consent.class, "/r4/myconsent-resource.json");
 
 		input.getPolicyFirstRep().setAuthority("http://foo");
 		//input.setScope(Consent.ConsentScope.ADR);
@@ -639,7 +634,7 @@ public class FhirInstanceValidatorR4Test {
 	@Test
 	@Ignore
 	public void testValidateQuestionnaireResponse() throws IOException {
-		String input = IOUtils.toString(FhirInstanceValidatorR4Test.class.getResourceAsStream("/qr_jon.xml"));
+		String input = loadResource("/qr_jon.xml");
 
 		ValidationResult output = myVal.validateWithResult(input);
 		logResultsAndReturnAll(output);
@@ -681,14 +676,18 @@ public class FhirInstanceValidatorR4Test {
 		ourLog.info(output.getMessages().get(0).getMessage());
 		assertEquals("/Patient", output.getMessages().get(0).getLocationString());
 		assertEquals("Unrecognised property '@foo'", output.getMessages().get(0).getMessage());
+
+		OperationOutcome operationOutcome = (OperationOutcome) output.toOperationOutcome();
+		ourLog.info(ourCtx.newJsonParser().setPrettyPrint(true).encodeResourceToString(operationOutcome));
+		assertEquals("Unrecognised property '@foo'", operationOutcome.getIssue().get(0).getDiagnostics());
+		assertEquals("/Patient", operationOutcome.getIssue().get(0).getLocation().get(0).getValue());
+		assertEquals("Line 5, Col 24", operationOutcome.getIssue().get(0).getLocation().get(1).getValue());
 	}
 
 	@Test
 	@Ignore
 	public void testValidateRawJsonResourceFromExamples() throws Exception {
-		// @formatter:off
+		String input = loadResource("/testscript-search.json");
-		String input = IOUtils.toString(FhirInstanceValidator.class.getResourceAsStream("/testscript-search.json"));
-		// @formatter:on
 
 		ValidationResult output = myVal.validateWithResult(input);
 		logResultsAndReturnNonInformationalOnes(output);
@@ -884,7 +883,7 @@ public class FhirInstanceValidatorR4Test {
 	 * A reference with only an identifier should be valid
 	 */
 	@Test
-	public void testValidateReferenceWithDisplayValid() throws Exception {
+	public void testValidateReferenceWithDisplayValid() {
 		Patient p = new Patient();
 		p.getText().setDiv(new XhtmlNode().setValue("<div>AA</div>")).setStatus(Narrative.NarrativeStatus.GENERATED);
 		p.getManagingOrganization().setDisplay("HELLO");
@@ -898,7 +897,7 @@ public class FhirInstanceValidatorR4Test {
 	 * A reference with only an identifier should be valid
 	 */
 	@Test
-	public void testValidateReferenceWithIdentifierValid() throws Exception {
+	public void testValidateReferenceWithIdentifierValid() {
 		Patient p = new Patient();
 		p.getText().setDiv(new XhtmlNode().setValue("<div>AA</div>")).setStatus(Narrative.NarrativeStatus.GENERATED);
 		p.getManagingOrganization().getIdentifier().setSystem("http://acme.org");
@@ -1166,11 +1165,9 @@ public class FhirInstanceValidatorR4Test {
 
 		ValidationResult output = myVal.validateWithResult(o);
 		List<SingleValidationMessage> valMessages = logResultsAndReturnAll(output);
-		List<String> messages = new ArrayList<>();
+		for (SingleValidationMessage msg : valMessages) {
-		for (String msg : messages) {
+			assertThat(msg.getMessage(), not(containsString("have a performer")));
-			messages.add(msg);
 		}
-		assertThat(messages, not(hasItem("All observations should have a performer")));
 	}
 
 	@Test
@@ -1194,7 +1191,7 @@ public class FhirInstanceValidatorR4Test {
 	@Test
 	@Ignore
 	public void testValidateStructureDefinition() throws IOException {
-		String input = IOUtils.toString(FhirInstanceValidatorR4Test.class.getResourceAsStream("/sdc-questionnaire.profile.xml"));
+		String input = loadResource("/sdc-questionnaire.profile.xml");
 
 		ValidationResult output = myVal.validateWithResult(input);
 		logResultsAndReturnAll(output);