Featured image of post コードメカニック【God Object】通知を直したら課金まで壊れた〜溶接した塊を部品に切り出す〜

コードメカニック【God Object】通知を直したら課金まで壊れた〜溶接した塊を部品に切り出す〜

本番サービスの通知改善PRが課金と設定同期を壊した。God Objectを責務ごとに切り出し、Facadeで入口を一本化するまでの整備記録。

月曜朝、三連アラート

金曜夜の八時頃、僕は小さなPRをマージした。

管理サービスの通知機能には、ユーザー情報を取得するたびにDBへ問い合わせるという無駄があった。currentUser() というプライベートヘルパーに、簡単なキャッシュを追加しただけだ。変更は三十行程度で、レビューも二人に通した。週末は何もなかった。

月曜の朝八時半、PagerDutyが三連続で鳴った。

1
2
3
課金処理: panic: runtime error: nil pointer dereference
設定同期: panic: runtime error: nil pointer dereference
通知:     OK

三番目の行を見たとき、頭が一瞬停止した。自分が触ったのは通知だけだ。なのに課金と設定が壊れて、通知だけが正常に動いている。

スタックトレースを読んでいても理屈が分からなかった。Slackで「コードを直す人がいる」と紹介されていた人に連絡した。半信半疑だったが、他に手がなかった。


九時過ぎ、その人がオフィスにやってきた。三十代くらいの、物静かな目をした女性だった。

薄いノートPCとUSB-Cハブ。工具箱はそれだけだった。挨拶は「よろしくお願いします」の一言だけで、隣の席に静かに座った。

「状況を話しますね。金曜夜にPRをマージして——」

「そのサービスのstructを開けてください」

エラーログじゃなくて?

戸惑いながら service_manager.go を開いた。

1
2
3
4
5
6
7
8
9
// 管理サービスの全部入り
type ServiceManager struct {
    db        *sql.DB
    cfg       *Config
    mailer    *Mailer
    stripeKey string
    userCache map[string]*User  // 今回のPRで追加
    logger    *slog.Logger
}

ファイルを下にスクロールしても、メソッドの宣言が終わらない。五十本以上ある。通知を担うメソッド、課金を担うメソッド、設定同期を担うメソッド、ユーザー取得を担うメソッドが、全部ここに並んでいる。

数秒、画面を眺めてからその人が言った。

「全部溶接で一塊になってますね」

「これで二年間動いてたんです」

防御的な返しだと分かりながら、口から出た。この人のことを、なんとなく「親方」と呼ぶことにした。


とりあえず止める

親方が currentUser() を開いた。

1
2
3
4
5
6
7
8
func (s *ServiceManager) currentUser(id string) (*User, error) {
    if u, ok := s.userCache[id]; ok {
        return u, nil  // バグ: nilがキャッシュされていると (nil, nil) を返す
    }
    u := s.fetchUserFromDB(id)  // 退会済みユーザーはnilが返る
    s.userCache[id] = u         // バグ: nilをそのままキャッシュする
    return u, nil               // バグ: ユーザー未存在でもerrorを返さない
}

「週末に退会したユーザーがいます」と親方が言った。「fetchUserFromDB がnilを返す。そのnilがキャッシュされる。次の呼び出しもエラーなくnilが返る。課金と設定同期は全ユーザーに対して処理を走らせる。nilを使った瞬間にruntimeパニックになる」

「通知は?」

「通知はアクティブユーザーだけに送る絞り込みが手前にある。退会済みのユーザーには呼ばれない。だから正常のまま」

金曜夜、パフォーマンスのために足したキャッシュが、月曜の事故を作り出した。そこまでは分かった。

親方が課金メソッドを開き、一行追加した。

1
2
3
4
5
6
func (s *ServiceManager) ProcessBilling(userID string) error {
    user, err := s.currentUser(userID)
    if err != nil { return err }
    if user == nil { return nil }  // ← 月曜朝の応急パッチ
    // ... 課金処理
}

「これで月曜の火は止まります」

再起動した。PagerDutyが静かになった。

「直った…。ありがとうございます。これで大丈夫ですか?」

「動いた。でも根は別です」

「でも今は動いてるじゃないですか」

親方がターミナルで実行した。

1
$ grep -n "currentUser" *.go

十二行のヒットが出た。

currentUser() を使っているメソッドが他に十一本あります。今日は課金と設定にガードを書きました。次に誰かが十二本のうち一本を変更するとき、また同じことが起きます」

沈黙した。反論できなかった。

「じゃあ currentUser() 自体をもっとしっかり書けばいいんじゃないですか。nilを絶対返さないように」


ボンネットを開ける

親方が ServiceManager のメソッド一覧を出した。

課金担当のメソッド群。通知担当のメソッド群。設定同期担当のメソッド群。ユーザー取得の共有ヘルパー。全部が同じ struct に貼り付いている。

