Refactor new callbacks and AR implementation.

Signed-off-by: Joshua Peek <josh@joshpeek.com>

8 files changed, 50 insertions, 91 deletions

diff --git a/activerecord/lib/active_record/callbacks.rb b/activerecord/lib/active_record/callbacks.rb
index 361c7b2ef4..40a25811c4 100644
--- a/activerecord/lib/active_record/callbacks.rb
+++ b/activerecord/lib/active_record/callbacks.rb
@@ -214,8 +214,9 @@ module ActiveRecord
 
     CALLBACKS = [
       :after_initialize, :after_find, :before_validation, :after_validation,
-      :before_save, :after_save, :before_create, :after_create, :before_update,
-      :after_update, :before_destroy, :after_destroy
+      :before_save, :around_save, :after_save, :before_create, :around_create,
+      :after_create, :before_update, :around_update, :after_update,
+      :before_destroy, :around_destroy, :after_destroy
     ]
 
     included do
@@ -223,7 +224,8 @@ module ActiveRecord
         alias_method_chain method, :callbacks
       end
 
-      define_callbacks :initialize, :find, :save, :create, :update, :destroy, :validation, "result == false"
+      define_callbacks :initialize, :find, :save, :create, :update, :destroy,
+                       :validation, :terminator => "result == false", :scope => [:kind, :name]
     end
 
     module ClassMethods
@@ -239,64 +241,23 @@ module ActiveRecord
         set_callback(:find, :after, *(args << options), &block)
       end
 
