Railsのモデル名をすべて変更した話

SmartHRでは開発にRuby on Railsを広く採用しています。
今日は負債解消のために、開発しているサービスでRailsのモデル名をすべて変更した話を紹介します。

既存のモデル構造のつらみ

私達が開発しているサービスでは、モデルの親子構造が分かりやすいということで、モデルをネストした構造にしていました。
例えば、 User に紐づくプロフィール画像 User::ProfileImage は、 app/models/user/profile_image.rb に配置する具合です。

パッと見の構造が分かりやすいのですが、時が経つにつれて次のようなつらさが顕在化してきました。

• Railsの規約(推奨ルールのようなもの)に則っていないので、関連定義が冗長になる
• テーブル名が長くなる。
• 外部キーや関連名が長くなる。
• 関連名と外部キー名が一致せず、カラムを呼び出したいときにDB定義を見ないとカラム名分からない
• 親子構造を表しづらい中間テーブルが無理な命名規則になって直感的ではない

さらに、開発が進んで深いネストのモデルが登場するにつれて、段々と開発メンバーの間で「この構造は厳しい...。なんだか脳が疲れるぞ...。」という空気が漂っていました。

そして、ある時事件は起きました。

世界で我々だけが踏んだRailsのバグ

ネストしたモデルで呼び出していた group + count のクエリが不自然な挙動をしていることに気づきました。

HogeModel.group(
  :toooooooo_long_a_id,
  :toooooooo_long_b_id
).count #=> 挙動がおかしい

調査を進めるうちに分かったのですが、これは長すぎるカラムに対して group + count した時だけに発生するRailsのバグでした。

調べてもrails/railsにissueは立っていません。
どうやら我々は、不自然なモデル構造にしてしまったばかりに、世界の誰も踏んだことがないであろうバグを発生させてしまったようです。

思えば、普段の開発でもテーブル名の長さがPostgreSQLの上限に引っかかったり、様々な事情を考慮して開発をする必要あったりと、ネストしたモデル構造によって苦慮していたなと、チームが再認識しました。

おそらく世界で我々だけが、無駄な領域で苦しんでいます。つらい、つらすぎる！
この不具合を大きな教訓として、我々のチームはモデル構造の見直しに取り掛かることにしました。

