レガシーをクリーンアーキ化するとき「バグを直さない」勇気が要る — characterizationテストと原典忠実移植

概要

肥大化したコントローラを抱えた業務システムを、クリーンアーキテクチャ寄りの構成(Controller → UseCase → Repository)へ段階移植した経験を一般化してまとめます。核心は3つです。

  1. ふるまいを characterizationテスト で固定してから動かす
  2. 元コードを 「原典」として忠実に移植する(直したくなっても我慢する)
  3. 見つけた潜在バグは 直さず「検出して起票」する

社名・プロダクト名・具体的な件数は伏せ、再現可能なパターンだけを抽出しています。

レガシー移植には相反する2つの力がある

この種の作業には、構造的に相反する2つの力が働きます。

  • 構造を変えたい:コントローラにDB操作・整形・分岐が詰まっていて層分離したい
  • ふるまいを変えたくない:本番が今の挙動に(バグ込みで)依存している

厄介なのは、移植中に「成功レスポンスを返すのに保存処理が呼ばれていないエンドポイント」や「フロントから既に呼ばれていない死んだ機能」といった明らかな不具合を見つけてしまうことです。直したくなりますが、移植と同じPRで直すと「構造変更で壊れたのか/元の挙動を変えたのか」を切り分けられず、レビューも不能になります。

根本原因は、「正しい仕様」と「現在のふるまい」を混同すること。リファクタリングの定義は「外部から見たふるまいを変えずに内部構造を改善すること」であり、移植中に守る基準は “正しさ” ではなく “今の出力と同じか” でしかありません。

ステップ1:characterizationテストでふるまいを固定する

characterizationテスト(仕様化テスト)は、「正しい期待値」ではなく 「今まさに返ってくる値」 をそのまま期待値にするテストです。目的は正しさの保証ではなく、変更検知の網を張ること。

public function test_search_endpoint_keeps_current_behavior(): void
{
    $response = $this->postJson('/legacy/search', $this->fixture('case_a'));

    // 「正しい仕様」ではなく、今の実装の出力をそのまま貼る(バグ込みで固定する)。
    $response->assertStatus(200);
    $response->assertExactJson($this->snapshot('search_case_a'));
}

スナップショットは初回実行で生成し、以降は差分が出たら落とす。これで「移植してもふるまいが1バイトも変わっていない」ことを機械的に証明できます。移植より前に、対象エンドポイントぶん全て書き切るのが肝です。

ステップ2:層分離は「原典忠実」で行う

コントローラの処理を UseCase / Repository に移すとき、ロジックを 1行も改善しません。整形順も無駄な分岐もそのまま運びます。

移植前:

class StockController
{
    public function search(Request $request)
    {
        $builder = DB::table('items')->where('status', $request->status);
        // 条件分岐・結合・整形が延々と続く
        $rows = $builder->get();
        return response()->json($this->format($rows));
    }
}

移植後(構造だけ変え、挙動は等価):

class StockController
{
    public function search(SearchRequest $request, SearchItemsUseCase $useCase)
    {
        return response()->json($useCase->handle($request->toCriteria()));
    }
}

final class SearchItemsUseCase
{
    public function __construct(private ItemRepositoryInterface $items) {}

    public function handle(SearchCriteria $criteria): array
    {
        // 元のクエリ・整形をそのまま移す。「無駄だな」と思っても今は触らない。
        return $this->items->search($criteria);
    }
}

characterizationテストが緑のままなら成功。赤くなったら、良かれと思って何かを「直して」しまっています。

ステップ3:潜在バグは「直さず検出して起票」する

移植中に出会う典型的な地雷と検出パターンです。

(A) 成功を返すが保存しない書き込みEP

public function update(Request $request)
{
    $model = $this->repo->find($request->id);
    $model->fill($request->validated());
    // ← save() が無い。でも成功レスポンスは返る。
    return response()->json(['result' => 'ok']);
}

検出パターン:書き込み系エンドポイントを総ざらいし、「成功を返すのに永続化呼び出しが経路上に無いもの」を全数洗い出す。見つけても その場で save() を足さない(挙動が変わる=別案件)。

(B) フロントから呼ばれていない死んだ機能

検出パターン:エンドポイント一覧とフロントの呼び出し箇所を突き合わせ、参照ゼロを列挙する。移植対象から外す判断材料にするが、削除自体は別タスクにする。

(C) 「発明実装」の削除

移植者が気を利かせて元コードに無い処理(署名付与など)を足してしまうことがあります。原典忠実主義の違反なので、移植で追加した分を削って原典に戻す。足すのではなく合わせる。

いずれも共通する原則は 「見つけること」と「直すこと」を分離する こと。検出はレポート+起票にし、修正は別PR・別承認で行います。

実務の段取り:バッチ分割と着手前実測

大きなコントローラを一気に移すとPRが巨大化しレビューが破綻します。エンドポイントの性質で分割しましょう。

  • 参照系(読むだけ)→ まとめて移しやすい
  • 書き込み系 → 1つずつ慎重に
  • フィルタ・集計系 → 共通化の起点

そして重要なのが、スコープは着手前に実測して確定すること。計画段階で量を見積もっても、実際に開くと前提がずれていることが頻繁にあります(想定より多い/逆に半分は既に別経路へ吸収済み、など)。前提が崩れたら見積もりに固執せず、縮小して理由を記録します。見積もりは作業前の仮説でしかありません。

マージ基準(確認方法)

各移植PRのマージ基準を以下で固定すると安定します。

  1. 対象エンドポイントの characterizationテストが移植前に全て存在する
  2. 移植後、それらが1件も差分なく緑
  3. 移植PRの差分に、ふるまいを変える変更(save追加・分岐修正)が混ざっていない
  4. 移植中に見つけた潜在バグは、コードではなく issue に落ちている
# 移植前にスナップショットを確定
php artisan test --filter=Characterization --update-snapshots

# 移植後は差分ゼロのみ確認(更新はしない)
php artisan test --filter=Characterization

注意点

  • characterizationテストは「正しさ」を保証しません。バグも固定します。仕様理解が追いつく前に安全に構造を変えるための足場です。
  • スナップショットを安易に更新しないこと。差分が出たら、まず「自分が挙動を変えたのでは」を疑う。
  • 「ついで修正」の誘惑が最も強いのは、移植が順調なときです。順調なときほど分離を徹底する。

まとめ

レガシーのクリーンアーキ化で効いたのは、技術というより 規律 でした。

  • ふるまいは characterizationテストで固定してから動かす
  • 移植は原典忠実。構造変更と仕様変更を同じPRに混ぜない
  • 見つけた潜在バグは直さず、検出して起票する
  • スコープは着手前に実測し、崩れたら縮小して記録する

「良いコードにしたい」気持ちを一旦止め、「今と同じふるまいのまま構造だけ変える」に集中する。正しさの回復はその後で十分です。この順序を守れるかどうかが、レガシー移植が終わるかどうかを分けます。

\ 最新情報をチェック /

コメントを残す

このサイトはスパムを低減するために Akismet を使っています。コメントデータの処理方法の詳細はこちらをご覧ください。