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.

Sign up for GitHub

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

Jump to bottom

fix: correct route path for MenuEntry #20235

Merged
merged 1 commit into from
Oct 14, 2024
Merged

fix: correct route path for MenuEntry #20235

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
54 changes: 49 additions & 5 deletions flow-server/src/main/java/com/vaadin/flow/internal/menu/MenuRegistry.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@
import java.util.Set;
import java.util.function.Predicate;
import java.util.stream.Collectors;
import java.util.stream.Stream;

import com.fasterxml.jackson.core.type.TypeReference;
import com.fasterxml.jackson.databind.DeserializationFeature;
Expand Down Expand Up @@ -82,10 +83,8 @@ public class MenuRegistry {
public static Map<String, AvailableViewInfo> collectMenuItems() {
Map<String, AvailableViewInfo> menuRoutes = MenuRegistry
.getMenuItems(true);
menuRoutes.entrySet()
.removeIf(entry -> Optional.ofNullable(entry.getValue())
.map(AvailableViewInfo::menu).map(MenuData::isExclude)
.orElse(false));
filterMenuItems(menuRoutes);

return menuRoutes;
}

Expand Down Expand Up @@ -115,7 +114,8 @@ public static List<AvailableViewInfo> collectMenuItemsList(Locale locale) {
return collectMenuItems().entrySet().stream().map(entry -> {
AvailableViewInfo value = entry.getValue();
return new AvailableViewInfo(value.title(), value.rolesAllowed(),
value.loginRequired(), entry.getKey(), value.lazy(),
value.loginRequired(),
getMenuLink(entry.getValue(), entry.getKey()), value.lazy(),
value.register(), value.menu(), value.children(),
value.routeParameters(), value.flowLayout());
}).sorted(getMenuOrderComparator(
Expand Down Expand Up @@ -437,6 +437,17 @@ private static void removeChildren(
}
}

private static boolean hasRequiredParameter(AvailableViewInfo viewInfo) {
final Map<String, RouteParamType> routeParameters = viewInfo
.routeParameters();
if (routeParameters != null && !routeParameters.isEmpty()
&& routeParameters.values().stream().anyMatch(
paramType -> paramType == RouteParamType.REQUIRED)) {
return true;
}
return false;
}

/**
* Check view against authentication state.
* <p>
Expand Down Expand Up @@ -540,4 +551,37 @@ private static Comparator<AvailableViewInfo> getMenuOrderComparator(
: collator.compare(o1.route(), o2.route());
};
}

private static String getMenuLink(AvailableViewInfo info,
String defaultMenuLink) {
if (info.routeParameters() == null
|| info.routeParameters().isEmpty()) {
return (defaultMenuLink.startsWith("/")) ? defaultMenuLink
: "/" + defaultMenuLink;
}
// menu link with omitted route parameters
final var parameterNames = info.routeParameters().keySet();
return Stream.of(defaultMenuLink.split("/")).filter(
part -> parameterNames.stream().noneMatch(part::startsWith))
.collect(Collectors.joining("/"));
}

private static void filterMenuItems(
Map<String, AvailableViewInfo> menuRoutes) {
for (var path : new HashSet<>(menuRoutes.keySet())) {
if (!menuRoutes.containsKey(path)) {
continue;
}
var viewInfo = menuRoutes.get(path);
// Remove following, including nested ones:
// - routes with required parameters
// - routes with exclude=true
if (viewInfo.menu().isExclude() || hasRequiredParameter(viewInfo)) {
menuRoutes.remove(path);
if (viewInfo.children() != null) {
removeChildren(menuRoutes, viewInfo, path);
}
}
}
}
}
17 changes: 9 additions & 8 deletions flow-server/src/test/java/com/vaadin/flow/server/menu/MenuConfigurationTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -143,9 +143,9 @@ public void testWithLoggedInUser_userHasRoles() throws IOException {
List<MenuEntry> menuEntries = MenuConfiguration.getMenuEntries();
Assert.assertEquals(
"List of menu items has incorrect size. Excluded menu item like /login is not expected.",
4, menuEntries.size());
assertOrder(menuEntries,
new String[] { "", "/about", "/hilla", "/hilla/sub" });
7, menuEntries.size());
assertOrder(menuEntries, new String[] { "/", "/about", "/hilla",
"/hilla/sub", "/opt_params", "/params", "/wc_params" });
}

@Test
Expand All @@ -166,9 +166,10 @@ public void getMenuItemsList_returnsCorrectPaths() throws IOException {
.forEach(routeConfiguration::setAnnotatedRoute);

List<MenuEntry> menuEntries = MenuConfiguration.getMenuEntries();
Assert.assertEquals(5, menuEntries.size());
assertOrder(menuEntries, new String[] { "", "/home", "/info", "/param",
"/param/varargs" });
Assert.assertEquals(8, menuEntries.size());
assertOrder(menuEntries,
new String[] { "/", "/home", "/info", "/opt_params", "/param",
"/param/varargs", "/params", "/wc_params" });

Map<String, MenuEntry> mapMenuItems = menuEntries.stream()
.collect(Collectors.toMap(MenuEntry::path, item -> item));
Expand Down Expand Up @@ -385,8 +386,8 @@ private void assertOrder(List<MenuEntry> menuEntries,
private void assertClientRoutes(Map<String, MenuEntry> menuOptions,
boolean authenticated, boolean hasRole) {
Assert.assertTrue("Client route '' missing",
menuOptions.containsKey(""));
Assert.assertEquals("Public", menuOptions.get("").title());
menuOptions.containsKey("/"));
Assert.assertEquals("Public", menuOptions.get("/").title());

if (authenticated) {
Assert.assertTrue("Client route 'about' missing",
Expand Down
89 changes: 72 additions & 17 deletions flow-server/src/test/java/com/vaadin/flow/server/menu/MenuRegistryTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@ public void getMenuItemsContainsExpectedClientPaths() throws IOException {
Map<String, AvailableViewInfo> menuItems = new MenuRegistry()
.getMenuItems(true);

Assert.assertEquals(2, menuItems.size());
Assert.assertEquals(8, menuItems.size());
assertClientRoutes(menuItems);
}

Expand All @@ -157,7 +157,7 @@ public void getMenuItemsNoFilteringContainsAllClientPaths()
Map<String, AvailableViewInfo> menuItems = new MenuRegistry()
.getMenuItems(false);

Assert.assertEquals(5, menuItems.size());
Assert.assertEquals(11, menuItems.size());
// Validate as if logged in as all routes should be available
assertClientRoutes(menuItems, true, true, false);
}
Expand All @@ -184,7 +184,7 @@ public void productionMode_getMenuItemsContainsExpectedClientPaths()
Map<String, AvailableViewInfo> menuItems = new MenuRegistry()
.getMenuItems(true);

Assert.assertEquals(2, menuItems.size());
Assert.assertEquals(8, menuItems.size());
assertClientRoutes(menuItems);
}
}
Expand Down Expand Up @@ -218,7 +218,7 @@ public void getMenuItemsContainBothClientAndServerPaths()
Map<String, AvailableViewInfo> menuItems = new MenuRegistry()
.getMenuItems(true);

Assert.assertEquals(4, menuItems.size());
Assert.assertEquals(10, menuItems.size());
assertClientRoutes(menuItems);
assertServerRoutes(menuItems);
}
Expand All @@ -239,7 +239,7 @@ public void collectMenuItems_returnsCorrectPaths() throws IOException {
Map<String, AvailableViewInfo> menuItems = MenuRegistry
.collectMenuItems();

Assert.assertEquals(5, menuItems.size());
Assert.assertEquals(8, menuItems.size());
assertClientRoutes(menuItems, false, false, true);
assertServerRoutes(menuItems);
assertServerRoutesWithParameters(menuItems, true);
Expand All @@ -259,17 +259,17 @@ public void testWithLoggedInUser_userHasRoles() throws IOException {
Map<String, AvailableViewInfo> menuItems = new MenuRegistry()
.getMenuItems(true);

Assert.assertEquals(5, menuItems.size());
Assert.assertEquals(11, menuItems.size());
assertClientRoutes(menuItems, true, true, false);

// Verify that getMenuItemsList returns the same data
List<AvailableViewInfo> menuItemsList = MenuRegistry
.collectMenuItemsList();
Assert.assertEquals(
"List of menu items has incorrect size. Excluded menu item like /login is not expected.",
4, menuItemsList.size());
assertOrder(menuItemsList,
new String[] { "", "/about", "/hilla", "/hilla/sub" });
7, menuItemsList.size());
assertOrder(menuItemsList, new String[] { "/", "/about", "/hilla",
"/hilla/sub", "/opt_params", "/params", "/wc_params" });
}

@Test
Expand All @@ -286,7 +286,7 @@ public void testWithLoggedInUser_noMatchingRoles() throws IOException {
Map<String, AvailableViewInfo> menuItems = new MenuRegistry()
.getMenuItems(true);

Assert.assertEquals(3, menuItems.size());
Assert.assertEquals(9, menuItems.size());
assertClientRoutes(menuItems, true, false, false);
}

Expand All @@ -304,14 +304,15 @@ public void getMenuItemsList_returnsCorrectPaths() throws IOException {
.forEach(routeConfiguration::setAnnotatedRoute);

List<AvailableViewInfo> menuItems = MenuRegistry.collectMenuItemsList();
Assert.assertEquals(5, menuItems.size());
assertOrder(menuItems, new String[] { "", "/home", "/info", "/param",
"/param/varargs" });
Assert.assertEquals(8, menuItems.size());
assertOrder(menuItems,
new String[] { "/", "/home", "/info", "/opt_params", "/param",
"/param/varargs", "/params", "/wc_params" });
// verifying that data is same as with collectMenuItems
Map<String, AvailableViewInfo> mapMenuItems = menuItems.stream()
.collect(Collectors.toMap(AvailableViewInfo::route,
item -> item));
assertClientRoutes(mapMenuItems, false, false, true);
assertClientRoutes(mapMenuItems, false, false, true, "/");
assertServerRoutes(mapMenuItems);
assertServerRoutesWithParameters(mapMenuItems, true);
}
Expand Down Expand Up @@ -343,10 +344,18 @@ private void assertClientRoutes(Map<String, AvailableViewInfo> menuItems) {

private void assertClientRoutes(Map<String, AvailableViewInfo> menuItems,
boolean authenticated, boolean hasRole, boolean excludeExpected) {
Assert.assertTrue("Client route '' missing", menuItems.containsKey(""));
Assert.assertEquals("Public", menuItems.get("").title());
assertClientRoutes(menuItems, authenticated, hasRole, excludeExpected,
"");
}

private void assertClientRoutes(Map<String, AvailableViewInfo> menuItems,
boolean authenticated, boolean hasRole, boolean excludeExpected,
String expectedRootPath) {
Assert.assertTrue("Client route '" + expectedRootPath + "' missing",
menuItems.containsKey(expectedRootPath));
Assert.assertEquals("Public", menuItems.get(expectedRootPath).title());
Assert.assertNotNull("Public should contain default menu data",
menuItems.get("").menu());
menuItems.get(expectedRootPath).menu());

if (authenticated) {
Assert.assertTrue("Client route 'about' missing",
Expand Down Expand Up @@ -576,6 +585,52 @@ public Instantiator getInstantiator() {
}
]
},
{
"route": "wc_params/:param?",
"loginRequired": false,
"params": {
":param": "*"
},
"title": "wc_params path is included in menu"
},
{
"route": "opt_params/:param?",
"loginRequired": false,
"params": {
":param": "opt"
},
"title": "opt_params path is included in menu"
},
{
"route": "req_params/:param",
"loginRequired": false,
"params": {
":param": "req"
},
"title": "req_params path is excluded from menu"
},
{
"route": "params",
"loginRequired": false,
"title": "params path is shown in menu",
"children": [
{
"route": ":param",
"loginRequired": false,
"params": {
":param": "req"
},
"title": "params/:param path is excluded from menu",
"children": [
{
"route": "sub",
"loginRequired": false,
"title": "params/:param/sub path is excluded from menu"
}
]
}
]
},
{
"route": "login",
"menu": {
Expand Down
Loading