Skip to content
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 @@ -3,6 +3,7 @@
#### Features

* Your contribution here.
* [#2157](https://github.com/ruby-grape/grape/pull/2157): Custom types can set a message to be used in the response when invalid - [@dnesteryuk](https://github.com/dnesteryuk).
* [#2145](https://github.com/ruby-grape/grape/pull/2145): Ruby 3.0 compatibility - [@ericproulx](https://github.com/ericproulx).
* [#2143](https://github.com/ruby-grape/grape/pull/2143): Enable GitHub Actions with updated RuboCop and Danger - [@anakinj](https://github.com/anakinj).

Expand Down
4 changes: 4 additions & 0 deletions Gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -35,3 +35,7 @@ group :test do
gem 'rspec', '~> 3.0'
gem 'ruby-grape-danger', '~> 0.2.0', require: false
end

platforms :jruby do
gem 'racc'
end
8 changes: 5 additions & 3 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -1183,7 +1183,8 @@ Aside from the default set of supported types listed above, any class can be
used as a type as long as an explicit coercion method is supplied. If the type
implements a class-level `parse` method, Grape will use it automatically.
This method must take one string argument and return an instance of the correct
type, or raise an exception to indicate the value was invalid. E.g.,
type, or return an instance of `Grape::Types::InvalidValue` which optionally
accepts a message to be returned in the response.

```ruby
class Color
Expand All @@ -1193,8 +1194,9 @@ class Color
end

def self.parse(value)
fail 'Invalid color' unless %w(blue red green).include?(value)
new(value)
return new(value) if %w[blue red green]).include?(value)

Grape::Types::InvalidValue.new('Unsupported color')
Copy link
Member

Choose a reason for hiding this comment

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

I feel like this should be a raise that gets handled, not a return.

Copy link
Member Author

Choose a reason for hiding this comment

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

How will we know that an exception message should be returned to the user if we rescue StandardError? I would like to avoid guessing 🙂 We already return the invalid value there and there and there.

In general, if there is an easy way to return a value indicating a failure, I prefer to do that. It is cheaper:

require 'benchmark/ips'

def return_value
  :invalid
end

def raise_exception
  raise StandardError
end

Benchmark.ips do |x|
  x.report('return value') do
    return_value
  end
  x.report('raise error') do
    begin
      raise_exception
    rescue StandardError
    end
  end

  x.compare!
end
Warming up --------------------------------------
        return value     1.990M i/100ms
         raise error   149.703k i/100ms
Calculating -------------------------------------
        return value     19.948M (± 0.4%) i/s -    101.470M in   5.086841s
         raise error      1.513M (± 0.5%) i/s -      7.635M in   5.044907s

Comparison:
        return value: 19947800.7 i/s
         raise error:  1513417.9 i/s - 13.18x  (± 0.00) slower

Copy link
Member

Choose a reason for hiding this comment

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

Ok, I clearly didn't do my homework, especially on the existing code.

end
end

Expand Down
5 changes: 1 addition & 4 deletions lib/grape/validations/types.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
require_relative 'types/variant_collection_coercer'
require_relative 'types/json'
require_relative 'types/file'
require_relative 'types/invalid_value'

module Grape
module Validations
Expand All @@ -21,10 +22,6 @@ module Validations
# and {Grape::Dsl::Parameters#optional}. The main
# entry point for this process is {Types.build_coercer}.
module Types
# Instances of this class may be used as tokens to denote that
# a parameter value could not be coerced.
class InvalidValue; end

# Types representing a single value, which are coerced.
PRIMITIVES = [
# Numerical
Expand Down
2 changes: 2 additions & 0 deletions lib/grape/validations/types/custom_type_coercer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,8 @@ def call(val)
return if val.nil?

coerced_val = @method.call(val)

return coerced_val if coerced_val.is_a?(InvalidValue)
return InvalidValue.new unless coerced?(coerced_val)
coerced_val
end
Expand Down
24 changes: 24 additions & 0 deletions lib/grape/validations/types/invalid_value.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
# frozen_string_literal: true

module Grape
module Validations
module Types
# Instances of this class may be used as tokens to denote that a parameter value could not be
# coerced. The given message will be used as a validation error.
class InvalidValue
attr_reader :message

def initialize(message = nil)
@message = message
end
end
end
end
end

# only exists to make it shorter for external use
module Grape
module Types
InvalidValue = Class.new(Grape::Validations::Types::InvalidValue)
end
end
9 changes: 6 additions & 3 deletions lib/grape/validations/validators/coerce.rb
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ def validate_param!(attr_name, params)

new_value = coerce_value(params[attr_name])

raise validation_exception(attr_name) unless valid_type?(new_value)
raise validation_exception(attr_name, new_value.message) unless valid_type?(new_value)

# Don't assign a value if it is identical. It fixes a problem with Hashie::Mash
# which looses wrappers for hashes and arrays after reassigning values
Expand Down Expand Up @@ -80,8 +80,11 @@ def type
@option[:type].is_a?(Hash) ? @option[:type][:value] : @option[:type]
end

def validation_exception(attr_name)
Grape::Exceptions::Validation.new(params: [@scope.full_name(attr_name)], message: message(:coerce))
def validation_exception(attr_name, custom_msg = nil)
Grape::Exceptions::Validation.new(
params: [@scope.full_name(attr_name)],
message: custom_msg || message(:coerce)
)
end
end
end
Expand Down
52 changes: 40 additions & 12 deletions spec/grape/validations/validators/coerce_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -227,23 +227,51 @@ def self.parsed?(value)
expect(last_response.body).to eq('NilClass')
end

it 'is a custom type' do
subject.params do
requires :uri, coerce: SecureURIOnly
end
subject.get '/secure_uri' do
params[:uri].class
context 'a custom type' do
it 'coerces the given value' do
subject.params do
requires :uri, coerce: SecureURIOnly
end
subject.get '/secure_uri' do
params[:uri].class
end

get 'secure_uri', uri: 'https://www.example.com'

expect(last_response.status).to eq(200)
expect(last_response.body).to eq('URI::HTTPS')

get 'secure_uri', uri: 'http://www.example.com'

expect(last_response.status).to eq(400)
expect(last_response.body).to eq('uri is invalid')
end

get 'secure_uri', uri: 'https://www.example.com'
context 'returning the InvalidValue instance when invalid' do
let(:custom_type) do
Class.new do
def self.parse(_val)
Grape::Types::InvalidValue.new('must be unique')
end
end
end

expect(last_response.status).to eq(200)
expect(last_response.body).to eq('URI::HTTPS')
it 'uses a custom message added to the invalid value' do
type = custom_type

subject.params do
requires :name, type: type
end
subject.get '/whatever' do
params[:name].class
end

get 'secure_uri', uri: 'http://www.example.com'
get 'whatever', name: 'Bob'

expect(last_response.status).to eq(400)
expect(last_response.body).to eq('uri is invalid')
expect(last_response.status).to eq(400)
expect(last_response.body).to eq('name must be unique')
end
end
end

context 'Array' do
Expand Down