mux/v2: need feedback

[moraes]

I'd like to start a conversation and brainstorm about the future of gorilla/mux.

Background: gorilla/mux has evolved to be the most featureful router in Go, but this came with a price: the API feels bloated and has unneeded complications. It is also far from shining in benchmarks compared to other routers (not that gorilla/mux is a bottleneck to care about, but not-so-shiny numbers give a bad impression to new users).

I'd like to take gorilla/mux to the next level with these goals in mind:

1. Slim API;
2. Intuitive usage: simple rules, minimized unexpected behavior;
3. Improved performance;

After playing around, I came up with an implementation draft that is a radical departure from what gorilla/mux is now, and to achieve the slim-intuitive-fast goals some cuts were made:

• Merged Path(), Handler() and HandlerFunc();
• Merged URL(), URLHost() and URLPath();
• Moved Scheme, Host and PathPrefix matching to the route declaration syntax;
• Dropped duplicated Router/Route methods everywhere;

...but the most controversial ones are probably:

• Dropped query, header and custom matching;
• Dropped support for regular expressions;

The initial result is a much slimmer mux which, besides the elemental features (scheme, host, path and method matching), offers some conveniences not easily found in other packages (subrouting, URL building), and a damn high speed (teaser: it almost beats net/http in static matching and knocks out everybody else in matching with variables; and, oh, it doesn't make a single memory allocation for any match — zero, zip, zilch, nada).

But this is not gorilla/mux. It is something else. Maybe it doesn't even exist! Well, maybe it does. Enough teasing for now: I'd like to hear your crazy ideas, frustrations and aspirations for gorilla/mux, and to ask how could we manage an eventual API breakage in the best possible way.

[elithrar]

• I'm definitely for a slimmer API, even if it means (spitballing here!) a new package under the gorilla organization to allow mux to stay "stable" for existing users.
• Could add a note to the current mux (v1) README and a bunch of "this is how you would achieve X in mux v1 in the new mux v1" examples elsewhere to help push users to the new package.
• Coin a new name - gorilla/multiplex or even just gorilla/router (naming is hard)

Other thoughts:

• Merging Path, Handler and HandlerFunc is A++ in my books.
• Dropping the regex support might hurt a few users (there at least seems to be a history around it here in the GitHub issues) but is probably the right decision in terms of complexity. Without knowing what the new API looks like, you could facilitate custom matching (be it regex-based or otherwise) with a Pattern interface similar to Goji's.
• I'm okay with the query and header matching going. Those can be replicated in middleware and we can solve that with examples: I'd hazard a guess that they aren't often used and the semantics of matching on headers/queries aren't that great either.

Also worth reading (in case you haven't yet) is the plan to move net/context into the std. lib and make Context part of the http.Request struct: https://groups.google.com/d/msg/golang-dev/cQs1z9LrJDU/6_9FMmGNCQAJ (may factor into design decisions)

Keen to see where this is going.

[moraes]

• I thought about coining a new name; gorilla/muxy maybe, but I was afraid of it having a bad connotation instead of a cute/snarky one (although the word is labeled as "dated" and "dialect", it sounds funny to me).
• The thing I'm playing with doesn't use gorilla/context to store variables. It's a muxy trick, though (wink wink nudge nudge :stuck_out_tongue_winking_eye:), and we should keep an eye on net/context.
• Dropping regexp support is a requirement for routes that doesn't depend on registered order. Registering a duplicated/ambiguous pattern should be an error and we can't detect this with regexps. The loss comes with some benefits: simplicity, predictability and speed. And there are always workarounds.
• Below is an API draft. These are not real interfaces in the Go sense; it is just to show how methods could look like, and how slim and simpler it could be. Everything is subject to change; I'm particularly inclined to try to find better names for Sub()/Subrouter:

Edit: I moved the API draft to a gist to update it as proposals arrive. Here it is:

https://gist.github.com/moraes/fba14ffd6c3e091b2c68

• Middleware support was not considered (yet); it may be included in the subrouter interface, I guess.

[moraes]

Declaration syntax draft. Nothing is set in stone, etc.

// creates a new router
r := New()

// -------------------------------------------
// basic usage: host, path and method matching
// -------------------------------------------
// matches a static path
r.Handle("/foo/bar", myHandler)
// matches a path with a variable
r.Handle("/foo/{bar}", myHandler)
// matches a path prefix
r.Handle("/foo/{*}", myHandler)
// matches a path with a variable and the given HTTP verb.
r.Handle("/foo/{bar}", myHandler).Methods("GET")
// matches a host and path
r.Handle("www.mydomain.com/foo/bar", myHandler)

// ------------------------------
// extended usage: path subrouter
// ------------------------------
s := r.Sub("/users")
s.Handle("/new", myHandler)      // matches "/users/new"
s2 := s.Sub("/{id}")
s2.Handle("/", myHandler)        // matches "/users/{id}/"
s2.Handle("/tickets", myHandler) // matches "/users/{id}/tickets"

// ------------------------------
// extended usage: host subrouter
// ------------------------------
s := r.Sub("{subdomain}.mydomain.com")
s.Handle("/foo/{bar}", myHandler) // matches "{subdomain}.mydomain.com/foo/{bar}"

[kisielk]

I think we should wait till the net/context situation is resolved before making the API. It could totally change the design and drop the need to use gorilla/context at all. The other option is to build it with the current x/net/context for now and then migrate to the standard library version.

[moraes]

Setting/retrieving variables can be left as "undefined" or "subject to changes" until a context resolution comes, and we take our time to discuss features, declaration syntax and the broader general API.

That said, I'll talk a bit about...

Declaration syntax

The general idea is that we don't need to have Schemes(), Host(), Path() and PathPrefix() methods: we can support them all in the declared pattern to be matched. As mentioned before:

r := mux.New()
r.Handle("/foo", myHandler)     // matches "/foo" in any scheme and host
r.Handle("/bar/{*}", myHandler) // matches the path prefix "/bar/" in any scheme and host
s1 := r.Sub("//{subdomain}.mydomain.com")
s1.Handle("/{baz}", myHandler)   // matches "//{subdomain}.mydomain.com/{baz}" in any scheme
s2 := r.Sub("https://{subdomain}.mydomain.com")
s2.Handle("/{baz}", myHandler)   // matches "https://{subdomain}.mydomain.com/{baz}"

Notes:

• Variables are declared like in gorilla/mux, minus the regexp support. That is, a variable name is enclosed by curled braces, as in {name};
• A special variable {*} is used exclusively in the end of a path to declare path prefixes;
• Extra constraint: each host or path segment can have only one variable (so you cant have a /user-{id} path; it must be /{user-id});
• If a scheme is not defined but a host is, the pattern must start with two slashes (like in the first subrouter above). In case scheme matching was not supported, this could be simplified a bit (we'd consider a host definition anything before the first slash), but I don't think it should be dropped;
• The simpler colon prefix syntax used by sinatra (and later pat and others, e.g. /:name) was considered, but several concerns came to mind: 1. It doesn't look good in host definitions (e.g. www.:domain.com; 2. It looks even worse with an eventual support for mixed static and variable parts in a segment (e.g. /user-:id); 3. In case point 2 ever happens, or no matter what happens, colon is a valid character in hosts and paths (RFC 3986) and curly braces are not valid characters in any URI part (this was taken into account to choose curly braces for gorilla/mux; the other considered options were [] or <>);

That's all for now. Let me know what you hate most, other ideas etc. :v:

[garyburd]

I like the model where dispatching is logically by path and then method:

type Router interface {
    Route(pattern string) *Route // defines a pattern to match: scheme, host, path and/or path prefix definitions
    ...
}

type Route interface {
    // Get specifies handler for GET and HEAD if not otherwise specified.
    Get(h http.HandlerFunc) Route

    // Post specifies the handler for POST.
    Post(h http.HandlerFunc)  Route

    // Handler specifies the handler the specified methods.  If len(m) == 0, then h is 
    // used as a fallback when there are no specific method matches for the route.
    Handler(h http.HandlerFunc, m ...string) Route 
}

Examples:

 r.Route("/foo/{bar}").Get(showFoo).Post(updateFoo)
 r.Route("/a").Handler(aHandler, "GET",  "POST") // GET and POST go to aHandler
 r.Route("/b").Handler(bHandler)  // all methods go to bHandler

If there's no match on method for a given route, than the mux replies with status 405.

[garyburd]

To handle URI path segments containing an encoded "/", the router should dispatch on req.RequestURI instead of req.URL.Path. Matched variables are percent decoded. For example:

r.Handle("/foo/{bar}/baz", serveFooBaz)

matches "/foo/hello%2fworld/baz" with the variable bar set to "hello/world".

[garyburd]

More specific patterns should override less specific patterns. In this example,

r.Handle("/{user}", serveUser)
r.Handle("/help", serveHelp)

the path "/help" should dispatch to serveHelp.

Left to right order should be used as a tiebreaker. In this example

r.Handle("/{user}/followers", serveUserFollwers)
r.Handle("/help/{topic}", serveHelpTopic)

the path "/help/folllowers" should dispatch to serveHelpTopic with variable "topic" set to followers.

[garyburd]

The router should automatically handle redirects for "/" at end of the path.

• If path does not match, path does not have "/" suffix and path + "/" does match, then redirect to path + "/".
• If path does not match, path has "/" suffix and path[:len(path) -1] does match, then redirect to path[:len(path) -1]

[moraes]

@margaery: I took a quick look at RFC 6570, and that's a huge spec. We are doing simple string expansion with {name}, which looks like their syntax; but this proposal has a lot more strict rules for their use ("alone in path or host segments"), so the similarity ends where it began. I like that it reminds RFC 6570, though; it suggests that braces are a good choice: they stand out and make variables easy to scan at a glance.

@garyburd:

1. Re: r.RequestURI: good point, or r.URL.RawPath but maybe it is too soon for this one;
2. Re: More specific patterns have priority and left to right specificity: that's the idea;
3. Re: redirect path-with-no-trailing-slash to path-with-trailing-slash and vice-versa: it's on the checklist;
4. Re: r.Route("/foo/{bar}").Get(showFoo).Post(updateFoo) (or .Handler(aHandler, "GET", "POST")): this looks much better than the initial proposal, and makes more sense too.

[elithrar]

Gary's requests are all great - automatic slashes are very useful.

One additional thing on my list: support for automatic OPTIONS responses, whether built-in or via providing a hook to construct the response. Being able to OPTIONS /some-path and get the supported methods makes CORS much easier to reason with.

[garyburd]

Some more suggestions:

• Provide a mechanism for wrapping subrouters with a "middleware" handler. This handler can be used to implement authorization for a branch of the namepace and other features.
• Add mechanism for specifying an application supplied function for handling 404 and 405 responses.

• Specify router options using functional options on the New() function. Here's an example:

  type Option struct { /* unexported fields */ }  // this can also be func(*options)  
  
  func New(options ...Option) Router
  
  // WithErrorHandler specifies the function the router calls to generate error
  //  responses. The router calls http.Error by default.
  func WithErrorHandler(f func(status int, w http.ResponseWriter, r *http.Request)) Option

[elithrar]

• Agreed RE: middleware. I touched on this in the mailing list. A Use(...http.Handler) method or Middleware method that you can use to chain http.Handlers to all the routes within a subrouter is definitely useful for auth, verbose logging, CSRF, etc. Middleware applied to a top-level router should also apply to all subrouters below it by default.
• I love functional options, but having ErrorHandler(http.Handler) as a method on *Router is probably easier for most to understand. ErrorHandler should accept a http.Handler so you pass it anything you want - either a simple http.HandlerFunc or something more complex.

[moraes]

@elithrar: A default OPTIONS handler would be trivial to define, as well as a 405 handler that sets a proper Allow header (somewhat related). This remained unsolvable in current gorilla/mux because the same URL pattern can lead to different Routes, and it is not easy to identify "same URL pattern" because of regexps and separated matchers for host, scheme etc. In the new design each URL pattern always leads to the same Route, which knows about the methods it supports.

@garyburd, @elithrar: RE middleware in subrouters: I think we are again on the right direction here, because subrouters are just dumb factories that create routes with a given pattern prefix (they are not "nested"; they just carry a prefix). A subrouter could also carry a middleware list to pass to routes (or to sub-subrouters, which could have extra middleware). The exact API is something to discuss (.Sub(pattern string, midlewares ...InterfaceOrWhatever) ?).

Re: functional options: I need to think about it. The example was not very obvious to me, but I'm tired or something is missing.

I moved the API draft to a gist to update it as proposals arrive. Here it is:

https://gist.github.com/moraes/fba14ffd6c3e091b2c68

[garyburd]

@moraes How about creating a repo for the new mux and writing the proposal as godoc? People can propose changes or request features using issues and PRs in that repo.

[moraes]

New repo or new branch? I want to push my prototype in the following days.

[garyburd]

I suggested a new repo because I assumed that the new mux will be published in a new repo. Where do you plan publish the new mux?

[elithrar]

New repo on a non-master branch (and push a one-liner README to the master)? Will prevent people from pulling it 'accidentally'.

On Mon, Oct 26, 2015 at 9:28 AM Gary Burd notifications@github.com wrote:

I suggested a new repo because I assumed that the new mux will be published in a new repo. Where do you plan publish the new mux?

— Reply to this email directly or view it on GitHub https://github.com/gorilla/mux/issues/130#issuecomment-150998473.

[moraes]

New repo is ok. :) The spice must flow.

[moraes]

This discussion has now its own place: gorilla/muxy. It may be a temporary repo or not. We will see.

• There isn't a lot more than the API skeleton there. Sorry for the vaporware sound (heh), but I'll need some time to put together what I've done so far;
• It gives a better idea of how subrouting is implemented: Subrouter is a factory for routes with a prefix. The implementation doesn't need much more than what is there already (except that it may include middleware);
• Let's create separated issues for each subject that comes up (middleware, config, declaration syntax, naming, testing, performance etc).

Thanks for the great feedback so far. :+1:

[CpuID]

+1 to the idea of returning HTTP 405's in the appropriate places (eg. if a request is made to a URI that has a .Methods() filter on it that doesn't allow it)

[elithrar]

The new (in progress) https://github.com/gorilla/muxy library should be able to handle this.

On Thu, Jan 7, 2016 at 12:42 PM Nathan Sullivan notifications@github.com wrote:

+1 to the idea of returning HTTP 405's in the appropriate places (eg. if a request is made to a URI that has a .Methods() filter on it that doesn't allow it)

— Reply to this email directly or view it on GitHub https://github.com/gorilla/mux/issues/130#issuecomment-169799054.