Merge pull request #899 from ajvondrak/881

Fix for #881

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)

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.

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

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

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

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