From 22b3aefda6ef31cda2379b3d80ab8299adddd27a Mon Sep 17 00:00:00 2001 From: dblock Date: Thu, 21 Nov 2013 14:10:32 -0500 Subject: [PATCH] Don't create Grape::Request multiple times. --- CHANGELOG.md | 2 ++ README.md | 5 ++--- lib/grape/endpoint.rb | 18 +++++------------- lib/grape/http/request.rb | 4 ++-- lib/grape/middleware/auth/oauth2.rb | 10 +++++++++- lib/grape/middleware/base.rb | 9 ++++----- lib/grape/middleware/formatter.rb | 4 ++++ lib/grape/middleware/versioner/param.rb | 5 ++--- spec/grape/middleware/base_spec.rb | 5 ----- spec/grape/middleware/formatter_spec.rb | 5 ++++- spec/grape/middleware/versioner/param_spec.rb | 1 + 11 files changed, 35 insertions(+), 33 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ccb4f62f62..230794ad62 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,7 @@ Next Release ============ #### Features + * [#510](https://github.com/intridea/grape/pull/510): Support lambda-based default values for params - [@myitcv](https://github.com/myitcv). * [#511](https://github.com/intridea/grape/pull/511): Add `required` option for OAuth2 middleware - [@bcm](https://github.com/bcm). * Your contribution here. @@ -13,6 +14,7 @@ Next Release * [#495](https://github.com/intridea/grape/pull/495): Fix `ParamsScope#params` for parameters nested inside arrays - [@asross](https://github.com/asross). * [#498](https://github.com/intridea/grape/pull/498): Dry up options and headers logic, allow headers to be passed to OPTIONS requests - [@karlfreeman](https://github.com/karlfreeman). * [#500](https://github.com/intridea/grape/pull/500): Skip entity auto-detection when explicitely passed - [@yaneq](https://github.com/yaneq). +* [#512](https://github.com/intridea/grape/pull/512): Don't create `Grape::Request` multiple times - [@dblock](https://github.com/dblock). * Your contribution here. diff --git a/README.md b/README.md index 32ec0c6621..c454085ec2 100644 --- a/README.md +++ b/README.md @@ -360,14 +360,14 @@ Optional parameters can have a default value. params do optional :color, type: String, default: 'blue' optional :random_number, type: Integer, default: -> { Random.rand(1..100) } - optional :non_random_number, type: Integer, default: Random.rand(1..100) + optional :non_random_number, type: Integer, default: Random.rand(1..100) end ``` Parameters can be restricted to a specific set of values with the `:values` option. Default values are eagerly evaluated. Above `:non_random_number` will evaluate to the same -number for each call to the endpoint of this `params` block. To have the default evaluate +number for each call to the endpoint of this `params` block. To have the default evaluate at calltime use a lambda, like `:random_number` above. ```ruby @@ -1236,7 +1236,6 @@ GET /foo # 'root - foo - blah' GET /foo/bar # 'root - foo - bar - blah' ``` - ## Anchoring Grape by default anchors all request paths, which means that the request URL diff --git a/lib/grape/endpoint.rb b/lib/grape/endpoint.rb index 71a3043966..5118c6d441 100644 --- a/lib/grape/endpoint.rb +++ b/lib/grape/endpoint.rb @@ -5,7 +5,7 @@ module Grape # from inside a `get`, `post`, etc. class Endpoint attr_accessor :block, :source, :options, :settings - attr_reader :env, :request + attr_reader :env, :request, :headers, :params class << self # @api private @@ -156,12 +156,6 @@ def call!(env) end end - # The parameters passed into the request as - # well as parsed from URL segments. - def params - @params ||= @request.params - end - # A filtering method that will return a hash # consisting only of keys that have been declared by a # `params` statement. @@ -260,11 +254,6 @@ def header(key = nil, val = nil) end end - # Retrieves all available request headers. - def headers - @headers ||= @request.headers - end - # Set response content-type def content_type(val) header('Content-Type', val) @@ -377,7 +366,10 @@ def endpoints def run(env) @env = env @header = {} - @request = Grape::Request.new(@env) + + @request = Grape::Request.new(env) + @params = @request.params + @headers = @request.headers extend helpers cookies.read(@request) diff --git a/lib/grape/http/request.rb b/lib/grape/http/request.rb index 0e07307913..10cfe415e4 100644 --- a/lib/grape/http/request.rb +++ b/lib/grape/http/request.rb @@ -2,7 +2,7 @@ module Grape class Request < Rack::Request def params - @env['grape.request.params'] = begin + @params ||= begin params = Hashie::Mash.new(super) if env['rack.routing_args'] args = env['rack.routing_args'].dup @@ -15,7 +15,7 @@ def params end def headers - @env['grape.request.headers'] ||= @env.dup.inject({}) do |h, (k, v)| + @headers ||= env.dup.inject({}) do |h, (k, v)| if k.to_s.start_with? 'HTTP_' k = k[5..-1].gsub('_', '-').downcase.gsub(/^.|[-_\s]./) { |x| x.upcase } h[k] = v diff --git a/lib/grape/middleware/auth/oauth2.rb b/lib/grape/middleware/auth/oauth2.rb index 4bc999d09e..6a056493f5 100644 --- a/lib/grape/middleware/auth/oauth2.rb +++ b/lib/grape/middleware/auth/oauth2.rb @@ -16,9 +16,17 @@ def before verify_token(token_parameter || token_header) end + def request + @request ||= Grape::Request.new(env) + end + + def params + @params ||= request.params + end + def token_parameter Array(options[:parameter]).each do |p| - return request[p] if request[p] + return params[p] if params[p] end nil end diff --git a/lib/grape/middleware/base.rb b/lib/grape/middleware/base.rb index 9dc051326f..26d129bac9 100644 --- a/lib/grape/middleware/base.rb +++ b/lib/grape/middleware/base.rb @@ -27,14 +27,13 @@ def call!(env) # @abstract # Called before the application is called in the middleware lifecycle. - def before; end + def before + end + # @abstract # Called after the application is called in the middleware lifecycle. # @return [Response, nil] a Rack SPEC response or nil to call the application afterwards. - def after; end - - def request - Grape::Request.new(env) + def after end def response diff --git a/lib/grape/middleware/formatter.rb b/lib/grape/middleware/formatter.rb index 6f47b1bb23..cc83a9c4ca 100644 --- a/lib/grape/middleware/formatter.rb +++ b/lib/grape/middleware/formatter.rb @@ -42,6 +42,10 @@ def after private + def request + @request ||= Rack::Request.new(env) + end + # store read input in env['api.request.input'] def read_body_input if (request.post? || request.put? || request.patch? || request.delete?) && diff --git a/lib/grape/middleware/versioner/param.rb b/lib/grape/middleware/versioner/param.rb index cd1deb8161..19e95615ab 100644 --- a/lib/grape/middleware/versioner/param.rb +++ b/lib/grape/middleware/versioner/param.rb @@ -27,14 +27,13 @@ def default_options def before paramkey = options[:parameter] - potential_version = request.params[paramkey] - + potential_version = Rack::Utils.parse_nested_query(env['QUERY_STRING'])[paramkey] unless potential_version.nil? if options[:versions] && !options[:versions].find { |v| v.to_s == potential_version } throw :error, status: 404, message: "404 API Version Not Found", headers: { 'X-Cascade' => 'pass' } end env['api.version'] = potential_version - env['rack.request.query_hash'].delete(paramkey) + env['rack.request.query_hash'].delete(paramkey) if env.key? 'rack.request.query_hash' end end diff --git a/spec/grape/middleware/base_spec.rb b/spec/grape/middleware/base_spec.rb index 346d09943e..4d5a116440 100644 --- a/spec/grape/middleware/base_spec.rb +++ b/spec/grape/middleware/base_spec.rb @@ -13,11 +13,6 @@ subject.app.should == blank_app end - it 'is able to access the request' do - subject.call({}) - subject.request.should be_kind_of(Rack::Request) - end - it 'calls through to the app' do subject.call({}).should == [200, {}, 'Hi there.'] end diff --git a/spec/grape/middleware/formatter_spec.rb b/spec/grape/middleware/formatter_spec.rb index 3eae471fbd..42e6d0c99d 100644 --- a/spec/grape/middleware/formatter_spec.rb +++ b/spec/grape/middleware/formatter_spec.rb @@ -72,9 +72,12 @@ def to_xml context 'detection' do - it 'uses the extension if one is provided' do + it 'uses the xml extension if one is provided' do subject.call('PATH_INFO' => '/info.xml') subject.env['api.format'].should == :xml + end + + it 'uses the json extension if one is provided' do subject.call('PATH_INFO' => '/info.json') subject.env['api.format'].should == :json end diff --git a/spec/grape/middleware/versioner/param_spec.rb b/spec/grape/middleware/versioner/param_spec.rb index f397af3027..5080a18238 100644 --- a/spec/grape/middleware/versioner/param_spec.rb +++ b/spec/grape/middleware/versioner/param_spec.rb @@ -12,6 +12,7 @@ it 'cuts (only) the version out of the params', focus: true do env = Rack::MockRequest.env_for("/awesome", { params: { "apiver" => "v1", "other_param" => "5" } }) + env['rack.request.query_hash'] = Rack::Utils.parse_nested_query(env['QUERY_STRING']) subject.call(env)[1]['rack.request.query_hash']["apiver"].should be_nil subject.call(env)[1]['rack.request.query_hash']["other_param"].should == "5" end