Multiple ways of defining routes, params, validations etc.

[Brad Robertson]

Looking for a bit of feedback here. I want to rewrite the grape-swagger gem to make it a bit more flexible/useful, but in looking through grape I realized there are numerous ways of achieving the same thing (ie defining routes, documenting params etc).

I created a sample repo that includes a few of these differences and I just wanted to get your feedback as to which method going forward is going to be the standard API for such definitions.

As an example, when defining params for a route, one can define a desc method with a hash including aparameters key, or they can use the desc method with a block and call the parameters method, OR they can directly provide params using the params block. Should grape-swagger work for all of these methods?

I just want to find out exactly what I should be using to make sure my impl isn't obsolete when it comes out.

If someone can look through the repo, there's lots of comments in there outlining my confusing, and you can also run the app to see what I'm talking about (for param generation for instance). If you can just comment here with a few clarifications that will help me get started.

[Brad Robertson]

Here's what I've gleaned so far:

1. Block style `desc` seems to be the new grape API for describing methods. If both a block and hash are provided, the block overrides the hash (ie no keys from the hash get documented)

2. Hash style args to `desc` also work, but as mentioned, if a block is given, the hash seems to be ignored

3. Params specified inside the `desc`, using either block or hash method (above), don't actually lead to validation errors. They appear to be more about documentation

4. Params using the `params` block actually affect validation of the endpoint.

5. On inspecting a route, it's impossible to know if the params listed come from the desc or the params hash, thus we MUST assume to use ALL of the params listed when generating a spec like grape-swagger. This could lead to invalid documentation (ie if you listed a param as required in the desc, but didn't enforce that in the implementation) but I guess it's up to the person writing the Api to use the params block for strict validation.

6. I still don't know why `params` declarations from routes defined for an earlier route, apply to routes defined after, but params listed in the `desc` at the top do NOT apply to routes underneath.

[Daniel Doubrovkine]

Thanks Brad,

1. Desc should take a string and a block.
2. Hash is old style and is deprecated, I would be in favor of explicitly having a warning for now.
3. Params inside desc aren't a thing, the block however seems to take them. I am not sure whether we should make it into a bug and make the block DSL strict. I think we shouldn't because people put all kinds of things there.
4. Params with a block is the correct and preferred syntax.
5. Since (3), then you should just not worry about this.
6. Can you explain? Params apply to the next route only. There're namespace params that apply to the route.

--
You received this message because you are subscribed to the Google Groups "Grape Framework Discussion" group.
To unsubscribe from this group and stop receiving emails from it, send an email to ruby-grape+...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

--

dB. | Moscow - Geneva - Seattle - New York
code.dblock.org - @dblockdotorg - artsy.net - github/dblock

[Brad Robertson]

Thanks for the response Daniel,

For #6, if you check out https://github.com/bradrobertson/grape_api_questions you'll see what I'm talking about

If you clone it and load up irb, you can require the app.rb file which will give require all the endpoints.

Check out the windings.rb file. There's a Api::V1::Wingdings class. If you inspect on that, you'll see what I"m talking about.

In the first route 'POST /', we have 2 params that we require on wingding creation called "something" and "else". Further down, in the last route for wingdings 'POST /thangs', we ONLY have an optional "all" param. If I inspect Api::V1::Wingdings.routes.last however, I get the following:

#<Grape::Route:0x007f98023f36e8 @options={:params=>{"something"=>{:required=>true, :type=>"String"}, "else"=>{:required=>false, :type=>"String", :documentation=>{:example=>"Something Else"}}, "page"=>{:required=>false, :type=>"Integer"}, "per_page"=>{:required=>false, :type=>"Integer"}, "all"=>{:required=>false}}, :description=>"another way of creating", :prefix=>:special, :version=>"v1", :namespace=>"/wingdings", :method=>"POST", :path=>"/w/special/wingdings/thangs(.:format)", :compiled=>/\A\/w\/special\/wingdings\/thangs(?:\.(?<format>[^\/.?]+))?\Z/}>

Notice how "something" and "else" are listed here. Is this a bug or am I not specifying these params for each route properly?

[Daniel Doubrovkine]

The params thing sounds like a bug. Could you write a small spec for this in Grape maybe?

[Daniel Doubrovkine]

It took me a while to make heads and tails of the class inheritance you have here, maybe some of these issues come from there?

You have:

App < Grape::API

  mount V1

  mount V2

But then V1 and V2 inherit from App, sounds like a circular thing going on. You have to break that up.

[Daniel Doubrovkine]

I checked out the parameter accumulation problem and it's a bug on HEAD, https://github.com/intridea/grape/issues/833. 

[Daniel Doubrovkine]

And fixed, a bit tricky, https://github.com/intridea/grape/pull/834.

[Brad Robertson]

Ok didn't realize there might be an inheritance issue. I've fixed that up. Thanks for the tip.

Looks like your change works from a base 'params' standpoint, but the moment we try to route_param, the previous params declaration still leaks.

See this commit. You'll find once again if you look at the last route, it has 'page' and 'per_page' from the previous route.

I'd love to contribute in any way but I can't really make heads or tails of the source, nor the fix. 

[Daniel Doubrovkine]

As a general rule of thumb: if something doesn't work as expected, just open an issue with the details. I just did for this https://github.com/intridea/grape/issues/844.

Something that would be extremely helpful is if you tried to build a test case in Grape that reproduces the problem (an RSpec test that fails). Then PR it into Grape.