52
36

Delete article

Deleted articles cannot be recovered.

Draft of this article would be also deleted.

Are you sure you want to delete this article?

More than 5 years have passed since last update.

valid?がtrueを返しても、after_save、after_create、after_updateによって保存が失敗する可能性を考慮する

Last updated at Posted at 2019-06-13

はじめに

Railsのモデル(ActiveRecord)には様々なコールバックが用意されています。

参考: Active Record コールバック - Rails ガイド

コールバックの中にはafter_saveafter_createafter_updateといった、保存処理が終わった後に呼ばれるコールバックがあります。

あまりお行儀がよいコードとは言えませんが、たとえばとあるプロジェクトで、コールバックを使った次のような実装があったとします。

class User < ApplicationRecord
  has_many :messages
  after_create :create_initial_message
  
  private
  
  # User作成直後に、User向けのメッセージを作成する
  def create_initial_message
    messages.create!(content: 'Welcome!')
  end
end

こうすれば、Userの作成と同時にメッセージも作成できます。

user = User.new(email: 'new_user@example.com')
user.save!

# コールバックによってメッセージが作成されている
user.messages.count
#=> 1

はい、ここまでは問題ありませんね。

valid?では検知できない、コールバックのエラー

ところで、様々な事情があってこのプロジェクトでは次のような実装が書かれていました。

user = User.new(email: 'new_user@example.com')
# 検証エラーがないことを確認した上で、保存を実行する
if user.valid?
  # ...
  # (他の様々な処理)
  # ...
  
  user.save
end

最初は問題なく動いていたのですが、ある日Messageに新しいバリデーションが追加されました。

class Message < ApplicationRecord
  belongs_to :user
  
  # ステータスを必須とするように仕様が変わった
  validates :status, presence: true
end

こうなるとUserのコールバックは失敗します。

def create_initial_message
  # ステータスが未入力なのでApplicationRecord::RecordInvalid例外が発生する
  messages.create!(content: 'Welcome!')
end

「いや、ステータスに初期値を設定しておけばいいじゃん」というツッコミが入りそうですが、ここでは問題を単純化するために初期値は設定されないことにします。

こうなると、user.savefalseを返して保存に失敗します。

user = User.new(email: 'new_user@example.com')
if user.valid?
  # ...
  # (他の様々な処理)
  # ...
  
  # valid?がtrueだからといって安心していると、Userが保存されずにビックリする
  user.save
end

というわけで、valid?の結果がtrueだからといって、必ずしも保存が成功するとは限りません。
saveメソッドであれば戻り値をチェックするか、save!メソッドに変更して保存失敗時に例外が発生するようにしましょう。

unless user.save
  # 保存に失敗したときの処理を書く
end
# もしくは保存失敗時に例外を発生させる
user.save!

補足:コールバックの有無に限らず失敗する可能性はあります

この記事ではコールバックに注目して書いていますが、コールバックの有無に限らず、valid?のあとに呼んだsaveは失敗する可能性があります。

なぜなら、valid?が呼ばれてからsaveが呼ばれるまでに、データベースの状態が変わるかもしれないからです。

# この瞬間にはvalidだった
if user.valid?
  # ...
  # 他の処理を実行している間にデータベースの状態が変わる
  # ...

  # データベースの状態が変わったことでvalidではなくなり、saveに失敗する可能性がある
  user.save
end

ただし、この記事ではそのケースについては考慮せずに説明を続けます。

実際に困った事例

ある程度Railsに詳しい人なら、わざわざsaveの戻り値を無視するようなコードは書かないかもしれません。

ですが、実際にはdevise_invitableのinvite!メソッドと組み合わせたときに困りました。

user = User.new(email: 'new_user@example.com')
# passwordの未入力を無視するcontextを指定して検証する
if user.valid?(:some_context)
  # ...
  # (他の様々な処理)
  # ...
  
  # invite!メソッドを使ってUserを保存する(招待メールも送る)
  user.invite!
end

ちなみに、対象バージョンは以下のとおりです。

  • devise_invitable 2.0.1
  • devise 4.6.2
  • Rails 5.2.3

そもそも、インスタンスメソッド版のinvite!は新規User作成時に使うべきではないのですが、ここではそれを知らずに使ってしまいました。

参考:devise_invitableのインスタンスメソッド版invite!は、原則として保存済みのUserに対して使う - Qiita

さらに、invite!メソッドは検証エラーが発生しても例外は起きませんし、戻り値で成功/失敗を判断することもできません(明示的に成功/失敗を返すような実装になっていないため)。

もう少し詳しい話をすると、invite!メソッドの内部では以下のようなコードでsave(validate: false)が呼ばれています(参考)。

def invite!(invited_by = nil, options = {})
  # ...

  run_callbacks :invitation_created do
    # ...

    if save(validate: false)
      self.invited_by.decrement_invitation_limit! if !was_invited and self.invited_by.present?
      deliver_invitation(options) unless skip_invitation
    end
  end
end

save(validate: false)の戻り値がfalse、つまり保存に失敗した場合は何も行われないのですが、invite!メソッドの内部実装をきちんと理解していなかったために、結果としてsaveメソッドの戻り値を無視したのとほぼ同じコードを書いていました。

本当は自前のトランザクションを書いていたことでさらに困っていた

