Railsアプリケーションのテストが失敗したときにどうしたらいいのか

本エントリはiCARE Advent Calendar 2020の25日目です。

僕はiCARE社内で技術顧問としていろんなことをやっていますが、そのうちの一つとしてRailsアプリケーションのテスト改善があります。具体的には「たまに失敗するテスト」で難しいものがあったときに調査して解決をしています。この「たまに失敗するテスト」はiCAREに限らず、ほとんどの会社が苦しめられているのではないでしょうか。僕のお手伝いしている他の会社でも同様なので、複数社の社内ドキュメントツールに「こういうふうに調査するといいですよ」という文章を書いています。しかしこれらはどれも社内wikiどまりで、現時点で公開されている文章が存在していません。

そこで今回この場を借り「失敗したテストがあったときにどうしたらいいのか」の決定版を書いて、今後は「これ読んでおいてください」で済ませたいなと思っています。

前提

RailsアプリケーションのテストをRSpec、Capybara、Selenium、headless chromeを利用して書いている

まず、悩むのをやめよう

テストが失敗したとき、その原因になり得るものはたくさんあります。

• テストコードが悪い
• アプリケーションコードが悪い
• テストの環境が悪い

テストコードやアプリケーションコードそのものが原因であれば比較的簡単に修正することができるはずです。しかし、それ以外に原因があったときに悩んでしまう人が多い気がします。しかし悩んでも解決することはまずありません。大事なのは必要な情報を収集することです。悩む前にありったけの情報をかき集めましょう。

• binding.pry、binding.irb、byebugなどのブレークポイントを活用し、該当するテストコードやアプリケーションコードが想定通り動いているかを確認する

• E2Eのテストであれば、スクリーンショットをとりテスト失敗時の画面の状況を見る

  • CI側の設定で見れない(Artifactsが取得できない)のであればまずCIの設定を直すところから始めてみましょう
  • スクリーンショットで足りない場合は、headlessではないchromeを動かすとブレークポイントで実際どのような画面になっているのか目視できるので便利
    • docker環境だと↑はできませんが、chromeのremote debugging機能を使うことで同等のことができます。やり方はRails アプリケーションの不安定なテストを撲滅したい 〜system spec のデバッグ方法とテストを不安定にさせる要因〜 - あらびき日記 が参考になります
• テスト用のログ(例: log/test.log)を確認してサーバが想定通りリクエストを受け取り、レスポンスを返しているかを確認する
• CIでたまに失敗するテストであれば、ローカルで再現するか試してみる
  • 例えば次のように書いて rspec --fail-fast ファイル名:行番号 とすると1000回テストを実行して最初に失敗したところで止まります。これが全部通るのであれば、順番依存などで失敗するテストの可能性が高いです。

1000.times do
  it do
    # たまに失敗するテスト
  end
end

テストがどのように動いているかを学ぼう

ログやスクリーンショットを見てもよくわからん、となるとき、そもそもテストがどう動いているかについてよく知らないのであたりがつけられない可能性があります。テストはあくまで本番を模して確認するためのもので、本番とは違ったテクニックで実現されています。それらのテクニックを把握してると、失敗の原因を推測できる可能性が高まります(もちろん本番環境がどのように動いているのか知るのも大事です)。

以下抑えておいてほしいテストの仕組みについて書いています。

テストの設定を知ろう

テストは基本的に次の3つのファイルで設定されているはずです。特に「開発環境だとちゃんと動いているように見えるんだけどテストだとなぜかうまくいかない」ようなケースではtest.rbとdevelopment.rbをみて、どのような差分があるかチェックしてみるとよさそうです。

• config/environments/test.rb
• spec/rails_helper.rb
• spec/spec_helper.rb

例えばActive Jobは各環境で別々のアダプタを使っている事が多いです。本番時にはsidekiqを利用するけれど、開発時にはasyncアダプタ(スレッドを利用した非同期実行)を利用し、テスト時にはtestアダプタ(テスト用のキューにジョブの情報を入れるだけでジョブ自体は実行しない)を選択するような設定が一般的です。

このように、テストを実行するために本番や開発時とは違う形にしている箇所はいくつかあるはず。思わぬ箇所の設定が影響していることがあるので、設定のそれぞれの行がなにを表しているのか説明できるようになっているのが望ましいです(大変だけど)。

各種設定の内容はRailsガイドにまとまっています。読んだけどよくわからない、という場合はお近くの技術顧問に質問ください。

テスト中に行った変更は必ずもとに戻そう

複数のテストを実行するときは、それぞれ独立した環境でテストを実行したいところです。例えばあるテスト中にレコードをinsertしたら、テスト終了時に何らかの形でDBをロールバックしてから次のテストを実行しないとテストが不安定になってしまいます。

