diff --git a/CHANGELOG.md b/CHANGELOG.md index 1b05154019..926e37e9da 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,7 @@ Next Release * [#879](https://github.com/intridea/grape/pull/879): The `route_info` value is no longer included in `params` Hash - [@rodzyn](https://github.com/rodzyn). * [#881](https://github.com/intridea/grape/issues/881): Fixed `Grape::Validations::ValuesValidator` support for `Range` type - [@ajvondrak](https://github.com/ajvondrak). * [#901](https://github.com/intridea/grape/pull/901): Fix: callbacks defined in a version block are only called for the routes defined in that block - [@kushkella](https://github.com/kushkella). +* [#886](https://github.com/intridea/grape/pull/886): Group of parameters made to require an explicit type of Hash or Array - [@jrichter1](https://github.com/jrichter1). * Your contribution here. 0.10.1 (12/28/2014) diff --git a/lib/grape.rb b/lib/grape.rb index b2471d04d3..12b347ae47 100644 --- a/lib/grape.rb +++ b/lib/grape.rb @@ -55,6 +55,8 @@ module Exceptions autoload :UnknownOptions, 'grape/exceptions/unknown_options' autoload :InvalidWithOptionForRepresent, 'grape/exceptions/invalid_with_option_for_represent' autoload :IncompatibleOptionValues, 'grape/exceptions/incompatible_option_values' + autoload :MissingGroupTypeError, 'grape/exceptions/missing_group_type' + autoload :UnsupportedGroupTypeError, 'grape/exceptions/unsupported_group_type' end module ErrorFormatter diff --git a/lib/grape/dsl/parameters.rb b/lib/grape/dsl/parameters.rb index 23e9c2cf2c..4bddd7a5a3 100644 --- a/lib/grape/dsl/parameters.rb +++ b/lib/grape/dsl/parameters.rb @@ -38,6 +38,13 @@ def optional(*attrs, &block) orig_attrs = attrs.clone opts = attrs.last.is_a?(Hash) ? attrs.pop : {} + type = opts[:type] + + # check type for optional parameter group + if attrs && block_given? + fail Grape::Exceptions::MissingGroupTypeError.new if type.nil? + fail Grape::Exceptions::UnsupportedGroupTypeError.new unless [Array, Hash].include?(type) + end validate_attributes(attrs, opts, &block) diff --git a/lib/grape/exceptions/missing_group_type.rb b/lib/grape/exceptions/missing_group_type.rb new file mode 100644 index 0000000000..cb05fdccaf --- /dev/null +++ b/lib/grape/exceptions/missing_group_type.rb @@ -0,0 +1,10 @@ +# encoding: utf-8 +module Grape + module Exceptions + class MissingGroupTypeError < Base + def initialize + super(message: compose_message('missing_group_type')) + end + end + end +end diff --git a/lib/grape/exceptions/unsupported_group_type.rb b/lib/grape/exceptions/unsupported_group_type.rb new file mode 100644 index 0000000000..1a09941e3a --- /dev/null +++ b/lib/grape/exceptions/unsupported_group_type.rb @@ -0,0 +1,10 @@ +# encoding: utf-8 +module Grape + module Exceptions + class UnsupportedGroupTypeError < Base + def initialize + super(message: compose_message('unsupported_group_type')) + end + end + end +end diff --git a/lib/grape/locale/en.yml b/lib/grape/locale/en.yml index 9badb6a1be..a061ba9343 100644 --- a/lib/grape/locale/en.yml +++ b/lib/grape/locale/en.yml @@ -33,4 +33,6 @@ en: at_least_one: 'are missing, at least one parameter must be provided' exactly_one: 'are missing, exactly one parameter must be provided' all_or_none: 'provide all or none of parameters' + missing_group_type: 'group type is required' + unsupported_group_type: 'group type must be Array or Hash' diff --git a/lib/grape/validations/params_scope.rb b/lib/grape/validations/params_scope.rb index bc2e97c95d..f1eafe9acf 100644 --- a/lib/grape/validations/params_scope.rb +++ b/lib/grape/validations/params_scope.rb @@ -71,6 +71,13 @@ def validate_attributes(attrs, opts, &block) end def new_scope(attrs, optional = false, &block) + # if required params are grouped and no type or unsupported type is provided, raise an error + type = attrs[1] ? attrs[1][:type] : nil + if attrs.first && !optional + fail Grape::Exceptions::MissingGroupTypeError.new if type.nil? + fail Grape::Exceptions::UnsupportedGroupTypeError.new unless [Array, Hash].include?(type) + end + opts = attrs[1] || { type: Array } ParamsScope.new(api: @api, element: attrs.first, parent: self, optional: optional, type: opts[:type], &block) end diff --git a/spec/grape/api_spec.rb b/spec/grape/api_spec.rb index 556d8a153d..1c72be52ee 100644 --- a/spec/grape/api_spec.rb +++ b/spec/grape/api_spec.rb @@ -2166,11 +2166,11 @@ def static it 'groups nested params and prevents overwriting of params with same name in different groups' do subject.desc 'method' subject.params do - group :group1 do + group :group1, type: Array do optional :param1, desc: 'group1 param1 desc' requires :param2, desc: 'group1 param2 desc' end - group :group2 do + group :group2, type: Array do optional :param1, desc: 'group2 param1 desc' requires :param2, desc: 'group2 param2 desc' end @@ -2190,7 +2190,7 @@ def static subject.desc 'nesting' subject.params do requires :root_param, desc: 'root param' - group :nested do + group :nested, type: Array do requires :nested_param, desc: 'nested param' end end diff --git a/spec/grape/validations/params_scope_spec.rb b/spec/grape/validations/params_scope_spec.rb index a8d465b01a..c93bbf1ba1 100644 --- a/spec/grape/validations/params_scope_spec.rb +++ b/spec/grape/validations/params_scope_spec.rb @@ -102,4 +102,80 @@ def app end end end + + context 'parameters in group' do + it 'errors when no type is provided' do + expect do + subject.params do + group :a do + requires :b + end + end + end.to raise_error Grape::Exceptions::MissingGroupTypeError + + expect do + subject.params do + optional :a do + requires :b + end + end + end.to raise_error Grape::Exceptions::MissingGroupTypeError + end + + it 'allows Hash as type' do + subject.params do + group :a, type: Hash do + requires :b + end + end + subject.get('/group') { 'group works' } + get '/group', a: { b: true } + expect(last_response.status).to eq(200) + expect(last_response.body).to eq('group works') + + subject.params do + optional :a, type: Hash do + requires :b + end + end + get '/optional_type_hash' + end + + it 'allows Array as type' do + subject.params do + group :a, type: Array do + requires :b + end + end + subject.get('/group') { 'group works' } + get '/group', a: [{ b: true }] + expect(last_response.status).to eq(200) + expect(last_response.body).to eq('group works') + + subject.params do + optional :a, type: Array do + requires :b + end + end + get '/optional_type_array' + end + + it 'errors with an unsupported type' do + expect do + subject.params do + group :a, type: Set do + requires :b + end + end + end.to raise_error Grape::Exceptions::UnsupportedGroupTypeError + + expect do + subject.params do + optional :a, type: Set do + requires :b + end + end + end.to raise_error Grape::Exceptions::UnsupportedGroupTypeError + end + end end diff --git a/spec/grape/validations/validators/values_spec.rb b/spec/grape/validations/validators/values_spec.rb index cd4f05c2f8..a10fc328a9 100644 --- a/spec/grape/validations/validators/values_spec.rb +++ b/spec/grape/validations/validators/values_spec.rb @@ -71,7 +71,7 @@ class API < Grape::API end params do - optional :optional do + optional :optional, type: Array do requires :type, values: %w(a b) end end diff --git a/spec/grape/validations_spec.rb b/spec/grape/validations_spec.rb index e5cbab6666..cc6db1eda1 100644 --- a/spec/grape/validations_spec.rb +++ b/spec/grape/validations_spec.rb @@ -228,7 +228,7 @@ def define_requires_none it 'adds to declared parameters' do subject.params do - requires :items do + requires :items, type: Array do requires :key end end @@ -272,7 +272,7 @@ def define_requires_none it 'adds to declared parameters' do subject.params do - requires :items do + requires :items, type: Array do requires :key end end @@ -283,7 +283,7 @@ def define_requires_none context 'group' do before do subject.params do - group :items do + group :items, type: Array do requires :key end end @@ -306,7 +306,7 @@ def define_requires_none it 'adds to declared parameters' do subject.params do - group :items do + group :items, type: Array do requires :key end end @@ -319,7 +319,7 @@ def define_requires_none before do subject.params do - optional :items do + optional :items, type: Array do optional :key1, type: String optional :key2, type: String end @@ -395,9 +395,9 @@ def validate_param!(attr_name, params) context 'validation within arrays' do before do subject.params do - group :children do + group :children, type: Array do requires :name - group :parents do + group :parents, type: Array do requires :name end end @@ -457,7 +457,7 @@ def validate_param!(attr_name, params) context 'with block param' do before do subject.params do - requires :planets do + requires :planets, type: Array do requires :name end end @@ -469,7 +469,7 @@ def validate_param!(attr_name, params) end subject.params do - group :stars do + group :stars, type: Array do requires :name end end @@ -482,7 +482,7 @@ def validate_param!(attr_name, params) subject.params do requires :name - optional :moons do + optional :moons, type: Array do requires :name end end @@ -549,9 +549,9 @@ def validate_param!(attr_name, params) context 'validation within arrays with JSON' do before do subject.params do - group :children do + group :children, type: Array do requires :name - group :parents do + group :parents, type: Array do requires :name end end @@ -630,7 +630,7 @@ def validate_param!(attr_name, params) it 'adds to declared parameters' do subject.params do - optional :items do + optional :items, type: Array do requires :key end end @@ -692,10 +692,10 @@ def validate_param!(attr_name, params) it 'adds to declared parameters' do subject.params do - optional :items do + optional :items, type: Array do requires :key - optional(:optional_subitems) { requires :value } - requires(:required_subitems) { requires :value } + optional(:optional_subitems, type: Array) { requires :value } + requires(:required_subitems, type: Array) { requires :value } end end expect(subject.route_setting(:declared_params)).to eq([items: [:key, { optional_subitems: [:value] }, { required_subitems: [:value] }]])