Named Routing Parameters

vapor
feedback
vapor-4

(Tanner) #1

Named Routing Parameters

This is a request for feedback on an intended change to how routing works in Vapor. If feedback is positive, this change will likely land in Vapor 4.0.

Summary

Currently, in Vapor 3, route parameters are accessed by calling req.parameters.next(). Each call to next() removes and returns a single route parameter from the internal storage. Parameters are accessed in the order that they appear in the URL string.

/posts/42/comments/1337

r.get("posts", Int.parameter, "comments", Int.parameter) { req in
    let postID = try req.parameters.next(Int.self)
    let commentID = try req.parameters.next(Int.self)
    return "Post #\(postID) Comment #(commentID)"
}

Current Problem

Because parameters can only be accessed in the order they appear and parameter storage is mutated upon access, it is difficult to access arbitrary parameters without fetching all of them. It's also difficult to access parameters from different layers of an application, such as from both middleware and route closures. In these situations, parameter access must be carefully ordered to avoid unintended side effects.

The current API also does not make clear that registering multiple dynamic parameter types at a single point is not supported. For example, it is a common error to see routes registered like:

r.get("users", Int.parameter) { ... }
r.get("users", Double.parameter) { ... }

While one might expect both of these routes to be available, in reality only the Double route will be accessible since it was registered last.

Proposed Solution

In order to simplify parameter access, a system for naming route parameters is proposed. Each dynamic parameter in a given path will be given a unique name. The unique name can be used to later fetch the parameter from the request without mutating storage.

Here is an example of the proposed API:

/posts/42/comments/1337

r.get("posts", .postID, "comments", .commentID) { req in
    let commentID = try req.parameters.get(.commentID)
    let postID = try req.parameters.get(.postID)
    return "Post #\(postID) Comment #(commentID)"
}

extension PathComponent {
    static var postID: PathComponent {
        return .dynamic(name: "postID")
    }

    static var commentID: PathComponent {
        return .dynamic(name: "commentID")
    }
}

Note that the parameters can be accessed in any order.

By default, req.parameters.get(_:) will return a String. If a different type is desired, the get(_:as:) method can be used instead:

try req.parameters.get(.postID, as: Int.self)

The get(_:as:) method will also include a default value, allowing for compiler inference.

return try Post.find(req.parameters.get(.postID), on: self.db).first()

Stringly typed API

In addition to the strongly typed static extension to PathComponent, it could be possible to register dynamic path components by String using the prefix :.

/posts/42/comments/1337

r.get("posts", ":postID", "comments", ":commentID") { req in
    let postID = try req.parameters.get(":postID")
    let commentID = try req.parameters.get(":commentID")
    return "Post #\(postID) Comment #(commentID)"
}

While this API is slightly more prone to typos since the unique name must be written twice, it is less verbose than needing to add extensions. Inclusion of this stringly-typed addition to the API is dependent on feedback.

Detailed Design

PathComponent will be updated to the following:

enum PathComponent {
    case part(String)
    case dynamic(name: String)
    case anything
    case catchall
}

All router methods (i.e., r.get(...), r.post(...), etc.) will accept PathComponent....

ExpressibleByStringLiteral conformance will allow for paths to be registered concisely as strings.

extension PathComponent: ExpressibleByStringLiteral {
    init(stringLiteral value: String) {
        if value.hasPrefix(":") {
            self = .dynamic(name: value[1...])
        } else if value == ":" { 
            self = .anything
        } else if value == "*" { 
            self = .catchall
        } else {
            self = .part(value)
        }
    }
}

For example:

// normal route
r.get("users", "me") { ... }

// any parameter (discarded parameter)
r.get("users", ":", "posts")

// catchall
r.get("users", "*") { ... }

Deprecation of .next()

req.parameters.next() and Foo.parameter will continue to work after this change. However, I propose that the API should be deprecated in order to reduce confusion over existence of two separate methods for accessing parameters.


(Nathan Harris) #2

+1

-1. One of the big factors that lead me to choose Vapor (Swift in general) was the type safety introduced to establishing routes. This erases the expected value type when receiving it in the route handler at the configuration point and makes me search for the route handler to see how the parameter is being used.

My biggest fear is that, because this is the "easiest" to work with, learning materials and day-to-day implementations are going to favor this over using a property reference and we've lost a key point of using a static-typed language.

