Skip to content

Commit

Permalink
special types provided by Grape are custom ones
Browse files Browse the repository at this point in the history
Users trys to use types provided by Grape as an example how to write
custom types. So, it makes sense to treat them as custom.

Besides that, it fixes #1986 where a collection of a custom type wasn't
coerced properly.
  • Loading branch information
dnesteryuk committed Mar 29, 2020
1 parent c67aed3 commit d57740f
Show file tree
Hide file tree
Showing 6 changed files with 112 additions and 87 deletions.
11 changes: 6 additions & 5 deletions lib/grape/validations/types.rb
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,6 @@ class InvalidValue; end
Grape::API::Boolean,
String,
Symbol,
Rack::Multipart::UploadedFile,
TrueClass,
FalseClass
].freeze
Expand All @@ -54,8 +53,7 @@ class InvalidValue; end
Set
].freeze

# Types for which Grape provides special coercion
# and type-checking logic.
# Custom types provided by Grape.
SPECIAL = {
JSON => Json,
Array[JSON] => JsonArray,
Expand Down Expand Up @@ -130,7 +128,6 @@ def self.custom?(type)
!primitive?(type) &&
!structure?(type) &&
!multiple?(type) &&
!special?(type) &&
type.respond_to?(:parse) &&
type.method(:parse).arity == 1
end
Expand All @@ -143,7 +140,11 @@ def self.custom?(type)
def self.collection_of_custom?(type)
(type.is_a?(Array) || type.is_a?(Set)) &&
type.length == 1 &&
custom?(type.first)
(custom?(type.first) || special?(type.first))
end

def self.map_special(type)
SPECIAL.fetch(type, type)
end
end
end
Expand Down
6 changes: 3 additions & 3 deletions lib/grape/validations/types/build_coercer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,8 @@ def self.build_coercer(type, method: nil, strict: false)
end

def self.create_coercer_instance(type, method, strict)
type = Types.map_special(type)

# Use a special coercer for multiply-typed parameters.
if Types.multiple?(type)
MultipleTypeCoercer.new(type, method)
Expand All @@ -55,10 +57,8 @@ def self.create_coercer_instance(type, method, strict)
# method is supplied.
elsif Types.collection_of_custom?(type)
Types::CustomTypeCollectionCoercer.new(
type.first, type.is_a?(Set)
Types.map_special(type.first), type.is_a?(Set)
)
elsif Types.special?(type)
Types::SPECIAL[type].new
elsif type.is_a?(Array)
ArrayCoercer.new type, strict
elsif type.is_a?(Set)
Expand Down
28 changes: 15 additions & 13 deletions lib/grape/validations/types/file.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,21 +7,23 @@ module Types
# Actual handling of these objects is provided by +Rack::Request+;
# this class is here only to assert that rack's handling has succeeded.
class File
def call(input)
return if input.nil?
return InvalidValue.new unless coerced?(input)
class << self
def parse(input)
return if input.nil?
return InvalidValue.new unless parsed?(input)

# Processing of multipart file objects
# is already taken care of by Rack::Request.
# Nothing to do here.
input
end
# Processing of multipart file objects
# is already taken care of by Rack::Request.
# Nothing to do here.
input
end

def coerced?(value)
# Rack::Request creates a Hash with filename,
# content type and an IO object. Do a bit of basic
# duck-typing.
value.is_a?(::Hash) && value.key?(:tempfile) && value[:tempfile].is_a?(Tempfile)
def parsed?(value)
# Rack::Request creates a Hash with filename,
# content type and an IO object. Do a bit of basic
# duck-typing.
value.is_a?(::Hash) && value.key?(:tempfile) && value[:tempfile].is_a?(Tempfile)
end
end
end
end
Expand Down
76 changes: 40 additions & 36 deletions lib/grape/validations/types/json.rb
Original file line number Diff line number Diff line change
Expand Up @@ -12,35 +12,37 @@ module Types
# validation system will apply nested validation rules to
# all returned objects.
class Json
# Coerce the input into a JSON-like data structure.
#
# @param input [String] a JSON-encoded parameter value
# @return [Hash,Array<Hash>,nil]
def call(input)
return input if coerced?(input)
class << self
# Coerce the input into a JSON-like data structure.
#
# @param input [String] a JSON-encoded parameter value
# @return [Hash,Array<Hash>,nil]
def parse(input)
return input if parsed?(input)

# Allow nulls and blank strings
return if input.nil? || input.match?(/^\s*$/)
JSON.parse(input, symbolize_names: true)
end
# Allow nulls and blank strings
return if input.nil? || input.match?(/^\s*$/)
JSON.parse(input, symbolize_names: true)
end

# Checks that the input was parsed successfully
# and isn't something odd such as an array of primitives.
#
# @param value [Object] result of {#coerce}
# @return [true,false]
def coerced?(value)
value.is_a?(::Hash) || coerced_collection?(value)
end
# Checks that the input was parsed successfully
# and isn't something odd such as an array of primitives.
#
# @param value [Object] result of {#coerce}
# @return [true,false]
def parsed?(value)
value.is_a?(::Hash) || coerced_collection?(value)
end

protected
protected

# Is the value an array of JSON-like objects?
#
# @param value [Object] result of {#coerce}
# @return [true,false]
def coerced_collection?(value)
value.is_a?(::Array) && value.all? { |i| i.is_a? ::Hash }
# Is the value an array of JSON-like objects?
#
# @param value [Object] result of {#coerce}
# @return [true,false]
def coerced_collection?(value)
value.is_a?(::Array) && value.all? { |i| i.is_a? ::Hash }
end
end
end

Expand All @@ -49,18 +51,20 @@ def coerced_collection?(value)
# objects and arrays of objects, but wraps single objects
# in an Array.
class JsonArray < Json
# See {Json#coerce}. Wraps single objects in an array.
#
# @param input [String] JSON-encoded parameter value
# @return [Array<Hash>]
def call(input)
json = super
Array.wrap(json) unless json.nil?
end
class << self
# See {Json#coerce}. Wraps single objects in an array.
#
# @param input [String] JSON-encoded parameter value
# @return [Array<Hash>]
def parse(input)
json = super
Array.wrap(json) unless json.nil?
end

# See {Json#coerced_collection?}
def coerced?(value)
coerced_collection? value
# See {Json#coerced_collection?}
def parsed?(value)
coerced_collection? value
end
end
end
end
Expand Down
2 changes: 1 addition & 1 deletion spec/grape/validations/types_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ def self.parse; end
[
Integer, Float, Numeric, BigDecimal,
Grape::API::Boolean, String, Symbol,
Date, DateTime, Time, Rack::Multipart::UploadedFile
Date, DateTime, Time
].each do |type|
it "recognizes #{type} as a primitive" do
expect(described_class.primitive?(type)).to be_truthy
Expand Down
76 changes: 47 additions & 29 deletions spec/grape/validations/validators/coerce_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -354,42 +354,60 @@ def self.parsed?(value)
expect(last_response.body).to eq('TrueClass')
end

it 'Rack::Multipart::UploadedFile' do
subject.params do
requires :file, type: Rack::Multipart::UploadedFile
end
subject.post '/upload' do
params[:file][:filename]
end
context 'File' do
let(:file) { Rack::Test::UploadedFile.new(__FILE__) }
let(:filename) { File.basename(__FILE__).to_s }

post '/upload', file: Rack::Test::UploadedFile.new(__FILE__)
expect(last_response.status).to eq(201)
expect(last_response.body).to eq(File.basename(__FILE__).to_s)
it 'Rack::Multipart::UploadedFile' do
subject.params do
requires :file, type: Rack::Multipart::UploadedFile
end
subject.post '/upload' do
params[:file][:filename]
end

post '/upload', file: 'not a file'
expect(last_response.status).to eq(400)
expect(last_response.body).to eq('file is invalid')
end
post '/upload', file: file
expect(last_response.status).to eq(201)
expect(last_response.body).to eq(filename)

it 'File' do
subject.params do
requires :file, coerce: File
end
subject.post '/upload' do
params[:file][:filename]
post '/upload', file: 'not a file'
expect(last_response.status).to eq(400)
expect(last_response.body).to eq('file is invalid')
end

post '/upload', file: Rack::Test::UploadedFile.new(__FILE__)
expect(last_response.status).to eq(201)
expect(last_response.body).to eq(File.basename(__FILE__).to_s)
it 'File' do
subject.params do
requires :file, coerce: File
end
subject.post '/upload' do
params[:file][:filename]
end

post '/upload', file: 'not a file'
expect(last_response.status).to eq(400)
expect(last_response.body).to eq('file is invalid')
post '/upload', file: file
expect(last_response.status).to eq(201)
expect(last_response.body).to eq(filename)

post '/upload', file: { filename: 'fake file', tempfile: '/etc/passwd' }
expect(last_response.status).to eq(400)
expect(last_response.body).to eq('file is invalid')
post '/upload', file: 'not a file'
expect(last_response.status).to eq(400)
expect(last_response.body).to eq('file is invalid')

post '/upload', file: { filename: 'fake file', tempfile: '/etc/passwd' }
expect(last_response.status).to eq(400)
expect(last_response.body).to eq('file is invalid')
end

it 'collection' do
subject.params do
requires :files, type: Array[File]
end
subject.post '/upload' do
params[:files].first[:filename]
end

post '/upload', files: [file]
expect(last_response.status).to eq(201)
expect(last_response.body).to eq(filename)
end
end

it 'Nests integers' do
Expand Down

0 comments on commit d57740f

Please sign in to comment.