Railsの`CurrentAttributes`は有害である(翻訳)

概要 原著者の許諾を得て翻訳・公開いたします。 英語記事: Rails’ CurrentAttributes considered harmful 公開日: 2017/06/22 著者: Ryan Bigg: Ryan Bigg氏はRailsのcontributorであると同時にCulture Amp社のメンバーであり、RubyやElixirでの開発を行っています。RailsガイドのドキュメントやMultitenancy with Rails など多くの執筆実績があり、現在はDeep Dive Railsという書籍を執筆中です(現時点で45%↓)。 leanpub.com/ddrより RailsのCurrentAttributesは有害である(翻訳) 最近Rebecca Skinnerに教えてもらったこのcommitは、Railsアプリにいわゆる「グローバルステート(global state)」を効果的に導入するためのものです。 グローバルステートが一般によくない理由を述べる代わりに、Stack ExchangeのよくできたQ&Aのリンクをご紹介します。 ごく簡単に言えば、(グローバルステートがあると)プログラムのステートが予測不可能になります。 詳しくはこうです: 同じグローバル変数を共有しているオブジェクトが2つ(以上)あるとしましょう。ランダムさをもたらす不確定要素がモジュール内のどこにも使われていない前提条件においては、あるメソッドの実行前にシステムのステートが既知であれば、そのメソッドの出力は予測可能(したがってテスト可能)になります。 commitではスレッドローカルな変数も実装されていますが、このチョイスがよろしくない理由についてStack Overflowの回答から引用いたします。 テストの難易度が上がる: スレッドローカルな変数をコードで使うと、そのコードの外で書くテストでもスレッドローカルな変数を設定し忘れないよう注意が必要 スレッドローカルな変数を使うクラスは、オブジェクトが利用不可能になっているのではなくスレッドローカルな変数の内部にあることを認識できる必要がある: 通常、このような間接性(indirection)はデメテルの法則に反する スレッドローカルな変数をクリーンアップしないと、フレームワークでスレッドを再利用するときに問題が生じる可能性がある: スレッドローカルな変数が既に初期化され、変数の初期化で||= 呼び出しに依存するコードがコケる可能性がある CurrentAttributesがデメテルの法則にも反していることについては本記事では言及しません。ある日から突然、Currentがアプリのあらゆる場所で利用できるようになることを問題にしたいと思います。よいコードとは、メソッドや関数で利用できるようにする方法をコードで明示する(explicit)ものです。しかるに、CurrentAttributes機能はよくないコードであり、モデルでCurrent.userが利用可能になるまでの流れが明らかになりません。ただひたすら「魔法のように」そうしてくれるというだけです。 私はいつもRailsの魔法で楽しんでいますし、これまでにも随分と助けられてきました。render @collectionやポリモーフィックルーティングなどお気に入りの魔法はいくつもありますが、こうした魔法が優れているのはスコープが限定されているからです。コレクションをビューでレンダリングできることも、ポリモーフィックルーティングをコントローラやモデルやヘルパーで使えることも、ちゃんと察しがつきます。 私がいくら魔法が好きだからと言っても、CurrentAttributesの魔法はあまりに強力すぎます。スレッドローカルなグローバルステートを導入すると、Currentに実際に値を設定するコードが覆い隠されてしまいますし、値の出どころも暗黙(implicit)になってしまうからです。 「それ、コントローラで設定されるから」という反論もあるでしょう。ではコントローラがない場合はどうなるのでしょう。バックグラウンドジョブでは、あるいはテストではどうなるのでしょう。確かに、どちらの場合についても以下のようにCurrentで値を指定できます。 def perform(user_id, post_id) Current.user = User.find(user_id) post = Post.find_by(post_id) post.run_long_running_thing # ジョブで実行するコードをここに書く end 上のPost#run_long_running_thingは、Current.userにアクセスして現在のユーザーを取得するだけのシンプルなメソッドです。しかし、本当にそうなるかどうかはその場ではわかりません。Post#run_long_running_thing methodのことだけ考えれば、Current.userは最初の段階で設定されます。Current.userが他のどこかで設定されることぐらいはコードからもうかがえますが、この文脈ではどのコードで設定されるのかを突き止めるのは難しいでしょう。プロジェクトでCurrent.user =という文字列を検索しても、コントローラやジョブなど、変数を設定するあちこちのコードで見つかるでしょう。この文脈に合った本物のCurrent.user =は、果たしてその中のどれなのでしょう。 テストの場合も、Current.userに依存するコードがあればCurrent.userの設定が必要になるでしょう。以下のようなテスト例を考えてみましょう。 let(:user) { FactoryGirl.create(:user) } before { Current.user = user } it “時間のかかるテストを走らせる” do post.run_long_running_thing end やはり、run_long_running_thingメソッドを見てもテストコードを見ても、Current.userが設定されるタイミングは明らかになりません。 私の見る限り、このCurrentオブジェクトのステートをテストのたびにリセットすると考えられる部分はCurrentAttributesのコードにも見当たらないようです。したがって、上のテストコード例のように、あるテストでの設定が「魔法のように」他のテストに漏れてしまいます。こうした挙動がコードベースに存在するのは恐ろしいことではないでしょうか。テストでCurrent.userがnilになることを期待している状況で、他のいずれかのテストで別の値が設定されてしまうことは十分ありえます。アプリでテストを500回まわしたとして、その中のどのテストのしわざなのでしょうか。首尾よく見つけられるかどうかは運次第です。 「設定より規約」に続く原則は「暗黙的プログラミングより明示的プログラミング」ではないか 訳注: Rails生みの親であるDHHは、エッセイなどでたびたび暗黙的なプログラミングを賞賛しています。 Railsは今も優れたフレームワークです。私のこうした主張に対してDHHが「いやなら使うな」といった調子で反駁するであろうことも察しがつきます。おそらく、少し前に私がコールバックのsuppressを元に戻そうとしたときのDHHの反応↓と同じように。 Railsの憲章には、有効なユースケースもあればよくない使い方もある新機能の導入時に、プログラマーを自滅から保護することについては明記されていない。 #25115のDHHコメントよりより 要するにDHHの説得は私には無理だということです。こうした点について私とDHHの意見は大きく食い違っています。 私は、今回のCurrentの件のようにRailsが暗黙的プログラミングの極北に向けてひた走ることは、大きな間違いだと考えます。こうした機能によってRailsのコードベースでたくさんのフラストレーションが生み出されます。今回のような状況であれば、Railsはあえて暗黙的プログラミングより明示的なプログラミングを選ぶべきです。Railsには既に十分な魔法が備わっているのですから、これ以上新しい魔法を編み出す必要はないはずです。 今回の機能は私が求めていたものではありません(DHHはある日この着想を得てそれをよしとし、そのまま実装したようです)し、もっとよいやり方が他にもあります。たとえば前述のジョブコードだったら、次のように値を明示的に渡す方がずっとよいのではないでしょうか。 def perform(user_id, post_id) user = User.find(user_id) post = Post.find_by(post_id) post.run_long_running_thing(user) # ジョブで実行するコードをここに書く end テストコードも、次のように明示的に書くほうがずっと有利です。 let(:user) { FactoryGirl.create(:user) } it “時間のかかるテストを走らせる” do post.run_long_running_thing(user) end どちらのコードも、userがどのようにしてrun_long_running_thingメソッドに到達するかがはっきりわかります。何しろ引数として渡しているのですから。 最後に、今回のプルリクのコードを明示的プログラミングに沿って書き直せることを簡単にお見せしたいと思います。 対決! DHHのCurrentAttributesのコード vs 私の明示的プログラミングのコード # app/models/current.rb class Current < ActiveSupport::CurrentAttributes attribute :account, :user attribute :request_id, :user_agent, :ip_address resets { Time.zone = nil } def user=(user) super self.account = user.account Time.zone = user.time_zone end end # app/controllers/concerns/authentication.rb module Authentication extend ActiveSupport::Concern included do before_action :set_current_authenticated_user end private def set_current_authenticated_user Current.user = User.find(cookies.signed[:user_id]) end end # app/controllers/concerns/set_current_request_details.rb module SetCurrentRequestDetails extend ActiveSupport::Concern included do before_action do Current.request_id = request.uuid Current.user_agent … Continue reading Railsの`CurrentAttributes`は有害である(翻訳)