I think this is a great starting point for a discussion - but I think we can do better, either by passing a tuple or taking inspiration from Codable where a Router can be configured with say something like CodingKeys that provides the parameter and name.

protocol NamedParameter: CodingKey {
    var value: Parameter
}

enum MyRouteParameters: NamedParameter {
    case postID, commentID

    var parameter: Parameter {
        // switch on cases, eg. case .commentID: return Int.parameter
    }
}

let pr = r.withParameters(keyedBy: MyRouteParameters.self)
pr.get("posts", .postID, "comments", .commentID) { req in
    let commentID = try req.parameters.get(.postID)
    // etc.
}

(Gwendal Roué) #3

Hello,

I'm not a Vapor user, so please take my answer with a grain of salt.

I expect you'll get some pushback on the string-based API because many readers of those forums favor type-safety above all.

It may well be useful to explain the purpose of the string-based API. The reason why it is there. I'm not sure, but I guess that those strings are not only a leak of the underlying implementation that should be hidden. I also expect those strings to allow the design of dynamic routes (ones that are not tied to statically defined constants). But you know better.


(Jhonny Bill) #4

Deprecation of .next()

+1

Stringly typed API

-1
I think this will be too error prone + we will lose the type safety. I think the best is to enforce the use of a type safety approach [as in 'general' in Swift].

Proposed Solution

I like it, good start, - it's just I don't like to do:

 r.get("posts", .postID, "comments", .commentID)

And I think is kind of confusing.

Instead I would like for it to be:

 r.get(.postID, .commentID)

How?

- I'm sorry, I am too noob, but I'll write what I think -

We can make a protocol [ParameterProtocol], and then ask users to make an Enum conforming to that protocol so .get has a variadic ParameterProtocol argument.
E.g

ParameterProtocol

protocol ParameterProtocol {
    var routeParameter: String { get }
    var pathComponent: PathComponent { get }
}

Then, we can define an Enum like:

enum MyPathComponents: ParameterProtocol {
    case postID
    case commentID
    
    var routeParameter: String {
        switch self {
        case .postID:
            return "posts"
        case .commentID:
            return "comments"
        }
    }
    
    var pathComponent: PathComponent {
        switch self {
        case .postID:
            return .dynamic(name: "postID")
        case .commentID:
            return .dynamic(name: "commentID")
        }
    }
}

with this we can make the call:

r.get(MyPathComponents.postID, MyPathComponents.commentID)

which I think is cleaner, Swifty and safer.

The ultimate goal would be to get this call as

r.get(.postID, .commentID)

What I am not so sure about this approach is what if it is too much? or how easily will this be to newcomers ?


(Tim) #5

I mostly agree with the above - though having an optional stringly-typed method available to be more flexible isn't a bad idea, as long as the priority was given to those who require the flexibility.

IMO this is a bad idea - how do you then differentiate between /posts/<POSTID>/comments/<COMMENTID> and /<SOME_ID>/<ANOTHER_ID>. Additionally if the IDs were Strings (which Fluent supports) then you could cause route conflicts.

One thing that would be nice to see would be compiler warnings if routes were registered over the top of existing routes - not sure how feasible this is though


#6

+1

I'm not a huge fan of this being here along with the other method. I think if the proposed solution is too much boilerplate, the proposed solution probably isn't sufficient. I don't have a problem with the stringly typed API itself, but I think there should probably be one way to do it unless one of them offers different abilities.


#7

I pretty much agree with everyone's rejection of Stringly typed APIs.

Have any tests been run to see what sort of performance hit you'd take if you typecasted parameters when doing route matching? I think it would be doable as long as the parameter type conforms to LosslessDataConvertable

Edit: Got a PR for typecasting parameters, but this doesn't actually solve the problem at hand. Ignore me.


(Francois Green) #8

I like having a choice of APIs: Not everyone coming to Vapor will be steeped in Swift lore, nor will they care learn immediately. Further, for devs porting existing applications from other platforms, where "stringly" typed APIs might be common, the familiarity might allow them to get up and running faster. After all, no one is being forced to use it; it's an option.


(Kaden Wilkinson) #9

Could any of the new String Interpolation features be used here? If I remember correctly they add some great type safety that didn’t exist previously. Seemed like there was already some discussions to improve logging with Swift on the server and might be useful here as well?


(Tanner) #10

We can definitely do runtime warnings, but I can't think of anyway to push it to compile time.

