Allow validation of nested parameters.

[tim-vandecasteele]

Based on the feedback of #222, I tried my own way at validation of nested parameters:

Using the group syntax which allows for:

group :user do
  requires :user_name
  group :admin do
    requires :first_admin_name, :last_admin_name
  end
end

In this implementation, I'm passing the scope to the validators, so they can take the relevant parameters.

[travisbot]

This pull request passes (merged d021e22 into 9bd4f23).

[bobbytables]

I think adding a 4'th parameter to the Validator class is starting to make it inundated. It might be better to start moving things into an options hash.

One thing I can see in the future is params requirements getting out of hand. Maybe we can build on this and give the ability to give it a class for defining requirements.

Example:

params UserParams

Where UserParams is:

class UserParams < Grape::ParamsScope
  group do
  end
end

[bobbytables]

Other than that, I love the concept of this and your implementation looks pretty good. I'd love to use this.

[kristianmandrup]

Never use 4 arguments. Usually 2 should be enough, a primary and an options IMO. Also, I recommend not to have more than 4 lines in any method... purist!

[dblock]

This looks like what doctor ordered, thanks @tim-vandecasteele. I want an update to the README, please and I'll merge it unless there're objections.

[travisbot]

This pull request passes (merged ec1fe76 into 9bd4f23).

[travisbot]

This pull request passes (merged 026c26e into 9bd4f23).

[schmurfy]

nice implementation, glad someone found the time for it !
There is a missing piece though, the documentation hash should includes the group, currently it does not:

I used this as reference:

require 'grape'

class TestAPI < Grape::API

  desc "add a user"
  params do
    group :user do
      requires :name, type: String
      requires :type, type: String
    end
  end
  post '/add_user' do

  end

end


p TestAPI.endpoints[0].options[:route_options][:params]

The current output of this script is:

{
  "name"=>{:required=>true, :type=>"String"},
  "type"=>{:required=>true, :type=>"String"}
}

Where the expected output would be more like this for me:

{
  "user" => {
    "name"=>{:required=>true, :type=>"String"},
    "type"=>{:required=>true, :type=>"String"}
  }
}

[tim-vandecasteele]

In fact I hit the same thing when trying to update grape-swagger. For now I adapted this by giving a full_name to a parameter, meaning you would get something like

{
  "name"=>{:required=>true, :type=>"String", :full_name=>"user[name]"},
  "type"=>{:required=>true, :type=>"String", :full_name=>"user[type]"}
}

Although I agree a nested hash would be better (for one to avoid clashes in parameters between different groups).

I'd have to check if I can change the params hash, although I'd think that will have a much bigger impact.

[schmurfy]

oh, that's funny: https://github.com/schmurfy/grape_apidoc/tree/ ;)

[travisbot]

This pull request passes (merged 3715500 into 9bd4f23).

[dblock]

I'm merging this, thank you. It also wraps up the parameter block implementation sufficiently IMO to make a release soon. Anybody things we're still missing something?

[dblock]

... maybe whitelisting discussed in #219 ...

[dpsk]

Thanks a lot for this feature!

[schmurfy]

I really don't like the user[name] syntax but it is better than nothing I guess.

[dpsk]

Yes, definitely better, i'm looking for solution like for 2 days and almost start to write it from scratch.
Not really related to this issue question, why i can't use validations in before block?

[schmurfy]

what do you want to do in the before block exactly ?
The validations implemented do not need any action from the user, just declare the rules and they will be enforced.

Do you want some global validations for your api ?

[dpsk]

I have same parametrs in every action of my api that should be sent, so i want to declare validations for them in one place.
Nested validations from your merge works perfectly when i drop them inside namespace block, but if i drop them outside nothing happen. I'm not sure if that is acceptable to declare validations in the root level of api.

[schmurfy]

global validations for every endpoint in one api class would indeed be interesting to have.

[dpsk]

Is it a big feature? Maybe you could guide me a bit and i will send a pull request with it?

[dblock]

+1 @dpsk I have pagination parameters that apply in many API calls, would be silly to have to repeat them everywhere. I think I would want to write something like this:

params do
   include pagination_params
end

This could be as simple as to look for a PaginationParams module and evaluate it in the context of the params block.

[schmurfy]

You can have a look at "lib/validations.rb" if you want to give it a try, it should not be too hard to implement.
How it works is that the api class has a settings class variables holding the parameters for the next defined endpoint, when the endpoint is created this hash like variables is cloned into it and emptied for the next endpoint definition.

My first thought would be to add another global hash holding options for all the endpoints and not only the next one and which would be merged into the endpoint specific one before the endpoint is created.

we could have something like:

global_params do
  requires :user_id, type: Integer
end

params do
  # ...
end

[dblock]

@schmurfy If you take my pagination example API that won't work. I don't want all my APIs to get this, just those that support pagination. A fully global feature isn't a bad idea either, authentication is a good example where someone just wants a global parameter set.

[schmurfy]

I agree this is not a one size fit all, maybe there is room for both ^^
yours is definitely better if you need to reuse validations maybe even between two apis, maybe my example above could even be implemented on top of an include style feature just to simplify what I think is a common use case for it while still allowing more complex behaviors.