Skip to content

Commit

Permalink
fix: prevent usage of VaadinRequest in access
Browse files Browse the repository at this point in the history
VaadinRequest thread local is not available during a VaadinSession.access execution.
VaadinRequest is used to access the HTTP session wrapper, that can be obtained through
VaadinSession. The most important usage is however getting the update version header,
but the lookup can be done outside the access block.
This change also removes ClusterSupport registration in CurrentInstance becuase it
seems not to be used anywhere and furhtermore the thread local is set only on the
thread that calls serviceInit and never cleaned up.

Fixes #148
  • Loading branch information
mcollovati committed Oct 28, 2024
1 parent f5d38fd commit a9ff736
Show file tree
Hide file tree
Showing 2 changed files with 14 additions and 32 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -51,13 +51,6 @@ public class ClusterSupport implements VaadinServiceInitListener {

private String appVersion;

/**
* Get the current instance of the ClusterSupport.
*/
public static ClusterSupport getCurrent() {
return CurrentInstance.get(ClusterSupport.class);
}

/**
* Register the global version switch listener. If set to <code>null</code>
* the current session and the sticky cluster cookie are removed without any
Expand All @@ -82,24 +75,20 @@ public void serviceInit(ServiceInitEvent serviceInitEvent) {
"ClusterSupport service initialized. Registering RequestHandler with Application Version: "
+ appVersion);

// Set the thread local instance
CurrentInstance.set(ClusterSupport.class, this);

// Register a generic request handler for all the requests
serviceInitEvent.addRequestHandler(this::handleRequest);
}

private boolean handleRequest(VaadinSession vaadinSession,
VaadinRequest vaadinRequest, VaadinResponse vaadinResponse) {
String versionHeader = vaadinRequest
.getHeader(UPDATE_VERSION_HEADER);

vaadinSession.access(() -> {
// Always check for the update version header
String versionHeader = vaadinRequest
.getHeader(UPDATE_VERSION_HEADER);

// Always check for the update version header
WrappedSession session = vaadinSession.getSession();
vaadinSession.getUIs().forEach(ui -> {
WrappedSession session = VaadinRequest.getCurrent()
.getWrappedSession();
Optional<Component> versionNotifier = ui.getChildren()
.filter(child -> (child instanceof VersionNotifier))
.findFirst();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,10 @@
import java.util.Collections;
import java.util.stream.Stream;

import org.junit.Ignore;
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Disabled;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.ExtendWith;
import org.junit.jupiter.params.ParameterizedTest;
Expand Down Expand Up @@ -84,16 +86,6 @@ void tearDown() {
vaadinResponseMockedStatic.close();
}

@Test
void serviceInit_clusterSupportIsSetInCurrentInstance() {
ServiceInitEvent serviceInitEvent = mock(ServiceInitEvent.class);

clusterSupport.serviceInit(serviceInitEvent);

currentInstanceMockedStatic
.verify(() -> CurrentInstance.set(any(), any()));
}

@Test
void serviceInit_requestHandlerIsAdded() {
ServiceInitEvent serviceInitEvent = mock(ServiceInitEvent.class);
Expand Down Expand Up @@ -122,10 +114,10 @@ void handleRequest_versionNotifierIsRemoved_ifAlreadyPresent_And_VersionHeaderIs

when(vaadinRequest.getHeader(ClusterSupport.UPDATE_VERSION_HEADER))
.thenReturn(null);
when(vaadinRequest.getWrappedSession()).thenReturn(wrappedSession);
vaadinRequestMockedStatic.when(VaadinRequest::getCurrent)
.thenReturn(vaadinRequest);
when(vaadinSession.getUIs()).thenReturn(Collections.singletonList(ui));
when(vaadinSession.getSession()).thenReturn(wrappedSession);
when(ui.getChildren()).thenReturn(Stream.of(versionNotifier));

clusterSupport.serviceInit(serviceInitEvent);
Expand All @@ -148,7 +140,7 @@ void handleRequest_versionNotifierIsRemoved_ifAlreadyPresent_And_VersionHeaderIs

when(vaadinRequest.getHeader(ClusterSupport.UPDATE_VERSION_HEADER))
.thenReturn("");
when(vaadinRequest.getWrappedSession()).thenReturn(wrappedSession);
when(vaadinSession.getSession()).thenReturn(wrappedSession);
vaadinRequestMockedStatic.when(VaadinRequest::getCurrent)
.thenReturn(vaadinRequest);
when(vaadinSession.getUIs()).thenReturn(Collections.singletonList(ui));
Expand All @@ -174,7 +166,7 @@ void handleRequest_versionNotifierIsRemoved_ifAlreadyPresent_And_VersionHeaderEq

when(vaadinRequest.getHeader(ClusterSupport.UPDATE_VERSION_HEADER))
.thenReturn("1.0.0");
when(vaadinRequest.getWrappedSession()).thenReturn(wrappedSession);
when(vaadinSession.getSession()).thenReturn(wrappedSession);
vaadinRequestMockedStatic.when(VaadinRequest::getCurrent)
.thenReturn(vaadinRequest);
when(vaadinSession.getUIs()).thenReturn(Collections.singletonList(ui));
Expand All @@ -200,7 +192,7 @@ void handleRequest_versionNotifierIsNotRemoved_ifAlreadyPresent_And_VersionHeade

when(vaadinRequest.getHeader(ClusterSupport.UPDATE_VERSION_HEADER))
.thenReturn("2.0.0");
when(vaadinRequest.getWrappedSession()).thenReturn(wrappedSession);
when(vaadinSession.getSession()).thenReturn(wrappedSession);
vaadinRequestMockedStatic.when(VaadinRequest::getCurrent)
.thenReturn(vaadinRequest);
when(vaadinSession.getUIs()).thenReturn(Collections.singletonList(ui));
Expand All @@ -225,7 +217,7 @@ void handleRequest_versionNotifierIsAdded_ifNotPresent_And_VersionHeaderEqualsAp

when(vaadinRequest.getHeader(ClusterSupport.UPDATE_VERSION_HEADER))
.thenReturn("2.0.0");
when(vaadinRequest.getWrappedSession()).thenReturn(wrappedSession);
when(vaadinSession.getSession()).thenReturn(wrappedSession);
vaadinRequestMockedStatic.when(VaadinRequest::getCurrent)
.thenReturn(vaadinRequest);
when(vaadinSession.getUIs()).thenReturn(Collections.singletonList(ui));
Expand All @@ -243,7 +235,7 @@ void handleRequest_versionNotifierIsAdded_ifNotPresent_And_VersionHeaderEqualsAp
}

@ParameterizedTest(name = "{index} And_IfNodeSwitchIs_{0}_doAppCleanupIsCalled_{1}_times")
@CsvSource({ "true, 1, 1, 2, 1", "false, 0, 0, 1, 0" })
@CsvSource({ "true, 1, 1, 1, 1", "false, 0, 0, 0, 0" })
void onComponentEvent_removesStickyClusterCookieAndInvalidatesSession(
boolean nodeSwitch, int doAppCleanupTimes, int addCookieTimes,
int getWrappedSessionTimes, int invalidateTimes)
Expand All @@ -257,11 +249,12 @@ void onComponentEvent_removesStickyClusterCookieAndInvalidatesSession(

when(vaadinRequest.getHeader(ClusterSupport.UPDATE_VERSION_HEADER))
.thenReturn("2.0.0");
when(vaadinRequest.getWrappedSession()).thenReturn(wrappedSession);
vaadinRequestMockedStatic.when(VaadinRequest::getCurrent)
.thenReturn(vaadinRequest);
vaadinResponseMockedStatic.when(VaadinResponse::getCurrent)
.thenReturn(vaadinResponse);
when(vaadinRequest.getWrappedSession()).thenReturn(wrappedSession);
when(vaadinSession.getSession()).thenReturn(wrappedSession);
when(vaadinSession.getUIs()).thenReturn(Collections.singletonList(ui));
when(ui.getChildren()).thenReturn(Stream.empty());
when(switchVersionListener.nodeSwitch(any(), any()))
Expand Down

0 comments on commit a9ff736

Please sign in to comment.