Merge pull request #5 from vapor/multi-slug

2.0 - multi slugs

.gitignore

@@ -2,4 +2,4 @@ Packages
 .build
 .DS_Store
 *.xcodeproj
-
+Package.pins

.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)"

Sources/Routing/Branch.swift

@@ -115,23 +115,14 @@ public class Branch<Output> { // 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<Output>()
 
     /**
          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<Output> { // 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<Output> { // 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
     }
 }

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
     }
 }

Sources/Routing/SubBranchMap.swift

@@ -0,0 +1,46 @@
+struct SubBranchMap<Output> {
+    var paramBranches: [String: Branch<Output>] = [:]
+    var slugBranches: [Branch<Output>] = []
+    var wildcard: Branch<Output>?
+
+    var allBranches: [Branch<Output>] {
+        var all = [Branch<Output>]()
+        all += paramBranches.values
+        all += slugBranches
+        if let wildcard = wildcard {
+            all.append(wildcard)
+        }
+        return all
+    }
+
+    init() {}
+
+    subscript(key: String) -> Branch<Output>? {
+        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
+            }
+        }
+    }
+}

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 {

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<Int>()
+        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")
+    }
 }

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)"