Rails: コントローラで避けたい3つのパターン（翻訳）

Rails: コントローラで避けたい3つのパターン（翻訳）

MVCアーキテクチャのコントローラ層は、モデルが捉えたドメインロジックを活かしてビューを構築し、アプリケーションのユーザーに返すという重要な役割を果たします。Railsアプリのコントローラは、この作業の完了を支援する便利機能を豊富に提供しています。しかしそうした便利機能の中には、どちらかというと害になる厄介な部分も潜んでいます。本記事では、そうした害を避ける3つのベストプラクティスを紹介します。

🔗 はじめに

MVC（モデル・ビュー・コントローラ）のアーキテクチャ設計は、Webアプリケーションを構築する定番の選択肢として定着しています。これは、DjangoやASP.NET、Railsなど多くの人気フレームワークの基盤となっています。

モデル層の責務は、アプリケーションのデータを永続化し、取得することです。

ビュー層とは、ユーザーに表示されるものです。ビュー層には、アプリケーションに含まれるデータの表現が含まれており、このデータと対話するためのインターフェイス（フォーム、リンク、ボタンなど）を含む場合もあります。

コントローラ層は、ユーザーが対話する相手です。コントローラ層は、ビューで表示するデータを要求したり、データを挿入・更新したりします。コントローラはユーザーからの要求を受け取り、モデル層へアクセスしてデータを取得または更新し、適切なビューをユーザーに返します。コントローラはこのように、モデル層とビュー層の間で協調動作することで実際に機能するアプリケーションを作成するものであるとみなせます。

Railsアプリケーションのコンテキストにおけるルーター（router）は、受信したリクエストを受け入れ、どのコントローラーアクションにリクエストを処理させるかを決定する責任があります（アクションが存在する場合）。

リクエストを受け取ったコントローラーは、多くの場合重要なタスクを担当します。たとえば、対象のユーザーがアクションにアクセスする権限を持っていることを確認し、リクエストパラメーターを解析してサニタイズし、ドメインモデルとやり取りし、ユーザーに返すビュー（通常はHTML）を構築します。

Railsは、コントローラーにおけるさまざまな重要な作業をサポートするために多くの機能を提供しています。ほとんどの機能は必要かつ有用ですが、私は、一部の機能は少なくとも疑わしいか、さもなければ選択肢としてまったく不適切であると考えています。

それではアンチパターンを避けるためのリストを見てみましょう。

🔗 1. コントローラのインターフェイスには必要な制約を加えること

Railsのコントローラーは、アクション（つまりコントローラーのメソッド）を公開する形でフレームワークに組み込まれます。

これらのアクションは、config/routes.rbで設定されたアプリケーションルーティングから参照可能であり、アクションは各コントローラークラスのpublic APIとなります。これらのAPIは、以下のように必要かつ十分なものであるべきです。

• 必要条件：アプリケーションのルーティングをサポートしない非公開のコントローラメソッドが存在すべきではありません。
• 十分条件：アプリケーションで公開される個別のルーティングには、それをサポートするコントローラアクションが存在するべきです。

必要条件の基準に違反する例を、以下の架空のBarsControllerで示します（十分条件の基準については、この後の2で説明します）。なお、本記事のコードサンプルは、GitHubリポジトリで参照できます。

# config/routes.rb
Rails.application.routes.draw do
  …
  resource :bar, only: [:show]
  …
end

# app/controllers/bars_controller.rb
class BarsController < ApplicationController
  def show
    create unless @bar
    # このアクションはapp/views/bars/show.html.erbをレンダリングする
  end

  def create
    # Barのインスタンスを作成する
    # このメソッドは、単にshowアクションをサポートするために存在する
  end
end

config/routes.rbから見ると、BarsContollerに存在するのはshowというアクションだけであることがわかります。しかし、私たちはうっかりcreateメソッドをBarsControllerのpublicインターフェイスに混入させてしまいました。このメソッドはshowアクションをサポートするためだけのつもりでしたが、publicメソッドとしても利用可能になってしまっています。

これは、メソッドをprivateにするだけで簡単に修正できるので、さほど大きな問題ではありません。

しかし、コントローラークラスのAPIを適切に制御しないと、以下の2つの問題が発生します。

