fix interceptor hooks from requestDetails not getting called for STORAGE_PRECHECK_FOR_CACHED_SEARCH (#6436)

* fix interceptor hooks from requestDetails not getting called for STORAGE_PRECHECK_FOR_CACHED_SEARCH

* added unit tests and updated changelog

* added one more test case
This commit is contained in:
Emre Dincturk 2024-11-06 15:50:52 -05:00 committed by GitHub
parent 57f40762c1
commit 6c4c6bde2c
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
6 changed files with 178 additions and 3 deletions

View File

@ -0,0 +1,8 @@
---
type: fix
issue: 6440
title: "Previously, if an `IInterceptorBroadcaster` was set in a `RequestDetails` object,
`STORAGE_PRECHECK_FOR_CACHED_SEARCH` hooks that were registered to that `IInterceptorBroadcaster` were not
called. Also, if an `IInterceptorBroadcaster` was set in the `RequestDetails` object, the boolean return value of the hooks
registered to that `IInterceptorBroadcaster` were not taken into account. This second issue existed for all pointcuts
that returned a boolean type, not just for `STORAGE_PRECHECK_FOR_CACHED_SEARCH`. These issues have now been fixed."

View File

@ -600,12 +600,12 @@ public class SearchCoordinatorSvcImpl implements ISearchCoordinatorSvc<JpaPid> {
.add(SearchParameterMap.class, theParams)
.add(RequestDetails.class, theRequestDetails)
.addIfMatchesType(ServletRequestDetails.class, theRequestDetails);
Object outcome = CompositeInterceptorBroadcaster.doCallHooksAndReturnObject(
boolean canUseCache = CompositeInterceptorBroadcaster.doCallHooks(
myInterceptorBroadcaster,
theRequestDetails,
Pointcut.STORAGE_PRECHECK_FOR_CACHED_SEARCH,
params);
if (Boolean.FALSE.equals(outcome)) {
if (!canUseCache) {
return null;
}

View File

@ -1,6 +1,7 @@
package ca.uhn.fhir.jpa.mdm.helper;
import ca.uhn.fhir.context.FhirContext;
import ca.uhn.fhir.interceptor.api.HookParams;
import ca.uhn.fhir.interceptor.api.IInterceptorBroadcaster;
import ca.uhn.fhir.interceptor.api.IInterceptorService;
import ca.uhn.fhir.interceptor.api.Pointcut;
@ -23,6 +24,7 @@ import org.springframework.beans.factory.annotation.Autowired;
import java.util.function.Supplier;
import static org.awaitility.Awaitility.await;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.Mockito.when;
/**
@ -78,6 +80,7 @@ public abstract class BaseMdmHelper implements BeforeEachCallback, AfterEachCall
//they are coming from an external HTTP Request.
MockitoAnnotations.initMocks(this);
when(myMockSrd.getInterceptorBroadcaster()).thenReturn(myMockInterceptorBroadcaster);
when(myMockInterceptorBroadcaster.callHooks(any(Pointcut.class), any(HookParams.class))).thenReturn(true);
when(myMockSrd.getServletRequest()).thenReturn(myMockServletRequest);
when(myMockSrd.getServer()).thenReturn(myMockRestfulServer);
when(myMockSrd.getRequestId()).thenReturn("MOCK_REQUEST");

View File

@ -62,6 +62,9 @@ public abstract class BaseComboParamsR4Test extends BaseJpaR4Test {
myMessages.add("REUSING CACHED SEARCH");
return null;
});
// allow searches to use cached results
when(myInterceptorBroadcaster.callHooks(eq(Pointcut.STORAGE_PRECHECK_FOR_CACHED_SEARCH), ArgumentMatchers.any(HookParams.class))).thenReturn(true);
}
@AfterEach

View File

@ -81,7 +81,7 @@ public class CompositeInterceptorBroadcaster {
}
if (theRequestDetails != null && theRequestDetails.getInterceptorBroadcaster() != null && retVal) {
IInterceptorBroadcaster interceptorBroadcaster = theRequestDetails.getInterceptorBroadcaster();
interceptorBroadcaster.callHooks(thePointcut, theParams);
retVal = interceptorBroadcaster.callHooks(thePointcut, theParams);
}
return retVal;
}

View File

@ -0,0 +1,161 @@
package ca.uhn.fhir.rest.server.util;
import ca.uhn.fhir.interceptor.api.HookParams;
import ca.uhn.fhir.interceptor.api.IInterceptorBroadcaster;
import ca.uhn.fhir.interceptor.api.Pointcut;
import ca.uhn.fhir.rest.api.server.RequestDetails;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.ExtendWith;
import org.mockito.Mock;
import org.mockito.junit.jupiter.MockitoExtension;
import static org.assertj.core.api.Assertions.assertThat;
import static org.mockito.Mockito.never;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;
@ExtendWith(MockitoExtension.class)
class CompositeInterceptorBroadcasterTest {
@Mock
private IInterceptorBroadcaster myModuleBroadcasterMock;
@Mock
private IInterceptorBroadcaster myReqDetailsBroadcasterMock;
@Mock
private Pointcut myPointcutMock;
@Mock
private HookParams myHookParamsMock;
@Mock
private RequestDetails myRequestDetailsMock;
@Test
void doCallHooks_WhenModuleBroadcasterReturnsTrue_And_RequestDetailsBroadcasterReturnsTrue_ThenReturnsTrue() {
when(myRequestDetailsMock.getInterceptorBroadcaster()).thenReturn(myReqDetailsBroadcasterMock);
when(myModuleBroadcasterMock.callHooks(myPointcutMock, myHookParamsMock)).thenReturn(true);
when(myReqDetailsBroadcasterMock.callHooks(myPointcutMock, myHookParamsMock)).thenReturn(true);
boolean retVal = CompositeInterceptorBroadcaster.doCallHooks(myModuleBroadcasterMock, myRequestDetailsMock,
myPointcutMock, myHookParamsMock);
assertThat(retVal).isTrue();
verify(myModuleBroadcasterMock).callHooks(myPointcutMock, myHookParamsMock);
verify(myReqDetailsBroadcasterMock).callHooks(myPointcutMock, myHookParamsMock);
}
@Test
void doCallHooks_WhenModuleBroadcasterReturnsTrue_And_RequestDetailsBroadcasterReturnsFalse_ThenReturnsFalse() {
when(myRequestDetailsMock.getInterceptorBroadcaster()).thenReturn(myReqDetailsBroadcasterMock);
when(myModuleBroadcasterMock.callHooks(myPointcutMock, myHookParamsMock)).thenReturn(true);
when(myReqDetailsBroadcasterMock.callHooks(myPointcutMock, myHookParamsMock)).thenReturn(false);
boolean retVal = CompositeInterceptorBroadcaster.doCallHooks(myModuleBroadcasterMock, myRequestDetailsMock,
myPointcutMock, myHookParamsMock);
assertThat(retVal).isFalse();
verify(myModuleBroadcasterMock).callHooks(myPointcutMock, myHookParamsMock);
verify(myReqDetailsBroadcasterMock).callHooks(myPointcutMock, myHookParamsMock);
}
@Test
void doCallHooks_WhenModuleBroadcasterReturnsFalse_ThenSkipsBroadcasterInRequestDetails_And_ReturnsFalse() {
when(myRequestDetailsMock.getInterceptorBroadcaster()).thenReturn(myReqDetailsBroadcasterMock);
when(myModuleBroadcasterMock.callHooks(myPointcutMock, myHookParamsMock)).thenReturn(false);
boolean retVal = CompositeInterceptorBroadcaster.doCallHooks(myModuleBroadcasterMock, myRequestDetailsMock,
myPointcutMock, myHookParamsMock);
assertThat(retVal).isFalse();
verify(myModuleBroadcasterMock).callHooks(myPointcutMock, myHookParamsMock);
verify(myReqDetailsBroadcasterMock, never()).callHooks(myPointcutMock, myHookParamsMock);
}
@Test
void doCallHooks_WhenModuleBroadcasterReturnsTrue_And_NullRequestDetailsBroadcaster_ThenReturnsTrue() {
when(myModuleBroadcasterMock.callHooks(myPointcutMock, myHookParamsMock)).thenReturn(true);
when(myRequestDetailsMock.getInterceptorBroadcaster()).thenReturn(null);
boolean retVal = CompositeInterceptorBroadcaster.doCallHooks(myModuleBroadcasterMock, myRequestDetailsMock, myPointcutMock,
myHookParamsMock);
assertThat(retVal).isTrue();
verify(myModuleBroadcasterMock).callHooks(myPointcutMock, myHookParamsMock);
}
@Test
void doCallHooks_WhenModuleBroadcasterReturnsFalse_And_NullRequestDetailsBroadcaster_ThenReturnsFalse() {
when(myModuleBroadcasterMock.callHooks(myPointcutMock, myHookParamsMock)).thenReturn(false);
when(myRequestDetailsMock.getInterceptorBroadcaster()).thenReturn(null);
boolean retVal = CompositeInterceptorBroadcaster.doCallHooks(myModuleBroadcasterMock, myRequestDetailsMock, myPointcutMock,
myHookParamsMock);
assertThat(retVal).isFalse();
verify(myModuleBroadcasterMock).callHooks(myPointcutMock, myHookParamsMock);
}
@Test
void doCallHooks_WhenModuleBroadcasterReturnsTrue_And_NullRequestDetails_ThenReturnsTrue() {
when(myModuleBroadcasterMock.callHooks(myPointcutMock, myHookParamsMock)).thenReturn(true);
boolean retVal = CompositeInterceptorBroadcaster.doCallHooks(myModuleBroadcasterMock, null, myPointcutMock, myHookParamsMock);
assertThat(retVal).isTrue();
verify(myModuleBroadcasterMock).callHooks(myPointcutMock, myHookParamsMock);
}
@Test
void doCallHooks_WhenModuleBroadcasterReturnsFalse_And_NullRequestDetails_ThenReturnsFalse() {
when(myModuleBroadcasterMock.callHooks(myPointcutMock, myHookParamsMock)).thenReturn(false);
boolean retVal = CompositeInterceptorBroadcaster.doCallHooks(myModuleBroadcasterMock, null, myPointcutMock, myHookParamsMock);
assertThat(retVal).isFalse();
verify(myModuleBroadcasterMock).callHooks(myPointcutMock, myHookParamsMock);
}
@Test
void doCallHooks_WhenNullModuleBroadcaster_And_NullRequestDetails_ThenReturnsTrue() {
boolean retVal = CompositeInterceptorBroadcaster.doCallHooks(null, null, myPointcutMock, myHookParamsMock);
assertThat(retVal).isTrue();
}
@Test
void doCallHooks_WhenNullModuleBroadcaster_And_RequestDetailsBroadcasterReturnsTrue_ThenReturnsTrue() {
when(myRequestDetailsMock.getInterceptorBroadcaster()).thenReturn(myReqDetailsBroadcasterMock);
when(myReqDetailsBroadcasterMock.callHooks(myPointcutMock, myHookParamsMock)).thenReturn(true);
boolean retVal = CompositeInterceptorBroadcaster.doCallHooks(null, myRequestDetailsMock, myPointcutMock, myHookParamsMock);
assertThat(retVal).isTrue();
verify(myReqDetailsBroadcasterMock).callHooks(myPointcutMock, myHookParamsMock);
}
@Test
void doCallHooks_WhenNullModuleBroadcaster_And_RequestDetailsBroadcasterReturnsFalse_ThenReturnsFalse() {
when(myRequestDetailsMock.getInterceptorBroadcaster()).thenReturn(myReqDetailsBroadcasterMock);
when(myReqDetailsBroadcasterMock.callHooks(myPointcutMock, myHookParamsMock)).thenReturn(false);
boolean retVal = CompositeInterceptorBroadcaster.doCallHooks(null, myRequestDetailsMock, myPointcutMock, myHookParamsMock);
assertThat(retVal).isFalse();
verify(myReqDetailsBroadcasterMock).callHooks(myPointcutMock, myHookParamsMock);
}
}