「課金の担当、通知の担当、設定の担当——三つが全部溶接で一台になってる。currentUser() は三つが共有する燃料管だ。管を一本直そうとすると三台全部に影響する」

つまり currentUser() を変えると、それを呼んでいる課金・通知・設定すべてのメソッドが影響を受ける——。

そこで初めて分かった。問題は currentUser() の堅牢性ではなかった。あのヘルパーが三つの責務を繋ぐ「溶接」そのものだった。どれだけnilガードを足しても、次に誰かが currentUser() を変えたとき、課金と通知と設定が同時に揺れる。

「これ… God Objectって言うやつですか。聞いたことあります」

「そうです。God Object——本来は無関係な複数の責務を一つの型に集めすぎたアンチパターン。どこかを触ると別のどこかへ波及する」

Monolithic God Object: ServiceManager welds notification, billing, and config sync into a single massive class

引き継いだコードだからと言い訳する気にはなれなかった。二年間、機能を追加するたびに ServiceManagerfunc (s *ServiceManager) Do... を書き続けた。それが正解だと思っていた。今は「通知を改善したいのに課金コードを確認しながら作業する」状態になっていた。

「テストはありますか」と親方に聞かれた。

「ひどいことになってます。ServiceManager のテストを書こうとすると、DBもメーラーもStripeも全部初期化しないと動かない。どこかで諦めました」


溶接を外してボルト留めに

「まず溶接を外します」と親方は言った。「責務ごとに専門型に切り出す」

新しいファイルが開いていく。

1
2
3
4
5
// 責務ごとに専門型
type UserStore  struct { db        *sql.DB }
type Notifier   struct { mailer    *Mailer }
type Billing    struct { stripeKey string  }
type ConfigSync struct { cfg       *Config }

型が増えていく。それぞれが自分の担当だけを持っている。

「これはどこで繋げるんですか。呼び出す側がそれぞれを知らないといけないですよね」

次に ManagementFacade というファイルが開いた。その中で親方はインターフェースを書き始めた。

1
2
3
4
5
// Facade側で、使う操作だけを小さなinterfaceとして定義する
type UserGetter            interface { GetUser(id string) (*User, error) }
type SubscriptionCanceller interface { CancelSubscription(userID string) error }
type GoodbyeMailer         interface { SendGoodbye(email, name string) error }
type AccessRevoker         interface { RevokeAccess(userID string) error }

「あれ、インターフェースを使う側が定義するんですか。型の側に書くイメージが…」

「Goは使う側が “これだけ繋がれれば中身は問わない” と宣言する。規格の口さえ合えば、どのメーカーの部品でも繋がる。implements と書く必要はない。メソッドのシグネチャが合っていれば、自動的に満たしたと判断される」

インターフェースが「接続口の規格」として機能する——そういうことか。UserStoreGetUser を持っているだけで、UserGetter を満たしたとGoが判断する。そして本物の UserStore でも、テスト用の偽物でも、規格さえ合えばFacadeに差し込める。

Facadeの実装が続く。

 1
 2
 3
 4
 5
 6
 7
 8
 9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
// ManagementFacade: 段取りを知る薄い委譲層
type ManagementFacade struct {
    users   UserGetter
    billing SubscriptionCanceller
    notify  GoodbyeMailer
    config  AccessRevoker
}

func NewManagementFacade(
    u UserGetter,
    b SubscriptionCanceller,
    n GoodbyeMailer,
    c AccessRevoker,
) *ManagementFacade {
    return &ManagementFacade{users: u, billing: b, notify: n, config: c}
}

func (f *ManagementFacade) HandleOffboarding(userID string) error {
    user, err := f.users.GetUser(userID)
    if err != nil { return err }  // 存在しないユーザー → panicではなくerrorで返る
    if err := f.billing.CancelSubscription(user.ID); err != nil { return err }
    if err := f.notify.SendGoodbye(user.Email, user.Name); err != nil { return err }
    return f.config.RevokeAccess(user.ID)
}

「じゃあFacadeって… 接続口の仕様書みたいなものですか」と口から出た。

「Facadeは段取りを知る実行役です。HandleOffboarding を呼んだら、UserStore.GetUserBilling.CancelSubscriptionNotifier.SendGoodbyeConfigSync.RevokeAccess の順に動く。その段取りだけを知る型です」

実行役。仕様書じゃない。

「だったら最初からFacadeを作って、中身は後から整理すれば良かったんじゃないですか」

親方が別のバッファを開いた。何も言わず、書き始める。

1
2
3
4
5
6
7
// 「Facadeを先に作った」ケース
type ManagementFacade struct {
    db        *sql.DB  // ← また全フィールドが集まってきた
    mailer    *Mailer
    stripeKey string
    cfg       *Config
}

「見てください」

画面を見て、固まった。

名前が変わっただけだ。中身は ServiceManager と同じ構造になっている。

