From f0ed9c504edb7e700096b33ba13225cf8ec3688e Mon Sep 17 00:00:00 2001
From: Nick Bienko <bikolya@gmail.com>
Date: Fri, 30 Aug 2019 16:09:46 +0200
Subject: [PATCH 1/8] Refactor mutual exclusion spec to use fake API

---
 .../validators/mutual_exclusion_spec.rb       | 211 +++++++++++++++---
 1 file changed, 184 insertions(+), 27 deletions(-)

diff --git a/spec/grape/validations/validators/mutual_exclusion_spec.rb b/spec/grape/validations/validators/mutual_exclusion_spec.rb
index 9fa6375b5..0e8fbb06c 100644
--- a/spec/grape/validations/validators/mutual_exclusion_spec.rb
+++ b/spec/grape/validations/validators/mutual_exclusion_spec.rb
@@ -2,61 +2,218 @@
 
 describe Grape::Validations::MutualExclusionValidator do
   describe '#validate!' do
-    let(:scope) do
-      Struct.new(:opts) do
-        def params(arg)
-          arg
+    subject(:validate) { post path, params }
+
+    module ValidationsSpec
+      module MutualExclusionValidatorSpec
+        class API < Grape::API
+          rescue_from Grape::Exceptions::ValidationErrors do |e|
+            error!(e.errors.transform_keys! { |key| key.join(',') }, 400)
+          end
+
+          params do
+            optional :beer
+            optional :wine
+            optional :grapefruit
+            mutually_exclusive :beer, :wine, :grapefruit
+          end
+          post do
+          end
+
+          params do
+            optional :beer
+            optional :wine
+            optional :grapefruit
+            optional :other
+            mutually_exclusive :beer, :wine, :grapefruit
+          end
+          post 'mixed-params' do
+          end
+
+          params do
+            optional :beer
+            optional :wine
+            optional :grapefruit
+            mutually_exclusive :beer, :wine, :grapefruit, message: 'you should not mix beer and wine'
+          end
+          post '/custom-message' do
+          end
+
+          params do
+            requires :item, type: Hash do
+              optional :beer
+              optional :wine
+              optional :grapefruit
+              mutually_exclusive :beer, :wine, :grapefruit
+            end
+          end
+          post '/nested-hash' do
+          end
+
+          params do
+            optional :item, type: Hash do
+              optional :beer
+              optional :wine
+              optional :grapefruit
+              mutually_exclusive :beer, :wine, :grapefruit
+            end
+          end
+          post '/nested-optional-hash' do
+          end
+
+          params do
+            requires :items, type: Array do
+              optional :beer
+              optional :wine
+              optional :grapefruit
+              mutually_exclusive :beer, :wine, :grapefruit
+            end
+          end
+          post '/nested-array' do
+          end
+
+          params do
+            requires :items, type: Array do
+              requires :nested_items, type: Array do
+                optional :beer, :wine, :grapefruit, type: Boolean
+                mutually_exclusive :beer, :wine, :grapefruit
+              end
+            end
+          end
+          post '/deeply-nested-array' do
+          end
         end
       end
     end
-    let(:mutually_exclusive_params) { %i[beer wine grapefruit] }
-    let(:validator) { described_class.new(mutually_exclusive_params, {}, false, scope.new) }
+
+    def app
+      ValidationsSpec::MutualExclusionValidatorSpec::API
+    end
 
     context 'when all mutually exclusive params are present' do
+      let(:path) { '/' }
       let(:params) { { beer: true, wine: true, grapefruit: true } }
 
-      it 'raises a validation exception' do
-        expect do
-          validator.validate! params
-        end.to raise_error(Grape::Exceptions::Validation)
+      it 'returns a validation error' do
+        validate
+        expect(last_response.status).to eq 400
+        expect(JSON.parse(last_response.body)).to eq(
+          'beer,wine,grapefruit' => ['are mutually exclusive']
+        )
       end
 
       context 'mixed with other params' do
-        let(:mixed_params) { params.merge!(other: true, andanother: true) }
+        let(:path) { '/mixed-params' }
+        let(:params) { { beer: true, wine: true, grapefruit: true, other: true } }
 
-        it 'still raises a validation exception' do
-          expect do
-            validator.validate! mixed_params
-          end.to raise_error(Grape::Exceptions::Validation)
+        it 'returns a validation error' do
+          validate
+          expect(last_response.status).to eq 400
+          expect(JSON.parse(last_response.body)).to eq(
+            'beer,wine,grapefruit' => ['are mutually exclusive']
+          )
         end
       end
     end
 
     context 'when a subset of mutually exclusive params are present' do
+      let(:path) { '/' }
       let(:params) { { beer: true, grapefruit: true } }
 
-      it 'raises a validation exception' do
-        expect do
-          validator.validate! params
-        end.to raise_error(Grape::Exceptions::Validation)
+      it 'returns a validation error' do
+        validate
+        expect(last_response.status).to eq 400
+        expect(JSON.parse(last_response.body)).to eq(
+          'beer,grapefruit' => ['are mutually exclusive']
+        )
       end
     end
 
-    context 'when params keys come as strings' do
-      let(:params) { { 'beer' => true, 'grapefruit' => true } }
+    context 'when custom message is specified' do
+      let(:path) { '/custom-message' }
+      let(:params) { { beer: true, wine: true } }
 
-      it 'raises a validation exception' do
-        expect do
-          validator.validate! params
-        end.to raise_error(Grape::Exceptions::Validation)
+      it 'returns a validation error' do
+        validate
+        expect(last_response.status).to eq 400
+        expect(JSON.parse(last_response.body)).to eq(
+          'beer,wine' => ['you should not mix beer and wine']
+        )
       end
     end
 
     context 'when no mutually exclusive params are present' do
+      let(:path) { '/' }
       let(:params) { { beer: true, somethingelse: true } }
 
-      it 'params' do
-        expect(validator.validate!(params)).to eql params
+      it 'does not return a validation error' do
+        validate
+        expect(last_response.status).to eq 201
+      end
+    end
+
+    context 'when mutually exclusive params are nested inside required hash' do
+      let(:path) { '/nested-hash' }
+      let(:params) { { item: { beer: true, wine: true } } }
+
+      it 'returns a validation error with full names of the params' do
+        validate
+        expect(last_response.status).to eq 400
+        expect(JSON.parse(last_response.body)).to eq(
+          'item[beer],item[wine]' => ['are mutually exclusive']
+        )
+      end
+    end
+
+    context 'when mutually exclusive params are nested inside optional hash' do
+      let(:path) { '/nested-optional-hash' }
+
+      context 'when params are passed' do
+        let(:params) { { item: { beer: true, wine: true } } }
+
+        it 'returns a validation error with full names of the params' do
+          validate
+          expect(last_response.status).to eq 400
+          expect(JSON.parse(last_response.body)).to eq(
+            'item[beer],item[wine]' => ['are mutually exclusive']
+          )
+        end
+      end
+
+      context 'when params are empty' do
+        let(:params) { {} }
+
+        it 'does not return a validation error' do
+          validate
+          expect(last_response.status).to eq 201
+        end
+      end
+    end
+
+    context 'when mutually exclusive params are nested inside array' do
+      let(:path) { '/nested-array' }
+      let(:params) { { items: [{ beer: true, wine: true }, { wine: true, grapefruit: true }] } }
+
+      it 'returns a validation error with full names of the params' do
+        validate
+        expect(last_response.status).to eq 400
+        expect(JSON.parse(last_response.body)).to eq(
+          'items[0][beer],items[0][wine]' => ['are mutually exclusive'],
+          'items[1][wine],items[1][grapefruit]' => ['are mutually exclusive']
+        )
+      end
+    end
+
+    context 'when mutually exclusive params are deeply nested' do
+      let(:path) { '/deeply-nested-array' }
+      let(:params) { { items: [{ nested_items: [{ beer: true, wine: true }] }] } }
+
+      it 'returns a validation error with full names of the params' do
+        validate
+        expect(last_response.status).to eq 400
+        expect(JSON.parse(last_response.body)).to eq(
+          'items[0][nested_items][0][beer],items[0][nested_items][0][wine]' => ['are mutually exclusive']
+        )
       end
     end
   end

