Skip to content
Browse files

Fix observer callbacks firing multiple times on descendant instances

  • Loading branch information...
1 parent 30a0e3f commit ee491b064bff126a67600d694511986b8636d47b @kamal kamal committed with tenderlove Feb 9, 2011
Showing with 22 additions and 1 deletion.
  1. +8 −1 activerecord/lib/active_record/observer.rb
  2. +14 −0 activerecord/test/cases/lifecycle_test.rb
View
9 activerecord/lib/active_record/observer.rb
@@ -108,10 +108,17 @@ def add_observer!(klass)
def define_callbacks(klass)
observer = self
+ observer_name = observer.class.name.underscore.gsub('/', '__')
ActiveRecord::Callbacks::CALLBACKS.each do |callback|
next unless respond_to?(callback)
- klass.send(callback){|record| observer.send(callback, record)}
+ callback_meth = :"_notify_#{observer_name}_for_#{callback}"
+ unless klass.respond_to?(callback_meth)
+ klass.send(:define_method, callback_meth) do
+ observer.send(callback, self)
+ end
+ klass.send(callback, callback_meth)
+ end
end
end
end
View
14 activerecord/test/cases/lifecycle_test.rb
@@ -7,6 +7,12 @@
class SpecialDeveloper < Developer; end
+class DeveloperObserver < ActiveRecord::Observer
+ def before_save(developer)
+ developer.salary += 1
+ end
+end
+
class SalaryChecker < ActiveRecord::Observer
observe :special_developer
attr_accessor :last_saved
@@ -195,4 +201,12 @@ def test_invalid_observer
assert_equal developer, SalaryChecker.instance.last_saved
end
+ test "callback observing the ancestor does not fire multiple times on descendent" do
+ DeveloperObserver.instance # activate
+ developer = Developer.create! :name => 'Ancestor', :salary => 100000
+ assert_equal 100001, developer.salary, 'ancestor callback fired multiple times'
+ developer = SpecialDeveloper.create! :name => 'Descendent', :salary => 100000
+ assert_equal 100001, developer.salary, 'descendent callback fired multiple times'
+ end
+
end

4 comments on commit ee491b0

@paneq
paneq commented on ee491b0 Feb 23, 2011

Why was this fireing twice on descendants ? I think the callbacks are copied right ? Maybe then they should not be copied ?

@tenderlove
Ruby on Rails member

I'm not sure. I'm looking in to it as I don't really like observer code. However, this is a bug so I had to apply. :-(

@paneq
paneq commented on ee491b0 Feb 23, 2011

I'm not saying that it was bad to apply, quite contrary, good job on catching and fixing it guys.
Now when we have it working and a proper test is written, we can think about keeping it beautiful :-)

@tenderlove
Ruby on Rails member

Exactly. :-D

Please sign in to comment.
Something went wrong with that request. Please try again.