fix: check for client route conflicts (#20188)

* fix: check for client route conflicts Fixes #20144 --------- Co-authored-by: Tomi Virtanen <[email protected]>
vaadin · Oct 21, 2024 · ec0652d · ec0652d
1 parent 3bd7a7b
commit ec0652d
Show file tree

Hide file tree

Showing 20 changed files with 257 additions and 50 deletions.
diff --git a/flow-data/src/test/java/com/vaadin/flow/data/provider/DataCommunicatorAsyncTest.java b/flow-data/src/test/java/com/vaadin/flow/data/provider/DataCommunicatorAsyncTest.java
@@ -17,18 +17,18 @@
 import org.junit.runner.RunWith;
 import org.junit.runners.Parameterized;
 import org.mockito.Mock;
+import org.mockito.Mockito;
 import org.mockito.MockitoAnnotations;
 
 import com.vaadin.flow.component.UI;
 import com.vaadin.flow.dom.Element;
 import com.vaadin.flow.internal.Range;
+import com.vaadin.flow.server.RouteRegistry;
 import com.vaadin.flow.server.VaadinRequest;
 import com.vaadin.flow.server.VaadinService;
-import com.vaadin.flow.server.VaadinServlet;
 import com.vaadin.flow.server.VaadinServletService;
 import com.vaadin.flow.server.VaadinSession;
 import com.vaadin.flow.shared.communication.PushMode;
-
 import elemental.json.JsonValue;
 
 @RunWith(Parameterized.class)
@@ -253,8 +253,14 @@ protected void init(VaadinRequest request) {
         private static VaadinSession findOrcreateSession() {
             VaadinSession session = VaadinSession.getCurrent();
             if (session == null) {
-                session = new AlwaysLockedVaadinSession(
-                        new VaadinServletService(new VaadinServlet(), null));
+                RouteRegistry routeRegistry = Mockito.mock(RouteRegistry.class);
+                VaadinServletService service = new VaadinServletService() {
+                    @Override
+                    protected RouteRegistry getRouteRegistry() {
+                        return routeRegistry;
+                    }
+                };
+                session = new AlwaysLockedVaadinSession(service);
                 VaadinSession.setCurrent(session);
             }
             return session;

diff --git a/flow-data/src/test/java/com/vaadin/flow/data/provider/DataCommunicatorTest.java b/flow-data/src/test/java/com/vaadin/flow/data/provider/DataCommunicatorTest.java
@@ -51,8 +51,10 @@
 import com.vaadin.flow.function.SerializablePredicate;
 import com.vaadin.flow.internal.Range;
 import com.vaadin.flow.internal.StateNode;
+import com.vaadin.flow.server.RouteRegistry;
 import com.vaadin.flow.server.VaadinRequest;
 import com.vaadin.flow.server.VaadinService;
+import com.vaadin.flow.server.VaadinServletService;
 import com.vaadin.flow.server.VaadinSession;
 
 import elemental.json.JsonValue;
@@ -1835,13 +1837,24 @@ protected void init(VaadinRequest request) {
         private static VaadinSession findOrcreateSession() {
             VaadinSession session = VaadinSession.getCurrent();
             if (session == null) {
-                session = new AlwaysLockedVaadinSession(null);
+                MockService service = Mockito.mock(MockService.class);
+                Mockito.when(service.getRouteRegistry())
+                        .thenReturn(Mockito.mock(RouteRegistry.class));
+                session = new AlwaysLockedVaadinSession(service);
                 VaadinSession.setCurrent(session);
             }
             return session;
         }
     }
 
+    public static class MockService extends VaadinServletService {
+
+        @Override
+        public RouteRegistry getRouteRegistry() {
+            return super.getRouteRegistry();
+        }
+    }
+
     public static class AlwaysLockedVaadinSession extends MockVaadinSession {
 
         public AlwaysLockedVaadinSession(VaadinService service) {

diff --git a/...c/test/java/com/vaadin/flow/data/provider/hierarchy/HierarchicalCommunicatorDataTest.java b/...c/test/java/com/vaadin/flow/data/provider/hierarchy/HierarchicalCommunicatorDataTest.java
@@ -35,6 +35,7 @@
 import com.vaadin.flow.dom.Element;
 import com.vaadin.flow.function.SerializablePredicate;
 import com.vaadin.flow.function.ValueProvider;
+import com.vaadin.flow.server.RouteRegistry;
 import com.vaadin.flow.server.VaadinRequest;
 import com.vaadin.flow.server.VaadinService;
 import com.vaadin.flow.server.VaadinSession;
@@ -368,22 +369,15 @@ protected void init(VaadinRequest request) {
         private static VaadinSession findOrCreateSession() {
             VaadinSession session = VaadinSession.getCurrent();
             if (session == null) {
+                DataCommunicatorTest.MockService service = Mockito
+                        .mock(DataCommunicatorTest.MockService.class);
+                Mockito.when(service.getRouteRegistry())
+                        .thenReturn(Mockito.mock(RouteRegistry.class));
                 session = new DataCommunicatorTest.AlwaysLockedVaadinSession(
-                        null);
+                        service);
                 VaadinSession.setCurrent(session);
             }
             return session;
         }
     }
-
-    public static class AlwaysLockedVaadinSession
-            extends DataCommunicatorTest.MockVaadinSession {
-
-        public AlwaysLockedVaadinSession(VaadinService service) {
-            super(service);
-            lock();
-        }
-
-    }
-
 }
diff --git a/flow-server/src/main/java/com/vaadin/flow/router/internal/AbstractRouteRegistry.java b/flow-server/src/main/java/com/vaadin/flow/router/internal/AbstractRouteRegistry.java
@@ -403,6 +403,8 @@ public Optional<String> getTemplate(
     public void setRoute(String path,
             Class<? extends Component> navigationTarget,
             List<Class<? extends RouterLayout>> parentChain) {
+        RouteUtil.checkForClientRouteCollisions(VaadinService.getCurrent(),
+                HasUrlParameterFormat.getTemplate(path, navigationTarget));
         configureWithFullTemplate(path, navigationTarget,
                 (configuration, fullTemplate) -> configuration
                         .setRoute(fullTemplate, navigationTarget, parentChain));

diff --git a/flow-server/src/main/java/com/vaadin/flow/router/internal/RouteUtil.java b/flow-server/src/main/java/com/vaadin/flow/router/internal/RouteUtil.java
@@ -16,6 +16,7 @@
 package com.vaadin.flow.router.internal;
 
 import java.util.ArrayList;
+import java.util.Arrays;
 import java.util.Collection;
 import java.util.Collections;
 import java.util.HashSet;
@@ -46,13 +47,17 @@
 import com.vaadin.flow.router.RouteAlias;
 import com.vaadin.flow.router.RouteBaseData;
 import com.vaadin.flow.router.RouteConfiguration;
+import com.vaadin.flow.router.RouteData;
 import com.vaadin.flow.router.RoutePathProvider;
 import com.vaadin.flow.router.RoutePrefix;
 import com.vaadin.flow.router.RouterLayout;
 import com.vaadin.flow.server.AbstractConfiguration;
+import com.vaadin.flow.server.InvalidRouteConfigurationException;
 import com.vaadin.flow.server.RouteRegistry;
 import com.vaadin.flow.server.SessionRouteRegistry;
 import com.vaadin.flow.server.VaadinContext;
+import com.vaadin.flow.server.VaadinService;
+import com.vaadin.flow.server.frontend.FrontendUtils;
 import com.vaadin.flow.server.menu.AvailableViewInfo;
 
 /**
@@ -587,6 +592,67 @@ public static boolean isAutolayoutEnabled(Class<?> target, String path) {
 
     }
 
+    /**
+     * Checks the given list of Flow routes for potential collisions with Hilla
+     * routes.
+     *
+     * Note: Routes will only be checked in development mode, when Hilla is in
+     * use.
+     *
+     * @param service
+     *            VaadinService instance
+     * @param flowRoutes
+     *            Flow routes to check against
+     * @throws InvalidRouteConfigurationException
+     *             if a collision is detected
+     */
+    public static void checkForClientRouteCollisions(VaadinService service,
+            List<RouteData> flowRoutes)
+            throws InvalidRouteConfigurationException {
+        checkForClientRouteCollisions(service, flowRoutes.stream()
+                .map(RouteData::getTemplate).toArray(String[]::new));
+    }
+
+    /**
+     * Checks the given array of Flow route templates for potential collisions
+     * with Hilla routes.
+     *
+     * Note: Routes will only be checked in development mode, when Hilla is in
+     * use.
+     *
+     * @param service
+     *            VaadinService instance
+     * @param flowRouteTemplates
+     *            Flow routes to check against
+     * @throws InvalidRouteConfigurationException
+     *             if a collision is detected
+     */
+    public static void checkForClientRouteCollisions(VaadinService service,
+            String... flowRouteTemplates)
+            throws InvalidRouteConfigurationException {
+        if (service == null
+                || service.getDeploymentConfiguration().isProductionMode()
+                || !FrontendUtils.isHillaUsed(service
+                        .getDeploymentConfiguration().getFrontendFolder())) {
+            return;
+        }
+
+        List<String> collisions = MenuRegistry
+                .collectClientMenuItems(false,
+                        service.getDeploymentConfiguration())
+                .keySet().stream().map(PathUtil::trimPath)
+                .filter(clientRoute -> Arrays.stream(flowRouteTemplates)
+                        .map(PathUtil::trimPath).anyMatch(clientRoute::equals))
+                .toList();
+        if (!collisions.isEmpty()) {
+            String msg = String.format(
+                    "Invalid route configuration. The following Hilla "
+                            + "route(s) conflict with configured Flow routes: %s",
+                    String.join(", ", collisions));
+            throw new InvalidRouteConfigurationException(msg);
+        }
+    }
+
     /**
      * Check if the given registry has any auto layouts added
      * with @{@link Layout} annotation.

diff --git a/flow-server/src/main/java/com/vaadin/flow/server/VaadinService.java b/flow-server/src/main/java/com/vaadin/flow/server/VaadinService.java
@@ -316,6 +316,9 @@ public void init() throws ServiceException {
             UsageStatistics.markAsUsed("flow/bun", null);
         }
 
+        RouteUtil.checkForClientRouteCollisions(this,
+                getRouteRegistry().getRegisteredRoutes());
+
         initialized = true;
     }
 

diff --git a/flow-server/src/main/java/com/vaadin/flow/server/VaadinSession.java b/flow-server/src/main/java/com/vaadin/flow/server/VaadinSession.java
@@ -16,6 +16,9 @@
 
 package com.vaadin.flow.server;
 
+import jakarta.servlet.http.HttpSession;
+import jakarta.servlet.http.HttpSessionBindingEvent;
+import jakarta.servlet.http.HttpSessionBindingListener;
 import java.io.IOException;
 import java.io.ObjectInputStream;
 import java.io.Serializable;
@@ -52,10 +55,6 @@
 import com.vaadin.flow.shared.Registration;
 import com.vaadin.flow.shared.communication.PushMode;
 
-import jakarta.servlet.http.HttpSession;
-import jakarta.servlet.http.HttpSessionBindingEvent;
-import jakarta.servlet.http.HttpSessionBindingListener;
-
 /**
  * Contains everything that Vaadin needs to store for a specific user. This is
  * typically stored in a {@link HttpSession}, but others storage mechanisms

diff --git a/flow-server/src/test/java/com/vaadin/flow/component/internal/UIInternalsTest.java b/flow-server/src/test/java/com/vaadin/flow/component/internal/UIInternalsTest.java
@@ -22,7 +22,6 @@
 import com.vaadin.flow.component.Tag;
 import com.vaadin.flow.component.UI;
 import com.vaadin.flow.component.page.Push;
-import com.vaadin.flow.di.DefaultInstantiator;
 import com.vaadin.flow.dom.Element;
 import com.vaadin.flow.function.DeploymentConfiguration;
 import com.vaadin.flow.function.SerializableConsumer;
@@ -34,8 +33,7 @@
 import com.vaadin.flow.router.ParentLayout;
 import com.vaadin.flow.router.Route;
 import com.vaadin.flow.router.RouterLayout;
-import com.vaadin.flow.server.VaadinContext;
-import com.vaadin.flow.server.VaadinService;
+import com.vaadin.flow.server.MockVaadinServletService;
 import com.vaadin.flow.shared.Registration;
 import com.vaadin.flow.shared.communication.PushMode;
 import com.vaadin.tests.util.AlwaysLockedVaadinSession;
@@ -44,8 +42,8 @@ public class UIInternalsTest {
 
     @Mock
     UI ui;
-    @Mock
-    VaadinService vaadinService;
+
+    MockVaadinServletService vaadinService;
 
     UIInternals internals;
 
@@ -118,14 +116,11 @@ public void init() {
         Element body = new Element("body");
         Mockito.when(ui.getElement()).thenReturn(body);
 
+        vaadinService = new MockVaadinServletService();
         internals = new UIInternals(ui);
         AlwaysLockedVaadinSession session = new AlwaysLockedVaadinSession(
                 vaadinService);
         session.setConfiguration(Mockito.mock(DeploymentConfiguration.class));
-        VaadinContext context = Mockito.mock(VaadinContext.class);
-        Mockito.when(vaadinService.getContext()).thenReturn(context);
-        Mockito.when(vaadinService.getInstantiator())
-                .thenReturn(new DefaultInstantiator(vaadinService));
         internals.setSession(session);
         Mockito.when(ui.getSession()).thenReturn(session);
     }
@@ -554,8 +549,7 @@ public void setTitle_titleAndPendingJsInvocationSetsCorrectTitle() {
     private PushConfiguration setUpInitialPush() {
         DeploymentConfiguration config = Mockito
                 .mock(DeploymentConfiguration.class);
-        Mockito.when(vaadinService.getDeploymentConfiguration())
-                .thenReturn(config);
+        vaadinService.setConfiguration(config);
 
         PushConfiguration pushConfig = Mockito.mock(PushConfiguration.class);
         Mockito.when(ui.getPushConfiguration()).thenReturn(pushConfig);

diff --git a/...-server/src/test/java/com/vaadin/flow/component/webcomponent/WebComponentWrapperTest.java b/...-server/src/test/java/com/vaadin/flow/component/webcomponent/WebComponentWrapperTest.java
@@ -32,7 +32,7 @@
 import com.vaadin.flow.dom.Element;
 import com.vaadin.flow.function.DeploymentConfiguration;
 import com.vaadin.flow.server.MockInstantiator;
-import com.vaadin.flow.server.VaadinService;
+import com.vaadin.flow.server.MockVaadinServletService;
 import com.vaadin.flow.server.VaadinSession;
 import com.vaadin.flow.server.webcomponent.WebComponentBinding;
 import com.vaadin.tests.util.AlwaysLockedVaadinSession;
@@ -244,7 +244,7 @@ private static WebComponentUI constructWebComponentUI(
 
         UIInternals internals = new UIInternals(ui);
         internals.setSession(
-                new AlwaysLockedVaadinSession(mock(VaadinService.class)));
+                new AlwaysLockedVaadinSession(new MockVaadinServletService()));
         when(ui.getInternals()).thenReturn(internals);
 
         Component parent = new Parent();

diff --git a/flow-server/src/test/java/com/vaadin/flow/router/RouteConfigurationTest.java b/flow-server/src/test/java/com/vaadin/flow/router/RouteConfigurationTest.java
@@ -1,13 +1,15 @@
 package com.vaadin.flow.router;
 
 import jakarta.servlet.ServletContext;
+import java.io.File;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collections;
 import java.util.List;
 import java.util.Optional;
 import java.util.concurrent.CountDownLatch;
 
+import com.vaadin.flow.function.DeploymentConfiguration;
 import com.vaadin.flow.router.internal.HasUrlParameterFormat;
 import net.jcip.annotations.NotThreadSafe;
 import org.junit.Assert;
@@ -47,9 +49,16 @@ public void init() {
         vaadinContext = new MockVaadinContext(servletContext);
         registry = ApplicationRouteRegistry.getInstance(vaadinContext);
 
+        DeploymentConfiguration configuration = Mockito
+                .mock(DeploymentConfiguration.class);
+        Mockito.when(configuration.getFrontendFolder())
+                .thenReturn(new File("/frontend"));
+
         vaadinService = Mockito.mock(MockService.class);
         Mockito.when(vaadinService.getRouteRegistry()).thenReturn(registry);
         Mockito.when(vaadinService.getContext()).thenReturn(vaadinContext);
+        Mockito.when(vaadinService.getDeploymentConfiguration())
+                .thenReturn(configuration);
 
         VaadinService.setCurrent(vaadinService);
 

diff --git a/flow-server/src/test/java/com/vaadin/flow/router/RouterLinkTest.java b/flow-server/src/test/java/com/vaadin/flow/router/RouterLinkTest.java
@@ -15,6 +15,7 @@
  */
 package com.vaadin.flow.router;
 
+import java.io.File;
 import java.util.Arrays;
 import java.util.Collections;
 import java.util.HashMap;
@@ -31,6 +32,7 @@
 import com.vaadin.flow.component.Component;
 import com.vaadin.flow.component.Tag;
 import com.vaadin.flow.component.UI;
+import com.vaadin.flow.function.DeploymentConfiguration;
 import com.vaadin.flow.internal.HasCurrentService;
 import com.vaadin.flow.router.internal.HasUrlParameterFormat;
 import com.vaadin.flow.server.InvalidRouteConfigurationException;
@@ -61,6 +63,13 @@ public void setParameter(BeforeEvent event, String parameter) {
     @Before
     public void setUp() throws NoSuchFieldException, IllegalAccessException,
             InvalidRouteConfigurationException {
+        VaadinService service = VaadinService.getCurrent();
+        DeploymentConfiguration config = Mockito
+                .mock(DeploymentConfiguration.class);
+        Mockito.when(config.getFrontendFolder())
+                .thenReturn(new File("/frontend"));
+        Mockito.when(service.getDeploymentConfiguration()).thenReturn(config);
+
         registry = new TestRouteRegistry();
         RouteConfiguration routeConfiguration = RouteConfiguration
                 .forRegistry(registry);
@@ -74,7 +83,7 @@ public void setUp() throws NoSuchFieldException, IllegalAccessException,
         router = new Router(registry);
 
         ui = new RoutingTestBase.RouterTestUI(router);
-        VaadinService service = VaadinService.getCurrent();
+
         Mockito.when(service.getRouter()).thenReturn(router);
     }