From fd7d674a3943b977af569be15916900fa0381a5d Mon Sep 17 00:00:00 2001 From: Vaadin Bot Date: Thu, 3 Oct 2024 07:59:16 +0200 Subject: [PATCH] fix: consider layout prefixes when checkin for route and alias paths (#20126) (CP: 23.5) (#20129) * fix: consider layout prefixes when checking for route and alias paths (#20126) Fixes #20124 * fix test --------- Co-authored-by: Marco Collovati --- .../AbstractRouteRegistryInitializer.java | 20 ++-- .../AbstractRouteRegistryInitializerTest.java | 100 +++++++++++++++++- 2 files changed, 108 insertions(+), 12 deletions(-) diff --git a/flow-server/src/main/java/com/vaadin/flow/server/startup/AbstractRouteRegistryInitializer.java b/flow-server/src/main/java/com/vaadin/flow/server/startup/AbstractRouteRegistryInitializer.java index bf897da7b1c..623a2e702b5 100644 --- a/flow-server/src/main/java/com/vaadin/flow/server/startup/AbstractRouteRegistryInitializer.java +++ b/flow-server/src/main/java/com/vaadin/flow/server/startup/AbstractRouteRegistryInitializer.java @@ -96,23 +96,27 @@ private void checkForConflictingAnnotations(VaadinContext context, RouteAlias[] aliases = route.getAnnotationsByType(RouteAlias.class); if (aliases.length > 0) { - Route routeAnnotation = route.getAnnotation(Route.class); + String routePath = RouteUtil.getRoutePath(context, route); Map stats = Arrays.stream(aliases) - .map(RouteAlias::value).collect(Collectors.groupingBy( - Function.identity(), Collectors.counting())); - if (stats.containsKey(routeAnnotation.value())) { + .map(ann -> RouteUtil.getRouteAliasPath(route, ann)) + .collect(Collectors.groupingBy(Function.identity(), + Collectors.counting())); + if (stats.containsKey(routePath)) { throw new InvalidRouteConfigurationException(String.format( - "'%s' declares '@%s' and '@%s' with the same path '%s'", + "'%s' declares '@%s' and '@%s' with the same path '%s'. " + + "Make sure paths are different by checking annotation values " + + "and prefixes defined by layouts", route.getCanonicalName(), Route.class.getSimpleName(), - RouteAlias.class.getSimpleName(), - routeAnnotation.value())); + RouteAlias.class.getSimpleName(), routePath)); } 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.", + "'%s' declares multiple '@%s' with same paths: %s." + + "Make sure paths are different by checking annotation values " + + "and prefixes defined by layouts.", route.getCanonicalName(), RouteAlias.class.getSimpleName(), repeatedAliases)); } diff --git a/flow-server/src/test/java/com/vaadin/flow/server/startup/AbstractRouteRegistryInitializerTest.java b/flow-server/src/test/java/com/vaadin/flow/server/startup/AbstractRouteRegistryInitializerTest.java index d1aaca20045..09924ff7bc0 100644 --- a/flow-server/src/test/java/com/vaadin/flow/server/startup/AbstractRouteRegistryInitializerTest.java +++ b/flow-server/src/test/java/com/vaadin/flow/server/startup/AbstractRouteRegistryInitializerTest.java @@ -12,16 +12,23 @@ import java.util.stream.Stream; import org.junit.Assert; +import org.junit.Before; import org.junit.Test; +import org.mockito.Mockito; import com.vaadin.flow.component.Component; import com.vaadin.flow.component.Tag; +import com.vaadin.flow.di.Lookup; +import com.vaadin.flow.router.DefaultRoutePathProvider; import com.vaadin.flow.router.ParentLayout; import com.vaadin.flow.router.Route; import com.vaadin.flow.router.RouteAlias; +import com.vaadin.flow.router.RoutePathProvider; +import com.vaadin.flow.router.RoutePrefix; import com.vaadin.flow.router.RouterLayout; import com.vaadin.flow.server.InvalidRouteConfigurationException; import com.vaadin.flow.server.InvalidRouteLayoutConfigurationException; +import com.vaadin.flow.server.VaadinContext; public class AbstractRouteRegistryInitializerTest { @@ -29,6 +36,15 @@ public class AbstractRouteRegistryInitializerTest { }; + VaadinContext context = Mockito.mock(VaadinContext.class); + + @Before + public void setUp() throws Exception { + Lookup lookup = Lookup.of(new DefaultRoutePathProvider(), + RoutePathProvider.class); + Mockito.when(context.getAttribute(Lookup.class)).thenReturn(lookup); + } + @Tag(Tag.DIV) public static class TestParentLayout extends Component implements RouterLayout { @@ -57,6 +73,43 @@ public static class RouteAndAliasWithSamePath extends Component { } + @Tag(Tag.DIV) + @RoutePrefix("parent") + public static class PrefixedParentLayout extends Component + implements RouterLayout { + } + + @Tag(Tag.DIV) + @RoutePrefix("nested") + @ParentLayout(PrefixedParentLayout.class) + public static class NestedPrefixedParentLayout extends Component + implements RouterLayout { + } + + @Tag(Tag.DIV) + @Route("foo") + @RouteAlias(value = "foo", layout = PrefixedParentLayout.class) + public static class RouteAndAliasWithSamePathDifferentLayoutPrefix + extends Component { + + } + + @Tag(Tag.DIV) + @Route(value = "foo", layout = PrefixedParentLayout.class) + @RouteAlias(value = "foo", layout = PrefixedParentLayout.class) + public static class RouteAndAliasWithSamePathSameLayoutPrefix + extends Component { + + } + + @Tag(Tag.DIV) + @Route(value = "foo", layout = NestedPrefixedParentLayout.class) + @RouteAlias(value = "foo", layout = NestedPrefixedParentLayout.class) + public static class RouteAndAliasWithSamePathSameNestedLayoutPrefix + extends Component { + + } + @Tag(Tag.DIV) @Route("foo") @RouteAlias("bar") @@ -70,7 +123,7 @@ public static class AliasesWithSamePath extends Component { @Test(expected = InvalidRouteLayoutConfigurationException.class) public void routeAndParentLayout_notRouterLayout_throws() { - initializer.validateRouteClasses(null, + initializer.validateRouteClasses(context, Stream.of(RouteAndParentLayout.class)); } @@ -78,7 +131,7 @@ public void routeAndParentLayout_notRouterLayout_throws() { public void validateRouteClasses_samePathForRouteAndAlias_throws() { InvalidRouteConfigurationException exception = Assert.assertThrows( InvalidRouteConfigurationException.class, - () -> initializer.validateRouteClasses(null, + () -> initializer.validateRouteClasses(context, Stream.of(RouteAndAliasWithSamePath.class))); Assert.assertTrue(containsQuotedAnnotationName(exception.getMessage(), Route.class)); @@ -92,7 +145,7 @@ public void validateRouteClasses_samePathForRouteAndAlias_throws() { public void validateRouteClasses_samePathForRepeatableAlias_throws() { InvalidRouteConfigurationException exception = Assert.assertThrows( InvalidRouteConfigurationException.class, - () -> initializer.validateRouteClasses(null, + () -> initializer.validateRouteClasses(context, Stream.of(AliasesWithSamePath.class))); Assert.assertFalse(containsQuotedAnnotationName(exception.getMessage(), Route.class)); @@ -105,10 +158,49 @@ public void validateRouteClasses_samePathForRepeatableAlias_throws() { Assert.assertFalse(exception.getMessage().contains("hey")); } + @Test + public void validateRouteClasses_samePathForRouteAndAlias_sameLayoutPrefix_throws() { + InvalidRouteConfigurationException exception = Assert.assertThrows( + InvalidRouteConfigurationException.class, + () -> initializer.validateRouteClasses(context, Stream + .of(RouteAndAliasWithSamePathSameLayoutPrefix.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_samePathForRouteAndAlias_sameNestedLayoutPrefix_throws() { + InvalidRouteConfigurationException exception = Assert.assertThrows( + InvalidRouteConfigurationException.class, + () -> initializer.validateRouteClasses(context, Stream.of( + RouteAndAliasWithSamePathSameNestedLayoutPrefix.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_samePathForRouteAndAlias_differentLayoutPrefix_doNotThrow() { + Set> classes = initializer + .validateRouteClasses(context, Stream.of( + RouteAndAliasWithSamePathDifferentLayoutPrefix.class)); + Assert.assertEquals(1, classes.size()); + Assert.assertEquals( + RouteAndAliasWithSamePathDifferentLayoutPrefix.class, + classes.iterator().next()); + } + @Test public void routeAndParentLayout_routerLayout_returnsValidatedClass() { Set> classes = initializer - .validateRouteClasses(null, + .validateRouteClasses(context, Stream.of(RouteAndParentRouterLayout.class)); Assert.assertEquals(1, classes.size()); Assert.assertEquals(RouteAndParentRouterLayout.class,