Skip to content

Commit

Permalink
Avoid polluting Grape::Middleware::Error
Browse files Browse the repository at this point in the history
  • Loading branch information
namusyaka committed Jun 10, 2016
1 parent 0fdf327 commit 051f465
Show file tree
Hide file tree
Showing 4 changed files with 41 additions and 18 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
* [#1384](https://github.com/ruby-grape/grape/pull/1384): Fix parameter validation with an empty optional nested `Array` - [@ipkes](https://github.com/ipkes).
* [#1414](https://github.com/ruby-grape/grape/pull/1414): Fix multiple version definitions for path versioning - [@304](https://github.com/304).
* [#1415](https://github.com/ruby-grape/grape/pull/1415): Fix `declared(params, include_parent_namespaces: false)` - [@304](https://github.com/304).
* [#1421](https://github.com/ruby-grape/grape/pull/1421): Avoid polluting `Grape::Middleware::Error` - [@namusyaka](https://github.com/namusyaka).

0.16.2 (4/12/2016)
==================
Expand Down
7 changes: 4 additions & 3 deletions lib/grape/endpoint.rb
Original file line number Diff line number Diff line change
Expand Up @@ -244,11 +244,12 @@ def run
end
end

def build_stack
def build_stack(helpers)
stack = Grape::Middleware::Stack.new

stack.use Rack::Head
stack.use Grape::Middleware::Error,
stack.use Class.new(Grape::Middleware::Error),
helpers: helpers,
format: namespace_inheritable(:format),
content_types: namespace_stackable_with_hash(:content_types),
default_status: namespace_inheritable(:default_error_status),
Expand Down Expand Up @@ -300,10 +301,10 @@ def lazy_initialize!
@lazy_initialize_lock.synchronize do
return true if @lazy_initialized

@app = options[:app] || build_stack
@helpers = build_helpers.tap do |mod|
self.class.send(:include, mod)
end
@app = options[:app] || build_stack(@helpers)

@lazy_initialized = true
end
Expand Down
21 changes: 6 additions & 15 deletions lib/grape/middleware/error.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ def default_options
default_status: 500, # default status returned on error
default_message: '',
format: :txt,
helpers: nil,
formatters: {},
error_formatters: {},
rescue_all: false, # true to rescue all exceptions
Expand All @@ -19,11 +20,14 @@ def default_options
}
end

def initialize(app, options = {})
super
self.class.send(:include, @options[:helpers]) if @options[:helpers]
end

def call!(env)
@env = env

inject_helpers!

begin
error_response(catch(:error) do
return @app.call(@env)
Expand Down Expand Up @@ -67,19 +71,6 @@ def exec_handler(e, &handler)
end
end

def inject_helpers!
return if helpers_available?
endpoint = @env['api.endpoint']
self.class.instance_eval do
include endpoint.send(:helpers)
end if endpoint.is_a?(Grape::Endpoint)
@helpers = true
end

def helpers_available?
@helpers
end

def error!(message, status = options[:default_status], headers = {}, backtrace = [])
headers = headers.reverse_merge(Grape::Http::Headers::CONTENT_TYPE => content_type)
rack_response(format_message(message, backtrace), status, headers)
Expand Down
30 changes: 30 additions & 0 deletions spec/grape/api_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -1456,6 +1456,36 @@ def custom_error!(name)
expect(last_response.body).to eq 'hello bob'
end

context 'with multiple apis' do
let(:a) { Class.new(Grape::API) }
let(:b) { Class.new(Grape::API) }

before do
a.helpers do
def foo
error!('foo', 401)
end
end
a.rescue_from(:all) { foo }
a.get { raise 'boo' }
b.helpers do
def foo
error!('bar', 401)
end
end
b.rescue_from(:all) { foo }
b.get { raise 'boo' }
end

it 'avoids polluting global namespace' do
env = Rack::MockRequest.env_for('/')

expect(a.call(env)[2].body).to eq(['foo'])
expect(b.call(env)[2].body).to eq(['bar'])
expect(a.call(env)[2].body).to eq(['foo'])
end
end

it 'rescues all errors if rescue_from :all is called' do
subject.rescue_from :all
subject.get '/exception' do
Expand Down

0 comments on commit 051f465

Please sign in to comment.