22
14

Delete article

Deleted articles cannot be recovered.

Draft of this article would be also deleted.

Are you sure you want to delete this article?

More than 5 years have passed since last update.

Business Bank Group DevelopersAdvent Calendar 2018

Day 19

Ruby on Rails 4.2 → 5.0に上げる過程で行った変更を全部書きます

Last updated at Posted at 2018-12-18

この記事は、Business Bank Group Advent Calendar 19日目の記事です。

Rails 5.0 updated!

 というわけで、先日弊社のクラウドサービスであるALL-INをRails 4.2から5.0にアップデートしました。12月中にはアップデート完了するだろうと思って記事を書く予定を入れてましたが、無事に間に合って良かったです。なお、ツイートにもある通り、引き続きRails 5.1へのアップデート作業を行っています。

規模感

rails statsの結果を記載します。業務システムのため、テストについては通常のサービスより、密度を高く作っています。(rails statsの対象ディレクトリを追加して、より実体に近い数字にしました。)

 このテストを全部回しきるのに4並列のCircleCIで2時間半程度かかってしまうため、rrrspecを使用してAWSインスタンスを30並列にして回しています。この仕組みがなければ、テスト結果が返ってくるまでの時間、ずっと待つことになるためRails5へのアップデートは出来なかったと思います。(参考: rrrspecを導入してCI実行時間を2.5時間から10分に短縮した話

+----------------------+--------+--------+---------+---------+-----+-------+
| Name                 |  Lines |    LOC | Classes | Methods | M/C | LOC/M |
+----------------------+--------+--------+---------+---------+-----+-------+
| Controllers          |    849 |    685 |     140 |      39 |   0 |    15 |
| Helpers              |     36 |     24 |       0 |       5 |   0 |     2 |
| Models               |  47695 |  27279 |     465 |    2377 |   5 |     9 |
| Mailers              |    679 |    468 |      15 |      54 |   3 |     6 |
| Javascripts          |  66424 |  43765 |       0 |    5908 |   0 |     5 |
| Libraries            |   5608 |   4588 |      56 |     391 |   6 |     9 |
| Tasks                |   1210 |   1046 |       0 |      16 |   0 |    63 |
| View_object specs    |    944 |    849 |       0 |       1 |   0 |   847 |
| Extension specs      |    197 |    192 |       0 |       0 |   0 |     0 |
| Helper specs         |     89 |     73 |       0 |       0 |   0 |     0 |
| Worker specs         |  15015 |  13411 |       0 |       3 |   0 |  4468 |
| Api specs            |  12446 |  10215 |       2 |       2 |   1 |  5105 |
| Controller specs     |    821 |    699 |       0 |       0 |   0 |     0 |
| Report specs         |   8915 |   7887 |       0 |       0 |   0 |     0 |
| Initializer specs    |    121 |    103 |       0 |       0 |   0 |     0 |
| Query specs          |  27212 |  23665 |       1 |      10 |  10 |  2364 |
| Validator specs      |    127 |    104 |       3 |       0 |   0 |     0 |
| Service specs        | 144050 | 129287 |       4 |      31 |   7 |  4168 |
| Mailer specs         |    945 |    826 |       0 |       0 |   0 |     0 |
| Lib specs            |   7510 |   6068 |       0 |       2 |   0 |  3032 |
| View specs           |  20573 |  18368 |       0 |     147 |   0 |   122 |
| Model specs          |  92812 |  70440 |      13 |      28 |   2 |  2513 |
| Services             |  49572 |  34858 |     566 |    3167 |   5 |     9 |
| Apis                 |  11321 |   9681 |     208 |     239 |   1 |    38 |
| Queries              |   5783 |   4492 |     164 |     261 |   1 |    15 |
| Workers              |   8480 |   6471 |      58 |     436 |   7 |    12 |
+----------------------+--------+--------+---------+---------+-----+-------+
| Total                | 529434 | 415544 |    1695 |   13117 |   7 |    29 |
+----------------------+--------+--------+---------+---------+-----+-------+
  Code LOC: 133357     Test LOC: 282187     Code to Test Ratio: 1:2.1

やったこと

基本方針はRails Guideのアップグレードのところにあるように、以下の手順で修正しています。

  • 各種Gemのバージョンを最新にする
  • Railsのバージョンを4.2.8 → 5.0.7にする
  • Rails Guideの項目を参考にしながら、落ちたRSpecをすべて修正する

 今日の記事は、タイトルにあるように、アップデートブランチのコミットログを振り返り、どのような変更を行ったかを書こうと思います。

RSpecが走るようになるまで

 まずは、単純にGemを最新バージョンにして、Rails 5.0.7にしただけではRSpecのエラーどころか、起動すらしなかったため、修正していきます。主に以下のようなことをしています。

  • FactoryGirlをFactoryBotに名前変更
  • Faker Gemのアップデートに伴う、細かい属性の変更
  • ActiveRecord::Baseを継承したApplicationRecordクラスを作成し、全てのActiveRecordモデルがApplicationRecordを継承するように変更
  • ActiveRecord::Migrationを、ActiveRecord::Migration[4.2]に書き換え
  • CI実行中にdb:dropを叩いているところが通らない(db:environment:setを叩く必要があるため、環境変数DISABLE_DATABASE_ENVIRONMENT_CHECK=1を追加)

 またパット見て分かる、以下のDEPLICATION WARNINGを修正しています。

  • FactoryBotへのアップデートに伴う、Static Attributes使用によるDEPLICATION WARNINGの修正
  • alias_method_chain廃止に伴う、prependへの置き換え
  • before_filterをbefore_actionに変更
  • rails-controller-testing gemの追加
  • .css.sassを.sassにリネーム

 これで、ようやく最低限のrspecが走るようになり、テスト可能状態になりました。

スクリーンショット 2018-12-18 01.07.28.png

Failed 1319件

スクリーンショット 2018-12-18 01.39.23.png

RSpec All Greenを目指して

 ここからは、RSpecが落ちた原因となるエラーから対策を検討し、実際に対策していくことになります。Rails5 Updateに際してありがちなエラーでネット上で調べれば出てくるものもあれば、弊社特有の現象もあります。
コミットログの対策順に紹介します。

ActionController::ParametersがHashのサブクラスではなくなった

-      fail ArgumentError unless value || value.is_a?(Hash)
+      fail ArgumentError unless value || value.is_a?(ActionController::Parameters)

Rails5 Updateをした記事を見ると大抵書いてあるTipsです。Rails5からはActionController::ParametersがHash継承ではなくなったため、上記のようにis_a?(Hash)で判定しているところが全滅しました。
 また、その他にもmerge!してデフォルトのパラメータを結合していたり、has_key?でパラメータの値をチェックしているところもあり、この関連の修正箇所は多かったです。

     let(:params) { ActionController::Parameters.new(data) }
-    let(:data) { nil }
+    let(:data) { {} }

 また、RSpec中にもActionController::Parameters.new(nil)のような記述をしていたところがすべて落ちたため、ActionController::Parameters.new({})に修正しています。

DatabaseCleanerでtransactional fixturesを使うテストで、after_commitが走るようになった

 元々RSpecのテストではDatabaseCleanerを併用していて、DatabaseCleaner.strategyを:transactionにしていました。それによりテスト中はトランザクションの中でテストが走るため、テスト終了後にrollbackがかかるようになっていたのですが、Rails4ではその状態ではafter_commitが走らないという問題があったようです。(参考: https://qiita.com/vivid_muimui/items/92bedef4c30bcbf194a2)

 そのため、Rails4.2ではafter_commitのイベントで作成されるレコードを、別途RSpec内で作成していたのですが、Rails5でこれが修正され、after_commitが動作するようになったため、作成したレコードがUnique Indexの一意性制約に引っかかり、DBエラーになるという事象が発生しました。

 こちらの対策としては、そもそもRails4.2でafter_commitのイベントフックがかからなかったことの問題が大きいため、一度Rails4.2のブランチに戻り、下記のようにRails.env.test?でコールバックイベントをafter_saveとafter_destroyで動作するような修正を入れた上で、落ちたテストをすべて直しました。

if Rails.env.test?
  after_save :do_something
  after_destroy :do_something
else
  after_commit :do_something
end

落ちたテストは、主にFacotry内でdo_somethingのメソッドを特異メソッドで再定義してnilにする、という対応をしています。(skip_callbackの代わり)

AssociationProxyから直接Arrayのメソッドを呼べなくなった

 以前は呼べてたはずなのですが、Assocaitionから直接Arrayのメソッドを呼べなくなったようなので、to_aを入れて、Arrayに変換する処理を追加しています。

-      i = lines.map(&:line_subtotal?)
+      i = lines.to_a.map(&:line_subtotal?)

elasticsearch gemの戻り値のHashのキーがシンボルで返ってくるようになった

 これはRailsというより、ElasticsearchのGemをアップデートしたことによる問題ですが、戻り値のHashが文字列からシンボルに変更されていたため、RSpecを修正しています。

-              "doc_count_error_upper_bound" => 0,
-              "sum_other_doc_count" => 0,
-              "buckets" => expected_buckets
+              doc_count_error_upper_bound: 0,
+              sum_other_doc_count: 0,
+              buckets: expected_buckets

Controller Specの引数が変わった

 Controller SpecのパラメータがHashで渡すように変更されたようなので、それに伴ってController Specを修正しています。

-    subject { post(:authenticate, params) }
+    subject { post(:authenticate, params: params) }

レスポンスボディが空のデータの返し方が変わった

 これまでnothing: trueで返していたレスポンスボディですが、body: nilで返す必要があるようです。

-    render(nothing: true, status: 406) and return unless notification
+    render(body: nil, status: 406) and return unless notification

jquery-ui-railsが標準から外れた

 Rails4.2ではjquery-ui-railsが必要なかったように記憶していますが、Rails5からはjquery-ui-railsを明示的にGemfileに追加する必要がありました。

 gem "jquery-rails"
+gem "jquery-ui-rails"

clear_association_cacheがプライベートメソッドに変更された

 一部更新したデータを正しく取得するため、clear_association_cacheを呼び出して、キャッシュしていた関連を削除している箇所が数カ所ありましたが、Rails5になり、clear_association_cacheがプライベートメソッドに変更されました。

 代替となるメソッドを探していたのですが、#reloadが内部でclear_association_cacheを呼んでいるみたいなので、reloadに変更することで対応しました。

-      clear_association_cache
+      reload

 ただし、この対応によって、未保存のデータがあった場合に、reloadを呼び出すことで変更前まで状態が戻り、RSpecが落ちるようになってしまったため、そこだけ__send__(:clear_association_cache)を呼び出して回避しています。(何か他に良い方法があれば是非教えてください。)

Associationへの代入にIndexによる添字が使えなくなった

 これまで、associationに対してIndexでアクセスしていた方法が廃止され、NoMethodErrorが出るようになってしまったため、<<を使った代入に変更しています。

-            before { model.lines[0] = line }
+            before { model.lines << line }

ActiveRecordの型に関する諸問題

 これについては、Rails4.2 → Rails5アップデートで苦労した事例(その1)で一部のケースについては書かせていただきました。Rails5.1やRails5.2に上げれば直るものもあったため、それまでモンキーパッチを当てています。

 この問題は普通の環境では発生しないため、ネットを探しても全く情報がなく、また対応箇所も多かったため、今回最も苦労した問題でした。

# FIXME remove at rails 5.1
raise RuntimeError, "Remove these files when upgrade to Rails 5.1" if Rails.version.to_f >= 5.1
require Rails.root.join('lib', 'activerecord', 'associations', 'alias_tracker.rb')
require Rails.root.join('lib', 'activerecord', 'associations', 'join_dependency.rb')
require Rails.root.join('lib', 'activerecord', 'associations', 'association_scope.rb')
require Rails.root.join('lib', 'activerecord', 'validations', 'uniqueness.rb')
require Rails.root.join('lib', 'activerecord', 'log_subscriber.rb')

ActiveModel::Errorsのエラー数で重複したものについては、複数カウントされなくなった

 Rails4.2では、同一カラムで同名のエラーが複数出た場合、それらについては別のエラーとしてカウントしていましたが、Rails5では複数カウントされなくなりました。代わりにerrors#detailsというメソッドに個別に表示するようになっています。(参考: Rails5からActiveModel::Errorsにdetailsがはえてた

-        expect(model).to have(5).errors_on(:start_date)
+        expect(model).to have(3).errors_on(:start_date)

ActiveRecordのenumが数値ではなく、対応した値を返すように変更されていた

参考:Rails5でenum定義したカラムの元の値を取得

-      read_attribute(:import_status)
+      self.class.import_statuses[self.import_status]

 この問題が原因だったのですが、下記のように{ "Sunday" => 0, "Monday" => 1, ... }、となるようなenumが定義されていたのですが、これを比較するテストケースでRails4.2時代は0 <=> 1-1となることを期待していたテストケースがあったのですが、Rails5では"Sunday" <=> "Monday"となり、1となってテストケースが失敗していたのが味わい深かったです。

-        self[:weekday] <=> other[:weekday]
+        Setting.weekdays[self.weekday] <=> Setting.weekdays[other.weekday]

Databaseでdecimal型を使用しているattributeにnumericality_validatorのonly_integerオプションを指定すると、エラーになる

 参考:Numercality validator only_integer is not reliable with decimal storag

 これまで使用していた decimal型はprecision 18, scale 4となっており、小数点4位まで記録できるような形式で定義していました。ところがRails5になって、よりデータベースの型に忠実に変換をするようになったためか、例えば42という値だったとしても、42.0と値が変換されてしまい、結果的にnumericality_validatorのonly_integerのオプションがあるバリデーションで失敗するという問題がありました。

対策としてはRails5から導入されたActiveRecord::Attributeを使用して、データベース上の型はdecimalであるものの、モデル上ではIntegerとして扱う、と明示することで解決しました。

+  attribute :total, ActiveModel::Type::Integer.new

ActiveRecordのuniqメソッドがdistinctメソッドに置き換える警告が出るようになった

 uniqは廃止されるようなので、distinctに変更しました。こちらは単純な置換のみでOKでした。

-      uniq
+      distinct

 最初は大きめの問題を解消していたので、エラーの解消速度も早かったですが、残り100を切ったぐらいからが辛く、長い道のりだったように感じます。

スクリーンショット 2018-12-19 03.19.48.png

最後に

 これだけの苦労をしてRailsをアップデートする価値があるのか、という話をしようと思います。
 今回アップデートにあたり、Rails5のバグも大量に踏みましたが、そのバグの中には最新では既にIssueとして報告され、修正されているものも結構ありました。ただし、まだメンテナンス期間内のRailsであっても最新のRailsでなければ修正されてないものも多く、自力でパッチを当てたりする必要がありました。

 そう考えると古いRailsを継続してメンテナンスしていくだけのコストも高いため、開発を続けていくのであれば、可能な限り最新に追従して、そしてテストをしっかりと書いて安全にアップデートしていくという方針でRailsを使っていくのが良いと思いました。

 今回テストをしっかり書いていたのはそういう意味で本当に助かりました。

明日の担当

明日は、当社で最も若いエンジニアである@shota_yamashitaがプログラミング言語について書いてくれるようです。

22
14
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
22
14

Delete article

Deleted articles cannot be recovered.

Draft of this article would be also deleted.

Are you sure you want to delete this article?