Merge pull request #1710 from pablonahuelgomez/make-declared-transform-to-what-is-expected

Fix wrong transformation of empty Array in declared params

Author: dblock

CHANGELOG.md

@@ -8,6 +8,7 @@
 
 #### Fixes
 
+* [#1710](https://github.com/ruby-grape/grape/pull/1710): Fix wrong transformation of empty Array in declared params - [@pablonahuelgomez](https://github.com/pablonahuelgomez).
 * Your contribution here.
 
 ### 1.0.1 (9/8/2017)

lib/grape/dsl/inside_route.rb

@@ -47,9 +47,20 @@ def declared_array(passed_params, options, declared_params)
 
         def declared_hash(passed_params, options, declared_params)
           declared_params.each_with_object(passed_params.class.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)
+            if declared_param.is_a?(Hash)
+              declared_param.each_pair do |declared_parent_param, declared_children_params|
+                next unless options[:include_missing] || passed_params.key?(declared_parent_param)
+
+                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(declared_parent_param, passed_children_params) do
+                  declared(passed_children_params, options, declared_children_params)
+                end
+              end
+            else
+              # If it is not a Hash then it does not have children.
+              # Find its value or set it to nil.
               has_alias = route_setting(:aliased_params) && route_setting(:aliased_params).find { |current| current[declared_param] }
               param_alias = has_alias[declared_param] if has_alias
 
@@ -60,17 +71,26 @@ def declared_hash(passed_params, options, declared_params)
               else
                 memo[optioned_param_key(declared_param, options)] = passed_params[declared_param]
               end
-            else
-              declared_param.each_pair do |declared_parent_param, declared_children_params|
-                next unless options[:include_missing] || passed_params.key?(declared_parent_param)
-
-                passed_children_params = passed_params[declared_parent_param] || passed_params.class.new
-                memo[optioned_param_key(declared_parent_param, options)] = declared(passed_children_params, options, declared_children_params)
-              end
             end
           end
         end
 
+        def handle_passed_param(declared_param, passed_children_params, &_block)
+          should_be_empty_array?(declared_param, passed_children_params) ? [] : yield
+        end
+
+        def should_be_empty_array?(declared_param, passed_children_params)
+          declared_param_is_array?(declared_param) && passed_children_params.empty?
+        end
+
+        def declared_param_is_array?(declared_param)
+          route_options_params[declared_param.to_s] && route_options_params[declared_param.to_s][:type] == 'Array'
+        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

spec/grape/endpoint_spec.rb

@@ -295,6 +295,9 @@ def app
             end
           end
         end
+        optional :nested_arr, type: Array do
+          optional :eighth
+        end
       end
     end
 
@@ -366,7 +369,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(4)
+      expect(JSON.parse(last_response.body).keys.size).to eq(5)
     end
 
     it 'has a optional param with default value all the time' do
@@ -408,8 +411,8 @@ def app
       expect(JSON.parse(last_response.body)['nested'].size).to eq 2
     end
 
-    context 'sets nested hash when the param is missing' do
-      it 'to be array when include_missing is true' do
+    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
@@ -419,6 +422,16 @@ def app
         expect(JSON.parse(last_response.body)['nested']).to be_a(Hash)
       end
 
+      it 'to be an array when 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_arr']).to be_a(Array)
+      end
+
       it 'to be nil when include_missing is false' do
         subject.get '/declared' do
           declared(params, include_missing: false)

spec/grape/validations/params_scope_spec.rb

@@ -339,6 +339,18 @@ def initialize(value)
       get '/optional_type_array'
     end
 
+    it 'handles missing optional Array type' do
+      subject.params do
+        optional :a, type: Array do
+          requires :b
+        end
+      end
+      subject.get('/test') { declared(params).to_json }
+      get '/test'
+      expect(last_response.status).to eq(200)
+      expect(last_response.body).to eq('{"a":[]}')
+    end
+
     it 'errors with an unsupported type' do
       expect do
         subject.params do