From 4bfb1c5d214536d19c199cd4856d4dcc7a6a3308 Mon Sep 17 00:00:00 2001
From: Nick Bienko <bikolya@gmail.com>
Date: Tue, 3 Sep 2019 16:57:54 +0200
Subject: [PATCH 2/8] Refactor at_least_one_of spec to use fake API

---
 .../validators/at_least_one_of_spec.rb        | 210 ++++++++++++++----
 1 file changed, 172 insertions(+), 38 deletions(-)

diff --git a/spec/grape/validations/validators/at_least_one_of_spec.rb b/spec/grape/validations/validators/at_least_one_of_spec.rb
index 17eeb90a1..35044c899 100644
--- a/spec/grape/validations/validators/at_least_one_of_spec.rb
+++ b/spec/grape/validations/validators/at_least_one_of_spec.rb
@@ -2,75 +2,209 @@
 
 describe Grape::Validations::AtLeastOneOfValidator do
   describe '#validate!' do
-    let(:scope) do
-      Struct.new(:opts) do
-        def params(arg)
-          arg
-        end
-
-        def required?; end
-
-        # mimics a method from Grape::Validations::ParamsScope which decides how a parameter must
-        # be named in errors
-        def full_name(name)
-          "food[#{name}]"
+    subject(:validate) { post path, params }
+
+    module ValidationsSpec
+      module AtLeastOneOfValidatorSpec
+        class API < Grape::API
+          rescue_from Grape::Exceptions::ValidationErrors do |e|
+            error!(e.errors.transform_keys! { |key| key.join(',') }, 400)
+          end
+
+          params do
+            optional :beer, :wine, :grapefruit
+            at_least_one_of :beer, :wine, :grapefruit
+          end
+          post do
+          end
+
+          params do
+            optional :beer, :wine, :grapefruit, :other
+            at_least_one_of :beer, :wine, :grapefruit
+          end
+          post 'mixed-params' do
+          end
+
+          params do
+            optional :beer, :wine, :grapefruit
+            at_least_one_of :beer, :wine, :grapefruit, message: 'you should choose something'
+          end
+          post '/custom-message' do
+          end
+
+          params do
+            requires :item, type: Hash do
+              optional :beer, :wine, :grapefruit
+              at_least_one_of :beer, :wine, :grapefruit, message: 'fail'
+            end
+          end
+          post '/nested-hash' do
+          end
+
+          params do
+            requires :items, type: Array do
+              optional :beer, :wine, :grapefruit
+              at_least_one_of :beer, :wine, :grapefruit, message: 'fail'
+            end
+          end
+          post '/nested-array' do
+          end
+
+          params do
+            requires :items, type: Array do
+              requires :nested_items, type: Array do
+                optional :beer, :wine, :grapefruit
+                at_least_one_of :beer, :wine, :grapefruit, message: 'fail'
+              end
+            end
+          end
+          post '/deeply-nested-array' do
+          end
         end
       end
     end
 
-    let(:at_least_one_of_params) { %i[beer wine grapefruit] }
-    let(:validator) { described_class.new(at_least_one_of_params, {}, false, scope.new) }
+    def app
+      ValidationsSpec::AtLeastOneOfValidatorSpec::API
+    end
 
     context 'when all restricted params are present' do
+      let(:path) { '/' }
       let(:params) { { beer: true, wine: true, grapefruit: true } }
 
-      it 'does not raise a validation exception' do
-        expect(validator.validate!(params)).to eql params
+      it 'does not return a validation error' do
+        validate
+        expect(last_response.status).to eq 201
       end
 
       context 'mixed with other params' do
-        let(:mixed_params) { params.merge!(other: true, andanother: true) }
+        let(:path) { '/mixed-params' }
+        let(:params) { { beer: true, wine: true, grapefruit: true, other: true } }
 
-        it 'does not raise a validation exception' do
-          expect(validator.validate!(mixed_params)).to eql mixed_params
+        it 'does not return a validation error' do
+          validate
+          expect(last_response.status).to eq 201
         end
       end
     end
 
     context 'when a subset of restricted params are present' do
+      let(:path) { '/' }
       let(:params) { { beer: true, grapefruit: true } }
 
-      it 'does not raise a validation exception' do
-        expect(validator.validate!(params)).to eql params
+      it 'does not return a validation error' do
+        validate
+        expect(last_response.status).to eq 201
       end
     end
 
-    context 'when params keys come as strings' do
-      let(:params) { { 'beer' => true, 'grapefruit' => true } }
+    context 'when none of the restricted params is selected' do
+      let(:path) { '/' }
+      let(:params) { { other: true } }
+
+      it 'returns a validation error' do
+        validate
+        expect(last_response.status).to eq 400
+        expect(JSON.parse(last_response.body)).to eq(
+          'beer,wine,grapefruit' => ['are missing, at least one parameter must be provided']
+        )
+      end
+
+      context 'when custom message is specified' do
+        let(:path) { '/custom-message' }
 
-      it 'does not raise a validation exception' do
-        expect(validator.validate!(params)).to eql params
+        it 'returns a validation error' do
+          validate
+          expect(last_response.status).to eq 400
+          expect(JSON.parse(last_response.body)).to eq(
+            'beer,wine,grapefruit' => ['you should choose something']
+          )
+        end
       end
     end
 
-    context 'when none of the restricted params is selected' do
-      let(:params) { { somethingelse: true } }
-      it 'raises a validation exception' do
-        expected_params = at_least_one_of_params.map { |p| "food[#{p}]" }
-
-        expect { validator.validate! params }.to raise_error do |error|
-          expect(error).to be_a(Grape::Exceptions::Validation)
-          expect(error.params).to eq(expected_params)
-          expect(error.message).to eq(I18n.t('grape.errors.messages.at_least_one'))
+    context 'when exactly one of the restricted params is selected' do
+      let(:path) { '/' }
+      let(:params) { { beer: true } }
+
+      it 'does not return a validation error' do
+        validate
+        expect(last_response.status).to eq 201
+      end
+    end
+
+    context 'when restricted params are nested inside hash' do
+      let(:path) { '/nested-hash' }
+
+      context 'when at least one of them is present' do
+        let(:params) { { item: { beer: true, wine: true } } }
+
+        it 'does not return a validation error' do
+          validate
+          expect(last_response.status).to eq 201
+        end
+      end
+
+      context 'when none of them are present' do
+        let(:params) { { item: { other: true } } }
+
+        it 'returns a validation error with full names of the params' do
+          validate
+          expect(last_response.status).to eq 400
+          expect(JSON.parse(last_response.body)).to eq(
+            'item[beer],item[wine],item[grapefruit]' => ['fail']
+          )
         end
       end
     end
 
