From a77be516ee897dc95265cb60766f6f92b889374b Mon Sep 17 00:00:00 2001 From: Juan Carlos Garcia Date: Tue, 21 Nov 2023 23:41:52 +0100 Subject: [PATCH 1/6] fix(#1922): Allow to use instance variables defined in the endpoints when rescue_from --- lib/grape/dsl/inside_route.rb | 17 +++++++++++++++++ lib/grape/middleware/error.rb | 18 ++++++++++++++---- spec/grape/api_spec.rb | 19 +++++++++++++++++++ 3 files changed, 50 insertions(+), 4 deletions(-) diff --git a/lib/grape/dsl/inside_route.rb b/lib/grape/dsl/inside_route.rb index 7ebd6aab8f..e4937fd240 100644 --- a/lib/grape/dsl/inside_route.rb +++ b/lib/grape/dsl/inside_route.rb @@ -167,6 +167,23 @@ def error!(message, status = nil, additional_headers = nil) throw :error, message: message, status: self.status, headers: headers end + # Creates a Rack response based on the provided message, status, and headers. + # The content type in the headers is set to the default content type unless provided. + # The message is HTML-escaped if the content type is 'text/html'. + # + # @param message [String] The content of the response. + # @param status [Integer] The HTTP status code. + # @params headers [Hash] (optional) Headers for the response + # (default: {Rack::CONTENT_TYPE => content_type}). + # + # Returns: + # A Rack::Response object containing the specified message, status, and headers. + # + def rack_response(message, status, headers = { Rack::CONTENT_TYPE => content_type }) + message = ERB::Util.html_escape(message) if headers[Rack::CONTENT_TYPE] == 'text/html' + Rack::Response.new([message], Rack::Utils.status_code(status), headers) + end + # Redirect to a new url. # # @param url [String] The url to be redirect. diff --git a/lib/grape/middleware/error.rb b/lib/grape/middleware/error.rb index 05fc0312f2..02efe27526 100644 --- a/lib/grape/middleware/error.rb +++ b/lib/grape/middleware/error.rb @@ -46,7 +46,7 @@ def call!(env) rescue_handler_for_any_class(e.class) || raise - run_rescue_handler(handler, e) + run_rescue_handler(@env[Grape::Env::API_ENDPOINT], handler, e) end end @@ -119,21 +119,31 @@ def rescue_handler_for_any_class(klass) options[:all_rescue_handler] || :default_rescue_handler end - def run_rescue_handler(handler, error) + def run_rescue_handler(endpoint, handler, error) if handler.instance_of?(Symbol) raise NoMethodError, "undefined method '#{handler}'" unless respond_to?(handler) handler = public_method(handler) end - response = handler.arity.zero? ? instance_exec(&handler) : instance_exec(error, &handler) + response = (catch(:error) do + handler.arity.zero? ? endpoint.instance_exec(&handler) : endpoint.instance_exec(error, &handler) + end) + + if error?(response) + response = error!(response[:message], response[:status], response[:headers]) + end if response.is_a?(Rack::Response) response else - run_rescue_handler(:default_rescue_handler, Grape::Exceptions::InvalidResponse.new) + run_rescue_handler(endpoint, :default_rescue_handler, Grape::Exceptions::InvalidResponse.new) end end + + def error?(response) + response && response.is_a?(Hash) && response[:message] && response[:status] && response[:headers] + end end end end diff --git a/spec/grape/api_spec.rb b/spec/grape/api_spec.rb index b67dcbe24e..ee7a427be7 100644 --- a/spec/grape/api_spec.rb +++ b/spec/grape/api_spec.rb @@ -2130,6 +2130,25 @@ class CustomError < Grape::Exceptions::Base; end expect(last_response.status).to be 500 expect(last_response.body).to eql 'Invalid response' end + + context 'when using instance variables inside the rescue_from' do + it 'is able to access the values' do + expected_instance_variable_value = 'wadus' + + subject.rescue_from(:all) do + body = { my_var: @my_var } + error!(body, 400) + end + subject.get('/') do + @my_var = expected_instance_variable_value + raise + end + + get '/' + expect(last_response.status).to be 400 + expect(last_response.body).to eq({ my_var: expected_instance_variable_value }.to_json) + end + end end describe '.rescue_from klass, block' do From 7486d3c64963564727ae64e5234b1153cd695f2d Mon Sep 17 00:00:00 2001 From: Juan Carlos Garcia Date: Wed, 22 Nov 2023 00:04:30 +0100 Subject: [PATCH 2/6] fix(#1922): Update CHANGELOG and running rubocop --- CHANGELOG.md | 1 + lib/grape/dsl/inside_route.rb | 2 +- lib/grape/middleware/error.rb | 6 ++---- 3 files changed, 4 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 40acedb2af..f06411a660 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,7 @@ #### Features * [#2371](https://github.com/ruby-grape/grape/pull/2371): Use a param value as the `default` value of other param - [@jcagarcia](https://github.com/jcagarcia). +* [#2377](https://github.com/ruby-grape/grape/pull/2377): Allow to use instance variables values inside rescue_from - [@jcagarcia](https://github.com/jcagarcia). * Your contribution here. #### Fixes diff --git a/lib/grape/dsl/inside_route.rb b/lib/grape/dsl/inside_route.rb index e4937fd240..0286595b8c 100644 --- a/lib/grape/dsl/inside_route.rb +++ b/lib/grape/dsl/inside_route.rb @@ -179,7 +179,7 @@ def error!(message, status = nil, additional_headers = nil) # Returns: # A Rack::Response object containing the specified message, status, and headers. # - def rack_response(message, status, headers = { Rack::CONTENT_TYPE => content_type }) + def rack_response(message, status = 200, headers = { Rack::CONTENT_TYPE => content_type }) message = ERB::Util.html_escape(message) if headers[Rack::CONTENT_TYPE] == 'text/html' Rack::Response.new([message], Rack::Utils.status_code(status), headers) end diff --git a/lib/grape/middleware/error.rb b/lib/grape/middleware/error.rb index 02efe27526..2703d6849e 100644 --- a/lib/grape/middleware/error.rb +++ b/lib/grape/middleware/error.rb @@ -130,9 +130,7 @@ def run_rescue_handler(endpoint, handler, error) handler.arity.zero? ? endpoint.instance_exec(&handler) : endpoint.instance_exec(error, &handler) end) - if error?(response) - response = error!(response[:message], response[:status], response[:headers]) - end + response = error!(response[:message], response[:status], response[:headers]) if error?(response) if response.is_a?(Rack::Response) response @@ -142,7 +140,7 @@ def run_rescue_handler(endpoint, handler, error) end def error?(response) - response && response.is_a?(Hash) && response[:message] && response[:status] && response[:headers] + response.is_a?(Hash) && response[:message] && response[:status] && response[:headers] end end end From cef14da797b05d9fcf23aef598c7fe88b60b39c2 Mon Sep 17 00:00:00 2001 From: Juan Carlos Garcia Date: Thu, 23 Nov 2023 22:36:15 +0100 Subject: [PATCH 3/6] fix(#1922): Updating UPGRADING and README files explaining the instance variables behavior. Extra tests added --- README.md | 37 ++++++++++++++++++++ UPGRADING.md | 8 ++++- spec/grape/api_spec.rb | 76 +++++++++++++++++++++++++++++++----------- 3 files changed, 101 insertions(+), 20 deletions(-) diff --git a/README.md b/README.md index e6b68bc9d3..f599d5bc44 100644 --- a/README.md +++ b/README.md @@ -121,6 +121,7 @@ - [Current Route and Endpoint](#current-route-and-endpoint) - [Before, After and Finally](#before-after-and-finally) - [Anchoring](#anchoring) +- [Instance Variables](#instance-variables) - [Using Custom Middleware](#using-custom-middleware) - [Grape Middleware](#grape-middleware) - [Rails Middleware](#rails-middleware) @@ -3595,6 +3596,42 @@ end This will match all paths starting with '/statuses/'. There is one caveat though: the `params[:status]` parameter only holds the first part of the request url. Luckily this can be circumvented by using the described above syntax for path specification and using the `PATH_INFO` Rack environment variable, using `env['PATH_INFO']`. This will hold everything that comes after the '/statuses/' part. +## Instance Variables + +You can use instance variables to pass information across the various stages of a request. An instance variable set within a `before` validator is accessible within the endpoint's code and can also be utilized within the `rescue_from` handler. + +```ruby +class TwitterAPI < Grape::API + before do + @var = 1 + end + + get '/' do + puts @var # => 1 + raise + end + + rescue_from :all do + puts @var # => 1 + end +end +``` + +The values of instance variables cannot be shared among various endpoints within the same API. This limitation arises due to Grape generating a new instance for each request made. Consequently, instance variables set within an endpoint during one request differ from those set during a subsequent request, as they exist within separate instances. + +```ruby +class TwitterAPI < Grape::API + get '/first' do + @var = 1 + puts @var # => 1 + end + + get '/second' do + puts @var # => nil + end +end +``` + ## Using Custom Middleware ### Grape Middleware diff --git a/UPGRADING.md b/UPGRADING.md index 4cb1d4c62a..89c3ddadf0 100644 --- a/UPGRADING.md +++ b/UPGRADING.md @@ -1,7 +1,7 @@ Upgrading Grape =============== -### Upgrading to >= 2.0.1 +### Upgrading to >= 2.1.0 #### Grape::Router::Route.route_xxx methods have been removed @@ -9,6 +9,12 @@ Upgrading Grape - `route_path` is accessible through `path` - Any other `route_xyz` are accessible through `options[xyz]` +#### Instance variables scope + +Due to the changes done in [#2377](https://github.com/ruby-grape/grape/pull/2377), the instance variables defined inside each of the endpoints (or inside a `before` validator) are now accessible inside the `rescue_from`. This means the scope of the instance variables has changed. + +If you were using the same variable name defined inside an endpoint or `before` validator inside a `rescue_from` handler, you need to take in mind that you can start getting different values or you can be overriding values. + ### Upgrading to >= 2.0.0 #### Headers diff --git a/spec/grape/api_spec.rb b/spec/grape/api_spec.rb index ee7a427be7..f2ca6d55d7 100644 --- a/spec/grape/api_spec.rb +++ b/spec/grape/api_spec.rb @@ -2130,25 +2130,6 @@ class CustomError < Grape::Exceptions::Base; end expect(last_response.status).to be 500 expect(last_response.body).to eql 'Invalid response' end - - context 'when using instance variables inside the rescue_from' do - it 'is able to access the values' do - expected_instance_variable_value = 'wadus' - - subject.rescue_from(:all) do - body = { my_var: @my_var } - error!(body, 400) - end - subject.get('/') do - @my_var = expected_instance_variable_value - raise - end - - get '/' - expect(last_response.status).to be 400 - expect(last_response.body).to eq({ my_var: expected_instance_variable_value }.to_json) - end - end end describe '.rescue_from klass, block' do @@ -4371,4 +4352,61 @@ def uniqe_id_route expect(last_response.body).to be_eql('1-2') end end + + context 'instance variables' do + context 'when setting instance variables in a before validation' do + it 'is accessible inside the endpoint' do + expected_instance_variable_value = 'wadus' + + subject.before do + @my_var = expected_instance_variable_value + end + + subject.get('/') do + { my_var: @my_var }.to_json + end + + get '/' + expect(last_response.body).to eq({ my_var: expected_instance_variable_value }.to_json) + end + end + + context 'when setting instance variables inside the endpoint code' do + it 'is accessible inside the rescue_from handler' do + expected_instance_variable_value = 'wadus' + + subject.rescue_from(:all) do + body = { my_var: @my_var } + error!(body, 400) + end + + subject.get('/') do + @my_var = expected_instance_variable_value + raise + end + + get '/' + expect(last_response.status).to be 400 + expect(last_response.body).to eq({ my_var: expected_instance_variable_value }.to_json) + end + + it 'is NOT available in other endpoints of the same api' do + expected_instance_variable_value = 'wadus' + + subject.get('/first') do + @my_var = expected_instance_variable_value + { my_var: @my_var }.to_json + end + + subject.get('/second') do + { my_var: @my_var }.to_json + end + + get '/first' + expect(last_response.body).to eq({ my_var: expected_instance_variable_value }.to_json) + get '/second' + expect(last_response.body).to eq({ my_var: nil }.to_json) + end + end + end end From 229875e5cf524d1b7431c084bd8fe1a54fc57f4a Mon Sep 17 00:00:00 2001 From: Juan Carlos Garcia Date: Thu, 23 Nov 2023 23:18:16 +0100 Subject: [PATCH 4/6] fix(#1922): Send endpoint parameter as the last param of the run_rescue_handler method --- lib/grape/middleware/error.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/grape/middleware/error.rb b/lib/grape/middleware/error.rb index 2703d6849e..02db9aae0d 100644 --- a/lib/grape/middleware/error.rb +++ b/lib/grape/middleware/error.rb @@ -46,7 +46,7 @@ def call!(env) rescue_handler_for_any_class(e.class) || raise - run_rescue_handler(@env[Grape::Env::API_ENDPOINT], handler, e) + run_rescue_handler(handler, e, @env[Grape::Env::API_ENDPOINT]) end end @@ -119,7 +119,7 @@ def rescue_handler_for_any_class(klass) options[:all_rescue_handler] || :default_rescue_handler end - def run_rescue_handler(endpoint, handler, error) + def run_rescue_handler(handler, error, endpoint) if handler.instance_of?(Symbol) raise NoMethodError, "undefined method '#{handler}'" unless respond_to?(handler) @@ -135,7 +135,7 @@ def run_rescue_handler(endpoint, handler, error) if response.is_a?(Rack::Response) response else - run_rescue_handler(endpoint, :default_rescue_handler, Grape::Exceptions::InvalidResponse.new) + run_rescue_handler(:default_rescue_handler, Grape::Exceptions::InvalidResponse.new, endpoint) end end From b333940fbcd22a5527e9aad8d24da43937e18021 Mon Sep 17 00:00:00 2001 From: Juan Carlos Garcia Date: Fri, 24 Nov 2023 15:33:49 +0100 Subject: [PATCH 5/6] fix(#1922): Adding short before/after example to UPGRADING --- UPGRADING.md | 38 +++++++++++++++++++++++++++++++++++++- 1 file changed, 37 insertions(+), 1 deletion(-) diff --git a/UPGRADING.md b/UPGRADING.md index 89c3ddadf0..fb53565d07 100644 --- a/UPGRADING.md +++ b/UPGRADING.md @@ -11,10 +11,46 @@ Upgrading Grape #### Instance variables scope -Due to the changes done in [#2377](https://github.com/ruby-grape/grape/pull/2377), the instance variables defined inside each of the endpoints (or inside a `before` validator) are now accessible inside the `rescue_from`. This means the scope of the instance variables has changed. +Due to the changes done in [#2377](https://github.com/ruby-grape/grape/pull/2377), the instance variables defined inside each of the endpoints (or inside a `before` validator) are now accessible inside the `rescue_from`. The behavior of the instance variables was undefined until `2.1.0`. If you were using the same variable name defined inside an endpoint or `before` validator inside a `rescue_from` handler, you need to take in mind that you can start getting different values or you can be overriding values. +Before: +```ruby +class TwitterAPI < Grape::API + before do + @var = 1 + end + + get '/' do + puts @var # => 1 + raise + end + + rescue_from :all do + puts @var # => nil + end +end +``` + +After: +```ruby +class TwitterAPI < Grape::API + before do + @var = 1 + end + + get '/' do + puts @var # => 1 + raise + end + + rescue_from :all do + puts @var # => 1 + end +end +``` + ### Upgrading to >= 2.0.0 #### Headers From 1d1b40a8416bbc204b3d824d43d5780dae7125f5 Mon Sep 17 00:00:00 2001 From: Juan Carlos Garcia Date: Fri, 24 Nov 2023 15:35:34 +0100 Subject: [PATCH 6/6] fix(#1922): Fixing CHANGELOG format style --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f06411a660..c1fa986c24 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,7 +3,7 @@ #### Features * [#2371](https://github.com/ruby-grape/grape/pull/2371): Use a param value as the `default` value of other param - [@jcagarcia](https://github.com/jcagarcia). -* [#2377](https://github.com/ruby-grape/grape/pull/2377): Allow to use instance variables values inside rescue_from - [@jcagarcia](https://github.com/jcagarcia). +* [#2377](https://github.com/ruby-grape/grape/pull/2377): Allow to use instance variables values inside `rescue_from` - [@jcagarcia](https://github.com/jcagarcia). * Your contribution here. #### Fixes