「Facadeは委譲する部品が要ります。部品なしのFacadeは名前を変えたGod Objectです」

「切り出してから、窓口を立てる——順番が大事なんですね」

Facade(ファサード)——複数のサブシステムをまとめ、呼び出し側に単一の入口を提供するデザインパターン(構造パターン)の一つ。薄い委譲層として機能するためには、サブシステムが先に独立していることが前提になる。

整備後の構造はこうなった。

Refactored Architecture with Facade Pattern: ManagementFacade delegates tasks via thin interfaces (UserGetter, SubscriptionCanceller, GoodbyeMailer, AccessRevoker) to specialized service components

「通知の改善を加えるとき、変更は Notifier の中で閉じます」と親方が言った。「以前は通知の変更が currentUser() という課金と設定の通り道にいた。溶接をボルト留めに換えた——部品を一本ずつ外して整備できる形になった」

「前は通知を直しに行ったら、課金の通り道を踏んでたってことか」

「そうです」


試運転

テストを書いてみた。

以前の ServiceManager にテストを追加しようとすると、全フィールドを初期化しなければ動かなかった。DBもメーラーもStripeも全部モックが要った。途中で嫌になって止めていた。

今は違う。UserStore のテストを書くとき、必要なのは *sql.DB のfakeだけだ。Billing のモックは要らない。Notifier の存在すら知らなくていい。

ManagementFacade のテストでは、各インターフェースに対してfake実装を渡す。

 1
 2
 3
 4
 5
 6
 7
 8
 9
10
11
12
13
14
15
16
17
18
19
type fakeUserStore struct{ users map[string]*User }

func (f *fakeUserStore) GetUser(id string) (*User, error) {
    u, ok := f.users[id]
    if !ok { return nil, fmt.Errorf("user not found: %s", id) }
    return u, nil
}

func TestHandleOffboarding_UserNotFound_ReturnsError(t *testing.T) {
    facade := NewManagementFacade(
        &fakeUserStore{users: map[string]*User{}},  // 空DB
        &fakeBilling{}, &fakeNotifier{}, &fakeConfigSync{},
    )
    err := facade.HandleOffboarding("ghost-user")
    if err == nil {
        t.Fatal("存在しないユーザー → errorが返るはず")
    }
    // God Object版ではpanicだった。今はerrorで制御が戻る。
}

fakeBillingfakeNotifierfakeConfigSync も同じ構造——各インターフェースを満たすメソッドを1本実装するだけで差し込める(fakeUserStore と同じパターンのため省略)。

1
2
3
$ go test ./...
ok  code-mechanic/god-object-facade/after  0.008s
ok  code-mechanic/god-object-facade/before  0.012s

「走れます」と親方が言った。

テストを書きながら、気づいたことがある。以前は ServiceManager に何かを追加するたびに「他のどこかが壊れないか」と確認しながら書いていた。今はそれがない。Billing を変えるとき、Notifier のことを考えなくていい。UserStore を変えるとき、ConfigSync を確認しなくていい。

不安なく書ける、というのはこういうことか。

「次に責務を増やすとき、約束してください。この型に追加しないこと」

「新しい責務は、型を追加します」

親方がノートPCを閉じ、ハブを外した。

一人になって、画面を眺めた。

窓口が一本。ManagementFacade だけ知っていればいい。中の部品は入れ替え放題だ。


整備記録簿

症状原因修理内容
通知PRが課金と設定同期のpanicを引き起こしたcurrentUser() 共有ヘルパーがnilをキャッシュし、全メソッドへ波及UserStore / Notifier / Billing / ConfigSync に責務分割。変更の影響が型の境界で止まる
テストが書けず放置されていたServiceManager のテストには全依存の初期化が必要だった各専門型は独立してテスト可能に。Facadeテストはfake実装を差し替えるだけ

整備手順

  1. ServiceManager の責務を洗い出す(課金・通知・設定同期・ユーザー取得)
  2. 責務ごとに専門型を作り、関連するフィールドとメソッドを移す
  3. ManagementFacade 側で、自分が使う操作だけを小さなinterfaceとして定義する(Goのイディオム: 利用側でinterfaceを定義する)
  4. Facadeのコンストラクタはインターフェースを受け取り、go test でfake実装を差し込めるようにする

注意: Facadeを先に作ると、分割の前に窓口だけを立てることになる。中身に全フィールドを持ち込んだ時点で、名前を変えただけのGod Objectになる。「切り出してから窓口を立てる」が正しい順序。

親方より

「溶接を全部外した。ボルト留めになった。Notifier を直すとき、Billing のコードは読まなくていい。それが整備できるコードだ。Facadeは最後に立てる窓口——部品が揃ってから、初めて意味を持つ」

comments powered by Disqus
Hugo で構築されています。
テーマ StackJimmy によって設計されています。