r4738@asus: jeremy | 2006-06-29 20:18:43 -0700

 Observers also watch subclasses created after they are declared. Closes #5535.


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

3 files changed, 96 insertions, 49 deletions

diff --git a/activerecord/CHANGELOG b/activerecord/CHANGELOG
index 2c71b3ce7c..855192e189 100644
--- a/activerecord/CHANGELOG
+++ b/activerecord/CHANGELOG
@@ -1,5 +1,7 @@
 *SVN*
 
+* Observers also watch subclasses created after they are declared. #5535 [daniels@pronto.com.au]
+
 * Removed deprecated timestamps_gmt class methods. [Jeremy Kemper]
 
 * rake build_mysql_database grants permissions to rails@localhost. #5501 [brianegge@yahoo.com]
diff --git a/activerecord/lib/active_record/observer.rb b/activerecord/lib/active_record/observer.rb
index 8fe71f82f5..a2030b84b3 100644
--- a/activerecord/lib/active_record/observer.rb
+++ b/activerecord/lib/active_record/observer.rb
@@ -1,4 +1,5 @@
 require 'singleton'
+require 'set'
 
 module ActiveRecord
   module Observing # :nodoc:
@@ -12,18 +13,30 @@ module ActiveRecord
       #   # Calls PersonObserver.instance
       #   ActiveRecord::Base.observers = :person_observer
       #
-      #   # Calls Cacher.instance and GarbageCollector.instance 
+      #   # Calls Cacher.instance and GarbageCollector.instance
       #   ActiveRecord::Base.observers = :cacher, :garbage_collector
       #
       #   # Same as above, just using explicit class references
       #   ActiveRecord::Base.observers = Cacher, GarbageCollector
       def observers=(*observers)
-        observers = [ observers ].flatten.each do |observer| 
-          observer.is_a?(Symbol) ? 
-            observer.to_s.camelize.constantize.instance :
+        observers.flatten.each do |observer|
+          if observer.respond_to?(:to_sym) # Symbol or String
+            observer.to_s.camelize.constantize.instance
+          elsif observer.respond_to?(:instance)
             observer.instance
+          else
+            raise ArgumentError, "#{observer} must be a lowercase, underscored class name (or an instance of the class itself) responding to the instance method. Example: Person.observers = :big_brother # calls BigBrother.instance"
+          end
         end
       end
+
+      protected
+        # Notify observers when the observed class is subclassed.
+        def inherited(subclass)
+          super
+          changed
+          notify_observers :observed_class_inherited, subclass
+        end
     end
   end
 
@@ -84,12 +97,12 @@ module ActiveRecord
   # The observer can implement callback methods for each of the methods described in the Callbacks module.
   #
   # == Storing Observers in Rails
-  # 
+  #
   # If you're using Active Record within Rails, observer classes are usually stored in app/models with the
   # naming convention of app/models/audit_observer.rb.
   #
   # == Configuration
-  # 
+  #
   # In order to activate an observer, list it in the <tt>config.active_record.observers</tt> configuration setting in your
   # <tt>config/environment.rb</tt> file.
   #
@@ -103,36 +116,49 @@ module ActiveRecord
     # Observer subclasses should be reloaded by the dispatcher in Rails
     # when Dependencies.mechanism = :load.
     include Reloadable::Subclasses
-    
-    # Attaches the observer to the supplied model classes.
-    def self.observe(*models)
-      define_method(:observed_class) { models }
+
+    class << self
+      # Attaches the observer to the supplied model classes.
+      def observe(*models)
+        define_method(:observed_classes) { Set.new(models) }
+      end
+
+      # The class observed by default is inferred from the observer's class name:
+      #   assert_equal [Person], PersonObserver.observed_class
+      def observed_class
+        name.scan(/(.*)Observer/)[0][0].constantize
+      end
     end
 
+    # Start observing the declared classes and their subclasses.
     def initialize
-      observed_classes = [ observed_class ].flatten
-      observed_subclasses_class = observed_classes.collect {|c| c.send(:subclasses) }.flatten!
-      (observed_classes + observed_subclasses_class).each do |klass| 
-        klass.add_observer(self)
-        klass.send(:define_method, :after_find) unless klass.respond_to?(:after_find)
-      end
+      Set.new(observed_classes + observed_subclasses).each { |klass| add_observer! klass }
     end
