ページネーションのバグを解消した話
この記事は、Merpay Advent Calendar 2022 の21日目の記事です。
こんにちは。メルペイBackendエンジニアのfivestar（@fivestr）です。
本記事では「SOLID原則」と呼ばれる設計原則に沿って実際に行ったリファクタリングについて、メルペイの「あと払い」サービスの開発現場事情を踏まえながらご紹介していきます。
あと払いの歴史とコード負債
私が所属するCredit Designチームではメルペイの「あと払い」や「メルペイスマートマネー」といった与信サービスの開発を行っています。中でも「あと払い」はメルカリが2017年にリリースした「メルカリ月イチ払い」を前身とする歴史の長いプロダクトであり、単純な機能追加だけでなく、設計上大掛かりな変更を伴う修正を繰り返しながら今日まで成長してきました。
例えば、あと払いをメルカードの決済・清算のバックエンドとして統合させるために従来のオーソリ時請求に加え実売上時請求とのハイブリッド化を実現したり、メルペイスマートマネーの導入のためにあと払いとの請求・清算のバックエンドを共通化し新たなマイクロサービスを導入したりと、既存のアセットを拡張しながらメルペイとして一貫したお客さま体験を提供するためにその都度設計をアップデートさせていっています。
こうしたアーキテクチャレベルの意思決定は事業拡大に必要なことからも十分な検討の上で拡張を進めることができています。
しかしこれらとは別にコードレベルの負債も少なからず存在し、サービスの成長の影でじわじわ開発チームを蝕んでいました。そのようなコード負債の解消を進める上で判断の基準となったのが「SOLID原則」です。
SOLID原則
SOLID原則と呼ばれるオブジェクト指向設計に関する設計原則があることをご存知でしょうか。 Robert C. Martin氏により提唱された設計原則の中から、次の5つの原則の頭文字をとってその名がつけられています。（名称の日本語表記や説明は「アジャイルソフトウェア開発の奥義」に準じています）
- 単一責任の原則（SRP）
- オープン・クローズドの原則（OCP）
- リスコフの置換原則（LIP）
- インタフェース分離の原則（ISP）
- 依存関係逆転の原則（DIP）
オブジェクト指向とありますが、ガチガチなオブジェクト指向のデザインパターンのようなものというよりは、現代的なプログラミングにおけるエッセンスとなるもので、多くの方が知っておいて損はないものだと考えています。
今回はあと払いのマイクロサービスで生じていた課題を説明するために、依存関係逆転の原則（DIP）とインタフェース分離の原則（ISP）の2つの原則を紹介します。
依存関係逆転の原則（DIP）
DIPは、次の2つを示した原則です。
- 上位モジュールは下位モジュールに依存してはならない。どちらも「抽象」に依存すべきである。
- 「抽象」は実装の詳細に依存してはならない。実装の詳細が「抽象」に依存すべきである。
これを図で解説します。「GrpcServer」はgRPCのリクエストハンドラを実装するレイヤー、「DomainLayer」がビジネスロジックを実装するレイヤーで、上位モジュールであるGrpcServerが下位モジュールであるDomainLayerに直接依存している状態です。
これをDIPに従って置き換えると次の図のようになります。ここでは「DomainService」というインターフェイス、つまり「抽象」を定義し、GrpcServerは抽象に依存し、DomainLayerは抽象に基づいて実装している状態となります。
DIPに基づいて抽象（インターフェイス）に依存するような設計にする利点として、下位モジュールが差し替え可能になることです。例えば今回のケースですと、GrpcServerのユニットテストを行うにあたりDomainLayerのテストスタブへの差し替えが容易になり、テスタビリティが向上します。
この考えはオブジェクト指向における「多様性」を実現する上で大変重要な考え方です。
インタフェース分離の原則（ISP）
DIPで抽象（インターフェイス）に依存する必要性は説明しました。ただ闇雲にインターフェイスを定義すればよいかと言うとそうではありません。
そこで登場するのがISPです。ISPは次を示した原則です。
- クライアントに、クライアントが利用しないメソッドへの依存を強制してはならない。
これも図で解説していきます。先程のGrpcServerとDomainLayerに加え、CLIを実装するCommandを追加した例で、GrpcServerはGetUserメソッドを、CommandはCreateInvoiceメソッドを、DomainServiceインターフェイスを経由して呼び出しています。外枠はパッケージを表しています。これは一見するとDIPに則っていて問題ないように見えますが、ISPに違反している状態です。
これをISPに従って置き換えると次の図のようになります。DomainLayerの実装自体は変わっていませんが、各クライアント（ここではGrpcServerとCommand）がそれぞれ必要なメソッドを個別にインターフェイスとして定義しています。
ISPに違反している場合の問題として、本当にその処理が必要とするメソッドが何かわからなくなりコード全体の複雑化を招いたり、テストを書く際に本来不要な依存コードを記述しなければならないといったことにも繋がります。
ISPを意識する上で大事な視点が「インターフェイスのオーナーが誰なのか？」という点です。インターフェイスは本来機能を「使う側」、つまりクライアント側が主体となって定義するべきものです。今回の図ではよりイメージしやすいよう、オーナーシップを持つべきクライアントと同じパッケージにインターフェイスを配置しています。
あと払いマイクロサービスのリファクタリング
あと払いサービスのコードの話に戻します。あと払いのマイクロサービスであるDeferred Paymentマイクロサービスには次のようなコードがある状態でした。実際には様々な箇所でこのようにISPに違反した状態がみられ、処理の依存関係の見通しがわるく、インターフェイス（ここではdomain.DeferredPaymentService）に変更が入った際に本来影響がないはずの箇所にまで影響が及びコードの変更箇所が膨らむなどの悪影響を及ぼしていました。
package grpc
type DeferredPaymentServer struct {
service domain.DeferredPaymentService
}
func (s *DeferredPaymentServer) GetUser(req *GetUserRequest) (*GetUserResponse, error) {
...
user, err := s.service.GetUser(req.UserID)
...
}
package command
type Command struct {
service domain.DeferredPaymentService
}
func (c *Command) CreateInvoice() error {
...
err := s.service.CreateInvoice(invoice)
...
}
package domain
type DeferredPaymentService interface {
CreateInvoice(invoice *Invoice) error
GetUser(userID string) (*User, error)
}
type DeferredPaymentImpl struct {}
func (d *DeferredPaymentImpl) CreateInvoice(invoice *Invoice) error { ... }
func (d *DeferredPaymentImpl) GetUser(userID string) (*User, error) { ... }これをリファクタリングした結果が次になります。前述の図の構成とおおよそ同一で、各クライアントごとにインターフェイスを定義する形式となっています。Goの場合、インターフェイスを実装しているかどうかは暗黙的に解釈されるため、各クライアントにDeferredPaymentImplを注入すれば、それぞれのインターフェイスの実装として振る舞えます。
package grpc
type DeferredPaymentService interface {
GetUser(userID string) (*domain.User, error)
}
type DeferredPaymentServer struct {
service DeferredPaymentService
}
func (s *DeferredPaymentServer) GetUser(req *GetUserRequest) (*GetUserResponse, error) {
...
user, err := s.service.GetUser(req.UserID)
...
}
package command
type DeferredPaymentService interface {
CreateInvoice(invoice *domain.Invoice) error
}
type Command struct {
service DeferredPaymentService
}
func (c *Command) CreateInvoice() error {
...
err := s.service.CreateInvoice(invoice)
...
}
package domain
type DeferredPaymentImpl struct {}
func (d *DeferredPaymentImpl) CreateInvoice(invoice *Invoice) error { ... }
func (d *DeferredPaymentImpl) GetUser(userID string) (*User, error) { ... }これを行った結果、コード変更時に影響を受けるクライアントが明確になり、影響範囲を正しく特定できるようになりました。また、クライアントがテストスタブを用いたユニットテストを行いたい場合に本当に必要な振る舞いにのみ注力できるようになったなど、生産性の改善につながっています。
ISPに沿ったリファクタリング手順
今回のリファクタリングはインターフェイスの分離をした上で既存のインターフェイスを削除するところまでをゴールに、次の手順で進めました。
- 分離したいインターフェイスを参照しているクライアントを探す
- クライアントが必要なメソッドのみを持つ新しいインターフェイスを、クライアントと同一パッケージに定義する
- クライアントが依存するインターフェイスを 2 で定義したものに置き換える
- 当該範囲のテストスタブが分離元のインターフェイスに依存していた場合、 2 で定義したものに対応するテストスタブを実装する
- 分離元のインターフェイスの参照がなくなるまで1〜3を繰り返す
- 分離元のインターフェイス定義を削除する
内容的には単純に見えますが、あと払いのマイクロサービスは歴史的な経緯により対象となるコード量が多く、結果的に数千行の差分を伴う修正となりました。また、日々開発とデプロイを繰り返す中でインターフェイスの拡張を伴う修正はコンフリクトのリスクが高まるため、開発チーム内で綿密なコミュニケーションをとり、タイミングを熟慮の末マージに至っています。
継続的なリファクタリングの文化づくり
今回のリファクタリングはCredit Designチームで導入している「CD Be a Pro Days」という取り組みの中で実施しました。このリファクタリングを進めるにあたってコードを書いてチームに見せたほうが話が早いと思い、まずはCD Be a Pro Daysの2日間を使ってリファクタリングを実施してPull Requestを作成し、開発チームの定例会で提案という形で説明をしました。他のメンバーも認識していた問題だったため提案自体はすぐ受け入れられましたが、差分が数千行と影響範囲も広かったためチーム内で相談の上1ヶ月程度の期間をとって、作業日を決めてリリースを行いました。
生産性改善の取り組みを自らが主体性を持って進められるように導入した取り組みでしたが、このおかげで今回のように少々複雑なリファクタリングも実施できており、よりよいプロダクトづくりをするための環境にチームとして近づけられている実感を得ています。
おわりに
本記事は普遍的な開発組織で起こる課題に向き合うような内容をお送りしました。自分自身技術者として「基礎」を大事にしており、SOLID原則はまさしく基礎となる知恵です。
SOLID原則のような基礎的な設計原則への違反は、影響範囲が小さい間はあまり気になりませんが、影響範囲が大きくなるに連れまさしく利息が膨らむかのように開発者を蝕んでいきます。
そうした問題へ我々開発者が正しく向き合えるよう、基礎技術の活用事例としてご紹介させていただきました。皆さまのご参考になれば幸いです。
なおSOLID原則についてより詳しく知りたい方は「アジャイルソフトウェア開発の奥義」に詳細な解説がされていますので、ぜひ機会があれば手にとってみてください。
明日の記事はfoostanさんです。引き続きお楽しみください。