RailsはデフォルトでDBのトランザクションを利用してロールバックする仕組みを持っています。最近のRailsプロジェクトだとすべてその方式(トランザクションを利用してロールバックする)で統一できますが、昔のRailsプロジェクトだとfeature spec用にdatabase_cleanerやdatabase_rewinderを利用してDBを毎回空にして対応しているケースもあります(詳細はコラム参照のこと)。

つまり、DBに関しては特に意識しなくても自動で元の状態に戻るようなっているはずです。またrspec-mocksによるモックの設定などもテスト終了時に自動でもとに戻してくれています。べんりですね。

しかし、ツール側で対応していないものは手動で元に戻さないといけません。例えば、csvファイルを出力するメソッドをテストする場合、作ったcsvファイルをテスト終了時に削除しないとファイルがずっと残ってしまいます。そのcsvファイルの有無で挙動が変わるようなアプリケーションコードがあったら、別のテストが失敗してしまうかもしれませんね。

このように、テスト中の変更をもとに戻さないと順番依存で失敗するテストが出来上がるので、意識して変更をもとに戻すようにしましょう。順番依存で失敗するテストは再現がなかなか難しいです。機械的に順番依存失敗テストを再現させる方法としてはrspec の --bisectオプションがあります(ローカルで大量のテストを--bisectするのは骨が折れるので、実際に適用できるケースは少なそうですが)。

参考: Rails tips: ランダムにコケるRSpecテストの修正に便利なbisect（翻訳）

[コラム] feature specと system specの違い

feature specでは、テストコード用のスレッドとテストサーバのスレッドが異なります。結果としてDBのコネクションが別々になり、テストコード用のスレッドでトランザクションを利用するとテストサーバ側のスレッドはそれを参照できません。つまりトランザクションによるロールバックの仕組みを利用できないことになります。なのでfeature specではそれぞれの変更を都度COMMITし、テスト終了時にTRUNCATE文やDELETE文でレコードを削除するのが通例でした。それを自動で行うためにdatabase_cleanerやdatabase_rewinderのgemが使われていました。Rails 5.1から利用できるsystem specではこの仕組をハックして各スレッド間で同一コネクションを使えるようにして、E2Eのテストでもトランザクションを利用できるようにしています。そのため、system specではdatabase_cleanerやdatabase_rewinderは必須ではなくなっています。

E2Eテストの仕組みを知ろう

E2Eのテストは実際にブラウザを動かしてテストするため他のテストよりも複雑で、デバッグも大変なことが多いです。まず、昨今の一般的なRailsのE2Eテストではheadless chromeを利用することが多いと思いますが、その際には次の3つのプロセスが起動します(アプリケーションサーバにpumaを使用していると仮定)。

• capybara(selenium driver)とpuma
  • ※ capybara(selenium driver)とpumaは同一プロセスだが別スレッドで動いている
• chrome driver
• chrome

capybara がchrome driverのAPIを叩き、chrome driver が chrome のAPIを叩き、chromeはpumaにアクセスするという流れ。

それぞれが独立したプロセスで動いているので、テストを実行している最中に前のテスト関連のリクエストがpumaに届く、というケースがあります。ajaxで非同期にリクエストを投げる画面で起こりやすいです。これが原因でテストが時々落ちることがあります。

たとえばdeviseを利用しているプロジェクトでrequest specを実行するときに、deviseが提供するsign_inというヘルパメソッドを利用してログイン済み状態のリクエストを作ることがあります。このsign_inメソッドは、「次のリクエストをログイン済み状態として扱う」という挙動をします。これと先ほど説明した「テスト中に前のテストのリクエストが届く」を組み合わせるとたまに失敗するテストの一丁上がりです。前のテストのリクエストでsign_inメソッドによるログイン済み状態が解除され、request specが投げるリクエストが未ログイン状態になりテストが失敗します。

[コラム] chrome driverとchromeのバージョンを合わせる

chrome driverはバージョンごとにサポートしているchromeバージョンが違うので、ここが一致していないとE2Eのテストが全部失敗したりします。webdrivers gemを利用するとテスト実行時に自動でインストール済みのchromeに合ったバージョンのchrome driverをダウンロードしてくれるので積極的に使いましょう。

ただしwebdriversは並列テスト時に同時にchrome driverをダウンロードしようとして挙動がおかしくなることがあります。そのようなケースでは並列テストを実行する前に明示的にWebdrivers::Chromedriver.updateを実行してchrome driverをダウンロードすると回避できます。

capybaraの要素が現れる(or 消える)のを自動で待ってくれるのを知ろう

E2Eテストで問題になるのはajaxを利用している箇所とアニメーションを利用している箇所が多いのではないかと思います。

