diff --git a/cover_image_serving_plan.md b/cover_image_serving_plan.md index 078fef3..7079d27 100644 --- a/cover_image_serving_plan.md +++ b/cover_image_serving_plan.md @@ -357,7 +357,7 @@ func (h *OPDSHandler) GetCoverImage(c echo.Context) error { } ``` -**Note**: OPDSHandler already has `libraryService` injected, so it can use the same resolution logic. +**Note**: OPDSHandler uses `ResolveMediaPath()` to resolve to **filesystem path** (not URL) because OPDS serves files directly from the local filesystem. This is different from API handlers which resolve to `/uploads/library-{id}/...` URLs. **If OPDSHandler doesn't have libraryService**, add it: @@ -379,15 +379,378 @@ func NewOPDSHandler(db *database.Queries, libraryService *services.LibraryServic --- -## Phase 7: Frontend - No Changes Needed (SSR) +## Phase 7: Update ALL Handlers to Resolve URLs in API Responses -The existing frontend code should work without modification: +Every handler that returns `cover_image_path` or `file_path` in API responses must resolve the relative path to a full URL before sending to the client. -- **dashboard.templ**: Uses `item.CoverImagePath` directly in SSR -- **dashboard.ts**: Uses `book.cover_image_path` from API response -- **bookshelf.ts**: Uses `book.cover_image_path` from API response +### URL Resolution Flow -**Note**: Backend resolves URLs when building API responses, so frontend just uses the URL directly - no extra requests. +1. **Database stores**: Relative path (e.g., `Author/Book/cover.jpg`) +2. **Handler fetches**: Gets relative path from DB (as pgtype.Text) +3. **Handler resolves**: Calls `ResolveCoverURL()` → returns `/uploads/library-{id}/Author/Book/cover.jpg` +4. **Handler replaces**: Sets resolved URL string in response (not pgtype.Text) +5. **Frontend/mobile**: Uses URL directly (authenticated via JWT) + +### Step 1: Add URL Resolution Helper to MediaHandler + +**File**: `internal/handlers/media.go` + +Add these helper methods after the `NewMediaHandler` function (around line 100): + +```go +// ResolveCoverURL resolves a relative cover path to a full URL for API responses +func (mh *MediaHandler) ResolveCoverURL(libraryID pgtype.UUID, coverPath pgtype.Text) string { + if !coverPath.Valid || coverPath.String == "" { + return "" + } + + return mh.resolveMediaURL(libraryID, coverPath.String) +} + +// ResolveFileURL resolves a relative file path to a full URL for API responses +func (mh *MediaHandler) ResolveFileURL(libraryID pgtype.UUID, filePath pgtype.Text) string { + if !filePath.Valid || filePath.String == "" { + return "" + } + + return mh.resolveMediaURL(libraryID, filePath.String) +} + +// resolveMediaURL is the internal helper that does the actual resolution +func (mh *MediaHandler) resolveMediaURL(libraryID pgtype.UUID, relativePath string) string { + // Already a full URL? Return as-is + if strings.HasPrefix(relativePath, "/uploads/") { + return relativePath + } + + // Already absolute path? Convert to URL format (backward compatibility) + // Note: This loses library ID info, but existing data won't have it + if filepath.IsAbs(relativePath) { + return relativePath + } + + // Resolve relative path to URL format + libraryIDStr := libraryID.Bytes.String() + return fmt.Sprintf("/uploads/library-%s/%s", libraryIDStr, relativePath) +} +``` + +**Note**: Add `"strings"` and `"fmt"` to the imports if not already present. + +--- + +### Step 2: Update collections.go - GetCollectionBooks (line ~199) + +**File**: `internal/handlers/collections.go` + +**Current code** (lines 193-201): +```go +bookList := make([]BookInfo, 0, len(books)) +for _, book := range books { + bookList = append(bookList, BookInfo{ + MediaItemID: uuid.UUID(book.MediaItemID.Bytes).String(), + Title: book.Title, + Author: textToString(book.Author), + CoverImagePath: textToString(book.CoverImagePath), + }) +} +``` + +**New code**: +```go +bookList := make([]BookInfo, 0, len(books)) +for _, book := range books { + bookList = append(bookList, BookInfo{ + MediaItemID: uuid.UUID(book.MediaItemID.Bytes).String(), + Title: book.Title, + Author: textToString(book.Author), + CoverImagePath: h.resolveCoverURL(book.LibraryID, book.CoverImagePath), + }) +} +``` + +**Add helper method** to CollectionHandler struct (near line 10): + +```go +// resolveCoverURL resolves a relative cover path to a full URL +func (h *CollectionHandler) resolveCoverURL(libraryID pgtype.UUID, coverPath pgtype.Text) string { + if !coverPath.Valid || coverPath.String == "" { + return "" + } + + // Already a full URL? Return as-is + if strings.HasPrefix(coverPath.String, "/uploads/") { + return coverPath.String + } + + // Already absolute path? Return as-is (backward compatibility) + if filepath.IsAbs(coverPath.String) { + return coverPath.String + } + + // Resolve relative path to URL format + libraryIDStr := libraryID.Bytes.String() + return fmt.Sprintf("/uploads/library-%s/%s", libraryIDStr, coverPath.String) +} +``` + +**Add imports** if not present: `"fmt"`, `"path/filepath"`, `"strings"` + +--- + +### Step 3: Update collections.go - CheckMatchRules (line ~626) + +**File**: `internal/handlers/collections.go` + +**Current code** (lines 620-641): +```go +var matches []BookMatch +for _, item := range mediaItems { + matchReason := h.checkRulesAgainstBook(item, req.Rules) + if matchReason != "" { + coverPath := "" + if item.CoverImagePath.Valid { + coverPath = item.CoverImagePath.String + } + author := "" + if item.Author.Valid { + author = item.Author.String + } + + matches = append(matches, BookMatch{ + MediaItemID: uuid.UUID(item.ID.Bytes).String(), + Title: item.Title, + Author: author, + CoverImagePath: coverPath, + MatchReason: matchReason, + }) + } +} +``` + +**New code**: +```go +var matches []BookMatch +for _, item := range mediaItems { + matchReason := h.checkRulesAgainstBook(item, req.Rules) + if matchReason != "" { + author := "" + if item.Author.Valid { + author = item.Author.String + } + + matches = append(matches, BookMatch{ + MediaItemID: uuid.UUID(item.ID.Bytes).String(), + Title: item.Title, + Author: author, + CoverImagePath: h.resolveCoverURL(item.LibraryID, item.CoverImagePath), + MatchReason: matchReason, + }) + } +} +``` + +--- + +### Step 4: Update collections.go - ListCollectionBooks (line ~935) + +**File**: `internal/handlers/collections.go` + +**Current code** (lines 925-954 - the entire function): +```go +return database.ListMediaItemsRow{ + ID: item.ID, + LibraryID: item.LibraryID, + Title: item.Title, + Author: item.Author, + Isbn: item.Isbn, + Description: item.Description, + FilePath: item.FilePath, + FileSize: item.FileSize, + MimeType: item.MimeType, + CoverImagePath: item.CoverImagePath, + // ... rest of fields +} +``` + +**New code**: +```go +return database.ListMediaItemsRow{ + ID: item.ID, + LibraryID: item.LibraryID, + Title: item.Title, + Author: item.Author, + Isbn: item.Isbn, + Description: item.Description, + FilePath: pgtype.Text{String: h.resolveFileURL(item.LibraryID, item.FilePath), Valid: item.FilePath.Valid}, + FileSize: item.FileSize, + MimeType: item.MimeType, + CoverImagePath: pgtype.Text{String: h.resolveCoverURL(item.LibraryID, item.CoverImagePath), Valid: item.CoverImagePath.Valid}, + // ... rest of fields +} +``` + +**Add helper method** for file URL resolution: + +```go +// resolveFileURL resolves a relative file path to a full URL +func (h *CollectionHandler) resolveFileURL(libraryID pgtype.UUID, filePath pgtype.Text) string { + if !filePath.Valid || filePath.String == "" { + return "" + } + + if strings.HasPrefix(filePath.String, "/uploads/") { + return filePath.String + } + + if filepath.IsAbs(filePath.String) { + return filePath.String + } + + libraryIDStr := libraryID.Bytes.String() + return fmt.Sprintf("/uploads/library-%s/%s", libraryIDStr, filePath.String) +} +``` + +--- + +### Step 5: Update progress.go - two locations (lines ~288 and ~359) + +**File**: `internal/handlers/progress.go` + +First, add helper methods to ProgressHandler struct (find struct definition and add after it): + +```go +// resolveCoverURL resolves a relative cover path to a full URL +func (h *ProgressHandler) resolveCoverURL(libraryID pgtype.UUID, coverPath pgtype.Text) string { + if !coverPath.Valid || coverPath.String == "" { + return "" + } + + if strings.HasPrefix(coverPath.String, "/uploads/") { + return coverPath.String + } + + if filepath.IsAbs(coverPath.String) { + return coverPath.String + } + + libraryIDStr := libraryID.Bytes.String() + return fmt.Sprintf("/uploads/library-%s/%s", libraryIDStr, coverPath.String) +} +``` + +**Location 1 - GetReadingProgress function** (around line 286-289): + +**Current code**: +```go +coverPath := "" +if mediaItem.CoverImagePath.Valid { + coverPath = mediaItem.CoverImagePath.String +} +``` + +**New code** (remove the manual resolution, use helper): +```go +coverPath := h.resolveCoverURL(mediaItem.LibraryID, mediaItem.CoverImagePath) +``` + +**Location 2 - GetAllReadingProgress function** (around line 357-360): + +**Current code**: +```go +coverPath := "" +if mediaItem.CoverImagePath.Valid { + coverPath = mediaItem.CoverImagePath.String +} +``` + +**New code**: +```go +coverPath := h.resolveCoverURL(mediaItem.LibraryID, mediaItem.CoverImagePath) +``` + +--- + +### Step 6: Update media.go - GetMediaItem and ListMediaItems + +**File**: `internal/handlers/media.go` + +The current implementation returns raw database rows directly. We need to convert them to API-safe responses with resolved URLs. + +**Option A: Quick fix** - Modify the response before returning (lines 609, 620, 639) + +For `ListMediaItems` (around line 609 and 620), add a helper to convert each item: + +```go +// Add this function somewhere in media.go +func resolveMediaItemCoverAndFile(item database.ListMediaItemsRow) database.ListMediaItemsRow { + // This is a placeholder - in practice you'd need to add libraryService to MediaHandler + // For now, return as-is. Full implementation requires adding libraryService dependency. + return item +} +``` + +**Note**: The `ListMediaItems` and `GetMediaItem` functions currently return raw database rows. To properly resolve URLs, you would need to either: + +1. **Add libraryService to MediaHandler** and call the resolution helpers, OR +2. **Create a separate response struct** that converts pgtype.Text to resolved URLs + +For this implementation, the recommended approach is: + +**Modify GetMediaItem** (line 639): + +**Current code**: +```go +return c.JSON(http.StatusOK, item) +``` + +**New code**: +```go +return c.JSON(http.StatusOK, map[string]interface{}{ + "id": uuid.UUID(item.ID.Bytes).String(), + "library_id": uuid.UUID(item.LibraryID.Bytes).String(), + "title": item.Title, + "author": textToString(item.Author), + "cover_image_path": mh.ResolveCoverURL(item.LibraryID, item.CoverImagePath), + "file_path": mh.ResolveFileURL(item.LibraryID, item.FilePath), + // ... add other fields as needed +}) +``` + +Similarly for `ListMediaItems`, wrap the results in a map with resolved URLs. + +--- + +### Step 7: Fix Frontend /covers/ Prefix + +**File**: `web/src/bookshelf.ts` + +**Current code** (line 49-50): +```typescript +${book.cover_image_path ? + `${book.title}` : +``` + +**New code**: +```typescript +${book.cover_image_path ? + `${book.title}` : +``` + +The backend now returns full URLs like `/uploads/library-{id}/path/to/cover.jpg`, so no prefix is needed. + +--- + +### Summary of Changes for Phase 7 + +| File | Changes | +|------|---------| +| `internal/handlers/media.go` | Add `ResolveCoverURL()`, `ResolveFileURL()`, `resolveMediaURL()` helpers | +| `internal/handlers/collections.go` | Add `resolveCoverURL()`, `resolveFileURL()` to CollectionHandler; update lines ~199, ~626, ~935 | +| `internal/handlers/progress.go` | Add `resolveCoverURL()` to ProgressHandler; update lines ~288, ~359 | +| `internal/handlers/media.go` | Update `GetMediaItem` to return resolved URLs in response map | +| `web/src/bookshelf.ts` | Remove `/covers/` prefix from cover image URL | --- @@ -417,7 +780,7 @@ func (mh *MediaHandler) getFullFilePath(ctx context.Context, libraryID pgtype.UU --- -## Phase 8: Tests +## Phase 9: Tests ### Unit Tests @@ -659,7 +1022,6 @@ curl -H "Authorization: Bearer YOUR_TOKEN" \ - Supports backward compatibility with existing absolute paths - Images are cached for 24 hours by clients - All endpoints require authentication (JWT) -``` ### File: `docs/developer/api/media-items/download_book.md` @@ -681,7 +1043,10 @@ Update existing documentation to note: | 5 | `internal/handlers/media.go` | Add `ServeFile()` handler for authenticated static-style routes | | 5 | `internal/router/media.go` | Add route `GET /uploads/library-:id/*` (register LAST) | | 6 | `internal/handlers/opds.go` | Add `libraryService` to struct; update `GetCoverImage` to use service | -| 7 | Frontend files | No changes needed (backend resolves URLs) | +| 7 | `internal/handlers/collections.go` | Resolve cover paths to URLs in API responses | +| 7 | `internal/handlers/progress.go` | Resolve cover paths to URLs in API responses | +| 7 | `internal/handlers/media.go` | Resolve file paths to URLs in API responses | +| 7 | `web/src/bookshelf.ts` | Remove `/covers/` prefix (use resolved URL directly) | | 8 | Runtime resolution | Handles both absolute (old) and relative (new) paths | | 9 | `internal/handlers/media_test.go` | Add unit tests for path resolution | | 9 | `internal/services/media_scanner_test.go` | Add unit tests for `getRelativePath()` | diff --git a/force-rescan-plan.md b/force-rescan-plan.md deleted file mode 100644 index 1afa598..0000000 --- a/force-rescan-plan.md +++ /dev/null @@ -1,417 +0,0 @@ -# Implementation Plan: Force Rescan Feature - -## Overview - -Add a `force` parameter to the library scan endpoint that allows re-processing existing media items. Currently, the scanner skips files that already exist in the database (based on file path). The force flag will bypass this check and re-process all files. - -## Current Behavior - -When scanning a library: -1. `ScanLibrary` handler receives scan request -2. Worker enqueues a `JobTypeScan` job -3. `processScanJob` creates a `MediaScanner` and calls `ScanFolders()` -4. For each file, `processMediaFile()` checks if file exists in DB -5. If exists with same size → **skip** (return `false, nil`) -6. If exists with different size → **update** -7. If doesn't exist → **create new** - -## Proposed Changes - -### 1. Backend: Add `force` parameter support - -#### File: `internal/handlers/scanner.go` - -**Change 1.1** - Add `Force` field to `ScanLibraryRequest` struct (line ~89-92): - -```go -type ScanLibraryRequest struct { - FolderPaths []string `json:"folder_paths,omitempty"` - LibraryID string `json:"library_id,omitempty"` - Force bool `json:"force,omitempty"` // NEW: Force rescan of existing files -} -``` - -**Change 1.2** - Pass `force` to job params in `ScanLibrary` function (line ~153): - -```go -job := &services.Job{ - ID: jobID, - Type: services.JobTypeScan, - Params: map[string]interface{}{ - "library_id": req.LibraryID, - "folders": folderPaths, - "admin_id": userUUID.String(), - "db": h.db, - "force": req.Force, // NEW - }, - // ... rest unchanged -} -``` - -#### File: `internal/services/worker.go` - -**Change 2.1** - Extract `force` param in `processScanJob` function (after line ~195): - -```go -// Existing code: -db, ok := job.Params["db"].(*database.Queries) -if !ok { - return nil, fmt.Errorf("database queries required") -} - -// NEW: Extract force parameter -force := false -if forceVal, ok := job.Params["force"].(bool); ok { - force = forceVal -} -``` - -**Change 2.2** - Pass force to MediaScanner (after line ~220): - -```go -scanner.SetAdminID(adminUUID) - -// NEW: Set force flag -scanner.SetForce(force) // NEW -``` - -#### File: `internal/services/media_scanner.go` - -**Change 3.1** - Add `forceRescan` field to MediaScanner struct (find struct definition): - -```go -type MediaScanner struct { - db *database.Queries - folders []string - adminID pgtype.UUID - job *Job - watcher *fsnotify.Watcher - totalFiles int - newItems int - errors int - forceRescan bool // NEW: Force re-scan of existing files -} -``` - -**Change 3.2** - Add `SetForce` method (anywhere in file, after existing setters): - -```go -func (s *MediaScanner) SetForce(force bool) { - s.forceRescan = force -} -``` - -**Change 3.3** - Modify `processMediaFile` to respect force flag (line ~356-366): - -Current code: -```go -// Check if media item already exists in database -existingItem, err := s.getMediaItemByFilePath(ctx, path) -if err == nil { - fmt.Printf("Media item already exists in database: %s (size: %d vs %d)\n", path, existingItem.FileSize.Int64, info.Size()) - // Media item exists, check if file has changed (by size) - if existingItem.FileSize.Int64 != info.Size() { - fmt.Printf("File size changed, updating media item: %s\n", path) - _ = s.updateMediaItem(ctx, existingItem.ID, path, info) - return false, nil - } - fmt.Printf("Media item already exists with same size, skipping: %s\n", path) - return false, nil // <-- THIS IS WHERE WE SKIP -} -``` - -New code: -```go -// Check if media item already exists in database -existingItem, err := s.getMediaItemByFilePath(ctx, path) -if err == nil { - fmt.Printf("Media item already exists in database: %s (size: %d vs %d)\n", path, existingItem.FileSize.Int64, info.Size()) - - // If force rescan is enabled, always re-process - if s.forceRescan { - fmt.Printf("Force rescan enabled, re-processing existing media item: %s\n", path) - // Force update: delete existing and re-create - if err := s.db.DeleteMediaItem(ctx, existingItem.ID); err != nil { - fmt.Printf("Warning: failed to delete existing media item: %v\n", err) - } - // Continue to create new entry below - } else { - // Normal behavior: check if file has changed (by size) - if existingItem.FileSize.Int64 != info.Size() { - fmt.Printf("File size changed, updating media item: %s\n", path) - _ = s.updateMediaItem(ctx, existingItem.ID, path, info) - return false, nil - } - fmt.Printf("Media item already exists with same size, skipping: %s\n", path) - return false, nil - } -} -``` - -### 2. Frontend: Update button and API call - -#### File: `web/src/admin.ts` - -**Change 4.1** - Update `scanAllLibraries` function to send `force: true` (line ~129): - -Current code: -```typescript -const scanResp = await fetch(`/api/libraries/${lib.id}/scan`, { - method: 'POST', - headers: { 'Authorization': `Bearer ${token}` } -}); -``` - -New code: -```typescript -const scanResp = await fetch(`/api/libraries/${lib.id}/scan`, { - method: 'POST', - headers: { - 'Authorization': `Bearer ${token}`, - 'Content-Type': 'application/json' - }, - body: JSON.stringify({ force: true }) -}); -``` - -#### File: `templates/admin.templ` - -**Change 5.1** - Update button text and description (line ~55-58): - -Current: -```html - -``` - -New: -```html - -``` - -### 3. API Documentation - -#### File: `docs/developer/api/scanner/scan_library.md` - -**Change 6.1** - Update documentation to reflect force parameter (line ~15): - -Current: -```markdown -| force | boolean | No | Force rescan of existing files (default: false) | -``` - -This is already documented. Ensure the description is accurate: -```markdown -| force | boolean | No | Force rescan of existing files. When true, re-processes all files in library regardless of whether they already exist in database (default: false) | -``` - -### 4. Tests - -#### Unit Tests: `internal/services/worker_test.go` - -**Change 7.1** - Add test case for force parameter in existing scan job tests: - -```go -func TestWorker_ProcessJob_ScanJob_ForceRescan(t *testing.T) { - // Setup test with existing media item in database - // Create job with force: true - // Verify media item is re-processed -} -``` - -#### Integration Tests: `cmd/server/tests/scanner_integration_test.go` - -**Change 7.2** - Add integration tests covering three contexts (per guidelines line 140): - -Follow existing pattern from `scanner_integration_test.go`: - -```go -func TestScanLibrary_ForceFlag(t *testing.T) { - s := setupTestServer(t) - defer s.TearDown() - - // Test 1: No user context (unauthenticated) - expect 401 - // Test 2: Regular user context - expect 403 (admin only) - // Test 3: Admin context with force=true - expect 200, verify re-scan - // Test 4: Admin context with force=false - expect 200, verify skip -} -``` - -#### Bruno OpenCollection YAML Tests - -**Change 7.3** - Add force parameter test case to existing Bruno test: - -File: `bruno/scanner/Scan Media Items.yml` - -Add a new test request or modify existing to include: -```yaml -body: { - "force": true -} -``` - -### 5. Frontend: Add Watch Status Display - -Replace the "Settings" card on /admin page with a Watch Status display. - -#### File: `templates/admin.templ` - -**Change 8.1** - Replace Settings card with Watch Status (lines 38-47): - -Current: -```html -
-
-
⚙️
-
-

Settings

-

Configure your preferences

-
-
- Manage Settings -
-``` - -New: -```html -
-
-
👁️
-
-

Scan Watch Status

-

Auto-detecting new files

-
-
-
- - Watching 0 libraries -
-
-``` - -#### File: `web/src/admin.ts` - -**Change 8.2** - Add function to fetch and display watch status: - -```typescript -async function loadWatchStatus() { - const token = localStorage.getItem('token'); - if (!token) return; - - try { - const response = await fetch('/api/scanner/watch/status', { - headers: { 'Authorization': `Bearer ${token}` } - }); - if (response.ok) { - const data = await response.json(); - const countEl = document.getElementById('watch-count'); - if (countEl) { - countEl.textContent = data.total_watching?.toString() || '0'; - } - } - } catch (error) { - console.error('Failed to load watch status:', error); - } -} - -function logout(): void { - localStorage.removeItem('token'); - localStorage.removeItem('user'); - window.location.href = '/'; -} - -// Initialize on page load -document.addEventListener('DOMContentLoaded', function() { - loadWatchStatus(); -}); -``` - -**Change 8.3** - Move inline script from admin.templ to admin.ts: - -Current admin.templ has inline script (lines 121-131): -```html - -``` - -Remove the entire `