From 1b97701e51667e6040b4c576cce9234edef1019e Mon Sep 17 00:00:00 2001
From: wycats <wycats@gmail.com>
Date: Mon, 26 Jul 2010 00:59:54 -0700
Subject: [PATCH] Fix a bug where requires inside of autoloads were being added
 to the autoloaded_constants list, causing mayhem. [#5165 state:resolved]

---
 .../lib/active_support/dependencies.rb        | 17 +++++--
 .../load_path/loaded_constant.rb              |  3 ++
 .../autoloading_fixtures/loads_constant.rb    |  4 ++
 .../autoloading_fixtures/requires_constant.rb |  5 ++
 activesupport/test/dependencies_test.rb       | 49 +++++++++++++++++++
 5 files changed, 73 insertions(+), 5 deletions(-)
 create mode 100644 activesupport/test/autoloading_fixtures/load_path/loaded_constant.rb
 create mode 100644 activesupport/test/autoloading_fixtures/loads_constant.rb
 create mode 100644 activesupport/test/autoloading_fixtures/requires_constant.rb

diff --git a/activesupport/lib/active_support/dependencies.rb b/activesupport/lib/active_support/dependencies.rb
index 9a6da38b1cc9b..2b80bd214f6ec 100644
--- a/activesupport/lib/active_support/dependencies.rb
+++ b/activesupport/lib/active_support/dependencies.rb
@@ -72,10 +72,6 @@ def self.locked(*methods)
         methods.each { |m| class_eval "def #{m}(*) lock { super } end", __FILE__, __LINE__ }
       end
 
-      def get(key)
-        (val = assoc(key)) ? val[1] : []
-      end
-
       locked :concat, :each, :delete_if, :<<
 
       def new_constants_for(frames)
@@ -85,7 +81,18 @@ def new_constants_for(frames)
           next unless mod.is_a?(Module)
 
           new_constants = mod.local_constant_names - prior_constants
-          get(mod_name).concat(new_constants)
+
+          # If we are checking for constants under, say, :Object, nested under something
+          # else that is checking for constants also under :Object, make sure the
+          # parent knows that we have found, and taken care of, the constant.
+          #
+          # In particular, this means that since Kernel.require discards the constants
+          # it finds, parents will be notified that about those constants, and not
+          # consider them "new". As a result, they will not be added to the
+          # autoloaded_constants list.
+          each do |key, value|
+            value.concat(new_constants) if key == mod_name
+          end
 
           new_constants.each do |suffix|
             constants << ([mod_name, suffix] - ["Object"]).join("::")
diff --git a/activesupport/test/autoloading_fixtures/load_path/loaded_constant.rb b/activesupport/test/autoloading_fixtures/load_path/loaded_constant.rb
new file mode 100644
index 0000000000000..e3d1218c96d4e
--- /dev/null
+++ b/activesupport/test/autoloading_fixtures/load_path/loaded_constant.rb
@@ -0,0 +1,3 @@
+module LoadedConstant
+end
+
diff --git a/activesupport/test/autoloading_fixtures/loads_constant.rb b/activesupport/test/autoloading_fixtures/loads_constant.rb
new file mode 100644
index 0000000000000..b5b80c46da77f
--- /dev/null
+++ b/activesupport/test/autoloading_fixtures/loads_constant.rb
@@ -0,0 +1,4 @@
+module LoadsConstant
+end
+
+RequiresConstant
diff --git a/activesupport/test/autoloading_fixtures/requires_constant.rb b/activesupport/test/autoloading_fixtures/requires_constant.rb
new file mode 100644
index 0000000000000..14804a0de057b
--- /dev/null
+++ b/activesupport/test/autoloading_fixtures/requires_constant.rb
@@ -0,0 +1,5 @@
+require "loaded_constant"
+
+module RequiresConstant
+end
+
diff --git a/activesupport/test/dependencies_test.rb b/activesupport/test/dependencies_test.rb
index d7bde185bd930..ec5116bff47cd 100644
--- a/activesupport/test/dependencies_test.rb
+++ b/activesupport/test/dependencies_test.rb
@@ -213,6 +213,48 @@ def test_nested_class_can_access_sibling
     end
   end
 
+  def test_doesnt_break_normal_require
+    path = File.expand_path("../autoloading_fixtures/load_path", __FILE__)
+    original_path = $:.dup
+    original_features = $".dup
+    $:.push(path)
+
+    with_autoloading_fixtures do
+      RequiresConstant
+      assert defined?(RequiresConstant)
+      assert defined?(LoadedConstant)
+      ActiveSupport::Dependencies.clear
+      RequiresConstant
+      assert defined?(RequiresConstant)
+      assert defined?(LoadedConstant)
+    end
+  ensure
+    remove_constants(:RequiresConstant, :LoadedConstant, :LoadsConstant)
+    $".replace(original_features)
+    $:.replace(original_path)
+  end
+
+  def test_doesnt_break_normal_require_nested
+    path = File.expand_path("../autoloading_fixtures/load_path", __FILE__)
+    original_path = $:.dup
+    original_features = $".dup
+    $:.push(path)
+
+    with_autoloading_fixtures do
+      LoadsConstant
+      assert defined?(LoadsConstant)
+      assert defined?(LoadedConstant)
+      ActiveSupport::Dependencies.clear
+      LoadsConstant
+      assert defined?(LoadsConstant)
+      assert defined?(LoadedConstant)
+    end
+  ensure
+    remove_constants(:RequiresConstant, :LoadedConstant, :LoadsConstant)
+    $".replace(original_features)
+    $:.replace(original_path)
+  end
+
   def failing_test_access_thru_and_upwards_fails
     with_autoloading_fixtures do
       assert ! defined?(ModuleFolder)
@@ -797,4 +839,11 @@ def test_unhook
   ensure
     ActiveSupport::Dependencies.hook!
   end
+
+private
+  def remove_constants(*constants)
+    constants.each do |constant|
+      Object.send(:remove_const, constant) if Object.const_defined?(constant)
+    end
+  end
 end