Skip to content

Commit 7805dd0

Browse files
namusyakadblock
authored andcommitted
Avoid polluting Grape::Middleware::Error (#1421)
1 parent 0fdf327 commit 7805dd0

File tree

4 files changed

+41
-18
lines changed

4 files changed

+41
-18
lines changed

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
* [#1384](https://github.com/ruby-grape/grape/pull/1384): Fix parameter validation with an empty optional nested `Array` - [@ipkes](https://github.com/ipkes).
1818
* [#1414](https://github.com/ruby-grape/grape/pull/1414): Fix multiple version definitions for path versioning - [@304](https://github.com/304).
1919
* [#1415](https://github.com/ruby-grape/grape/pull/1415): Fix `declared(params, include_parent_namespaces: false)` - [@304](https://github.com/304).
20+
* [#1421](https://github.com/ruby-grape/grape/pull/1421): Avoid polluting `Grape::Middleware::Error` - [@namusyaka](https://github.com/namusyaka).
2021

2122
0.16.2 (4/12/2016)
2223
==================

lib/grape/endpoint.rb

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -244,11 +244,12 @@ def run
244244
end
245245
end
246246

247-
def build_stack
247+
def build_stack(helpers)
248248
stack = Grape::Middleware::Stack.new
249249

250250
stack.use Rack::Head
251-
stack.use Grape::Middleware::Error,
251+
stack.use Class.new(Grape::Middleware::Error),
252+
helpers: helpers,
252253
format: namespace_inheritable(:format),
253254
content_types: namespace_stackable_with_hash(:content_types),
254255
default_status: namespace_inheritable(:default_error_status),
@@ -300,10 +301,10 @@ def lazy_initialize!
300301
@lazy_initialize_lock.synchronize do
301302
return true if @lazy_initialized
302303

303-
@app = options[:app] || build_stack
304304
@helpers = build_helpers.tap do |mod|
305305
self.class.send(:include, mod)
306306
end
307+
@app = options[:app] || build_stack(@helpers)
307308

308309
@lazy_initialized = true
309310
end

lib/grape/middleware/error.rb

Lines changed: 6 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ def default_options
88
default_status: 500, # default status returned on error
99
default_message: '',
1010
format: :txt,
11+
helpers: nil,
1112
formatters: {},
1213
error_formatters: {},
1314
rescue_all: false, # true to rescue all exceptions
@@ -19,11 +20,14 @@ def default_options
1920
}
2021
end
2122

23+
def initialize(app, options = {})
24+
super
25+
self.class.send(:include, @options[:helpers]) if @options[:helpers]
26+
end
27+
2228
def call!(env)
2329
@env = env
2430

25-
inject_helpers!
26-
2731
begin
2832
error_response(catch(:error) do
2933
return @app.call(@env)
@@ -67,19 +71,6 @@ def exec_handler(e, &handler)
6771
end
6872
end
6973

70-
def inject_helpers!
71-
return if helpers_available?
72-
endpoint = @env['api.endpoint']
73-
self.class.instance_eval do
74-
include endpoint.send(:helpers)
75-
end if endpoint.is_a?(Grape::Endpoint)
76-
@helpers = true
77-
end
78-
79-
def helpers_available?
80-
@helpers
81-
end
82-
8374
def error!(message, status = options[:default_status], headers = {}, backtrace = [])
8475
headers = headers.reverse_merge(Grape::Http::Headers::CONTENT_TYPE => content_type)
8576
rack_response(format_message(message, backtrace), status, headers)

spec/grape/api_spec.rb

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1456,6 +1456,36 @@ def custom_error!(name)
14561456
expect(last_response.body).to eq 'hello bob'
14571457
end
14581458

1459+
context 'with multiple apis' do
1460+
let(:a) { Class.new(Grape::API) }
1461+
let(:b) { Class.new(Grape::API) }
1462+
1463+
before do
1464+
a.helpers do
1465+
def foo
1466+
error!('foo', 401)
1467+
end
1468+
end
1469+
a.rescue_from(:all) { foo }
1470+
a.get { raise 'boo' }
1471+
b.helpers do
1472+
def foo
1473+
error!('bar', 401)
1474+
end
1475+
end
1476+
b.rescue_from(:all) { foo }
1477+
b.get { raise 'boo' }
1478+
end
1479+
1480+
it 'avoids polluting global namespace' do
1481+
env = Rack::MockRequest.env_for('/')
1482+
1483+
expect(a.call(env)[2].body).to eq(['foo'])
1484+
expect(b.call(env)[2].body).to eq(['bar'])
1485+
expect(a.call(env)[2].body).to eq(['foo'])
1486+
end
1487+
end
1488+
14591489
it 'rescues all errors if rescue_from :all is called' do
14601490
subject.rescue_from :all
14611491
subject.get '/exception' do

0 commit comments

Comments
 (0)