M3 DigiKar Rails5への道 - エムスリーテックブログ

エンジニアリンググループの@suusan2goです。現在は所属としてはAIチームで、機械学習以外のことを色々やっています。が、諸事情により少し暇になったので、弊社のクラウド電子カルテ「M3 DigiKar」チームに入ってRailsアップグレードをやっています。

今回の記事ではM3 DigiKar(以下デジカル)のRailsアップグレード(4.2 => 5.2)で、得られた知見をお話したいと思います。

デジカルの技術スタックについて

デジカルは様々な技術スタックから出来ています。WEBアプリケーションとしてはRailsがメインですが、診療報酬の点数を計算したり保険請求をしたりといった複雑な業務知識が求められる箇所ではScalaを使っていたりします。エムスリーデジカルCTOの冨岡の発表資料を見ていただくと雰囲気がつかめるかと思いますので、以下の記事をご確認ください。

logmi.jp

デジカルは2015年に開発が始まっており、現在のRailsのバージョンは 4.2.10、 rake stats の内容を抜粋すると以下のようになります。電子カルテという複雑なドメインと向き合っているためかモデルの数は開発期間に対して結構多めで、それなりに規模の大きなシステムとなります。

+----------------------+--------+--------+---------+---------+-----+-------+-----+
| Name                 |  Lines |   LOC  | Classes | Methods | M/C | LOC/M | T/C |
+----------------------+--------+--------+---------+---------+-----+-------+-----+
| Controllers          |  10790 |   8861 |     172 |     881 |   5 |     8 | 1.3 |
| Helpers              |     31 |     25 |       0 |       5 |   0 |     3 |     |
| Jobs                 |   1291 |   1035 |      21 |      81 |   3 |    10 |     |
| Models               |  25933 |  17398 |     489 |    1624 |   3 |     8 | 1.0 |
| Mailers              |    151 |    115 |       7 |      12 |   1 |     7 |     |
| Libraries            |   1315 |   1003 |      39 |     119 |   3 |     6 |     |
| Services             |  30767 |  24888 |     364 |    2080 |   5 |     9 | 1.4 |

アップグレード方針

小規模かつ開発が活発でないアプリであれば、Rails5アップグレード用のブランチを作りそこで開発を進めてもいいかもしれません。しかし複数人でアクティブに開発が進んでおり、Rails5対応のために開発を長期間止められない場合にはあまりよい選択とは言えないでしょう。アップグレードに限らず大きな機能を作るときにも言えることですが、大きなフィーチャーブランチを作る開発には以下のようなデメリットがあります。

こまめにリベースしていてもmasterブランチと頻繁にコンフリクトする
最終的にもはやGitHub/GitLab上でレビューできる差分でなくなるので、レビューが雑になりやすい
差分が大きくなるのでリリースの心理的ハードルが上がり、結果としていつまでもマージできなくなる(特にリファクタリングや、今回のアップグレードなど明確なビジネス上の締切がない開発だと特に)

デジカルも複数人で機能開発が行われており、アップグレードのために機能開発を止めたくなかったので、以下の方針でアップグレード作業を進めました。

Rails4 と並行してアップグレード作業が行えるよう、Rails5では動かない互換性のない箇所を少しずつ書き換えていく
まずはRails4 と Rails5で既存のテストが同時に通るようにすることを目指す

Rails4.2とRails5.2を同時に起動できるようにする

Rails5.2で互換性が失われた箇所を確かめるには、何はともあれ現在のコードをRails5で起動できるようにする必要があります。とはいえRails4も起動できるようにしたいので、アプリケーショントップのGemfile を直接いじるわけには行きません。そこでRails5用の Gemfile を用意してあげます。bundler では BUNDLE_GEMFILE という環境変数で Gemfile のパスを指定できるので、この環境変数によりRails4とRails5どちらで起動するか選択できるようになります。

Rails4ではそのままアプリケーショントップのGemfileを使う
Rails5では BUNDLE_GEMFILE=./rails5/Gemfile を指定し、Rails5用のGemfileを使う

実際のコードは以下のようになります。Rails4のGemfileを一度取り込んだ上で、Rails5では不要なGem、Rails5では必要になるGemなどがあれば、以下のように追加・削除をしていきます。

# ./rails5/Gemfile

# Rails4用のGemfileを読み込み
eval_gemfile File.expand_path(File.join(File.dirname(__FILE__), '../Gemfile'))

# Rails4でしか使わないものをリストから削除
ignored_gems = %w(
  rails
  has_secure_token
  backport_new_renderer
  rails_kwargs_testing
)

dependencies.delete_if do |g|
  ignored_gems.include?(g.name)
end

# Rails5でしか使わないgemをリストに追加
gem 'rails', '~> 5.2.2'

