Rails

ActiveRecordのモデルが1つだとつらい

Railsあるある

何気ないモデルの変更がアプリケーション全体を傷つけた

TL;DR

コントローラーごとに分けろといいたいわけではなく、アプリケーション全体で1テーブル1 ActiveRecordモデルをやめて

経緯

子どものお世話を記録するウェッブアプリケーションを正月休みあたりから書いています。1
https://github.com/hanachin/bblog

その中でこういう気持ちが生まれました(Refinements過激派)。

リプライで以下のようなツイートを受け取りました。

1月のOkinawa.rbで最近試してる話をしたとツイートしたら反響をいただき23記事にした次第です。

何気ないモデルの変更がアプリケーション全体を傷つける原因

アプリケーションの規模がどんなに大きく成長しても1つのテーブルに対応するActiveRecordのモデルは常に1つ。アプリケーション全体が1つのモデルに依存!
なのでモデルの振る舞いに影響が出る機能を使うとアプリケーション全体に影響がでる。

具体的にどういう機能で問題が出やすいか

Railsあるある4を参考にいくつか挙げてみます。

  • default_scope
  • validation
  • callbacks
  • as_json

どれもモデルの振る舞いに影響が出るものばかりですね。

特にdefault_scopeに関しては地雷メソッド5とか撒いてはいけない種6とかevil7など過激な呼ばれ方をされています。

逃れ方

一応それぞれ色々な方法で外したりスキップ出来ます。

default_scope

  • unscopedでスコープを外す
  • reorder, rewhereなどで条件を変更する

validation

  • validationがかからないAPIやsave(validate: false)を使う8
  • contextを設定して特定の場合だけ実行されるようにするsave(context: :account_setup)9
  • 特定の条件に合致する場合しかバリデーションしない10

callbacks

  • callbackが実行されないAPIを使う11
  • 特定の条件に合致する場合しかバリデーションしない12

as_json

ActiveRecordの:only, :except, :methods, :includeのオプションを指定するとある程度カスタマイズできる13

逃れるの無理説

アプリケーション全体でモデル1つだとモデルが大きくなるにつれ複雑に組み合わさる。モデルを使う場所全部でこれらの機能を意識しながら書くの無理では...。
ということで最初から使わないほうがよいのでは、みたいな結論になりがちです。

単純にConcernに切り出してファイルを分けてもモデルが1つのままではモデルの変更がアプリケーション全体に影響します。

モデル全体に影響する機能を使わない場合、代わりに何を使うの?

レールの伸ばし方14ではモデルの責務をPORO15, FormObject, ServiceObjectに分ける方法が紹介されています。

ActiveRecord以外の層つくると意外と面倒

ActiveRecordを使うとparamsで受け取った文字列を渡すだけでいい感じに型変換してくれてべんりです。
Form Objectなどを分けた場合、このあたりの型変換のコードでかなり記述量が増えたりします。16
なのでForm Objectを作りやすくするためのまた別のgemを導入することが多いです。17

例えばモデルのクラスを分ける

モデルの振る舞いの影響範囲がアプリケーション全体に及んでしまうのがつらみの原因なら、責務ごとにモデルごと別々に分けると疎になって便利では?
以下で普段の開発の中でActiveRecordのクラスを分ける例を挙げます。

例: マイグレーション実行時に使うモデル

マイグレーション作成時のチェックポイント18から引用します。

app/models 下のモデルクラスなど、マイグレーションファイルの外部に定義している、将来実装を変更する可能性のあるクラスを直接利用することは禁じ手と考えた方がいいでしょう。
なぜかというと、マイグレーションファイルというのは、未来にわたって末永く、書いたときの意図どおりに動く 必要があるからです。言い換えれば、マイグレーションファイルのコードは、マイグレーションファイル内で閉じていて、凍結されていることが望ましい のです。

問題: 1つのクラスに2つの責務

# app/models/user.rb
class User < ApplicationRecord
  UNKNOWN_BIRTHDAY = Date.new(9999, 1, 1)
