Params validations do not run for route_param

[dapicester]

Hi, I found that validations do not run for route_param, here is a minimal example:

# file: route_param.ru

require 'grape'

class Bang < Grape::Validations::Base
  def validate_param!(attr_name, params)
    fail Grape::Exceptions::Validation, params: [@scope.full_name(attr_name)], message: 'bang'
  end
end

class RouteParam < Grape::API
  format :json
  default_format :json

  params do
    requires :name, type: String, bang: true
  end
  route_param :name do
    get do
      present :name, params[:name]
    end
  end
end

run RouteParam

Run with rackup route_param.ru and then curl localhost:9292/foo.
I expect a validation failed message, but instead I get {"name":"foo"}.

I am using version 0.14.0.

Updated: fix sample code typos

[dblock]

Routes are a little special. Route parameters are required to match the route and that their format is driven by requirements. However route parameters end up being regular parameters inside the API call, so validations would theoretically apply. Can you check that it's the case for a plain api call (not with route_param?). If that's the case then this would be a bug.

[dapicester]

It was a typo in my sample code. Now the sample works as expected but in the project I am working on it seems that validation is not run. Let me double check and try to reproduce the error. I will eventually close this if I cannot reproduce the error.

[dapicester]

I reproduced the problem. It seems that validations do not run when using mount in a route_param block.

Here is a sample code:

# file: route_param.rb

require 'grape'

class Bang < Grape::Validations::Base
  def validate_param!(attr_name, params)
    fail Grape::Exceptions::Validation, params: [@scope.full_name(attr_name)],
                                        message: 'Bang!'
  end
end

class Params < Grape::API
  get do
    { params: params }
  end
end

class RouteParam < Grape::API
  format :json
  default_format :json

  version 'v1', using: :path do
    params do
      requires :name, type: String, allow_blank: false, bang: true
    end
    route_param :name do
      get do
        { params: params }
      end
    end
  end

  version 'v2', using: :path do
    params do
      requires :name, type: String, allow_blank: false, bang: true
    end
    route_param :name do
      mount Params
    end
  end
end

run RouteParam

Here is the error:

$ curl localhost:9292/v1/foo
{"error":"name Bang!"}

This is correct because the validation always fails.

$ curl localhost:9292/v2/foo
{"params":{"name":"foo"}}

This is not correct because the validation didn't run.

[dblock]

You should turn this into a minimal spec next. We have other issues around mount, it doesn't inherit parent settings and such, which is a symptom of a bigger problem, see #1177.

[qd3v]

route_param :party_id, type: Integer do
  mount Tracks => 'tracks'
end

# generates parties/tracks/:party_id on grape 0.12.0

That is really required feature (it's how I got here). Subscribing.

[dblock]

Thanks @vanburg, looking forward to some PRs!

[Arkanain]

Hi guys. Any news about this issue? Because it's really uncomfortable to use(in case of last example) constructions like declared(params).merge(party_id: params.party_id) inside mounted class instead of regular declared(params).

[Arkanain]

you can close this issue because all needed changes is already in latest master.

[dblock]

How was it fixed? What was the PR?

[dblock]

Oh I see, this was resolved via #1422.

[dblock]

@namusyaka you might want to double-check this issue in case I missed something

[Arkanain]

I check some specs on my project and I find out that this functionality work little bit wrong with declared when we have several route_param in one namespace.
For example we have:

class Artists
  namespace :artists do
    route_param :id, type: Integer do
      get do
        Artist.find(declared(params))
      end
    end
    route_param :artist_id, type: Integer do
      mount Compositions
    end
  end
end

In Artist.find(declared(params)) code declared(params) should return us next hash {id: 1}, but it return hash {id: 1, artist_id: nil}.

[dblock]

@Arkanain Reopen a new issue please with hopefully a spec? Thanks.

[namusyaka]

@dblock Yeah, I'll be careful. Thanks.

@Arkanain Your case can be reproduced in my environment and it seems like a bug. I'm going to dig the problem before long.

[Arkanain]

@dblock I will create new issue little bit later.
@namusyaka I find out where we have problem! Problem is in options for route_param. where I use route_param :id without type: Integer everything is ok. Maybe it's because in sources, as I understand, route_param is a 'sugar' for structure like:

params do
  requires :id, type: Integer
end
namespace ':id' do
  # some endpoints
end

That's why when we have two route_param in one namespace we have this wrong behaviour.
I rewrite my code from previous comment to:

class Artists
  namespace :artists do
    namespace ':id' do
      params do
        requires :id, type: Integer
      end
      get do
        Artist.find(declared(params))
      end
    end
    route_param :artist_id, type: Integer do
      mount Compositions
    end
  end
end

and bug disappeared. In inner namespace namespace ':id' I have correct hash {id: 1} when I call declared(params), and in mounted class, inside route_params :artist_id when I call declared(params) I also has right behaviour.

[dblock]

We do document the first behavior is legal, but I bet params is not being properly reset with each new namespace declaration. Looks like a bug, thanks for opening it @Arkanain, please.

[Arkanain]

@dblock As I promise previously I create pull request with specs for issue about I told you. Also I find one more bug in declared behaviour. Everything is here - #1430
@namusyaka I think you should also check this pull request.