diff --git a/CHANGELOG.md b/CHANGELOG.md index de61b49111..661cc4c444 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,7 @@ Next Release * [#559](https://github.com/intridea/grape/issues/559): Support Rack 1.6.0 to parse requests larger than 128KB - [@myitcv](https://github.com/myitcv). * [#876](https://github.com/intridea/grape/pull/876): declared(params) now returning Hashie::Mash - [@rodzyn](https://github.com/rodzyn). * [#879](https://github.com/intridea/grape/pull/879): route_info is not included in params Hash anymore - [@rodzyn](https://github.com/rodzyn). +* [#881](https://github.com/intridea/grape/issues/881): fix bug so that values validator could accept ranges again - [@ajvondrak](https://github.com/ajvondrak). * Your contribution here. 0.10.1 (12/28/2014) diff --git a/README.md b/README.md index 4bc9dd3119..941c4c447f 100644 --- a/README.md +++ b/README.md @@ -780,6 +780,25 @@ params do end ``` +Supplying a range to the `:values` option ensures that the parameter is (or parameters are) included in that range (using `Range#include?`). + +```ruby +params do + requires :latitude, type: Float, values: -90.0..+90.0 + requires :longitude, type: Float, values: -180.0..+180.0 + optional :letters, type: Array[String], values: 'a'..'z' +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: + +```ruby +params do + requires :invalid1, type: Float, values: 0..10 # 0.kind_of?(Float) => false + optional :invalid2, values: 0..10.0 # 10.0.kind_of?(0.class) => false +end +``` + The `:values` option can also be supplied with a `Proc`, evaluated lazily with each request. For example, given a status model you may want to restrict by hashtags that you have previously defined in the `HashTag` model. diff --git a/lib/grape/validations/params_scope.rb b/lib/grape/validations/params_scope.rb index cdabd6e492..49565f50bc 100644 --- a/lib/grape/validations/params_scope.rb +++ b/lib/grape/validations/params_scope.rb @@ -141,7 +141,7 @@ def validates(attrs, validations) 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.empty? + return values.first.class if coerce_type == Array && (values.is_a?(Range) || !values.empty?) coerce_type end @@ -167,7 +167,8 @@ def validate_value_coercion(coerce_type, values) return unless coerce_type && values return if values.is_a?(Proc) coerce_type = coerce_type.first if coerce_type.kind_of?(Array) - if values.any? { |v| !v.kind_of?(coerce_type) } + value_types = values.is_a?(Range) ? [values.begin, values.end] : values + if value_types.any? { |v| !v.kind_of?(coerce_type) } fail Grape::Exceptions::IncompatibleOptionValues.new(:type, coerce_type, :values, values) end end diff --git a/lib/grape/validations/validators/values.rb b/lib/grape/validations/validators/values.rb index 72209ab0a6..6c22b2b83a 100644 --- a/lib/grape/validations/validators/values.rb +++ b/lib/grape/validations/validators/values.rb @@ -11,7 +11,7 @@ def validate_param!(attr_name, params) values = @values.is_a?(Proc) ? @values.call : @values param_array = params[attr_name].nil? ? [nil] : Array.wrap(params[attr_name]) - unless (param_array - values).empty? + unless param_array.all? { |param| values.include?(param) } fail Grape::Exceptions::Validation, params: [@scope.full_name(attr_name)], message_key: :values end end diff --git a/spec/grape/validations/params_scope_spec.rb b/spec/grape/validations/params_scope_spec.rb index fdf1b0cad3..a8d465b01a 100644 --- a/spec/grape/validations/params_scope_spec.rb +++ b/spec/grape/validations/params_scope_spec.rb @@ -53,5 +53,53 @@ def app subject.params { requires :numbers, type: Array, values: [1, 'definitely not a number', 3] } end.to raise_error Grape::Exceptions::IncompatibleOptionValues end + + it 'raises exception when range values have different endpoint types' do + expect do + subject.params { requires :numbers, type: Array, values: 0.0..10 } + end.to raise_error Grape::Exceptions::IncompatibleOptionValues + end + end + + context 'with range values' do + context "when left range endpoint isn't #kind_of? the type" do + it 'raises exception' do + expect do + subject.params { requires :latitude, type: Integer, values: -90.0..90 } + end.to raise_error Grape::Exceptions::IncompatibleOptionValues + end + end + + context "when right range endpoint isn't #kind_of? the type" do + it 'raises exception' do + expect do + subject.params { requires :latitude, type: Integer, values: -90..90.0 } + end.to raise_error Grape::Exceptions::IncompatibleOptionValues + end + end + + context 'when both range endpoints are #kind_of? the type' do + it 'accepts values in the range' do + subject.params do + requires :letter, type: String, values: 'a'..'z' + end + subject.get('/letter') { params[:letter] } + + get '/letter', letter: 'j' + expect(last_response.status).to eq(200) + expect(last_response.body).to eq('j') + end + + it 'rejects values outside the range' do + subject.params do + requires :letter, type: String, values: 'a'..'z' + end + subject.get('/letter') { params[:letter] } + + get '/letter', letter: 'J' + expect(last_response.status).to eq(400) + expect(last_response.body).to eq('letter does not have a valid value') + end + end end end diff --git a/spec/grape/validations/validators/values_spec.rb b/spec/grape/validations/validators/values_spec.rb index ed052c839a..cd4f05c2f8 100644 --- a/spec/grape/validations/validators/values_spec.rb +++ b/spec/grape/validations/validators/values_spec.rb @@ -221,4 +221,48 @@ def app expect(last_response.status).to eq 200 end end + + context 'with a range of values' do + subject(:app) do + Class.new(Grape::API) do + params do + optional :value, type: Float, values: 0.0..10.0 + end + get '/value' do + { value: params[:value] }.to_json + end + + params do + optional :values, type: Array[Float], values: 0.0..10.0 + end + get '/values' do + { values: params[:values] }.to_json + end + end + end + + it 'allows a single value inside of the range' do + get('/value', value: 5.2) + expect(last_response.status).to eq 200 + expect(last_response.body).to eq({ value: 5.2 }.to_json) + end + + it 'allows an array of values inside of the range' do + get('/values', values: [8.6, 7.5, 3, 0.9]) + expect(last_response.status).to eq 200 + expect(last_response.body).to eq({ values: [8.6, 7.5, 3.0, 0.9] }.to_json) + end + + it 'rejects a single value outside the range' do + get('/value', value: 'a') + expect(last_response.status).to eq 400 + expect(last_response.body).to eq('value is invalid, value does not have a valid value') + end + + it 'rejects an array of values if any of them are outside the range' do + get('/values', values: [8.6, 75, 3, 0.9]) + expect(last_response.status).to eq 400 + expect(last_response.body).to eq('values does not have a valid value') + end + end end