LoginSignup
1
0

More than 3 years have passed since last update.

@linyclar

Fat ControllerをリファクタリングしてDDDっぽくする

Posted at

いきなりですが、こんな感じのFat Controllerのアクションがあるとします。

reports_controller.rb
class ReportsController < ApplicationController
  def create
    @report = Report.new(report_params)
    if @report.status == Report::STATUS_DRAFT
      if @report.save
        redirect_to @report
      else
        render :new
      end
    else
      result = false
      ApplicationRecord.transaction do
        # レポートのレビューリクエスト
        if @report.report_type == Report::REPORT_TYPE_REVIEW_REQUEST
          # レビュワーに通知したり、メール送信したりといった処理・・・
        end
        @report.save!
        result = true
      end
      if result
        redirect_to @report
      else
        render :new
      end
    end
  rescue StanderdError
    render :new
  end
end

早期returnやReportクラスにメソッドを作ったりすれば多少改善しますが、別のアプローチを今回は紹介します。

リファクタリングの前にテストを用意する？

Fat Controllerとなっているクラスはテストがないか、やりやすいところだけテストしているような状態になっていると思います。
一般原則に照らして、リファクタリングの前にテストを用意したいところです。
しかし、テストが簡単に書けるならテストはすでに存在しているはずで、テストを書こうとすると生半可でない苦痛を感じ心が折れてしまうと思います。
なので、リファクタリング後のコードだけテストを書き、リファクタリングが終わった時にリファクタリング前のコードに戻してテストが通ることを確認する方法をとります。
事前にテストを書く方法に比べるとリスクは上がりますが、現実的な方法だと思います。

ApplicationServiceを用意する

まず、ApplicationServiceクラスを用意してactionをそっくりコピーします。

app/services/create_report_service.rb
class CreateReportService
  def create
    @report = Report.new(report_params)
    if @report.status == Report::STATUS_DRAFT
      if @report.save
        redirect_to @report
      else
        render :new
      end
    else
      result = false
      ApplicationRecord.transaction do
        # レポートのレビューリクエスト
        if @report.report_type == Report::REPORT_TYPE_REVIEW_REQUEST
          # レビュワーに通知したり、メール送信したりといった処理・・・
        end
        @report.save!
        result = true
      end
      if result
        redirect_to @report
      else
        render :new
      end
    end
  rescue StanderdError
    render :new
  end
end

当然動きませんが、一旦このコードをコミットします。次に、このコードが動くようにControllerに依存している処理を置き換えます。redirect_toやrenderはreturnに置き換え、session、paramsなどは必要な値だけ引数で受け取るようにします。

app/services/create_report_service.rb
class CreateReportService
  def create(report_params)
    @report = Report.new(report_params)
    if @report.status == Report::STATUS_DRAFT
      if @report.save
        return { result: true, data: { report: @report }}
      else
        return { result: false, data: { report: @report }}
      end
    else
      result = false
      ApplicationRecord.transaction do
        # レポートのレビューリクエスト
        if @report.report_type == Report::REPORT_TYPE_REVIEW_REQUEST
          # レビュワーに通知したり、メール送信したりといった処理・・・
        end
        @report.save!
        result = true
      end
      return { result: result, data: { report: @report }}
    end
  rescue StanderdError
    return { result: false, data: { report: @report }}
  end
end

ここでコミットし、同僚にレビューを依頼してください。Controllerに依存している処理の置き換えが正しく行われていることを確認してもらいます。テストがないため、レビューで品質を担保させます。
レビューでOKをもらえたらリファクタリングを開始します。Pull Requestでレビューを依頼したのであれば、マージせずにクローズさせておいてください。

ValueObjectを作る

どこから手をつけるかは、コードによりけりだと思います。今回は、わかりやすいValueObjectから着手します。
statusとreport_typeは定型文な条件式があるので、ValueObjecctにしましょう。

app/models/report.rb
class Report < ApplicationRecord
  # ここにはその他の処理が大量にある...

  def status_value_object
    StatusValueObject.new(status)
  end

  def report_type_value_object
    ReportTypeValueObject.new(report_type)
  end

  class StatusValueObject
    attr_accessor :status
    DRAFT = 1 # Report::STATUS_DRAFTを引越し
    def initialize(status)
      self.status = status
    end

    def draft?
      status == DRAFT
    end
  end

  class ReportTypeValueObject
    # 同じようにreport_typeのValueObjectを作る
  end
end

暫定的にReportクラス配下にValueObjectを定義します。ValueObjectのテストを作成したら一旦コミットしましょう。

下書き保存と投稿のEntityを分ける

下書き保存の場合はバリデーションをスキップするという要求はよくあると思います。

app/models/report.rb
class Report < ApplicationRecord
  STATUS_DRAFT = 1

  validates :status, presence: true
  validates :title, presence: true, unless: :draft?

  def draft?
    status == STATUS_DRAFT
  end
end

