目次
はじめに
この記事は 食べログアドベントカレンダー2024 の10日目の記事です。
こんにちは。食べログ開発本部ウェブ開発2部でサーバーサイドエンジニアをしている朽木です。
食べログのサービスを提供しているシステムの歴史は長く、蓄積された技術的負債の解消は開発効率を向上させるための重要な課題です。
古いコードや仕様の不明確さが原因となり、シンプルな要件であっても一筋縄ではいかないことがしばしばあります。
- 既存コードの調査をしていたが、依存関係の分かりづらい実装がされており調査が難航する
- 既存のコードに合わせて機能の修正を行ったが、対応箇所が多く想定以上に工数がかかってしまう
- 設計を見直したいが、リファクタリングを実施すると工数が膨らむので諦めて既存の設計を踏襲する
似たような課題を抱えている開発現場も多いのではないでしょうか。
こうした課題を解決するため、自分が所属する店舗情報充実ユニットでは開発中に感じたシステムの辛いポイントを「ツラみ」と呼んでAsanaのボードに集約し、負債の根本原因の分析・対策に取り組んでいます。
この記事ではこのボードを分析することで見えてきた課題に対し、負債発生を予防するために設計方針を立て、新規案件に適用した例を紹介します。
具体的には下記のような流れで構成されています。
- 「ツラみ」の収集
- 開発者が感じるシステムの課題を「ツラみ」としてAsanaのボードに収集するフェーズの解説をしています
- 「ツラみ」の分析
- 1.で収集した「ツラみ」を分析した方法とその結果見えてきた課題について解説しています
- 負債解消のアプローチ
- 「ツラみ」を生んでしまう課題を解消するため、入念に設計方針を立てました。特に重視した高凝集・疎結合を実現するためのクラス構造について詳しく解説しています。
- 案件への適用
- 立てた設計方針を新規案件に適用した例とその結果感じた良かった点/改善が必要な点について解説しています。
- まとめと今後の展望
- 本記事まとめと今後予定している負債解消の取り組みについて解説しています。
この取り組みを通じて得られた知見を共有し、同様の課題を抱える開発者の方々にとって参考になる情報を提供できれば幸いです。
「ツラみ」の収集
最初にやるべきことは開発者が感じたシステムに対する課題の収集です。
開発者が案件を通して感じたシステムに対する課題を蓄積するため、店舗充実ユニットではAsanaに
「システムのツラみバックログ」というボードを作成しました。
個人的に感じたことでもよいのでハードルを感じずにチケットを作成できるように「改善案」でも「課題」でもなく「ツラみ」という表現を使用しています。
追加されたチケットタイトルの例としては以下のようなものがあります。
- 異なるデバイス(PC/スマートフォン)、セグメント(国内店舗/海外店舗)で新規店舗登録が実現することは共通しているはずなのに、店舗登録処理が統一されていない。
- 昔の案件で使用された一時処理が残っていてソースコードをとても複雑にしている。
- デバイスごとに分けたい処理がModelクラスに定義されていてModelクラスが肥大化している。
そしてチケットの内容として下記のフォーマットを用意しています。
- 何が、どの箇所が負債になっているのか
- いつ困ったのか
- なぜ困ったのか
- どのくらい困ったのか
「店舗登録処理が統一されていない」の例であれば下記のような具合です。
項目 | 内容 |
---|---|
何が、どの箇所が負債になっているのか | Modelに定義されている各デバイス(国内/海外)の店舗登録ロジック (※実際はGithubリンク等を併せて添付) |
いつ困ったのか | 店舗の電話番号に関する改修をした時 |
なぜ困ったのか | 同じような改修を4箇所に対して実装する必要があり開発の工数が膨らんだ |
どのぐらい困ったのか | 単純計算で工数が4倍増加した |
入力のハードルが上がりチケット作成自体が行われなくなると元も子もないので、このフォーマットは必須項目とはしていません。
まずは気がついたタイミングでタイトルだけ追加し、後から詳細を追記することが多いです。
また、チケットの追加タイミングは特に決まっていません。 開発者の思い立ったタイミングで作成する運用になっています。 チケットが貯まってきたら案件の区切りのタイミングで「ツラみ」の分析を行います。
「ツラみ」の分析
全体を俯瞰して「ツラみ」の根本原因を探るため、個別の負債を潰す対症療法ではなく貯まったチケットの特徴を分析しました。
具体的には貯まったチケットを下記の2軸で割り振ります。
- 重要が高い or 低い
- 具体性が高い or 低い
そして、重要度、具体性の高いチケットを抽象化し、「ツラみ」が生まれた原因の共通点を洗い出します。
すると、いくつかある問題の中でも特に以下の責務分離に課題が集中していることがわかりました。
- MVC+Usecaseにおける各要素の役割が混在している
- 共通の業務ロジックが複数のRailsアプリに繰り返し定義されている
この点について簡単に説明します。
1. MVC+Usecaseにおける各要素の役割が混在している
食べログでは、MVCに加えて業務ロジックを納める場をUsecaseと定めています。 しかし、古くから存在するコードにはUsecaseが無く、ControllerやModelに業務ロジックが染み出している箇所があります。
1つのメソッドで多くの処理を行いすぎて他のレイヤーで行うべき処理を抱えていたり、同じリソースに対するロジックが別々のレイヤーに散らばっていることもあります。
これらのロジックを再整理して適切なModel、Usecaseに定義し直すことが必要です。
2. 共通の業務ロジックが複数のRailsアプリに繰り返し定義されている
食べログのシステムではメインリポジトリ内に複数のRailsアプリケーションが存在しています。
PC用Railsアプリ、SP(スマートフォンWeb)用Railsアプリ、モバイルアプリ向けAPI用Railsアプリ...というイメージです。
これらのRailsアプリではModelとLibは共有していますが、それ以外はすべて独立しています。
この構成により各アプリ間は疎結合になりますが、同じ変更理由を持つ業務ロジックがそれぞれのRailsアプリで定義されてしまうとメンテナンスにコストがかかるというデメリットがあります。
以下の図をご覧ください。これは「ツラみ」として上がった店舗登録処理の例です。
PC用Railsアプリ、SP用Railsアプリの店舗登録処理はUsecase化されておらず、Modelに定義したものをControllerから直接呼び出しています。 各Railsアプリ固有の業務ロジックであるならUsecaseに定義するべきです。
ただし、2.で挙げたようにこれらのメソッドにも共通の処理があります。 どのRailsアプリから店舗を登録したとしても叶えなければいけない業務ロジックの部分です。
前述の通り個々のRailsアプリ同士は原則独立しているのでこのこと自体が悪いわけではありません。 しかし、コアな業務ロジックの変更が入る場合はこれら両方で定義された処理に修正を入れなければいけない状態でした。 この場合、単純計算でも機能追加や修正に2倍の工数がかかってしまいます。 各Railsアプリ同士の独立性は維持しつつ、コアな業務ロジックを集約できる仕組みが実現できれば理想的です。
負債解消へのアプローチ
課題が見えてきたところでこれ以上負債が発生しない状態を作るアプローチをとることにしました。 既存の負債を解消できても新しく実装した箇所で負債が発生するのであればイタチごっこになってしまうため、まずは負債の発生を防止しようという方針です。
具体的にはMVC+Usecaseの役割を明確にするべくそれぞれの役割の整理した上で、 各Railsアプリ間で低凝集が起こっている問題を解消できるような設計方針を立てました。
先の例では各Railsアプリにおいて変更理由が同じになるようなコアな業務ロジックが散らばっていることが問題でした。 これらを別モジュールに切り出して共通化できればメンテナンスコストが半減します。
食べログにはこのような業務ロジックを集約するため、メインリポジトリとは別のリポジトリで管理されている食べログ基盤APIシステムというものが存在しています。
先の例に適用するのであれば下記のようなイメージです。
しかし、頻繁に利用される機能をいきなり食べログ基盤APIシステムに移行してしまうと、トラフィックや通信分の処理速度の増加を気にする必要がある上、問題があって切り戻しを行う場合も一苦労です。
また、中途半端に移行すると分散されたモノリス状態になるため、それこそ管理コストが肥大化してしまいます。
そこで、まずは共通ライブラリを使用する設計方針を考えました。
各Railsアプリ間で共通のライブラリを利用し、そこにService層を設けます。 これらのServiceはその名の通り外部に機能を提供するための1サービスであり、そのServiceが扱うドメインごとにモジュールを分けて定義します。
例えば共通ライブラリ/restaurant_services/restaurant_entry/restaurant_creator.rb
のようなクラスを定義し、ここにはレストラン登録処理におけるコアな業務ロジックを記載します。
しかし、このままだとUsecaseが直接Serviceに依存してしまいます。
食べログ基盤APIシステムに移行する用意が整い、APIへの切り替えを行う場合には各Usecaseのロジックに直接手を入れなければいけません。
同じRailsアプリ内で別々のUsecaseから1つのServiceを呼び出すこともあり得るため、その分修正しなければならない箇所も増えてしまいます。 そこで、Usecase側ではどの外部処理を実行するか意識しなくて良いようにServiceClientという層を挟むようにしました。
元々ServiceClientは外部のAPI等を呼び出すために使用されていたものです。
この層を挟むことでUsecaseはServiceClientの呼び出し先を気にする必要がなくなります。
食べログ基盤APIシステムへ移行する際にも各RailsアプリごとにServiceClientに定義されている呼び出し処理を修正すればよいだけになりました。
単体テストにおいてもあくまでUsecase自身に定義されている業務ロジックのみ検証します。 Serviceを外部のシステムとみなし、Usecaseを検証するRSpecではServiceClientをMockする方針です。
案件への適用
立てた方針を早速次の案件で適用することにしました。
適用対象となった案件はつい先日リリースされたばかりの「食べトクプログラム」です。
この案件について簡単に説明すると、ユーザーが予約した数に応じてスタンプやスクラッチを発行し、 スクラッチを実行すると当選結果に応じて特典がもらえるといった内容です。 詳細が気になる方は下記のページをご覧ください。
こちらは新規で機能開発を行うものだったので、立てた設計方針を適用するにはもってこいの案件でした。 少し簡略化していますが、一例としてスクラッチ発行部分のアーキテクチャを紹介します。
「食べトクプログラム」のModuleを切り、さらにその中でもスクラッチとスタンプ用にModuleを分けています。 新規獲得スタンプに対するスクラッチの発行を実現するUsecaseはバッジ処理用Railsアプリ側に定義し、このUsecaseが下記の流れでスタンプに対するスクラッチ発行を実現します。
- ServiceClientを介して新規で獲得したスクラッチを取得するServiceを呼び出す
- 1.で取得した情報からスクラッチ発行に必要な情報を生成
- 別のServiceClientを介してスクラッチ発行Serviceを呼び出す
実際に方針を適用することで感じた良かった点と改善点/懸念点をいくつか紹介します。
良かった点
実際に適用しながら開発していて感じた良い点としては以下があります。
- 各Serviceの事情がRailsアプリ側に漏れず、疎結合高凝集な状態が作れている。
- 今後別のRailsアプリに今回実装した機能が必要になったとしても実装が容易。
設計方針の内容と重なる部分があるのでかいつまんで説明します。 狙っていた効果が一定得られた、という点だけ伝われば読み飛ばしていただいても構いません。
1. 各Serviceの事情がRailsアプリに漏れず、疎結合高凝集な状態が作れている
「食べトクプログラム」関連の業務ロジックは共通ライブラリ下の専用ディレクトリに集約される形になっています。 さらに、Railsアプリ側から参照されるServiceクラスのメソッドの返り値にはStringやIntegerなどのプリミティブな型か、Struct型のみとするようにしました。 そのため、Railsアプリ側からは不必要なModelインスタンスの参照が行えなくなっており、システムのあちこちにスクラッチに関するロジックが散らばっている状況を避けられています。 また、ServiceClientを挟んでいるおかげでそれより外側からは共通ライブラリを参照していようが、外部のAPIを叩いていようが気にしなくて良いのも狙い通りです。
2. 今後別のRailsアプリに今回実装した機能が必要になったとしても実装が容易
前述の通り、「食べトクプログラム」のコアな業務ロジックは全て共通ライブラリ下にまとまっているため、 今回機能追加を行ったものとは異なるRailsアプリで「食べトクプログラム」の機能が必要になった場合もServiceClientを用意して呼び出すだけで事足ります。 1つのRailsアプリの仕様を変えたいのであればUsecase、「食べトクプログラム」全体で仕様を変えたいのであればServiceを修正すれば良いのでメンテナンスが行いやすい状態になっています。
改善点/懸念点
反対に、適用しながら感じた改善点/懸念点としては以下があります。
- Usecaseの処理がServiceClientの呼び出しだけで事足りる場合、冗長な構成になる
- Serviceから他のServiceの呼び出しを許可するべきか
これだけだとピンとこないと思うので細かく解説します。
1. Usecaseの処理がServiceClientの呼び出しだけで事足りる場合、冗長な構成になる
その画面やバッチ処理でやりたいことがServiceの呼び出しだけで完結してしまうような場合、 UsecaseはServiceClientの呼び出すだけのクラスになり余分に1層増えている状態になってしまいます。 スクラッチ実施の例を見てみます。
注目すべきはスクラッチ実施のUsecaseとServiceClientの関係性です。 この例では1対1となっており、UsecaseはControllerから受け取ったパラメータをServiceClientに渡すだけとなっています。 コードにすると下記のようなイメージです。
class ScratchPlayerUsecase def initialize(...) # ServiceClientの呼び出しに必要なパラメータをインスタンス変数に格納 end def execute # ScratchPlayerServiceClientの呼び出し ScratchPlayerServiceClient.new(...).execute end end
class ScratchPlayerServiceClient def initialize(...) # Serviceの呼び出しに必要なパラメータをインスタンス変数に格納 end def execute # スクラッチ実施サービスの呼び出し ScratchServices::ScratchPlayer.new(...).execute end end
class ScratchController < ApplicationController def play ... # Usecaseの呼び出し result = ScratchPlayerUsecase.new(...).execute ... end end
エラーハンドリングやStructへの格納によるOutput形式の整理があるので実際のコードはここまで単純ではありませんが、UsecaseがServiceの仲介人であるServiceClientに対する仲介人のような形になり、冗長な構成になっていることは否めません。
また、仮に複数のServiceClientが必要な場合もこれらを組み合わせて呼び出すだけであればそれはControllerの役割と被ってしまいます。 Usecaseには各Railsアプリ特有の業務ロジックのみを実装し、ControllerからUsecaseとServiceClientの呼び出しを行う形で整理するとこのような冗長性を排除できます。 一般化すると下記のような形になります。
- 外部(共通ライブラリや食べログ基盤APIシステム等)に定義されたコアな業務ロジックはServiceに定義しServiceClientから呼び出す
- 内部に定義された各Railsアプリ特有の業務ロジックはUsecaseに定義し直接呼び出す
という形の整理をするとUsecaseがServiceClientに依存しない形にもなるので今回はこちらの形を適用すると良かったのではないかと思います。
2.Serviceから他のServiceの呼び出しを許可するべきか
元々方針を立てた際には個々のRailsアプリ側に実装されているコアなロジックを切り離すという視点で設計したため、個々のServiceが依存するケースに重きを置いていませんでした。
そこで、「Serviceから他のServiceを呼び出す場合にServiceClientは挟むのか」という点が議論になりました。 先ほどの例では簡略化していましたが、スクラッチ発行では現在のユーザー属している会員ステージに応じてスクラッチの発行枚数を決定します。 会員ステージに関するServiceは会員ステージ用のModuleに集約しているので、 スクラッチ発行の処理ではスクラッチModuleから会員ステージModuleのServiceを参照する必要があります。
この場合ServiceClientは必要でしょうか。
結論としては食べログ基盤APIシステムの移行理由が同じであれば不要、異なるのであれば必要としました。
例えば「食べトクプログラム」の機能を食べログ基盤APIシステムに移行する場合を考えてみます。
段階的に移行することもありえますが、「食べトクプログラム」の一部は食べログ基盤APIシステム、残りは共通ライブラリに実装されている状態になると前述の通りメンテナンスコストが増えることになります。
全ての機能をまとめて移行することが望ましい場合、これらのServiceはひとまとまりとして扱うのが妥当です。
つまりスクラッチModuleも会員ステージModuleもまとめて移行するため、ここではServiceClientは介さないといった方針でした。
一方そうではないケースも存在します。 スクラッチの一覧情報を取得する際、付随するポイントの情報も取得するServiceが存在しています。
このServiceではポイントの情報を取得するのに「食べトクプログラム」に限らないキャンペーン用のポイント付与を管理するためのModuleで定義されているServiceを参照する必要があります。 すると、まずはキャンペーン用のModuleを食べログ基盤APIシステムに移行し、そのあとで「食べトクプログラム」の移行を検討する段取りになるはずです。 よって、この場合はまとめて移行するケースに該当しないのでServiceClientを挟む必要があります。
ただ、そもそも移行理由が異なるService同士に依存関係が発生すること自体が望ましくないという懸念も残ります。 上記の例では先にキャンペーン用のModuleを食べログ基盤APIシステムに移行させないと「食べトクプログラム」のServiceが移行できない状態になっています。 移行の順番に制限が発生しない設計についてはまだまだ検討の余地があると考えています。
まとめと今後の展望
本記事では、食べログの技術的負債解消に向けた取り組みとして「ツラみ」の収集と分析、設計方針の策定、実際の案件への適用を行った例を紹介しました。
コアな業務ロジックをServiceとしてRailsアプリから分離し、ServiceClientを通じて参照することで疎結合・高凝集な構造を少しずつ実現できたと感じています。
今回は新規案件への適用でしたが、次は「ツラみ」として上がることの多かった店舗登録処理に対して立てた設計方針の適用を計画しています。
リファクタリングや設計の見直しに時間をとれないという場合でもまずは「ツラみ」の収集から始めてみてはいかがでしょうか。
本記事の内容が同様の課題を抱える開発者の方々にとって少しでも役に立つことができていれば幸いです。
最後まで読んでいただきありがとうございました。
明日は @isemono さんの「つまずきから学ぶ、機能開発で回り道を減らす方法 〜トリミング機能を添えて〜」です。お楽しみに!