Merge pull request #1567 from jlfaber/fix_values_validator

Fix ValuesValidator and simplify code

Author: dblock

CHANGELOG.md

@@ -11,6 +11,7 @@
 * [#1562](https://github.com/ruby-grape/grape/pull/1562): Fix rainbow gem installation failure above ruby 2.3.3 on travis-ci - [@brucehsu](https://github.com/brucehsu).
 * [#1561](https://github.com/ruby-grape/grape/pull/1561): Fix performance issue introduced by duplicated calls in StackableValue#[] - [@brucehsu](https://github.com/brucehsu).
 * [#1564](https://github.com/ruby-grape/grape/pull/1564): Fix declared params bug with nested namespaces - [@bmarini](https://github.com/bmarini).
+* [#1567](https://github.com/ruby-grape/grape/pull/1567): Fix values validator when value is empty array and apply except to input array - [@jlfaber](https://github.com/jlfaber).
 * Your contribution here.
 
 ### 0.19.1 (1/9/2017)

lib/grape/validations/validators/values.rb

@@ -2,10 +2,12 @@ module Grape
   module Validations
     class ValuesValidator < Base
       def initialize(attrs, options, required, scope, opts = {})
-        @excepts = (options_key?(:except, options) ? options[:except] : [])
-        @values = (options_key?(:value, options) ? options[:value] : [])
-
-        @values = options if @excepts == [] && @values == []
+        if options.is_a?(Hash)
+          @excepts = options[:except]
+          @values = options[:value]
+        else
+          @values = options
+        end
         super
       end
 
@@ -17,12 +19,11 @@ def validate_param!(attr_name, params)
         excepts = @excepts.is_a?(Proc) ? @excepts.call : @excepts
         param_array = params[attr_name].nil? ? [nil] : Array.wrap(params[attr_name])
 
-        if !param_array.empty? && param_array.all? { |param| excepts.include?(param) }
-          raise Grape::Exceptions::Validation, params: [@scope.full_name(attr_name)], message: except_message
-        end
+        raise Grape::Exceptions::Validation, params: [@scope.full_name(attr_name)], message: except_message \
+          if !excepts.nil? && param_array.any? { |param| excepts.include?(param) }
 
-        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)
+        raise Grape::Exceptions::Validation, params: [@scope.full_name(attr_name)], message: message(:values) \
+          if !values.nil? && !param_array.all? { |param| values.include?(param) }
       end
 
       private

spec/grape/validations/validators/values_spec.rb

@@ -76,6 +76,11 @@ class API < Grape::API
           { type: params[:type] }
         end
 
+        params do
+          requires :type, values: []
+        end
+        get '/empty'
+
         params do
           optional :type, values: ValuesModel.values, default: 'valid-type2'
         end
@@ -90,6 +95,11 @@ class API < Grape::API
           { type: params[:type] }
         end
 
+        params do
+          requires :type, values: -> { [] }
+        end
+        get '/empty_lambda'
+
         params do
           optional :type, values: ValuesModel.values, default: -> { ValuesModel.values.sample }
         end
@@ -236,6 +246,12 @@ def app
     expect(last_response.body).to eq({ error: 'type does not have a valid value' }.to_json)
   end
 
+  it 'rejects all values if values is an empty array' do
+    get('/empty', 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
+
   context 'nil value for a parameter' do
     it 'does not allow for root params scope' do
       get('/', type: nil)
@@ -288,6 +304,12 @@ def app
     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
+    expect(last_response.body).to eq({ error: 'type does not have a valid value' }.to_json)
+  end
+
   it 'validates default value from proc' do
     get('/default_lambda')
     expect(last_response.status).to eq 200
@@ -428,6 +450,12 @@ def app
       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 an array of values if any of them matches except' do
+      get '/except/exclusive', type: %w(valid1 valid2 invalid-type1 valid4)
+      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 'exclusive excepts with lambda' do