Update Initializer to use load_once_paths to avoid plugin reloading. References #5852.

Add Dependencies.load_once_paths.


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

5 files changed, 88 insertions, 48 deletions

diff --git a/activesupport/CHANGELOG b/activesupport/CHANGELOG
index d35191717d..4eded063aa 100644
--- a/activesupport/CHANGELOG
+++ b/activesupport/CHANGELOG
@@ -1,5 +1,7 @@
 *SVN*
 
+* Update Initializer to use load_once_paths to avoid plugin reloading. References #5852. [Nicholas Seckar]
+
 * Use Array#assoc in ActiveSupport::OrderedHash. [Mauricio Fernandez]
 
 * Greatly increased performance of String.to_json, which speeds up RJS considerably on large pages, fixes #3473 [Shugo Maeda]
diff --git a/activesupport/lib/active_support/dependencies.rb b/activesupport/lib/active_support/dependencies.rb
index 30129d1a1b..ca418fe500 100644
--- a/activesupport/lib/active_support/dependencies.rb
+++ b/activesupport/lib/active_support/dependencies.rb
@@ -21,11 +21,18 @@ module Dependencies #:nodoc:
   # Should we load files or require them?
   mattr_accessor :mechanism
   self.mechanism = :load
-
-  # The set of directories from which we may autoload files
-  mattr_accessor :autoload_paths
-  self.autoload_paths = []
-
+  
+  # The set of directories from which we may automatically load files. Files
+  # under these directories will be reloaded on each request in development mode,
+  # unless the directory also appears in load_once_paths.
+  mattr_accessor :load_paths
+  self.load_paths = []
+  
+  # The set of directories from which automatically loaded constants are loaded
+  # only once. All directories in this set must also be present in +load_paths+.
+  mattr_accessor :load_once_paths
+  self.load_once_paths = []
+  
   # An array of qualified constant names that have been loaded. Adding a name to
   # this array will cause it to be unloaded the next time Dependencies are cleared.
   mattr_accessor :autoloaded_constants
@@ -40,7 +47,7 @@ module Dependencies #:nodoc:
   end
 
   def depend_on(file_name, swallow_load_errors = false)
-    path = search_for_autoload_file(file_name)
+    path = search_for_file(file_name)
     require_or_load(path || file_name)
   rescue LoadError
     raise unless swallow_load_errors
@@ -110,12 +117,13 @@ module Dependencies #:nodoc:
     return true
   end
   
-  # Given +path+ return an array of constant paths which would cause Dependencies
-  # to attempt to load +path+.
-  def autoloadable_constants_for_path(path)
+  # Given +path+, a filesystem path to a ruby file, return an array of constant
+  # paths which would cause Dependencies to attempt to load this file.
+  # 
+  def loadable_constants_for_path(path, bases = load_paths - load_once_paths)
     path = $1 if path =~ /\A(.*)\.rb\Z/
     expanded_path = File.expand_path(path)
-    autoload_paths.collect do |root|
+    bases.collect do |root|
       expanded_root = File.expand_path root
       next unless expanded_path.starts_with? expanded_root
       
@@ -127,10 +135,10 @@ module Dependencies #:nodoc:
     end.compact.uniq
   end
   
-  # Search for a file in the autoload_paths matching the provided suffix.
-  def search_for_autoload_file(path_suffix)
+  # Search for a file in load_paths matching the provided suffix.
+  def search_for_file(path_suffix)
     path_suffix = path_suffix + '.rb' unless path_suffix.ends_with? '.rb'
-    autoload_paths.each do |root|
+    load_paths.each do |root|
       path = File.join(root, path_suffix)
       return path if File.file? path
     end
@@ -138,10 +146,25 @@ module Dependencies #:nodoc:
   end
   
   # Does the provided path_suffix correspond to an autoloadable module?
+  # Instead of returning a boolean, the autoload base for this module is returned. 
   def autoloadable_module?(path_suffix)
-    autoload_paths.any? do |autoload_path|
-      File.directory? File.join(autoload_path, path_suffix)
+    load_paths.each do |load_path|
+      return load_path if File.directory? File.join(load_path, path_suffix)
     end
+    nil
+  end
+  
+  # Attempt to autoload the provided module name by searching for a directory
+  # matching the expect path suffix. If found, the module is created and assigned
+  # to +into+'s constants with the name +const_name+. Provided that the directory
+  # was loaded from a reloadable base path, it is added to the set of constants
+  # that are to be unloaded.
+  def autoload_module!(into, const_name, qualified_name, path_suffix)
+    return nil unless base_path = autoloadable_module?(path_suffix)
+    mod = Module.new
+    into.const_set const_name, mod
+    autoloaded_constants << qualified_name unless load_once_paths.include?(base_path)
+    return mod
   end
   
   # Load the file at the provided path. +const_paths+ is a set of qualified
