Skip to content

Commit d3806a1

Browse files
committed
Fix an issue in the HttpRouter causing the trail overlapping fails
* Remove the set of the empty string for not matched parameters, by default should be `nil` * Add unit tests to verify the trail dynamic identifier as end of route and with more dynamic path after the end of the route and the overlapping during the middle of the route * Update the `XCTestManifest` with the new tests for Linux
1 parent bdfd5f5 commit d3806a1

File tree

3 files changed

+114
-46
lines changed

3 files changed

+114
-46
lines changed

Sources/HttpRouter.swift

Lines changed: 16 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -93,7 +93,10 @@ open class HttpRouter {
9393
private func findHandler(_ node: inout Node, params: inout [String: String], generator: inout IndexingIterator<[String]>) -> ((HttpRequest) -> HttpResponse)? {
9494

9595
var matchedRoutes = [Node]()
96-
findHandler(&node, params: &params, generator: &generator, matchedNodes: &matchedRoutes, index: 0, count: generator.reversed().count)
96+
let pattern = generator.map { $0 }
97+
let numberOfElements = pattern.count
98+
99+
findHandler(&node, params: &params, pattern: pattern , matchedNodes: &matchedRoutes, index: 0, count: numberOfElements)
97100
return matchedRoutes.first?.handler
98101
}
99102

@@ -102,21 +105,21 @@ open class HttpRouter {
102105
/// - Parameters:
103106
/// - node: The root node of the tree representing all the routes
104107
/// - params: The parameters of the match
105-
/// - generator: The IndexingIterator to iterate through the pattern to match
108+
/// - pattern: The pattern or route to find in the routes tree
106109
/// - matchedNodes: An array with the nodes matching the route
107110
/// - index: The index of current position in the generator
108111
/// - count: The number of elements if the route to match
109-
private func findHandler(_ node: inout Node, params: inout [String: String], generator: inout IndexingIterator<[String]>, matchedNodes: inout [Node], index: Int, count: Int) {
112+
private func findHandler(_ node: inout Node, params: inout [String: String], pattern: [String], matchedNodes: inout [Node], index: Int, count: Int) {
110113

111-
if let pathToken = generator.next()?.removingPercentEncoding {
114+
if index < count, let pathToken = pattern[index].removingPercentEncoding {
112115

113116
var currentIndex = index + 1
114117
let variableNodes = node.nodes.filter { $0.0.first == ":" }
115118
if let variableNode = variableNodes.first {
116119
if currentIndex == count && variableNode.1.isEndOfRoute {
117120
// if it's the last element of the pattern and it's a variable, stop the search and
118121
// append a tail as a value for the variable.
119-
let tail = generator.joined(separator: "/")
122+
let tail = pattern[currentIndex..<count].joined(separator: "/")
120123
if tail.count > 0 {
121124
params[variableNode.0] = pathToken + "/" + tail
122125
} else {
@@ -127,37 +130,31 @@ open class HttpRouter {
127130
return
128131
}
129132
params[variableNode.0] = pathToken
130-
findHandler(&node.nodes[variableNode.0]!, params: &params, generator: &generator, matchedNodes: &matchedNodes, index: currentIndex, count: count)
133+
findHandler(&node.nodes[variableNode.0]!, params: &params, pattern: pattern, matchedNodes: &matchedNodes, index: currentIndex, count: count)
131134
}
132135

133136
if var node = node.nodes[pathToken] {
134-
findHandler(&node, params: &params, generator: &generator, matchedNodes: &matchedNodes, index: currentIndex, count: count)
137+
findHandler(&node, params: &params, pattern: pattern, matchedNodes: &matchedNodes, index: currentIndex, count: count)
135138
}
136139

137140
if var node = node.nodes["*"] {
138-
findHandler(&node, params: &params, generator: &generator, matchedNodes: &matchedNodes, index: currentIndex, count: count)
141+
findHandler(&node, params: &params, pattern: pattern, matchedNodes: &matchedNodes, index: currentIndex, count: count)
139142
}
140143

141144
if let startStarNode = node.nodes["**"] {
142145
let startStarNodeKeys = startStarNode.nodes.keys
143-
while let pathToken = generator.next() {
146+
currentIndex += 1
147+
while currentIndex < count, let pathToken = pattern[currentIndex].removingPercentEncoding {
144148
currentIndex += 1
145149
if startStarNodeKeys.contains(pathToken) {
146-
findHandler(&startStarNode.nodes[pathToken]!, params: &params, generator: &generator, matchedNodes: &matchedNodes, index: currentIndex, count: count)
150+
findHandler(&startStarNode.nodes[pathToken]!, params: &params, pattern: pattern, matchedNodes: &matchedNodes, index: currentIndex, count: count)
147151
}
148152
}
149153
}
150-
} else if let variableNode = node.nodes.filter({ $0.0.first == ":" }).first {
151-
// if it's the last element of the requested URL, check if there is a pattern with variable tail.
152-
if variableNode.value.nodes.isEmpty {
153-
params[variableNode.0] = ""
154-
matchedNodes.append(variableNode.value)
155-
return
156-
}
157154
}
158155

159-
// if it's the last element and the path to match is done then it's a pattern matching
160-
if node.nodes.isEmpty && index == count {
156+
if node.isEndOfRoute && index == count {
157+
// if it's the last element and the path to match is done then it's a pattern matching
161158
matchedNodes.append(node)
162159
return
163160
}

XCode/Tests/SwifterTestsHttpRouter.swift

Lines changed: 85 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -103,7 +103,7 @@ class SwifterTestsHttpRouter: XCTestCase {
103103

104104
XCTAssertEqual(router.route(nil, path: "/a/b/value1")?.0[":var"], "value1")
105105

106-
XCTAssertEqual(router.route(nil, path: "/a/b/")?.0[":var"], "")
106+
XCTAssertEqual(router.route(nil, path: "/a/b/")?.0[":var"], nil)
107107
}
108108

109109
func testHttpRouterPercentEncodedPathSegments() {
@@ -188,4 +188,88 @@ class SwifterTestsHttpRouter: XCTestCase {
188188
XCTAssertTrue(foundFirstVariableRoute)
189189
XCTAssertTrue(foundSecondVariableRoute)
190190
}
191+
192+
func testHttpRouterShouldHandleOverlappingRoutesInTrail() {
193+
let router = HttpRouter()
194+
let request = HttpRequest()
195+
196+
let firstVariableRouteExpectation = expectation(description: "First Variable Route")
197+
var foundFirstVariableRoute = false
198+
router.register("GET", path: "/a/:id") { request in
199+
foundFirstVariableRoute = true
200+
firstVariableRouteExpectation.fulfill()
201+
return HttpResponse.accepted
202+
}
203+
204+
let secondVariableRouteExpectation = expectation(description: "Second Variable Route")
205+
var foundSecondVariableRoute = false
206+
router.register("GET", path: "/a") { _ in
207+
foundSecondVariableRoute = true
208+
secondVariableRouteExpectation.fulfill()
209+
return HttpResponse.accepted
210+
}
211+
212+
let thirdVariableRouteExpectation = expectation(description: "Third Variable Route")
213+
var foundThirdVariableRoute = false
214+
router.register("GET", path: "/a/:id/b") { request in
215+
foundThirdVariableRoute = true
216+
thirdVariableRouteExpectation.fulfill()
217+
return HttpResponse.accepted
218+
}
219+
220+
let firstRouteResult = router.route("GET", path: "/a")
221+
let firstRouterHandler = firstRouteResult?.1
222+
XCTAssertNotNil(firstRouteResult)
223+
_ = firstRouterHandler?(request)
224+
225+
let secondRouteResult = router.route("GET", path: "/a/b")
226+
let secondRouterHandler = secondRouteResult?.1
227+
XCTAssertNotNil(secondRouteResult)
228+
_ = secondRouterHandler?(request)
229+
230+
let thirdRouteResult = router.route("GET", path: "/a/b/b")
231+
let thirdRouterHandler = thirdRouteResult?.1
232+
XCTAssertNotNil(thirdRouteResult)
233+
_ = thirdRouterHandler?(request)
234+
235+
waitForExpectations(timeout: 10, handler: nil)
236+
XCTAssertTrue(foundFirstVariableRoute)
237+
XCTAssertTrue(foundSecondVariableRoute)
238+
XCTAssertTrue(foundThirdVariableRoute)
239+
}
240+
241+
func testHttpRouterHandlesOverlappingPathsInDynamicRoutesInTheMiddle() {
242+
let router = HttpRouter()
243+
let request = HttpRequest()
244+
245+
let firstVariableRouteExpectation = expectation(description: "First Variable Route")
246+
var foundFirstVariableRoute = false
247+
router.register("GET", path: "/a/b/c/d/e") { request in
248+
foundFirstVariableRoute = true
249+
firstVariableRouteExpectation.fulfill()
250+
return HttpResponse.accepted
251+
}
252+
253+
let secondVariableRouteExpectation = expectation(description: "Second Variable Route")
254+
var foundSecondVariableRoute = false
255+
router.register("GET", path: "/a/:id/f/g") { _ in
256+
foundSecondVariableRoute = true
257+
secondVariableRouteExpectation.fulfill()
258+
return HttpResponse.accepted
259+
}
260+
261+
let firstRouteResult = router.route("GET", path: "/a/b/c/d/e")
262+
let firstRouterHandler = firstRouteResult?.1
263+
XCTAssertNotNil(firstRouteResult)
264+
_ = firstRouterHandler?(request)
265+
266+
let secondRouteResult = router.route("GET", path: "/a/b/f/g")
267+
let secondRouterHandler = secondRouteResult?.1
268+
XCTAssertNotNil(secondRouteResult)
269+
_ = secondRouterHandler?(request)
270+
271+
waitForExpectations(timeout: 10, handler: nil)
272+
XCTAssertTrue(foundFirstVariableRoute)
273+
XCTAssertTrue(foundSecondVariableRoute)
274+
}
191275
}

XCode/Tests/XCTestManifests.swift

Lines changed: 13 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,7 @@
1-
#if !canImport(ObjectiveC)
21
import XCTest
32

43
extension MimeTypeTests {
5-
// DO NOT MODIFY: This is autogenerated, use:
6-
// `swift test --generate-linuxmain`
7-
// to regenerate.
8-
static let __allTests__MimeTypeTests = [
4+
static let __allTests = [
95
("testCaseInsensitivity", testCaseInsensitivity),
106
("testCorrectTypes", testCorrectTypes),
117
("testDefaultValue", testDefaultValue),
@@ -14,24 +10,20 @@ extension MimeTypeTests {
1410
}
1511

1612
extension SwifterTestsHttpParser {
17-
// DO NOT MODIFY: This is autogenerated, use:
18-
// `swift test --generate-linuxmain`
19-
// to regenerate.
20-
static let __allTests__SwifterTestsHttpParser = [
13+
static let __allTests = [
2114
("testParser", testParser),
2215
]
2316
}
2417

2518
extension SwifterTestsHttpRouter {
26-
// DO NOT MODIFY: This is autogenerated, use:
27-
// `swift test --generate-linuxmain`
28-
// to regenerate.
29-
static let __allTests__SwifterTestsHttpRouter = [
19+
static let __allTests = [
3020
("testHttpRouterEmptyTail", testHttpRouterEmptyTail),
3121
("testHttpRouterHandlesOverlappingPaths", testHttpRouterHandlesOverlappingPaths),
3222
("testHttpRouterHandlesOverlappingPathsInDynamicRoutes", testHttpRouterHandlesOverlappingPathsInDynamicRoutes),
23+
("testHttpRouterHandlesOverlappingPathsInDynamicRoutesInTheMiddle", testHttpRouterHandlesOverlappingPathsInDynamicRoutesInTheMiddle),
3324
("testHttpRouterMultiplePathSegmentWildcards", testHttpRouterMultiplePathSegmentWildcards),
3425
("testHttpRouterPercentEncodedPathSegments", testHttpRouterPercentEncodedPathSegments),
26+
("testHttpRouterShouldHandleOverlappingRoutesInTrail", testHttpRouterShouldHandleOverlappingRoutesInTrail),
3527
("testHttpRouterSimplePathSegments", testHttpRouterSimplePathSegments),
3628
("testHttpRouterSinglePathSegmentWildcard", testHttpRouterSinglePathSegmentWildcard),
3729
("testHttpRouterSlashRoot", testHttpRouterSlashRoot),
@@ -40,10 +32,7 @@ extension SwifterTestsHttpRouter {
4032
}
4133

4234
extension SwifterTestsStringExtensions {
43-
// DO NOT MODIFY: This is autogenerated, use:
44-
// `swift test --generate-linuxmain`
45-
// to regenerate.
46-
static let __allTests__SwifterTestsStringExtensions = [
35+
static let __allTests = [
4736
("testBASE64", testBASE64),
4837
("testMiscRemovePercentEncoding", testMiscRemovePercentEncoding),
4938
("testMiscReplace", testMiscReplace),
@@ -54,21 +43,19 @@ extension SwifterTestsStringExtensions {
5443
}
5544

5645
extension SwifterTestsWebSocketSession {
57-
// DO NOT MODIFY: This is autogenerated, use:
58-
// `swift test --generate-linuxmain`
59-
// to regenerate.
60-
static let __allTests__SwifterTestsWebSocketSession = [
46+
static let __allTests = [
6147
("testParser", testParser),
6248
]
6349
}
6450

51+
#if !os(macOS)
6552
public func __allTests() -> [XCTestCaseEntry] {
6653
return [
67-
testCase(MimeTypeTests.__allTests__MimeTypeTests),
68-
testCase(SwifterTestsHttpParser.__allTests__SwifterTestsHttpParser),
69-
testCase(SwifterTestsHttpRouter.__allTests__SwifterTestsHttpRouter),
70-
testCase(SwifterTestsStringExtensions.__allTests__SwifterTestsStringExtensions),
71-
testCase(SwifterTestsWebSocketSession.__allTests__SwifterTestsWebSocketSession),
54+
testCase(MimeTypeTests.__allTests),
55+
testCase(SwifterTestsHttpParser.__allTests),
56+
testCase(SwifterTestsHttpRouter.__allTests),
57+
testCase(SwifterTestsStringExtensions.__allTests),
58+
testCase(SwifterTestsWebSocketSession.__allTests),
7259
]
7360
}
7461
#endif

0 commit comments

Comments
 (0)