Fix coerce_nil when Array of Type

CHANGELOG.md

@@ -9,6 +9,7 @@
 #### Fixes
 
 * Your contribution here.
+* [#2040](https://github.com/ruby-grape/grape/pull/2040): Fix a regression with Array of type nil - [@ericproulx](https://github.com/ericproulx).
 
 ### 1.3.2 (2020/04/12)
 

UPGRADING.md

@@ -1,6 +1,65 @@
 Upgrading Grape
 ===============
 
+### Upgrading to >= 1.4.0
+
+#### Nil values for structures
+
+Nil values always been a special case when dealing with types especially with the following structures:
+ - Array
+ - Hash
+ - Set
+
+The behaviour for these structures has change through out the latest releases. For instance:
+
+```ruby
+class Api < Grape::API
+  params do
+    require :my_param, type: Array[Integer]
+  end
+
+  get 'example' do
+     params[:my_param]
+  end
+  get '/example', params: { my_param: nil }
+  # 1.3.1 = []
+  # 1.3.2 = nil
+end
+```
+For now on, `nil` values stay `nil` values for all types, including arrays, sets and hashes.
+
+If you want to have the same behavior as 1.3.1, apply a `default` validator
+
+```ruby
+class Api < Grape::API
+  params do
+    require :my_param, type: Array[Integer], default: []
+  end
+
+  get 'example' do
+     params[:my_param]
+  end
+  get '/example', params: { my_param: nil } # => []
+end
+```
+
+#### Default validator
+
+Default validator is now applied for `nil` values.
+
+```ruby
+class Api < Grape::API
+  params do
+    requires :my_param, type: Integer, default: 0
+  end
+
+  get 'example' do
+     params[:my_param]
+  end
+  get '/example', params: { my_param: nil } #=> before: nil, after: 0
+end
+```
+
 ### Upgrading to >= 1.3.0
 
 #### Ruby

lib/grape/validations/types/array_coercer.rb

@@ -32,6 +32,8 @@ def call(_val)
         protected
 
         def coerce_elements(collection)
+          return if collection.nil?
+
           collection.each_with_index do |elem, index|
             return InvalidValue.new if reject?(elem)
 

lib/grape/validations/types/dry_type_coercer.rb

@@ -27,6 +27,8 @@ def initialize(type, strict = false)
         #
         # @param val [Object]
         def call(val)
+          return if val.nil?
+
           @coercer[val]
         rescue Dry::Types::CoercionError => _e
           InvalidValue.new

lib/grape/validations/types/variant_collection_coercer.rb

@@ -33,7 +33,7 @@ def initialize(types, method = nil)
         #   the coerced result, or an instance
         #   of {InvalidValue} if the value could not be coerced.
         def call(value)
-          return InvalidValue.new unless value.is_a? Array
+          return unless value.is_a? Array
 
           value =
             if @method

lib/grape/validations/validators/coerce.rb

@@ -67,21 +67,12 @@ def valid_type?(val)
       end
 
       def coerce_value(val)
-        val.nil? ? coerce_nil(val) : converter.call(val)
+        converter.call(val)
       # Some custom types might fail, so it should be treated as an invalid value
       rescue StandardError
         Types::InvalidValue.new
       end
 
-      def coerce_nil(val)
-        # define default values for structures, the dry-types lib which is used
-        # for coercion doesn't accept nil as a value, so it would fail
-        return []      if type == Array
-        return Set.new if type == Set
-        return {}      if type == Hash
-        val
-      end
-
       # Type to which the parameter will be coerced.
       #
       # @return [Class]

lib/grape/validations/validators/default.rb

@@ -9,7 +9,6 @@ def initialize(attrs, options, required, scope, opts = {})
       end
 
       def validate_param!(attr_name, params)
-        return if params.key? attr_name
         params[attr_name] = if @default.is_a? Proc
                               @default.call
                             elsif @default.frozen? || !duplicatable?(@default)

spec/grape/validations/validators/coerce_spec.rb

@@ -424,6 +424,79 @@ def self.parsed?(value)
         expect(last_response.status).to eq(200)
         expect(last_response.body).to eq(integer_class_name)
       end
+
+      context 'nil values' do
+        context 'primitive types' do
+          Grape::Validations::Types::PRIMITIVES.each do |type|
+            it 'respects the nil value' do
+              subject.params do
+                requires :param, type: type
+              end
+              subject.get '/nil_value' do
+                params[:param].class
+              end
+
+              get '/nil_value', param: nil
+              expect(last_response.status).to eq(200)
+              expect(last_response.body).to eq('NilClass')
+            end
+          end
+        end
+
+        context 'structures types' do
+          Grape::Validations::Types::STRUCTURES.each do |type|
+            it 'respects the nil value' do
+              subject.params do
+                requires :param, type: type
+              end
+              subject.get '/nil_value' do
+                params[:param].class
+              end
+
+              get '/nil_value', param: nil
+              expect(last_response.status).to eq(200)
+              expect(last_response.body).to eq('NilClass')
+            end
+          end
+        end
+
+        context 'special types' do
+          Grape::Validations::Types::SPECIAL.each_key do |type|
+            it 'respects the nil value' do
+              subject.params do
+                requires :param, type: type
+              end
+              subject.get '/nil_value' do
+                params[:param].class
+              end
+
+              get '/nil_value', param: nil
+              expect(last_response.status).to eq(200)
+              expect(last_response.body).to eq('NilClass')
+            end
+          end
+
+          context 'variant-member-type collections' do
+            [
+              Array[Integer, String],
+              [Integer, String, Array[Integer, String]]
+            ].each do |type|
+              it 'respects the nil value' do
+                subject.params do
+                  requires :param, type: type
+                end
+                subject.get '/nil_value' do
+                  params[:param].class
+                end
+
+                get '/nil_value', param: nil
+                expect(last_response.status).to eq(200)
+                expect(last_response.body).to eq('NilClass')
+              end
+            end
+          end
+        end
+      end
     end
 
     context 'using coerce_with' do
@@ -752,19 +825,6 @@ def self.parsed?(value)
         expect(last_response.body).to eq('String')
       end
 
-      it 'respects nil values' do
-        subject.params do
-          optional :a, types: [File, String]
-        end
-        subject.get '/' do
-          params[:a].class.to_s
-        end
-
-        get '/', a: nil
-        expect(last_response.status).to eq(200)
-        expect(last_response.body).to eq('NilClass')
-      end
-
       it 'fails when no coercion is possible' do
         subject.params do
           requires :a, types: [Boolean, Integer]

spec/grape/validations/validators/default_spec.rb

@@ -298,4 +298,125 @@ def app
       end
     end
   end
+
+  context 'optional with nil as value' do
+    subject do
+      Class.new(Grape::API) do
+        default_format :json
+      end
+    end
+
+    def app
+      subject
+    end
+
+    context 'primitive types' do
+      [
+        [Integer, 0],
+        [Integer, 42],
+        [Float, 0.0],
+        [Float, 4.2],
+        [BigDecimal, 0.0],
+        [BigDecimal, 4.2],
+        [Numeric, 0],
+        [Numeric, 42],
+        [Date, Date.today],
+        [DateTime, DateTime.now],
+        [Time, Time.now],
+        [Time, Time.at(0)],
+        [Grape::API::Boolean, false],
+        [String, ''],
+        [String, 'non-empty-string'],
+        [Symbol, :symbol],
+        [TrueClass, true],
+        [FalseClass, false]
+      ].each do |type, default|
+        it 'respects the default value' do
+          subject.params do
+            optional :param, type: type, default: default
+          end
+          subject.get '/default_value' do
+            params[:param]
+          end
+
+          get '/default_value', param: nil
+          expect(last_response.status).to eq(200)
+          expect(last_response.body).to eq(default.to_json)
+        end
+      end
+    end
+
+    context 'structures types' do
+      [
+        [Hash, {}],
+        [Hash, { test: 'non-empty' }],
+        [Array, []],
+        [Array, ['non-empty']],
+        [Array[Integer], []],
+        [Set, []],
+        [Set, [1]]
+      ].each do |type, default|
+        it 'respects the default value' do
+          subject.params do
+            optional :param, type: type, default: default
+          end
+          subject.get '/default_value' do
+            params[:param]
+          end
+
+          get '/default_value', param: nil
+          expect(last_response.status).to eq(200)
+          expect(last_response.body).to eq(default.to_json)
+        end
+      end
+    end
+
+    context 'special types' do
+      [
+        [JSON, ''],
+        [JSON, { test: 'non-empty-string' }.to_json],
+        [Array[JSON], []],
+        [Array[JSON], [{ test: 'non-empty-string' }.to_json]],
+        [::File, ''],
+        [::File, { test: 'non-empty-string' }.to_json],
+        [Rack::Multipart::UploadedFile, ''],
+        [Rack::Multipart::UploadedFile, { test: 'non-empty-string' }.to_json]
+      ].each do |type, default|
+        it 'respects the default value' do
+          subject.params do
+            optional :param, type: type, default: default
+          end
+          subject.get '/default_value' do
+            params[:param]
+          end
+
+          get '/default_value', param: nil
+          expect(last_response.status).to eq(200)
+          expect(last_response.body).to eq(default.to_json)
+        end
+      end
+    end
+
+    context 'variant-member-type collections' do
+      [
+        [Array[Integer, String], [0, '']],
+        [Array[Integer, String], [42, 'non-empty-string']],
+        [[Integer, String, Array[Integer, String]], [0, '', [0, '']]],
+        [[Integer, String, Array[Integer, String]], [42, 'non-empty-string', [42, 'non-empty-string']]]
+      ].each do |type, default|
+        it 'respects the default value' do
+          subject.params do
+            optional :param, type: type, default: default
+          end
+          subject.get '/default_value' do
+            params[:param]
+          end
+
+          get '/default_value', param: nil
+          expect(last_response.status).to eq(200)
+          expect(last_response.body).to eq(default.to_json)
+        end
+      end
+    end
+  end
 end

spec/grape/validations/validators/values_spec.rb

@@ -319,7 +319,7 @@ def app
       expect(last_response.status).to eq 200
     end
 
-    it 'allows for an optional param with a list of values' do
+    it 'accepts for an optional param with a list of values' do
       put('/optional_with_array_of_string_values', optional: nil)
       expect(last_response.status).to eq 200
     end