Rails アンチパターン - Fixtureの憂鬱(Fixture Blues)
引き続きRails AntiPatternsという本を読んでいます。
https://www.amazon.co.jp/dp/B004C04QE0/ref=dp-kindle-redirect?_encoding=UTF8&btkr=1
前回は4 Controllersの3つめ「Bloated Sessions」についてまとめました。 waterlow2013.hatenablog.com
今回は7 Testの1つめ「Fixture Blues」についてまとめます。
Fixture Blues
fixtureはRailsにデフォルトで組み込まれているテストデータ作成機能だが、以下のような難点がある。
- 共通化すると変更したときに壊れやすい。
- 毎回作っているとfixtureが大量にできてしまう。
Solution: Make Use of Factories
- FactoryGirl gemを使おう。
- 各フィールド のオーバーライドが容易なので、fixture量産しなくていい。
正直fixtureをそこまでちゃんと使ったことがいので、fixture vs factoryに関しては何も言えません。rspec bookもfactory girlを取り上げつつも反対する意見があることも書かれています。 fixture vs factoryに関してはググると色々出てきますが、以下の記事がわかりやすいかも。 blog.jnito.com
Solution: Refactor into Contexts
- testunitでもcontext分けが出来るようになるgemの紹介と、context入れると明示的でかつDRYになるという話。特にテストデータのセットアップ。
これもrspecしか使ったことがないので、良さに関してはよくわからず…。
rspecにおけるdescribe, context辺の使い方は以下のwillnet氏のrspec coding stileを読むと良さそう。 github.com
まとめ
Rspecやfactory gilrに関してまだまだ知らない事があるなと思ったのでドキュメント読んだりして身につけていく必要があるなと思いました。 あとはfixtureやminitestもどこかでやりたい。
Rails アンチパターン - 太ったセッション(Bloated Sessions)
引き続きRails AntiPatternsという本を読んでいます。
https://www.amazon.co.jp/dp/B004C04QE0/ref=dp-kindle-redirect?_encoding=UTF8&btkr=1
前回は10 Building for Failureの2つめ「Inaudible Failures」についてまとめました。 waterlow2013.hatenablog.com
今回は7 Controllersの3つめ「Bloated Sessions」についてまとめます。
Bloated Sessions
セッションに情報を入れておくとユーザやDBとの通信を節約でき、コードがシンプルになる可能性もある。 しかし、RESTの原則から外れるし、スケールアウトに向かない。またActiveRecordのオブジェクトをセッションやキャッシュに入れておくと、Railsのバージョン変更のときに動かなくなることがある。
Solution: Store References Instead of Instances
- sessionにはidのみを入れる 残りはDBに入れる。 current_userとか。Railsチュートリアルにもあったと思う。
- hiddenタグに入れてクライアントに持たせる。 formとかでよくやるやつですね。
まとめ
今回も反論の余地なしの回でした。 キャッシュにARのオブジェクト入れるのはよくやってしまうので気をつけたいです!
Rails アンチパターン - 無視されるエラー(Inaudible Failures)
引き続きRails AntiPatternsという本を読んでいます。
https://www.amazon.co.jp/dp/B004C04QE0/ref=dp-kindle-redirect?_encoding=UTF8&btkr=1
前回は10 Building for Failureの1つめ「Continual Catastrophe」についてまとめました。
今回は10 Building for Failureの2つめ「Inaudible Failures」についてまとめます。
Inaudible Failures
エラー対策はUXに影響を与える。
class Ticket < ApplicationRecord def self.bulk_change_owner(user) all.each do |ticket| ticket.owner = user ticket.save end end end
たとえばこのようなコードでは、Ticket
モデルのバリデーションでエラーになってしまってもスルーしてしまう。
ユーザにとって同じ結果でも削除されていたりされていなかったりする。
Solution: Never Fail Quietly
そんなことはやめようという話。
上のサンプルコードでれば、saveは保存できなかったときに例外をあげるsave!に変える。 またエラーが発生した場合にロールバックするためにトランザクションで囲む。そのほうが一貫性がある。
class Ticket < ApplicationRecord def self.bulk_change_owner(user) transaction do all.each do |ticket| ticket.owner = user ticket.save! end end end end
その他以下のようなtipsがある。
- controllerのrescue_fromで500用エラー画面を出す。
- rescue nilは使わない。
- 外部サービスを使ってエラーを蓄積・通知する。
まとめ
この章もその通りだなという感じでした。rescue_from使うとか、外部サービス使うとかは確実にやっていきたいです。
rescueについては前も書いたのですが、StandardError
ではなく限定された例外をrescueするのが良さそう。
Rails アンチパターン - 継続的な大災害(Continual Catastrophe)
引き続きRails AntiPatternsという本を読んでいます。
https://www.amazon.co.jp/dp/B004C04QE0/ref=dp-kindle-redirect?_encoding=UTF8&btkr=1
前回は6 Using Third-Party Codeの1つめ「Recutting the Gem」についてまとめました。
今回は10 Building for Failureの1つめ「Continual Catastrophe」についてまとめていきます。
Continual Catastrophe
サンプルとして以下のようなスクリプトを挙げる。
cd /data/tmp/ rm-rf*
/data/tmp/
がない場合に、ディレクトリ移動せず2行目が実行されたらどうなるか。ディレクトリが存在しない場合そこで終了するのが望ましい。(bashには-eや&&演算子がある)
Solution: Fail Fast
以下のようなコードがあるとする。
class Portfolio < ApplicationRecord def self.close_all! all.each do |portfolio| unless portfolio.photos.empty? raise "Can't close a portfolio with photos." end portfolio.close! end end end
これは途中でエラーになった場合に中途半端にcloseされてしまうので、初めにチェックするようにする。
class Portfolio < ApplicationRecord def self.close_all! all.each do |portfolio| unless portfolio.photos.empty? raise "Can't close a portfolio with photos." end end all.each do |portfolio| portfolio.close! end end end
更に早くチェックする方法として、UIからclose_allできないようにする,コントローラでリクエストを弾くなどがあげられる。
Fail Fastのメリットは
- 大災害を防げる
- はじめにチェックが宣言的に書かれているのでわかりやすい
- よくわからんところでぬるぽがでるのはうんざりだ!
たとえばARのfind_byとかも、レコードあり前提であればfind_by!で適切なエラーで失敗させるのがわかりやすいかなと思います。(ただし、例外を制御フローに組み込むのは絶対NG)createとかも同様ですね。(controllerのcreate, update内は別)
まとめ
その通りだなと思っていました。(それ以上の感想がない…。)
Rails アンチパターン - gemを使わない(Recutting the Gem)
引き続きRails AntiPatternsという本を読んでいます。
https://www.amazon.co.jp/dp/B004C04QE0/ref=dp-kindle-redirect?_encoding=UTF8&btkr=1
前回は5 Servicesの1つめ「Fire and Forget」についてまとめました。
今回は6 Using Third-Party Codeの1つめ「Recutting the Gem」についてまとめていきます。
Recutting the Gem
いくつかのRailsアプリケーションに関わっていると、共通のパターンまたは機能を実装していることがあります。 その機能を必要としている人は世界にたくさんいるはずです。
「だから、gem作りましょう」という話ではないんですよね笑 Solution見ます。
Solution: Look for a Gem First
- コードを書く前にgemを探しましょうという話
- 同じような実装を何回もやっていたとしたら、それはgemが使えるかもしれないというサイン
自分も新しいアプリや新しい機能をRailsで実装するときは、めちゃくちゃググりまくってgemや実装、アーキテクチャ等参考にできないか調べているような感じがします。 探し方にもコツが有るのですが、それは次の章で書いてくれているらしいのでそのときに触れようと思います。
まとめ
「ぼくのかんがえたさいきょうのRails開発」みたいなやつどっかでまとめたくなった!
Rails アンチパターン - ファイア・アンド・フォーゲット(Fire and Forget)
引き続きRails AntiPatternsという本を読んでいます。
https://www.amazon.co.jp/dp/B004C04QE0/ref=dp-kindle-redirect?_encoding=UTF8&btkr=1
前回は4 Controllersの2つめ「Fat Controller」についてまとめました。 waterlow2013.hatenablog.com
今回は5 Servicesの1つめ「Fire and Forget」についてまとめていきます。 ここで言うServicesとは外部サービスのことで、サービスクラスやアプリケーションサービスのことではありません。
Fire and Forget
外部サービスを利用するとき、レスポンスの扱いとしては以下のようなものがある。
- レスポンスをチェックし、エラーごとに適切な処理を行う。
- 成功、エラーのみチェックする。
- 何もチェックしない。
今回のアンチパターンは3つ目のケース。コントローラで直接呼んでいるとなるとユーザに500エラーを返すことになってしまう。
Solution: Know What Exceptions to Look Out For
発生するエラーを列挙しておき、適切にハンドリングする。たとえばHTTPクライアントなら以下のようなエラーがある。
HTTP_ERRORS = [Timeout::Error, Errno::EINVAL, Errno::ECONNRESET, EOFError, Net::HTTPBadResponse, Net::HTTPHeaderSyntaxError, Net::ProtocolError]
rescue => e
しないのは、Timeout::Error
がRuntimeError
を継承していないためらしいが、今ならRuntimeError
を継承しているので、雑にやりたければrescue => e
でもいいと思う。
Message in a Bottle
config.action_mailer.raise_delivery_errors
の設定に気をつける- エラーには大きく分けて2種類ある。クライアント側とサーバ側
- クライアント側のエラー(メールアドレスがおかしい等)はユーザに即通知する
- サーバ側のエラーはスタッフに通知する等して、何か対応を入れる。
You Don’t Know What You Don’t Know
- rescueしすぎない
- 少なめのrescueから初めて必要なものを追加する
- エラーログサービス(NewRelic, errbit等)も重要
- エラーハンドリングの詳しくはchapter10を
まとめ
特に違和感を感じたところはなかったです!外部サービスを叩くものは常にエラーが起こりうるという扱いをするのが自然なんだなと思いました。
あとは早めにchapter10を読みたい。
Rails アンチパターン - ファットコントローラー(Fat Controller)
引き続きRails AntiPatternsという本を読んでいます。
https://www.amazon.co.jp/dp/B004C04QE0/ref=dp-kindle-redirect?_encoding=UTF8&btkr=1
前回は2 Modelsの1つめ「Voyeuristic Models」についてまとめました。
今回は4 Controllersの2つめ「Fat Controller」についてまとめていきます。 Solutionが2つあるのですが、今回は1つ目について書きます。
Fat Controller
コントローラーからビジネスロジックを削除し、それをモデルに適切に配置します。
コールバック、セッター、データベースのデフォルトなど、Active Recordによって提供される機能は、このタスクの重要なツールセットです。
Solution: Use Active Record Callbacks and Setters
サンプルコード
class ArticlesController < ApplicationController def create @article = Article.new(params[:article]) @article.reporter_id = current_user.id begin Article.transaction do @version = @article.create_version!(params[:version], current_user) end rescue ActiveRecord::RecordNotSaved, ActiveRecord::RecordInvalid render :action => :new and return false end end redirect_to article_path(@article) end end
コントローラで手続き型っぽく書いてしまっているというざっくりとした理由の他に、以下のような具体的理由があります。
- transactionをコントローラで呼んでいる
- saveが使われていない
- transactionを貼らないといけない?
- 例外を制御フローに使っている
例外的
状況ではない- GOTO文と可読性が変わらない
目標
class ArticlesController < ApplicationController def create @article = Article.new(article_params) if @article.save redirect_to article_path(@article) else render :action => :new end end end
ActiveRecordのsaveメソッドの戻り値で判断します。scaffoldで作られたものと同じですね。
create_version!メソッドを消す必要があるので、見ていきましょう。
def create_version!(attributes, user) if self.versions.empty? return create_first_version!(attributes, user) end # mark old related links as not current if self.current_version.relateds.any? self.current_version.relateds.each do |rel| rel.update_attribute(:current, false) end end version = self.versions.build(attributes) version.article_id = self.id version.written_at = Time.now version.writer_id = user.id version.version = self.current_verison.version + 1 self.save! self.update_attribute(:current_version_id, version.id) version end def create_first_version!(attributes, user) version = self.versions.build(attributes) version.written_at = Time.now version.writer_id = user.id version.state ||= "Raw" version.version = 1 self.save! self.update_attribute(:current_version_id, version.id) version end
必要に応じて他のメソッドversionモデルも見ていきます。
消せるものを消す。
version.written_at = Time.now
はcreated_atとupdated_atで代用できる。version.state ||= "Raw"
はDBのデフォルト値を使う(必要ならgemをつかってもいい)version.article_id = self.id
はself.versions.build
で勝手に入る。
def create_version!(attributes, user) if self.versions.empty? return create_first_version!(attributes, user) end # mark old related links as not current if self.current_version.relateds.any? self.current_version.relateds.each do |rel| rel.update_attribute(:current, false) end end version = self.versions.build(attributes) version.writer_id = user.id version.version = self.current_verison.version + 1 self.save! self.update_attribute(:current_version_id, version.id) version end def create_first_version!(attributes, user) version = self.versions.build(attributes) version.writer_id = user.id version.version = 1 self.save! self.update_attribute(:current_version_id, version.id) version end
コールバックをつかう
コールバックにするのは以下の3つ。
version.version
の設定current_version.relateds
のupdateself.update_attribute(:current_version_id, version.id)
version.version
の設定
class Version < ApplicationRecord before_validation :set_version_number, on: :create validates :version, presence: true private def set_version_number self.version = (article.current_version ? article.current_version.version : 0) + 1 end end
self.versions.empty?
はself.current_version
の存在確認でOK。create_first_version!
メソッドが必要なくなるので。以下のようになる。
def create_version!(attributes, user) # mark old related links as not current if self.current_version && self.current_version.relateds.any? self.current_version.relateds.each do |rel| rel.update_attribute(:current, false) end end version = self.versions.build(attributes) version.writer_id = user.id self.save! self.update_attribute(:current_version_id, version.id) version end
* current_version.relateds
のupdate
class Version < ApplicationRecord before_validation :set_version_number, on: :create before_create :mark_related_links_not_current, if: :current_version validates :version, presence: true private def current_version article.current_version end def set_version_number self.version = (current_version ? current_version.version : 0) + 1 end def mark_related_links_not_current current_version.relateds.each do |rel| rel.update_attribute(:current, false) end end end
途中細かなリファクタリングをはさんでいますが、最後の結果だけ書いています。
def create_version!(attributes, user) version = self.versions.build(attributes) version.writer_id = user.id self.save! self.update_attribute(:current_version_id, version.id) version end
だいぶスッキリしてきました!
self.update_attribute(:current_version_id, version.id)
class Version < ApplicationRecord before_validation :set_version_number, on: :create before_create :mark_related_links_not_current, if: :current_version after_create :set_current_version_on_article validates :version, presence: true private def set_current_version_on_article article.update_attribute :current_version_id, self.id end def current_version article.current_version end def set_version_number self.version = (current_version ? current_version.version : 0) + 1 end def mark_related_links_not_current current_version.relateds.each do |rel| rel.update_attribute(:current, false) end end end
メインのデータ更新以外は全てコールバックに、という流れですね。
create_version!
消すまではもう一息という感じです。
def create_version!(attributes, user) version = self.versions.build(attributes) version.writer_id = user.id self.save! version end
create_version!
を消す
ここでcreate_version!
を消します。
create_version!
のversion
に関するコードはコントローラに移します。
class ArticlesController < ApplicationController def create @article = Article.new(params[:article]) @article.reporter_id = current_user.id @version = self.versions.build(attributes) @version.writer_id = current_user.id if @article.save redirect_to article_path(@article) else render action: :new end end end
これで目標とする形にかなり近づきました。
その先
この書籍ではnested_attributesを使う方法が書かれていました。最近のRailsバージョンだとaccept_nseted_attributes_for
を使うのかと思いますが、実はこの機能、頃合いを見て消される方針のようです。
https://github.com/rails/rails/pull/26976#discussion_r87855694
代替としては、自分が知っているのはFormオブジェクト使うやり方があるのですが、だいぶ本に載っている内容からはそれるので今回はパスします。 Formオブジェクトの作り方はいろいろあると思いますが、パーフェクトRailsあたりに載っていた気がしますし、有料ですがgorailsにも。
まとめ
fat controller解消は何かパターンがあるわけではなく、リファクタリングとコールバックの積み重ね、最終的にどうしようもない所だけFormオブジェクトにするという泥臭い感じになるのだなと思いました。ただこうゆうことをサクッとできないとfat controllerと付き合うしか無いと思うので、この辺できるようにしておきたいです。
scaffoldに近づけるのが理想的という話はめちゃくちゃ同意できました!
[2017-05-07 20:50追記] 2つ目の解決策であるMove to a Presenterはactive_presenterというgemを使うのですが、今はメンテされていない様子です。内容としては上で言うFormオブジェクトっぽいものを宣言的にくためのgemのようでした。こちらに関しては深入りしないことにします。