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