Go to Qiita Advent Calendar Top
LoginSignup
0
0

Delete article

Deleted articles cannot be recovered.

Draft of this article would be also deleted.

Are you sure you want to delete this article?

@sho_fcafein株式会社エフカフェ

【どう実装する？シリーズ #7】バリデーションで何度もthrow、フローが追いづらくない？

Posted at

前回の記事では、更新と取得を分離しました。メソッドの責務が明確になり、トランザクションも軽くなりました。

今回は、まだ残っている問題の1つ「早期throwによる複雑なフロー」に取り組みます。

問題の再確認

現在のLP保存処理では、事前チェックで何度も例外を投げています。

class LandingPageService
{
    public function update(int $lpId, array $data, int $userId): int
    {
        // チェック1: LP作成上限
        $lp = Lp::find($lpId);
        if (!$lp && !$this->canCreateLP($userId)) {
            throw new LpLimitException('LP作成上限に達しています');
        }

        // チェック2: ABテスト関連
        if ($lp && $lp->hasActiveABTest()) {
            if ($this->hasNoVisibleSteps($data['steps'])) {
                throw new ValidationException('表示ステップが必要です');
            }
            if ($data['status'] === 'inactive') {
                throw new ValidationException('公開中のABテストがあります');
            }
        }

        // チェック3: 公開時のメディア存在確認
        if ($data['status'] === 'active') {
            $mediaIds = $this->extractMediaIds($data);
            if (!$this->allMediaExists($mediaIds)) {
                throw new MediaNotFoundException('メディアが見つかりません');
            }
        }

        DB::beginTransaction();
        try {
            // 更新処理
            // ...
        }
    }
}

エラーが見つかった時点で、すぐに例外を投げています。

何が問題か

1. フローが追いづらい

// どこでthrowされるか分からない
public function update(int $lpId, array $data, int $userId): int
{
    // ← ここでthrowされるかも
    $lp = Lp::find($lpId);
    if (!$lp && !$this->canCreateLP($userId)) {
        throw new LpLimitException('...');
    }
    
    // ← ここでthrowされるかも
    if ($lp && $lp->hasActiveABTest()) {
        if ($this->hasNoVisibleSteps($data['steps'])) {
            throw new ValidationException('...');
        }
        // ← ここでもthrowされるかも
        if ($data['status'] === 'inactive') {
            throw new ValidationException('...');
        }
    }
    
    // ← ここでもthrowされるかも
    if ($data['status'] === 'active') {
        // ...
    }
    
    // やっと本処理
    DB::beginTransaction();
}

メソッドの至る所でthrowされる可能性があり、正常フローを追うのが大変です。

2. エラーメッセージが1つしか返らない

// ユーザーの入力
{
  "name": "",           // 空（エラー）
  "status": "inactive", // ABテスト公開中なのにinactive（エラー）
  "steps": []           // ステップなし（エラー）
}

このデータで保存しようとすると：

POST /api/lp/update
→ 400 Bad Request
{
  "error": "表示ステップが必要です"
}

最初のエラーだけが返り、他のエラーは分かりません。

ユーザーは：

  1. 「表示ステップが必要です」を見て修正
  2. 再度送信
  3. 「公開中のABテストがあります」エラー
  4. また修正
  5. 再度送信
  6. ...

何度もやり直しが必要です。

3. ネストが深くなりがち

if ($lp && $lp->hasActiveABTest()) {
    if ($this->hasNoVisibleSteps($data['steps'])) {
        throw new ValidationException('...');
    }
    if ($data['status'] === 'inactive') {
        throw new ValidationException('...');
    }
}

条件が複雑になると、ネストがどんどん深くなります。

4. テストケースが組み合わせ爆発

// チェック1でエラー
public function test_LP作成上限でエラー() { }

// チェック1はOK、チェック2でエラー
public function test_表示ステップなしでエラー() { }

// チェック1,2はOK、チェック3でエラー
public function test_メディアなしでエラー() { }

// 全部OK
public function test_正常に保存できる() { }

チェックの数が増えるほど、テストケースが爆発的に増えます。

改善案: バリデーションクラスに集約

すべてのチェックを実行し、エラーを配列で返すようにします。

