いきなりですが、こんな感じのFat Controllerのアクションがあるとします。
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をそっくりコピーします。
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などは必要な値だけ引数で受け取るようにします。
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にしましょう。
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を分ける
下書き保存の場合はバリデーションをスキップするという要求はよくあると思います。
class Report < ApplicationRecord
STATUS_DRAFT = 1
validates :status, presence: true
validates :title, presence: true, unless: :draft?
def draft?
status == STATUS_DRAFT
end
end
これを下書き保存用のクラスと投稿用のクラスに分けます。
# 研究調査のレポート提出機能ということにします
module ResearchModule
class DraftReport
attr_accessor :report
def initialize(report)
self.report = report
end
def valid?
report.valid?
end
end
end
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に関連づけます。
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に移動させます。
module ResearchModule
# StatusValueObjectから名称変更
class ReportStatusValueObject
# 省略
end
end
module ResearchModule
class ReportTypeValueObject
# 省略
end
end
DraftReportとPublishReportにReport親クラスを作成します。ResearchModule::Reportなので、ActiveRecordクラスのReportとは衝突しません。
Report#status_value_objectなどをResearchModule::Reportに移動させます。
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
module ResearchModule
class DraftReport < Report
# 共通なのでコンストラクタなどは削除
def valid?
report.valid?
end
end
end
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?メソッドを作成します。
module ResearchModule
class DraftReport < Report
# 他は省略...
def send_review_request?
false
end
end
end
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クラスを使うようにします。
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?を使えば下書きかどうかチェックする必要がなくなるため、下書きの条件分岐を削除します。
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?を呼び出す必要があります。
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を呼び出すか切り替えるだけになります。
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モデルだけしか使っていませんが、色々と派生させることができます。
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っぽいことする話です。
-
他にReportのバリデーションを使っている箇所があれば、壊れている可能性があります。対応が必要か確認してください。 ↩