Validation with unknown profile (hapifhir#6608)

* failing test for invalid profile during validation

* to address validation to fail for unknown profile

* updates to address bringing back the class name that was mistakenly dropped out

* updated the logic and some dependent test cases

* spotless:apply changes

* fixed the existing test cases that were failling due to invalid profile validation logic being added

* remove unnecessary comments

* added changelog

* added jira: SMILE-9129 to changelog

* addressing PR comments

* spotless:apply updates

* addressing code review suggestions

* code review updates

---------

Co-authored-by: leif stawnyczy <[email protected]>
Co-authored-by: Buland Malik <[email protected]>
Co-authored-by: peartree <[email protected]>

Author: 4 people

hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/8_2_0/6608-fail-on-invalid-profile.yaml

@@ -0,0 +1,5 @@
+---
+type: fix
+issue: 6608
+jira: SMILE-9129
+title: "Previously, attempting to validate a resource ($validate) with an invalid profile was not generating any error. This issue is fixed."

hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4ValidateTest.java

@@ -32,6 +32,7 @@
 import ca.uhn.fhir.util.OperationOutcomeUtil;
 import ca.uhn.fhir.util.StopWatch;
 import ca.uhn.fhir.validation.IValidatorModule;
+import ca.uhn.fhir.validation.ResultSeverityEnum;
 import ca.uhn.test.util.LogbackTestExtension;
 import ca.uhn.test.util.LogbackTestExtensionAssert;
 import ch.qos.logback.classic.Level;
@@ -981,7 +982,10 @@ public void testValidateValueSet() {
 		OperationOutcome oo = validateAndReturnOutcome(vs);
 		ourLog.debug(myFhirContext.newJsonParser().setPrettyPrint(true).encodeResourceToString(oo));
 
-		assertEquals("The code '123' is not valid in the system https://bb (Validation failed)", oo.getIssue().get(0).getDiagnostics());
+		assertThat(oo.getIssue().stream())
+			.anyMatch(r ->
+				r.getDiagnostics().equals("The code '123' is not valid in the system https://bb (Validation failed)") );
+
 	}
 
 	@Test

hapi-fhir-validation/src/main/java/org/hl7/fhir/common/hapi/validation/validator/ValidatorWrapper.java

@@ -143,8 +143,9 @@ public List<ValidationMessage> validate(
 		List<ValidationMessage> messages = new ArrayList<>();
 
 		List<StructureDefinition> profiles = new ArrayList<>();
+		List<ValidationMessage> invalidProfileValidationMessages = new ArrayList<>();
 		for (String nextProfileUrl : theValidationContext.getOptions().getProfiles()) {
-			fetchAndAddProfile(theWorkerContext, profiles, nextProfileUrl, messages);
+			fetchAndAddProfile(theWorkerContext, profiles, nextProfileUrl, invalidProfileValidationMessages);
 		}
 
 		String input = theValidationContext.getResourceAsString();
@@ -167,7 +168,7 @@ public List<ValidationMessage> validate(
 			// Determine if meta/profiles are present...
 			ArrayList<String> profileUrls = determineIfProfilesSpecified(document);
 			for (String nextProfileUrl : profileUrls) {
-				fetchAndAddProfile(theWorkerContext, profiles, nextProfileUrl, messages);
+				fetchAndAddProfile(theWorkerContext, profiles, nextProfileUrl, invalidProfileValidationMessages);
 			}
 
 			Manager.FhirFormat format = Manager.FhirFormat.XML;
@@ -185,7 +186,8 @@ public List<ValidationMessage> validate(
 					JsonArray profilesArray = profileElement.getAsJsonArray();
 					for (JsonElement element : profilesArray) {
 						String nextProfileUrl = element.getAsString();
-						fetchAndAddProfile(theWorkerContext, profiles, nextProfileUrl, messages);
+						fetchAndAddProfile(
+								theWorkerContext, profiles, nextProfileUrl, invalidProfileValidationMessages);
 					}
 				}
 			}
@@ -196,6 +198,10 @@ public List<ValidationMessage> validate(
 			throw new IllegalArgumentException(Msg.code(649) + "Unknown encoding: " + encoding);
 		}
 
+		if (profiles.isEmpty() && !invalidProfileValidationMessages.isEmpty()) {
+			messages.addAll(invalidProfileValidationMessages);
+		}
+
 		// TODO: are these still needed?
 		messages = messages.stream()
 				.filter(m -> m.getMessageId() == null
@@ -234,11 +240,17 @@ private void fetchAndAddProfile(
 			IWorkerContext theWorkerContext,
 			List<StructureDefinition> theProfileStructureDefinitions,
 			String theUrl,
-			List<ValidationMessage> theMessages) {
+			List<ValidationMessage> theValidationMessages) {
 		try {
 			StructureDefinition structureDefinition = theWorkerContext.fetchResource(StructureDefinition.class, theUrl);
 			if (structureDefinition != null) {
 				theProfileStructureDefinitions.add(structureDefinition);
+			} else {
+				ValidationMessage m = new ValidationMessage();
+				m.setMessageId(I18nConstants.VALIDATION_VAL_PROFILE_UNKNOWN);
+				m.setLevel(ValidationMessage.IssueSeverity.ERROR);
+				m.setMessage("Invalid profile. Failed to retrieve profile with url=" + theUrl);
+				theValidationMessages.add(m);
 			}
 		} catch (FHIRException e) {
 			ourLog.debug("Failed to load profile: {}", theUrl);

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

@@ -233,7 +233,6 @@ public void testValidateStorageResponseCodeBad() {
 		List<SingleValidationMessage> all = logResultsAndReturnErrorOnes(result);
 		assertThat(result.isSuccessful()).as(all.toString()).isFalse();
 		assertThat(result.getMessages().get(0).getMessage()).startsWith("Unknown code 'https://hapifhir.io/fhir/CodeSystem/hapi-fhir-storage-response-code#foo'");
-
 	}
 
 	@Test
@@ -1123,9 +1122,11 @@ public void testValidateResourceContainingProfileDeclarationDoesntResolve() {
 		ValidationResult output = myFhirValidator.validateWithResult(input);
 		List<SingleValidationMessage> errors = logResultsAndReturnNonInformationalOnes(output);
 
-		assertThat(errors).hasSize(1);
-		assertEquals("Profile reference 'http://foo/structuredefinition/myprofile' has not been checked because it could not be found", errors.get(0).getMessage());
-		assertEquals(ResultSeverityEnum.ERROR, errors.get(0).getSeverity());
+		assertThat(errors).hasSize(2);
+		assertThat(errors.stream())
+			.anyMatch(r ->
+				(r.getSeverity() == ResultSeverityEnum.ERROR) &&
+					(r.getMessage().equals("Profile reference 'http://foo/structuredefinition/myprofile' has not been checked because it could not be found")) );
 	}
 
 	@Test

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

@@ -4,15 +4,23 @@
 import ca.uhn.fhir.context.support.DefaultProfileValidationSupport;
 import ca.uhn.fhir.context.support.IValidationSupport;
 import ca.uhn.fhir.fhirpath.BaseValidationTestWithInlineMocks;
+import ca.uhn.fhir.parser.IParser;
 import ca.uhn.fhir.validation.FhirValidator;
+import ca.uhn.fhir.validation.ResultSeverityEnum;
+import ca.uhn.fhir.validation.ValidationOptions;
 import ca.uhn.fhir.validation.ValidationResult;
 import org.hl7.fhir.common.hapi.validation.support.ValidationSupportChain;
 import org.hl7.fhir.common.hapi.validation.validator.FhirInstanceValidator;
 import org.hl7.fhir.r4.model.MedicationRequest;
+import org.hl7.fhir.r4.model.Patient;
 import org.junit.jupiter.api.Test;
 
 import java.io.IOException;
 
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
 public class ParserWithValidationR4Test extends BaseValidationTestWithInlineMocks {
 	private static final FhirContext ourCtx = FhirContext.forR4();
 
@@ -30,6 +38,46 @@ public void testActivityDefinitionElementsOrder() throws IOException {
 		validationResult.getMessages().forEach(System.out::println);
 	}
 
+	// https://www.hl7.org/fhir/r4/resource-operation-validate.html
+	@Test
+	public void validate_withUnknownProfile_shouldFail() {
+		// setup
+		IParser parser = ourCtx.newJsonParser();
+		Patient patient;
+		{
+			String patientStr = """
+    			{
+    				"resourceType": "Patient",
+    				"name": [{
+    					"family": "Hirasawa",
+    					"given": [ "yui" ]
+    				}]
+    			}
+				""";
+			patient = parser.parseResource(Patient.class, patientStr);
+		}
+
+		// test
+		ValidationOptions options = new ValidationOptions();
+		String profileUrl = "http://tempuri.org/does-not-exist";
+		options.addProfile(profileUrl);
+
+		final FhirInstanceValidator instanceValidator = new FhirInstanceValidator(ourCtx);
+		FhirValidator validator = ourCtx.newValidator();
+
+		// test
+		validator.registerValidatorModule(instanceValidator);
+		ValidationResult validationResult = validator.validateWithResult(patient, options);
+
+		// verify
+		assertFalse(validationResult.isSuccessful());
+		assertThat(validationResult.getMessages().stream())
+			.anyMatch(r ->
+						(r.getSeverity() == ResultSeverityEnum.ERROR) &&
+						(r.getMessage().equals("Invalid profile. Failed to retrieve profile with url="+profileUrl)) );
+	}
+
+
 	private IValidationSupport getValidationSupport() {
 		return new ValidationSupportChain(new DefaultProfileValidationSupport(ourCtx));
 	}

hapi-fhir-validation/src/test/java/org/hl7/fhir/r4b/validation/FhirInstanceValidatorR4BTest.java

@@ -1022,9 +1022,12 @@ public void testValidateResourceContainingProfileDeclarationDoesntResolve() {
 		ValidationResult output = myFhirValidator.validateWithResult(input);
 		List<SingleValidationMessage> errors = logResultsAndReturnNonInformationalOnes(output);
 
-		assertThat(errors).hasSize(1);
-		assertEquals("Profile reference 'http://foo/structuredefinition/myprofile' has not been checked because it could not be found", errors.get(0).getMessage());
-		assertEquals(ResultSeverityEnum.ERROR, errors.get(0).getSeverity());
+		assertThat(errors).hasSize(2);
+		assertThat(errors.stream())
+			.anyMatch(r ->
+				(r.getSeverity() == ResultSeverityEnum.ERROR) &&
+					(r.getMessage().equals("Profile reference 'http://foo/structuredefinition/myprofile' has not been checked because it could not be found")) );
+
 	}
 
 	@Test