Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Bug] Fix validations nested under a given #1691

Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
10 changes: 7 additions & 3 deletions lib/grape/validations/validators/all_or_none.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,16 +4,20 @@ 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
end

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
Expand Down
10 changes: 7 additions & 3 deletions lib/grape/validations/validators/at_least_one_of.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,16 +4,20 @@ 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
end

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
Expand Down
2 changes: 2 additions & 0 deletions lib/grape/validations/validators/default.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
10 changes: 7 additions & 3 deletions lib/grape/validations/validators/exactly_one_of.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down
4 changes: 4 additions & 0 deletions lib/grape/validations/validators/multiple_params_base.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
6 changes: 4 additions & 2 deletions lib/grape/validations/validators/mutual_exclusion.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,16 +6,18 @@ 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
end

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
Expand Down
54 changes: 54 additions & 0 deletions spec/grape/validations/params_scope_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down