From 95fe849eebecac7f8dcce8f46ec405f4d6bffb0e Mon Sep 17 00:00:00 2001 From: John O'Keefe Date: Fri, 30 Jan 2026 20:16:19 -0500 Subject: [PATCH] Fix test infrastructure and device UUID handling - Remove manual device ID generation, use database-generated IDs - Add comprehensive test helpers (setupTestServer, loginTestUser, getTestUserID) - Add cleanup step for existing test users in integration tests - Fix UUID parsing from database responses --- cmd/server/tests/device_test.go | 44 +++--- cmd/server/tests/phase1_integration_test.go | 73 ++++++++++ cmd/server/tests/test_helpers.go | 143 ++++++++++++++++++++ 3 files changed, 239 insertions(+), 21 deletions(-) diff --git a/cmd/server/tests/device_test.go b/cmd/server/tests/device_test.go index e7eac07..4e1dd2a 100644 --- a/cmd/server/tests/device_test.go +++ b/cmd/server/tests/device_test.go @@ -3,12 +3,12 @@ package main import ( "bookmann/internal/database" "bytes" + "context" "encoding/json" "fmt" "net/http" "net/http/httptest" "testing" - "time" "github.com/google/uuid" "github.com/jackc/pgx/v5/pgtype" @@ -16,7 +16,7 @@ import ( ) func TestDeviceRegistrationFlow(t *testing.T) { - _, _, _, ts := setupTestServer(t) + ts, _, _, _ := setupTestServer(t) defer ts.Close() // Step 1: Initiate device registration @@ -115,7 +115,7 @@ func TestDeviceRegistrationFlow(t *testing.T) { } func TestListDevices(t *testing.T) { - db, _, _, ts := setupTestServer(t) + ts, db, _, _ := setupTestServer(t) defer ts.Close() // Login to get token @@ -123,11 +123,9 @@ func TestListDevices(t *testing.T) { // Create a device directly in the database userID := getTestUserID(t, db) - deviceID := uuid.New() deviceToken := fmt.Sprintf("dev_%s", uuid.New().String()) _, err := db.CreateDevice(context.Background(), database.CreateDeviceParams{ - ID: pgtype.UUID{Bytes: [16]byte(deviceID), Valid: true}, UserID: pgtype.UUID{Bytes: [16]byte(userID), Valid: true}, DeviceName: "Test Device", DeviceType: "koreader", @@ -162,7 +160,7 @@ func TestListDevices(t *testing.T) { } func TestUpdateDevice(t *testing.T) { - db, _, _, ts := setupTestServer(t) + ts, db, _, _ := setupTestServer(t) defer ts.Close() // Login to get token @@ -170,11 +168,9 @@ func TestUpdateDevice(t *testing.T) { // Create a device directly in the database userID := getTestUserID(t, db) - deviceID := uuid.New() deviceToken := fmt.Sprintf("dev_%s", uuid.New().String()) - _, err := db.CreateDevice(context.Background(), database.CreateDeviceParams{ - ID: pgtype.UUID{Bytes: [16]byte(deviceID), Valid: true}, + device, err := db.CreateDevice(context.Background(), database.CreateDeviceParams{ UserID: pgtype.UUID{Bytes: [16]byte(userID), Valid: true}, DeviceName: "Test Device", DeviceType: "koreader", @@ -187,6 +183,11 @@ func TestUpdateDevice(t *testing.T) { }) assert.NoError(t, err, "Should create device") + // Get the device ID from the created device + deviceIDBytes := device.ID.Bytes[0:16] + deviceID, err := uuid.FromBytes(deviceIDBytes) + assert.NoError(t, err, "Should parse device ID") + // Update device updateRequest := map[string]interface{}{ "device_name": "Updated Device Name", @@ -208,14 +209,14 @@ func TestUpdateDevice(t *testing.T) { assert.True(t, response["device_updated"].(bool), "Should confirm device updated") - device := response["device"].(map[string]interface{}) - assert.Equal(t, "Updated Device Name", device["device_name"], "Should have updated name") - assert.Equal(t, false, device["sync_enabled"], "Should be disabled") - assert.Equal(t, int32(10), device["sync_frequency"], "Should have updated frequency") + updatedDevice := response["device"].(map[string]interface{}) + assert.Equal(t, "Updated Device Name", updatedDevice["device_name"], "Should have updated name") + assert.Equal(t, false, updatedDevice["sync_enabled"], "Should be disabled") + assert.Equal(t, int32(10), updatedDevice["sync_frequency"], "Should have updated frequency") } func TestDeleteDevice(t *testing.T) { - db, _, _, ts := setupTestServer(t) + ts, db, _, _ := setupTestServer(t) defer ts.Close() // Login to get token @@ -223,11 +224,9 @@ func TestDeleteDevice(t *testing.T) { // Create a device directly in the database userID := getTestUserID(t, db) - deviceID := uuid.New() deviceToken := fmt.Sprintf("dev_%s", uuid.New().String()) - _, err := db.CreateDevice(context.Background(), database.CreateDeviceParams{ - ID: pgtype.UUID{Bytes: [16]byte(deviceID), Valid: true}, + newDevice, err := db.CreateDevice(context.Background(), database.CreateDeviceParams{ UserID: pgtype.UUID{Bytes: [16]byte(userID), Valid: true}, DeviceName: "Test Device", DeviceType: "koreader", @@ -240,6 +239,11 @@ func TestDeleteDevice(t *testing.T) { }) assert.NoError(t, err, "Should create device") + // Get the device ID from the created device + deviceIDBytes := newDevice.ID.Bytes[0:16] + deviceID, err := uuid.FromBytes(deviceIDBytes) + assert.NoError(t, err, "Should parse device ID") + // Delete device req := httptest.NewRequest("DELETE", fmt.Sprintf("/api/devices/%s", deviceID.String()), nil) req.Header.Set("Authorization", "Bearer "+token) @@ -249,21 +253,19 @@ func TestDeleteDevice(t *testing.T) { assert.Equal(t, http.StatusNoContent, rec.Code, "Should delete device") // Verify device is deleted - _, err = db.GetDevice(context.Background(), pgtype.UUID{Bytes: [16]byte(deviceID), Valid: true}) + _, err = db.GetDevice(context.Background(), newDevice.ID) assert.Error(t, err, "Device should be deleted") } func TestDeviceAuthentication(t *testing.T) { - db, _, _, ts := setupTestServer(t) + ts, db, _, _ := setupTestServer(t) defer ts.Close() // Create a device directly in the database userID := getTestUserID(t, db) - deviceID := uuid.New() deviceToken := fmt.Sprintf("dev_%s", uuid.New().String()) _, err := db.CreateDevice(context.Background(), database.CreateDeviceParams{ - ID: pgtype.UUID{Bytes: [16]byte(deviceID), Valid: true}, UserID: pgtype.UUID{Bytes: [16]byte(userID), Valid: true}, DeviceName: "Test Device", DeviceType: "koreader", diff --git a/cmd/server/tests/phase1_integration_test.go b/cmd/server/tests/phase1_integration_test.go index 6b81dc1..42ea93d 100644 --- a/cmd/server/tests/phase1_integration_test.go +++ b/cmd/server/tests/phase1_integration_test.go @@ -20,6 +20,79 @@ func TestPhase1Integration(t *testing.T) { t.Skip("Skipping integration test in short mode") } + // Cleanup: Try to delete test user if it exists from previous test runs + t.Run("Cleanup_ExistingTestUser", func(t *testing.T) { + // Try to login as the test user first + loginReq := map[string]interface{}{ + "login": "admin@bookmann.test", + "password": "SecurePass123!", + } + + body, _ := json.Marshal(loginReq) + resp, err := http.Post(baseTestURL+"/auth/login", "application/json", bytes.NewBuffer(body)) + if err != nil { + t.Logf("Cleanup: No existing test user to delete (server not available)") + return + } + defer resp.Body.Close() + + // If login succeeds, try to delete the user + if resp.StatusCode == http.StatusOK { + var result map[string]interface{} + json.NewDecoder(resp.Body).Decode(&result) + + if token, ok := result["access_token"].(string); ok { + // Delete the user using the token + req, _ := http.NewRequest("DELETE", baseTestURL+"/auth/account", bytes.NewBuffer([]byte{})) + req.Header.Set("Authorization", "Bearer "+token) + req.Header.Set("Content-Type", "application/json") + + client := &http.Client{} + delResp, err := client.Do(req) + if err == nil { + defer delResp.Body.Close() + if delResp.StatusCode == http.StatusNoContent { + t.Logf("Cleanup: Deleted existing test user") + } else { + t.Logf("Cleanup: Could not delete existing test user (HTTP %d)", delResp.StatusCode) + } + } + + // Also try to delete any libraries created by this user + req, _ = http.NewRequest("GET", baseTestURL+"/libraries", bytes.NewBuffer([]byte{})) + req.Header.Set("Authorization", "Bearer "+token) + + listResp, err := client.Do(req) + if err == nil { + defer listResp.Body.Close() + if listResp.StatusCode == http.StatusOK { + var libsResult map[string]interface{} + json.NewDecoder(listResp.Body).Decode(&libsResult) + + if data, ok := libsResult["data"].([]interface{}); ok { + for _, lib := range data { + if libMap, ok := lib.(map[string]interface{}); ok { + if libID, ok := libMap["id"].(string); ok { + // Delete the library + req, _ = http.NewRequest("DELETE", baseTestURL+"/libraries/"+libID, bytes.NewBuffer([]byte{})) + req.Header.Set("Authorization", "Bearer "+token) + delLibResp, _ := client.Do(req) + if delLibResp != nil { + delLibResp.Body.Close() + } + } + } + } + } + } + } + } + } + + // Wait a bit for cleanup to complete + time.Sleep(500 * time.Millisecond) + }) + // Step 1: Create first user (should be admin) t.Run("Step1_CreateFirstUser", func(t *testing.T) { userReq := map[string]interface{}{ diff --git a/cmd/server/tests/test_helpers.go b/cmd/server/tests/test_helpers.go index 4e3baab..55b9556 100644 --- a/cmd/server/tests/test_helpers.go +++ b/cmd/server/tests/test_helpers.go @@ -1,7 +1,25 @@ package main import ( + "bookmann/internal/config" + "bookmann/internal/database" + "bookmann/internal/handlers" + ratelimit "bookmann/internal/middleware" + "bytes" + "context" + "encoding/json" + "net/http" + "net/http/httptest" "strings" + "testing" + "time" + + "github.com/google/uuid" + "github.com/jackc/pgx/v5/pgtype" + "github.com/jackc/pgx/v5/pgxpool" + "github.com/labstack/echo/v4" + echomiddleware "github.com/labstack/echo/v4/middleware" + "github.com/stretchr/testify/require" ) // Helper functions for testing @@ -16,3 +34,128 @@ func contains(s, substr string) bool { func trimSpace(s string) string { return strings.TrimSpace(s) } + +// setupTestServer creates a test server with a test database +// Returns: (*httptest.Server, *database.Queries, *config.Config, *handlers.Handler) +func setupTestServer(t *testing.T) (*httptest.Server, *database.Queries, *config.Config, *handlers.Handler) { + // Load test configuration + cfg := &config.Config{ + ServerPort: "0", // Use random port for tests + BaseURL: "http://localhost", + DatabaseHost: "localhost", + DatabasePort: "5432", + DatabaseUser: "postgres", + DatabasePassword: "password", + DatabaseName: "bookmann", + JWTSecret: "test-secret-key", + UploadPath: "./test-uploads", + TestMode: true, + RateLimitEnabled: false, + RequestsPerMinute: 1000, + } + + // Connect to test database + dbPool, err := pgxpool.New(context.Background(), cfg.DatabaseURL()) + require.NoError(t, err, "Failed to connect to test database") + + queries := database.New(dbPool) + + // Create login attempt tracker + loginAttemptTracker := ratelimit.NewLoginAttemptTracker(5, 15*time.Minute, 5*time.Minute) + + // Create handlers + authHandler := handlers.NewAuthHandler(queries, cfg.JWTSecret, loginAttemptTracker) + deviceHandler := handlers.NewDeviceHandler(queries, cfg.JWTSecret, cfg) + + // Create Echo instance + e := echo.New() + + // Middleware + e.Use(echomiddleware.Logger()) + e.Use(echomiddleware.Recover()) + e.Use(echomiddleware.CORS()) + + // Setup routes + protected := e.Group("/api") + h := handlers.SetupRoutes(protected, queries) + + // Device management routes (public - for registration) + e.POST("/api/devices/register", deviceHandler.InitiateRegistration) + e.POST("/api/devices/register/status", deviceHandler.CheckRegistrationStatus) + e.GET("/devices/approve/:registration_id", deviceHandler.ApproveDevice) + e.POST("/devices/reject/:registration_id", deviceHandler.RejectDevice) + + // Device management routes (protected - require user auth) + devices := protected.Group("/devices") + devices.GET("", deviceHandler.ListDevices) + devices.GET("/:id", deviceHandler.GetDevice) + devices.PUT("/:id", deviceHandler.UpdateDevice) + devices.DELETE("/:id", deviceHandler.DeleteDevice) + devices.GET("/pending", deviceHandler.ListPendingRegistrations) + + // Auth routes (public - for testing) + e.POST("/api/auth/register", authHandler.Register) + e.POST("/api/auth/login", authHandler.Login) + + // Create test server + ts := httptest.NewServer(e) + + // Return server, queries, config, and handler + return ts, queries, cfg, h +} + +// loginTestUser logs in a test user and returns the JWT token +func loginTestUser(t *testing.T, ts *httptest.Server) string { + loginRequest := map[string]interface{}{ + "login": "testuser@example.com", + "password": "testpass123", + } + body, _ := json.Marshal(loginRequest) + + req, _ := http.NewRequest("POST", ts.URL+"/api/auth/login", bytes.NewBuffer(body)) + req.Header.Set("Content-Type", "application/json") + + client := &http.Client{} + resp, err := client.Do(req) + require.NoError(t, err, "Failed to login test user") + defer resp.Body.Close() + + require.Equal(t, http.StatusOK, resp.StatusCode, "Login should succeed") + + var result map[string]interface{} + json.NewDecoder(resp.Body).Decode(&result) + + token, ok := result["access_token"].(string) + require.True(t, ok, "Response should contain access_token") + require.NotEmpty(t, token, "Token should not be empty") + + return token +} + +// getTestUserID retrieves the test user ID from the database +func getTestUserID(t *testing.T, db *database.Queries) uuid.UUID { + // Try to get the test user by email + user, err := db.GetUserByEmail(context.Background(), "testuser@example.com") + if err == nil { + userUUID, err := uuid.FromBytes(user.ID.Bytes[:]) + require.NoError(t, err, "Failed to parse user UUID") + return userUUID + } + + // If user doesn't exist, create one + passwordHash := "$2a$10$N9qo8uLOickgx2ZMRZoMyeIjZAgcfl7p92ldGxad68LJZdL17lhWy" // "testpass123" hashed + + newUser, err := db.CreateUser(context.Background(), database.CreateUserParams{ + Email: "testuser@example.com", + Username: "testuser", + PasswordHash: passwordHash, + FirstName: pgtype.Text{String: "Test", Valid: true}, + LastName: pgtype.Text{String: "User", Valid: true}, + Role: "user", + }) + require.NoError(t, err, "Failed to create test user") + + userUUID, err := uuid.FromBytes(newUser.ID.Bytes[:]) + require.NoError(t, err, "Failed to parse user UUID") + return userUUID +}