diff --git a/.gitignore b/.gitignore index cff3dd9c..7fca6d36 100644 --- a/.gitignore +++ b/.gitignore @@ -2,4 +2,4 @@ Packages .build .DS_Store *.xcodeproj - +Package.pins diff --git a/.travis.yml b/.travis.yml index e806e0fd..42c79c20 100644 --- a/.travis.yml +++ b/.travis.yml @@ -6,5 +6,5 @@ sudo: required dist: trusty osx_image: xcode8 script: - - eval "$(curl -sL https://swift.vapor.sh/ci)" + - eval "$(curl -sL https://swift.vapor.sh/ci-3.1)" - eval "$(curl -sL https://swift.vapor.sh/codecov)" diff --git a/Sources/Routing/Branch.swift b/Sources/Routing/Branch.swift index 077853f8..a02082c9 100644 --- a/Sources/Routing/Branch.swift +++ b/Sources/Routing/Branch.swift @@ -115,23 +115,14 @@ public class Branch { // TODO: Rename Context return output != nil } - /** - key or : or * - - If it is a `key`, then it connects to an additional branch. - - If it is `:`, it is a slug point and the name - represents a key for a dynamic value. - */ - internal fileprivate(set) var subBranches: [String: Branch] = [:] - + internal fileprivate(set) var subBranches = SubBranchMap() /** Fallback routes allow various handlers to "catch" any subsequent paths on its branch that weren't otherwise matched */ private var fallback: Output? { - return subBranches["*"]?.value + return subBranches.wildcard?.value } /** @@ -173,23 +164,29 @@ public class Branch { // TODO: Rename Context var comps = path guard let key = comps.next() else { return BranchResult(self, comps) } - if let result = subBranches[key]?.fetch(comps), result.branch.hasValidOutput { + // first check if direct path exists + if let result = subBranches.paramBranches[key]?.fetch(comps), result.branch.hasValidOutput { return result } - if let result = subBranches[":"]?.fetch(comps), result.branch.hasValidOutput { + // next attempt to find slug matches if any exist + for slug in subBranches.slugBranches { + guard let result = slug.fetch(comps), result.branch.hasValidOutput else { continue } return result } - if let result = subBranches["*"]?.fetch(comps), result.branch.hasValidOutput { + // see if wildcard with path exists + if let result = subBranches.wildcard?.fetch(comps), result.branch.hasValidOutput { return result } - if let wildcard = subBranches["*"], wildcard.hasValidOutput { + // use fallback + if let wildcard = subBranches.wildcard, wildcard.hasValidOutput { let subRoute = [key] + comps return BranchResult(wildcard, subRoute.makeIterator()) } + // unmatchable route return nil } @@ -216,27 +213,21 @@ public class Branch { // TODO: Rename Context return self } - let link = key.characters.first == ":" ? ":" : key - let next = subBranches[link] ?? type(of: self).init(name: key, output: nil) - if next.name != key { - var warning = "[WARNING] Mismatched Slugs:\n" - warning += "Attempted to overwrite \(next.name) with \(key)\n" - warning += "Please use the same slug name for all routes on shared branch" - print(warning) - } + let next = subBranches[key] ?? type(of: self).init(name: key, output: nil) next.parent = self // trigger lazy loads at extension time -- seek out cleaner way to do this _ = next.path _ = next.depth _ = next.slugIndexes - subBranches[link] = next + + subBranches[key] = next return next.extend(path, output: output) } } extension Branch { internal func testableSetBranch(key: String, branch: Branch) { - subBranches[key] = branch + subBranches.paramBranches[key] = branch branch.parent = self } } diff --git a/Sources/Routing/Router+Routes.swift b/Sources/Routing/Router+Routes.swift index c5934e3a..1eb11827 100644 --- a/Sources/Routing/Router+Routes.swift +++ b/Sources/Routing/Router+Routes.swift @@ -37,7 +37,7 @@ extension Branch { // Get all individual branch nodes extending out from, and including self internal var allIndividualBranchesInTreeIncludingSelf: [Branch] { var branches: [Branch] = [self] - branches += subBranches.values.flatMap { $0.allIndividualBranchesInTreeIncludingSelf } + branches += subBranches.allBranches.flatMap { $0.allIndividualBranchesInTreeIncludingSelf } return branches } } diff --git a/Sources/Routing/SubBranchMap.swift b/Sources/Routing/SubBranchMap.swift new file mode 100644 index 00000000..52107cb1 --- /dev/null +++ b/Sources/Routing/SubBranchMap.swift @@ -0,0 +1,46 @@ +struct SubBranchMap { + var paramBranches: [String: Branch] = [:] + var slugBranches: [Branch] = [] + var wildcard: Branch? + + var allBranches: [Branch] { + var all = [Branch]() + all += paramBranches.values + all += slugBranches + if let wildcard = wildcard { + all.append(wildcard) + } + return all + } + + init() {} + + subscript(key: String) -> Branch? { + get { + if key == "*" { + return wildcard + } else if key.hasPrefix(":") { + return slugBranches.lazy.filter { $0.name == key } .first + } else { + return paramBranches[key] + } + } + set { + if key == "*" { + wildcard = newValue + } else if key.hasPrefix(":") { + if let existing = slugBranches.index(where: { $0.name == key }) { + if let value = newValue { + slugBranches[existing] = value + } else { + slugBranches.remove(at: existing) + } + } else if let value = newValue { + slugBranches.append(value) + } + } else { + paramBranches[key] = newValue + } + } + } +} diff --git a/Tests/RoutingTests/RouteExtractionTests.swift b/Tests/RoutingTests/RouteExtractionTests.swift index dcc13988..f07a9770 100644 --- a/Tests/RoutingTests/RouteExtractionTests.swift +++ b/Tests/RoutingTests/RouteExtractionTests.swift @@ -43,7 +43,7 @@ class RouteExtractionTests: XCTestCase { c.testableSetBranch(key: "e", branch: e) let allBranches = a.allIndividualBranchesInTreeIncludingSelf.map { $0.name } - XCTAssertEqual(allBranches, [a, b, c, d, e].map { $0.name }) + XCTAssertEqual(Set(allBranches), Set([a, b, c, d, e].map { $0.name })) } func testIndividualBranchesWithOutput() throws { diff --git a/Tests/RoutingTests/RouterTests.swift b/Tests/RoutingTests/RouterTests.swift index 86743daf..0dc9a014 100644 --- a/Tests/RoutingTests/RouterTests.swift +++ b/Tests/RoutingTests/RouterTests.swift @@ -6,7 +6,7 @@ import URI extension String: Swift.Error {} class RouterTests: XCTestCase { - static var allTests = [ + static let allTests = [ ("testRouter", testRouter), ("testWildcardMethod", testWildcardMethod), ("testWildcardHost", testWildcardHost), @@ -15,7 +15,8 @@ class RouterTests: XCTestCase { ("testWildcardPath", testWildcardPath), ("testParameters", testParameters), ("testEmpty", testEmpty), - ("testNoHostWildcard", testNoHostWildcard) + ("testNoHostWildcard", testNoHostWildcard), + ("testRouterDualSlugRoutes", testRouterDualSlugRoutes), ] func testRouter() throws { @@ -173,4 +174,20 @@ class RouterTests: XCTestCase { let response = try handler?(request).makeResponse() XCTAssert(response?.body.bytes?.string == "Hello, World!") } + + func testRouterDualSlugRoutes() throws { + let router = Router() + router.register(path: ["*", "GET", "foo", ":a", "one"], output: 1) + router.register(path: ["*", "GET", "foo", ":b", "two"], output: 2) + + let containerOne = BasicContainer() + let outputOne = router.route(path: ["*", "GET", "foo", "slug-val", "one"], with: containerOne) + XCTAssertEqual(outputOne, 1) + XCTAssertEqual(containerOne.parameters["a"]?.string, "slug-val") + + let containerTwo = BasicContainer() + let outputTwo = router.route(path: ["*", "GET", "foo", "slug-val", "two"], with: containerTwo) + XCTAssertEqual(outputTwo, 2) + XCTAssertEqual(containerTwo.parameters["b"]?.string, "slug-val") + } } diff --git a/circle.yml b/circle.yml index 279237bf..ec872e2d 100644 --- a/circle.yml +++ b/circle.yml @@ -1,3 +1,3 @@ test: override: - - eval "$(curl -sL https://swift.vapor.sh/ci)" + - eval "$(curl -sL https://swift.vapor.sh/ci-3.1)"