Merge pull request #512 from dblock/request-globals

Don't create Grape::Request multiple times.

Author: dblock

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.
 
 

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

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)

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

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

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

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?) &&

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
 

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

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

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