Route accepts optional list of guard blocks

[1000jobboersen]

This feature brings the routing features in line with that of the erlang version. Thus, the following:

%% https://github.com/basho/riak_kv/blob/master/src/riak_kv_web.erl
[
  {["buckets", bucket, "keys"], fun is_post/1,
    riak_kv_wm_object, Props2},

  {["buckets", bucket, "keys"],
    riak_kv_wm_keylist, Props2},

  {["buckets", bucket, "keys", key],
    riak_kv_wm_object, Props2}
].

Becomes:

Webmachine.application.routes do
  add ["buckets", :bucket, "keys"],
    ->(r) { r.method == "POST" },
    Riak::KV::WM::Object
  add ["buckets", :bucket, "keys"], Riak::KV::WM::KeyList
  add ["buckets", :bucket, "keys", :key], Riak::KV::WM::Object
end

[seancribbs]

Why pass a proc and not a block?

add ["buckets", :bucket, "keys" ], Riak::KV::WM::Object do |r|
  r.method == 'POST'
end

I'm not diametrically opposed to procs, it just seems more Rubyish to use a block.

[bernerdschaefer]

I started with that originally, but thought there might be cases where you want multiple guards. Though, now I'm finding it difficult to come up with a real world example, but something like:

is_post = ->(r) { r.method == "POST" }
require_https = ->(r) { r.uri.scheme == "https" }

add ["*"], is_post, require_https,
  ->(r) { r.other_test == false },
  Resource

[bernerdschaefer]

Perhaps this would be a decent example -- something like a facebook graph API, where some information is public and available over http, while the rest must be accessed over https:

is_post = ->(r) { r.method == "POST" }
require_https = ->(r) { r.uri.scheme == "https" }

# GET  https://site/graph/user/:id
# POST https://site/graph/user/:id/messages
# GET  https://site/graph/user/:id/messages
# GET  https://site/graph/user/:id/messages/:id
# PUT  https://site/graph/user/:id/messages/:id
add ["graph", "user", :id], require_https,
  Graph::Private::User
add ["graph", "user", :user, "messages"], require_https, is_post,
  Graph::Private::Message
add ["graph", "user", :user, "messages"], require_https,
  Graph::Private::MessageList
add ["graph", "user", :user, "messages", :id], require_https,
  Graph::Private::Message

# GET  http://site/graph/user/:id
# GET  http://site/graph/user/:id/messages
# GET  http://site/graph/user/:id/messages/:id
add ["graph", "user", :user], Graph::Public::User
add ["graph", "user", :user, "messages"], Graph::Public::MessageList
add ["graph", "user", :user, "messages", :id], Graph::Public::Message

[emmanuel]

I think this is a great addition, in either block or list of procs form. I lean marginally towards the block form over the list of procs, but I think either would be great.

This is a fairly minor point, but I think Webmachine::Request should have some methods added to encapsulate access to common queries: HTTP methods, HTTPS, and probably some other things I'm not thinking of at the moment. Example:

class Webmachine::Request
  def post?
    method == "POST"
  end

  def put?
    method == "PUT"
  end

  def https?
    uri.scheme == "https"
  end
end

[emmanuel]

OK, almost easier to add the code than my preceding comment: #27

[jonathan]

I slightly prefer a list of procs over a block. The reason being that you could have a separate guards file (guards.rb?) that you include into your routes file. It would help keep your routes file smaller and you could more easily test your guard methods.

[seancribbs]

I've thought about this a bit longer, and I think it would be valuable to support both. We could go even farther and ducktype the procs (so as long as it responds to call it should be ok).

[emmanuel]

Using #call as the interface for the guard list is a great idea. This will make it easy to compose guards.

[emmanuel]

I'm personally not a fan of slurping args in an internal interface like this. I would propose either 1) use a signature that can work without the slurp in the public API (eg., put resource before guards and require an empty list be passed for guards if binding is provided: def initialize(path_spec, resource, guards, bindings = {})), or 2) implement #initialize as described in (1) and support the flexible (slurp-based) public API via a factory method that encapsulates this args processing and not burden #initialize with this logic (eg., def self.build(path_spec, *args)... new(path_spec, resource, guards, bindings)).

[seancribbs]

Anything that can reduce the complexity here would be helpful. I've done many splat-argument methods and they are difficult to maintain and understand. Definitely one place where I wish for pattern-matched function headers like Erlang has!

[seancribbs]

@bernerdschaefer I'm wanting to push a new gem version soon. Could you make the suggested changes in this PR so we can close it out?