end
require 'date'

class AddBirthdayDateToUsers < ActiveRecord::Migration[5.1]
  def up
    add_column :users, :birthday_date, :date
    User.reset_column_information
    User.find_each do |u|
      birthday_date = Date.new(u.year, u.month, u.day) rescue nil
      u.update(birthday_date: birthday_date || User::UNKNOWN_BIRTHDAY)
    end
    change_column_null :users, :birthday_date, false
  end

end

上記のようにマイグレーション実行時にアプリケーションで定義したActiveRecordのクラスを参照すると、アプリケーションコードにマイグレーションのコードが依存し、1つのクラスに2つの責務が生まれます💪

  • アプリケーションを実行するための責務
  • マイグレーションを実行するための責務

この場合、アプリケーションを実行するための修正がマイグレーション実行に影響を及ぼす可能性があります。
具体的な例をRails で信頼性の高い Migration を書くには19から引用すると以下のような感じです。

特に Model を使ってデータの移行を行う場合は注意が必要です。create, update, where など一部のメソッドしか使わないつもりでついついそのまま使ってしまいがちですが、hook や default_scope、validation などの変化によって知らぬうちに挙動が変わってしまいます。Migration 毎に専用の Model を作りましょう。

解決策: マイグレーション用のモデルをつくる

マイグレーションファイル中でマイグレーションの実行に必要な責務だけを持ったActiveRecordのクラスを宣言します。アプリケーションコードの変更がマイグレーションに影響することはありません。20

require 'date'

class AddBirthdayDateToUsers < ActiveRecord::Migration[5.1]
  class User < ActiveRecord::Base
    UNKNOWN_BIRTHDAY = Date.new(9999, 1, 1)
  end

  def up
    add_column :users, :birthday_date, :date
    User.find_each do |u|
      birthday_date = Date.new(u.year, u.month, u.day) rescue nil
      u.update(birthday_date: birthday_date || User::UNKNOWN_BIRTHDAY)
    end
    change_column_null :users, :birthday_date, false
    User.reset_column_information
  end

  def down
    remove_column :users, :birthday_date
  end
end

責務に応じてモデルを分けるとよいのでは

上記の例ではActiveRecordのモデルをわけた例を紹介しました。
モデルを分けた結果、モデルが単一責任になり、モデルへの変更が別のモデルやアプリケーションコードに影響しなくなりました。
ふつうのアプリケーションのコードも無理して1つのモデルに全部詰め込まず、マイグレーションのようにActiveRecordのモデルを分けるとよいのでは?

影響範囲が狭くなる

目的ごとにモデルを定義すると影響範囲がアプリケーション全体から狭まります。
default_scopevalidationcallbacksas_jsonを書き散らかしても、他のモデルに影響しないので便利そうです。

Railsの機能がそのまま使えて便利

ふつうのActiveRecordのクラスなので型変換や慣れ親しんだAPIをそのまま使えます。
他のgemの使い方を覚える必要はありません。

やりかた

例: 登録が完了したときメールを送りたい

app/models/signup_user.rb
class SignupUser < User
  after_save :send_signup_email

  private

  def send_signup_email
    UserMailer.signup(self).deliver_later
  end
end
app/controllers/signup_controller.rb
class SignupController < ApplicationController
  def create
    user = SignupUser.new(params)
    if user.save
      redirect_to root_path
    else
      render 'new'
    end
  end
end

例: 公開されている記事だけを表示したい

app/models/article.rb
class Article < ApplicationRecord
end
app/models/published_article.rb
class PublishedArticle < ApplicationRecord
  self.table_name = "articles"
  default_scope -> { where(published: true) }
end
app/controllers/articles_controller.rb
class ArticlesController < ApplicationController
  def index
    # 一覧用
    @articles = PublishedArticle.order(published_at: :desc)

    # 新規作成用
    @new_article = Article.new
  end
end

