Further improvements to reloading code

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

7 files changed, 84 insertions, 35 deletions

diff --git a/activesupport/CHANGELOG b/activesupport/CHANGELOG
index d1e9c1c331..9fe54c2397 100644
--- a/activesupport/CHANGELOG
+++ b/activesupport/CHANGELOG
@@ -1,5 +1,13 @@
 *SVN*
 
+* Further improvements to reloading code [Nicholas Seckar, Trevor Squires]
+  
+  - All classes/modules which include Reloadable can define reloadable? for fine grained control of reloading
+  - Class.remove_class uses Module#parent to access the parent module
+  - Class.remove_class expanded to handle multiple classes in a single call
+  - LoadingModule.clear! has been removed as it is no longer required
+  - Module#remove_classes_including has been removed in favor of Reloadable.reloadable_classes
+
 * Added reusable reloading support through the inclusion of the Relodable module that all subclasses of ActiveRecord::Base, ActiveRecord::Observer, ActiveController::Base, and ActionMailer::Base automatically gets. This means that these classes will be reloaded by the dispatcher when Dependencies.mechanism = :load. You can make your own models reloadable easily:
 
     class Setting
diff --git a/activesupport/lib/active_support/core_ext/class/removal.rb b/activesupport/lib/active_support/core_ext/class/removal.rb
index 8fc4d728b3..468c77d81b 100644
--- a/activesupport/lib/active_support/core_ext/class/removal.rb
+++ b/activesupport/lib/active_support/core_ext/class/removal.rb
@@ -7,15 +7,11 @@ class Class #:nodoc:
     Object.subclasses_of(self).map { |o| o.to_s }
   end
 
-  def remove_class(klass)
-    if klass.to_s.include? "::"
-      modules     = klass.to_s.split("::")
-      final_klass = modules.pop
-              
-      final_module = modules.inject(Object) { |final_type, part| final_type.const_get(part) }
-      final_module.send(:remove_const, final_klass) rescue nil
-    else
-      Object.send(:remove_const, klass.to_s) rescue nil
+  def remove_class(*klasses)
+    klasses.each do |klass|
+      basename = klass.to_s.split("::").last
+      parent = klass.parent
+      parent.send :remove_const, basename unless parent == klass
     end
   end
 end
\ No newline at end of file
diff --git a/activesupport/lib/active_support/core_ext/module/inclusion.rb b/activesupport/lib/active_support/core_ext/module/inclusion.rb
index 124c4551bc..efc00d6f28 100644
--- a/activesupport/lib/active_support/core_ext/module/inclusion.rb
+++ b/activesupport/lib/active_support/core_ext/module/inclusion.rb
@@ -1,8 +1,4 @@
 class Module
-  def remove_classes_including
-    included_in_classes.each { |klass| Class.remove_class(klass) }
-  end
-  
   def included_in_classes
     classes = []
     ObjectSpace.each_object(Class) { |k| classes << k if k.included_modules.include?(self) }
diff --git a/activesupport/lib/active_support/dependencies.rb b/activesupport/lib/active_support/dependencies.rb
index 37a3f4acb3..7f73f9bf40 100644
--- a/activesupport/lib/active_support/dependencies.rb
+++ b/activesupport/lib/active_support/dependencies.rb
@@ -159,14 +159,6 @@ module Dependencies #:nodoc:
     def load_file!(file_path)
       require_dependency(file_path)
     end
-
-    # Erase all items in this module
-    def clear!
-      constants.each do |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
   end
   
   # This object defines a path from which Constants can be loaded.
diff --git a/activesupport/lib/active_support/reloadable.rb b/activesupport/lib/active_support/reloadable.rb
index fca397afeb..49e6442a37 100644
--- a/activesupport/lib/active_support/reloadable.rb
+++ b/activesupport/lib/active_support/reloadable.rb
@@ -1,4 +1,17 @@
 # Classes that include this module will automatically be reloaded
 # by the Rails dispatcher when Dependencies.mechanism = :load.
 module Reloadable
