fix: preserve internal JS UI state properly (#10823) (CP: 6.0) (#10838)

* fix: preserve internal JS UI state properly (#10823) fixes #10757 * chore: add missing import Co-authored-by: Denis <denis@vaadin.com> Co-authored-by: Manolo Carrasco <manolo@vaadin.com>
vaadin · Apr 30, 2021 · d418151 · d418151
1 parent d7b59dd
commit d418151
Show file tree

Hide file tree

Showing 2 changed files with 63 additions and 24 deletions.
diff --git a/flow-server/src/main/java/com/vaadin/flow/component/internal/JavaScriptBootstrapUI.java b/flow-server/src/main/java/com/vaadin/flow/component/internal/JavaScriptBootstrapUI.java
@@ -190,28 +190,29 @@ && sameLocation(getInternals().getLastHandledLocation(),
             }
 
             navigationInProgress = true;
-            Optional<NavigationState> navigationState = getInternals()
-                    .getRouter().resolveNavigationTarget(location);
-
-            if (navigationState.isPresent()) {
-                // Navigation can be done in server side without extra
-                // round-trip
-                handleNavigation(location, navigationState.get(),
-                        NavigationTrigger.UI_NAVIGATE);
-                if (getForwardToClientUrl() != null) {
-                    navigationInProgress = false;
-                    // Server is forwarding to a client route from a
-                    // BeforeEnter.
-                    navigateToClient(getForwardToClientUrl());
-                    return;
+            try {
+                Optional<NavigationState> navigationState = getInternals()
+                        .getRouter().resolveNavigationTarget(location);
+
+                if (navigationState.isPresent()) {
+                    // Navigation can be done in server side without extra
+                    // round-trip
+                    handleNavigation(location, navigationState.get(),
+                            NavigationTrigger.UI_NAVIGATE);
+                    if (getForwardToClientUrl() != null) {
+                        // Server is forwarding to a client route from a
+                        // BeforeEnter.
+                        navigateToClient(getForwardToClientUrl());
+                    }
+                } else {
+                    // Server cannot resolve navigation, let client-side to
+                    // handle it.
+                    navigateToClient(location.getPathWithQueryParameters());
                 }
-            } else {
-                // Server cannot resolve navigation, let client-side to handle
-                // it.
-                navigateToClient(location.getPathWithQueryParameters());
+            } finally {
+                navigationInProgress = false;
             }
 
-            navigationInProgress = false;
         }
     }
 

diff --git a/flow-server/src/test/java/com/vaadin/flow/component/internal/JavaScriptBootstrapUITest.java b/flow-server/src/test/java/com/vaadin/flow/component/internal/JavaScriptBootstrapUITest.java
@@ -5,6 +5,12 @@
 import java.util.Optional;
 import java.util.UUID;
 
+import org.junit.Assert;
+import org.junit.Before;
+import org.junit.Test;
+import org.mockito.ArgumentCaptor;
+import org.mockito.Mockito;
+
 import com.vaadin.flow.component.Component;
 import com.vaadin.flow.component.HasComponents;
 import com.vaadin.flow.component.Tag;
@@ -24,16 +30,15 @@
 import com.vaadin.flow.router.InternalServerError;
 import com.vaadin.flow.router.Location;
 import com.vaadin.flow.router.PageTitle;
+import com.vaadin.flow.router.QueryParameters;
 import com.vaadin.flow.router.Route;
+import com.vaadin.flow.router.Router;
 import com.vaadin.flow.router.RouterLink;
 import com.vaadin.flow.server.InvalidRouteConfigurationException;
 import com.vaadin.flow.server.MockServletServiceSessionSetup;
 import com.vaadin.flow.server.VaadinRequest;
-import org.junit.Assert;
-import org.junit.Before;
-import org.junit.Test;
-import org.mockito.ArgumentCaptor;
-import org.mockito.Mockito;
+import com.vaadin.flow.server.VaadinService;
+import com.vaadin.flow.server.VaadinSession;
 
 import static com.vaadin.flow.component.internal.JavaScriptBootstrapUI.CLIENT_NAVIGATE_TO;
 import static com.vaadin.flow.component.internal.JavaScriptBootstrapUI.SERVER_ROUTING;
@@ -608,6 +613,39 @@ public void should_caught_and_show_exception_during_navigation_in_internalServer
         assertExceptionComponent(InternalServerError.class, errorMessage);
     }
 
+    @Test
+    public void navigate_firsClientSideRoutingThrows_navigationInProgressIsReset_secondClientSideRoutingWorks() {
+        VaadinSession session = Mockito.mock(VaadinSession.class);
+
+        VaadinService service = Mockito.mock(VaadinService.class);
+        Mockito.when(session.getService()).thenReturn(service);
+        Router router = Mockito.mock(Router.class);
+        Mockito.when(service.getRouter()).thenReturn(router);
+
+        Mockito.doThrow(RuntimeException.class).when(router)
+                .resolveNavigationTarget(Mockito.any());
+
+        JavaScriptBootstrapUI ui = new JavaScriptBootstrapUI();
+
+        ui.getInternals().setSession(session);
+
+        try {
+            ui.navigate("foo", QueryParameters.empty());
+        } catch (RuntimeException expected) {
+            router = Mockito.mock(Router.class);
+            Mockito.when(service.getRouter()).thenReturn(router);
+
+            Mockito.when(router.resolveNavigationTarget(Mockito.any()))
+                    .thenReturn(Optional.empty());
+            ui.navigate("foo", QueryParameters.empty());
+
+            Mockito.verify(router).resolveNavigationTarget(Mockito.any());
+            return;
+        }
+        // self control: code inside catch should be invoked
+        Assert.fail();
+    }
+
     private void assertExceptionComponent(Class<?> errorClass,
             String... exceptionTexts) {
         Optional<Component> visibleComponent = ui.getElement().getChild(0)