diff --git a/CHANGELOG.md b/CHANGELOG.md index 85f64896c..0996ff3f6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -83,6 +83,7 @@ * [#3018](https://github.com/ruby-grape/grape/pull/3018): Stop copying the auth middleware per request - [@ericproulx](https://github.com/ericproulx). * [#3023](https://github.com/ruby-grape/grape/pull/3023): Raise from `error!` instead of throwing `:error`, so a transaction it leaves rolls back - [@ericproulx](https://github.com/ericproulx). * [#3027](https://github.com/ruby-grape/grape/pull/3027): Scrub outside input through one helper, `Grape::Util::Scrub` - [@ericproulx](https://github.com/ericproulx). +* [#3028](https://github.com/ruby-grape/grape/pull/3028): Resolve a scope's request params in `ParamsScope` instead of the params DSL - [@ericproulx](https://github.com/ericproulx). * [#3030](https://github.com/ruby-grape/grape/pull/3030): Restore the Error middleware's two defaults without restating every option - [@ericproulx](https://github.com/ericproulx). * Your contribution here. diff --git a/lib/grape/dsl/parameters.rb b/lib/grape/dsl/parameters.rb index 122437438..f87ff90ac 100644 --- a/lib/grape/dsl/parameters.rb +++ b/lib/grape/dsl/parameters.rb @@ -179,26 +179,6 @@ def declared_param?(param) alias group requires - EmptyOptionalValue = Object.new.freeze - - # @param params [Hash] initial hash of parameters - # @return hash of parameters relevant for the current scope - # @api private - # Only a +given+ scope ever stores qualifying params, and a scope on an - # independent chain has none above it (see ParamsScope#independent?), so - # the lookup and the +presence+ test that follows it are skipped -- both - # of them, at every level, on every call. - def params(params) - scoped = if @parent.nil? - params - elsif @independent - @parent.params(params) - else - @parent.qualifying_params.presence || @parent.params(params) - end - @element ? map_params(scoped, @element) : scoped - end - private # +requires+ and +optional+ differ only in whether what they declare is @@ -245,21 +225,6 @@ def redispatch_legacy_options(method_name, args, opts, &) def first_hash_key_or_param(parameter) parameter.is_a?(Hash) ? parameter.keys.first : parameter end - - def map_params(params, element, is_array: false) - case params - when Array - params.map do |el| - map_params(el, element, is_array: true) - end - when Hash - params[element] || (@optional && is_array ? EmptyOptionalValue : {}) - when EmptyOptionalValue - EmptyOptionalValue - else - {} - end - end end end end diff --git a/lib/grape/validations/attributes_iterator.rb b/lib/grape/validations/attributes_iterator.rb index e4249cf2b..d058de69a 100644 --- a/lib/grape/validations/attributes_iterator.rb +++ b/lib/grape/validations/attributes_iterator.rb @@ -117,7 +117,7 @@ def store_indices(tracker, target_scope, index, parent_indices) # at the parameter parsing stage as they are required to ensure # the correct indexing is maintained def skip?(val) - val == Grape::DSL::Parameters::EmptyOptionalValue + ParamsScope::EmptyOptionalValue.equal?(val) end end end diff --git a/lib/grape/validations/params_scope.rb b/lib/grape/validations/params_scope.rb index 4d653a12f..654e7c368 100644 --- a/lib/grape/validations/params_scope.rb +++ b/lib/grape/validations/params_scope.rb @@ -16,6 +16,24 @@ def qualifying_params ParamScopeTracker.current&.qualifying_params(self) end + # @param params [Hash] initial hash of parameters + # @return hash of parameters relevant for the current scope + # @api private + # Only a +given+ scope ever stores qualifying params, and a scope on an + # independent chain has none above it (see #independent?), so the + # lookup and the +presence+ test that follows it are skipped -- both of + # them, at every level, on every call. + def params(params) + scoped = if @parent.nil? + params + elsif @independent + @parent.params(params) + else + @parent.qualifying_params.presence || @parent.params(params) + end + @element ? map_params(scoped, @element) : scoped + end + include Grape::DSL::Parameters include Grape::Validations::ParamsDocumentation @@ -33,6 +51,12 @@ def qualifying_params NOT_VALIDATED = Object.new.freeze private_constant :NOT_VALIDATED + # What {#params} hands back for an element of an Array that left an + # optional scope out, in place of that scope's params. Dropping the + # element instead would shift the indices the errors name, so it stays, + # and AttributesIterator and the validators pass over it. + EmptyOptionalValue = Object.new.freeze + class Attr attr_reader :key, :scope @@ -283,6 +307,21 @@ def push_declared_params(attrs, as: nil, declared_params_scope: self) private + def map_params(params, element, is_array: false) + case params + when Array + params.map do |el| + map_params(el, element, is_array: true) + end + when Hash + params[element] || (@optional && is_array ? EmptyOptionalValue : {}) + when EmptyOptionalValue + EmptyOptionalValue + else + {} + end + end + def build_full_path return @parent.full_path + [@element] if nested? return @parent.full_path if lateral? diff --git a/lib/grape/validations/validators/base.rb b/lib/grape/validations/validators/base.rb index d59f7260b..43984413f 100644 --- a/lib/grape/validations/validators/base.rb +++ b/lib/grape/validations/validators/base.rb @@ -178,7 +178,7 @@ def validate_elements!(elements) elements.each_with_index do |element, index| tracker&.store_index(scope, index) - next if Grape::DSL::Parameters::EmptyOptionalValue.equal?(element) + next if ParamsScope::EmptyOptionalValue.equal?(element) @attrs.each do |attr_name| validate_param!(attr_name, element) if required? || (hash_like?(element) && element.key?(attr_name)) @@ -203,7 +203,7 @@ def passed_over?(val, element) # Array instead of a Hash. An empty element of the scope's own Array is # passed over only when every element of that Array is empty. def empty_element?(element) - return true if Grape::DSL::Parameters::EmptyOptionalValue.equal?(element) + return true if ParamsScope::EmptyOptionalValue.equal?(element) element.respond_to?(:empty?) ? element.empty? : element.nil? end diff --git a/spec/grape/dsl/parameters_spec.rb b/spec/grape/dsl/parameters_spec.rb index ce5f11cee..6351b36f9 100644 --- a/spec/grape/dsl/parameters_spec.rb +++ b/spec/grape/dsl/parameters_spec.rb @@ -7,7 +7,7 @@ Class.new do include Grape::DSL::Parameters - attr_accessor :api, :element, :parent + attr_accessor :api def initialize @validates = [] @@ -302,34 +302,4 @@ def new_group_scope(group) expect { subject.requires(type: Integer) }.not_to raise_error end end - - describe '#params' do - it 'inherits params from parent' do - parent_params = { foo: 'bar' } - subject.parent = Object.new - allow(subject.parent).to receive_messages(params: parent_params, qualifying_params: nil) - expect(subject.params({})).to eq parent_params - end - - describe 'when params argument is an array of hashes' do - it 'returns values of each hash for @element key' do - subject.element = :foo - expect(subject.params([{ foo: 'bar' }, { foo: 'baz' }])).to eq(%w[bar baz]) - end - end - - describe 'when params argument is a hash' do - it 'returns value for @element key' do - subject.element = :foo - expect(subject.params(foo: 'bar')).to eq('bar') - end - end - - describe 'when params argument is not a array or a hash' do - it 'returns empty hash' do - subject.element = Object.new - expect(subject.params(Object.new)).to eq({}) - end - end - end end diff --git a/spec/grape/validations/attributes_iterator_spec.rb b/spec/grape/validations/attributes_iterator_spec.rb index fcac9153b..1b1dfa1f0 100644 --- a/spec/grape/validations/attributes_iterator_spec.rb +++ b/spec/grape/validations/attributes_iterator_spec.rb @@ -27,7 +27,7 @@ end context 'when an element is the empty optional placeholder' do - let(:params) { [Grape::DSL::Parameters::EmptyOptionalValue, 10] } + let(:params) { [Grape::Validations::ParamsScope::EmptyOptionalValue, 10] } it 'does not yield it' do expect { |b| iterator.each(params, &b) }.to yield_successive_args([params[1], false]) diff --git a/spec/grape/validations/params_scope_spec.rb b/spec/grape/validations/params_scope_spec.rb index 2c5b4f5c1..527e8219a 100644 --- a/spec/grape/validations/params_scope_spec.rb +++ b/spec/grape/validations/params_scope_spec.rb @@ -2241,6 +2241,54 @@ def initialize(value) end end + # A nested scope's block runs with that scope as self, which is how these + # examples get hold of one. + describe '#params' do + let(:api) { Class.new(Grape::API) } + + it 'answers the params it is given at the root' do + params = { foo: 'bar' } + expect(described_class.new(api:).params(params)).to be(params) + end + + it 'answers the value under its element' do + foo = nil + described_class.new(api:) do + requires :foo, type: Hash do + foo = self + requires :bar + end + end + expect(foo.params({ foo: { bar: 1 } })).to eq({ bar: 1 }) + end + + it 'answers the value under its element in each element of an Array further up' do + sub = nil + described_class.new(api:) do + requires :items, type: Array do + requires :sub, type: Hash do + sub = self + requires :a + end + end + end + expect(sub.params({ items: [{ sub: { a: 1 } }, { sub: { a: 2 } }] })).to eq([{ a: 1 }, { a: 2 }]) + end + + it 'answers an empty Hash when the scope above resolved to neither a Hash nor an Array' do + bar = nil + described_class.new(api:) do + requires :foo, type: Hash do + requires :bar, type: Hash do + bar = self + requires :baz + end + end + end + expect(bar.params({ foo: 'not a hash' })).to eq({}) + end + end + describe Grape::Validations::ParamsScope::Attr do let(:scope) { instance_double(Grape::Validations::ParamsScope) }