fix evaluate given in array (#2287)

* fix evaluate given in array

* changelog

* avoid a bug that may be caused by rack-test by modifying spec

* changelog, spec

* meets_dependency will use params directly instead of @parent.params(request_params) when request_params is not given

* create attr_meets_dependency instand of changing meets_dependency to more complex

Author: zysend

CHANGELOG.md

@@ -12,7 +12,7 @@
 * [#2272](https://github.com/ruby-grape/grape/pull/2272): Added error on param init when provided type does not have `[]` coercion method, previously validation silently failed for any value - [@vasfed](https://github.com/Vasfed).
 * [#2274](https://github.com/ruby-grape/grape/pull/2274): Error middleware support using rack util's symbols as status - [@dhruvCW](https://github.com/dhruvCW).
 * [#2276](https://github.com/ruby-grape/grape/pull/2276): Fix exception super - [@ericproulx](https://github.com/ericproulx).
-* [#2285](https://github.com/ruby-grape/grape/pull/2285): Added :evaluate_given to declared(params) - [@zysend](https://github.com/zysend).
+* [#2285](https://github.com/ruby-grape/grape/pull/2285), [#2287](https://github.com/ruby-grape/grape/pull/2287): Added :evaluate_given to declared(params) - [@zysend](https://github.com/zysend).
 * Your contribution here.
 
 #### Fixes

lib/grape/dsl/inside_route.rb

@@ -28,7 +28,7 @@ def self.post_filter_methods(type)
       # has completed
       module PostBeforeFilter
         def declared(passed_params, options = {}, declared_params = nil, params_nested_path = [])
-          options = options.reverse_merge(include_missing: true, include_parent_namespaces: true, evaluate_given: false, request_params: passed_params)
+          options = options.reverse_merge(include_missing: true, include_parent_namespaces: true, evaluate_given: false)
           declared_params ||= optioned_declared_params(**options)
 
           if passed_params.is_a?(Array)
@@ -48,7 +48,7 @@ def declared_array(passed_params, options, declared_params, params_nested_path)
 
         def declared_hash(passed_params, options, declared_params, params_nested_path)
           declared_params.each_with_object(passed_params.class.new) do |declared_param_attr, memo|
-            next if options[:evaluate_given] && !declared_param_attr.meets_dependency?(options[:request_params])
+            next if options[:evaluate_given] && !declared_param_attr.scope.attr_meets_dependency?(passed_params)
 
             declared_hash_attr(passed_params, options, declared_param_attr.key, params_nested_path, memo)
           end

lib/grape/validations/params_scope.rb

@@ -21,12 +21,6 @@ def initialize(key, scope)
           @scope = scope
         end
 
-        def meets_dependency?(request_params)
-          return true if scope.nil?
-
-          scope.meets_dependency?(scope.params(request_params), request_params)
-        end
-
         # @return Array[Symbol, Hash[Symbol => Array]] declared_params with symbol instead of Attr
         def self.attrs_keys(declared_params)
           declared_params.map do |declared_param_attr|
@@ -101,6 +95,18 @@ def meets_dependency?(params, request_params)
 
         return params.any? { |param| meets_dependency?(param, request_params) } if params.is_a?(Array)
 
+        meets_hash_dependency?(params)
+      end
+
+      def attr_meets_dependency?(params)
+        return true unless @dependent_on
+
+        return false if @parent.present? && !@parent.attr_meets_dependency?(params)
+
+        meets_hash_dependency?(params)
+      end
+
+      def meets_hash_dependency?(params)
         # params might be anything what looks like a hash, so it must implement a `key?` method
         return false unless params.respond_to?(:key?)
 

spec/grape/validations/params_scope_spec.rb

@@ -690,7 +690,7 @@ def initialize(value)
         end
       end
 
-      context 'nested parameter' do
+      context 'lateral hash parameter' do
         before do
           [true, false].each do |evaluate_given|
             subject.params do
@@ -729,7 +729,7 @@ def initialize(value)
         end
       end
 
-      context 'nested given parameter' do
+      context 'lateral parameter within lateral hash parameter' do
         before do
           [true, false].each do |evaluate_given|
             subject.params do
@@ -789,6 +789,191 @@ def initialize(value)
           expect(JSON.parse(last_response.body)).to eq('a' => 'y', 'b' => { 'd' => 'd', 'f' => nil, 'e' => { 'i' => nil } })
         end
       end
+
+      context 'lateral parameter within an array param' do
+        before do
+          [true, false].each do |evaluate_given|
+            subject.params do
+              optional :array, type: Array do
+                optional :a
+                given :a do
+                  optional :b
+                end
+              end
+            end
+            subject.post("/evaluate_given_#{evaluate_given}") do
+              declared(params, evaluate_given: evaluate_given).to_json
+            end
+          end
+        end
+
+        it 'evaluate_given_false' do
+          post '/evaluate_given_false', { array: [{ b: 'b' }, { a: 'a', b: 'b' }] }.to_json, 'CONTENT_TYPE' => 'application/json'
+          expect(JSON.parse(last_response.body)).to eq('array' => [{ 'a' => nil, 'b' => 'b' }, { 'a' => 'a', 'b' => 'b' }])
+        end
+
+        it 'evaluate_given_true' do
+          post '/evaluate_given_true', { array: [{ b: 'b' }, { a: 'a', b: 'b' }] }.to_json, 'CONTENT_TYPE' => 'application/json'
+          expect(JSON.parse(last_response.body)).to eq('array' => [{ 'a' => nil }, { 'a' => 'a', 'b' => 'b' }])
+        end
+      end
+
+      context 'nested given parameter' do
+        before do
+          [true, false].each do |evaluate_given|
+            subject.params do
+              optional :a
+              optional :c
+              given :a do
+                given :c do
+                  optional :b
+                end
+              end
+            end
+            subject.post("/evaluate_given_#{evaluate_given}") do
+              declared(params, evaluate_given: evaluate_given).to_json
+            end
+          end
+        end
+
+        it 'evaluate_given_false' do
+          post '/evaluate_given_false', { a: 'a', b: 'b' }.to_json, 'CONTENT_TYPE' => 'application/json'
+          expect(JSON.parse(last_response.body)).to eq('a' => 'a', 'b' => 'b', 'c' => nil)
+
+          post '/evaluate_given_false', { c: 'c', b: 'b' }.to_json, 'CONTENT_TYPE' => 'application/json'
+          expect(JSON.parse(last_response.body)).to eq('a' => nil, 'b' => 'b', 'c' => 'c')
+
+          post '/evaluate_given_false', { a: 'a', c: 'c', b: 'b' }.to_json, 'CONTENT_TYPE' => 'application/json'
+          expect(JSON.parse(last_response.body)).to eq('a' => 'a', 'b' => 'b', 'c' => 'c')
+        end
+
+        it 'evaluate_given_true' do
+          post '/evaluate_given_true', { a: 'a', b: 'b' }.to_json, 'CONTENT_TYPE' => 'application/json'
+          expect(JSON.parse(last_response.body)).to eq('a' => 'a', 'c' => nil)
+
+          post '/evaluate_given_true', { c: 'c', b: 'b' }.to_json, 'CONTENT_TYPE' => 'application/json'
+          expect(JSON.parse(last_response.body)).to eq('a' => nil, 'c' => 'c')
+
+          post '/evaluate_given_true', { a: 'a', c: 'c', b: 'b' }.to_json, 'CONTENT_TYPE' => 'application/json'
+          expect(JSON.parse(last_response.body)).to eq('a' => 'a', 'b' => 'b', 'c' => 'c')
+        end
+      end
+
+      context 'nested given parameter within an array param' do
+        before do
+          [true, false].each do |evaluate_given|
+            subject.params do
+              optional :array, type: Array do
+                optional :a
+                optional :c
+                given :a do
+                  given :c do
+                    optional :b
+                  end
+                end
+              end
+            end
+            subject.post("/evaluate_given_#{evaluate_given}") do
+              declared(params, evaluate_given: evaluate_given).to_json
+            end
+          end
+        end
+
+        let :evaluate_given_params do
+          {
+            array: [
+              { a: 'a', b: 'b' },
+              { c: 'c', b: 'b' },
+              { a: 'a', c: 'c', b: 'b' }
+            ]
+          }
+        end
+
+        it 'evaluate_given_false' do
+          post '/evaluate_given_false', evaluate_given_params.to_json, 'CONTENT_TYPE' => 'application/json'
+          expect(JSON.parse(last_response.body)).to eq('array' => [{ 'a' => 'a', 'b' => 'b', 'c' => nil }, { 'a' => nil, 'b' => 'b', 'c' => 'c' }, { 'a' => 'a', 'b' => 'b', 'c' => 'c' }])
+        end
+
+        it 'evaluate_given_true' do
+          post '/evaluate_given_true', evaluate_given_params.to_json, 'CONTENT_TYPE' => 'application/json'
+          expect(JSON.parse(last_response.body)).to eq('array' => [{ 'a' => 'a', 'c' => nil }, { 'a' => nil, 'c' => 'c' }, { 'a' => 'a', 'b' => 'b', 'c' => 'c' }])
+        end
+      end
+
+      context 'nested given parameter within a nested given parameter within an array param' do
+        before do
+          [true, false].each do |evaluate_given|
+            subject.params do
+              optional :array, type: Array do
+                optional :a
+                optional :c
+                given :a do
+                  given :c do
+                    optional :array, type: Array do
+                      optional :a
+                      optional :c
+                      given :a do
+                        given :c do
+                          optional :b
+                        end
+                      end
+                    end
+                  end
+                end
+              end
+            end
+            subject.post("/evaluate_given_#{evaluate_given}") do
+              declared(params, evaluate_given: evaluate_given).to_json
+            end
+          end
+        end
+
+        let :evaluate_given_params do
+          {
+            array: [{
+              a: 'a',
+              c: 'c',
+              array: [
+                { a: 'a', b: 'b' },
+                { c: 'c', b: 'b' },
+                { a: 'a', c: 'c', b: 'b' }
+              ]
+            }]
+          }
+        end
+
+        it 'evaluate_given_false' do
+          expected_response_hash = {
+            'array' => [{
+              'a' => 'a',
+              'c' => 'c',
+              'array' => [
+                { 'a' => 'a', 'b' => 'b', 'c' => nil },
+                { 'a' => nil, 'c' => 'c', 'b' => 'b' },
+                { 'a' => 'a', 'c' => 'c', 'b' => 'b' }
+              ]
+            }]
+          }
+          post '/evaluate_given_false', evaluate_given_params.to_json, 'CONTENT_TYPE' => 'application/json'
+          expect(JSON.parse(last_response.body)).to eq(expected_response_hash)
+        end
+
+        it 'evaluate_given_true' do
+          expected_response_hash = {
+            'array' => [{
+              'a' => 'a',
+              'c' => 'c',
+              'array' => [
+                { 'a' => 'a', 'c' => nil },
+                { 'a' => nil, 'c' => 'c' },
+                { 'a' => 'a', 'b' => 'b', 'c' => 'c' }
+              ]
+            }]
+          }
+          post '/evaluate_given_true', evaluate_given_params.to_json, 'CONTENT_TYPE' => 'application/json'
+          expect(JSON.parse(last_response.body)).to eq(expected_response_hash)
+        end
+      end
     end
   end