blog.waterlow.work

Ruby, Rails, js, etc...

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」についてまとめました。

waterlow2013.hatenablog.com

今回は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」についてまとめました。

waterlow2013.hatenablog.com

今回は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」についてまとめました。

waterlow2013.hatenablog.com

今回は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

ファイア・アンド・フォーゲット - Wikipedia

外部サービスを利用するとき、レスポンスの扱いとしては以下のようなものがある。

  • レスポンスをチェックし、エラーごとに適切な処理を行う。
  • 成功、エラーのみチェックする。
  • 何もチェックしない。

今回のアンチパターンは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::ErrorRuntimeErrorを継承していないためらしいが、今なら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」についてまとめました。

waterlow2013.hatenablog.com

今回は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.idself.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のupdate
  • self.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にも。

gorails.com

まとめ

fat controller解消は何かパターンがあるわけではなく、リファクタリングとコールバックの積み重ね、最終的にどうしようもない所だけFormオブジェクトにするという泥臭い感じになるのだなと思いました。ただこうゆうことをサクッとできないとfat controllerと付き合うしか無いと思うので、この辺できるようにしておきたいです。
scaffoldに近づけるのが理想的という話はめちゃくちゃ同意できました!

[2017-05-07 20:50追記] 2つ目の解決策であるMove to a Presenterはactive_presenterというgemを使うのですが、今はメンテされていない様子です。内容としては上で言うFormオブジェクトっぽいものを宣言的にくためのgemのようでした。こちらに関しては深入りしないことにします。