LpUpdateValidator の作成

class LpUpdateValidator
{
    private array $errors = [];

    public function validate(int $lpId, array $data, int $userId): bool
    {
        $lp = Lp::find($lpId);

        // すべてのチェックを実行
        $this->checkLpLimit($lp, $userId);
        $this->checkActiveABTest($lp, $data);
        $this->checkMediaExists($data);

        // エラーがなければtrue
        return empty($this->errors);
    }

    public function errors(): array
    {
        return $this->errors;
    }

    private function checkLpLimit(?Lp $lp, int $userId): void
    {
        if (!$lp && !$this->canCreateLP($userId)) {
            $this->errors['lp_limit'] = 'LP作成上限に達しています';
        }
    }

    private function checkActiveABTest(?Lp $lp, array $data): void
    {
        if (!$lp || !$lp->hasActiveABTest()) {
            return;
        }

        if ($this->hasNoVisibleSteps($data['steps'])) {
            $this->errors['visible_steps'] = '表示ステップが必要です';
        }

        if ($data['status'] === 'inactive') {
            $this->errors['abtest_status'] = '公開中のABテストがあります';
        }
    }

    private function checkMediaExists(array $data): void
    {
        if ($data['status'] !== 'active') {
            return;
        }

        $mediaIds = $this->extractMediaIds($data);
        if (!$this->allMediaExists($mediaIds)) {
            $this->errors['media'] = 'メディアが見つかりません';
        }
    }

    private function canCreateLP(int $userId): bool
    {
        // LP作成可能数のチェックロジック
    }

    private function hasNoVisibleSteps(array $steps): bool
    {
        // 表示ステップの存在チェックロジック
    }

    private function extractMediaIds(array $data): array
    {
        // メディアID抽出ロジック
    }

    private function allMediaExists(array $mediaIds): bool
    {
        // メディア存在確認ロジック
    }
}

ポイント:

  • すべてのチェックを実行する（早期returnしない）
  • エラーは配列に蓄積
  • validate()がfalseなら、errors()でエラー一覧を取得

Service（シンプルに）

class LandingPageService
{
    public function __construct(
        private LpUpdateValidator $validator
    ) {}

    public function update(int $lpId, array $data, int $userId): int
    {
        // バリデーション実行
        if (!$this->validator->validate($lpId, $data, $userId)) {
            throw new ValidationException($this->validator->errors());
        }

        // バリデーションを通過したら、更新処理
        DB::beginTransaction();
        try {
            $lp = Lp::updateOrCreate(['id' => $lpId], [
                'name' => $data['name'],
                'status' => $data['status'],
                'template' => $data['template'],
            ]);

            // ... 更新処理 ...

            DB::commit();
            return $lp->id;
            
        } catch (\Throwable $e) {
            DB::rollBack();
            throw $e;
        }
    }
}

Beforeとの違い:

  • 事前チェックのコードがすべて消えた
  • Validatorに委譲
  • フローが追いやすい（バリデーション → 更新）

Controller

class LandingPageController extends Controller
{
    public function update(Request $request)
    {
        try {
            $lpId = $this->service->update(
                $request->input('id'),
                $request->all(),
                Auth::id()
            );

            return $this->success(['lp_id' => $lpId]);
            
        } catch (ValidationException $e) {
            // すべてのエラーを返す
            return response()->json([
                'errors' => $e->errors()
            ], 400);
        }
    }
}

レスポンス例

POST /api/lp/update
→ 400 Bad Request
{
  "errors": {
    "visible_steps": "表示ステップが必要です",
    "abtest_status": "公開中のABテストがあります",
    "media": "メディアが見つかりません"
  }
}

すべてのエラーが一度に返ります！

改善によるメリット

1. フローが追いやすくなった

public function update(int $lpId, array $data, int $userId): int
{
    // バリデーション
    if (!$this->validator->validate($lpId, $data, $userId)) {
        throw new ValidationException($this->validator->errors());
    }

    // 更新処理
    DB::beginTransaction();
    // ...
}

処理の流れが明確です：

  1. バリデーション
  2. 更新

2. すべてのエラーを一度に返せる