例: 作るときだけ関連レコードのpresenceを確認したい

app/models/article.rb
class Article < ApplicationRecord
  belongs_to :author, optional: true
end
app/models/new_article.rb
class NewArticle < Article
  self.table_name = "articles"
  validates :author, presence: true
end
app/controllers/articles_controller.rb
class ArticlesController < ApplicationController
  def create
    @new_article = NewArticle.new(params)

    if @new_article.save
      redirect_to @new_article
    else
      render 'new'
    end
  end
end

まとめ

アプリケーション全体で1つのテーブルに対応するActiveRecordのモデルが1つだとモデル全体に影響でる機能がアプリケーション全体に影響でてつらい。
default_scopevalidationcallbacksas_jsonなどモデル全体に影響が出るメソッドでつらみが生まれるのは、それらの機能自体が悪いわけではなく、アプリケーション全体で1モデルを共有しているのが原因ではないか?
アプリケーションの様々な場面での責務を1つのActiveRecordのモデルに詰め込むとつらいので、責務に応じて同じテーブルを参照するActiveRecordのモデルを分けるとよいのでは、影響範囲が狭まるしActiveRecordの機能がそのまま使えて便利!というご提案でした。

1つのテーブルに対応するActiveRecordのモデルを分けるのはマイグレーションやマイクロサービスなどで既にやっている人も多いと思いますが、アプリケーションコードでも分けてこ💪

懸念

最近の趣味のアプリケーションでちょっと試した感じよさそうでしたが大きいアプリケーションになるとまた別のつらみが発生しそう。21

はてなブックマークへのレス

お気持ちです。

Controllerでrefineは分かる。modelが大きくなると、色々とツラくなる。

1 controllerでしか使わない機能が1 modelに雑多に混ざると凝集度下がってつらくなる感じの印象があります。

わかるが、沢山モデルが作られて、しまうのも…という。モジュール使えばいいのかな。テスト書きにくそうだけれど

とても細かい粒度で分けるとクラス爆発は起こりそうです。同じ動作があればモジュールにまとめてincludeするとよいと思います。
テストに関しては書きづらそうなポイントがいまいちわかりませんが、スーパークラスでテストしていることはしなくてもよさそうですし、モジュールに対応したshared_examplesを書けばテストも共用できそうですし、書きづらそうなポイントがいまいちわかりませんでした。

以下のような感じで考えています。

app/models/foo.rb
class Foo < ApplicationRecord
  validates :foo, presence: true
end
app/models/foobar.rb
class Foobar < Foo
  include HasBar
end
app/models/foobar2000.rb
class Foobar2000 < Foo
  include HasBar
end
app/models/concerns/has_bar.rb
module HasBar
  extend ActiveSupport::Concern

  included do
    validates :bar, presence: true
  end
end
spec/models/foo_spec.rb
require 'rails_helper'

RSpec.describe Foo, type: :model do
  it { is_expected.to validate_presence_of(:foo) }
end
spec/support/shared_examples/models/has_bar.rb
RSpec.shared_examples 'model has bar' do
  it { is_expected.to validate_presence_of(:bar) }
end
spec/models/foobar_spec.rb
require 'rails_helper'

RSpec.describe Foobar, type: :model do
  include_examples 'model has bar'
end
spec/models/foobar2000_spec.rb
require 'rails_helper'

RSpec.describe Foobar2000, type: :model do
  include_examples 'model has bar'
end

ここまで来るとRailsあんま使いたくないなーって感じ

どうしてそう感じるのかよくわからなかった

特定のcontrollerでしか使わないコードをmodelに書きたくないよね的なお話

特定のcontrollerでしか使わないコードをアプリケーション全体で1つのモデルに詰め込みたくないよね的なお話です。
モデルに書きたいですが、全部1つのモデルに詰め込んで書くのは違うと思います。

なんかDDDっぽいことをしたい的な雰囲気を感じる

層を増やすよりActiveRecordのcallbackなどの機能を活用したほうがよいのでは、と思っています。

