diff --git a/CHANGELOG.md b/CHANGELOG.md index 303c4c0b31..2d6afb97bc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,7 @@ #### Fixes +* [#1691](https://github.com/ruby-grape/grape/pull/1691): Fix validations nested under `given` - [@rnubel](https://github.com/rnubel). * Your contribution here. ### 1.0.1 (9/8/2017) diff --git a/lib/grape/validations/validators/all_or_none.rb b/lib/grape/validations/validators/all_or_none.rb index 02a06851de..0bc6d66631 100644 --- a/lib/grape/validations/validators/all_or_none.rb +++ b/lib/grape/validations/validators/all_or_none.rb @@ -4,7 +4,7 @@ module Validations class AllOrNoneOfValidator < MultipleParamsBase def validate!(params) super - if scope_requires_params && only_subset_present + if scope_requires_params && only_subset_present(params) raise Grape::Exceptions::Validation, params: all_keys, message: message(:all_or_none) end params @@ -12,8 +12,12 @@ def validate!(params) private - def only_subset_present - scoped_params.any? { |resource_params| !keys_in_common(resource_params).empty? && keys_in_common(resource_params).length < attrs.length } + def only_subset_present(params) + scoped_params.any? do |resource_params| + next unless scope_should_validate?(resource_params, params) + + !keys_in_common(resource_params).empty? && keys_in_common(resource_params).length < attrs.length + end end end end diff --git a/lib/grape/validations/validators/at_least_one_of.rb b/lib/grape/validations/validators/at_least_one_of.rb index 077393b639..b8a4d1c1e8 100644 --- a/lib/grape/validations/validators/at_least_one_of.rb +++ b/lib/grape/validations/validators/at_least_one_of.rb @@ -4,7 +4,7 @@ module Validations class AtLeastOneOfValidator < MultipleParamsBase def validate!(params) super - if scope_requires_params && no_exclusive_params_are_present + if scope_requires_params && no_exclusive_params_are_present(params) raise Grape::Exceptions::Validation, params: all_keys, message: message(:at_least_one) end params @@ -12,8 +12,12 @@ def validate!(params) private - def no_exclusive_params_are_present - scoped_params.any? { |resource_params| keys_in_common(resource_params).empty? } + def no_exclusive_params_are_present(params) + scoped_params.any? do |resource_params| + next unless scope_should_validate?(resource_params, params) + + keys_in_common(resource_params).empty? + end end end end diff --git a/lib/grape/validations/validators/default.rb b/lib/grape/validations/validators/default.rb index c472cf31b9..799ea59d22 100644 --- a/lib/grape/validations/validators/default.rb +++ b/lib/grape/validations/validators/default.rb @@ -20,6 +20,8 @@ def validate_param!(attr_name, params) def validate!(params) attrs = AttributesIterator.new(self, @scope, params) attrs.each do |resource_params, attr_name| + next unless @scope.meets_dependency?(resource_params, params) + if resource_params.is_a?(Hash) && resource_params[attr_name].nil? validate_param!(attr_name, resource_params) end diff --git a/lib/grape/validations/validators/exactly_one_of.rb b/lib/grape/validations/validators/exactly_one_of.rb index 07283c783c..650d38bfba 100644 --- a/lib/grape/validations/validators/exactly_one_of.rb +++ b/lib/grape/validations/validators/exactly_one_of.rb @@ -4,7 +4,7 @@ module Validations class ExactlyOneOfValidator < MutualExclusionValidator def validate!(params) super - if scope_requires_params && none_of_restricted_params_is_present + if scope_requires_params && none_of_restricted_params_is_present(params) raise Grape::Exceptions::Validation, params: all_keys, message: message(:exactly_one) end params @@ -21,8 +21,12 @@ def message(default_key = nil) private - def none_of_restricted_params_is_present - scoped_params.any? { |resource_params| keys_in_common(resource_params).empty? } + def none_of_restricted_params_is_present(params) + scoped_params.any? do |resource_params| + next unless scope_should_validate?(resource_params, params) + + keys_in_common(resource_params).empty? + end end end end diff --git a/lib/grape/validations/validators/multiple_params_base.rb b/lib/grape/validations/validators/multiple_params_base.rb index 14209dff8c..ff0515caba 100644 --- a/lib/grape/validations/validators/multiple_params_base.rb +++ b/lib/grape/validations/validators/multiple_params_base.rb @@ -10,6 +10,10 @@ def validate!(params) private + def scope_should_validate?(scoped_params, params) + @scope.meets_dependency?(scoped_params, params) + end + def scope_requires_params @scope.required? || scoped_params.any?(&:any?) end diff --git a/lib/grape/validations/validators/mutual_exclusion.rb b/lib/grape/validations/validators/mutual_exclusion.rb index d46d563516..8cff367fe9 100644 --- a/lib/grape/validations/validators/mutual_exclusion.rb +++ b/lib/grape/validations/validators/mutual_exclusion.rb @@ -6,7 +6,7 @@ class MutualExclusionValidator < MultipleParamsBase def validate!(params) super - if two_or_more_exclusive_params_are_present + if two_or_more_exclusive_params_are_present(params) raise Grape::Exceptions::Validation, params: processing_keys_in_common, message: message(:mutual_exclusion) end params @@ -14,8 +14,10 @@ def validate!(params) private - def two_or_more_exclusive_params_are_present + def two_or_more_exclusive_params_are_present(params) scoped_params.any? do |resource_params| + next unless scope_should_validate?(resource_params, params) + @processing_keys_in_common = keys_in_common(resource_params) @processing_keys_in_common.length > 1 end diff --git a/spec/grape/validations/params_scope_spec.rb b/spec/grape/validations/params_scope_spec.rb index e668fd9ecc..176dc54431 100644 --- a/spec/grape/validations/params_scope_spec.rb +++ b/spec/grape/validations/params_scope_spec.rb @@ -407,6 +407,60 @@ def initialize(value) expect(last_response.status).to eq(200) end + it 'sets default values only if the dependency is met' do + subject.params do + optional :a + given :a do + optional :b, default: 'default value' + end + end + subject.get('/defaults') { params.to_json } + + get '/defaults' + expect(last_response.status).to eq(200) + expect(last_response.body).to eq('{}') + + get '/defaults', a: true + expect(last_response.status).to eq(200) + expect(JSON.parse(last_response.body)).to eq('a' => 'true', 'b' => 'default value') + end + + it 'conditionally applies all multi-param validators' do + subject.params do + optional :a + given :a do + optional :b, :c, :d, :e, :f, :g + + exactly_one_of :b, :c + all_or_none_of :d, :e + at_least_one_of :f, :g + end + end + subject.get('/exactly_one') { params.to_json } + + get '/exactly_one' + expect(last_response.status).to eq(200) + expect(last_response.body).to eq('{}') + + get '/exactly_one', a: true, b: true, f: true + expect(last_response.status).to eq(200) + + get '/exactly_one', a: true, c: true, f: true + expect(last_response.status).to eq(200) + + get '/exactly_one', a: true, b: true, c: true, f: true + expect(last_response.status).to eq(400) + + get '/exactly_one', d: true + expect(last_response.status).to eq(200) + + get '/exactly_one', a: true, b: true, d: true, e: true, f: true + expect(last_response.status).to eq(200) + + get '/exactly_one', a: true, b: true, f: true, g: true + expect(last_response.status).to eq(200) + end + it 'applies only the appropriate validation' do subject.params do optional :a