+  class << self
+    def included(base) #nodoc:
+      if base.is_a?(Class) && ! base.respond_to?(:reloadable?)
+        class << base
+          define_method(:reloadable?) { true }
+        end
+      end
+    end
+    
+    def reloadable_classes
+      included_in_classes.select { |klass| klass.reloadable? }
+    end
+  end
 end
\ No newline at end of file
diff --git a/activesupport/test/core_ext/module_test.rb b/activesupport/test/core_ext/module_test.rb
index 3d11f1001f..d6ea595ae2 100644
--- a/activesupport/test/core_ext/module_test.rb
+++ b/activesupport/test/core_ext/module_test.rb
@@ -34,18 +34,4 @@ class ModuleTest < Test::Unit::TestCase
     assert !One.included_in_classes.include?(De)
   end
 
-  def test_remove_classes_including
-    assert Ab.is_a?(Class)
-    assert Xy::Bc.is_a?(Class)
-    assert Yz::Zy::Cd.is_a?(Class)
-    assert De.is_a?(Class)
-
-    One.remove_classes_including
-
-    assert_raises(NameError) { Ae.is_a?(Class) }
-    assert_raises(NameError) { Xy::Bc.is_a?(Class) }
-    assert_raises(NameError) { Yz::Zy::Cd.is_a?(Class) }
-    
-    assert De.is_a?(Class)
-  end
 end
\ No newline at end of file
diff --git a/activesupport/test/reloadable_test.rb b/activesupport/test/reloadable_test.rb
new file mode 100644
index 0000000000..adac2fccec
--- /dev/null
+++ b/activesupport/test/reloadable_test.rb
@@ -0,0 +1,58 @@
+require 'test/unit'
+require File.dirname(__FILE__) + '/../lib/active_support/core_ext/class'
+require File.dirname(__FILE__) + '/../lib/active_support/core_ext/module'
+require File.dirname(__FILE__) + '/../lib/active_support/reloadable'
+
+module ReloadableTestSandbox
+  
+  module AModuleIncludingReloadable
+    include Reloadable
+  end
+  class AReloadableClass
+    include Reloadable
+  end
+  class AReloadableClassWithSubclasses
+    include Reloadable
+  end
+  class AReloadableSubclass < AReloadableClassWithSubclasses
+  end
+  class ANonReloadableSubclass < AReloadableClassWithSubclasses
+    def self.reloadable?
+      false
+    end
+  end
+  class AClassWhichDefinesItsOwnReloadable
+    def self.reloadable?
+      10
+    end
+    include Reloadable
+  end
+end
+
+class ReloadableTest < Test::Unit::TestCase
+  def test_modules_do_not_receive_reloadable_method
+    assert ! ReloadableTestSandbox::AModuleIncludingReloadable.respond_to?(:reloadable?)
+  end
+  def test_classes_receive_reloadable
+    assert ReloadableTestSandbox::AReloadableClass.respond_to?(:reloadable?)
+  end
+  def test_classes_inherit_reloadable
+    assert ReloadableTestSandbox::AReloadableSubclass.respond_to?(:reloadable?)
+  end
+  def test_reloadable_is_not_overwritten_if_present
+    assert_equal 10, ReloadableTestSandbox::AClassWhichDefinesItsOwnReloadable.reloadable?
+  end
+  
+  def test_removable_classes
+    reloadables = %w(AReloadableClass AReloadableClassWithSubclasses AReloadableSubclass AClassWhichDefinesItsOwnReloadable)
+    non_reloadables = %w(ANonReloadableSubclass AModuleIncludingReloadable)
+    
+    results = Reloadable.reloadable_classes
+    reloadables.each do |name|
+      assert results.include?(ReloadableTestSandbox.const_get(name)), "Expected #{name} to be reloadable"
+    end
+    non_reloadables.each do |name|
+      assert ! results.include?(ReloadableTestSandbox.const_get(name)), "Expected #{name} NOT to be reloadable"
+    end
+  end
+end
\ No newline at end of file