diff options
author | wycats <wycats@gmail.com> | 2010-04-03 20:23:23 -0700 |
---|---|---|
committer | wycats <wycats@gmail.com> | 2010-04-03 20:24:30 -0700 |
commit | ab8bf9e152ad75c8b358c85e4c95cfde578de127 (patch) | |
tree | 1e5887a4c0bdbf0750c359dc8d1c6d2d5585a5bf | |
parent | 512b4bccfbe222bd7f94adf6f9af07c2e856767d (diff) | |
download | rails-ab8bf9e152ad75c8b358c85e4c95cfde578de127.tar.gz rails-ab8bf9e152ad75c8b358c85e4c95cfde578de127.tar.bz2 rails-ab8bf9e152ad75c8b358c85e4c95cfde578de127.zip |
* 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
-rw-r--r-- | actionpack/CHANGELOG | 15 | ||||
-rw-r--r-- | actionpack/lib/action_controller/metal/responder.rb | 2 | ||||
-rw-r--r-- | actionpack/lib/action_controller/metal/verification.rb | 2 | ||||
-rwxr-xr-x | actionpack/lib/action_dispatch/http/request.rb | 60 | ||||
-rw-r--r-- | actionpack/lib/action_dispatch/routing/mapper.rb | 13 | ||||
-rw-r--r-- | actionpack/lib/action_dispatch/routing/route_set.rb | 10 | ||||
-rw-r--r-- | actionpack/lib/action_dispatch/testing/integration.rb | 3 | ||||
-rw-r--r-- | actionpack/lib/action_view/helpers/url_helper.rb | 7 | ||||
-rw-r--r-- | actionpack/test/controller/integration_test.rb | 2 | ||||
-rw-r--r-- | actionpack/test/controller/verification_test.rb | 2 | ||||
-rw-r--r-- | actionpack/test/dispatch/rack_test.rb | 2 | ||||
-rw-r--r-- | actionpack/test/dispatch/request_test.rb | 20 | ||||
-rw-r--r-- | actionpack/test/dispatch/routing_test.rb | 57 |
13 files changed, 152 insertions, 43 deletions
diff --git a/actionpack/CHANGELOG b/actionpack/CHANGELOG index 7c2cebcff1..c4ebedf1b0 100644 --- a/actionpack/CHANGELOG +++ b/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] diff --git a/actionpack/lib/action_controller/metal/responder.rb b/actionpack/lib/action_controller/metal/responder.rb index 0b2cee6868..d97c10a293 100644 --- a/actionpack/lib/action_controller/metal/responder.rb +++ b/actionpack/lib/action_controller/metal/responder.rb @@ -216,7 +216,7 @@ module ActionController #:nodoc: # the verb is POST. # def default_action - @action ||= ACTIONS_FOR_VERBS[request.method] + @action ||= ACTIONS_FOR_VERBS[request.method_symbol] end end end diff --git a/actionpack/lib/action_controller/metal/verification.rb b/actionpack/lib/action_controller/metal/verification.rb index bce942b588..b7fc2b7421 100644 --- a/actionpack/lib/action_controller/metal/verification.rb +++ b/actionpack/lib/action_controller/metal/verification.rb @@ -108,7 +108,7 @@ module ActionController #: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: diff --git a/actionpack/lib/action_dispatch/http/request.rb b/actionpack/lib/action_dispatch/http/request.rb index 8b8426b5aa..8560a6fc9c 100755 --- a/actionpack/lib/action_dispatch/http/request.rb +++ b/actionpack/lib/action_dispatch/http/request.rb @@ -45,47 +45,65 @@ module ActionDispatch 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 @@ module ActionDispatch 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 diff --git a/actionpack/lib/action_dispatch/routing/mapper.rb b/actionpack/lib/action_dispatch/routing/mapper.rb index 2c7ee7dad0..ffa178345b 100644 --- a/actionpack/lib/action_dispatch/routing/mapper.rb +++ b/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 @@ module ActionDispatch def app Constraints.new( to.respond_to?(:call) ? to : Routing::RouteSet::Dispatcher.new(:defaults => defaults), - blocks + blocks, + @set.request_class ) end diff --git a/actionpack/lib/action_dispatch/routing/route_set.rb b/actionpack/lib/action_dispatch/routing/route_set.rb index c8e4371bb7..f1965f38b9 100644 --- a/actionpack/lib/action_dispatch/routing/route_set.rb +++ b/actionpack/lib/action_dispatch/routing/route_set.rb @@ -187,18 +187,19 @@ module ActionDispatch 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 @@ module ActionDispatch @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) diff --git a/actionpack/lib/action_dispatch/testing/integration.rb b/actionpack/lib/action_dispatch/testing/integration.rb index 031fa1dfb4..8d107d9aa5 100644 --- a/actionpack/lib/action_dispatch/testing/integration.rb +++ b/actionpack/lib/action_dispatch/testing/integration.rb @@ -266,11 +266,12 @@ module ActionDispatch "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 diff --git a/actionpack/lib/action_view/helpers/url_helper.rb b/actionpack/lib/action_view/helpers/url_helper.rb index 1415966869..5925faf810 100644 --- a/actionpack/lib/action_view/helpers/url_helper.rb +++ b/actionpack/lib/action_view/helpers/url_helper.rb @@ -10,6 +10,12 @@ module ActionView # 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 @@ module ActionView # # </div> # # </form>" # # - def button_to(name, options = {}, html_options = {}) html_options = html_options.stringify_keys convert_boolean_attributes!(html_options, %w( disabled )) diff --git a/actionpack/test/controller/integration_test.rb b/actionpack/test/controller/integration_test.rb index 1e2ee06adc..14c0c3708b 100644 --- a/actionpack/test/controller/integration_test.rb +++ b/actionpack/test/controller/integration_test.rb @@ -236,7 +236,7 @@ class IntegrationProcessTest < ActionController::IntegrationTest end def method - render :text => "method: #{request.method}" + render :text => "method: #{request.method.downcase}" end def cookie_monster diff --git a/actionpack/test/controller/verification_test.rb b/actionpack/test/controller/verification_test.rb index 11d0d10897..0600ec2ec1 100644 --- a/actionpack/test/controller/verification_test.rb +++ b/actionpack/test/controller/verification_test.rb @@ -71,7 +71,7 @@ class VerificationTest < ActionController::TestCase end def guarded_by_method - render :text => "#{request.method}" + render :text => "#{request.method.downcase}" end def guarded_by_xhr diff --git a/actionpack/test/dispatch/rack_test.rb b/actionpack/test/dispatch/rack_test.rb index 504bebbb86..698f980296 100644 --- a/actionpack/test/dispatch/rack_test.rb +++ b/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 diff --git a/actionpack/test/dispatch/request_test.rb b/actionpack/test/dispatch/request_test.rb index 9093e1ed65..e5ee412021 100644 --- a/actionpack/test/dispatch/request_test.rb +++ b/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 diff --git a/actionpack/test/dispatch/routing_test.rb b/actionpack/test/dispatch/routing_test.rb index 19538cb88b..bb7c322790 100644 --- a/actionpack/test/dispatch/routing_test.rb +++ b/actionpack/test/dispatch/routing_test.rb @@ -187,6 +187,63 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest 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 |