From 7fe9a065dc4077b7cb1a2790b8f49d40cfad51a0 Mon Sep 17 00:00:00 2001 From: Alexander Koltun Date: Thu, 6 Oct 2016 16:44:06 +0200 Subject: [PATCH 1/3] Allow to use regexp validator with arrays --- CHANGELOG.md | 1 + lib/grape/validations/validators/regexp.rb | 2 +- .../validations/validators/regexp_spec.rb | 72 +++++++++++++++++++ 3 files changed, 74 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6ac5d7c493..db52a5c905 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,7 @@ Next Release ============ * Your contribution here. +* [#1503](/~https://github.com/ruby-grape/grape/pull/1503): Allow to use regexp validator with arrays - [@akoltun](/~https://github.com/akoltun). 0.18.0 (10/7/2016) ================== diff --git a/lib/grape/validations/validators/regexp.rb b/lib/grape/validations/validators/regexp.rb index 80fe4c12df..9c20043b70 100644 --- a/lib/grape/validations/validators/regexp.rb +++ b/lib/grape/validations/validators/regexp.rb @@ -2,7 +2,7 @@ module Grape module Validations class RegexpValidator < Base def validate_param!(attr_name, params) - return unless params.key?(attr_name) && !params[attr_name].nil? && !(params[attr_name].to_s =~ (options_key?(:value) ? @option[:value] : @option)) + return if Array.wrap(params[attr_name]).all? { |param| param.nil? || (param.to_s =~ (options_key?(:value) ? @option[:value] : @option)) } raise Grape::Exceptions::Validation, params: [@scope.full_name(attr_name)], message: message(:regexp) end end diff --git a/spec/grape/validations/validators/regexp_spec.rb b/spec/grape/validations/validators/regexp_spec.rb index 659bc61d29..0295c95978 100644 --- a/spec/grape/validations/validators/regexp_spec.rb +++ b/spec/grape/validations/validators/regexp_spec.rb @@ -12,6 +12,12 @@ class API < Grape::API end get do end + + params do + requires :names, type: { value: Array[String], message: 'can\'t be nil' }, regexp: { value: /^[a-z]+$/, message: 'format is invalid' } + end + get 'regexp_with_array' do + end end params do @@ -19,6 +25,12 @@ class API < Grape::API end get do end + + params do + requires :names, type: Array[String], regexp: /^[a-z]+$/ + end + get 'regexp_with_array' do + end end end end @@ -51,6 +63,36 @@ def app get '/custom_message', name: 'bob' expect(last_response.status).to eq(200) end + + context 'regexp with array' do + it 'refuses inapppopriate items' do + get '/custom_message/regexp_with_array', names: ['invalid name', 'abc'] + expect(last_response.status).to eq(400) + expect(last_response.body).to eq('{"error":"names format is invalid"}') + end + + it 'refuses empty items' do + get '/custom_message/regexp_with_array', names: ['', 'abc'] + expect(last_response.status).to eq(400) + expect(last_response.body).to eq('{"error":"names format is invalid"}') + end + + it 'refuses nil items' do + get '/custom_message/regexp_with_array', names: [nil, 'abc'] + expect(last_response.status).to eq(400) + expect(last_response.body).to eq('{"error":"names can\'t be nil"}') + end + + it 'accepts valid items' do + get '/custom_message/regexp_with_array', names: ['bob'] + expect(last_response.status).to eq(200) + end + + it 'accepts nil instead of array' do + get '/custom_message/regexp_with_array', names: nil + expect(last_response.status).to eq(200) + end + end end context 'invalid input' do @@ -76,4 +118,34 @@ def app get '/', name: 'bob' expect(last_response.status).to eq(200) end + + context 'regexp with array' do + it 'refuses inapppopriate items' do + get '/regexp_with_array', names: ['invalid name', 'abc'] + expect(last_response.status).to eq(400) + expect(last_response.body).to eq('{"error":"names is invalid"}') + end + + it 'refuses empty items' do + get '/regexp_with_array', names: ['', 'abc'] + expect(last_response.status).to eq(400) + expect(last_response.body).to eq('{"error":"names is invalid"}') + end + + it 'refuses nil items' do + get '/regexp_with_array', names: [nil, 'abc'] + expect(last_response.status).to eq(400) + expect(last_response.body).to eq('{"error":"names is invalid"}') + end + + it 'accepts valid items' do + get '/regexp_with_array', names: ['bob'] + expect(last_response.status).to eq(200) + end + + it 'accepts nil instead of array' do + get '/regexp_with_array', names: nil + expect(last_response.status).to eq(200) + end + end end From 8305dcfaf7ad6dcaa3e26338c71dd3fce25ea22a Mon Sep 17 00:00:00 2001 From: Alexander Koltun Date: Thu, 6 Oct 2016 18:04:30 +0200 Subject: [PATCH 2/3] Fix bug when Array is a nested attribute and the parent attribute is of incorrect type --- lib/grape/validations/validators/regexp.rb | 1 + spec/grape/validations/validators/regexp_spec.rb | 16 ++++++++++++++++ 2 files changed, 17 insertions(+) diff --git a/lib/grape/validations/validators/regexp.rb b/lib/grape/validations/validators/regexp.rb index 9c20043b70..1a171b308a 100644 --- a/lib/grape/validations/validators/regexp.rb +++ b/lib/grape/validations/validators/regexp.rb @@ -2,6 +2,7 @@ module Grape module Validations class RegexpValidator < Base def validate_param!(attr_name, params) + return unless params.respond_to?(:has_key?) && params.has_key?(attr_name) return if Array.wrap(params[attr_name]).all? { |param| param.nil? || (param.to_s =~ (options_key?(:value) ? @option[:value] : @option)) } raise Grape::Exceptions::Validation, params: [@scope.full_name(attr_name)], message: message(:regexp) end diff --git a/spec/grape/validations/validators/regexp_spec.rb b/spec/grape/validations/validators/regexp_spec.rb index 0295c95978..cc2970a887 100644 --- a/spec/grape/validations/validators/regexp_spec.rb +++ b/spec/grape/validations/validators/regexp_spec.rb @@ -31,6 +31,14 @@ class API < Grape::API end get 'regexp_with_array' do end + + params do + requires :people, type: Hash do + requires :names, type: Array[String], regexp: /^[a-z]+$/ + end + end + get 'nested_regexp_with_array' do + end end end end @@ -148,4 +156,12 @@ def app expect(last_response.status).to eq(200) end end + + context 'nested regexp with array' do + it 'refuses inapppopriate' do + get '/nested_regexp_with_array', people: 'invalid name' + expect(last_response.status).to eq(400) + expect(last_response.body).to eq('{"error":"people is invalid, people[names] is missing, people[names] is invalid"}') + end + end end From f54f9b456d1a85df2ad0881afec98f8efff148d3 Mon Sep 17 00:00:00 2001 From: Alexander Koltun Date: Fri, 7 Oct 2016 09:35:46 +0200 Subject: [PATCH 3/3] Fix: Change :has_key? to :key? according rubocop --- lib/grape/validations/validators/regexp.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/grape/validations/validators/regexp.rb b/lib/grape/validations/validators/regexp.rb index 1a171b308a..749a42ce2c 100644 --- a/lib/grape/validations/validators/regexp.rb +++ b/lib/grape/validations/validators/regexp.rb @@ -2,7 +2,7 @@ module Grape module Validations class RegexpValidator < Base def validate_param!(attr_name, params) - return unless params.respond_to?(:has_key?) && params.has_key?(attr_name) + return unless params.respond_to?(:key?) && params.key?(attr_name) return if Array.wrap(params[attr_name]).all? { |param| param.nil? || (param.to_s =~ (options_key?(:value) ? @option[:value] : @option)) } raise Grape::Exceptions::Validation, params: [@scope.full_name(attr_name)], message: message(:regexp) end