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

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