Skip to content

Commit

Permalink
fix: client route collisions (#20327) (#20330)
Browse files Browse the repository at this point in the history 
Fix the client server route
collision to not take into
account client layouts as
they are not route targets.

Co-authored-by: caalador <mikael.grankvist@vaadin.com>
  • Loading branch information
@vaadin-bot @caalador
vaadin-bot and caalador authored Oct 24, 2024
1 parent 6d299d3 commit 248d699
Show file tree
Hide file tree
Showing 3 changed files with 103 additions and 3 deletions.
4 changes: 3 additions & 1 deletion flow-server/src/main/java/com/vaadin/flow/router/internal/RouteUtil.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -641,7 +641,9 @@ public static void checkForClientRouteCollisions(VaadinService service,
List<String> collisions = MenuRegistry
.collectClientMenuItems(false,
service.getDeploymentConfiguration())
.keySet().stream().map(PathUtil::trimPath)
.entrySet().stream()
.filter(entry -> entry.getValue().children() == null)
.map(Map.Entry::getKey).map(PathUtil::trimPath)
.filter(clientRoute -> Arrays.stream(flowRouteTemplates)
.map(PathUtil::trimPath).anyMatch(clientRoute::equals))
.toList();
Expand Down
98 changes: 98 additions & 0 deletions flow-server/src/test/java/com/vaadin/flow/router/internal/RouteUtilTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,9 @@
*/
package com.vaadin.flow.router.internal;

import java.io.File;
import java.lang.reflect.Field;
import java.util.Arrays;
import java.util.Collections;
import java.util.HashMap;
import java.util.List;
Expand All @@ -25,22 +27,34 @@
import org.hamcrest.MatcherAssert;
import org.hamcrest.collection.IsIterableContainingInOrder;
import org.junit.Assert;
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;
import com.vaadin.flow.component.Tag;
import com.vaadin.flow.function.DeploymentConfiguration;
import com.vaadin.flow.internal.ReflectTools;
import com.vaadin.flow.internal.menu.MenuRegistry;
import com.vaadin.flow.router.Layout;
import com.vaadin.flow.router.Menu;
import com.vaadin.flow.router.ParentLayout;
import com.vaadin.flow.router.Route;
import com.vaadin.flow.router.RouteAlias;
import com.vaadin.flow.router.RouteConfiguration;
import com.vaadin.flow.router.RoutePrefix;
import com.vaadin.flow.router.RouterLayout;
import com.vaadin.flow.server.InvalidRouteConfigurationException;
import com.vaadin.flow.server.MockVaadinContext;
import com.vaadin.flow.server.MockVaadinServletService;
import com.vaadin.flow.server.SessionRouteRegistry;
import com.vaadin.flow.server.VaadinContext;
import com.vaadin.flow.server.VaadinService;
import com.vaadin.flow.server.frontend.BundleUtils;
import com.vaadin.flow.server.frontend.FrontendUtils;
import com.vaadin.flow.server.menu.AvailableViewInfo;
import com.vaadin.flow.server.startup.ApplicationRouteRegistry;
import com.vaadin.tests.util.AlwaysLockedVaadinSession;

Expand All @@ -50,6 +64,9 @@
*/
public class RouteUtilTest {

@Rule
public ExpectedException expectedEx = ExpectedException.none();

@Tag(Tag.DIV)
public static class Parent extends Component implements RouterLayout {
}
Expand Down Expand Up @@ -1017,6 +1034,87 @@ class View extends Component {
registry.hasLayout("auto"));
}

@Test
public void clientHasMappedLayout_validateNoClientRouteCollisions() {
Map<String, AvailableViewInfo> clientRoutes = new HashMap<>();

clientRoutes.put("", new AvailableViewInfo("public", null, false, "",
false, false, null, null, null, false));
clientRoutes
.put("/flow",
new AvailableViewInfo("public", null, false, "", false,
false, null,
Arrays.asList(new AvailableViewInfo("child",
null, false, "", false, false, null,
null, null, false)),
null, false));
clientRoutes.put("/hilla/components", new AvailableViewInfo("public",
null, false, "", false, false, null, null, null, false));
clientRoutes.put("/hilla", new AvailableViewInfo("public", null, false,
"", false, false, null, null, null, false));

try (MockedStatic<MenuRegistry> registry = Mockito
.mockStatic(MenuRegistry.class, Mockito.CALLS_REAL_METHODS);
MockedStatic<FrontendUtils> frontendUtils = Mockito.mockStatic(
FrontendUtils.class, Mockito.CALLS_REAL_METHODS);) {
VaadinService service = Mockito.mock(VaadinService.class);
DeploymentConfiguration conf = Mockito
.mock(DeploymentConfiguration.class);
Mockito.when(service.getDeploymentConfiguration()).thenReturn(conf);
Mockito.when(conf.isProductionMode()).thenReturn(false);
Mockito.when(conf.getFrontendFolder())
.thenReturn(Mockito.mock(File.class));

registry.when(
() -> MenuRegistry.collectClientMenuItems(false, conf))
.thenReturn(clientRoutes);
frontendUtils.when(() -> FrontendUtils.isHillaUsed(Mockito.any()))
.thenReturn(true);

RouteUtil.checkForClientRouteCollisions(service, "flow",
"flow/hello-world", "hilla/flow");
}
}

@Test
public void clientHasOverlappingTarget_validateClientRouteCollision() {
expectedEx.expect(InvalidRouteConfigurationException.class);
expectedEx.expectMessage(
"Invalid route configuration. The following Hilla route(s) conflict with configured Flow routes: flow");
Map<String, AvailableViewInfo> clientRoutes = new HashMap<>();

clientRoutes.put("", new AvailableViewInfo("public", null, false, "",
false, false, null, null, null, false));
clientRoutes.put("/flow", new AvailableViewInfo("public", null, false,
"", false, false, null, null, null, false));
clientRoutes.put("/hilla/components", new AvailableViewInfo("public",
null, false, "", false, false, null, null, null, false));
clientRoutes.put("/hilla", new AvailableViewInfo("public", null, false,
"", false, false, null, null, null, false));

try (MockedStatic<MenuRegistry> registry = Mockito
.mockStatic(MenuRegistry.class, Mockito.CALLS_REAL_METHODS);
MockedStatic<FrontendUtils> frontendUtils = Mockito.mockStatic(
FrontendUtils.class, Mockito.CALLS_REAL_METHODS);) {
VaadinService service = Mockito.mock(VaadinService.class);
DeploymentConfiguration conf = Mockito
.mock(DeploymentConfiguration.class);
Mockito.when(service.getDeploymentConfiguration()).thenReturn(conf);
Mockito.when(conf.isProductionMode()).thenReturn(false);
Mockito.when(conf.getFrontendFolder())
.thenReturn(Mockito.mock(File.class));

registry.when(
() -> MenuRegistry.collectClientMenuItems(false, conf))
.thenReturn(clientRoutes);
frontendUtils.when(() -> FrontendUtils.isHillaUsed(Mockito.any()))
.thenReturn(true);

RouteUtil.checkForClientRouteCollisions(service, "flow",
"flow/hello-world", "hilla/flow");
}
}

@SuppressWarnings("unchecked")
private void tamperLayouts(ApplicationRouteRegistry registry,
Consumer<Map<String, Class<? extends RouterLayout>>> consumer) {
Expand Down
4 changes: 2 additions & 2 deletions flow-server/src/test/java/com/vaadin/flow/server/menu/MenuRegistryTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -581,8 +581,8 @@ public static class MyInfo extends Component {
}

@Tag("div")
@Route("hilla")
@Menu(title = "hilla")
@Route("about")
@Menu(title = "about")
public static class ConflictRoute extends Component {
}

Expand Down

0 comments on commit 248d699

Please sign in to comment.