diff --git a/app/Exceptions/BookingRuleViolationException.php b/app/Exceptions/BookingRuleViolationException.php new file mode 100644 index 0000000..153f8c6 --- /dev/null +++ b/app/Exceptions/BookingRuleViolationException.php @@ -0,0 +1,5 @@ +find($schedule->id); if ($data['participants'] > $schedule->remainingSpots()) { - throw new \RuntimeException('名額不足，請重新選擇'); + throw new BookingRuleViolationException('名額不足，請重新選擇'); } // 重複預約檢查 @@ -77,7 +78,7 @@ class MemberBookingController extends Controller ->whereIn('status', [BookingStatus::Pending->value, BookingStatus::Confirmed->value]) ->exists(); if ($duplicate) { - throw new \RuntimeException('您已預約此時段'); + throw new BookingRuleViolationException('您已預約此時段'); } return Booking::create([ @@ -89,7 +90,7 @@ class MemberBookingController extends Controller 'notes' => $data['notes'] ?? null, ]); }); - } catch (\RuntimeException $e) { + } catch (BookingRuleViolationException $e) { return response()->json(['status' => false, 'message' => $e->getMessage()], 422); } diff --git a/app/Http/Controllers/API/ProviderBookingController.php b/app/Http/Controllers/API/ProviderBookingController.php index ecd52ab..4384204 100644 --- a/app/Http/Controllers/API/ProviderBookingController.php +++ b/app/Http/Controllers/API/ProviderBookingController.php @@ -4,6 +4,7 @@ namespace App\Http\Controllers\API; use App\Enums\BookingStatus; use App\Enums\ScheduleStatus; +use App\Exceptions\BookingRuleViolationException; use App\Http\Controllers\Controller; use App\Models\Booking; use App\Notifications\BookingCancelledNotification; @@ -43,7 +44,7 @@ class ProviderBookingController extends Controller $remaining = $schedule->max_participants - $schedule->current_participants; if ($booking->participants > $remaining) { - throw new \RuntimeException('名額不足，無法確認此預約'); + throw new BookingRuleViolationException('名額不足，無法確認此預約'); } $booking->update(['status' => BookingStatus::Confirmed]); @@ -54,7 +55,7 @@ class ProviderBookingController extends Controller $schedule->update(['status' => ScheduleStatus::Full]); } }); - } catch (\RuntimeException $e) { + } catch (BookingRuleViolationException $e) { return response()->json(['status' => false, 'message' => $e->getMessage()], 422); } diff --git a/openspec/changes/fix-booking-exception-handling/tasks.md b/openspec/changes/fix-booking-exception-handling/tasks.md index dbf841c..0f8555d 100644 --- a/openspec/changes/fix-booking-exception-handling/tasks.md +++ b/openspec/changes/fix-booking-exception-handling/tasks.md @@ -4,21 +4,21 @@ ## 1. 業務例外與 Controller -- [ ] 1.1 [後端] 新增 `app/Exceptions/BookingRuleViolationException.php`（`final class`，繼承 `\RuntimeException`，不加額外方法）；驗證：2.x 測試通過 -- [ ] 1.2 [後端] `app/Http/Controllers/API/MemberBookingController.php` 的 `store`：交易閉包內兩處 `throw new \RuntimeException(...)` 改為 `throw new BookingRuleViolationException(...)`，訊息文字不變；`catch (\RuntimeException $e)` 改為 `catch (BookingRuleViolationException $e)` -- [ ] 1.3 [後端] `app/Http/Controllers/API/ProviderBookingController.php` 的 `confirm`：同 1.2，處理 `名額不足，無法確認此預約` -- [ ] 1.4 [後端] 確認 `grep -rn "RuntimeException" app/` 只剩 `BookingRuleViolationException` 本身的繼承宣告 +- [x] 1.1 [後端] 新增 `app/Exceptions/BookingRuleViolationException.php`（`final class`，繼承 `\RuntimeException`，不加額外方法）；驗證：2.x 測試通過 +- [x] 1.2 [後端] `app/Http/Controllers/API/MemberBookingController.php` 的 `store`：交易閉包內兩處 `throw new \RuntimeException(...)` 改為 `throw new BookingRuleViolationException(...)`，訊息文字不變；`catch (\RuntimeException $e)` 改為 `catch (BookingRuleViolationException $e)` +- [x] 1.3 [後端] `app/Http/Controllers/API/ProviderBookingController.php` 的 `confirm`：同 1.2，處理 `名額不足，無法確認此預約` +- [x] 1.4 [後端] 確認 `grep -rn "RuntimeException" app/` 只剩 `BookingRuleViolationException` 本身的繼承宣告 ## 2. 測試 -- [ ] 2.1 [整合測試] `test_duplicate_booking_returns_422_with_rule_message`：member 已有該時段 `pending` 預約，再次 `POST /api/member/bookings`，回 422，`assertExactJson(['status' => false, 'message' => '您已預約此時段'])`，該 member 在此時段的預約仍為 1 筆；測試開頭 `Exceptions::fake()`，最後 `Exceptions::assertNotReported(BookingRuleViolationException::class)`（對應 Scenario「重複預約仍回 422 與既定訊息」） -- [ ] 2.2 [整合測試] `test_capacity_lost_inside_transaction_returns_422_with_rule_message`：以 `CourseSchedule::retrieved` 事件在交易內把 `current_participants` 設為滿額（事件只能改動交易內 `lockForUpdate()->find()` 那次讀取；交易前的第一次讀取若被改動，會被前置檢查以「人數超過剩餘名額」攔下，測不到交易內路徑），`POST /api/member/bookings` 回 422，`assertExactJson(['status' => false, 'message' => '名額不足，請重新選擇'])`，`bookings` 筆數為 0；同 2.1 以 `Exceptions::fake()` 斷言未回報（對應 Scenario「交易內名額不足仍回 422 與既定訊息」） -- [ ] 2.3 [整合測試] `test_confirm_without_capacity_returns_422_with_rule_message`：provider 確認人數大於剩餘名額的 `pending` 預約，回 422，`assertExactJson(['status' => false, 'message' => '名額不足，無法確認此預約'])`，預約仍為 `pending`、`current_participants` 不變；同 2.1 以 `Exceptions::fake()` 斷言未回報（對應 Scenario「確認預約時名額不足仍回 422 與既定訊息」） -- [ ] 2.4 [整合測試] `test_database_error_on_create_returns_500_without_sql`：`config(['app.debug' => false])`、`Exceptions::fake()`，`Booking::created`（INSERT 已執行之後）拋出 SQL 含 `insert into "bookings"` 與欄位名稱的 `QueryException`；合法 `POST /api/member/bookings` 回 500，`assertExactJson(['message' => 'Server Error'])`，body 不含 `insert into`、`bookings`、`member_id`，`Exceptions::assertReported(QueryException::class)`，`bookings` 筆數為 0（INSERT 已發生，筆數為 0 才能證明 rollback）（對應 Scenario「建立預約時資料庫錯誤回 500 且不洩漏細節」） -- [ ] 2.5 [整合測試] `test_database_error_on_confirm_returns_500_without_sql`：同 2.4 的設定，`CourseSchedule::updating`（在交易內預約狀態已更新、`increment` 執行時觸發）拋出 SQL 含 `update "course_schedules"` 的 `QueryException`；provider 確認名額足夠的 `pending` 預約，回 500，`assertExactJson(['message' => 'Server Error'])`，body 不含 `update`、`course_schedules`、`current_participants`，`Exceptions::assertReported(QueryException::class)`，預約仍為 `pending`（狀態更新已執行，仍為 `pending` 才能證明 rollback）、`current_participants` 不變（對應 Scenario「確認預約時資料庫錯誤回 500 且不洩漏細節」） -- [ ] 2.6 [整合測試] 執行 `php artisan test --filter='BookingLifecycleTest|BookingOversellTest'`，確認既有 422 行為不變；完整 `php artisan test` 需在有 GD 的環境執行（見 `cfdive-test-gd` image 的做法） +- [x] 2.1 [整合測試] `test_duplicate_booking_returns_422_with_rule_message`：member 已有該時段 `pending` 預約，再次 `POST /api/member/bookings`，回 422，`assertExactJson(['status' => false, 'message' => '您已預約此時段'])`，該 member 在此時段的預約仍為 1 筆；測試開頭 `Exceptions::fake()`，最後 `Exceptions::assertNotReported(BookingRuleViolationException::class)`（對應 Scenario「重複預約仍回 422 與既定訊息」） +- [x] 2.2 [整合測試] `test_capacity_lost_inside_transaction_returns_422_with_rule_message`：以 `CourseSchedule::retrieved` 事件在交易內把 `current_participants` 設為滿額（事件只能改動交易內 `lockForUpdate()->find()` 那次讀取；交易前的第一次讀取若被改動，會被前置檢查以「人數超過剩餘名額」攔下，測不到交易內路徑），`POST /api/member/bookings` 回 422，`assertExactJson(['status' => false, 'message' => '名額不足，請重新選擇'])`，`bookings` 筆數為 0；同 2.1 以 `Exceptions::fake()` 斷言未回報（對應 Scenario「交易內名額不足仍回 422 與既定訊息」） +- [x] 2.3 [整合測試] `test_confirm_without_capacity_returns_422_with_rule_message`：provider 確認人數大於剩餘名額的 `pending` 預約，回 422，`assertExactJson(['status' => false, 'message' => '名額不足，無法確認此預約'])`，預約仍為 `pending`、`current_participants` 不變；同 2.1 以 `Exceptions::fake()` 斷言未回報（對應 Scenario「確認預約時名額不足仍回 422 與既定訊息」） +- [x] 2.4 [整合測試] `test_database_error_on_create_returns_500_without_sql`：`config(['app.debug' => false])`、`Exceptions::fake()`，`Booking::created`（INSERT 已執行之後）拋出 SQL 含 `insert into "bookings"` 與欄位名稱的 `QueryException`；合法 `POST /api/member/bookings` 回 500，`assertExactJson(['message' => 'Server Error'])`，body 不含 `insert into`、`bookings`、`member_id`，`Exceptions::assertReported(QueryException::class)`，`bookings` 筆數為 0（INSERT 已發生，筆數為 0 才能證明 rollback）（對應 Scenario「建立預約時資料庫錯誤回 500 且不洩漏細節」） +- [x] 2.5 [整合測試] `test_database_error_on_confirm_returns_500_without_sql`：同 2.4 的設定，`CourseSchedule::updating`（在交易內預約狀態已更新、`increment` 執行時觸發）拋出 SQL 含 `update "course_schedules"` 的 `QueryException`；provider 確認名額足夠的 `pending` 預約，回 500，`assertExactJson(['message' => 'Server Error'])`，body 不含 `update`、`course_schedules`、`current_participants`，`Exceptions::assertReported(QueryException::class)`，預約仍為 `pending`（狀態更新已執行，仍為 `pending` 才能證明 rollback）、`current_participants` 不變（對應 Scenario「確認預約時資料庫錯誤回 500 且不洩漏細節」） +- [x] 2.6 [整合測試] 執行 `php artisan test --filter='BookingLifecycleTest|BookingOversellTest'`，確認既有 422 行為不變；完整 `php artisan test` 需在有 GD 的環境執行（見 `cfdive-test-gd` image 的做法） ## 3. 手動驗證 -- [ ] 3.1 [手動驗證] 變異驗證（只在本機執行，不提交）：暫時把 1.2、1.3 的 catch 改回 `\RuntimeException`，確認 2.4、2.5 失敗後再還原，證明測試抓得到原本的問題 +- [x] 3.1 [手動驗證] 變異驗證（只在本機執行，不提交）：暫時把 1.2、1.3 的 catch 改回 `\RuntimeException`，確認 2.4、2.5 失敗後再還原，證明測試抓得到原本的問題 - [ ] 3.2 [手動驗證] 部署後在正式環境建立一筆預約、確認一筆預約，確認業務流程正常（201／200）；Sentry 是否收到預約端點的 500 只能等實際錯誤發生，若期間沒有錯誤則記錄為「未觀察到」，不視為失敗（自動化驗證以 2.4、2.5 為準） diff --git a/tests/Feature/BookingExceptionHandlingTest.php b/tests/Feature/BookingExceptionHandlingTest.php new file mode 100644 index 0000000..99a99e1 --- /dev/null +++ b/tests/Feature/BookingExceptionHandlingTest.php @@ -0,0 +1,201 @@ +create(['role' => 'provider']); + + ProviderProfile::create([ + 'user_id' => $provider->id, + 'verification_status' => 'approved', + ]); + + $offer = DivingOffer::create([ + 'provider_id' => $provider->id, + 'title' => 'Exception Course', + 'location' => 'Test', + 'spot' => 'Test Spot', + 'price' => 1000, + 'region' => '南部', + 'rating' => 0, + 'reviews' => 0, + ]); + + $schedule = CourseSchedule::create([ + 'diving_offer_id' => $offer->id, + 'provider_id' => $provider->id, + 'scheduled_date' => now()->addDays(7)->toDateString(), + 'start_time' => '09:00', + 'max_participants' => $maxParticipants, + 'current_participants' => $currentParticipants, + 'status' => ScheduleStatus::Open, + ]); + + return [$provider, $schedule]; + } + + private function makePendingBooking(CourseSchedule $schedule, int $participants, ?User $member = null): Booking + { + return Booking::create([ + 'schedule_id' => $schedule->id, + 'member_id' => ($member ?? User::factory()->create(['role' => 'member']))->id, + 'participants' => $participants, + 'total_price' => 1000 * $participants, + 'status' => BookingStatus::Pending, + ]); + } + + // 刻意不用 "database is locked" 這類併發錯誤訊息：巢狀交易（RefreshDatabase）中 Laravel 會改拋 DeadlockException + private function makeQueryException(string $sql): QueryException + { + return new QueryException('sqlite', $sql, [], new \PDOException('SQLSTATE[23000]: Integrity constraint violation')); + } + + /** + * 業務失敗的訊息是前端直接顯示給使用者的文字，改型別不能改到它； + * 也不應回報到 Sentry，否則正常的業務拒絕會淹沒真正的伺服器故障。 + */ + public function test_duplicate_booking_returns_422_with_rule_message(): void + { + [, $schedule] = $this->makeProviderWithSchedule(5); + $member = User::factory()->create(['role' => 'member']); + $this->makePendingBooking($schedule, 1, $member); + + $this->actingAs($member) + ->postJson('/api/member/bookings', ['schedule_id' => $schedule->id, 'participants' => 1]) + ->assertStatus(422) + ->assertExactJson(['status' => false, 'message' => '您已預約此時段']); + + $this->assertSame(1, Booking::where('member_id', $member->id)->where('schedule_id', $schedule->id)->count()); + Exceptions::assertNotReported(BookingRuleViolationException::class); + } + + /** + * 交易內名額不足只在併發時發生：交易前的檢查通過後，另一筆預約搶先佔位。 + * 只改動第二次讀取（交易內 lockForUpdate），第一次讀取若被改動會被前置檢查攔下，測不到這條路徑。 + */ + public function test_capacity_lost_inside_transaction_returns_422_with_rule_message(): void + { + [, $schedule] = $this->makeProviderWithSchedule(2); + $member = User::factory()->create(['role' => 'member']); + + $retrievalCount = 0; + CourseSchedule::retrieved(function (CourseSchedule $retrieved) use (&$retrievalCount) { + $retrievalCount++; + if ($retrievalCount === 2) { + $retrieved->current_participants = $retrieved->max_participants; + } + }); + + $this->actingAs($member) + ->postJson('/api/member/bookings', ['schedule_id' => $schedule->id, 'participants' => 1]) + ->assertStatus(422) + ->assertExactJson(['status' => false, 'message' => '名額不足，請重新選擇']); + + $this->assertSame(0, Booking::count()); + Exceptions::assertNotReported(BookingRuleViolationException::class); + } + + /** + * 確認時的名額檢查是防超賣的最後一道防線，其 422 訊息與「不留下部分寫入」都必須維持。 + */ + public function test_confirm_without_capacity_returns_422_with_rule_message(): void + { + [$provider, $schedule] = $this->makeProviderWithSchedule(2, 1); + $booking = $this->makePendingBooking($schedule, 2); + + $this->actingAs($provider) + ->putJson("/api/provider/bookings/{$booking->id}/confirm") + ->assertStatus(422) + ->assertExactJson(['status' => false, 'message' => '名額不足，無法確認此預約']); + + $this->assertSame(BookingStatus::Pending, $booking->fresh()->status); + $this->assertSame(1, $schedule->fresh()->current_participants); + Exceptions::assertNotReported(BookingRuleViolationException::class); + } + + /** + * 資料庫錯誤曾以 422 把 SQL 回給使用者且不會送到 Sentry。 + * 例外在 INSERT 之後才拋出，筆數為 0 才能證明交易確實 rollback。 + */ + public function test_database_error_on_create_returns_500_without_sql(): void + { + config(['app.debug' => false]); + [, $schedule] = $this->makeProviderWithSchedule(5); + $member = User::factory()->create(['role' => 'member']); + + Booking::created(function () { + throw $this->makeQueryException('insert into "bookings" ("schedule_id", "member_id", "participants") values (?, ?, ?)'); + }); + + $this->actingAs($member) + ->postJson('/api/member/bookings', ['schedule_id' => $schedule->id, 'participants' => 1]) + ->assertStatus(500) + ->assertExactJson(['message' => 'Server Error']) + ->assertDontSee('insert into', false) + ->assertDontSee('bookings', false) + ->assertDontSee('member_id', false); + + Exceptions::assertReported(QueryException::class); + $this->assertSame(0, Booking::count()); + } + + /** + * 同上，針對確認預約。例外在預約狀態更新之後才拋出，預約仍為 pending 才能證明 rollback。 + */ + public function test_database_error_on_confirm_returns_500_without_sql(): void + { + config(['app.debug' => false]); + [$provider, $schedule] = $this->makeProviderWithSchedule(5); + $booking = $this->makePendingBooking($schedule, 2); + + CourseSchedule::updating(function () { + throw $this->makeQueryException('update "course_schedules" set "current_participants" = "current_participants" + 2 where "id" = ?'); + }); + + $this->actingAs($provider) + ->putJson("/api/provider/bookings/{$booking->id}/confirm") + ->assertStatus(500) + ->assertExactJson(['message' => 'Server Error']) + ->assertDontSee('update', false) + ->assertDontSee('course_schedules', false) + ->assertDontSee('current_participants', false); + + Exceptions::assertReported(QueryException::class); + $this->assertSame(BookingStatus::Pending, $booking->fresh()->status); + $this->assertSame(0, $schedule->fresh()->current_participants); + } +}