Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Don't create Grape::Request multiple times. #512

Merged
merged 1 commit into from
Nov 24, 2013
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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.


Expand Down
5 changes: 2 additions & 3 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
18 changes: 5 additions & 13 deletions lib/grape/endpoint.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down
4 changes: 2 additions & 2 deletions lib/grape/http/request.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down
10 changes: 9 additions & 1 deletion lib/grape/middleware/auth/oauth2.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
9 changes: 4 additions & 5 deletions lib/grape/middleware/base.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 4 additions & 0 deletions lib/grape/middleware/formatter.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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?) &&
Expand Down
5 changes: 2 additions & 3 deletions lib/grape/middleware/versioner/param.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
5 changes: 0 additions & 5 deletions spec/grape/middleware/base_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
5 changes: 4 additions & 1 deletion spec/grape/middleware/formatter_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions spec/grape/middleware/versioner/param_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down