みなさん、こんにちは！
最近はウマ娘のトレーナー業に勤しんでいるサーバーサイド開発部(PHP)のやまごし(id:ymgn)です。
去年の11月頃に一度Creators Noteへ記事を書かせていただいたのですが、それが社内でも評判が良かったのか
今回Chatworkの10周年記念で行われる lp.chatwork.com
Chatwork DEV DAY 2021 の開催にあたってCreators Noteへまた何か記事を書いてもらえないか？とお声がけを頂いたので馳せ参じました。
余談(宣伝)ですが、去年書いた記事はこちらになります。参考にしていただけるとChatworkが更に便利に使えるようになるので ぜひ一度ご覧いただけると嬉しいです！
さて、最近の私の近況としては、8月に入社してから現行のChatworkのシステムの改善や保守などをメインで担当するチームの一員となって、既存のChatworkのいろいろな箇所に触れながら様々な知識を学んでいき、またたく間に約半年以上が経過しました。
そんな中、最近では新機能などの開発を進めるとともに、保守運用観点でのリファクタリングなども日々おこなっています。
今回はChatworkが急成長を続けるうちに生まれてしまった、技術的負債を一つ解消した話をお伝えできればと思います。
ざっくりした結論
既存のライブラリを継承するのは可能な限りやめよう！(可能であれば委譲しよう)
はじめに
Chatworkはこの度、2021年3月1日にめでたく10周年を迎えることができました。これもひとえに皆様の応援のおかげです。
Chatworkというプロダクトの最初は社内ツールとして開発されていたという歴史があり、今現在でも当時作られた独自フレームワークの上で10年以上急成長しながら開発が続けられているプロダクトになります。
日々、我々のようなエンジニア達によって改善は続けられているものの、急成長を続けてきた中で開発リソースや時間などを考慮した上でその時点での最適解を通ってきていることもあり、今となって世知辛い状態になっている箇所も数多くあります。
その中でも技術的負債となってから約4年もの間、放置され続けていた問題の討伐に成功したので、それがどういったものだったかをお伝えさせていただこうと思います。
目次
- ざっくりした結論
- はじめに
- 目次
- 2つの日付ライブラリと1つの日付クラス
- 今回のリファクタリングのすゝめ
- リファクタリングの方針を決めるための確認
- 決めたリファクタリングの方針
- 実際に取り掛かってみて困ったポイント
- リファクタリングをした結果
- まとめ
- 最後に
2つの日付ライブラリと1つの日付クラス
まず前提として、この記事を書いている現在時点のPHPにおいて、日付や時刻などを操作する際に用いる日付ライブラリのデファクトスタンダードとも言えるものは、2つ存在しています。
1つ目はLaravelなどにも採用されている Carbon
2つ目はCakePHPなどで採用されている Chronos
Carbonは2012年生まれで、Chronosは2015年生まれとなります。
PHP界隈的には、Carbonの方が古参である点やLaravelで採用されているという点、Qiitaの記事などでもかなり知名度があるライブラリだと思います。
一方のChronosですがCakePHPで採用されているものの、私もChatworkに入社するまではCarbonをつかった経験しかなく、そもそもChronos自体知りませんでした。
Chronosが生まれた経緯などについて詳しくはこちらのスライドがわかりやすいのでオススメです。
この2つの日付ライブラリの大きな違いは、
Carbon は ミュータブル (DateTimeクラスをベース)
Chronos は イミュータブル (DateTimeImmutableクラスを拡張)
という上記の2点になります。
Chatworkとしては、2017年末頃からイミュータブルで使いやすい、dateとdatetimeが別れているなどの理由からChronosが導入され、現在は日付ライブラリを使う場合はChronosを使う事が推奨されています。
一方のCarbonについては、Chatworkでは2015年中旬頃に今回の話のメインとなる とある日付クラスを実装するのと共に利用されはじめました。
その話のメインとなる日付クラスというのが、「Timepoint」と呼ばれるものです。
日付ラッパークラス Timepoint とは
当時、私はもちろんいなかったので多少の推察が入りますが、2015年頃にデフォルトのDateTimeだと機能不足だったため、当時主流だったCarbonを利用することになり、そのCarbonでも足りない日付ロジックを扱うためにCarbonを継承する形でTimepointが生み出されたようです。
そうして生まれたTimepointはCarbonを継承しているため非常に便利ということもあり、Chronosが導入されて推奨されるまでは日付操作や計算を行いたい数多くのクラスで利用されるようになりました。
そして現代になり生まれた問題
Chronosの利用を推奨するルールができてから新しいコードではCarbonではなくChronosが使われるようになっていたものの、単純にCarbonの利用している箇所をChronosに置き換えるだけではCarbonはコード上からなくすことが出来ない理由がありました。
既にお気づきの方もいらっしゃると思いますが、理由としては単純で「様々な箇所で使われているTimepointがCarbonを継承していたから」です。
先述した通りTimepointは生まれてからChatworkの様々な箇所で活躍しており、コアに近い部分でいうとフレームワークの一部分でも使われていたり、
その他にもTimepoint越しにCarbonのメソッドを活用している箇所も数多く存在し、簡単に引き剥がすことは出来ない状態なのに変更による影響範囲は計り知れない…… という誰も触れたがらない状態になっており、近日まで放置されていました。
そんなTimepointに対して、私がメスを入れよう！となった理由としては下記の2つがありました。
① composer2へのバージョンアップ時に障壁となった
1つ目の理由ですが、Chatworkもお話してきた通りPHPで書かれているプロジェクトであるため、ComposerというPHPのデファクトスタンダードであるパッケージ管理ツールをつかっています。
簡単に説明すると、こちらはcomposer.jsonに書かれているパッケージ及び依存関係にあるパッケージなども含めて全てコマンド一発でインストール&依存解決してくれる素晴らしいツールです。
そしてそのComposerの新しいメジャーバージョンであるComposer2が2020年10月24日にリリースされたのですが、それから少しした頃にチャットで以下のような事を話しているのを目撃しました。
composer2にあげたいのにCarbonが引き剥がせないと難しいといった話題で、現在は使わないようにしていたとしても思いがけない所でそのままにしているライブラリが障壁となることもあるのだなと思いました。
それとともに、剥がすの大変すぎるって書いてたので「ならちょっとやってみたろうじゃん！」といった気持ちになりました。
（ただ結局Carbonはそのままの状態でもcomposer2には上げられたらしいです、流石！）
② ユニットテストでChronos::setTestNow()で日付を指定してもDBの日付に反映されない
この2つ目の理由が、個人的に改善しようと踏み切ったポイントになります。
ある日新しい機能の実装を進めていた時に、とある謎の挙動と出会いました。
現在の新しいコードではChronosを使うようになっているため、新規のテストを書く場合にChronos::setTestNowを使ってテスト用の日付をセットするのですが、どうもセットしたテスト用の日付がDBに保存した時に自動で付与される日付には反映されておらず、テストが通らないという現象が発生しました。
私はまだChatworkの実装に明るくなかったため、詳しい同じチームのメンバーに聞いてみた所、
Chatworkの独自フレームワークで使っているORMを使ってDBへ書き込みした時に自動で付与される「create_dateitme」や「update_datetime」などの時刻はTimepointを使って実装されており、そのTimepointはCarbonを継承しているためChronos::setTestNow()が効かないのだと判明しました。
もしこのままの状態でDB側の日付などに対してもテスト日付を反映させたいとなった場合は、Carbon側とChronos側の両方でsetTestNow()で設定をおこなう必要があり、そういったフレームワークの内部的な仕様も理解した上で開発をする必要が出てきてしまいます。
また実際にコードを調べてみた所、既にChronosとCarbonの両方でsetTestNow()をおこなっているテストケースがあったりなど、同じような問題を以前から抱えている現状が見えてきました。
そこで、保守運用観点から今後より一層しっかりとユニットテストの整備を進めていくためにもTimepointのリファクタリングに踏み切ることにしました。
駆逐してやる・・・一匹残らず！
今回のリファクタリングのすゝめ
ガチガチに依存しており、どこで使われていてどこに影響が及ぶかわからないTimepointをリファクタリングするにあたって、まずはどういった状態を目指すかを考えました。
目指す状態
setTestNow()の問題を解消したいというのが第一目標で、あわよくばCarbonを駆逐したいという第二目標があったため3つほど案を考えてみました。
① ORMで使われているTimepointをChronosに置き換える
比較的簡単なようですが、DBに関わるコアな部分なため単純に差し替えてみただけでユニットテストが大量にコケました。その上、プロダクトにCarbonとChronosが混じってしまっているままであり、Timepoint自体もそのままなので根本解決にはなっていません。
② Timepointの継承元をCarbonからChronosに差し替える
Timepoint越しにCarbonのメソッドを使っている箇所も多く、CarbonとChronosの違いによって影響が出てきてしまうため難しい所ではあるのですが、成し遂げることが出来ると社内で非推奨となっているCarbonを消すことができ、大きな技術的な負債を返済することが出来ます。
③ Timepointを外部ライブラリから引き剥がし、継承しないようにする
こちらはいわゆる継承ではなく委譲するように修正するというものです。
委譲というのはつまり、元クラスのインスタンスを内部に持っている状態でメソッド内部でその元インスタンスのメソッドを呼び出すという、真の意味でのラッパークラスへとTimepointを改修するという案です。
3つの案の中でこの③の案がクラスを継承させることもなくなり、外部ライブラリへの依存を減らせるといった面で惹かれます。
結論
結論としましては、[ ② Timepointの継承元をCarbonからChronosに差し替える ]の案で進むこととしました。
上記3つの案を出した後にCarbonとChronosの2つの違いについてもざっくりと検証してみた所、用意されているメソッドの挙動についてもそこまで大きく乖離しているわけではなかったこともあり、時間的なコストと与えられるインパクトなどの理由から選びました。
もちろん、③の案が継承を使うこともなく委譲になるので一番理想的ではあったのですが、
CarbonをTimepoint越しに使っている箇所も多いため、日付時刻ライブラリで実装されているメソッドをTimepoint側で再定義してライブラリのメソッドを呼び出すだけになる＆抜け漏れもが発生しそう＆何より時間がかかるといった点、
また、Carbonを駆逐しChronosに統一さえしてしまえば、新たな日付ライブラリに今の時点から切り替えるという選択肢を取ることもなさそうといった理由があり見送りました。
リファクタリングの方針を決めるための確認
リファクタリングをするにあたって、どういった修正方針で進めるかを決めるために調査をはじめました。
まず、Timepointの継承元をCarbonからChronosに差し替えるという手段を取るにあたって、まずは生のCarbonが使われている箇所でどういったメソッドが利用されているかを確認していきました。
そこで、発見したCarbonには存在するがChronos側には存在しなかったメソッドを一旦Carbonの本体からTimepointにまるっと移築した上で、Carbonが利用していた箇所をTimepointを使うように変更します。
その後、一気にTimepointの継承元をChronosに書き換えてみた上で、ユニットテストを走らせた結果のテストが失敗した箇所を見て行くことにしました。
そうして確認を進めた結果、若干CarbonとChronosで挙動や出力が異なるメソッドなどは存在したものの、現在のコード上で使われているメソッドの大半はほぼ同じ挙動をしてくれることが判明しました。
そうして、今回のリファクタリングの方針を決めました。
決めたリファクタリングの方針
以下のような方針でリファクタリングを行うことにしました。
- 現時点でTimepointを使っているコードはChronos化で影響する箇所を治すだけで、基本は触らない
- Carbonを生で使っているコードはTimepointではなく、Chronosに置き換える
- Carbonを型指定しているメソッドの内部でDatetimeで問題ない場合は、DateTimeInterfaceを使う
- Carbonを型指定しているメソッドの内部で日付計算をしている場合は、ChronosInterfaceを使う
- DateTimeを型指定しているメソッドにChronosを渡しても動かないため、DateTimeInterfaceに修正する
既存のTimepointを使っているコードは基本は触らないという意図としては、Timepointである必要がない箇所見つけ判別してChronos化するというのをしてもインパクトが少ない上、更に影響範囲や作業量が莫大になってしまうという事があります。
Carbonを生で使っている箇所をどうするべきかについては、Timepointに置き換えるかChronosに置き換えるかで社内のエンジニアの中でも議論になりました。
そこで「本来Timepointはどういう役割なのか」という地点に立ち返って考えてみた結果、「Timepointは日付ライブラリだけでは解消できない日付・時間ロジックを持たせるもの」と改めてエンジニアの中で再定義し、基本はChronosを使うようにしてTimepointの利用は最小限にするという方針の共通認識を持つことができました。
実際に取り掛かってみて困ったポイント
メソッドの差分
toDateTimeString()
Carbonで toDateTimeString() をするとformat('Y-m-d H:i:s') で表示されるのですが、Chronosだとマイクロ秒も含まれたformat('Y-m-d H:i:s.u')で出力されるといった違いがありました。
その違いによって、toDatetimeStringを使って想定した日時を保持しているかなどのチェックをしているユニットテストなどが落ちました。
解決法
生のCarbonでtoDateTimeString()使っていた箇所のみformat('Y-m-d H:i:s.u')で出力するように修正しました。
(ちなみに元々TimepointではtoDatetimeString()をメソッドオーバーライドしてマイクロ秒で出力するようになっていました)
formatLocalized()
こちらはCarbonにしか存在しないメソッドで、formatでセットされているlocaleに合わせた曜日などを表示したりできるそうです。
調べるとlocaleなども考慮する必要があるため、癖が強いメソッドという説明があったりしたので厄介そうなものだなと感じました。
そのためとりあえず細かいことを抜きして一旦、TimepointにCarbonからformatLocalized()をメソッドごとまるっとコピーしてきて動くようにした後にどうすべきか検討することとしました。
解決方法
コード内のformatLocalized()の利用箇所を改めて確認してみた所、すべての箇所で曜日の変換などはおこなっておらずformatLocalized('hogehoge_%y%m%d') のように日時と決まった文字列を結合して表示するためだけに使われていました。(sprintfかな？)
そのため、シンプルに ‘hogehoge_’ . format('Ymd') という書き方に変更することで、一旦Timepointに移築していたformatLocalized()自体を無事削除することが出来ました。
Carbon::SUNDAY
意外だったのがこちらで、日付ライブラリなので曜日を表す定数が存在するのですが、Carbonの曜日の定数はグレゴリオ暦を採用しており、日曜日が「0」で開始するようになっていました。
一方ChronosはISO（国際標準化機構）に則っており月曜日「1」で始まり、日曜日が「7」で終わるようになっていました。
そうなっていたことで、そのまま利用してしまうと曜日に関連する既存のデータなどは全て日曜日は「0」となっているのに、そのままだと日曜日が「7」になってしまうという問題がありました。
解決方法
こちらについては日付ライブラリ単独ではカバーできないものを担うというTimepointの存在を活かし、Timepoint::SUNDAY_GREGORIAN という定数で「0」定義するようにして、Carbon::SUNDAYをTimepoint::SUNDAY_GREGORIANに置き換えました。
型の違い
Carbonを全てChronosに置き換えた後に細かい箇所も修正し終え、UnitTestが通った！と思っていざ画面にアクセスしてみたところ、動きませんでした。
ログを確認してみると、なんとはなしにDateTimeを型指定されている引数があり、それに対してCarbonの代わりにChronosが渡されるように変わった事で動かなくなっていました。
CarbonはDatetime自身を継承しているためDateTimeを指定している引数にも問題なく渡すことが可能なのですが、ChronosはDateTime自体は継承しておらずDateTimeInterfaceを実装しているDateTimeImmutableを継承しているため、動かなくなっていたのでした。
各クラス単位ではユニットテストは問題なかったのですが実際のコードで動かしてみると結合部分でその違いによる問題が出てくるのだというのを実感しました。（結合テストは大事）
解決方法
元々Carbonを利用していた箇所でDateTimeを型指定していたコードに関しては、全てDateTimeInterfaceを使うように変更しました。
リファクタリングをした結果
業務の大半はモブワークなどをしているためコアタイム外の時間などでちょっとずつ、コツコツ一人でリファクタリングをやっていきました。2月中旬からはじめ、3月頃中旬にレビューでOKをもらった後4月に入ってからリリースを行いました。
普段はコミット数やファイル変更数共に2桁にならないようにPRを分割していますが、今回は対応を進めていくうちに純粋な差分のコード量的にはあまりないのですが、修正に必要なファイル数が多くなってしまい、レビュアーの方々には負担をかけてしまうPRとなってしまいました・・・
ユニットテストは通っているのに動かないといったこともあったので、本番でも想定外の箇所で問題がおこるなどの事が懸念されてリリースの瞬間は非常にドキドキしていました。
結果としては、4月某日にリリースをおこなったのですが、リリース完了後もエラーなどもなく無事Carbonへの依存をなくすことが出来ました🎉
難易度的に火中の栗を拾うようなものでしたが、今回の対応によってChronos::setTestNow()でDBで使われる日付にもChronos::setTestNow()だけでテストの日付を設定することが出来るようになったり、副産物としてsetTestNow()の初期化を忘れてしまっていて別テストに影響を及ぼしていたテストなども見つかって修正することが出来たり、なによりもChatwork社内では非推奨となったCarbonをChatwork上のコードから駆逐することが出来たので、地味ですが良い改善活動を出来たのではないかと思います。
まとめ
今回のTimepointのリファクタリングをして、いろいろな経験ができました。
日付クラスの取り扱いについてのルールを見つめ直すきっかけになった
日付ライブラリの取り扱いなどについて、Chatwork社内ではChronosが推奨でCarbonが非推奨という認識はされていたものの、今回のリファクタリングをきっかけにエンジニアの中でTimepointの取り扱いについての共通認識を持つことができたのは非常に良いことだと思います。
身を持って継承の大変さを体感出来た
これまで、継承はやめよう！といった文献や記事などは数多く見てきたのですが、継承を使ったり使っているコードを使うことはあってもあまり気になったことがありませんでした。
そんな中、今回はその継承しているもの自体をリファクタリングする機会があり、どういった辛さがあるのかというのを体感することが出来たのでいい経験になったなと思います。
その他
開発をしている時などは、私自身の分報に考えている事を色々書きつらねたりしている事が多いのですが、
急に小学生男児が現れたりしました。
最後に
中にはお堅い会社だというイメージを持たれている方もいると噂のChatworkですが、実際の社内ではこのように役職の垣根なく楽しく開発しています！
そして、ChatworkといえばScala！といったイメージも強くなってきていますが、まだまだ沢山のPHPが元気に動いております！
歴史の詰まったPHPを紐解いて新たなアーキテクチャやScalaへ生まれ変わらせて行きたいといった方や、「Chatworkの進化がもどかしい！俺がChatworkをより一層使いやすいプロダクトに変えてやんよ！」といった方はぜひご応募お待ちしてます！
一緒に楽しくChatworkをよりよいプロダクトに成長させていきましょう！
