refactor declared and include nil values for missing nested params (#1575)

Author: thogg4

CHANGELOG.md

@@ -9,6 +9,7 @@
 
 * [#1555](https://github.com/ruby-grape/grape/pull/1555): Added code coverage w/Coveralls - [@dblock](https://github.com/dblock).
 * [#1568](https://github.com/ruby-grape/grape/pull/1568): Add `proc` option to `values` validator to allow custom checks - [@jlfaber](https://github.com/jlfaber).
+* [#1575](https://github.com/ruby-grape/grape/pull/1575): Include nil values for missing nested params in declared - [@thogg4](https://github.com/thogg4).
 * Your contribution here.
 
 #### Fixes

lib/grape/dsl/inside_route.rb

@@ -26,41 +26,57 @@ def self.post_filter_methods(type)
       # Methods which should not be available in filters until the before filter
       # has completed
       module PostBeforeFilter
-        def declared(params, options = {}, declared_params = nil)
+        def declared(passed_params, options = {}, declared_params = nil)
           options = options.reverse_merge(include_missing: true, include_parent_namespaces: true)
+          declared_params ||= optioned_declared_params(options)
 
-          # Declared params including parent namespaces
-          all_declared_params = route_setting(:saved_declared_params).flatten | Array(route_setting(:declared_params))
+          if passed_params.is_a?(Array)
+            declared_array(passed_params, options, declared_params)
+          else
+            declared_hash(passed_params, options, declared_params)
+          end
+        end
 
-          # Declared params at current namespace
-          current_namespace_declared_params = route_setting(:saved_declared_params).last & Array(route_setting(:declared_params))
+        private
 
-          declared_params ||= options[:include_parent_namespaces] ? all_declared_params : current_namespace_declared_params
+        def declared_array(passed_params, options, declared_params)
+          passed_params.map do |passed_param|
+            declared(passed_param || {}, options, declared_params)
+          end
+        end
 
-          raise ArgumentError, 'Tried to filter for declared parameters but none exist.' unless declared_params
+        def declared_hash(passed_params, options, declared_params)
+          declared_params.each_with_object(Hashie::Mash.new) do |declared_param, memo|
+            # If it is not a Hash then it does not have children.
+            # Find its value or set it to nil.
+            if !declared_param.is_a?(Hash)
+              memo[optioned_param_key(declared_param, options)] = passed_params[declared_param]
+            else
+              declared_param.each_pair do |declared_parent_param, declared_children_params|
+                next unless options[:include_missing] || passed_params.key?(declared_parent_param)
 
-          if params.is_a? Array
-            params.map do |param|
-              declared(param || {}, options, declared_params)
+                passed_children_params = passed_params[declared_parent_param] || Hashie::Mash.new
+                memo[optioned_param_key(declared_parent_param, options)] = declared(passed_children_params, @options, declared_children_params)
+              end
             end
-          else
-            declared_params.each_with_object(Hashie::Mash.new) do |key, hash|
-              key = { key => nil } unless key.is_a? Hash
+          end
+        end
 
-              key.each_pair do |parent, children|
-                output_key = options[:stringify] ? parent.to_s : parent.to_sym
+        def optioned_param_key(declared_param, options)
+          options[:stringify] ? declared_param.to_s : declared_param.to_sym
+        end
 
-                next unless options[:include_missing] || params.key?(parent)
+        def optioned_declared_params(options)
+          declared_params = if options[:include_parent_namespaces]
+                              # Declared params including parent namespaces
+                              route_setting(:saved_declared_params).flatten | Array(route_setting(:declared_params))
+                            else
+                              # Declared params at current namespace
+                              route_setting(:saved_declared_params).last & Array(route_setting(:declared_params))
+                            end
 
-                hash[output_key] = if children
-                                     children_params = params[parent] || (children.is_a?(Array) ? [] : {})
-                                     declared(children_params, options, Array(children))
-                                   else
-                                     params[parent]
-                                   end
-              end
-            end
-          end
+          raise ArgumentError, 'Tried to filter for declared parameters but none exist.' unless declared_params
+          declared_params
         end
       end
 

spec/grape/endpoint_spec.rb

@@ -251,16 +251,34 @@ def app
 
   describe '#declared' do
     before do
+      subject.format :json
       subject.params do
         requires :first
         optional :second
         optional :third, default: 'third-default'
         optional :nested, type: Hash do
           optional :fourth
+          optional :fifth
+          optional :nested_two, type: Hash do
+            optional :sixth
+            optional :nested_three, type: Hash do
+              optional :seventh
+            end
+          end
         end
       end
     end
 
+    it 'should show nil for nested params 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)['nested']['fourth']).to be_nil
+    end
+
     it 'does not work in a before filter' do
       subject.before do
         declared(params)
@@ -273,37 +291,31 @@ def app
     end
 
     it 'has as many keys as there are declared params' do
-      inner_params = nil
       subject.get '/declared' do
-        inner_params = declared(params).keys
-        ''
+        declared(params)
       end
       get '/declared?first=present'
       expect(last_response.status).to eq(200)
-      expect(inner_params.size).to eq(4)
+      expect(JSON.parse(last_response.body).keys.size).to eq(4)
     end
 
     it 'has a optional param with default value all the time' do
-      inner_params = nil
       subject.get '/declared' do
-        inner_params = declared(params)
-        ''
+        declared(params)
       end
       get '/declared?first=one'
       expect(last_response.status).to eq(200)
-      expect(inner_params[:third]).to eql('third-default')
+      expect(JSON.parse(last_response.body)['third']).to eql('third-default')
     end
 
     it 'builds nested params' do
-      inner_params = nil
       subject.get '/declared' do
-        inner_params = declared(params)
-        ''
+        declared(params)
       end
 
       get '/declared?first=present&nested[fourth]=1'
       expect(last_response.status).to eq(200)
-      expect(inner_params[:nested].keys.size).to eq 1
+      expect(JSON.parse(last_response.body)['nested'].keys.size).to eq 3
     end
 
     it 'builds nested params when given array' do
@@ -317,76 +329,54 @@ def app
           optional :fourth
         end
       end
-      inner_params = nil
       subject.get '/declared' do
-        inner_params = declared(params)
-        ''
+        declared(params)
       end
 
       get '/declared?first=present&nested[][fourth]=1&nested[][fourth]=2'
       expect(last_response.status).to eq(200)
-      expect(inner_params[:nested].size).to eq 2
+      expect(JSON.parse(last_response.body)['nested'].size).to eq 2
     end
 
-    it 'builds nested params' do
-      inner_params = nil
-      subject.get '/declared' do
-        inner_params = declared(params)
-        ''
-      end
-
-      get '/declared?first=present&nested[fourth]=1'
-      expect(last_response.status).to eq(200)
-      expect(inner_params[:nested].keys.size).to eq 1
-    end
-
-    context 'sets nested array when the param is missing' do
+    context 'sets nested hash when the param is missing' do
       it 'to be array when include_missing is true' do
-        inner_params = nil
         subject.get '/declared' do
-          inner_params = declared(params, include_missing: true)
-          ''
+          declared(params, include_missing: true)
         end
 
         get '/declared?first=present'
         expect(last_response.status).to eq(200)
-        expect(inner_params[:nested]).to be_a(Array)
+        expect(JSON.parse(last_response.body)['nested']).to be_a(Hash)
       end
 
       it 'to be nil when include_missing is false' do
-        inner_params = nil
         subject.get '/declared' do
-          inner_params = declared(params, include_missing: false)
-          ''
+          declared(params, include_missing: false)
         end
 
         get '/declared?first=present'
         expect(last_response.status).to eq(200)
-        expect(inner_params[:nested]).to be_nil
+        expect(JSON.parse(last_response.body)['nested']).to be_nil
       end
     end
 
     it 'filters out any additional params that are given' do
-      inner_params = nil
       subject.get '/declared' do
-        inner_params = declared(params)
-        ''
+        declared(params)
       end
       get '/declared?first=one&other=two'
       expect(last_response.status).to eq(200)
-      expect(inner_params.key?(:other)).to eq false
+      expect(JSON.parse(last_response.body).key?(:other)).to eq false
     end
 
     it 'stringifies if that option is passed' do
-      inner_params = nil
       subject.get '/declared' do
-        inner_params = declared(params, stringify: true)
-        ''
+        declared(params, stringify: true)
       end
 
       get '/declared?first=one&other=two'
       expect(last_response.status).to eq(200)
-      expect(inner_params['first']).to eq 'one'
+      expect(JSON.parse(last_response.body)['first']).to eq 'one'
     end
 
     it 'does not include missing attributes if that option is passed' do
@@ -447,20 +437,18 @@ def app
         end
       end
 
-      inner_params = nil
       subject.get '/declared' do
-        inner_params = declared(params, include_missing: false)
-        ''
+        declared(params, include_missing: false)
       end
 
       get '/declared?first=present&nested[fourth]=&nested[nested_nested][sixth]=sixth'
-
+      json = JSON.parse(last_response.body)
       expect(last_response.status).to eq(200)
-      expect(inner_params[:first]).to eq 'present'
-      expect(inner_params[:nested].keys).to eq %w(fourth fifth nested_nested)
-      expect(inner_params[:nested][:fourth]).to eq ''
-      expect(inner_params[:nested][:nested_nested].keys).to eq %w(sixth seven)
-      expect(inner_params[:nested][:nested_nested][:sixth]).to eq 'sixth'
+      expect(json['first']).to eq 'present'
+      expect(json['nested'].keys).to eq %w(fourth fifth nested_nested)
+      expect(json['nested']['fourth']).to eq ''
+      expect(json['nested']['nested_nested'].keys).to eq %w(sixth seven)
+      expect(json['nested']['nested_nested']['sixth']).to eq 'sixth'
     end
   end