Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: check for client route conflicts #20188

Open
wants to merge 19 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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();
}

}

}
Original file line number Diff line number Diff line change
Expand Up @@ -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));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;

/**
Expand Down Expand Up @@ -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.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -316,6 +316,9 @@ public void init() throws ServiceException {
UsageStatistics.markAsUsed("flow/bun", null);
}

RouteUtil.checkForClientRouteCollisions(this,
getRouteRegistry().getRegisteredRoutes());

initialized = true;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand All @@ -44,8 +42,8 @@ public class UIInternalsTest {

@Mock
UI ui;
@Mock
VaadinService vaadinService;

MockVaadinServletService vaadinService;

UIInternals internals;

Expand Down Expand Up @@ -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);
}
Expand Down Expand Up @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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();
Expand Down
Original file line number Diff line number Diff line change
@@ -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;
Expand Down Expand Up @@ -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);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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);
Expand All @@ -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);
}

Expand Down
Loading
Loading