Merge branch 'ruby-grape:master' into master

CHANGELOG.md

@@ -8,10 +8,12 @@
 * [#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
 
+* [#2339](https://github.com/ruby-grape/grape/pull/2339): Documentation and specs for remountable configuration in params - [@myxoh](https://github.com/myxoh).
 * [#2328](https://github.com/ruby-grape/grape/pull/2328): Don't cache Class.instance_methods - [@byroot](https://github.com/byroot).
 * [#2337](https://github.com/ruby-grape/grape/pull/2337): Fix: allow custom validators that do not end with _validator - [@ericproulx](https://github.com/ericproulx).
 * Your contribution here.

README.md

@@ -526,15 +526,15 @@ end
 ```ruby
 class BasicAPI < Grape::API
   desc 'Statuses index' do
-    params: mounted { configuration[:entity] || API::Entities::Status }.documentation
+    params: (configuration[:entity] || API::Entities::Status).documentation
   end
   params do
-    requires :all, using: mounted { configuration[:entity] || API::Entities::Status }.documentation
+    requires :all, using: (configuration[:entity] || API::Entities::Status).documentation
   end
   get '/statuses' do
     statuses = Status.all
     type = current_user.admin? ? :full : :default
-    present statuses, with: mounted { configuration[:entity] || API::Entities::Status }, type: type
+    present statuses, with: (configuration[:entity] || API::Entities::Status), type: type
   end
 end
 

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/api_remount_spec.rb

@@ -147,6 +147,42 @@ def app
         end
       end
 
+      context 'when the params are configured via a configuration' do
+        subject(:a_remounted_api) do
+          Class.new(described_class) do
+            params do
+              requires configuration[:required_attr_name], type: String
+            end
+
+            get(mounted { configuration[:endpoint] }) do
+              status 200
+            end
+          end
+        end
+
+        context 'when the configured param is my_attr' do
+          it 'requires the configured params' do
+            root_api.mount a_remounted_api, with: {
+              required_attr_name: 'my_attr',
+              endpoint: 'test'
+            }
+            get 'test?another_attr=1'
+            expect(last_response.status).to eq 400
+            get 'test?my_attr=1'
+            expect(last_response.status).to eq 200
+
+            root_api.mount a_remounted_api, with: {
+              required_attr_name: 'another_attr',
+              endpoint: 'test_b'
+            }
+            get 'test_b?another_attr=1'
+            expect(last_response.status).to eq 200
+            get 'test_b?my_attr=1'
+            expect(last_response.status).to eq 400
+          end
+        end
+      end
+
       context 'when executing a standard block within a `mounted` block with all dynamic params' do
         subject(:a_remounted_api) do
           Class.new(described_class) do

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