From acb86d10d07dcfc3577dbc4dc13882103efe3aa8 Mon Sep 17 00:00:00 2001 From: Eric Proulx Date: Sat, 9 May 2026 21:31:27 +0200 Subject: [PATCH] Make ParamsScope#validates pure via ValidationsSpec MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `Grape::Validations::ParamsScope#validates(attrs, validations)` used to consume the validations hash destructively across six helpers (`infer_coercion`, `derive_validator_options`, `validates_presence`, `coerce_type`, `document_params`, `extract_details`). The caller's hash was mutated in place; passing a frozen hash would crash the pipeline. Introduces `Grape::Validations::ValidationsSpec` — a small frozen value object that parses the raw hash once and exposes named accessors. `validates` becomes a thin dispatcher that hands the spec to the helpers; the raw hash is never written to. Helpers that used to read/delete are gone: * `infer_coercion` → `ValidationsSpec#parse_coerce` (private) * `derive_validator_options` → `ValidationsSpec#shared_opts` * `validates_presence` → `ParamsScope#validate_presence(spec, ...)` * `coerce_type` (the helper) → `ParamsScope#validate_coerce(spec, ...)` * `guess_coerce_type` → `ValidationsSpec#guess_coerce_type` (private) * `options_key?` → no longer needed; `ValidationsSpec#resolve_value` `ParamsDocumentation#document_params` now takes the spec instead of the raw hash and reads via `spec.raw[...]` without deletion. The `do_not_document` short-circuit no longer mutates either; it just returns early. Behaviour preserved: * Order of validator dispatch (presence → coerce → others). * `:message` belonging to presence is suppressed for later validators (it's in `SPEC_CONSUMED_KEYS`). * `:allow_blank` and `:length` remain dual-purpose (shared opt + own validator entry; doc source + own validator entry). * `check_coerce_with` still raises `ArgumentError` at definition time when `coerce_with:` is supplied without a type. Adds: * `lib/grape/validations/validations_spec.rb` — value object. * `spec/grape/validations/validations_spec_spec.rb` — unit tests including a frozen-hash test that asserts `from(frozen_hash)` does not raise. Incidentally fixes #2517: because `:message` is in `SPEC_CONSUMED_KEYS`, `optional :foo, message: 'oops'` no longer raises `UnknownValidator`. Regression spec added. Co-Authored-By: Claude Opus 4.7 (1M context) --- CHANGELOG.md | 2 + lib/grape/validations/params_documentation.rb | 35 ++-- lib/grape/validations/params_scope.rb | 161 ++++-------------- lib/grape/validations/validations_spec.rb | 115 +++++++++++++ .../validations/params_documentation_spec.rb | 41 +++-- .../validations/validations_spec_spec.rb | 104 +++++++++++ spec/grape/validations_spec.rb | 13 ++ 7 files changed, 309 insertions(+), 162 deletions(-) create mode 100644 lib/grape/validations/validations_spec.rb create mode 100644 spec/grape/validations/validations_spec_spec.rb diff --git a/CHANGELOG.md b/CHANGELOG.md index c0584c57c..303727704 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -22,6 +22,7 @@ * [#2693](https://github.com/ruby-grape/grape/pull/2693): Introduce `Grape::Exceptions::ErrorResponse` value object to replace the implicit-schema Hash thrown via `throw` - [@ericproulx](https://github.com/ericproulx). * [#2701](https://github.com/ruby-grape/grape/pull/2701): Replace `.tap` usages in `lib/` with explicit local variables - [@ericproulx](https://github.com/ericproulx). * [#2704](https://github.com/ruby-grape/grape/pull/2704): Add `Grape::Endpoint#logger` so the API's configured logger is reachable inside route handlers, filters, and `rescue_from` blocks without a helper - [@ericproulx](https://github.com/ericproulx). +* [#2706](https://github.com/ruby-grape/grape/pull/2706): Refactor `ParamsScope#validates` and `ParamsDocumentation` around a frozen `Grape::Validations::ValidationsSpec` value object; the validations hash supplied by the DSL is no longer mutated and the helper chain becomes pure - [@ericproulx](https://github.com/ericproulx). * Your contribution here. #### Fixes @@ -30,6 +31,7 @@ * [#2682](https://github.com/ruby-grape/grape/pull/2682): Fix `Style/OptionalBooleanParameter` offenses - [@ericproulx](https://github.com/ericproulx). * [#2699](https://github.com/ruby-grape/grape/pull/2699): Fix `Grape::Validations::Types::CustomTypeCoercer` dropping symbolized hash keys for `Array`/`Set` types; refactor the class for readability - [@ericproulx](https://github.com/ericproulx). * [#2700](https://github.com/ruby-grape/grape/pull/2700): Fix README typos, remove obsolete Ruby 2.4 / Fixnum section, and replace incorrect `requires + values + allow_blank` note with a correct one covering `optional + values` semantics (closes #2631) - [@ericproulx](https://github.com/ericproulx). +* [#2706](https://github.com/ruby-grape/grape/pull/2706): Fix `optional :foo, message: 'oops'` raising `UnknownValidator` - [@ericproulx](https://github.com/ericproulx). * Your contribution here. ### 3.2.1 (2026-04-16) diff --git a/lib/grape/validations/params_documentation.rb b/lib/grape/validations/params_documentation.rb index 22d55f21e..a17e80b0d 100644 --- a/lib/grape/validations/params_documentation.rb +++ b/lib/grape/validations/params_documentation.rb @@ -2,37 +2,38 @@ module Grape module Validations - # Documents parameters of an endpoint. If documentation isn't needed (for instance, it is an - # internal API), the class only cleans up attributes to avoid junk in RAM. - + # Documents parameters of an endpoint. Reads from a frozen + # +ValidationsSpec+; never mutates the user's validations hash. module ParamsDocumentation - def document_params(attrs, validations, type = nil, values = nil, except_values = nil) - return validations.except!(:desc, :description, :documentation) if @api.inheritable_setting.namespace_inheritable[:do_not_document] + def document_params(attrs, spec) + return if @api.inheritable_setting.namespace_inheritable[:do_not_document] documented_attrs = attrs.to_h do |name| - [full_name(name), extract_details(validations, type, values, except_values)] + [full_name(name), extract_details(spec)] end @api.inheritable_setting.namespace_stackable[:params] = documented_attrs end private - def extract_details(validations, type, values, except_values) + def extract_details(spec) details = {} - details[:required] = validations.key?(:presence) - details[:type] = TypeCache[type] if type - details[:values] = values if values - details[:except_values] = except_values if except_values - details[:default] = validations[:default] if validations.key?(:default) - if validations.key?(:length) - details[:min_length] = validations[:length][:min] if validations[:length].key?(:min) - details[:max_length] = validations[:length][:max] if validations[:length].key?(:max) + details[:required] = spec.required? + details[:type] = TypeCache[spec.coerce_type] if spec.coerce_type + details[:values] = spec.values if spec.values + details[:except_values] = spec.except_values if spec.except_values + details[:default] = spec.default unless spec.default.nil? + + length = spec.raw[:length] + if length.is_a?(Hash) + details[:min_length] = length[:min] if length.key?(:min) + details[:max_length] = length[:max] if length.key?(:max) end - desc = validations.delete(:desc) || validations.delete(:description) + desc = spec.raw[:desc] || spec.raw[:description] details[:desc] = desc if desc - documentation = validations.delete(:documentation) + documentation = spec.raw[:documentation] details[:documentation] = documentation if documentation details diff --git a/lib/grape/validations/params_scope.rb b/lib/grape/validations/params_scope.rb index b9002a7e7..c8650c958 100644 --- a/lib/grape/validations/params_scope.rb +++ b/lib/grape/validations/params_scope.rb @@ -334,129 +334,52 @@ def find_nearest_array_ancestor end def validates(attrs, validations) - coerce_type = infer_coercion(validations) - required = validations.key?(:presence) - default = validations[:default] - values = validations[:values].is_a?(Hash) ? validations.dig(:values, :value) : validations[:values] - except_values = validations[:except_values].is_a?(Hash) ? validations.dig(:except_values, :value) : validations[:except_values] + spec = ValidationsSpec.from(validations) - # NB. values and excepts should be nil, Proc, Array, or Range. - # Specifically, values should NOT be a Hash - # use values or excepts to guess coerce type when stated type is Array - coerce_type = guess_coerce_type(coerce_type, values, except_values) + check_incompatible_option_values(spec.default, spec.values, spec.except_values) + validate_value_coercion(spec.coerce_type, spec.values, spec.except_values) + document_params(attrs, spec) - # default value should be present in values array, if both exist and are not procs - check_incompatible_option_values(default, values, except_values) + # Presence runs first — `required` is forwarded to every subsequent + # validator (some short-circuit on it). + validate_presence(spec, attrs) - # type should be compatible with values array, if both exist - validate_value_coercion(coerce_type, values, except_values) + # Coerce runs second — later validators see the typed value. + validate_coerce(spec, attrs) - document_params attrs, validations, coerce_type, values, except_values - - opts = derive_validator_options(validations).freeze - - # Validate for presence before any other validators - validates_presence(validations, attrs, opts) - - # Before we run the rest of the validators, let's handle - # whatever coercion so that we are working with correctly - # type casted values - coerce_type validations.extract!(:coerce, :coerce_with, :coerce_message), attrs, required, opts - - validations.each do |type, options| - # Don't try to look up validators for documentation params that don't have one. - next if RESERVED_DOCUMENTATION_KEYWORDS.include?(type) - - validate(type, options, attrs, required, opts) + spec.validator_entries.each do |type, options| + validate(type, options, attrs, spec.required?, spec.shared_opts) end end - # Validate and comprehend the +:type+, +:types+, and +:coerce_with+ - # options that have been supplied to the parameter declaration. - # The +:type+ and +:types+ options will be removed from the - # validations list, replaced appropriately with +:coerce+ and - # +:coerce_with+ options that will later be passed to - # {Validators::CoerceValidator}. The type that is returned may be - # used for documentation and further validation of parameter - # options. - # - # @param validations [Hash] list of validations supplied to the - # parameter declaration - # @return [class-like] type to which the parameter will be coerced - # @raise [ArgumentError] if the given type options are invalid - def infer_coercion(validations) - raise ArgumentError, ':type may not be supplied with :types' if validations.key?(:type) && validations.key?(:types) - - validations[:coerce] = (options_key?(:type, :value, validations) ? validations[:type][:value] : validations[:type]) if validations.key?(:type) - validations[:coerce_message] = (options_key?(:type, :message, validations) ? validations[:type][:message] : nil) if validations.key?(:type) - validations[:coerce] = (options_key?(:types, :value, validations) ? validations[:types][:value] : validations[:types]) if validations.key?(:types) - validations[:coerce_message] = (options_key?(:types, :message, validations) ? validations[:types][:message] : nil) if validations.key?(:types) - - validations.delete(:types) if validations.key?(:types) - - coerce_type = validations[:coerce] - - # Special case - when the argument is a single type that is a - # variant-type collection. - if Types.multiple?(coerce_type) && validations.key?(:type) - validations[:coerce] = Types::VariantCollectionCoercer.new( - coerce_type, - validations.delete(:coerce_with) - ) - end - validations.delete(:type) - - coerce_type - end - - # Enforce correct usage of :coerce_with parameter. - # We do not allow coercion without a type, nor with - # +JSON+ as a type since this defines its own coercion - # method. - def check_coerce_with(validations) - return unless validations.key?(:coerce_with) - # type must be supplied for coerce_with.. - raise ArgumentError, 'must supply type for coerce_with' unless validations.key?(:coerce) - - # but not special JSON types, which - # already imply coercion method - return unless SPECIAL_JSON.include?(validations[:coerce]) + # Enforce correct usage of :coerce_with on a coerce_options hash. + # We do not allow coercion without a type, nor with +JSON+ as a type + # since that defines its own coercion method. + def check_coerce_with(coerce_options) + return unless coerce_options[:method] + raise ArgumentError, 'must supply type for coerce_with' unless coerce_options[:type] + return unless SPECIAL_JSON.include?(coerce_options[:type]) raise ArgumentError, 'coerce_with disallowed for type: JSON' end - # Add type coercion validation to this scope, - # if any has been specified. - # This validation has special handling since it is - # composited from more than one +requires+/+optional+ - # parameter, and needs to be run before most other - # validations. - def coerce_type(validations, attrs, required, opts) - check_coerce_with(validations) - - # Falsy check (not key?) is intentional: when a remountable API is first - # evaluated on its base instance (no configuration supplied yet), - # configuration[:some_type] evaluates to nil. Skipping instantiation - # here is correct — the real mounted instance will replay this step with - # the actual type value. - return unless validations[:coerce] + def validate_presence(spec, attrs) + return unless spec.required? - coerce_options = { - type: validations[:coerce], - method: validations[:coerce_with], - message: validations[:coerce_message] - } - validate('coerce', coerce_options, attrs, required, opts) + validate('presence', spec.presence_options, attrs, true, spec.shared_opts) end - def guess_coerce_type(coerce_type, *values_list) - return coerce_type unless coerce_type == Array + def validate_coerce(spec, attrs) + coerce_options = spec.coerce_options + check_coerce_with(coerce_options) + # Falsy check is intentional: when a remountable API is first evaluated + # on its base instance (no configuration supplied yet), + # configuration[:some_type] evaluates to nil. Skipping instantiation + # here is correct — the real mounted instance will replay this step + # with the actual type value. + return unless coerce_options[:type] - values_list.each do |values| - next if !values || values.is_a?(Proc) - return values.first.class if values.is_a?(Range) || !values.empty? - end - coerce_type + validate('coerce', coerce_options, attrs, spec.required?, spec.shared_opts) end def check_incompatible_option_values(default, values, except_values) @@ -494,31 +417,9 @@ def validate_value_coercion(coerce_type, *values_list) end end - def options_key?(type, key, validations) - validations[type].respond_to?(:key?) && validations[type].key?(key) && !validations[type][key].nil? - end - def all_element_blank?(scoped_params) scoped_params.respond_to?(:all?) && scoped_params.all?(&:blank?) end - - # Validators don't have access to each other and they don't need, however, - # some validators might influence others, so their options should be shared - def derive_validator_options(validations) - allow_blank = validations[:allow_blank] - - { - allow_blank: allow_blank.is_a?(Hash) ? allow_blank[:value] : allow_blank, - fail_fast: validations.delete(:fail_fast) || false - } - end - - def validates_presence(validations, attrs, opts) - return unless validations.key?(:presence) && validations[:presence] - - validate('presence', validations.delete(:presence), attrs, true, opts) - validations.delete(:message) if validations.key?(:message) - end end end end diff --git a/lib/grape/validations/validations_spec.rb b/lib/grape/validations/validations_spec.rb new file mode 100644 index 000000000..40f3b549b --- /dev/null +++ b/lib/grape/validations/validations_spec.rb @@ -0,0 +1,115 @@ +# frozen_string_literal: true + +module Grape + module Validations + # Frozen value object holding everything {ParamsScope#validates} needs to + # know about a single +requires+/+optional+ declaration. Built once from + # the raw validations hash supplied by the DSL; the raw hash is never + # mutated. + # + # Splits the raw entries into three logical buckets: + # + # * Spec-consumed keys (type/types/coerce*, presence/message, + # default/fail_fast, doc keys) — exposed via named accessors and never + # handed to validator dispatch. + # * Shared opts (allow_blank, fail_fast) — read by every validator at + # construction time via {#shared_opts}. + # * Validator entries (everything else, e.g. +regexp+, +length+, +values+, + # +allow_blank+, custom validators) — exposed via {#validator_entries} + # for the dispatch loop. + # + # Same key can land in more than one bucket (e.g. +allow_blank+ is both a + # shared opt and a validator entry; +length+ is both a doc source and a + # validator entry). + class ValidationsSpec + # Keys consumed by the spec itself; must NOT be dispatched as validators + # by the caller. Documentation-only keys are filtered through a separate + # set so that dual-purpose keys (length, default, values, except_values) + # aren't accidentally swallowed. + SPEC_CONSUMED_KEYS = %i[ + type types coerce coerce_with coerce_message + presence message + fail_fast + desc description documentation + ].freeze + + attr_reader :raw, :coerce_type, :coerce_method, :coerce_message, :presence_options, :values, :except_values, :default, :allow_blank, :fail_fast, :shared_opts, :validator_entries + + def self.from(validations) + new(validations) + end + + def initialize(raw) + raise ArgumentError, ':type may not be supplied with :types' if raw.key?(:type) && raw.key?(:types) + + @raw = raw + @coerce_type, @coerce_message, @coerce_method = parse_coerce(raw) + @values = resolve_value(raw[:values]) + @except_values = resolve_value(raw[:except_values]) + @default = raw[:default] + @presence_options = raw[:presence] + @allow_blank = resolve_value(raw[:allow_blank]) + @fail_fast = raw[:fail_fast] || false + + @coerce_type = guess_coerce_type(@coerce_type, @values, @except_values) + @shared_opts = { allow_blank: @allow_blank, fail_fast: @fail_fast }.freeze + @validator_entries = build_validator_entries(raw) + + freeze + end + + def required? + !@presence_options.nil? && @presence_options != false + end + + def coerce_options + { type: @coerce_type, method: @coerce_method, message: @coerce_message } + end + + private + + def build_validator_entries(raw) + raw.reject do |k, _| + SPEC_CONSUMED_KEYS.include?(k) || ParamsScope::RESERVED_DOCUMENTATION_KEYWORDS.include?(k) + end.freeze + end + + def parse_coerce(raw) + if raw.key?(:type) + coerce, coerce_message = extract_value_and_message(raw[:type]) + coerce_with = raw[:coerce_with] + return [Types::VariantCollectionCoercer.new(coerce, coerce_with), coerce_message, nil] if Types.multiple?(coerce) + elsif raw.key?(:types) + coerce, coerce_message = extract_value_and_message(raw[:types]) + coerce_with = raw[:coerce_with] + else + coerce = raw[:coerce] + coerce_message = raw[:coerce_message] + coerce_with = raw[:coerce_with] + end + + [coerce, coerce_message, coerce_with] + end + + def extract_value_and_message(opt) + return [opt, nil] unless opt.is_a?(Hash) + + [opt[:value], opt[:message]] + end + + def resolve_value(opt) + opt.is_a?(Hash) ? opt[:value] : opt + end + + def guess_coerce_type(coerce_type, *values_list) + return coerce_type unless coerce_type == Array + + values_list.each do |values| + next if !values || values.is_a?(Proc) + return values.first.class if values.is_a?(Range) || !values.empty? + end + coerce_type + end + end + end +end diff --git a/spec/grape/validations/params_documentation_spec.rb b/spec/grape/validations/params_documentation_spec.rb index a88b171f5..e6acedb07 100644 --- a/spec/grape/validations/params_documentation_spec.rb +++ b/spec/grape/validations/params_documentation_spec.rb @@ -25,22 +25,27 @@ def full_name(name) end end + def spec_for(validations) + Grape::Validations::ValidationsSpec.from(validations) + end + describe '#document_params' do it 'stores documented params with all details' do attrs = %i[foo bar] validations = { presence: true, + type: Integer, default: 42, length: { min: 1, max: 10 }, desc: 'A foo', - documentation: { note: 'doc' } + documentation: { note: 'doc' }, + values: [1, 2, 3], + except_values: [4, 5, 6] } - type = Integer - values = [1, 2, 3] - except_values = [4, 5, 6] - subject.document_params(attrs, validations.dup, type, values, except_values) - expect(api_double.inheritable_setting.namespace_stackable[:params].first.keys).to include('full_name_foo', 'full_name_bar') - expect(api_double.inheritable_setting.namespace_stackable[:params].first['full_name_foo']).to include( + subject.document_params(attrs, spec_for(validations)) + stored = api_double.inheritable_setting.namespace_stackable[:params].first + expect(stored.keys).to include('full_name_foo', 'full_name_bar') + expect(stored['full_name_foo']).to include( required: true, type: 'Integer', values: [1, 2, 3], @@ -62,9 +67,15 @@ def full_name(name) api_double.inheritable_setting.namespace_inheritable[:do_not_document] = true end - it 'removes desc, description, and documentation' do - subject.document_params([:foo], validations) - expect(validations).to eq({ another_param: 'test' }) + it 'does not store any documented params' do + subject.document_params([:foo], spec_for(validations)) + expect(api_double.inheritable_setting.namespace_stackable[:params]).to be_empty + end + + it 'does not mutate the input validations' do + original = validations.dup + subject.document_params([:foo], spec_for(validations)) + expect(validations).to eq(original) end end @@ -74,7 +85,7 @@ def full_name(name) end it 'does not raise an error' do - expect { subject.document_params([:foo], validations) }.not_to raise_error + expect { subject.document_params([:foo], spec_for(validations)) }.not_to raise_error expect(api_double.inheritable_setting.namespace_stackable[:params].first['full_name_foo']).to eq({ required: false }) end end @@ -85,7 +96,7 @@ def full_name(name) end it 'uses description if desc is not present' do - subject.document_params([:foo], validations) + subject.document_params([:foo], spec_for(validations)) expect(api_double.inheritable_setting.namespace_stackable[:params].first['full_name_foo'][:desc]).to eq('desc2') end end @@ -96,7 +107,7 @@ def full_name(name) end it 'uses description if desc is not present' do - subject.document_params([:foo], validations) + subject.document_params([:foo], spec_for(validations)) expect(api_double.inheritable_setting.namespace_stackable[:params].first['full_name_foo']).to eq({ required: false }) end end @@ -107,7 +118,7 @@ def full_name(name) end it 'does not include documentation' do - subject.document_params([:foo], validations) + subject.document_params([:foo], spec_for(validations)) expect(api_double.inheritable_setting.namespace_stackable[:params].first['full_name_foo']).not_to have_key(:documentation) end end @@ -118,7 +129,7 @@ def full_name(name) end it 'sets type as nil' do - subject.document_params([:foo], validations) + subject.document_params([:foo], spec_for(validations)) expect(api_double.inheritable_setting.namespace_stackable[:params].first['full_name_foo'][:type]).to be_nil end end diff --git a/spec/grape/validations/validations_spec_spec.rb b/spec/grape/validations/validations_spec_spec.rb new file mode 100644 index 000000000..8414a8d31 --- /dev/null +++ b/spec/grape/validations/validations_spec_spec.rb @@ -0,0 +1,104 @@ +# frozen_string_literal: true + +describe Grape::Validations::ValidationsSpec do + describe '.from' do + it 'leaves the input hash untouched' do + raw = { type: Integer, presence: { value: true, message: nil }, length: { min: 1 } } + original = raw.dup + described_class.from(raw) + expect(raw).to eq(original) + end + + it 'accepts a frozen input hash without raising' do + raw = { type: Integer, presence: { value: true, message: nil } }.freeze + expect { described_class.from(raw) }.not_to raise_error + end + + it 'raises when both :type and :types are supplied' do + expect { described_class.from(type: Integer, types: [String, Integer]) } + .to raise_error(ArgumentError, ':type may not be supplied with :types') + end + end + + describe 'coerce parsing' do + it 'extracts a plain :type into coerce_type' do + spec = described_class.from(type: Integer) + expect(spec.coerce_type).to eq(Integer) + expect(spec.coerce_message).to be_nil + end + + it 'extracts the value/message form of :type' do + spec = described_class.from(type: { value: Integer, message: 'must be int' }) + expect(spec.coerce_type).to eq(Integer) + expect(spec.coerce_message).to eq('must be int') + end + + it 'wraps a multiple type from :type in a VariantCollectionCoercer' do + multi_spec = described_class.from(type: [Integer, String]) + expect(multi_spec.coerce_type).to be_a(Grape::Validations::Types::VariantCollectionCoercer) + expect(multi_spec.coerce_method).to be_nil + end + + it 'extracts :types as a plain coerce list' do + spec = described_class.from(types: [Integer, String]) + expect(spec.coerce_type).to eq([Integer, String]) + end + end + + describe 'shared opts' do + it 'exposes allow_blank and fail_fast as a frozen hash' do + spec = described_class.from(allow_blank: false, fail_fast: true) + expect(spec.shared_opts).to eq(allow_blank: false, fail_fast: true) + expect(spec.shared_opts).to be_frozen + end + + it 'unwraps the value/message form of allow_blank' do + spec = described_class.from(allow_blank: { value: false, message: 'no blanks' }) + expect(spec.shared_opts[:allow_blank]).to be(false) + end + + it 'defaults fail_fast to false when absent' do + spec = described_class.from({}) + expect(spec.shared_opts[:fail_fast]).to be(false) + end + end + + describe 'required?' do + it 'is true when :presence is set to a truthy value' do + expect(described_class.from(presence: { value: true })).to be_required + end + + it 'is false when :presence is absent' do + expect(described_class.from({})).not_to be_required + end + + it 'is false when :presence is explicitly false' do + expect(described_class.from(presence: false)).not_to be_required + end + end + + describe 'validator_entries' do + it 'excludes spec-consumed keys' do + spec = described_class.from( + type: Integer, + presence: { value: true }, + message: 'oops', + fail_fast: true, + desc: 'foo', + regexp: /\d+/, + values: [1, 2, 3] + ) + expect(spec.validator_entries.keys).to contain_exactly(:regexp, :values) + end + + it 'keeps :allow_blank and :length even though they have other roles' do + spec = described_class.from(allow_blank: false, length: { min: 1 }) + expect(spec.validator_entries.keys).to contain_exactly(:allow_blank, :length) + end + + it 'excludes documentation-only keys' do + spec = described_class.from(as: :other, required: true, format: :json) + expect(spec.validator_entries).to be_empty + end + end +end diff --git a/spec/grape/validations_spec.rb b/spec/grape/validations_spec.rb index f3bdc3186..9080710b4 100644 --- a/spec/grape/validations_spec.rb +++ b/spec/grape/validations_spec.rb @@ -45,6 +45,19 @@ response = Rack::MockResponse[*subject.call(env)] expect(JSON.parse(response.body)).to eq('some_param' => nil) end + + # Regression for https://github.com/ruby-grape/grape/issues/2517 — + # `optional :foo, message: 'oops'` used to raise UnknownValidator + # because `:message` survived dispatch when `:presence` was absent. + it 'accepts a :message option without raising' do + subject.params do + optional :foo, message: 'is required' + end + subject.get('/with_message') { 'ok' } + get '/with_message' + expect(last_response.status).to eq(200) + expect(last_response.body).to eq('ok') + end end context 'optional using Grape::Entity documentation' do