diff --git a/CHANGELOG.md b/CHANGELOG.md index fdc7d3a3e..333801352 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,7 @@ * [#2918](https://github.com/ruby-grape/grape/pull/2918): Skip the dry-types round trip when a value already is the declared type - [@ericproulx](https://github.com/ericproulx). * [#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). * 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 be26ab992..90b0b7503 100644 --- a/lib/grape/validations/params_scope.rb +++ b/lib/grape/validations/params_scope.rb @@ -166,6 +166,14 @@ def root? !@parent end + # Whether #should_validate? answers true for every request: this scope + # and each of its ancestors is required and depends on nothing, so + # neither the params nor the parent chain have anything to say. + # @return [Boolean] + def always_validated? + !@optional && !@dependent_on && (@parent.nil? || @parent.always_validated?) + end + # A nested scope is contained in one of its parent's elements. # @return [Boolean] whether or not this scope is nested def nested? diff --git a/lib/grape/validations/validators/base.rb b/lib/grape/validations/validators/base.rb index 735661eff..f37e26329 100644 --- a/lib/grape/validations/validators/base.rb +++ b/lib/grape/validations/validators/base.rb @@ -69,6 +69,10 @@ def initialize(attrs, options, required, scope, opts) @opts = SharedOptions.new(**opts.slice(:allow_blank, :fail_fast)) @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? end # Validates a given request. @@ -78,7 +82,7 @@ def initialize(attrs, options, required, scope, opts) # @return [void] def validate(request) params = request.params - return unless scope.should_validate?(params) + return unless @direct || scope.should_validate?(params) validate!(params) end @@ -90,6 +94,9 @@ def validate(request) # @raise [Grape::Exceptions::Validation] if validation failed # @return [void] def validate!(params) + scoped = scope.params(params) if @direct + return validate_attributes!(scoped) if scoped.is_a?(Hash) + # we collect errors inside array because # there may be more than one error per field array_errors = nil @@ -123,6 +130,25 @@ def validate_param!(attr_name, params) alias required? required + # #validate! on a scope that always validates and does not iterate + # elements, once its params resolved to a Hash: the root scope and the + # required Hash scopes under it, which is most validators of most + # endpoints. There the iterator hands back that Hash once per + # attribute, and every scope on the chain is required with no + # dependency, so the per-attribute checks reduce to this. The + # machinery cost more than the validation itself. + def validate_attributes!(params) + array_errors = nil + + @attrs.each do |attr_name| + validate_param!(attr_name, params) if required? || params.key?(attr_name) + rescue Grape::Exceptions::Validation => e + (array_errors ||= []) << e + end + + raise Grape::Exceptions::ValidationArrayErrors.new(array_errors) if array_errors + 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 099e9a6f0..668a2cb6c 100644 --- a/spec/grape/validations/params_scope_spec.rb +++ b/spec/grape/validations/params_scope_spec.rb @@ -209,6 +209,24 @@ def initialize(value) end end + # An Array group handed something other than an Array fails its own type + # check, and its members are not then validated against that value as + # though it were one element. + context 'array group given a Hash' do + it 'reports the group as invalid without validating its members' do + subject.params do + requires :items, type: Array do + requires :id, type: Integer + end + end + subject.post('/items') { 'ok' } + + post '/items', { items: { name: 'x' } }.to_json, 'CONTENT_TYPE' => 'application/json' + expect(last_response.status).to eq(400) + expect(last_response.body).to eq('items 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