-  
-    def update(callback_method, object) #:nodoc:
-      send(callback_method, object) if respond_to?(callback_method)
+
+    # Send observed_method(object) if the method exists.
+    def update(observed_method, object) #:nodoc:
+      send(observed_method, object) if respond_to?(observed_method)
     end
-    
-    private
-      def observed_class
-        if self.class.respond_to? "observed_class"
-          self.class.observed_class
-        else
-          Object.const_get(infer_observed_class_name)
-        end
+
+    # Special method sent by the observed class when it is inherited.
+    # Passes the new subclass.
+    def observed_class_inherited(subclass) #:nodoc:
+      self.class.observe(observed_classes + [subclass])
+      add_observer!(subclass)
+    end
+
+    protected
+      def observed_classes
+        Set.new([self.class.observed_class].flatten)
+      end
+
+      def observed_subclasses
+        observed_classes.sum(&:subclasses)
       end
-      
-      def infer_observed_class_name
-        self.class.name.scan(/(.*)Observer/)[0][0]
+
+      def add_observer!(klass)
+        klass.add_observer(self)
+        klass.class_eval 'def after_find() end' unless klass.respond_to?(:after_find)
       end
   end
 end
diff --git a/activerecord/test/lifecycle_test.rb b/activerecord/test/lifecycle_test.rb
index ddac6f7c4c..b411524cff 100755
--- a/activerecord/test/lifecycle_test.rb
+++ b/activerecord/test/lifecycle_test.rb
@@ -46,13 +46,22 @@ end
 
 class MultiObserver < ActiveRecord::Observer
   attr_reader :record
-  
+
   def self.observed_class() [ Topic, Developer ] end
 
+  cattr_reader :last_inherited
+  @@last_inherited = nil
+
+  def observed_class_inherited_with_testing(subclass)
+    observed_class_inherited_without_testing(subclass)
+    @@last_inherited = subclass
+  end
+
+  alias_method_chain :observed_class_inherited, :testing
+
   def after_find(record)
     @record = record
   end
-
 end
 
 class LifecycleTest < Test::Unit::TestCase
@@ -63,54 +72,64 @@ class LifecycleTest < Test::Unit::TestCase
     Topic.find(1).destroy
     assert_equal 0, Topic.count
   end
-  
+
   def test_after_save
     ActiveRecord::Base.observers = :topic_manual_observer
 
     topic = Topic.find(1)
     topic.title = "hello"
     topic.save
-    
+
     assert TopicManualObserver.instance.has_been_notified?
     assert_equal :after_save, TopicManualObserver.instance.callbacks.last["callback_method"]
   end
-  
+
   def test_observer_update_on_save
     ActiveRecord::Base.observers = TopicManualObserver
 
-    topic = Topic.find(1)    
+    topic = Topic.find(1)
     assert TopicManualObserver.instance.has_been_notified?
     assert_equal :after_find, TopicManualObserver.instance.callbacks.first["callback_method"]
   end
-  
+
   def test_auto_observer
     topic_observer = TopicaObserver.instance
 
-    topic = Topic.find(1)    
-    assert_equal topic_observer.topic.title, topic.title
+    topic = Topic.find(1)
+    assert_equal topic.title, topic_observer.topic.title
   end
-  
-  def test_infered_auto_observer
+
+  def test_inferred_auto_observer
     topic_observer = TopicObserver.instance
 
-    topic = Topic.find(1)    
-    assert_equal topic_observer.topic.title, topic.title
+    topic = Topic.find(1)
+    assert_equal topic.title, topic_observer.topic.title
   end
-  
+
   def test_observing_two_classes
     multi_observer = MultiObserver.instance
 
     topic = Topic.find(1)
-    assert_equal multi_observer.record.title, topic.title
+    assert_equal topic.title, multi_observer.record.title
 
-    developer = Developer.find(1)    
-    assert_equal multi_observer.record.name, developer.name
+    developer = Developer.find(1)
+    assert_equal developer.name, multi_observer.record.name
   end
-  
+
   def test_observing_subclasses
     multi_observer = MultiObserver.instance
 
     developer = SpecialDeveloper.find(1)
-    assert_equal multi_observer.record.name, developer.name
+    assert_equal developer.name, multi_observer.record.name
+
+    klass = Class.new(Developer)
+    assert_equal klass, multi_observer.last_inherited
+
+    developer = klass.find(1)
+    assert_equal developer.name, multi_observer.record.name
+  end
+
+  def test_invalid_observer
+    assert_raise(ArgumentError) { Topic.observers = Object.new }
   end
 end