• 問題1: コントローラのメソッドを意図せずにpublicにしてしまうことは、アプリケーション内で意図しない情報漏洩が起こる一歩手前ということです。
  情報漏洩は、設定を誤ったルーティングと運悪く一致しただけで発生する可能性があります。悪意のあるユーザーは、その時点でコントローラのメソッドをアクションとして利用可能になってしまいます。
  この時点で引き起こされる可能性のある被害は、システムの詳細によって異なりますが、私たちは絶対に自分たちを危険にさらすことを選んだりしません！この脆弱性が成立するには、誤まって配置されたルーティング名とコントローラのメソッド名が、非常に運の悪い形で一致する必要があります。
  上のコード例では、createという一般的な名前のコントローラメソッドを使っていますが、これは単純な設定ミスで簡単に公開されてしまう可能性があります。コントローラ内でメソッド名をカスタマイズしておけば、ルーティングアクションとの意図しない衝突はほぼ起こりませんが、それでもこのリスクをわざわざ冒す必要はありません。

• 問題2は、開発者のエルゴノミクスに関連しています。
  コントローラーのソースコードを読むチームメンバー（または未来の自分自身）が、「コントローラークラスのpublicメソッドは、アプリケーション上で公開されているルーティングと対応するべき」であると期待するのは当然です。この契約に違反すると、コントローラーコードの理解しやすさや保守性が低下してしまいます。

なお、traceroute gemは、プロジェクト内でこの原則に違反している箇所を見つけるのに有用です。アプリケーションで設定されているすべてのルーティングを考慮し、ルーティングで到達できないコントローラーアクションをハイライト表示します。

通常、これは「コントローラー上でpublicになっているメソッドをprivateにしなければならない」か、「以前サポートしていたルーティングをルーター設定から削除したが、関連するコントローラーアクションを整理していなかった」かのどちらかを意味します。どちらの場合も緊急で対応が必要です。

ダミープロジェクト上で以下のようにtraceroute を実行すると、BarsController#create に設定されたルートが存在しないか到達不能であることを検出できます。

> rake traceroute
…
Unreachable action methods (3):
  …
  bars#create

なお、誤って設定したルーティングによって既にコントローラメソッドがpublicになってしまっていた場合は、明らかにtracerouteは助けになりません🥴

🔗 2. アクションメソッドを不在にしないこと

突然妙な話をしているように思われるかもしれません。特定のルーティングが存在していれば、そのルーティングをサポートするために、それに対応するコントローラーメソッドも明らかに存在しなければならないんですよね？

実はそうとは限りません。適切な名前付きビューテンプレートが利用可能になっていれば、ルーティングに対応する名前付きメソッドが必ずしもコントローラ内に存在していなくても動作するのです。

たとえば、config/routes.rbのエントリーと、それに対応するPagesControllerの実装を考えてみましょう。

# config/routes.rb
Rails.application.routes.draw do
  resources :pages, only: [:index] do
    get :has_template, on: :collection
  end
end

# app/controller/pages_controller.rb
class PagesController < ApplicationController
  before_action { @other_stuff = "Yes action still fires" }

  def index
  end
end

このとき、app/views/pages/has_template.html.erb というテンプレートが存在しているとしましょう。このテンプレート名はconfig/routes.rb 内のアクション名と一致しています。

ここでは、次のような単純なテンプレートをサンプルとして利用します。

<h3>Template with no action</h3>
<p>Hi, I'm a template with no action</p>
<p>Here is a non-existent instance variable <%= @stuff %></p>
<p>Does before_action fire?  <%= @other_stuff %></p>
<%= link_to "<< Back to index", pages_path %>

ここでRailsサーバーを起動し、/pages/has_tempateをブラウザで表示すると、レンダリングされたhas_template.html.erbテンプレートが表示されます！
つまり、ルーティングが定義されていて、かつ適切な名前のテンプレートが存在していれば、コントローラーに対応するアクションが定義されていなくてもテンプレートは表示されるのです。

ここで注意していただきたい点を以下に述べておきます。

• コントローラーは必ず存在していなければなりません。コントローラー自体が存在しない場合はuninitialized constant PagesControllerエラーが発生します。
• Railsのレンダリングプロセスは非常に寛容にできていて、インスタンス変数（例: 上記の@stuff）が存在していなくても、エラーなしでレンダリングされることがよくあります。
• コントローラーアクションは通常どおり実行されます。コントローラにメソッド定義が存在していなくても、コントローラは空のメソッド定義があるかのように振る舞います。

