こんにちは。
Railsエンジニアの横山です。

私がRailsで開発を行うようになって2年弱になりますが、日々、知らない事・分からない事・理解の不十分な事があり勉強の毎日です。

今回は、そのなかでも、理解が不十分であった故にActiveRecordのコールバックでハマってしまったお話をしようかと思います。

ActiveRecordのコールバックについて

細かい説明は省略させていただきますが、RailsはActiveRecordの操作（検索・新規登録・更新・削除)を行う際に、自動で呼び出されるメソッドがあります。
それがコールバックと呼ばれる処理で、以下のような種類があります。

after_findafter_initializebefore_validationafter_validationbefore_saveafter_savebefore_createafter_createbefore_updateafter_updateafter_commitafter_rollbackafter_touch

今回はこの中でもafter_saveのお話になります。

新規追加時と更新時で挙動が違う？

やりたかった処理はオブジェクトの更新後、自身と子モデルの状態をチェックして、条件に一致したら自身の情報を一部変更する、というものです。
子モデルの更新をafter_saveで行っていたためbefore_validation、after_validation、before_saveなどを使えなかった、という事情もありました。
（今回の説明では簡略化のため、子モデルの処理は省略）

以下のコマンドでarticleモデルを作成します。

rails g model article name:string text:text

articleのtextに”hogehoge”という文字が含まれていたら”xxxxx”に置き換えて保存するという処理を行う事にします。

当初は単純に「after_saveの中に混ぜ込めばいいだろ」と思っていたので、以下のように記述しました。

class Article < ActiveRecord::Base

  NG_WORD = "hogehoge"

  after_save :replace_text

  def replace_text
    return unless self.text.index(NG_WORD)

    replace_word = self.text.gsub(NG_WORD, "xxxxx")
    Article.transaction do
      self.update_attributes!(text: replace_word)
    end
  end

end

賢明な皆様なら一発かと思いますが、上記の記載方法だと、新規作成の処理ではうまくいきますが、更新処理ではうまく機能しない時があります。

手順
１．新規にtext：ああhogehogeを登録
　　=>これは普通に成功し、「ああxxxxx」になるはず
２．追加したarticleのtext：ああxxxxxに対して「ああhogehoge」と再登録
　　=>これは失敗。「ああhogehoge」で登録されてしまいます。

これ、なんで駄目なのか良く分からないのでRailsの中まで追ってみました。
コールバックのタイミングの問題かと思いきや、どうもupdate_attributes!とafter_saveの相性？の問題のようでした。

問題はここ
v3.2.17/activerecord/lib/active_record/attribute_methods/dirty.rb#L56

def write_attribute(attr, value)
  attr = attr.to_s

  # The attribute already has an unsaved change.
  if attribute_changed?(attr)
    old = @changed_attributes[attr]
    @changed_attributes.delete(attr) unless _field_changed?(attr, old, value)
  else
    old = clone_attribute_value(:read_attribute, attr)
    # Save Time objects as TimeWithZone if time_zone_aware_attributes == true
    old = old.in_time_zone if clone_with_time_zone_conversion_attribute?(attr, old)
    @changed_attributes[attr] = old if _field_changed?(attr, old, value)
  end

  # Carry on.
  super(attr, value)
end

と、attribute_changed?の中身であるここ
v3.2.17/activemodel/lib/active_model/dirty.rb#L137

# Map of change <tt>attr => original value</tt>.
def changed_attributes
   @changed_attributes ||= {}
end

#中略

# Handle <tt>*_changed?</tt> for +method_missing+.
def attribute_changed?(attr)
  changed_attributes.include?(attr)
end

write_attributeの中で実際に差し替える処理を行い、attribute_changed?の中で、その差し替え必要性の判定を行っています。
上記のケースだと、最初のsave処理で@changed_attributes内に修正するデータが格納されます。
その後、after_saveでupdate_attributes!が呼ばれ、再度、ここの判定処理を行いますが
一回目のsaveで登録した@changed_attributesの情報がそのままであるため、2回目のsaveでは「さっき更新しただろ？」と判定されてしまい、after_saveで行ったupdate_attributes!が無効化されてしまう、というオチでした。

まとめ

今回のケースに限っていえば、after_saveではなく、after_commitを使えば @changed_attributesの情報が初期化されるので、こっちを使うようにしています。

とはいえ、save終わった後に@changed_attributesの情報を保持しとく理由ってなんかあるのかな？ 初期化すべきでは？
または

def attribute_changed?(attr)
  changed_attributes.include?(attr)
end

で、Hashのkeyの有無だけで更新の必要性を判定するのではなく、中身もちゃんと見るようにしても良いかもしれませんね。
など、もうちょっと調べてpull requestも検討してみます。

最後に

マネーフォワードでは、新しい事にチャレンジできるエンジニアを募集しています！
私たちと一緒に最高のサービスを作っていきませんか？

マネーフォワード採用サイト
https://recruit.moneyforward.com/

日本No.1お金のサービスを創り上げるRailsエンジニアWanted！
https://www.wantedly.com/projects/9979