@@ -151,10 +174,9 @@ module Dependencies #:nodoc:
   # 
   # If the second parameter is left off, then Dependencies will construct a set
   # of names that the file at +path+ may define. See
-  # +autoloadable_constants_for_path+ for more details.
-  def load_file(path, const_paths = autoloadable_constants_for_path(path))
+  # +loadable_constants_for_path+ for more details.
+  def load_file(path, const_paths = loadable_constants_for_path(path))
     log_call path, const_paths
-    
     const_paths = [const_paths].compact unless const_paths.is_a? Array
     undefined_before = const_paths.reject(&method(:qualified_const_defined?))
     
@@ -192,15 +214,12 @@ module Dependencies #:nodoc:
     path_suffix = qualified_name.underscore
     name_error = NameError.new("uninitialized constant #{qualified_name}")
     
-    file_path = search_for_autoload_file(path_suffix)
+    file_path = search_for_file(path_suffix)
     if file_path && ! loaded.include?(File.expand_path(file_path)) # We found a matching file to load
-      require_or_load file_path, qualified_name
+      require_or_load file_path
       raise LoadError, "Expected #{file_path} to define #{qualified_name}" unless from_mod.const_defined?(const_name)
       return from_mod.const_get(const_name)
-    elsif autoloadable_module? path_suffix # Create modules for directories
-      mod = Module.new
-      from_mod.const_set const_name, mod
-      autoloaded_constants << qualified_name
+    elsif mod = autoload_module!(from_mod, const_name, qualified_name, path_suffix)
       return mod
     elsif (parent = from_mod.parent) && parent != from_mod &&
           ! from_mod.parents.any? { |p| p.const_defined?(const_name) }
diff --git a/activesupport/test/dependencies/requires_nonexistent0.rb b/activesupport/test/dependencies/requires_nonexistent0.rb
new file mode 100644
index 0000000000..7e24b3916c
--- /dev/null
+++ b/activesupport/test/dependencies/requires_nonexistent0.rb
@@ -0,0 +1 @@
+require 'RMagickDontExistDude'
diff --git a/activesupport/test/dependencies/requires_nonexistent1.rb b/activesupport/test/dependencies/requires_nonexistent1.rb
new file mode 100644
index 0000000000..41e6668164
--- /dev/null
+++ b/activesupport/test/dependencies/requires_nonexistent1.rb
@@ -0,0 +1 @@
+require_dependency 'requires_nonexistent0'
diff --git a/activesupport/test/dependencies_test.rb b/activesupport/test/dependencies_test.rb
index 8b74bd4b93..5960425e8e 100644
--- a/activesupport/test/dependencies_test.rb
+++ b/activesupport/test/dependencies_test.rb
@@ -1,4 +1,5 @@
 require File.dirname(__FILE__) + '/abstract_unit'
+require 'pp'
 
 module ModuleWithMissing
   mattr_accessor :missing_count
@@ -17,11 +18,11 @@ class DependenciesTest < Test::Unit::TestCase
   def with_loading(*from)
     old_mechanism, Dependencies.mechanism = Dependencies.mechanism, :load
     dir = File.dirname(__FILE__)
-    prior_autoload_paths = Dependencies.autoload_paths
-    Dependencies.autoload_paths = from.collect { |f| "#{dir}/#{f}" }
+    prior_load_paths = Dependencies.load_paths
+    Dependencies.load_paths = from.collect { |f| "#{dir}/#{f}" }
     yield
   ensure
-    Dependencies.autoload_paths = prior_autoload_paths
+    Dependencies.load_paths = prior_load_paths
     Dependencies.mechanism = old_mechanism
   end
 
@@ -219,33 +220,33 @@ class DependenciesTest < Test::Unit::TestCase
     end
   end
   
-  def test_autoloadable_constants_for_path_should_handle_empty_autoloads
-    assert_equal [], Dependencies.autoloadable_constants_for_path('hello')
+  def test_loadable_constants_for_path_should_handle_empty_autoloads
+    assert_equal [], Dependencies.loadable_constants_for_path('hello')
   end
   
-  def test_autoloadable_constants_for_path_should_handle_relative_paths
+  def test_loadable_constants_for_path_should_handle_relative_paths
     fake_root = 'dependencies'
     relative_root = File.dirname(__FILE__) + '/dependencies'
     ['', '/'].each do |suffix|
       with_loading fake_root + suffix do