class PagesController < ApplicationController
  before_action { @other_stuff = "Yes action still fires" }

  def index
  end

  # この空のメソッド定義が存在しなくても振る舞いが変わらない
  def has_template
  end
end

この振る舞いは混乱の元となり、潜在的なセキュリティ上の問題となります。開発者がRailsアプリケーションを理解しようとする際には、ルーターがpublicインターフェイス（つまりユーザーに公開されるURL）とサーバーロジックの接続を提供します。通常、これはルーターが特定のコントローラーアクションにURLを接続することで実現します。

ルーティングに対応するコントローラーアクションが不在になっていると、この基本的な前提が崩れてしまいます。さらに、リファクタリングでコントローラーアクションを削除すると、関連するルーティングとテンプレートの整理を忘れてしまう可能性もあります。そのような状況では、望ましくないものがユーザーに露出しやすくなってしまいます。

私はこの振る舞いを決して当てにしないようにしています。Railsからこの「魔法」を取り除くべきかどうかについて激しい論争もあると思います。私は、この振る舞いが望ましいと考えられる強力なユースケースを思いつけません。仮に何らかのマイナーなユースケースで必要だったとしても、これをフレームワークのデフォルトの振る舞いにせずに、明示的に同じ結果を得る方法は他にもあるのです。

まとめると、アプリケーション内の各ルーティングは、実在する既存のコントローラーアクションを指すようにしておくべきです。このアクションメソッドの中身は空でも構いませんが、安全のためにメソッド自身は存在するべきです。

ここでも、Railsアプリ内でtraceroute gem を使えば、対応するコントローラーアクションが存在しないルーティングを特定できます。それによって、このアドバイスに違反している箇所を特定し、対応するコントローラーアクションメソッドが存在しないルーティングを誤って公開しないようにできます。

以下のようにダミープロジェクトに対してtracerouteを実行すると、PagesController#has_templateは「未使用のルーティング」の可能性があることが表示されます。

> rake traceroute
Unused routes (6):
  pages#has_template
  …

🔗 3. フィルタをexceptで設定しないこと

コントローラーのフィルタ操作は、好き嫌いを別にしても、リクエストの横断的な関心事を実装するための強力なメカニズムであることは確かです。

たとえば、架空のFoosControllerでキャッシュ管理を行いたい場合、after_actionフィルタを使います。

class FoosController < ApplicationController
   after_action :clear_cache, except: [:show]

   def show
      # レコードを取得して表示する
   end

   def update
      # レコードを更新する
   end

   private

   def clear_cache
      # キャッシュを削除する
   end
end

このコントローラーは、showとupdateの2つのアクションを公開しています。updateアクションでは、更新したモデルのキャッシュをクリアしたいと考えています。モデルを表示するだけの場合にはキャッシュをクリアしたくないので、次のパターンを利用してこれを実現したとします。

after_action :clear_cache, except: [:show]

こういう「除外」リストを使うと、以下のようにコントローラーに新しいアクションを追加したときに、除外リストの更新を忘れてしまう可能性が生じてしまいます。

class FoosController < ApplicationController
   after_action :clear_cache, except: [:show]
   …
   def edit
      # 編集フォームを表示するだけなので、キャッシュをクリアしてはならない
   end
   …
end

この場合、editアクションは単にモデルの更新用フォームを表示するだけであり、実際には背後のモデルの更新を表していません。すなわち、このeditアクションではキャッシュをクリアしません。しかし、私たちはexcept句を更新し忘れてしまっていたので、このeditアクションによって不要な（おそらく高コストな）キャッシュ削除が引き起されます。

今の例は非常に具体的でしたが、これは一般的な問題です。このような文脈では、一般に除外リストではなく「許可リスト」を使うべきなのです。

許可リストによる方法なら、after_action をトリガーするタイミングを明示的に指定することが義務付けられるので、意図しないフィルタが誤って発動することを防げます。

class FoosController < ApplicationController
   after_action :clear_cache, only: [:update]
   …
   def edit
      # editフォームを表示するだけの場合はキャッシュのクリアをトリガー「すべきではない」
   end
   …
end

その一方で、除外リストによる方法を支持する正当な議論も存在します。除外リストを正当化できる強力な反論のひとつは、以下のようなセキュリティ層（認証や認可など）の設定時です。

class PrivateStuffController < ApplicationController
   before_action :ensure_permission!, except: [:something_public]
   …
