From 5d89605c11cc54acadfdd76ccd226d38989ec600 Mon Sep 17 00:00:00 2001 From: Joshua Peek Date: Sun, 28 Dec 2008 15:31:03 -0600 Subject: Make router and controller classes better rack citizens --- actionpack/lib/action_controller/base.rb | 7 ++ actionpack/lib/action_controller/dispatcher.rb | 11 +- actionpack/lib/action_controller/request.rb | 4 +- actionpack/lib/action_controller/rescue.rb | 4 +- .../lib/action_controller/routing/route_set.rb | 6 + actionpack/test/controller/dispatcher_test.rb | 4 +- actionpack/test/controller/rescue_test.rb | 6 +- actionpack/test/controller/routing_test.rb | 133 ++++++++++----------- 8 files changed, 88 insertions(+), 87 deletions(-) diff --git a/actionpack/lib/action_controller/base.rb b/actionpack/lib/action_controller/base.rb index 5b83494eb4..da3d1f46ee 100644 --- a/actionpack/lib/action_controller/base.rb +++ b/actionpack/lib/action_controller/base.rb @@ -382,6 +382,13 @@ module ActionController #:nodoc: attr_accessor :action_name class << self + def call(env) + # HACK: For global rescue to have access to the original request and response + request = env["actioncontroller.rescue.request"] ||= Request.new(env) + response = env["actioncontroller.rescue.response"] ||= Response.new + process(request, response) + end + # Factory for the standard create, process loop where the controller is discarded after processing. def process(request, response) #:nodoc: new.process(request, response) diff --git a/actionpack/lib/action_controller/dispatcher.rb b/actionpack/lib/action_controller/dispatcher.rb index 4dc76e1b49..c4e7357b81 100644 --- a/actionpack/lib/action_controller/dispatcher.rb +++ b/actionpack/lib/action_controller/dispatcher.rb @@ -60,11 +60,10 @@ module ActionController def dispatch begin run_callbacks :before_dispatch - controller = Routing::Routes.recognize(@request) - controller.process(@request, @response).to_a + Routing::Routes.call(@env) rescue Exception => exception if controller ||= (::ApplicationController rescue Base) - controller.process_with_exception(@request, @response, exception).to_a + controller.call_with_exception(@env, exception).to_a else raise exception end @@ -83,8 +82,7 @@ module ActionController end def _call(env) - @request = Request.new(env) - @response = Response.new + @env = env dispatch end @@ -110,8 +108,7 @@ module ActionController def checkin_connections # Don't return connection (and peform implicit rollback) if this request is a part of integration test - # TODO: This callback should have direct access to env - return if @request.key?("rack.test") + return if @env.key?("rack.test") ActiveRecord::Base.clear_active_connections! end end diff --git a/actionpack/lib/action_controller/request.rb b/actionpack/lib/action_controller/request.rb index 3390324162..ba27c0d294 100755 --- a/actionpack/lib/action_controller/request.rb +++ b/actionpack/lib/action_controller/request.rb @@ -398,7 +398,7 @@ EOM end def path_parameters=(parameters) #:nodoc: - @path_parameters = parameters + @env["routing_args"] = parameters @symbolized_path_parameters = @parameters = nil end @@ -414,7 +414,7 @@ EOM # # See symbolized_path_parameters for symbolized keys. def path_parameters - @path_parameters ||= {} + @env["routing_args"] ||= {} end def body diff --git a/actionpack/lib/action_controller/rescue.rb b/actionpack/lib/action_controller/rescue.rb index 5ef79a36ce..3a5e5071bb 100644 --- a/actionpack/lib/action_controller/rescue.rb +++ b/actionpack/lib/action_controller/rescue.rb @@ -59,7 +59,9 @@ module ActionController #:nodoc: end module ClassMethods - def process_with_exception(request, response, exception) #:nodoc: + def call_with_exception(env, exception) #:nodoc: + request = env["actioncontroller.rescue.request"] + response = env["actioncontroller.rescue.response"] new.process(request, response, :rescue_action, exception) end end diff --git a/actionpack/lib/action_controller/routing/route_set.rb b/actionpack/lib/action_controller/routing/route_set.rb index 5975977365..06aef6e169 100644 --- a/actionpack/lib/action_controller/routing/route_set.rb +++ b/actionpack/lib/action_controller/routing/route_set.rb @@ -427,6 +427,12 @@ module ActionController end end + def call(env) + request = Request.new(env) + app = Routing::Routes.recognize(request) + app.call(env).to_a + end + def recognize(request) params = recognize_path(request.path, extract_request_environment(request)) request.path_parameters = params.with_indifferent_access diff --git a/actionpack/test/controller/dispatcher_test.rb b/actionpack/test/controller/dispatcher_test.rb index fd06b4ea99..da87d26146 100644 --- a/actionpack/test/controller/dispatcher_test.rb +++ b/actionpack/test/controller/dispatcher_test.rb @@ -96,9 +96,7 @@ class DispatcherTest < Test::Unit::TestCase private def dispatch(cache_classes = true) - controller = mock() - controller.stubs(:process).returns([200, {}, 'response']) - ActionController::Routing::Routes.stubs(:recognize).returns(controller) + ActionController::Routing::RouteSet.any_instance.stubs(:call).returns([200, {}, 'response']) Dispatcher.define_dispatcher_callbacks(cache_classes) @dispatcher.call({}) end diff --git a/actionpack/test/controller/rescue_test.rb b/actionpack/test/controller/rescue_test.rb index 63f9827f4a..49aca3a6ee 100644 --- a/actionpack/test/controller/rescue_test.rb +++ b/actionpack/test/controller/rescue_test.rb @@ -367,7 +367,11 @@ class RescueControllerTest < ActionController::TestCase end def test_rescue_dispatcher_exceptions - RescueController.process_with_exception(@request, @response, ActionController::RoutingError.new("Route not found")) + env = @request.env + env["actioncontroller.rescue.request"] = @request + env["actioncontroller.rescue.response"] = @response + + RescueController.call_with_exception(env, ActionController::RoutingError.new("Route not found")) assert_equal "no way", @response.body end diff --git a/actionpack/test/controller/routing_test.rb b/actionpack/test/controller/routing_test.rb index d5b6bd6b2a..b981119e1e 100644 --- a/actionpack/test/controller/routing_test.rb +++ b/actionpack/test/controller/routing_test.rb @@ -706,7 +706,7 @@ uses_mocha 'LegacyRouteSet, Route, RouteSet and RouteLoading' do port_string = port == 80 ? '' : ":#{port}" protocol = options.delete(:protocol) || "http" - host = options.delete(:host) || "named.route.test" + host = options.delete(:host) || "test.host" anchor = "##{options.delete(:anchor)}" if options.key?(:anchor) path = routes.generate(options) @@ -715,27 +715,7 @@ uses_mocha 'LegacyRouteSet, Route, RouteSet and RouteLoading' do end def request - @request ||= MockRequest.new(:host => "named.route.test", :method => :get) - end - end - - class MockRequest - attr_accessor :path, :path_parameters, :host, :subdomains, :domain, :method - - def initialize(values={}) - values.each { |key, value| send("#{key}=", value) } - if values[:host] - subdomain, self.domain = values[:host].split(/\./, 2) - self.subdomains = [subdomain] - end - end - - def protocol - "http://" - end - - def host_with_port - (subdomains * '.') + '.' + domain + @request ||= ActionController::TestRequest.new end end @@ -900,7 +880,7 @@ uses_mocha 'LegacyRouteSet, Route, RouteSet and RouteLoading' do def test_basic_named_route rs.add_named_route :home, '', :controller => 'content', :action => 'list' x = setup_for_named_route - assert_equal("http://named.route.test/", + assert_equal("http://test.host/", x.send(:home_url)) end @@ -908,7 +888,7 @@ uses_mocha 'LegacyRouteSet, Route, RouteSet and RouteLoading' do rs.add_named_route :home, '', :controller => 'content', :action => 'list' x = setup_for_named_route ActionController::Base.relative_url_root = "/foo" - assert_equal("http://named.route.test/foo/", + assert_equal("http://test.host/foo/", x.send(:home_url)) assert_equal "/foo/", x.send(:home_path) ActionController::Base.relative_url_root = nil @@ -917,14 +897,14 @@ uses_mocha 'LegacyRouteSet, Route, RouteSet and RouteLoading' do def test_named_route_with_option rs.add_named_route :page, 'page/:title', :controller => 'content', :action => 'show_page' x = setup_for_named_route - assert_equal("http://named.route.test/page/new%20stuff", + assert_equal("http://test.host/page/new%20stuff", x.send(:page_url, :title => 'new stuff')) end def test_named_route_with_default rs.add_named_route :page, 'page/:title', :controller => 'content', :action => 'show_page', :title => 'AboutPage' x = setup_for_named_route - assert_equal("http://named.route.test/page/AboutRails", + assert_equal("http://test.host/page/AboutRails", x.send(:page_url, :title => "AboutRails")) end @@ -932,21 +912,21 @@ uses_mocha 'LegacyRouteSet, Route, RouteSet and RouteLoading' do def test_named_route_with_name_prefix rs.add_named_route :page, 'page', :controller => 'content', :action => 'show_page', :name_prefix => 'my_' x = setup_for_named_route - assert_equal("http://named.route.test/page", + assert_equal("http://test.host/page", x.send(:my_page_url)) end def test_named_route_with_path_prefix rs.add_named_route :page, 'page', :controller => 'content', :action => 'show_page', :path_prefix => 'my' x = setup_for_named_route - assert_equal("http://named.route.test/my/page", + assert_equal("http://test.host/my/page", x.send(:page_url)) end def test_named_route_with_nested_controller rs.add_named_route :users, 'admin/user', :controller => 'admin/user', :action => 'index' x = setup_for_named_route - assert_equal("http://named.route.test/admin/user", + assert_equal("http://test.host/admin/user", x.send(:users_url)) end @@ -985,7 +965,7 @@ uses_mocha 'LegacyRouteSet, Route, RouteSet and RouteLoading' do map.root :controller => "hello" end x = setup_for_named_route - assert_equal("http://named.route.test/", x.send(:root_url)) + assert_equal("http://test.host/", x.send(:root_url)) assert_equal("/", x.send(:root_path)) end @@ -1001,7 +981,7 @@ uses_mocha 'LegacyRouteSet, Route, RouteSet and RouteLoading' do # x.send(:article_url, :title => 'hi') # ) assert_equal( - "http://named.route.test/page/2005/6/10/hi", + "http://test.host/page/2005/6/10/hi", x.send(:article_url, :title => 'hi', :day => 10, :year => 2005, :month => 6) ) end @@ -1202,7 +1182,7 @@ uses_mocha 'LegacyRouteSet, Route, RouteSet and RouteLoading' do assert_equal '/test', rs.generate(:controller => 'post', :action => 'show', :year => nil) x = setup_for_named_route - assert_equal("http://named.route.test/test", + assert_equal("http://test.host/test", x.send(:blog_url)) end @@ -1249,7 +1229,7 @@ uses_mocha 'LegacyRouteSet, Route, RouteSet and RouteLoading' do assert_equal '/', rs.generate(:controller => 'content') x = setup_for_named_route - assert_equal("http://named.route.test/", + assert_equal("http://test.host/", x.send(:home_url)) end @@ -1591,7 +1571,7 @@ uses_mocha 'LegacyRouteSet, Route, RouteSet and RouteLoading' do end def request - @request ||= MockRequest.new(:host => "named.routes.test", :method => :get) + @request ||= ActionController::TestRequest.new end def test_generate_extras @@ -1692,13 +1672,13 @@ uses_mocha 'LegacyRouteSet, Route, RouteSet and RouteLoading' do def test_named_route_url_method controller = setup_named_route_test - assert_equal "http://named.route.test/people/5", controller.send(:show_url, :id => 5) + assert_equal "http://test.host/people/5", controller.send(:show_url, :id => 5) assert_equal "/people/5", controller.send(:show_path, :id => 5) - assert_equal "http://named.route.test/people", controller.send(:index_url) + assert_equal "http://test.host/people", controller.send(:index_url) assert_equal "/people", controller.send(:index_path) - assert_equal "http://named.route.test/admin/users", controller.send(:users_url) + assert_equal "http://test.host/admin/users", controller.send(:users_url) assert_equal '/admin/users', controller.send(:users_path) assert_equal '/admin/users', set.generate(controller.send(:hash_for_users_url), {:controller => 'users', :action => 'index'}) end @@ -1706,28 +1686,28 @@ uses_mocha 'LegacyRouteSet, Route, RouteSet and RouteLoading' do def test_named_route_url_method_with_anchor controller = setup_named_route_test - assert_equal "http://named.route.test/people/5#location", controller.send(:show_url, :id => 5, :anchor => 'location') + assert_equal "http://test.host/people/5#location", controller.send(:show_url, :id => 5, :anchor => 'location') assert_equal "/people/5#location", controller.send(:show_path, :id => 5, :anchor => 'location') - assert_equal "http://named.route.test/people#location", controller.send(:index_url, :anchor => 'location') + assert_equal "http://test.host/people#location", controller.send(:index_url, :anchor => 'location') assert_equal "/people#location", controller.send(:index_path, :anchor => 'location') - assert_equal "http://named.route.test/admin/users#location", controller.send(:users_url, :anchor => 'location') + assert_equal "http://test.host/admin/users#location", controller.send(:users_url, :anchor => 'location') assert_equal '/admin/users#location', controller.send(:users_path, :anchor => 'location') - assert_equal "http://named.route.test/people/go/7/hello/joe/5#location", + assert_equal "http://test.host/people/go/7/hello/joe/5#location", controller.send(:multi_url, 7, "hello", 5, :anchor => 'location') - assert_equal "http://named.route.test/people/go/7/hello/joe/5?baz=bar#location", + assert_equal "http://test.host/people/go/7/hello/joe/5?baz=bar#location", controller.send(:multi_url, 7, "hello", 5, :baz => "bar", :anchor => 'location') - assert_equal "http://named.route.test/people?baz=bar#location", + assert_equal "http://test.host/people?baz=bar#location", controller.send(:index_url, :baz => "bar", :anchor => 'location') end def test_named_route_url_method_with_port controller = setup_named_route_test - assert_equal "http://named.route.test:8080/people/5", controller.send(:show_url, 5, :port=>8080) + assert_equal "http://test.host:8080/people/5", controller.send(:show_url, 5, :port=>8080) end def test_named_route_url_method_with_host @@ -1737,30 +1717,30 @@ uses_mocha 'LegacyRouteSet, Route, RouteSet and RouteLoading' do def test_named_route_url_method_with_protocol controller = setup_named_route_test - assert_equal "https://named.route.test/people/5", controller.send(:show_url, 5, :protocol => "https") + assert_equal "https://test.host/people/5", controller.send(:show_url, 5, :protocol => "https") end def test_named_route_url_method_with_ordered_parameters controller = setup_named_route_test - assert_equal "http://named.route.test/people/go/7/hello/joe/5", + assert_equal "http://test.host/people/go/7/hello/joe/5", controller.send(:multi_url, 7, "hello", 5) end def test_named_route_url_method_with_ordered_parameters_and_hash controller = setup_named_route_test - assert_equal "http://named.route.test/people/go/7/hello/joe/5?baz=bar", + assert_equal "http://test.host/people/go/7/hello/joe/5?baz=bar", controller.send(:multi_url, 7, "hello", 5, :baz => "bar") end def test_named_route_url_method_with_ordered_parameters_and_empty_hash controller = setup_named_route_test - assert_equal "http://named.route.test/people/go/7/hello/joe/5", + assert_equal "http://test.host/people/go/7/hello/joe/5", controller.send(:multi_url, 7, "hello", 5, {}) end def test_named_route_url_method_with_no_positional_arguments controller = setup_named_route_test - assert_equal "http://named.route.test/people?baz=bar", + assert_equal "http://test.host/people?baz=bar", controller.send(:index_url, :baz => "bar") end @@ -1896,49 +1876,54 @@ uses_mocha 'LegacyRouteSet, Route, RouteSet and RouteLoading' do end request.path = "/people" - request.method = :get + request.env["REQUEST_METHOD"] = "GET" assert_nothing_raised { set.recognize(request) } assert_equal("index", request.path_parameters[:action]) + request.recycle! - request.method = :post + request.env["REQUEST_METHOD"] = "POST" assert_nothing_raised { set.recognize(request) } assert_equal("create", request.path_parameters[:action]) + request.recycle! - request.method = :put + request.env["REQUEST_METHOD"] = "PUT" assert_nothing_raised { set.recognize(request) } assert_equal("update", request.path_parameters[:action]) + request.recycle! - begin - request.method = :bacon + assert_raises(ActionController::UnknownHttpMethod) { + request.env["REQUEST_METHOD"] = "BACON" set.recognize(request) - flunk 'Should have raised NotImplemented' - rescue ActionController::NotImplemented => e - assert_equal [:get, :post, :put, :delete], e.allowed_methods - end + } + request.recycle! request.path = "/people/5" - request.method = :get + request.env["REQUEST_METHOD"] = "GET" assert_nothing_raised { set.recognize(request) } assert_equal("show", request.path_parameters[:action]) assert_equal("5", request.path_parameters[:id]) + request.recycle! - request.method = :put + request.env["REQUEST_METHOD"] = "PUT" assert_nothing_raised { set.recognize(request) } assert_equal("update", request.path_parameters[:action]) assert_equal("5", request.path_parameters[:id]) + request.recycle! - request.method = :delete + request.env["REQUEST_METHOD"] = "DELETE" assert_nothing_raised { set.recognize(request) } assert_equal("destroy", request.path_parameters[:action]) assert_equal("5", request.path_parameters[:id]) + request.recycle! begin - request.method = :post + request.env["REQUEST_METHOD"] = "POST" set.recognize(request) flunk 'Should have raised MethodNotAllowed' rescue ActionController::MethodNotAllowed => e assert_equal [:get, :put, :delete], e.allowed_methods end + request.recycle! ensure Object.send(:remove_const, :PeopleController) @@ -1954,13 +1939,13 @@ uses_mocha 'LegacyRouteSet, Route, RouteSet and RouteLoading' do end request.path = "/people" - request.method = :get + request.env["REQUEST_METHOD"] = "GET" assert_nothing_raised { set.recognize(request) } assert_equal("people", request.path_parameters[:controller]) assert_equal("index", request.path_parameters[:action]) request.path = "/" - request.method = :get + request.env["REQUEST_METHOD"] = "GET" assert_nothing_raised { set.recognize(request) } assert_equal("people", request.path_parameters[:controller]) assert_equal("index", request.path_parameters[:action]) @@ -1978,7 +1963,7 @@ uses_mocha 'LegacyRouteSet, Route, RouteSet and RouteLoading' do end request.path = "/articles/2005/11/05/a-very-interesting-article" - request.method = :get + request.env["REQUEST_METHOD"] = "GET" assert_nothing_raised { set.recognize(request) } assert_equal("permalink", request.path_parameters[:action]) assert_equal("2005", request.path_parameters[:year]) @@ -2015,17 +2000,19 @@ uses_mocha 'LegacyRouteSet, Route, RouteSet and RouteLoading' do end request.path = "/people/5" - request.method = :get + request.env["REQUEST_METHOD"] = "GET" assert_nothing_raised { set.recognize(request) } assert_equal("show", request.path_parameters[:action]) assert_equal("5", request.path_parameters[:id]) + request.recycle! - request.method = :put + request.env["REQUEST_METHOD"] = "PUT" assert_nothing_raised { set.recognize(request) } assert_equal("update", request.path_parameters[:action]) + request.recycle! request.path = "/people/5.png" - request.method = :get + request.env["REQUEST_METHOD"] = "GET" assert_nothing_raised { set.recognize(request) } assert_equal("show", request.path_parameters[:action]) assert_equal("5", request.path_parameters[:id]) @@ -2050,7 +2037,7 @@ uses_mocha 'LegacyRouteSet, Route, RouteSet and RouteLoading' do set.draw { |map| map.root :controller => "people" } request.path = "" - request.method = :get + request.env["REQUEST_METHOD"] = "GET" assert_nothing_raised { set.recognize(request) } assert_equal("people", request.path_parameters[:controller]) assert_equal("index", request.path_parameters[:action]) @@ -2070,7 +2057,7 @@ uses_mocha 'LegacyRouteSet, Route, RouteSet and RouteLoading' do end request.path = "/api/inventory" - request.method = :get + request.env["REQUEST_METHOD"] = "GET" assert_nothing_raised { set.recognize(request) } assert_equal("api/products", request.path_parameters[:controller]) assert_equal("inventory", request.path_parameters[:action]) @@ -2090,7 +2077,7 @@ uses_mocha 'LegacyRouteSet, Route, RouteSet and RouteLoading' do end request.path = "/api" - request.method = :get + request.env["REQUEST_METHOD"] = "GET" assert_nothing_raised { set.recognize(request) } assert_equal("api/products", request.path_parameters[:controller]) assert_equal("index", request.path_parameters[:action]) @@ -2110,7 +2097,7 @@ uses_mocha 'LegacyRouteSet, Route, RouteSet and RouteLoading' do end request.path = "/prefix/inventory" - request.method = :get + request.env["REQUEST_METHOD"] = "GET" assert_nothing_raised { set.recognize(request) } assert_equal("api/products", request.path_parameters[:controller]) assert_equal("inventory", request.path_parameters[:action]) @@ -2246,7 +2233,7 @@ uses_mocha 'LegacyRouteSet, Route, RouteSet and RouteLoading' do end request.path = "/projects/1/milestones" - request.method = :get + request.env["REQUEST_METHOD"] = "GET" assert_nothing_raised { set.recognize(request) } assert_equal("milestones", request.path_parameters[:controller]) assert_equal("index", request.path_parameters[:action]) -- cgit v1.2.3 From c20c72e3d9321f8c00587aab479d962e80b02c35 Mon Sep 17 00:00:00 2001 From: Joshua Peek Date: Sun, 28 Dec 2008 15:34:59 -0600 Subject: Use rack namespace for routing args --- actionpack/lib/action_controller/request.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/actionpack/lib/action_controller/request.rb b/actionpack/lib/action_controller/request.rb index ba27c0d294..822955d1db 100755 --- a/actionpack/lib/action_controller/request.rb +++ b/actionpack/lib/action_controller/request.rb @@ -398,7 +398,7 @@ EOM end def path_parameters=(parameters) #:nodoc: - @env["routing_args"] = parameters + @env["rack.routing_args"] = parameters @symbolized_path_parameters = @parameters = nil end @@ -414,7 +414,7 @@ EOM # # See symbolized_path_parameters for symbolized keys. def path_parameters - @env["routing_args"] ||= {} + @env["rack.routing_args"] ||= {} end def body -- cgit v1.2.3