Skip to content
Browse files

* Change the object used in routing constraints to be an instance of

  ActionDispatch::Request rather than Rack::Request.

* Changed ActionDispatch::Request#method to return a String, to be
  compatible with the Rack::Request superclass.

* Changed ActionDispatch::Request#method to return the original
  method in the case of methodoverride and #request_method not to,
  to be compatible with Rack::Request
  • Loading branch information...
1 parent 512b4bc commit ab8bf9e152ad75c8b358c85e4c95cfde578de127 @wycats wycats committed Apr 3, 2010
View
15 actionpack/CHANGELOG
@@ -1,3 +1,18 @@
+*Rails 3.0.0 [Edge] (pending)*
+
+* Changed the object used in routing constraints to be an instance of
+ ActionDispatch::Request rather than Rack::Request
+
+* Changed ActionDispatch::Request#method to return a String, to be compatible
+ with Rack::Request. Added ActionDispatch::Request#method_symbol to
+ return a symbol form of the request method.
+
+* Changed ActionDispatch::Request#method to return the original
+ method and #request_method to return the overridden method in the
+ case of methodoverride being used (this means that #method returns
+ "HEAD" and #request_method returns "GET" in HEAD requests). This
+ is for compatibility with Rack::Request
+
*Rails 3.0.0 [beta 2] (April 1st, 2010)*
* #concat is now deprecated in favor of using <%= %> helpers [YK]
View
2 actionpack/lib/action_controller/metal/responder.rb
@@ -216,7 +216,7 @@ def has_errors?
# the verb is POST.
#
def default_action
- @action ||= ACTIONS_FOR_VERBS[request.method]
+ @action ||= ACTIONS_FOR_VERBS[request.method_symbol]
@Sutto
Sutto added a note May 1, 2010

Wouldn't this make more sense as request.request_method_symbol?

Seeing as method_symbol will typically return GET or POST, not the PUT or POST from method override.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
end
end
end
View
2 actionpack/lib/action_controller/metal/verification.rb
@@ -108,7 +108,7 @@ def verify_presence_of_keys_in_hash_flash_or_params(options) # :nodoc:
end
def verify_method(options) # :nodoc:
- [*options[:method]].all? { |v| request.method != v.to_sym } if options[:method]
+ [*options[:method]].all? { |v| request.method_symbol != v.to_sym } if options[:method]
end
def verify_request_xhr_status(options) # :nodoc:
View
60 actionpack/lib/action_dispatch/http/request.rb
@@ -45,47 +45,65 @@ def key?(key)
HTTP_METHODS = %w(get head put post delete options)
HTTP_METHOD_LOOKUP = HTTP_METHODS.inject({}) { |h, m| h[m] = h[m.upcase] = m.to_sym; h }
- # Returns the true HTTP request \method as a lowercase symbol, such as
- # <tt>:get</tt>. If the request \method is not listed in the HTTP_METHODS
- # constant above, an UnknownHttpMethod exception is raised.
+ # Returns the HTTP \method that the application should see.
+ # In the case where the \method was overridden by a middleware
+ # (for instance, if a HEAD request was converted to a GET,
+ # or if a _method parameter was used to determine the \method
+ # the application should use), this \method returns the overridden
+ # value, not the original.
def request_method
- method = env["rack.methodoverride.original_method"] || env["REQUEST_METHOD"]
+ method = env["REQUEST_METHOD"]
HTTP_METHOD_LOOKUP[method] || raise(ActionController::UnknownHttpMethod, "#{method}, accepted HTTP methods are #{HTTP_METHODS.to_sentence(:locale => :en)}")
+ method
+ end
+
+ # Returns a symbol form of the #request_method
+ def request_method_symbol
+ HTTP_METHOD_LOOKUP[request_method]
end
- # Returns the HTTP request \method used for action processing as a
- # lowercase symbol, such as <tt>:post</tt>. (Unlike #request_method, this
- # method returns <tt>:get</tt> for a HEAD request because the two are
- # functionally equivalent from the application's perspective.)
+ # Returns the original value of the environment's REQUEST_METHOD,
+ # even if it was overridden by middleware. See #request_method for
+ # more information.
def method
- method = env["REQUEST_METHOD"]
+ method = env["rack.methodoverride.original_method"] || env['REQUEST_METHOD']
HTTP_METHOD_LOOKUP[method] || raise(ActionController::UnknownHttpMethod, "#{method}, accepted HTTP methods are #{HTTP_METHODS.to_sentence(:locale => :en)}")
+ method
+ end
+
+ # Returns a symbol form of the #method
+ def method_symbol
+ HTTP_METHOD_LOOKUP[method]
end
- # Is this a GET (or HEAD) request? Equivalent to <tt>request.method == :get</tt>.
+ # Is this a GET (or HEAD) request?
+ # Equivalent to <tt>request.request_method == :get</tt>.
def get?
- method == :get
+ HTTP_METHOD_LOOKUP[request_method] == :get
end
- # Is this a POST request? Equivalent to <tt>request.method == :post</tt>.
+ # Is this a POST request?
+ # Equivalent to <tt>request.request_method == :post</tt>.
def post?
- method == :post
+ HTTP_METHOD_LOOKUP[request_method] == :post
end
- # Is this a PUT request? Equivalent to <tt>request.method == :put</tt>.
+ # Is this a PUT request?
+ # Equivalent to <tt>request.request_method == :put</tt>.
def put?
- method == :put
+ HTTP_METHOD_LOOKUP[request_method] == :put
end
- # Is this a DELETE request? Equivalent to <tt>request.method == :delete</tt>.
+ # Is this a DELETE request?
+ # Equivalent to <tt>request.request_method == :delete</tt>.
def delete?
- method == :delete
+ HTTP_METHOD_LOOKUP[request_method] == :delete
end
- # Is this a HEAD request? Since <tt>request.method</tt> sees HEAD as <tt>:get</tt>,
- # this \method checks the actual HTTP \method directly.
+ # Is this a HEAD request?
+ # Equivalent to <tt>request.method == :head</tt>.
def head?
- request_method == :head
+ HTTP_METHOD_LOOKUP[method] == :head
end
# Provides access to the request's HTTP headers, for example:
@@ -96,7 +114,7 @@ def headers
end
def forgery_whitelisted?
- method == :get || xhr? || content_mime_type.nil? || !content_mime_type.verify_request?
+ get? || xhr? || content_mime_type.nil? || !content_mime_type.verify_request?
end
def media_type
View
13 actionpack/lib/action_dispatch/routing/mapper.rb
@@ -5,20 +5,20 @@ module ActionDispatch
module Routing
class Mapper
class Constraints #:nodoc:
- def self.new(app, constraints = [])
+ def self.new(app, constraints, request = Rack::Request)
if constraints.any?
- super(app, constraints)
+ super(app, constraints, request)
else
app
end
end
- def initialize(app, constraints = [])
- @app, @constraints = app, constraints
+ def initialize(app, constraints, request)
+ @app, @constraints, @request = app, constraints, request
end
def call(env)
- req = Rack::Request.new(env)
+ req = @request.new(env)
@constraints.each { |constraint|
if constraint.respond_to?(:matches?) && !constraint.matches?(req)
@@ -83,7 +83,8 @@ def normalize_path(path)
def app
Constraints.new(
to.respond_to?(:call) ? to : Routing::RouteSet::Dispatcher.new(:defaults => defaults),
- blocks
+ blocks,
+ @set.request_class
)
end
View
10 actionpack/lib/action_dispatch/routing/route_set.rb
@@ -187,18 +187,19 @@ def #{selector}(*args)
attr_accessor :routes, :named_routes
attr_accessor :disable_clear_and_finalize, :resources_path_names
- attr_accessor :default_url_options
+ attr_accessor :default_url_options, :request_class
def self.default_resources_path_names
{ :new => 'new', :edit => 'edit' }
end
- def initialize
+ def initialize(request_class = ActionDispatch::Request)
self.routes = []
self.named_routes = NamedRouteCollection.new
self.resources_path_names = self.class.default_resources_path_names.dup
self.controller_namespaces = Set.new
self.default_url_options = {}
+ self.request_class = request_class
@disable_clear_and_finalize = false
clear!
@@ -232,7 +233,10 @@ def clear!
@finalized = false
routes.clear
named_routes.clear
- @set = ::Rack::Mount::RouteSet.new(:parameters_key => PARAMETERS_KEY)
+ @set = ::Rack::Mount::RouteSet.new(
+ :parameters_key => PARAMETERS_KEY,
+ :request_class => request_class
+ )
end
def install_helpers(destinations = [ActionController::Base, ActionView::Base], regenerate_code = false)
View
3 actionpack/lib/action_dispatch/testing/integration.rb
@@ -266,11 +266,12 @@ def process(method, path, parameters = nil, rack_environment = nil)
"HTTP_ACCEPT" => accept
}
+ session = Rack::Test::Session.new(@mock_session)
+
(rack_environment || {}).each do |key, value|
env[key] = value
end
- session = Rack::Test::Session.new(@mock_session)
session.request(path, env)
@request_count += 1
View
7 actionpack/lib/action_view/helpers/url_helper.rb
@@ -10,6 +10,12 @@ module Helpers #:nodoc:
# This allows you to use the same format for links in views
# and controllers.
module UrlHelper
+ # This helper may be included in any class that includes the
+ # URL helpers of a router (router.url_helpers). Some methods
+ # provided here will only work in the context of a request
+ # (link_to_unless_current, for instance), which must be provided
+ # as a method called #request on the context.
+
extend ActiveSupport::Concern
include ActionDispatch::Routing::UrlFor
@@ -307,7 +313,6 @@ def link_to(*args, &block)
# # </div>
# # </form>"
# #
-
def button_to(name, options = {}, html_options = {})
html_options = html_options.stringify_keys
convert_boolean_attributes!(html_options, %w( disabled ))
View
2 actionpack/test/controller/integration_test.rb
@@ -236,7 +236,7 @@ def post
end
def method
- render :text => "method: #{request.method}"
+ render :text => "method: #{request.method.downcase}"
end
def cookie_monster
View
2 actionpack/test/controller/verification_test.rb
@@ -71,7 +71,7 @@ def multi_two
end
def guarded_by_method
- render :text => "#{request.method}"
+ render :text => "#{request.method.downcase}"
end
def guarded_by_xhr
View
2 actionpack/test/dispatch/rack_test.rb
@@ -142,7 +142,7 @@ class RackRequestTest < BaseRackTest
assert_equal "google.com", @request.remote_host
assert_equal "kevin", @request.remote_ident
assert_equal "kevin", @request.remote_user
- assert_equal :get, @request.request_method
+ assert_equal "GET", @request.request_method
assert_equal "/dispatch.fcgi", @request.script_name
assert_equal "glu.ttono.us", @request.server_name
assert_equal 8007, @request.server_port
View
20 actionpack/test/dispatch/request_test.rb
@@ -223,10 +223,17 @@ class RequestTest < ActiveSupport::TestCase
assert request.ssl?
end
- test "symbolized request methods" do
+ test "String request methods" do
[:get, :post, :put, :delete].each do |method|
request = stub_request 'REQUEST_METHOD' => method.to_s.upcase
- assert_equal method, request.method
+ assert_equal method.to_s.upcase, request.method
+ end
+ end
+
+ test "Symbol forms of request methods via method_symbol" do
+ [:get, :post, :put, :delete].each do |method|
+ request = stub_request 'REQUEST_METHOD' => method.to_s.upcase
+ assert_equal method, request.method_symbol
end
end
@@ -238,9 +245,9 @@ class RequestTest < ActiveSupport::TestCase
end
test "allow method hacking on post" do
- [:get, :options, :put, :post, :delete].each do |method|
+ %w(GET OPTIONS PUT POST DELETE).each do |method|
request = stub_request "REQUEST_METHOD" => method.to_s.upcase
- assert_equal(method == :head ? :get : method, request.method)
+ assert_equal(method == "HEAD" ? "GET" : method, request.method)
end
end
@@ -255,13 +262,14 @@ class RequestTest < ActiveSupport::TestCase
[:get, :put, :delete].each do |method|
request = stub_request 'REQUEST_METHOD' => method.to_s.upcase,
'action_dispatch.request.request_parameters' => { :_method => 'put' }
- assert_equal method, request.method
+ assert_equal method.to_s.upcase, request.method
end
end
test "head masquerading as get" do
request = stub_request 'REQUEST_METHOD' => 'GET', "rack.methodoverride.original_method" => "HEAD"
- assert_equal :get, request.method
+ assert_equal "HEAD", request.method
+ assert_equal "GET", request.request_method
assert request.get?
assert request.head?
end
View
57 actionpack/test/dispatch/routing_test.rb
@@ -187,6 +187,63 @@ def self.matches?(request)
end
end
+ class TestAltApp < ActionController::IntegrationTest
+ class AltRequest
+ def initialize(env)
+ @env = env
+ end
+
+ def path_info
+ "/"
+ end
+
+ def request_method
+ "GET"
+ end
+
+ def x_header
+ @env["HTTP_X_HEADER"] || ""
+ end
+ end
+
+ class XHeader
+ def call(env)
+ [200, {"Content-Type" => "text/html"}, ["XHeader"]]
+ end
+ end
+
+ class AltApp
+ def call(env)
+ [200, {"Content-Type" => "text/html"}, ["Alternative App"]]
+ end
+ end
+
+ AltRoutes = ActionDispatch::Routing::RouteSet.new(AltRequest)
+ AltRoutes.draw do
+ get "/" => XHeader.new, :constraints => {:x_header => /HEADER/}
+ get "/" => AltApp.new
+ end
+
+ def app
+ AltRoutes
+ end
+
+ def test_alt_request_without_header
+ get "/"
+ assert_equal "Alternative App", @response.body
+ end
+
+ def test_alt_request_with_matched_header
+ get "/", {}, "HTTP_X_HEADER" => "HEADER"
+ assert_equal "XHeader", @response.body
+ end
+
+ def test_alt_request_with_unmatched_header
+ get "/", {}, "HTTP_X_HEADER" => "NON_MATCH"
+ assert_equal "Alternative App", @response.body
+ end
+ end
+
def app
Routes
end

0 comments on commit ab8bf9e

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