■ 1. DDDに基づくGoのコードベース（DMMポイントクラブ）
- DDDの実践サイクル:
- 特定領域の問題に対してドメインモデルを設計し、設計したモデルをコードへ反映するサイクルを回す
- このサイクルによりソフトウェアの品質を高め、問題解決を行う
- オニオンアーキテクチャの4層:
- UI層でエンドポイントの定義、ユースケース層でビジネスロジックの実装を行う
- ドメイン層でドメインモデルの実装、インフラ層で技術的なロジックの実装を行う
- 依存関係の逆転:
- 技術的なロジックはインフラ層で実装し、そのインターフェイスをドメイン層のリポジトリに定義する
- インターフェイスをドメイン層側に置くことで、ドメイン層がインフラ層に依存しない形で実装できる
- リポジトリとクエリの併用:
- ポイントクラブではリポジトリに加えクエリにも技術的ロジックのインターフェイスを定義している
- 両者の使い分けが当初は理解できなかった
- 参照系処理の問題点:
- 通常はユースケース層でリポジトリを呼び出してドメインモデルを参照し、ユースケース用の構造体に詰め替える
- 単一のドメインモデル参照なら問題ないが、複数のドメインモデルを参照する場合に問題が発生する
- 複数のリポジトリから各モデルの値を取得して詰め替える必要があり、ループが増加する
- 構造体への詰め替え時に不必要なパラメーターを取得してしまう
- インフラ層で事前に絞り込みをすることができない
- ユースケース専用モデル再定義の却下:
- 特定のユースケース用にドメインモデルを再定義する案は、モデルとコードの乖離をなくすというDDDの思想に反する
- CQRSの部分導入:
- 更新系の処理と参照系の処理を分ける考え方を部分的に導入している
- ドメイン層のリポジトリには更新系の処理を書く、更新系はドメインサービスに書く方法もある
- 参照系のうち純粋な単一ドメインモデルの参照もリポジトリに書く
- 複数モデルの参照など特定のユースケースに依存する参照系は、ユースケース層のクエリに定義する
- コードベースから得た学び:
- ルールさえ理解できればどこにどの実装をすればよいかが明確で、誰が実装しても一定のコード品質を保てる
- 配属当初もルールを覚えるだけである程度のコードが書けた
- 層間にインターフェイスを挟んで独立して実装するため、単体テストが実施しやすい
■ 2. マイクロサービスを横断したGoのコードレビュー
- 発表の位置付け:
- 前回第6回DMM.goで発表された「組織のコード品質を向上させるレビューシステムの取り組み」のバージョン1.0に当たる具体的な話
- DMMプラットフォームの役割:
- 動画や電子書籍など各サービスで共通して使われる機能を提供する基盤
- 会員管理、ユーザーの認証・認可、各種決済、DMMポイントやトレタプラスといった電子マネーの仕組みを提供する
- ツーピザルールのような基準で、マイクロサービスごとに開発チームが分かれている
- 技術戦略としてのGo採用:
- バックエンドAPIの開発にGoを使い、既存のアプリケーションはGoでリライトする
- マイクロサービス共通のエコシステムでもGoを採用し、負荷試験基盤では試験スクリプトをGoで書けるよう整備した
- レビュー提供を始めた背景:
- 昔のDMMプラットフォームはPHP、Java、Go、Scala、Kotlinが混在するカオスな状態だった
- 当時の開発者はGoでの開発が初めての人も多く、詳しい人にレビューしてもらう方が開発効率が上がる
- 先行してGoを採用していたマイクロサービスアーキテクトグループのメンバーとしてレビュアーに参加した
- 提供のルール:
- 依頼は強制ではなく任意で、各チームからプロダクト単位で依頼を受ける
- 指摘内容は強制ではなく提案であり、受け入れ判断は本人に委ね、判断できない場合はチームで議論してもらう
- ベストエフォートで実施し、レビュー待ちで開発が止まらないようマージを許可し、マージ後のレビューも認めてもらう
- マージ後の追加レビューやレビュー内容の訂正が発生する可能性を開発チームに承諾いただいている
- レビュー内容1: Goの書き方:
- 開始から半年ほどはGoの基本的な書き方に関するレビューが中心だった
- Go本体や有名ライブラリのコードを引用し、よく参照されるコーディングスタイルガイドも紹介した
- 名前付き戻り値の扱い、値レシーバーとポインタレシーバーの使い分けを頻繁にレビューした
- HTTPレスポンスボディのクローズ忘れも注意し、これは検証環境のアプリケーションが落ちる原因となっていた
- 変数命名の考え方:
- Goは型がある言語であり、IDEで型が表示されるため、場合によっては簡易的な変数名でも問題ない
- 変数のスコープが短ければプリミティブ型でも簡易的な名前でよく、周辺のコードを読めば用途が分かる
- forループのインデックスを
iとする例がこれに当たる- 名前付き戻り値の使いどき:
- 戻り値がプリミティブ型の場合、名前でどういう値が返るかが分かる利点がある
- 一方で変数のスコープが広くなり可読性が悪くなるというデメリットがある
- 基準を決めずに使っているケースが多く、多くの場合は使わなくてもシンプルに書ける
GetBasketServiceNameのように関数名から返る値が分かる場合は使う必要がない- レビュー内容2: 一般的な開発の知識:
- 命名、DRY、コメントの書き方、モジュールの結合度、デザインパターンなどをレビューを通して説明した
- トランザクションやバリデーションの実装もレビューし、希望するチームにはDDDの導入支援を行った
- 名前と実態の乖離:
PathNameExistsがbool型とerror型に加えint型を返しており、何の数値か分からず使い勝手が悪い- 数値の内容を知るために実装を見に行く必要が生じるため、
GetPathNameExistsAndIDのように名前へ説明を入れる- 多義語の使用回避:
- 「サービス」「アイテム」「データ」といった多義語が変数名に使われている点をレビューした
- DMMのサービスを指すなら
DMMServicesのように具体化する- ファーストクラスコレクション:
- データとデータの振る舞いをセットにし、振る舞いがコードのあちこちに散らばることを防ぐ設計テクニック
- ドメインオブジェクトを構造体として定義し、振る舞いを構造体のメソッドとして実装するチームルールを作っていた
- 一覧に対する操作をサービス層に書いていたため、一覧の型を作成しその型のメソッドとして実装するよう指摘した
- トランザクション処理の配置:
- インフラ層のメソッド内でトランザクション用のコネクションを発行する実装になっていた
- この実装ではドメインオブジェクトをまたいだ処理を作るたびに
UpdateAAndBのようなメソッドをインフラ層に生やすことになる- 結果としてインフラ層がサービス層に依存し、インフラ層の再利用性が低くなる
- MVC+サービスのアーキテクチャであるため、サービス層でトランザクション用のコネクションを発行するよう提案した
- レビュー内容3: チーム間の知見共有:
- チーム間の知見を共有するハブとなり、別チームの知見やプロダクトの実装を紹介した
- 同様の仕組みを実装済みのチームのリポジトリのリンクを貼るといった形で共有した
- ライブラリの使い方、アクセスログ出力ミドルウェア、エラーハンドリングの仕組みが対象となった
- Datadogの分散トレーシング用HTTPクライアントの実装も共有対象だった
- コードレビューのレビュー:
- レビューしたプルリクエストは全てスプレッドシートで管理する
- 週に1回上司とミーティングを行い、自分の指摘内容やコメントについてレビューを受ける
- 提案にはメリット・デメリットを付ける:
- 修正内容だけを挙げてメリット・デメリットを説明できていないことがあった
- 説明がないとレビューされた人が「指摘されたから直す」になり、以後自分で同じコードを書けるようにならない
- メリット・デメリットを具体的に話せない提案は好みの話であり、レビュー対象ではない
- 抽象的な言葉を避ける:
- 「UI層の責務で」といった説明に対し「責務とは何か」と指摘された
- 責務という語は言わんとすることは伝わるが説明にはなっていない
- 以前決めたルールを挙げ、ハンドラー層でHTTPリクエストのパラメーターの有無をチェックする、といった具体的な説明にする
- 半年に1回の利用者アンケート:
- レビューを受けた側の満足度、良かった点、改善してほしい点をヒアリングした
- 5チーム21名中17名が別の開発案件でもコードレビューを受けたいと回答した
- コードレビューに対する好意的なコメントも多く、現場の開発者から割と好評だった
- 良かったこと1: Goのノウハウ共有:
- Goを初めて書くチームにノウハウを共有し、開始から半年ほどでGoの書き方に関する質問はほとんどなくなった
- 半年で書けるようになる姿を見て、Goの学習コストは低いと感覚的に実感した
- 良かったこと2: コード品質のノウハウ共有:
- しばらく経つと品質の高いコードを書くためのレビューをチーム内だけで回せるようになった
- 結果として自分のレビューが徐々に不要になっていった
- チーム内に有識者がいないために導入できなかった設計手法の導入支援もできた
- 良かったこと3: チーム横断の知見共有:
- あるチームが問題に当たった際、同じ問題に当たった別チームの解決策を共有して開発効率を上げた
- 良かったこと4: 課題の可視化:
- 各チームのプロダクトのコード品質はブラックボックス化されていた
- レビューの結果コード品質が高くないと分かり、今後求められる開発者のレベルより低いと判断された
- 最低基準まで引き上げるため、自チーム外へのコードレビューを継続することが決まった
- 大変だったこと1: ハレーションの発生:
- 基本的なプログラミング原則を指摘するレビューを口頭で実施した際、強く反論された
- 直後に上司へ報告し、レビュー先のチームとマネージャーを含めた話し合いで、お互い悪かったという形で収まった
- 提案とルール化していたが、レビューされた側には強制に見えてしまった可能性がある
- 基本がテキストコミュニケーションであったため提案感が伝わらなかった可能性がある
- 普段から口頭でのコミュニケーションを併用し、提案感や気持ちの部分を伝える努力をすべきだった
- 大変だったこと2: 残業の増加:
- 多い時には4つのプロダクトを担当し、1日10プルリク以上を1人でレビューしていた
- プルリクエストが作成されたその日中にレビューする必要があり、残業が発生した
- その日中のレビューが必要だった理由:
- レビュー先のチームにコーディングルールがなく、レビューによってルールの遵守を徹底していた
- ベストエフォートのため本業が忙しいとレビューが遅れ、その間にプルリクエストがマージされる
- ルールを守っていないコードは追加後に修正を頼みづらく、そのまま残りがちだった
- 残ったコードを参考にして書く人が出るため、ルールを守っていないコードが徐々に増えてしまう
- 改善すべきだった点:
- マージ後でもコードを改善してもらうよう働きかけるべきだった
- スプレッドシートで改善の工数を管理し、開発の何パーセントかを改善に充てる提案ができればよかった
- 今後の展望:
- 1年ほど続けるとGoの書き方や品質の高いコードに関するレビューは少なくなった
- 現在はコードレビューだけでなく、生産性を上げるための取り組みを各チームに提供している
- プルリクエストのリードタイム削減に取り組み、デベロッパープロダクティビティチームを創設した
■ 3. DMMポイントクラブでのバックエンド開発
- 開発対象:
- Web版・アプリ版への機能提供としてAPIを実装する
- プッシュ通知の送信やポイントゲットランキングの集計をバッチコマンドとして実装する
- アーキテクチャとディレクトリ構成:
- UI層がAPIとバッチで分かれている
- appディレクトリの中に各層のディレクトリを置き、UI層のディレクトリ配下をbatchとhttpに分けている
- API開発の流れ:
- ドメイン層の実装、Goaによる仕様記述、UI層、ユースケース層、インフラ層の順で進める
- どの層から実装するかはメンバーによってばらつきがある
- ドメイン層の実装:
- ドメインモデルを定義し、リポジトリとしてインターフェイスを定義する
- ドメインモデルにプラスアルファのデータが必要な場合はドメインサービスとしてインターフェイスを定義する
- Goaによる仕様記述:
- DSLでリクエストのパラメーターとレスポンスを定義する
- この定義からOpenAPIのYAMLとGoのコードを生成する
- UI層の実装:
- Goaで生成されたインターフェイスを満たすようにハンドラーを実装する
- ハンドラーの構造体と、ハンドラー生成のための関数を用意する
- ユースケース層の実装:
- リポジトリのメソッドやドメインサービスのメソッドを組み合わせ、ユースケースの一連の処理を実装する
- インフラ層の実装:
- ドメイン層で定義したリポジトリのインターフェイスを満たすように実装する
- DBの操作や外部APIとのやり取りをこの層で実装する
- バッチ開発の流れ:
- UI層の実装のみがAPIと異なり、その他の層はAPIと共通で既存実装を流用できる
- CLIのフレームワークとしてCobraを使用する
- ハンドラーのインターフェイスを定義し、構造体を用意して実装し、最後にコマンドの仕様を定義する
- 新卒エンジニアとしての所感:
- GoaもCobraも未経験だったが、既存コードを参考にすれば開発を進められるほど学習コストが低い
- UI層以外がAPIとバッチで共通化されているため、初のバッチ実装もUI層の違いだけ理解すればよかった
- API実装と同じくらいのコストで完了でき、共通化のメリットを実感できた
- API設計書の共有についての質疑:
- APIのドキュメントはできた時点でS3にアップロードし、モバイルチームからも閲覧できる
- バックエンドのコードができている前提ではなく、コードができるまではモックを使って実装を進めてもらう
■ 4. 単体テストのスケルトンコード自動生成ツール
- ツールの概要:
go installでインストールでき、コマンドで自動的にコードを生成する- テストケースドリブンの形で実装されたコードが出力される
- 必要なテストケース数の考え方:
- ロジックが分岐する箇所でテストケースが増える
- if・else・else if文の数+1と、switch文のcaseの数で必要なテストケース数を考える
- ASTによる分岐の数え上げ:
- 関数やメソッド単位でif・else文やswitchのcase文を数え上げるためASTから解析する
- ASTは木構造であり、ブロックステートメント内のステートメントのスライスを再帰的に処理して数え上げる
- 関数単位で必要なテストケース数を出力するOSSを別途公開しており、
go installで実行できる- ASTとGoコードの等価性:
- ASTはGoのコードをプログラム上で扱えるようにしたデータ構造であり、Goのコードと等価である
- go/astは双方向に変換が可能で、テストコードのASTさえ作ればGoのコードとして出力できる
- スケルトンコードのAST構築:
- テストケースの構造体を定義するASTは、データ構造の定義のみのため宣言的に書ける
- テストケースをループするレンジステートメントのASTを作成する
- ループ内の関数のASTを定義し、Bodyの要素に呼び出す関数を生成するASTを定義する
- 実行する無名関数の引数と型を実装する部分が長くなる
- 無名関数の中身は
fmt.Println("write your unit test")を実装するASTとして作る- gomockの課題1: Returnの型補完:
- gomock側で自動生成されるコードの受け取る型が
anyで定義されているため型補完が効かないanyのスライスであるためIDE等の補完が効かず、任意の値を設定できてしまう- 型の正しさは単体テストのランタイムでしか検証できない
- 本来errorを返す位置にboolを返しても渡せてしまい、実行時にargumentが不正であるとエラーになる
- gomockの課題2: モック実装の負担:
- 関数の中で呼び出しているインターフェイスのメソッドが増えると、モックが不足したり過剰に実装したりする
- 不足や過剰はエラーで返され、実装を見直す工程が必要になり開発体験が悪い
- gomockがanyを採用する理由:
- モックフレームワークには柔軟性が求められ、
any型であれば柔軟に値を設定できる- テスト用の関数を埋め込んで必要なデータ型を返す実装も可能になる
- ただし特定のケースを単体テストで実現する必要がある時点で、対象の関数が複雑になっている
anyに型を渡す場合は内部で型アサーションが起こるため、潜在的に制御結合の可能性がある- 機能拡張の方針:
- gomockの柔軟性を落とさず、かつIDEの恩恵を受けられるツールを目指す
- 自動生成されるコードには変更を加えず、
anyの部分はanyのまま扱うReturnに渡す値が定義したインターフェイスと一致する型となるスケルトンコードを生成する- 一致する型を定義することでIDEが型を読み込み、フィールドに何をセットすべきか補完が効く
EXPECTでmodel.Userを受け取り、リターンは空のerrorを返す形の生成をイメージしている- モックを含む生成の工程:
- 自動生成されたモックファイルのコメント部分を解析し、依存元の情報を取得する
- import文からインターフェイスとモックの依存関係を紐付ける
- 関数の中で呼び出されているCallExpressionと生成されたモックファイルを紐付ける
- ASTを作成し、テストファイルとして出力する
- 依存関係の解決手順:
- モックパッケージを見て、モックの依存元の情報とモックファイルの置き場を取得する
- インターフェイスに依存しているパッケージ、Goファイル、関数、インターフェイスを全て取得する
- 依存元のパッケージ、関数、インターフェイスも全て取得し、依存関係の紐付けとモックのAST作成を行う
- 依存関係の紐付けはロジックが複雑であり、現状はその部分を実装中である
- CallExpressionの解析:
- CallExpressionはExpressionStatementやAssignStatementから取得できる
- CallExpressionからレシーバーの型情報を取得でき、インターフェイスでの型アサーションが可能である
- これによりインターフェイス経由の呼び出しか直接呼び出しかを判定できる
- go/packagesの利用:
- go/astだけではパッケージをまたいだパースができず、実装が複雑で独自実装になる
- golang.org/x/tools/go/packagesはパッケージごとのASTへのパースと型情報の取得を容易にする
- go/astのみで実装した既存ツールとは互換がない部分があり辛い
- go/astでパースした
FuncDeclとgo/packagesでパースしたFuncDeclは==で比較できずtoken.Posで比較する必要がある- モックコード生成に必要な要素:
- モック対象の構造体と、インターフェイスのメソッドに対応する引数および戻り値の型が必要である
- 最終的に出力したいものはモックの構造体を初期化する関数である
- ファイルへの出力は
os.Statで存在を確認し、format.Nodeで書き込む- まとめ:
- テストカバレッジを満たすために必要なテストケースを自動生成するツールを作った
- 必要なテストケースは基本的に分岐を考えればよい
- エクスポートされた関数から呼ばれる非エクスポート関数の分岐はまだ考慮できておらず今後の課題である
- gomockで補完が効かない箇所を自動生成し、楽にコーディングできるツールを作ろうとしている
- パッケージ間の依存をgo/astのみで解決するのは大変なのでgo/packagesを使うべきである
- 質疑: 分岐の網羅性:
- if・else、switchのcaseは全て網羅する形でテストケースを作り、テストカバレッジは全て満たす
- 全てのロジックの分岐を網羅的にするのは無理であり、そこまでは考慮していない
- 「AまたはB」のような複合条件がある場合も、生成されるテストパターンはif文の数+1になる
- 質疑: 文字列連結ではなくASTを使う理由:
- 文字列連結では自動生成されるコードがGoのコンパイルを通るかどうかを検証できない
- 1ケースなら正しいコードを出力できるが、テストケースを増やすとコードが複雑になる
- 複雑になった後でそのコードが妥当であるかを検証できなくなる
- 質疑: ツールの保守:
- 自分が使う分で作っているため、メンテナンスも自分が使う範囲で行っている
- 実際にユーザーがいて運用する場合は、メンテナンスのための仕組みを作る必要がある
- 質疑: テストケースの妥当性:
- このツールはテストカバレッジを満たすことを目的としている
- テストケースの妥当性の担保は難しい話であり、自分の目で確認する形にしている
- テストケースが妥当でない場合は、単体テストだけでなく実装自体を見直す必要がある
- 質疑: パース対象と実行時間:
- 自分が書いたinternalのパッケージをベースにテストケースを作る
- 再帰的にはパースせず、go/packagesで一括してパースする
- 実行は0.0何秒程度で完了し、時間はそれほどかからない
- internalのパッケージが数万行から数百万行に及ぶ場合はASTがメモリに乗り切らず、生成ロジックの見直しが必要になる