何かをクリックしたときにJavaScriptによって新しい要素が表示され、その新しい要素をクリックしたい場合は通常と変わらずclick_on '新しい要素'のように書きます。このときAPIを実行するなどでJavaScript中の処理が遅くclick_on実行時に新しい要素が表示されていないような場合、capybaraは一定時間待って再度要素を探す、ということを繰り返します。最大でCapybara.default_max_wait_time秒待ちます。デフォルトでは2秒ですが、5秒などにしているプロジェクトも多いのではないでしょうか。

これだけみるとJavaScript(ajax)を利用した画面でもなにも考えずにテストできるように思えますが、そんなことはありません。明示的に、何かが起きるのを待つためのコードを挿入しないとテストが失敗するケースがあります。

例えばモーダルダイアログがリンクの上に表示されているようなケースで「モーダルを閉じるボタンをクリックする→リンクをクリックする」という処理を書いたとします。このときはモーダルを閉じるときにすでにリンク自体は存在するので、先程書いた「要素が現れるのを待つ」は実行されません。なのでモーダルを閉じるのに時間がかかると、次のリンククリックに失敗します(上にモーダルが残っているのでクリックできない)。そのようなケースでは、リンクをクリックする前にモーダルが閉じられていることを明示的に確認し、もしモーダルが閉じられていなかったら待つようにするとうまくいくでしょう。例えば page.has_no_css?('.modal') のように書くとそれができます。

関連: capybaraの公式ドキュメント

capybara(selenium)によるクリックの仕組みを知ろう

capybaraのクリックの挙動も知っておく必要があります。capybaraは要素をクリックするときに

1. 要素の座標を取得する
2. 取得した座標をクリックする

という手順をとります。このとき1と2の間にタイムラグがあるため、そのときに

• cssアニメーションの途中である
• 画像ローディングの途中である

などの理由により要素が移動するとクリックに失敗します。

cssアニメーションをオフにする機能はcapybaraが用意してくれています(Capybara.disable_animation = true)。これはcssアニメーションをオフにするstyleタグをRackミドルウェアを利用してすべてのページに挿入することで実現しています。ただし、あくまでこのようなscriptやstylesheetがテスト中のHTML中に差し込まれるだけなので、ここでオフにしている以外の方法でアニメーションを実現していたら別の方法が必要です。

画像のローディングに関しては特に公式のサポートはないので、↓のようなヘルパメソッドを定義して必要なところで呼び出してあげるとよいでしょう。

  def wait_for_image_loading
    Timeout.timeout(Capybara.default_max_wait_time) do
      sleep 0.5 until evaluate_script(<<~JS)
        Array.prototype.every.call(
          document.querySelectorAll('img'),
          (e) => e.complete
        )
      JS
    end
  end

まとめ

ここまでテスト固有のTipsについて書いてきました。

しかしRailsアプリケーションのデバッグに必要な知識はこれにとどまりません。Rubyそのものに対する知識も必要ですし、Railsを始めとしたgemのソースコードを読み解かなければ解決できない問題に遭遇することもあるでしょう。他にもプロセス、スレッド、ソケットやJavaScriptなど、Railsアプリケーションが関わっている技術の知識も欲しいところです。

範囲が膨大で大変ですが、近道はないので少しずつ勉強してきましょう。僕もがんばります。

APIに利用制限をかけるとしたらどういうやりかたがあるのか

この記事はSmartHR Advent Calendar 2020 11日目の記事です。

僕のお手伝いしているSmartHRでは、毎週バックエンドエンジニアが集まり、技術的なトピックについて共有、相談しあうミーティングを開催しています。そのミーティングでは僕がTipsなどを共有するコーナーが常設されています*1。

このエントリでは、そのコーナーで共有した内容をひとつ紹介します。

APIに制限をかける方法について

APIを外部に提供するとき、一定の制限をかけてユーザがAPIを乱用するのを防ぐことはよくあることではないでしょうか。素直に考えると「1時間に5000回までAPIを実行できる」のようなやり方を思いつきますね。GitHubのAPIもそのやり方ですし、SmartHRのAPIも同様です。

じゃあそれでいいのでは。となるかもしれませんが少し待ってください。いろんなクライアントがAPIを大量に叩くことを想像してみてください。12:00、13:00、14:00 のような時間ごとにAPI回数をリセットすると仮定すると、例えば12:00~12:01の間に大量にアクセスが来てそれぞれのクライアントがリミットに達し、残りの59分間はほとんどリクエストが来ない、みたいなことになりそうです。そうすると1分間の爆発的なアクセスのためだけにサーバを増強しなければなりません。

もうちょっとアクセスが均等になるような制限が欲しいと思いませんか？そこでLeaky Bucketアルゴリズムの登場です。

Leaky Bucketアルゴリズムとは

名前の通りAPIの利用を「穴の空いているバケツに水を入れる」と考えます。

次のようなアルゴリズムです。

