diff --git a/CHANGELOG.md b/CHANGELOG.md index 3ef794ae80..6788392a39 100644 --- a/CHANGELOG.md +++ b/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) diff --git a/lib/grape/dsl/inside_route.rb b/lib/grape/dsl/inside_route.rb index 55660e94c2..ba9155dbd4 100644 --- a/lib/grape/dsl/inside_route.rb +++ b/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 diff --git a/spec/grape/endpoint_spec.rb b/spec/grape/endpoint_spec.rb index ff06105388..dbe1d2cab6 100644 --- a/spec/grape/endpoint_spec.rb +++ b/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) diff --git a/spec/grape/validations/params_scope_spec.rb b/spec/grape/validations/params_scope_spec.rb index e668fd9ecc..e048496edf 100644 --- a/spec/grape/validations/params_scope_spec.rb +++ b/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