Ensure complete declared params structure is present

Author: Tim Connor

CHANGELOG.md

@@ -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).

README.md

@@ -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
@@ -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
@@ -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
 
@@ -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.

UPGRADING.md

@@ -1,6 +1,45 @@
 Upgrading Grape
 ===============
 
+### Upgrading to >= 1.4.1
+
+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 the behavior was changed, so a missing Hash with or without nested parameters would always resolve to `{}`.
+
+In 1.4.1 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.4.0 = {}
+# 1.4.1 = {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
@@ -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
@@ -47,7 +86,7 @@ class MyObject
       yield process_records(records, first)
       first = false
     end
-    yield ']'  
+    yield ']'
   end
 
   def process_records(records, first)

lib/grape/dsl/inside_route.rb

@@ -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
@@ -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

spec/grape/endpoint/declared_spec.rb

@@ -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
 
@@ -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
@@ -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
@@ -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(body['empty_set']).to eq('#<Set: {}>')
+        expect(body['empty_typed_set']).to eq('#<Set: {}>')
+        expect(body['nested']['empty_set']).to eq('#<Set: {}>')
+        expect(body['nested']['empty_typed_set']).to eq('#<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