Further improvements to reloading code

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

10 files changed, 87 insertions, 44 deletions

diff --git a/actionpack/lib/action_controller/caching.rb b/actionpack/lib/action_controller/caching.rb
index df25fed3b2..e62fe11a19 100644
--- a/actionpack/lib/action_controller/caching.rb
+++ b/actionpack/lib/action_controller/caching.rb
@@ -523,8 +523,8 @@ module ActionController #:nodoc:
 
         # ActiveRecord::Observer will mark this class as reloadable even though it should not be.
         # However, subclasses of ActionController::Caching::Sweeper should be Reloadable
-        def self.included_modules
-          self == Sweeper ? super() - [ Reloadable ] : super()
+        def self.reloadable? #:nodoc:
+          self != Sweeper
         end
 
         def before(controller)
diff --git a/actionpack/lib/action_controller/session/active_record_store.rb b/actionpack/lib/action_controller/session/active_record_store.rb
index 02384021d8..7642320747 100644
--- a/actionpack/lib/action_controller/session/active_record_store.rb
+++ b/actionpack/lib/action_controller/session/active_record_store.rb
@@ -66,11 +66,6 @@ class CGI
 
         class << self
           # Don't try to reload ARStore::Session in dev mode.
-          def included_modules
-            super() - [ Reloadable ]
-          end
-
-          # Don't try to reload ARStore::Session in dev mode.
           def reloadable? #:nodoc:
             false
           end
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
diff --git a/railties/lib/dispatcher.rb b/railties/lib/dispatcher.rb
index 7f03294860..eab8b3a1d8 100644
--- a/railties/lib/dispatcher.rb
+++ b/railties/lib/dispatcher.rb
@@ -50,10 +50,9 @@ class Dispatcher
     # mailers, and so forth. This allows them to be loaded again without having
     # to restart the server (WEBrick, FastCGI, etc.).
     def reset_application!
-      Controllers.clear!
       Dependencies.clear
       ActiveRecord::Base.reset_subclasses
-      Reloadable.remove_classes_including
+      Class.remove_classes(*Reloadable.reloadable_classes)
     end
 
     private