From 1e68cbebf73ba1344421548d714a53309995a300 Mon Sep 17 00:00:00 2001 From: Marco Collovati Date: Mon, 20 Jan 2025 09:42:04 +0100 Subject: [PATCH] fix: trigger refresh from client on hotswap with PUSH (#20848) * fix: trigger refresh from client on hotswap with PUSH When PUSH is enabled, Flow Hotswapper uses it to refresh only the affected UIs. However, this can cause issues if Flow views contain code that relies on VaadinRequest thread local, since hotswap refresh start in a background thread. This change defines a client side event listener to trigger the refresh, and uses PUSH only to fires the event. Fixes #20843 * fix test --- .../com/vaadin/flow/hotswap/Hotswapper.java | 22 ++++++++++++--- .../vaadin/flow/hotswap/HotswapperTest.java | 27 ++++++++++++++----- 2 files changed, 38 insertions(+), 11 deletions(-) diff --git a/flow-server/src/main/java/com/vaadin/flow/hotswap/Hotswapper.java b/flow-server/src/main/java/com/vaadin/flow/hotswap/Hotswapper.java index be363ed3033..9cc56f6cd3e 100644 --- a/flow-server/src/main/java/com/vaadin/flow/hotswap/Hotswapper.java +++ b/flow-server/src/main/java/com/vaadin/flow/hotswap/Hotswapper.java @@ -472,9 +472,11 @@ private void triggerClientUpdate( LOGGER.debug( "Triggering re-navigation to current route for UIs affected by classes changes."); for (UIRefreshStrategy action : uisToRefresh.keySet()) { - uisToRefresh.get(action) - .forEach(ui -> ui.access(() -> ui.refreshCurrentRoute( - action == UIRefreshStrategy.PUSH_REFRESH_CHAIN))); + String triggerEventJS = String.format( + "window.dispatchEvent(new CustomEvent(\"vaadin-refresh-ui\", { detail: { fullRefresh: %s }}));", + action == UIRefreshStrategy.PUSH_REFRESH_CHAIN); + uisToRefresh.get(action).forEach(ui -> ui + .access(() -> ui.getPage().executeJs(triggerEventJS))); } } } @@ -507,7 +509,19 @@ public void serviceDestroy(ServiceDestroyEvent event) { @Override public void uiInit(UIInitEvent event) { - sessions.add(event.getUI().getSession()); + UI ui = event.getUI(); + sessions.add(ui.getSession()); + ui.getPage().executeJs( + """ + const $wnd = window; + window.addEventListener('vaadin-ui-refresh', (ev) => { + const senderFn = $wnd.Vaadin?.Flow?.clients[$0]?.sendEventMessage; + if (senderFn) { + senderFn(1, "ui-refresh", ev.detail); + } + }); + """, + ui.getInternals().getAppId()); } /** diff --git a/flow-server/src/test/java/com/vaadin/flow/hotswap/HotswapperTest.java b/flow-server/src/test/java/com/vaadin/flow/hotswap/HotswapperTest.java index 1a283c648e5..9b856e8fb5e 100644 --- a/flow-server/src/test/java/com/vaadin/flow/hotswap/HotswapperTest.java +++ b/flow-server/src/test/java/com/vaadin/flow/hotswap/HotswapperTest.java @@ -24,6 +24,8 @@ import java.util.UUID; import java.util.concurrent.atomic.AtomicBoolean; import java.util.function.Supplier; +import java.util.regex.Matcher; +import java.util.regex.Pattern; import org.junit.Assert; import org.junit.Before; @@ -35,6 +37,7 @@ import com.vaadin.flow.component.HasComponents; import com.vaadin.flow.component.HasElement; import com.vaadin.flow.component.Tag; +import com.vaadin.flow.component.page.Page; import com.vaadin.flow.di.Lookup; import com.vaadin.flow.internal.BrowserLiveReload; import com.vaadin.flow.internal.BrowserLiveReloadAccessor; @@ -1153,20 +1156,30 @@ private RefreshTestingUI initUIAndNavigateTo(VaadinSession session, private static class RefreshTestingUI extends MockUI { + private static final Pattern UI_REFRESH_EVENT = Pattern.compile( + ".*new CustomEvent\\(\"vaadin-refresh-ui\",\\s*\\{\\s*detail:\\s*\\{\\s*fullRefresh:\\s*(true|false)\\s*}\\s*}\\).*"); private Boolean refreshRouteChainRequested; + private final Page pageSpy; + public RefreshTestingUI(VaadinSession session) { super(session); + pageSpy = Mockito.spy(super.getPage()); + // Intercept javascript executions to check if the custom ui refresh + // event dispatch has been registered. + Mockito.doAnswer(i -> { + Matcher matcher = UI_REFRESH_EVENT.matcher(i.getArgument(0)); + if (matcher.matches()) { + refreshRouteChainRequested = Boolean + .parseBoolean(matcher.group(1)); + } + return null; + }).when(pageSpy).executeJs(Mockito.anyString()); } @Override - public void refreshCurrentRoute(boolean refreshRouteChain) { - refreshRouteChainRequested = refreshRouteChain; - // No need to perform real navigation, tests only need to know if - // the method has been invoked. - // Navigation would fail anyway because of usage of method scoped - // classes. Blocking navigation prevents logs to be bloated by - // exception stack traces. + public Page getPage() { + return pageSpy; } void assertNotRefreshed() {