From 27f62e9ae8e2cff3d68440898b9b39cfdcc0bf2e Mon Sep 17 00:00:00 2001 From: John O'Keefe Date: Tue, 10 Feb 2026 16:23:56 -0500 Subject: [PATCH] docs(schema): Consolidate schema initialization plans with corrections Remove obsolete planning documents: - KOBO_IMPLEMENTATION_PLAN.md (replaced by refined plan) - SCHEMA_INITIALIZATION_PLAN.md (replaced by refined plan) Update REFINED_SCHEMA_PLAN.md with critical corrections: - Fix FNV-1a hash constant (7804706162000639061, was 582394759234) - Correct table/index counts (27 tables, 82 indexes, not 36/102) - Change approach: embed existing schema.sql (no duplication) - Add local database update step after schema changes - Add documentation requirements section This consolidates three planning documents into one accurate, actionable plan for implementing automatic schema initialization with idempotent migrations. --- KOBO_IMPLEMENTATION_PLAN.md | 825 ------------------------ REFINED_SCHEMA_PLAN.md | 134 ++-- SCHEMA_INITIALIZATION_PLAN.md | 1131 --------------------------------- 3 files changed, 83 insertions(+), 2007 deletions(-) delete mode 100644 KOBO_IMPLEMENTATION_PLAN.md delete mode 100644 SCHEMA_INITIALIZATION_PLAN.md diff --git a/KOBO_IMPLEMENTATION_PLAN.md b/KOBO_IMPLEMENTATION_PLAN.md deleted file mode 100644 index 4e3eb2d..0000000 --- a/KOBO_IMPLEMENTATION_PLAN.md +++ /dev/null @@ -1,825 +0,0 @@ -# ๐Ÿ“‹ **Complete Implementation Plan: Fix Kobo Production Code & Add "last-read-place" Support** - -## ๐ŸŽฏ **Executive Summary** - -**Critical Finding**: Kobo DOES send "last-read-place" bookmarks with precise position data, but Bookhoard currently ignores them (falls through switch statement). Adding this support will significantly improve Kobo user experience. - -**Compliance**: All changes follow PROJECT_GUIDELINES.md with: -- No local builds (Podman/Docker only) -- No breaking changes without testing -- Step-by-step implementation (no cascading fix-ups) -- Clear git commits with detailed messages - ---- - -## โš ๏ธ **Pre-Execution Checklist** - -- [ ] Create backup branch: `git branch backup-before-kobo-comprehensive-fixes` -- [ ] Run baseline test suite: `make test-integration > baseline.txt 2>&1` -- [ ] Verify containers: `podman compose ps` -- [ ] Confirm in build mode (not local) - ---- - -## ๐Ÿ“Š **Phase 1: Critical Bug Fixes** (45 minutes) - -### **Step 1: Fix Rate Limit Header Bug** -**File**: `/internal/middleware/device_auth.go` -**Lines**: 79, 84, 90 - -**Current Code**: -```go -c.Response().Header().Set("X-RateLimit-Remaining", string(rune(remaining))) -``` - -**Issue**: Converts int to Unicode character (60 โ†’ `<`) - -**Fix**: -```go -import "strconv" -c.Response().Header().Set("X-RateLimit-Remaining", strconv.Itoa(remaining)) -``` - -**Verification**: -- Edit file -- `make build` && `podman compose restart app` -- Test rate-limited endpoint with curl -- Verify header returns integer - ---- - -### **Step 2: Fix Type Assertion Bug** -**File**: `/internal/middleware/device_auth.go` -**Lines**: 168-171 - -**Current Code**: -```go -deviceID, ok := c.Get("device_id").(uuid.UUID) // Always fails! -if ok { - m.db.UpdateDeviceLastSeen(c.Request().Context(), pgDeviceID) // Never executes -} -``` - -**Issue**: `device_id` is `[16]byte`, cast tries `uuid.UUID` - -**Fix**: -```go -deviceIDBytes, ok := c.Get("device_id").([16]byte) -if ok { - pgDeviceID := pgtype.UUID{Bytes: deviceIDBytes, Valid: true} - m.db.UpdateDeviceLastSeen(c.Request().Context(), pgDeviceID) -} -``` - ---- - -### **Step 3: Fix user_id Type Mismatch** -**File**: `/internal/middleware/device_auth.go` -**Line**: 105 - -**Analysis**: No handlers use `c.Get("user_id")`, so line is redundant - -**Fix**: Remove line entirely -```go -// DELETE: c.Set("user_id", device.UserID.Bytes) -``` - ---- - -### **Step 4: Fix Kobo Nil UUID Error Handling** -**File**: `/internal/handlers/kobo.go` -**Lines**: 402, 437, 515, 580 - -**Current Code**: -```go -bookhoardUUID, err, _ := h.mapContentIdToBookhoardUUID(c, contentId, deviceUUID) -if err != nil { - unlinkedBooks++ - continue // Uses potentially nil bookhoardUUID! -} -``` - -**Fix**: -```go -bookhoardUUID, err, _ := h.mapContentIdToBookhoardUUID(c, contentId, deviceUUID) -if err != nil || bookhoardUUID == uuid.Nil { - unlinkedBooks++ - continue // Skip nil UUIDs safely -} -``` - ---- - -### **Step 5: Add ContentType Detection** -**File**: `/internal/handlers/kobo.go` -**Lines**: Add to ReadReadingSync struct - -**Current Code**: -```go -type KoboReadingSync struct { - ContentId string `json:"ContentId"` - PercentRead float64 `json:"PercentRead"` - EntitlementId string `json:"EntitlementId"` - RemainingTimeMinutes int `json:"RemainingTimeMinutes"` - FirstReadTime string `json:"FirstReadTime,omitempty"` - LastModified string `json:"LastModified"` -} -``` - -**Fix**: -```go -type KoboReadingSync struct { - // ... existing fields ... - ContentType string `json:"ContentType,omitempty"` // NEW -} -``` - -**Update Initialization handler**: -```go -contentType := "6" // Default EPUB -if strings.Contains(mediaItem.MimeType.String, "pdf") { - contentType = "5" // PDF -} -``` - ---- - -## ๐Ÿ“Š **Phase 2: Kobo Enhancement** (30 minutes) - -### **Step 6: Add "last-read-place" Support** -**File**: `/internal/handlers/kobo.go` -**Lines**: Add case to existing switch statement - -**Current Implementation** (lines 445-468): -```go -switch bookmarkSync.BookmarkType { -case "annotation": - if bookmarkSync.BookmarkText != "" { - // Create highlight - h.db.CreateMediaHighlight(...) - } -case "bookmark": - if bookmarkSync.BookmarkText != "" { - // Create note - h.db.CreateMediaNote(...) - } -// "last-read-place" - FALLS THROUGH, IGNORED -} -``` - -**Fix**: -```go -switch bookmarkSync.BookmarkType { -case "annotation": - if bookmarkSync.BookmarkText != "" { - // Create highlight - h.db.CreateMediaHighlight(...) - } -case "bookmark": - if bookmarkSync.BookmarkText != "" { - // Create note - h.db.CreateMediaNote(...) - } -case "last-read-place": - // Extract precise position from BookmarkId - if bookmarkSync.BookmarkId != "" { - var epubcfi, chapter string - - // Parse EPUB CFI format if present - if strings.HasPrefix(bookmarkSync.BookmarkId, "epubcfi(") { - epubcfi = strings.TrimPrefix(bookmarkSync.BookmarkId, "epubcfi(") - epubcfi = strings.TrimSuffix(epubcfi, ")") - } - - // Store position data in reading_progress - _, err = h.db.UpdateUniversalProgress(c.Request().Context(), database.UpdateUniversalProgressParams{ - MediaItemID: pgMediaUUID, - UserID: pgUserID, - Epubcfi: pgtype.Text{String: epubcfi, Valid: true}, - Chapter: pgtype.Int4{Int32: int32(chapter), Valid: true}, - ChapterProgress: pgtype.Float8{Float64: 0.5, Valid: true}, - DeviceSyncData: pgtype.JSONB{ - Bytes: []byte(fmt.Sprintf(`{"kobo_bookmark_id": "%s", "hidden": %v}`, - bookmarkSync.BookmarkId, bookmarkSync.Hidden)), - Valid: true, - }, - LastSyncDevice: pgtype.Text{String: "kobo", Valid: true}, - LastSyncSource: pgtype.Text{String: "kobo", Valid: true}, - }) - if err != nil { - log.Printf("Failed to store last-read-place: %v", err) - } - bookmarksSynced++ - } -default: - log.Printf("Unknown bookmark type: %s", bookmarkSync.BookmarkType) -} -``` - ---- - -## ๐Ÿ“Š **Phase 3: Test Implementation & Updates** (45 minutes) - -### **Step 7: Add Debug Logging** -**File**: `/cmd/server/tests/kobo_test.go` - -**Add before each test**: -```go -log.Printf("[DEBUG] Kobo test setup: Creating device and media") -log.Printf("[DEBUG] Kobo device: ID=%s, Token=%s", device.ID, device.AuthToken) -``` - -**Add to Markup function**: -```go -log.Printf("[DEBUG] Processing Kobo bookmark: Type=%s, Text=%q, Hidden=%v, ID=%s", - bookmarkSync.BookmarkType, - bookmarkSync.BookmarkText, - bookmarkSync.Hidden, - bookmarkSync.BookmarkId) -``` - ---- - -### **Step 8: Fix TestKoboInitialization** -**File**: `/cmd/server/tests/kobo_test.go` -**Lines**: 27-38 - -**Current Issues**: -1. No device created -2. Wrong route path (`/test-token/` segment) -3. Hardcoded token doesn't exist - -**Fix**: -```go -func TestKoboInitialization(t *testing.T) { - if testing.Short() { - t.Skip("Skipping integration test in short mode") - } - - ts, db, _ := setupTestServer(t) - defer closeTestServer(t, ts, db) - - token := loginTestUser(t, ts, db) - _ = createTestMediaItemID(t, ts, token) - - // Create Kobo device for proper authentication - device := setup.CreateDevice(t, "Test Kobo", "kobo", "kobo-clara-test") - - t.Run("successful initialization", func(t *testing.T) { - req, _ := http.NewRequest("GET", ts.URL+"/api/sync/kobo/v1/initialization", nil) - req.Header.Set("Authorization", "Bearer "+device.AuthToken) - req.Header.Set("x-kobo-device", fmt.Sprintf(`{"DeviceId":"%s","Model":"Kobo Clara","SerialNumber":"%s"}`, - device.ID.String(), device.Identifier)) - - client := &http.Client{} - resp, err := client.Do(req) - require.NoError(t, err) - defer resp.Body.Close() - - assert.Equal(t, http.StatusOK, resp.StatusCode) - }) -} -``` - ---- - -### **Step 9: Fix TestKoboLibrarySync** -**File**: `/cmd/server/tests/kobo_test.go` -**Lines**: 51-62 - -**Fixes**: Same pattern as TestKoboInitialization -```go -func TestKoboLibrarySync(t *testing.T) { - if testing.Short() { - t.Skip("Skipping integration test in short mode") - } - - ts, db, _ := setupTestServer(t) - defer closeTestServer(t, ts, db) - - token := loginTestUser(t, ts, db) - _ = createTestMediaItemID(t, ts, token) - - // Create Kobo device - device := setup.CreateDevice(t, "Test Kobo", "kobo", "kobo-clara-test") - - t.Run("successful library sync", func(t *testing.T) { - req, _ := http.NewRequest("GET", ts.URL+"/api/sync/kobo/v1/initialization", nil) - req.Header.Set("Authorization", "Bearer "+device.AuthToken) - req.Header.Set("x-kobo-device", fmt.Sprintf(`{"DeviceId":"%s","Model":"Kobo Clara","SerialNumber":"%s"}`, - device.ID.String(), device.Identifier)) - - client := &http.Client{} - resp, err := client.Do(req) - require.NoError(t, err) - defer resp.Body.Close() - - assert.Equal(t, http.StatusOK, resp.StatusCode) - }) -} -``` - ---- - -### **Step 10: Fix TestKoboMarkupSync** -**File**: `/cmd/server/tests/kobo_test.go` -**Lines**: 74-118 - -**Add device creation**: -```go -ts, db, _ := setupTestServer(t) -defer closeTestServer(t, ts, db) - -token := loginTestUser(t, ts, db) -mediaItemID := createTestMediaItemID(t, ts, token) - -// Create Kobo device -device := setup.CreateDevice(t, "Test Kobo", "kobo", "kobo-clara-test") - -t.Run("successful markup sync with annotations and bookmarks", func(t *testing.T) { - reqBody := map[string]interface{}{ - "ReadingSync": []map[string]interface{}{ - { - "ContentId": mediaItemID, - "PercentRead": 45.6, - "EntitlementId": "ent-123", - "RemainingTimeMinutes": 120, - "LastModified": "2026-01-30T20:00:00Z", - }, - }, - "BookmarkSync": []map[string]interface{}{ - { - "BookmarkId": "bookmark-1", - "ContentId": mediaItemID, - "BookmarkText": "This is highlighted text", - "BookmarkType": "annotation", - "BookmarkTitle": "Chapter 3", - "DateCreated": "2026-01-30T19:55:00Z", - }, - { - "BookmarkId": "bookmark-2", - "ContentId": mediaItemID, - "BookmarkText": "This is my note about book", - "BookmarkType": "bookmark", - "DateCreated": "2026-01-30T19:55:00Z", - }, - { - "BookmarkId": "epubcfi(/6/4[chap1]!/4/2/1:0)", - "ContentId": mediaItemID, - "BookmarkType": "last-read-place", - "Hidden": true, - "DateCreated": "2026-01-30T19:55:00Z", - }, // NEW: Test last-read-place bookmark! - }, - } - - body, _ := json.Marshal(reqBody) - req, _ := http.NewRequest("POST", ts.URL+"/api/sync/kobo/markup", bytes.NewReader(body)) - req.Header.Set("Content-Type", "application/json") - req.Header.Set("Authorization", "Bearer "+device.AuthToken) - req.Header.Set("x-kobo-device", fmt.Sprintf(`{"DeviceId":"%s","Model":"Kobo Clara","SerialNumber":"%s"}`, - device.ID.String(), device.Identifier)) - - client := &http.Client{} - resp, err := client.Do(req) - require.NoError(t, err) - defer resp.Body.Close() - - assert.Equal(t, http.StatusOK, resp.StatusCode) - var result map[string]interface{} - json.NewDecoder(resp.Body).Decode(&result) - assert.Contains(t, result, "Status") - - // NEW: Verify last-read-place was processed - assert.Contains(t, result, "bookmarks_synced") -}) -``` - ---- - -### **Step 11: Fix TestKoboBookmarkSync** -**File**: `/cmd/server/tests/kobo_test.go` -**Lines**: 130-159 - -**Add device creation and last-read-place test**: -```go -t.Run("successful bookmark sync with last-read-place", func(t *testing.T) { - reqBody := map[string]interface{}{ - "BookmarkSync": []map[string]interface{}{ - { - "BookmarkId": "bookmark-3", - "ContentId": mediaItemID, - "BookmarkText": "Important note about book", - "BookmarkType": "bookmark", - "DateCreated": "2026-01-30T19:55:00Z", - }, - { - "BookmarkId": "epubcfi(/6/4[chap1]!/4/2/1:156)", - "ContentId": mediaItemID, - "BookmarkType": "last-read-place", - "Hidden": true, - "DateCreated": "2026-01-30T19:55:00Z", - }, // NEW: Test precise position bookmark - }, - } - - body, _ := json.Marshal(reqBody) - req, _ := http.NewRequest("POST", ts.URL+"/api/sync/kobo/bookmark", bytes.NewReader(body)) - req.Header.Set("Content-Type", "application/json") - req.Header.Set("Authorization", "Bearer "+device.AuthToken) - req.Header.Set("x-kobo-device", fmt.Sprintf(`{"DeviceId":"%s","Model":"Kobo Clara","SerialNumber":"%s"}`, - device.ID.String(), device.Identifier)) - - client := &http.Client{} - resp, err := client.Do(req) - require.NoError(t, err) - defer resp.Body.Close() - - assert.Equal(t, http.StatusOK, resp.StatusCode) - var result map[string]interface{} - json.NewDecoder(resp.Body).Decode(&result) - assert.Contains(t, result, "bookmarks_synced") -}) -``` - ---- - -### **Step 12: Fix TestKoboAnalyticsGettests** -**File**: `/cmd/server/tests/kobo_test.go` -**Lines**: 172-198 - -**Add device creation**: -```go -ts, db, _ := setupTestServer(t) -defer closeTestServer(t, ts, db) - -token := loginTestUser(t, ts, db) -mediaItemID := createTestMediaItemID(t, ts, token) - -// Create Kobo device -device := setup.CreateDevice(t, "Test Kobo", "kobo", "kobo-clara-test") - -t.Run("successful analytics tests", func(t *testing.T) { - reqBody := map[string]interface{}{ - "meta": map[string]string{ - "name": "Kobo Analytics Tests", - }, - "ContentId": mediaItemID, - "ReadingEvent": "Reading", - "RemainingTimeMin": 180, - "PercentRead": 67.8, - } - - body, _ := json.Marshal(reqBody) - req, _ := http.NewRequest("POST", ts.URL+"/api/sync/kobo/v1/analytics/gettests", bytes.NewReader(body)) - req.Header.Set("Content-Type", "application/json") - req.Header.Set("Authorization", "Bearer "+device.AuthToken) - req.Header.Set("x-kobo-device", fmt.Sprintf(`{"DeviceId":"%s","Model":"Kobo Clara","SerialNumber":"%s"}`, - device.ID.String(), device.Identifier)) - - client := &http.Client{} - resp, err := client.Do(req) - require.NoError(t, err) - defer resp.Body.Close() - - assert.Equal(t, http.StatusOK, resp.StatusCode) - var result map[string]interface{} - json.NewDecoder(resp.Body).Decode(&result) - assert.Contains(t, result, "Status") -}) -``` - ---- - -## ๐Ÿ“Š **Phase 4: Verification** (30 minutes) - -### **Step 13: Critical Bug Verification** -```bash -# Test rate limit headers -curl -H "Authorization: Bearer dev_test123" \ - -H "x-kobo-device: {\"DeviceId\":\"test\",\"Model\":\"test\"}" \ - http://localhost:8765/api/sync/kobo/markup | \ - -i "X-RateLimit-Remaining" - -# Test device last_seen updates -podman exec bookhoard_db psql -U postgres -d bookhoard -c \ - "SELECT device_name, last_seen FROM devices WHERE device_type = 'kobo' ORDER BY last_seen DESC LIMIT 5" -``` - -### **Step 14: Full Test Suite** -```bash -make test-integration > test-results.txt 2>&1 -grep -E "(RUN|PASS|FAIL).*TestKobo" test-results.txt -``` - -### **Step 15: Debug Log Analysis** -```bash -podman compose logs app | grep -i "last-read-place" | tail -10 -podman compose logs app | grep -i "kobo.*bookmark" | tail -20 -``` - ---- - -## ๐Ÿ“ **Documentation Updates Required** - -### **1. API Documentation Updates** - -**File**: `/docs/developer/api/kobo/markup_sync.md` - -**Add to "Bookmark Types" section**: -```markdown -#### Bookmark Types - -| Type | Description | Storage | -|-------|-------------|----------| -| "annotation" | Highlighted text with optional notes | `media_highlights` table | -| "bookmark" | User-created bookmarks with notes | `media_notes` table | -| "last-read-place" | Auto-generated position marker | `reading_progress` table (position fields) | -``` - -**Add to "Response Format" section**: -```markdown -#### Response Example - -```json -{ - "bookmarks_synced": 3, - "highlights_synced": 1, - "notes_synced": 1, - "positions_updated": 1 -} -``` - -### **File**: `/docs/developer/api/kobo/bookmark_sync.md` - -**Add similar Bookmark Type documentation** - ---- - -### **2. Bruno API Test Updates** - -**File**: `/bruno/devices/Kobo Bookmark Sync.bru` - -**Add new request example**: -```json -{ - "name": "Kobo Bookmark Sync with Last-Read-Place", - "method": "POST", - "url": "{{baseUrl}}/api/sync/kobo/bookmark", - "headers": { - "Content-Type": "application/json", - "Authorization": "Bearer {{deviceAuthToken}}", - "x-kobo-device": "{{koboDeviceHeader}}" - }, - "body": { - "BookmarkSync": [ - { - "BookmarkId": "epubcfi(/6/4[chap1]!/4/2/1:156)", - "ContentId": "{{mediaItemId}}", - "BookmarkType": "last-read-place", - "Hidden": true, - "DateCreated": "2026-01-30T19:55:00Z" - } - ] - }, - "tests": { - "shouldProcessPositionBookmarks": { - "status": 200, - "body": { - "bookmarks_synced": 1 - } - } - } - } -} -``` - ---- - -### **3. New Bruno Test File** - -**Create**: `/bruno/devices/Kobo Last-Read-Place Sync.bru` - -**Content**: -```json -{ - "name": "Kobo Last-Read-Place Position Sync", - "method": "POST", - "url": "{{baseUrl}}/api/sync/kobo/markup", - "headers": { - "Content-Type": "application/json", - "Authorization": "Bearer {{deviceAuthToken}}", - "x-kobo-device": "{{koboDeviceHeader}}" - }, - "body": { - "ReadingSync": [ - { - "ContentId": "{{mediaItemId}}", - "PercentRead": 45.6, - "LastModified": "2026-01-30T20:00:00Z" - } - ], - "BookmarkSync": [ - { - "BookmarkId": "epubcfi(/6/4[chap1]!/4/2/1:0)", - "ContentId": "{{mediaItemId}}", - "BookmarkType": "last-read-place", - "Hidden": true, - "DateCreated": "2026-01-30T19:55:00Z" - } - ] - }, - "tests": { - "shouldSyncPrecisePosition": { - "status": 200, - "body": { - "bookmarks_synced": 1, - "positions_updated": 1 - } - }, - "checkProgressUpdate": { - "status": 200, - "body": { - "percentage": 0.456, - "epubcfi": "epubcfi(/6/4[chap1]!/4/2/1:0)" - } - } - } - } -} -``` - ---- - -## ๐Ÿ“ **Phase 5: Git Commits** (15 minutes) - -### **Commit 1**: Fix device auth string conversion -``` -fix(middleware): Correct rate limit header type conversion - -- Fix string(rune(remaining)) to strconv.Itoa(remaining) in device_auth.go -- Prevents garbage characters in X-RateLimit-Remaining header -- No functionality changes, only fixes broken headers - -Testing: Verified with curl that headers return proper integers -Fixes: #XXX -``` - -### **Commit 2**: Fix device auth type assertion -``` -fix(middleware): Enable device last_seen timestamp updates - -- Fix type assertion from uuid.UUID to [16]byte in UpdateDeviceLastSeen -- UpdateDeviceLastSeen now executes correctly after device authentication -- Devices will now track last activity timestamp for conflict detection - -Testing: Confirmed last_seen field updates in database after device requests -Fixes: #XXX -``` - -### **Commit 3**: Remove redundant user_id context -``` -fix(middleware): Remove redundant user_id context setting - -- Removed redundant c.Set("user_id", device.UserID.Bytes) from device auth -- Device authentication provides device context, no need for JWT user_id field -- No handlers use device-sourced user_id, simplifying authentication flow - -Testing: Verified KOReader and Kobo sync still work correctly -Fixes: #XXX -``` - -### **Commit 4**: Fix Kobo nil UUID handling -``` -fix(handlers): Add nil UUID checks in Kobo sync handlers - -- Check for uuid.Nil after mapContentIdToBookhoardUUID in multiple locations -- Skip database operations with invalid UUIDs to prevent errors -- Improves error handling robustness across all Kobo sync functions - -Testing: Confirmed nil UUIDs are properly skipped without database errors -Fixes: #XXX -``` - -### **Commit 5**: Add ContentType detection for Kobo -``` -feat(handlers): Add ContentType detection for Kobo EPUB/PDF sync - -- Add ContentType field to KoboReadingSync struct for dynamic content type -- Map PDF mime types to ContentType "5" for proper device rendering -- Maintains backward compatibility with existing EPUB default "6" - -Testing: Verified EPUBs return "6", PDFs return "5" in responses -Fixes: #XXX -``` - -### **Commit 6**: Add "last-read-place" bookmark support for Kobo -``` -feat(handlers): Add Kobo "last-read-place" bookmark support - -- Add handling for "last-read-place" bookmark type in Kobo bookmark sync -- Extract precise EPUB CFI position from BookmarkId field -- Update reading_progress table with exact location and chapter data -- Set Hidden=true to exclude position bookmarks from UI lists -- Enables Kobo users to resume reading at precise paragraph location - -Testing: Confirmed position data stored in reading_progress.epubcfi field -Fixes: #XXX -``` - -### **Commit 7**: Update Kobo API documentation -``` -docs(kobo): Update bookmark sync API documentation - -- Document "last-read-place" bookmark type in bookmark sync endpoints -- Add examples of position-based bookmarks vs user annotations -- Update response format to include position update confirmation -- Clarify Hidden flag usage for auto-generated bookmarks - -Fixes: #XXX -``` - -### **Commit 8**: Update Kobo Bruno API tests -``` -test(bruno): Add comprehensive Kobo bookmark sync examples - -- Add example for "last-read-place" bookmark sync in Kobo Bookmark Sync.bru -- Create new test file Kobo Last-Read-Place Sync.bru for position validation -- Include device authentication headers and proper request structure -- Add tests for position bookmark processing and response validation - -Testing: All Bruno examples work with updated API -Fixes: #XXX -``` - -### **Commit 9**: Fix Kobo integration tests -``` -test(kobo): Fix failing Kobo integration tests - -- Create Kobo devices using CreateDevice helper for proper authentication -- Use device auth tokens instead of JWT/hardcoded tokens -- Fix route path in TestKoboInitialization (remove /test-token/ segment) -- Add Authorization: Bearer headers to all Kobo requests -- Add x-kobo-device headers with proper device metadata -- Add test case for "last-read-place" bookmark processing -- All 5 Kobo tests now pass: TestKoboInitialization, TestKoboLibrarySync, - TestKoboMarkupSync, TestKoboBookmarkSync, TestKoboAnalyticsGettests - -Testing: make test-integration shows all Kobo tests PASS -Fixes: #XXX -``` - ---- - -## โœ… **Success Criteria** - -### **Phase 1 Success**: -- [ ] Rate limit headers show correct integers (verified with curl) -- [ ] Device last_seen timestamps update in database -- [ ] No panics in error scenarios -- [ ] Kobo nil UUIDs properly handled -- [ ] ContentType varies by mime type (EPUB=6, PDF=5) -- [ ] No regressions in existing tests - -### **Phase 2 Success**: -- [ ] "last-read-place" bookmarks processed and stored in reading_progress -- [ ] EPUB CFI position data extracted correctly -- [ ] Position bookmarks excluded from UI (Hidden=true) -- [ ] Debug logs confirm bookmark processing -- [ ] All 5 Kobo tests pass -- [ ] Tests include realistic "last-read-place" payloads - -### **Phase 3 Success**: -- [ ] Full integration test suite runs successfully -- [ ] Git history shows clean, logical commits -- [ ] Documentation updated and renders at /docs endpoint -- [ ] Bruno tests include new functionality -- [ ] Real-world Kobo device behavior patterns supported - -### **Phase 4 Success**: -- [ ] All 7 commits are independent and can be reverted if needed -- [ ] Each commit builds and tests successfully -- [ ] No critical functionality was broken -- [ ] Git diff shows only intended changes -- [ ] All modified files compile successfully - ---- - -## ๐Ÿš€ **Total Estimated Time: ~3 hours** - ---- - -## ๐ŸŽฏ **Ready to Execute** - -This plan is **fully compliant** with PROJECT_GUIDELINES.md: -- Uses Podman/Docker for all builds -- Follows KISS/DRY/YAGNI principles -- No breaking changes without verification -- Step-by-step implementation with testing -- Clear, logical git commits -- Proper error recovery procedures -- Documentation updates included - -**All 9 commits are independent and can be reverted if needed.** - -**Proceed with implementation?** \ No newline at end of file diff --git a/REFINED_SCHEMA_PLAN.md b/REFINED_SCHEMA_PLAN.md index 6784928..205da4d 100644 --- a/REFINED_SCHEMA_PLAN.md +++ b/REFINED_SCHEMA_PLAN.md @@ -16,11 +16,15 @@ ## ๐Ÿ“Š Technical Decisions -### 1. File Structure: Single Cohesive Package +### 1. File Structure: Single Source of Truth -**Decision:** Single `internal/database/schema/schema.go` file containing all schema logic. +**Decision:** Single `internal/database/schema.go` file referencing existing `database/schema/schema.sql`. -**Rationale:** Follows Go ecosystem standards and PostgreSQL/pgx patterns. +**Rationale:** +- **No duplication**: Single schema file remains the source of truth +- **Clear separation**: `/database/` for schema definitions, `/internal/database/` for Go logic +- **Maintainability**: Only one schema file to maintain +- **Follows existing patterns**: Database definitions stay where they belong ### 2. Function Location: All in One Package @@ -71,44 +75,69 @@ ## ๐Ÿ“ Implementation File Structure ``` -internal/database/schema/ +internal/database/ โ”œโ”€โ”€ schema.go # Main initialization logic -โ”œโ”€โ”€ schema.sql # Embedded idempotent schema copy -โ””โ”€โ”€ verification.go # REMOVED - consolidated into schema.go +โ”œโ”€โ”€ db.go # Existing sqlc-generated code +โ”œโ”€โ”€ models.go # Existing sqlc-generated models +โ”œโ”€โ”€ queries.sql.go # Existing sqlc-generated queries +โ””โ”€โ”€ ... + +database/ +โ””โ”€โ”€ schema/ + โ””โ”€โ”€ schema.sql # Existing single source of truth schema ``` -**Single Responsibility:** `schema.go` handles all database initialization logic. +**Single Responsibility:** `schema.go` handles all database initialization logic, referencing the existing schema file. --- ## ๐Ÿ”ง Implementation Plan -### Phase 1: Cleanup (Start Fresh) +### Phase 1: Schema File Verification -#### Step 1.1: Remove Duplicate Files +#### Step 1.1: Ensure Schema is Idempotent ```bash -rm -f /home/nymusicman/Code/bookhoard/internal/database/schema/verification.go +# Convert remaining statements to idempotent form +# Current state: 9/27 tables, 20/82 indexes, 1/3 inserts are idempotent + +# Tables needing IF NOT EXISTS (18): +# - library_types, users, system_settings, refresh_tokens, libraries +# - library_folders, library_visibility, media_items, reading_progress +# - media_ratings, media_notes, media_highlights, devices, sync_queue +# - sync_conflicts, kobo_shelves, kobo_entitlements, reading_history + +# Indexes: Convert remaining 62 CREATE INDEX to CREATE INDEX IF NOT EXISTS + +# Inserts needing ON CONFLICT (2): +# - Line 14: library_types (name) +# - Line 44: system_settings (setting_key) ``` -#### Step 1.2: Clean Up Schema File +#### Step 1.2: Verify Schema Syntax ```bash -# Ensure schema.sql is fully idempotent -grep "CREATE TABLE IF NOT EXISTS" database/schema/schema.sql | wc -l # Should be 36 -grep "CREATE INDEX IF NOT EXISTS" database/schema/schema.sql | wc -l # Should be 102 -grep "ON CONFLICT" database/schema/schema.sql | wc -l # Should be 3 +# Test schema.sql syntax validity +psql -h localhost -U postgres -d postgres -f database/schema/schema.sql --echo-errors --quiet +``` + +#### Step 1.3: Update Local Database +```bash +# CRITICAL: This is pre-production, update local DB after schema.sql changes +podman compose down -v # WARNING: loses all data +podman compose up -d ``` --- -### Phase 2: Create Single Schema Package +### Phase 2: Create Schema Initialization Logic -#### Step 2.1: Create `internal/database/schema/schema.go` +#### Step 2.1: Create `internal/database/schema.go` **Implementation Strategy:** ```go -package schema +package database import ( + "bufio" "context" "embed" "fmt" @@ -121,13 +150,13 @@ import ( "github.com/jackc/pgx/v5/pgxpool" ) -//go:embed schema.sql +//go:embed ../../database/schema/schema.sql var SchemaFile string const ( // Pre-computed FNV-1a hash of "bookhoard:schema:init" - // Generated using: generateLockID("bookhoard:schema:init") = 582394759234 - schemaInitLockID = 582394759234 + // Generated using: generateLockID("bookhoard:schema:init") + schemaInitLockID = 7804706162000639061 ) // Hash calculation function (for reference/testing) @@ -301,15 +330,14 @@ func Initialize(ctx context.Context, db *pgxpool.Pool) error { } ``` -#### Step 2.2: Copy schema.sql to Package +#### Step 2.2: Verify Embed Path ```bash -cp database/schema/schema.sql internal/database/schema/schema.sql -``` - -**Verification:** -```bash -diff database/schema/schema.sql internal/database/schema/schema.sql -# Should produce no output +# Test that the embed path resolves correctly +cd /home/nymusicman/Code/bookhoard/internal/database +go test -c -o /tmp/test_embed . +# Or simply verify the relative path exists: +ls -la ../../database/schema/schema.sql +# Should show the file exists at the correct relative location ``` --- @@ -324,7 +352,7 @@ diff database/schema/schema.sql internal/database/schema/schema.sql ```go import ( // ... existing imports ... - "bookhoard/internal/database/schema" + "bookhoard/internal/database" ) func main() { @@ -341,7 +369,7 @@ func main() { // ===== NEW: Schema Initialization ===== log.Println("๐Ÿ”ง Ensuring database schema is initialized...") ctx := context.Background() - if err := schema.Initialize(ctx, dbPool); err != nil { + if err := database.Initialize(ctx, dbPool); err != nil { log.Fatal("โŒ Database schema initialization failed:", err) } log.Println("โœ… Database schema initialized and verified, starting server...") @@ -358,25 +386,27 @@ func main() { ### Before Declaring Complete **Phase 1 Verification:** -- [ ] Cleaned up duplicate verification.go file -- [ ] Verified all CREATE TABLE use IF NOT EXISTS (36 total) -- [ ] Verified all CREATE INDEX use IF NOT EXISTS (102 total) -- [ ] Verified all INSERT have ON CONFLICT (3 total) +- [ ] Converted 18 CREATE TABLE โ†’ CREATE TABLE IF NOT EXISTS (27 total) +- [ ] Converted 62 CREATE INDEX โ†’ CREATE INDEX IF NOT EXISTS (82 total) +- [ ] Added ON CONFLICT to 2 INSERT statements (3 total) - [ ] Verified schema.sql syntax is valid +- [ ] Confirmed single schema.sql exists as source of truth **Phase 2 Verification:** -- [ ] `internal/database/schema/schema.go` compiles without errors -- [ ] Single cohesive package with all related functions -- [ ] Imports only what's used (embed, pgx v5) +- [ ] `internal/database/schema.go` compiles without errors +- [ ] Single file with all schema initialization functions +- [ ] Imports only what's used (bufio, embed, pgx v5) - [ ] Uses *pgxpool.Pool concrete type -- [ ] Pre-computed constant for lock ID (582394759234) +- [ ] Pre-computed constant for lock ID (7804706162000639061) - [ ] Stream scanning implementation for performance -- [ ] schema.sql successfully embedded and copied +- [ ] Successfully embeds existing schema.sql file +- [ ] No schema file duplication (single source of truth) **Phase 3 Verification:** - [ ] `cmd/server/main.go` compiles without errors - [ ] Schema initialization call added before handler creation - [ ] Proper error handling with Fatal on failure +- [ ] Uses correct import path (`database.Initialize`) **Integration Testing:** - [ ] Fresh database initializes correctly @@ -425,18 +455,12 @@ func main() { If critical errors occur: -### Option 1: Restore Single File +### Option 1: Restore Schema File ```bash -git checkout HEAD -- internal/database/schema/schema.go +git checkout HEAD -- internal/database/schema.go ``` -### Option 2: Restore Package -```bash -rm -rf internal/database/schema -git checkout HEAD -- internal/database/schema/ -``` - -### Option 3: Restore Integration +### Option 2: Restore Integration ```bash git checkout HEAD -- cmd/server/main.go ``` @@ -458,7 +482,7 @@ The app will initialize the database automatically on first startup. **Monitor logs for:** ``` -โœ… Database schema initialized and verified (36 tables verified) +โœ… Database schema initialized and verified (27 tables verified) ``` ### Subsequent Deployments @@ -494,7 +518,7 @@ podman exec bookhoard_db psql -U postgres -d bookhoard -c "\dt" ### Check advisory locks: ```bash podman exec bookhoard_db psql -U postgres -d bookhoard -c " - SELECT * FROM pg_locks WHERE objid = 582394759234; + SELECT * FROM pg_locks WHERE objid = 7804706162000639061; " ``` @@ -506,4 +530,12 @@ podman compose up -d --- +## ๐Ÿ“š Documentation + +Create documentation explaining the automatic schema initialization: +- **docs/contributing/database-schema.md** - How schema initialization works, how to modify schema safely +- **README.md** - Add "Database Initialization" section documenting first-run behavior + +--- + **End of Refined Implementation Plan** diff --git a/SCHEMA_INITIALIZATION_PLAN.md b/SCHEMA_INITIALIZATION_PLAN.md deleted file mode 100644 index a821f1e..0000000 --- a/SCHEMA_INITIALIZATION_PLAN.md +++ /dev/null @@ -1,1131 +0,0 @@ -# Implementation Plan: Idempotent Database Schema Initialization - -## ๐ŸŽฏ Executive Summary - -**Problem:** Application starts before Docker init scripts complete, causing race conditions where `library_types` table doesn't exist when the app tries to create libraries. - -**Solution:** Make entire schema.sql idempotent and run it on every application startup with: -- PostgreSQL advisory locking (prevents concurrent execution) -- Single atomic transaction (all-or-nothing execution) -- Post-execution verification (confirms all tables exist) -- Tiered error logging (DEBUG โ†’ ERROR โ†’ FATAL summary) - -**Impact:** -- โœ… Self-healing database initialization -- โœ… Works in production, development, and testing -- โœ… Horizontal-scale safe with advisory locks -- โœ… No dependency on Docker init scripts -- โœ… Clear error messages for debugging - ---- - -## ๐Ÿ“Š Change Overview - -### Schema Conversions Required -- **27** `CREATE TABLE` โ†’ `CREATE TABLE IF NOT EXISTS` -- **82** `CREATE INDEX` โ†’ `CREATE INDEX IF NOT EXISTS` -- **3** `INSERT INTO` โ†’ Add `ON CONFLICT (...) DO NOTHING` -- **0** `ALTER TABLE` โ†’ Already idempotent โœ“ - -### New Files Created -1. `internal/database/schema/schema.go` - Main initialization logic -2. `internal/database/schema/schema.sql` - Embedded idempotent schema copy -3. `internal/database/schema/verification.go` - Table existence checker - -### Files Modified -1. `database/schema/schema.sql` - Convert to idempotent statements -2. `cmd/server/main.go` - Add schema initialization call - ---- - -## ๐Ÿ›ก๏ธ Safety Measures - -### Pre-Change Checklist (per PROJECT_GUIDELINES.md) -- โœ… Read current schema completely (959 lines) -- โœ… Identify all columns that must be preserved (none - adding safety clauses only) -- โœ… Plan exact changes needed (converting to idempotent forms) -- โœ… Set up verification step (schema parser + table existence check) -- โœ… Will verify by reading back after each major section - -### Backup Strategy -```bash -# Before starting, create backup branch -git branch backup-before-schema-idempotent - -# If mistakes occur, recovery protocol: -# 1. STOP - don't make more edits -# 2. git diff to see exact changes -# 3. git checkout HEAD -- database/schema/schema.sql if needed -# 4. Verify with go build -``` - -### Post-Edit Verification -- After each file edit: `go build ./internal/database/...` -- After schema.sql changes: Verify SQL syntax is valid -- Before committing: `bash scripts/verify-guidelines.sh` -- Before declaring complete: Full test suite passes - ---- - -## ๐Ÿ”ง Technical Decisions - -### 1. Advisory Lock ID: Hash-Based - -**Decision:** Use FNV-1a hash of semantic key - -```go -func generateLockID(key string) int64 { - hash := uint64(14695981039346656037) // FNV offset basis - for _, c := range key { - hash ^= uint64(c) - hash *= 1099511628211 // FNV prime - } - return int64(hash) -} - -const schemaInitLockID = generateLockID("bookhoard:schema:init") -``` - -**Rationale:** -- Semantic meaning ("this lock is for Bookhoard's schema initialization") -- Deterministic (same key always produces same ID: 582394759234) -- Low collision risk (64-bit hash space) -- Clear intent in code - -### 2. Error Log Verbosity: Tiered - -**Decision:** Three-tier logging system - -```go -// Tier 1: Detailed SQL errors (for debugging) -log.Printf("DEBUG: SQL Error at line %d: %v", lineNum, sqlErr) -log.Printf("DEBUG: Statement: %s", statement) - -// Tier 2: Contextual error (for developers) -log.Printf("ERROR: Schema initialization failed at step %q: %v", stepName, err) - -// Tier 3: Actionable summary (for everyone) -log.Fatalf("FATAL: Database schema initialization failed. Run 'podman logs bookhoard_app' for details.") -``` - -**Example Output:** -``` -DEBUG: SQL Error at line 45: relation "library_types" does not exist -DEBUG: Statement: CREATE TABLE IF NOT EXISTS library_types... -ERROR: Schema initialization failed at step "create_base_tables": relation "library_types" does not exist -FATAL: Database schema initialization failed. Run 'podman logs bookhoard_app' for details. -``` - -### 3. Verification Timing: After Execution - -**Decision:** Verify tables exist AFTER schema execution - -**Rationale:** -- Detects partial state from crashes -- Self-healing (idempotent schema fixes partial state) -- Confirms ALL expected tables exist, not just critical ones -- Provides clear error messages - -**With crash scenario:** -``` -Instance A: Creates library_types โ†’ crashes -Instance B: Runs schema (CREATE IF NOT EXISTS safe) โ†’ completes โ†’ verifies โœ… -``` - -### 4. Transaction Scope: Single Giant Transaction - -**Decision:** Entire schema.sql in one transaction - -**Rationale:** -- Startup is not performance-critical (500ms-1s acceptable) -- All-or-nothing execution (cleanest failure mode) -- No one uses app during startup (won't block queries) -- Idempotent statements make retry safe -- Verification catches failures before server starts - ---- - -## ๐Ÿ“‹ Implementation Phases - -### Phase 1: Backup & Preparation - -#### Step 1.1: Create backup branch -```bash -git branch backup-before-schema-idempotent -``` - -#### Step 1.2: Create new package directory -```bash -mkdir -p internal/database/schema -``` - ---- - -### Phase 2: Convert schema.sql to Idempotent - -#### Section 2.1: Convert CREATE TABLE statements - -**Lines affected (27 total):** -- Line 5: `library_types` -- Line 20: `users` -- Line 35: `system_settings` -- Line 49: `refresh_tokens` -- Line 59: `libraries` -- Line 70: `library_folders` -- Line 79: `library_visibility` -- Line 90: `media_items` -- Line 147: `reading_progress` -- Line 177: `media_ratings` -- Line 188: `media_notes` -- Line 207: `media_highlights` -- Line 235: `devices` -- Line 255: `sync_queue` -- Line 273: `sync_conflicts` -- Line 289: `kobo_shelves` -- Line 307: `kobo_entitlements` -- Line 332: `reading_history` - -**Pattern:** -```sql --- BEFORE: -CREATE TABLE table_name ( - --- AFTER: -CREATE TABLE IF NOT EXISTS table_name ( -``` - -**Verification:** -```bash -grep "CREATE TABLE IF NOT EXISTS" database/schema/schema.sql | wc -l -# Should show 36 (27 new + 9 already existing) -``` - -#### Section 2.2: Convert CREATE INDEX statements - -**Lines affected:** Approximately 82 index statements - -**Pattern:** -```sql --- BEFORE: -CREATE INDEX index_name ON table_name( - --- AFTER: -CREATE INDEX IF NOT EXISTS index_name ON table_name( -``` - -**Verification:** -```bash -grep "CREATE INDEX IF NOT EXISTS" database/schema/schema.sql | wc -l -# Should show 102 (82 new + 20 already existing) -``` - -#### Section 2.3: Add ON CONFLICT to INSERT statements - -**Line 14 - library_types:** -```sql -INSERT INTO library_types (name, description, allowed_extensions) VALUES -('ebooks', 'Ebook files including EPUB, PDF, MOBI, etc.', ARRAY['.epub', '.pdf', '.mobi', '.azw', '.azw3', '.txt', '.rtf', '.doc', '.docx', '.lit', '.fb2', '.pdb']), -('comics', 'Comic book archives and image formats', ARRAY['.cbz', '.cbr', '.cb7', '.cbt', '.pdf']), -('manga', 'Manga files including archives and image folders', ARRAY['.cbz', '.cbr', '.png', '.jpg', '.jpeg', '.gif', '.bmp', '.webp']) -ON CONFLICT (name) DO NOTHING; -``` - -**Line 44 - system_settings:** -```sql -INSERT INTO system_settings (setting_key, setting_value, description) VALUES -('scan_frequency_minutes', '60', 'How often to scan all libraries in minutes'), -('auto_scan_enabled', 'true', 'Whether auto-scanning is enabled system-wide') -ON CONFLICT (setting_key) DO NOTHING; -``` - -**Line 913 - system_config:** -```sql -# Already has ON CONFLICT - verify correct -INSERT INTO system_config (key, value) VALUES -('base_url', 'https://bookhoard.example.com'), -('opds_base_url', 'https://bookhoard.example.com/opds'), -('api_base_url', 'https://bookhoard.example.com/api') -ON CONFLICT (key) DO NOTHING; -``` - -**Verification:** -```bash -grep -A 10 "^INSERT INTO" database/schema/schema.sql | grep -c "ON CONFLICT" -# Should show 3 -``` - -#### Section 2.4: Final verification - -```bash -# Confirm all statements are idempotent -grep "^CREATE TABLE " database/schema/schema.sql | grep -v "IF NOT EXISTS" | wc -l -# Should be 0 - -grep "^CREATE INDEX " database/schema/schema.sql | grep -v "IF NOT EXISTS" | wc -l -# Should be 0 -``` - ---- - -### Phase 3: Create Schema Runner Package - -#### Step 3.1: Create `internal/database/schema/schema.go` - -```go -package schema - -import ( - "context" - "embed" - "fmt" - "log" - "regexp" - "strings" - - "github.com/jackc/pgx/v5" -) - -//go:embed schema.sql -var SchemaFile string - -// generateLockID creates a deterministic 64-bit hash from a string key -// Using FNV-1a hash algorithm for fast, low-collision hashing -func generateLockID(key string) int64 { - hash := uint64(14695981039346656037) // FNV offset basis - for _, c := range key { - hash ^= uint64(c) - hash *= 1099511628211 // FNV prime - } - return int64(hash) -} - -const ( - // PostgreSQL advisory lock ID for schema initialization - // Generated from "bookhoard:schema:init" using FNV-1a hash - schemaInitLockID = generateLockID("bookhoard:schema:init") -) - -// Initialize ensures the database schema is up-to-date -// Runs idempotently on every startup with paranoid verification -func Initialize(ctx context.Context, db DBTX) error { - log.Println("๐Ÿ” Acquiring PostgreSQL advisory lock for schema initialization...") - - // Get database connection - conn, err := db.Acquire(ctx) - if err != nil { - return fmt.Errorf("failed to acquire database connection: %w", err) - } - defer conn.Release() - - // Get advisory lock (blocks other instances) - log.Printf("DEBUG: Attempting to acquire advisory lock %d...", schemaInitLockID) - var lockAcquired bool - err = conn.QueryRow(ctx, "SELECT pg_try_advisory_lock($1)", schemaInitLockID).Scan(&lockAcquired) - if err != nil { - return fmt.Errorf("failed to acquire advisory lock: %w", err) - } - - if !lockAcquired { - log.Println("โณ Another instance is initializing schema, waiting...") - // Use pg_advisory_lock instead (blocks until available) - _, err = conn.Exec(ctx, "SELECT pg_advisory_lock($1)", schemaInitLockID) - if err != nil { - return fmt.Errorf("failed to wait for advisory lock: %w", err) - } - } - log.Println("โœ… Advisory lock acquired") - - defer func() { - // Release lock when done - _, err = conn.Exec(ctx, "SELECT pg_advisory_unlock($1)", schemaInitLockID) - if err != nil { - log.Printf("WARNING: Failed to release advisory lock: %v", err) - } else { - log.Println("๐Ÿ”“ Advisory lock released") - } - }() - - // Parse schema.sql to extract expected table names - log.Println("๐Ÿ“‹ Parsing schema.sql for expected tables...") - expectedTables, err := parseTableNames() - if err != nil { - return fmt.Errorf("failed to parse schema.sql: %w", err) - } - log.Printf("DEBUG: Found %d expected tables in schema.sql", len(expectedTables)) - - // Execute schema in a single transaction - log.Println("๐Ÿ”ง Executing schema.sql in transaction...") - err = executeSchema(ctx, db) - if err != nil { - log.Printf("ERROR: Schema execution failed: %v", err) - return fmt.Errorf("schema execution failed: %w", err) - } - log.Println("โœ… Schema executed successfully") - - // Verify all expected tables exist - log.Println("๐Ÿ” Verifying all expected tables exist...") - err = verifyTables(ctx, db, expectedTables) - if err != nil { - log.Printf("ERROR: Schema verification failed: %v", err) - return fmt.Errorf("schema verification failed: %w", err) - } - log.Println("โœ… All expected tables verified") - - log.Printf("โœ… Database schema initialization complete (%d tables verified)", len(expectedTables)) - return nil -} - -// DBTX is the interface database transactions must implement -type DBTX interface { - Exec(ctx context.Context, sql string, args ...interface{}) (pgconn.CommandTag, error) - Query(ctx context.Context, sql string, args ...interface{}) (pgx.Rows, error) - QueryRow(ctx context.Context, sql string, args ...interface{}) pgx.Row -} - -// executeSchema runs the entire schema.sql in a single transaction -func executeSchema(ctx context.Context, db DBTX) error { - // Start transaction - tx, err := db.Begin(ctx) - if err != nil { - return fmt.Errorf("failed to start transaction: %w", err) - } - defer tx.Rollback(ctx) - - // Execute schema.sql - _, err = tx.Exec(ctx, SchemaFile) - if err != nil { - // Provide detailed error information - return fmt.Errorf("schema execution failed: %w", err) - } - - // Commit transaction - if err := tx.Commit(ctx); err != nil { - return fmt.Errorf("failed to commit schema transaction: %w", err) - } - - return nil -} -``` - -#### Step 3.2: Create `internal/database/schema/verification.go` - -```go -package schema - -import ( - "context" - "fmt" - "regexp" - "strings" -) - -// parseTableNames extracts all table names from CREATE TABLE statements -func parseTableNames() ([]string, error) { - // Regex to match: CREATE TABLE IF NOT EXISTS table_name or CREATE TABLE IF NOT EXISTS schema.table_name - pattern := regexp.MustCompile(`CREATE TABLE IF NOT EXISTS (?:\w+\.)?(\w+)`) - - matches := pattern.FindAllStringSubmatch(SchemaFile, -1) - - tableMap := make(map[string]bool) - for _, match := range matches { - if len(match) > 1 { - tableName := match[1] - tableMap[tableName] = true - } - } - - // Convert map to slice - tables := make([]string, 0, len(tableMap)) - for table := range tableMap { - tables = append(tables, table) - } - - return tables, nil -} - -// verifyTables checks all expected tables exist in database -func verifyTables(ctx context.Context, db DBTX, expectedTables []string) error { - // Query information_schema for existing tables - rows, err := db.Query(ctx, ` - SELECT table_name - FROM information_schema.tables - WHERE table_schema = 'public' - AND table_type = 'BASE TABLE' - `) - if err != nil { - return fmt.Errorf("failed to query existing tables: %w", err) - } - defer rows.Close() - - // Build set of existing tables - existingTables := make(map[string]bool) - for rows.Next() { - var tableName string - if err := rows.Scan(&tableName); err != nil { - return fmt.Errorf("failed to scan table name: %w", err) - } - existingTables[tableName] = true - } - - if err := rows.Err(); err != nil { - return fmt.Errorf("error iterating tables: %w", err) - } - - // Check all expected tables exist - var missing []string - for _, expected := range expectedTables { - if !existingTables[expected] { - missing = append(missing, expected) - } - } - - if len(missing) > 0 { - return fmt.Errorf("missing tables: %s", strings.Join(missing, ", ")) - } - - return nil -} -``` - -#### Step 3.3: Copy schema.sql to package - -```bash -cp database/schema/schema.sql internal/database/schema/schema.sql -``` - -**Verification:** -```bash -# Ensure files are identical -diff database/schema/schema.sql internal/database/schema/schema.sql -# Should produce no output -``` - ---- - -### Phase 4: Integrate into main.go - -**File:** `/home/nymusicman/Code/bookhoard/cmd/server/main.go` - -**Location:** After database connection establishment, before handler creation - -**Code to add:** - -```go -import ( - // ... existing imports ... - "bookhoard/internal/database/schema" -) - -func main() { - // ... existing config loading ... - - dbPool, err := pgxpool.New(context.Background(), cfg.DatabaseURL()) - if err != nil { - log.Fatal("Failed to connect to database:", err) - } - defer dbPool.Close() - - queries := database.New(dbPool) - - // ===== NEW: Schema Initialization ===== - log.Println("๐Ÿ”ง Ensuring database schema is initialized...") - ctx := context.Background() - if err := schema.Initialize(ctx, dbPool); err != nil { - log.Fatal("โŒ Database schema initialization failed:", err) - } - log.Println("โœ… Database schema initialized and verified, starting server...") - // ===== END NEW ===== - - // ... continue with existing startup code ... -} -``` - ---- - -## ๐Ÿงช Testing Strategy - -### Test 1: Fresh Database Initialization - -**Purpose:** Verify app starts with empty database - -**Steps:** -```bash -# Delete all volumes (fresh start) -podman compose down -v - -# Start containers -podman compose up -d db app - -# Check logs -podman logs bookhoard_app | grep -E "schema|Schema|initializ" - -# Expected output: -# ๐Ÿ” Acquiring PostgreSQL advisory lock for schema initialization... -# โœ… Advisory lock acquired -# ๐Ÿ“‹ Parsing schema.sql for expected tables... -# DEBUG: Found 36 expected tables in schema.sql -# ๐Ÿ”ง Executing schema.sql in transaction... -# โœ… Schema executed successfully -# ๐Ÿ” Verifying all expected tables exist... -# โœ… All expected tables verified -# โœ… Database schema initialization complete (36 tables verified) -# โœ… Database schema initialized and verified, starting server... -``` - -**Success criteria:** -- All log messages appear -- App starts successfully -- All tables created in database -- TestKoboInitialization passes - -### Test 2: Existing Database (Already Initialized) - -**Purpose:** Verify re-running is safe - -**Steps:** -```bash -# App is already running from Test 1 -podman compose restart app - -# Check logs -podman logs bookhoard_app | grep -E "schema|Schema|initializ" - -# Expected output: -# Same as Test 1, but execution should be faster (CREATE IF NOT EXISTS skips existing tables) -``` - -**Success criteria:** -- No errors -- App starts successfully -- No duplicate data -- Tables remain intact - -### Test 3: Concurrent Startup (Horizontal Scaling) - -**Purpose:** Verify advisory lock prevents race conditions - -**Steps:** -```bash -# Delete volumes -podman compose down -v - -# Start multiple app instances simultaneously -podman compose up -d --scale app=3 - -# Check logs for all instances -for i in 1 2 3; do - echo "=== Instance $i ===" - podman logs bookhoard_app-$i | grep -E "Advisory lock|acquir|Schema" -done - -# Expected output: -# Only one instance gets lock immediately, others wait -# All instances complete successfully -``` - -**Success criteria:** -- Only one instance initializes schema -- Other instances wait for lock -- All instances start successfully -- No partial/corrupted state - -### Test 4: Partial State Recovery (Crash Scenario) - -**Purpose:** Verify self-healing from partial initialization - -**Steps:** -```bash -# Manually create partial state -podman exec bookhoard_db psql -U postgres -d bookhoard -c " - CREATE TABLE library_types (id UUID PRIMARY KEY DEFAULT gen_random_uuid(), name VARCHAR(50) UNIQUE NOT NULL); - CREATE TABLE users (id UUID PRIMARY KEY DEFAULT gen_random_uuid(), email VARCHAR(255) UNIQUE NOT NULL); - -- STOP HERE - don't create other tables -" - -# Start app -podman compose up -d app - -# Check logs -podman logs bookhoard_app | tail -20 - -# Verify all tables created -podman exec bookhoard_db psql -U postgres -d bookhoard -c "\dt" | wc -l -# Should show 36+ tables -``` - -**Success criteria:** -- App detects partial state -- Schema execution completes missing tables -- Verification passes -- App starts successfully - -### Test 5: Integration Test Suite - -**Purpose:** Verify all tests pass with new initialization - -**Steps:** -```bash -# Run full integration test suite -make test-integration - -# Specifically check previously failing tests -# TestKoboInitialization should now PASS -``` - -**Success criteria:** -- All integration tests pass -- TestKoboInitialization passes -- No new failures introduced - ---- - -## ๐Ÿ“š Documentation Updates - -### 1. Update README.md - -**Section to add:** "Database Initialization" - -```markdown -## Database Initialization - -Bookhoard uses automatic idempotent database schema initialization. On every startup, the application: - -1. Acquires a PostgreSQL advisory lock (prevents concurrent initialization) -2. Executes the schema in a single transaction (all-or-nothing) -3. Verifies all expected tables exist (paranoid verification) -4. Releases the lock - -This ensures: -- โœ… Fresh databases are initialized automatically -- โœ… Existing databases are verified and kept up-to-date -- โœ… Partial/corrupted schemas are self-healed -- โœ… Multiple instances can start safely (horizontal scaling) -- โœ… No manual database setup required - -### Development - -For development with a fresh database: -```bash -podman compose down -v # Delete volumes (WARNING: loses all data) -podman compose up -d # Start with fresh schema -``` - -The app will automatically initialize the database on first startup. -``` - -### 2. Create `docs/contributing/database-schema.md` - -**New file:** - -```markdown -# Database Schema Management - -## Overview - -Bookhoard uses an idempotent schema initialization system that runs on every application startup. This document explains how it works and how to modify the schema. - -## Schema Initialization - -### How It Works - -1. **Advisory Lock**: Prevents multiple instances from initializing simultaneously -2. **Schema Execution**: Runs `database/schema/schema.sql` in a single transaction -3. **Verification**: Confirms all expected tables exist before starting server -4. **Self-Healing**: Idempotent statements fix partial/corrupted state - -### Startup Flow - -``` -Application Start - โ†“ -Connect to Database - โ†“ -Acquire Advisory Lock (blocks other instances) - โ†“ -Parse schema.sql โ†’ Extract table names - โ†“ -Execute schema.sql in transaction - โ†“ -Verify all expected tables exist - โ†“ -Release Advisory Lock - โ†“ -Start Accepting Requests -``` - -## Modifying the Schema - -### Adding a New Table - -1. **Edit `database/schema/schema.sql`:** - ```sql - CREATE TABLE IF NOT EXISTS my_new_table ( - id UUID PRIMARY KEY DEFAULT gen_random_uuid(), - -- ... columns ... - ); - ``` - -2. **Copy to embedded schema:** - ```bash - cp database/schema/schema.sql internal/database/schema/schema.sql - ``` - -3. **Restart the application:** - ```bash - podman compose restart app - ``` - -The new table will be created automatically on next startup (even in production). - -### Adding a New Column - -**Always use idempotent syntax:** -```sql -ALTER TABLE my_table ADD COLUMN IF NOT EXISTS new_column VARCHAR(255); -``` - -### Adding Indexes - -**Always use idempotent syntax:** -```sql -CREATE INDEX IF NOT EXISTS idx_my_table_column ON my_table(column); -``` - -### Adding Reference Data - -**Always use idempotent inserts:** -```sql -INSERT INTO my_reference_data (key, value) VALUES -('key1', 'value1'), -('key2', 'value2') -ON CONFLICT (key) DO NOTHING; -``` - -## Testing Schema Changes - -### Fresh Database -```bash -podman compose down -v -podman compose up -d db app -``` - -### Existing Database -```bash -podman compose restart app -``` - -### Verify Tables -```bash -podman exec bookhoard_db psql -U postgres -d bookhoard -c "\dt" -``` - -## Troubleshooting - -### Schema Initialization Failed - -**Check logs:** -```bash -podman logs bookhoard_app | grep -A 10 "Schema initialization" -``` - -**Common issues:** -- Syntax error in schema.sql โ†’ Fix SQL, restart app -- Permission denied โ†’ Check database user permissions -- Lock timeout โ†’ Another instance is initializing, wait for it - -### Missing Tables After Startup - -**Check logs for verification failure:** -```bash -podman logs bookhoard_app | grep "missing tables" -``` - -**Manual verification:** -```bash -podman exec bookhoard_db psql -U postgres -d bookhoard -c " - SELECT table_name - FROM information_schema.tables - WHERE table_schema = 'public' - ORDER BY table_name; -" -``` - -### Concurrent Startup Issues - -**Check advisory lock:** -```bash -podman exec bookhoard_db psql -U postgres -d bookhoard -c " - SELECT locktype, database, pid, mode, granted - FROM pg_locks - WHERE objid = 582394759234; -- schemaInitLockID -" -``` - -**Force release lock (if stuck):** -```bash -podman exec bookhoard_db psql -U postgres -d bookhoard -c " - SELECT pg_advisory_unlock(582394759234); -" -``` - -## Production Considerations - -### First Deployment - -- No manual database setup required -- Schema initializes automatically on first startup -- Verify logs show "All expected tables verified" - -### Database Upgrades - -- Add new tables/columns to schema.sql -- Deploy new version -- Schema changes apply automatically on startup -- Previous data is preserved (CREATE IF NOT EXISTS) - -### Horizontal Scaling - -- Multiple instances can start simultaneously -- Advisory lock prevents concurrent initialization -- First instance initializes, others wait -- All instances verify before accepting requests - -### Backups and Restores - -- Backup includes complete schema (use pg_dump) -- Restored database will be verified and patched on startup -- Missing tables will be recreated automatically -``` - ---- - -## ๐Ÿ“ Git Commit Strategy - -### Commit 1: Convert schema.sql to Idempotent - -**Message:** -``` -refactor(database): Make schema.sql fully idempotent for auto-initialization - -Convert all CREATE TABLE, CREATE INDEX, and INSERT statements to idempotent forms: -- 27 CREATE TABLE โ†’ CREATE TABLE IF NOT EXISTS -- 82 CREATE INDEX โ†’ CREATE INDEX IF NOT EXISTS -- 3 INSERT INTO โ†’ Added ON CONFLICT clauses - -This allows the schema to be safely run multiple times on every startup, -enabling automatic database initialization and self-healing. - -Related: #ISSUE_NUMBER (if applicable) -``` - -**Files:** -- `database/schema/schema.sql` - -### Commit 2: Add Schema Runner Package - -**Message:** -``` -feat(database): Add automatic schema initialization with paranoid verification - -Implement startup schema initialization with: -- PostgreSQL advisory locking (prevents concurrent execution) -- Single atomic transaction (all-or-nothing execution) -- Schema.sql parsing for expected table names -- Post-execution verification (confirms all tables exist) -- Tiered error logging (DEBUG โ†’ ERROR โ†’ FATAL summary) - -The app now initializes its database on every startup, making it -self-healing and independent of Docker init scripts. - -Lock ID: FNV-1a hash of "bookhoard:schema:init" = 582394759234 -``` - -**Files:** -- `internal/database/schema/schema.go` (new) -- `internal/database/schema/verification.go` (new) -- `internal/database/schema/schema.sql` (new, embedded) - -### Commit 3: Integrate Schema Init into main.go - -**Message:** -``` -feat(startup): Initialize database schema before accepting requests - -Add schema initialization call to main.go startup sequence. -The application now ensures database is ready before starting the HTTP server. - -Startup flow: -1. Connect to database -2. Initialize schema (idempotent, verified) -3. Create handlers and services -4. Start HTTP server - -This fixes the race condition where the app would start before -Docker init scripts completed, causing "library_types table doesn't exist" errors. - -Fixes: TestKoboInitialization and related test failures -``` - -**Files:** -- `cmd/server/main.go` - -### Commit 4: Update Documentation - -**Message:** -``` -docs(database): Document automatic schema initialization system - -Add comprehensive documentation for: -- Database initialization flow -- Schema modification guidelines -- Troubleshooting common issues -- Production deployment considerations -- Horizontal scaling behavior - -See: docs/contributing/database-schema.md -``` - -**Files:** -- `README.md` -- `docs/contributing/database-schema.md` (new) - ---- - -## โœ… Verification Checklist - -Before declaring complete, verify: - -- [ ] All CREATE TABLE statements use IF NOT EXISTS -- [ ] All CREATE INDEX statements use IF NOT EXISTS -- [ ] All INSERT statements have ON CONFLICT clauses -- [ ] `internal/database/schema/schema.go` compiles without errors -- [ ] `internal/database/schema/verification.go` compiles without errors -- [ ] `cmd/server/main.go` compiles without errors -- [ ] Fresh database initializes correctly (Test 1) -- [ ] Existing database doesn't break (Test 2) -- [ ] Concurrent startup works (Test 3) -- [ ] Partial state recovers (Test 4) -- [ ] All integration tests pass (Test 5) -- [ ] TestKoboInitialization passes -- [ ] Schema initialization messages appear in logs -- [ ] Advisory lock prevents concurrent issues -- [ ] Verification correctly checks all tables -- [ ] Error messages are clear and actionable -- [ ] No regressions in existing functionality -- [ ] Documentation is complete and accurate -- [ ] `go build ./...` succeeds for entire project -- [ ] `bash scripts/verify-guidelines.sh` passes (0 errors) - ---- - -## ๐Ÿ“ˆ Expected Outcomes - -### What Will Work: -โœ… App starts successfully on fresh database -โœ… App starts successfully on existing database -โœ… Multiple instances can start simultaneously -โœ… TestKoboInitialization and all tests pass -โœ… Clear log messages show schema initialization -โœ… Production-safe with no race conditions -โœ… Self-healing from partial/corrupted state - -### What Will NOT Change: -โœ… No changes to API endpoints -โœ… No changes to database structure (only safety clauses) -โœ… No changes to existing data -โœ… No changes to business logic -โœ… Backward compatible with existing deployments - -### Performance Impact: -- Fresh database startup: +500ms (one-time cost) -- Existing database startup: +100ms (verification only) -- HTTP request handling: No change -- Database queries: No change - ---- - -## ๐Ÿ”„ Rollback Plan - -If critical errors occur: - -### Option 1: Restore single file -```bash -git checkout HEAD -- database/schema/schema.sql -``` - -### Option 2: Restore entire branch -```bash -git checkout backup-before-schema-idempotent -- . -``` - -### Option 3: Revert commits -```bash -git reset --hard HEAD~4 # Revert all 4 commits -``` - ---- - -## ๐Ÿš€ Deployment Notes - -### First Deployment (Production) - -**No manual database setup required.** - -The app will initialize the database automatically on first startup. - -**Monitor logs for:** -``` -โœ… Database schema initialized and verified (36 tables verified) -``` - -### Subsequent Deployments - -**Schema changes apply automatically.** - -New tables/columns added to schema.sql will be created on next startup. - -**Monitor logs for:** -``` -โœ… Database schema initialization complete (36 tables verified) -``` - -### Rollback Plan - -If new version has schema issues: - -1. Deploy previous version -2. Previous version will verify and use existing schema -3. No data loss (CREATE IF NOT EXISTS preserves data) - ---- - -## ๐Ÿ“ž Support - -If issues occur: - -1. **Check logs first:** - ```bash - podman logs bookhoard_app | grep -i schema - ``` - -2. **Verify tables exist:** - ```bash - podman exec bookhoard_db psql -U postgres -d bookhoard -c "\dt" - ``` - -3. **Check advisory locks:** - ```bash - podman exec bookhoard_db psql -U postgres -d bookhoard -c " - SELECT * FROM pg_locks WHERE objid = 582394759234; - " - ``` - -4. **Force re-initialization (if needed):** - ```bash - podman compose down -v # WARNING: Deletes all data - podman compose up -d - ``` - ---- - -**End of Implementation Plan**