-      def before_save(*args, &block)
-        set_callback(:save, :before, *args, &block)
-      end
-
-      def around_save(*args, &block)
-        set_callback(:save, :around, *args, &block)
-      end
-
-      def after_save(*args, &block)
-        options = args.extract_options!
-        options[:prepend] = true
-        options[:if] = Array(options[:if]) << "!halted && value != false"
-        set_callback(:save, :after, *(args << options), &block)
-      end
-
-      def before_create(*args, &block)
-        set_callback(:create, :before, *args, &block)
-      end
-
-      def around_create(*args, &block)
-        set_callback(:create, :around, *args, &block)
-      end
-
-      def after_create(*args, &block)
-        options = args.extract_options!
-        options[:prepend] = true
-        options[:if] = Array(options[:if]) << "!halted && value != false"
-        set_callback(:create, :after, *(args << options), &block)
-      end
-
-      def before_update(*args, &block)
-        set_callback(:update, :before, *args, &block)
-      end
-
-      def around_update(*args, &block)
-        set_callback(:update, :around, *args, &block)
-      end
-
-      def after_update(*args, &block)
-        options = args.extract_options!
-        options[:prepend] = true
-        options[:if] = Array(options[:if]) << "!halted && value != false"
-        set_callback(:update, :after, *(args << options), &block)
-      end
+      [:save, :create, :update, :destroy].each do |callback|
+        module_eval <<-CALLBACKS, __FILE__, __LINE__
+          def before_#{callback}(*args, &block)
+            set_callback(:#{callback}, :before, *args, &block)
+          end
 
-      def before_destroy(*args, &block)
-        set_callback(:destroy, :before, *args, &block)
-      end
+          def around_#{callback}(*args, &block)
+            set_callback(:#{callback}, :around, *args, &block)
+          end
 
-      def around_destroy(*args, &block)
-        set_callback(:destroy, :around, *args, &block)
-      end
-
-      def after_destroy(*args, &block)
-        options = args.extract_options!
-        options[:prepend] = true
-        options[:if] = Array(options[:if]) << "!halted && value != false"
-        set_callback(:destroy, :after, *(args << options), &block)
+          def after_#{callback}(*args, &block)
+            options = args.extract_options!
+            options[:prepend] = true
+            options[:if] = Array(options[:if]) << "!halted && value != false"
+            set_callback(:#{callback}, :after, *(args << options), &block)
+          end
+        CALLBACKS
       end
 
       def before_validation(*args, &block)
diff --git a/activerecord/test/cases/callbacks_observers_test.rb b/activerecord/test/cases/callbacks_observers_test.rb
index 87de524923..52ce384844 100644
--- a/activerecord/test/cases/callbacks_observers_test.rb
+++ b/activerecord/test/cases/callbacks_observers_test.rb
@@ -5,7 +5,7 @@ class Comment < ActiveRecord::Base
 
   before_validation :record_callers
 
-  def after_validation
+  after_validation do
     record_callers
   end
 
@@ -32,7 +32,6 @@ class CallbacksObserversTest < ActiveRecord::TestCase
     CommentObserver.instance.callers = callers
 
     comment.valid?
-
     assert_equal [Comment, Comment, CommentObserver], callers, "model callbacks did not fire before observers were notified"
   end
 end
diff --git a/activerecord/test/cases/callbacks_test.rb b/activerecord/test/cases/callbacks_test.rb
index 092522b441..5a084a611e 100644
--- a/activerecord/test/cases/callbacks_test.rb
+++ b/activerecord/test/cases/callbacks_test.rb
@@ -18,11 +18,10 @@ class CallbackDeveloper < ActiveRecord::Base
       end
     end
 
-    def callback_object(callback_symbol)
+    def callback_object(callback_method)
       klass = Class.new
-      callback_method = callback_symbol.to_s.split('_').first.to_sym
       klass.send(:define_method, callback_method) do |model|
-        model.history << [callback_symbol, :object]
+        model.history << [callback_method, :object]
       end
       klass.new
     end
@@ -30,6 +29,7 @@ class CallbackDeveloper < ActiveRecord::Base
 
   ActiveSupport::Deprecation.silence do
     ActiveRecord::Callbacks::CALLBACKS.each do |callback_method|
+      next if callback_method.to_s =~ /^around_/
       define_callback_method(callback_method)
       send(callback_method, callback_string(callback_method))
       send(callback_method, callback_proc(callback_method))
diff --git a/activerecord/test/cases/lifecycle_test.rb b/activerecord/test/cases/lifecycle_test.rb
index ebf2e87cd5..aa7ce2ecb6 100644
--- a/activerecord/test/cases/lifecycle_test.rb
+++ b/activerecord/test/cases/lifecycle_test.rb
@@ -4,8 +4,6 @@ require 'models/developer'
 require 'models/reply'
 require 'models/minimalistic'
 
-class Topic; def after_find() end end
-class Developer; def after_find() end end
 class SpecialDeveloper < Developer; end
 
 class TopicManualObserver
@@ -164,22 +162,6 @@ class LifecycleTest < ActiveRecord::TestCase
     assert_equal topic, observer.topic
   end
 
-  def test_after_find_is_not_clobbered_if_it_already_exists
-    # use a fresh observer class so we can instantiate it (Observer is
-    # a Singleton)
-    model_class = Class.new(ActiveRecord::Base) do
-      def after_find; end
-    end
-    original_method = model_class.instance_method(:after_find)
-    observer_class = Class.new(ActiveRecord::Observer) do
-      def after_find; end
-    end
-    observer_class.observe(model_class)
-
-    observer = observer_class.instance
-    assert_equal original_method, model_class.instance_method(:after_find)
-  end
-
   def test_invalid_observer
     assert_raise(ArgumentError) { Topic.observers = Object.new; Topic.instantiate_observers }
   end
diff --git a/activerecord/test/cases/transactions_test.rb b/activerecord/test/cases/transactions_test.rb
index 66baf1008a..aca70b4238 100644
--- a/activerecord/test/cases/transactions_test.rb
+++ b/activerecord/test/cases/transactions_test.rb
@@ -382,28 +382,28 @@ class TransactionTest < ActiveRecord::TestCase
 
   private
     def add_exception_raising_after_save_callback_to_topic
-        Topic.class_eval "def after_save; raise 'Make the transaction rollback' end"
+      Topic.class_eval "def after_save_for_transaction; raise 'Make the transaction rollback' end"
     end
 
     def remove_exception_raising_after_save_callback_to_topic
-      Topic.class_eval "def after_save; end"
+      Topic.class_eval "def after_save_for_transaction; end"
     end
 
     def add_exception_raising_after_create_callback_to_topic
-        Topic.class_eval "def after_create; raise 'Make the transaction rollback' end"
+      Topic.class_eval "def after_create_for_transaction; raise 'Make the transaction rollback' end"
     end
 
     def remove_exception_raising_after_create_callback_to_topic
-      Topic.class_eval "def after_create; end"
+      Topic.class_eval "def after_create_for_transaction; end"
     end
 
     %w(validation save destroy).each do |filter|
       define_method("add_cancelling_before_#{filter}_with_db_side_effect_to_topic") do
-        Topic.class_eval "def before_#{filter}() Book.create; false end"
+        Topic.class_eval "def before_#{filter}_for_transaction() Book.create; false end"
       end
 
       define_method("remove_cancelling_before_#{filter}_with_db_side_effect_to_topic") do
-        Topic.class_eval "def before_#{filter}; end"
+        Topic.class_eval "def before_#{filter}_for_transaction; end"
       end
     end
 end
diff --git a/activerecord/test/models/author.rb b/activerecord/test/models/author.rb
index f264f980d6..7cbc6e803f 100644
--- a/activerecord/test/models/author.rb
+++ b/activerecord/test/models/author.rb
@@ -94,8 +94,9 @@ class Author < ActiveRecord::Base
   belongs_to :author_address_extra, :dependent => :delete, :class_name => "AuthorAddress"
 
   attr_accessor :post_log
+  after_initialize :set_post_log
 
-  def after_initialize
+  def set_post_log
     @post_log = []
   end
 
diff --git a/activerecord/test/models/project.rb b/activerecord/test/models/project.rb
index 422b12dc83..416032cb75 100644
--- a/activerecord/test/models/project.rb
+++ b/activerecord/test/models/project.rb
@@ -22,8 +22,9 @@ class Project < ActiveRecord::Base
   has_and_belongs_to_many :well_payed_salary_groups, :class_name => "Developer", :group => "developers.salary", :having => "SUM(salary) > 10000", :select => "SUM(salary) as salary"
 
   attr_accessor :developers_log
+  after_initialize :set_developers_log
 
-  def after_initialize
+  def set_developers_log
     @developers_log = []
   end
 
diff --git a/activerecord/test/models/topic.rb b/activerecord/test/models/topic.rb
index c16a6f2be9..baca4972cb 100644
--- a/activerecord/test/models/topic.rb
+++ b/activerecord/test/models/topic.rb
@@ -52,6 +52,15 @@ class Topic < ActiveRecord::Base
     id
   end
 
+  before_validation :before_validation_for_transaction
+  before_save :before_save_for_transaction
+  before_destroy :before_destroy_for_transaction
+
+  after_save :after_save_for_transaction
+  after_create :after_create_for_transaction
+
+  after_initialize :set_email_address
+
   protected
     def approved=(val)
       @custom_approved = val
@@ -66,11 +75,17 @@ class Topic < ActiveRecord::Base
       self.class.delete_all "parent_id = #{id}"
     end
 
-    def after_initialize
+    def set_email_address
       if self.new_record?
         self.author_email_address = 'test@test.com'
       end
     end
+
+    def before_validation_for_transaction; end
+    def before_save_for_transaction; end
+    def before_destroy_for_transaction; end
+    def after_save_for_transaction; end
+    def after_create_for_transaction; end
 end
 
 module Web