This project is archived and is in readonly mode.

#5472 open
James Conroy-Finn

Potentially resolved… Callback API change breaks #run_callbacks

Reported by James Conroy-Finn | August 27th, 2010 @ 11:44 AM | in 3.0.6

It appears that calling Model#run_callbacks is broken in the Rails 3 release candidates (rc and rc2), however callbacks do work when Rails runs them internally.

Page.before_save lambda { puts "Preventing save!"; return false }
page = Page.new
page.save
# Preventing save!
# => false
page.new_record?
# => true

Now if we try to do the same thing but this time we trigger the callback directly with…

page.run_callbacks(:before_save)

We get an undefined method error as follows.

NoMethodError: undefined method `_run_before_save_callbacks' for #<Page:0x00000102e24548>
from /Users/jcf/.rvm/gems/ruby-1.9.2-p0@pages/bundler/gems/rails-0953c04/activemodel/lib/active_model/attribute_methods.rb:364:in `method_missing'
from /Users/jcf/.rvm/gems/ruby-1.9.2-p0@pages/bundler/gems/rails-0953c04/activerecord/lib/active_record/attribute_methods.rb:46:in `method_missing'
from /Users/jcf/.rvm/gems/ruby-1.9.2-p0@pages/bundler/gems/rails-0953c04/activesupport/lib/active_support/callbacks.rb:93:in `run_callbacks'
from (irb):6
from /Users/jcf/.rvm/gems/ruby-1.9.2-p0@pages/bundler/gems/rails-0953c04/railties/lib/rails/commands/console.rb:44:in `start'
from /Users/jcf/.rvm/gems/ruby-1.9.2-p0@pages/bundler/gems/rails-0953c04/railties/lib/rails/commands/console.rb:8:in `start'
from /Users/jcf/.rvm/gems/ruby-1.9.2-p0@pages/bundler/gems/rails-0953c04/railties/lib/rails/commands.rb:23:in `<top (required)>'
from script/rails:6:in `require'
from script/rails:6:in `<main>'

I'm going to do some digging around and try to work out why callbacks work internally but fail when calling #run_callbacks directly.

Comments and changes to this ticket

  • James Conroy-Finn

    James Conroy-Finn August 27th, 2010 @ 12:30 PM

    I've written a quick test in activerecord/test/cases/callbacks_test.rb that demonstrates the exact failure I'm experiencing and have included a patch file with the test in case that's any use. I'll see if I can fix it myself and upload another patch if and when I get it done.

    BTW I'm not a test-unit kinda guy so it may be there's a better way to implement the test. Apologies for any cardinal test-unit sin I may have committed.

  • James Conroy-Finn
  • James Conroy-Finn

    James Conroy-Finn August 28th, 2010 @ 12:11 AM

    OK… We've worked this one out. The API has changed but the documentation doesn't reflect the changes too clearly. Hopefully Yehuda can clarify this but essentially callbacks work as follows…

    This will run only the before save callback…

    p = Post.new
    p.run_callbacks(:save) { false }
    

    …and this will run both the before and after callbacks…

    p = Post.new
    p.run_callbacks(:save) { true }
    

    Essentially you can't specify the before or after anymore (using #run_callbacks(:before_save) etc.) because callbacks work like this.

    You call a group of callbacks, for example save, create, validation. There will potentially be a before and/or after callback to any of these groups. All will be executed but only if the block supplied evaluates to true. Examples will explain this better so here we go…

    # Before and after callbacks
    # This will run both the before_save and after_save callbacks…
    Post.new.run_callbacks(:save)
    
    # …as will this
    Post.new.run_callbacks(:save) { true }
    
    # This will only run the before_save and ignore any after_save callback
    Post.new.run_callbacks(:save) { false }
    

    This is darned handy if you have a condition that determines whether or not to execute the next callback as you do in ActiveRecord when saving (e.g. errors.empty?).

    I might be mistaken but after some debugging, source tasting and with some mad class/instance method magic I'm pretty sure this is how things work in Rails 3.

    Again, if Mr. Katz or one of his learned friends could confirm this it would be appreciated.

    Thanks.new.run_callbacks(:finished) { true }

    puts "James ;)"

  • James Conroy-Finn

    James Conroy-Finn August 28th, 2010 @ 12:12 AM

    • Title changed from “Unable to manually run ActiveRecord object callbacks” to “Potentially resolved… Callback API change breaks #run_callbacks”
  • Jeremy Kemper

    Jeremy Kemper August 28th, 2010 @ 12:17 AM

    • Milestone cleared.
    • State changed from “new” to “open”
    • Assigned user set to “Yehuda Katz (wycats)”
    • Importance changed from “” to “Low”
  • Neeraj Singh

    Neeraj Singh August 28th, 2010 @ 04:25 AM

    I recently fixed #5419 http://github.com/rails/rails/commit/2ffa50f5a9fac08e08869687006031... where after_validation was not getting called if valid? returns false.

    I will have to check with rails 2.3.x to see if what is the existing behavior. Any change in behavior should be documented.

  • Neeraj Singh

    Neeraj Singh August 28th, 2010 @ 05:33 AM

    @James you are spot on.

    Here is definition of method _define_after_model_callback

        def _define_after_model_callback(klass, callback) #:nodoc:
          klass.class_eval <<-CALLBACK, __FILE__, __LINE__ + 1
            def self.after_#{callback}(*args, &block)
              options = args.extract_options!
              options[:prepend] = true
              options[:if] = Array.wrap(options[:if]) << "!halted && value != false"
              set_callback(:#{callback}, :after, *(args << options), &block)
            end
          CALLBACK
        end
      end
    

    Notice the part where it says value != false . That code is saying that proceed with callback only if the returned value is NOT false. Otherwise halt the chain.

    if you change line from

    options[:if] = Array.wrap(options[:if]) << "!halted && value != false"
    

    to

    options[:if] = Array.wrap(options[:if]) << "!halted"
    

    then after_save callback will be called irrespective of the returned value from the save operation.

    Hope that helps you understand why the code is behaving the way it is behaving.

  • Jeremy Kemper
  • Jeremy Kemper

    Jeremy Kemper October 15th, 2010 @ 11:02 PM

    • Milestone set to 3.0.2
  • Ryan Bigg

    Ryan Bigg October 19th, 2010 @ 08:35 AM

    • Tag cleared.

    Automatic cleanup of spam.

  • Santiago Pastorino
  • Santiago Pastorino
  • Santiago Pastorino

    Santiago Pastorino February 27th, 2011 @ 03:15 AM

    • Milestone changed from 3.0.5 to 3.0.6

Create your profile

Help contribute to this project by taking a few moments to create your personal profile. Create your profile »

<h2 style="font-size: 14px">Tickets have moved to Github</h2>

The new ticket tracker is available at <a href="https://github.com/rails/rails/issues">https://github.com/rails/rails/issues</a>

Attachments

Pages