Rails知らないけど、Rails的にはUserモデルはデータストアでもあるしメール配信もする、プレゼンテーション層のロジックも全部やるよ、ってことなのかな。

Rails的にはそのへん決まってなくてユーザーが自由に書けると思いますよ。UserモデルがActiveRecordを継承する必要もないです。ふつうのRubyのクラスもapp/modelsの下に置いていいです。メール配信は例で出したようにActionMailerが行います。ActiveRecordのモデルにはメールを送信する機能はないです。

ActiveRecordの主な機能についてはRails Guidesでは6ページも紹介されています。多機能ですね。

  • Active Record Basics
  • Active Record Migrations
  • Active Record Validations
  • Active Record Callbacks
  • Active Record Associations
  • Active Record Query Interface

詳しくはRails Guidesを呼んでもらいたいですが、callbackを使うとデータが保存されたタイミングで必要な処理を行うこともできます。
登録時にメール送信が必要であれば、callbackのタイミングでActionMailerを使ってメールを送ることもできます。

プレゼンテーション層のロジックについては適宜active_decorator22やdraper23で書いたほうが分かりやすいのではないでしょうか。基本的に1モデル1デコレーターみたいな感じで書くと思うので、モデルを細かく分けるのであれば対応するデコレーターも分けて書けそうですね。

app/models/article.rb
class Article < ApplicationRecord
end
app/models/published_article.rb
class PublishedArticle < Article
end
app/models/draft.rb
class Draft < Article
end
app/decorators/published_article_decorator.rb
module PublishedArticleDecorator
  def title
    "[公開中] " + self[:title]
  end
end
app/decorators/draft_decorator.rb
module DraftDecorator
  def title
    "[下書き] " + self[:title]
  end
end

これは結局コントローラに書いてることには変わりないので別のインターフェースから同じ処理を行おうとした時にコピペなどのつらみが発生するやつだ。

コントローラーに書いているのとは違います。selfを確認するとよいのでは?

app/controllers/articles_controller.rb
class ArticlesController
  # ここでのselfはArticlesController
  class PublishedArticle < Article
    # ここでのselfはPublishedArticle
  end
end

同じ処理を行うのであればモジュールに切り出して再利用するなり

app/models/concerns/send_confirm_email.rb
module SendConfirmEmail
  extend ActiveSupport::Concern

  included do
    after_save :send_confirm_email
  end

  private

  def send_confirm_email
    # メール送信
  end  
end
app/controllers/signup_controller.rb
class SignupController
  class User < ::User
    include SendConfirmEmail
  end
end
app/controllers/signup_with_coupon_controller.rb
class SignupWithCouponController
  class User < ::User
    include SendConfirmEmail
  end
end

モデルをControllerの外に出すなりするとよいと思います。

app/models/published_article.rb
class PublishedArticle < Article
end
app/controllers/articles_controller.rb
class ArticlesController
  def index
    @articles = PublishedArticle.all
  end
end
app/controllers/top_page_controller.rb
class TopPageController
  def index
    @articles = PublishedArticle.all
  end
end

コピペするとコピペのつらみが発生するのでモジュールに切り出しましょう。

「コピペなどのつらみ」のなど部分が何を指すのかブコメからはよくわかりませんでした。

やってることはクエリオブジェクト、コマンド(フォーム)オブジェクトだけど一回性と割り切って直接コントローラーに格納しているということね。ここから始めてコード共有すると判明してから移動させてもいいよね

もともとActiveRecordはクエリやバリデーションなど豊富な機能を備えているのでそれを使うのがよいと思います。Rails Guidesでは6ページも紹介されています。多機能ですね。

  • Active Record Basics
  • Active Record Migrations
  • Active Record Validations
  • Active Record Callbacks
  • Active Record Associations
  • Active Record Query Interface

そのコントローラー以外から使わないなら、コントローラーのネームスペースの下にモデル置いてもよいのでは、と思います。
必要になればapp/models直下に移動すればよいだけです。移動させる場合の影響範囲はそのコントローラーの下だけなので比較的楽だと思います。

