Skip to content

Commit 3bbb4fe

Browse files
committed
Explicit return calls do not raise LocalJumpError
Instead of calling instance_eval on the block passed into Endpoint#initialize, create an anonymous UnboundMethod with the block as the method body. When executing the block bind the instance of Endpoint to the UnboundMethod and call it. This behavior and solution is taken from Sinatra. This solution also makes it possible to pass the values of Endpoint#params as arguments to the block. That feature is not present in this commit, but is trivial to implement.
1 parent e2f6403 commit 3bbb4fe

File tree

3 files changed

+59
-4
lines changed

3 files changed

+59
-4
lines changed

CHANGELOG.markdown

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
====================
33

44
* [#265](https://github.com/intridea/grape/issues/264): Fix: The class ValidationError should be in the module "Grape::Exceptions". Fixes [#264](https://github.com/intridea/grape/issues/264) - [@thepumpkin1979](https://github.com/thepumpkin1979).
5+
* Fix: LocalJumpError will not be raised when using explict return in API methods - [@simulacre](https://github.com/simulacre)
56
* Your contribution here.
67

78
0.2.2

lib/grape/endpoint.rb

Lines changed: 31 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -11,9 +11,37 @@ class Endpoint
1111
attr_accessor :block, :options, :settings
1212
attr_reader :env, :request
1313

14+
class << self
15+
# @api private
16+
#
17+
# Create an UnboundMethod that is appropriate for executing an endpoint
18+
# route.
19+
#
20+
# The unbound method allows explicit calls to +return+ without raising a
21+
# +LocalJumpError+. The method will be removed, but a +Proc+ reference to
22+
# it will be returned. The returned +Proc+ expects a single argument: the
23+
# instance of +Endpoint+ to bind to the method during the call.
24+
#
25+
# @param [String, Symbol] method_name
26+
# @return [Proc]
27+
# @raise [NameError] an instance method with the same name already exists
28+
def generate_api_method(method_name, &block)
29+
if instance_methods.include?(method_name.to_sym)
30+
raise NameError.new("method #{method_name.inspect} already exists and cannot be used as an unbound method name")
31+
end
32+
define_method(method_name, &block)
33+
method = instance_method(method_name)
34+
remove_method(method_name)
35+
proc { |endpoint_instance| method.bind(endpoint_instance).call }
36+
end
37+
end
38+
1439
def initialize(settings, options = {}, &block)
1540
@settings = settings
16-
@block = block
41+
if block_given?
42+
method_name = "#{options[:method]} #{settings.gather(:namespace).join( "/")} #{Array(options[:path]).join("/")}"
43+
@block = self.class.generate_api_method(method_name, &block)
44+
end
1745
@options = options
1846

1947
raise ArgumentError, "Must specify :path option." unless options.key?(:path)
@@ -135,7 +163,7 @@ def declared(params, options = {})
135163
unless settings[:declared_params]
136164
raise ArgumentError, "Tried to filter for declared parameters but none exist."
137165
end
138-
166+
139167
settings[:declared_params].inject({}){|h,k|
140168
output_key = options[:stringify] ? k.to_s : k.to_sym
141169
if params.key?(output_key) || options[:include_missing]
@@ -324,7 +352,7 @@ def run(env)
324352

325353
run_filters after_validations
326354

327-
response_text = instance_eval &self.block
355+
response_text = @block.call(self)
328356
run_filters afters
329357
cookies.write(header)
330358

spec/grape/endpoint_spec.rb

Lines changed: 27 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -293,7 +293,7 @@ def app; subject end
293293
last_response.body.should == '{"dude":"rad"}'
294294
end
295295
end
296-
296+
297297
describe "#redirect" do
298298
it "should redirect to a url with status 302" do
299299
subject.get('/hey') do
@@ -354,6 +354,32 @@ def memoized
354354
last_response.body.should == 'yo'
355355
end
356356

357+
it 'should allow explicit return calls' do
358+
subject.get('/home') do
359+
return "Hello"
360+
end
361+
362+
get '/home'
363+
last_response.status.should == 200
364+
last_response.body.should == "Hello"
365+
end
366+
367+
describe ".generate_api_method" do
368+
it "should raise NameError if the method name is already in use" do
369+
expect {
370+
Grape::Endpoint.generate_api_method("version", &proc{})
371+
}.to raise_error(NameError)
372+
end
373+
it "should raise ArgumentError if a block is not given" do
374+
expect {
375+
Grape::Endpoint.generate_api_method("GET without a block method")
376+
}.to raise_error(ArgumentError)
377+
end
378+
it "should return a Proc" do
379+
Grape::Endpoint.generate_api_method("GET test for a proc", &proc{}).should be_a Proc
380+
end
381+
end
382+
357383
describe '#present' do
358384
it 'should just set the object as the body if no options are provided' do
359385
subject.get '/example' do

0 commit comments

Comments
 (0)