Skip to content

Coerce collections of parseable types #1711

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Nov 26, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
* [#1686](https://github.com/ruby-grape/grape/pull/1686): Avoid coercion of a value if it is valid - [@timothysu](https://github.com/timothysu).
* [#1688](https://github.com/ruby-grape/grape/pull/1688): Removes yard docs - [@ramkumar-kr](https://github.com/ramkumar-kr).
* [#1702](https://github.com/ruby-grape/grape/pull/1702): Added danger-toc, verify correct TOC in README - [@dblock](https://github.com/dblock).
* [#1711](https://github.com/ruby-grape/grape/pull/1711): Automatically coerce arrays and sets of types that implement a `parse` method - [@dslh](https://github.com/dslh).

#### Fixes

Expand Down
22 changes: 22 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -909,6 +909,8 @@ end

params do
requires :color, type: Color, default: Color.new('blue')
requires :more_colors, type: Array[Color] # Collections work
optional :unique_colors, type: Set[Color] # Duplicates discarded
end

get '/stuff' do
Expand Down Expand Up @@ -943,6 +945,26 @@ params do
end
```

Grape will assert that coerced values match the given `type`, and will reject the request
if they do not. To override this behaviour, custom types may implement a `parsed?` method
that should accept a single argument and return `true` if the value passes type validation.

```ruby
class SecureUri
def self.parse(value)
URI.parse value
end

def self.parsed?(value)
value.is_a? URI::HTTPS
end
end

params do
requires :secure_uri, type: SecureUri
end
```

### Multipart File Parameters

Grape makes use of `Rack::Request`'s built-in support for multipart file parameters. Such parameters can be declared with `type: File`:
Expand Down
15 changes: 14 additions & 1 deletion lib/grape/validations/types.rb
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
require_relative 'types/build_coercer'
require_relative 'types/custom_type_coercer'
require_relative 'types/custom_type_collection_coercer'
require_relative 'types/multiple_type_coercer'
require_relative 'types/variant_collection_coercer'
require_relative 'types/json'
Expand Down Expand Up @@ -143,7 +144,8 @@ def self.group?(type)
end

# A valid custom type must implement a class-level `parse` method, taking
# one String argument and returning the parsed value in its correct type.
# one String argument and returning the parsed value in its correct type.
#
# @param type [Class] type to check
# @return [Boolean] whether or not the type can be used as a custom type
def self.custom?(type)
Expand All @@ -155,6 +157,17 @@ def self.custom?(type)
type.respond_to?(:parse) &&
type.method(:parse).arity == 1
end

# Is the declared type an +Array+ or +Set+ of a {#custom?} type?
#
# @param type [Array<Class>,Class] type to check
# @return [Boolean] true if +type+ is a collection of a type that implements
# its own +#parse+ method.
def self.collection_of_custom?(type)
(type.is_a?(Array) || type.is_a?(Set)) &&
type.length == 1 &&
custom?(type.first)
end
end
end
end
8 changes: 8 additions & 0 deletions lib/grape/validations/types/build_coercer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,14 @@ def self.create_coercer_instance(type, method = nil)
elsif method || Types.custom?(type)
converter_options[:coercer] = Types::CustomTypeCoercer.new(type, method)

# Special coercer for collections of types that implement a parse method.
# CustomTypeCoercer (above) already handles such types when an explicit coercion
# method is supplied.
elsif Types.collection_of_custom?(type)
converter_options[:coercer] = Types::CustomTypeCollectionCoercer.new(
type.first, type.is_a?(Set)
)

# Grape swaps in its own Virtus::Attribute implementations
# for certain special types that merit first-class support
# (but not if a custom coercion method has been supplied).
Expand Down
71 changes: 71 additions & 0 deletions lib/grape/validations/types/custom_type_collection_coercer.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
module Grape
module Validations
module Types
# Instances of this class may be passed to
# +Virtus::Attribute.build+ as the +:coercer+
# option, to handle collections of types that
# provide their own parsing (and optionally,
# type-checking) functionality.
#
# See {CustomTypeCoercer} for details on types
# that will be supported by this by this coercer.
# This coercer works in the same way as +CustomTypeCoercer+
# except that it expects to receive an array of strings to
# coerce and will return an array (or optionally, a set)
# of coerced values.
#
# +CustomTypeCoercer+ is already capable of providing type
# checking for arrays where an independent coercion method
# is supplied. As such, +CustomTypeCollectionCoercer+ does
# not allow for such a method to be supplied independently
# of the type.
class CustomTypeCollectionCoercer < CustomTypeCoercer
# A new coercer for collections of the given type.
#
# @param type [Class,#parse]
# type to which items in the array should be coerced.
# Must implement a +parse+ method which accepts a string,
# and for the purposes of type-checking it may either be
# a class, or it may implement a +coerced?+, +parsed?+ or
# +call+ method (in that order of precedence) which
# accepts a single argument and returns true if the given
# array item has been coerced correctly.
# @param set [Boolean]
# when true, a +Set+ will be returned by {#call} instead
# of an +Array+ and duplicate items will be discarded.
def initialize(type, set = false)
super(type)
@set = set
end

# This method is called from somewhere within
# +Virtus::Attribute::coerce+ in order to coerce
# the given value.
#
# @param value [Array<String>] an array of values to be coerced
# @return [Array,Set] the coerced result. May be an +Array+ or a
# +Set+ depending on the setting given to the constructor
def call(value)
coerced = value.map { |item| super(item) }
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm aware this will fail if value is a string but I wasn't sure of the best thing to do about it. Maybe Array.wrap?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To allow for a single item to be passed in as such (without the array wrapping/declaration) is a convenience, but could be improper. I suppose it depends on how strict Grape wants to be.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would deal with that scenario later with a proper spec.


@set ? Set.new(coerced) : coerced
end

# This method is called from somewhere within
# +Virtus::Attribute::value_coerced?+ in order to assert
# that the all of the values in the array have been coerced
# successfully.
#
# @param primitive [Axiom::Types::Type] primitive type for
# the coercion as deteced by axiom-types' inference system.
# @param value [Enumerable] a coerced result returned from {#call}
# @return [true,false] whether or not all of the coerced values in
# the collection satisfy type requirements.
def success?(primitive, value)
value.is_a?(@set ? Set : Array) &&
value.all? { |item| super(primitive, item) }
end
end
end
end
end
49 changes: 49 additions & 0 deletions spec/grape/validations/validators/coerce_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -225,6 +225,55 @@ class User
expect(last_response.status).to eq(200)
expect(last_response.body).to eq('1')
end

it 'Array of type implementing parse' do
subject.params do
requires :uri, type: Array[URI]
end
subject.get '/uri_array' do
params[:uri][0].class
end
get 'uri_array', uri: ['http://www.google.com']
expect(last_response.status).to eq(200)
expect(last_response.body).to eq('URI::HTTP')
end

it 'Set of type implementing parse' do
subject.params do
requires :uri, type: Set[URI]
end
subject.get '/uri_array' do
"#{params[:uri].class},#{params[:uri].first.class},#{params[:uri].size}"
end
get 'uri_array', uri: Array.new(2) { 'http://www.example.com' }
expect(last_response.status).to eq(200)
expect(last_response.body).to eq('Set,URI::HTTP,1')
end

it 'Array of class implementing parse and parsed?' do
class SecureURIOnly
def self.parse(value)
URI.parse(value)
end

def self.parsed?(value)
value.is_a? URI::HTTPS
end
end

subject.params do
requires :uri, type: Array[SecureURIOnly]
end
subject.get '/secure_uris' do
params[:uri].first.class
end
get 'secure_uris', uri: ['https://www.example.com']
expect(last_response.status).to eq(200)
expect(last_response.body).to eq('URI::HTTPS')
get 'secure_uris', uri: ['https://www.example.com', 'http://www.example.com']
expect(last_response.status).to eq(400)
expect(last_response.body).to eq('uri is invalid')
end
end

context 'Set' do
Expand Down