-    context 'when exactly one of the restricted params is selected' do
-      let(:params) { { beer: true, somethingelse: true } }
+    context 'when restricted params are nested inside array' do
+      let(:path) { '/nested-array' }
+
+      context 'when at least one of them is present' do
+        let(:params) { { items: [{ beer: true, wine: true }, { grapefruit: true }] } }
+
+        it 'does not return a validation error' do
+          validate
+          expect(last_response.status).to eq 201
+        end
+      end
+
+      context 'when none of them are present' do
+        let(:params) { { items: [{ beer: true, other: true }, { other: true }] } }
+
+        it 'returns a validation error with full names of the params' do
+          validate
+          expect(last_response.status).to eq 400
+          expect(JSON.parse(last_response.body)).to eq(
+            'items[1][beer],items[1][wine],items[1][grapefruit]' => ['fail']
+          )
+        end
+      end
+    end
+
+    context 'when restricted params are deeply nested' do
+      let(:path) { '/deeply-nested-array' }
 
-      it 'does not raise a validation exception' do
-        expect(validator.validate!(params)).to eql params
+      context 'when at least one of them is present' do
+        let(:params) { { items: [{ nested_items: [{ wine: true }] }] } }
+
+        it 'does not return a validation error' do
+          validate
+          expect(last_response.status).to eq 201
+        end
+      end
+
+      context 'when none of them are present' do
+        let(:params) { { items: [{ nested_items: [{ other: true }] }] } }
+
+        it 'returns a validation error with full names of the params' do
+          validate
+          expect(last_response.status).to eq 400
+          expect(JSON.parse(last_response.body)).to eq(
+            'items[0][nested_items][0][beer],items[0][nested_items][0][wine],items[0][nested_items][0][grapefruit]' => ['fail']
+          )
+        end
       end
     end
   end

From 96e4e1044e73e64414af709352b270ab4f88c864 Mon Sep 17 00:00:00 2001
From: Nick Bienko <bikolya@gmail.com>
Date: Fri, 4 Oct 2019 17:42:06 +0200
Subject: [PATCH 3/8] Refactor exacly_one_of spec to use fake API

---
 .../validations/validators/exactly_one_of.rb  |  25 +-
 .../validators/exactly_one_of_spec.rb         | 240 +++++++++++++++---
 2 files changed, 206 insertions(+), 59 deletions(-)

diff --git a/lib/grape/validations/validators/exactly_one_of.rb b/lib/grape/validations/validators/exactly_one_of.rb
index 07283c783..dccdbb7d5 100644
--- a/lib/grape/validations/validators/exactly_one_of.rb
+++ b/lib/grape/validations/validators/exactly_one_of.rb
@@ -1,28 +1,11 @@
 module Grape
   module Validations
-    require 'grape/validations/validators/mutual_exclusion'
-    class ExactlyOneOfValidator < MutualExclusionValidator
-      def validate!(params)
-        super
-        if scope_requires_params && none_of_restricted_params_is_present
+    require 'grape/validations/validators/multiple_params_base'
+    class ExactlyOneOfValidator < MultipleParamsBase
+      def validate_params!(params)
+        if keys_in_common(params).length != 1
           raise Grape::Exceptions::Validation, params: all_keys, message: message(:exactly_one)
         end
-        params
-      end
-
-      def message(default_key = nil)
-        options = instance_variable_get(:@option)
-        if options_key?(:message)
-          (options_key?(default_key, options[:message]) ? options[:message][default_key] : options[:message])
-        else
-          default_key
-        end
-      end
-
-      private
-
-      def none_of_restricted_params_is_present
-        scoped_params.any? { |resource_params| keys_in_common(resource_params).empty? }
       end
     end
   end
diff --git a/spec/grape/validations/validators/exactly_one_of_spec.rb b/spec/grape/validations/validators/exactly_one_of_spec.rb
index 8bdb23073..de06414dc 100644
--- a/spec/grape/validations/validators/exactly_one_of_spec.rb
+++ b/spec/grape/validations/validators/exactly_one_of_spec.rb
@@ -2,73 +2,237 @@
 
 describe Grape::Validations::ExactlyOneOfValidator do
   describe '#validate!' do
-    let(:scope) do
-      Struct.new(:opts) do
-        def params(arg)
-          arg
-        end
+    subject(:validate) { post path, params }
+
+    module ValidationsSpec
+      module ExactlyOneOfValidatorSpec
+        class API < Grape::API
+          rescue_from Grape::Exceptions::ValidationErrors do |e|
+            error!(e.errors.transform_keys! { |key| key.join(',') }, 400)
+          end
+
+          params do
+            optional :beer
+            optional :wine
+            optional :grapefruit
+            exactly_one_of :beer, :wine, :grapefruit
+          end
+          post do
+          end
+
+          params do
+            optional :beer
+            optional :wine
+            optional :grapefruit
+            optional :other
+            exactly_one_of :beer, :wine, :grapefruit
+          end
+          post 'mixed-params' do
+          end
+
+          params do
+            optional :beer
+            optional :wine
+            optional :grapefruit
+            exactly_one_of :beer, :wine, :grapefruit, message: 'you should choose one'
+          end
+          post '/custom-message' do
+          end
 
-        def required?; end
+          params do
+            requires :item, type: Hash do
+              optional :beer
+              optional :wine
+              optional :grapefruit
+              exactly_one_of :beer, :wine, :grapefruit
+            end
+          end
+          post '/nested-hash' do
+          end
+
+          params do
+            optional :item, type: Hash do
+              optional :beer
+              optional :wine
+              optional :grapefruit
+              exactly_one_of :beer, :wine, :grapefruit
+            end
+          end
+          post '/nested-optional-hash' do
+          end
+
+          params do
+            requires :items, type: Array do
+              optional :beer
+              optional :wine
+              optional :grapefruit
+              exactly_one_of :beer, :wine, :grapefruit
+            end
+          end
+          post '/nested-array' do
+          end
+
+          params do
+            requires :items, type: Array do
+              requires :nested_items, type: Array do
+                optional :beer, :wine, :grapefruit, type: Boolean
+                exactly_one_of :beer, :wine, :grapefruit
+              end
+            end
+          end
+          post '/deeply-nested-array' do
+          end
+        end
       end
     end
-    let(:exactly_one_of_params) { %i[beer wine grapefruit] }
-    let(:validator) { described_class.new(exactly_one_of_params, {}, false, scope.new) }
 
-    context 'when all restricted params are present' do
+    def app
+      ValidationsSpec::ExactlyOneOfValidatorSpec::API
+    end
+
+    context 'when all params are present' do
+      let(:path) { '/' }
       let(:params) { { beer: true, wine: true, grapefruit: true } }
 
-      it 'raises a validation exception' do
-        expect do
-          validator.validate! params
-        end.to raise_error(Grape::Exceptions::Validation)
+      it 'returns a validation error' do
+        validate
+        expect(last_response.status).to eq 400
+        expect(JSON.parse(last_response.body)).to eq(
+          'beer,wine,grapefruit' => ['are missing, exactly one parameter must be provided']
+        )
       end
 
       context 'mixed with other params' do
-        let(:mixed_params) { params.merge!(other: true, andanother: true) }
+        let(:path) { '/mixed-params' }
+        let(:params) { { beer: true, wine: true, grapefruit: true, other: true } }
 