さらに、先ほどあげた実際に困った事例では、本当は次のように自前のトランザクション処理を書いていました。

transaction do
  # Userを保存する前の処理あれこれ
  
  user = User.new(email: 'new_user@example.com')
  if user.valid?(:some_context)
    # ...
    # (他の様々な処理)
    # ...
    
    user.invite!
  end
end

一見するとわからないかもしれませんが、invite!の内部で呼ばれるsaveメソッドがトランザクションを開始するため、このコードではトランザクションのネストが発生しています。

このコードを実行すると、Userのレコードだけが保存されて、Messageは保存されない、招待メールも送信されないという摩訶不思議な事態が発生します。

これは次のような処理フローになるためです。

  1. transactionブロックでトランザクションが開始される
  2. invite!メソッドが呼ばれる
  3. invite!メソッドの内部でsaveメソッドが呼ばれる
  4. saveメソッドがトランザクションを開始する(が、実際には上記1のトランザクションに合流する)
  5. saveメソッドがUserの保存を実行する(usersテーブルに対するINSERT文がDBに発行される)
  6. after_createコールバックが走るが、検証エラーのため、Messageの保存に失敗する
  7. saveメソッドはActiveRecord::Rollback例外を発行する
  8. Rollback例外は上記4のトランザクション(Rails内のtransactionブロック)の内部で捕捉される。が、上記1のトランザクション(外側のトランザクション)には通知されない
  9. invite!メソッドはsaveに失敗したので招待メールを送らずに処理を終了する
  10. 上記1のトランザクションは何もエラーが起きていないと判断し、トランザクションをコミットする
  11. 上記5でINSERT文が発行されているため、usersテーブルへのINSERTだけが確定する

じゃあどうすればよかったの?

上のコードは複数の問題が絡み合っていて、なかなか大変です。あらためて整理すると、以下のような点に問題がありました。

  • valid?の戻り値がtrueなら、保存は成功すると考えていたが、after_createコールバックで失敗する可能性も考慮すべきだった(そもそもコールバックに依存すること自体が良くなかったかも)
  • 暗黙的なトランザクションのネストに気づかず、Rollback例外が無視されてしまった
  • インスタンスメソッド版のinvite!メソッドは、新規レコードに対して実行するべきではなかった

問題点はわかるのですが、修正方法はなかなか悩ましいところです。
修正方法が明確なのはトランザクションのネストのところですね。
これはjoinable: falseを指定することで、saveメソッドのトランザクションが合流してくることを解消できます。
ついでに、requires_new: trueも指定しておくと、自分の外にトランザクションが存在していたときも自分がそこに合流することを防止できます。

# 内側であろうと外側であろうと、勝手にトランザクションに合流することを防止する
transaction(joinable: false, requires_new: true) do
  # ...

なお、トランザクションのネストに関しては以下の記事で詳しく説明しているので、「何のことか、ようわからん」という人はこちらの記事をご覧ください。

【翻訳】ActiveRecordにおける、ネストしたトランザクションの落とし穴 - Qiita

他はちょっと無理矢理ですが、こんな感じで対応することになるでしょうか。

transaction(joinable: false, requires_new: true) do
  # Userを保存する前の処理あれこれ
  
  user = User.new(email: 'new_user@example.com')
  # いったんINSERTを実行してしまう(after_createに失敗したらfalseが返る)
  if user.save(context: :some_context)
    # ...
    # (他の様々な処理)
    # ...
    
    # 保存済みのUserに対してinvite!を呼び出す
    user.invite!
  else
    # saveに失敗していたらロールバックする
    raise ActiveRecord::Rollback
  end
end

ただ、この場合でも100%完璧ではなくて、もしafter_saveafter_updateが用意されていて、その呼び出しに失敗していたら招待メールが送信できていないことになります。(先ほど示したinvite!メソッドの実装コードを参照)

であれば、あえて次のように新規Userに対してinvite!を呼びだして、persisted?がtrueを返すかどうかで成功/失敗を判断するのがよいかもしれません。

transaction(joinable: false, requires_new: true) do
  # Userを保存する前の処理あれこれ
  
  user = User.new(email: 'new_user@example.com')
  
  # ...
  # (他の様々な処理)
  # ...
  
  # 新規Userに対してあえてinvite!を呼び出す
  user.invite!
  
  unless user.persisted?
    # Userが保存されていなければinvite!の失敗と見なしてロールバックする
    raise ActiveRecord::Rollback
  end
end

まあ、いずれのコードもかなり無理矢理な実装になってしまうので、この場合はそもそもプログラムの設計から見直すのが一番よいかもしれません。

まとめ

というわけで、この記事ではafter_saveafter_createafter_updateといった、保存処理が終わった後に呼ばれるコールバックで発生しうる不具合について説明してみました。

先ほども書いたとおり、コールバックに大きく依存したコード自体がそもそも「臭うコード」であると言えます。
ですので、こういった問題に遭遇した場合は、設計を見直してもっとシンプルな実装で済ませる方法を検討してみましょう。

52
36
0

Register as a new user and use Qiita more conveniently

  1. You get articles that match your needs
  2. You can efficiently read back useful information
  3. You can use dark theme
What you can do with signing up
52
36

Delete article

Deleted articles cannot be recovered.

Draft of this article would be also deleted.

Are you sure you want to delete this article?