Array[Array[X]]] and Array[Hash[X]]] return wrong indices in errors

[senhalil]

Basically the indices of nested arguments are not kept correctly during validation process. It happens both for Hashes and Arrays declared inside another Array.

In the case of Array[Array[X]] the inner index keeps increasing (in the error messages) suggesting there might be something wrong in the recursion logic of validators -- i.e., it acts as a global count.

Created the following spec to explain the issue (disclaimer: first spec file, not sure if I got it right).

# frozen_string_literal: true

require 'spec_helper'

describe Grape::Validations::DefaultValidator do
  describe '#validate!' do
    subject(:validate) { post path, params }

    module ValidationsSpec
      module DefaultValidatorSpec
        class API < Grape::API
          rescue_from Grape::Exceptions::ValidationErrors do |e|
            error!(e.errors.transform_keys! { |key| key.join(',') }, 400)
          end

          params do
            requires :points, type: Array do
              # Following optional can be replaced with requires
              optional :location, type: Hash, allow_blank: false do
                requires :lat, type: Float, allow_blank: false
                requires :lon, type: Float, allow_blank: false
              end
            end
          end
          post '/nested-array-hash' do
          end


          params do
            requires :points, type: Array do
              # Following optional can be replaced with requires
              optional :location, type: Array, allow_blank: false do
                requires :id, type: Fixnum, allow_blank: false
              end
            end
          end
          post '/nested-array-array' do
          end

        end
      end
    end

    def app
      ValidationsSpec::DefaultValidatorSpec::API
    end

      context 'when params are nested inside hash which is inside an array' do
        let(:path) { '/nested-array-hash' }
        let(:params) {
          {
            points: [
              { location: nil },
              { location: { lat: nil, lon: 0.0 } },
              { location: { lat: "string", lon: 0.0 } },
              { location: { lat: 0.0, lon: "string" } },
              { location: { lat: 0.0, lon: nil } },
              { location: nil },
            ]
          }
        }

        it 'does return a validation error with correct array indices' do
          validate
          expect(JSON.parse(last_response.body)).to eq(
            'points[0][location]'      => ['is empty'],
            'points[1][location][lat]' => ['is empty'],
            'points[2][location][lat]' => ['is invalid'],
            'points[3][location][lon]' => ['is invalid'],
            'points[4][location][lon]' => ['is empty'],
            'points[5][location]'      => ['is empty'],
          )
        end
    end

    context 'when params are nested inside an array which is inside an array' do
      let(:path) { '/nested-array-array' }
      let(:params) {
        {
          points: [
            { location: [{ id: 0 }, { id: 1 }] }, # valid entry: to shift the indicies
            { location: [{ id: nil }, { id: 0 }] },
            { location: [{ id: 0.0 }, { id: 0 }] },
            { location: [{ id: 0 }, { id: nil }] },
            { location: [{ id: 0 }, { id: 0.0 }] },
            { location: 0 }, # This line creates multiple errors
            { location: nil },
            { location: "" }
          ]
        }
      }

      it 'does return a validation error with correct array indices' do
        validate
        expect(JSON.parse(last_response.body)).to eq(
          "points[1][location][0][id]" => ["is empty"],
          "points[2][location][0][id]" => ["is invalid"],
          "points[3][location][1][id]" => ["is empty"],
          "points[4][location][1][id]" => ["is invalid"],
          "points[5][location]" => ["is invalid"],
          "points[6][location]" => ["is empty"],
          "points[7][location]" => ["is empty"]
        )
      end
    end
  end
end

[dblock]

Maybe PR the failing spec? Make sure it's still failing against HEAD.

[diei]

I think a have the same wrong behaviour with grape 1.3.0.

My controller with validations:

class OrderController < Grape::API
  rescue_from Grape::Exceptions::ValidationErrors do |e|
    error!({ error: e.full_messages }, 422)
  end

  resources :orders do
    params do
      requires :orders, type: Array do
        requires :address, type: Hash do
          requires :name, type: String, allow_blank: false, desc: 'Name row 1'
          optional :name2, type: String, desc: 'Name row 2'
          requires :city, type: String, allow_blank: false
        end
        requires :positions, type: Array do
          requires :quantity, type: Integer, allow_blank: false
          requires :article, type: String, allow_blank: false
        end
      end
    end
    post do
    end
  end
end

A request with an array of orders as hashes. First order address's city is empty and in the first order the order quantity type in the first position invalid.

{
"orders": [
    {
      "address": {"name": "John Doe", "city": ""},
      "positions": [
        {"article": "Article 1", "quantity": [3]},
        {"article": "Article 2", "quantity": 1}
      ]
    },
   {
      "address": {"name": "Marie Smith", "city": "Miami"},
      "positions": [
        {"article": "Article 1", "quantity": 1},
        {"article": "Article 2", "quantity": 1}
      ]
    }
  ]
}

The Grape validation errors are:

{
    "error": [
        "orders[1][address][city] is empty",
        "orders[0][positions][0][quantity] is invalid"
    ]
}

The result for the validation error has the correct indeces for the error in the position, but for the address the wrong one, it is always the last index. Even if there are many orders with errors in their addresses, only the last index is shown.

[infiniteluke]

hey @dblock - I added a failing test for this in #2055. I don't have time to investigate what a fix would look like, but hopefully this can help push this forward.

Thanks for maintaining this project! it's great

[infiniteluke]

https://travis-ci.org/github/ruby-grape/grape/jobs/689366240#L9187

[dblock]

Thanks for the test @infiniteluke. Don't hold back if you find that time ;)

[StefanHagen]

Hey @infiniteluke ! Did you find a workaround for this problem? I see this issue is still open and your spec is still in draft. I am experiencing the same difficulties regarding indices not being correct when they get raised as validation errors, and I was wondering if you managed to find a temporary solution. Thanks for writing the spec btw!

[infiniteluke]

Unfortunately, I haven't been able to get to this and likely will not have a fix anytime soon as it's not longer a priority for me