-        it 'still raises a validation exception' do
-          expect do
-            validator.validate! mixed_params
-          end.to raise_error(Grape::Exceptions::Validation)
+        it 'returns a validation error' do
+          validate
+          expect(last_response.status).to eq 400
+          expect(JSON.parse(last_response.body)).to eq(
+            'beer,wine,grapefruit' => ['are missing, exactly one parameter must be provided']
+          )
         end
       end
     end
 
-    context 'when a subset of restricted params are present' do
+    context 'when a subset of params are present' do
+      let(:path) { '/' }
       let(:params) { { beer: true, grapefruit: true } }
 
-      it 'raises a validation exception' do
-        expect do
-          validator.validate! params
-        end.to raise_error(Grape::Exceptions::Validation)
+      it 'returns a validation error' do
+        validate
+        expect(last_response.status).to eq 400
+        expect(JSON.parse(last_response.body)).to eq(
+          'beer,wine,grapefruit' => ['are missing, exactly one parameter must be provided']
+        )
       end
     end
 
-    context 'when params keys come as strings' do
-      let(:params) { { 'beer' => true, 'grapefruit' => true } }
+    context 'when custom message is specified' do
+      let(:path) { '/custom-message' }
+      let(:params) { { beer: true, wine: true } }
 
-      it 'raises a validation exception' do
-        expect do
-          validator.validate! params
-        end.to raise_error(Grape::Exceptions::Validation)
+      it 'returns a validation error' do
+        validate
+        expect(last_response.status).to eq 400
+        expect(JSON.parse(last_response.body)).to eq(
+          'beer,wine,grapefruit' => ['you should choose one']
+        )
       end
     end
 
-    context 'when none of the restricted params is selected' do
+    context 'when exacly one param is present' do
+      let(:path) { '/' }
+      let(:params) { { beer: true, somethingelse: true } }
+
+      it 'does not return a validation error' do
+        validate
+        expect(last_response.status).to eq 201
+      end
+    end
+
+    context 'when none of the params are present' do
+      let(:path) { '/' }
       let(:params) { { somethingelse: true } }
 
-      it 'raises a validation exception' do
-        expect do
-          validator.validate! params
-        end.to raise_error(Grape::Exceptions::Validation)
+      it 'returns a validation error' do
+        validate
+        expect(last_response.status).to eq 400
+        expect(JSON.parse(last_response.body)).to eq(
+          'beer,wine,grapefruit' => ['are missing, exactly one parameter must be provided']
+        )
       end
     end
 
-    context 'when exactly one of the restricted params is selected' do
-      let(:params) { { beer: true, somethingelse: true } }
+    context 'when params are nested inside required hash' do
+      let(:path) { '/nested-hash' }
+      let(:params) { { item: { beer: true, wine: true } } }
+
+      it 'returns a validation error with full names of the params' do
+        validate
+        expect(last_response.status).to eq 400
+        expect(JSON.parse(last_response.body)).to eq(
+          'item[beer],item[wine],item[grapefruit]' => ['are missing, exactly one parameter must be provided']
+        )
+      end
+    end
+
+    context 'when params are nested inside optional hash' do
+      let(:path) { '/nested-optional-hash' }
+
+      context 'when params are passed' do
+        let(:params) { { item: { beer: true, wine: true } } }
+
+        it 'returns a validation error with full names of the params' do
+          validate
+          expect(last_response.status).to eq 400
+          expect(JSON.parse(last_response.body)).to eq(
+            'item[beer],item[wine],item[grapefruit]' => ['are missing, exactly one parameter must be provided']
+          )
+        end
+      end
+
+      context 'when params are empty' do
+        let(:params) { { other: true } }
+
+        it 'does not return a validation error' do
+          validate
+          expect(last_response.status).to eq 201
+        end
+      end
+    end
+
+    context 'when params are nested inside array' do
+      let(:path) { '/nested-array' }
+      let(:params) { { items: [{ beer: true, wine: true }, { wine: true, grapefruit: true }] } }
+
+      it 'returns a validation error with full names of the params' do
+        validate
+        expect(last_response.status).to eq 400
+        expect(JSON.parse(last_response.body)).to eq(
+          'items[0][beer],items[0][wine],items[0][grapefruit]' => [
+            'are missing, exactly one parameter must be provided'
+          ],
+          'items[1][beer],items[1][wine],items[1][grapefruit]' => [
+            'are missing, exactly one parameter must be provided'
+          ]
+        )
+      end
+    end
+
+    context 'when params are deeply nested' do
+      let(:path) { '/deeply-nested-array' }
+      let(:params) { { items: [{ nested_items: [{ beer: true, wine: true }] }] } }
 
-      it 'does not raise a validation exception' do
-        expect(validator.validate!(params)).to eql params
+      it 'returns a validation error with full names of the params' do
+        validate
+        expect(last_response.status).to eq 400
+        expect(JSON.parse(last_response.body)).to eq(
+          'items[0][nested_items][0][beer],items[0][nested_items][0][wine],items[0][nested_items][0][grapefruit]' => [
+            'are missing, exactly one parameter must be provided'
+          ]
+        )
       end
     end
   end

From 462b6a8b0e05b1c7993623d50eaa0a945fe0dcdb Mon Sep 17 00:00:00 2001
From: Nick Bienko <bikolya@gmail.com>
Date: Fri, 4 Oct 2019 18:10:17 +0200
Subject: [PATCH 4/8] Refactor all_or_none spec to use fake API

---
 .../validators/all_or_none_spec.rb            | 168 ++++++++++++++----
 1 file changed, 138 insertions(+), 30 deletions(-)

diff --git a/spec/grape/validations/validators/all_or_none_spec.rb b/spec/grape/validations/validators/all_or_none_spec.rb
index 98d82ce8f..31abc57cf 100644
--- a/spec/grape/validations/validators/all_or_none_spec.rb
+++ b/spec/grape/validations/validators/all_or_none_spec.rb
@@ -2,58 +2,166 @@
 
 describe Grape::Validations::AllOrNoneOfValidator do
   describe '#validate!' do
-    let(:scope) do
-      Struct.new(:opts) do
-        def params(arg)
-          arg
-        end
+    subject(:validate) { post path, params }
+
+    module ValidationsSpec
+      module AllOrNoneOfValidatorSpec
+        class API < Grape::API
+          rescue_from Grape::Exceptions::ValidationErrors do |e|
+            error!(e.errors.transform_keys! { |key| key.join(',') }, 400)
+          end
+
+          params do
+            optional :beer, :wine, type: Boolean
+            all_or_none_of :beer, :wine
+          end
+          post do
+          end
+
+          params do
+            optional :beer, :wine, :other, type: Boolean
+            all_or_none_of :beer, :wine
+          end
+          post 'mixed-params' do
+          end
+
+          params do
+            optional :beer, :wine, type: Boolean
+            all_or_none_of :beer, :wine, message: 'choose all or none'
+          end
+          post '/custom-message' do
+          end
+
+          params do
+            requires :item, type: Hash do
+              optional :beer, :wine, type: Boolean
+              all_or_none_of :beer, :wine
+            end
+          end
+          post '/nested-hash' do
+          end
+
+          params do
+            requires :items, type: Array do
+              optional :beer, :wine, type: Boolean
+              all_or_none_of :beer, :wine
+            end
+          end
+          post '/nested-array' do
+          end
 
