Add validation results to oo (#595)

* Add interceptor

* Add changelog entry

* Update changes.xml

Corrected reference to issue 586 (was 585). Corrected order of actions for issues 586 and 595 (was reversed).

* Update changes.xml

Should have been 585 after all. Whoops!

* Update changes.xml

Adding an item for pull request 565 that was previously approved and merged.

* Fixed test with English String in assertion.

hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/server/interceptor/BaseValidatingInterceptor.java

@@ -267,7 +267,10 @@ abstract class BaseValidatingInterceptor<T> extends InterceptorAdapter {
 	 */
 	protected void postProcessResult(RequestDetails theRequestDetails, ValidationResult theValidationResult) { }
 
-	protected void validate(T theRequest, RequestDetails theRequestDetails) {
+	/**
+	 * Note: May return null
+	 */
+	protected ValidationResult validate(T theRequest, RequestDetails theRequestDetails) {
 		FhirValidator validator = theRequestDetails.getServer().getFhirContext().newValidator();
 		if (myValidatorModules != null) {
 			for (IValidatorModule next : myValidatorModules) {
@@ -276,7 +279,7 @@ abstract class BaseValidatingInterceptor<T> extends InterceptorAdapter {
 		}
 
 		if (theRequest == null) {
-			return;
+			return null;
 		}
 
 		ValidationResult validationResult;
@@ -285,7 +288,7 @@ abstract class BaseValidatingInterceptor<T> extends InterceptorAdapter {
 		} catch (Exception e) {
 			if (myIgnoreValidatorExceptions) {
 				ourLog.warn("Validator threw an exception during validation", e);
-				return;
+				return null;
 			}
 			if (e instanceof BaseServerResponseException) {
 				throw (BaseServerResponseException)e;
@@ -312,7 +315,7 @@ abstract class BaseValidatingInterceptor<T> extends InterceptorAdapter {
 			for (SingleValidationMessage next : validationResult.getMessages()) {
 				if (next.getSeverity().ordinal() >= myFailOnSeverity) {
 					fail(theRequestDetails, validationResult);
-					return;
+					return validationResult;
 				}
 			}
 		}
@@ -342,6 +345,8 @@ abstract class BaseValidatingInterceptor<T> extends InterceptorAdapter {
 		}
 
 		postProcessResult(theRequestDetails, validationResult);
+		
+		return validationResult;
 	}
 
 	private static class MyLookup extends StrLookup<String> {

hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/server/interceptor/RequestValidatingInterceptor.java

@@ -27,6 +27,9 @@ import java.nio.charset.Charset;
 import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.HttpServletResponse;
 
+import org.hl7.fhir.instance.model.api.IBaseOperationOutcome;
+import org.hl7.fhir.instance.model.api.IBaseResource;
+
 import ca.uhn.fhir.rest.method.RequestDetails;
 import ca.uhn.fhir.rest.param.ResourceParameter;
 import ca.uhn.fhir.rest.server.EncodingEnum;
@@ -51,6 +54,19 @@ public class RequestValidatingInterceptor extends BaseValidatingInterceptor<Stri
 
 	private static final org.slf4j.Logger ourLog = org.slf4j.LoggerFactory.getLogger(RequestValidatingInterceptor.class);
 
+	/**
+	 * A {@link RequestDetails#getUserData() user data} entry will be created with this
+	 * key which contains the {@link ValidationResult} from validating the request.
+	 */
+	public static final String REQUEST_VALIDATION_RESULT = RequestValidatingInterceptor.class.getName() + "_REQUEST_VALIDATION_RESULT";
+
+	private boolean myAddValidationResultsToResponseOperationOutcome = true;
+
+	@Override
+	ValidationResult doValidate(FhirValidator theValidator, String theRequest) {
+		return theValidator.validateWithResult(theRequest);
+	}
+
 	@Override
 	public boolean incomingRequestPostProcessed(RequestDetails theRequestDetails, HttpServletRequest theRequest, HttpServletResponse theResponse) throws AuthenticationException {
 		EncodingEnum encoding = RestfulServerUtils.determineRequestEncodingNoDefault(theRequestDetails);
@@ -67,11 +83,59 @@ public class RequestValidatingInterceptor extends BaseValidatingInterceptor<Stri
 			return true;
 		}
 
-		validate(requestText, theRequestDetails);
+		ValidationResult validationResult = validate(requestText, theRequestDetails);
+
+		// The JPA server will use this
+		theRequestDetails.getUserData().put(REQUEST_VALIDATION_RESULT, validationResult);
 
 		return true;
 	}
 
+	/**
+	 * If set to {@literal true} (default is true), the validation results
+	 * will be added to the OperationOutcome being returned to the client,
+	 * unless the response being returned is not an OperationOutcome
+	 * to begin with (e.g. if the client has requested 
+	 * <code>Return: prefer=representation</code>)
+	 */
+	public boolean isAddValidationResultsToResponseOperationOutcome() {
+		return myAddValidationResultsToResponseOperationOutcome;
+	}
+
+	@Override
+	public boolean outgoingResponse(RequestDetails theRequestDetails, IBaseResource theResponseObject) {
+		if (myAddValidationResultsToResponseOperationOutcome) {
+			if (theResponseObject instanceof IBaseOperationOutcome) {
+				IBaseOperationOutcome oo = (IBaseOperationOutcome) theResponseObject;
+
+				if (theRequestDetails != null) {
+					ValidationResult validationResult = (ValidationResult) theRequestDetails.getUserData().get(RequestValidatingInterceptor.REQUEST_VALIDATION_RESULT);
+					if (validationResult != null) {
+						validationResult.populateOperationOutcome(oo);
+					}
+				}
+
+			}
+		}
+
+		return true;
+	}
+
+	@Override
+	String provideDefaultResponseHeaderName() {
+		return DEFAULT_RESPONSE_HEADER_NAME;
+	}
+
+	/**
+	 * If set to {@literal true} (default is true), the validation results
+	 * will be added to the OperationOutcome being returned to the client,
+	 * unless the response being returned is not an OperationOutcome
+	 * to begin with (e.g. if the client has requested 
+	 * <code>Return: prefer=representation</code>)
+	 */
+	public void setAddValidationResultsToResponseOperationOutcome(boolean theAddValidationResultsToResponseOperationOutcome) {
+		myAddValidationResultsToResponseOperationOutcome = theAddValidationResultsToResponseOperationOutcome;
+	}
 
 	/**
 	 * Sets the name of the response header to add validation failures to
@@ -84,17 +148,4 @@ public class RequestValidatingInterceptor extends BaseValidatingInterceptor<Stri
 		super.setResponseHeaderName(theResponseHeaderName);
 	}
 
-
-	@Override
-	String provideDefaultResponseHeaderName() {
-		return DEFAULT_RESPONSE_HEADER_NAME;
-	}
-
-
-	@Override
-	ValidationResult doValidate(FhirValidator theValidator, String theRequest) {
-		return theValidator.validateWithResult(theRequest);
-	}
-
-
 }

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

@@ -102,6 +102,14 @@ public class ValidationResult {
 	 */
 	public IBaseOperationOutcome toOperationOutcome() {
 		IBaseOperationOutcome oo = (IBaseOperationOutcome) myCtx.getResourceDefinition("OperationOutcome").newInstance();
+		populateOperationOutcome(oo);
+		return oo;
+	}
+
+	/**
+	 * Populate an operation outcome with the results of the validation 
+	 */
+	public void populateOperationOutcome(IBaseOperationOutcome theOperationOutcome) {
 		for (SingleValidationMessage next : myMessages) {
 			String location;
 			if (isNotBlank(next.getLocationString())) {
@@ -112,15 +120,13 @@ public class ValidationResult {
 				location = null;
 			}
 			String severity = next.getSeverity() != null ? next.getSeverity().getCode() : null;
-			OperationOutcomeUtil.addIssue(myCtx, oo, severity, next.getMessage(), location, ExceptionHandlingInterceptor.PROCESSING);
+			OperationOutcomeUtil.addIssue(myCtx, theOperationOutcome, severity, next.getMessage(), location, ExceptionHandlingInterceptor.PROCESSING);
 		}
 
 		if (myMessages.isEmpty()) {
 			String message = myCtx.getLocalizer().getMessage(ValidationResult.class, "noIssuesDetected");
-			OperationOutcomeUtil.addIssue(myCtx, oo, "information", message, null, "informational");
+			OperationOutcomeUtil.addIssue(myCtx, theOperationOutcome, "information", message, null, "informational");
 		}
-
-		return oo;
 	}
 
 	@Override

hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/dstu3/ResourceProviderDstu3Test.java

@@ -138,6 +138,7 @@ import ca.uhn.fhir.rest.server.exceptions.InvalidRequestException;
 import ca.uhn.fhir.rest.server.exceptions.PreconditionFailedException;
 import ca.uhn.fhir.rest.server.exceptions.ResourceGoneException;
 import ca.uhn.fhir.rest.server.exceptions.UnprocessableEntityException;
+import ca.uhn.fhir.rest.server.interceptor.RequestValidatingInterceptor;
 import ca.uhn.fhir.util.TestUtil;
 import ca.uhn.fhir.util.UrlUtil;
 
@@ -402,6 +403,36 @@ public class ResourceProviderDstu3Test extends BaseResourceProviderDstu3Test {
 		ourClient.create().resource(input).execute().getResource();
 	}
 
+	@Test
+	public void testCreateIncludesRequestValidatorInterceptorOutcome() throws IOException {
+		RequestValidatingInterceptor interceptor = new RequestValidatingInterceptor();
+		assertTrue(interceptor.isAddValidationResultsToResponseOperationOutcome());
+		interceptor.setFailOnSeverity(null);
+		
+		ourRestServer.registerInterceptor(interceptor);
+		try {
+			// Missing status, which is mandatory
+			Observation obs = new Observation();
+			obs.addIdentifier().setSystem("urn:foo").setValue("bar");
+			IBaseResource outcome = ourClient.create().resource(obs).execute().getOperationOutcome();
+
+			String encodedOo = myFhirCtx.newJsonParser().setPrettyPrint(true).encodeResourceToString(outcome);
+			ourLog.info(encodedOo);
+			assertThat(encodedOo, containsString("cvc-complex-type.2.4.b"));
+			assertThat(encodedOo, containsString("Successfully created resource \\\"Observation/"));
+			
+			interceptor.setAddValidationResultsToResponseOperationOutcome(false);
+			outcome = ourClient.create().resource(obs).execute().getOperationOutcome();
+			encodedOo = myFhirCtx.newJsonParser().setPrettyPrint(true).encodeResourceToString(outcome);
+			ourLog.info(encodedOo);
+			assertThat(encodedOo, not(containsString("cvc-complex-type.2.4.b")));
+			assertThat(encodedOo, containsString("Successfully created resource \\\"Observation/"));
+
+		} finally {
+			ourRestServer.unregisterInterceptor(interceptor);
+		}
+	}
+
 	@Test
 	public void testCreateConditional() {
 		Patient patient = new Patient();
@@ -3658,7 +3689,8 @@ public class ResourceProviderDstu3Test extends BaseResourceProviderDstu3Test {
 			assertThat(resp, not(containsString("Resource has no id")));
 			assertThat(resp, containsString("<pre>No issues detected during validation</pre>"));
 			assertThat(resp,
-					stringContainsInOrder("<issue>", "<severity value=\"information\"/>", "<code value=\"informational\"/>", "<diagnostics value=\"No issues detected during validation\"/>", "</issue>"));
+					stringContainsInOrder("<issue>", "<severity value=\"information\"/>", "<code value=\"informational\"/>", "<diagnostics value=\"No issues detected during validation\"/>",
+							"</issue>"));
 		} finally {
 			IOUtils.closeQuietly(response.getEntity().getContent());
 			response.close();
@@ -3684,7 +3716,8 @@ public class ResourceProviderDstu3Test extends BaseResourceProviderDstu3Test {
 			assertThat(resp, not(containsString("Resource has no id")));
 			assertThat(resp, containsString("<pre>No issues detected during validation</pre>"));
 			assertThat(resp,
-					stringContainsInOrder("<issue>", "<severity value=\"information\"/>", "<code value=\"informational\"/>", "<diagnostics value=\"No issues detected during validation\"/>", "</issue>"));
+					stringContainsInOrder("<issue>", "<severity value=\"information\"/>", "<code value=\"informational\"/>", "<diagnostics value=\"No issues detected during validation\"/>",
+							"</issue>"));
 		} finally {
 			IOUtils.closeQuietly(response.getEntity().getContent());
 			response.close();

hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/dstu3/ResourceProviderInterceptorDstu3Test.java

@@ -5,6 +5,7 @@ import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertThat;
 import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.reset;
 import static org.mockito.Mockito.times;
 import static org.mockito.Mockito.any;
 import static org.mockito.Mockito.verify;
@@ -13,6 +14,7 @@ import static org.mockito.Mockito.when;
 import java.io.IOException;
 import java.nio.charset.StandardCharsets;
 
+import javax.servlet.ServletException;
 import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.HttpServletResponse;
 
@@ -25,13 +27,17 @@ import org.hl7.fhir.dstu3.model.Bundle;
 import org.hl7.fhir.dstu3.model.Bundle.BundleEntryComponent;
 import org.hl7.fhir.dstu3.model.Bundle.BundleType;
 import org.hl7.fhir.dstu3.model.Bundle.HTTPVerb;
+import org.hl7.fhir.dstu3.model.Organization;
 import org.hl7.fhir.dstu3.model.Patient;
+import org.hl7.fhir.dstu3.model.Reference;
 import org.hl7.fhir.instance.model.api.IBaseResource;
+import org.hl7.fhir.instance.model.api.IIdType;
 import org.junit.After;
 import org.junit.AfterClass;
 import org.junit.Test;
 import org.mockito.ArgumentCaptor;
 
+import ca.uhn.fhir.parser.IParser;
 import ca.uhn.fhir.rest.api.RestOperationTypeEnum;
 import ca.uhn.fhir.rest.method.RequestDetails;
 import ca.uhn.fhir.rest.server.Constants;
@@ -64,11 +70,7 @@ public class ResourceProviderInterceptorDstu3Test extends BaseResourceProviderDs
 		myServerInterceptor = mock(IServerInterceptor.class);
 		myDaoInterceptor = mock(IServerInterceptor.class);
 
-		when(myServerInterceptor.handleException(any(RequestDetails.class), any(BaseServerResponseException.class), any(HttpServletRequest.class), any(HttpServletResponse.class))).thenReturn(true);
+		resetServerInterceptor();
-		when(myServerInterceptor.incomingRequestPostProcessed(any(RequestDetails.class), any(HttpServletRequest.class), any(HttpServletResponse.class))).thenReturn(true);
-		when(myServerInterceptor.incomingRequestPreProcessed(any(HttpServletRequest.class), any(HttpServletResponse.class))).thenReturn(true);
-		when(myServerInterceptor.outgoingResponse(any(RequestDetails.class), any(IBaseResource.class))).thenReturn(true);
-		when(myServerInterceptor.outgoingResponse(any(RequestDetails.class), any(IBaseResource.class), any(HttpServletRequest.class), any(HttpServletResponse.class))).thenReturn(true);
 
 		myDaoConfig.getInterceptors().add(myDaoInterceptor);
 		ourRestServer.registerInterceptor(myServerInterceptor);
@@ -77,10 +79,20 @@ public class ResourceProviderInterceptorDstu3Test extends BaseResourceProviderDs
 			@Override
 			public void incomingRequestPreHandled(RestOperationTypeEnum theOperation, ActionRequestDetails theProcessedRequest) {
 				super.incomingRequestPreHandled(theOperation, theProcessedRequest);
-		 }});
+			}
+		});
 
 	}
 
+	private void resetServerInterceptor() throws ServletException, IOException {
+		reset(myServerInterceptor);
+		when(myServerInterceptor.handleException(any(RequestDetails.class), any(BaseServerResponseException.class), any(HttpServletRequest.class), any(HttpServletResponse.class))).thenReturn(true);
+		when(myServerInterceptor.incomingRequestPostProcessed(any(RequestDetails.class), any(HttpServletRequest.class), any(HttpServletResponse.class))).thenReturn(true);
+		when(myServerInterceptor.incomingRequestPreProcessed(any(HttpServletRequest.class), any(HttpServletResponse.class))).thenReturn(true);
+		when(myServerInterceptor.outgoingResponse(any(RequestDetails.class), any(IBaseResource.class))).thenReturn(true);
+		when(myServerInterceptor.outgoingResponse(any(RequestDetails.class), any(IBaseResource.class), any(HttpServletRequest.class), any(HttpServletResponse.class))).thenReturn(true);
+	}
+
 	@Test
 	public void testCreateResource() throws IOException {
 		String methodName = "testCreateResource";
@@ -117,6 +129,55 @@ public class ResourceProviderInterceptorDstu3Test extends BaseResourceProviderDs
 		assertNotNull(ardCaptor.getValue().getResource());
 	}
 
+	@Test
+	public void testCreateResourceWithVersionedReference() throws IOException, ServletException {
+		String methodName = "testCreateResourceWithVersionedReference";
+
+		Organization org = new Organization();
+		org.setName("orgName");
+		IIdType orgId = ourClient.create().resource(org).execute().getId().toUnqualified();
+		assertNotNull(orgId.getVersionIdPartAsLong());
+
+		resetServerInterceptor();
+		
+		Patient pt = new Patient();
+		pt.addName().setFamily(methodName);
+		pt.setManagingOrganization(new Reference(orgId));
+
+		IParser parser = myFhirCtx.newXmlParser();
+		parser.setDontStripVersionsFromReferencesAtPaths("Patient.managingOrganization");
+		parser.setPrettyPrint(true);
+		String resource = parser.encodeResourceToString(pt);
+
+		ourLog.info(resource);
+
+		HttpPost post = new HttpPost(ourServerBase + "/Patient");
+		post.setEntity(new StringEntity(resource, ContentType.create(Constants.CT_FHIR_XML, "UTF-8")));
+		CloseableHttpResponse response = ourHttpClient.execute(post);
+		try {
+			String resp = IOUtils.toString(response.getEntity().getContent(), StandardCharsets.UTF_8);
+			ourLog.info("Response was: {}", resp);
+			assertEquals(201, response.getStatusLine().getStatusCode());
+			String newIdString = response.getFirstHeader(Constants.HEADER_LOCATION_LC).getValue();
+			assertThat(newIdString, startsWith(ourServerBase + "/Patient/"));
+		} finally {
+			response.close();
+		}
+
+		ArgumentCaptor<ActionRequestDetails> ardCaptor = ArgumentCaptor.forClass(ActionRequestDetails.class);
+		ArgumentCaptor<RestOperationTypeEnum> opTypeCaptor = ArgumentCaptor.forClass(RestOperationTypeEnum.class);
+		verify(myServerInterceptor, times(1)).incomingRequestPreHandled(opTypeCaptor.capture(), ardCaptor.capture());
+
+		assertEquals(RestOperationTypeEnum.CREATE, opTypeCaptor.getValue());
+		assertEquals("Patient", ardCaptor.getValue().getResourceType());
+		assertNotNull(ardCaptor.getValue().getResource());
+
+		Patient patient;
+		patient = (Patient) ardCaptor.getAllValues().get(0).getResource();
+		assertEquals(orgId.getValue(), patient.getManagingOrganization().getReference());
+
+	}
+
 	@Test
 	public void testCreateResourceInTransaction() throws IOException {
 		String methodName = "testCreateResourceInTransaction";

src/changes/changes.xml

@@ -166,10 +166,18 @@
 				JPA server interceptor methods for create/update/delete provided
 				the wrong version ID to the interceptors
 			</action>
+			<action type="add">
+				A post-processing hook for subclasses of BaseValidatingInterceptor is now available.
+			</action>
 			<action type="add" issue="585">
 				AuthorizationInterceptor can now authorize (allow/deny) extended operations
 				on instances and types by wildcard (on any type, or on any instance)
 			</action>
+			<action type="add" issue="595">
+				When RequestValidatingInterceptor is used, the validation results
+				are now populated into the OperationOutcome produced by
+				create and update operations
+			</action>
 		</release>
 		<release version="2.2" date="2016-12-20">
 			<action type="add">