Fix regressions introduced by add-on customized RouteNotFoundError su…

…bclass (#657) Replace RouteNotFoundError from Flow with SpringRouteNotFoundError instead of adding it when setting up route registry. Also only show detailed message in dev mode.
vaadin · Aug 31, 2020 · 6ac2557 · 6ac2557
1 parent f428adf
commit 6ac2557
Show file tree

Hide file tree

Showing 3 changed files with 128 additions and 21 deletions.
diff --git a/vaadin-spring/src/main/java/com/vaadin/flow/spring/VaadinServletContextInitializer.java b/vaadin-spring/src/main/java/com/vaadin/flow/spring/VaadinServletContextInitializer.java
@@ -42,6 +42,8 @@
 import java.util.stream.Stream;
 
 import com.googlecode.gentyref.GenericTypeReflector;
+
+import com.vaadin.flow.router.RouteNotFoundError;
 import com.vaadin.flow.server.startup.ServletDeployer;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -291,12 +293,17 @@ public void failFastContextInitialized(ServletContextEvent event) {
                     .getInstance(new VaadinServletContext(
                             event.getServletContext()));
 
-            Stream<Class<? extends Component>> hasErrorComponents = findBySuperType(
+            Set<Class<? extends Component>> errorComponents = findBySuperType(
                     getErrorParameterPackages(), HasErrorParameter.class)
                             .filter(Component.class::isAssignableFrom)
-                            .map(clazz -> (Class<? extends Component>) clazz);
-            registry.setErrorNavigationTargets(
-                    hasErrorComponents.collect(Collectors.toSet()));
+                            // Replace Flow default with custom version for Spring
+                            .filter(clazz -> clazz != RouteNotFoundError.class)
+                            .map(clazz -> (Class<? extends Component>) clazz)
+                    .collect(Collectors.toSet());
+            if (errorComponents.stream().noneMatch(RouteNotFoundError.class::isAssignableFrom)) {
+                errorComponents.add(SpringRouteNotFoundError.class);
+            }
+            registry.setErrorNavigationTargets(errorComponents);
         }
     }
 
@@ -632,8 +639,7 @@ private Collection<String> getWebComponentPackages() {
 
     private Collection<String> getErrorParameterPackages() {
         return Stream.concat(
-                Stream.of(HasErrorParameter.class.getPackage().getName(),
-                        SpringRouteNotFoundError.class.getPackage().getName()),
+                Stream.of(HasErrorParameter.class.getPackage().getName()),
                 getDefaultPackages().stream()).collect(Collectors.toSet());
     }
 

diff --git a/vaadin-spring/src/main/java/com/vaadin/flow/spring/router/SpringRouteNotFoundError.java b/vaadin-spring/src/main/java/com/vaadin/flow/spring/router/SpringRouteNotFoundError.java
@@ -12,17 +12,18 @@ public int setErrorParameter(BeforeEnterEvent event,
             ErrorParameter<NotFoundException> parameter) {
         int retval = super.setErrorParameter(event, parameter);
 
-        // Alert user about potential issue with Spring Boot Devtools losing
-        // routes https://github.com/spring-projects/spring-boot/issues/19543
-        String customMessage = "<span>When using Spring Boot Devtools with "
-                + "automatic reload, please note that routes can sometimes be "
-                + "lost due to a <a href ='https://github.com/spring-projects/spring-boot/issues/19543'>"
-                + "compilation race condition</a>. See "
-                + "<a href='https://vaadin.com/docs/flow/workflow/setup-live-reload-springboot.html'>"
-                + "the documentation</a> for further workarounds and other "
-                + "live reload alternatives.";
-        getElement().appendChild(new Html(customMessage).getElement());
-
+        if (!event.getUI().getSession().getConfiguration().isProductionMode()) {
+            // Alert user about potential issue with Spring Boot Devtools losing
+            // routes https://github.com/spring-projects/spring-boot/issues/19543
+            String customMessage = "<span>When using Spring Boot Devtools with "
+                    + "automatic reload, please note that routes can sometimes be "
+                    + "lost due to a <a href ='https://github.com/spring-projects/spring-boot/issues/19543'>"
+                    + "compilation race condition</a>. See "
+                    + "<a href='https://vaadin.com/docs/flow/workflow/setup-live-reload-springboot.html'>"
+                    + "the documentation</a> for further workarounds and other "
+                    + "live reload alternatives.";
+            getElement().appendChild(new Html(customMessage).getElement());
+        }
         return retval;
     }
 }
diff --git a/vaadin-spring/src/test/java/com/vaadin/flow/spring/VaadinServletContextInitializerTest.java b/vaadin-spring/src/test/java/com/vaadin/flow/spring/VaadinServletContextInitializerTest.java
@@ -1,10 +1,19 @@
 package com.vaadin.flow.spring;
 
 import com.google.common.collect.Maps;
+
+import com.vaadin.flow.component.Component;
 import com.vaadin.flow.function.DeploymentConfiguration;
+import com.vaadin.flow.router.NotFoundException;
+import com.vaadin.flow.router.RouteNotFoundError;
+import com.vaadin.flow.server.VaadinServletContext;
+import com.vaadin.flow.server.startup.ApplicationRouteRegistry;
 import com.vaadin.flow.server.startup.DevModeInitializer;
 import com.vaadin.flow.server.startup.ServletDeployer;
+import com.vaadin.flow.spring.router.SpringRouteNotFoundError;
+
 import org.apache.commons.lang3.StringUtils;
+import org.junit.Assert;
 import org.junit.Before;
 import org.junit.Test;
 import org.junit.runner.RunWith;
@@ -13,6 +22,7 @@
 import org.powermock.api.mockito.PowerMockito;
 import org.powermock.core.classloader.annotations.PrepareForTest;
 import org.powermock.modules.junit4.PowerMockRunner;
+import org.springframework.boot.autoconfigure.AutoConfigurationPackages;
 import org.springframework.context.ApplicationContext;
 import org.springframework.core.env.Environment;
 
@@ -21,14 +31,16 @@
 import javax.servlet.ServletContextListener;
 import java.util.Collections;
 import java.util.Map;
+import java.util.concurrent.atomic.AtomicReference;
 import java.util.stream.Stream;
 
 @RunWith(PowerMockRunner.class)
 @PrepareForTest({DevModeInitializer.class,
-                VaadinServletContextInitializer.SpringStubServletConfig.class,
-                VaadinServletContextInitializer.class,
-                ServletDeployer.class,
-                ServletDeployer.StubServletConfig.class})
+        VaadinServletContextInitializer.SpringStubServletConfig.class,
+        VaadinServletContextInitializer.class,
+        ServletDeployer.class,
+        ServletDeployer.StubServletConfig.class,
+        AutoConfigurationPackages.class})
 public class VaadinServletContextInitializerTest {
 
     @Mock
@@ -49,6 +61,7 @@ public void init() {
         PowerMockito.mockStatic(ServletDeployer.class);
         PowerMockito.mockStatic(ServletDeployer.StubServletConfig.class);
         PowerMockito.mockStatic(DevModeInitializer.class);
+        PowerMockito.mockStatic(AutoConfigurationPackages.class);
     }
 
     @Test
@@ -94,6 +107,93 @@ public void onStartup_devModeAlreadyInitialized_devModeInitializationSkipped() t
         DevModeInitializer.initDevModeHandler(Mockito.any(), Mockito.any(), Mockito.any());
     }
 