-        def required?; end
+          params do
+            requires :items, type: Array do
+              requires :nested_items, type: Array do
+                optional :beer, :wine, type: Boolean
+                all_or_none_of :beer, :wine
+              end
+            end
+          end
+          post '/deeply-nested-array' do
+          end
+        end
       end
     end
-    let(:all_or_none_params) { %i[beer wine grapefruit] }
-    let(:validator) { described_class.new(all_or_none_params, {}, false, scope.new) }
+
+    def app
+      ValidationsSpec::AllOrNoneOfValidatorSpec::API
+    end
 
     context 'when all restricted params are present' do
-      let(:params) { { beer: true, wine: true, grapefruit: true } }
+      let(:path) { '/' }
+      let(:params) { { beer: true, wine: true } }
 
-      it 'does not raise a validation exception' do
-        expect(validator.validate!(params)).to eql params
+      it 'does not return a validation error' do
+        validate
+        expect(last_response.status).to eq 201
       end
 
       context 'mixed with other params' do
-        let(:mixed_params) { params.merge!(other: true, andanother: true) }
+        let(:path) { '/mixed-params' }
+        let(:params) { { beer: true, wine: true, other: true } }
 
-        it 'does not raise a validation exception' do
-          expect(validator.validate!(mixed_params)).to eql mixed_params
+        it 'does not return a validation error' do
+          validate
+          expect(last_response.status).to eq 201
         end
       end
     end
 
-    context 'when none of the restricted params is selected' do
+    context 'when a subset of restricted params are present' do
+      let(:path) { '/' }
+      let(:params) { { beer: true } }
+
+      it 'returns a validation error' do
+        validate
+        expect(last_response.status).to eq 400
+        expect(JSON.parse(last_response.body)).to eq(
+          'beer,wine' => ['provide all or none of parameters']
+        )
+      end
+    end
+
+    context 'when custom message is specified' do
+      let(:path) { '/custom-message' }
+      let(:params) { { beer: true } }
+
+      it 'returns a validation error' do
+        validate
+        expect(last_response.status).to eq 400
+        expect(JSON.parse(last_response.body)).to eq(
+          'beer,wine' => ['choose all or none']
+        )
+      end
+    end
+
+    context 'when no restricted params are present' do
+      let(:path) { '/' }
       let(:params) { { somethingelse: true } }
 
-      it 'does not raise a validation exception' do
-        expect(validator.validate!(params)).to eql params
+      it 'does not return a validation error' do
+        validate
+        expect(last_response.status).to eq 201
       end
     end
 
-    context 'when only a subset of restricted params are present' do
-      let(:params) { { beer: true, grapefruit: true } }
+    context 'when restricted params are nested inside required hash' do
+      let(:path) { '/nested-hash' }
+      let(:params) { { item: { beer: true } } }
 
-      it 'raises a validation exception' do
-        expect do
-          validator.validate! params
-        end.to raise_error(Grape::Exceptions::Validation)
+      it 'returns a validation error with full names of the params' do
+        validate
+        expect(last_response.status).to eq 400
+        expect(JSON.parse(last_response.body)).to eq(
+          'item[beer],item[wine]' => ['provide all or none of parameters']
+        )
       end
-      context 'mixed with other params' do
-        let(:mixed_params) { params.merge!(other: true, andanother: true) }
+    end
 
-        it 'raise a validation exception' do
-          expect do
-            validator.validate! params
-          end.to raise_error(Grape::Exceptions::Validation)
-        end
+    context 'when mutually exclusive params are nested inside array' do
+      let(:path) { '/nested-array' }
+      let(:params) { { items: [{ beer: true, wine: true }, { wine: true }] } }
+
+      it 'returns a validation error with full names of the params' do
+        validate
+        expect(last_response.status).to eq 400
+        expect(JSON.parse(last_response.body)).to eq(
+          'items[1][beer],items[1][wine]' => ['provide all or none of parameters']
+        )
+      end
+    end
+
+    context 'when mutually exclusive params are deeply nested' do
+      let(:path) { '/deeply-nested-array' }
+      let(:params) { { items: [{ nested_items: [{ beer: true }] }] } }
+
+      it 'returns a validation error with full names of the params' do
+        validate
+        expect(last_response.status).to eq 400
+        expect(JSON.parse(last_response.body)).to eq(
+          'items[0][nested_items][0][beer],items[0][nested_items][0][wine]' => [
+            'provide all or none of parameters'
+          ]
+        )
       end
     end
   end

From dec377a61125a1cccc58e9a83c0bc87449b21720 Mon Sep 17 00:00:00 2001
From: Nick Bienko <bikolya@gmail.com>
Date: Tue, 3 Sep 2019 17:12:45 +0200
Subject: [PATCH 5/8] Refactor unless condition in validate! method in base
 validator

---
 lib/grape/validations/validators/base.rb | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/lib/grape/validations/validators/base.rb b/lib/grape/validations/validators/base.rb
index c62956acc..673db01c3 100644
--- a/lib/grape/validations/validators/base.rb
+++ b/lib/grape/validations/validators/base.rb
@@ -36,17 +36,18 @@ def validate(request)
       # @return [void]
       def validate!(params)
         attributes = AttributesIterator.new(self, @scope, params)
+        # we collect errors inside array because
+        # there may be more than one error per field
         array_errors = []
+
         attributes.each do |resource_params, attr_name|
           next if !@scope.required? && resource_params.empty?
-          next unless @required || (resource_params.respond_to?(:key?) && resource_params.key?(attr_name))
           next unless @scope.meets_dependency?(resource_params, params)
-
           begin
-            validate_param!(attr_name, resource_params)
+            if @required || resource_params.respond_to?(:key?) && resource_params.key?(attr_name)
+              validate_param!(attr_name, resource_params)
+            end
           rescue Grape::Exceptions::Validation => e
-            # we collect errors inside array because
-            # there may be more than one error per field
             array_errors << e
           end
         end

From ab1a256ff4217fa3b3f5976fa44e3d505aad175d Mon Sep 17 00:00:00 2001
From: Nick Bienko <bikolya@gmail.com>
Date: Fri, 4 Oct 2019 17:58:43 +0200
Subject: [PATCH 6/8] Fix multiple params validators to return correct messages

Previously all the multiple params validators returned messages
without considering the scope.
For example, in case of hash params:
`{ "param_1,param_2": 'error message' }`
instead of:
`{ "hash[param_1],hash[param_2]": 'error message' }`

Or in case of arrays indexes were ignored completely:
`{ "param_1,param_2": 'error message' }`
instead of:
`{ "array[1][param_1],array[1][param_2]": 'error message' }`

