fix: Annotate default error handlers. (#11063) (#11089) (#11092)

Annotate default error handlers with
the new annotaton DefaultErrorHandler
so that we can also in add-ons have default
handlers that can be overridden by user
handlers.

Part of vaadin/spring#661

Co-authored-by: caalador <mikael.grankvist@vaadin.com>

flow-server/src/main/java/com/vaadin/flow/router/InternalServerError.java

@@ -30,6 +30,7 @@
 import com.vaadin.flow.component.Tag;
 import com.vaadin.flow.dom.Element;
 import com.vaadin.flow.dom.ElementFactory;
+import com.vaadin.flow.router.internal.DefaultErrorHandler;
 import com.vaadin.flow.server.VaadinService;
 
 /**
@@ -38,6 +39,7 @@
  * @since 1.0
  */
 @Tag(Tag.DIV)
+@DefaultErrorHandler
 public class InternalServerError extends Component
         implements HasErrorParameter<Exception> {
 

flow-server/src/main/java/com/vaadin/flow/router/RouteNotFoundError.java

@@ -31,13 +31,15 @@
 import com.vaadin.flow.component.Component;
 import com.vaadin.flow.component.Html;
 import com.vaadin.flow.component.Tag;
+import com.vaadin.flow.router.internal.DefaultErrorHandler;
 
 /**
  * This is a basic default error view shown on routing exceptions.
  *
  * @since 1.0
  */
 @Tag(Tag.DIV)
+@DefaultErrorHandler
 public class RouteNotFoundError extends Component
         implements HasErrorParameter<NotFoundException> {
 

flow-server/src/main/java/com/vaadin/flow/router/internal/AbstractRouteRegistry.java

@@ -38,7 +38,6 @@
 import com.vaadin.flow.router.RoutesChangedListener;
 import com.vaadin.flow.server.Command;
 import com.vaadin.flow.server.InvalidRouteConfigurationException;
-import com.vaadin.flow.server.InvalidRouteLayoutConfigurationException;
 import com.vaadin.flow.server.RouteRegistry;
 import com.vaadin.flow.shared.Registration;
 
@@ -411,14 +410,19 @@ protected void addErrorTarget(Class<? extends Component> target,
      * Register a child handler if parent registered or leave as is if child
      * registered.
      * <p>
-     * If the target is not related to the registered handler then throw
+     * If the target is not related to the registered handler and neither
+     * handler is annotated as {@link DefaultErrorHandler} then throw
      * configuration exception as only one handler for each exception type is
      * allowed.
      *
      * @param target
      *            target being handled
      * @param exceptionType
      *            type of the handled exception
+     * @throws InvalidRouteConfigurationException
+     *             thrown if multiple exception handlers are registered for the
+     *             same exception without relation or the other being a default
+     *             handler
      */
     private void handleRegisteredExceptionType(
             Map<Class<? extends Exception>, Class<? extends Component>> exceptionTargetsMap,
@@ -430,11 +434,15 @@ private void handleRegisteredExceptionType(
         if (registered.isAssignableFrom(target)) {
             exceptionTargetsMap.put(exceptionType, target);
         } else if (!target.isAssignableFrom(registered)) {
-            String msg = String.format(
-                    "Only one target for an exception should be defined. Found '%s' and '%s' for exception '%s'",
-                    target.getName(), registered.getName(),
-                    exceptionType.getName());
-            throw new InvalidRouteLayoutConfigurationException(msg);
+            if (registered.isAnnotationPresent(DefaultErrorHandler.class)) {
+                exceptionTargetsMap.put(exceptionType, target);
+            } else if (!target.isAnnotationPresent(DefaultErrorHandler.class)) {
+                String msg = String.format(
+                        "Only one target for an exception should be defined. Found '%s' and '%s' for exception '%s'",
+                        target.getName(), registered.getName(),
+                        exceptionType.getName());
+                throw new InvalidRouteConfigurationException(msg);
+            }
         }
     }
 

flow-server/src/main/java/com/vaadin/flow/router/internal/DefaultErrorHandler.java

@@ -0,0 +1,35 @@
+/*
+ * Copyright 2000-2021 Vaadin Ltd.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License"); you may not
+ * use this file except in compliance with the License. You may obtain a copy of
+ * the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations under
+ * the License.
+ */
+
+package com.vaadin.flow.router.internal;
+
+import java.lang.annotation.Documented;
+import java.lang.annotation.ElementType;
+import java.lang.annotation.Retention;
+import java.lang.annotation.RetentionPolicy;
+import java.lang.annotation.Target;
+
+/**
+ * Marks an HasErrorParameter view as Framework default handler so it can be
+ * disregarded if there is a custom view for the same Exception.
+ *
+ * @since
+ */
+@Retention(RetentionPolicy.RUNTIME)
+@Target(ElementType.TYPE)
+@Documented
+public @interface DefaultErrorHandler {
+}

flow-server/src/main/java/com/vaadin/flow/server/startup/ApplicationRouteRegistry.java

@@ -23,14 +23,11 @@
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
-import java.util.Objects;
 import java.util.Optional;
 import java.util.ServiceLoader;
 import java.util.Set;
 import java.util.concurrent.CopyOnWriteArrayList;
 import java.util.concurrent.atomic.AtomicReference;
-import java.util.stream.Collectors;
-import java.util.stream.Stream;
 
 import org.slf4j.LoggerFactory;
 
@@ -42,7 +39,6 @@
 import com.vaadin.flow.router.RouteConfiguration;
 import com.vaadin.flow.router.RouteData;
 import com.vaadin.flow.router.RouteNotFoundError;
-import com.vaadin.flow.router.RouteParameters;
 import com.vaadin.flow.router.RouterLayout;
 import com.vaadin.flow.router.RoutesChangedEvent;
 import com.vaadin.flow.router.internal.AbstractRouteRegistry;
@@ -232,9 +228,6 @@ public void removeRoute(String path,
     }
 
     private AtomicReference<Class<?>> pwaConfigurationClass = new AtomicReference<>();
-    private static final Set<Class<? extends Component>> defaultErrorHandlers = Stream
-            .of(RouteNotFoundError.class, InternalServerError.class)
-            .collect(Collectors.toSet());
 
     private final ArrayList<NavigationTargetFilter> routeFilters = new ArrayList<>();
 
@@ -354,9 +347,7 @@ public void setErrorNavigationTargets(
 
         exceptionTargetsMap.putAll(getConfiguration().getExceptionHandlers());
 
-        errorNavigationTargets.stream()
-                .filter(target -> !defaultErrorHandlers.contains(target))
-                .filter(this::allErrorFiltersMatch)
+        errorNavigationTargets.stream().filter(this::allErrorFiltersMatch)
                 .filter(handler -> !Modifier.isAbstract(handler.getModifiers()))
                 .forEach(target -> addErrorTarget(target, exceptionTargetsMap));
 

flow-server/src/test/java/com/vaadin/flow/router/RouterTest.java

@@ -58,6 +58,7 @@
 import com.vaadin.flow.router.BeforeLeaveEvent.ContinueNavigationAction;
 import com.vaadin.flow.router.RouterTest.CombinedObserverTarget.Enter;
 import com.vaadin.flow.router.RouterTest.CombinedObserverTarget.Leave;
+import com.vaadin.flow.router.internal.DefaultErrorHandler;
 import com.vaadin.flow.router.internal.HasUrlParameterFormat;
 import com.vaadin.flow.router.internal.RouteUtil;
 import com.vaadin.flow.server.InvalidRouteConfigurationException;
@@ -2510,24 +2511,65 @@ public void exception_during_navigation_is_caught_and_show_in_internalServerErro
     }
 
     @Test
-    public void fail_for_multiple_of_the_same_class()
+    public void fail_for_multiple_classes_extending_the_same_exception_class()
             throws InvalidRouteConfigurationException {
-        setErrorNavigationTargets(ErrorTarget.class, RouteNotFoundError.class);
+        expectedEx.expect(InvalidRouteConfigurationException.class);
+        setErrorNavigationTargets(ErrorTarget.class,
+                CustomNotFoundTarget.class);
+    }
 
-        int result = router.navigate(ui, new Location("exception"),
+    @Route("npe")
+    @Tag(Tag.DIV)
+    public static class NpeNavigationTarget extends Component {
+        public NpeNavigationTarget() {
+            throw new NullPointerException("Null was found");
+        }
+    }
+
+    @Tag(Tag.DIV)
+    @DefaultErrorHandler
+    public static class DefaultNullPointerException extends Component
+            implements HasErrorParameter<NullPointerException> {
+
+        @Override
+        public int setErrorParameter(BeforeEnterEvent event,
+                ErrorParameter<NullPointerException> parameter) {
+            return HttpServletResponse.SC_UNAUTHORIZED;
+        }
+    }
+
+    @Tag(Tag.DIV)
+    public static class NullPointerExceptionHandler extends Component
+            implements HasErrorParameter<NullPointerException> {
+
+        @Override
+        public int setErrorParameter(BeforeEnterEvent event,
+                ErrorParameter<NullPointerException> parameter) {
+            return HttpServletResponse.SC_INTERNAL_SERVER_ERROR;
+        }
+    }
+
+    @Test // spring #661
+    public void pick_custom_from_multiple_error_targets_when_other_is_default_annotated() {
+        setNavigationTargets(NpeNavigationTarget.class);
+        setErrorNavigationTargets(DefaultNullPointerException.class,
+                NullPointerExceptionHandler.class);
+
+        int result = router.navigate(ui, new Location("npe"),
                 NavigationTrigger.PROGRAMMATIC);
-        Assert.assertEquals("Non existent route should have returned.",
-                HttpServletResponse.SC_NOT_FOUND, result);
+        Assert.assertEquals(
+                "Null pointer should return the server error of the custom implementation.",
+                HttpServletResponse.SC_INTERNAL_SERVER_ERROR, result);
 
         Assert.assertEquals(
                 "Expected the extending class to be used instead of the super class",
-                ErrorTarget.class, getUIComponent());
+                NullPointerExceptionHandler.class, getUIComponent());
     }
 
     @Test
     public void do_not_accept_same_exception_targets() {
 
-        expectedEx.expect(InvalidRouteLayoutConfigurationException.class);
+        expectedEx.expect(InvalidRouteConfigurationException.class);
         expectedEx.expectMessage(startsWith(
                 "Only one target for an exception should be defined. Found "));