diff --git a/CHANGELOG.md b/CHANGELOG.md index 6974707f95..6dccef1db0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,7 @@ #### Features * [#1594](https://github.com/ruby-grape/grape/pull/1594): Replace `Hashie::Mash` parameters with `ActiveSupport::HashWithIndifferentAccess` - [@james2m](https://github.com/james2m), [@dblock](https://github.com/dblock). +* [#1622](https://github.com/ruby-grape/grape/pull/1622): Add `except_values` validator to replace `except` option of `values` validator - [@jlfaber](https://github.com/jlfaber). * Your contribution here. #### Fixes diff --git a/README.md b/README.md index 8ea740fb2f..6bcd17ee08 100644 --- a/README.md +++ b/README.md @@ -807,6 +807,10 @@ end Note that default values will be passed through to any validation options specified. The following example will always fail if `:color` is not explicitly provided. +Default values are eagerly evaluated. Above `:non_random_number` will evaluate to the same +number for each call to the endpoint of this `params` block. To have the default evaluate +lazily with each request use a lambda, like `:random_number` above. + ```ruby params do optional :color, type: String, default: 'blue', values: ['red', 'green'] @@ -1114,9 +1118,6 @@ end Parameters can be restricted to a specific set of values with the `:values` option. -Default values are eagerly evaluated. Above `:non_random_number` will evaluate to the same -number for each call to the endpoint of this `params` block. To have the default evaluate -lazily with each request use a lambda, like `:random_number` above. ```ruby params do @@ -1135,7 +1136,7 @@ params do end ``` -Note that *both* range endpoints have to be a `#kind_of?` your `:type` option (if you don't supplied the `:type` option, it will be guessed to be equal to the class of the range's first endpoint). So the following is invalid: +Note that *both* range endpoints have to be a `#kind_of?` your `:type` option (if you don't supply the `:type` option, it will be guessed to be equal to the class of the range's first endpoint). So the following is invalid: ```ruby params do @@ -1145,6 +1146,9 @@ end ``` The `:values` option can also be supplied with a `Proc`, evaluated lazily with each request. +If the Proc has arity zero (i.e. it takes no arguments) it is expected to return either a list +or a range which will then be used to validate the parameter. + For example, given a status model you may want to restrict by hashtags that you have previously defined in the `HashTag` model. @@ -1154,40 +1158,34 @@ params do end ``` -The values validator can also validate that the value is explicitly not within a specific -set of values by passing ```except```. ```except``` accepts the same types of parameters as -values (Procs, ranges, etc.). +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. ```ruby params do - requires :browsers, values: { except: [ 'ie6', 'ie7', 'ie8' ] } + requires :number, type: Integer, values: ->(v) { v.even? && v < 25 } end ``` -Values and except can be combined to define a range of accepted values while not allowing -certain values within the set. Custom error messages can be defined for both when the parameter -passed falls within the ```except``` list or when it falls entirely outside the ```value``` list. +While Procs are convenient for single cases, consider using [Custom Validators](#custom-validators) in cases where a validation is used more than once. -```ruby -params do - requires :number, type: Integer, values: { value: 1..20, except: [4, 13], except_message: 'includes unsafe numbers', message: 'is outside the range of numbers allowed' } -end -``` +#### `except_values` -Finally, for even greater control, an explicit validation Proc may be supplied using ```proc```. -It will be called with a single argument (the input value), and should return -a truthy value if the value passes validation. If the input is an array, the Proc will be called -multiple times, once for each element in the array. +Parameters can be restricted from having a specific set of values with the `:except_values` option. + +The `except_values` validator behaves similarly to the `values` validator in that it accepts either +an Array, a Range, or a Proc. Unlike the `values` validator, however, `except_values` only accepts +Procs with arity zero. ```ruby params do - requires :number, type: Integer, values: { proc: ->(v) { v.even? && v < 25 }, message: 'is odd or greater than 25' } + requires :browser, except_values: [ 'ie6', 'ie7', 'ie8' ] + requires :port, except_values: { value: 0..1024, message: 'is not allowed' } + requires :hashtag, except_values: -> { Hashtag.FORBIDDEN_LIST } end ``` -While ```proc``` is convenient for single cases, consider using [Custom Validators](#custom-validators) in cases where a validation is used more than once. - - #### `regexp` Parameters can be restricted to match a specific regular expression with the `:regexp` option. If the value diff --git a/UPGRADING.md b/UPGRADING.md index 371fbc9e71..c26b9588d3 100644 --- a/UPGRADING.md +++ b/UPGRADING.md @@ -52,6 +52,34 @@ end See [#1610](https://github.com/ruby-grape/grape/pull/1610) for more information. +#### The `except`, `except_message`, and `proc` options of the `values` validator are deprecated. + +The new `except_values` validator should be used in place of the `except` and `except_message` options of +the `values` validator. + +Arity one Procs may now be used directly as the `values` option to explicitly test param values. + +**Deprecated** +```ruby +params do + requires :a, values: { value: 0..99, except: [3] } + requires :b, values: { value: 0..99, except: [3], except_message: 'not allowed' } + requires :c, values: { except: ['admin'] } + requires :d, values: { proc: -> (v) { v.even? } } +end +``` +**New** +```ruby +params do + requires :a, values: 0..99, except_values: [3] + requires :b, values: 0..99, except_values: { value: [3], message: 'not allowed' } + requires :c, except_values: ['admin'] + requires :d, values: -> (v) { v.even? } +end +``` + +See [#1616](https://github.com/ruby-grape/grape/pull/1616) for more information. + ### Upgrading to >= 0.19.1 #### DELETE now defaults to status code 200 for responses with a body, or 204 otherwise diff --git a/lib/grape.rb b/lib/grape.rb index 932ef5107c..27a2e15280 100644 --- a/lib/grape.rb +++ b/lib/grape.rb @@ -206,6 +206,7 @@ module ServeFile require 'grape/validations/validators/presence' require 'grape/validations/validators/regexp' require 'grape/validations/validators/values' +require 'grape/validations/validators/except_values' require 'grape/validations/params_scope' require 'grape/validations/validators/all_or_none' require 'grape/validations/types' diff --git a/lib/grape/locale/en.yml b/lib/grape/locale/en.yml index 986273f009..b78de1ed41 100644 --- a/lib/grape/locale/en.yml +++ b/lib/grape/locale/en.yml @@ -8,7 +8,7 @@ en: regexp: 'is invalid' blank: 'is empty' values: 'does not have a valid value' - except: 'has a value not allowed' + except_values: 'has a value not allowed' missing_vendor_option: problem: 'missing :vendor option.' summary: 'when version using header, you must specify :vendor option. ' diff --git a/lib/grape/validations/params_scope.rb b/lib/grape/validations/params_scope.rb index c3e5cf3731..c57ca5f142 100644 --- a/lib/grape/validations/params_scope.rb +++ b/lib/grape/validations/params_scope.rb @@ -234,26 +234,26 @@ def validates(attrs, validations) if (values_hash = validations[:values]).is_a? Hash values = values_hash[:value] + # NB: excepts is deprecated excepts = values_hash[:except] else values = validations[:values] end doc_attrs[:values] = values if values + except_values = options_key?(:except_values, :value, validations) ? validations[:except_values][:value] : validations[:except_values] + # NB. values and excepts should be nil, Proc, Array, or Range. # Specifically, values should NOT be a Hash # use values or excepts to guess coerce type when stated type is Array - coerce_type = guess_coerce_type(coerce_type, values) - coerce_type = guess_coerce_type(coerce_type, excepts) + coerce_type = guess_coerce_type(coerce_type, values, except_values, excepts) # default value should be present in values array, if both exist and are not procs - check_incompatible_option_values(values, default) + check_incompatible_option_values(default, values, except_values, excepts) # type should be compatible with values array, if both exist - validate_value_coercion(coerce_type, values) - # type should be compatible with excepts array, if both exist - validate_value_coercion(coerce_type, excepts) + validate_value_coercion(coerce_type, values, except_values, excepts) doc_attrs[:documentation] = validations.delete(:documentation) if validations.key?(:documentation) @@ -358,17 +358,31 @@ def coerce_type(validations, attrs, doc_attrs, opts) validations.delete(:coerce_message) end - def guess_coerce_type(coerce_type, values) - return coerce_type if !values || values.is_a?(Proc) - return values.first.class if coerce_type == Array && (values.is_a?(Range) || !values.empty?) + def guess_coerce_type(coerce_type, *values_list) + return coerce_type unless coerce_type == Array + values_list.each do |values| + next if !values || values.is_a?(Proc) + return values.first.class if values.is_a?(Range) || !values.empty? + end coerce_type end - def check_incompatible_option_values(values, default) - return unless values && default - return if values.is_a?(Proc) || default.is_a?(Proc) - return if values.include?(default) || (Array(default) - Array(values)).empty? - raise Grape::Exceptions::IncompatibleOptionValues.new(:default, default, :values, values) + def check_incompatible_option_values(default, values, except_values, excepts) + return unless default && !default.is_a?(Proc) + + if values && !values.is_a?(Proc) + raise Grape::Exceptions::IncompatibleOptionValues.new(:default, default, :values, values) \ + unless Array(default).all? { |def_val| values.include?(def_val) } + end + + if except_values && !except_values.is_a?(Proc) + raise Grape::Exceptions::IncompatibleOptionValues.new(:default, default, :except, except_values) \ + unless Array(default).none? { |def_val| except_values.include?(def_val) } + end + + return unless excepts && !excepts.is_a?(Proc) + raise Grape::Exceptions::IncompatibleOptionValues.new(:default, default, :except, excepts) \ + unless Array(default).none? { |def_val| excepts.include?(def_val) } end def validate(type, options, attrs, doc_attrs, opts) @@ -380,16 +394,19 @@ def validate(type, options, attrs, doc_attrs, opts) @api.namespace_stackable(:validations, value) end - def validate_value_coercion(coerce_type, values) - return unless coerce_type && values - return if values.is_a?(Proc) + def validate_value_coercion(coerce_type, *values_list) + return unless coerce_type coerce_type = coerce_type.first if coerce_type.is_a?(Array) - value_types = values.is_a?(Range) ? [values.begin, values.end] : values - if coerce_type == Virtus::Attribute::Boolean - value_types = value_types.map { |type| Virtus::Attribute.build(type) } + values_list.each do |values| + next if !values || values.is_a?(Proc) + value_types = values.is_a?(Range) ? [values.begin, values.end] : values + if coerce_type == Virtus::Attribute::Boolean + value_types = value_types.map { |type| Virtus::Attribute.build(type) } + end + unless value_types.all? { |v| v.is_a? coerce_type } + raise Grape::Exceptions::IncompatibleOptionValues.new(:type, coerce_type, :values, values) + end end - return unless value_types.any? { |v| !v.is_a?(coerce_type) } - raise Grape::Exceptions::IncompatibleOptionValues.new(:type, coerce_type, :values, values) end def extract_message_option(attrs) diff --git a/lib/grape/validations/validators/except_values.rb b/lib/grape/validations/validators/except_values.rb new file mode 100644 index 0000000000..a98fc193d3 --- /dev/null +++ b/lib/grape/validations/validators/except_values.rb @@ -0,0 +1,20 @@ +module Grape + module Validations + class ExceptValuesValidator < Base + def initialize(attrs, options, required, scope, opts = {}) + @except = options.is_a?(Hash) ? options[:value] : options + super + end + + def validate_param!(attr_name, params) + return unless params.respond_to?(:key?) && params.key?(attr_name) + + excepts = @except.is_a?(Proc) ? @except.call : @except + return if excepts.nil? + + param_array = params[attr_name].nil? ? [nil] : Array.wrap(params[attr_name]) + raise Grape::Exceptions::Validation, params: [@scope.full_name(attr_name)], message: message(:except_values) if param_array.any? { |param| excepts.include?(param) } + end + end + end +end diff --git a/lib/grape/validations/validators/values.rb b/lib/grape/validations/validators/values.rb index 1926bc9020..2204692252 100644 --- a/lib/grape/validations/validators/values.rb +++ b/lib/grape/validations/validators/values.rb @@ -6,7 +6,13 @@ def initialize(attrs, options, required, scope, opts = {}) @excepts = options[:except] @values = options[:value] @proc = options[:proc] + + warn '[DEPRECATION] The values validator except option is deprecated. ' \ + 'Use the except validator instead.' if @excepts + raise ArgumentError, 'proc must be a Proc' if @proc && !@proc.is_a?(Proc) + warn '[DEPRECATION] The values validator proc option is deprecated. ' \ + 'The lambda expression can now be assigned directly to values.' if @proc else @values = options end @@ -17,15 +23,13 @@ def validate_param!(attr_name, params) return unless params.is_a?(Hash) return unless params[attr_name] || required_for_root_scope? - values = @values.is_a?(Proc) ? @values.call : @values - excepts = @excepts.is_a?(Proc) ? @excepts.call : @excepts param_array = params[attr_name].nil? ? [nil] : Array.wrap(params[attr_name]) raise Grape::Exceptions::Validation, params: [@scope.full_name(attr_name)], message: except_message \ - if !excepts.nil? && param_array.any? { |param| excepts.include?(param) } + unless check_excepts(param_array) raise Grape::Exceptions::Validation, params: [@scope.full_name(attr_name)], message: message(:values) \ - if !values.nil? && !param_array.all? { |param| values.include?(param) } + unless check_values(param_array) raise Grape::Exceptions::Validation, params: [@scope.full_name(attr_name)], message: message(:values) \ if @proc && !param_array.all? { |param| @proc.call(param) } @@ -33,9 +37,22 @@ def validate_param!(attr_name, params) private + def check_values(param_array) + 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 + param_array.all? { |param| values.include?(param) } + end + + def check_excepts(param_array) + excepts = @excepts.is_a?(Proc) ? @excepts.call : @excepts + return true if excepts.nil? + param_array.none? { |param| excepts.include?(param) } + end + def except_message options = instance_variable_get(:@option) - options_key?(:except_message) ? options[:except_message] : message(:except) + options_key?(:except_message) ? options[:except_message] : message(:except_values) end def required_for_root_scope? diff --git a/spec/grape/validations/validators/except_values_spec.rb b/spec/grape/validations/validators/except_values_spec.rb new file mode 100644 index 0000000000..7030774cc4 --- /dev/null +++ b/spec/grape/validations/validators/except_values_spec.rb @@ -0,0 +1,191 @@ +require 'spec_helper' + +describe Grape::Validations::ExceptValuesValidator do + module ValidationsSpec + class ExceptValuesModel + DEFAULT_EXCEPTS = ['invalid-type1', 'invalid-type2', 'invalid-type3'].freeze + class << self + attr_accessor :excepts + def excepts + @excepts ||= [] + [DEFAULT_EXCEPTS + @excepts].flatten.uniq + end + end + end + + TEST_CASES = { + req_except: { + requires: { except_values: ExceptValuesModel.excepts }, + tests: [ + { value: 'invalid-type1', rc: 400, body: { error: 'type has a value not allowed' }.to_json }, + { value: 'invalid-type3', rc: 400, body: { error: 'type has a value not allowed' }.to_json }, + { value: 'valid-type', rc: 200, body: { type: 'valid-type' }.to_json } + ] + }, + req_except_hash: { + requires: { except_values: { value: ExceptValuesModel.excepts } }, + tests: [ + { value: 'invalid-type1', rc: 400, body: { error: 'type has a value not allowed' }.to_json }, + { value: 'invalid-type3', rc: 400, body: { error: 'type has a value not allowed' }.to_json }, + { value: 'valid-type', rc: 200, body: { type: 'valid-type' }.to_json } + ] + }, + req_except_custom_message: { + requires: { except_values: { value: ExceptValuesModel.excepts, message: 'is not allowed' } }, + tests: [ + { value: 'invalid-type1', rc: 400, body: { error: 'type is not allowed' }.to_json }, + { value: 'invalid-type3', rc: 400, body: { error: 'type is not allowed' }.to_json }, + { value: 'valid-type', rc: 200, body: { type: 'valid-type' }.to_json } + ] + }, + req_except_no_value: { + requires: { except_values: { message: 'is not allowed' } }, + tests: [ + { value: 'invalid-type1', rc: 200, body: { type: 'invalid-type1' }.to_json } + ] + }, + req_except_empty: { + requires: { except_values: [] }, + tests: [ + { value: 'invalid-type1', rc: 200, body: { type: 'invalid-type1' }.to_json } + ] + }, + req_except_lambda: { + requires: { except_values: -> { ExceptValuesModel.excepts } }, + add_excepts: ['invalid-type4'], + tests: [ + { value: 'invalid-type1', rc: 400, body: { error: 'type has a value not allowed' }.to_json }, + { value: 'invalid-type4', rc: 400, body: { error: 'type has a value not allowed' }.to_json }, + { value: 'valid-type', rc: 200, body: { type: 'valid-type' }.to_json } + ] + }, + req_except_lambda_custom_message: { + requires: { except_values: { value: -> { ExceptValuesModel.excepts }, message: 'is not allowed' } }, + add_excepts: ['invalid-type4'], + tests: [ + { value: 'invalid-type1', rc: 400, body: { error: 'type is not allowed' }.to_json }, + { value: 'invalid-type4', rc: 400, body: { error: 'type is not allowed' }.to_json }, + { value: 'valid-type', rc: 200, body: { type: 'valid-type' }.to_json } + ] + }, + opt_except_default: { + optional: { except_values: ExceptValuesModel.excepts, default: 'valid-type2' }, + tests: [ + { value: 'invalid-type1', rc: 400, body: { error: 'type has a value not allowed' }.to_json }, + { value: 'invalid-type3', rc: 400, body: { error: 'type has a value not allowed' }.to_json }, + { value: 'valid-type', rc: 200, body: { type: 'valid-type' }.to_json }, + { rc: 200, body: { type: 'valid-type2' }.to_json } + ] + }, + opt_except_lambda_default: { + optional: { except_values: -> { ExceptValuesModel.excepts }, default: 'valid-type2' }, + tests: [ + { value: 'invalid-type1', rc: 400, body: { error: 'type has a value not allowed' }.to_json }, + { value: 'invalid-type3', rc: 400, body: { error: 'type has a value not allowed' }.to_json }, + { value: 'valid-type', rc: 200, body: { type: 'valid-type' }.to_json }, + { rc: 200, body: { type: 'valid-type2' }.to_json } + ] + }, + req_except_type_coerce: { + requires: { type: Integer, except_values: [10, 11] }, + tests: [ + { value: 'invalid-type1', rc: 400, body: { error: 'type is invalid' }.to_json }, + { value: 11, rc: 400, body: { error: 'type has a value not allowed' }.to_json }, + { value: '11', rc: 400, body: { error: 'type has a value not allowed' }.to_json }, + { value: '3', rc: 200, body: { type: 3 }.to_json }, + { value: 3, rc: 200, body: { type: 3 }.to_json } + ] + }, + opt_except_type_coerce_default: { + optional: { type: Integer, except_values: [10, 11], default: 12 }, + tests: [ + { value: 'invalid-type1', rc: 400, body: { error: 'type is invalid' }.to_json }, + { value: 10, rc: 400, body: { error: 'type has a value not allowed' }.to_json }, + { value: '3', rc: 200, body: { type: 3 }.to_json }, + { value: 3, rc: 200, body: { type: 3 }.to_json }, + { rc: 200, body: { type: 12 }.to_json } + ] + }, + opt_except_array_type_coerce_default: { + optional: { type: Array[Integer], except_values: [10, 11], default: 12 }, + tests: [ + { value: 'invalid-type1', rc: 400, body: { error: 'type is invalid' }.to_json }, + { value: 10, rc: 400, body: { error: 'type has a value not allowed' }.to_json }, + { value: [10], rc: 400, body: { error: 'type has a value not allowed' }.to_json }, + { value: ['3'], rc: 200, body: { type: [3] }.to_json }, + { value: [3], rc: 200, body: { type: [3] }.to_json }, + { rc: 200, body: { type: 12 }.to_json } + ] + }, + req_except_range: { + optional: { type: Integer, except_values: 10..12 }, + tests: [ + { value: 11, rc: 400, body: { error: 'type has a value not allowed' }.to_json }, + { value: 13, rc: 200, body: { type: 13 }.to_json } + ] + } + }.freeze + + module ExceptValidatorSpec + class API < Grape::API + default_format :json + + TEST_CASES.each_with_index do |(k, v), _i| + params do + requires :type, v[:requires] if v.key? :requires + optional :type, v[:optional] if v.key? :optional + end + get k do + { type: params[:type] } + end + end + end + end + end + + it 'raises IncompatibleOptionValues on a default value in exclude' do + subject = Class.new(Grape::API) + expect do + subject.params do + optional :type, except_values: ValidationsSpec::ExceptValuesModel.excepts, + default: ValidationsSpec::ExceptValuesModel.excepts.sample + end + end.to raise_error Grape::Exceptions::IncompatibleOptionValues + end + + it 'raises IncompatibleOptionValues when a default array has excluded values' do + subject = Class.new(Grape::API) + expect do + subject.params do + optional :type, type: Array[Integer], + except_values: 10..12, + default: [8, 9, 10] + end + end.to raise_error Grape::Exceptions::IncompatibleOptionValues + end + + it 'raises IncompatibleOptionValues when type is incompatible with values array' do + subject = Class.new(Grape::API) + expect do + subject.params { optional :type, except_values: ['valid-type1', 'valid-type2', 'valid-type3'], type: Symbol } + end.to raise_error Grape::Exceptions::IncompatibleOptionValues + end + + def app + ValidationsSpec::ExceptValidatorSpec::API + end + + ValidationsSpec::TEST_CASES.each_with_index do |(k, v), i| + v[:tests].each do |t| + it "#{i}: #{k} - #{t[:value]}" do + ValidationsSpec::ExceptValuesModel.excepts = v[:add_excepts] if v.key? :add_excepts + body = {} + body[:type] = t[:value] if t.key? :value + get k.to_s, **body + expect(last_response.status).to eq t[:rc] + expect(last_response.body).to eq t[:body] + ValidationsSpec::ExceptValuesModel.excepts = nil + end + end + end +end diff --git a/spec/grape/validations/validators/values_spec.rb b/spec/grape/validations/validators/values_spec.rb index 76c896b3e3..0d12381f51 100644 --- a/spec/grape/validations/validators/values_spec.rb +++ b/spec/grape/validations/validators/values_spec.rb @@ -103,6 +103,13 @@ class API < Grape::API { type: params[:type] } end + params do + requires :type, values: ->(v) { ValuesModel.values.include? v } + end + get '/lambda_val' do + { type: params[:type] } + end + params do requires :type, values: -> { [] } end @@ -350,6 +357,18 @@ def app expect(last_response.body).to eq({ error: 'type does not have a valid value' }.to_json) end + it 'allows value using lambda' do + get('/lambda_val', type: 'valid-type1') + expect(last_response.status).to eq 200 + expect(last_response.body).to eq({ type: 'valid-type1' }.to_json) + end + + it 'does not allow invalid value using lambda' do + get('/lambda_val', type: 'invalid-type') + expect(last_response.status).to eq 400 + expect(last_response.body).to eq({ error: 'type does not have a valid value' }.to_json) + end + it 'validates against an empty array in a proc' do get('/empty_lambda', type: 'any') expect(last_response.status).to eq 400