Fixed that in some circumstances controllers outside of modules may have hidden ones inside modules. For example, admin/content might have been hidden by /content. #1075 [Nicholas Seckar]

git-svn-id: http://svn-commit.rubyonrails.org/rails/trunk@1125 5ecf4fe2-1ee6-0310-87b1-e25e094e27de

6 files changed, 50 insertions, 11 deletions

diff --git a/actionpack/CHANGELOG b/actionpack/CHANGELOG
index f85f28adb1..70658fc781 100644
--- a/actionpack/CHANGELOG
+++ b/actionpack/CHANGELOG
@@ -1,5 +1,7 @@
 *SVN*
 
+* Fixed that in some circumstances controllers outside of modules may have hidden ones inside modules. For example, admin/content might have been hidden by /content. #1075 [Nicholas Seckar]
+
 * Added JavascriptHelper#periodically_call_remote in order to create areas of a page that update automatically at a set interval #945 [Jon Tirsen]
 
 * Fixed Cache#expire_matched_fragments that couldn't recognize the difference between string and url_for options #1030 [skaes@web.de]
diff --git a/actionpack/lib/action_controller/routing.rb b/actionpack/lib/action_controller/routing.rb
index 28d692aa5b..406c6acb75 100644
--- a/actionpack/lib/action_controller/routing.rb
+++ b/actionpack/lib/action_controller/routing.rb
@@ -162,7 +162,7 @@ module ActionController
             name = name.camelize
             return nil, nil unless /^[A-Z][_a-zA-Z\d]*$/ =~ name
             controller_name = name + "Controller"
-            return mod.const_get(controller_name), path[length..-1] if mod.const_available? controller_name
+            return eval("mod::#{controller_name}"), path[length..-1] if mod.const_available? controller_name
             return nil, nil unless mod.const_available? name
             [mod.const_get(name), length + 1]
           end
diff --git a/activesupport/CHANGELOG b/activesupport/CHANGELOG
index a2528696f4..ff7f51ca5e 100644
--- a/activesupport/CHANGELOG
+++ b/activesupport/CHANGELOG
@@ -1,5 +1,7 @@
 *SVN*
 
+* Fixed that in some circumstances controllers outside of modules may have hidden ones inside modules. For example, admin/content might have been hidden by /content. #1075 [Nicholas Seckar]
+
 * Fixed inflection of perspectives and similar words #1045 [thijs@vandervossen.net]
 
 * Added Fixnum#even? and Fixnum#odd?
diff --git a/activesupport/lib/active_support/dependencies.rb b/activesupport/lib/active_support/dependencies.rb
index 91f1f54f5a..dd71949399 100644
--- a/activesupport/lib/active_support/dependencies.rb
+++ b/activesupport/lib/active_support/dependencies.rb
@@ -55,7 +55,7 @@ module Dependencies
     def self.root(*load_paths)
       RootLoadingModule.new(*load_paths)
     end
-            
+    
     def initialize(root, path=[])
       @path = path.clone.freeze
       @root = root
@@ -81,9 +81,11 @@ module Dependencies
           new_module = LoadingModule.new(self.root, self.path + [name])
           self.const_set name, new_module
           if self.root?
-            raise NameError, "Cannot load controller module named #{name}: An object of type #{Object.const_get(name).class.to_s} already exists." \
-              if Object.const_defined?(name)
-            Object.const_set(name, new_module) 
+            if Object.const_defined?(name)
+              msg = "Cannot load module #{name}: Object::#{name} is set to #{Object.const_get(name).inspect}"
+              raise NameError, msg
+            end
+            Object.const_set(name, new_module)
           end
           break
         elsif File.file?(fs_path)
@@ -94,7 +96,7 @@ module Dependencies
           break
         end
       end
-
+      
       return self.const_defined?(name)
     end
     
@@ -124,7 +126,7 @@ module Dependencies
     # Erase all items in this module
     def clear!
       constants.each do |name|
-        Object.send(:remove_const, name) if Object.const_defined?(name)
+        Object.send(:remove_const, name) if Object.const_defined?(name) && Object.const_get(name).object_id == self.const_get(name).object_id
         self.send(:remove_const, name)
       end
     end
diff --git a/activesupport/test/loading_module/content_controller.rb b/activesupport/test/loading_module/content_controller.rb
index f0870161e5..78317b0518 100644
--- a/activesupport/test/loading_module/content_controller.rb
+++ b/activesupport/test/loading_module/content_controller.rb
@@ -1,2 +1,3 @@
 class ContentController
