Skip to content

Commit

Permalink
fix: support client route parameters in auto layout path matching
Browse files Browse the repository at this point in the history 
Improves navigation path matching with client route templates. Adds support for matching with route parameters to find automatic Flow main layout for client routes.

Fixes: #20268
  • Loading branch information
@tltv
tltv committed Oct 18, 2024
1 parent 3bd7a7b commit 173766a
Show file tree
Hide file tree
Showing 8 changed files with 263 additions and 20 deletions.
24 changes: 24 additions & 0 deletions flow-server/src/main/java/com/vaadin/flow/internal/menu/MenuRegistry.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -528,6 +528,16 @@ private static boolean hasRequiredParameter(AvailableViewInfo viewInfo) {
return false;
}

private static boolean hasOptionalOrVarargsParameter(
AvailableViewInfo viewInfo) {
final Map<String, RouteParamType> routeParameters = viewInfo
.routeParameters();
return routeParameters != null && !routeParameters.isEmpty()
&& routeParameters.values().stream().anyMatch(
paramType -> paramType == RouteParamType.OPTIONAL
|| paramType == RouteParamType.WILDCARD);
}

/**
* Check view against authentication state.
* <p>
Expand Down Expand Up @@ -666,12 +676,26 @@ private static void filterMenuItems(
// Remove following, including nested ones:
// - routes with required parameters
// - routes with exclude=true
// Remove following without including nested ones:
// - routes with direct children with optional or varargs parameter
if (viewInfo.menu().isExclude() || hasRequiredParameter(viewInfo)) {
menuRoutes.remove(path);
if (viewInfo.children() != null) {
removeChildren(menuRoutes, viewInfo, path);
}
} else if (childrenHasRouteParameter(viewInfo.children())) {
menuRoutes.remove(path);
}
}
}

private static boolean childrenHasRouteParameter(
List<AvailableViewInfo> children) {
if (children == null || children.isEmpty()) {
return false;
}
return children.stream()
.anyMatch(viewInfo -> viewInfo.routeParameters() != null
&& !viewInfo.routeParameters().isEmpty());
}
}
11 changes: 8 additions & 3 deletions flow-server/src/main/java/com/vaadin/flow/router/internal/DefaultRouteResolver.java
View file
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.Collections;
import java.util.Optional;

import org.slf4j.LoggerFactory;

Expand Down Expand Up @@ -47,10 +48,14 @@ public NavigationState resolve(ResolveRequest request) {
.getNavigationRouteTarget(path);

if (!navigationResult.hasTarget()) {
if (MenuRegistry.hasClientRoute(path)) {
Optional<String> clientNavigationTargetPath = RouteUtil
.getClientNavigationRouteTargetTemplate(path);
if (clientNavigationTargetPath.isPresent()) {
String clientPath = clientNavigationTargetPath.get();
AvailableViewInfo viewInfo = MenuRegistry.getClientRoutes(false)
.get(path.isEmpty() ? path
: path.startsWith("/") ? path : "/" + path);
.get(clientPath.isEmpty() ? clientPath
: clientPath.startsWith("/") ? clientPath
: "/" + clientPath);
if (viewInfo != null && viewInfo.flowLayout()) {

Class<? extends Component> layout = (Class<? extends Component>) registry
Expand Down
27 changes: 27 additions & 0 deletions flow-server/src/main/java/com/vaadin/flow/router/internal/RouteTarget.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,8 @@ public class RouteTarget implements Serializable {
private final boolean annotatedRoute;
private final boolean registeredAtStartup;

private String template;

/**
* Create a new Route target holder with the given target registered.
*
Expand Down Expand Up @@ -74,6 +76,23 @@ public RouteTarget(Class<? extends Component> target) {
this(target, null);
}

/**
* Create a new Route target holder with the given target registered and
* route template and empty parent layouts.
*
* @param target
* navigation target
* @param parents
* parent layout chain
* @param template
* route template
*/
RouteTarget(Class<? extends Component> target,
List<Class<? extends RouterLayout>> parents, String template) {
this(target, parents);
this.template = template;
}

/**
* Get the component route target.
*
Expand Down Expand Up @@ -126,4 +145,12 @@ boolean isRegisteredAtStartup() {
return registeredAtStartup;
}

/**
* Get the route template.
*
* @return route template
*/
String getTemplate() {
return template;
}
}
32 changes: 32 additions & 0 deletions 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 @@ -50,6 +50,7 @@
import com.vaadin.flow.router.RoutePrefix;
import com.vaadin.flow.router.RouterLayout;
import com.vaadin.flow.server.AbstractConfiguration;
import com.vaadin.flow.server.AmbiguousRouteConfigurationException;
import com.vaadin.flow.server.RouteRegistry;
import com.vaadin.flow.server.SessionRouteRegistry;
import com.vaadin.flow.server.VaadinContext;
Expand Down Expand Up @@ -660,4 +661,35 @@ public static Optional<String> getDynamicTitle(UI ui) {
.map(element -> ((HasDynamicTitle) element).getPageTitle())
.filter(Objects::nonNull).findFirst();
}

