Skip to content
Browse files

Dont use Memoizable for ActionController::Request

  • Loading branch information...
1 parent 17db28f commit 57b156b33874431fedd1dacc952a4bc781628a07 @lifo lifo committed Jan 27, 2009
View
42 actionpack/lib/action_controller/request.rb
@@ -7,7 +7,6 @@
module ActionController
class Request < Rack::Request
- extend ActiveSupport::Memoizable
%w[ AUTH_TYPE GATEWAY_INTERFACE
PATH_TRANSLATED REMOTE_HOST
@@ -33,9 +32,8 @@ def key?(key)
# <tt>:get</tt>. If the request \method is not listed in the HTTP_METHODS
# constant above, an UnknownHttpMethod exception is raised.
def request_method
- HTTP_METHOD_LOOKUP[super] || raise(UnknownHttpMethod, "#{super}, accepted HTTP methods are #{HTTP_METHODS.to_sentence}")
+ @request_method ||= HTTP_METHOD_LOOKUP[super] || raise(UnknownHttpMethod, "#{super}, accepted HTTP methods are #{HTTP_METHODS.to_sentence}")
end
- memoize :request_method
# Returns the HTTP request \method used for action processing as a
# lowercase symbol, such as <tt>:post</tt>. (Unlike #request_method, this
@@ -75,9 +73,8 @@ def head?
#
# request.headers["Content-Type"] # => "text/plain"
def headers
- ActionController::Http::Headers.new(@env)
+ @headers ||= ActionController::Http::Headers.new(@env)
end
- memoize :headers
# Returns the content length of the request as an integer.
def content_length
@@ -89,32 +86,33 @@ def content_length
# For backward compatibility, the post \format is extracted from the
# X-Post-Data-Format HTTP header if present.
def content_type
- if @env['CONTENT_TYPE'] =~ /^([^,\;]*)/
- Mime::Type.lookup($1.strip.downcase)
- else
- nil
+ @content_type ||= begin
+ if @env['CONTENT_TYPE'] =~ /^([^,\;]*)/
+ Mime::Type.lookup($1.strip.downcase)
+ else
+ nil
+ end
end
end
- memoize :content_type
# Returns the accepted MIME type for the request.
def accepts
- header = @env['HTTP_ACCEPT'].to_s.strip
+ @accepts ||= begin
+ header = @env['HTTP_ACCEPT'].to_s.strip
- if header.empty?
- [content_type, Mime::ALL].compact
- else
- Mime::Type.parse(header)
+ if header.empty?
+ [content_type, Mime::ALL].compact
+ else
+ Mime::Type.parse(header)
+ end
end
end
- memoize :accepts
def if_modified_since
if since = env['HTTP_IF_MODIFIED_SINCE']
Time.rfc2822(since) rescue nil
end
end
- memoize :if_modified_since
def if_none_match
env['HTTP_IF_NONE_MATCH']
@@ -248,25 +246,21 @@ def remote_ip
@env['REMOTE_ADDR']
end
- memoize :remote_ip
# Returns the lowercase name of the HTTP server software.
def server_software
(@env['SERVER_SOFTWARE'] && /^([a-zA-Z]+)/ =~ @env['SERVER_SOFTWARE']) ? $1.downcase : nil
end
- memoize :server_software
# Returns the complete URL used for this request.
def url
protocol + host_with_port + request_uri
end
- memoize :url
# Returns 'https://' if this is an SSL request and 'http://' otherwise.
def protocol
ssl? ? 'https://' : 'http://'
end
- memoize :protocol
# Is this an SSL request?
def ssl?
@@ -286,14 +280,12 @@ def raw_host_with_port
def host
raw_host_with_port.sub(/:\d+$/, '')
end
- memoize :host
# Returns a \host:\port string for this request, such as "example.com" or
# "example.com:8080".
def host_with_port
"#{host}#{port_string}"
end
- memoize :host_with_port
# Returns the port number of this request as an integer.
def port
@@ -303,7 +295,6 @@ def port
standard_port
end
end
- memoize :port
# Returns the standard \port number for this request's protocol.
def standard_port
@@ -341,7 +332,6 @@ def subdomains(tld_length = 1)
def query_string
@env['QUERY_STRING'].present? ? @env['QUERY_STRING'] : (@env['REQUEST_URI'].split('?', 2)[1] || '')
end
- memoize :query_string
# Returns the request URI, accounting for server idiosyncrasies.
# WEBrick includes the full URL. IIS leaves REQUEST_URI blank.
@@ -367,7 +357,6 @@ def request_uri
end
end
end
- memoize :request_uri
# Returns the interpreted \path to requested resource after all the installation
# directory of this application was taken into account.
@@ -376,7 +365,6 @@ def path
path.sub!(/\A#{ActionController::Base.relative_url_root}/, '')
path
end
- memoize :path
# Read the request \body. This is useful for web services that need to
# work with raw requests directly.
View
15 actionpack/lib/action_controller/test_process.rb
@@ -35,7 +35,6 @@ def raw_post
def port=(number)
@env["SERVER_PORT"] = number.to_i
- port(true)
end
def action=(action_name)
@@ -49,18 +48,20 @@ def set_REQUEST_URI(value)
@env["REQUEST_URI"] = value
@request_uri = nil
@path = nil
- request_uri(true)
- path(true)
end
def request_uri=(uri)
@request_uri = uri
@path = uri.split("?").first
end
+ def request_method=(method)
+ @request_method = method
+ end
+
def accept=(mime_types)
@env["HTTP_ACCEPT"] = Array(mime_types).collect { |mime_types| mime_types.to_s }.join(",")
- accepts(true)
+ @accepts = nil
end
def if_modified_since=(last_modified)
@@ -76,11 +77,11 @@ def remote_addr=(addr)
end
def request_uri(*args)
- @request_uri || super
+ @request_uri || super()
end
def path(*args)
- @path || super
+ @path || super()
end
def assign_parameters(controller_path, action, parameters)
@@ -107,7 +108,7 @@ def assign_parameters(controller_path, action, parameters)
def recycle!
self.query_parameters = {}
self.path_parameters = {}
- unmemoize_all
+ @headers, @request_method, @accepts, @content_type = nil, nil, nil, nil
end
def user_agent=(user_agent)
View
26 actionpack/test/controller/rack_test.rb
@@ -63,61 +63,61 @@ def set_content_data(data)
class RackRequestTest < BaseRackTest
def test_proxy_request
- assert_equal 'glu.ttono.us', @request.host_with_port(true)
+ assert_equal 'glu.ttono.us', @request.host_with_port
end
def test_http_host
@env.delete "HTTP_X_FORWARDED_HOST"
@env['HTTP_HOST'] = "rubyonrails.org:8080"
- assert_equal "rubyonrails.org", @request.host(true)
- assert_equal "rubyonrails.org:8080", @request.host_with_port(true)
+ assert_equal "rubyonrails.org", @request.host
+ assert_equal "rubyonrails.org:8080", @request.host_with_port
@env['HTTP_X_FORWARDED_HOST'] = "www.firsthost.org, www.secondhost.org"
- assert_equal "www.secondhost.org", @request.host(true)
+ assert_equal "www.secondhost.org", @request.host
end
def test_http_host_with_default_port_overrides_server_port
@env.delete "HTTP_X_FORWARDED_HOST"
@env['HTTP_HOST'] = "rubyonrails.org"
- assert_equal "rubyonrails.org", @request.host_with_port(true)
+ assert_equal "rubyonrails.org", @request.host_with_port
end
def test_host_with_port_defaults_to_server_name_if_no_host_headers
@env.delete "HTTP_X_FORWARDED_HOST"
@env.delete "HTTP_HOST"
- assert_equal "glu.ttono.us:8007", @request.host_with_port(true)
+ assert_equal "glu.ttono.us:8007", @request.host_with_port
end
def test_host_with_port_falls_back_to_server_addr_if_necessary
@env.delete "HTTP_X_FORWARDED_HOST"
@env.delete "HTTP_HOST"
@env.delete "SERVER_NAME"
- assert_equal "207.7.108.53", @request.host(true)
- assert_equal 8007, @request.port(true)
- assert_equal "207.7.108.53:8007", @request.host_with_port(true)
+ assert_equal "207.7.108.53", @request.host
+ assert_equal 8007, @request.port
+ assert_equal "207.7.108.53:8007", @request.host_with_port
end
def test_host_with_port_if_http_standard_port_is_specified
@env['HTTP_X_FORWARDED_HOST'] = "glu.ttono.us:80"
- assert_equal "glu.ttono.us", @request.host_with_port(true)
+ assert_equal "glu.ttono.us", @request.host_with_port
end
def test_host_with_port_if_https_standard_port_is_specified
@env['HTTP_X_FORWARDED_PROTO'] = "https"
@env['HTTP_X_FORWARDED_HOST'] = "glu.ttono.us:443"
- assert_equal "glu.ttono.us", @request.host_with_port(true)
+ assert_equal "glu.ttono.us", @request.host_with_port
end
def test_host_if_ipv6_reference
@env.delete "HTTP_X_FORWARDED_HOST"
@env['HTTP_HOST'] = "[2001:1234:5678:9abc:def0::dead:beef]"
- assert_equal "[2001:1234:5678:9abc:def0::dead:beef]", @request.host(true)
+ assert_equal "[2001:1234:5678:9abc:def0::dead:beef]", @request.host
end
def test_host_if_ipv6_reference_with_port
@env.delete "HTTP_X_FORWARDED_HOST"
@env['HTTP_HOST'] = "[2001:1234:5678:9abc:def0::dead:beef]:8008"
- assert_equal "[2001:1234:5678:9abc:def0::dead:beef]", @request.host(true)
+ assert_equal "[2001:1234:5678:9abc:def0::dead:beef]", @request.host
end
def test_cgi_environment_variables
View
58 actionpack/test/controller/request_test.rb
@@ -14,53 +14,53 @@ def test_remote_ip
assert_equal '0.0.0.0', @request.remote_ip
@request.remote_addr = '1.2.3.4'
- assert_equal '1.2.3.4', @request.remote_ip(true)
+ assert_equal '1.2.3.4', @request.remote_ip
@request.remote_addr = '1.2.3.4,3.4.5.6'
- assert_equal '1.2.3.4', @request.remote_ip(true)
+ assert_equal '1.2.3.4', @request.remote_ip
@request.env['HTTP_CLIENT_IP'] = '2.3.4.5'
- assert_equal '1.2.3.4', @request.remote_ip(true)
+ assert_equal '1.2.3.4', @request.remote_ip
@request.remote_addr = '192.168.0.1'
- assert_equal '2.3.4.5', @request.remote_ip(true)
+ assert_equal '2.3.4.5', @request.remote_ip
@request.env.delete 'HTTP_CLIENT_IP'
@request.remote_addr = '1.2.3.4'
@request.env['HTTP_X_FORWARDED_FOR'] = '3.4.5.6'
- assert_equal '1.2.3.4', @request.remote_ip(true)
+ assert_equal '1.2.3.4', @request.remote_ip
@request.remote_addr = '127.0.0.1'
@request.env['HTTP_X_FORWARDED_FOR'] = '3.4.5.6'
- assert_equal '3.4.5.6', @request.remote_ip(true)
+ assert_equal '3.4.5.6', @request.remote_ip
@request.env['HTTP_X_FORWARDED_FOR'] = 'unknown,3.4.5.6'
- assert_equal '3.4.5.6', @request.remote_ip(true)
+ assert_equal '3.4.5.6', @request.remote_ip
@request.env['HTTP_X_FORWARDED_FOR'] = '172.16.0.1,3.4.5.6'
- assert_equal '3.4.5.6', @request.remote_ip(true)
+ assert_equal '3.4.5.6', @request.remote_ip
@request.env['HTTP_X_FORWARDED_FOR'] = '192.168.0.1,3.4.5.6'
- assert_equal '3.4.5.6', @request.remote_ip(true)
+ assert_equal '3.4.5.6', @request.remote_ip
@request.env['HTTP_X_FORWARDED_FOR'] = '10.0.0.1,3.4.5.6'
- assert_equal '3.4.5.6', @request.remote_ip(true)
+ assert_equal '3.4.5.6', @request.remote_ip
@request.env['HTTP_X_FORWARDED_FOR'] = '10.0.0.1, 10.0.0.1, 3.4.5.6'
- assert_equal '3.4.5.6', @request.remote_ip(true)
+ assert_equal '3.4.5.6', @request.remote_ip
@request.env['HTTP_X_FORWARDED_FOR'] = '127.0.0.1,3.4.5.6'
- assert_equal '3.4.5.6', @request.remote_ip(true)
+ assert_equal '3.4.5.6', @request.remote_ip
@request.env['HTTP_X_FORWARDED_FOR'] = 'unknown,192.168.0.1'
- assert_equal 'unknown', @request.remote_ip(true)
+ assert_equal 'unknown', @request.remote_ip
@request.env['HTTP_X_FORWARDED_FOR'] = '9.9.9.9, 3.4.5.6, 10.0.0.1, 172.31.4.4'
- assert_equal '3.4.5.6', @request.remote_ip(true)
+ assert_equal '3.4.5.6', @request.remote_ip
@request.env['HTTP_CLIENT_IP'] = '8.8.8.8'
e = assert_raises(ActionController::ActionControllerError) {
- @request.remote_ip(true)
+ @request.remote_ip
}
assert_match /IP spoofing attack/, e.message
assert_match /HTTP_X_FORWARDED_FOR="9.9.9.9, 3.4.5.6, 10.0.0.1, 172.31.4.4"/, e.message
@@ -72,11 +72,11 @@ def test_remote_ip
# leap of faith to assume that their proxies are ever going to set the
# HTTP_CLIENT_IP/HTTP_X_FORWARDED_FOR headers properly.
ActionController::Base.ip_spoofing_check = false
- assert_equal('8.8.8.8', @request.remote_ip(true))
+ assert_equal('8.8.8.8', @request.remote_ip)
ActionController::Base.ip_spoofing_check = true
@request.env['HTTP_X_FORWARDED_FOR'] = '8.8.8.8, 9.9.9.9'
- assert_equal '8.8.8.8', @request.remote_ip(true)
+ assert_equal '8.8.8.8', @request.remote_ip
@request.env.delete 'HTTP_CLIENT_IP'
@request.env.delete 'HTTP_X_FORWARDED_FOR'
@@ -189,8 +189,8 @@ def test_request_uri
@request.env['PATH_INFO'] = "/path/of/some/uri?mapped=1"
@request.env['SCRIPT_NAME'] = "/path/dispatch.rb"
@request.set_REQUEST_URI nil
- assert_equal "/path/of/some/uri?mapped=1", @request.request_uri(true)
- assert_equal "/of/some/uri", @request.path(true)
+ assert_equal "/path/of/some/uri?mapped=1", @request.request_uri
+ assert_equal "/of/some/uri", @request.path
ActionController::Base.relative_url_root = nil
@request.env['PATH_INFO'] = "/path/of/some/uri"
@@ -225,21 +225,21 @@ def test_request_uri
@request.set_REQUEST_URI '/hieraki/dispatch.cgi'
ActionController::Base.relative_url_root = '/hieraki'
- assert_equal "/dispatch.cgi", @request.path(true)
+ assert_equal "/dispatch.cgi", @request.path
ActionController::Base.relative_url_root = nil
@request.set_REQUEST_URI '/hieraki/dispatch.cgi'
ActionController::Base.relative_url_root = '/foo'
- assert_equal "/hieraki/dispatch.cgi", @request.path(true)
+ assert_equal "/hieraki/dispatch.cgi", @request.path
ActionController::Base.relative_url_root = nil
# This test ensures that Rails uses REQUEST_URI over PATH_INFO
ActionController::Base.relative_url_root = nil
@request.env['REQUEST_URI'] = "/some/path"
@request.env['PATH_INFO'] = "/another/path"
@request.env['SCRIPT_NAME'] = "/dispatch.cgi"
- assert_equal "/some/path", @request.request_uri(true)
- assert_equal "/some/path", @request.path(true)
+ assert_equal "/some/path", @request.request_uri
+ assert_equal "/some/path", @request.path
end
def test_host_with_default_port
@@ -255,13 +255,13 @@ def test_host_with_non_default_port
end
def test_server_software
- assert_equal nil, @request.server_software(true)
+ assert_equal nil, @request.server_software
@request.env['SERVER_SOFTWARE'] = 'Apache3.422'
- assert_equal 'apache', @request.server_software(true)
+ assert_equal 'apache', @request.server_software
@request.env['SERVER_SOFTWARE'] = 'lighttpd(1.1.4)'
- assert_equal 'lighttpd', @request.server_software(true)
+ assert_equal 'lighttpd', @request.server_software
end
def test_xml_http_request
@@ -299,21 +299,21 @@ def test_symbolized_request_methods
def test_invalid_http_method_raises_exception
assert_raises(ActionController::UnknownHttpMethod) do
self.request_method = :random_method
+ @request.request_method
end
end
def test_allow_method_hacking_on_post
[:get, :head, :options, :put, :post, :delete].each do |method|
self.request_method = method
- @request.request_method(true)
assert_equal(method == :head ? :get : method, @request.method)
end
end
def test_invalid_method_hacking_on_post_raises_exception
assert_raises(ActionController::UnknownHttpMethod) do
self.request_method = :_random_method
- @request.request_method(true)
+ @request.request_method
end
end
@@ -402,6 +402,6 @@ def test_parameters
protected
def request_method=(method)
@request.env['REQUEST_METHOD'] = method.to_s.upcase
- @request.request_method(true)
+ @request.request_method = nil # Reset the ivar cache
end
end

3 comments on commit 57b156b

@lmarlow

What is the reason for this change?

@lifo
Ruby on Rails member
lifo commented on 57b156b Jan 28, 2009

Because memoized overhead was more than than the method itself.

@lmarlow

Thanks

Please sign in to comment.
Something went wrong with that request. Please try again.