これを下書き保存用のクラスと投稿用のクラスに分けます。

app/domains/research_module/draft_report.rb
# 研究調査のレポート提出機能ということにします
module ResearchModule
  class DraftReport
    attr_accessor :report
    def initialize(report)
      self.report = report
    end

    def valid?
      report.valid?
    end
  end
end
app/domains/research_module/publish_report.rb
module ResearchModule
  class PublishReport
    attr_accessor :report
    # 使うものだけdelegateで呼び出せるようにする。何のカラムを利用しているかわかりやすくなる。
    delegate :title, to: :report
    def initialize(report)
      self.report = report
    end

    def valid?
      report.valid?
      report.errors.add(:title, :blank) if title.blank?
      report.errors.blank? # valid?だとerrorsの内容がリセットされてしまう
    end
  end
end

Report#to_research_moduleメソッドを作成し、DraftReportクラス、PublishReportクラスをReportに関連づけます。

app/models/report.rb
class Report < ApplicationRecord
  # 下書き保存でも投稿でもチェックするバリデーションはここに残します
  validates :status, presence: true

  def to_research_module
    if status_value_object.draft?
      return ResearchModule::DraftReport.new(self)
    end
    # もし、アソシエーション先のレコードが必要ならコンストラクタ引数で渡してしまいます
    # report.userみたいな呼び出しはPublishReportの中では禁止
    ResearchModule::PublishReport.new(self)
  end

  # 他の処理...
end

バリデーションのテストを作成して動くことを確認してコミットします1。

ValueObjectをModuleに移動させる

暫定でReportクラスに作っていたValueObjectをResearchModuleに移動させます。

app/domains/research_module/report_status_value_object.rb
module ResearchModule
  # StatusValueObjectから名称変更
  class ReportStatusValueObject
    # 省略
  end
end
app/domains/research_module/report_type_value_object.rb
module ResearchModule
  class ReportTypeValueObject
    # 省略
  end
end

DraftReportとPublishReportにReport親クラスを作成します。ResearchModule::Reportなので、ActiveRecordクラスのReportとは衝突しません。
Report#status_value_objectなどをResearchModule::Reportに移動させます。

app/domains/research_module/report.rb
module ResearchModule
  class Report
    attr_accessor :report
    delegate :status, :report_type, to: :report
    def initialize(report)
      self.report = report
    end

    def status_value_object
      ReportStatusValueObject.new(status)
    end

    def report_type_value_object
      ReportTypeValueObject.new(report_type)
    end
  end
end
app/domains/research_module/draft_report.rb
module ResearchModule
  class DraftReport < Report
    # 共通なのでコンストラクタなどは削除
    def valid?
      report.valid?
    end
  end
end
app/domains/research_module/publish_report.rb
module ResearchModule
  class PublishReport < Report
    delegate :title, to: :report
    def valid?
      report.valid?
      report.errors.add(:title, :blank) if title.blank?
      report.errors.blank? # valid?だとerrorsの内容がリセットされてしまう
    end
  end
end

Report#to_research_moduleでstatus_value_objectメソッドを呼び出していますが、ResearchModule::ReportStatusValueObject.new(status)に置き換えておきます。
クラスの場所が変わったのでテストを修正して、動くことを確認してコミットします。

レビューリクエストの送信判定をリファクタリング

DraftReportだったら常にレビューリクエストは送らず、PublishReportだったらreport_typeがレビューリクエストなら送る処理になっているので、send_review_request?メソッドを作成します。

app/domains/research_module/draft_report.rb
module ResearchModule
  class DraftReport < Report
    # 他は省略...

    def send_review_request?
      false
    end
  end
end
app/domains/research_module/publish_report.rb
module ResearchModule
  class PublishReport < Report
    # 他は省略...

    def send_review_request?
      report_type_value_object.review_request?
    end
  end
end

テストを書いてコミットします。

ApplicationServiceのリファクタリングに着手

リファクタリングに着手する前に、メソッド全体をコピーしてold_createのようなメソッド名で残しておきます。リファクタリングが終わった後、メソッド名を差し替えて古いコードでもテストがパスすることを確認するためです。
まず、Report#to_research_moduleを呼び出して、DraftReportクラス、PublishReportクラスを使うようにします。

app/services/create_report_service.rb
class CreateReportService
  def create(report_params)
    @report = Report.new(report_params)
    report_entity = @report.to_research_module # 追加した
    if @report.status == Report::STATUS_DRAFT
      if @report.save
        return { result: true, data: { report: @report }}
      else
        return { result: false, data: { report: @report }}
      end
    else
      result = false
      ApplicationRecord.transaction do
        # レポートのレビューリクエスト
        if @report.report_type == Report::REPORT_TYPE_REVIEW_REQUEST
          # レビュワーに通知したり、メール送信したりといった処理・・・
        end
        @report.save!
        result = true
      end
      return { result: result, data: { report: @report }}
    end
  rescue StanderdError
    return { result: false, data: { report: @report }}
  end
