Fix route matching order mistake

apache · Nov 25, 2024 · 73eafc1 · 73eafc1
1 parent 9753552
commit 73eafc1
Show file tree

Hide file tree

Showing 7 changed files with 21 additions and 18 deletions.
diff --git a/...main/java/org/apache/dubbo/rpc/protocol/tri/rest/filter/RestExtensionExecutionFilter.java b/...main/java/org/apache/dubbo/rpc/protocol/tri/rest/filter/RestExtensionExecutionFilter.java
@@ -171,7 +171,7 @@ private RestFilter[] matchFilters(RestFilter[] filters, String path) {
                 if (size > 1) {
                     Collections.sort(matches);
                 }
-                if (matches.get(size - 1).getValue()) {
+                if (matches.get(0).getValue()) {
                     continue;
                 }
             }

diff --git a/...src/main/java/org/apache/dubbo/rpc/protocol/tri/rest/mapping/condition/PathCondition.java b/...src/main/java/org/apache/dubbo/rpc/protocol/tri/rest/mapping/condition/PathCondition.java
@@ -105,7 +105,9 @@ public PathCondition match(HttpRequest request) {
             }
         }
         if (matches != null) {
-            Collections.sort(matches);
+            if (matches.size() > 1) {
+                Collections.sort(matches);
+            }
             Set<String> result = CollectionUtils.newLinkedHashSet(matches.size());
             for (int i = 0, size = matches.size(); i < size; i++) {
                 result.add(matches.get(i).getPath());

diff --git a/...rc/main/java/org/apache/dubbo/rpc/protocol/tri/rest/mapping/condition/PathExpression.java b/...rc/main/java/org/apache/dubbo/rpc/protocol/tri/rest/mapping/condition/PathExpression.java
@@ -104,7 +104,7 @@ public int compareTo(PathExpression other) {
                 return result;
             }
         }
-        return otherSize - size;
+        return size - otherSize;
     }
 
     @Override

diff --git a/...e/src/main/java/org/apache/dubbo/rpc/protocol/tri/rest/mapping/condition/PathSegment.java b/...e/src/main/java/org/apache/dubbo/rpc/protocol/tri/rest/mapping/condition/PathSegment.java
@@ -193,9 +193,9 @@ public int compareTo(PathSegment other) {
                 return comparison;
             }
         }