-        assert_equal ["A::B"], Dependencies.autoloadable_constants_for_path(relative_root + '/a/b')
+        assert_equal ["A::B"], Dependencies.loadable_constants_for_path(relative_root + '/a/b')
       end
     end
   end
   
-  def test_autoloadable_constants_for_path_should_provide_all_results
+  def test_loadable_constants_for_path_should_provide_all_results
     fake_root = '/usr/apps/backpack'
     with_loading fake_root, fake_root + '/lib' do
-      root = Dependencies.autoload_paths.first
-      assert_equal ["Lib::A::B", "A::B"], Dependencies.autoloadable_constants_for_path(root + '/lib/a/b')
+      root = Dependencies.load_paths.first
+      assert_equal ["Lib::A::B", "A::B"], Dependencies.loadable_constants_for_path(root + '/lib/a/b')
     end
   end
   
-  def test_autoloadable_constants_for_path_should_uniq_results
+  def test_loadable_constants_for_path_should_uniq_results
     fake_root = '/usr/apps/backpack/lib'
     with_loading fake_root, fake_root + '/' do
-      root = Dependencies.autoload_paths.first
-      assert_equal ["A::B"], Dependencies.autoloadable_constants_for_path(root + '/a/b')
+      root = Dependencies.load_paths.first
+      assert_equal ["A::B"], Dependencies.loadable_constants_for_path(root + '/a/b')
     end
   end
   
@@ -300,28 +301,28 @@ class DependenciesTest < Test::Unit::TestCase
   
   def test_file_search
     with_loading 'dependencies' do
-      root = Dependencies.autoload_paths.first
-      assert_equal nil, Dependencies.search_for_autoload_file('service_three')
-      assert_equal nil, Dependencies.search_for_autoload_file('service_three.rb')
-      assert_equal root + '/service_one.rb', Dependencies.search_for_autoload_file('service_one')
-      assert_equal root + '/service_one.rb', Dependencies.search_for_autoload_file('service_one.rb')
+      root = Dependencies.load_paths.first
+      assert_equal nil, Dependencies.search_for_file('service_three')
+      assert_equal nil, Dependencies.search_for_file('service_three.rb')
+      assert_equal root + '/service_one.rb', Dependencies.search_for_file('service_one')
+      assert_equal root + '/service_one.rb', Dependencies.search_for_file('service_one.rb')
     end
   end
   
-  def test_file_search_uses_first_in_autoload_path
+  def test_file_search_uses_first_in_load_path
     with_loading 'dependencies', 'autoloading_fixtures' do
-      deps, autoload = Dependencies.autoload_paths
+      deps, autoload = Dependencies.load_paths
       assert_match %r/dependencies/, deps
       assert_match %r/autoloading_fixtures/, autoload
       
-      assert_equal deps + '/conflict.rb', Dependencies.search_for_autoload_file('conflict')
+      assert_equal deps + '/conflict.rb', Dependencies.search_for_file('conflict')
     end
     with_loading 'autoloading_fixtures', 'dependencies' do
-      autoload, deps = Dependencies.autoload_paths
+      autoload, deps = Dependencies.load_paths
       assert_match %r/dependencies/, deps
       assert_match %r/autoloading_fixtures/, autoload
       
-      assert_equal autoload + '/conflict.rb', Dependencies.search_for_autoload_file('conflict')
+      assert_equal autoload + '/conflict.rb', Dependencies.search_for_file('conflict')
     end
     
   end
@@ -374,7 +375,7 @@ class DependenciesTest < Test::Unit::TestCase
   
   def test_removal_from_tree_should_be_detected
     with_loading 'dependencies' do
-      root = Dependencies.autoload_paths.first
+      root = Dependencies.load_paths.first
       c = ServiceOne
       Dependencies.clear
       assert ! defined?(ServiceOne)
@@ -395,4 +396,20 @@ class DependenciesTest < Test::Unit::TestCase
     end
   end
   
+  def test_load_once_paths_do_not_add_to_autoloaded_constants
+    with_loading 'autoloading_fixtures' do
+      Dependencies.load_once_paths = Dependencies.load_paths.dup
+      
+      assert ! Dependencies.autoloaded?("ModuleFolder")
+      assert ! Dependencies.autoloaded?("ModuleFolder::NestedClass")
+      assert ! Dependencies.autoloaded?(ModuleFolder)
+      
+      ModuleFolder::NestedClass
+      assert ! Dependencies.autoloaded?(ModuleFolder::NestedClass)
+    end
+  ensure
+    Object.send(:remove_const, :ModuleFolder) if defined?(ModuleFolder)
+    Dependencies.load_once_paths = []
+  end
+  
 end