From 051b7da4c569e9485f8251c4fe9b41465a569b55 Mon Sep 17 00:00:00 2001 From: Eric Proulx Date: Fri, 8 May 2026 13:55:10 +0200 Subject: [PATCH] Fix Array/Set custom-type coercer dropping symbolized hash keys The Array/Set branch of `enforce_symbolized_keys` built a symbolized copy via non-mutating `deep_symbolize_keys` and discarded it, so the returned collection still carried string-keyed hashes. Switch to `map!` so the symbolized values replace the originals, matching the Hash branch's contract. While here, refactor the class for readability: hoist the `[Array, Set]` and `[:coerced?, :parsed?]` arrays to private frozen constants, lift trailing `if/else` chains into guard clauses, extract `hash_symbolizer` / `collection_symbolizer` / `enumerable_type_check` helpers, collapse the `infer_coercion_method` + symbolize-wrap chain into a single `build_coercion_method` entry point, and reorder the private methods to follow call order from `initialize`. Add `spec/grape/validations/types/custom_type_coercer_spec.rb` covering the Array and Set symbolization paths through the public API. The Array case is the regression test for the bug above. Co-Authored-By: Claude Opus 4.7 (1M context) --- CHANGELOG.md | 1 + .../validations/types/custom_type_coercer.rb | 111 ++++++------------ .../types/custom_type_coercer_spec.rb | 28 +++++ 3 files changed, 66 insertions(+), 74 deletions(-) create mode 100644 spec/grape/validations/types/custom_type_coercer_spec.rb diff --git a/CHANGELOG.md b/CHANGELOG.md index 887135197..99d3ad606 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -26,6 +26,7 @@ * [#2678](https://github.com/ruby-grape/grape/pull/2678): Update rubocop to 1.86.0 and autocorrect offenses - [@ericproulx](https://github.com/ericproulx). * [#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). * Your contribution here. ### 3.2.1 (2026-04-16) diff --git a/lib/grape/validations/types/custom_type_coercer.rb b/lib/grape/validations/types/custom_type_coercer.rb index 1f72a016f..a83badd57 100644 --- a/lib/grape/validations/types/custom_type_coercer.rb +++ b/lib/grape/validations/types/custom_type_coercer.rb @@ -33,6 +33,10 @@ module Types # contract as +coerced?+, and must be supplied with a coercion # +method+. class CustomTypeCoercer + TYPE_CHECK_METHODS = %i[coerced? parsed?].freeze + COLLECTION_TYPES = [Array, Set].freeze + private_constant :TYPE_CHECK_METHODS, :COLLECTION_TYPES + # A new coercer for the given type specification # and coercion method. # @@ -41,8 +45,7 @@ class CustomTypeCoercer # @param method [#parse,#call] # optional coercion method. See class docs. def initialize(type, method = nil) - coercion_method = infer_coercion_method type, method - @method = enforce_symbolized_keys type, coercion_method + @method = build_coercion_method(type, method) @type_check = infer_type_check(type) end @@ -66,12 +69,14 @@ def coerced?(val) private - # Determine the coercion method we're expected to use - # based on the parameters given. - # - # @param type see #new - # @param method see #new - # @return [#call] coercion method + def build_coercion_method(type, method) + coercion_method = infer_coercion_method(type, method) + return hash_symbolizer(coercion_method) if type == Hash + return collection_symbolizer(coercion_method) if COLLECTION_TYPES.include?(type) + + coercion_method + end + def infer_coercion_method(type, method) return type.method(:parse) unless method return method unless method.respond_to?(:parse) @@ -79,77 +84,35 @@ def infer_coercion_method(type, method) method.method(:parse) end - # Determine how the type validity of a coerced - # value should be decided. - # - # @param type see #new - # @return [#call] a procedure which accepts a single parameter - # and returns +true+ if the passed object is of the correct type. + def hash_symbolizer(method) + ->(val) { method.call(val).deep_symbolize_keys } + end + + def collection_symbolizer(method) + ->(val) { method.call(val).map! { |item| symbolize_if_hash(item) } } + end + + def symbolize_if_hash(item) + item.is_a?(Hash) ? item.deep_symbolize_keys : item + end + def infer_type_check(type) - # First check for special class methods - if type.respond_to? :coerced? - type.method :coerced? - elsif type.respond_to? :parsed? - type.method :parsed? - elsif type.respond_to? :call - # Arbitrary proc passed for type validation. - # Note that this will fail unless a method is also - # passed, or if the type also implements a parse() method. - type - elsif type.is_a?(Enumerable) - lambda do |value| - value.is_a?(Enumerable) && value.all? do |val| - recursive_type_check(type.first, val) - end - end - else - # By default, do a simple type check - ->(value) { value.is_a? type } - end + method_name = TYPE_CHECK_METHODS.detect { |m| type.respond_to?(m) } + return type.method(method_name) if method_name + return type if type.respond_to?(:call) + return enumerable_type_check(type) if type.is_a?(Enumerable) + + ->(value) { value.is_a? type } end - def recursive_type_check(type, value) - if type.is_a?(Enumerable) && value.is_a?(Enumerable) - value.all? { |val| recursive_type_check(type.first, val) } - else - !type.is_a?(Enumerable) && value.is_a?(type) - end + def enumerable_type_check(type) + ->(value) { value.is_a?(Enumerable) && value.all? { |val| recursive_type_check(type.first, val) } } end - # Enforce symbolized keys for complex types - # by wrapping the coercion method such that - # any Hash objects in the immediate heirarchy - # have their keys recursively symbolized. - # This helps common libs such as JSON to work easily. - # - # @param type see #new - # @param method see #infer_coercion_method - # @return [#call] +method+ wrapped in an additional - # key-conversion step, or just returns +method+ - # itself if no conversion is deemed to be - # necessary. - def enforce_symbolized_keys(type, method) - # Collections have all values processed individually - if [Array, Set].include?(type) - lambda do |val| - method.call(val).tap do |new_val| - new_val.map do |item| - item.is_a?(Hash) ? item.deep_symbolize_keys : item - end - end - end - - # Hash objects are processed directly - elsif type == Hash - lambda do |val| - method.call(val).deep_symbolize_keys - end - - # Simple types are not processed. - # This includes Array types. - else - method - end + def recursive_type_check(type, value) + return value.all? { |val| recursive_type_check(type.first, val) } if type.is_a?(Enumerable) && value.is_a?(Enumerable) + + !type.is_a?(Enumerable) && value.is_a?(type) end end end diff --git a/spec/grape/validations/types/custom_type_coercer_spec.rb b/spec/grape/validations/types/custom_type_coercer_spec.rb new file mode 100644 index 000000000..c4cb70b84 --- /dev/null +++ b/spec/grape/validations/types/custom_type_coercer_spec.rb @@ -0,0 +1,28 @@ +# frozen_string_literal: true + +describe Grape::Validations::Types::CustomTypeCoercer do + describe '#call' do + context 'when the type is a collection of hashes' do + subject(:coercer) { described_class.new(type, coerce_method) } + + let(:coerce_method) { ->(val) { JSON.parse(val) } } + + context 'with an Array type' do + let(:type) { Array } + + it 'symbolizes keys of nested hashes' do + expect(coercer.call('[{"foo":"bar"}]')).to eq([{ foo: 'bar' }]) + end + end + + context 'with a Set type' do + let(:type) { Set } + let(:coerce_method) { ->(val) { Set.new(JSON.parse(val)) } } + + it 'symbolizes keys of nested hashes' do + expect(coercer.call('[{"foo":"bar"}]')).to eq(Set[{ foo: 'bar' }]) + end + end + end + end +end