Rails: before_destroyによる関連チェックに注意

既に色んな方が記事にされていますが、さっそく自分もハマったのでメモ。 (環境: Rails 4.1.1, Ruby 2.1.0)

やりたかった事

あるモデルを削除する前にバリデーションを走らせ、バリデーションが通った時のみ削除するようにする。 そのために、before_destroyコールバックを登録する。

ハマった事

before_destroyに登録したメソッドを単体で実行してみると正しく動くのに、 いざdestroyすると期待する挙動にならない。例えば以下のようなコードだと、 before_destroyのバリデーションが必ず通ってしまう。

class User < ActiveRecord::Base
  has_many :events, dependents: :destroy

  before_destroy :check_all_events_finished

  def check_all_events_finished
    now = Time.zone.now
    if events.where('? < start_time', now).exists?
      errors[:base] << '未終了のイベントがあります'
    end
    errors.blank?
  end
end

原因

今回の場合はbefore_destroyの中で関連するモデルの存在をチェックしているのですが、 上記のコードではその関連がbefore_destroyの前に削除されていた事が原因でした。 そうなるとそもそも紐づくeventがなくなるため、このバリデーションは常にtrueを返してしまいます。

問題は、例におけるhas_manyの関連がdependentsオプションを持っている点にあります。というのも、

  • dependentsオプションの処理はbefore_destroyコールバックを使って実現される(Github)
  • before_destroyなどのコールバックは定義した順番に実行される

からです。つまり上記の例でいうと、has_manyを定義した段階で関連を削除するためのbefore_destroyがこっそり登録されており、 バリデーション用に定義したbefore_destroyはそのdependent: :destroy用コールバックの後で実行されるため、 バリデーション時にはeventsが必ず空を返すようになっていたのでした。

解決策

下記いずれかの方法でバリデーション用のbefore_destroyを先に走らせるようにすれば、この問題は回避できます。

1. before_destroyhas_many定義より前に宣言する

class User < ActiveRecord::Base
  before_destroy :check_all_events_finished

  has_many :events, dependents: :destroy

  # ...

2. prependオプションをtrueにする

このオプションつきで登録されたコールバックは、コールバックキューの末尾でなく先頭に追加されます (Ordering callbacks)。

class User < ActiveRecord::Base
  has_many :events, dependents: :destroy

  before_destroy :check_all_events_finished, prepend: true

  # ...

原因がわかれば納得はできるものの、一見無関係に見えるクラスマクロの定義順序によって動作が変わってしまうというのはわかりづらいですね。 Githubを見てみると2011年から既にissueが上がっていました(rails/rails#3458)。

参考