coerce_with should be called for params with nil value (#2164)

Author: Gwénaël Rault

CHANGELOG.md

@@ -10,6 +10,7 @@
 
 * [#2161](https://github.com/ruby-grape/grape/pull/2157): Handle EOFError from Rack when given an empty multipart body - [@bschmeck](https://github.com/bschmeck).
 * [#2162](https://github.com/ruby-grape/grape/pull/2162): Corrected a hash modification while iterating issue - [@Jack12816](https://github.com/Jack12816).
+* [#2164](https://github.com/ruby-grape/grape/pull/2164): Fix: `coerce_with` is now called for params with `nil` value - [@braktar](https://github.com/braktar).
 
 ### 1.5.2 (2021/02/06)
 

README.md

@@ -1228,6 +1228,7 @@ params do
   end
 end
 ```
+Note that, a `nil` value will call the custom coercion method, while a missing parameter will not.
 
 Example of use of `coerce_with` with a lambda (a class with a `parse` method could also have been used)
 It will parse a string and return an Array of Integers, matching the `Array[Integer]` `type`.

UPGRADING.md

@@ -1,6 +1,33 @@
 Upgrading Grape
 ===============
 
+
+### Upgrading to >= 1.5.3
+
+### Nil value and coercion
+
+Prior to 1.2.5 version passing a `nil` value for a parameter with a custom coercer would invoke the coercer, and not passing a parameter would not invoke it.
+This behavior was not tested or documented. Version 1.3.0 quietly changed this behavior, in such that `nil` values skipped the coercion. Version 1.5.3 fixes and documents this as follows:
+
+```ruby
+class Api < Grape::API
+  params do
+    optional :value, type: Integer, coerce_with: ->(val) { val || 0 }
+  end
+
+  get 'example' do
+     params[:my_param]
+  end
+  get '/example', params: { value: nil }
+  # 1.5.2 = nil
+  # 1.5.3 = 0
+  get '/example', params: {}
+  # 1.5.2 = nil
+  # 1.5.3 = nil
+end
+```
+See [#2164](https://github.com/ruby-grape/grape/pull/2164) for more information.
+
 ### Upgrading to >= 1.5.1
 
 #### Dependent params

lib/grape/validations/types/custom_type_coercer.rb

@@ -52,8 +52,6 @@ def initialize(type, method = nil)
         #   this should always be a string.
         # @return [Object] the coerced result
         def call(val)
-          return if val.nil?
-
           coerced_val = @method.call(val)
 
           return coerced_val if coerced_val.is_a?(InvalidValue)

spec/grape/validations/validators/coerce_spec.rb

@@ -706,6 +706,44 @@ def self.parse(_val)
         expect(JSON.parse(last_response.body)).to eq([1, 1, 1, 1])
       end
 
+      context 'Array type and coerce_with should' do
+        before do
+          subject.params do
+            optional :arr, type: Array, coerce_with: (lambda do |val|
+              if val.nil?
+                []
+              else
+                val
+              end
+            end)
+          end
+          subject.get '/' do
+            params[:arr].class.to_s
+          end
+        end
+
+        it 'coerce nil value to array' do
+          get '/', arr: nil
+
+          expect(last_response.status).to eq(200)
+          expect(last_response.body).to eq('Array')
+        end
+
+        it 'not coerce missing field' do
+          get '/'
+
+          expect(last_response.status).to eq(200)
+          expect(last_response.body).to eq('NilClass')
+        end
+
+        it 'coerce array as array' do
+          get '/', arr: []
+
+          expect(last_response.status).to eq(200)
+          expect(last_response.body).to eq('Array')
+        end
+      end
+
       it 'uses parse where available' do
         subject.params do
           requires :ints, type: Array, coerce_with: JSON do
@@ -754,6 +792,44 @@ def self.parse(_val)
         expect(last_response.body).to eq('3')
       end
 
+      context 'Integer type and coerce_with should' do
+        before do
+          subject.params do
+            optional :int, type: Integer, coerce_with: (lambda do |val|
+              if val.nil?
+                0
+              else
+                val.to_i
+              end
+            end)
+          end
+          subject.get '/' do
+            params[:int].class.to_s
+          end
+        end
+
+        it 'coerce nil value to integer' do
+          get '/', int: nil
+
+          expect(last_response.status).to eq(200)
+          expect(last_response.body).to eq('Integer')
+        end
+
+        it 'not coerce missing field' do
+          get '/'
+
+          expect(last_response.status).to eq(200)
+          expect(last_response.body).to eq('NilClass')
+        end
+
+        it 'coerce integer as integer' do
+          get '/', int: 1
+
+          expect(last_response.status).to eq(200)
+          expect(last_response.body).to eq('Integer')
+        end
+      end
+
       context 'Integer type and coerce_with potentially returning nil' do
         before do
           subject.params do