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

Improve collection of validation errors #462

Merged
merged 4 commits into from
Aug 20, 2013
Merged
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
4 changes: 1 addition & 3 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
Next Release
============

#### Features

* Grape is no longer tested against Ruby 1.8.7.
Expand All @@ -11,14 +10,13 @@ Next Release
* [#450](https://github.com/intridea/grape/pull/450): Added option to pass an exception handler lambda as an argument to `rescue_from` - [@robertopedroso](https://github.com/robertopedroso).
* [#443](https://github.com/intridea/grape/pull/443): Let `requires` and `optional` take blocks that initialize new scopes - [@asross](https://github.com/asross).
* [#452](https://github.com/intridea/grape/pull/452): Added `with` as a hash option to specify handlers for `rescue_from` and `error_formatter` [@robertopedroso](https://github.com/robertopedroso).
* [#433](https://github.com/intridea/grape/issues/433): API change: validation errors are now collected and a `Grape::Exceptions::Validations` is raised; the individual validation errors can be accessed via `Grape::Exceptions::Validations#errors` - [@stevschmid](https://github.com/stevschmid).
* [#433](https://github.com/intridea/grape/issues/433), [#462](https://github.com/intridea/grape/issues/462): API change: validation errors are now collected and `Grape::Exceptions::ValidationErrors` is raised - [@stevschmid](https://github.com/stevschmid).
* Your contribution here.

#### Fixes

* [#428](https://github.com/intridea/grape/issues/428): Removes memoization from `Grape::Request` params to prevent middleware from freezing parameter values before `Formatter` can get them - [@mbleigh](https://github.com/mbleigh).


0.5.0 (6/14/2013)
=================

Expand Down
12 changes: 7 additions & 5 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -388,7 +388,7 @@ The `namespace` method has a number of aliases, including: `group`, `resource`,
class AlphaNumeric < Grape::Validations::Validator
def validate_param!(attr_name, params)
unless params[attr_name] =~ /^[[:alnum:]]+$/
throw :error, status: 400, message: "#{attr_name}: must consist of alpha-numeric characters"
raise Grape::Exceptions::Validation, param: @scope.full_name(attr_name), message: "must consist of alpha-numeric characters"
end
end
end
Expand All @@ -406,7 +406,7 @@ You can also create custom classes that take parameters.
class Length < Grape::Validations::SingleOptionValidator
def validate_param!(attr_name, params)
unless params[attr_name].length <= @option
throw :error, status: 400, message: "#{attr_name}: must be at the most #{@option} characters long"
raise Grape::Exceptions::Validation, param: @scope.full_name(attr_name), message: "must be at the most #{@option} characters long"
end
end
end
Expand All @@ -420,12 +420,12 @@ end

### Validation Errors

Validation and coercion errors are collected and an exception of type `Grape::Exceptions::Validations` is raised.
Validation and coercion errors are collected and an exception of type `Grape::Exceptions::ValidationErrors` is raised.
If the exception goes uncaught it will respond with a status of 400 and an error message.
You can rescue a `Grape::Exceptions::Validations` and respond with a custom response.
You can rescue a `Grape::Exceptions::ValidationErrors` and respond with a custom response.

```ruby
rescue_from Grape::Exceptions::Validations do |e|
rescue_from Grape::Exceptions::ValidationErrors do |e|
Rack::Response.new({
'status' => e.status,
'message' => e.message,
Expand All @@ -434,6 +434,8 @@ rescue_from Grape::Exceptions::Validations do |e|
end
```

The validation errors are grouped by parameter name and can be accessed via ``Grape::Exceptions::ValidationErrors#errors``.

### I18n

Grape supports I18n for parameter-related error messages, but will fallback to English if
Expand Down
2 changes: 1 addition & 1 deletion lib/grape.rb
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ module Grape
module Exceptions
autoload :Base, 'grape/exceptions/base'
autoload :Validation, 'grape/exceptions/validation'
autoload :Validations, 'grape/exceptions/validations'
autoload :ValidationErrors, 'grape/exceptions/validation_errors'
autoload :MissingVendorOption, 'grape/exceptions/missing_vendor_option'
autoload :MissingMimeType, 'grape/exceptions/missing_mime_type'
autoload :MissingOption, 'grape/exceptions/missing_option'
Expand Down
2 changes: 1 addition & 1 deletion lib/grape/endpoint.rb
Original file line number Diff line number Diff line change
Expand Up @@ -401,7 +401,7 @@ def run(env)
end

if validation_errors.any?
raise Grape::Exceptions::Validations, errors: validation_errors
raise Grape::Exceptions::ValidationErrors, errors: validation_errors
end

run_filters after_validations
Expand Down
2 changes: 1 addition & 1 deletion lib/grape/exceptions/base.rb
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ def translate_attribute(key, options = {})
end

def translate_message(key, options = {})
translate("#{BASE_MESSAGES_KEY}.#{key}", {:default => '' }.merge(options))
translate("#{BASE_MESSAGES_KEY}.#{key}", { :default => '' }.merge(options))
end

def translate(key, options = {})
Expand Down
16 changes: 13 additions & 3 deletions lib/grape/exceptions/validation.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,21 @@ class Validation < Grape::Exceptions::Base
attr_accessor :param

def initialize(args = {})
@param = args[:param].to_s if args.has_key? :param
attribute = translate_attribute(@param)
args[:message] = translate_message(args[:message_key], :attribute => attribute) if args.has_key? :message_key
raise "Param is missing:" unless args.has_key? :param
@param = args[:param]
args[:message] = translate_message(args[:message_key]) if args.has_key? :message_key
super
end

# remove all the unnecessary stuff from Grape::Exceptions::Base like status
# and headers when converting a validation error to json or string
def as_json(*a)
self.to_s
end

def to_s
message
end
end
end
end
42 changes: 42 additions & 0 deletions lib/grape/exceptions/validation_errors.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
require 'grape/exceptions/base'

module Grape
module Exceptions
class ValidationErrors < Grape::Exceptions::Base
include Enumerable

attr_reader :errors

def initialize(args = {})
@errors = {}
args[:errors].each do |validation_error|
@errors[validation_error.param] ||= []
@errors[validation_error.param] << validation_error
end
super message: full_messages.join(', '), status: 400
end

def each
errors.each_pair do |attribute, errors|
errors.each do |error|
yield attribute, error
end
end
end

private

def full_messages
map { |attribute, error| full_message(attribute, error) }
end

def full_message(attribute, error)
I18n.t(:"grape.errors.format", {
default: "%{attribute} %{message}",
attribute: translate_attribute(attribute),
message: error.message
})
end
end
end
end
15 changes: 0 additions & 15 deletions lib/grape/exceptions/validations.rb

This file was deleted.

7 changes: 4 additions & 3 deletions lib/grape/locale/en.yml
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
en:
grape:
errors:
format: ! '%{attribute} %{message}'
messages:
coerce: 'invalid parameter: %{attribute}'
presence: 'missing parameter: %{attribute}'
regexp: 'invalid parameter: %{attribute}'
coerce: 'is invalid'
presence: 'is missing'
regexp: 'is invalid'
missing_vendor_option:
problem: 'missing :vendor option.'
summary: 'when version using header, you must specify :vendor option. '
Expand Down
3 changes: 1 addition & 2 deletions lib/grape/validations/coerce.rb
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,7 @@ def validate_param!(attr_name, params)
if valid_type?(new_value)
params[attr_name] = new_value
else
raise Grape::Exceptions::Validation, :status => 400,
:param => @scope.full_name(attr_name), :message_key => :coerce
raise Grape::Exceptions::Validation, :param => @scope.full_name(attr_name), :message_key => :coerce
end
end

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

def validate_param!(attr_name, params)
unless params.has_key?(attr_name)
raise Grape::Exceptions::Validation, :status => 400,
:param => @scope.full_name(attr_name), :message_key => :presence
raise Grape::Exceptions::Validation, :param => @scope.full_name(attr_name), :message_key => :presence
end
end
end
Expand Down
3 changes: 1 addition & 2 deletions lib/grape/validations/regexp.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,7 @@ module Validations
class RegexpValidator < SingleOptionValidator
def validate_param!(attr_name, params)
if params[attr_name] && !( params[attr_name].to_s =~ @option )
raise Grape::Exceptions::Validation, :status => 400,
:param => @scope.full_name(attr_name), :message_key => :regexp
raise Grape::Exceptions::Validation, :param => @scope.full_name(attr_name), :message_key => :regexp
end
end
end
Expand Down
18 changes: 9 additions & 9 deletions spec/grape/validations/coerce_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,13 @@
def app; subject end

describe 'coerce' do

context "i18n" do

after :each do
I18n.locale = :en
end

it "i18n error on malformed input" do
I18n.load_path << File.expand_path('../zh-CN.yml',__FILE__)
I18n.reload!
Expand All @@ -24,26 +24,26 @@ def app; subject end
last_response.status.should == 400
last_response.body.should == '年龄格式不正确'
end

it 'gives an english fallback error when default locale message is blank' do
I18n.locale = :'pt-BR'
subject.params { requires :age, :type => Integer }
subject.get '/single' do 'int works'; end

get '/single', :age => '43a'
last_response.status.should == 400
last_response.body.should == 'invalid parameter: age'
last_response.body.should == 'age is invalid'
end

end

it 'error on malformed input' do
subject.params { requires :int, :type => Integer }
subject.get '/single' do 'int works'; end

get '/single', :int => '43a'
last_response.status.should == 400
last_response.body.should == 'invalid parameter: int'
last_response.body.should == 'int is invalid'

get '/single', :int => '43'
last_response.status.should == 200
Expand All @@ -56,7 +56,7 @@ def app; subject end

get 'array', { :ids => ['1', '2', 'az'] }
last_response.status.should == 400
last_response.body.should == 'invalid parameter: ids'
last_response.body.should == 'ids is invalid'

get 'array', { :ids => ['1', '2', '890'] }
last_response.status.should == 200
Expand All @@ -78,7 +78,7 @@ class User

get '/user', :user => "32"
last_response.status.should == 400
last_response.body.should == 'invalid parameter: user'
last_response.body.should == 'user is invalid'

get '/user', :user => { :id => 32, :name => 'Bob' }
last_response.status.should == 200
Expand Down
24 changes: 12 additions & 12 deletions spec/grape/validations/presence_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -66,13 +66,13 @@ def app
it 'validates id' do
post '/'
last_response.status.should == 400
last_response.body.should == '{"error":"missing parameter: id"}'
last_response.body.should == '{"error":"id is missing"}'

io = StringIO.new('{"id" : "a56b"}')
post '/', {}, 'rack.input' => io,
'CONTENT_TYPE' => 'application/json',
'CONTENT_LENGTH' => io.length
last_response.body.should == '{"error":"invalid parameter: id"}'
last_response.body.should == '{"error":"id is invalid"}'
last_response.status.should == 400

io = StringIO.new('{"id" : 56}')
Expand All @@ -86,11 +86,11 @@ def app
it 'validates name, company' do
get('/')
last_response.status.should == 400
last_response.body.should == '{"error":"missing parameter: name"}'
last_response.body.should == '{"error":"name is missing"}'

get('/', :name => "Bob")
last_response.status.should == 400
last_response.body.should == '{"error":"missing parameter: company"}'
last_response.body.should == '{"error":"company is missing"}'

get('/', :name => "Bob", :company => "TestCorp")
last_response.status.should == 200
Expand All @@ -100,11 +100,11 @@ def app
it 'validates nested parameters' do
get('/nested')
last_response.status.should == 400
last_response.body.should == '{"error":"missing parameter: user[first_name]"}'
last_response.body.should == '{"error":"user[first_name] is missing"}'

get('/nested', :user => {:first_name => "Billy"})
last_response.status.should == 400
last_response.body.should == '{"error":"missing parameter: user[last_name]"}'
last_response.body.should == '{"error":"user[last_name] is missing"}'

get('/nested', :user => {:first_name => "Billy", :last_name => "Bob"})
last_response.status.should == 200
Expand All @@ -114,27 +114,27 @@ def app
it 'validates triple nested parameters' do
get('/nested_triple')
last_response.status.should == 400
last_response.body.should == '{"error":"missing parameter: admin[admin_name], missing parameter: admin[super][user][first_name]"}'
last_response.body.should == '{"error":"admin[admin_name] is missing, admin[super][user][first_name] is missing"}'

get('/nested_triple', :user => {:first_name => "Billy"})
last_response.status.should == 400
last_response.body.should == '{"error":"missing parameter: admin[admin_name], missing parameter: admin[super][user][first_name]"}'
last_response.body.should == '{"error":"admin[admin_name] is missing, admin[super][user][first_name] is missing"}'

get('/nested_triple', :admin => {:super => {:first_name => "Billy"}})
last_response.status.should == 400
last_response.body.should == '{"error":"missing parameter: admin[admin_name], missing parameter: admin[super][user][first_name]"}'
last_response.body.should == '{"error":"admin[admin_name] is missing, admin[super][user][first_name] is missing"}'

get('/nested_triple', :super => {:user => {:first_name => "Billy", :last_name => "Bob"}})
last_response.status.should == 400
last_response.body.should == '{"error":"missing parameter: admin[admin_name], missing parameter: admin[super][user][first_name]"}'
last_response.body.should == '{"error":"admin[admin_name] is missing, admin[super][user][first_name] is missing"}'

get('/nested_triple', :admin => {:super => {:user => {:first_name => "Billy"}}})
last_response.status.should == 400
last_response.body.should == '{"error":"missing parameter: admin[admin_name], missing parameter: admin[super][user][last_name]"}'
last_response.body.should == '{"error":"admin[admin_name] is missing, admin[super][user][last_name] is missing"}'

get('/nested_triple', :admin => { :admin_name => 'admin', :super => {:user => {:first_name => "Billy"}}})
last_response.status.should == 400
last_response.body.should == '{"error":"missing parameter: admin[super][user][last_name]"}'
last_response.body.should == '{"error":"admin[super][user][last_name] is missing"}'

get('/nested_triple', :admin => { :admin_name => 'admin', :super => {:user => {:first_name => "Billy", :last_name => "Bob"}}})
last_response.status.should == 200
Expand Down
7 changes: 4 additions & 3 deletions spec/grape/validations/zh-CN.yml
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
zh-CN:
grape:
errors:
format: ! '%{attribute}%{message}'
attributes:
age: 年龄
messages:
coerce: '%{attribute}格式不正确'
presence: '请填写{attribute}'
regexp: '%{attribute}格式不正确'
coerce: '格式不正确'
presence: '请填写'
regexp: '格式不正确'
Loading