group :test do
  gem 'rails-controller-testing' # assign などコントローラのテストでRails5で本体から削除された機能の代替
end

今回は開発環境でRails4 / Rails5を並行稼動させるにとどめましたが、バージョン違いのRailsの並行稼動に関しては私の古巣でもあるクラウドワークスさんの以下のブログが大変参考なりました。

engineer.crowdworks.jp

Rails5.2で動かないコードを修正する

デジカルにおいてRails5.2で動かなくなってしまったコードと、それに対する対応をいくつか紹介させていただきます。

Rails5ではテストのget, postなどのメソッドがキーワード引数になった

github.com

Rails5 ではコントローラのテストの書き方が少し変わり、キーワード引数を利用するようになっています。

# Rails4
get :users, search: 'bayleef', format: :json
expect(response).to be_success

# Rails5
get :users, params: { search: 'bayleef' }, format: :json
expect(response).to be_success

当然、このままだとRails4とRails5で同時にテストを通すことはできません。そこでRails5向けのテストコードをRails4でも動かすためのGem 「rails_kwargs_testing」を利用しました。

詳しい使い方については割愛しますが、rspecを使う場合には以下のようにrspecのconfigに追加してあげることで、Rails4でのテストでもキーワード引数を使うように変更ができます。

RSpec.configure do |config|
  config.prepend RailsKwargsTesting::ControllerMethods, type: :controller
  config.prepend RailsKwargsTesting::RequestMethods, type: :request
end

あとはRails4の書き方からRails5の書き方にすればよいですが、こちらも以下のGemを使うことで一括して変換が可能になっています。一部変換結果に不備があるところもありましたが、数ファイル直すだけで動かすことができました。

github.com

コントローラのテストでパラメータの扱いに差分がある

コントローラのテストで渡されるパラメータの扱いに差分があり、Rails5では通らないテストがいくつか出てきました。

nilのパラメータがRails4ではコントローラで削除されるが、Rails5では空文字で入ってくる

# nilの扱いが異なる
post :create, params: { hoge: nil, fuga: "piyo" }

# Rails4のコントローラでは
params
=> { fuga: "piyo" }

# Rails5のコントローラでは
params
=> { hoge: "", fuga: "piyo" }

これのためコントローラで params[:hoge] != nil のようなコードが意図しない動きをするようになりました。

github.com

Rails側の修正は入らなさそうなので、判定を present? に変えることで対応しています

Rails5からコントローラのテストで渡すパラメータが全てStringになっている

特にJSONのリクエストを期待しているコントローラでこの挙動でテストが落ちるようになりました。

# Rails5ではstringに変換されている
post :create, params: { hoge: 1 }

# Rails4のコントローラでは
params
=> { hoge: 1 }

# Rails5のコントローラでは
params
=> { hoge: "1" }

このため、 params[:hoge] == 1 のようなコードが意図しない動きをするようになりました。対策としては、以下のIssueのコメントにあるように、 as: :json オプションをつければRails5でもテストが通るようになります。

github.com

しかしRails4には as オプションはなく、rails_kwargs_testing も as オプションには対応していません。そこで少しrails_kwargs_testingに手をいれて as オプションがRails4でも動くようにパッチを当てています。

github.com

Rails5からはActionController::ParametersがHashを継承していない

これは有名なやつですが、ActionController::Parameters が Hash 継承ではなくなっているので、Hash のメソッドを使ってパラメータをいじっている箇所が undefined method で例外になるようになります。ただ例外で落ちるようなのは分かりやすいパターンで、厄介だったのは is_a? Hash のようなメソッドで分岐をしている箇所です。

実際にデジカルで問題になったのはパラメータを変換する以下のようなコードでした。Rails5では ActionController::Parameters は Hash ではないので、パラメータがそのまま戻ってきます。このメソッドが ActionController::Parameters のみを対象にしていなかったこともあり、原因にたどり着くのは一苦労でした。

  def modify_params(params)
    if params.is_a? Hash
      # DO SOMETHING
    elsif params.is_a? Array
      # DO SOMETHING
    else
      params
    end
  end

Rails5からは `ActiveRecord::Type::Value#type_cast_from_user` が `cast` にリネームされている

Rails5からは ActiveRecord::Type::Value#type_cast_from_user が cast にリネームされています。これだけならまだ良かったのですが、厄介だったのはデジカルでは ActiveRecord::Type::Date に以下のようなモンキーパッチをあてていた点です。

    def type_cast_from_user(value)
      cast_value = # DO SOMETHING...
      super(cast_value)
    end

