Merge pull request #382 from Vkt0r/dynamic-overlapping-trail

Fix the HttpRouter problem with overlapping in trail and the middle of routes

Author: Vkt0r

Sources/HttpRouter.swift

@@ -93,7 +93,10 @@ open class HttpRouter {
     private func findHandler(_ node: inout Node, params: inout [String: String], generator: inout IndexingIterator<[String]>) -> ((HttpRequest) -> HttpResponse)? {
 
         var matchedRoutes = [Node]()
-        findHandler(&node, params: &params, generator: &generator, matchedNodes: &matchedRoutes, index: 0, count: generator.reversed().count)
+        let pattern = generator.map { $0 }
+        let numberOfElements = pattern.count
+        
+        findHandler(&node, params: &params, pattern: pattern , matchedNodes: &matchedRoutes, index: 0, count: numberOfElements)
         return matchedRoutes.first?.handler
     }
 
@@ -102,21 +105,21 @@ open class HttpRouter {
     /// - Parameters:
     ///   - node: The root node of the tree representing all the routes
     ///   - params: The parameters of the match
-    ///   - generator: The IndexingIterator to iterate through the pattern to match
+    ///   - pattern: The pattern or route to find in the routes tree
     ///   - matchedNodes: An array with the nodes matching the route
     ///   - index: The index of current position in the generator
     ///   - count: The number of elements if the route to match
-    private func findHandler(_ node: inout Node, params: inout [String: String], generator: inout IndexingIterator<[String]>, matchedNodes: inout [Node], index: Int, count: Int) {
+    private func findHandler(_ node: inout Node, params: inout [String: String], pattern: [String], matchedNodes: inout [Node], index: Int, count: Int) {
 
-        if let pathToken = generator.next()?.removingPercentEncoding {
+        if index < count, let pathToken = pattern[index].removingPercentEncoding {
 
             var currentIndex = index + 1
             let variableNodes = node.nodes.filter { $0.0.first == ":" }
             if let variableNode = variableNodes.first {
                 if currentIndex == count && variableNode.1.isEndOfRoute {
                     // if it's the last element of the pattern and it's a variable, stop the search and
                     // append a tail as a value for the variable.
-                    let tail = generator.joined(separator: "/")
+                    let tail = pattern[currentIndex..<count].joined(separator: "/")
                     if tail.count > 0 {
                         params[variableNode.0] = pathToken + "/" + tail
                     } else {
@@ -127,37 +130,31 @@ open class HttpRouter {
                     return
                 }
                 params[variableNode.0] = pathToken
-                findHandler(&node.nodes[variableNode.0]!, params: &params, generator: &generator, matchedNodes: &matchedNodes, index: currentIndex, count: count)
+                findHandler(&node.nodes[variableNode.0]!, params: &params, pattern: pattern, matchedNodes: &matchedNodes, index: currentIndex, count: count)
             }
 
             if var node = node.nodes[pathToken] {
-                findHandler(&node, params: &params, generator: &generator, matchedNodes: &matchedNodes, index: currentIndex, count: count)
+                findHandler(&node, params: &params, pattern: pattern, matchedNodes: &matchedNodes, index: currentIndex, count: count)
             }
 
             if var node = node.nodes["*"] {
-                findHandler(&node, params: &params, generator: &generator, matchedNodes: &matchedNodes, index: currentIndex, count: count)
+                findHandler(&node, params: &params, pattern: pattern, matchedNodes: &matchedNodes, index: currentIndex, count: count)
             }
 
             if let startStarNode = node.nodes["**"] {
                 let startStarNodeKeys = startStarNode.nodes.keys
-                while let pathToken = generator.next() {
+                currentIndex += 1
+                while currentIndex < count, let pathToken = pattern[currentIndex].removingPercentEncoding {
                     currentIndex += 1
                     if startStarNodeKeys.contains(pathToken) {
-                        findHandler(&startStarNode.nodes[pathToken]!, params: &params, generator: &generator, matchedNodes: &matchedNodes, index: currentIndex, count: count)
+                        findHandler(&startStarNode.nodes[pathToken]!, params: &params, pattern: pattern, matchedNodes: &matchedNodes, index: currentIndex, count: count)
                     }
                 }
             }
-        } else if let variableNode = node.nodes.filter({ $0.0.first == ":" }).first {
-            // if it's the last element of the requested URL, check if there is a pattern with variable tail.
-            if variableNode.value.nodes.isEmpty {
-                params[variableNode.0] = ""
-                matchedNodes.append(variableNode.value)
-                return
-            }
         }
 
-        // if it's the last element and the path to match is done then it's a pattern matching
-        if node.nodes.isEmpty && index == count {
+        if node.isEndOfRoute && index == count {
+            // if it's the last element and the path to match is done then it's a pattern matching
             matchedNodes.append(node)
             return
         }

XCode/Tests/SwifterTestsHttpRouter.swift

@@ -103,7 +103,7 @@ class SwifterTestsHttpRouter: XCTestCase {
 
         XCTAssertEqual(router.route(nil, path: "/a/b/value1")?.0[":var"], "value1")
 
-        XCTAssertEqual(router.route(nil, path: "/a/b/")?.0[":var"], "")
+        XCTAssertEqual(router.route(nil, path: "/a/b/")?.0[":var"], nil)
     }
 
     func testHttpRouterPercentEncodedPathSegments() {
@@ -188,4 +188,88 @@ class SwifterTestsHttpRouter: XCTestCase {
         XCTAssertTrue(foundFirstVariableRoute)
         XCTAssertTrue(foundSecondVariableRoute)
     }
+    
+    func testHttpRouterShouldHandleOverlappingRoutesInTrail() {
+        let router = HttpRouter()
+        let request = HttpRequest()
+        
+        let firstVariableRouteExpectation = expectation(description: "First Variable Route")
+        var foundFirstVariableRoute = false
+        router.register("GET", path: "/a/:id") { request in
+            foundFirstVariableRoute = true
+            firstVariableRouteExpectation.fulfill()
+            return HttpResponse.accepted
+        }
+        
+        let secondVariableRouteExpectation = expectation(description: "Second Variable Route")
+        var foundSecondVariableRoute = false
+        router.register("GET", path: "/a") { _ in
+            foundSecondVariableRoute = true
+            secondVariableRouteExpectation.fulfill()
+            return HttpResponse.accepted
+        }
+        
+        let thirdVariableRouteExpectation = expectation(description: "Third Variable Route")
+        var foundThirdVariableRoute = false
+        router.register("GET", path: "/a/:id/b") { request in
+            foundThirdVariableRoute = true
+            thirdVariableRouteExpectation.fulfill()
+            return HttpResponse.accepted
+        }
+        
+        let firstRouteResult = router.route("GET", path: "/a")
+        let firstRouterHandler = firstRouteResult?.1
+        XCTAssertNotNil(firstRouteResult)
+        _ = firstRouterHandler?(request)
+        
+        let secondRouteResult = router.route("GET", path: "/a/b")
+        let secondRouterHandler = secondRouteResult?.1
+        XCTAssertNotNil(secondRouteResult)
+        _ = secondRouterHandler?(request)
+        
+        let thirdRouteResult = router.route("GET", path: "/a/b/b")
+        let thirdRouterHandler = thirdRouteResult?.1
+        XCTAssertNotNil(thirdRouteResult)
+        _ = thirdRouterHandler?(request)
+        
+        waitForExpectations(timeout: 10, handler: nil)
+        XCTAssertTrue(foundFirstVariableRoute)
+        XCTAssertTrue(foundSecondVariableRoute)
+        XCTAssertTrue(foundThirdVariableRoute)
+    }
+    
+    func testHttpRouterHandlesOverlappingPathsInDynamicRoutesInTheMiddle() {
+        let router = HttpRouter()
+        let request = HttpRequest()
+        
+        let firstVariableRouteExpectation = expectation(description: "First Variable Route")
+        var foundFirstVariableRoute = false
+        router.register("GET", path: "/a/b/c/d/e") { request in
+            foundFirstVariableRoute = true
+            firstVariableRouteExpectation.fulfill()
+            return HttpResponse.accepted
+        }
+        
+        let secondVariableRouteExpectation = expectation(description: "Second Variable Route")
+        var foundSecondVariableRoute = false
+        router.register("GET", path: "/a/:id/f/g") { _ in
+            foundSecondVariableRoute = true
+            secondVariableRouteExpectation.fulfill()
+            return HttpResponse.accepted
+        }
+        
+        let firstRouteResult = router.route("GET", path: "/a/b/c/d/e")
+        let firstRouterHandler = firstRouteResult?.1
+        XCTAssertNotNil(firstRouteResult)
+        _ = firstRouterHandler?(request)
+        
+        let secondRouteResult = router.route("GET", path: "/a/b/f/g")
+        let secondRouterHandler = secondRouteResult?.1
+        XCTAssertNotNil(secondRouteResult)
+        _ = secondRouterHandler?(request)
+        
+        waitForExpectations(timeout: 10, handler: nil)
+        XCTAssertTrue(foundFirstVariableRoute)
+        XCTAssertTrue(foundSecondVariableRoute)
+    }
 }

XCode/Tests/XCTestManifests.swift

@@ -1,11 +1,7 @@
-#if !canImport(ObjectiveC)
 import XCTest
 
 extension MimeTypeTests {
-    // DO NOT MODIFY: This is autogenerated, use:
-    //   `swift test --generate-linuxmain`
-    // to regenerate.
-    static let __allTests__MimeTypeTests = [
+    static let __allTests = [
         ("testCaseInsensitivity", testCaseInsensitivity),
         ("testCorrectTypes", testCorrectTypes),
         ("testDefaultValue", testDefaultValue),
@@ -14,24 +10,20 @@ extension MimeTypeTests {
 }
 
 extension SwifterTestsHttpParser {
-    // DO NOT MODIFY: This is autogenerated, use:
-    //   `swift test --generate-linuxmain`
-    // to regenerate.
-    static let __allTests__SwifterTestsHttpParser = [
+    static let __allTests = [
         ("testParser", testParser),
     ]
 }
 
 extension SwifterTestsHttpRouter {
-    // DO NOT MODIFY: This is autogenerated, use:
-    //   `swift test --generate-linuxmain`
-    // to regenerate.
-    static let __allTests__SwifterTestsHttpRouter = [
+    static let __allTests = [
         ("testHttpRouterEmptyTail", testHttpRouterEmptyTail),
         ("testHttpRouterHandlesOverlappingPaths", testHttpRouterHandlesOverlappingPaths),
         ("testHttpRouterHandlesOverlappingPathsInDynamicRoutes", testHttpRouterHandlesOverlappingPathsInDynamicRoutes),
+        ("testHttpRouterHandlesOverlappingPathsInDynamicRoutesInTheMiddle", testHttpRouterHandlesOverlappingPathsInDynamicRoutesInTheMiddle),
         ("testHttpRouterMultiplePathSegmentWildcards", testHttpRouterMultiplePathSegmentWildcards),
         ("testHttpRouterPercentEncodedPathSegments", testHttpRouterPercentEncodedPathSegments),
+        ("testHttpRouterShouldHandleOverlappingRoutesInTrail", testHttpRouterShouldHandleOverlappingRoutesInTrail),
         ("testHttpRouterSimplePathSegments", testHttpRouterSimplePathSegments),
         ("testHttpRouterSinglePathSegmentWildcard", testHttpRouterSinglePathSegmentWildcard),
         ("testHttpRouterSlashRoot", testHttpRouterSlashRoot),
@@ -40,10 +32,7 @@ extension SwifterTestsHttpRouter {
 }
 
 extension SwifterTestsStringExtensions {
-    // DO NOT MODIFY: This is autogenerated, use:
-    //   `swift test --generate-linuxmain`
-    // to regenerate.
-    static let __allTests__SwifterTestsStringExtensions = [
+    static let __allTests = [
         ("testBASE64", testBASE64),
         ("testMiscRemovePercentEncoding", testMiscRemovePercentEncoding),
         ("testMiscReplace", testMiscReplace),
@@ -54,21 +43,19 @@ extension SwifterTestsStringExtensions {
 }
 
 extension SwifterTestsWebSocketSession {
-    // DO NOT MODIFY: This is autogenerated, use:
-    //   `swift test --generate-linuxmain`
-    // to regenerate.
-    static let __allTests__SwifterTestsWebSocketSession = [
+    static let __allTests = [
         ("testParser", testParser),
     ]
 }
 
+#if !os(macOS)
 public func __allTests() -> [XCTestCaseEntry] {
     return [
-        testCase(MimeTypeTests.__allTests__MimeTypeTests),
-        testCase(SwifterTestsHttpParser.__allTests__SwifterTestsHttpParser),
-        testCase(SwifterTestsHttpRouter.__allTests__SwifterTestsHttpRouter),
-        testCase(SwifterTestsStringExtensions.__allTests__SwifterTestsStringExtensions),
-        testCase(SwifterTestsWebSocketSession.__allTests__SwifterTestsWebSocketSession),
+        testCase(MimeTypeTests.__allTests),
+        testCase(SwifterTestsHttpParser.__allTests),
+        testCase(SwifterTestsHttpRouter.__allTests),
+        testCase(SwifterTestsStringExtensions.__allTests),
+        testCase(SwifterTestsWebSocketSession.__allTests),
     ]
 }
 #endif