Header image

クラッソーネの開発者がエンジニアリングに関することもそうでないことも綴っています!

.orを使ってActiveRecordのスコープを定義するときは.mergeで囲わないと意図しない挙動になるかも?

.orを使ってActiveRecordのスコープを定義するときは.mergeで囲わないと意図しない挙動になるかも?

こんにちは、山口拓弥(@yamat47)です。

最近、いまさらながら Netflix の梨泰院クラス にハマっています。
勧善懲悪なストーリーはわかりやすく面白いし、見れば見るほどチャミスルが飲みたくなりますね 🍻

https://twitter.com/yamat47/status/1586206595842646016

さて本日は、最近業務で書いた ActiveRecord の scope の実装の話をします。
危うく重大な不具合を含んだままアプリをリリースしそうになったので... 自戒も込めて記事にします...!

不具合を含んでいたコードを早速紹介します

サンプルで扱うモデルの説明

業務のコードをそのまま公開するのは文脈を伝えづらく難しかったので、この記事ではシンプルなまったく別の状況を扱います。
大まかな説明ですが、こんな感じの状況を想定してください:

  • 不特定多数とオンライン対戦ができるような SNS アプリ。
    ユーザーはギルドに所属することができ、このギルド同士で対戦をする。
  • ユーザーは年齢を登録する必要がある。
    また 18 歳未満のユーザーは保護者による同意を必要とする。

登場するモデル同士の関連付けはこうなっています:

  • guild has many users.
  • user has many parental_consent_logs.

意図しない挙動の自動テストでの再現を含めて、サンプルコードは公開してあります。
参考にしてください:

https://github.com/yamat47/activerecord-or-merge-sample

問題を含んでいたスコープの実装

アプリを利用できるユーザーのリスト、すなわち「年齢が 18 歳以上」もしくは「保護者が同意済み」なユーザーのリストを取得したいとします。
いろいろなところで再利用しそうだったため、モデルに scope として実装しました。

app/models/user.rb
class User < ApplicationRecord
  scope :adult_or_parental_consented, lambda {
    left_joins(:parental_consent_logs)
      .where(age: 18..)
      .or(ParentalConsentLog.where.not(id: nil))
  }
end

このスコープが、使われ方によっては意図しない挙動になってしまっていました。

何が問題だった?

スコープを単独で使う分にはまったく問題ない

このスコープだけを使ってデータを絞り込むのであれば問題なく動作します。
実際に発行されるクエリを Rails console で試してみたのがこちらです:

bin/rails console (SQLite を使っています)
irb(main):001:0> puts User.adult_or_parental_consented.to_sql
SELECT "users".* FROM "users" LEFT OUTER JOIN "parental_consent_logs" ON "parental_consent_logs"."user_id" = "users"."id" WHERE ("users"."age" >= 18 OR "parental_consent_logs"."id" IS NOT NULL)
=> nil

他の条件を組み合わせると、その順番によっては意図しない挙動になる

例えば「あるギルドに所属しているユーザーのうちでアプリを利用できる人のリスト」を取得したいとします。
ID が 1 のギルドに所属しているという条件を追加してみるとこのようになります:

bin/rails console(SQLite を使っています)
irb(main):006:0> puts User.where(guild: 1).adult_or_parental_consented.to_sql
SELECT "users".* FROM "users" LEFT OUTER JOIN "parental_consent_logs" ON "parental_consent_logs"."user_id" = "users"."id" WHERE ("users"."guild_id" = 1 AND "users"."age" >= 18 OR "parental_consent_logs"."id" IS NOT NULL)
=> nil

わかりやすくするために WHERE 句だけを取り出したのがこちらです:

WHERE ("users"."guild_id" = 1 AND "users"."age" >= 18 OR "parental_consent_logs"."id" IS NOT NULL)

これは「ギルドの ID が 1 で年齢が 18 歳以上」もしくは「保護者が同意済み」のいずれかの条件を満たすレコードに絞り込みます。[1]
ギルドによる絞り込みが全体に効いてほしかったにも関わらず、最初の条件にしか効きませんでした。

すなわち、保護者が同意済みな他ギルドのユーザーも絞り込んだ結果に含まれてしまいます。
これはまったく意図していない挙動でした...。

ではどうすれば解決できる?

解決策 ① .merge で囲う

記事のタイトル通りですが、スコープの中身全体を .merge で囲うことで解決します。
条件が長くなりすぎないように微調整しつつ[2]、具体的にはこんな感じにします:

app/models/user.rb
class User < ApplicationRecord
  scope :adult_or_parental_consented, lambda {
    or_condition = unscoped
                     .left_joins(:parental_consent_logs)
                     .where(age: 18..)
                     .or(ParentalConsentLog.where.not(id: nil))

    merge(or_condition)
  }
end

.merge で囲うことで、スコープで定義した条件をひとまとまりなものとして扱うことができます。
そこに他の条件を組み合わせたとしても、最終的には複数の条件を AND で直列につなげたものとして扱われます。

Rails console(SQLite を使っています)
irb(main):001:0> puts User.where(guild: 1).adult_or_parental_consented.to_sql
SELECT "users".* FROM "users" LEFT OUTER JOIN "parental_consent_logs" ON "parental_consent_logs"."user_id" = "users"."id" WHERE "users"."guild_id" = 1 AND ("users"."age" >= 18 OR "parental_consent_logs"."id" IS NOT NULL)
=> nil

WHERE 句だけを取り出したのがこちら:

WHERE "users"."guild_id" = 1 AND ("users"."age" >= 18 OR "parental_consent_logs"."id" IS NOT NULL)

いい感じに絞り込みの条件を組み立てることができました!

解決策 ② unscoped な条件を渡すのをやめる

ParentalConsentLog.where.not(id: nil) という条件は、それまでに渡された他の条件と切り離された unscoped な条件です。
これをそのまま .or に渡していたことで、ActiveRecord がその条件をうまく解釈できず素朴に連結をしていました。

今回のケースであれば、scoped な状態をもとに組み立てた条件を渡すようにすることでも解決ができます。
具体的にはこちらです:

app/models/user.rb
class User < ApplicationRecord
  scope :adult_or_parental_consented, lambda {
    left_joins(:parental_consent_logs)
      .where(age: 18..)
      .or(where(parental_consent_logs: { id: nil }))
  }
end

(Rails console の結果などは ① と一緒なので省略します)

まとめ

以上、ActiveRecord のスコープを作るときにハマった課題とその解決策でした。

「他条件と組み合わせたときの挙動」はなかなか単体テストでは検証しづらく、動作確認をしていたときにたまたま気付けた不具合でした。
こういうケースに実装中に気付くにはどうしたらよかったか...
「こんなの試しています」などあれば、Twitter などで気軽に教えていただけると嬉しいです!

クラッソーネでは失敗を周囲にシェアして知見にしてもらうのが好きなエンジニアを大募集しています。
興味ある方はぜひ採用サイトをご覧ください!

https://www.crassone.co.jp/recruit/engineer/

脚注
  1. 正確には RDBMS の実装に依存しますが、このような挙動になるものばかりです。(参考:MySQL の演算子の優先順位SQLite の演算子の優先順位↩︎

  2. 参考: ActiveRecordのorクエリをscope化する - Qiita ↩︎


クラッソーネで Ruby on Rails を使ってサービス開発をしています。週末はアメフトのコーチをしたり選手もしたり、たまに審判もしたりしてます。エビ中もすき。