From 5d30d8b79cebc8aafbd05936e6c46b8eb7db3c70 Mon Sep 17 00:00:00 2001 From: namusyaka Date: Tue, 27 Dec 2016 22:35:05 +0900 Subject: [PATCH 1/2] Add ruby-2.4 to .travis.yml --- .travis.yml | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/.travis.yml b/.travis.yml index 97638ebe0f..58d20313cc 100644 --- a/.travis.yml +++ b/.travis.yml @@ -4,9 +4,19 @@ sudo: false matrix: include: - - rvm: 2.3.3 + - rvm: 2.4.0 script: - bundle exec danger + - rvm: 2.4.0 + gemfile: Gemfile + - rvm: 2.4.0 + gemfile: gemfiles/rack_edge.gemfile + - rvm: 2.4.0 + gemfile: gemfiles/rack_1.5.2.gemfile + - rvm: 2.4.0 + gemfile: gemfiles/rails_edge.gemfile + - rvm: 2.4.0 + gemfile: gemfiles/rails_5.gemfile - rvm: 2.3.3 gemfile: Gemfile - rvm: 2.3.3 From 96fd175d3e8b4648015a3ae0b4fe0be4f6d1df7a Mon Sep 17 00:00:00 2001 From: namusyaka Date: Tue, 27 Dec 2016 22:35:18 +0900 Subject: [PATCH 2/2] Use Integer instead of Fixnum --- CHANGELOG.md | 9 +- README.md | 25 ++++- lib/grape/dsl/inside_route.rb | 4 +- spec/grape/api_spec.rb | 4 +- spec/grape/dsl/inside_route_spec.rb | 6 +- spec/grape/dsl/request_response_spec.rb | 2 +- .../validators/allow_blank_spec.rb | 16 ++-- .../validations/validators/coerce_spec.rb | 14 +-- spec/spec_helper.rb | 1 + spec/support/basic_auth_encode_helpers.rb | 10 +- spec/support/content_type_helpers.rb | 18 ++-- spec/support/integer_helpers.rb | 11 +++ spec/support/versioned_helpers.rb | 95 ++++++++++--------- 13 files changed, 135 insertions(+), 80 deletions(-) create mode 100644 spec/support/integer_helpers.rb diff --git a/CHANGELOG.md b/CHANGELOG.md index 6e54e57e28..acd448090d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,13 @@ -#### 0.19.1 (Next) +### 0.19.1 (Next) + +#### Features * [#1536](/~https://github.com/ruby-grape/grape/pull/1536): Updates `invalid_versioner_option` translation - [@Lavode](/~https://github.com/Lavode). +* [#1543](/~https://github.com/ruby-grape/grape/pull/1543): Support ruby 2.4 - [@LeFnord](/~https://github.com/LeFnord), [@namusyaka](/~https://github.com/namusyaka). +* Your contribution here. + +#### Fixes + * Your contribution here. ### 0.19.0 (12/18/2016) diff --git a/README.md b/README.md index b3ae70ebc3..f68df4a22d 100644 --- a/README.md +++ b/README.md @@ -32,6 +32,7 @@ - [Include Missing](#include-missing) - [Parameter Validation and Coercion](#parameter-validation-and-coercion) - [Supported Parameter Types](#supported-parameter-types) + - [Integer/Fixnum and Coercions](#integerfixnum-and-coercions) - [Custom Types and Coercions](#custom-types-and-coercions) - [Multipart File Parameters](#multipart-file-parameters) - [First-Class `JSON` Types](#first-class-json-types) @@ -799,6 +800,28 @@ The following are all valid types, supported out of the box by Grape: * Rack::Multipart::UploadedFile (alias `File`) * JSON +### Integer/Fixnum and Coercions + +Please be aware that the behavior differs between Ruby 2.4 and earlier versions. +In Ruby 2.4, values consisting of numbers are converted to Integer, but in earlier versions it will be treated as Fixnum. + +```ruby +params do + requires :integers, type: Hash do + requires :int, coerce: Integer + end +end +get '/int' do + params[:integers][:int].class +end + +... + +get '/int' integers: { int: '45' } + #=> Integer in ruby 2.4 + #=> Fixnum in earlier ruby versions +``` + ### Custom Types and Coercions Aside from the default set of supported types listed above, any class can be @@ -2878,7 +2901,7 @@ end The behaviour is then: ```bash -GET /123 # 'Fixnum' +GET /123 # 'Integer' GET /foo # 400 error - 'blah is invalid' ``` diff --git a/lib/grape/dsl/inside_route.rb b/lib/grape/dsl/inside_route.rb index 76a74c65be..28e37873ee 100644 --- a/lib/grape/dsl/inside_route.rb +++ b/lib/grape/dsl/inside_route.rb @@ -122,7 +122,7 @@ def status(status = nil) when Symbol raise ArgumentError, "Status code :#{status} is invalid." unless Rack::Utils::SYMBOL_TO_STATUS_CODE.keys.include?(status) @status = Rack::Utils.status_code(status) - when Fixnum + when Integer @status = status when nil return @status if @status @@ -135,7 +135,7 @@ def status(status = nil) 200 end else - raise ArgumentError, 'Status code must be Fixnum or Symbol.' + raise ArgumentError, 'Status code must be Integer or Symbol.' end end diff --git a/spec/grape/api_spec.rb b/spec/grape/api_spec.rb index 72b4beef22..4913067aec 100644 --- a/spec/grape/api_spec.rb +++ b/spec/grape/api_spec.rb @@ -925,7 +925,7 @@ class DummyFormatClass end get '/', id: '32' - expect(last_response.body).to eql 'first 32:Fixnum second' + expect(last_response.body).to eql "first 32:#{integer_class_name} second" end it 'adds a after filter' do @@ -2506,7 +2506,7 @@ def static end end describe 'status' do - it 'can be set to arbitrary Fixnum value' do + it 'can be set to arbitrary Integer value' do subject.get '/foo' do status 210 end diff --git a/spec/grape/dsl/inside_route_spec.rb b/spec/grape/dsl/inside_route_spec.rb index f262851662..5cbd4e7c33 100644 --- a/spec/grape/dsl/inside_route_spec.rb +++ b/spec/grape/dsl/inside_route_spec.rb @@ -126,13 +126,13 @@ def initialize .to raise_error(ArgumentError, 'Status code :foo_bar is invalid.') end - it 'accepts unknown Fixnum status codes' do + it 'accepts unknown Integer status codes' do expect { subject.status 210 }.to_not raise_error end - it 'raises error if status is not a fixnum or symbol' do + it 'raises error if status is not a integer or symbol' do expect { subject.status Object.new } - .to raise_error(ArgumentError, 'Status code must be Fixnum or Symbol.') + .to raise_error(ArgumentError, 'Status code must be Integer or Symbol.') end end diff --git a/spec/grape/dsl/request_response_spec.rb b/spec/grape/dsl/request_response_spec.rb index d8dee46b75..7974537fb8 100644 --- a/spec/grape/dsl/request_response_spec.rb +++ b/spec/grape/dsl/request_response_spec.rb @@ -131,7 +131,7 @@ def self.imbue(key, value) end it 'abort if :with option value is not Symbol, String or Proc' do - expect { subject.rescue_from :all, with: 1234 }.to raise_error(ArgumentError, 'with: Fixnum, expected Symbol, String or Proc') + expect { subject.rescue_from :all, with: 1234 }.to raise_error(ArgumentError, "with: #{integer_class_name}, expected Symbol, String or Proc") end it 'abort if both :with option and block are passed' do diff --git a/spec/grape/validations/validators/allow_blank_spec.rb b/spec/grape/validations/validators/allow_blank_spec.rb index 3921a42310..643ba04bec 100644 --- a/spec/grape/validations/validators/allow_blank_spec.rb +++ b/spec/grape/validations/validators/allow_blank_spec.rb @@ -57,9 +57,9 @@ class API < Grape::API get '/allow_float_blank' params do - requires :val, type: Fixnum, allow_blank: true + requires :val, type: Integer, allow_blank: true end - get '/allow_fixnum_blank' + get '/allow_integer_blank' params do requires :val, type: Symbol, allow_blank: true @@ -173,9 +173,9 @@ class API < Grape::API get '/allow_float_blank' params do - requires :val, type: Fixnum, allow_blank: true + requires :val, type: Integer, allow_blank: true end - get '/allow_fixnum_blank' + get '/allow_integer_blank' params do requires :val, type: Symbol, allow_blank: true @@ -345,8 +345,8 @@ def app expect(last_response.status).to eq(200) end - it 'accepts empty when fixnum allow_blank' do - get '/custom_message/allow_fixnum_blank', val: '' + it 'accepts empty when integer allow_blank' do + get '/custom_message/allow_integer_blank', val: '' expect(last_response.status).to eq(200) end end @@ -483,8 +483,8 @@ def app expect(last_response.status).to eq(200) end - it 'accepts empty when fixnum allow_blank' do - get '/allow_fixnum_blank', val: '' + it 'accepts empty when integer allow_blank' do + get '/allow_integer_blank', val: '' expect(last_response.status).to eq(200) end end diff --git a/spec/grape/validations/validators/coerce_spec.rb b/spec/grape/validations/validators/coerce_spec.rb index 07ad2cfb16..ca7ef224ae 100644 --- a/spec/grape/validations/validators/coerce_spec.rb +++ b/spec/grape/validations/validators/coerce_spec.rb @@ -175,7 +175,7 @@ class User get '/int', int: '45' expect(last_response.status).to eq(200) - expect(last_response.body).to eq('Fixnum') + expect(last_response.body).to eq(integer_class_name) end context 'Array' do @@ -189,7 +189,7 @@ class User get '/array', arry: %w(1 2 3) expect(last_response.status).to eq(200) - expect(last_response.body).to eq('Fixnum') + expect(last_response.body).to eq(integer_class_name) end it 'Array of Bools' do @@ -238,7 +238,7 @@ class User get '/set', set: Set.new([1, 2, 3, 4]).to_a expect(last_response.status).to eq(200) - expect(last_response.body).to eq('Fixnum') + expect(last_response.body).to eq(integer_class_name) end it 'Set of Bools' do @@ -326,7 +326,7 @@ class User get '/int', integers: { int: '45' } expect(last_response.status).to eq(200) - expect(last_response.body).to eq('Fixnum') + expect(last_response.body).to eq(integer_class_name) end end @@ -525,11 +525,11 @@ class User get '/', splines: '{"x":"1","y":"woof"}' expect(last_response.status).to eq(200) - expect(last_response.body).to eq('Fixnum.String') + expect(last_response.body).to eq("#{integer_class_name}.String") get '/', splines: '[{"x":1,"y":2},{"x":1,"y":"quack"}]' expect(last_response.status).to eq(200) - expect(last_response.body).to eq('Fixnum.Fixnum') + expect(last_response.body).to eq("#{integer_class_name}.#{integer_class_name}") get '/', splines: '{"x":"4","y":"woof"}' expect(last_response.status).to eq(400) @@ -576,7 +576,7 @@ class User get '/', a: '5' expect(last_response.status).to eq(200) - expect(last_response.body).to eq('Fixnum') + expect(last_response.body).to eq(integer_class_name) get '/', a: 'anything else' expect(last_response.status).to eq(200) diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 9f346cf08c..1e1cb10311 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -16,6 +16,7 @@ RSpec.configure do |config| config.include Rack::Test::Methods + config.include Spec::Support::Helpers config.raise_errors_for_deprecations! config.before(:each) { Grape::Util::InheritableSetting.reset_global! } diff --git a/spec/support/basic_auth_encode_helpers.rb b/spec/support/basic_auth_encode_helpers.rb index a5ab5553a9..0a841aa4a5 100644 --- a/spec/support/basic_auth_encode_helpers.rb +++ b/spec/support/basic_auth_encode_helpers.rb @@ -1,3 +1,9 @@ -def encode_basic_auth(username, password) - 'Basic ' + Base64.encode64("#{username}:#{password}") +module Spec + module Support + module Helpers + def encode_basic_auth(username, password) + 'Basic ' + Base64.encode64("#{username}:#{password}") + end + end + end end diff --git a/spec/support/content_type_helpers.rb b/spec/support/content_type_helpers.rb index 801b315a14..1ec1851b3f 100644 --- a/spec/support/content_type_helpers.rb +++ b/spec/support/content_type_helpers.rb @@ -1,11 +1,13 @@ -module ContentTypeHelpers - %w(put patch post delete).each do |method| - define_method :"#{method}_with_json" do |uri, params = {}, env = {}, &block| - params = params.to_json - env['CONTENT_TYPE'] ||= 'application/json' - send(method, uri, params, env, &block) +module Spec + module Support + module Helpers + %w(put patch post delete).each do |method| + define_method :"#{method}_with_json" do |uri, params = {}, env = {}, &block| + params = params.to_json + env['CONTENT_TYPE'] ||= 'application/json' + send(method, uri, params, env, &block) + end + end end end end - -include(ContentTypeHelpers) diff --git a/spec/support/integer_helpers.rb b/spec/support/integer_helpers.rb new file mode 100644 index 0000000000..88fba900f9 --- /dev/null +++ b/spec/support/integer_helpers.rb @@ -0,0 +1,11 @@ +module Spec + module Support + module Helpers + INTEGER_CLASS_NAME = 0.to_i.class.to_s.freeze + + def integer_class_name + INTEGER_CLASS_NAME + end + end + end +end diff --git a/spec/support/versioned_helpers.rb b/spec/support/versioned_helpers.rb index 045c9f521b..7423bdf927 100644 --- a/spec/support/versioned_helpers.rb +++ b/spec/support/versioned_helpers.rb @@ -1,50 +1,55 @@ # Versioning +module Spec + module Support + module Helpers + # Returns the path with options[:version] prefixed if options[:using] is :path. + # Returns normal path otherwise. + def versioned_path(options = {}) + case options[:using] + when :path + File.join('/', options[:prefix] || '', options[:version], options[:path]) + when :param + File.join('/', options[:prefix] || '', options[:path]) + when :header + File.join('/', options[:prefix] || '', options[:path]) + when :accept_version_header + File.join('/', options[:prefix] || '', options[:path]) + else + raise ArgumentError.new("unknown versioning strategy: #{options[:using]}") + end + end -# Returns the path with options[:version] prefixed if options[:using] is :path. -# Returns normal path otherwise. -def versioned_path(options = {}) - case options[:using] - when :path - File.join('/', options[:prefix] || '', options[:version], options[:path]) - when :param - File.join('/', options[:prefix] || '', options[:path]) - when :header - File.join('/', options[:prefix] || '', options[:path]) - when :accept_version_header - File.join('/', options[:prefix] || '', options[:path]) - else - raise ArgumentError.new("unknown versioning strategy: #{options[:using]}") - end -end - -def versioned_headers(options) - case options[:using] - when :path - {} # no-op - when :param - {} # no-op - when :header - { - 'HTTP_ACCEPT' => [ - "application/vnd.#{options[:vendor]}-#{options[:version]}", - options[:format] - ].compact.join('+') - } - when :accept_version_header - { - 'HTTP_ACCEPT_VERSION' => options[:version].to_s - } - else - raise ArgumentError.new("unknown versioning strategy: #{options[:using]}") - end -end + def versioned_headers(options) + case options[:using] + when :path + {} # no-op + when :param + {} # no-op + when :header + { + 'HTTP_ACCEPT' => [ + "application/vnd.#{options[:vendor]}-#{options[:version]}", + options[:format] + ].compact.join('+') + } + when :accept_version_header + { + 'HTTP_ACCEPT_VERSION' => options[:version].to_s + } + else + raise ArgumentError.new("unknown versioning strategy: #{options[:using]}") + end + end -def versioned_get(path, version_name, version_options = {}) - path = versioned_path(version_options.merge(version: version_name, path: path)) - headers = versioned_headers(version_options.merge(version: version_name)) - params = {} - if version_options[:using] == :param - params = { version_options[:parameter] => version_name } + def versioned_get(path, version_name, version_options = {}) + path = versioned_path(version_options.merge(version: version_name, path: path)) + headers = versioned_headers(version_options.merge(version: version_name)) + params = {} + if version_options[:using] == :param + params = { version_options[:parameter] => version_name } + end + get path, params, headers + end + end end - get path, params, headers end