end

この議論は理解できますし、私自身もこのパターンを使っています。このパターンでは、まずデフォルトで「あらゆる」アクションの前に:ensure_permission!を実行する形で安全策を取ろうとしており、その後一部のアクションだけを除外することになります。そして、今後別のアクションが追加された場合にフィルタを適用し忘れることのないよう、予防策を講じます。

しかし、私は時とともにこの方法が好きでなくなりました。

たとえば、認証のような基本的な機能については、認証用のアクションと認証用でないアクションを別々のコントローラーに分ける方が優れたソリューションになると感じています。

さらに、今後新しいアクションが必要になった場合は、アクションをどのように実装するかをその時点で選択する方が、問題を事前に解決しようとするよりも優れているでしょう。この議論は「早すぎる最適化」に関する議論と類似しており、またYAGNI原則とも密接に関連しています。

将来の互換性を重視する議論については、コストをかけずに実行できるのであれば、私ももっと共感できるでしょう。しかし、実際にはコストが存在します。

第1のコストとして挙げられるのは、意図しないフィルタがアクションの前（before）・後（after）・前後（around）で不要な処理を行ってしまう可能性です。こうした意図しないフィルタアクションは、最もましな場合でも計算時間の無駄遣いになりますし、最悪の場合はコントローラーアクションの機能やセキュリティに影響を及ぼす可能性さえあります。

第2のコストはメンテナンス性です。再びFoosControllerの例を取り上げますが、今度は時とともにいくつかのアクションが追加される場合を想定します。

class FoosController < ApplicationController
   after_action :clear_cache, except: [:show]
   def update
      # このupdateアクションを削除したいと思うようになる
   end

   def bar
      # 後で別のアクションを追加する
   end
   …
end

さて、このupdateアクションを削除したくなったとしましょう。after_actionフィルタを必要とするアクションはこれだけなのですが、このupdateアクションを削除することにためらいを感じてしまいます。

後からコントローラーに追加したbarアクションは、実はこのフィルタに暗黙で依存しているのでしょうか？これを疑い始めると、単純なはずのリファクタリング作業がややこしくなり始めます。barアクションがこの:clear_cacheフィルタに実際に依存していないことを確認する調査が多少なりとも必要になるでしょう。

つまり、:except句を使うと、フィルタロジックが意図に反して他のコントローラアクションに漏れ出し、メンテナンスの負担が増えることがわかります。

以上をまとめると、フィルタアクションの:except句はあっという間に劣化してしまうので、メンテナンス上の問題になる可能性があります。ベストな方法は、次のいずれかです。

• コントローラーアクション全体に、曖昧でない明示的なフィルタを適用する（つまり:only句や:except句を指定しない）
• 特定のアクションをフィルタで制限する必要がある場合は、明示的にそのアクションを許可リスト（:only句）で指定する

:except句の利用は控えるべきであり、可能な限り避けるべきです。

🔗 まとめ

本記事では、Railsのコントローラを作成するときに避けたい3つのパターンを紹介しました。

1. コントローラーの各publicメソッドは、アプリケーションのルーティングで実際にアクセス可能なアクションに対応させるべきです。
2. ルーティングには、常にアクションメソッドを提供すること（メソッドの中身は空であってもよい）。ルーティングに対応する名前付きテンプレートが自動的にレンダリングされるRailsの機能に依存してはいけません（さもないとアクションが未定義のままになってしまいます）。
3. コントローラーのフィルタを適用すべきアクションを絞り込む必要がある場合は、フィルタが必要なアクションを:only句でホワイトリスト化することを推奨します。:exceptの利用は避けること。

これについて同意しかねる点のある方や、リストに追加したいパターンが他にもある方は、ぜひ元記事にコメントでお知らせください。

本記事が役に立ったと思われましたら、ぜひメーリングリストの購読をどうぞ。新着記事をメールにてお知らせいたします。

参考情報

1. WikipediaのMVCパターン
2. Railsガイド: Action Controller の概要
3. Martin FowlerによるYAGNI記事
4. Railsガイド: Rails のルーティング
5. traceroute gem
6. Railsガイド: §8 フィルタ -- Action Controller の概要
7. コントローラのアンチパターンGitHubリポジトリ（コントローラの振る舞いをデモするためのRailsプロジェクト）

関連記事

Railsのパターンとアンチパターン4: コントローラ編（翻訳）