はじめに

Railsのhas_oneやhas_manyといったアソシエーションは、モデル同士の関連を簡単に扱える強力な仕組みです。しかし、このアソシエーションメソッドを安易にオーバーライドすると、思わぬ副作用が発生し、予期しないオブジェクト生成やテスト失敗の原因となることがあります。

今回の記事はオーバーライドした結果、バリデーションが追加されたときに、どのように予期せぬ動作が発生したのかを書いていきたいと思います。

前回の記事の続きになります。

hisakit.hatenablog.com

背景・動機

実務でhas_oneをオーバーライドして関連オブジェクトを必ず用意するようにした結果、バリデーションやテストで予期せぬ動作が発生しました。

今回記事では、実際にどのように予期せぬ動作が発生したのかを書いていきたいと思います。

実例・やってみたこと

今回のコードは以下のようになります。（前回からの続きで、カスタムバリデーションを追加した形になります）

class User < ApplicationRecord
  has_one :setting
  has_one :address

  validate : check_setting_flag

  def setting
    super | build_setting
  end

  private

  def check_setting_flag
    if setting&.some_flag && address&.prefecture.blank?
      errors.add(:base, "some_flagがtrueのときは、prefectureを入力してください")
  end
end

何が起きたか

UserとSettingを使ったRspecが全て落ちるようになりました。

以下のような書き方がある箇所になります

let(:user) { create(:user) }
let(:setting) { create(:setting, user:) }

userをcreateした時点で、バリデーションが実行される
バリデーション内でsettingを呼び出すと、オーバーライドによりbuild_settingが走り、新しいSettingが関連付けられる
その後create(:setting, user:)を呼ぶと、すでにuserに紐づいたsettingが存在しているため、DBの一意制約によりduplication errorが発生する

どう回避するか

回避策は大きく分けて「実装を直す」か「テストの書き方を直す」かの2つです。

不要なオーバーライドのメソッドは使わないようにすべき(前編記事でも記載しています)
オーバーライドしているメソッドを使わない
- associationメソッドなどを使うようにする

associationメソッドでアソシエーションを確認することができます

  def check_setting_flag
    if association(:setting).reader&.some_flag && address&.prefecture.blank?
      errors.add(:base, "some_flagがtrueのときは、prefectureを入力してください")
    end
  end

Rspecの書き方を直す。

createを乱用しない

下記のように、createは一箇所にして他の関連するレコードはbuildにするべき(意図的にcreateするケースなどもあるかもしれませんが、今回は考えてません)

let(:user) { create(:user, setting:) }
let(:setting) { build(:setting) }

FactoryBotでtraitを追加する

factory :user do
  trait :with_setting do
    after(:create) { |u| create(:setting, user: u) }
  end 
end

学び・気付き

前編でも触れましたが、改めて強調したいのは以下の点です。

アソシエーションのリーダーメソッドは読み取り専用で扱うべき
「関連を必ず用意する」責務は、オーバーライドではなくコールバックやサービスクラスで担保するべき
テストではcreateをむやみに使わず、Factoryやbuildを活用して意図した状態を再現するべき

まとめ

今回のまとめは、前回と重複するかもしれませんが、

アソシエーションのオーバーライドに生成処理を仕込むと、バリデーションやテストのタイミングで副作用が発生する
特にhas_oneは一意制約が絡むため、テストが一斉に落ちる原因になりやすい
読み取りと生成の責務を分離することで、予期しない不具合を防ぐ

Hisakeyのブログ

エンジニアが色々呟くブログです。

後編：【Rails失敗談】has_oneをオーバーライドした結果、バリデーションを追加したら、全テストが失敗した