Merge pull request #687 from dblock/consistently-label-errors

Fix: mutually_exclusive and exactly_one_of validation error messages now label parameters as strings, consistently with requires and optional.

CHANGELOG.md

@@ -1,6 +1,7 @@
 0.8.1 (Next)
 ============
 
+* [#687](https://github.com/intridea/grape/pull/687): Fix: `mutually_exclusive` and `exactly_one_of` validation error messages now label parameters as strings, consistently with `requires` and `optional` - [@dblock](https://github.com/dblock).
 * Your contribution here.
 
 0.8.0 (7/10/2014)
@@ -9,8 +10,8 @@
 #### Features
 
 * [#639](https://github.com/intridea/grape/pull/639): Added support for blocks with reusable params - [@mibon](https://github.com/mibon).
-* [#637](https://github.com/intridea/grape/pull/637): Added 'exactly_one_of' validation - [@Morred](https://github.com/Morred).
-* [#626](https://github.com/intridea/grape/pull/626): Mutually exclusive params - [@oliverbarnes](https://github.com/oliverbarnes).
+* [#637](https://github.com/intridea/grape/pull/637): Added support for `exactly_one_of` parameter validation - [@Morred](https://github.com/Morred).
+* [#626](https://github.com/intridea/grape/pull/626): Added support for `mutually_exclusive` parameters - [@oliverbarnes](https://github.com/oliverbarnes).
 * [#617](https://github.com/intridea/grape/pull/617): Running tests on Ruby 2.1.1, Rubinius 2.1 and 2.2, Ruby and JRuby HEAD - [@dblock](https://github.com/dblock).
 * [#397](https://github.com/intridea/grape/pull/397): Adds `Grape::Endpoint.before_each` to allow easy helper stubbing - [@mbleigh](https://github.com/mbleigh).
 * [#673](https://github.com/intridea/grape/pull/673): Avoid requiring non-existent fields when using Grape::Entity documentation - [@qqshfox](https://github.com/qqshfox).

README.md

@@ -445,7 +445,7 @@ params do
 end
 ```
 
-The :values option can also be supplied with a `Proc` to be evalutated at runtime. For example, given a status
+The `:values` option can also be supplied with a `Proc` to be evalutated at runtime. For example, given a status
 model you may want to restrict by hashtags that you have previously defined in the `HashTag` model.
 
 ```ruby
@@ -589,21 +589,28 @@ end
 
 ### Validation Errors
 
-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::ValidationErrors` and respond with a custom response.
+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. The validation errors are grouped by parameter name and can be accessed via `Grape::Exceptions::ValidationErrors#errors`.
+
+
+The default response from a `Grape::Exceptions::ValidationErrors` is a humanly readable string, such as "beer, wine are mutually exclusive", in the following example.
 
 ```ruby
-rescue_from Grape::Exceptions::ValidationErrors do |e|
-    Rack::Response.new({
-      status: e.status,
-      message: e.message,
-      errors: e.errors
-    }.to_json, e.status)
+params do
+  optional :beer
+  optional :wine
+  optional :juice
+  exactly_one_of :beer, :wine, :juice
 end
 ```
 
-The validation errors are grouped by parameter name and can be accessed via ``Grape::Exceptions::ValidationErrors#errors``.
+You can rescue a `Grape::Exceptions::ValidationErrors` and respond with a custom response or turn the response into well-formatted JSON for a JSON API that separates individual parameters and the corresponding error messages. The following `rescue_from` example produces `[{"params":["beer","wine"],"messages":["are mutually exclusive"]}]`.
+
+```ruby
+format :json
+subject.rescue_from Grape::Exceptions::ValidationErrors do |e|
+  rack_response e.to_json, 400
+end
+```
 
 ### I18n
 

lib/grape/exceptions/base.rb

@@ -49,6 +49,12 @@ def resolution(key, attributes)
         translate_message("#{key}.resolution", attributes)
       end
 
+      def translate_attributes(keys, options = {})
+        keys.map do |key|
+          translate("#{BASE_ATTRIBUTES_KEY}.#{key}", { default: key }.merge(options))
+        end.join(", ")
+      end
+
       def translate_attribute(key, options = {})
         translate("#{BASE_ATTRIBUTES_KEY}.#{key}", { default: key }.merge(options))
       end

lib/grape/exceptions/validation.rb

@@ -3,11 +3,11 @@
 module Grape
   module Exceptions
     class Validation < Grape::Exceptions::Base
-      attr_accessor :param
+      attr_accessor :params
 
       def initialize(args = {})
-        raise "Param is missing:" unless args.key? :param
-        @param = args[:param]
+        raise "Params are missing:" unless args.key? :params
+        @params = args[:params]
         args[:message] = translate_message(args[:message_key]) if args.key? :message_key
         super
       end

lib/grape/exceptions/validation_errors.rb

@@ -10,8 +10,8 @@ class ValidationErrors < Grape::Exceptions::Base
       def initialize(args = {})
         @errors = {}
         args[:errors].each do |validation_error|
-          @errors[validation_error.param] ||= []
-          @errors[validation_error.param] << validation_error
+          @errors[validation_error.params] ||= []
+          @errors[validation_error.params] << validation_error
         end
         super message: full_messages.join(', '), status: 400
       end
@@ -24,17 +24,30 @@ def each
         end
       end
 
+      def as_json
+        errors.map do |k, v|
+          {
+            params: k,
+            messages: v.map(&:to_s)
+          }
+        end
+      end
+
+      def to_json
+        as_json.to_json
+      end
+
       private
 
       def full_messages
-        map { |attribute, error| full_message(attribute, error) }.uniq
+        map { |attributes, error| full_message(attributes, error) }.uniq
       end
 
-      def full_message(attribute, error)
+      def full_message(attributes, error)
         I18n.t(
           "grape.errors.format".to_sym,
-          default: "%{attribute} %{message}",
-          attribute: translate_attribute(attribute),
+          default: "%{attributes} %{message}",
+          attributes: attributes.count > 1 ? translate_attributes(attributes) : translate_attribute(attributes.first),
           message: error.message
         )
       end

lib/grape/locale/en.yml

@@ -1,7 +1,7 @@
 en:
   grape:
     errors:
-      format: ! '%{attribute} %{message}'
+      format: ! '%{attributes} %{message}'
       messages:
         coerce: 'is invalid'
         presence: 'is missing'
@@ -10,7 +10,7 @@ en:
         missing_vendor_option:
           problem: 'missing :vendor option.'
           summary: 'when version using header, you must specify :vendor option. '
-          resolution: "eg: version 'v1', :using => :header, :vendor => 'twitter'"
+          resolution: "eg: version 'v1', using: :header, vendor: 'twitter'"
         missing_mime_type:
           problem: 'missing mime type for %{new_format}'
           resolution:
@@ -29,4 +29,4 @@ en:
         unknown_options: 'unknown options: %{options}'
         incompatible_option_values: '%{option1}: %{value1} is incompatible with %{option2}: %{value2}'
         mutual_exclusion: 'are mutually exclusive'
-        exactly_one: "- exactly one parameter must be provided"
+        exactly_one: 'are missing, exactly one parameter must be provided'

lib/grape/validations/coerce.rb

@@ -6,12 +6,12 @@ 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
+        raise Grape::Exceptions::Validation, params: [@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
         else
-          raise Grape::Exceptions::Validation, param: @scope.full_name(attr_name), message_key: :coerce
+          raise Grape::Exceptions::Validation, params: [@scope.full_name(attr_name)], message_key: :coerce
         end
       end
 

lib/grape/validations/exactly_one_of.rb

@@ -7,7 +7,7 @@ class ExactlyOneOfValidator < MutualExclusionValidator
       def validate!(params)
         super
         if none_of_restricted_params_is_present
-          raise Grape::Exceptions::Validation, param: "#{all_keys}", message_key: :exactly_one
+          raise Grape::Exceptions::Validation, params: all_keys, message_key: :exactly_one
         end
         params
       end
@@ -19,7 +19,7 @@ def none_of_restricted_params_is_present
       end
 
       def all_keys
-        attrs.map(&:to_sym)
+        attrs.map(&:to_s)
       end
     end
   end

lib/grape/validations/mutual_exclusion.rb

@@ -6,7 +6,7 @@ class MutualExclusionValidator < Validator
       def validate!(params)
         @params = params
         if two_or_more_exclusive_params_are_present
-          raise Grape::Exceptions::Validation, param: "#{keys_in_common.map(&:to_sym)}", message_key: :mutual_exclusion
+          raise Grape::Exceptions::Validation, params: keys_in_common, message_key: :mutual_exclusion
         end
         params
       end
@@ -18,7 +18,7 @@ def two_or_more_exclusive_params_are_present
       end
 
       def keys_in_common
-        attrs.map(&:to_s) & params.stringify_keys.keys
+        (attrs.map(&:to_s) & params.stringify_keys.keys).map(&:to_s)
       end
     end
   end

lib/grape/validations/presence.rb

@@ -8,7 +8,7 @@ def validate!(params)
 
       def validate_param!(attr_name, params)
         unless params.respond_to?(:key?) && params.key?(attr_name)
-          raise Grape::Exceptions::Validation, param: @scope.full_name(attr_name), message_key: :presence
+          raise Grape::Exceptions::Validation, params: [@scope.full_name(attr_name)], message_key: :presence
         end
       end
     end

lib/grape/validations/regexp.rb

@@ -4,7 +4,7 @@ class RegexpValidator < SingleOptionValidator
       def validate_param!(attr_name, params)
         if params.key?(attr_name) &&
           (params[attr_name].nil? || !(params[attr_name].to_s =~ @option))
-          raise Grape::Exceptions::Validation, param: @scope.full_name(attr_name), message_key: :regexp
+          raise Grape::Exceptions::Validation, params: [@scope.full_name(attr_name)], message_key: :regexp
         end
       end
     end

lib/grape/validations/values.rb

@@ -9,7 +9,7 @@ def initialize(attrs, options, required, scope)
 
       def validate_param!(attr_name, params)
         if (params[attr_name] || required_for_root_scope?) && !(@values.is_a?(Proc) ? @values.call : @values).include?(params[attr_name])
-          raise Grape::Exceptions::Validation, param: @scope.full_name(attr_name), message_key: :values
+          raise Grape::Exceptions::Validation, params: [@scope.full_name(attr_name)], message_key: :values
         end
       end
 

spec/grape/exceptions/validation_errors_spec.rb

@@ -3,7 +3,7 @@
 
 describe Grape::Exceptions::ValidationErrors do
   let(:validation_message) { "FooBar is invalid" }
-  let(:validation_error) { OpenStruct.new(param: validation_message) }
+  let(:validation_error) { OpenStruct.new(params: [validation_message]) }
 
   context "message" do
     context "is not repeated" do

spec/grape/validations/zh-CN.yml

@@ -1,7 +1,7 @@
 zh-CN:
   grape:
     errors:
-      format: ! '%{attribute}%{message}'
+      format: ! '%{attributes}%{message}'
       attributes:
         age: 年龄
       messages:

spec/grape/validations_spec.rb

@@ -659,7 +659,7 @@ module CustomValidations
         class Customvalidator < Grape::Validations::Validator
           def validate_param!(attr_name, params)
             unless params[attr_name] == 'im custom'
-              raise Grape::Exceptions::Validation, param: @scope.full_name(attr_name), message: "is not custom!"
+              raise Grape::Exceptions::Validation, params: [@scope.full_name(attr_name)], message: "is not custom!"
             end
           end
         end
@@ -929,7 +929,7 @@ module SharedParams
 
           get '/mutually_exclusive', beer: 'string', wine: 'anotherstring'
           expect(last_response.status).to eq(400)
-          expect(last_response.body).to eq("[:beer, :wine] are mutually exclusive")
+          expect(last_response.body).to eq "beer, wine are mutually exclusive"
         end
       end
 
@@ -949,8 +949,7 @@ module SharedParams
 
           get '/mutually_exclusive', beer: 'true', wine: 'true', scotch: 'true', aquavit: 'true'
           expect(last_response.status).to eq(400)
-          expect(last_response.body).to match(/\[:beer, :wine\] are mutually exclusive/)
-          expect(last_response.body).to match(/\[:scotch, :aquavit\] are mutually exclusive/)
+          expect(last_response.body).to eq "beer, wine are mutually exclusive, scotch, aquavit are mutually exclusive"
         end
       end
     end
@@ -970,7 +969,30 @@ module SharedParams
 
           get '/exactly_one_of', beer: 'string', wine: 'anotherstring'
           expect(last_response.status).to eq(400)
-          expect(last_response.body).to eq("[:beer, :wine] are mutually exclusive")
+          expect(last_response.body).to eq "beer, wine are mutually exclusive"
+        end
+
+        it 'can return structured json with separate fields' do
+          subject.format :json
+          subject.rescue_from Grape::Exceptions::ValidationErrors do |e|
+            rack_response e.to_json, 400
+          end
+          subject.params do
+            optional :beer
+            optional :wine
+            optional :juice
+            exactly_one_of :beer, :wine, :juice
+          end
+          subject.get '/exactly_one_of' do
+            'exactly_one_of works!'
+          end
+
+          get '/exactly_one_of', beer: 'string', wine: 'anotherstring'
+          expect(last_response.status).to eq(400)
+          expect(JSON.parse(last_response.body)).to eq([
+            "params" => ["beer", "wine"],
+            "messages" => ["are mutually exclusive"]
+          ])
         end
 
         it 'errors when none is selected' do
@@ -986,7 +1008,7 @@ module SharedParams
 
           get '/exactly_one_of'
           expect(last_response.status).to eq(400)
-          expect(last_response.body).to eq("[:beer, :wine, :juice] - exactly one parameter must be provided")
+          expect(last_response.body).to eq "beer, wine, juice are missing, exactly one parameter must be provided"
         end
       end
     end