+  def identifier() :outer end
 end
diff --git a/activesupport/test/loading_module_tests.rb b/activesupport/test/loading_module_tests.rb
index b6587d48ed..01361fae1e 100644
--- a/activesupport/test/loading_module_tests.rb
+++ b/activesupport/test/loading_module_tests.rb
@@ -8,11 +8,13 @@ COMPONENTS_DIRECTORY = File.join(File.dirname(__FILE__), 'loading_module_compone
 class LoadingModuleTests < Test::Unit::TestCase
   def setup
     @loading_module = Dependencies::LoadingModule.root(STAGING_DIRECTORY)
+    Object.send(:remove_const, :Controllers) if Object.const_defined?(:Controllers)
     Object.const_set(:Controllers, @loading_module)
   end
   def teardown
-    @loading_module.clear
     Object.send :remove_const, :Controllers
+    @loading_module.clear!
+    Dependencies.clear
   end
   
   def test_setup
@@ -29,6 +31,19 @@ class LoadingModuleTests < Test::Unit::TestCase
     assert_equal false, @loading_module.const_available?(:RandomName)
   end
   
+  def test_nested_const_available
+    assert @loading_module::Admin.const_available?(:AccessController)
+    assert @loading_module::Admin.const_available?(:UserController)
+    assert @loading_module::Admin.const_available?(:ContentController)
+    assert ! @loading_module::Admin.const_available?(:ResourceController)
+  end
+  
+  def test_nested_module_export
+    @loading_module::Admin
+    assert_equal @loading_module::Admin.object_id, Object::Admin.object_id
+    assert_equal @loading_module::Admin.object_id, Controllers::Admin.object_id
+  end
+  
   def test_const_load_module
     assert @loading_module.const_load!(:Admin)
     assert_kind_of Module, @loading_module::Admin
@@ -42,8 +57,8 @@ class LoadingModuleTests < Test::Unit::TestCase
   
   def test_const_load_nested_controller
     assert @loading_module.const_load!(:Admin)
+    assert_kind_of Dependencies::LoadingModule, @loading_module::Admin
     assert @loading_module::Admin.const_available?(:UserController)
-    assert @loading_module::Admin.const_load!(:UserController)
     assert_kind_of Class, @loading_module::Admin::UserController
   end
   
@@ -61,6 +76,22 @@ class LoadingModuleTests < Test::Unit::TestCase
     assert_raises(NameError) {@loading_module::PersonController}
     assert_raises(NameError) {@loading_module::Admin::FishController}
   end
+  
+  def test_name_clash
+    assert ! @loading_module::const_defined?(:ContentController)
+    assert_equal :outer, @loading_module::ContentController.new.identifier
+    assert ! @loading_module::Admin.const_defined?(:ContentController)
+    assert_equal :inner, @loading_module::Admin::ContentController.new.identifier
+    assert @loading_module::ContentController.object_id != @loading_module::Admin::ContentController.object_id
+  end
+
+  def test_name_clash_other_way
+    assert ! @loading_module::Admin.const_defined?(:ContentController)
+    assert_equal :inner, @loading_module::Admin::ContentController.new.identifier
+    assert ! @loading_module::const_defined?(:ContentController)
+    assert_equal :outer, @loading_module::ContentController.new.identifier
+    assert @loading_module::ContentController.object_id != @loading_module::Admin::ContentController.object_id
+  end
 end
 
 class LoadingModuleMultiPathTests < Test::Unit::TestCase
@@ -69,8 +100,9 @@ class LoadingModuleMultiPathTests < Test::Unit::TestCase
     Object.const_set(:Controllers, @loading_module)
   end
   def teardown
-    @loading_module.clear
     Object.send :remove_const, :Controllers
+    @loading_module.clear!
+    Dependencies.clear
   end
   
   def test_access_from_first
@@ -81,7 +113,7 @@ class LoadingModuleMultiPathTests < Test::Unit::TestCase
   def test_access_from_second
     assert_kind_of Module, @loading_module::List
     assert_kind_of Dependencies::LoadingModule, @loading_module::List
-    assert @loading_module::List.const_load! :ListController
+    assert @loading_module::List.const_load!(:ListController)
     assert_kind_of Class, @loading_module::List::ListController
   end
 end
\ No newline at end of file