Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

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

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!

like image 357
Josh Glover Avatar asked Mar 31 '11 14:03

Josh Glover


2 Answers

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

like image 158
Douglas F Shearer Avatar answered Nov 20 '22 00:11

Douglas F Shearer


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
    
like image 1
Simone Carletti Avatar answered Nov 20 '22 00:11

Simone Carletti