Merge pull request #2103 from tlconnor/deepDeclared

Ensure complete declared params structure is present
ruby-grape · Sep 30, 2020 · 7e43215 · 7e43215
2 parents 0f3d30e + 678cd13
commit 7e43215
Show file tree

Hide file tree

Showing 7 changed files with 694 additions and 582 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,4 +1,4 @@
-### 1.4.1 (Next)
+### 1.5.0 (Next)
 
 #### Features
 
@@ -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).

diff --git a/README.md b/README.md
@@ -156,7 +156,7 @@ content negotiation, versioning and much more.
 
 ## Stable Release
 
-You're reading the documentation for the next release of Grape, which should be **1.4.1**.
+You're reading the documentation for the next release of Grape, which should be **1.5.0**.
 Please read [UPGRADING](UPGRADING.md) when upgrading from a previous version.
 The current stable release is [1.4.0](https://github.com/ruby-grape/grape/blob/v1.4.0/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.

diff --git a/UPGRADING.md b/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
@@ -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)

diff --git a/lib/grape/dsl/inside_route.rb b/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

diff --git a/lib/grape/version.rb b/lib/grape/version.rb
@@ -2,5 +2,5 @@
 
 module Grape
   # The current version of Grape.
-  VERSION = '1.4.1'
+  VERSION = '1.5.0'
 end