-        int size = variables == null ? 0 : variables.size();
-        int otherSize = other.variables == null ? 0 : other.variables.size();
-        return otherSize - size;
+        int size = variables == null ? 99 : variables.size();
+        int otherSize = other.variables == null ? 99 : other.variables.size();
+        return size - otherSize;
     }
 
     public enum Type {
@@ -212,18 +212,18 @@ public enum Type {
         LITERAL(1),
         /**
          * A wildcard segment.
-         * E.g.: 't?st*uv' and '/foo/&ast;/bar'
+         * E.g.: 't?st*uv'
          */
         WILDCARD,
         /**
          * A wildcard matching suffix.
          * Transient type used for parsing, will not be present in the PathExpression
-         * E.g.: '/foo/**' and '/**' and '/{*bar}'
+         * E.g.: '/foo/**' or '/**' or '/{*bar}'
          */
         WILDCARD_TAIL,
         /**
          * A template variable segment.
-         * E.g.: '{foo}'
+         * E.g.: '{foo}' or '/foo/&ast;/bar'
          */
         VARIABLE(10),
         /**

diff --git a/...e/src/main/java/org/apache/dubbo/rpc/protocol/tri/rest/openapi/DefaultOpenAPIService.java b/...e/src/main/java/org/apache/dubbo/rpc/protocol/tri/rest/openapi/DefaultOpenAPIService.java
@@ -25,8 +25,7 @@
 import org.apache.dubbo.remoting.http12.HttpRequest;
 import org.apache.dubbo.remoting.http12.HttpResponse;
 import org.apache.dubbo.remoting.http12.HttpResult;
-import org.apache.dubbo.remoting.http12.HttpStatus;
-import org.apache.dubbo.remoting.http12.exception.HttpStatusException;
+import org.apache.dubbo.remoting.http12.exception.HttpResultPayloadException;
 import org.apache.dubbo.remoting.http12.message.MediaType;
 import org.apache.dubbo.remoting.http12.rest.OpenAPIRequest;
 import org.apache.dubbo.rpc.RpcContext;
@@ -176,7 +175,6 @@ private List<OpenAPI> resolveOpenAPIs() {
     @Override
     public String getDocument(OpenAPIRequest request) {
         String path = null;
-        HttpResult<?> result;
         try {
             request = Helper.formatRequest(request);
 
@@ -187,13 +185,14 @@ public String getDocument(OpenAPIRequest request) {
 
             path = RequestUtils.getPathVariable(httpRequest, "path");
             if (StringUtils.isEmpty(path)) {
-                throw HttpResult.found(PathUtils.join(httpRequest.path(), "swagger-ui/index.html")).toPayload();
+                String url = PathUtils.join(httpRequest.path(), "swagger-ui/index.html");
+                throw HttpResult.found(url).toPayload();
             }
 
             path = '/' + path;
             List<Match<OpenAPIRequestHandler>> matches = tree.matchRelaxed(path);
             if (matches.isEmpty()) {
-                throw new HttpStatusException(HttpStatus.NOT_FOUND.getCode());
+                throw HttpResult.notFound().toPayload();
             }
 
             Collections.sort(matches);
@@ -204,13 +203,14 @@ public String getDocument(OpenAPIRequest request) {
             }
             httpRequest.setAttribute(OpenAPIRequest.class.getName(), request);
             httpRequest.setAttribute(RestConstants.URI_TEMPLATE_VARIABLES_ATTRIBUTE, match.getVariableMap());
-            result = match.getValue().handle(path, httpRequest, httpResponse);
+            throw match.getValue().handle(path, httpRequest, httpResponse).toPayload();
+        } catch (HttpResultPayloadException e) {
+            throw e;
         } catch (Throwable t) {
-            Level level = ExceptionUtils.resolveLogLevel(t);
+            Level level = ExceptionUtils.resolveLogLevel(ExceptionUtils.unwrap(t));
             LOG.log(level, "Failed to processing OpenAPI request {} for path: '{}'", request, path, t);
             throw t;
         }
-        throw result.toPayload();
     }
 
     private String handleDocument(OpenAPIRequest request) {

diff --git a/...c-triple/src/main/java/org/apache/dubbo/rpc/protocol/tri/rest/openapi/SchemaResolver.java b/...c-triple/src/main/java/org/apache/dubbo/rpc/protocol/tri/rest/openapi/SchemaResolver.java
@@ -250,7 +250,7 @@ private boolean isClassExcluded(Class<?> clazz) {
         } else if (size > 1) {
             Collections.sort(matches);
         }
-        return matches.get(size - 1).getValue();
+        return matches.get(0).getValue();
     }
 
     public static void addPath(RadixTree<Boolean> tree, String path) {

diff --git a/...groovy/org/apache/dubbo/rpc/protocol/tri/rest/mapping/condition/PathExpressionTest.groovy b/...groovy/org/apache/dubbo/rpc/protocol/tri/rest/mapping/condition/PathExpressionTest.groovy
@@ -206,6 +206,7 @@ class PathExpressionTest extends Specification {
             '/one'                  | '/one'            | 0
             '/one'                  | '/two'            | 0
             '/one/{two}/three'      | '/one/{t}/three'  | 0
+            '/one/two'              | '/one/{two}'      | -1
             '/one/{two}/three'      | '/one/*/three'    | -1
             '/one/*/three'          | '/one/**/three'   | -1
             '/one/two'              | '/one'            | -1