pockestrap

Programmer's memo

Rails 7で導入される invert_where メソッドが危険そうだったので調べた

Programming Ruby RuboCop Ruby on Rails

この記事は現時点(2021-04-28)のrails/railsのmainブランチの最新のコミットでテストしています。

TL;DR

  • invert_whereはすべてのwhereをinvertする
  • 使い方によっては意図しない条件をinvertして危険

invert_whereとは

invert_whereは、Rails 7でActiveRecord::Relationに追加される予定のメソッドです。 このメソッドは、relationにチェーンして呼び出すことで、それまでのwhereの条件を反転できます。

つまり次のようになります。(CHANGELOGから引用)

class User
  scope :active, -> { where(accepted: true, locked: false) }
end

User.active
# ... WHERE `accepted` = 1 AND `locked` = 0

User.active.invert_where
# ... WHERE NOT (`accepted` = 1 AND `locked` = 0)

このメソッドは、#40249で導入されました。

日本語でも紹介記事が書かれています。

techracho.bpsinc.jp

invert_whereの危険性

では、invert_whereはなにが危険なのでしょうか。今までの例だけだと何が危険なのか分かりづらいと思います。

この危険性はinvert_whereの前のscopeを複数にすると見えてきます。たとえば、次の例を考えます。

puts Post.where(a: 'a').where(b: 'b').invert_where.to_sql

この場合、条件が反転するのはwhere(b: 'b')だけでしょうか？それともwhere(a: 'a').where(b: 'b')の両方でしょうか？

答えは、両方が反転されます。つまり、上のコードを実行すると次の出力が得られます。

SELECT "posts".* FROM "posts" WHERE NOT ("posts"."a" = 'a' AND "posts"."b" = 'b')

2つのwhere全体がNOTで囲まれていますね。

この「invert_whereを呼ぶ以前のwhere全体の条件が反転する」性質は、具体的に次に上げる3つのコードで問題になります。 1つずつ見ていきましょう。

なお、各再現コードは次のコードに続けて書くことで実行できます。

require "bundler/inline"

gemfile(true) do
  source "https://rubygems.org"

  git_source(:github) { |repo| "https://github.com/#{repo}.git" }

  gem "activerecord", github: 'rails/rails', ref: 'adc0146'
  gem "activemodel", github: 'rails/rails', ref: 'adc0146'
  gem "activesupport", github: 'rails/rails', ref: 'adc0146'
  gem "sqlite3"
end

require "active_record"
require "logger"

ActiveRecord::Base.establish_connection(adapter: "sqlite3", database: ":memory:")

ActiveRecord::Schema.define do
  create_table :users, force: true do |t|
    t.string :role
    t.datetime :disabled_at
  end
end

1. スコープの中でinvert_whereを使った場合

スコープの中でinvert_whereを使った場合でも、そのスコープの外にinvert_whereが影響します。 これは分かりづらい挙動でしょう。

次のようなコードで確認できます。

class User < ActiveRecord::Base
  scope :alive, -> { where(disabled_at: nil) }
  scope :disabled, -> { alive.invert_where }

  scope :admin, -> { where(role: 'admin') }
end

puts User.admin.disabled.to_sql
# => SELECT "users".* FROM "users" WHERE NOT ("users"."role" = 'admin' AND "users"."disabled_at" IS NULL)

このUser.admin.disabledというコードは、「adminかつ無効化されたユーザー」を返すことを意図しているように読めます。 ですが実際のクエリでは「(adminかつ有効)ではないユーザー」、つまり「adminでないか、もしくは無効化されたユーザー」が返ってきてしまいます。

これはinvert_whereがscopeをまたぐかどうかに関わらず、invert_whereより手前の条件をすべて反転するためです。

2. invert_whereとrelationの開始位置が離れている場合

これは1のケースと似ています。このケースではinvert_whereはscopeの中に隠れていませんが、その一連のrelationの開始位置が離れている場合です。 これはinvert_whereが実装されたPRのコメントの例が分かりやすいので、引用します。

Imagine, somewhere within the request flow, perhaps automatically scoped by Pundit,

ruby current_user = User.first posts = Post.where(author: current_user)

And then later on in a controller…

ruby published_posts = posts.where(published: true) draft_posts = published_posts.invert_where

The draft_posts variable is now all draft posts NOT by the current user. Putting the above logic in published and unpublished scopes, respectively, would be make this issue even more confusing to debug if it did occur.

このケースではwhere(published: true)を反転させたい意図のはずが、誤ってwhere(author: current_user)も反転させてしまっています。

3. default_scopeと組み合わせた場合

また、invert_whereとdefault_scopeが組み合わさった場合、予期せぬ挙動になります。 次のコードを見てみましょう。

class User < ActiveRecord::Base
  default_scope -> { where(disabled_at: nil) }

  scope :admin, -> { where(role: 'admin') }
end

puts User.admin.invert_where.to_sql
# => SELECT "users".* FROM "users" WHERE NOT ("users"."disabled_at" IS NULL AND "users"."role" = 'admin')

この例ではdefault_scopeを使って無効なユーザーを常に除外しています。 そして最後のUser.admin.invert_whereでは、「(有効な)admin以外のユーザー」を取得しようとしています(カッコ内はdefault_scopeによる条件)。

ところが出力されたSQLを見ると分かるとおり、意図とは違ったクエリが出力されています。 このクエリは「(有効かつadmin)ではないユーザー」、つまり「無効、またはadminではないユーザー」を出力しています。

default_scopeはそもそもとてもバグの温床となりやすい機能なので使うことを避けたほうが良いですが、invert_whereと組み合わさると更にバグを生みやすくなります。

まとめ

invert_whereにはこのような問題があるため、バグを生みやすいメソッドだと思います。

この問題を常に念頭に置きつつコードを書ければ実際にバグにはならないでしょうが、なかなかそうはいかないでしょう。 今回挙げた2番目の問題はついうっかり書いてしまうことは考えられますし、Railsに明るくない人が見様見真似で危険な書き方をしてしまう可能性もあります。

このリスクを軽減しつつinvert_whereを使うため、RuboCop Railsにcopを提案しました。

github.com

とはいえ危険なメソッドであるとは思うので、Rails 7リリースまでにrevertされてほしいなと、今回調査して感じました。