fix: consider first added route as main route and always return it first (#10556) (#10568)

fixes #10528

Co-authored-by: Denis <denis@vaadin.com>

Author: vaadin-bot

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

@@ -23,6 +23,7 @@
 import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.Map;
+import java.util.Map.Entry;
 import java.util.Objects;
 import java.util.Optional;
 import java.util.Set;
@@ -31,6 +32,8 @@
 import java.util.regex.Pattern;
 
 import com.vaadin.flow.component.Component;
+import com.vaadin.flow.router.Route;
+import com.vaadin.flow.router.RouteAlias;
 import com.vaadin.flow.router.RouteParameters;
 import com.vaadin.flow.server.AmbiguousRouteConfigurationException;
 
@@ -101,11 +104,18 @@ final class RouteSegment implements Serializable {
      */
     private Map<String, RouteSegment> allSegments;
 
-    private RouteSegment() {
-    }
+    private final boolean isRoot;
+
+    /**
+     * Main route segment should be returned first in the {@link #getRoutes()}
+     * method to support {@link Route}/{@link RouteAlias} annotation semantic :
+     * {@link Route} should take precedence over {@link RouteAlias}.
+     */
+    private boolean isMainRouteSegment;
 
-    private RouteSegment(String segmentTemplate) {
+    private RouteSegment(String segmentTemplate, boolean isRoot) {
         this.template = segmentTemplate;
+        this.isRoot = isRoot;
 
         if (RouteFormat.isParameter(segmentTemplate)) {
             info = new RouteFormat.ParameterInfo(segmentTemplate);
@@ -124,6 +134,8 @@ public RouteSegment(RouteSegment original) {
         this.info = original.info;
         this.pattern = original.pattern;
         this.target = original.target;
+        this.isRoot = original.isRoot;
+        this.isMainRouteSegment = original.isMainRouteSegment;
 
         original.getStaticSegments().entrySet()
                 .forEach(e -> this.addSegment(new RouteSegment(e.getValue()),
@@ -146,7 +158,7 @@ public RouteSegment(RouteSegment original) {
      * Create a new root segment instance.
      */
     static RouteSegment createRoot() {
-        return new RouteSegment("");
+        return new RouteSegment("", true);
     }
 
     String getName() {
@@ -203,24 +215,38 @@ boolean isEligible(String value) {
      * @return a {@link Map} containing all templates and their specific
      *         targets.
      */
-    Map<String, RouteTarget> getRoutes() {
-
-        Map<String, RouteTarget> result = new LinkedHashMap<>();
+    LinkedHashMap<String, RouteTarget> getRoutes() {
+
+        Map<String, RouteSegment> leafSegments = getLeafStaticSegments();
+
+        String mainRoutePath = null;
+        RouteTarget mainRouteTarget = null;
+
+        /*
+         * Find the first main route segment in the static segments which should
+         * go first to be able to support Route/RouteAlias semantic. Main route
+         * doesn't have to exist though: it may be removed at any point. In this
+         * case the order doesn't matter since the RouteAlias ordering is not
+         * defined.
+         */
+        for (Entry<String, RouteSegment> entry : leafSegments.entrySet()) {
+            if (entry.getValue().isMainRouteSegment) {
+                mainRoutePath = entry.getKey();
+                mainRouteTarget = entry.getValue().target;
+                break;
+            }
+        }
 
-        if (target != null) {
-            result.put("", target);
+        // If there is the main route : add it as first
+        LinkedHashMap<String, RouteTarget> result = new LinkedHashMap<>();
+        if (mainRoutePath != null) {
+            result.put(mainRoutePath, mainRouteTarget);
         }
 
-        collectRoutes(result, getStaticSegments());
-        collectRoutes(result, getParameterSegments());
-        collectRoutes(result, getOptionalSegments());
-        collectRoutes(result, getVarargsSegments());
+        getLeafSegments()
+                .forEach((path, segment) -> result.put(path, segment.target));
 
-        if (getTemplate().isEmpty()) {
-            return Collections.unmodifiableMap(result);
-        } else {
-            return result;
-        }
+        return result;
     }
 
     void removeSubRoute(String template) {
@@ -292,13 +318,53 @@ String formatTemplate(String template,
         }
     }
 
-    private void collectRoutes(Map<String, RouteTarget> result,
+    private LinkedHashMap<String, RouteSegment> getLeafSegments() {
+        LinkedHashMap<String, RouteSegment> result = new LinkedHashMap<>();
+
+        if (target != null) {
+            result.put("", this);
+        }
+
+        collectLeafSegments(result, getStaticSegments());
+        collectLeafSegments(result, getParameterSegments());
+        collectLeafSegments(result, getOptionalSegments());
+        collectLeafSegments(result, getVarargsSegments());
+
+        return result;
+    }
+
+    private Map<String, RouteSegment> getLeafStaticSegments() {
+        Map<String, RouteSegment> result = new HashMap<>();
+
+        if (target != null) {
+            result.put("", this);
+        }
+
+        for (Map.Entry<String, RouteSegment> segmentEntry : getStaticSegments()
+                .entrySet()) {
+            RouteSegment segment = segmentEntry.getValue();
+
+            for (Map.Entry<String, RouteSegment> targetEntry : segment
+                    .getLeafStaticSegments().entrySet()) {
+
+                final String key = targetEntry.getKey();
+                result.put(
+                        segmentEntry.getKey()
+                                + (key.isEmpty() ? "" : ("/" + key)),
+                        targetEntry.getValue());
+            }
+        }
+        return result;
+    }
+
+    private void collectLeafSegments(Map<String, RouteSegment> result,
             Map<String, RouteSegment> children) {
         for (Map.Entry<String, RouteSegment> segmentEntry : children
                 .entrySet()) {
+            RouteSegment segment = segmentEntry.getValue();
 
-            for (Map.Entry<String, RouteTarget> targetEntry : segmentEntry
-                    .getValue().getRoutes().entrySet()) {
+            for (Map.Entry<String, RouteSegment> targetEntry : segment
+                    .getLeafSegments().entrySet()) {
 
                 final String key = targetEntry.getKey();
                 result.put(
@@ -309,6 +375,16 @@ private void collectRoutes(Map<String, RouteTarget> result,
         }
     }
 
+    private RouteSegment getFirstLeafSegment() {
+        if (target != null) {
+            return this;
+        }
+        Map<String, RouteSegment> segments = getAllSegments();
+        assert !segments.isEmpty();
+        RouteSegment first = segments.values().iterator().next();
+        return first.getFirstLeafSegment();
+    }
+
     private void removeSubRoute(List<String> segmentPatterns) {
         RouteSegment routeSegment;
         String segmentPattern = null;
@@ -341,6 +417,7 @@ private void removeSubRoute(List<String> segmentPatterns) {
     }
 
     private void addSubRoute(List<String> segmentPatterns, RouteTarget target) {
+        boolean isMainRoute = isEmpty() && isRoot;
 
         RouteSegment routeSegment;
         String segmentPattern = null;
@@ -379,6 +456,11 @@ private void addSubRoute(List<String> segmentPatterns, RouteTarget target) {
         }
 
         routeSegment.setRouteTarget(segmentPatterns, target);
+
+        if (isMainRoute) {
+            RouteSegment firstSegment = getFirstLeafSegment();
+            firstSegment.isMainRouteSegment = true;
+        }
     }
 
     private void setRouteTarget(List<String> segmentPatterns,
@@ -730,7 +812,7 @@ boolean isEmpty() {
 
     private RouteSegment addSegment(String segmentTemplate,
             Map<String, RouteSegment> children) {
-        RouteSegment routeSegment = new RouteSegment(segmentTemplate);
+        RouteSegment routeSegment = new RouteSegment(segmentTemplate, false);
         addSegment(routeSegment, children);
         return routeSegment;
     }

flow-server/src/test/java/com/vaadin/flow/server/SessionRouteRegistryTest.java

@@ -1,6 +1,7 @@
 package com.vaadin.flow.server;
 
 import javax.servlet.ServletContext;
+
 import java.io.ByteArrayInputStream;
 import java.io.ByteArrayOutputStream;
 import java.io.ObjectInputStream;
@@ -40,7 +41,9 @@
 import com.vaadin.flow.router.Route;
 import com.vaadin.flow.router.RouteAlias;
 import com.vaadin.flow.router.RouteBaseData;
+import com.vaadin.flow.router.RouteConfiguration;
 import com.vaadin.flow.router.RouteData;
+import com.vaadin.flow.router.RouteParameters;
 import com.vaadin.flow.router.RouterLayout;
 import com.vaadin.flow.router.RoutesChangedEvent;
 import com.vaadin.flow.router.internal.HasUrlParameterFormat;
@@ -988,6 +991,36 @@ public void serialize_deserialize_parentRegistryIsANewOne()
 
     }
 
+    @Test
+    public void getTargetUrl_annotatedRoute_rootIsAlias_mainRouteIsNotRoot_mainRouteIsReturned() {
+        SessionRouteRegistry registry = getRegistry(session);
+        RouteConfiguration configuration = RouteConfiguration
+                .forRegistry(registry);
+
+        configuration.setAnnotatedRoute(RouteWithRootAlias.class);
+
+        Optional<String> url = registry.getTargetUrl(RouteWithRootAlias.class,
+                RouteParameters.empty());
+
+        Assert.assertTrue(url.isPresent());
+        Assert.assertEquals("foo", url.get());
+    }
+
+    @Test
+    public void getTargetUrl_annotatedRoute_rootIsAlias_mainRouteIsParamerterized_routeAliasIsReturned() {
+        SessionRouteRegistry registry = getRegistry(session);
+        RouteConfiguration configuration = RouteConfiguration
+                .forRegistry(registry);
+
+        configuration.setAnnotatedRoute(ParameterizedRouteWithRootAlias.class);
+
+        Optional<String> url = registry.getTargetUrl(
+                ParameterizedRouteWithRootAlias.class, RouteParameters.empty());
+
+        Assert.assertTrue(url.isPresent());
+        Assert.assertEquals("", url.get());
+    }
+
     private <T> T serializeAndDeserialize(T instance) throws Throwable {
         ByteArrayOutputStream bs = new ByteArrayOutputStream();
         ObjectOutputStream out = new ObjectOutputStream(bs);
@@ -1053,6 +1086,20 @@ public int setErrorParameter(BeforeEnterEvent event,
         }
     }
 
+    @Tag("div")
+    @Route("foo")
+    @RouteAlias("")
+    private static class RouteWithRootAlias extends Component {
+
+    }
+
+    @Tag("div")
+    @Route(":foo")
+    @RouteAlias("")
+    private static class ParameterizedRouteWithRootAlias extends Component {
+
+    }
+
     /**
      * Extending class to let us mock the getRouteRegistry method for testing.
      */