How to properly handle changed attributes in a Rails before_save hook?

Question

I have a model that looks like this:

class StopWord < ActiveRecord::Base
  UPDATE_KEYWORDS_BATCH_SIZE = 1000

  before_save :update_keywords

  def update_keywords
    offset = 0
    max_id = ((max_kw = Keyword.first(:order => 'id DESC')) and max_kw.id) || 0
    while offset <= max_id
      begin
        conditions = ['id >= ? AND id < ? AND language = ? AND keyword RLIKE ?',
            offset, offset + UPDATE_KEYWORDS_BATCH_SIZE, language]

        # Clear keywords that matched the old stop word
        if @changed_attributes and (old_stop_word = @changed_attributes['stop_word']) and not @new_record
          Keyword.update_all 'stopword = 0', conditions + [old_stop_word]
        end

        Keyword.update_all 'stopword = 1', conditions + [stop_word]

      rescue Exception => e
        logger.error "Skipping batch of #{UPDATE_KEYWORDS_BATCH_SIZE} keywords at offset #{offset}"
        logger.error "#{e.message}: #{e.backtrace.join "\n    "}"

      ensure
        offset += UPDATE_KEYWORDS_BATCH_SIZE
      end
    end
  end
end

This works just fine, as the unit tests show:

class KeywordStopWordTest < ActiveSupport::TestCase
  def test_stop_word_applied_on_create
    kw = Factory.create :keyword, :keyword => 'foo bar baz', :language => 'en'
    assert !kw.stopword, 'keyword is not a stop word by default'

    sw = Factory.create :stop_word, :stop_word => kw.keyword.split(' ')[1], :language => kw.language
    kw.reload
    assert kw.stopword, 'keyword is a stop word'
  end

  def test_stop_word_applied_on_save
    kw = Factory.create :keyword, :keyword => 'foo bar baz', :language => 'en', :stopword => true
    sw = Factory.create :keyword_stop_word, :stop_word => kw.keyword.split(' ')[1], :language => kw.language

    sw.stop_word = 'blah'
    sw.save

    kw.reload
    assert !kw.stopword, 'keyword is not a stop word'
  end
end

But mucking with the @changed_attributes instance variable just feels wrong. Is there a standard Rails-y way to get the old value of an attribute that is being modified on a save?

Update: Thanks to Douglas F Shearer and Simone Carletti (who apparently prefers Murphy's to Guinness), I have a cleaner solution:

  def update_keywords
    offset = 0
    max_id = ((max_kw = Keyword.first(:order => 'id DESC')) and max_kw.id) || 0
    while offset <= max_id
      begin
        conditions = ['id >= ? AND id < ? AND language = ? AND keyword RLIKE ?',
            offset, offset + UPDATE_KEYWORDS_BATCH_SIZE, language]

        # Clear keywords that matched the old stop word
        if stop_word_changed? and not @new_record
          Keyword.update_all 'stopword = 0', conditions + [stop_word_was]
        end

        Keyword.update_all 'stopword = 1', conditions + [stop_word]

      rescue StandardError => e
        logger.error "Skipping batch of #{UPDATE_KEYWORDS_BATCH_SIZE} keywords at offset #{offset}"
        logger.error "#{e.message}: #{e.backtrace.join "\n    "}"

      ensure
        offset += UPDATE_KEYWORDS_BATCH_SIZE
      end
    end
  end

Thanks, guys!

Answer 1

You want ActiveModel::Dirty.

Examples:

person = Person.find_by_name('Uncle Bob')
person.changed?       # => false

person.name = 'Bob'
person.changed?       # => true
person.name_changed?  # => true
person.name_was       # => 'Uncle Bob'
person.name_change    # => ['Uncle Bob', 'Bob']

Full documentation: http://api.rubyonrails.org/classes/ActiveModel/Dirty.html

Answer 2

You're using the right feature but the wrong API. You should #changes and #changed?.

See this article and the official API.

Two additional notes about your code:

1. Never rescue Exception directly when you actually want to rescue execution errors. This is Java-style. You should rescue StandardError instead because lower errors are normally compilation error or system error.

2. You don't need the begin block in this case.

   def update_keywords
     ...
     rescue => e
     ...
     ensure
     ...
   end