Stop yielding skip value (#2341)

* Not yielding when skipping
Adjust specs

* Remove begin in rescue blocks

* CHANGELOG entry

* Fix typo

Author: ericproulx

CHANGELOG.md

@@ -8,6 +8,7 @@
 * [#2331](https://github.com/ruby-grape/grape/pull/2331): Memory optimization when running validators - [@ericproulx](https://github.com/ericproulx).
 * [#2332](https://github.com/ruby-grape/grape/pull/2332): Use ActiveSupport configurable - [@ericproulx](https://github.com/ericproulx).
 * [#2333](https://github.com/ruby-grape/grape/pull/2333): Use custom messages in parameter validation with arity 1 - [@thedevjoao](https://github.com/TheDevJoao).
+* [#2341](https://github.com/ruby-grape/grape/pull/2341): Stop yielding skip value - [@ericproulx](https://github.com/ericproulx).
 * Your contribution here.
 
 #### Fixes

lib/grape/validations/multiple_attributes_iterator.rb

@@ -6,7 +6,7 @@ class MultipleAttributesIterator < AttributesIterator
       private
 
       def yield_attributes(resource_params, _attrs)
-        yield resource_params, skip?(resource_params)
+        yield resource_params unless skip?(resource_params)
       end
     end
   end

lib/grape/validations/single_attribute_iterator.rb

@@ -6,8 +6,10 @@ class SingleAttributeIterator < AttributesIterator
       private
 
       def yield_attributes(val, attrs)
+        return if skip?(val)
+
         attrs.each do |attr_name|
-          yield val, attr_name, empty?(val), skip?(val)
+          yield val, attr_name, empty?(val)
         end
       end
 

lib/grape/validations/validators/base.rb

@@ -46,16 +46,13 @@ def validate!(params)
           # there may be more than one error per field
           array_errors = []
 
-          attributes.each do |val, attr_name, empty_val, skip_value|
-            next if skip_value
+          attributes.each do |val, attr_name, empty_val|
             next if !@scope.required? && empty_val
             next unless @scope.meets_dependency?(val, params)
 
-            begin
-              validate_param!(attr_name, val) if @required || (val.respond_to?(:key?) && val.key?(attr_name))
-            rescue Grape::Exceptions::Validation => e
-              array_errors << e
-            end
+            validate_param!(attr_name, val) if @required || (val.respond_to?(:key?) && val.key?(attr_name))
+          rescue Grape::Exceptions::Validation => e
+            array_errors << e
           end
 
           raise Grape::Exceptions::ValidationArrayErrors.new(array_errors) if array_errors.any?

lib/grape/validations/validators/multiple_params_base.rb

@@ -8,14 +8,10 @@ def validate!(params)
           attributes = MultipleAttributesIterator.new(self, @scope, params)
           array_errors = []
 
-          attributes.each do |resource_params, skip_value|
-            next if skip_value
-
-            begin
-              validate_params!(resource_params)
-            rescue Grape::Exceptions::Validation => e
-              array_errors << e
-            end
+          attributes.each do |resource_params|
+            validate_params!(resource_params)
+          rescue Grape::Exceptions::Validation => e
+            array_errors << e
           end
 
           raise Grape::Exceptions::ValidationArrayErrors.new(array_errors) if array_errors.any?

spec/grape/validations/multiple_attributes_iterator_spec.rb

@@ -12,8 +12,8 @@
         { first: 'string', second: 'string' }
       end
 
-      it 'yields the whole params hash and the skipped flag without the list of attrs' do
-        expect { |b| iterator.each(&b) }.to yield_with_args(params, false)
+      it 'yields the whole params hash without the list of attrs' do
+        expect { |b| iterator.each(&b) }.to yield_with_args(params)
       end
     end
 
@@ -23,17 +23,15 @@
       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], false], [params[1], false])
+        expect { |b| iterator.each(&b) }.to yield_successive_args(params[0], params[1])
       end
     end
 
     context 'when params is empty optional placeholder' do
-      let(:params) do
-        [Grape::DSL::Parameters::EmptyOptionalValue, { first: 'string2', second: 'string2' }]
-      end
+      let(:params) { [Grape::DSL::Parameters::EmptyOptionalValue] }
 
-      it 'yields each element of the array without the list of attrs' do
-        expect { |b| iterator.each(&b) }.to yield_successive_args([Grape::DSL::Parameters::EmptyOptionalValue, true], [params[1], false])
+      it 'does not yield it' do
+        expect { |b| iterator.each(&b) }.to yield_successive_args
       end
     end
   end

spec/grape/validations/single_attribute_iterator_spec.rb

@@ -14,7 +14,7 @@
 
       it 'yields params and every single attribute from the list' do
         expect { |b| iterator.each(&b) }
-          .to yield_successive_args([params, :first, false, false], [params, :second, false, false])
+          .to yield_successive_args([params, :first, false], [params, :second, false])
       end
     end
 
@@ -25,8 +25,8 @@
 
       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, false, false], [params[0], :second, false, false],
-          [params[1], :first, false, false], [params[1], :second, false, false]
+          [params[0], :first, false], [params[0], :second, false],
+          [params[1], :first, false], [params[1], :second, false]
         )
       end
 
@@ -35,20 +35,19 @@
 
         it 'marks params with empty values' do
           expect { |b| iterator.each(&b) }.to yield_successive_args(
-            [params[0], :first, true, false], [params[0], :second, true, false],
-            [params[1], :first, true, false], [params[1], :second, true, false],
-            [params[2], :first, false, false], [params[2], :second, false, false]
+            [params[0], :first, true], [params[0], :second, true],
+            [params[1], :first, true], [params[1], :second, true],
+            [params[2], :first, false], [params[2], :second, false]
           )
         end
       end
 
       context 'when missing optional value' do
         let(:params) { [Grape::DSL::Parameters::EmptyOptionalValue, 10] }
 
-        it 'marks params with skipped values' do
+        it 'does not yield skipped values' do
           expect { |b| iterator.each(&b) }.to yield_successive_args(
-            [params[0], :first, false, true], [params[0], :second, false, true],
-            [params[1], :first, false, false], [params[1], :second, false, false]
+            [params[1], :first, false], [params[1], :second, false]
           )
         end
       end