Skip to content

Commit

Permalink
coerce_with should be called for params with nil value (#2164)
Browse files Browse the repository at this point in the history
  • Loading branch information
Gwénaël Rault committed Mar 5, 2021
1 parent 8996fb1 commit e0f412c
Show file tree
Hide file tree
Showing 5 changed files with 105 additions and 2 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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`.
Expand Down
27 changes: 27 additions & 0 deletions UPGRADING.md
Original file line number Diff line number Diff line change
@@ -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
Expand Down
2 changes: 0 additions & 2 deletions lib/grape/validations/types/custom_type_coercer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
76 changes: 76 additions & 0 deletions spec/grape/validations/validators/coerce_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down

0 comments on commit e0f412c

Please sign in to comment.