Skip to content

Commit

Permalink
fix: delay session expiration handling to prevent canceling ongoing n…
Browse files Browse the repository at this point in the history
…avigation (#19983) (CP: 2.11) (#20456)

* fix: delay session expiration handling to prevent canceling ongoing navigation (#19983)

Attempts to fix the synchronization issue related to the usage of the Login reported in #12640.
The Login component sends the UIDL request for the login event to the server and concurrently submits the form.
If processing the form submission performs a session ID change and a request redirect, the UIDL requests might fail with a session expiration response. The Flow client then can cancel the first redirect because it reloads the page due to the session expiration. Lastly, the beacon request hits again a valid session, but a resynchronization is triggered because the previous UIDL request was rejected.

This change delays a bit the session expiration handling on Flow client, to allow a potential redirect to complete without being cancelled. However, the client application is immediately set in TERMINATED state.

* fix
  • Loading branch information
mcollovati authored Nov 12, 2024
1 parent 0858723 commit bcfa4ec
Show file tree
Hide file tree
Showing 4 changed files with 40 additions and 24 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -418,9 +418,14 @@ assert getServerId(valueMap) == -1
if (nextResponseSessionExpiredHandler != null) {
nextResponseSessionExpiredHandler.execute();
} else if (uiState != UIState.TERMINATED) {
registry.getSystemErrorHandler()
.handleSessionExpiredError(null);
registry.getUILifecycle().setState(UIState.TERMINATED);
// Delay the session expiration handling to prevent
// canceling potential ongoing page redirect/reload
Scheduler.get().scheduleFixedDelay(() -> {
registry.getSystemErrorHandler()
.handleSessionExpiredError(null);
return false;
}, 250);
}
} else if (meta.containsKey("appError")
&& uiState != UIState.TERMINATED) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -152,12 +152,13 @@ public void handleSessionExpiredError(String details) {

@Override
public void handleUnrecoverableError(String caption, String message,
String details, String url, String querySelector) {
String details, String url, String querySelector) {
unrecoverableErrorHandled = true;
}
}

private static class TestApplicationConfiguration extends ApplicationConfiguration {
private static class TestApplicationConfiguration
extends ApplicationConfiguration {
@Override
public String getApplicationId() {
return "test-application-id";
Expand Down Expand Up @@ -223,8 +224,7 @@ public void testMessageProcessing_moduleDependencyIsHandledBeforeApplyingChanges
assertEquals(ResourceLoader.class.getName(),
eventsOrder.sources.get(0));
// the second one is applying changes to StatTree
assertEquals(StateTree.class.getName(),
eventsOrder.sources.get(1));
assertEquals(StateTree.class.getName(), eventsOrder.sources.get(1));
});
}

Expand Down Expand Up @@ -301,11 +301,13 @@ public void testHandleJSON_uiTerminated_sessionExpiredMessageNotShown() {

doAssert(() -> {
// then: no session expire and unrecoverable error handling expected
assertFalse("Session Expired Message handling is not expected " +
"when the page is being redirected",
assertFalse(
"Session Expired Message handling is not expected "
+ "when the page is being redirected",
getSystemErrorHandler().sessionExpiredMessageHandled);
assertFalse("Unrecoverable Error Message handling was not " +
"expected when the page is being redirected",
assertFalse(
"Unrecoverable Error Message handling was not "
+ "expected when the page is being redirected",
getSystemErrorHandler().unrecoverableErrorHandled);
assertEquals(UILifecycle.UIState.TERMINATED,
getUILifecycle().getState());
Expand Down Expand Up @@ -333,11 +335,13 @@ public void testHandleJSON_uiTerminated_unrecoverableErrorMessageNotShown() {

doAssert(() -> {
// then: no session expire and unrecoverable error handling expected
assertFalse("Session Expired Message handling is not expected " +
"when the page is being redirected",
assertFalse(
"Session Expired Message handling is not expected "
+ "when the page is being redirected",
getSystemErrorHandler().sessionExpiredMessageHandled);
assertFalse("Unrecoverable Error Message handling was not " +
"expected when the page is being redirected",
assertFalse(
"Unrecoverable Error Message handling was not "
+ "expected when the page is being redirected",
getSystemErrorHandler().unrecoverableErrorHandled);
assertEquals(UILifecycle.UIState.TERMINATED,
getUILifecycle().getState());
Expand Down Expand Up @@ -369,7 +373,7 @@ public void testHandleJSON_sessionExpiredAndUIRunning_sessionExpiredMessageShown
getSystemErrorHandler().unrecoverableErrorHandled);
assertEquals(UILifecycle.UIState.TERMINATED,
getUILifecycle().getState());
});
}, 300);
}

public void testHandleJSON_unrecoverableErrorAndUIRunning_unrecoverableErrorMessageShown() {
Expand Down Expand Up @@ -416,8 +420,7 @@ private void doAssert(Runnable assertions) {
doAssert(assertions, 100);
}

private void doAssert(Runnable assertions,
int assertDelayInMillis) {
private void doAssert(Runnable assertions, int assertDelayInMillis) {
delayTestFinish(500);
new Timer() {
@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,8 @@ public void enableSessionExpiredNotification_sessionExpired_notificationShown()
// Just click on any button to make a request after killing the session
clickButton(CLOSE_SESSION);

waitUntil(d -> isSessionExpiredNotificationPresent());

Assert.assertTrue("After enabling the 'Session Expired' notification, "
+ "the page should not be refreshed "
+ "after killing the session", isMessageUpdated());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,21 +22,23 @@ protected String getTestPath() {
}

@Test
public void navigationAfterSessionExpired() {
public void should_HaveANewSessionId_when_NavigationAfterSessionExpired() {
openUrl("/new-router-session/NormalView");

navigateToAnotherView();
String sessionId = getSessionId();
navigateToFirstView();
Assert.assertEquals(sessionId, getSessionId());

navigateToSesssionExpireView();
// expired session causes page reload, after the page reload there will
// be a new session
Assert.assertNotEquals(sessionId, getSessionId());
sessionId = getSessionId();
navigateToFirstView();
waitUntil(d -> !sessionId.equals(getSessionId()));

String newSessionId = getSessionId();
navigateToAnotherView();
// session is preserved
Assert.assertEquals(sessionId, getSessionId());
Assert.assertEquals(newSessionId, getSessionId());
}

@Test
Expand Down Expand Up @@ -66,7 +68,7 @@ private void navigateToAnotherView() {
}

private void navigateToSesssionExpireView() {
navigateTo("ViewWhichInvalidatesSession");
findElement(By.linkText("ViewWhichInvalidatesSession")).click();
}

private void navigateToInternalErrorView() {
Expand All @@ -76,8 +78,12 @@ private void navigateToInternalErrorView() {

private void navigateTo(String linkText) {
findElement(By.linkText(linkText)).click();
assertTextAvailableInView(linkText);

}

private void assertTextAvailableInView(String linkText) {
Assert.assertNotNull(
findElement(By.xpath("//strong[text()='" + linkText + "']")));

}
}

0 comments on commit bcfa4ec

Please sign in to comment.