From 7317d9ef4c1361219671dc4405a02ed3896f1742 Mon Sep 17 00:00:00 2001 From: Joshua Peek Date: Sun, 28 Feb 2010 16:39:01 -0600 Subject: [PATCH] Remove implicit controller namespacing from new dsl --- .../routing/deprecated_mapper.rb | 25 +++++++++++++++++++ .../lib/action_dispatch/routing/mapper.rb | 1 - .../lib/action_dispatch/routing/route_set.rb | 24 ++---------------- actionpack/test/abstract_unit.rb | 8 +++--- .../controller/action_pack_assertions_test.rb | 9 ++++--- actionpack/test/controller/render_xml_test.rb | 3 ++- railties/lib/rails/engine.rb | 3 ++- 7 files changed, 42 insertions(+), 31 deletions(-) diff --git a/actionpack/lib/action_dispatch/routing/deprecated_mapper.rb b/actionpack/lib/action_dispatch/routing/deprecated_mapper.rb index 1417a9d8c0..dd650e83d9 100644 --- a/actionpack/lib/action_dispatch/routing/deprecated_mapper.rb +++ b/actionpack/lib/action_dispatch/routing/deprecated_mapper.rb @@ -1,5 +1,30 @@ module ActionDispatch module Routing + class RouteSet + attr_accessor :controller_namespaces + + CONTROLLER_REGEXP = /[_a-zA-Z0-9]+/ + + def controller_constraints + @controller_constraints ||= begin + namespaces = controller_namespaces + in_memory_controller_namespaces + source = namespaces.map { |ns| "#{Regexp.escape(ns)}/#{CONTROLLER_REGEXP.source}" } + source << CONTROLLER_REGEXP.source + Regexp.compile(source.sort.reverse.join('|')) + end + end + + def in_memory_controller_namespaces + namespaces = Set.new + ActionController::Base.subclasses.each do |klass| + controller_name = klass.underscore + namespaces << controller_name.split('/')[0...-1].join('/') + end + namespaces.delete('') + namespaces + end + end + # Mapper instances are used to build routes. The object passed to the draw # block in config/routes.rb is a Mapper instance. # diff --git a/actionpack/lib/action_dispatch/routing/mapper.rb b/actionpack/lib/action_dispatch/routing/mapper.rb index bead321c9c..52e7b0e77d 100644 --- a/actionpack/lib/action_dispatch/routing/mapper.rb +++ b/actionpack/lib/action_dispatch/routing/mapper.rb @@ -88,7 +88,6 @@ def requirements @requirements ||= returning(@options[:constraints] || {}) do |requirements| requirements.reverse_merge!(@scope[:constraints]) if @scope[:constraints] @options.each { |k, v| requirements[k] = v if v.is_a?(Regexp) } - requirements[:controller] ||= @set.controller_constraints end end diff --git a/actionpack/lib/action_dispatch/routing/route_set.rb b/actionpack/lib/action_dispatch/routing/route_set.rb index 6bc4303be3..99436e3cb0 100644 --- a/actionpack/lib/action_dispatch/routing/route_set.rb +++ b/actionpack/lib/action_dispatch/routing/route_set.rb @@ -1,5 +1,6 @@ require 'rack/mount' require 'forwardable' +require 'action_dispatch/routing/deprecated_mapper' module ActionDispatch module Routing @@ -208,7 +209,7 @@ def #{selector}(*args) end end - attr_accessor :routes, :named_routes, :controller_namespaces + attr_accessor :routes, :named_routes attr_accessor :disable_clear_and_finalize, :resources_path_names def self.default_resources_path_names @@ -291,27 +292,6 @@ def empty? routes.empty? end - CONTROLLER_REGEXP = /[_a-zA-Z0-9]+/ - - def controller_constraints - @controller_constraints ||= begin - namespaces = controller_namespaces + in_memory_controller_namespaces - source = namespaces.map { |ns| "#{Regexp.escape(ns)}/#{CONTROLLER_REGEXP.source}" } - source << CONTROLLER_REGEXP.source - Regexp.compile(source.sort.reverse.join('|')) - end - end - - def in_memory_controller_namespaces - namespaces = Set.new - ActionController::Base.subclasses.each do |klass| - controller_name = klass.underscore - namespaces << controller_name.split('/')[0...-1].join('/') - end - namespaces.delete('') - namespaces - end - def add_route(app, conditions = {}, requirements = {}, defaults = {}, name = nil) route = Route.new(app, conditions, requirements, defaults, name) @set.add_route(*route) diff --git a/actionpack/test/abstract_unit.rb b/actionpack/test/abstract_unit.rb index c178dc481c..7bcaf0a5eb 100644 --- a/actionpack/test/abstract_unit.rb +++ b/actionpack/test/abstract_unit.rb @@ -73,11 +73,13 @@ class TestCase # have been loaded. setup_once do SharedTestRoutes.draw do |map| - match ':controller(/:action(/:id))' + # FIXME: match ':controller(/:action(/:id))' + map.connect ':controller/:action/:id' end - ActionController::IntegrationTest.app.router.draw do - match ':controller(/:action(/:id))' + ActionController::IntegrationTest.app.router.draw do |map| + # FIXME: match ':controller(/:action(/:id))' + map.connect ':controller/:action/:id' end end end diff --git a/actionpack/test/controller/action_pack_assertions_test.rb b/actionpack/test/controller/action_pack_assertions_test.rb index 7c83a91f4d..6906dc97e8 100644 --- a/actionpack/test/controller/action_pack_assertions_test.rb +++ b/actionpack/test/controller/action_pack_assertions_test.rb @@ -258,7 +258,8 @@ def test_assert_redirect_to_nested_named_route with_routing do |set| set.draw do |map| match 'admin/inner_module', :to => 'admin/inner_module#index', :as => :admin_inner_module - match ':controller/:action' + # match ':controller/:action' + map.connect ':controller/:action/:id' end process :redirect_to_index # redirection is <{"action"=>"index", "controller"=>"admin/admin/inner_module"}> @@ -272,7 +273,8 @@ def test_assert_redirected_to_top_level_named_route_from_nested_controller with_routing do |set| set.draw do |map| match '/action_pack_assertions/:id', :to => 'action_pack_assertions#index', :as => :top_level - match ':controller/:action' + # match ':controller/:action' + map.connect ':controller/:action/:id' end process :redirect_to_top_level_named_route # assert_redirected_to "http://test.host/action_pack_assertions/foo" would pass because of exact match early return @@ -287,7 +289,8 @@ def test_assert_redirected_to_top_level_named_route_with_same_controller_name_in set.draw do |map| # this controller exists in the admin namespace as well which is the only difference from previous test match '/user/:id', :to => 'user#index', :as => :top_level - match ':controller/:action' + # match ':controller/:action' + map.connect ':controller/:action/:id' end process :redirect_to_top_level_named_route # assert_redirected_to top_level_url('foo') would pass because of exact match early return diff --git a/actionpack/test/controller/render_xml_test.rb b/actionpack/test/controller/render_xml_test.rb index b5b0d0b9d5..4da6c954cf 100644 --- a/actionpack/test/controller/render_xml_test.rb +++ b/actionpack/test/controller/render_xml_test.rb @@ -62,7 +62,8 @@ def test_rendering_with_object_location_should_set_header_with_url_for with_routing do |set| set.draw do |map| resources :customers - match ':controller/:action' + # match ':controller/:action' + map.connect ':controller/:action/:id' end get :render_with_object_location diff --git a/railties/lib/rails/engine.rb b/railties/lib/rails/engine.rb index b6ac48768d..af47227510 100644 --- a/railties/lib/rails/engine.rb +++ b/railties/lib/rails/engine.rb @@ -43,7 +43,7 @@ def find_root_with_flag(flag, default=nil) delegate :middleware, :paths, :root, :to => :config def load_tasks - super + super config.paths.lib.tasks.to_a.sort.each { |ext| load(ext) } end @@ -77,6 +77,7 @@ def load_tasks end end + # DEPRECATED: Remove in 3.1 initializer :add_routing_namespaces do |app| paths.app.controllers.to_a.each do |load_path| load_path = File.expand_path(load_path)