Skip to content

Commit

Permalink
Address ruby-grape#543 - raise proper validation errors on array/hash…
Browse files Browse the repository at this point in the history
… types

 * Also adds a :type option to group/requires/optional with block,
   which can either be Array or Hash. It defaults to Array.

 * Fixes (2) and (3) as mentioned in ruby-grape#543

 * There is a quirk around query parameters: an empty array should
   pass validation if the array itself is required, but with how
   query parameters work, it doesn't. It does work fine with body
   parameters using JSON or similar, which do have a concept of an
   empty array.
  • Loading branch information
bwalex committed Jan 1, 2014
1 parent c4fc22f commit 82ea1a2
Show file tree
Hide file tree
Showing 4 changed files with 27 additions and 11 deletions.
1 change: 1 addition & 0 deletions lib/grape.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
require 'rack/auth/basic'
require 'rack/auth/digest/md5'
require 'hashie'
require 'set'
require 'active_support/core_ext/hash/indifferent_access'
require 'active_support/ordered_hash'
require 'active_support/core_ext/object/conversions'
Expand Down
30 changes: 20 additions & 10 deletions lib/grape/validations.rb
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,7 @@ def initialize(opts, &block)
@parent = opts[:parent]
@api = opts[:api]
@optional = opts[:optional] || false
@type = opts[:type]
@declared_params = []

instance_eval(&block)
Expand All @@ -99,29 +100,35 @@ def initialize(opts, &block)
end

def should_validate?(parameters)
return false if @optional && params(parameters).all?(&:blank?)
return false if @optional && params(parameters).respond_to?(:'all?') && params(parameters).all?(&:blank?)
return true if parent.nil?
parent.should_validate?(parameters)
end

def requires(*attrs, &block)
return new_scope(attrs, &block) if block_given?
orig_attrs = attrs.clone

validations = { presence: true }
validations.merge!(attrs.pop) if attrs.last.is_a?(Hash)
validations[:type] ||= Array if block_given?
validates(attrs, validations)

return new_scope(orig_attrs, &block) if block_given?

push_declared_params(attrs)
validates(attrs, validations)
end

def optional(*attrs, &block)
return new_scope(attrs, true, &block) if block_given?
orig_attrs = attrs

validations = {}
validations.merge!(attrs.pop) if attrs.last.is_a?(Hash)
validations[:type] ||= Array if block_given?
validates(attrs, validations)

return new_scope(orig_attrs, true, &block) if block_given?

push_declared_params(attrs)
validates(attrs, validations)
end

def group(element, &block)
Expand All @@ -132,9 +139,11 @@ def params(params)
params = @parent.params(params) if @parent
if @element
if params.is_a?(Array)
params = params.map { |el| el[@element] || {} }
else
params = params.map { |el| el[@element] || {} }.flatten
elsif params.is_a?(Hash)
params = params[@element] || {}
else
params = {}
end
end
params
Expand All @@ -154,8 +163,9 @@ def push_declared_params(attrs)
private

def new_scope(attrs, optional = false, &block)
raise ArgumentError unless attrs.size == 1
ParamsScope.new(api: @api, element: attrs.first, parent: self, optional: optional, &block)
opts = attrs[1] || { :type => Array }
raise ArgumentError unless opts.keys.to_set.subset? [ :type ].to_set
ParamsScope.new(api: @api, element: attrs.first, parent: self, optional: optional, type: opts[:type], &block)
end

# Pushes declared params to parent or settings
Expand Down Expand Up @@ -237,7 +247,7 @@ def reset_validations!
end

def params(&block)
ParamsScope.new(api: self, &block)
ParamsScope.new(api: self, type: Hash, &block)
end

def document_attribute(names, opts)
Expand Down
5 changes: 5 additions & 0 deletions lib/grape/validations/coerce.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ class API
module Validations
class CoerceValidator < SingleOptionValidator
def validate_param!(attr_name, params)
raise Grape::Exceptions::Validation, param: @scope.full_name(attr_name), message_key: :coerce unless params.is_a? Hash
new_value = coerce_value(@option, params[attr_name])
if valid_type?(new_value)
params[attr_name] = new_value
Expand Down Expand Up @@ -45,6 +46,10 @@ def valid_type?(val)
end

def coerce_value(type, val)
# Don't coerce things other than nil to Arrays or Hashes
return val || [] if type == Array
return val || {} if type == Hash

converter = Virtus::Attribute.build(type)
converter.coerce(val)

Expand Down
2 changes: 1 addition & 1 deletion lib/grape/validations/presence.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ def validate!(params)
end

def validate_param!(attr_name, params)
unless params.has_key?(attr_name)
unless params.respond_to? :'has_key?' and params.has_key?(attr_name)
raise Grape::Exceptions::Validation, param: @scope.full_name(attr_name), message_key: :presence
end
end
Expand Down

0 comments on commit 82ea1a2

Please sign in to comment.