This commit fixes it by using AttributesIterator in a similar way to
Grape::Validations::Base.
---
 CHANGELOG.md                                  |  1 +
 lib/grape/validations/attributes_iterator.rb  | 11 +++++---
 .../validations/validators/all_or_none.rb     | 19 +++++---------
 .../validations/validators/at_least_one_of.rb | 17 +++---------
 .../validations/validators/exactly_one_of.rb  |  8 +++---
 .../validators/multiple_params_base.rb        | 26 ++++++++++++-------
 .../validators/mutual_exclusion.rb            | 24 +++++------------
 7 files changed, 44 insertions(+), 62 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 50dbcd0eb..504f4227c 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -19,6 +19,7 @@
 * [#1898](https://github.com/ruby-grape/grape/pull/1898): Refactor `ValidatorFactory` to improve memory allocation - [@Bhacaz](https://github.com/Bhacaz).
 * [#1900](https://github.com/ruby-grape/grape/pull/1900): Define boolean for `Grape::Api::Instance` - [@Bhacaz](https://github.com/Bhacaz).
 * [#1903](https://github.com/ruby-grape/grape/pull/1903): Allow nested params renaming (Hash/Array) - [@bikolya](https://github.com/bikolya).
+* [#1913](https://github.com/ruby-grape/grape/pull/1913): Fix multiple params validators to return correct messages for nested params - [@bikolya](https://github.com/bikolya).
 
 ### 1.2.4 (2019/06/13)
 
diff --git a/lib/grape/validations/attributes_iterator.rb b/lib/grape/validations/attributes_iterator.rb
index 18b40cce7..5aad071f0 100644
--- a/lib/grape/validations/attributes_iterator.rb
+++ b/lib/grape/validations/attributes_iterator.rb
@@ -5,11 +5,12 @@ class AttributesIterator
 
       attr_reader :scope
 
-      def initialize(validator, scope, params)
+      def initialize(validator, scope, params, multiple_params: false)
         @scope = scope
         @attrs = validator.attrs
         @original_params = scope.params(params)
         @params = Array.wrap(@original_params)
+        @multiple_params = multiple_params
       end
 
       def each(&block)
@@ -41,8 +42,12 @@ def do_each(params_to_process, parent_indicies = [], &block)
             @scope.index = index
           end
 
-          @attrs.each do |attr_name|
-            yield resource_params, attr_name, inside_array
+          if @multiple_params
+            yield resource_params, @attrs
+          else
+            @attrs.each do |attr_name|
+              yield resource_params, attr_name
+            end
           end
         end
       end
diff --git a/lib/grape/validations/validators/all_or_none.rb b/lib/grape/validations/validators/all_or_none.rb
index 02a06851d..5062044c9 100644
--- a/lib/grape/validations/validators/all_or_none.rb
+++ b/lib/grape/validations/validators/all_or_none.rb
@@ -1,19 +1,12 @@
+require 'grape/validations/validators/multiple_params_base'
+
 module Grape
   module Validations
-    require 'grape/validations/validators/multiple_params_base'
     class AllOrNoneOfValidator < MultipleParamsBase
-      def validate!(params)
-        super
-        if scope_requires_params && only_subset_present
-          raise Grape::Exceptions::Validation, params: all_keys, message: message(:all_or_none)
-        end
-        params
-      end
-
-      private
-
-      def only_subset_present
-        scoped_params.any? { |resource_params| !keys_in_common(resource_params).empty? && keys_in_common(resource_params).length < attrs.length }
+      def validate_params!(params)
+        keys = keys_in_common(params)
+        return if keys.empty? || keys.length == all_keys.length
+        raise Grape::Exceptions::Validation, params: all_keys, message: message(:all_or_none)
       end
     end
   end
diff --git a/lib/grape/validations/validators/at_least_one_of.rb b/lib/grape/validations/validators/at_least_one_of.rb
index cfe006c89..5859d41d4 100644
--- a/lib/grape/validations/validators/at_least_one_of.rb
+++ b/lib/grape/validations/validators/at_least_one_of.rb
@@ -3,20 +3,9 @@
 module Grape
   module Validations
     class AtLeastOneOfValidator < MultipleParamsBase
-      def validate!(params)
-        super
-        if scope_requires_params && no_exclusive_params_are_present
-          scoped_params = all_keys.map { |key| @scope.full_name(key) }
-          raise Grape::Exceptions::Validation, params: scoped_params,
-                                               message: message(:at_least_one)
-        end
-        params
-      end
-
-      private
-
-      def no_exclusive_params_are_present
-        scoped_params.any? { |resource_params| keys_in_common(resource_params).empty? }
+      def validate_params!(params)
+        return unless keys_in_common(params).empty?
+        raise Grape::Exceptions::Validation, params: all_keys, message: message(:at_least_one)
       end
     end
   end
diff --git a/lib/grape/validations/validators/exactly_one_of.rb b/lib/grape/validations/validators/exactly_one_of.rb
index dccdbb7d5..36ca3b10d 100644
--- a/lib/grape/validations/validators/exactly_one_of.rb
+++ b/lib/grape/validations/validators/exactly_one_of.rb
@@ -1,11 +1,11 @@
+require 'grape/validations/validators/multiple_params_base'
+
 module Grape
   module Validations
-    require 'grape/validations/validators/multiple_params_base'
     class ExactlyOneOfValidator < MultipleParamsBase
       def validate_params!(params)
-        if keys_in_common(params).length != 1
-          raise Grape::Exceptions::Validation, params: all_keys, message: message(:exactly_one)
-        end
+        return if keys_in_common(params).length == 1
+        raise Grape::Exceptions::Validation, params: all_keys, message: message(:exactly_one)
       end
     end
   end
diff --git a/lib/grape/validations/validators/multiple_params_base.rb b/lib/grape/validations/validators/multiple_params_base.rb
index a8419661b..b733c65dd 100644
--- a/lib/grape/validations/validators/multiple_params_base.rb
+++ b/lib/grape/validations/validators/multiple_params_base.rb
@@ -1,26 +1,32 @@
 module Grape
   module Validations
     class MultipleParamsBase < Base
-      attr_reader :scoped_params
-
+      # rubocop:disable HashEachMethods
       def validate!(params)
-        @scoped_params = [@scope.params(params)].flatten
-        params
-      end
+        attributes = AttributesIterator.new(self, @scope, params, multiple_params: true)
+        array_errors = []
 
-      private
+        attributes.each do |resource_params, _|
+          begin
+            validate_params!(resource_params)
+          rescue Grape::Exceptions::Validation => e
+            array_errors << e
+          end
+        end
 
-      def scope_requires_params
-        @scope.required? || scoped_params.any? { |param| param.respond_to?(:any?) && param.any? }
+        raise Grape::Exceptions::ValidationArrayErrors, array_errors if array_errors.any?
       end
+      # rubocop:enable HashEachMethods
+
+      private
 
       def keys_in_common(resource_params)
         return [] unless resource_params.is_a?(Hash)
-        (all_keys & resource_params.stringify_keys.keys).map(&:to_s)
+        all_keys & resource_params.keys.map! { |attr| @scope.full_name(attr) }
       end
 
       def all_keys
-        attrs.map(&:to_s)
+        attrs.map { |attr| @scope.full_name(attr) }
       end
     end
   end
diff --git a/lib/grape/validations/validators/mutual_exclusion.rb b/lib/grape/validations/validators/mutual_exclusion.rb
index d46d56351..efe849f7c 100644
--- a/lib/grape/validations/validators/mutual_exclusion.rb
+++ b/lib/grape/validations/validators/mutual_exclusion.rb
@@ -1,24 +1,12 @@
+require 'grape/validations/validators/multiple_params_base'
+
 module Grape
   module Validations
-    require 'grape/validations/validators/multiple_params_base'
     class MutualExclusionValidator < MultipleParamsBase
-      attr_reader :processing_keys_in_common
-
-      def validate!(params)
-        super
-        if two_or_more_exclusive_params_are_present
-          raise Grape::Exceptions::Validation, params: processing_keys_in_common, message: message(:mutual_exclusion)
-        end
-        params
-      end
-
-      private
-
-      def two_or_more_exclusive_params_are_present
-        scoped_params.any? do |resource_params|
-          @processing_keys_in_common = keys_in_common(resource_params)
-          @processing_keys_in_common.length > 1
-        end
+      def validate_params!(params)
+        keys = keys_in_common(params)
+        return if keys.length <= 1
+        raise Grape::Exceptions::Validation, params: keys, message: message(:mutual_exclusion)
       end
     end
   end

From 31224dd2078d04ef38fba17305d459be94ed5d69 Mon Sep 17 00:00:00 2001
From: Nick Bienko <bikolya@gmail.com>
Date: Fri, 4 Oct 2019 18:47:39 +0200
Subject: [PATCH 7/8] Fix old validation specs for multiple params

TBH, no idea how they managed to pass before
---
 spec/grape/exceptions/validation_errors_spec.rb | 10 ++++++----
 spec/grape/validations_spec.rb                  | 16 +++++++++-------
 2 files changed, 15 insertions(+), 11 deletions(-)

diff --git a/spec/grape/exceptions/validation_errors_spec.rb b/spec/grape/exceptions/validation_errors_spec.rb
index cbf1a1853..174350e6e 100644
--- a/spec/grape/exceptions/validation_errors_spec.rb
+++ b/spec/grape/exceptions/validation_errors_spec.rb
@@ -77,10 +77,12 @@ def app
       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' => %w[beer wine],
-                                                     'messages' => ['are mutually exclusive']
-                                                   ])
+      expect(JSON.parse(last_response.body)).to eq(
+        [
+          'params' => %w[beer wine juice],
+          'messages' => ['are missing, exactly one parameter must be provided']
+        ]
+      )
     end
   end
 end
diff --git a/spec/grape/validations_spec.rb b/spec/grape/validations_spec.rb
index c8bd714d6..7650828fe 100644
--- a/spec/grape/validations_spec.rb
+++ b/spec/grape/validations_spec.rb
@@ -1233,7 +1233,9 @@ def validate_param!(attr_name, params)
             end
             get '/custom_message/mutually_exclusive', beer: 'true', wine: 'true', nested: { scotch: 'true', aquavit: 'true' }, nested2: [{ scotch2: 'true' }, { scotch2: 'true', aquavit2: 'true' }]
             expect(last_response.status).to eq(400)
-            expect(last_response.body).to eq 'beer, wine are mutually exclusive pass only one, scotch, aquavit are mutually exclusive pass only one, scotch2, aquavit2 are mutually exclusive pass only one'
+            expect(last_response.body).to eq(
+              'beer, wine are mutually exclusive pass only one, nested[scotch], nested[aquavit] are mutually exclusive pass only one, nested2[1][scotch2], nested2[1][aquavit2] are mutually exclusive pass only one'
+            )
           end
         end
 
@@ -1259,7 +1261,7 @@ def validate_param!(attr_name, params)
 
           get '/mutually_exclusive', beer: 'true', wine: 'true', nested: { scotch: 'true', aquavit: 'true' }, nested2: [{ scotch2: 'true' }, { scotch2: 'true', aquavit2: 'true' }]
           expect(last_response.status).to eq(400)
-          expect(last_response.body).to eq 'beer, wine are mutually exclusive, scotch, aquavit are mutually exclusive, scotch2, aquavit2 are mutually exclusive'
+          expect(last_response.body).to eq 'beer, wine are mutually exclusive, nested[scotch], nested[aquavit] are mutually exclusive, nested2[1][scotch2], nested2[1][aquavit2] are mutually exclusive'
         end
       end
 
@@ -1328,7 +1330,7 @@ def validate_param!(attr_name, params)
               optional :beer
               optional :wine
               optional :juice
-              exactly_one_of :beer, :wine, :juice, message: { exactly_one: 'are missing, exactly one parameter is required', mutual_exclusion: 'are mutually exclusive, exactly one parameter is required' }
+              exactly_one_of :beer, :wine, :juice, message: 'are missing, exactly one parameter is required'
             end
             get '/exactly_one_of' do
               'exactly_one_of works!'
@@ -1362,7 +1364,7 @@ def validate_param!(attr_name, params)
           it 'errors when two or more are present' do
             get '/custom_message/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, exactly one parameter is required'
+            expect(last_response.body).to eq 'beer, wine, juice are missing, exactly one parameter is required'
           end
         end
 
@@ -1381,7 +1383,7 @@ def validate_param!(attr_name, params)
         it 'errors when two or more are present' do
           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, juice are missing, exactly one parameter must be provided'
         end
       end
 
@@ -1409,7 +1411,7 @@ def validate_param!(attr_name, params)
         it 'errors when none are present' do
           get '/exactly_one_of_nested'
           expect(last_response.status).to eq(400)
-          expect(last_response.body).to eq 'nested is missing, beer_nested, wine_nested, juice_nested are missing, exactly one parameter must be provided'
+          expect(last_response.body).to eq 'nested is missing, nested[beer_nested], nested[wine_nested], nested[juice_nested] are missing, exactly one parameter must be provided'
         end
 
         it 'succeeds when one is present' do
@@ -1421,7 +1423,7 @@ def validate_param!(attr_name, params)
         it 'errors when two or more are present' do
           get '/exactly_one_of_nested', nested: { beer_nested: 'string' }, nested2: [{ beer_nested2: 'string', wine_nested2: 'anotherstring' }]
           expect(last_response.status).to eq(400)
-          expect(last_response.body).to eq 'beer_nested2, wine_nested2 are mutually exclusive'
+          expect(last_response.body).to eq 'nested2[0][beer_nested2], nested2[0][wine_nested2], nested2[0][juice_nested2] are missing, exactly one parameter must be provided'
         end
       end
     end

From b9c1a3279fd18ea57d402ab9ea5fb3b527be3839 Mon Sep 17 00:00:00 2001
From: Nick Bienko <bikolya@gmail.com>
Date: Thu, 24 Oct 2019 10:36:44 +0200
Subject: [PATCH 8/8] Split attributes iterator into single/multiple attributes
 iterators

---
 lib/grape.rb                                  |  2 ++
 lib/grape/validations/attributes_iterator.rb  | 15 ++++-----
 .../multiple_attributes_iterator.rb           | 11 +++++++
 .../validations/single_attribute_iterator.rb  | 13 ++++++++
 lib/grape/validations/validators/base.rb      |  2 +-
 lib/grape/validations/validators/default.rb   |  2 +-
 .../validators/multiple_params_base.rb        |  6 ++--
 .../multiple_attributes_iterator_spec.rb      | 29 ++++++++++++++++
 .../single_attribute_iterator_spec.rb         | 33 +++++++++++++++++++
 9 files changed, 98 insertions(+), 15 deletions(-)
 create mode 100644 lib/grape/validations/multiple_attributes_iterator.rb
 create mode 100644 lib/grape/validations/single_attribute_iterator.rb
 create mode 100644 spec/grape/validations/multiple_attributes_iterator_spec.rb
 create mode 100644 spec/grape/validations/single_attribute_iterator_spec.rb

diff --git a/lib/grape.rb b/lib/grape.rb
index 0708628e2..79818a1ab 100644
--- a/lib/grape.rb
+++ b/lib/grape.rb
@@ -225,6 +225,8 @@ module ServeFile
 
 require 'grape/validations/validators/base'
 require 'grape/validations/attributes_iterator'
+require 'grape/validations/single_attribute_iterator'
+require 'grape/validations/multiple_attributes_iterator'
 require 'grape/validations/validators/allow_blank'
 require 'grape/validations/validators/as'
 require 'grape/validations/validators/at_least_one_of'
diff --git a/lib/grape/validations/attributes_iterator.rb b/lib/grape/validations/attributes_iterator.rb
index 5aad071f0..5a9609b70 100644
--- a/lib/grape/validations/attributes_iterator.rb
+++ b/lib/grape/validations/attributes_iterator.rb
@@ -5,12 +5,11 @@ class AttributesIterator
 
       attr_reader :scope
 
-      def initialize(validator, scope, params, multiple_params: false)
+      def initialize(validator, scope, params)
         @scope = scope
         @attrs = validator.attrs
         @original_params = scope.params(params)
         @params = Array.wrap(@original_params)
-        @multiple_params = multiple_params
       end
 
       def each(&block)
@@ -42,15 +41,13 @@ def do_each(params_to_process, parent_indicies = [], &block)
             @scope.index = index
           end
 
-          if @multiple_params
-            yield resource_params, @attrs
-          else
-            @attrs.each do |attr_name|
-              yield resource_params, attr_name
-            end
-          end
+          yield_attributes(resource_params, @attrs, &block)
         end
       end
+
+      def yield_attributes(_resource_params, _attrs)
+        raise NotImplementedError
+      end
     end
   end
 end
diff --git a/lib/grape/validations/multiple_attributes_iterator.rb b/lib/grape/validations/multiple_attributes_iterator.rb
new file mode 100644
index 000000000..95c6b27c9
--- /dev/null
+++ b/lib/grape/validations/multiple_attributes_iterator.rb
@@ -0,0 +1,11 @@
+module Grape
+  module Validations
+    class MultipleAttributesIterator < AttributesIterator
+      private
+
+      def yield_attributes(resource_params, _attrs)
+        yield resource_params
+      end
+    end
+  end
+end
diff --git a/lib/grape/validations/single_attribute_iterator.rb b/lib/grape/validations/single_attribute_iterator.rb
new file mode 100644
index 000000000..1ad06fe71
--- /dev/null
+++ b/lib/grape/validations/single_attribute_iterator.rb
@@ -0,0 +1,13 @@
+module Grape
+  module Validations
+    class SingleAttributeIterator < AttributesIterator
+      private
+
+      def yield_attributes(resource_params, attrs)
+        attrs.each do |attr_name|
+          yield resource_params, attr_name
+        end
+      end
+    end
+  end
+end
diff --git a/lib/grape/validations/validators/base.rb b/lib/grape/validations/validators/base.rb
index 673db01c3..0594116eb 100644
--- a/lib/grape/validations/validators/base.rb
+++ b/lib/grape/validations/validators/base.rb
@@ -35,7 +35,7 @@ def validate(request)
       # @raise [Grape::Exceptions::Validation] if validation failed
       # @return [void]
       def validate!(params)
-        attributes = AttributesIterator.new(self, @scope, params)
+        attributes = SingleAttributeIterator.new(self, @scope, params)
         # we collect errors inside array because
         # there may be more than one error per field
         array_errors = []
diff --git a/lib/grape/validations/validators/default.rb b/lib/grape/validations/validators/default.rb
index c472cf31b..102458f14 100644
--- a/lib/grape/validations/validators/default.rb
+++ b/lib/grape/validations/validators/default.rb
@@ -18,7 +18,7 @@ def validate_param!(attr_name, params)
       end
 
       def validate!(params)
-        attrs = AttributesIterator.new(self, @scope, params)
+        attrs = SingleAttributeIterator.new(self, @scope, params)
         attrs.each do |resource_params, attr_name|
           if resource_params.is_a?(Hash) && resource_params[attr_name].nil?
             validate_param!(attr_name, resource_params)
diff --git a/lib/grape/validations/validators/multiple_params_base.rb b/lib/grape/validations/validators/multiple_params_base.rb
index b733c65dd..023e2f5c9 100644
--- a/lib/grape/validations/validators/multiple_params_base.rb
+++ b/lib/grape/validations/validators/multiple_params_base.rb
@@ -1,12 +1,11 @@
 module Grape
   module Validations
     class MultipleParamsBase < Base
-      # rubocop:disable HashEachMethods
       def validate!(params)
-        attributes = AttributesIterator.new(self, @scope, params, multiple_params: true)
+        attributes = MultipleAttributesIterator.new(self, @scope, params)
         array_errors = []
 
-        attributes.each do |resource_params, _|
+        attributes.each do |resource_params|
           begin
             validate_params!(resource_params)
           rescue Grape::Exceptions::Validation => e
@@ -16,7 +15,6 @@ def validate!(params)
 
         raise Grape::Exceptions::ValidationArrayErrors, array_errors if array_errors.any?
       end
-      # rubocop:enable HashEachMethods
 
       private
 
diff --git a/spec/grape/validations/multiple_attributes_iterator_spec.rb b/spec/grape/validations/multiple_attributes_iterator_spec.rb
new file mode 100644
index 000000000..ef7ead6f6
--- /dev/null
+++ b/spec/grape/validations/multiple_attributes_iterator_spec.rb
@@ -0,0 +1,29 @@
+require 'spec_helper'
+
+describe Grape::Validations::MultipleAttributesIterator do
+  describe '#each' do
+    subject(:iterator) { described_class.new(validator, scope, params) }
+    let(:scope) { Grape::Validations::ParamsScope.new(api: Class.new(Grape::API)) }
+    let(:validator) { double(attrs: %i[first second third]) }
+
+    context 'when params is a hash' do
+      let(:params) do
+        { first: 'string', second: 'string' }
+      end
+
+      it 'yields the whole params hash without the list of attrs' do
+        expect { |b| iterator.each(&b) }.to yield_with_args(params)
+      end
+    end
+
+    context 'when params is an array' do
+      let(:params) do
+        [{ first: 'string1', second: 'string1' }, { first: 'string2', second: 'string2' }]
+      end
+
+      it 'yields each element of the array without the list of attrs' do
+        expect { |b| iterator.each(&b) }.to yield_successive_args(params[0], params[1])
+      end
+    end
+  end
+end
diff --git a/spec/grape/validations/single_attribute_iterator_spec.rb b/spec/grape/validations/single_attribute_iterator_spec.rb
new file mode 100644
index 000000000..ab884f702
--- /dev/null
+++ b/spec/grape/validations/single_attribute_iterator_spec.rb
@@ -0,0 +1,33 @@
+require 'spec_helper'
+
+describe Grape::Validations::SingleAttributeIterator do
+  describe '#each' do
+    subject(:iterator) { described_class.new(validator, scope, params) }
+    let(:scope) { Grape::Validations::ParamsScope.new(api: Class.new(Grape::API)) }
+    let(:validator) { double(attrs: %i[first second third]) }
+
+    context 'when params is a hash' do
+      let(:params) do
+        { first: 'string', second: 'string' }
+      end
+
+      it 'yields params and every single attribute from the list' do
+        expect { |b| iterator.each(&b) }
+          .to yield_successive_args([params, :first], [params, :second], [params, :third])
+      end
+    end
+
+    context 'when params is an array' do
+      let(:params) do
+        [{ first: 'string1', second: 'string1' }, { first: 'string2', second: 'string2' }]
+      end
+
+      it 'yields every single attribute from the list for each of the array elements' do
+        expect { |b| iterator.each(&b) }.to yield_successive_args(
+          [params[0], :first], [params[0], :second], [params[0], :third],
+          [params[1], :first], [params[1], :second], [params[1], :third]
+        )
+      end
+    end
+  end
+end