Skip to content

Commit

Permalink
Rescue StandardError from explicit values validator procs (#1679)
Browse files Browse the repository at this point in the history
* Rescue StandardError from explicit values validator procs

* Update CHANGELOG.md

* Update CHANGELOG.md

* Code Review feedback

* Code Review feedback
  • Loading branch information
jlfaber authored and dblock committed Aug 25, 2017
1 parent af45b1d commit 37b7c47
Show file tree
Hide file tree
Showing 4 changed files with 36 additions and 4 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@

* [#1652](/~https://github.com/ruby-grape/grape/pull/1652): Fix missing backtrace that was not being bubbled up to the `error_formatter` - [@dcsg](/~https://github.com/dcsg).
* [#1661](/~https://github.com/ruby-grape/grape/pull/1661): Handle deeply-nested dependencies correctly - [@rnubel](/~https://github.com/rnubel), [@jnardone](/~https://github.com/jnardone).
* [#1679](/~https://github.com/ruby-grape/grape/pull/1679): Treat StandardError from explicit values validator proc as false - [@jlfaber](/~https://github.com/jlfaber).

### 1.0.0 (7/3/2017)

Expand Down
3 changes: 2 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -1179,7 +1179,8 @@ end

Alternatively, a Proc with arity one (i.e. taking one argument) can be used to explicitly validate
each parameter value. In that case, the Proc is expected to return a truthy value if the parameter
value is valid.
value is valid. The parameter will be considered invalid if the Proc returns a falsy value or if it
raises a StandardError.

```ruby
params do
Expand Down
11 changes: 8 additions & 3 deletions lib/grape/validations/validators/values.rb
Original file line number Diff line number Diff line change
Expand Up @@ -32,18 +32,23 @@ def validate_param!(attr_name, params)
unless check_excepts(param_array)

raise Grape::Exceptions::Validation, params: [@scope.full_name(attr_name)], message: message(:values) \
unless check_values(param_array)
unless check_values(param_array, attr_name)

raise Grape::Exceptions::Validation, params: [@scope.full_name(attr_name)], message: message(:values) \
if @proc && !param_array.all? { |param| @proc.call(param) }
end

private

def check_values(param_array)
def check_values(param_array, attr_name)
values = @values.is_a?(Proc) && @values.arity.zero? ? @values.call : @values
return true if values.nil?
return param_array.all? { |param| values.call(param) } if values.is_a? Proc
begin
return param_array.all? { |param| values.call(param) } if values.is_a? Proc
rescue StandardError => e
warn "Error '#{e}' raised while validating attribute '#{attr_name}'"
return false
end
param_array.all? { |param| values.include?(param) }
end

Expand Down
25 changes: 25 additions & 0 deletions spec/grape/validations/validators/values_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,13 @@ class API < Grape::API
{ type: params[:type] }
end

params do
requires :number, type: Integer, values: ->(v) { v > 0 }
end
get '/lambda_int_val' do
{ number: params[:number] }
end

params do
requires :type, values: -> { [] }
end
Expand Down Expand Up @@ -357,6 +364,24 @@ def app
expect(last_response.body).to eq({ error: 'type does not have a valid value' }.to_json)
end

it 'does not allow non-numeric string value for int value using lambda' do
get('/lambda_int_val', number: 'foo')
expect(last_response.status).to eq 400
expect(last_response.body).to eq({ error: 'number is invalid, number does not have a valid value' }.to_json)
end

it 'does not allow nil for int value using lambda' do
get('/lambda_int_val', number: nil)
expect(last_response.status).to eq 400
expect(last_response.body).to eq({ error: 'number does not have a valid value' }.to_json)
end

it 'allows numeric string for int value using lambda' do
get('/lambda_int_val', number: '3')
expect(last_response.status).to eq 200
expect(last_response.body).to eq({ number: 3 }.to_json)
end

it 'allows value using lambda' do
get('/lambda_val', type: 'valid-type1')
expect(last_response.status).to eq 200
Expand Down

0 comments on commit 37b7c47

Please sign in to comment.