概要
昔々の大規模リファクタリングをしたときに、どのように検証を行ってリリースをしたかの話です。
普通にコードのリファクタリングの話も色々と出来ると面白いのですが、今回はイベントとしてこちらのアドベントカレンダーに参加したため、ちょっと変わったリファクタリングの紹介をしたいと思い、今回の事例を書いてみました。
今考えると、もっと穏やかに進める方法があったのかどうかは、考える余地はありそうかなと思ったり、でもちょっと考えるとやっぱり無理かなと思ったりしています。
リファクタリング対象のコードの課題
① リファクタリングの対象のコードの課題
- 変更可能変数をメソッドの引数に渡すとそのオブジェクトが変更されるが、メソッドの外からはどのフィールドが必要なのかもわからず、何が起こっているか分からない
- 変更可能なオブジェクトが完成状態になるまでにかなりのステップが必要
- 変更可能なオブジェクトが完成するステップが整理されておらず、情報の収集と情報の更新が混ざっている状態だった
- if文が溢れていて何が起こっているか分からない
- Cookieの状態が特定オブジェクトの射ではなく、if文による組み立てと分解のため、想定されるパターンが不確定だった
② リファクタリングのコード修正後、確認するときに課題になったこと
- もともとテストが動いていない、動いているテストが少ししか無い
- パフォーマンス改善のためには構造的な変更が必要で、局所的変更ではなく全体の変更が必要だった
- Cookieを利用した処理があり、既存データのパターンを全て想定して変更することは不可能だった
- 既存の処理では非効率な部分や不必要な処理があり、全く同等のアウトプットを期待する処理ではなくなる変更もあった
強引ですがリファクタリングを進めました
既存コードが辛すぎて機能追加が出来ない気持ちだったので、ちょっと強引でしたがリファクタリングを進めました
懸念
細かなリファクタリングはユニットテストを作成しながら進めましたが、構造変更などの大規模な変更は細かなテストを書いても、全体として整合性が取れているかは分かりませんでした。
特に、cookieの読み込みや書き込みの部分は、既存の各ユーザーのブラウザにどんなデータが保存されているか分からないため、想定外のデータで読み込みから始まり、処理によってそのデータの追記や更新などが行われてCookieの書き込みで終わるため、全てのパターンの検証は不可能でした。
決断
思い切った決断として、まとめて気になるところを全てリファクタリングしました！！
構造的な変更や仕様の再検討などを含め、全てを整理し、ユニットテストも主要な正常系では実装を行いました。
結果として自動生成のファイルなどの変更を除き、200-300ファイルの変更と数万行のコードの変更を行ったPull Requestを作ることとなりました。
コミットは何回かに分けていましたが、構造変更の場合は１つの文脈の変更で100ファイル近い変更もありました。ブラウザ上のGithubでは全てのコードDIFFを表示することすら出来ませんでした 
このリファクタリングの詳細については、一部についてこちらの記事で紹介しています。
今日は個別のリファクタリングではなく、どうやって検証したかを説明してくので、細かなリファクタリングのコードは割愛します。
同僚にレビューをお願いしましたが、自分で迷惑極まりないよねと思ったりしました。
当然レビューは不可能なので、レビュー以外の方法で正しさを検証することを考えました。
当然だが普通の方法では検証出来ない
ユニットテストを追加して書いたのですが、最初に上げた理由によって既存の処理と同じになっているかの検証は出来ません。
そこで次の方法でリファクタリングした結果の検証を行いました。
リファクタリング結果の処理が正しいかどうか検証する
そこで、HTTPサーバのProxyサーバーを立ててMirrorリクエストを投げて検証することにしました。
既存の変更前のモジュールをデプロイしたサーバとコードを変更したモジュールをデプロイしたサーバの２つを用意して、それぞれのサーバーに本番のHTTPリクエストを投げるようにしました。
私が検証したときは別モジュールを導入してMirrorリクエストを投げる設定をしていましたが、今ではNginxに標準機能としてあるようです。
サーバの処理の流れ
① HTTPリクエストを投げる
② Proxy サーバーはMirrorリクエストを作成して、②で２つのサーバにHTTPリクエストを投げる
③ それぞれのサーバが処理を完了後、HTTPレスポンスを返す
④ それぞれのレスポンス結果をログに出す
⑤ ユーザーにOriginal Serverのレスポンスのみ返す
⑥ それぞれのレスポンス結果のDiffを取って、検証を行う
注意点
- A. ①のリクエストでルーターを挟み流量はコントロールしていたが、秒間数百リクエストを捌き、レイテンシは200ms以下を目安に動く
- B. ②で投げたリクエストの結果が③のタイミングで同時に返ってくるわけではない
サーバ上でDIFFが取れない
パフォーマンス面でも問題が無いことを測りつつ検証をしたかったため、それなりの負荷条件で動かしました。
リクエスト数が少ないサーバーであればFrontに立てたHttpサーバー上でDIFFの確認もできるかと思いますが、モジュールをカスタマイズする必要がある他、高負荷、低レイテンシのレスポンスを期待したときに、リクエストを投げた片方のサーバーに問題があると、一方のレスポンスを待っている間にメモリ上にデータが溜まってしまい、本番レスポンスを巻き込んで落ちてしまう可能性があります。
ログファイルのデータの問題
ログの出力順はレスポンスの順番で出力されますが、大量のリクエストが流れるため同じリクエストと同時にそれぞれのサーバーからレスポンスが返ってくるわけではないため、ログの順番がバラバラになります。
対応
ログファイルのDIFFを比較するためには、リクエスト時間とリクエストIDを付与しておき、レスポンスが返ってきたタイミングでログに出力するようにして、後で一意のキーで検索することで同一リクエストからのレスポンス結果の合致を取得して比較することが出来るようにしました。
ログファイルを読み込み、相互のDIFFを確認する小さな検証ツールアプリを作って、検証をしました。
結果
ユニットテストでは各所でテストコードは問題がない状態でしたが、当然ですがこれだけのコードを変更したためバグがたくさん見つかりました！ 
十数個のバグを修正後、大きな問題が無くなってから１週間程度運用し、問題が出ないことを確認して、サーバー全台に新しいモジュールを適応しました。
コードのリファクタリングをしたことによって、 イミュータブル前提のコードになったため、機能追加のときに変数の値が変更されるスコープが明確になり、機能追加がしやすいコードになりました！ 
まとめ
リファクタリングはまとめてやると辛いです。細かくやりましょう！ 
まとめてやっちゃったときは、mirrorサーバーを立ててレスポンスをまるごとテストするという荒業の紹介でした！
どうしても仕方が無いと思ってやりましたが、あまりお薦めはできない方法でした 