From 5e997b996eb08c562a8cbb3017767defc2888ac9 Mon Sep 17 00:00:00 2001 From: Jonathan Chan Date: Thu, 8 Sep 2016 09:57:24 -0400 Subject: [PATCH 1/3] implemented except in values validator --- README.md | 20 +++++ lib/grape/locale/en.yml | 1 + lib/grape/validations/validators/values.rb | 18 +++- .../validations/validators/values_spec.rb | 89 +++++++++++++++++++ 4 files changed, 126 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index 26e9f3a687..ba66a692b5 100644 --- a/README.md +++ b/README.md @@ -1065,6 +1065,26 @@ 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.). + +```ruby +params do + requires :browsers, values: { except: [ 'ie6', 'ie7', 'ie8' ] } +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. + +```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 +``` + #### `regexp` Parameters can be restricted to match a specific regular expression with the `:regexp` option. If the value diff --git a/lib/grape/locale/en.yml b/lib/grape/locale/en.yml index 0c4cb0d9bd..96a011b0a9 100644 --- a/lib/grape/locale/en.yml +++ b/lib/grape/locale/en.yml @@ -8,6 +8,7 @@ en: regexp: 'is invalid' blank: 'is empty' values: 'does not have a valid value' + except: '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/validators/values.rb b/lib/grape/validations/validators/values.rb index 27c186ff51..a450d494d5 100644 --- a/lib/grape/validations/validators/values.rb +++ b/lib/grape/validations/validators/values.rb @@ -2,7 +2,10 @@ module Grape module Validations class ValuesValidator < Base def initialize(attrs, options, required, scope) - @values = (options_key?(:value, options) ? options[:value] : options) + @excepts = (options_key?(:except, options) ? options[:except] : []) + @values = (options_key?(:value, options) ? options[:value] : []) + + @values = options if @excepts == [] && @values == [] super end @@ -11,13 +14,24 @@ def validate_param!(attr_name, params) 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]) - return if param_array.all? { |param| values.include?(param) } + + if param_array.all? { |param| excepts.include?(param) } + raise Grape::Exceptions::Validation, params: [@scope.full_name(attr_name)], message: except_message + end + + return if (values.is_a?(Array) && values.empty?) || param_array.all? { |param| values.include?(param) } raise Grape::Exceptions::Validation, params: [@scope.full_name(attr_name)], message: message(:values) end private + def except_message + options = instance_variable_get(:@option) + options_key?(:except_message) ? options[:except_message] : message(:except) + end + def required_for_root_scope? @required && @scope.root? end diff --git a/spec/grape/validations/validators/values_spec.rb b/spec/grape/validations/validators/values_spec.rb index 5fef155066..9faa8ec888 100644 --- a/spec/grape/validations/validators/values_spec.rb +++ b/spec/grape/validations/validators/values_spec.rb @@ -4,6 +4,7 @@ module ValidationsSpec class ValuesModel DEFAULT_VALUES = ['valid-type1', 'valid-type2', 'valid-type3'].freeze + DEFAULT_EXCEPTS = ['invalid-type1', 'invalid-type2', 'invalid-type3'].freeze class << self def values @values ||= [] @@ -14,6 +15,16 @@ def add_value(value) @values ||= [] @values << value end + + def excepts + @excepts ||= [] + [DEFAULT_EXCEPTS + @excepts].flatten.uniq + end + + def add_except(except) + @excepts ||= [] + @excepts << except + end end end @@ -35,6 +46,20 @@ class API < Grape::API get '/lambda' do { type: params[:type] } end + + params do + requires :type, values: { except: ValuesModel.excepts, except_message: 'value is on exclusions list', message: 'default exclude message' } + end + get '/exclude/exclude_message' do + { type: params[:type] } + end + + params do + requires :type, values: { except: ValuesModel.excepts, message: 'default exclude message' } + end + get '/exclude/fallback_message' do + { type: params[:type] } + end end params do @@ -99,6 +124,20 @@ class API < Grape::API end end get '/optional_with_required_values' + + params do + requires :type, values: { except: ValuesModel.excepts } + end + get '/except/exclusive' do + { type: params[:type] } + end + + params do + requires :type, type: Integer, values: { value: 1..5, except: [3] } + end + get '/mixed/value/except' do + { type: params[:type] } + end end end end @@ -135,6 +174,22 @@ def app end end + context 'with a custom exclude validation message' do + it 'does not allow an invalid value for a parameter' do + get('/custom_message/exclude/exclude_message', type: 'invalid-type1') + expect(last_response.status).to eq 400 + expect(last_response.body).to eq({ error: 'type value is on exclusions list' }.to_json) + end + end + + context 'exclude with a standard custom validation message' do + it 'does not allow an invalid value for a parameter' do + get('/custom_message/exclude/fallback_message', type: 'invalid-type1') + expect(last_response.status).to eq 400 + expect(last_response.body).to eq({ error: 'type default exclude message' }.to_json) + end + end + it 'allows a valid value for a parameter' do get('/', type: 'valid-type1') expect(last_response.status).to eq 200 @@ -321,4 +376,38 @@ def app expect(last_response.body).to eq('values does not have a valid value') end end + + context 'exclusive excepts' do + it 'allows any other value outside excepts' do + get '/except/exclusive', type: 'value' + expect(last_response.status).to eq 200 + expect(last_response.body).to eq({ type: 'value' }.to_json) + end + + it 'rejects values that matches except' do + get '/except/exclusive', type: 'invalid-type1' + expect(last_response.status).to eq 400 + expect(last_response.body).to eq({ error: 'type has a value not allowed' }.to_json) + end + end + + context 'with mixed values and excepts' do + it 'allows value, but not in except' do + get '/mixed/value/except', type: 2 + expect(last_response.status).to eq 200 + expect(last_response.body).to eq({ type: 2 }.to_json) + end + + it 'rejects except' do + get '/mixed/value/except', type: 3 + expect(last_response.status).to eq 400 + expect(last_response.body).to eq({ error: 'type has a value not allowed' }.to_json) + end + + it 'rejects outside except and outside value' do + get '/mixed/value/except', type: 10 + expect(last_response.status).to eq 400 + expect(last_response.body).to eq({ error: 'type does not have a valid value' }.to_json) + end + end end From 177932b7317efe729d33ee3fc45238a4bd0ae05b Mon Sep 17 00:00:00 2001 From: Jonathan Chan Date: Fri, 9 Sep 2016 08:28:50 -0400 Subject: [PATCH 2/3] updated CHANGELOG.md --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1d555c4c66..4531f56da3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,7 @@ ================== * [#1480](https://github.com/ruby-grape/grape/pull/1480): Use the ruby-grape-danger gem for PR linting - [@dblock](https://github.com/dblock). +* [#1486](https://github.com/ruby-grape/grape/pull/1486): implemented except in values validator - [@jonmchan](https://github.com/jonmchan). * Your contribution here. #### Fixes From 128d67d1f03605ace1e693d7adedaa65aa36b7ae Mon Sep 17 00:00:00 2001 From: Jonathan Chan Date: Fri, 9 Sep 2016 08:54:29 -0400 Subject: [PATCH 3/3] trying to make danger errors go away --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4531f56da3..ce9a6a590e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,7 +2,7 @@ ================== * [#1480](https://github.com/ruby-grape/grape/pull/1480): Use the ruby-grape-danger gem for PR linting - [@dblock](https://github.com/dblock). -* [#1486](https://github.com/ruby-grape/grape/pull/1486): implemented except in values validator - [@jonmchan](https://github.com/jonmchan). +* [#1486](https://github.com/ruby-grape/grape/pull/1486): Implemented except in values validator - [@jonmchan](https://github.com/jonmchan). * Your contribution here. #### Fixes