diff --git a/CHANGELOG.md b/CHANGELOG.md index 5c34e326f..83c226ec2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,7 @@ * [#2572](https://github.com/ruby-grape/grape/pull/2572): Drop support ruby 2.7 and active_support 6.1 - [@ericproulx](https://github.com/ericproulx). * [#2573](https://github.com/ruby-grape/grape/pull/2573): Clean up deprecated code - [@ericproulx](https://github.com/ericproulx). * [#2575](https://github.com/ruby-grape/grape/pull/2575): Refactor Api description class - [@ericproulx](https://github.com/ericproulx). +* [#2577](https://github.com/ruby-grape/grape/pull/2577): Deprecate `return` in endpoint execution - [@ericproulx](https://github.com/ericproulx). * Your contribution here. #### Fixes diff --git a/UPGRADING.md b/UPGRADING.md index 8eb728265..c459c789b 100644 --- a/UPGRADING.md +++ b/UPGRADING.md @@ -3,6 +3,12 @@ Upgrading Grape ### Upgrading to >= 3.0.0 +### Endpoint execution simplified and `return` deprecated + +Executing a endpoint's block has been simplified and calling `return` in it has been deprecated. + +See [#2577](https://github.com/ruby-grape/grape/pull/2577) for more information. + #### Old Deprecations Clean Up - `rack_response` has been removed in favor of using `error!`. diff --git a/lib/grape/endpoint.rb b/lib/grape/endpoint.rb index 9280a2147..6af3bed53 100644 --- a/lib/grape/endpoint.rb +++ b/lib/grape/endpoint.rb @@ -37,33 +37,6 @@ def run_before_each(endpoint) superclass.run_before_each(endpoint) unless self == Endpoint before_each.each { |blk| blk.try(:call, endpoint) } end - - # @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) - raise NameError.new("method #{method_name.inspect} already exists and cannot be used as an unbound method name") if method_defined?(method_name) - - define_method(method_name, &block) - method = instance_method(method_name) - remove_method(method_name) - - proc do |endpoint_instance| - ActiveSupport::Notifications.instrument('endpoint_render.grape', endpoint: endpoint_instance) do - method.bind_call(endpoint_instance) - end - end - end end # Create a new endpoint. @@ -108,12 +81,18 @@ def initialize(new_settings, options = {}, &block) @status = nil @stream = nil @body = nil - @proc = nil return unless block @source = block - @block = self.class.generate_api_method(method_name, &block) + @block = lambda do |endpoint_instance| + ActiveSupport::Notifications.instrument('endpoint_render.grape', endpoint: endpoint_instance) do + endpoint_instance.instance_exec(&block) + rescue LocalJumpError => e + Grape.deprecator.warn 'Using `return` in an endpoint has been deprecated.' + return e.exit_value + end + end end # Update our settings from a given set of stackable parameters. Used when @@ -131,14 +110,6 @@ def require_option(options, key) raise Grape::Exceptions::MissingOption.new(key) unless options.key?(key) end - def method_name - [options[:method], - Namespace.joined_space(namespace_stackable(:namespace)), - (namespace_stackable(:mount_path) || []).join('/'), - options[:path].join('/')] - .join(' ') - end - def routes @routes ||= endpoints&.collect(&:routes)&.flatten || to_routes end diff --git a/spec/grape/endpoint_spec.rb b/spec/grape/endpoint_spec.rb index 3f98aacb6..29c332e8e 100644 --- a/spec/grape/endpoint_spec.rb +++ b/spec/grape/endpoint_spec.rb @@ -785,29 +785,13 @@ def memoized return 'Hello' end + expect(Grape.deprecator).to receive(:warn).with('Using `return` in an endpoint has been deprecated.') + get '/home' expect(last_response.status).to eq(200) expect(last_response.body).to eq('Hello') end - describe '.generate_api_method' do - it 'raises NameError if the method name is already in use' do - expect do - described_class.generate_api_method('version', &proc {}) - end.to raise_error(NameError) - end - - it 'raises ArgumentError if a block is not given' do - expect do - described_class.generate_api_method('GET without a block method') - end.to raise_error(ArgumentError) - end - - it 'returns a Proc' do - expect(described_class.generate_api_method('GET test for a proc', &proc {})).to be_a Proc - end - end - context 'filters' do describe 'before filters' do it 'runs the before filter if set' do