このコードの移植にあたっては、Rails5では cast にパッチを当てればよいような気がしてきますが、それだけでは意図しない挙動になってしまいます。なぜなら ActiveModel::Type::Date は以下のような実装になっているからです。

      # Converts a value from database input to the appropriate ruby type
      def deserialize(value)
        cast(value)
      end

      # Type casts a value from user input (e.g. from a setter). 
      def cast(value)
        cast_value(value) unless value.nil?
      end

      # Casts a value from the ruby type to a type that the database knows how to understand.
      def serialize(value)
        cast(value)
      end

これを見るとわかるように、単純に cast をオーバーライドするとserialzie と deserialize にも影響を与えてしまいます。結局は以下のようなパッチを書くことで対応しました。

    def cast(value)
      cast_value = # DO SOMETHING
      super(cast_value)
    end

    # もともとcastで呼んでいた cast_value というプライベートメソッドを呼ぶ
    def deserialize(value)
      cast_value(value) unless value.nil?
    end

    # もともとcastで呼んでいた cast_value というプライベートメソッドを呼ぶ
    def serialize(value)
      cast_value(value) unless value.nil?
    end

しかし実はこれだけでも完全にはRails4の挙動と一致させることはできませんでした。具体的には activerecord-import というGemで挙動が少し変わってしまっていました。こちらは影響範囲が軽微だったこともあり対応はしていません。

https://github.com/zdennis/activerecord-import/blob/5b7cb98b17d95c0e26432c43e857bb1c3f5445c1/lib/activerecord-import/import.rb#L955-L985

ActiveRecordへのモンキーパッチは影響範囲が広く、アップグレードが本当に大変になります。今回のバッチは行数でいうと10行にもみたないレベルでしたがそれでも対応はかなり大変だったので、ActiveRecordへのモンキーパッチ、及びActiveRecordの挙動を拡張・変更するようなGemの採用には慎重になったほうがよいと改めて実感したのでした。

その他細かな対応事項

章立てにするほどでもないけど、対応したことを書いておきます。

ActiveRecord::SpawnMethods#merge に nil を渡すと例外がでるようになった

# Rails4
[6] pry(main)> User.where(id: 1).merge(nil)
=>   User Load (0.4ms)  SELECT "user".* FROM "user" WHERE "user"."id" = $1  [["id", 1]]

