fix: prevent Flow navigation for Hilla route (#19875)

* fix: prevent Flow navigation for Hilla route

* Add test and javadoc

* Remove null test; add mock for failing test

* fix mockstatics

---------

Co-authored-by: Mikhail Shabarov <61410877+mshabarov@users.noreply.github.com>

Author: tepi

flow-server/src/main/java/com/vaadin/flow/router/internal/AbstractNavigationStateRenderer.java

@@ -68,6 +68,7 @@
 import com.vaadin.flow.server.Constants;
 import com.vaadin.flow.server.HttpStatusCode;
 import com.vaadin.flow.server.VaadinSession;
+import com.vaadin.flow.server.menu.MenuRegistry;
 
 /**
  * Base class for navigation handlers that target a navigation state.
@@ -174,6 +175,12 @@ public int handle(NavigationEvent event) {
             return result.get();
         }
 
+        // If navigation target is Hilla route, terminate Flow navigation logic
+        // here.
+        if (MenuRegistry.hasClientRoute(event.getLocation().getPath(), true)) {
+            return HttpStatusCode.OK.getCode();
+        }
+
         final ArrayList<HasElement> chain;
 
         final boolean preserveOnRefreshTarget = isPreserveOnRefreshTarget(

flow-server/src/main/java/com/vaadin/flow/server/menu/MenuRegistry.java

@@ -408,13 +408,28 @@ public static ClassLoader getClassLoader() {
     }
 
     /**
-     * See if there is a client route available for give route path.
+     * See if there is a client route available for given route path.
      *
      * @param route
      *            route path to check
      * @return true if a client route is found.
      */
     public static boolean hasClientRoute(String route) {
+        return hasClientRoute(route, false);
+    }
+
+    /**
+     * See if there is a client route available for given route path, optionally
+     * excluding layouts (routes with children) from the check.
+     *
+     * @param route
+     *            route path to check
+     * @param excludeLayouts
+     *            {@literal true} to exclude layouts from the check,
+     *            {@literal false} to include them
+     * @return true if a client route is found.
+     */
+    public static boolean hasClientRoute(String route, boolean excludeLayouts) {
         if (VaadinSession.getCurrent() == null || route == null) {
             return false;
         }
@@ -423,8 +438,16 @@ public static boolean hasClientRoute(String route) {
         Map<String, AvailableViewInfo> clientItems = MenuRegistry
                 .collectClientMenuItems(true,
                         VaadinSession.getCurrent().getConfiguration());
-        Set<String> clientRoutes = clientItems.keySet();
+        final Set<String> clientRoutes = new HashSet<>();
+        clientItems.forEach((path, info) -> {
+            if (excludeLayouts) {
+                if (info.children() == null || info.children().isEmpty()) {
+                    clientRoutes.add(path);
+                }
+            } else {
+                clientRoutes.add(path);
+            }
+        });
         return clientRoutes.contains(route);
     }
-
 }

flow-server/src/test/java/com/vaadin/flow/component/internal/JavaScriptBootstrapUITest.java

@@ -15,6 +15,7 @@
 import org.junit.Before;
 import org.junit.Test;
 import org.mockito.ArgumentCaptor;
+import org.mockito.MockedStatic;
 import org.mockito.Mockito;
 
 import com.vaadin.flow.component.Component;
@@ -46,6 +47,7 @@
 import com.vaadin.flow.server.VaadinService;
 import com.vaadin.flow.server.VaadinSession;
 import com.vaadin.flow.server.VaadinSessionState;
+import com.vaadin.flow.server.menu.MenuRegistry;
 
 public class JavaScriptBootstrapUITest {
 
@@ -435,22 +437,30 @@ public void should_update_pushState_when_navigationHasBeenAlreadyStarted() {
         ArgumentCaptor<Serializable[]> execArg = ArgumentCaptor
                 .forClass(Serializable[].class);
 
-        ui.navigate("clean/1");
-        Mockito.verify(page).executeJs(execJs.capture(), execArg.capture());
-
-        boolean reactEnabled = ui.getSession().getConfiguration()
-                .isReactEnabled();
-
-        final Serializable[] execValues = execArg.getValue();
-        if (reactEnabled) {
-            assertEquals(REACT_PUSHSTATE_TO, execJs.getValue());
-            assertEquals(1, execValues.length);
-            assertEquals("clean/1", execValues[0]);
-        } else {
-            assertEquals(CLIENT_PUSHSTATE_TO, execJs.getValue());
-            assertEquals(2, execValues.length);
-            assertNull(execValues[0]);
-            assertEquals("clean/1", execValues[1]);
+        try (MockedStatic<MenuRegistry> menuRegistry = Mockito
+                .mockStatic(MenuRegistry.class)) {
+
+            menuRegistry
+                    .when(() -> MenuRegistry.hasClientRoute("clean/1", true))
+                    .thenReturn(false);
+
+            ui.navigate("clean/1");
+            Mockito.verify(page).executeJs(execJs.capture(), execArg.capture());
+
+            boolean reactEnabled = ui.getSession().getConfiguration()
+                    .isReactEnabled();
+
+            final Serializable[] execValues = execArg.getValue();
+            if (reactEnabled) {
+                assertEquals(REACT_PUSHSTATE_TO, execJs.getValue());
+                assertEquals(1, execValues.length);
+                assertEquals("clean/1", execValues[0]);
+            } else {
+                assertEquals(CLIENT_PUSHSTATE_TO, execJs.getValue());
+                assertEquals(2, execValues.length);
+                assertNull(execValues[0]);
+                assertEquals("clean/1", execValues[1]);
+            }
         }
     }
 

flow-server/src/test/java/com/vaadin/flow/router/internal/NavigationStateRendererTest.java

@@ -37,6 +37,7 @@
 import org.junit.Rule;
 import org.junit.Test;
 import org.junit.rules.ExpectedException;
+import org.mockito.MockedStatic;
 import org.mockito.Mockito;
 
 import com.vaadin.flow.component.Component;
@@ -65,6 +66,7 @@
 import com.vaadin.flow.router.PreserveOnRefresh;
 import com.vaadin.flow.router.QueryParameters;
 import com.vaadin.flow.router.Route;
+import com.vaadin.flow.router.RouteAlias;
 import com.vaadin.flow.router.RouteConfiguration;
 import com.vaadin.flow.router.Router;
 import com.vaadin.flow.router.RouterLayout;
@@ -77,6 +79,7 @@
 import com.vaadin.flow.server.RouteRegistry;
 import com.vaadin.flow.server.ServiceException;
 import com.vaadin.flow.server.WrappedSession;
+import com.vaadin.flow.server.menu.MenuRegistry;
 import com.vaadin.flow.server.startup.ApplicationRouteRegistry;
 import com.vaadin.tests.util.AlwaysLockedVaadinSession;
 import com.vaadin.tests.util.MockDeploymentConfiguration;
@@ -323,6 +326,21 @@ private static class SingleView extends Component {
         }
     }
 
+    @Route(value = "/:samplePersonID?/:action?(edit)")
+    @RouteAlias(value = "")
+    @Tag("div")
+    private static class RootRouteWithParam extends Component
+            implements BeforeEnterObserver {
+        RootRouteWithParam() {
+            addAttachListener(e -> viewAttachCount.getAndIncrement());
+        }
+
+        @Override
+        public void beforeEnter(BeforeEnterEvent event) {
+            beforeEnterCount.getAndIncrement();
+        }
+    }
+
     @Test
     public void handle_preserveOnRefreshAndWindowNameNotKnown_clientSideCallTriggered() {
         // given a service with instantiator
@@ -636,6 +654,7 @@ public void handle_preserveOnRefresh_sameUI_uiIsNotClosed_childrenAreNotRemoved(
 
     private static AtomicInteger layoutAttachCount;
     private static AtomicInteger viewAttachCount;
+    private static AtomicInteger beforeEnterCount;
     private static String layoutUUID;
     private static String viewUUID;
 
@@ -747,6 +766,52 @@ public void handle_normalView_refreshCurrentRouteRecreatesComponents() {
 
     }
 
+    @Test
+    public void handle_clientNavigation_withMatchingFlowRoute() {
+        viewAttachCount = new AtomicInteger();
+        beforeEnterCount = new AtomicInteger();
+
+        // given a service with instantiator
+        MockVaadinServletService service = createMockServiceWithInstantiator();
+
+        // given a locked session
+        MockVaadinSession session = new AlwaysLockedVaadinSession(service);
+        session.setConfiguration(new MockDeploymentConfiguration());
+
+        // given a NavigationStateRenderer mapping to PreservedNestedView
+        Router router = session.getService().getRouter();
+        NavigationStateRenderer renderer = new NavigationStateRenderer(
+                new NavigationStateBuilder(router)
+                        .withTarget(RootRouteWithParam.class).withPath("")
+                        .build());
+        router.getRegistry().setRoute("", RootRouteWithParam.class, null);
+
+        MockUI ui = new MockUI(session);
+
+        renderer.handle(new NavigationEvent(router, new Location(""), ui,
+                NavigationTrigger.PAGE_LOAD));
+
+        Assert.assertEquals(1, beforeEnterCount.get());
+        Assert.assertEquals(1, viewAttachCount.get());
+
+        ui.getInternals().clearLastHandledNavigation();
+
+        try (MockedStatic<MenuRegistry> menuRegistry = Mockito
+                .mockStatic(MenuRegistry.class)) {
+            menuRegistry.when(
+                    () -> MenuRegistry.hasClientRoute("client-route", true))
+                    .thenReturn(true);
+
+            // This should not call attach or beforeEnter on root route
+            renderer.handle(
+                    new NavigationEvent(router, new Location("client-route"),
+                            ui, NavigationTrigger.CLIENT_SIDE));
+
+            Assert.assertEquals(1, beforeEnterCount.get());
+            Assert.assertEquals(1, viewAttachCount.get());
+        }
+    }
+
     private MockVaadinServletService createMockServiceWithInstantiator() {
         MockVaadinServletService service = new MockVaadinServletService();
         service.init(new MockInstantiator() {