From 33c945159ce19890148ed118e930c0fe607aec43 Mon Sep 17 00:00:00 2001 From: Stephen Adamson Date: Sun, 5 Jul 2026 11:47:54 +0100 Subject: [PATCH] feat: add cancel reservation handler and background cleanup Add CancelReservationHandler (DELETE /api/bookings/reserve) to release authenticated user's active reservation. Register route in main.go. Add background goroutine for periodic reservation cleanup using CleanupOldReservations. Add idx_time_blockers_created_at index and extend anon cleanup to cover edit_request reservations in init-script.sql. Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus --- .../handlers/bookings/cancel_reservation.go | 35 ++ .../bookings/cancel_reservation_test.go | 301 ++++++++++++++++++ backend/main.go | 30 +- init-scripts/init-script.sql | 4 +- 4 files changed, 366 insertions(+), 4 deletions(-) create mode 100644 backend/handlers/bookings/cancel_reservation.go create mode 100644 backend/handlers/bookings/cancel_reservation_test.go diff --git a/backend/handlers/bookings/cancel_reservation.go b/backend/handlers/bookings/cancel_reservation.go new file mode 100644 index 0000000..4a613f5 --- /dev/null +++ b/backend/handlers/bookings/cancel_reservation.go @@ -0,0 +1,35 @@ +package bookings + +import ( + "log" + "net/http" + + "crussell/db" + "crussell/mw" +) + +// CancelReservationHandler releases the authenticated user's active reservation +// by deleting their RESERVATION:user entries from time_blockers. +// It does NOT create a booking — it only releases the temporary slot hold. +// +// DELETE /api/bookings/reserve +func CancelReservationHandler(w http.ResponseWriter, r *http.Request) { + userID, ok := r.Context().Value(mw.UserIDKey).(string) + if !ok || userID == "" { + log.Printf("CancelReservationHandler: user ID not found in context") + http.Error(w, "Authentication required", http.StatusUnauthorized) + return + } + + _, err := db.Conn.Exec(r.Context(), ` + DELETE FROM time_blockers + WHERE created_by = $1 AND description LIKE 'RESERVATION:user:%' + `, userID) + if err != nil { + log.Printf("CancelReservationHandler: failed to delete reservation for user %s: %v", userID, err) + http.Error(w, "Internal server error", http.StatusInternalServerError) + return + } + + mw.RespondJSON(w, http.StatusOK, map[string]string{"status": "reservation cancelled"}) +} diff --git a/backend/handlers/bookings/cancel_reservation_test.go b/backend/handlers/bookings/cancel_reservation_test.go new file mode 100644 index 0000000..ec730ca --- /dev/null +++ b/backend/handlers/bookings/cancel_reservation_test.go @@ -0,0 +1,301 @@ +//go:build test && dev +// +build test,dev + +package bookings + +import ( + "context" + "encoding/json" + "fmt" + "net/http" + "net/http/httptest" + "testing" + "time" + + "crussell/clock" + "crussell/mw" + "crussell/testutils" + "crussell/testutils/fixtures" + "crussell/testutils/jwt" + + "github.com/go-chi/chi/v5" +) + +// cancelReservationRequest creates and serves a DELETE /api/bookings/reserve request. +// The ctx carries the test transaction from SetupTestTx so the handler's +// db.Conn.Exec call routes through the same transaction. +// If userID is non-empty, it sets up the auth context (simulating RequireAuth). +func cancelReservationRequest(ctx context.Context, userID, role, token string) *httptest.ResponseRecorder { + req := httptest.NewRequest("DELETE", "/api/bookings/reserve", nil) + if token != "" { + req.Header.Set("Authorization", "Bearer "+token) + } + // Start with the test transaction context so db.Conn.Exec routes through it + rctx := chi.NewRouteContext() + reqCtx := context.WithValue(ctx, chi.RouteCtxKey, rctx) + if userID != "" { + reqCtx = context.WithValue(reqCtx, mw.UserIDKey, userID) + reqCtx = context.WithValue(reqCtx, mw.UserRoleKey, role) + } + req = req.WithContext(reqCtx) + + w := httptest.NewRecorder() + http.HandlerFunc(CancelReservationHandler).ServeHTTP(w, req) + return w +} + +// TestCancelReservation_Success creates a reservation for the user, cancels it, +// and verifies the time_blocker is deleted from the database. +func TestCancelReservation_Success(t *testing.T) { + t.Parallel() + ctx, tx := testutils.SetupTestTx(t) + + userID, err := fixtures.CreateTestUser(tx) + if err != nil { + t.Fatalf("failed to create user: %v", err) + } + + token := jwt.GenerateUserToken(userID) + startTime := clock.Now().Add(24 * time.Hour).Truncate(24 * time.Hour).Add(10 * time.Hour) + + // Create a reservation for this user + _, err = tx.Exec(ctx, fmt.Sprintf(` + INSERT INTO time_blockers (start_time, duration_minutes, description, created_by) + VALUES ($1, 60, 'RESERVATION:user:%s:12345', $2) + `, userID), startTime, userID) + if err != nil { + t.Fatalf("failed to create reservation: %v", err) + } + + // Verify reservation exists before cancel + var countBefore int + err = tx.QueryRow(ctx, + `SELECT COUNT(*) FROM time_blockers WHERE description LIKE 'RESERVATION:user:%' AND created_by = $1`, + userID, + ).Scan(&countBefore) + if err != nil { + t.Fatalf("failed to count reservations before: %v", err) + } + if countBefore != 1 { + t.Fatalf("expected 1 reservation before cancel, got %d", countBefore) + } + + // Call CancelReservationHandler + w := cancelReservationRequest(ctx, userID, "verified_email", token) + + if w.Code != http.StatusOK { + t.Errorf("expected status 200, got %d. body: %s", w.Code, w.Body.String()) + } + + // Parse response + var resp map[string]string + if err := json.Unmarshal(w.Body.Bytes(), &resp); err != nil { + t.Fatalf("failed to parse response: %v", err) + } + if resp["status"] != "reservation cancelled" { + t.Errorf("expected status 'reservation cancelled', got %q", resp["status"]) + } + + // Verify reservation was deleted + var countAfter int + err = tx.QueryRow(ctx, + `SELECT COUNT(*) FROM time_blockers WHERE description LIKE 'RESERVATION:user:%' AND created_by = $1`, + userID, + ).Scan(&countAfter) + if err != nil { + t.Fatalf("failed to count reservations after: %v", err) + } + if countAfter != 0 { + t.Errorf("expected 0 reservations after cancel, got %d", countAfter) + } +} + +// TestCancelReservation_NoActiveReservation verifies that calling cancel +// without an active reservation returns 200 (idempotent, no error). +func TestCancelReservation_NoActiveReservation(t *testing.T) { + t.Parallel() + ctx, tx := testutils.SetupTestTx(t) + + userID, err := fixtures.CreateTestUser(tx) + if err != nil { + t.Fatalf("failed to create user: %v", err) + } + + token := jwt.GenerateUserToken(userID) + + // Verify no reservations exist + var countBefore int + err = tx.QueryRow(ctx, + `SELECT COUNT(*) FROM time_blockers WHERE description LIKE 'RESERVATION:user:%' AND created_by = $1`, + userID, + ).Scan(&countBefore) + if err != nil { + t.Fatalf("failed to count reservations before: %v", err) + } + if countBefore != 0 { + t.Fatalf("expected 0 reservations before cancel, got %d", countBefore) + } + + // Call CancelReservationHandler + w := cancelReservationRequest(ctx, userID, "verified_email", token) + + if w.Code != http.StatusOK { + t.Errorf("expected status 200 (idempotent), got %d. body: %s", w.Code, w.Body.String()) + } + + // Verify still no reservations + var countAfter int + err = tx.QueryRow(ctx, + `SELECT COUNT(*) FROM time_blockers WHERE description LIKE 'RESERVATION:user:%' AND created_by = $1`, + userID, + ).Scan(&countAfter) + if err != nil { + t.Fatalf("failed to count reservations after: %v", err) + } + if countAfter != 0 { + t.Errorf("expected 0 reservations after cancel, got %d", countAfter) + } +} + +// TestCancelReservation_Unauthenticated verifies that calling the handler +// without a valid user in context returns 401 Unauthorized. +func TestCancelReservation_Unauthenticated(t *testing.T) { + t.Parallel() + + w := cancelReservationRequest(context.Background(), "", "", "") + + if w.Code != http.StatusUnauthorized { + t.Errorf("expected status 401 for unauthenticated request, got %d. body: %s", w.Code, w.Body.String()) + } +} + +// TestCancelReservation_EmptyUserIDInContext verifies that having a context +// with an empty userID string is treated as unauthenticated. +func TestCancelReservation_EmptyUserIDInContext(t *testing.T) { + t.Parallel() + + w := cancelReservationRequest(context.Background(), "", "verified_email", "") + + if w.Code != http.StatusUnauthorized { + t.Errorf("expected status 401 for empty userID in context, got %d. body: %s", w.Code, w.Body.String()) + } +} + +// TestCancelReservation_DeletesOnlyOwnUserReservation verifies that cancelling +// only deletes the requesting user's reservation, not another user's. +func TestCancelReservation_DeletesOnlyOwnUserReservation(t *testing.T) { + t.Parallel() + ctx, tx := testutils.SetupTestTx(t) + + user1ID, err := fixtures.CreateTestUser(tx) + if err != nil { + t.Fatalf("failed to create user1: %v", err) + } + user2ID, err := fixtures.CreateTestUser(tx) + if err != nil { + t.Fatalf("failed to create user2: %v", err) + } + + token1 := jwt.GenerateUserToken(user1ID) + startTime := clock.Now().Add(24 * time.Hour).Truncate(24 * time.Hour).Add(10 * time.Hour) + + // Create reservation for user1 + _, err = tx.Exec(ctx, fmt.Sprintf(` + INSERT INTO time_blockers (start_time, duration_minutes, description, created_by) + VALUES ($1, 60, 'RESERVATION:user:%s:11111', $2) + `, user1ID), startTime, user1ID) + if err != nil { + t.Fatalf("failed to create user1 reservation: %v", err) + } + + // Create reservation for user2 + _, err = tx.Exec(ctx, fmt.Sprintf(` + INSERT INTO time_blockers (start_time, duration_minutes, description, created_by) + VALUES ($1, 60, 'RESERVATION:user:%s:22222', $2) + `, user2ID), startTime, user2ID) + if err != nil { + t.Fatalf("failed to create user2 reservation: %v", err) + } + + // Call CancelReservationHandler as user1 + w := cancelReservationRequest(ctx, user1ID, "verified_email", token1) + + if w.Code != http.StatusOK { + t.Errorf("expected status 200, got %d. body: %s", w.Code, w.Body.String()) + } + + // Verify user1's reservation is gone + var user1Count int + err = tx.QueryRow(ctx, + `SELECT COUNT(*) FROM time_blockers WHERE description LIKE 'RESERVATION:user:%' AND created_by = $1`, + user1ID, + ).Scan(&user1Count) + if err != nil { + t.Fatalf("failed to count user1 reservations: %v", err) + } + if user1Count != 0 { + t.Errorf("expected user1 reservations to be deleted, got %d", user1Count) + } + + // Verify user2's reservation still exists + var user2Count int + err = tx.QueryRow(ctx, + `SELECT COUNT(*) FROM time_blockers WHERE description LIKE 'RESERVATION:user:%' AND created_by = $1`, + user2ID, + ).Scan(&user2Count) + if err != nil { + t.Fatalf("failed to count user2 reservations: %v", err) + } + if user2Count != 1 { + t.Errorf("expected user2 reservation to survive, got %d", user2Count) + } +} + +// TestCancelReservation_CleanIdempotent verifies calling cancel twice +// is safe (second call also succeeds, no error). +func TestCancelReservation_CleanIdempotent(t *testing.T) { + t.Parallel() + ctx, tx := testutils.SetupTestTx(t) + + userID, err := fixtures.CreateTestUser(tx) + if err != nil { + t.Fatalf("failed to create user: %v", err) + } + + token := jwt.GenerateUserToken(userID) + startTime := clock.Now().Add(24 * time.Hour).Truncate(24 * time.Hour).Add(10 * time.Hour) + + // Create a reservation for this user + _, err = tx.Exec(ctx, fmt.Sprintf(` + INSERT INTO time_blockers (start_time, duration_minutes, description, created_by) + VALUES ($1, 60, 'RESERVATION:user:%s:99999', $2) + `, userID), startTime, userID) + if err != nil { + t.Fatalf("failed to create reservation: %v", err) + } + + // First cancel + w1 := cancelReservationRequest(ctx, userID, "verified_email", token) + if w1.Code != http.StatusOK { + t.Fatalf("first cancel: expected 200, got %d. body: %s", w1.Code, w1.Body.String()) + } + + // Second cancel (no reservation left) + w2 := cancelReservationRequest(ctx, userID, "verified_email", token) + if w2.Code != http.StatusOK { + t.Errorf("second cancel (idempotent): expected 200, got %d. body: %s", w2.Code, w2.Body.String()) + } + + // Verify no reservations remain + var count int + err = tx.QueryRow(ctx, + `SELECT COUNT(*) FROM time_blockers WHERE description LIKE 'RESERVATION:user:%' AND created_by = $1`, + userID, + ).Scan(&count) + if err != nil { + t.Fatalf("failed to count reservations: %v", err) + } + if count != 0 { + t.Errorf("expected 0 reservations after two cancels, got %d", count) + } +} diff --git a/backend/main.go b/backend/main.go index b20d476..fce31f5 100644 --- a/backend/main.go +++ b/backend/main.go @@ -65,9 +65,11 @@ func limitBody(limit int64) func(http.Handler) http.Handler { } const ( - defaultBodyLimit int64 = 1 * 1024 * 1024 // 1MB - uploadBodyLimit int64 = 15 * 1024 * 1024 // 15MB - portfolioBodyLimit int64 = 40 * 1024 * 1024 // 40MB (7 variants from 20MB source) + defaultBodyLimit int64 = 1 * 1024 * 1024 // 1MB + uploadBodyLimit int64 = 15 * 1024 * 1024 // 15MB + portfolioBodyLimit int64 = 40 * 1024 * 1024 // 40MB (7 variants from 20MB source) + reservationCleanupInterval = 5 * time.Minute + reservationCleanupTimeout = 30 * time.Second ) func initDB() { @@ -287,6 +289,7 @@ func main() { r.Delete("/bookings/{id}/edit-request", bookings.DeleteEditRequestHandler) r.Get("/bookings/{id}/edit-request", bookings.GetMyEditRequestHandler) r.Get("/bookings/edit-requests", bookings.GetMyEditRequestsHandler) + r.Delete("/bookings/reserve", bookings.CancelReservationHandler) // User payment routes r.Post("/bookings/{id}/payment", payments.CreateBookingPayment) @@ -424,6 +427,26 @@ r.Route("/admin/users", func(r chi.Router) { // Webhooks (no auth - Square sends to base path) r.Post("/webhooks/square", webhooks.HandleSquareWebhook) + // Background cleanup of expired reservations + cleanupCtx, cleanupStop := context.WithCancel(context.Background()) + go func() { + ticker := time.NewTicker(reservationCleanupInterval) + defer ticker.Stop() + for { + select { + case <-ticker.C: + ctx, cancel := context.WithTimeout(context.Background(), reservationCleanupTimeout) + if err := scheduling.CleanupOldReservations(ctx); err != nil { + log.Printf("reservation cleanup error: %v", err) + } + cancel() + case <-cleanupCtx.Done(): + log.Println("reservation cleanup goroutine stopped") + return + } + } + }() + srv := &http.Server{ Addr: ":8080", Handler: r, @@ -439,6 +462,7 @@ r.Route("/admin/users", func(r chi.Router) { if err := srv.Shutdown(ctx); err != nil { log.Printf("Server forced to shutdown: %v", err) } + cleanupStop() }() fmt.Println("Server is listening on :8080") diff --git a/init-scripts/init-script.sql b/init-scripts/init-script.sql index 9dd543e..130e371 100644 --- a/init-scripts/init-script.sql +++ b/init-scripts/init-script.sql @@ -604,6 +604,7 @@ CREATE TABLE time_blockers ( CREATE INDEX idx_time_blockers_start_time ON time_blockers(start_time); CREATE INDEX idx_time_blockers_cron ON time_blockers(cron_expression) WHERE cron_expression IS NOT NULL; CREATE INDEX idx_time_blockers_created_by ON time_blockers(created_by); +CREATE INDEX idx_time_blockers_created_at ON time_blockers(created_at); CREATE INDEX IF NOT EXISTS idx_time_blockers_desc_trgm ON time_blockers USING GIN (description gin_trgm_ops); -- ======================================= @@ -918,7 +919,8 @@ BEGIN UPDATE time_blockers SET description = NULL WHERE created_by = target_id - AND description LIKE 'RESERVATION:user:%'; + AND (description LIKE 'RESERVATION:user:%' + OR description LIKE 'RESERVATION:edit_request:%'); -- Scrub notes on booking_edit_requests made by this user (free-text PII) UPDATE booking_edit_requests