STIは気をつければなんとかできるけどpolymorphic associationとの共存が難しそう

これは確かに...Railsがtypeカラムからクラス決めてしまうところどうやって変更すればいいのか、そもそもpublicなAPIあるのかも分からないので今後の課題ですね。
メソッド足すだけならRefinementsを使って足すのが一番手軽そうですが、バリデーションやスコープでもうまく動くのかは分からないところ。

コントローラで使うとか関係なく、ドメインモデルとして妥当かどうかで考えたい

確かにそのとおり。この記事書いた時点で趣味アプリケーションのコントローラーしか書いておらず「うちの場合は今のところコントローラーで割ってもいい感じ」程度にとらえてもらえれば...
マイグレーションの例を出しているので、コントローラに限らないとわかってもらえると思っていたんですが、認識不足でした。
まとめ部分やコード例などをコントローラーフィーチャーしない形に書き換えたんですが、TL;DR部分に引っ張られて今でも伝わっていなさそうです。(次にいかしたい)

使う経路によって期待するふるまいが変わるの、モデル設計を見直した方がよさそう…?という気持ちもした。

モデル設計を見直した結果、アプリケーション全体で1テーブルに対応するモデルが1つなのはおかしくないか?という気持ちになり、分割することにしました。期待するふるまいが違うのでモデルも分かれています。

同じテーブルに永続化されているデータがモデルによって違う振る舞いをしてもいいか?でいうと違う振る舞いをしてもよいのでは、と思っています。違う振る舞いをするのであればテーブル設計的に分けてしまう、でもよいと思います。

テーブル、モデルを分割した方が良さそう。

例に書いたようにモデルは分割しております。テーブルを分けたほうがいいかは状況次第ですね。

辛い所はよく分かるが、コントローラー主眼で書くことがそんなに無いのであまり共感は出来ないかも。Railsレイヤ足りない(足すの難しい)問題の亜種のようにも感じる。

Railsレイヤ足りない問題がなにかよくわかっていません。

FormやServiceなどのレイヤーを足して解決したい問題の一部は、モデルを分割すればActiveRecordのcallbacksやdefault_scopeなどの機能を使って楽にかけるのでは、というのが私の主張です。
そもそもアプリケーション全体で1つのモデルを共有していることに問題意識があり、モデルを分割したほうがよいのでは、という気持ちです。
コントローラに主眼を置いているわけではありません。

記事よりもプリペアードステートメント使用しないでSQLの条件を文字列連結させてる方に気を取られてしまった。hanachin/bblogは分割、共通化させるのが早すぎると思う。規模の割に凝り過ぎてるように見える。

プリペアードステートメントを使わなくても問題ないと判断した部分ではRubyの式展開で直接文字列を埋め込んでいます。
そこに気を取られるよりは、SQLでi18nしている部分に気を取られてほしいですね。
https://github.com/hanachin/bblog/blob/ba691a4bb9084ece53a26bc7027b5e62b47f2d7a/app/models/milk_log.rb#L17

冒頭で書いたように趣味のアプリケーションです。どのような設計で書くかは好みなので、凝りすぎてるように見える人には凝りすぎて見えると思います。

コントローラー毎にドメインモデルを作ったDDDってことなんだろうけど、そういうのは最低限静的型付け言語でやらないとコンテキストによって同じモデルでも異なるのを管理できなくて破綻するからやめた方がいいと思う

このコメントを読んでもどうして「最低限静的型付け言語でやらないとコンテキストによって同じモデルでも異なるのを管理できなくて破綻する」のかが分かりませんでした。

責務に応じて同じテーブルを参照するActiveRecordのモデルを分けるとよいのでは、影響範囲が狭まるしActiveRecordの機能がそのまま使えて便利

はい。

モデルとはいったいなんなのか?

モデルです。

Laravelの方が進んでるって思った次第

