SERVER_OUTGOING_RESPONSE invocation on manualResponse operations (#3315)

* Fix implementation so we can make the call anyhow

* modify test to ensure that pointcut is invoked

* Add changelog

* Tighten tests

* Add comment
This commit is contained in:
Tadgh 2022-01-19 08:24:59 -08:00 committed by GitHub
parent 80e4c3dd92
commit 71e959c1be
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 68 additions and 23 deletions

View File

@ -0,0 +1,6 @@
---
type: fix
issue: 3314
jira: SMILE-3464
title: "Previously, `$binary-access-read` and other operations that return neither method outcomes nor resources were causing no invocation of the `SERVER_OUTGOING_RESPONSE` pointcut. This has been corrected, and those
operations will now correctly invoke `SERVER_OUTGOING_RESPONSE`."

View File

@ -9,10 +9,13 @@ import ca.uhn.fhir.jpa.binstore.MemoryBinaryStorageSvcImpl;
import ca.uhn.fhir.jpa.interceptor.UserRequestRetryVersionConflictsInterceptor;
import ca.uhn.fhir.jpa.model.util.JpaConstants;
import ca.uhn.fhir.rest.api.Constants;
import ca.uhn.fhir.rest.api.server.RequestDetails;
import ca.uhn.fhir.rest.api.server.ResponseDetails;
import ca.uhn.fhir.rest.server.exceptions.InvalidRequestException;
import ca.uhn.fhir.rest.server.exceptions.ResourceGoneException;
import ca.uhn.fhir.rest.server.provider.ProviderConstants;
import ca.uhn.fhir.util.HapiExtensions;
import ca.uhn.test.concurrency.PointcutLatch;
import com.google.common.base.Charsets;
import org.apache.commons.io.IOUtils;
import org.apache.http.client.methods.CloseableHttpResponse;
@ -20,6 +23,7 @@ import org.apache.http.client.methods.HttpGet;
import org.apache.http.client.methods.HttpPost;
import org.apache.http.entity.ByteArrayEntity;
import org.apache.http.entity.ContentType;
import org.hl7.fhir.instance.model.api.IBaseResource;
import org.hl7.fhir.instance.model.api.IIdType;
import org.hl7.fhir.r4.model.Attachment;
import org.hl7.fhir.r4.model.Binary;
@ -37,14 +41,20 @@ import org.springframework.beans.factory.annotation.Autowired;
import java.io.ByteArrayOutputStream;
import java.io.IOException;
import java.util.List;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.containsString;
import static org.hamcrest.Matchers.equalTo;
import static org.hamcrest.Matchers.is;
import static org.hamcrest.Matchers.hasSize;
import static org.hamcrest.Matchers.matchesPattern;
import static org.hamcrest.Matchers.notNullValue;
import static org.junit.jupiter.api.Assertions.assertArrayEquals;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.fail;
import static org.mockito.ArgumentMatchers.notNull;
import static org.mockito.Mockito.any;
import static org.mockito.Mockito.doAnswer;
import static org.mockito.Mockito.eq;
@ -83,11 +93,12 @@ public class BinaryAccessProviderR4Test extends BaseResourceProviderR4Test {
}
@Test
public void testRead() throws IOException {
public void testRead() throws IOException, InterruptedException {
IIdType id = createDocumentReference(true);
IAnonymousInterceptor interceptor = mock(IAnonymousInterceptor.class);
myInterceptorRegistry.registerAnonymousInterceptor(Pointcut.STORAGE_PRESHOW_RESOURCES, interceptor);
doAnswer(t -> {
Pointcut pointcut = t.getArgument(0, Pointcut.class);
HookParams params = t.getArgument(1, HookParams.class);
@ -95,6 +106,7 @@ public class BinaryAccessProviderR4Test extends BaseResourceProviderR4Test {
return null;
}).when(interceptor).invoke(any(), any());
// Read it back using the operation
String path = ourServerBase +
@ -102,18 +114,18 @@ public class BinaryAccessProviderR4Test extends BaseResourceProviderR4Test {
JpaConstants.OPERATION_BINARY_ACCESS_READ +
"?path=DocumentReference.content.attachment";
HttpGet get = new HttpGet(path);
try (CloseableHttpResponse resp = ourHttpClient.execute(get)) {
try (CloseableHttpResponse resp = ourHttpClient.execute(get)) {
assertEquals(200, resp.getStatusLine().getStatusCode());
assertEquals("image/png", resp.getEntity().getContentType().getValue());
assertEquals(SOME_BYTES.length, resp.getEntity().getContentLength());
byte[] actualBytes = IOUtils.toByteArray(resp.getEntity().getContent());
assertArrayEquals(SOME_BYTES, actualBytes);
}
verify(interceptor, times(1)).invoke(eq(Pointcut.STORAGE_PRESHOW_RESOURCES), any());
}
@ -125,7 +137,6 @@ public class BinaryAccessProviderR4Test extends BaseResourceProviderR4Test {
myInterceptorRegistry.registerAnonymousInterceptor(Pointcut.STORAGE_PRESHOW_RESOURCES, interceptor);
// Read it back using the operation
String path = ourServerBase +
"/DocumentReference/" + id.getIdPart() + "/" +
JpaConstants.OPERATION_BINARY_ACCESS_READ +
@ -163,7 +174,6 @@ public class BinaryAccessProviderR4Test extends BaseResourceProviderR4Test {
}
@Test
public void testReadNoData() throws IOException {
IIdType id = createDocumentReference(false);
@ -181,9 +191,37 @@ public class BinaryAccessProviderR4Test extends BaseResourceProviderR4Test {
assertThat(response, matchesPattern(".*The resource with ID DocumentReference/[0-9]+ has no data at path.*"));
}
}
@Test
public void testManualResponseOperationsInvokeServerOutgoingResponsePointcut() throws IOException, InterruptedException {
IIdType id = createDocumentReference(true);
PointcutLatch latch = new PointcutLatch(Pointcut.SERVER_OUTGOING_RESPONSE);
ourRestServer.getInterceptorService().registerAnonymousInterceptor(Pointcut.SERVER_OUTGOING_RESPONSE, latch);
String path = ourServerBase +
"/DocumentReference/" + id.getIdPart() + "/" +
JpaConstants.OPERATION_BINARY_ACCESS_READ +
"?path=DocumentReference.content.attachment";
HttpGet get = new HttpGet(path);
latch.setExpectedCount(1);
try (CloseableHttpResponse resp = ourHttpClient.execute(get)) {
assertEquals(200, resp.getStatusLine().getStatusCode());
latch.awaitExpected();
RequestDetails requestDetails = latch.getLatchInvocationParameterOfType(RequestDetails.class);
ResponseDetails responseDetails= latch.getLatchInvocationParameterOfType(ResponseDetails.class);
assertThat(responseDetails, is(notNullValue()));
assertThat(requestDetails, is(notNullValue()));
assertThat(requestDetails.getId().toString(), is(equalTo(id.toString())));
}
}
@Test
public void testReadUnknownBlobId() throws IOException {
IIdType id = createDocumentReference(false);

View File

@ -430,26 +430,28 @@ public abstract class BaseResourceReturningMethodBinding extends BaseMethodBindi
@Override
public Object invokeServer(IRestfulServer<?> theServer, RequestDetails theRequest) throws BaseServerResponseException, IOException {
IBaseResource response = doInvokeServer(theServer, theRequest);
/*
When we write directly to an HttpServletResponse, the invocation returns null. However, we still want to invoke
the SERVER_OUTGOING_RESPONSE pointcut.
*/
if (response == null) {
ResponseDetails responseDetails = new ResponseDetails();
responseDetails.setResponseCode(Constants.STATUS_HTTP_200_OK);
callOutgoingResponseHook(theRequest, responseDetails);
return null;
}
} else {
Set<SummaryEnum> summaryMode = RestfulServerUtils.determineSummaryMode(theRequest);
ResponseDetails responseDetails = new ResponseDetails();
responseDetails.setResponseResource(response);
responseDetails.setResponseCode(Constants.STATUS_HTTP_200_OK);
if (!callOutgoingResponseHook(theRequest, responseDetails)) {
return null;
}
boolean prettyPrint = RestfulServerUtils.prettyPrintResponse(theServer, theRequest);
return theRequest.getResponse().streamResponseAsResource(responseDetails.getResponseResource(), prettyPrint, summaryMode, responseDetails.getResponseCode(), null, theRequest.isRespondGzip(), isAddContentLocationHeader());
}
}
public abstract Object invokeServer(IRestfulServer<?> theServer, RequestDetails theRequest, Object[] theMethodParams) throws InvalidRequestException, InternalErrorException;

View File

@ -188,7 +188,6 @@ public class PointcutLatch implements IAnonymousInterceptor, IPointcutLatch {
return retVal + " ]";
}
@Override
public void invoke(IPointcut thePointcut, HookParams theArgs) {
myLastInvoke.set(System.currentTimeMillis());