diff --git a/CHANGELOG.md b/CHANGELOG.md index e6b5f22ca3..34134ddbb6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,5 @@ Next Release ============ - #### Features * Grape is no longer tested against Ruby 1.8.7. @@ -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) ================= diff --git a/README.md b/README.md index 1dafafe504..8a0a5e5214 100644 --- a/README.md +++ b/README.md @@ -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 @@ -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 @@ -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, @@ -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 diff --git a/lib/grape.rb b/lib/grape.rb index aed1046200..8731e62232 100644 --- a/lib/grape.rb +++ b/lib/grape.rb @@ -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' diff --git a/lib/grape/endpoint.rb b/lib/grape/endpoint.rb index d866e25977..8eb7f3fa88 100644 --- a/lib/grape/endpoint.rb +++ b/lib/grape/endpoint.rb @@ -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 diff --git a/lib/grape/exceptions/base.rb b/lib/grape/exceptions/base.rb index 71363d3279..044b8b00c8 100644 --- a/lib/grape/exceptions/base.rb +++ b/lib/grape/exceptions/base.rb @@ -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 = {}) diff --git a/lib/grape/exceptions/validation.rb b/lib/grape/exceptions/validation.rb index 17566a4733..2482882495 100644 --- a/lib/grape/exceptions/validation.rb +++ b/lib/grape/exceptions/validation.rb @@ -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 diff --git a/lib/grape/exceptions/validation_errors.rb b/lib/grape/exceptions/validation_errors.rb new file mode 100644 index 0000000000..8093112ce0 --- /dev/null +++ b/lib/grape/exceptions/validation_errors.rb @@ -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 diff --git a/lib/grape/exceptions/validations.rb b/lib/grape/exceptions/validations.rb deleted file mode 100644 index fb2176d6d9..0000000000 --- a/lib/grape/exceptions/validations.rb +++ /dev/null @@ -1,15 +0,0 @@ -require 'grape/exceptions/base' - -module Grape - module Exceptions - class Validations < Grape::Exceptions::Base - attr_accessor :errors - - def initialize(args = {}) - @errors = args[:errors] - errors_as_sentence = @errors.collect { |e| e.message }.join(', ') - super message: errors_as_sentence, status: 400 - end - end - end -end diff --git a/lib/grape/locale/en.yml b/lib/grape/locale/en.yml index d77b13e42a..8239b33197 100644 --- a/lib/grape/locale/en.yml +++ b/lib/grape/locale/en.yml @@ -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. ' diff --git a/lib/grape/validations/coerce.rb b/lib/grape/validations/coerce.rb index 9144da680f..e5532646e7 100644 --- a/lib/grape/validations/coerce.rb +++ b/lib/grape/validations/coerce.rb @@ -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 diff --git a/lib/grape/validations/presence.rb b/lib/grape/validations/presence.rb index 136871881c..6d40d5045b 100644 --- a/lib/grape/validations/presence.rb +++ b/lib/grape/validations/presence.rb @@ -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 diff --git a/lib/grape/validations/regexp.rb b/lib/grape/validations/regexp.rb index a80830f8e6..fe26586b97 100644 --- a/lib/grape/validations/regexp.rb +++ b/lib/grape/validations/regexp.rb @@ -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 diff --git a/spec/grape/validations/coerce_spec.rb b/spec/grape/validations/coerce_spec.rb index 1d056e8936..c052cb59a2 100644 --- a/spec/grape/validations/coerce_spec.rb +++ b/spec/grape/validations/coerce_spec.rb @@ -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! @@ -24,7 +24,7 @@ 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 } @@ -32,18 +32,18 @@ def app; subject 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 @@ -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 @@ -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 diff --git a/spec/grape/validations/presence_spec.rb b/spec/grape/validations/presence_spec.rb index 9726db9be1..b0e6362cb5 100644 --- a/spec/grape/validations/presence_spec.rb +++ b/spec/grape/validations/presence_spec.rb @@ -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}') @@ -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 @@ -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 @@ -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 diff --git a/spec/grape/validations/zh-CN.yml b/spec/grape/validations/zh-CN.yml index 1d3f0f37e7..dbcca6ad3d 100644 --- a/spec/grape/validations/zh-CN.yml +++ b/spec/grape/validations/zh-CN.yml @@ -1,9 +1,10 @@ zh-CN: grape: errors: + format: ! '%{attribute}%{message}' attributes: age: 年龄 messages: - coerce: '%{attribute}格式不正确' - presence: '请填写{attribute}' - regexp: '%{attribute}格式不正确' + coerce: '格式不正确' + presence: '请填写' + regexp: '格式不正确' diff --git a/spec/grape/validations_spec.rb b/spec/grape/validations_spec.rb index e88bcec945..8f5452d0ba 100644 --- a/spec/grape/validations_spec.rb +++ b/spec/grape/validations_spec.rb @@ -12,7 +12,7 @@ def app; subject end get '/optional', { :a_number => 'string' } last_response.status.should == 400 - last_response.body.should == 'invalid parameter: a_number' + last_response.body.should == 'a_number is invalid' get '/optional', { :a_number => 45 } last_response.status.should == 200 @@ -43,7 +43,7 @@ def app; subject end it 'errors when param not present' do get '/required' last_response.status.should == 400 - last_response.body.should == 'missing parameter: key' + last_response.body.should == 'key is missing' end it "doesn't throw a missing param when param is present" do @@ -71,7 +71,7 @@ def app; subject end it 'errors when param not present' do get '/required' last_response.status.should == 400 - last_response.body.should == 'missing parameter: items[key]' + last_response.body.should == 'items[key] is missing' end it "doesn't throw a missing param when param is present" do @@ -113,7 +113,7 @@ def app; subject end it 'errors when param not present' do get '/required' last_response.status.should == 400 - last_response.body.should == 'missing parameter: items[key]' + last_response.body.should == 'items[key] is missing' end it "doesn't throw a missing param when param is present" do @@ -157,7 +157,7 @@ def app; subject end it "errors when group is present, but required param is not" do get '/optional_group', { :items => {:NOT_key => 'foo'} } last_response.status.should == 400 - last_response.body.should == 'missing parameter: items[key]' + last_response.body.should == 'items[key] is missing' end it 'adds to declared parameters' do @@ -191,7 +191,7 @@ def app; subject end it 'does internal validations if the outer group is present' do get '/nested_optional_group', { :items => {:key => 'foo' }} last_response.status.should == 400 - last_response.body.should == 'missing parameter: items[required_subitems][value]' + last_response.body.should == 'items[required_subitems][value] is missing' get '/nested_optional_group', { :items => { :key => 'foo', :required_subitems => {:value => 'bar'}}} last_response.status.should == 200 @@ -201,7 +201,7 @@ def app; subject end it 'handles deep nesting' do get '/nested_optional_group', { :items => { :key => 'foo', :required_subitems => {:value => 'bar'}, :optional_subitems => {:NOT_value => 'baz'}}} last_response.status.should == 400 - last_response.body.should == 'missing parameter: items[optional_subitems][value]' + last_response.body.should == 'items[optional_subitems][value] is missing' get '/nested_optional_group', { :items => { :key => 'foo', :required_subitems => {:value => 'bar'}, :optional_subitems => {:value => 'baz'}}} last_response.status.should == 200 @@ -229,8 +229,8 @@ def app; subject end it 'throws the validation errors' do get '/two_required' last_response.status.should == 400 - last_response.body.should =~ /missing parameter: yolo/ - last_response.body.should =~ /missing parameter: swag/ + last_response.body.should =~ /yolo is missing/ + last_response.body.should =~ /swag is missing/ end end @@ -239,7 +239,7 @@ module CustomValidations class Customvalidator < Grape::Validations::Validator def validate_param!(attr_name, params) unless params[attr_name] == 'im custom' - raise Grape::Exceptions::Validation, :status => 400, :message => "#{attr_name}: is not custom!" + raise Grape::Exceptions::Validation, :param => @scope.full_name(attr_name), :message => "is not custom!" end end end @@ -258,7 +258,7 @@ def validate_param!(attr_name, params) get '/optional_custom', { :custom => 'im wrong' } last_response.status.should == 400 - last_response.body.should == 'custom: is not custom!' + last_response.body.should == 'custom is not custom!' end it "skips validation when parameter isn't present" do @@ -272,7 +272,7 @@ def validate_param!(attr_name, params) get '/optional_custom', { :custom => 123 } last_response.status.should == 400 - last_response.body.should == 'custom: is not custom!' + last_response.body.should == 'custom is not custom!' end end @@ -285,7 +285,7 @@ def validate_param!(attr_name, params) it 'validates when param is present' do get '/required_custom', { :custom => 'im wrong, validate me' } last_response.status.should == 400 - last_response.body.should == 'custom: is not custom!' + last_response.body.should == 'custom is not custom!' get '/required_custom', { :custom => 'im custom' } last_response.status.should == 200 @@ -295,7 +295,7 @@ def validate_param!(attr_name, params) it 'validates when param is not present' do get '/required_custom' last_response.status.should == 400 - last_response.body.should == 'missing parameter: custom, custom: is not custom!' + last_response.body.should == 'custom is missing, custom is not custom!' end context 'nested namespaces' do @@ -327,13 +327,13 @@ def validate_param!(attr_name, params) specify 'the parent namespace uses the validator' do get '/nested/one', { :custom => 'im wrong, validate me'} last_response.status.should == 400 - last_response.body.should == 'custom: is not custom!' + last_response.body.should == 'custom is not custom!' end specify 'the nested namesapce inherits the custom validator' do get '/nested/nested/two', { :custom => 'im wrong, validate me'} last_response.status.should == 400 - last_response.body.should == 'custom: is not custom!' + last_response.body.should == 'custom is not custom!' end specify 'peer namesapces does not have the validator' do