• APIを利用するとバケツに水が入ります
• これ以上水を入れるとバケツから水があふれる！というときはAPIを利用することはできません
• バケツには穴が空いており、時間ごとに水が減っていきます

このような制限をかけることでクライアントに「ちょっとずつAPIを使う」ことを強いることができます。

ではこれをどうやって実装すると良いでしょうか。素朴に実装しようとすると、クライアントごとにバケツの水量を記録しておき、API利用時にそれを増加させ、一定時間ごとに減少させるような形になるでしょう。しかしそれだと「一定時間ごとに減少させる」のコストが高くなってしまいます。

例えば「1分毎に各クライアントの水を100減らす」という実装をしたとします。このときクライアントの数が多くなると、1分ではすべての処理が完了しないかもしれません。つまりこの実装はクライアントが多い場合は現実的ではありません。

同等のアルゴリズムで「一定時間ごとに減少させる」を不要にしたのがGCRA(Generic Cell Rate Algorithm)です

GCRA

GCRAは計算不要で自動的に増える値である「現在時間」を使って、時間を計算することでAPIの利用可否を判断します。

GCRAは水ではなくセルという名前をつかいますが、アルゴリズム的には同じです。他にもいくつか覚えないといけない用語があります。

用語名	内容
理論到着時間(TAT)	Tごとにセルが到着すると仮定したときの到着時間 。APIが実行されるたびにTの分加算される。現在時刻が加算前のTATより大きい場合もしくは初回は現在時刻+TがTATになる
T	セルが放出される間隔
τ	バケットの時間的な最大容量(バケットの最大容量 * T)

この用語でアルゴリズムを説明すると、「TAT-τが現在時刻以下であればAPI利用可能」となります。

なるほどわからん

具体的な数値をいれないとよくわからないですよね。仮の数値を入れて考えてみましょう。

前提

• バケットの最大容量2セル
• 1分毎に1のセルが放出される
  • つまりτは2分となる
• APIをそれぞれ12:00:00, 12:00:01, 12:00:02に実行した

12:00:00のAPI実行時

• TATは12:01(現在時刻+T)
• TAT - τが12:01 - 2分 = 11:59となり、現在時刻(12:00:00)より前なのでAPI実行可能

12:00:01のAPI実行時

• TATは12:02
• TAT - τが12:02 - 2分 = 12:00となり、現在時刻(12:00:01)より前なのでAPI実行可能

12:00:02のAPI実行時

• TATは12:03
• TAT - τが 12:03 - 2分 = 12:01となり、現在時刻(12:00:02)より後なのでAPI実行不可。実行するには12:01まで待つ必要がある

まだわからない

「なんかいい感じに均等にAPI利用制限できるアルゴリズムがあり、その名前はleakey bucket(もしくはGCRA)である」と覚えておいてください

使ってみたいけどイチから実装するのめんどくさいのでgemないですか

Sidekiq Enterpriseのv2.2から、GCRAのアルゴリズムで制限をかける機能が使えるようになっています。もともとSidekiq Enterpriseの機能として利用制限をかけるものはあったのですが、それにGCRA(leaky bucket)用のアルゴリズムが追加された形です。この機能はsidekiqのワーカ以外でも使うことができます。

次のようなコード(The Leaky Bucket rate limiter | Mike Perhamから引用)をRack Middlewareとして利用すると簡単にAPIサーバにleaky bucketな制限をかけることができます。

class LeakyLimiter
  def initialize(app)
    @app = app
  end

  def call(env)
    remote_ip = env["REMOTE_ADDR"].tr(':', '_')
    begin
      limiter = Sidekiq::Limiter.leaky("ip-#{remote_ip}", 10, 10, wait_timeout: 0)
      limiter.within_limit do
        @app.call(env)
      end
    rescue Sidekiq::Limiter::OverLimit => ex
      [429, {
        "Content-Type" => "text/plain",
        "X-Rate-Limited-For" => ex.limiter.next_drip.to_s,
      }, ["Rate limited"]]
    end
  end
end

もしこれから外部にAPIを公開していくぞ！となったときにこの話を思い出してみてください。

まとめ

だいたいこのような感じで毎週なにかしら共有をしています。自分も参加してみたい！という方はこちらのリンクからどうぞ。

*1:毎週ネタをひねり出すのに苦労しています＞＜

iCARE Dev Meetup #12 で登壇した

顧問先のiCAREさん主催のミートアップで登壇しました。技術顧問に対するインタビューを受けてから、Rails 6.1の新機能の話をするという構成。

技術顧問が語る最新Ruby on Rails/Vue.js iCARE Dev Meetup #12 - connpass

スライドはこちら。

speakerdeckだとPDFが持っていたリンク情報が消えてしまったので、gistにもほぼ同じ内容を置いておきました。どうぞご利用ください。

所感

