Skip to content

Commit

Permalink
Ensure complete declared params structure is present
Browse files Browse the repository at this point in the history
  • Loading branch information
Tim Connor committed Sep 28, 2020
1 parent 936009f commit 0a08f04
Show file tree
Hide file tree
Showing 6 changed files with 172 additions and 71 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Expand Up @@ -7,6 +7,7 @@
#### Fixes

* Your contribution here.
* [#2103](https://github.com/ruby-grape/grape/pull/2103): Ensure complete declared params structure is present - [@tlconnor](https://github.com/tlconnor).
* [#2099](https://github.com/ruby-grape/grape/pull/2099): Added truffleruby to Travis-CI - [@gogainda](https://github.com/gogainda).
* [#2089](https://github.com/ruby-grape/grape/pull/2089): Specify order of mounting Grape with Rack::Cascade in README - [@jonmchan](https://github.com/jonmchan).
* [#2083](https://github.com/ruby-grape/grape/pull/2083): Set `Cache-Control` header only for streamed responses - [@stanhu](https://github.com/stanhu).
Expand Down
53 changes: 48 additions & 5 deletions README.md
Expand Up @@ -353,7 +353,7 @@ use Rack::Session::Cookie
run Rack::Cascade.new [Web, API]
```

Note that order of loading apps using `Rack::Cascade` matters. The grape application must be last if you want to raise custom 404 errors from grape (such as `error!('Not Found',404)`). If the grape application is not last and returns 404 or 405 response, [cascade utilizes that as a signal to try the next app](https://www.rubydoc.info/gems/rack/Rack/Cascade). This may lead to undesirable behavior showing the [wrong 404 page from the wrong app](https://github.com/ruby-grape/grape/issues/1515).
Note that order of loading apps using `Rack::Cascade` matters. The grape application must be last if you want to raise custom 404 errors from grape (such as `error!('Not Found',404)`). If the grape application is not last and returns 404 or 405 response, [cascade utilizes that as a signal to try the next app](https://www.rubydoc.info/gems/rack/Rack/Cascade). This may lead to undesirable behavior showing the [wrong 404 page from the wrong app](https://github.com/ruby-grape/grape/issues/1515).


### Rails
Expand Down Expand Up @@ -787,7 +787,12 @@ Available parameter builders are `Grape::Extensions::Hash::ParamBuilder`, `Grape

### Declared

Grape allows you to access only the parameters that have been declared by your `params` block. It filters out the params that have been passed, but are not allowed. Consider the following API endpoint:
Grape allows you to access only the parameters that have been declared by your `params` block. It will:

* Filter out the params that have been passed, but are not allowed.
* Include any optional params that are declared but not passed.

Consider the following API endpoint:

````ruby
format :json
Expand Down Expand Up @@ -820,9 +825,9 @@ Once we add parameters requirements, grape will start returning only the declare
format :json

params do
requires :user, type: Hash do
requires :first_name, type: String
requires :last_name, type: String
optional :user, type: Hash do
optional :first_name, type: String
optional :last_name, type: String
end
end

Expand Down Expand Up @@ -850,6 +855,44 @@ curl -X POST -H "Content-Type: application/json" localhost:9292/users/signup -d
}
````

Missing params that are declared as type `Hash` or `Array` will be included.

````ruby
format :json

params do
optional :user, type: Hash do
optional :first_name, type: String
optional :last_name, type: String
end
optional :widgets, type: Array
end

post 'users/signup' do
{ 'declared_params' => declared(params) }
end
````

**Request**

````bash
curl -X POST -H "Content-Type: application/json" localhost:9292/users/signup -d '{}'
````

**Response**

````json
{
"declared_params": {
"user": {
"first_name": null,
"last_name": null
},
"widgets": []
}
}
````

The returned hash is an `ActiveSupport::HashWithIndifferentAccess`.

The `#declared` method is not available to `before` filters, as those are evaluated prior to parameter coercion.
Expand Down
47 changes: 43 additions & 4 deletions UPGRADING.md
@@ -1,6 +1,45 @@
Upgrading Grape
===============

### Upgrading to >= 1.5.0

Prior to 1.3.3, the `declared` helper would always return the complete params structure if `include_missing=true` was set. In 1.3.3 a regression was introduced such that a missing Hash with or without nested parameters would always resolve to `{}`.

In 1.5.0 this behavior is reverted, so the whole params structure will always be available via `declared`, regardless of whether any params are passed.

The following rules now apply to the `declared` helper when params are missing and `include_missing=true`:

* Hash params with children will resolve to a Hash with keys for each declared child.
* Hash params with no children will resolve to `{}`.
* Set params will resolve to `Set.new`.
* Array params will resolve to `[]`.
* All other params will resolve to `nil`.

#### Example

```ruby
class Api < Grape::API
params do
optional :outer, type: Hash do
optional :inner, type: Hash do
optional :value, type: String
end
end
end
get 'example' do
declared(params, include_missing: true)
end
end
```

```
get '/example'
# 1.3.3 = {}
# 1.5.0 = {outer: {inner: {value:null}}}
```

For more information see [#2103](https://github.com/ruby-grape/grape/pull/2103).

### Upgrading to >= 1.4.0

#### Reworking stream and file and un-deprecating stream like-objects
Expand Down Expand Up @@ -28,17 +67,17 @@ class API < Grape::API
end
```

Or use `stream` to stream other kinds of content. In the following example a streamer class
Or use `stream` to stream other kinds of content. In the following example a streamer class
streams paginated data from a database.

```ruby
class MyObject
class MyObject
attr_accessor :result

def initialize(query)
@result = query
end

def each
yield '['
# Do paginated DB fetches and return each page formatted
Expand All @@ -47,7 +86,7 @@ class MyObject
yield process_records(records, first)
first = false
end
yield ']'
yield ']'
end

def process_records(records, first)
Expand Down
59 changes: 23 additions & 36 deletions lib/grape/dsl/inside_route.rb
Expand Up @@ -58,7 +58,7 @@ def declared_hash(passed_params, options, declared_params, params_nested_path)
passed_children_params = passed_params[declared_parent_param] || passed_params.class.new
memo_key = optioned_param_key(declared_parent_param, options)

memo[memo_key] = handle_passed_param(passed_children_params, params_nested_path_dup) do
memo[memo_key] = handle_passed_param(params_nested_path_dup, passed_children_params.any?) do
declared(passed_children_params, options, declared_children_params, params_nested_path_dup)
end
end
Expand All @@ -70,57 +70,44 @@ def declared_hash(passed_params, options, declared_params, params_nested_path)

next unless options[:include_missing] || passed_params.key?(declared_param) || (param_renaming && passed_params.key?(param_renaming))

if param_renaming
memo[optioned_param_key(param_renaming, options)] = passed_params[param_renaming]
else
memo[optioned_param_key(declared_param, options)] = passed_params[declared_param]
memo_key = optioned_param_key(param_renaming || declared_param, options)
passed_param = passed_params[param_renaming || declared_param]

params_nested_path_dup = params_nested_path.dup
params_nested_path_dup << declared_param.to_s

memo[memo_key] = handle_passed_param(params_nested_path_dup) do
passed_param
end
end
end
end

def handle_passed_param(passed_children_params, params_nested_path, &_block)
if should_be_empty_hash?(passed_children_params, params_nested_path)
def handle_passed_param(params_nested_path, has_passed_children = false, &_block)
return yield if has_passed_children

key = params_nested_path[0]
key += '[' + params_nested_path[1..-1].join('][') + ']' if params_nested_path.size > 1

route_options_params = options[:route_options][:params] || {}
type = route_options_params.dig(key, :type)
has_children = route_options_params.keys.any? { |k| k != key && k.start_with?(key) }

if type == 'Hash' && !has_children
{}
elsif should_be_empty_array?(passed_children_params, params_nested_path)
elsif type == 'Array' || type&.start_with?('[')
[]
elsif type == 'Set' || type&.start_with?('#<Set')
Set.new
else
yield
end
end

def should_be_empty_array?(passed_children_params, params_nested_path)
passed_children_params.empty? && declared_param_is_array?(params_nested_path)
end

def declared_param_is_array?(params_nested_path)
key = route_options_params_key(params_nested_path)
route_options_params[key] && route_options_params[key][:type] == 'Array'
end

def should_be_empty_hash?(passed_children_params, params_nested_path)
passed_children_params.empty? && declared_param_is_hash?(params_nested_path)
end

def declared_param_is_hash?(params_nested_path)
key = route_options_params_key(params_nested_path)
route_options_params[key] && route_options_params[key][:type] == 'Hash'
end

def route_options_params
options[:route_options][:params] || {}
end

def optioned_param_key(declared_param, options)
options[:stringify] ? declared_param.to_s : declared_param.to_sym
end

def route_options_params_key(params_nested_path)
key = params_nested_path[0]
key += '[' + params_nested_path[1..-1].join('][') + ']' if params_nested_path.size > 1
key
end

def optioned_declared_params(**options)
declared_params = if options[:include_parent_namespaces]
# Declared params including parent namespaces
Expand Down
2 changes: 1 addition & 1 deletion lib/grape/version.rb
Expand Up @@ -2,5 +2,5 @@

module Grape
# The current version of Grape.
VERSION = '1.4.1'
VERSION = '1.5.0'
end
81 changes: 56 additions & 25 deletions spec/grape/endpoint/declared_spec.rb
Expand Up @@ -28,10 +28,20 @@ def app
optional :nested_arr, type: Array do
optional :eighth
end
optional :empty_arr, type: Array
optional :empty_typed_arr, type: Array[String]
optional :empty_hash, type: Hash
optional :empty_set, type: Set
optional :empty_typed_set, type: Set[String]
end
optional :arr, type: Array do
optional :nineth
end
optional :empty_arr, type: Array
optional :empty_typed_arr, type: Array[String]
optional :empty_hash, type: Hash
optional :empty_set, type: Set
optional :empty_typed_set, type: Set[String]
end
end

Expand Down Expand Up @@ -103,7 +113,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(5)
expect(JSON.parse(last_response.body).keys.size).to eq(10)
end

it 'has a optional param with default value all the time' do
Expand All @@ -122,7 +132,7 @@ def app

get '/declared?first=present&nested[fourth]=1'
expect(last_response.status).to eq(200)
expect(JSON.parse(last_response.body)['nested'].keys.size).to eq 4
expect(JSON.parse(last_response.body)['nested'].keys.size).to eq 9
end

it 'builds nested params when given array' do
Expand All @@ -145,45 +155,66 @@ def app
expect(JSON.parse(last_response.body)['nested'].size).to eq 2
end

context 'sets nested objects when the param is missing' do
it 'to be a hash when include_missing is true' do
subject.get '/declared' do
declared(params, include_missing: true)
end
context 'when the param is missing and include_missing=false' do
before do
subject.get('/declared') { declared(params, include_missing: false) }
end

it 'sets nested objects to be nil' do
get '/declared?first=present'
expect(last_response.status).to eq(200)
expect(JSON.parse(last_response.body)['nested']).to eq({})
expect(JSON.parse(last_response.body)['nested']).to be_nil
end
end

it 'to be an array when include_missing is true' do
subject.get '/declared' do
declared(params, include_missing: true)
end
context 'when the param is missing and include_missing=true' do
before do
subject.get('/declared') { declared(params, include_missing: true) }
end

it 'sets objects with type=Hash to be a hash' do
get '/declared?first=present'
expect(last_response.status).to eq(200)
expect(JSON.parse(last_response.body)['arr']).to be_a(Array)
end

it 'to be an array when nested and include_missing is true' do
subject.get '/declared' do
declared(params, include_missing: true)
end
body = JSON.parse(last_response.body)
expect(body['empty_hash']).to eq({})
expect(body['nested']).to be_a(Hash)
expect(body['nested']['empty_hash']).to eq({})
expect(body['nested']['nested_two']).to be_a(Hash)
end

get '/declared?first=present&nested[fourth]=1'
it 'sets objects with type=Set to be a set' do
get '/declared?first=present'
expect(last_response.status).to eq(200)
expect(JSON.parse(last_response.body)['nested']['nested_arr']).to be_a(Array)

body = JSON.parse(last_response.body)
expect(['#<Set: {}>', []]).to include(body['empty_set'])
expect(['#<Set: {}>', []]).to include(body['empty_typed_set'])
expect(['#<Set: {}>', []]).to include(body['nested']['empty_set'])
expect(['#<Set: {}>', []]).to include(body['nested']['empty_typed_set'])
end

it 'to be nil when include_missing is false' do
subject.get '/declared' do
declared(params, include_missing: false)
end
it 'sets objects with type=Array to be an array' do
get '/declared?first=present'
expect(last_response.status).to eq(200)

body = JSON.parse(last_response.body)
expect(body['empty_arr']).to eq([])
expect(body['empty_typed_arr']).to eq([])
expect(body['arr']).to eq([])
expect(body['nested']['empty_arr']).to eq([])
expect(body['nested']['empty_typed_arr']).to eq([])
expect(body['nested']['nested_arr']).to eq([])
end

it 'includes all declared children when type=Hash' do
get '/declared?first=present'
expect(last_response.status).to eq(200)
expect(JSON.parse(last_response.body)['nested']).to be_nil

body = JSON.parse(last_response.body)
expect(body['nested'].keys).to eq(%w[fourth fifth nested_two nested_arr empty_arr empty_typed_arr empty_hash empty_set empty_typed_set])
expect(body['nested']['nested_two'].keys).to eq(%w[sixth nested_three])
expect(body['nested']['nested_two']['nested_three'].keys).to eq(%w[seventh])
end
end

Expand Down

0 comments on commit 0a08f04

Please sign in to comment.