Skip to content

Commit f3073e6

Browse files
committed
special types provided by Grape are custom ones
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.
1 parent ee1f29f commit f3073e6

File tree

7 files changed

+114
-87
lines changed

7 files changed

+114
-87
lines changed

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
#### Fixes
1111

1212
* Your contribution here.
13+
* [#2031](https://github.com/ruby-grape/grape/pull/2031): Fix a regression with an array of a custom type - [@dnesteryuk](https://github.com/dnesteryuk).
1314
* [#2026](https://github.com/ruby-grape/grape/pull/2026): Fix a regression in `coerce_with` when coercion returns `nil` - [@misdoro](https://github.com/misdoro).
1415
* [#2025](https://github.com/ruby-grape/grape/pull/2025): Fix Decimal type category - [@kdoya](https://github.com/kdoya).
1516
* [#2019](https://github.com/ruby-grape/grape/pull/2019): Avoid coercing parameter with multiple types to an empty Array - [@stanhu](https://github.com/stanhu).

lib/grape/validations/types.rb

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,6 @@ class InvalidValue; end
4242
Grape::API::Boolean,
4343
String,
4444
Symbol,
45-
Rack::Multipart::UploadedFile,
4645
TrueClass,
4746
FalseClass
4847
].freeze
@@ -54,8 +53,7 @@ class InvalidValue; end
5453
Set
5554
].freeze
5655

57-
# Types for which Grape provides special coercion
58-
# and type-checking logic.
56+
# Special custom types provided by Grape.
5957
SPECIAL = {
6058
JSON => Json,
6159
Array[JSON] => JsonArray,
@@ -130,7 +128,6 @@ def self.custom?(type)
130128
!primitive?(type) &&
131129
!structure?(type) &&
132130
!multiple?(type) &&
133-
!special?(type) &&
134131
type.respond_to?(:parse) &&
135132
type.method(:parse).arity == 1
136133
end
@@ -143,7 +140,11 @@ def self.custom?(type)
143140
def self.collection_of_custom?(type)
144141
(type.is_a?(Array) || type.is_a?(Set)) &&
145142
type.length == 1 &&
146-
custom?(type.first)
143+
(custom?(type.first) || special?(type.first))
144+
end
145+
146+
def self.map_special(type)
147+
SPECIAL.fetch(type, type)
147148
end
148149
end
149150
end

lib/grape/validations/types/build_coercer.rb

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,9 @@ def self.build_coercer(type, method: nil, strict: false)
4242
end
4343

4444
def self.create_coercer_instance(type, method, strict)
45+
# Maps a custom type provided by Grape, it doesn't map types wrapped by collections!!!
46+
type = Types.map_special(type)
47+
4548
# Use a special coercer for multiply-typed parameters.
4649
if Types.multiple?(type)
4750
MultipleTypeCoercer.new(type, method)
@@ -55,10 +58,8 @@ def self.create_coercer_instance(type, method, strict)
5558
# method is supplied.
5659
elsif Types.collection_of_custom?(type)
5760
Types::CustomTypeCollectionCoercer.new(
58-
type.first, type.is_a?(Set)
61+
Types.map_special(type.first), type.is_a?(Set)
5962
)
60-
elsif Types.special?(type)
61-
Types::SPECIAL[type].new
6263
elsif type.is_a?(Array)
6364
ArrayCoercer.new type, strict
6465
elsif type.is_a?(Set)

lib/grape/validations/types/file.rb

Lines changed: 15 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -7,21 +7,23 @@ module Types
77
# Actual handling of these objects is provided by +Rack::Request+;
88
# this class is here only to assert that rack's handling has succeeded.
99
class File
10-
def call(input)
11-
return if input.nil?
12-
return InvalidValue.new unless coerced?(input)
10+
class << self
11+
def parse(input)
12+
return if input.nil?
13+
return InvalidValue.new unless parsed?(input)
1314

14-
# Processing of multipart file objects
15-
# is already taken care of by Rack::Request.
16-
# Nothing to do here.
17-
input
18-
end
15+
# Processing of multipart file objects
16+
# is already taken care of by Rack::Request.
17+
# Nothing to do here.
18+
input
19+
end
1920

20-
def coerced?(value)
21-
# Rack::Request creates a Hash with filename,
22-
# content type and an IO object. Do a bit of basic
23-
# duck-typing.
24-
value.is_a?(::Hash) && value.key?(:tempfile) && value[:tempfile].is_a?(Tempfile)
21+
def parsed?(value)
22+
# Rack::Request creates a Hash with filename,
23+
# content type and an IO object. Do a bit of basic
24+
# duck-typing.
25+
value.is_a?(::Hash) && value.key?(:tempfile) && value[:tempfile].is_a?(Tempfile)
26+
end
2527
end
2628
end
2729
end

lib/grape/validations/types/json.rb

Lines changed: 40 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -12,35 +12,37 @@ module Types
1212
# validation system will apply nested validation rules to
1313
# all returned objects.
1414
class Json
15-
# Coerce the input into a JSON-like data structure.
16-
#
17-
# @param input [String] a JSON-encoded parameter value
18-
# @return [Hash,Array<Hash>,nil]
19-
def call(input)
20-
return input if coerced?(input)
15+
class << self
16+
# Coerce the input into a JSON-like data structure.
17+
#
18+
# @param input [String] a JSON-encoded parameter value
19+
# @return [Hash,Array<Hash>,nil]
20+
def parse(input)
21+
return input if parsed?(input)
2122

22-
# Allow nulls and blank strings
23-
return if input.nil? || input.match?(/^\s*$/)
24-
JSON.parse(input, symbolize_names: true)
25-
end
23+
# Allow nulls and blank strings
24+
return if input.nil? || input.match?(/^\s*$/)
25+
JSON.parse(input, symbolize_names: true)
26+
end
2627

27-
# Checks that the input was parsed successfully
28-
# and isn't something odd such as an array of primitives.
29-
#
30-
# @param value [Object] result of {#coerce}
31-
# @return [true,false]
32-
def coerced?(value)
33-
value.is_a?(::Hash) || coerced_collection?(value)
34-
end
28+
# Checks that the input was parsed successfully
29+
# and isn't something odd such as an array of primitives.
30+
#
31+
# @param value [Object] result of {#parse}
32+
# @return [true,false]
33+
def parsed?(value)
34+
value.is_a?(::Hash) || coerced_collection?(value)
35+
end
3536

36-
protected
37+
protected
3738

38-
# Is the value an array of JSON-like objects?
39-
#
40-
# @param value [Object] result of {#coerce}
41-
# @return [true,false]
42-
def coerced_collection?(value)
43-
value.is_a?(::Array) && value.all? { |i| i.is_a? ::Hash }
39+
# Is the value an array of JSON-like objects?
40+
#
41+
# @param value [Object] result of {#parse}
42+
# @return [true,false]
43+
def coerced_collection?(value)
44+
value.is_a?(::Array) && value.all? { |i| i.is_a? ::Hash }
45+
end
4446
end
4547
end
4648

@@ -49,18 +51,20 @@ def coerced_collection?(value)
4951
# objects and arrays of objects, but wraps single objects
5052
# in an Array.
5153
class JsonArray < Json
52-
# See {Json#coerce}. Wraps single objects in an array.
53-
#
54-
# @param input [String] JSON-encoded parameter value
55-
# @return [Array<Hash>]
56-
def call(input)
57-
json = super
58-
Array.wrap(json) unless json.nil?
59-
end
54+
class << self
55+
# See {Json#parse}. Wraps single objects in an array.
56+
#
57+
# @param input [String] JSON-encoded parameter value
58+
# @return [Array<Hash>]
59+
def parse(input)
60+
json = super
61+
Array.wrap(json) unless json.nil?
62+
end
6063

61-
# See {Json#coerced_collection?}
62-
def coerced?(value)
63-
coerced_collection? value
64+
# See {Json#coerced_collection?}
65+
def parsed?(value)
66+
coerced_collection? value
67+
end
6468
end
6569
end
6670
end

spec/grape/validations/types_spec.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ def self.parse; end
1717
[
1818
Integer, Float, Numeric, BigDecimal,
1919
Grape::API::Boolean, String, Symbol,
20-
Date, DateTime, Time, Rack::Multipart::UploadedFile
20+
Date, DateTime, Time
2121
].each do |type|
2222
it "recognizes #{type} as a primitive" do
2323
expect(described_class.primitive?(type)).to be_truthy

spec/grape/validations/validators/coerce_spec.rb

Lines changed: 47 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -354,42 +354,60 @@ def self.parsed?(value)
354354
expect(last_response.body).to eq('TrueClass')
355355
end
356356

357-
it 'Rack::Multipart::UploadedFile' do
358-
subject.params do
359-
requires :file, type: Rack::Multipart::UploadedFile
360-
end
361-
subject.post '/upload' do
362-
params[:file][:filename]
363-
end
357+
context 'File' do
358+
let(:file) { Rack::Test::UploadedFile.new(__FILE__) }
359+
let(:filename) { File.basename(__FILE__).to_s }
364360

365-
post '/upload', file: Rack::Test::UploadedFile.new(__FILE__)
366-
expect(last_response.status).to eq(201)
367-
expect(last_response.body).to eq(File.basename(__FILE__).to_s)
361+
it 'Rack::Multipart::UploadedFile' do
362+
subject.params do
363+
requires :file, type: Rack::Multipart::UploadedFile
364+
end
365+
subject.post '/upload' do
366+
params[:file][:filename]
367+
end
368368

369-
post '/upload', file: 'not a file'
370-
expect(last_response.status).to eq(400)
371-
expect(last_response.body).to eq('file is invalid')
372-
end
369+
post '/upload', file: file
370+
expect(last_response.status).to eq(201)
371+
expect(last_response.body).to eq(filename)
373372

374-
it 'File' do
375-
subject.params do
376-
requires :file, coerce: File
377-
end
378-
subject.post '/upload' do
379-
params[:file][:filename]
373+
post '/upload', file: 'not a file'
374+
expect(last_response.status).to eq(400)
375+
expect(last_response.body).to eq('file is invalid')
380376
end
381377

382-
post '/upload', file: Rack::Test::UploadedFile.new(__FILE__)
383-
expect(last_response.status).to eq(201)
384-
expect(last_response.body).to eq(File.basename(__FILE__).to_s)
378+
it 'File' do
379+
subject.params do
380+
requires :file, coerce: File
381+
end
382+
subject.post '/upload' do
383+
params[:file][:filename]
384+
end
385385

386-
post '/upload', file: 'not a file'
387-
expect(last_response.status).to eq(400)
388-
expect(last_response.body).to eq('file is invalid')
386+
post '/upload', file: file
387+
expect(last_response.status).to eq(201)
388+
expect(last_response.body).to eq(filename)
389389

390-
post '/upload', file: { filename: 'fake file', tempfile: '/etc/passwd' }
391-
expect(last_response.status).to eq(400)
392-
expect(last_response.body).to eq('file is invalid')
390+
post '/upload', file: 'not a file'
391+
expect(last_response.status).to eq(400)
392+
expect(last_response.body).to eq('file is invalid')
393+
394+
post '/upload', file: { filename: 'fake file', tempfile: '/etc/passwd' }
395+
expect(last_response.status).to eq(400)
396+
expect(last_response.body).to eq('file is invalid')
397+
end
398+
399+
it 'collection' do
400+
subject.params do
401+
requires :files, type: Array[File]
402+
end
403+
subject.post '/upload' do
404+
params[:files].first[:filename]
405+
end
406+
407+
post '/upload', files: [file]
408+
expect(last_response.status).to eq(201)
409+
expect(last_response.body).to eq(filename)
410+
end
393411
end
394412

395413
it 'Nests integers' do

0 commit comments

Comments
 (0)