こういう形でインタビューを受けるのは初めてでした。技術顧問についての話なのでそれなりに興味のある人はいそうだけど、基本的には僕の個人的な体験や感想でしかないので聞いてる人面白かったのかな…。

Rails 6.1の機能については時間が足りないことは予めわかっていたので話せるところまで話しました*1。急いで話して紹介する機能の数を稼いだけど、例えばstrict_loadingだけにするなど、数を絞ってそのテーマについて深堀りする、みたいな形でもよかったかもしれませんね。

あと話すつもりだったのだけどどこかのタイミングで抜け落ちてしまったトピックがあったので、気力があったら次のブログエントリで取り上げるかもしれません。

取り上げられてなかったけどdelegated typeが気になっている #icare_meetup

— expa / Shu OGAWARA (@expajp) August 21, 2020

*1:お手伝い先各社については興味ある人いたら再演する予定

パーフェクト Ruby on Rails改訂2版のサンプルコードについて

パーフェクト Ruby on Railsの改訂2版を書きました - おもしろwebサービス開発日記の続き。

いよいよ明日発売日ですね。前のエントリで書き忘れてたことがあったので追記です。

本書の6章からは、Railsのサンプルアプリケーションを作っていきます。技術評論社さんのサポートページからサンプルアプリケーションのコードを手に入れることができますが、GitHub上でも手に入れることができます。

6章の内容を読みつつ写経する方は、GitHubのリポジトリを活用すると写経が捗ると思います。本の流れに沿ってコミットを積んでいるので「写経したんだけどうまく動かない！」という箇所があったら、リポジトリを巻き戻して比較することでtypoに気づけるはず*1。各節終わりの段階でタグも作っているので、途中の過程を飛ばして気になるところだけ写経してみるということも簡単です。

特にビューを一字一句間違えずに写経するのは大変だと思うので、本と一緒にリポジトリをうまく活用して要点を掴んでいってもらえると幸いです(\( ⁰⊖⁰)/)。

*1:本のほうが間違えているのでは、と思ったら別途教えていただけると…＞＜

パーフェクト Ruby on Railsの改訂2版を書きました

ここ数年、色んな人に「パーフェクト Ruby on Railsの改訂版まだですか」と言われて申し訳ない気持ちでいっぱいでした。が、ついに改訂版が発売されることになりました！もちろん最新のRailsである6.0に対応しています。

発売日は7月25日ですが、先行して発売している書店もあるそうです。

パーフェクトRuby on Rails　【増補改訂版】：書籍案内｜技術評論社

ブログで振り返ると、第1版を書いたのは6年前だったようです。6年前といえばRailsは4.1がリリースされた頃で、フロントエンドはCoffeeScriptを書いてSprocketsでコンパイル、デプロイはCapistranoを使うのが主流だったような気がします。6年でだいぶRailsによる開発の進め方が変わりましたね。このあたりはもちろん第2版で更新されて、WebpackerやDockerに置き換わっています。

改訂2版の内容は第1版のものをベースにしていますが、単にRails6.0仕様に変更しておしまい、ではなくこの6年で著者陣に蓄積された知見をモリモリ追加しているので、ぜひぜひ手にとって一読のほどお願いします🙏

我々はConcernsとどう向き合うか

この文章は先日開催された大阪Ruby会議02での登壇内容Concerns about Concernsをブログエントリにしたものです。書いている内容は登壇内容とだいたい同じですが完全一致ではなく、構成を変更したり喋っていない情報を足したりしてます*1。

大阪Ruby会議02に出席していない方でもスライドを読めば大体の内容を把握できると思いますが、これだと細かいニュアンスは伝えられない(し、この手の話はその細かいニュアンスが大事だったりする)のでちゃんとブログエントリにしておこうと思ったのでした。

意見がある人はこちらのスレに書いてもらえると嬉しいです(\( ⁰⊖⁰)/)

Concernsとはなにか

Concernsという概念は、Rails 4.0から導入されました。具体的にはrails newしたときに生成されるファイルたちの中に

• app/models/concerns
• app/controllers/concerns

という2つの空のディレクトリが追加され、かつConcernsを定義するのに便利なモジュールActiveSupport::Concernが追加されました。基本的にはこれだけです。これだけなのですが、僕たちにConcernsという概念を認識させるには十分でした。

使い方に関してはDHHのブログに書かれています*2。

Put chubby models on a diet with concerns – Signal v. Noise

このブログに端を発して、いろんなブログや書籍などでConcernsの説明が書かれましたが、基本的には「Concernsは関心事を分離するものである」というようなことが書いてあるはずです。みなさんもそのように認識しているのではないでしょうか。

例えばDHHのブログ中では次のようなサンプルコードが書かれています。

module Taggable
  extend ActiveSupport::Concern

  included do
    has_many :taggings, as: :taggable, dependent: :destroy
    has_many :tags, through: :taggings 
  end

  def tag_names
    tags.map(&:name)
  end