# Rails5
[2] pry(main)> User.where(id: 1).merge(nil)
ArgumentError: invalid argument: nil.
from /app/rails5/vendor/bundle/ruby/2.5.0/gems/activerecord-5.2.2/lib/active_record/relation/spawn_methods.rb:37:in `merge'

associationの定義の class_name にクラスオブジェクトを渡すと例外になるようになった

# Rails5だとNG
belongs_to :deleted_user, class_name: User, foreign_key: :deleted_by
# Rails5だとOK
belongs_to :deleted_user, class_name: User.name, foreign_key: :deleted_by

render :nothing は削除されている

        if Rails::VERSION::MAJOR < 5
          render nothing: true, status: 200
        else
          render body: nil
        end

timestampのカスタマイズの仕方が異なっている

  if Rails::VERSION::MAJOR < 5
    # Use approved_at instead of created_at for creation timestamp
    # http://stackoverflow.com/a/13457972
    def timestamp_attributes_for_create
      super << :approved_at
    end
  else
    def self.timestamp_attributes_for_create
      super << 'approved_at'
    end

    private_class_method :timestamp_attributes_for_create_in_model
  end

conditionalなdestroy_all、delete_allは使えない

      # Rails4はこれでも動く
      UserRegistrationToken.destroy_all(
        invalid: true
      )

      # Rails5はこれ
      UserRegistrationToken.where(
        invalid: true
      ).destroy_all

Rails4だとassociationで定義されるメソッドに引数を渡しても動くが、Rails5では例外になる

class User
  belongs_to :company
end

user = User.find(1)
user.company(hoge: "aaaa") # Rails4では引数に何を渡しても、例外にならず動いてしまう

Rails5からはmigrationにバージョンを指定してやる必要がある

ActiveRecord::Migration を直接継承できず、 ActiveRecord::Migration[4.2] のようにバージョンを指定する必要があります。 Rails5でもテストが動かせるよう、Rails4のときだけ以下のようなパッチをあてるようにしました。

if Rails::VERSION::MAJOR < 5
  class ActiveRecord::Migration
    def self.[](version)
      raise 'Unknown migration version. only support 4.2 at Rails4' unless version == 4.2
      self # 常に自分自身を返す
    end
  end
end

skip_before_actionは同一ファイルにメソッド定義がないとRails5からはデフォルトで例外になる

raise: false を与えて、例外にならないようにしてあげる必要があります。

    if Rails::VERSION::MAJOR >= 5
      skip_before_action :admin_login_required, raise: false
      skip_before_action :password_change_required, raise: false
    else
      skip_before_action :admin_login_required
      skip_before_action :password_change_required
    end

所感

Railsをバージョンアップし続けたいなら、こまめにバージョンアップしろ、非アクティブなGemに依存するな、モンキーパッチあてるな、テスト書けという学びを改めて得ている
— すーさん二号 (@suusan2go) 2019年2月15日

こまめにバージョンアップする

現状でもRails6のdeprecation warning が大量に出ていますが、そちらを地道に対処していけばRails 6へのアップグレードはそこまで苦労せずに対応できそうな感覚があります。Railsのアップグレードというのは大変なのですが、実は今回修正した内容はバージョンを1つずつあげていれば、きちんとdeprecation warningが出ていたものがほとんどです。こまめなアップグレードが、結果的には対応コストを下げることに繋がるということを感じました

非アクティブなGemに依存しない・使わなくなったGemは削除する

デジカルには過去の開発メンバーの独自Gemが２つ組み込まれており、どちらもRails4.2依存しているという状態でした。今回はそれらのGemが殆ど使われておらず、Gemの一つは完全に使用されていなかったのでGemfileから削除するだけで済みました。こういったGemは使わなくなったときに削除しないと、後から入ったメンバーの調査コストが高くつきます。今回はそれほどボトルネックになりませんでしたが、非アクティブなGemに依存しない・使わなくなったGemは削除することは常に心がけていたいです。

モンキーパッチをあてない

前述しましたが、特にActiveRecordへのモンキーパッチは影響範囲が大きく、アップグレード時の対応が大変になることを実感しました。ActiveRecordの内部実装に激しく依存したGemのアップグレードが辛い理由もよくわかったので、そういったGemを利用するときには慎重になりましょう。どうしてもパッチをあてたいときには、対応するテストをちゃんと書いておきましょう。今回はモンキーパッチに対するテストが厚く書いてあったので助かりました。

テストを書く

単体テストで潰せるRails4 / Rails5の差分はできるだけ潰しておきましょう。E2E等から原因を探っていくのは大変な作業になります。

まとめ

というわけで、現状ではとりあえずRails5で現状のテストが全てパスする状態に持っていくことができました。これから実際にQA等を通して、本番環境適用まで持っていくことになるわけですが、来たるRails6のリリースまでには何とか間に合いそうな雰囲気です。

f:id:suzan2go:20190218145152p:plain

デジカルチームにはこのRails5対応で初めて関わりましたが、Rails + Scala + NodeのサーバーサイドにReactのフロントとモダンな技術スタックで複雑なドメインと戦っており、エンジニアとしてはかなり魅力的な環境だなーと思いました。Scalaをやりたいエンジニア、Railsをメインでやっているけど自分の技術領域を広げたいエンジニア、複雑なドメインと向き合って設計力を鍛えたいエンジニアには特にオススメできそうです。

私は本日が最終出社日となりますので、あとはデジカルメンバーにバトンタッチして頑張ってもらいたいと思います！
また、Rails5からRails6にアップグレードしたいエンジニアの方はぜひ気軽にお問い合わせください！！

f:id:nsb248:20190219171247j:plain — 最後に”ろくろ”を回しています

f:id:hsasakawa:20190219172549j:plain — 集合写真！

jobs.m3.com

デジカルの技術スタックについて

アップグレード方針

Rails4.2とRails5.2を同時に起動できるようにする

Rails5.2で動かないコードを修正する

Rails5ではテストのget, postなどのメソッドがキーワード引数になった

コントローラのテストでパラメータの扱いに差分がある

nilのパラメータがRails4ではコントローラで削除されるが、Rails5では空文字で入ってくる

Rails5からコントローラのテストで渡すパラメータが全てStringになっている

Rails5からはActionController::ParametersがHashを継承していない

Rails5からは ActiveRecord::Type::Value#type_cast_from_user が cast にリネームされている

その他細かな対応事項

ActiveRecord::SpawnMethods#merge に nil を渡すと例外がでるようになった

associationの定義の class_name に クラスオブジェクトを渡すと例外になるようになった

render :nothing は削除されている

timestampのカスタマイズの仕方が異なっている

conditionalなdestroy_all、delete_allは使えない

Rails4だとassociationで定義されるメソッドに引数を渡しても動くが、Rails5では例外になる

Rails5からはmigrationにバージョンを指定してやる必要がある

skip_before_actionは同一ファイルにメソッド定義がないとRails5からはデフォルトで例外になる

所感

こまめにバージョンアップする

非アクティブなGemに依存しない・使わなくなったGemは削除する

モンキーパッチをあてない

テストを書く

まとめ

Rails5からは `ActiveRecord::Type::Value#type_cast_from_user` が `cast` にリネームされている

associationの定義の class_name にクラスオブジェクトを渡すと例外になるようになった