improve how ActiveRecord::Observer defines callbacks on observed models

Instead of using a single `notify_observers` call for every callback type,
each observer now registers a unique callback for itself. Example:

  before_save :_notify_user_observer_for_before_save

  def _notify_user_observer_for_before_save
    observer.update(:before_save, self)
  end

Benefit: "before" callbacks halt when `observer.update` returns false.
This way, ActiveRecord observers can prevent records from saving.

[#4087 state:committed]

Signed-off-by: Jeremy Kemper <jeremy@bitsweat.net>

2 files changed, 24 insertions, 7 deletions

diff --git a/activerecord/CHANGELOG b/activerecord/CHANGELOG
index 8ecef6574f..c0c4df5035 100644
--- a/activerecord/CHANGELOG
+++ b/activerecord/CHANGELOG
@@ -1,3 +1,8 @@
+*Rails 3.0.0 [beta 4/release candidate] (unreleased)*
+
+* Observers can prevent records from saving by returning false, just like before_save and friends.  #4087 [Mislav Marohnić]
+
+
 *Rails 3.0.0 [beta 3] (April 13th, 2010)*
 
 * Add Relation extensions. [Pratik Naik]
diff --git a/activerecord/lib/active_record/observer.rb b/activerecord/lib/active_record/observer.rb
index 0fbb1f0261..ed0f039597 100644
--- a/activerecord/lib/active_record/observer.rb
+++ b/activerecord/lib/active_record/observer.rb
@@ -96,7 +96,8 @@ module ActiveRecord
     end
 
     def self.method_added(method)
-      self.observed_methods += [method] if ActiveRecord::Callbacks::CALLBACKS.include?(method.to_sym)
+      method = method.to_sym
+      observed_methods << method if ActiveRecord::Callbacks::CALLBACKS.include?(method)
     end
 
     protected
@@ -104,16 +105,27 @@ module ActiveRecord
         observed_classes.sum([]) { |klass| klass.send(:subclasses) }
       end
 
+      def observe_callbacks?
+        self.class.observed_methods.any?
+      end
+
       def add_observer!(klass)
         super
+        define_callbacks klass if observe_callbacks?
+      end
+
+      def define_callbacks(klass)
+        existing_methods = klass.instance_methods.map(&:to_sym)
+        observer = self
+        observer_name = observer.class.name.underscore.gsub('/', '__')
 
-        # Check if a notifier callback was already added to the given class. If
-        # it was not, add it.
         self.class.observed_methods.each do |method|
-          callback = :"_notify_observers_for_#{method}"
-          if (klass.instance_methods & [callback, callback.to_s]).empty?
-            klass.class_eval "def #{callback}; notify_observers(:#{method}); end"
-            klass.send(method, callback)
+          callback = :"_notify_#{observer_name}_for_#{method}"
+          unless existing_methods.include? callback
+            klass.send(:define_method, callback) do  # def _notify_user_observer_for_before_save
+              observer.update(method, self)          #   observer.update(:before_save, self)
+            end                                      # end
+            klass.send(method, callback)             # before_save :_notify_user_observer_for_before_save
           end
         end
       end