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)