この記事でも「モデル分ければいい」というのを言っています。5年も言われているんですね...

あってます。

全体を把握できる程度の規模じゃないとつらそう。面白いけど。

どのぐらいの規模まで使えるのかわかっていないので、面白い設計導入できそうなとき使ってみてください。

Twitter反応

個別のツイートに飛ぶとぶら下がってる反応見れると思います。

記事の修正とか伝わりづらさについて話しました

ActiveRecordと組み合わせていい感じに使えるDCIのgemがわからんという話

コメント書いてほしい

Rails Guides読むとtable_nameとか色々な設定方法書いてて便利です

今のところ不便ない感じです、リプがいっぱいぶら下がっています。

自分としてはこの問題意識あんまり伝わってなさそうな気がしています。

層足しても解決しない感じの問題はありそうです

Form的な層たさなくてもForm的なモデル作れる説でした。

Migrationでapp/models以下のモデル触ってしまうのはやりがち...(最初の方ではあまり問題にならないことが多い)

そういう発想です。


  1. docker-composeで環境を整えたり、SQLでi18nした文字列をjsonとしてrenderしてARインスタンス経由せずに返したり、今回記事にしたARのモデルを分ける設計など普段仕事でやらない実験的なことを趣味でやっています 

  2. https://twitter.com/kimihito_/status/960332669954359297 

  3. https://twitter.com/kazumalab/status/961824048052281345 

  4. https://www.slideshare.net/tricknotes/rails-possiblestory 

  5. https://qiita.com/sinsoku/items/9cbdc5304aa3ede4a178 

  6. https://qiita.com/juntetsu_tei/items/a1b641f7f3b10d3ae6e1 

  7. https://rails-bestpractices.com/posts/2013/06/15/default_scope-is-evil/ 

  8. http://guides.rubyonrails.org/active_record_validations.html#skipping-validations 

  9. http://guides.rubyonrails.org/active_record_validations.html#on 

  10. http://guides.rubyonrails.org/active_record_validations.html#conditional-validation 

  11. http://guides.rubyonrails.org/active_record_callbacks.html#skipping-callbacks 

  12. http://guides.rubyonrails.org/active_record_callbacks.html#conditional-callbacks 

  13. http://api.rubyonrails.org/classes/ActiveModel/Serializers/JSON.html#method-i-as_json 

  14. https://speakerdeck.com/willnet/rerufalseshen-basifang 

  15. Plain Old Ruby Object、継承元なしのObjectを継承してるただのRubyのオブジェクト、class PORO; endこういうやつ。 

  16. ActiveModel::Attributesが使えるようになればPOROでも同じように型変換できるので問題なくなるかも https://qiita.com/alpaca_taichou/items/bebace92f06af3f32898 

  17. 学習コスト💪 

  18. https://qiita.com/nay3/items/ef773006cd7f815a07cd 

  19. https://qiita.com/shuhei/items/c0a6c3e29c87de6dff63#migration-%E6%AF%8E%E3%81%AB%E5%B0%82%E7%94%A8%E3%81%AE-model-%E3%82%92%E7%94%A8%E6%84%8F%E3%81%99%E3%82%8B 

  20. ActiveRecordではテーブルごとにスキーマの情報をキャッシュしておりクラスが分かれていても影響が出る場合があります。reset_column_informationを呼んでいるのはキャッシュをクリアするためです。詳しくはonkさんの記事を読みましょう。 https://blog.onk.ninja/2017/10/18/use_reset_column_information 

  21. 複雑な実業務でやると影響範囲がアプリケーション全体から特定のモデルを使う機能に変わるだけで結局モデルクラスが増えた分メンテコスト増えたり、同じテーブルに対する操作が複数のモデルに散らばってしまいそう(concernでまとめてあげれば再利用できそうですが)、これはFormオブジェクトに分けても同じかな。 

  22. https://github.com/amatsuda/active_decorator 

  23. https://github.com/drapergem/draper