Skip to content
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

Avoid noise when code runs with Ruby warnings #1526

Merged
merged 1 commit into from
Dec 1, 2016
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 @@ -13,6 +13,7 @@ Next Release
* [#1517](https://github.com/ruby-grape/grape/pull/1517), [#1089](https://github.com/ruby-grape/grape/pull/1089): Fix: priority of ANY routes - [@namusyaka](https://github.com/namusyaka), [@wagenet](https://github.com/wagenet).
* [#1512](https://github.com/ruby-grape/grape/pull/1512): Fix: deeply nested parameters are included within `#declared(params)` - [@krbs](https://github.com/krbs).
* [#1510](https://github.com/ruby-grape/grape/pull/1510): Fix: inconsistent validation for multiple parameters - [@dgasper](https://github.com/dgasper).
* [#1526](https://github.com/ruby-grape/grape/pull/1526): Reduce warnings caused by instance variables not initialized - [@cpetschnig](https://github.com/cpetschnig).

0.18.0 (10/7/2016)
==================
Expand Down
2 changes: 2 additions & 0 deletions lib/grape/endpoint.rb
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,8 @@ def initialize(new_settings, options = {}, &block)
@options[:route_options] ||= {}

@lazy_initialize_lock = Mutex.new
@lazy_initialized = nil
@block = nil

return unless block_given?

Expand Down
1 change: 1 addition & 0 deletions lib/grape/middleware/base.rb
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ class Base
def initialize(app, **options)
@app = app
@options = default_options.merge(**options)
@app_response = nil
end

def default_options
Expand Down
1 change: 1 addition & 0 deletions lib/grape/validations/params_scope.rb
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ def initialize(opts, &block)
@group = opts[:group] || {}
@dependent_on = opts[:dependent_on]
@declared_params = []
@index = nil

instance_eval(&block) if block_given?

Expand Down
2 changes: 1 addition & 1 deletion spec/grape/api_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -1202,7 +1202,7 @@ class PhonyMiddleware
def initialize(app, *args)
@args = args
@app = app
@block = true if block_given?
@block = block_given? ? true : nil
end

def call(env)
Expand Down
69 changes: 39 additions & 30 deletions spec/grape/middleware/formatter_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,46 +4,53 @@
subject { Grape::Middleware::Formatter.new(app) }
before { allow(subject).to receive(:dup).and_return(subject) }

let(:app) { ->(_env) { [200, {}, [@body || { 'foo' => 'bar' }]] } }
let(:body) { { 'foo' => 'bar' } }
let(:app) { ->(_env) { [200, {}, [body]] } }

context 'serialization' do
let(:body) { { 'abc' => 'def' } }
it 'looks at the bodies for possibly serializable data' do
@body = { 'abc' => 'def' }
_, _, bodies = *subject.call('PATH_INFO' => '/somewhere', 'HTTP_ACCEPT' => 'application/json')
bodies.each { |b| expect(b).to eq(MultiJson.dump(@body)) }
bodies.each { |b| expect(b).to eq(MultiJson.dump(body)) }
end

it 'calls #to_json since default format is json' do
@body = ['foo']
@body.instance_eval do
def to_json
'"bar"'
context 'default format' do
let(:body) { ['foo'] }
it 'calls #to_json since default format is json' do
body.instance_eval do
def to_json
'"bar"'
end
end
end

subject.call('PATH_INFO' => '/somewhere', 'HTTP_ACCEPT' => 'application/json').to_a.last.each { |b| expect(b).to eq('"bar"') }
subject.call('PATH_INFO' => '/somewhere', 'HTTP_ACCEPT' => 'application/json').to_a.last.each { |b| expect(b).to eq('"bar"') }
end
end

it 'calls #to_json if the content type is jsonapi' do
@body = { 'foos' => [{ 'bar' => 'baz' }] }
@body.instance_eval do
def to_json
'{"foos":[{"bar":"baz"}] }'
context 'jsonapi' do
let(:body) { { 'foos' => [{ 'bar' => 'baz' }] } }
it 'calls #to_json if the content type is jsonapi' do
body.instance_eval do
def to_json
'{"foos":[{"bar":"baz"}] }'
end
end
end

subject.call('PATH_INFO' => '/somewhere', 'HTTP_ACCEPT' => 'application/vnd.api+json').to_a.last.each { |b| expect(b).to eq('{"foos":[{"bar":"baz"}] }') }
subject.call('PATH_INFO' => '/somewhere', 'HTTP_ACCEPT' => 'application/vnd.api+json').to_a.last.each { |b| expect(b).to eq('{"foos":[{"bar":"baz"}] }') }
end
end

it 'calls #to_xml if the content type is xml' do
@body = 'string'
@body.instance_eval do
def to_xml
'<bar/>'
context 'xml' do
let(:body) { 'string' }
it 'calls #to_xml if the content type is xml' do
body.instance_eval do
def to_xml
'<bar/>'
end
end
end

subject.call('PATH_INFO' => '/somewhere.xml', 'HTTP_ACCEPT' => 'application/json').to_a.last.each { |b| expect(b).to eq('<bar/>') }
subject.call('PATH_INFO' => '/somewhere.xml', 'HTTP_ACCEPT' => 'application/json').to_a.last.each { |b| expect(b).to eq('<bar/>') }
end
end
end

Expand Down Expand Up @@ -189,10 +196,12 @@ def to_xml
_, _, body = subject.call('PATH_INFO' => '/info.custom')
expect(body.body).to eq(['CUSTOM FORMAT'])
end
it 'uses default json formatter' do
@body = ['blah']
_, _, body = subject.call('PATH_INFO' => '/info.json')
expect(body.body).to eq(['["blah"]'])
context 'default' do
let(:body) { ['blah'] }
it 'uses default json formatter' do
_, _, body = subject.call('PATH_INFO' => '/info.json')
expect(body.body).to eq(['["blah"]'])
end
end
it 'uses custom json formatter' do
subject.options[:formatters][:json] = ->(_obj, _env) { 'CUSTOM JSON FORMAT' }
Expand Down Expand Up @@ -284,10 +293,10 @@ def to_xml
end

context 'send file' do
let(:app) { ->(_env) { [200, {}, @body] } }
let(:body) { Grape::ServeFile::FileResponse.new('file') }
let(:app) { ->(_env) { [200, {}, body] } }

it 'returns Grape::Uril::SendFileReponse' do
@body = Grape::ServeFile::FileResponse.new('file')
env = { 'PATH_INFO' => '/somewhere', 'HTTP_ACCEPT' => 'application/json' }
expect(subject.call(env)).to be_a(Grape::ServeFile::SendfileResponse)
end
Expand Down
25 changes: 15 additions & 10 deletions spec/grape/middleware/versioner/param_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,8 @@

describe Grape::Middleware::Versioner::Param do
let(:app) { ->(env) { [200, env, env['api.version']] } }
subject { Grape::Middleware::Versioner::Param.new(app, @options || {}) }
let(:options) { {} }
subject { Grape::Middleware::Versioner::Param.new(app, options) }

it 'sets the API version based on the default param (apiver)' do
env = Rack::MockRequest.env_for('/awesome', params: { 'apiver' => 'v1' })
Expand All @@ -22,7 +23,7 @@
end

context 'with specified parameter name' do
before { @options = { version_options: { parameter: 'v' } } }
let(:options) { { version_options: { parameter: 'v' } } }
it 'sets the API version based on the custom parameter name' do
env = Rack::MockRequest.env_for('/awesome', params: { 'v' => 'v1' })
expect(subject.call(env)[1]['api.version']).to eq('v1')
Expand All @@ -34,7 +35,7 @@
end

context 'with specified versions' do
before { @options = { versions: %w(v1 v2) } }
let(:options) { { versions: %w(v1 v2) } }
it 'throws an error if a non-allowed version is specified' do
env = Rack::MockRequest.env_for('/awesome', params: { 'apiver' => 'v3' })
expect(catch(:error) { subject.call(env) }[:status]).to eq(404)
Expand All @@ -45,13 +46,17 @@
end
end

it 'returns a 200 when no version is set (matches the first version found)' do
@options = {
versions: ['v1'],
version_options: { using: :header }
}
env = Rack::MockRequest.env_for('/awesome', params: {})
expect(subject.call(env).first).to eq(200)
context 'when no version is set' do
let(:options) do
{
versions: ['v1'],
version_options: { using: :header }
}
end
it 'returns a 200 (matches the first version found)' do
env = Rack::MockRequest.env_for('/awesome', params: {})
expect(subject.call(env).first).to eq(200)
end
end

context 'when there are multiple versions without a custom param' do
Expand Down
7 changes: 4 additions & 3 deletions spec/grape/middleware/versioner/path_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,8 @@

describe Grape::Middleware::Versioner::Path do
let(:app) { ->(env) { [200, env, env['api.version']] } }
subject { Grape::Middleware::Versioner::Path.new(app, @options || {}) }
let(:options) { {} }
subject { Grape::Middleware::Versioner::Path.new(app, options) }

it 'sets the API version based on the first path' do
expect(subject.call('PATH_INFO' => '/v1/awesome').last).to eq('v1')
Expand All @@ -17,7 +18,7 @@
end

context 'with a pattern' do
before { @options = { pattern: /v./i } }
let(:options) { { pattern: /v./i } }
it 'sets the version if it matches' do
expect(subject.call('PATH_INFO' => '/v1/awesome').last).to eq('v1')
end
Expand All @@ -29,7 +30,7 @@

[%w(v1 v2), [:v1, :v2], [:v1, 'v2'], ['v1', :v2]].each do |versions|
context "with specified versions as #{versions}" do
before { @options = { versions: versions } }
let(:options) { { versions: versions } }

it 'throws an error if a non-allowed version is specified' do
expect(catch(:error) { subject.call('PATH_INFO' => '/v3/awesome') }[:status]).to eq(404)
Expand Down