ユーザーは一度の修正で、すべての問題を解決できます。

3. バリデーションロジックが独立した

// Validatorは単体でテストできる
$validator = new LpUpdateValidator();
$isValid = $validator->validate($lpId, $data, $userId);

$this->assertFalse($isValid);
$this->assertArrayHasKey('lp_limit', $validator->errors());

Serviceとは独立してテストできます。

4. ネストが浅くなった

// Before: ネストが深い
if ($lp && $lp->hasActiveABTest()) {
    if ($this->hasNoVisibleSteps($data['steps'])) {
        throw new ValidationException('...');
    }
    if ($data['status'] === 'inactive') {
        throw new ValidationException('...');
    }
}

// After: フラット
private function checkActiveABTest(?Lp $lp, array $data): void
{
    if (!$lp || !$lp->hasActiveABTest()) {
        return; // 早期return
    }

    if ($this->hasNoVisibleSteps($data['steps'])) {
        $this->errors['visible_steps'] = '表示ステップが必要です';
    }

    if ($data['status'] === 'inactive') {
        $this->errors['abtest_status'] = '公開中のABテストがあります';
    }
}

各チェックメソッドで早期returnを使えるため、ネストが浅くなります。

5. テストがシンプルになった

// Validatorのテスト
public function test_LP作成上限でエラー()
{
    $validator = new LpUpdateValidator();
    $isValid = $validator->validate(null, $data, $userId);
    
    $this->assertFalse($isValid);
    $this->assertArrayHasKey('lp_limit', $validator->errors());
}

public function test_複数エラー()
{
    $validator = new LpUpdateValidator();
    $isValid = $validator->validate($lpId, $invalidData, $userId);
    
    $this->assertFalse($isValid);
    $this->assertCount(3, $validator->errors()); // 3つのエラー
}

// Serviceのテスト（Validatorをモック）
public function test_バリデーションエラーで例外()
{
    $mockValidator = Mockery::mock(LpUpdateValidator::class);
    $mockValidator->shouldReceive('validate')->andReturn(false);
    $mockValidator->shouldReceive('errors')->andReturn(['error' => 'message']);
    
    $service = new LandingPageService($mockValidator);
    
    $this->expectException(ValidationException::class);
    $service->update($lpId, $data, $userId);
}

ValidatorとServiceを分けてテストできます。

LaravelのForm Requestとの使い分け

Laravelには、すでにFormRequestというバリデーション機構があります。

class LpUpdateRequest extends FormRequest
{
    public function rules()
    {
        return [
            'name' => 'required|string|max:255',
            'status' => 'required|in:active,inactive',
            'steps' => 'required|array',
        ];
    }
}

FormRequestが向いているケース

  • リクエストの形式チェック（型、必須、長さなど）
  • 静的なルールで表現できる

カスタムValidatorが向いているケース

  • DBの状態に依存するチェック（LP作成上限、ABテスト状態など）
  • 複雑な条件判定
  • ビジネスルール

組み合わせる

// FormRequest: 基本的な形式チェック
class LpUpdateRequest extends FormRequest
{
    public function rules()
    {
        return [
            'name' => 'required|string|max:255',
            'status' => 'required|in:active,inactive',
            'steps' => 'required|array',
        ];
    }
}

// Validator: ビジネスルール
class LpUpdateValidator
{
    public function validate(int $lpId, array $data, int $userId): bool
    {
        // LP作成上限、ABテスト状態、メディア存在など
    }
}

// Controller
public function update(LpUpdateRequest $request)
{
    // FormRequestで基本チェック済み
    
    // Validatorでビジネスルールチェック
    if (!$this->validator->validate($request->input('id'), $request->all(), Auth::id())) {
        return response()->json(['errors' => $this->validator->errors()], 400);
    }
    
    // 更新処理
    $lpId = $this->service->update($request->input('id'), $request->all(), Auth::id());
}

役割分担が明確です。

契約による設計

この使い分けは、**契約による設計(Design by Contract)**の考え方に基づいています。

