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

## はじめに

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

参考: [Active Record コールバック \- Rails ガイド](https://railsguides.jp/active_record_callbacks.html)

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

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

```ruby
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の作成と同時にメッセージも作成できます。

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

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

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

## valid?では検知できない、コールバックのエラー
ところで、様々な事情があってこのプロジェクトでは次のような実装が書かれていました。

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

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

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

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

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

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

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

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

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

```ruby
unless user.save
  # 保存に失敗したときの処理を書く
end
```

```ruby
# もしくは保存失敗時に例外を発生させる
user.save!
```

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

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

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

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

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

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

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

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

```ruby
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](https://qiita.com/jnchito/items/689a7069f02ce14d1a3f)

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

もう少し詳しい話をすると、`invite!`メソッドの内部では以下のようなコードで`save(validate: false)`が呼ばれています（[参考](https://github.com/scambra/devise_invitable/blob/v2.0.1/lib/devise_invitable/models.rb#L139-L166)）。

```ruby
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`メソッドの戻り値を無視したのとほぼ同じコードを書いていました。

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

```ruby
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`も指定しておくと、自分の外にトランザクションが存在していたときも自分がそこに合流することを防止できます。

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

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

[【翻訳】ActiveRecordにおける、ネストしたトランザクションの落とし穴 \- Qiita](https://qiita.com/jnchito/items/930575c18679a5dbe1a0)

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

```ruby
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_save`や`after_update`が用意されていて、その呼び出しに失敗していたら招待メールが送信できていないことになります。（先ほど示した`invite!`メソッドの実装コードを参照）

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

```ruby
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_save`、`after_create`、`after_update`といった、保存処理が終わった後に呼ばれるコールバックで発生しうる不具合について説明してみました。

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