From 1fdd620e60400a33cae828d1d0e43f45d7bdc392 Mon Sep 17 00:00:00 2001 From: Eric Proulx Date: Fri, 11 Sep 2026 11:20:02 +0200 Subject: [PATCH] Validate the elements of an Array params scope without the attributes iterator A validator on `requires :items, type: Array do ... end` (or `optional`) went through the AttributesIterator for every element: an index bookkeeping call, a yield per attribute, an emptiness test, and per attribute the scope's `required?` and `meets_dependency?`. When the scope depends on no other param, every scope above it is always validated, and it is the only one on the chain that iterates elements, those answers are fixed: its params are the request's Array as it came in, and the iterator only contributes the element index the error names carry and, for an optional scope, passing over empty elements. `Validators::Base#validate_elements!` covers that case the way #2927's `validate_attributes!` covers a Hash scope. `ParamsScope#validated_when_given?` names the condition, and `always_validated?` is that plus being required; a required scope like that also skips `should_validate?`, which always answers true for it. Nested Array scopes and `given` blocks keep the iterator. Adds three specs for behaviour nothing pinned, each found by a mutation that passed the suite: an optional Array scope skips its empty elements, it is not validated at all when every element is blank, and a Hash scope given an Array reports only itself, not the optional Array scope inside. Co-Authored-By: Claude Opus 5 --- CHANGELOG.md | 1 + lib/grape/validations/params_scope.rb | 11 ++++- lib/grape/validations/validators/base.rb | 54 +++++++++++++++++++-- spec/grape/validations/params_scope_spec.rb | 17 +++++++ spec/grape/validations_spec.rb | 26 ++++++++++ 5 files changed, 104 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 333801352..1f221cf2f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,7 @@ * [#2917](https://github.com/ruby-grape/grape/pull/2917): Read path captures out of the router's union match instead of re-running the route's pattern - [@ericproulx](https://github.com/ericproulx). * [#2921](https://github.com/ruby-grape/grape/pull/2921): Pin the router's request-time isolation regressions through requests instead of its instance variables - [@ericproulx](https://github.com/ericproulx). * [#2927](https://github.com/ruby-grape/grape/pull/2927): Validate required Hash params scopes without the attributes iterator - [@ericproulx](https://github.com/ericproulx). +* [#2933](https://github.com/ruby-grape/grape/pull/2933): Validate the elements of an Array params scope without the attributes iterator - [@ericproulx](https://github.com/ericproulx). * Your contribution here. ### 4.0.0 (2026-09-07) diff --git a/lib/grape/validations/params_scope.rb b/lib/grape/validations/params_scope.rb index 90b0b7503..c4c755be1 100644 --- a/lib/grape/validations/params_scope.rb +++ b/lib/grape/validations/params_scope.rb @@ -171,7 +171,16 @@ def root? # neither the params nor the parent chain have anything to say. # @return [Boolean] def always_validated? - !@optional && !@dependent_on && (@parent.nil? || @parent.always_validated?) + !@optional && validated_when_given? + end + + # Whether #should_validate? has nothing to ask but whether this scope's + # own params were given: it depends on no other param, and every scope + # above it is always validated. A required scope like that is then + # always validated, an optional one whenever its params are there. + # @return [Boolean] + def validated_when_given? + !@dependent_on && (@parent.nil? || @parent.always_validated?) end # A nested scope is contained in one of its parent's elements. diff --git a/lib/grape/validations/validators/base.rb b/lib/grape/validations/validators/base.rb index f37e26329..2881fe5fd 100644 --- a/lib/grape/validations/validators/base.rb +++ b/lib/grape/validations/validators/base.rb @@ -70,9 +70,11 @@ def initialize(attrs, options, required, scope, opts) @exception_message = message(self.class.default_message_key) if self.class.default_message_key @iterator = iterator_class.new(@attrs, @scope).freeze # A scope's parent, optionality, dependency and type are all set - # before its block declares anything, so this is settled by the time - # a validator is built. See #validate!. - @direct = scope.always_validated? && !scope.iterates_elements? + # before its block declares anything, so these are settled by the + # time a validator is built. See #validate and #validate!. + @always_validated = scope.always_validated? + @direct = @always_validated && !scope.iterates_elements? + @direct_elements = scope.validated_when_given? && scope.iterates_elements? && scope.array_depth == 1 end # Validates a given request. @@ -82,7 +84,7 @@ def initialize(attrs, options, required, scope, opts) # @return [void] def validate(request) params = request.params - return unless @direct || scope.should_validate?(params) + return unless @always_validated || scope.should_validate?(params) validate!(params) end @@ -94,6 +96,8 @@ def validate(request) # @raise [Grape::Exceptions::Validation] if validation failed # @return [void] def validate!(params) + return validate_elements!(scope.params(params)) if @direct_elements + scoped = scope.params(params) if @direct return validate_attributes!(scoped) if scoped.is_a?(Hash) @@ -149,6 +153,48 @@ def validate_attributes!(params) raise Grape::Exceptions::ValidationArrayErrors.new(array_errors) if array_errors end + # #validate_attributes! for each element of an Array scope, such as + # +requires :items, type: Array do+ at the root, when it is the only + # scope on the chain that iterates elements: its params are then the + # request's Array as it came in, with no nesting for the iterator to + # descend into. It depends on no other param and every scope above it + # is always validated, so the iterator's per-element checks come down + # to the index it records for the error names and, for an optional + # scope, passing over an empty element. An element that is not a Hash + # goes through the same +hash_like?+ test as on the iterator path, and + # the members of a scope that did not get an Array are left alone, as + # they are there: the scope's own type check reports it. + def validate_elements!(elements) + return unless elements.is_a?(Array) + + tracker = ParamScopeTracker.current + optional = !scope.required? + array_errors = nil + + elements.each_with_index do |element, index| + tracker&.store_index(scope, index) + next if optional && empty_element?(element) + + @attrs.each do |attr_name| + validate_param!(attr_name, element) if required? || (hash_like?(element) && element.key?(attr_name)) + rescue Grape::Exceptions::Validation => e + (array_errors ||= []) << e + end + end + + raise Grape::Exceptions::ValidationArrayErrors.new(array_errors) if array_errors + end + + # What the iterator passes over in an optional scope: an element given + # empty, or the placeholder +map_params+ puts where an optional scope's + # params were not given at all, which it can only do here when a scope + # above was handed an Array instead of a Hash. + def empty_element?(element) + return true if Grape::DSL::Parameters::EmptyOptionalValue.equal?(element) + + element.respond_to?(:empty?) ? element.empty? : element.nil? + end + # The AttributesIterator subclass used to walk this validator's # attributes. Built once in #initialize and reused across requests. def iterator_class diff --git a/spec/grape/validations/params_scope_spec.rb b/spec/grape/validations/params_scope_spec.rb index 668a2cb6c..c4e8baccd 100644 --- a/spec/grape/validations/params_scope_spec.rb +++ b/spec/grape/validations/params_scope_spec.rb @@ -227,6 +227,23 @@ def initialize(value) end end + context 'hash group given an Array' do + it 'reports the group as invalid without validating the optional array group inside it' do + subject.params do + requires :meta, type: Hash do + optional :items, type: Array do + requires :id, type: Integer + end + end + end + subject.post('/meta') { 'ok' } + + post '/meta', { meta: [{}] }.to_json, 'CONTENT_TYPE' => 'application/json' + expect(last_response.status).to eq(400) + expect(last_response.body).to eq('meta is invalid') + end + end + context 'coercing values validation with a variant-member-type collection' do it 'accepts values compatible with the declared member types' do expect do diff --git a/spec/grape/validations_spec.rb b/spec/grape/validations_spec.rb index ebe51b742..7d0f30cb7 100644 --- a/spec/grape/validations_spec.rb +++ b/spec/grape/validations_spec.rb @@ -881,6 +881,32 @@ def validate_param!(attr_name, params) expect(last_response.body).to eq('items[0][key] is missing') end + it 'skips the elements that are empty' do + subject.params do + optional :items, type: Array do + requires :key + end + end + subject.post('/optional_group') { 'optional group works' } + + post_with_json '/optional_group', items: [{}, { not_key: 'foo' }] + expect(last_response.status).to eq(400) + expect(last_response.body).to eq('items[1][key] is missing') + end + + it "doesn't validate the group when every element is blank" do + subject.params do + optional :items, type: Array do + requires :key + end + end + subject.post('/optional_group') { 'optional group works' } + + post_with_json '/optional_group', items: [false, ' '] + expect(last_response.status).to eq(201) + expect(last_response.body).to eq('optional group works') + end + it "errors when param is present but isn't an Array" do get '/optional_group', items: 'hello' expect(last_response.status).to eq(400)