end

このようにモデル中のタグ機能を分離するのはわかりやすい例ですね。しかし「関心事を分離する」というのは結構難しい作業です。僕は仕事柄いろんな会社のいろんなRailsプロジェクトのコードを読むのですが「名前をつけるのが難しいが処理としては複数のクラスで使われているので適当な単位でモジュールとして切り出されているなにか」をよく見かけます*3。

DHHは上記のブログエントリ内で、Concernsを使うとモデルの本質的ではない箇所を「別クラスに切り出して単一責任原則」とかせずに切り出せて便利である。と言っているように見えます(下記の意訳なのですが間違ってたらごめんなさい)。

Concerns are also a helpful way of extracting a slice of model that doesn’t seem part of its essence (what is and isn’t in the essence of a model is a fuzzy line and a longer discussion) without going full-bore Single Responsibility Principle and running the risk of ballooning your object inventory.

僕個人としては、このConcernsの使い方はよくないと考えています。恐らくDHHの所属するBasecamp社のエンジニアはみんな腕利きで、Concernsを大量に使っていても破綻せずに開発できているのではないかと思いますが、そのような会社はごくごく少数です。普通の会社でDHHの書いていることを鵜呑みにしてConcernsを多用すると負債になってしまうのではないでしょうか。

Concernsアンチパターン

ではどのようにConcernsを扱うのが良いのかをアンチパターンを提示し、その改善案を出すという流れで書いていこうと思います。

コントローラのビジネスロジックをConcernsにする

次のようなコントローラがあったとします。

class PostsController < ApplicationController
  def show
    @post = Post.find(params[:id])
    @same_category_posts = same_category_posts(@post)
  end

  # 他のアクション...

  private

  def same_category_posts(post)
    category_ids = post.category_ids
    same_category_post_ids = PostCategory.where(category_id: category_ids)
                                         .pluck(:post_id)
    Post.where(id: same_category_post_ids - [post.id])
        .includes(:categories)
        .order(updated_at: :desc).limit(5)
  end  
end

ここでprivateメソッドとして書かれているsame_category_postsは、引数として受け取ったPostオブジェクトと同カテゴリのPostオブジェクトを最大5つ返すメソッドです。

仮に、このメソッドが他のコントローラでも定義されていたとします。そうするとDRYにしたくなりますよね。ではConcernsとしてモジュールに切り出してみましょう。

module PostFindable
  extend ActiveSupport::Concern
  
  def same_category_posts(post)
    category_ids = post.category_ids
    same_category_post_ids = PostCategory.where(category_id: category_ids)
                                         .pluck(:post_id)
    Post.where(id: same_category_post_ids - [post.id])
        .includes(:categories)
        .order(updated_at: :desc).limit(5)
  end
end

はい。そのままモジュールとして切り出しました。するともとのコントローラは次のようになります。

class PostsController < ApplicationController
  include PostFindable

  def show
    @post = Post.find(params[:id])
    @same_category_posts = same_category_posts(@post)
  end

  # 他のアクション...

メソッドが減って、なんとなくスッキリしたような気がしますね。しかしこれは良くないリファクタリング方法です。

そもそもsame_category_postsメソッドはビジネスロジックであり、コントローラに書くべきものではないからです。そこでsame_category_postsをPostモデルに書いてみます*4。

class Post < ApplicationRecord
  def same_category_posts
    same_category_post_ids = PostCategory.where(category_id: category_ids)
                                         .pluck(:post_id)
    Post.where(id: same_category_post_ids - [id])
        .includes(:categories)
        .order(updated_at: :desc).limit(5)
  end
end

するとコントローラは次のようになります。

class PostsController < ApplicationController
  def show
    @post = Post.find(params[:id])
    @same_category_posts = @post.same_category_posts
  end

  # 他のアクション...

先程のConcernsを使った方法と対して変わらないのでは？という人もいるかも知れませんが、これはサンプルコードが単純だからそう見えるだけです*5。仕事で開発するRailsアプリケーションはもっと複雑なビジネスロジックを多数含んでいます。そのビジネスロジックがコントローラ、モデルの両方に存在していると、特定の処理を追いかけるときにいろんな場所を見に行かなければならず、コードの見通しが悪くなります。コントローラのConcernsはコントローラのコンテキストに存在するので、仮にコントローラそのものからメソッドが見えなくなっても「ビジネスロジックをモデルに寄せる」という原則からは外れることになります。

rubocopのClassLength対策でConcernsにする

開発フローにrubocopを取り入れており、ClassLength設定が有効なプロジェクトがあったとします。仮にClassLengthが300だとすると、次のように何らかの修正の結果Postモデルが300行を超えたときにCIが失敗します。

class Post < ApplicationRecord
  # たくさんのロジックが300行ある
