fix: fail fast if view defines same path for route and route alias (#…

…19977) (#19981) Fixes #19972 Co-authored-by: Marco Collovati <marco@vaadin.com>
vaadin · Sep 18, 2024 · 0cb0903 · 0cb0903
1 parent 8ade81c
commit 0cb0903
Show file tree

Hide file tree

Showing 2 changed files with 83 additions and 2 deletions.
diff --git a/...server/src/main/java/com/vaadin/flow/server/startup/AbstractRouteRegistryInitializer.java b/...server/src/main/java/com/vaadin/flow/server/startup/AbstractRouteRegistryInitializer.java
@@ -12,8 +12,11 @@
 
 import java.io.Serializable;
 import java.lang.annotation.Annotation;
+import java.util.Arrays;
 import java.util.List;
+import java.util.Map;
 import java.util.Set;
+import java.util.function.Function;
 import java.util.function.Supplier;
 import java.util.stream.Collectors;
 import java.util.stream.Stream;
@@ -27,6 +30,7 @@
 import com.vaadin.flow.router.RouteAlias;
 import com.vaadin.flow.router.RouterLayout;
 import com.vaadin.flow.router.internal.RouteUtil;
+import com.vaadin.flow.server.InvalidRouteConfigurationException;
 import com.vaadin.flow.server.InvalidRouteLayoutConfigurationException;
 import com.vaadin.flow.server.PWA;
 import com.vaadin.flow.server.PageConfigurator;
@@ -90,6 +94,30 @@ private void checkForConflictingAnnotations(VaadinContext context,
 
         }
 
+        RouteAlias[] aliases = route.getAnnotationsByType(RouteAlias.class);
+        if (aliases.length > 0) {
+            Route routeAnnotation = route.getAnnotation(Route.class);
+            Map<String, Long> stats = Arrays.stream(aliases)
+                    .map(RouteAlias::value).collect(Collectors.groupingBy(
+                            Function.identity(), Collectors.counting()));
+            if (stats.containsKey(routeAnnotation.value())) {
+                throw new InvalidRouteConfigurationException(String.format(
+                        "'%s' declares '@%s' and '@%s' with the same path '%s'",
+                        route.getCanonicalName(), Route.class.getSimpleName(),
+                        RouteAlias.class.getSimpleName(),
+                        routeAnnotation.value()));
+            }
+            String repeatedAliases = stats.entrySet().stream()
+                    .filter(e -> e.getValue() > 1).map(Map.Entry::getKey)
+                    .collect(Collectors.joining(", "));
+            if (!repeatedAliases.isEmpty()) {
+                throw new InvalidRouteConfigurationException(String.format(
+                        "'%s' declares multiple '@%s' with same paths: %s.",
+                        route.getCanonicalName(),
+                        RouteAlias.class.getSimpleName(), repeatedAliases));
+            }
+        }
+
         if (route.isAnnotationPresent(PageTitle.class)
                 && HasDynamicTitle.class.isAssignableFrom(route)) {
             throw new DuplicateNavigationTitleException(String.format(
@@ -104,8 +132,7 @@ private void checkForConflictingAnnotations(VaadinContext context,
 
             validateRouteAnnotation(context, route, annotation);
 
-            for (RouteAlias alias : route
-                    .getAnnotationsByType(RouteAlias.class)) {
+            for (RouteAlias alias : aliases) {
                 validateRouteAliasAnnotation(context, route, alias, annotation);
             }
         });

diff --git a/...er/src/test/java/com/vaadin/flow/server/startup/AbstractRouteRegistryInitializerTest.java b/...er/src/test/java/com/vaadin/flow/server/startup/AbstractRouteRegistryInitializerTest.java
@@ -18,7 +18,9 @@
 import com.vaadin.flow.component.Tag;
 import com.vaadin.flow.router.ParentLayout;
 import com.vaadin.flow.router.Route;
+import com.vaadin.flow.router.RouteAlias;
 import com.vaadin.flow.router.RouterLayout;
+import com.vaadin.flow.server.InvalidRouteConfigurationException;
 import com.vaadin.flow.server.InvalidRouteLayoutConfigurationException;
 
 public class AbstractRouteRegistryInitializerTest {
@@ -48,11 +50,59 @@ public static class RouteAndParentRouterLayout extends Component
 
     }
 
+    @Tag(Tag.DIV)
+    @Route("foo")
+    @RouteAlias("foo")
+    public static class RouteAndAliasWithSamePath extends Component {
+
+    }
+
+    @Tag(Tag.DIV)
+    @Route("foo")
+    @RouteAlias("bar")
+    @RouteAlias("baz")
+    @RouteAlias("bar")
+    @RouteAlias("baz")
+    @RouteAlias("hey")
+    public static class AliasesWithSamePath extends Component {
+
+    }
+
     @Test(expected = InvalidRouteLayoutConfigurationException.class)
     public void routeAndParentLayout_notRouterLayout_throws() {
         initializer.validateRouteClasses(null,
                 Stream.of(RouteAndParentLayout.class));
+    }
+
+    @Test
+    public void validateRouteClasses_samePathForRouteAndAlias_throws() {
+        InvalidRouteConfigurationException exception = Assert.assertThrows(
+                InvalidRouteConfigurationException.class,
+                () -> initializer.validateRouteClasses(null,
+                        Stream.of(RouteAndAliasWithSamePath.class)));
+        Assert.assertTrue(containsQuotedAnnotationName(exception.getMessage(),
+                Route.class));
+        Assert.assertTrue(containsQuotedAnnotationName(exception.getMessage(),
+                RouteAlias.class));
+        Assert.assertTrue(exception.getMessage().contains("same path"));
+        Assert.assertTrue(exception.getMessage().contains("foo"));
+    }
 
+    @Test
+    public void validateRouteClasses_samePathForRepeatableAlias_throws() {
+        InvalidRouteConfigurationException exception = Assert.assertThrows(
+                InvalidRouteConfigurationException.class,
+                () -> initializer.validateRouteClasses(null,
+                        Stream.of(AliasesWithSamePath.class)));
+        Assert.assertFalse(containsQuotedAnnotationName(exception.getMessage(),
+                Route.class));
+        Assert.assertTrue(containsQuotedAnnotationName(exception.getMessage(),
+                RouteAlias.class));
+        Assert.assertTrue(exception.getMessage().contains("same paths"));
+        Assert.assertTrue(exception.getMessage().contains("bar"));
+        Assert.assertTrue(exception.getMessage().contains("baz"));
+        Assert.assertFalse(exception.getMessage().contains("foo"));
+        Assert.assertFalse(exception.getMessage().contains("hey"));
     }
 
     @Test
@@ -65,4 +115,8 @@ public void routeAndParentLayout_routerLayout_returnsValidatedClass() {
                 classes.iterator().next());
     }
 
+    private static boolean containsQuotedAnnotationName(String message,
+            Class<?> clazz) {
+        return message.contains("'@" + clazz.getSimpleName() + "'");
+    }
 }