class LandingPageService
{
    /**
     * LP更新
     * 
     * 事前条件(呼び出し側が保証すること):
     * - $dataは基本的な形式チェック済み
     *   - nameは必須で255文字以内の文字列
     *   - statusは'active'か'inactive'
     *   - stepsは配列
     * 
     * 事後条件(このメソッドが保証すること):
     * - ビジネスルールを満たすデータのみ保存される
     * - 保存に成功したらLP IDが返る
     */
    public function update(int $lpId, array $data, int $userId): int
    {
        // 事前条件は満たされている前提で動く
        // （FormRequestで既にチェック済み）
        
        // このメソッドの責務：ビジネスルールのチェック
        if (!$this->validator->validate($lpId, $data, $userId)) {
            throw new ValidationException($this->validator->errors());
        }
        
        // 更新処理
        // ...
    }
}

契約の内容:

層 責務 検証内容
FormRequest 基本的な形式保証 型、必須、長さ、フォーマット
Service ビジネスルール保証 DB状態、ビジネス制約

メリット:

  • 各層の責務が明確
  • Serviceは「形式は正しい」という前提で動ける
  • 二重チェックを避けられる

注意点:

FormRequestを経由しない呼び出し（Job、コマンドなど）では、事前条件が満たされない可能性があります。

// Job から直接 Service を呼ぶ場合
class LpImportJob implements ShouldQueue
{
    public function handle(LandingPageService $service)
    {
        foreach ($this->importData as $data) {
            // FormRequestを経由しない
            // → 基本的な形式チェックがスキップされる
            $service->update(null, $data, $this->userId);
        }
    }
}

この場合の対策：

対策1: Jobでもバリデーションする

class LpImportJob implements ShouldQueue
{
    public function handle(LandingPageService $service)
    {
        foreach ($this->importData as $data) {
            // 基本的な形式チェック
            $validator = Validator::make($data, [
                'name' => 'required|string|max:255',
                'status' => 'required|in:active,inactive',
            ]);
            
            if ($validator->fails()) {
                Log::error('Import validation failed', $validator->errors());
                continue;
            }
            
            $service->update(null, $data, $this->userId);
        }
    }
}

対策2: Serviceで基本チェックも行う（防御的プログラミング）

class LandingPageService
{
    public function update(int $lpId, array $data, int $userId): int
    {
        // 念のため基本的な形式もチェック
        $this->validateBasicFormat($data);
        
        // ビジネスルールチェック
        if (!$this->validator->validate($lpId, $data, $userId)) {
            throw new ValidationException($this->validator->errors());
        }
        
        // 更新処理
    }
    
    private function validateBasicFormat(array $data): void
    {
        if (empty($data['name']) || strlen($data['name']) > 255) {
            throw new InvalidArgumentException('Invalid name format');
        }
        // ...
    }
}

どちらを選ぶかは、システムの特性次第です：

  • Controller経由が主なら、対策1（呼び出し側で保証）
  • 様々な経路から呼ばれるなら、対策2（Service側で防御）

契約による設計では、「誰が何を保証するか」を明確にすることが重要です。

まとめ

早期throwによる複雑なフローには、以下の問題がありました：

問題点

  • フローが追いづらい
  • エラーメッセージが1つしか返らない
  • ネストが深くなりがち
  • テストケースが組み合わせ爆発

バリデーションクラスに集約することで：

メリット

  • フローが追いやすくなった
  • すべてのエラーを一度に返せる
  • バリデーションロジックが独立した
  • ネストが浅くなった
  • テストがシンプルになった

さらに：

  • LaravelのFormRequestと役割分担できる
  • Result型/Either型という選択肢もある

バリデーションを「チェックに失敗したらすぐthrow」ではなく、「すべてチェックしてから結果を返す」設計にすることで、ユーザー体験もコードの保守性も向上します。

次回の記事では、まだ残っている問題「暗黙的な副作用」に取り組みます。削除対象の自動検出など、コードを読まないと分からない挙動を、明示的にする方法を考えます。

0
0
0

Register as a new user and use Qiita more conveniently

  1. You get articles that match your needs
  2. You can efficiently read back useful information
  3. You can use dark theme
What you can do with signing up
Sign upLogin
0
0

Delete article

Deleted articles cannot be recovered.

Draft of this article would be also deleted.

Are you sure you want to delete this article?