end

CIが失敗したときにPostモデルを修正していた人は、CIを通すため、なんとしてでもPostの行数を減らさなければなりません。そこで次のようにしてみます。

class Post < ApplicationRecord
  include Previewable
  include Reservable
  include Bookmarkable
end

ここで新しく定義したPreviewable, Resarvable, BookmarkableはもちろんConcernsとしてモジュールに切り出したものです。これでPostの行数は劇的に削減され、ClassLengthに引っかかることはなくなりました。めでたしめでたし…本当にそうでしょうか？

これらのConcernsはPostでのみ参照されるモジュールです。そしてPostの持っているメソッドは何一つとして減っていません。単にpost.rbから別のファイルに移動しただけで、実行時にはPostモデルのメソッドとしてこれまでと同様に振る舞います。

このようなケースでは、Concernsとしてではなく別クラスとして切り出し、Postモデルの責務を減らす必要があります。このアンチパターンは抽象的*6なので、別のクラスとして切り出す実例は次の「複雑なビジネスロジックをConcernsにする」を参照してください。

現実にはこのアンチパターンを擁護する人もいそうです。そのときに考えられる意見としては「コードの塊に名前がついて切り出されたぶん少し可読性が上がったのでは？」というのがあります。しかし、Active Recordは同じことを同一ファイル内で実現する方法を提供してくれているので「名前をつけて切り出す」のであればこちらの方法で良さそうです。

class Post < ApplicationRecord
  concerning :Previewable do
    # ...
  end

  concerning :Reservable do
    # ...
  end

  concerning :Bookmarkable do
    # ...
  end
end

参考: Module::Concerning

rubocopのClassLength警告は「該当クラスが責務を持ちすぎである」ということの指摘であるはずなので、実質的な責務を減らさないConcernsは意味がないですし、モデルに所属しているコードが追いづらくなるのでかえって可読性を下げる結果になるのではないでしょうか。

複雑なビジネスロジックをConcernsにする

次のモジュールは、ぱっと見ただけだとなにをするものなのか分かりづらいですね。引数として渡されたjsonの形式を見て、モデルを更新または削除するメソッドupdate_by_api!を提供するものです。

module UpdatableByApi
  extend ActiveSupport::Concern

  def update_by_api!(json)
    parsed_json = JSON.parse(json)

    if parsed_json['_destroy'].to_i == 1
      resouce.destroy!
    else
      raise '不正な値です' if parsed_json['reason'].blank?
      resource.attributes = parsed_json.except('_destroy').merge('updated_by' => 'api')
      resource.save!
    end
  end
end

(このくらいならすんなり読めるよ、という人もいるかも知れませんが)全体として何が行われているか、すぐにはわかりづらいコードであると感じます。リファクタリングしてみましょう。モジュールではなく、別クラスに切り出す(委譲)の方式で試してみます。

class UpdatingByApi
  def self.call(json)
    new(json).call
  end

  def initialize(json)
    @json = json
  end

  def call
    if destroy?
      resource.destroy!
    else
      raise '不正な値です' if invalid?
      resource.update!(new_attributes)
    end
  end

  private

  attr_reader :json

  def parsed_json
    @parsed_json ||= JSON.parse(json)
  end

  def destroy?
    parsed_json['_destroy'].to_i == 1
  end

  def invalid?
    parsed_json['reason'].blank?
  end

  def new_attributes
    parsed_json.except('_destroy').merge('updated_by' => 'api')
  end
end

行数は長くなりましたが、次のような効能が得られました。

• それぞれの処理の詳細がプライベートメソッドに切り出されたことで詳細を把握しやすくなった
• #callはプライベートメソッドの名前として説明されているメソッド名を読むことで、詳細を見ずに概要を把握できるようになった

今回わざわざモジュールからクラスに変更してリファクタリングしましたが、「プライベートメソッドとして切り出す」だけならモジュールでもできそうです。モジュールで同様にプライベートメソッドへの切り出しをやってみましょう。

module UpdatableByApi
  extend ActiveSupport::Concern

  def update_by_api!(json)
    if destroy?
      destroy!
    else
      raise '不正な値です' if invalid?
      update!(new_attributes)
    end
  end

  private

  def parsed_json
    @parsed_json ||= JSON.parse(json)
  end

  def destroy?
    parsed_json['_destroy'].to_i == 1
  end

  def invalid?
    parsed_json['reason'].blank?
  end

  def new_attributes
    parsed_json.except('_destroy').merge('updated_by' => 'api')
  end
end

同じようにできましたね。ではわざわざクラスにする必要はないのでしょうか？

そんなことはありません。この例だと、invalid?メソッドがActive Recordの提供するinvalid?メソッドをオーバーライドしてしまっているため、どこか別のコードが上手く動かなくなっている可能性が高いです。