/**
* Search for a client route using given navigation url and return target
* template.
*
* @param url
* the navigation url used to search a route target.
*
* @return a {@link Optional} containing the template of the client route
* target or an empty {@link Optional}.
*/
public static Optional<String> getClientNavigationRouteTargetTemplate(
String url) {
if (url == null) {
return Optional.empty();
}
RouteModel routeModel = RouteModel.create(true);
MenuRegistry.getClientRoutes(false).forEach((key, value) -> {
try {
routeModel.addRoute(key,
new RouteTarget(Component.class, null, key));
} catch (AmbiguousRouteConfigurationException tolerate) {
// tolerate ambiguous routes. First added route wins and
// declares returned template.
}
});
url = url.isEmpty() ? url : url.startsWith("/") ? url : "/" + url;
return Optional.ofNullable(routeModel.getNavigationRouteTarget(url))
.map(NavigationRouteTarget::getRouteTarget)
.map(RouteTarget::getTemplate);
}
}
2 changes: 1 addition & 1 deletion flow-server/src/main/java/com/vaadin/flow/server/menu/MenuConfiguration.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,7 @@ private static Optional<String> getPageHeaderFromMenuItems() {
.filter(menuEntry -> PathUtil.trimPath(menuEntry.getKey())
.equals(activeLocation))
.map(Map.Entry::getValue).map(AvailableViewInfo::title)
.findFirst();
.filter(Objects::nonNull).findFirst();
}
return Optional.empty();
}
Expand Down
142 changes: 138 additions & 4 deletions flow-server/src/test/java/com/vaadin/flow/router/DefaultRouteResolverTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@
import java.util.ArrayList;
import java.util.Collections;
import java.util.HashMap;
import java.util.LinkedHashMap;
import java.util.Map;
import java.util.Set;
import java.util.stream.Collectors;
import java.util.stream.Stream;
Expand Down Expand Up @@ -136,8 +138,6 @@ public void clientRouteRequest_getDefinedLayout() {

try (MockedStatic<MenuRegistry> menuRegistry = Mockito
.mockStatic(MenuRegistry.class)) {
menuRegistry.when(() -> MenuRegistry.hasClientRoute(path))
.thenReturn(true);
menuRegistry.when(() -> MenuRegistry.getClientRoutes(false))
.thenReturn(Collections.singletonMap("/route",
new AvailableViewInfo("", null, false, "/route",
Expand All @@ -149,6 +149,142 @@ public void clientRouteRequest_getDefinedLayout() {
}
}

@Test
public void clientRouteRequest_withRouteParameters_getDefinedLayout() {
router.getRegistry().setLayout(DefaultLayout.class);

try (MockedStatic<MenuRegistry> menuRegistry = Mockito
.mockStatic(MenuRegistry.class)) {
menuRegistry.when(() -> MenuRegistry.getClientRoutes(false))
// @formatter:off
.thenReturn(buildClientRoutes(
"/route/:param",
"/opt_route/:param?",
"/wildcard_route/:param*",
"/one/:param/two/:param2",
"/foo/:param?/bar/:param2?"));
Stream.of(
"route/1",
"/route/abc",
"/opt_route",
"/opt_route/",
"/opt_route/1",
"/wildcard_route",
"wildcard_route",
"wildcard_route/",
"wildcard_route/1",
"/wildcard_route/1/2",
"one/1/two/2",
"foo/bar",
"foo/a/bar",
"foo/bar/b",
"foo/a/bar/b")
// @formatter:on
.forEach(path -> {
String msg = String.format(
"Layout should be returned for path '%s' a non server route when matching @Layout exists",
path);
var state = resolveNavigationState(path);
Assert.assertNotNull(msg, state);
Assert.assertEquals(msg, DefaultLayout.class,
resolveNavigationState(path).getRouteTarget()
.getTarget());
});
}
}

@Test
public void clientRouteRequest_withRouteParameters_noLayout() {
router.getRegistry().setLayout(DefaultLayout.class);

try (MockedStatic<MenuRegistry> menuRegistry = Mockito
.mockStatic(MenuRegistry.class)) {
menuRegistry.when(() -> MenuRegistry.getClientRoutes(false))
// @formatter:off
.thenReturn(buildClientRoutes(
"/route/:param",
"/opt_route/:param?",
"/wildcard_route/:param*",
"/one/:param/two/:param2",
"/foo/:param?/bar/:param2?"));
// @formatter:off
Stream.of(
"route",
"/route/abc/def",
"/unknown_route",
"/opt_route/1/2",
"/",
"",
"one/1/two/2/3",
"one/two/2",
"one/two/",
"foo",
"foo/foo/foo/bar/bar/")
// @formatter:on
.forEach(path -> {
Assert.assertNull(String.format(
"Layout should not be returned for a non server route '%s' when matching @Layout doesn't exist",
path), resolveNavigationState(path));
});
}
}

/**
* Ambiguous routes should be resolved by the order they were added to the
* RouteModel. First added wins. Ambiguous route is detected by
* AmbiguousRouteConfigurationException.
*/
@Test
public void clientRouteRequest_withRouteParameters_ambiguousRoutesFirstAddedWins() {
router.getRegistry().setLayout(DefaultLayout.class);

try (MockedStatic<MenuRegistry> menuRegistry = Mockito
.mockStatic(MenuRegistry.class)) {
menuRegistry.when(() -> MenuRegistry.getClientRoutes(false))
// @formatter:off
.thenReturn(buildClientRoutes(
"/route",
"/route/:param?",
"/foo",
"/foo/:param*"));
// @formatter:off
Stream.of(
"route",
"foo",
"foo/1"
)
// @formatter:on
.forEach(path -> {
String msg = String.format(
"Layout should be returned for path '%s' a non server route when matching @Layout exists",
path);
var state = resolveNavigationState(path);
Assert.assertNotNull(msg, state);
Assert.assertEquals(msg, DefaultLayout.class,
resolveNavigationState(path).getRouteTarget()
.getTarget());
});
Stream.of("route/1").forEach(path -> {
Assert.assertNull(String.format(
"Layout should not be returned for a non server route '%s' when matching @Layout doesn't exist",
path), resolveNavigationState(path));
});
}
}

private Map<String, AvailableViewInfo> buildClientRoutes(String... routes) {
Map<String, AvailableViewInfo> clientRoutes = new LinkedHashMap<>();
for (String route : routes) {
clientRoutes.put(route, createAvailableViewInfo(route));
}
return clientRoutes;
}

private AvailableViewInfo createAvailableViewInfo(String route) {
return new AvailableViewInfo("", null, false, route, false, false, null,
null, null, true);
}

@Test
public void clientRouteRequest_noLayoutForPath_Throws() {
expectedEx.expect(NotFoundException.class);
Expand All @@ -158,8 +294,6 @@ public void clientRouteRequest_noLayoutForPath_Throws() {

try (MockedStatic<MenuRegistry> menuRegistry = Mockito
.mockStatic(MenuRegistry.class)) {
menuRegistry.when(() -> MenuRegistry.hasClientRoute(path))
.thenReturn(true);
menuRegistry.when(() -> MenuRegistry.getClientRoutes(false))
.thenReturn(Collections.singletonMap("/route",
new AvailableViewInfo("", null, false, "/route",
Expand Down
9 changes: 6 additions & 3 deletions flow-server/src/test/java/com/vaadin/flow/server/menu/MenuConfigurationTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -144,8 +144,10 @@ public void testWithLoggedInUser_userHasRoles() throws IOException {
Assert.assertEquals(
"List of menu items has incorrect size. Excluded menu item like /login is not expected.",
7, menuEntries.size());
assertOrder(menuEntries, new String[] { "/", "/about", "/hilla",
"/hilla/sub", "/opt_params", "/params", "/wc_params" });
assertOrder(menuEntries,
new String[] { "/", "/about", "/hilla", "/hilla/sub",
"/opt_params", "/params_with_opt_children",
"/wc_params" });
}

@Test
Expand All @@ -169,7 +171,8 @@ public void getMenuItemsList_returnsCorrectPaths() throws IOException {
Assert.assertEquals(8, menuEntries.size());
assertOrder(menuEntries,
new String[] { "/", "/home", "/info", "/opt_params", "/param",
"/param/varargs", "/params", "/wc_params" });
"/param/varargs", "/params_with_opt_children",
"/wc_params" });

Map<String, MenuEntry> mapMenuItems = menuEntries.stream()
.collect(Collectors.toMap(MenuEntry::path, item -> item));
Expand Down
Loading

0 comments on commit 173766a

Please sign in to comment.