Skip to content

Commit

Permalink
Explicit return calls do not raise LocalJumpError
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
gruis committed Nov 1, 2012
1 parent e2f6403 commit 3bbb4fe
Show file tree
Hide file tree
Showing 3 changed files with 59 additions and 4 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.markdown
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
====================

* [#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).
* Fix: LocalJumpError will not be raised when using explict return in API methods - [@simulacre](https://github.com/simulacre)
* Your contribution here.

0.2.2
Expand Down
34 changes: 31 additions & 3 deletions lib/grape/endpoint.rb
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,37 @@ class Endpoint
attr_accessor :block, :options, :settings
attr_reader :env, :request

class << self
# @api private
#
# Create an UnboundMethod that is appropriate for executing an endpoint
# route.
#
# The unbound method allows explicit calls to +return+ without raising a
# +LocalJumpError+. The method will be removed, but a +Proc+ reference to
# it will be returned. The returned +Proc+ expects a single argument: the
# instance of +Endpoint+ to bind to the method during the call.
#
# @param [String, Symbol] method_name
# @return [Proc]
# @raise [NameError] an instance method with the same name already exists
def generate_api_method(method_name, &block)
if instance_methods.include?(method_name.to_sym)
raise NameError.new("method #{method_name.inspect} already exists and cannot be used as an unbound method name")
end
define_method(method_name, &block)
method = instance_method(method_name)
remove_method(method_name)
proc { |endpoint_instance| method.bind(endpoint_instance).call }
end
end

def initialize(settings, options = {}, &block)
@settings = settings
@block = block
if block_given?
method_name = "#{options[:method]} #{settings.gather(:namespace).join( "/")} #{Array(options[:path]).join("/")}"
@block = self.class.generate_api_method(method_name, &block)
end
@options = options

raise ArgumentError, "Must specify :path option." unless options.key?(:path)
Expand Down Expand Up @@ -135,7 +163,7 @@ def declared(params, options = {})
unless settings[:declared_params]
raise ArgumentError, "Tried to filter for declared parameters but none exist."
end

settings[:declared_params].inject({}){|h,k|
output_key = options[:stringify] ? k.to_s : k.to_sym
if params.key?(output_key) || options[:include_missing]
Expand Down Expand Up @@ -324,7 +352,7 @@ def run(env)

run_filters after_validations

response_text = instance_eval &self.block
response_text = @block.call(self)
run_filters afters
cookies.write(header)

Expand Down
28 changes: 27 additions & 1 deletion spec/grape/endpoint_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -293,7 +293,7 @@ def app; subject end
last_response.body.should == '{"dude":"rad"}'
end
end

describe "#redirect" do
it "should redirect to a url with status 302" do
subject.get('/hey') do
Expand Down Expand Up @@ -354,6 +354,32 @@ def memoized
last_response.body.should == 'yo'
end

it 'should allow explicit return calls' do
subject.get('/home') do
return "Hello"
end

get '/home'
last_response.status.should == 200
last_response.body.should == "Hello"
end

describe ".generate_api_method" do
it "should raise NameError if the method name is already in use" do
expect {
Grape::Endpoint.generate_api_method("version", &proc{})
}.to raise_error(NameError)
end
it "should raise ArgumentError if a block is not given" do
expect {
Grape::Endpoint.generate_api_method("GET without a block method")
}.to raise_error(ArgumentError)
end
it "should return a Proc" do
Grape::Endpoint.generate_api_method("GET test for a proc", &proc{}).should be_a Proc
end
end

describe '#present' do
it 'should just set the object as the body if no options are provided' do
subject.get '/example' do
Expand Down

0 comments on commit 3bbb4fe

Please sign in to comment.