こんにちは。
プレミアム課金開発の @ingen084 です。
最近では Google Play 支払いの対応 に関わったりしていました。
はじめに
昨年、弊チームでは ニコニコで12年運用した決済システムを移行する上で必要だったこと1 など大きな変化があった2のですが、その後どのように分離されたものを改善できたかなどを書いていきたいと思います。
システム分離に比べ内容としてはごくごく普通なもの(のはず)ですので、あまり期待せずに見ていただけたらと思います。
ローカル開発環境の作成
入社すぐだったのもあり、まずはコードや環境の把握も兼ね、ローカル開発環境を作成することとしました3。
vagrant 上のVMで本番環境と同じ ansible-playbook を使用し、大体動く環境を作成しました。
あくまで ansible をそのまま動かすという目標だったので、結構いびつな環境にはなっています…。
さすがにコードを組む開発には大規模すぎるとか、ガッツリansibleを書くことも少なくなってきたなどもあり、ここ最近は docker-compose を使用して開発環境を作れるようにしています。
各種フレームワークの導入
詳しくは 10年戦士のレガシーPHPを改善するためにやってきたこと / レガシーなプロダクトに Laravel を導入する第一歩(Laravel DI と Facade) もご覧ください!
Laravel, phpcs などは既存のコードに直接導入したわけではなく、既存のコードとは別のディレクトリに Laravel のディレクトリを作成しました。
レガシーなコードからLaravel側に実装された機能を利用する形が整い、コーディング規則のチェックや単体テストが充実した環境で実装してゆけるようになりました。
結果として分離されたコードのリポジトリのディレクトリは下記のような形になっています。
/ - リポジトリのルート
/src/ - コードのルート
/src/web/ - Webフロントルート
/src/api/ - APIのルート
/src/laravel/ - 新設されたlaravelのディレクトリ
...
移行の度合いや実装状況に応じ、ルーティングなどもLaravelの仕組みを使用するように置き換えられていく予定です。
レガシーなコードへのphpcsの導入
各種フレームワークの揃った環境が用意でき新規で開発する分には気持ちよく開発ができるようになりましたが、とはいえまだ環境を用意しただけで、ほぼ全て従来のレガシーなコードをもとにページが動いています。
そのため保守などではまだまだレガシーなコードを触る必要がありますが、これらのコードは書かれた時代や場所によってフォーマットも統一されておらず、シンタックスのチェックなども全くされていませんでした。
そこで最低限の状態を保証すべく phpcs を導入することにしたのですが、システム分離の際に必要なコードをフォルダ単位で切り出してきたこともあり不要と思われるコードが大量に残っている状態でした。
不要ファイルの削除
そこで不要なファイルを削除するにあたり、まず実行中の状態により参照が変化する
- 変数付きの
require
/include
Reflection
クラスcall_user_func
- 可変変数 / 可変関数
などを重点的に調査することにしました。
幸い利用されている箇所はファイルの削除に影響を及ぼさないもので、実行の状態に左右されずファイルの削除をすることが可能ということがわかりました。
コードに記載されている文字列から機械的に削除できるということがわかったため、ファイルのパスや namespace
class
などでの定義、require
や use
new
などでの参照を正規表現で4ファイルごとにリストアップ・定義し参照を計算することで不要なファイルを削除するツールを作成しました。
最終的に利用していないのに require
しているファイルなどがいくつかあったため数日程度手動で参照を落としていく作業を行ったのですが、これらの作業で2000ファイル以上、割合としては70%あまりを削除することができました。
phpcsの導入
ファイルを粗方絞ることができたため、いよいよ本格的に phpcs を導入していくことにしました。
まずは phpcbf で自動整形をかけ、その後 phpcs の結果を見ながらどうしてもソフトウェアの設計などに影響してしまい軽い変更では対応できないものは除外することとしました。
その後どうしようもないものは除外を行いつつ、自動整形ができずに残ったものは手動で対応を行いました。
シンタックスエラー(!?)と、アクセス修飾子をつける物が多かったように感じます。
こうして完成したレガシーコード向けの phpcs の設定ファイルがこちらです:
<?xml version="1.0"?>
<ruleset name="PSR12/legacy">
<arg name="extensions" value="php" />
<!-- PSR12 から現状のレガシーなコードに合わせていくつかルールを無視させる -->
<rule ref="PSR12">
<!-- 1行の長さ 現状ではかえって見づらくなる可能性があるため一旦無視する -->
<exclude name="Generic.Files.LineLength.TooLong" />
<!-- autoloader を使用していない結果 require で出てしまう -->
<exclude name="PSR1.Files.SideEffects.FoundWithSymbols" />
<!-- 基本 namespace は使用していないため出てしまう -->
<exclude name="PSR1.Classes.ClassDeclaration.MissingNamespace" />
<!-- require 使用の都合上簡単に修正できない -->
<exclude name="PSR1.Classes.ClassDeclaration.MultipleClasses" />
<!-- 既存の名前付けの問題 -->
<exclude name="PSR1.Methods.CamelCapsMethodName.NotCamelCaps" />
<exclude name="Squiz.Classes.ValidClassName.NotCamelCaps" />
<exclude name="PSR2.Methods.MethodDeclaration.Underscore" />
</rule>
...
</ruleset>
一番大変だったのが const
にアクセス修飾子をつける対応で、参照の検索をし手動でつけるのが大変だったので似たような予定がある方は除外しておくことをおすすめします。
こうしてスペースや改行などが統一されたコードはコードレビューに出すのですが、大体100ファイル前後で変更が多く競合しやすいディレクトリは細かく分割してレビュー依頼を出すことで無事マージすることができました。
また、同時に動作検証も行ったのですがシステム分離ですべての機能に対する動作検証項目と手順がまとめっていたため、比較的スムーズに動作確認を行うことができました。
(改めて、大変な量のコードレビューと動作検証ありがとうございました!)
デプロイの改善
システム分離によって、コードのビルド+デプロイ(ローリングアップデート) という2つのアクションでリリースを行っていたのですが、これにはいくつか問題がありました。
- リリース時は基本的に連続してこの2つの操作が行われるため、操作が煩わしかった
- ビルド忘れや操作の競合により、意図しないブランチをデプロイする可能性があった
- デプロイのジョブですべてのサーバー群に順番でデプロイしていたためデプロイに非常に時間がかかっていた
このため、ビルドとデプロイジョブをキックするあたらしいジョブを作成し、デプロイの高速化を行いました。
これによりデプロイ作業中かどうかが1つのジョブの実行中かどうかで判断できるほか、万が一のロールバックの際にも素早く巻き戻すことができるようになりました。
この改善で作成したパイプラインのコードなどはチーム内の他のシステムのデプロイにも活用されることとなりました。
リファクタへの着手
こうしてある程度の環境が整い、いよいよレガシーなコードをモダンな環境へ移植していく準備ができました。
具体的な進め方としては現状の仕様などをドキュメントにまとめ、それをもとに・・・などできるはずもなく
ページ遷移や処理の箇所などもバラバラで複数の決済手段にまたがって共通で処理しているように見せかけて処理できていない箇所なども多く、現状の仕様がまとまった資料がないどころか新しくまとめることすらも難しい状態5で、地道に仕様や意図を読み解きながらあたらしい実装に乗り換えていく といった泥臭い方法6で今現在も移植していっています。
僕は入社した主な目的がレガシーな部分の改善だったため、楽しく(?)やらせてもらっています。
今後に向けて
こういったリファクタを行うことで大きく保守性が向上し、安全に新機能や改修が行えるようになるはずです。
その利点を活かし、現在の不便な部分や今の時代にそぐわない箇所の改修を進めていくことができればなと考えています7。
さいごに
完全にすべて僕がやった というわけではありませんが、主にチーム内での改善活動をまとめることができたかと思います。
ありきたりな内容ではありましたが、まだまだ残っているレガシーなPHPを改善する人の参考になればと思います!