+    @Test
+    public void errorParameterServletContextListenerEvent_defaultRouteNotFoundView_defaultRouteNotFoundViewIsRegistered() throws Exception {
+        // given
+        initDefaultMocks();
+
+        VaadinServletContextInitializer vaadinServletContextInitializer =
+                getStubbedVaadinServletContextInitializer();
+
+        AtomicReference<ServletContextListener> theListener = new AtomicReference<>();
+        Mockito.doAnswer(answer -> {
+            ServletContextListener listener = answer.getArgument(0);
+            if ("ErrorParameterServletContextListener".equals(
+                    listener.getClass().getSimpleName())) {
+                theListener.set(listener);
+            }
+            return null;
+        }).when(servletContext).addListener(Mockito.any(ServletContextListener.class));
+
+        vaadinServletContextInitializer.onStartup(servletContext);
+
+        Mockito.when(applicationContext.getBeanNamesForType(VaadinScanPackagesRegistrar.VaadinScanPackages.class))
+                .thenReturn(new String[]{});
+        PowerMockito.when(AutoConfigurationPackages.class, "has",
+                applicationContext).thenReturn(false);
+
+        // when
+        ServletContextEvent initEventMock = Mockito.mock(ServletContextEvent.class);
+        Mockito.when(initEventMock.getServletContext()).thenReturn(servletContext);
+        theListener.get().contextInitialized(initEventMock);
+
+        // then
+        ApplicationRouteRegistry registry = ApplicationRouteRegistry
+                .getInstance(new VaadinServletContext(servletContext));
+        final Class<? extends Component> navigationTarget =
+                registry.getErrorNavigationTarget(new NotFoundException()).get().getNavigationTarget();
+        Assert.assertEquals(SpringRouteNotFoundError.class, navigationTarget);
+    }
+
+    @Test
+    public void errorParameterServletContextListenerEvent_hasCustomRouteNotFoundView_customRouteNotFoundViewIsRegistered() throws Exception {
+        // given
+        initDefaultMocks();
+
+        VaadinServletContextInitializer vaadinServletContextInitializer =
+                getStubbedVaadinServletContextInitializer();
+
+        AtomicReference<ServletContextListener> theListener = new AtomicReference<>();
+        Mockito.doAnswer(answer -> {
+            ServletContextListener listener = answer.getArgument(0);
+            if ("ErrorParameterServletContextListener".equals(
+                    listener.getClass().getSimpleName())) {
+                theListener.set(listener);
+            }
+            return null;
+        }).when(servletContext).addListener(Mockito.any(ServletContextListener.class));
+
+        vaadinServletContextInitializer.onStartup(servletContext);
+
+        Mockito.when(applicationContext.getBeanNamesForType(VaadinScanPackagesRegistrar.VaadinScanPackages.class))
+                .thenReturn(new String[]{});
+        PowerMockito.when(AutoConfigurationPackages.class, "has",
+                applicationContext).thenReturn(false);
+
+        class TestErrorView extends RouteNotFoundError {
+        }
+
+        PowerMockito.doReturn(Stream.of(TestErrorView.class))
+                .when(vaadinServletContextInitializer,
+                        "findByAnnotationOrSuperType",
+                        Mockito.anyCollection(),
+                        Mockito.any(),
+                        Mockito.anyCollection(),
+                        Mockito.anyCollection());
+
+        // when
+        ServletContextEvent initEventMock = Mockito.mock(ServletContextEvent.class);
+        Mockito.when(initEventMock.getServletContext()).thenReturn(servletContext);
+        theListener.get().contextInitialized(initEventMock);
+
+        // then
+        ApplicationRouteRegistry registry = ApplicationRouteRegistry
+                .getInstance(new VaadinServletContext(servletContext));
+        final Class<? extends Component> navigationTarget =
+                registry.getErrorNavigationTarget(new NotFoundException()).get().getNavigationTarget();
+        Assert.assertEquals(TestErrorView.class, navigationTarget);
+    }
+
     private DevModeInitializer getStubbedDevModeInitializer() throws Exception {
         PowerMockito.when(ServletDeployer.StubServletConfig.class,
                 "createDeploymentConfiguration",