fix: forward with optional callback (#19959)

* fix: forwarding with optional callback
vaadin · Sep 18, 2024 · 62cae89 · 62cae89
1 parent c615be5
commit 62cae89
Show file tree

Hide file tree

Showing 9 changed files with 158 additions and 13 deletions.
diff --git a/flow-server/src/main/java/com/vaadin/flow/router/BeforeEvent.java b/flow-server/src/main/java/com/vaadin/flow/router/BeforeEvent.java
@@ -58,6 +58,7 @@ public abstract class BeforeEvent extends EventObject {
     private String unknownReroute = null;
 
     private String externalForwardUrl = null;
+    private boolean useForwardCallback;
 
     /**
      * Constructs event from a NavigationEvent.
@@ -340,6 +341,30 @@ public void forwardTo(Class<? extends Component> forwardTargetComponent) {
                 RouteParameters.empty(), null));
     }
 
+    /**
+     * Forward the navigation to show the given component instead of the
+     * component that is currently about to be displayed.
+     * <p>
+     * This function changes the browser URL as opposed to
+     * <code>rerouteTo()</code>.
+     * <p>
+     * Note that query parameters of the event are preserved in the forwarded
+     * URL.
+     *
+     * @param forwardTargetComponent
+     *            the component type to display, not {@code null}
+     * @param useForwardCallback
+     *            {@literal true} to request navigation callback from client
+     */
+    public void forwardTo(Class<? extends Component> forwardTargetComponent,
+            boolean useForwardCallback) {
+        Objects.requireNonNull(forwardTargetComponent,
+                "forwardTargetComponent cannot be null");
+        this.useForwardCallback = useForwardCallback;
+        forwardTo(getNavigationState(forwardTargetComponent,
+                RouteParameters.empty(), null));
+    }
+
     /**
      * Forward the navigation to show the given component with given route
      * parameter instead of the component that is currently about to be
@@ -1139,6 +1164,17 @@ public ErrorParameter<?> getErrorParameter() {
         return errorParameter;
     }
 
+    /**
+     * Determines is client side callback should be requested when executing
+     * pending forward operation.
+     *
+     * @return {@literal true} if callback should be used,
+     *         {@literal false otherwise}
+     */
+    public boolean isUseForwardCallback() {
+        return useForwardCallback;
+    }
+
     /**
      * Gets the UI this navigation takes place inside.
      *

diff --git a/...server/src/main/java/com/vaadin/flow/router/internal/AbstractNavigationStateRenderer.java b/...server/src/main/java/com/vaadin/flow/router/internal/AbstractNavigationStateRenderer.java
@@ -737,20 +737,11 @@ private int forwardToExternalUrl(NavigationEvent event,
     private int forward(NavigationEvent event, BeforeEvent beforeNavigation) {
         NavigationHandler handler = beforeNavigation.getForwardTarget();
 
-        Class<? extends Component> forwardTargetType = beforeNavigation
-                .getForwardTargetType();
-
-        List<Class<? extends RouterLayout>> parentLayouts = RouteUtil
-                .getParentLayouts(event.getUI().getRouter().getRegistry(),
-                        forwardTargetType, beforeNavigation.getForwardUrl());
-
-        boolean preserveOnRefreshTarget = isPreserveOnRefreshTarget(
-                forwardTargetType, parentLayouts);
-
         NavigationEvent newNavigationEvent = getNavigationEvent(event,
                 beforeNavigation);
         newNavigationEvent.getUI().getPage().getHistory().replaceState(null,
-                newNavigationEvent.getLocation(), !preserveOnRefreshTarget);
+                newNavigationEvent.getLocation(),
+                beforeNavigation.isUseForwardCallback());
 
         return handler.handle(newNavigationEvent);
     }

diff --git a/flow-server/src/main/java/com/vaadin/flow/server/auth/NavigationAccessControl.java b/flow-server/src/main/java/com/vaadin/flow/server/auth/NavigationAccessControl.java
@@ -246,7 +246,7 @@ public void beforeEnter(BeforeEnterEvent event) {
             if (context.getPrincipal() == null) {
                 storeRedirectURL(event, request);
                 if (loginView != null) {
-                    event.forwardTo(loginView);
+                    event.forwardTo(loginView, true);
                 } else {
                     if (loginUrl != null) {
                         event.forwardToUrl(loginUrl);

diff --git a/flow-server/src/main/java/com/vaadin/flow/server/auth/ViewAccessChecker.java b/flow-server/src/main/java/com/vaadin/flow/server/auth/ViewAccessChecker.java
@@ -206,7 +206,7 @@ public void beforeEnter(BeforeEnterEvent beforeEnterEvent) {
                 }
             }
             if (loginView != null) {
-                beforeEnterEvent.forwardTo(loginView);
+                beforeEnterEvent.forwardTo(loginView, true);
             } else {
                 if (loginUrl != null) {
                     beforeEnterEvent.forwardToUrl(loginUrl);

diff --git a/flow-tests/test-react-router/src/main/java/com/vaadin/flow/ForwardTargetView.java b/flow-tests/test-react-router/src/main/java/com/vaadin/flow/ForwardTargetView.java
@@ -0,0 +1,9 @@
+package com.vaadin.flow;
+
+import com.vaadin.flow.component.html.Div;
+import com.vaadin.flow.router.Route;
+
+@Route("com.vaadin.flow.ForwardTargetView")
+public class ForwardTargetView extends Div {
+
+}
diff --git a/...ests/test-react-router/src/main/java/com/vaadin/flow/ForwardTargetWithParametersView.java b/...ests/test-react-router/src/main/java/com/vaadin/flow/ForwardTargetWithParametersView.java
@@ -0,0 +1,19 @@
+package com.vaadin.flow;
+
+import com.vaadin.flow.component.html.Div;
+import com.vaadin.flow.component.html.Span;
+import com.vaadin.flow.router.BeforeEvent;
+import com.vaadin.flow.router.HasUrlParameter;
+import com.vaadin.flow.router.OptionalParameter;
+import com.vaadin.flow.router.Route;
+
+@Route("com.vaadin.flow.ForwardTargetWithParametersView")
+public class ForwardTargetWithParametersView extends Div
+        implements HasUrlParameter<String> {
+
+    @Override
+    public void setParameter(BeforeEvent event,
+            @OptionalParameter String parameter) {
+        add(new Span("setParameter"));
+    }
+}
diff --git a/flow-tests/test-react-router/src/main/java/com/vaadin/flow/ForwardingToParametersView.java b/flow-tests/test-react-router/src/main/java/com/vaadin/flow/ForwardingToParametersView.java
@@ -0,0 +1,15 @@
+package com.vaadin.flow;
+
+import com.vaadin.flow.component.html.Div;
+import com.vaadin.flow.router.BeforeEnterEvent;
+import com.vaadin.flow.router.BeforeEnterObserver;
+import com.vaadin.flow.router.Route;
+
+@Route("com.vaadin.flow.ForwardingToParametersView")
+public class ForwardingToParametersView extends Div
+        implements BeforeEnterObserver {
+    @Override
+    public void beforeEnter(BeforeEnterEvent event) {
+        event.forwardTo(ForwardTargetWithParametersView.class);
+    }
+}
diff --git a/flow-tests/test-react-router/src/main/java/com/vaadin/flow/ForwardingView.java b/flow-tests/test-react-router/src/main/java/com/vaadin/flow/ForwardingView.java
@@ -0,0 +1,14 @@
+package com.vaadin.flow;
+
+import com.vaadin.flow.component.html.Div;
+import com.vaadin.flow.router.BeforeEnterEvent;
+import com.vaadin.flow.router.BeforeEnterObserver;
+import com.vaadin.flow.router.Route;
+
+@Route("com.vaadin.flow.ForwardingView")
+public class ForwardingView extends Div implements BeforeEnterObserver {
+    @Override
+    public void beforeEnter(BeforeEnterEvent event) {
+        event.forwardTo(ForwardTargetView.class);
+    }
+}
diff --git a/flow-tests/test-react-router/src/test/java/com/vaadin/flow/ForwardTargetIT.java b/flow-tests/test-react-router/src/test/java/com/vaadin/flow/ForwardTargetIT.java
@@ -0,0 +1,61 @@
+package com.vaadin.flow;
+
+import org.junit.Assert;
+import org.junit.Test;
+import org.openqa.selenium.TimeoutException;
+
+import com.vaadin.flow.component.html.testbench.SpanElement;
+import com.vaadin.flow.testutil.ChromeBrowserTest;
+
+public class ForwardTargetIT extends ChromeBrowserTest {
+
+    public static final String FORWARD_TARGET_VIEW = "/view/com.vaadin.flow.ForwardTargetView";
+
+    // Test for https://github.com/vaadin/flow/issues/19794
+    @Test
+    public void testUrlIsCorrectAfterForward() {
+        getDriver().get(getTestURL(getRootURL(), FORWARD_TARGET_VIEW, null));
+
+        try {
+            waitUntil(arg -> driver.getCurrentUrl()
+                    .endsWith(FORWARD_TARGET_VIEW));
+        } catch (TimeoutException e) {
+            Assert.fail("URL wasn't updated to expected one: "
+                    + FORWARD_TARGET_VIEW);
+        }
+
+        getDriver().get(getTestURL(getRootURL(),
+                "/view/com.vaadin.flow.ForwardingView", null));
+
+        try {
+            waitUntil(arg -> driver.getCurrentUrl()
+                    .endsWith(FORWARD_TARGET_VIEW));
+        } catch (TimeoutException e) {
+            Assert.fail("URL wasn't updated to expected one: "
+                    + FORWARD_TARGET_VIEW);
+        }
+
+        Assert.assertTrue("URL was not the expected one after forward call",
+                driver.getCurrentUrl().endsWith(FORWARD_TARGET_VIEW));
+    }
+
+    // Test for https://github.com/vaadin/flow/issues/19822
+    @Test
+    public void testSetParameterCalledOnlyOnceAfterForward() {
+        getDriver().get(getTestURL(getRootURL(),
+                "/view/com.vaadin.flow.ForwardingToParametersView", null));
+
+        try {
+            waitUntil(arg -> driver.getCurrentUrl().endsWith(
+                    "/view/com.vaadin.flow.ForwardTargetWithParametersView"));
+        } catch (TimeoutException e) {
+            Assert.fail("URL wasn't updated to expected one: "
+                    + "/view/com.vaadin.flow.ForwardTargetWithParametersView");
+        }
+
+        Assert.assertEquals("setParameter was called more than once", 1,
+                $(SpanElement.class).all().stream()
+                        .filter(span -> span.getText().equals("setParameter"))
+                        .count());
+    }
+}