ではinvalid?をリネームしたら良いでしょうか？しかし他のメソッド名はどうでしょう。Active Recordの提供するメソッド名とは重複していませんが、別のモジュールやモデルで普通に定義していそうな名前ですよね。また、@parsed_jsonインスタンス変数も同様です。モジュールでプライベートメソッドやインスタンス変数を定義する場合、すべて他のモジュールやinclude先のクラスの定義と重複しないように気をつけて実装する必要があります。

では、Concernsはどのように使うとよいのでしょうか。例えば次のようにupdate_from_api!というインタフェースだけを提供するモジュールとして実装するという方法があります。update_from_api!メソッドの実態は別クラスとして実装することで、includeで手軽にメソッドを増やせるのと、コンテキストを分けることで実装をリファクタリングしやすくなるメリットがあります。

module UpdatableByApi
  def update_form_api!(user, json)
    UpdatingByAPI.call(user: user, json: json)
  end
end

hookをConcernsにする

次のように、Active RecordのコールバックをConcernsにしているコードを時々見かけます。

module PublishedAtSettable
  extend ActiveSupport::Concern

  included do
    before_create :set_published_at
  end

  private

  def set_published_at
    self.published_at ||= Time.zone.now
  end
end

class Post < ApplicationRecord
  include PublishedAtSettable
end

しかし、コールバックを共通化して複数のクラスに適用したいのであれば、次のようにコールバックメソッド(この例だと before_create)にオブジェクトを渡すやり方でも実現できます。

class SetPublishedAt
  def before_create(publishable)
    publishable.published_at ||= Time.zone.now
  end
end

class Post < ApplicationRecord
  before_create SetPublishedAt.new
end

こちらのほうが、「Postモデルはbefore_createコールバックを定義している」ということがわかりやすくなり可読性が高いのではないでしょうか。

まとめ

今回の話で特に言いたかったことをまとめると次の3点です。

• Concerns(module)はinclude先とコンテキストを共有するので、クラスから単純にConcernsに切り出しても責務が減るわけではない
• 責務を減らしたいのであればクラスとして切り出したほうが良い
• 安易にConcernsとして切り出すのではなく、まず別の方法も検討してみましょう

Concernsとして処理を切り出すと、目の前からコードがなくなるのでなんとなくリファクタリングできた気分になりますが、これは部屋にあるたくさんの荷物を部屋の隅に寄せて掃除をした気分になるのに似ていると感じます。これからConcernsを使うときには、これで本当に可読性が上がるのか一度考えてみてもらえると嬉しいです！

*1:例えばActiveSupport::Concernの説明は省略しています。ググればわかるので

*2:Railsガイドなど公式のドキュメントには特に言及がありません

*3:複数のクラスで使われていないけどなぜか切り出されているモジュールもたまに見かけます

*4:複数モデルが関わるメソッドなので別のクラスを作りそこにメソッドを定義すべき、という人もいるかも知れませんが、今回の主題から逸れるので一旦Postモデルで話をすすめます

*5:シンプルなコード例でもっといい感じに違いを表現できる方法が思いつかなかったのでした…

*6:実際に300行のPostモデルを用意するのは難しすぎ

Ruby on Rails 6 エンジニア 養成読本という本を書きました

@sugamasaoさんと共著でRails本を執筆しました。Railsを始めたばかりの人向けの特集から、Rails 6の新機能紹介まで幅広く書かれたムック本です。今日から9日後の10月26日に発売予定です(電子書籍も同じくらいに発売されるはず)。

Ruby on Rails 6 エンジニア 養成読本

posted with amazlet at 19.10.16

すがわら まさのり 前島 真一
技術評論社
売り上げランキング: 2,448

Amazon.co.jpで詳細を見る

@sugamasaoさんの書籍紹介エントリはこちら

sugamasao.hatenablog.com

内容紹介

@sugamasaoさんが目次や対象読者を紹介しているので、こちらでは具体的にどんな内容を扱っているかを箇条書きにしておきます。Railsの新機能を追いかけていない人にとっては、なにこれ？となったり、名前は知ってるけど具体的にどんな感じなのかは知らないぞ？となったり、内容知ってるけど今どうなってるんだっけ？となる単語が多いのではないでしょうか。

• Action Text
• Action Mailbox
• 複数DB対応
• 並列テスト
• Webpacker
• Sprockets
• Turbolinks
• Stimulus
• Active Storage
• Credentials
• Early Hints
• CSP

全体を通して、普通のRails本だとあまり取り上げられないだろうトピックが多めかと思います。例えばStimulusについては日本語の情報はほとんどないはず。

この中から興味のあるトピックを拾い読みする、というのがよい使い方ではないでしょうか。もちろん通して読んでもらっても 🙆です。ぜひお買い求めください(\( ⁰⊖⁰)/)