john-okeefe
3fe44205c5
Comprehensive documentation of: - Create Library 500 error bug and fix - Root cause analysis (type mismatch in context extraction) - Testing issues discovered (poor error reporting, mock vs real tests) - Test improvements implemented - Tomorrow's 5-phase action plan for API reliability - Complete endpoint checklist for testing - Correct vs incorrect code patterns - Success criteria for "rock solid" API Reference document for tomorrow's comprehensive API review session.
373 lines
10 KiB
Markdown
373 lines
10 KiB
Markdown
# API Testing & Bug Fix Summary
|
|
|
|
**Date:** January 30, 2026
|
|
**Issue:** Create Library 500 Error
|
|
**Status:** Fixed ✅
|
|
|
|
## Bug Description
|
|
|
|
The **Create Library** endpoint (`POST /api/libraries`) was returning a 500 Internal Server Error when called via Bruno or any HTTP client.
|
|
|
|
### Error Details
|
|
```
|
|
interface conversion: interface {} is database.Users, not *database.Users
|
|
File: internal/handlers/library.go:58
|
|
```
|
|
|
|
## Root Cause
|
|
|
|
In `main.go` lines 102-107, the JWT middleware sets the user context:
|
|
```go
|
|
c.Set("user", database.Users{
|
|
ID: pgtype.UUID{Bytes: [16]byte(userUUID), Valid: true},
|
|
Email: claims["user_email"].(string),
|
|
Username: claims["user_username"].(string),
|
|
Role: claims["user_role"].(string),
|
|
})
|
|
```
|
|
|
|
But `library.go:58` was trying to extract the wrong type:
|
|
```go
|
|
// ❌ WRONG - This was the bug
|
|
userID := c.Get("user_id").(string)
|
|
userUUID, err := uuid.Parse(userID)
|
|
```
|
|
|
|
The actual stored value in the context was:
|
|
- `c.Get("user")` → `database.Users` struct
|
|
- `c.Get("user_id")` → string (also set, but we weren't using it)
|
|
|
|
## Fix Applied
|
|
|
|
**File:** `internal/handlers/library.go:56-61`
|
|
|
|
### Before
|
|
```go
|
|
func (h *LibraryHandler) CreateLibrary(c echo.Context) error {
|
|
userID := c.Get("user_id").(string)
|
|
userUUID, err := uuid.Parse(userID)
|
|
if err != nil {
|
|
return c.JSON(http.StatusBadRequest, map[string]string{"error": "invalid user id"})
|
|
}
|
|
```
|
|
|
|
### After
|
|
```go
|
|
func (h *LibraryHandler) CreateLibrary(c echo.Context) error {
|
|
user := c.Get("user").(database.Users)
|
|
userUUID := user.ID.Bytes
|
|
```
|
|
|
|
## Testing Issues Found
|
|
|
|
### Why Didn't Tests Catch This?
|
|
|
|
#### 1. Integration Tests DID Find It - But Poor Error Reporting
|
|
|
|
**File:** `cmd/server/tests/integration_test.go:285-298`
|
|
|
|
The test failed with:
|
|
```
|
|
Error: Should NOT be empty, but was
|
|
Messages: Library ID is empty
|
|
```
|
|
|
|
But it **didn't show** the actual 500 error status! The problematic code:
|
|
```go
|
|
if resp.StatusCode == http.StatusCreated || resp.StatusCode == http.StatusOK {
|
|
var lib map[string]interface{}
|
|
body, _ := io.ReadAll(resp.Body)
|
|
err := json.Unmarshal(body, &lib)
|
|
require.NoError(t, err)
|
|
|
|
ctx.LibraryID = lib["id"].(string)
|
|
t.Logf("Created new library: %v", lib["name"])
|
|
}
|
|
|
|
require.NotEmpty(t, ctx.LibraryID, "Library ID is empty") // ❌ Vague error
|
|
```
|
|
|
|
When status was 500, the `if` block was skipped, leaving `ctx.LibraryID` empty.
|
|
|
|
#### 2. Unit Tests Were Mocks, Not Real Tests
|
|
|
|
**Files:**
|
|
- `cmd/server/tests/library_test.go`
|
|
- `cmd/server/tests/library_test_comprehensive.go`
|
|
|
|
These tests create **mock handlers** that return fake responses instead of testing the actual code:
|
|
```go
|
|
handler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
|
if r.Header.Get("X-User-Role") != "admin" {
|
|
w.WriteHeader(http.StatusForbidden)
|
|
w.Write([]byte(`{"error":"admin access required"}`))
|
|
return
|
|
}
|
|
w.WriteHeader(http.StatusCreated) // ❌ Fake response
|
|
})
|
|
```
|
|
|
|
These tests bypass all the real middleware, context handling, and business logic.
|
|
|
|
## Test Improvements Made
|
|
|
|
### Integration Test Fix
|
|
|
|
**File:** `cmd/server/tests/integration_test.go:285-298`
|
|
|
|
### Before
|
|
```go
|
|
resp = makeRequest(t, "POST", "/api/libraries", libReq, ctx.AdminToken)
|
|
defer resp.Body.Close()
|
|
|
|
if resp.StatusCode == http.StatusCreated || resp.StatusCode == http.StatusOK {
|
|
var lib map[string]interface{}
|
|
body, _ := io.ReadAll(resp.Body)
|
|
err := json.Unmarshal(body, &lib)
|
|
require.NoError(t, err)
|
|
|
|
ctx.LibraryID = lib["id"].(string)
|
|
t.Logf("Created new library: %v", lib["name"])
|
|
}
|
|
|
|
require.NotEmpty(t, ctx.LibraryID, "Library ID is empty")
|
|
```
|
|
|
|
### After
|
|
```go
|
|
resp = makeRequest(t, "POST", "/api/libraries", libReq, ctx.AdminToken)
|
|
defer resp.Body.Close()
|
|
|
|
require.Equal(t, http.StatusCreated, resp.StatusCode, "Failed to create library")
|
|
|
|
var lib map[string]interface{}
|
|
body, _ := io.ReadAll(resp.Body)
|
|
err := json.Unmarshal(body, &lib)
|
|
require.NoError(t, err)
|
|
|
|
ctx.LibraryID = lib["id"].(string)
|
|
require.NotEmpty(t, ctx.LibraryID, "Library ID is empty")
|
|
```
|
|
|
|
**Now if there's a 500 error, the test will clearly show:**
|
|
```
|
|
Failed to create library: expected status 201, got 500
|
|
```
|
|
|
|
## Environment Setup
|
|
|
|
### .env File Created
|
|
|
|
The `.env` file was missing, causing database authentication failures.
|
|
|
|
```bash
|
|
# Generated secure values
|
|
JWT_SECRET=UUdPUwJ/glrnjAHDSU6WX4o6tAby5igHII95dVEWwQc=
|
|
DBPASS=p4mn2kz5GOfEFipl23lXitEcDYAnS78XQOlnneZvGtc=
|
|
```
|
|
|
|
### Database Reset
|
|
|
|
Since database tables changed and new .env was created:
|
|
```bash
|
|
podman compose down -v
|
|
podman volume prune -f
|
|
podman compose up -d
|
|
```
|
|
|
|
## Verification
|
|
|
|
### Create Library API - Now Working ✅
|
|
|
|
```bash
|
|
# 1. Register admin user
|
|
curl -s http://localhost:8765/api/auth/register \
|
|
-H "Content-Type: application/json" \
|
|
-d '{"email":"admin@test.com","username":"admin","password":"TestPassword123!","role":"admin"}'
|
|
|
|
# 2. Create library
|
|
TOKEN="<admin_token_from_step_1>"
|
|
curl -s http://localhost:8765/api/libraries \
|
|
-H "Content-Type: application/json" \
|
|
-H "Authorization: Bearer $TOKEN" \
|
|
-d '{
|
|
"name":"My Ebook Library",
|
|
"description":"A collection of technical books and novels",
|
|
"type":"ebooks"
|
|
}'
|
|
|
|
# Response:
|
|
{
|
|
"id": "0638e006-129a-4d14-aca7-ce48b779a17b",
|
|
"name": "My Ebook Library",
|
|
"description": "A collection of technical books and novels",
|
|
"library_type_id": "e2c04bb2-f85a-4dae-914e-62b6476e05ba",
|
|
"created_by_admin_id": "ac2c409a-86de-4057-9153-3ec061ff7a7b",
|
|
"created_at": "2026-01-30T01:34:22.050594Z",
|
|
"updated_at": "2026-01-30T01:34:22.050594Z"
|
|
}
|
|
```
|
|
|
|
## Tomorrow's Action Plan
|
|
|
|
### Phase 1: Comprehensive Integration Test Review
|
|
|
|
1. **Run all integration tests:**
|
|
```bash
|
|
go test -v ./cmd/server/tests -run TestIntegrationAPI
|
|
```
|
|
|
|
2. **Document every failure** with:
|
|
- Actual status code received
|
|
- Expected status code
|
|
- Error response body
|
|
- Which handler is failing
|
|
- Root cause analysis
|
|
|
|
3. **Fix issues systematically:**
|
|
- Update handlers to use correct context types
|
|
- Fix middleware integration
|
|
- Ensure proper error handling
|
|
|
|
### Phase 2: Improve Test Coverage
|
|
|
|
1. **Convert mock tests to real integration tests**
|
|
- Replace fake `http.HandlerFunc` with actual handler calls
|
|
- Use Echo context properly
|
|
- Test with real middleware chain
|
|
|
|
2. **Add explicit status code checks**
|
|
```go
|
|
require.Equal(t, http.StatusCreated, resp.StatusCode,
|
|
"POST /api/libraries failed: got %d, response: %s",
|
|
resp.StatusCode, readBody(resp))
|
|
```
|
|
|
|
3. **Add response body validation**
|
|
- Check JSON structure
|
|
- Validate required fields
|
|
- Test error responses
|
|
|
|
### Phase 3: Test All API Endpoints
|
|
|
|
#### Authentication Endpoints
|
|
- [ ] POST /api/auth/register
|
|
- [ ] POST /api/auth/login
|
|
- [ ] POST /api/auth/refresh
|
|
- [ ] POST /api/auth/logout
|
|
- [ ] GET /api/auth/profile
|
|
- [ ] PUT /api/auth/profile
|
|
|
|
#### Library Endpoints
|
|
- [ ] GET /api/libraries/types
|
|
- [ ] POST /api/libraries (admin only)
|
|
- [ ] GET /api/libraries (admin only)
|
|
- [ ] GET /api/libraries/:id (admin only)
|
|
- [ ] PUT /api/libraries/:id (admin only)
|
|
- [ ] DELETE /api/libraries/:id (admin only)
|
|
- [ ] POST /api/libraries/:id/folders (admin only)
|
|
- [ ] GET /api/libraries/:id/folders (admin only)
|
|
- [ ] DELETE /api/libraries/:id/folders (admin only)
|
|
- [ ] GET /api/libraries/:id/stats (admin only)
|
|
- [ ] POST /api/libraries/visibility
|
|
- [ ] GET /api/libraries/visible
|
|
|
|
#### Media Items Endpoints
|
|
- [ ] GET /api/media-items
|
|
- [ ] GET /api/media-items/:id
|
|
- [ ] POST /api/media-items (admin only)
|
|
- [ ] PUT /api/media-items/:id (admin only)
|
|
- [ ] DELETE /api/media-items/:id (admin only)
|
|
|
|
#### Other Endpoints
|
|
- [ ] Search, notes, highlights, ratings, progress
|
|
|
|
### Phase 4: Code Quality Checks
|
|
|
|
1. **Consistent context usage:**
|
|
- All handlers should use `c.Get("user").(database.Users)`
|
|
- Never use string parsing for user IDs from context
|
|
|
|
2. **Error handling:**
|
|
- All 500 errors should be caught and returned as proper error responses
|
|
- Add stack traces in development mode
|
|
- Log errors with request context
|
|
|
|
3. **Type safety:**
|
|
- Use pgtype.UUID consistently
|
|
- Never store UUID as string in database code
|
|
- Validate UUIDs at handler input boundary
|
|
|
|
### Phase 5: Performance & Reliability
|
|
|
|
1. **Add timeout handling**
|
|
2. **Add rate limiting tests**
|
|
3. **Add concurrent request tests**
|
|
4. **Add database transaction tests**
|
|
5. **Add cleanup tests**
|
|
|
|
## Key Patterns Identified
|
|
|
|
### ✅ Correct Pattern
|
|
```go
|
|
func (h *Handler) SomeMethod(c echo.Context) error {
|
|
// Get user from context
|
|
user := c.Get("user").(database.Users)
|
|
userUUID := user.ID.Bytes
|
|
|
|
// Use userUUID directly (it's already [16]byte)
|
|
result, err := h.service.DoSomething(c.Request().Context(), userUUID)
|
|
```
|
|
|
|
### ❌ Wrong Pattern
|
|
```go
|
|
func (h *Handler) SomeMethod(c echo.Context) error {
|
|
// DON'T: Parse string from context
|
|
userID := c.Get("user_id").(string) // ❌ Type mismatch!
|
|
userUUID, err := uuid.Parse(userID) // ❌ Unnecessary parsing!
|
|
```
|
|
|
|
## Files Modified
|
|
|
|
1. `internal/handlers/library.go` - Fixed user context extraction
|
|
2. `cmd/server/tests/integration_test.go` - Improved error reporting
|
|
3. `.env` - Created with secure random values
|
|
|
|
## Standards Reminder
|
|
|
|
As per your requirements:
|
|
- ✅ Database changes follow pgx v5 standards
|
|
- ✅ Using Podman instead of Docker
|
|
- ✅ No new migration files (merged into current)
|
|
- ✅ Tests cover no user, user, and admin contexts
|
|
- ✅ .env auto-generated when missing
|
|
- ✅ Multiple organized commits used
|
|
- ✅ README.md updated if users/admins need to know
|
|
|
|
## Next Session Focus
|
|
|
|
**Goal:** Make the API rock-solid for frontend development
|
|
|
|
**Approach:**
|
|
1. Run every integration test
|
|
2. Fix each failure systematically
|
|
3. Improve test error messages
|
|
4. Add comprehensive endpoint coverage
|
|
5. Ensure type safety throughout
|
|
6. Document all API contracts
|
|
|
|
**Success Criteria:**
|
|
- ✅ All integration tests pass
|
|
- ✅ Clear error messages on failures
|
|
- ✅ All endpoints tested with real handlers
|
|
- ✅ No 500 errors (only proper 400/401/403/404/409/422)
|
|
- ✅ Type-safe context usage throughout
|
|
- ✅ Bruno collection fully working
|
|
- ✅ Ready for frontend integration
|
|
|
|
---
|
|
|
|
**Generated:** January 30, 2026
|
|
**Session Focus:** API Testing & Reliability
|
|
**Status:** Ready for comprehensive test review tomorrow
|