Fix array indicies in error messages. (#1463)

Author: ffloyd

.rubocop_todo.yml

@@ -17,7 +17,7 @@ Metrics/BlockNesting:
 # Offense count: 7
 # Configuration parameters: CountComments.
 Metrics/ClassLength:
-  Max: 264
+  Max: 266
 
 # Offense count: 24
 Metrics/CyclomaticComplexity:

CHANGELOG.md

@@ -3,6 +3,7 @@
 
 #### Features
 
+* [#1463](https://github.com/ruby-grape/grape/pull/1463): Fix array indicies in error messages - [@ffloyd](https://github.com/ffloyd).
 * [#1393](https://github.com/ruby-grape/grape/pull/1393): Middleware can be inserted before or after default Grape middleware - [@ridiculous](https://github.com/ridiculous).
 * [#1390](https://github.com/ruby-grape/grape/pull/1390): Allows inserting middleware at arbitrary points in the middleware stack - [@Rosa](https://github.com/Rosa).
 * [#1366](https://github.com/ruby-grape/grape/pull/1366): Stores `message_key` on `Grape::Exceptions::Validation` - [@mkou](https://github.com/mkou).

lib/grape.rb

@@ -58,6 +58,7 @@ module Exceptions
     extend ActiveSupport::Autoload
     autoload :Base
     autoload :Validation
+    autoload :ValidationArrayErrors
     autoload :ValidationErrors
     autoload :MissingVendorOption
     autoload :MissingMimeType

lib/grape/dsl/parameters.rb

@@ -191,7 +191,8 @@ def params(params)
         params = @parent.params(params) if @parent
         if @element
           params = if params.is_a?(Array)
-                     params.flat_map { |el| el[@element] || {} }
+                     # used for calculating parent array indices for error messages
+                     params.map { |el| el[@element] || {} }
                    elsif params.is_a?(Hash)
                      params[@element] || {}
                    else

lib/grape/endpoint.rb

@@ -335,6 +335,8 @@ def run_validators(validators, request)
           validator.validate(request)
         rescue Grape::Exceptions::Validation => e
           validation_errors << e
+        rescue Grape::Exceptions::ValidationArrayErrors => e
+          validation_errors += e.errors
         end
       end
 

lib/grape/exceptions/validation_array_errors.rb

@@ -0,0 +1,11 @@
+module Grape
+  module Exceptions
+    class ValidationArrayErrors < Base
+      attr_reader :errors
+
+      def initialize(errors)
+        @errors = errors
+      end
+    end
+  end
+end

lib/grape/validations/attributes_iterator.rb

@@ -8,16 +8,41 @@ class AttributesIterator
       def initialize(validator, scope, params)
         @scope = scope
         @attrs = validator.attrs
-        @params = Array.wrap(scope.params(params))
+        @original_params = scope.params(params)
+        @params = Array.wrap(@original_params)
       end
 
-      def each
-        @params.each do |resource_params|
-          @attrs.each_with_index do |attr_name, index|
-            if resource_params.is_a?(Hash) && resource_params[attr_name].is_a?(Array)
-              scope.index = index
+      def each(&block)
+        do_each(@params, &block) # because we need recursion for nested arrays
+      end
+
+      private
+
+      def do_each(params_to_process, parent_indicies = [], &block)
+        params_to_process.each_with_index do |resource_params, index|
+          # when we get arrays of arrays it means that target element located inside array
+          # we need this because we want to know parent arrays indicies
+          if resource_params.is_a?(Array)
+            do_each(resource_params, [index] + parent_indicies, &block)
+            next
+          end
+
+          if @scope.type == Array
+            next unless @original_params.is_a?(Array) # do not validate content of array if it isn't array
+            inside_array = true
+          end
+          if inside_array
+            # fill current and parent scopes with correct array indicies
+            parent_scope = @scope.parent
+            parent_indicies.each do |parent_index|
+              parent_scope.index = parent_index
+              parent_scope = parent_scope.parent
             end
-            yield resource_params, attr_name
+            @scope.index = index
+          end
+
+          @attrs.each do |attr_name|
+            yield resource_params, attr_name, inside_array
           end
         end
       end

lib/grape/validations/params_scope.rb

@@ -2,6 +2,7 @@ module Grape
   module Validations
     class ParamsScope
       attr_accessor :element, :parent, :index
+      attr_reader :type
 
       include Grape::DSL::Parameters
 
@@ -57,7 +58,7 @@ def full_name(name)
         case
         when nested?
           # Find our containing element's name, and append ours.
-          "#{@parent.full_name(@element)}#{parent_index}[#{name}]"
+          "#{@parent.full_name(@element)}#{array_index}[#{name}]"
         when lateral?
           # Find the name of the element as if it was at the
           # same nesting level as our parent.
@@ -68,8 +69,8 @@ def full_name(name)
         end
       end
 
-      def parent_index
-        "[#{@parent.index}]" if @parent.present? && @parent.index.present?
+      def array_index
+        "[#{@index}]" if @index.present?
       end
 
       # @return [Boolean] whether or not this scope is the root-level scope

lib/grape/validations/types/build_coercer.rb

@@ -26,7 +26,15 @@ def self.build_coercer(type, method = nil)
         converter_options = {
           nullify_blank: true
         }
-        conversion_type = type
+        conversion_type = if method == JSON
+                            Object
+                            # because we want just parsed JSON content:
+                            # if type is Array and data is `"{}"`
+                            # result will be [] because Virtus converts hashes
+                            # to arrays
+                          else
+                            type
+                          end
 
         # Use a special coercer for multiply-typed parameters.
         if Types.multiple?(type)

lib/grape/validations/validators/base.rb

@@ -35,11 +35,21 @@ def validate(request)
       # @return [void]
       def validate!(params)
         attributes = AttributesIterator.new(self, @scope, params)
-        attributes.each do |resource_params, attr_name|
-          if @required || (resource_params.respond_to?(:key?) && resource_params.key?(attr_name))
+        array_errors = []
+        attributes.each do |resource_params, attr_name, inside_array|
+          next unless @required || (resource_params.respond_to?(:key?) && resource_params.key?(attr_name))
+
+          begin
             validate_param!(attr_name, resource_params)
+          rescue Grape::Exceptions::Validation => e
+            raise error unless inside_array
+            # we collect errors inside array because
+            # there may be more than one error per field
+            array_errors << e
           end
         end
+
+        raise Grape::Exceptions::ValidationArrayErrors, array_errors if array_errors.any?
       end
 
       def self.convert_to_short_name(klass)