Skip to content

Commit

Permalink
Fix declared_params regression with multiple allowed types
Browse files Browse the repository at this point in the history
Prior to Grape v1.5.0 and #2103, the following
would return `nil`:

```
params do
  optional :status_code, types: [Integer, String]
end
get '/' do
  declared_params
end
```

However, now it turns an empty `Array`.

We restore the previous behavior by not returning an empty `Array` if multiple
types are used.

Closes #2115
  • Loading branch information
stanhu committed Oct 9, 2020
1 parent 02d7113 commit 44217dd
Show file tree
Hide file tree
Showing 3 changed files with 14 additions and 2 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
#### Fixes

* Your contribution here.
* [#2115](https://github.com/ruby-grape/grape/pull/2115): Fix declared_params regression with multiple allowed types - [@stanhu](https://github.com/stanhu).

### 1.5.0 (2020/10/05)

Expand Down
2 changes: 1 addition & 1 deletion lib/grape/dsl/inside_route.rb
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ def handle_passed_param(params_nested_path, has_passed_children = false, &_block

if type == 'Hash' && !has_children
{}
elsif type == 'Array' || type&.start_with?('[')
elsif type == 'Array' || type&.start_with?('[') && !type&.include?(',')
[]
elsif type == 'Set' || type&.start_with?('#<Set')
Set.new
Expand Down
13 changes: 12 additions & 1 deletion spec/grape/endpoint/declared_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ def app
requires :first
optional :second
optional :third, default: 'third-default'
optional :multiple_types, types: [Integer, String]
optional :nested, type: Hash do
optional :fourth
optional :fifth
Expand Down Expand Up @@ -96,6 +97,16 @@ def app
expect(JSON.parse(last_response.body)['nested']['fourth']).to be_nil
end

it 'should show nil for multiple allowed types if include_missing is true' do
subject.get '/declared' do
declared(params, include_missing: true)
end

get '/declared?first=present'
expect(last_response.status).to eq(200)
expect(JSON.parse(last_response.body)['multiple_types']).to be_nil
end

it 'does not work in a before filter' do
subject.before do
declared(params)
Expand All @@ -113,7 +124,7 @@ def app
end
get '/declared?first=present'
expect(last_response.status).to eq(200)
expect(JSON.parse(last_response.body).keys.size).to eq(10)
expect(JSON.parse(last_response.body).keys.size).to eq(11)
end

it 'has a optional param with default value all the time' do
Expand Down

0 comments on commit 44217dd

Please sign in to comment.