Enable warnings on first load only. File which are loaded but raise an exception are not added to loaded set.

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

5 files changed, 91 insertions, 18 deletions

diff --git a/activesupport/CHANGELOG b/activesupport/CHANGELOG
index e8ef43acc2..5731d94911 100644
--- a/activesupport/CHANGELOG
+++ b/activesupport/CHANGELOG
@@ -2,7 +2,7 @@
 
 * Active Support is warnings-safe.  #1792 [Eric Hodel]
 
-* Introduce enable_warnings counterpart to silence_warnings.  Turn warnings on when loading a file if Dependencies.mechanism == :load.  Common mistakes such as redefined methods will print warnings to stderr.  [Jeremy Kemper]
+* Introduce enable_warnings counterpart to silence_warnings.  Turn warnings on when loading a file for the first time if Dependencies.mechanism == :load.  Common mistakes such as redefined methods will print warnings to stderr.  [Jeremy Kemper]
 
 * Add Symbol#to_proc, which allows for, e.g. [:foo, :bar].map(&:to_s). [Marcel Molina Jr.]
 
diff --git a/activesupport/lib/active_support/dependencies.rb b/activesupport/lib/active_support/dependencies.rb
index b0aca1ee97..ed32334240 100644
--- a/activesupport/lib/active_support/dependencies.rb
+++ b/activesupport/lib/active_support/dependencies.rb
@@ -1,3 +1,4 @@
+require 'set'
 require File.dirname(__FILE__) + '/module_attribute_accessors'
 require File.dirname(__FILE__) + '/core_ext/load_error'
 require File.dirname(__FILE__) + '/core_ext/kernel'
@@ -5,20 +6,24 @@ require File.dirname(__FILE__) + '/core_ext/kernel'
 module Dependencies #:nodoc:
   extend self
 
-  @@loaded = [ ]
+  # All files ever loaded.
+  mattr_accessor :history
+  self.history = Set.new
+
+  # All files currently loaded.
   mattr_accessor :loaded
+  self.loaded = Set.new
 
-  @@mechanism = :load
+  # Should we load files or require them?
   mattr_accessor :mechanism
-  
+  self.mechanism = :load
+
   def load?
     mechanism == :load
   end
-  
+
   def depend_on(file_name, swallow_load_errors = false)
     unless loaded.include?(file_name)
-      loaded << file_name
-
       begin
         require_or_load(file_name)
       rescue LoadError
@@ -30,18 +35,29 @@ module Dependencies #:nodoc:
   def associate_with(file_name)
     depend_on(file_name, true)
   end
-  
+
   def clear
-    self.loaded = [ ]
+    loaded.clear
   end
 
   def require_or_load(file_name)
-    file_name = "#{file_name}.rb" unless ! load? || file_name[-3..-1] == '.rb'
     if load?
-      enable_warnings { load file_name }
+      # Append .rb if we have a bare file name.
+      load_file_name = "#{file_name}.rb" unless file_name[-3..-1] == '.rb'
+
+      # Enable warnings iff this file has not been loaded before.
+      if history.include?(file_name)
+        load load_file_name
+      else
+        enable_warnings { load load_file_name }
+      end
     else
       require file_name
     end
+
+    # Record that we've seen this file.
+    loaded  << file_name
+    history << file_name
   end
 
   def remove_subclasses_for(*classes)
diff --git a/activesupport/test/dependencies/check_warnings.rb b/activesupport/test/dependencies/check_warnings.rb
new file mode 100644
index 0000000000..03c3dca1d6
--- /dev/null
+++ b/activesupport/test/dependencies/check_warnings.rb
@@ -0,0 +1,2 @@
+$check_warnings_load_count += 1
+$checked_verbose = $VERBOSE
diff --git a/activesupport/test/dependencies/raises_exception.rb b/activesupport/test/dependencies/raises_exception.rb
new file mode 100644
index 0000000000..df8d8b79d0
--- /dev/null
+++ b/activesupport/test/dependencies/raises_exception.rb
@@ -0,0 +1,2 @@
+$raises_exception_load_count += 1
+raise 'Loading me failed, so do not add to loaded or history.'
diff --git a/activesupport/test/dependencies_test.rb b/activesupport/test/dependencies_test.rb
index 767795f6ac..a59b0284a9 100644
--- a/activesupport/test/dependencies_test.rb
+++ b/activesupport/test/dependencies_test.rb
@@ -7,23 +7,76 @@ class DependenciesTest < Test::Unit::TestCase
     Dependencies.clear
   end
 
-  def test_require_dependency
+  def test_tracking_loaded_files
     require_dependency(File.dirname(__FILE__) + "/dependencies/service_one")
     require_dependency(File.dirname(__FILE__) + "/dependencies/service_two")
     assert_equal 2, Dependencies.loaded.size
   end
-  
-  def test_require_dependency_two_times
+
+  def test_tracking_identical_loaded_files
     require_dependency(File.dirname(__FILE__) + "/dependencies/service_one")
     require_dependency(File.dirname(__FILE__) + "/dependencies/service_one")
     assert_equal 1, Dependencies.loaded.size
   end
 
-  def test_require_missing_dependency
+  def test_missing_dependency_raises_missing_source_file
     assert_raises(MissingSourceFile) { require_dependency("missing_service") }
   end
-  
-  def test_require_missing_association
+
+  def test_missing_association_raises_nothing
     assert_nothing_raised { require_association("missing_model") }
   end
-end
\ No newline at end of file
+
+  def test_dependency_which_raises_exception_isnt_added_to_loaded_set
+    old_mechanism, Dependencies.mechanism = Dependencies.mechanism, :load
+
+    filename = "#{File.dirname(__FILE__)}/dependencies/raises_exception"
+    $raises_exception_load_count = 0
+
+    5.times do |count|
+      assert_raises(RuntimeError) { require_dependency filename }
+      assert_equal count + 1, $raises_exception_load_count
+
+      assert !Dependencies.loaded.include?(filename)
+      assert !Dependencies.history.include?(filename)
+    end
+  ensure
+    Dependencies.mechanism = old_mechanism
+  end
+
+  def test_warnings_should_be_enabled_on_first_load
+    old_mechanism, Dependencies.mechanism = Dependencies.mechanism, :load
+
+    filename = "#{File.dirname(__FILE__)}/dependencies/check_warnings"
+    $check_warnings_load_count = 0
+
+    assert !Dependencies.loaded.include?(filename)
+    assert !Dependencies.history.include?(filename)
+
+    silence_warnings { require_dependency filename }
+    assert_equal 1, $check_warnings_load_count
+    assert_equal true, $checked_verbose, 'On first load warnings should be enabled.'
+
+    assert Dependencies.loaded.include?(filename)
+    Dependencies.clear
+    assert !Dependencies.loaded.include?(filename)
+    assert Dependencies.history.include?(filename)
+
+    silence_warnings { require_dependency filename }
+    assert_equal 2, $check_warnings_load_count
+    assert_equal nil, $checked_verbose, 'After first load warnings should be left alone.'
+
+    assert Dependencies.loaded.include?(filename)
+    Dependencies.clear
+    assert !Dependencies.loaded.include?(filename)
+    assert Dependencies.history.include?(filename)
+
+    enable_warnings { require_dependency filename }
+    assert_equal 3, $check_warnings_load_count
+    assert_equal true, $checked_verbose, 'After first load warnings should be left alone.'
+
+    assert Dependencies.loaded.include?(filename)
+  ensure
+    Dependencies.mechanism = old_mechanism
+  end
+end