Skip to content

Support fail_fast param validation option #1499

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 8 commits into from
Oct 5, 2016
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 5 additions & 5 deletions .rubocop_todo.yml
Original file line number Diff line number Diff line change
@@ -1,29 +1,29 @@
# This configuration was generated by
# `rubocop --auto-gen-config`
# on 2016-09-11 17:59:25 +0900 using RuboCop version 0.39.0.
# on 2016-09-28 13:52:41 +0200 using RuboCop version 0.39.0.
# The point is for the user to remove these configuration records
# one by one as the offenses are removed from the code base.
# Note that changes in the inspected code, or installation of new
# versions of RuboCop, may require this file to be generated again.

# Offense count: 42
# Offense count: 41
Metrics/AbcSize:
Max: 44

# Offense count: 1
Metrics/BlockNesting:
Max: 4

# Offense count: 7
# Offense count: 8
# Configuration parameters: CountComments.
Metrics/ClassLength:
Max: 277
Max: 279

# Offense count: 28
Metrics/CyclomaticComplexity:
Max: 14

# Offense count: 933
# Offense count: 955
# Configuration parameters: AllowHeredoc, AllowURI, URISchemes.
# URISchemes: http, https
Metrics/LineLength:
Expand Down
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
* [#1486](https://github.com/ruby-grape/grape/pull/1486): Implemented except in values validator - [@jonmchan](https://github.com/jonmchan).
* [#1470](https://github.com/ruby-grape/grape/pull/1470): Drop support for ruby-2.0 - [@namusyaka](https://github.com/namusyaka).
* [#1490](https://github.com/ruby-grape/grape/pull/1490): Switch to Ruby-2.x+ syntax - [@namusyaka](https://github.com/namusyaka).
* [#1499](https://github.com/ruby-grape/grape/pull/1499): Support fail_fast param validation option - [@dgasper](https://github.com/dgasper).
* Your contribution here.

#### Fixes
Expand Down
19 changes: 19 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -1355,6 +1355,25 @@ subject.rescue_from Grape::Exceptions::ValidationErrors do |e|
end
```

Grape returns all validation and coercion errors found by default.
To skip all subsequent validation checks when a specific param is found invalid, use `fail_fast: true`.

The following example will not check if `:wine` is present unless it finds `:beer`.
```ruby
params do
required :beer, fail_fast: true
required :wine
end
```
The result of empty params would be a single `Grape::Exceptions::ValidationErrors` error.

Similarly, no regular expression test will be performed if `:blah` is blank in the following example.
```ruby
params do
required :blah, allow_blank: false, regexp: /blah/, fail_fast: true
end
```

### I18n

Grape supports I18n for parameter-related error messages, but will fallback to English if
Expand Down
2 changes: 2 additions & 0 deletions lib/grape/endpoint.rb
Original file line number Diff line number Diff line change
Expand Up @@ -343,8 +343,10 @@ def run_validators(validators, request)
validator.validate(request)
rescue Grape::Exceptions::Validation => e
validation_errors << e
break if validator.fail_fast?
rescue Grape::Exceptions::ValidationArrayErrors => e
validation_errors += e.errors
break if validator.fail_fast?
end
end

Expand Down
25 changes: 15 additions & 10 deletions lib/grape/validations/params_scope.rb
Original file line number Diff line number Diff line change
Expand Up @@ -140,9 +140,14 @@ def require_optional_fields(context, opts)
end

def validate_attributes(attrs, opts, &block)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Have you tried to do this?

def validate_attributes(attrs, validations, opts = {}, &block)

This way there wouldn't be any reason to do the selection below.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Either way this seems to be a bit too much future-proof ;)

validations = opts.clone
validations[:type] ||= Array if block

options = {}
options[:fail_fast] = validations.delete(:fail_fast)

Copy link
Member

@dblock dblock Oct 3, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

... which begs the question, why is fail_fast so special? Why does it need to be split at all? Why can't it just be @fail_fast = validations[:fail_fast] and not pass anything more around?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Then I would have to split the options here, no?

Hmm, I guess I got a bit carried away with the future other options idea :)

In my opinion, the difference is that the rest of the validations refer to a specific validation type with options whereas fail_fast (and possible future validation options) influences the validation process across multiple validations.

But I could be wrong. I just dove into grape code for the first time :)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Try to make it simpler for now, if not splitting is less code, we should go with that.

validations = opts.clone
# additional options
option_keys = [:fail_fast]
options = opts.select { |key, _val| option_keys.include?(key) }

validations = opts.reject { |key, _val| option_keys.include?(key) }
validations[:type] ||= Array if block
validates(attrs, validations)

validates(attrs, validations, options)
end

# Returns a new parameter scope, subordinate to the current one and nested
Expand Down Expand Up @@ -201,7 +206,7 @@ def configure_declared_params
end
end

def validates(attrs, validations)
def validates(attrs, validations, opts = {})
doc_attrs = { required: validations.keys.include?(:presence) }

coerce_type = infer_coercion(validations)
Expand Down Expand Up @@ -232,18 +237,18 @@ def validates(attrs, validations)

# Validate for presence before any other validators
if validations.key?(:presence) && validations[:presence]
validate('presence', validations[:presence], attrs, doc_attrs)
validate('presence', validations[:presence], attrs, doc_attrs, opts)
validations.delete(:presence)
validations.delete(:message) if validations.key?(:message)
end

# Before we run the rest of the validators, let's handle
# whatever coercion so that we are working with correctly
# type casted values
coerce_type validations, attrs, doc_attrs
coerce_type validations, attrs, doc_attrs, opts

validations.each do |type, options|
validate(type, options, attrs, doc_attrs)
validate(type, options, attrs, doc_attrs, opts)
end
end

Expand Down Expand Up @@ -308,7 +313,7 @@ def check_coerce_with(validations)
# composited from more than one +requires+/+optional+
# parameter, and needs to be run before most other
# validations.
def coerce_type(validations, attrs, doc_attrs)
def coerce_type(validations, attrs, doc_attrs, opts)
check_coerce_with(validations)

return unless validations.key?(:coerce)
Expand All @@ -318,7 +323,7 @@ def coerce_type(validations, attrs, doc_attrs)
method: validations[:coerce_with],
message: validations[:coerce_message]
}
validate('coerce', coerce_options, attrs, doc_attrs)
validate('coerce', coerce_options, attrs, doc_attrs, opts)
validations.delete(:coerce_with)
validations.delete(:coerce)
validations.delete(:coerce_message)
Expand All @@ -337,12 +342,12 @@ def check_incompatible_option_values(values, default)
raise Grape::Exceptions::IncompatibleOptionValues.new(:default, default, :values, values)
end

def validate(type, options, attrs, doc_attrs)
def validate(type, options, attrs, doc_attrs, opts)
validator_class = Validations.validators[type.to_s]

raise Grape::Exceptions::UnknownValidator.new(type) unless validator_class

value = validator_class.new(attrs, options, doc_attrs[:required], self)
value = validator_class.new(attrs, options, doc_attrs[:required], self, opts)
@api.namespace_stackable(:validations, value)
end

Expand Down
8 changes: 7 additions & 1 deletion lib/grape/validations/validators/base.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,13 @@ class Base
# @param options [Object] implementation-dependent Validator options
# @param required [Boolean] attribute(s) are required or optional
# @param scope [ParamsScope] parent scope for this Validator
def initialize(attrs, options, required, scope)
# @param opts [Hash] additional validation options
def initialize(attrs, options, required, scope, opts = {})
@attrs = Array(attrs)
@option = options
@required = required
@scope = scope
@fail_fast = opts[:fail_fast] || false
end

# Validates a given request.
Expand Down Expand Up @@ -76,6 +78,10 @@ def options_key?(key, options = nil)
options = instance_variable_get(:@option) if options.nil?
options.respond_to?(:key?) && options.key?(key) && !options[key].nil?
end

def fail_fast?
@fail_fast
end
end
end
end
2 changes: 1 addition & 1 deletion lib/grape/validations/validators/default.rb
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
module Grape
module Validations
class DefaultValidator < Base
def initialize(attrs, options, required, scope)
def initialize(attrs, options, required, scope, opts = {})
@default = options
super
end
Expand Down
2 changes: 1 addition & 1 deletion lib/grape/validations/validators/values.rb
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
module Grape
module Validations
class ValuesValidator < Base
def initialize(attrs, options, required, scope)
def initialize(attrs, options, required, scope, opts = {})
@excepts = (options_key?(:except, options) ? options[:except] : [])
@values = (options_key?(:value, options) ? options[:value] : [])

Expand Down
40 changes: 40 additions & 0 deletions spec/grape/validations/params_scope_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -584,4 +584,44 @@ def initialize(value)
get '/nested', bar: { a: 'x', c: { b: 'yes' } }
expect(JSON.parse(last_response.body)).to eq('bar' => { 'a' => 'x', 'c' => { 'b' => 'yes' } })
end

context 'failing fast' do
context 'when fail_fast is not defined' do
it 'does not stop validation' do
subject.params do
requires :one
requires :two
requires :three
end
subject.get('/fail-fast') { declared(params).to_json }

get '/fail-fast'
expect(last_response.status).to eq(400)
expect(last_response.body).to eq('one is missing, two is missing, three is missing')
end
end
context 'when fail_fast is defined it stops the validation' do
it 'of other params' do
subject.params do
requires :one, fail_fast: true
requires :two
end
subject.get('/fail-fast') { declared(params).to_json }

get '/fail-fast'
expect(last_response.status).to eq(400)
expect(last_response.body).to eq('one is missing')
end
it 'for a single param' do
subject.params do
requires :one, allow_blank: false, regexp: /[0-9]+/, fail_fast: true
end
subject.get('/fail-fast') { declared(params).to_json }

get '/fail-fast', one: ''
expect(last_response.status).to eq(400)
expect(last_response.body).to eq('one is empty')
end
end
end
end