From 0be818c31cec55d82eb9e161cf6e17a50f3fe502 Mon Sep 17 00:00:00 2001 From: James Agnew Date: Thu, 16 Mar 2017 13:31:25 -0400 Subject: [PATCH] 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. --- .../BaseValidatingInterceptor.java | 13 ++- .../RequestValidatingInterceptor.java | 81 +++++++++++++---- .../uhn/fhir/validation/ValidationResult.java | 14 ++- .../dstu3/ResourceProviderDstu3Test.java | 39 +++++++- .../ResourceProviderInterceptorDstu3Test.java | 89 ++++++++++++++++--- src/changes/changes.xml | 8 ++ 6 files changed, 204 insertions(+), 40 deletions(-) diff --git a/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/server/interceptor/BaseValidatingInterceptor.java b/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/server/interceptor/BaseValidatingInterceptor.java index 1d9f11dbfc9..e6170c7e00d 100644 --- a/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/server/interceptor/BaseValidatingInterceptor.java +++ b/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/server/interceptor/BaseValidatingInterceptor.java @@ -267,7 +267,10 @@ abstract class BaseValidatingInterceptor 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 extends InterceptorAdapter { } if (theRequest == null) { - return; + return null; } ValidationResult validationResult; @@ -285,7 +288,7 @@ abstract class BaseValidatingInterceptor 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 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 extends InterceptorAdapter { } postProcessResult(theRequestDetails, validationResult); + + return validationResult; } private static class MyLookup extends StrLookup { diff --git a/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/server/interceptor/RequestValidatingInterceptor.java b/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/server/interceptor/RequestValidatingInterceptor.java index d41b0aabed1..760eb19645a 100644 --- a/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/server/interceptor/RequestValidatingInterceptor.java +++ b/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/server/interceptor/RequestValidatingInterceptor.java @@ -12,7 +12,7 @@ import static org.apache.commons.lang3.StringUtils.isBlank; * 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, @@ -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 BaseValidatingInterceptorReturn: prefer=representation) + */ + 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 + * Return: prefer=representation) + */ + 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 BaseValidatingInterceptorNo issues detected during validation")); assertThat(resp, - stringContainsInOrder("", "", "", "", "")); + stringContainsInOrder("", "", "", "", + "")); } 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("
No issues detected during validation
")); assertThat(resp, - stringContainsInOrder("", "", "", "", "")); + stringContainsInOrder("", "", "", "", + "")); } finally { IOUtils.closeQuietly(response.getEntity().getContent()); response.close(); diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/dstu3/ResourceProviderInterceptorDstu3Test.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/dstu3/ResourceProviderInterceptorDstu3Test.java index 839c867212a..59124b6b519 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/dstu3/ResourceProviderInterceptorDstu3Test.java +++ b/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,21 +70,27 @@ public class ResourceProviderInterceptorDstu3Test extends BaseResourceProviderDs myServerInterceptor = mock(IServerInterceptor.class); myDaoInterceptor = mock(IServerInterceptor.class); + resetServerInterceptor(); + + myDaoConfig.getInterceptors().add(myDaoInterceptor); + ourRestServer.registerInterceptor(myServerInterceptor); + + ourRestServer.registerInterceptor(new InterceptorAdapter() { + @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); - - myDaoConfig.getInterceptors().add(myDaoInterceptor); - ourRestServer.registerInterceptor(myServerInterceptor); - - ourRestServer.registerInterceptor(new InterceptorAdapter() { - @Override - public void incomingRequestPreHandled(RestOperationTypeEnum theOperation, ActionRequestDetails theProcessedRequest) { - super.incomingRequestPreHandled(theOperation, theProcessedRequest); - }}); - } @Test @@ -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 ardCaptor = ArgumentCaptor.forClass(ActionRequestDetails.class); + ArgumentCaptor 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"; @@ -144,7 +205,7 @@ public class ResourceProviderInterceptorDstu3Test extends BaseResourceProviderDs } /* - * Server Interceptor + * Server Interceptor */ ArgumentCaptor ardCaptor = ArgumentCaptor.forClass(ActionRequestDetails.class); @@ -163,9 +224,9 @@ public class ResourceProviderInterceptorDstu3Test extends BaseResourceProviderDs verify(myServerInterceptor, times(1)).incomingRequestPostProcessed(rdCaptor.capture(), srCaptor.capture(), sRespCaptor.capture()); /* - * DAO Interceptor + * DAO Interceptor */ - + ardCaptor = ArgumentCaptor.forClass(ActionRequestDetails.class); opTypeCaptor = ArgumentCaptor.forClass(RestOperationTypeEnum.class); verify(myDaoInterceptor, times(2)).incomingRequestPreHandled(opTypeCaptor.capture(), ardCaptor.capture()); @@ -175,7 +236,7 @@ public class ResourceProviderInterceptorDstu3Test extends BaseResourceProviderDs assertEquals(RestOperationTypeEnum.CREATE, opTypeCaptor.getAllValues().get(1)); assertEquals("Patient", ardCaptor.getAllValues().get(1).getResourceType()); assertNotNull(ardCaptor.getAllValues().get(1).getResource()); - + rdCaptor = ArgumentCaptor.forClass(RequestDetails.class); srCaptor = ArgumentCaptor.forClass(HttpServletRequest.class); sRespCaptor = ArgumentCaptor.forClass(HttpServletResponse.class); diff --git a/src/changes/changes.xml b/src/changes/changes.xml index d0d475e21a7..fb449251b35 100644 --- a/src/changes/changes.xml +++ b/src/changes/changes.xml @@ -166,10 +166,18 @@ JPA server interceptor methods for create/update/delete provided the wrong version ID to the interceptors + + A post-processing hook for subclasses of BaseValidatingInterceptor is now available. + AuthorizationInterceptor can now authorize (allow/deny) extended operations on instances and types by wildcard (on any type, or on any instance) + + When RequestValidatingInterceptor is used, the validation results + are now populated into the OperationOutcome produced by + create and update operations +