From 18ed28c5c1e8dd99f23d66a97408e421e290c87e Mon Sep 17 00:00:00 2001 From: Dmitriy Nesteryuk Date: Sun, 17 May 2020 00:38:30 +0300 Subject: [PATCH] coercing of nested arrays (#2054) --- CHANGELOG.md | 1 + lib/grape/validations/types/array_coercer.rb | 17 ++++++--- lib/grape/validations/types/build_coercer.rb | 6 +--- .../validations/types/dry_type_coercer.rb | 35 ++++++++++++++++++- lib/grape/validations/types/set_coercer.rb | 10 +++--- .../validations/types/array_coercer_spec.rb | 35 +++++++++++++++++++ .../types/primitive_coercer_spec.rb | 2 +- .../validations/types/set_coercer_spec.rb | 34 ++++++++++++++++++ 8 files changed, 124 insertions(+), 16 deletions(-) create mode 100644 spec/grape/validations/types/array_coercer_spec.rb create mode 100644 spec/grape/validations/types/set_coercer_spec.rb diff --git a/CHANGELOG.md b/CHANGELOG.md index c17ac74323..dd6324d7fe 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,7 @@ * [#2049](https://github.com/ruby-grape/grape/pull/2049): Coerce an empty string to nil in case of the bool type - [@dnesteryuk](https://github.com/dnesteryuk). * [#2043](https://github.com/ruby-grape/grape/pull/2043): Modify declared for nested array and hash - [@kadotami](https://github.com/kadotami). * [#2040](https://github.com/ruby-grape/grape/pull/2040): Fix a regression with Array of type nil - [@ericproulx](https://github.com/ericproulx). +* [#2054](https://github.com/ruby-grape/grape/pull/2054): Coercing of nested arrays - [@dnesteryuk](https://github.com/dnesteryuk). * Your contribution here. ### 1.3.2 (2020/04/12) diff --git a/lib/grape/validations/types/array_coercer.rb b/lib/grape/validations/types/array_coercer.rb index 8dd819e0d7..d3aeb21464 100644 --- a/lib/grape/validations/types/array_coercer.rb +++ b/lib/grape/validations/types/array_coercer.rb @@ -6,7 +6,7 @@ module Grape module Validations module Types # Coerces elements in an array. It might be an array of strings or integers or - # anything else. + # an array of arrays of integers. # # It could've been possible to use an +of+ # method (https://dry-rb.org/gems/dry-types/1.2/array-with-member/) @@ -14,16 +14,17 @@ module Types # behavior of Virtus which was used earlier, a `Grape::Validations::Types::PrimitiveCoercer` # maintains Virtus behavior in coercing. class ArrayCoercer < DryTypeCoercer + register_collection Array + def initialize(type, strict = false) super @coercer = scope::Array - @elem_coercer = PrimitiveCoercer.new(type.first, strict) + @subtype = type.first end def call(_val) collection = super - return collection if collection.is_a?(InvalidValue) coerce_elements collection @@ -31,13 +32,15 @@ def call(_val) protected + attr_reader :subtype + def coerce_elements(collection) return if collection.nil? collection.each_with_index do |elem, index| return InvalidValue.new if reject?(elem) - coerced_elem = @elem_coercer.call(elem) + coerced_elem = elem_coercer.call(elem) return coerced_elem if coerced_elem.is_a?(InvalidValue) @@ -47,11 +50,15 @@ def coerce_elements(collection) collection end - # This method maintaine logic which was defined by Virtus for arrays. + # This method maintains logic which was defined by Virtus for arrays. # Virtus doesn't allow nil in arrays. def reject?(val) val.nil? end + + def elem_coercer + @elem_coercer ||= DryTypeCoercer.coercer_instance_for(subtype, strict) + end end end end diff --git a/lib/grape/validations/types/build_coercer.rb b/lib/grape/validations/types/build_coercer.rb index b867485c16..c55e048dbd 100644 --- a/lib/grape/validations/types/build_coercer.rb +++ b/lib/grape/validations/types/build_coercer.rb @@ -60,12 +60,8 @@ def self.create_coercer_instance(type, method, strict) Types::CustomTypeCollectionCoercer.new( Types.map_special(type.first), type.is_a?(Set) ) - elsif type.is_a?(Array) - ArrayCoercer.new type, strict - elsif type.is_a?(Set) - SetCoercer.new type, strict else - PrimitiveCoercer.new type, strict + DryTypeCoercer.coercer_instance_for(type, strict) end end diff --git a/lib/grape/validations/types/dry_type_coercer.rb b/lib/grape/validations/types/dry_type_coercer.rb index 06d234d550..0a682e53e8 100644 --- a/lib/grape/validations/types/dry_type_coercer.rb +++ b/lib/grape/validations/types/dry_type_coercer.rb @@ -17,8 +17,41 @@ module Types # but check its type. More information there # https://dry-rb.org/gems/dry-types/1.2/built-in-types/ class DryTypeCoercer + class << self + # Registers a collection coercer which could be found by a type, + # see +collection_coercer_for+ method below. This method is meant for inheritors. + def register_collection(type) + DryTypeCoercer.collection_coercers[type] = self + end + + # Returns a collection coercer which corresponds to a given type. + # Example: + # + # collection_coercer_for(Array) + # #=> Grape::Validations::Types::ArrayCoercer + def collection_coercer_for(type) + collection_coercers[type] + end + + # Returns an instance of a coercer for a given type + def coercer_instance_for(type, strict = false) + return PrimitiveCoercer.new(type, strict) if type.class == Class + + # in case of a collection (Array[Integer]) the type is an instance of a collection, + # so we need to figure out the actual type + collection_coercer_for(type.class).new(type, strict) + end + + protected + + def collection_coercers + @collection_coercers ||= {} + end + end + def initialize(type, strict = false) @type = type + @strict = strict @scope = strict ? DryTypes::Strict : DryTypes::Params end @@ -36,7 +69,7 @@ def call(val) protected - attr_reader :scope, :type + attr_reader :scope, :type, :strict end end end diff --git a/lib/grape/validations/types/set_coercer.rb b/lib/grape/validations/types/set_coercer.rb index 1ddc887c21..dc76fc7733 100644 --- a/lib/grape/validations/types/set_coercer.rb +++ b/lib/grape/validations/types/set_coercer.rb @@ -1,18 +1,20 @@ # frozen_string_literal: true require 'set' -require_relative 'dry_type_coercer' +require_relative 'array_coercer' module Grape module Validations module Types # Takes the given array and converts it to a set. Every element of the set # is also coerced. - class SetCoercer < DryTypeCoercer + class SetCoercer < ArrayCoercer + register_collection Set + def initialize(type, strict = false) super - @elem_coercer = PrimitiveCoercer.new(type.first, strict) + @coercer = nil end def call(value) @@ -25,7 +27,7 @@ def call(value) def coerce_elements(collection) collection.each_with_object(Set.new) do |elem, memo| - coerced_elem = @elem_coercer.call(elem) + coerced_elem = elem_coercer.call(elem) return coerced_elem if coerced_elem.is_a?(InvalidValue) diff --git a/spec/grape/validations/types/array_coercer_spec.rb b/spec/grape/validations/types/array_coercer_spec.rb new file mode 100644 index 0000000000..f2bfb6c6d5 --- /dev/null +++ b/spec/grape/validations/types/array_coercer_spec.rb @@ -0,0 +1,35 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Grape::Validations::Types::ArrayCoercer do + subject { described_class.new(type) } + + describe '#call' do + context 'an array of primitives' do + let(:type) { Array[String] } + + it 'coerces elements in the array' do + expect(subject.call([10, 20])).to eq(%w[10 20]) + end + end + + context 'an array of arrays' do + let(:type) { Array[Array[Integer]] } + + it 'coerces elements in the nested array' do + expect(subject.call([%w[10 20]])).to eq([[10, 20]]) + expect(subject.call([['10'], ['20']])).to eq([[10], [20]]) + end + end + + context 'an array of sets' do + let(:type) { Array[Set[Integer]] } + + it 'coerces elements in the nested set' do + expect(subject.call([%w[10 20]])).to eq([Set[10, 20]]) + expect(subject.call([['10'], ['20']])).to eq([Set[10], Set[20]]) + end + end + end +end diff --git a/spec/grape/validations/types/primitive_coercer_spec.rb b/spec/grape/validations/types/primitive_coercer_spec.rb index d0e1b137b1..dbfd338261 100644 --- a/spec/grape/validations/types/primitive_coercer_spec.rb +++ b/spec/grape/validations/types/primitive_coercer_spec.rb @@ -7,7 +7,7 @@ subject { described_class.new(type, strict) } - describe '.call' do + describe '#call' do context 'Boolean' do let(:type) { Grape::API::Boolean } diff --git a/spec/grape/validations/types/set_coercer_spec.rb b/spec/grape/validations/types/set_coercer_spec.rb new file mode 100644 index 0000000000..d78f5f5118 --- /dev/null +++ b/spec/grape/validations/types/set_coercer_spec.rb @@ -0,0 +1,34 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Grape::Validations::Types::SetCoercer do + subject { described_class.new(type) } + + describe '#call' do + context 'a set of primitives' do + let(:type) { Set[String] } + + it 'coerces elements to the set' do + expect(subject.call([10, 20])).to eq(Set['10', '20']) + end + end + + context 'a set of sets' do + let(:type) { Set[Set[Integer]] } + + it 'coerces elements in the nested set' do + expect(subject.call([%w[10 20]])).to eq(Set[Set[10, 20]]) + expect(subject.call([['10'], ['20']])).to eq(Set[Set[10], Set[20]]) + end + end + + context 'a set of sets of arrays' do + let(:type) { Set[Set[Array[Integer]]] } + + it 'coerces elements in the nested set' do + expect(subject.call([[['10'], ['20']]])).to eq(Set[Set[Array[10], Array[20]]]) + end + end + end +end