I haven't, but it would definitely be slower. Though my main concern here is less about speed and more about having a simpler, more familiar model.

I agree 100%. This was the main reasoning behind adding it.


Furthering what I said in the initial post, I think having a simpler model here for how routing works would actually help us build better APIs going forward. For example, imagine something like this:

/posts/42/comments/1337

r.get("posts", \Post.id.parameter, "comments", \Comment.id.parameter) { req in
    let commentID = try req.parameters.get(\Comment.id)
    let postID = try req.parameters.get(\Post.id)
    return "Post #\(postID) Comment #(commentID)"
}

This would be possible with a couple simple extensions:

extension KeyPath {
    static var parameter: PathComponent {
        return .dynamic(name: "\(Root.self)_\(Value.self)")
    }
}

extension Parameters {
    func get<R, V>(_ keyPath: KeyPath<R, V>) throws -> V {
        return self.get(keyPath.parameter, as: V.self)
    }
}

(Eneko Alonso) #11

Would it be possible to do this third option in addition to the two above?

/posts/42/comments/1337

r.get("posts/:postID/comments/:commentID") { req in
    let commentID = try req.parameters.commentID
    let postID = try req.parameters.postID
    return "Post #\(postID) Comment #(commentID)"
}

Yes, yes, strings! I know, they lead to typos.

This, however, could make it super easy for people coming from other platforms. The idea is to parse the string url to extract the two parameters, then use Swift 4 dynamic properties to access them (as Strings)

Thoughts?


#12

Allowing the route to be written like

r.get("posts/:postID/comments/:commentID") { req in

makes more sense to me than letting you use ":postID" in the comma separated list, as this is commonly how you write routes in other frameworks.

Still not sure how I feel about dynamic properties


(Nathan Harris) #13

I don't see how they would be mutually exclusive.


#14

they aren't, you're right.


(Eneko Alonso) #15

Swift 4.2 dynamic properties are a bit scary, at first. But in reality, is not much different than retrieving a value from a dictionary, which needs to check at run-time if a value is stored under a given key.

This is, both req.parameters["postID"] and req.parameters.postID are resolved at run-time.

This is an excellent use case for dynamic properties in Swift.


(Tanner) #16
r.get("posts/:postID/comments/:commentID")

This part, yes. An additional case could be added to PathComponent to support an array of itself. i.e., PathComponent.array([PathComponent]).

let commentID = try req.parameters.commentID

This would not be possible since computed properties / subscripts cannot throw. The most we could do here is return an optional.

However, this would still work:

let commentID = try req.parameters.get(":commentID")

(Eneko Alonso) #17

Nice to hear about the path string, I feel it would be great and preferred by many developers.

For parameter retrieval, could we have all these?

let commentID = req.parameters.commentID // Optional
let commentID = try req.parameters.get(":commentID") // Non-optional
let commentID = try req.parameters.get(.commentID) // Non-optional

Using guard + throw for the optional one would be a bit redundant, though, as the assumption is this request handler would never get called if any of those two parameters were missing.


(Tanner) #18

Previously, we've used a combination of get and subscript for throwing and optional methods, respectively. One advantage to using subscripts that is we can have more than one argument. For example:

try req.parameters.get(":commentID", as: Int.self) // Int
req.parameters[":commentID", as: Int.self] // Int?

Dynamic property access could only result in a String. Although, I'm not sure that's necessarily a bad thing. It's pretty easy to map the String to another type, albeit more verbose.

guard let commentID = req.parameters.commentID.flatMap(Int.init) else {
    throw ...
}

That said, I think it might be confusing to have both a subscript and dynamic property API at the same time. Given the subscript API is more flexible, I would lean toward that.


#19

This is kind of an oddball, but what about something like this?

func get<T1, T2>(url: String, _ paramType1: T1.Type, _ paramType2.Type callback: (Request, T1, T2) -> Response)

r.get("posts/:postID/comments/:commentID", params: Post.ID.self, Comment.ID.self) { req, postID /*: Post.ID*/, commentID/*: Comment.ID*/ in
   ...
}

We could code gen up to ~6 parameters to capture the majority of use cases.

Edit: There's nothing stopping a mismatch between the number of dynamic params in the path and the number of parameter types, so that's something to either solve or mitigate with this one.


(Tim) #20

That’s like the routing from Vapor 1 I believe (@tanner0101 can correct me), which worked up to 3 parameters. Any more than that and you’d end up with compile times of around an hour....