end

次に、send_review_request?を使えば下書きかどうかチェックする必要がなくなるため、下書きの条件分岐を削除します。

app/services/create_report_service.rb
class CreateReportService
  def create(report_params)
    @report = Report.new(report_params)
    report_entity = @report.to_research_module
    result = false
    # 下書きのケースを削除して通常の投稿処理と一本化した
    ApplicationRecord.transaction do
      # レポートのレビューリクエスト
      if report_entity.send_review_request? # ここの条件を変更
        # レビュワーに通知したり、メール送信したりといった処理・・・
      end
      @report.save!
      result = true
    end
    return { result: result, data: { report: @report }}
  rescue StanderdError
    return { result: false, data: { report: @report }}
  end
end

Reportクラスのリファクタリングでバリデーションが変わっているので、report_entity.valid?を呼び出す必要があります。

app/services/create_report_service.rb
class CreateReportService
  def create(report_params)
    @report = Report.new(report_params)
    report_entity = @report.to_research_module
    result = false
    ApplicationRecord.transaction do
      # Entityのバリデーションを呼び出す
      unless report_entity.valid?
        return { result: false, data: { report: @report }}
      end
      # レポートのレビューリクエスト
      if report_entity.send_review_request?
        # レビュワーに通知したり、メール送信したりといった処理・・・
      end
      @report.save!
      result = true
    end
    return { result: result, data: { report: @report }}
  rescue StanderdError
    return { result: false, data: { report: @report }}
  end
end

もし、レビュワーへの通知のためにreportの内容を整形しているなら、PublishReportクラスに移動させたりするとよいでしょう。他に@reportのようにインスタンス変数に格納する必要はないのでローカル変数に変更しておきましょう。

テストを作成しパスすることを確認します。FactoryBotを使っているなら、factory :research_module_draft_report, class: ReportのようにModuleのEntityに対応付けて定義するのをお勧めします。この時点で一旦コミットします（古いコードのコピーをコミットしたくないなら、消しておきましょう）。

リファクタリング前の古いコードでテストを実行する

残しておいた古いコードでもパスするか確認します。方法は、メソッド名を差し替えてテストを実行するだけでよいはずです。
もし、パスしないならリファクタリングで壊してしまったことになるので、テストをパスする形に修正します。そして、リファクタリング後のコードに戻して、テストがパスするようにプロダクトコードを修正します。

コントローラーのリファクタリング

コントローラーではApplicationServiceを呼び出して、結果でredirect_toを呼び出すかrenderを呼び出すか切り替えるだけになります。

reports_controller.rb
class ReportsController < ApplicationController
  def create
    service = CreateReportService.new
    service_result = service.create(report_params)
    @report = service_result[:data][:report]
    if service_result[:result]
      redirect_to @report
      return
    end
    render :new
  end
end

バリデーションの方法を変えましたが、ActiveRecordの仕組みから外れていないので、Viewは一切修正する必要がありません。

最後に

RailsはDDDに向かないと言われますが、Report#to_research_moduleのようにEntityに対応付けるメソッドを用意すれば如何様にもできると考えています。
この例だと、Reportモデルだけしか使っていませんが、色々と派生させることができます。

app/models/report.rb
class Report < ApplicationRecord
  belongs_to :reporter
  def to_review_module
    # アソシエーションを使いたいならコンストラクタで渡してしまう
    ReviewModule::Report.new(self, reporter)
  end

  def to_review_module2
    # Aggregateとして構築したいならReporterにもto_review_moduleメソッドを作る
    ReviewModule::Report.new(self, reporter.to_review_module)
  end


  def to_review_module3
    # 必要なのはタイトルと本文、レポーターの名前だけならこうしても良い
    ReviewModule::Report.new(title, body, reporter.name)
  end
end

究極的には、ActiveRecordのモデルにはbelongs_toのようなアソシエーションとバリデーション、スコープ、そしてEntityに対応付けるto_research_moduleのようなメソッドだけしか残らなくなるはずです。これはすなわち、ActiveRecordのモデルにドメインロジックを書かないようにするということです。
また、アソシエーションの参照はto_research_moduleのようなメソッドに限定させることで、どのテーブルを使っているのかはto_research_moduleのようなメソッドを確認するだけでよくなります。

このエントリは要求分析駆動設計のユースケース(ユーザーストーリー）、スイムレーンとデータモデリングを再編しコントローラーとActiveRecordのモデルのリファクタリングという観点で書き換えたものです。
めっちゃ長いけど、RailsでDDDっぽいことする話です。

  1. 他にReportのバリデーションを使っている箇所があれば、壊れている可能性があります。対応が必要か確認してください。 ↩

1
0
0

Register as a new user and use Qiita more conveniently

  1. You get articles that match your needs
  2. You can efficiently read back useful information
  3. You can use dark theme
What you can do with signing up
Sign upLogin
1
0