今回の記事は、2021年7月・8月の2ヶ月間で実施したリファクタリング「Repository層の新設」の記録です。
TL;DR .
- 2ヶ月間、新規開発を一時的に中断してシステムのリファクタリングを実施し、バックエンドに永続化責務を集約したRepository層を新設しました
- コードの行数も大幅に圧縮された結果ソースコードの見通しが大きく改善し、開発者体験の改善、開発能率向上・保守性向上・今後のバグ発生予防に寄与しました
リファクタリング前のバックエンド構成
ALMSシステムのバックエンドは、いわゆるMVC的な構成で、Controller層、Service層の2層構成となっていました。*1Controller層は薄く切られており、Controller層と同一名のメソッドを持つService層にビジネスロジックが詰まっているような、シンプルでわかりやすさを重視した構成です。
今回のリファクタリングは*2は、Service層の中でもMongoDBとのやりとり部分(永続化責務)を担う層をRepository層として切り出した取り組みとなります。
なぜRepository層が必要だったのか
RDB移管に向けた必要性
ALMSシステムはNoSQL(MongoDB)を採用していますが、RDBへのDB移管を検討しており、その前工程としてRepository層に責務集約が必要という考え方がありました。
従前からシステムの一部の機能について、パフォーマンス上の課題(特定画面の表示の遅延、データ生成の遅延等)が問題視されていました。各課題は今でこそNoSQLのままで解決しましたが、「RDB化することでトランザクション等を活用しないと速くならない、NoSQLでは今後のシステムの安定性や拡張性を阻害する」といった仮説から、RDB移行に向けた準備は数年前から進められていました。*3*4*5*6
今回新規開発を止めて設定された2ヶ月間のリファクタリング期間については、長年稼働を投入しながらもなかなか完了しないRDB移管について、早期決着を目指す上での集中期間という位置付けもありました。
いずれにしても、RDB移管をする前提での
- 7, 8月については準備期間(Repository設置)
- 11,12月で本格的なRDB移行作業を実施(予定)
というスケジュールの下で、開発チームの工数を総投入した2ヶ月間となりました。 *7
同一責務のクエリが無名のまま大量に散在する状態の解消
Repository層の新設については、RDB移管に向けての必要性という観点もありましたが、RDB移管の実施是非を度外視しても、システムの規模を鑑みればやはりRepositoryという概念・責務の切り出しが必要だと考えました。
例えば、「ユーザーが所属しているチーム*8のIDとユーザー自身のIDから、ユーザーと所属チーム情報を取得する」というのは、どんなシステムにでもありそうでシンプルな処理ですが、所属チームの判定やその他細かい条件判定も含めてクエリに起こすと30行弱の分量にはなるものです。この頻出の処理が、従前では無名のままソースコード全体に散在している状況でした。*9
そこで今回は、リファクタリングのスコープを「DBとの入出力関連箇所をバックエンド全体で漏れなくRepository層に起こす」*10と絞った上で、各Collectionと1対1対応でRepositoryを新設する設計としました。*11
実施内容
RDB、NoSQL両方をカバーするRepository層を新設
RDB移管については、原則実施する前提としながらも「する・しない」は実は確定していない状況*12であったために、Repository層は
RDB移管を実施する場合・しない場合いずれにおいても、意義のあるリファクタリングとする
という要件を持つこととなりました。つまり、Repository層はRDB・NoSQLいずれかに最適化することはせずに、両者のインターフェースとして機能するものにします。
全く異なる種別のDBをまたぐRepositoryとなることは一見とても厳しいですが、
と考えて、「焦って最適化せずに多少抽象度が低くてもしっかり具体的な責務に沿った関数命名をしてRepositoryを書くことで、リーダブルなRepositoryを作る」という指針で取り組むことにしました。*13
Repository実装の技術的な指針議論
前提として、今回の対象スコープは「DBにて永続化されるデータ」に関わる改修となり、不正クエリの実行はフロントの表示バグ等とは異なり取り返しがつかないことから、非常にシビアでクリティカルな領域です。
そのため移植の安全性を再重点において、既存のクエリロジックを一旦は愚直に移植して適切に命名をすることを原則としました。
多少議論はありましたが、今回作るRepositoryは「初段リリース」として具体性の高いメソッドで書き起こして*14、将来Repositoryの状況を見て必要に応じて抽象度を上げた書き方に(もしくはRDB、NoSQLに最適化した書き方)に変えていく段階的リファクタリングにするべきという方針でコンセンサスを取りました。*15
開発チームでタスク分担
上記のとおり抽象度を下げた実装方針としたことから、役割を分担して他エンジニアにタスク分担する上でも方針や実装の品質差分が出にくく、チームワーク・タスク分担を機能させやすくなりました。
対象コード量がMongoDBとのやりとり箇所全体となり凄まじい分量となること、また作業のシビアさから、役割分担して入念に検証工程を組みました。
- 1つのRepositoryに対して、
- 実装者・レビュー者(自分)を設定
- 実装者以外のレビュワーもアサインして多重チェック
- 動作検証についても期間を設けてチェックリスト化、分担
- 様々なチームの環境にて検証
段取り化と慎重な準備をしたことで、不具合やパフォーマンス悪化という取りこぼしを出さずにオンスケジュールでリリースまで結了できました。
工夫
泥臭い取組みでしたが、以下の観点については多少の品質上の工夫・技術的な工夫ができたと思います。
命名規則などは、横櫛で目を通して品質標準化
品質標準化については、日次でエンジニアのMTGルームを設けて細かい目線合わせをしながら実装とレビューを進めたことで、異なる実装者間での品質差分が少ないコードベースとなりました。
各RepositoryがBaseRepositoryをimplementする形に
各Repositoryの土台となるBaseRepositoryを実装して、各RepositoryはBaseRepositoryを継承*16する形としました。
- 大量データ取得でDBへの負荷が高い処理については、sleepを入れた処理にする必要がある
- id一覧からデータを取得するクエリがMongoのクエリ文字数上限に引っかかりやすいので、安全に操作したい
といったような、元々存在しているRepository横断の関心事*17を安全に解決するためです。
単体テストの実装
Repositoryのメソッドの中でも特にビジネスロジックのシビアな判定がある箇所について、新たに単体テストを実装しています。
なお、各RepositoryはClassで実装*18しており、今後はMockで使い回すことも可能となるため、Service層以上の他レイヤーでの単体テストへのInjectが可能となります。このことで、将来のテストの疎結合化や高速化*19にも寄与していく見込みです。
実施の成果
ソースコードの見通し向上
VSCodeのようなエディタ上で一目で関数全体を見渡して変数スコープも識別できる理想的な状態に、かなり近づきました。
DB関連の箇所が適切に関数命名されているため、機能追加や保守に当たってもビジネスロジックに集中できることから、リーダブルになりました。今後のDX向上、開発生産性向上、保守性向上、バグ発生数の低減に寄与していくはずです。
凝集度が高まり、システムとしてより堅牢で開発効率の高い姿に
責務を1箇所に固めすぎずに適切な粒度で切り出す・分解するということは、システム開発に限らずどの職務においても、業務をスケールさせていく上で必要不可欠な営みだと思います。
今回「データの永続化」というスコープで責務を切り出したことで、コードの凝集度が高まり堅牢性を高めることができました。
コード行数の削減
いかに重畳したコードが多かったのかの示唆になりますが、データ入出力に関するコードの総行数は従前から4割減を達成してます。*20
特に、RPGにおけるラスボスともいうべき1万行を超える肥大化したファイルがありましたが、リファクタリングを通じて6,000行台まで削減しました。今後更に責務分解して分解していける見通しも立っています。
振り返り
リファクタリングは、ビジネスサイドのメンバーやユーザーに喜んでいただける新規機能の開発とは異なり、
- 既に動いているコードの改修となるため、成功しても直接的に見える価値が生まれない
- 万が一何かミスが生じていた際には、関係各所に迷惑をかける
という厳しい仕事です。成功の恩恵は将来的な開発工程を通じて長期にわたって発揮されて回収されていくため、短期的な成果は出ず、技術チーム以外へのインパクトも薄いものとなりがちです。
とはいえ、近年の障害・不具合の増加からシステムの内部構造を改善する必要性は開発チームに浸透していたことから、メンバー一丸となって根気強く取り組めました。結果、大きな不具合も出さずに無事8月末リリースを結了できたことが、一番良かったポイントだと感じています。
今後の開発能率改善・エンジニアDXの改善にも大きく寄与したはずですし、「RDB移管を実施するか否か」という大きな論点に対しても、実施の是非に対しての明確な根拠・立ち位置を見出すことができました。
今後も継続するリファクタリングの取り組みに向けて、非常に実りのある2ヶ月間になったと振り返っています。
*1:SPAのためViewはないです
*2:厳密には命名に沿ってませんが、ドメイン層とインフラ層をTypeScriptでクラスが型情報を兼ねること、Repositoryのinterfaceとimplで層が別れると開発者の混乱を招く懸念もあったことから、一旦はInterfaceを切り出さずに「Repository層」という広めの取り方をしています。
*3:私が入社する前からの取り組みとなるため、半ば伝聞です
*4:RDB移管を実施するべきか否かについては、難しい課題でした。今のAidemyのサービス特性・システム規模からは、白紙から作れるのであれば関係データベースが理想であることは論を俟たないと思いますが、現行のNoSQLからの移管についてはシステムの規模から逆算した実現可能性・工数・効果といった観点も考慮しながら、慎重に準備・検討が進められてきたポイントです。
*5:特に、システム障害発生が頻発していた時期には、「NoSQLからRDBへの移管を達成することで、パフォーマンス課題が解決するとともに、データの不整合も解消してシステムの堅牢性が向上する」という言説も強まっていたと記憶しています。
*6:NoSQLの場合は1事実1箇所となりづらかったり、不整合データのバリデーションが甘かったりと、テーブル正規化の概念があったり二次元表で見やすいRDBに比較すると、どうしても堅牢性に欠ける側面はあります。ただし、大規模障害発生の原因となったシステムへの過負荷については、NoSQLを利用していたことによるものではなく、ソースコード上の問題(1アクセスで同じAPIを1個ずつ数百回叩いている初歩的なものから、前計算がないために計算量的に厳しくなったクエリ、等多岐に渡ります)に起因する箇所が大半で、バグ頻発の悪循環についても永続化の箇所ではなくソースコード品質起因のため、NoSQLであることが原因ではなかったと理解しています。
*7:7・8月をRDB移管実施ではなくその準備期間とできたのは、以下の議論について合意形成を得られたためです。1. 現時点では次章の課題があり、延べ数千箇所をRDBクエリ版に書き換えることとなるので工数的に不可能。書き換えても今以上にコードベースが発散して悲惨なことになるため、前準備としての整理(≒Repository層の新設)を欠かしてはならない。2. 移行によるPros, Cons(特にRDB側)を十分に検証できていない。Repository層を起こす中で見える部分も大きいため、7, 8月はその検証期間としての位置付けにもしたい。
*8:チーム: 会社の社員などが所属する特定のユーザーの集団のこと
*9:あくまで例ではシンプルなものとしましたが、より複雑な条件のクエリで繰り返し利用されているものも多く、箇所によっては意味が明らかでない微妙な差異が生じていたりと、厳しい状況でした
*10:実は他にも、キャッシュ(アプリ初期化時にDBからlistAllして手元のシングルトンオブジェクトに状態保持する戦略)への依存解消という論点もありました。ここはシビアに計算量を考慮する必要がありパフォーマンス懸念も強い箇所でしたが、無事依存解消しています
*11:各CollectionのMongoスキーマ(JSON構造)とドメインモデルがイコールの状態で、Service内の各ロジックはドメインモデル自体に対して全面的に依存している状態でした。そのため、RDBに移すとなった場合でも既存ドメインモデルへの依存を解消することはできないため、RDB移管の実施自体を最優先する場合には第一ステップとしてはモデルをRDBのテーブルから再構築させる形での移管となりますが、その場合には特にネックとなっていた箇所でSQLの良さを引き出す書き方がし難く、当初目的としていたパフォーマンスアップにはつながらないことが想定されています
*12:NoSQLでもパフォーマンス箇所は解消可能であれば移行不要という考え方もありました。その立証のために、Repository設置の裏側で同時並行的にCSV等のパフォーマンス懸念箇所についての改善にも取り組んでいます。詳細: https://alms.dev/entry/2021/09/10/174857
*13:具象的であることでRepositoryがfatになる懸念はありましたが、実際整理したところ全く問題のない規模に収まっています。従来コードに重畳が多かったことも起因して、責務自体の数はそれほど多くなかった印象です。
*14:但し、NoSQLもしくはRDBの片方を想起させる関数命名は許容しませんでした
*15:とはいえ、コード箇所によってはクエリを変更するなどかなり踏み込んでリファクタリングしたケースもあります。
*16:Scalaなど他言語であれば、Traitにメソッドを持たせてComposeするやり方もあったかとは思います。TypeScriptについてはInterfaceのみでメソッドを持てないこと、今回の継承はリスコフの置換原則には沿っていることから、今回は継承の形としました
*17:実は、BaseRepositoryが担うこれらの関心事はMongoDB向けに特化しているので、RepositoryをNoSQL(Mongo)とRDB共用にするという前提に反しているため問題ですが、Base自体は薄くRDB移管しても組み直しは容易と考えたので、この形としました
*18:ClassやInterfaceを使わずに関数のみで実装されている箇所が現状だと過半です
*19:現行のテストはDBに直接読み書きする形式で直列実行のため、結構時間がかかっています
*20:Pythonのスクリプトでソースコードを文字列として解析して、スプレッドシートにて集計・前後比較して効果測定しています