Skip to content

Commit

Permalink
fix: prevent Flow navigation for Hilla route (#19875)
Browse files Browse the repository at this point in the history 
* fix: prevent Flow navigation for Hilla route

* Add test and javadoc

* Remove null test; add mock for failing test

* fix mockstatics

---------

Co-authored-by: Mikhail Shabarov <[email protected]>
  • Loading branch information
@tepi @mshabarov
tepi and mshabarov authored Sep 4, 2024
1 parent 8cfca6e commit 4363bbf
Show file tree
Hide file tree
Showing 4 changed files with 124 additions and 19 deletions.
7 changes: 7 additions & 0 deletions ...server/src/main/java/com/vaadin/flow/router/internal/AbstractNavigationStateRenderer.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@
import com.vaadin.flow.server.Constants;
import com.vaadin.flow.server.HttpStatusCode;
import com.vaadin.flow.server.VaadinSession;
import com.vaadin.flow.server.menu.MenuRegistry;

/**
* Base class for navigation handlers that target a navigation state.
Expand Down Expand Up @@ -174,6 +175,12 @@ public int handle(NavigationEvent event) {
return result.get();
}

// If navigation target is Hilla route, terminate Flow navigation logic
// here.
if (MenuRegistry.hasClientRoute(event.getLocation().getPath(), true)) {
return HttpStatusCode.OK.getCode();
}

final ArrayList<HasElement> chain;

final boolean preserveOnRefreshTarget = isPreserveOnRefreshTarget(
Expand Down
29 changes: 26 additions & 3 deletions flow-server/src/main/java/com/vaadin/flow/server/menu/MenuRegistry.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -408,13 +408,28 @@ public static ClassLoader getClassLoader() {
}

/**
* See if there is a client route available for give route path.
* See if there is a client route available for given route path.
*
* @param route
* route path to check
* @return true if a client route is found.
*/
public static boolean hasClientRoute(String route) {
return hasClientRoute(route, false);
}

/**
* See if there is a client route available for given route path, optionally
* excluding layouts (routes with children) from the check.
*
* @param route
* route path to check
* @param excludeLayouts
* {@literal true} to exclude layouts from the check,
* {@literal false} to include them
* @return true if a client route is found.
*/
public static boolean hasClientRoute(String route, boolean excludeLayouts) {
if (VaadinSession.getCurrent() == null || route == null) {
return false;
}
Expand All @@ -423,8 +438,16 @@ public static boolean hasClientRoute(String route) {
Map<String, AvailableViewInfo> clientItems = MenuRegistry
.collectClientMenuItems(true,
VaadinSession.getCurrent().getConfiguration());
Set<String> clientRoutes = clientItems.keySet();
final Set<String> clientRoutes = new HashSet<>();
clientItems.forEach((path, info) -> {
if (excludeLayouts) {
if (info.children() == null || info.children().isEmpty()) {
clientRoutes.add(path);
}
} else {
clientRoutes.add(path);
}
});
return clientRoutes.contains(route);
}

}
42 changes: 26 additions & 16 deletions flow-server/src/test/java/com/vaadin/flow/component/internal/JavaScriptBootstrapUITest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
import org.junit.Before;
import org.junit.Test;
import org.mockito.ArgumentCaptor;
import org.mockito.MockedStatic;
import org.mockito.Mockito;

import com.vaadin.flow.component.Component;
Expand Down Expand Up @@ -46,6 +47,7 @@
import com.vaadin.flow.server.VaadinService;
import com.vaadin.flow.server.VaadinSession;
import com.vaadin.flow.server.VaadinSessionState;
import com.vaadin.flow.server.menu.MenuRegistry;

public class JavaScriptBootstrapUITest {

Expand Down Expand Up @@ -435,22 +437,30 @@ public void should_update_pushState_when_navigationHasBeenAlreadyStarted() {
ArgumentCaptor<Serializable[]> execArg = ArgumentCaptor
.forClass(Serializable[].class);

ui.navigate("clean/1");
Mockito.verify(page).executeJs(execJs.capture(), execArg.capture());

boolean reactEnabled = ui.getSession().getConfiguration()
.isReactEnabled();

final Serializable[] execValues = execArg.getValue();
if (reactEnabled) {
assertEquals(REACT_PUSHSTATE_TO, execJs.getValue());
assertEquals(1, execValues.length);
assertEquals("clean/1", execValues[0]);
} else {
assertEquals(CLIENT_PUSHSTATE_TO, execJs.getValue());
assertEquals(2, execValues.length);
assertNull(execValues[0]);
assertEquals("clean/1", execValues[1]);
try (MockedStatic<MenuRegistry> menuRegistry = Mockito
.mockStatic(MenuRegistry.class)) {

menuRegistry
.when(() -> MenuRegistry.hasClientRoute("clean/1", true))
.thenReturn(false);

ui.navigate("clean/1");
Mockito.verify(page).executeJs(execJs.capture(), execArg.capture());

boolean reactEnabled = ui.getSession().getConfiguration()
.isReactEnabled();

final Serializable[] execValues = execArg.getValue();
if (reactEnabled) {
assertEquals(REACT_PUSHSTATE_TO, execJs.getValue());
assertEquals(1, execValues.length);
assertEquals("clean/1", execValues[0]);
} else {
assertEquals(CLIENT_PUSHSTATE_TO, execJs.getValue());
assertEquals(2, execValues.length);
assertNull(execValues[0]);
assertEquals("clean/1", execValues[1]);
}
}
}

Expand Down
65 changes: 65 additions & 0 deletions flow-server/src/test/java/com/vaadin/flow/router/internal/NavigationStateRendererTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@
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;
Expand Down Expand Up @@ -65,6 +66,7 @@
import com.vaadin.flow.router.PreserveOnRefresh;
import com.vaadin.flow.router.QueryParameters;
import com.vaadin.flow.router.Route;
import com.vaadin.flow.router.RouteAlias;
import com.vaadin.flow.router.RouteConfiguration;
import com.vaadin.flow.router.Router;
import com.vaadin.flow.router.RouterLayout;
Expand All @@ -77,6 +79,7 @@
import com.vaadin.flow.server.RouteRegistry;
import com.vaadin.flow.server.ServiceException;
import com.vaadin.flow.server.WrappedSession;
import com.vaadin.flow.server.menu.MenuRegistry;
import com.vaadin.flow.server.startup.ApplicationRouteRegistry;
import com.vaadin.tests.util.AlwaysLockedVaadinSession;
import com.vaadin.tests.util.MockDeploymentConfiguration;
Expand Down Expand Up @@ -323,6 +326,21 @@ private static class SingleView extends Component {
}
}

@Route(value = "/:samplePersonID?/:action?(edit)")
@RouteAlias(value = "")
@Tag("div")
private static class RootRouteWithParam extends Component
implements BeforeEnterObserver {
RootRouteWithParam() {
addAttachListener(e -> viewAttachCount.getAndIncrement());
}

@Override
public void beforeEnter(BeforeEnterEvent event) {
beforeEnterCount.getAndIncrement();
}
}

@Test
public void handle_preserveOnRefreshAndWindowNameNotKnown_clientSideCallTriggered() {
// given a service with instantiator
Expand Down Expand Up @@ -636,6 +654,7 @@ public void handle_preserveOnRefresh_sameUI_uiIsNotClosed_childrenAreNotRemoved(

private static AtomicInteger layoutAttachCount;
private static AtomicInteger viewAttachCount;
private static AtomicInteger beforeEnterCount;
private static String layoutUUID;
private static String viewUUID;

Expand Down Expand Up @@ -747,6 +766,52 @@ public void handle_normalView_refreshCurrentRouteRecreatesComponents() {

}

@Test
public void handle_clientNavigation_withMatchingFlowRoute() {
viewAttachCount = new AtomicInteger();
beforeEnterCount = new AtomicInteger();

// given a service with instantiator
MockVaadinServletService service = createMockServiceWithInstantiator();

// given a locked session
MockVaadinSession session = new AlwaysLockedVaadinSession(service);
session.setConfiguration(new MockDeploymentConfiguration());

// given a NavigationStateRenderer mapping to PreservedNestedView
Router router = session.getService().getRouter();
NavigationStateRenderer renderer = new NavigationStateRenderer(
new NavigationStateBuilder(router)
.withTarget(RootRouteWithParam.class).withPath("")
.build());
router.getRegistry().setRoute("", RootRouteWithParam.class, null);

MockUI ui = new MockUI(session);

renderer.handle(new NavigationEvent(router, new Location(""), ui,
NavigationTrigger.PAGE_LOAD));

Assert.assertEquals(1, beforeEnterCount.get());
Assert.assertEquals(1, viewAttachCount.get());

ui.getInternals().clearLastHandledNavigation();

try (MockedStatic<MenuRegistry> menuRegistry = Mockito
.mockStatic(MenuRegistry.class)) {
menuRegistry.when(
() -> MenuRegistry.hasClientRoute("client-route", true))
.thenReturn(true);

// This should not call attach or beforeEnter on root route
renderer.handle(
new NavigationEvent(router, new Location("client-route"),
ui, NavigationTrigger.CLIENT_SIDE));

Assert.assertEquals(1, beforeEnterCount.get());
Assert.assertEquals(1, viewAttachCount.get());
}
}

private MockVaadinServletService createMockServiceWithInstantiator() {
MockVaadinServletService service = new MockVaadinServletService();
service.init(new MockInstantiator() {
Expand Down

0 comments on commit 4363bbf

Please sign in to comment.