Call startOperation for paging requests (#2370)

This commit is contained in:
James Agnew 2021-02-09 15:47:11 -05:00 committed by GitHub
parent 7f127f75f9
commit dd7a156d19
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
12 changed files with 95 additions and 18 deletions

View File

@ -0,0 +1,5 @@
---
type: fix
issue: 2369
title: "When using the Consent Interceptor, the `startOperation` method was not invoked for search paging requests. This
has been corrected. Thanks to Tue Toft Nørgård for reporting!"

View File

@ -221,12 +221,15 @@ public abstract class BaseMethodBinding<T> {
public abstract Object invokeServer(IRestfulServer<?> theServer, RequestDetails theRequest) throws BaseServerResponseException, IOException;
protected final Object invokeServerMethod(IRestfulServer<?> theServer, RequestDetails theRequest, Object[] theMethodParams) {
protected final Object invokeServerMethod(RequestDetails theRequest, Object[] theMethodParams) {
// Handle server action interceptors
RestOperationTypeEnum operationType = getRestOperationType(theRequest);
if (operationType != null) {
ActionRequestDetails details = new ActionRequestDetails(theRequest);
populateActionRequestDetailsForInterceptor(theRequest, details, theMethodParams);
// Interceptor invoke: SERVER_INCOMING_REQUEST_PRE_HANDLED
HookParams preHandledParams = new HookParams();
preHandledParams.add(RestOperationTypeEnum.class, theRequest.getRestOperationType());
preHandledParams.add(RequestDetails.class, theRequest);
@ -237,6 +240,7 @@ public abstract class BaseMethodBinding<T> {
.getInterceptorBroadcaster()
.callHooks(Pointcut.SERVER_INCOMING_REQUEST_PRE_HANDLED, preHandledParams);
}
}
// Actually invoke the method
@ -268,7 +272,7 @@ public abstract class BaseMethodBinding<T> {
}
/**
* Subclasses may override this method (but should also call super.{@link #populateActionRequestDetailsForInterceptor(RequestDetails, ActionRequestDetails, Object[])} to provide method specifics to the
* Subclasses may override this method (but should also call super) to provide method specifics to the
* interceptors.
*
* @param theRequestDetails The server request details

View File

@ -151,7 +151,7 @@ abstract class BaseOutcomeReturningMethodBinding extends BaseMethodBinding<Metho
* on them
*/
MethodOutcome response;
Object methodReturn = invokeServerMethod(theServer, theRequest, params);
Object methodReturn = invokeServerMethod(theRequest, params);
if (methodReturn instanceof IBaseOperationOutcome) {
response = new MethodOutcome();

View File

@ -39,7 +39,6 @@ import ca.uhn.fhir.rest.server.exceptions.MethodNotAllowedException;
import ca.uhn.fhir.rest.server.interceptor.IServerInterceptor;
import ca.uhn.fhir.rest.server.servlet.ServletRequestDetails;
import org.hl7.fhir.instance.model.api.IBaseConformance;
import org.hl7.fhir.instance.model.api.IBaseResource;
import javax.annotation.Nonnull;
import java.lang.reflect.Method;
@ -142,7 +141,7 @@ public class ConformanceMethodBinding extends BaseResourceReturningMethodBinding
}
if (conf == null) {
conf = (IBaseConformance) invokeServerMethod(theServer, theRequest, theMethodParams);
conf = (IBaseConformance) invokeServerMethod(theRequest, theMethodParams);
if (myCacheMillis > 0) {
// Interceptor hook: SERVER_CAPABILITY_STATEMENT_GENERATED

View File

@ -101,7 +101,7 @@ public class GraphQLMethodBinding extends BaseMethodBinding<String> {
methodParams[myIdParamIndex] = theRequest.getId();
}
String responseString = (String) invokeServerMethod(theServer, theRequest, methodParams);
String responseString = (String) invokeServerMethod(theRequest, methodParams);
int statusCode = Constants.STATUS_HTTP_200_OK;
String statusMessage = Constants.HTTP_STATUS_NAMES.get(statusCode);

View File

@ -144,7 +144,7 @@ public class HistoryMethodBinding extends BaseResourceReturningMethodBinding {
theMethodParams[myIdParamIndex] = theRequest.getId();
}
Object response = invokeServerMethod(theServer, theRequest, theMethodParams);
Object response = invokeServerMethod(theRequest, theMethodParams);
final IBundleProvider resources = toResourceList(response);

View File

@ -326,7 +326,7 @@ public class OperationMethodBinding extends BaseResourceReturningMethodBinding {
theMethodParams[myIdParamIndex] = theRequest.getId();
}
Object response = invokeServerMethod(theServer, theRequest, theMethodParams);
Object response = invokeServerMethod(theRequest, theMethodParams);
if (myManualResponseMode) {
return null;
}

View File

@ -21,6 +21,8 @@ package ca.uhn.fhir.rest.server.method;
*/
import ca.uhn.fhir.context.FhirContext;
import ca.uhn.fhir.interceptor.api.HookParams;
import ca.uhn.fhir.interceptor.api.Pointcut;
import ca.uhn.fhir.model.api.Include;
import ca.uhn.fhir.model.valueset.BundleTypeEnum;
import ca.uhn.fhir.rest.api.Constants;
@ -36,6 +38,9 @@ import ca.uhn.fhir.rest.server.RestfulServerUtils.ResponseEncoding;
import ca.uhn.fhir.rest.server.exceptions.InternalErrorException;
import ca.uhn.fhir.rest.server.exceptions.InvalidRequestException;
import ca.uhn.fhir.rest.server.exceptions.ResourceGoneException;
import ca.uhn.fhir.rest.server.interceptor.IServerInterceptor;
import ca.uhn.fhir.rest.server.servlet.ServletRequestDetails;
import ca.uhn.fhir.util.ReflectionUtil;
import org.hl7.fhir.instance.model.api.IBaseResource;
import javax.annotation.Nonnull;
@ -84,6 +89,20 @@ public class PageMethodBinding extends BaseResourceReturningMethodBinding {
throw new InvalidRequestException("This server does not support paging");
}
// Interceptor invoke: SERVER_INCOMING_REQUEST_PRE_HANDLED
IServerInterceptor.ActionRequestDetails details = new IServerInterceptor.ActionRequestDetails(theRequest);
populateActionRequestDetailsForInterceptor(theRequest, details, ReflectionUtil.EMPTY_OBJECT_ARRAY);
HookParams preHandledParams = new HookParams();
preHandledParams.add(RestOperationTypeEnum.class, theRequest.getRestOperationType());
preHandledParams.add(RequestDetails.class, theRequest);
preHandledParams.addIfMatchesType(ServletRequestDetails.class, theRequest);
preHandledParams.add(IServerInterceptor.ActionRequestDetails.class, details);
if (theRequest.getInterceptorBroadcaster() != null) {
theRequest
.getInterceptorBroadcaster()
.callHooks(Pointcut.SERVER_INCOMING_REQUEST_PRE_HANDLED, preHandledParams);
}
Integer offsetI;
int start = 0;
IBundleProvider bundleProvider;

View File

@ -173,7 +173,7 @@ public class ReadMethodBinding extends BaseResourceReturningMethodBinding {
theMethodParams[myIdIndex] = ParameterUtil.convertIdToType(requestId, myIdParameterType);
Object response = invokeServerMethod(theServer, theRequest, theMethodParams);
Object response = invokeServerMethod(theRequest, theMethodParams);
IBundleProvider retVal = toResourceList(response);

View File

@ -277,7 +277,7 @@ public class SearchMethodBinding extends BaseResourceReturningMethodBinding {
theMethodParams[myIdParamIndex] = theRequest.getId();
}
Object response = invokeServerMethod(theServer, theRequest, theMethodParams);
Object response = invokeServerMethod(theRequest, theMethodParams);
return toResourceList(response);

View File

@ -112,12 +112,12 @@ public class TransactionMethodBinding extends BaseResourceReturningMethodBinding
*/
if (myTransactionParamStyle == ParamStyle.RESOURCE_BUNDLE) {
// This is the DSTU2 style
Object response = invokeServerMethod(theServer, theRequest, theMethodParams);
Object response = invokeServerMethod(theRequest, theMethodParams);
return response;
}
// Call the server implementation method
Object response = invokeServerMethod(theServer, theRequest, theMethodParams);
Object response = invokeServerMethod(theRequest, theMethodParams);
IBundleProvider retVal = toResourceList(response);
/*

View File

@ -8,6 +8,7 @@ import ca.uhn.fhir.rest.api.Constants;
import ca.uhn.fhir.rest.api.server.RequestDetails;
import ca.uhn.fhir.rest.client.api.IGenericClient;
import ca.uhn.fhir.rest.param.StringParam;
import ca.uhn.fhir.rest.server.FifoMemoryPagingProvider;
import ca.uhn.fhir.rest.server.RestfulServer;
import ca.uhn.fhir.rest.server.exceptions.BaseServerResponseException;
import ca.uhn.fhir.rest.server.interceptor.consent.ConsentInterceptor;
@ -85,6 +86,7 @@ public class ConsentInterceptorTest {
public void before() {
myInterceptor = new ConsentInterceptor(myConsentSvc);
ourServlet.registerInterceptor(myInterceptor);
ourServlet.setPagingProvider(new FifoMemoryPagingProvider(10));
ourPatientProvider.clear();
}
@ -162,7 +164,7 @@ public class ConsentInterceptorTest {
}
@Test
public void testSeeResourceAuthorizesOuterBundle() throws IOException {
public void testSearch_SeeResourceAuthorizesOuterBundle() throws IOException {
ourPatientProvider.store((Patient) new Patient().setActive(true).setId("PTA"));
ourPatientProvider.store((Patient) new Patient().setActive(false).setId("PTB"));
@ -189,7 +191,7 @@ public class ConsentInterceptorTest {
@Test
public void testSeeResourceRejectsOuterBundle_ProvidesOperationOutcome() throws IOException {
public void testSearch_SeeResourceRejectsOuterBundle_ProvidesOperationOutcome() throws IOException {
ourPatientProvider.store((Patient) new Patient().setActive(true).setId("PTA"));
ourPatientProvider.store((Patient) new Patient().setActive(false).setId("PTB"));
@ -220,7 +222,7 @@ public class ConsentInterceptorTest {
}
@Test
public void testSeeResourceRejectsOuterBundle_ProvidesNothing() throws IOException {
public void testSearch_SeeResourceRejectsOuterBundle_ProvidesNothing() throws IOException {
ourPatientProvider.store((Patient) new Patient().setActive(true).setId("PTA"));
ourPatientProvider.store((Patient) new Patient().setActive(false).setId("PTB"));
@ -247,7 +249,7 @@ public class ConsentInterceptorTest {
}
@Test
public void testSeeResourceRejectsInnerResource_ProvidesOperationOutcome() throws IOException {
public void testSearch_SeeResourceRejectsInnerResource_ProvidesOperationOutcome() throws IOException {
ourPatientProvider.store((Patient) new Patient().setActive(true).setId("PTA"));
ourPatientProvider.store((Patient) new Patient().setActive(false).setId("PTB"));
@ -286,7 +288,7 @@ public class ConsentInterceptorTest {
}
@Test
public void testSeeResourceReplacesInnerResource() throws IOException {
public void testSearch_SeeResourceReplacesInnerResource() throws IOException {
ourPatientProvider.store((Patient) new Patient().setActive(true).setId("PTA"));
ourPatientProvider.store((Patient) new Patient().setActive(false).setId("PTB"));
@ -326,7 +328,7 @@ public class ConsentInterceptorTest {
}
@Test
public void testSeeResourceModifiesInnerResource() throws IOException {
public void testSearch_SeeResourceModifiesInnerResource() throws IOException {
ourPatientProvider.store((Patient) new Patient().setActive(true).setId("PTA"));
ourPatientProvider.store((Patient) new Patient().setActive(false).setId("PTB"));
@ -362,6 +364,54 @@ public class ConsentInterceptorTest {
verifyNoMoreInteractions(myConsentSvc);
}
@Test
public void testPage_SeeResourceReplacesInnerResource() throws IOException {
ourPatientProvider.store((Patient) new Patient().setActive(true).setId("PTA"));
ourPatientProvider.store((Patient) new Patient().setActive(false).setId("PTB"));
when(myConsentSvc.startOperation(any(), any())).thenReturn(ConsentOutcome.PROCEED);
when(myConsentSvc.canSeeResource(any(), any(), any())).thenReturn(ConsentOutcome.PROCEED);
when(myConsentSvc.willSeeResource(any(RequestDetails.class), any(IBaseResource.class), any())).thenReturn(ConsentOutcome.PROCEED);
String nextPageLink;
HttpGet httpGet = new HttpGet("http://localhost:" + ourPort + "/Patient?_count=1");
try (CloseableHttpResponse status = ourClient.execute(httpGet)) {
assertEquals(200, status.getStatusLine().getStatusCode());
String responseContent = IOUtils.toString(status.getEntity().getContent(), Charsets.UTF_8);
ourLog.info("Response: {}", responseContent);
Bundle bundle = ourCtx.newJsonParser().parseResource(Bundle.class, responseContent);
nextPageLink = bundle.getLink(Constants.LINK_NEXT).getUrl();
}
// Now perform a page request
reset(myConsentSvc);
when(myConsentSvc.startOperation(any(), any())).thenReturn(ConsentOutcome.PROCEED);
when(myConsentSvc.willSeeResource(any(RequestDetails.class), any(IBaseResource.class), any())).thenAnswer(t->{
IBaseResource resource = (IBaseResource) t.getArguments()[1];
if (resource.getIdElement().getIdPart().equals("PTB")) {
Patient replacement = new Patient();
replacement.setId("PTB");
replacement.addIdentifier().setSystem("REPLACEMENT");
return new ConsentOutcome(ConsentOperationStatusEnum.PROCEED, replacement);
}
return ConsentOutcome.PROCEED;
});
httpGet = new HttpGet(nextPageLink);
try (CloseableHttpResponse status = ourClient.execute(httpGet)) {
assertEquals(200, status.getStatusLine().getStatusCode());
String responseContent = IOUtils.toString(status.getEntity().getContent(), Charsets.UTF_8);
ourLog.info("Response: {}", responseContent);
Bundle response = ourCtx.newJsonParser().parseResource(Bundle.class, responseContent);
assertEquals(Patient.class, response.getEntry().get(0).getResource().getClass());
assertEquals("PTB", response.getEntry().get(0).getResource().getIdElement().getIdPart());
assertEquals("REPLACEMENT", ((Patient)response.getEntry().get(0).getResource()).getIdentifierFirstRep().getSystem());
}
verify(myConsentSvc, times(1)).startOperation(any(), any());
}
@Test
public void testOutcomeException() throws IOException {
when(myConsentSvc.startOperation(any(), any())).thenReturn(ConsentOutcome.PROCEED);