(なお、このバグは社内勉強会で修正されました。 rails/rails#46287 )

CoC(設定より規約)改善のロードマップ

既存のネストしたモデルは、チームでの議論の末に app/models 直下に再配置することに決まりました。
およそ60ファイル程度のモデルを対象として、改善プロジェクトがスタートです。

改善プロジェクトは、次の手順で進行することになりました。

1. 新しいモデル名を決める
2. ネストしているモデル群を、一括で app/models 直下に移動する
3. 関連操作を解析・置換する仕組みを作って、関連名を置換していく

続けて、具体的な進め方を紹介します。

1. 新しいモデル名を決める

Google Sheetsにモデル名を転記して、チームで議論しながら新しいモデル名を決めました。

モデル名一覧

モザイクがかかっているところに実際のモデル名が記載されているのですが、おおよそのRailsプロジェクトで目にすることがない文字長であることをお分かりいただけると思います。

2. モデルを一括で app/models 直下に移動する

先ほどのGoogle Sheetsを元に、Rubyスクリプトを書いて一括で置換します。 この作業は、簡単なファイル操作と文字列操作で完了しました。

1. git mv で app/models 直下に移動する
2. self.table_name = "xxx" を各モデルの先頭に追記する
3. 関連定義(belongs_to/has_one/has_many)にclass_name: "新しいクラス名" を追記する
4. git grep '[[:<:]]古いモデル名[[:>:]]' で探索して、見つかった箇所を新しいモデル名で置換する。(記号は、単語の先頭・終端を表しています。)

この作業はスムーズに完了できました。

3. 関連操作を解析・置換する仕組みを作って、関連名を置換していく

続いて、古いモデル名のままになっている関連定義や関連名を、新しいモデル名のCoCに沿うように直していきます。
ActiveRecordでは関連名を様々な箇所で利用しており、これらの多彩な利用シーンを考慮する必要があります。

• belongs_to/has_one/has_many による関連定義
• preload/where/joins...etc などのクエリ系メソッドに渡す関連名
• user.items/user.build_profile のような関連名メソッドを使った呼び出し

これらを機械的に置換するために、メソッドの利用箇所を収集して、その情報を元にコードを置換する仕組みを作ることにしました。

3-1. CI上で関連を利用したメソッド呼び出しを収集する

関連名を利用したメソッド呼び出しの情報を集めるスクリプトを作成して、CI上で情報を収集します。
TracePointを使って、関連名を利用したメソッド呼び出しを片っ端から集めて解析していきます。

3-2. has_one/has_many/belongs_to を使った関連定義

has_one/has_many/belongs_to の定義は、モデルを読み込めば実行されるので解析が楽ちんです。
細かい点は省略していますが、おおむね以下のスクリプトで情報を集めることができました。

MethodDumper はTracePointをwrapして、特定のメソッドだけ反応するようにしている独自クラスです。

# ./bin/rails r this_file.rb
dumper = MethodDumper.new do
  Rails.application.reloader.reload!
  Rails.application.eager_load!
end

# メソッド呼び出しをdumpするメソッド群を定義する
dumper.add_method_trace("ActiveRecord::Associations::ClassMethods.has_one")
dumper.add_method_trace("ActiveRecord::Associations::ClassMethods.has_many")
dumper.add_method_trace("ActiveRecord::Associations::ClassMethods.belongs_to")

dumper.run do |row|
  trace_point = row[:trace_point]
  pp race_point.send(:caller)
  # do something
end

MethodDumperの実装を含む完全版はこちら

3-3. user.items/user.build_profile のような関連名を使ったメソッド

続いて、関連名を利用したメソッド呼び出しの情報を収集します。 MethodDumperの対象となるメソッドを関連名で指定して、 MethodDumper にRSpecの実行を解析させることで関連名のメソッド呼び出しを収集することができました。

dumper = MethodDumper.new do
  run_rspec
end

Rails.application.eager_load!

# 関連名を使っているメソッド群を解析対象とする
klasses = ApplicationRecord.descendants.reject(&:abstract_class?)
klasses.each do |klass|
  klass.reflections.each do |reflection_name, reflection|
    if reflection.belongs_to? || reflection.has_one?
      dumper.add_method_trace("#{klass.generated_association_methods}#reload_#{reflection_name}")
      dumper.add_method_trace("#{klass.generated_association_methods}#build_#{reflection_name}")
      dumper.add_method_trace("#{klass.generated_association_methods}#create_#{reflection_name}")
      dumper.add_method_trace("#{klass.generated_association_methods}#create_#{reflection_name}!")
    end

    dumper.add_method_trace("#{klass.generated_association_methods}##{reflection_name}")
    dumper.add_method_trace("#{klass.generated_association_methods}##{reflection_name}=")
  end
end

dumper.run do |row|
  trace_point = row[:trace_point]

  pp(trace_point.send(:caller))
  # do something
end

完全版はこちら

3-4. joins/where/includes... のようなメソッドの呼び出し元と、引数

最後に、クエリ系メソッド(joins/where/includes/preload/find_by/create_by...他多数)の収集処理を作っていきます。
これらは呼び出し方が多彩なので、なかなか厄介です。

以下は抜粋したコードですが、メソッドごとに解析処理を分岐させるマッチョ実装で情報を集めることにしました。

dumper = MethodDumper.new { ... }
dumper.add_method_trace("ActiveRecord::QueryMethods#joins")
dumper.add_method_trace("ActiveRecord::QueryMethods#joins!")

dumper.run do |row|
  reflections = ReflectionExtractor.new(row[:trace_point], row[:method_name]).extract_reflections
  pp reflections
end

# ./reflection_extractor.rb
class ReflectionExtractor
  attr_reader :trace_point
  attr_reader :method_name

  def initialize(trace_point, method_name)
    @trace_point = trace_point
    @method_name = method_name
  end

  def extract_reflections
    case @method_name
    when "ActiveRecord::QueryMethods#joins"
      args = trace_point.binding.local_variable_get(:args)
      collect_reflections_from_joins_clause(trace_point.self, *args)
    ...
    end
  end

  private

  def flat_arguments(args, collector = [])
    case args
    when Array
      args.each { flat_arguments(_1, collector) }
    when Hash
      args.each do |key, value|
        collector << key
        flat_arguments(value, collector)
      end
    when Symbol, String
      collector << args
    else
      raise NotImplementedError, "not implemented yet #{args.class}"
    end

    collector
  end

  def collect_reflections_from_joins_clause(receiver, *args)
    associations = args
    candidates = flat_arguments(args)

    join_dependencies = []

    join_dependencies.unshift(
      receiver.construct_join_dependency(
        receiver.send(:select_association_list, associations, join_dependencies), nil
      ),
    )

    join_dependencies.flat_map(&:reflections).uniq.filter_map do |reflection|
      [reflection.active_record.name, reflection.name] if candidates.include?(reflection.name)
    end
  end
end

完全版はこちら

また、where/find系のメソッドは where(user_profile: { id: 1 }) のように関連を指定して呼び出すこともできます。 これらのメソッドの解析は、ActiveRecordの内部で使っている処理を上書きすることで情報を収集する仕組みにしました。

以下、その辺りの処理を抜粋したコードです。

ActiveRecord::TableMetadata.singleton_class.prepend(Module.new do
  def collect_reflections
    self.enabled_reflections_collector = true
    self.reflections = []
    yield
    reflections
  ensure
    self.enabled_reflections_collector = false
  end
end)

ActiveRecord::TableMetadata.prepend(Module.new do
  def associated_table(table_name)
    if self.class.enabled_reflections_collector
      reflection = klass._reflect_on_association(table_name) || klass._reflect_on_association(table_name.singularize)
      self.class.reflections << [klass.name, table_name.to_sym] if reflection
    end

    super
  end
end)

class ReflectionExtractor
  ...

  def extract_reflections
    case @method_name.remove(/!$/)
    ...
    when "ActiveRecord::QueryMethods#find_or_create_by"
      attributes = trace_point.binding.local_variable_get(:attributes)
      collect_reflections_from_where_clause(trace_point.self, attributes)
    ...
  end

  def collect_reflections_from_where_clause(receiver, *args)
    if args.empty? || (args.length == 1 && args.first.blank?)
      []
    else
      opts, *rest = *args

      ActiveRecord::TableMetadata.collect_reflections do
        receiver.send(:build_where_clause, opts, rest)
      end
    end
  end
end

3-5. 置換用のパッチ生成

CI上で関連の呼び出し箇所を収集できたので、続いて関連の置換処理を作っていきます。
着手前は、コードの一部を書き換えた文字列から git diff でパッチを大量に生成して、git apply でまとめてパッチを適用すればいいと思っていました。

しかし、収集したのはメソッド呼び出し単位の情報なので、次の例のように複数のメソッドが同一行で呼ばれていると、joins と where それぞれでパッチを生成することになります。

User.joins(...).where(...)

残念ながら git diff は行単位のパッチを生成するので、同一行に対して2つのパッチを適用するとコンフリクトします。

うーん、困りました。
何度も解析・パッチを当てれば解決しますが面倒です。代わりに、たくさんのパッチを1パッチに結合する方法を考えました。

収集したメソッドごとにコードを書き換える

まずは、コード上の関連名を書き換える文字列操作を作っていきます。 文字列の書き換え処理は RubyVM::AbstractSyntaxTree や parser をゴリゴリ使って行いました。

これは面白かったですが、説明が長くなるので省略します。

パッチを作る

コードの一部分を変更した文字列を、パッチファイルに変換する処理を書いていきます。 git diff にお任せしちゃいます。

path = ...
new_content = replace_reflection(File.read(path))
tempfile = build_tempfile(new_content)

relative_path = Pathname.new(path).relative_path_from(Rails.root).to_s
patch = `git diff --unified=0 --no-index --no-color #{path} #{tempfile.path}`
patch.gsub!(path) { "/#{relative_path}" }
patch.gsub!(tempfile.path) { "/#{relative_path}" }
File.write('patches/xxx.patch', patch)

--unified=0 は、パッチファイルに含める前後行の行数を指定するオプションです。git apply でパッチをあてる際に、gitは変更行を特定するために前後行の情報を見て探索します。前後行は他のパッチで書き換わる可能性があるので、0行を指定します。
--no-index は、git管理外のファイルと差分を取ることを許可するオプションです。

これでパッチファイルを大量生成することができました。

パッチを結合する

同一行のパッチがコンフリクトしないように、パッチを可能な限り結合していきます。

まずはgit diffで生成したパッチをパースします。手頃なgemが見つからなかったので自作しました。 GitDiff

次に、比較アルゴリズムを考えます。 結合も比較もアルゴリズムを全然知らないので、ググって見つかったdiff-match-patchを利用することにしました。こちらもrubygemsに公開されていたgemは壊れていたので、修正して利用します。alpaca-tc/diff_match_patch

最後に、パッチの結合処理を作っていきます。 あまり面白くないのですが、ナイーブに書いたら動いたのでこれで大量のパッチを結合しました。 GitDiffMerger

あとは、それぞれのパッチを git apply して、関連名の置換は完了しました。

# パッチを可能な限り結合
diffs = Dir["./patches/*.patch"].map { GitDiff.parse(File.read(_1)) }
group_files = diffs.flat_map(&:files).group_by(&:old_path)

group_files.each do |path, files|
  files = GitDiffMerger.new(files).compact_files
  git_diff = GitDiff.new(files:)

  File.write("#{__dir__}/patches/combined_patch_#{SecureRandom.hex}.patch", git_diff.to_s)
end

# パッチを適用
Dir["./patches/combined_*.patch"].each { `git apply --unidiff-zero #{_1}` }

終わりに

ざっと書きましたが、これらの取り組みを通じてアプリケーションコードを機械的に置換することができました。 (実際には、それなりの手作業もありましたが。)

ようやく我々のチームも普通のモデル構造になり、生産性を低下させていた要因を解消できて嬉しいです。

また、この取り組みを通じてTracePointの知見が溜まったのは嬉しい副産物でした。
動的言語であっても影響範囲の調査や破壊的変更が容易になる可能性があり、今後も大規模な改修で活用していこうと思います。

最後に、変更にかかったPR数などを載せておきます。

PR数 54
commits数  1,100
追加行数  61,039
削除行数  23,058

みなさま、Railsは普通にCoCに則って使いましょうね！約束ですよ！