docs: refactor cover image serving plan with unified URL strategy

This commit updates the cover image serving plan to use a more streamlined, universal approach for file serving across all clients. Key changes to the plan: - Adopt unified URL format `/uploads/library-{id}/relative/path` for both covers and book files, replacing separate /api/files and /api/covers endpoints - Centralize path resolution through LibraryService.ResolveMediaPath() as the single source of truth for all handlers - Consolidate file serving into one authenticated ServeFile handler that works for web, mobile, and device clients - Update OPDS handler integration to use the same resolution logic - Reorganize implementation phases to reflect the unified architecture Benefits of this approach: - Simpler routing with one wildcard handler instead of multiple endpoints - Consistent path resolution logic across MediaHandler, OPDSHandler, and future handlers - Better support for multiple libraries and mount points - Single authentication flow for all file access - Easier maintenance and testing with centralized resolution This plan change does not modify any implementation code, only the documentation for the intended implementation.
2026-02-26 21:30:53 -05:00
parent 27c6738e02
commit 749cbd91ff
1 changed files with 162 additions and 109 deletions
@@ -21,15 +21,26 @@ Fix file and cover image serving to support:
 - File path stored as relative: `Jane Austen/Pride and Prejudice/book.epub`
 - Cover path stored as relative: `Jane Austen/Pride and Prejudice/cover.jpg`
 - Handler resolves relative path using library folder base path
- All file/cover requests go through authenticated API endpoints
- Mobile app can use same endpoints with JWT auth
+- Authenticated static-style handler serves files: `/uploads/library-{id}/path/to/file`
+- Backend resolves full URLs in API/SSR responses (one source of truth)
+- Works with mobile apps, Kobo, KOReader devices via same endpoints

 ### URL Format
-To handle same relative paths in different libraries:
+To handle same relative paths in different libraries, use:
 ```
-/api/files/{media_item_id}/content  → serves the book file
-/api/covers/{media_item_id}         → serves the cover image
+/uploads/library-{library_id}/relative/path
 ```
+- Requires JWT authentication (like API endpoints)
+- Works for both covers and book files
+- Single handler handles all file serving
+
+### Universal Path Resolution
+All handlers use the same `LibraryService.ResolveMediaPath()` function:
+- MediaHandler (downloads)
+- OPDSHandler (device cover images)
+- Future handlers
+
+This ensures one source of truth for path resolution.

 ---

@@ -145,56 +156,19 @@ func (s *LibraryService) ResolveMediaPath(ctx context.Context, libraryID pgtype.

 ---

-## Phase 3: Modify Existing Endpoints to Return Resolved URLs
+## Phase 3: Add URL Resolution Helper to MediaHandler

 ### Strategy

-Instead of creating new endpoints, modify existing API responses to include resolved/usable URLs. This avoids extra HTTP calls.
+Use `LibraryService.ResolveMediaPath()` to resolve paths. Add a simple wrapper in the handler for convenience.

 ### File: `internal/handlers/media.go`

-#### Update ListMediaItems response
-
-**Location**: Around line 612-620
-
-Create a wrapper struct or modify the response to include resolved cover URLs:
+Add helper method that uses the service:

 ```go
-// MediaItemResponse includes resolved URLs for frontend use
-type MediaItemResponse struct {
-    database.ListMediaItemsRow
-    CoverURL string `json:"cover_url"`
-    FileURL  string `json:"file_url"`
-}
-
-// In ListMediaItems handler:
-items, err := mh.db.ListMediaItems(...)
-// ... existing code ...
-
-// Transform to response with resolved URLs
-response := make([]MediaItemResponse, len(items))
-for i, item := range items {
-    response[i] = MediaItemResponse{
-        ListMediaItemsRow: item,
-        CoverURL: mh.resolveCoverURL(item.LibraryID, item.CoverImagePath.String),
-    }
-}
-```
-
-#### Add resolve helper methods
-
-```go
-// resolveCoverURL returns a usable URL for the cover image
-func (mh *MediaHandler) resolveCoverURL(libraryID pgtype.UUID, coverPath string) string {
-    if coverPath == "" {
-        return ""
-    }
-    // For now, return empty - frontend uses existing cover endpoint
-    // Later this could return /api/covers/{id} pattern
-    return ""
-}
-
 // getFullFilePath returns the absolute filesystem path for a media item
+// Uses LibraryService for resolution (one source of truth)
 func (mh *MediaHandler) getFullFilePath(ctx context.Context, libraryID pgtype.UUID, relativePath string) (string, error) {
    if relativePath == "" {
        return "", fmt.Errorf("no file path")
@@ -205,11 +179,13 @@ func (mh *MediaHandler) getFullFilePath(ctx context.Context, libraryID pgtype.UU
        return relativePath, nil
    }
    
-    // Resolve relative path using library folders
+    // Use service for resolution (one source of truth)
    return mh.libraryService.ResolveMediaPath(ctx, libraryID, relativePath)
 }
 ```

+Note: The handler already has `libraryService` injected, so this just calls through to it.
+
 ---

 ## Phase 4: Update Download Handler to Use Relative Paths
@@ -262,56 +238,57 @@ func (h *MediaHandler) DownloadBook(c echo.Context) error {

 ---

-## Phase 5: Create Cover Image Endpoint (For Cases Needing Direct Access)
+## Phase 5: Create Authenticated File Serving Handler

 ### File: `internal/handlers/media.go`

-While Phase 3 handles cover URLs in existing responses, this endpoint is useful for cases where you need direct cover access (e.g., dynamic updates, specific UI components).
+Create a single handler that serves both covers and book files:

 ```go
-// GetCoverImage handles GET /api/covers/:id
-// Returns the cover image for a media item
-func (mh *MediaHandler) GetCoverImage(c echo.Context) error {
-    mediaID := c.Param("id")
-    mediaUUID, err := uuid.Parse(mediaID)
+// ServeFile serves files (covers or books) via /uploads/library-{id}/path
+// Requires JWT authentication
+func (mh *MediaHandler) ServeFile(c echo.Context) error {
+    // URL format: /uploads/library-{libraryID}/{relativePath}
+    path := c.Param("*")  // Gets everything after /uploads/library-{id}/
+    
+    // Extract library ID from path
+    parts := strings.SplitN(path, "/", 2)
+    if len(parts) < 2 {
+        return c.JSON(http.StatusBadRequest, map[string]string{"error": "invalid path"})
+    }
+    
+    libraryIDStr := strings.TrimPrefix(parts[0], "library-")
+    libraryUUID, err := uuid.Parse(libraryIDStr)
    if err != nil {
-        return c.JSON(http.StatusBadRequest, map[string]string{"error": "invalid media item id"})
+        return c.JSON(http.StatusBadRequest, map[string]string{"error": "invalid library ID"})
    }
    
-    item, err := mh.db.GetMediaItem(c.Request().Context(), pgtype.UUID{Bytes: mediaUUID, Valid: true})
+    relativePath := parts[1]
+    
+    // Resolve using service
+    fullPath, err := mh.getFullFilePath(c.Request().Context(), pgtype.UUID{Bytes: libraryUUID, Valid: true}, relativePath)
    if err != nil {
-        if err == pgx.ErrNoRows {
-            return c.JSON(http.StatusNotFound, map[string]string{"error": "media item not found"})
-        }
-        return c.JSON(http.StatusInternalServerError, map[string]string{"error": err.Error()})
-    }
-
-    coverPath := item.CoverImagePath.String
-    if coverPath == "" {
-        return c.JSON(http.StatusNotFound, map[string]string{"error": "cover image not found"})
-    }
-
-    // Resolve relative path to absolute filesystem path
-    fullPath, err := mh.getFullFilePath(c.Request().Context(), item.LibraryID, coverPath)
-    if err != nil {
-        return c.JSON(http.StatusNotFound, map[string]string{"error": "cover image not found"})
+        return c.JSON(http.StatusNotFound, map[string]string{"error": "file not found"})
    }
    
+    // Check if file exists
    if _, err := os.Stat(fullPath); os.IsNotExist(err) {
-        return c.JSON(http.StatusNotFound, map[string]string{"error": "cover image file not found"})
+        return c.JSON(http.StatusNotFound, map[string]string{"error": "file not found"})
    }
    
+    // Determine content type
    ext := strings.ToLower(filepath.Ext(fullPath))
-    contentType := map[string]string{
-        ".jpg":  "image/jpeg",
-        ".jpeg": "image/jpeg",
-        ".png":  "image/png",
-        ".gif":  "image/gif",
-        ".webp": "image/webp",
-    }[ext]
-
-    if contentType == "" {
-        contentType = "application/octet-stream"
+    contentType := "application/octet-stream"
+    if ext == ".jpg" || ext == ".jpeg" {
+        contentType = "image/jpeg"
+    } else if ext == ".png" {
+        contentType = "image/png"
+    } else if ext == ".webp" {
+        contentType = "image/webp"
+    } else if ext == ".epub" {
+        contentType = "application/epub+zip"
+    } else if ext == ".pdf" {
+        contentType = "application/pdf"
    }
    
    c.Response().Header().Set("Content-Type", contentType)
@@ -325,13 +302,84 @@ func (mh *MediaHandler) GetCoverImage(c echo.Context) error {
 **Location**: After existing media routes

 ```go
-// Cover images - authenticated
-protected.GET("/covers/:id", cfg.MediaHandler.GetCoverImage)
+// File serving - authenticated
+// Note: Must be registered LAST as it's a wildcard route
+protected.GET("/uploads/library-:id/*", cfg.MediaHandler.ServeFile)
+```
+
+**Important**: This route must be registered LAST because `/*` is a wildcard that matches everything.
+
+---
+
+## Phase 6: Update OPDS Handler for Device Support
+
+### File: `internal/handlers/opds.go`
+
+#### Modify GetCoverImage function
+
+**Current code** (around line 477-549):
+```go
+func (h *OPDSHandler) GetCoverImage(c echo.Context) error {
+    // ...
+    coverPath := mediaItem.CoverImagePath.String
+    
+    // Check if file exists
+    if _, err := os.Stat(coverPath); os.IsNotExist(err) {
+        return c.NoContent(http.StatusNoContent)
+    }
+    
+    // Open file
+    file, err := os.Open(coverPath)
+    // ...
+}
+```
+
+**New code**:
+```go
+func (h *OPDSHandler) GetCoverImage(c echo.Context) error {
+    // ...
+    coverPath := mediaItem.CoverImagePath.String
+    
+    // Resolve relative path using library service
+    fullPath, err := h.libraryService.ResolveMediaPath(c.Request().Context(), mediaItem.LibraryID, coverPath)
+    if err != nil {
+        return c.NoContent(http.StatusNoContent)
+    }
+    
+    // Check if file exists
+    if _, err := os.Stat(fullPath); os.IsNotExist(err) {
+        return c.NoContent(http.StatusNoContent)
+    }
+    
+    // Open file
+    file, err := os.Open(fullPath)
+    // ...
+}
+```
+
+**Note**: OPDSHandler already has `libraryService` injected, so it can use the same resolution logic.
+
+**If OPDSHandler doesn't have libraryService**, add it:
+
+```go
+type OPDSHandler struct {
+    db                *database.Queries
+    libraryService    *services.LibraryService
+    conversionService interface {...}
+}
+
+func NewOPDSHandler(db *database.Queries, libraryService *services.LibraryService, conversionService ...) *OPDSHandler {
+    return &OPDSHandler{
+        db:             db,
+        libraryService: libraryService,
+        conversionService: conversionService,
+    }
+}
 ```

 ---

-## Phase 6: Frontend - No Changes Needed (SSR)
+## Phase 7: Frontend - No Changes Needed (SSR)

 The existing frontend code should work without modification:

@@ -339,13 +387,11 @@ The existing frontend code should work without modification:
 - **dashboard.ts**: Uses `book.cover_image_path` from API response
 - **bookshelf.ts**: Uses `book.cover_image_path` from API response

-**Note**: If covers don't load initially, it's because the handler needs to resolve the URL. The frontend code doesn't need to change - the backend just needs to provide usable URLs.
-
-If the endpoint responses include a resolved URL field (like `cover_url`), update the frontend to use that field. Otherwise, the existing `/api/covers/:id` endpoint can be used.
+**Note**: Backend resolves URLs when building API responses, so frontend just uses the URL directly - no extra requests.

 ---

-## Phase 7: Backward Compatibility
+## Phase 8: Backward Compatibility

 Handle existing absolute paths in database:

@@ -621,19 +667,20 @@ Update existing documentation to note:
 | Phase | File | Change |
 |-------|------|--------|
 | 1 | `internal/services/media_scanner.go` | Add `getRelativePath()` function; use for both file_path and cover_path |
-| 2 | `internal/services/library_service.go` | Add `ResolveMediaPath()` function |
-| 3 | `internal/handlers/media.go` | Modify `ListMediaItems` to include resolved URLs if needed |
-| 4 | `internal/handlers/media.go` | Modify `DownloadBook` to resolve relative paths |
-| 5 | `internal/handlers/media.go` | Add `GetCoverImage` function |
-| 5 | `internal/router/media.go` | Add route `GET /api/covers/:id` |
-| 6 | Frontend files | No changes needed (verify in testing) |
-| 7 | Backward compatibility | Runtime resolution handles absolute paths |
-| 8 | `internal/handlers/media_test.go` | Add unit tests for GetCoverImage and path resolution |
-| 8 | `internal/services/media_scanner_test.go` | Add unit tests for getRelativePath |
-| 8 | `cmd/server/tests/cover_file_serving_test.go` | Add integration tests using test_helpers |
-| 9 | `bruno/media-items/Get Cover Image.yml` | Add Bruno API test for cover endpoint |
-| 9 | `bruno/media-items/EPUB Download.yml` | Update to document relative path handling |
-| 9 | `docs/developer/api/media-items/get_cover_image.md` | Add API documentation |
+| 2 | `internal/services/library_service.go` | Add `ResolveMediaPath()` function (one source of truth) |
+| 3 | `internal/handlers/media.go` | Add `getFullFilePath()` helper that calls service |
+| 4 | `internal/handlers/media.go` | Modify `DownloadBook` to use `getFullFilePath()` |
+| 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) |
+| 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()` |
+| 9 | `cmd/server/tests/cover_file_serving_test.go` | Add integration tests using test_helpers |
+| 10 | `bruno/media-items/Get Cover Image.yml` | Add Bruno API test |
+| 10 | `bruno/media-items/EPUB Download.yml` | Update to document relative path handling |
+| 10 | `docs/developer/api/media-items/` | Update API documentation |

 ---

@@ -644,8 +691,8 @@ After implementation:
 1. **Test new scan**: Add a new book with cover, verify:
   - Database `file_path` is relative (e.g., `Author/Book/book.epub`)
   - Database `cover_image_path` is relative (e.g., `Author/Book/cover.jpg`)
+   - GET `/uploads/library-{id}/Author/Book/cover.jpg` returns the image
   - GET `/api/media-items/:id/download` returns the file
-   - GET `/api/covers/:id` returns the image

 2. **Test existing data**: For items with absolute paths:
   - Downloads still work (backward compatibility)
@@ -654,15 +701,21 @@ After implementation:
 3. **Test multiple mount points**:
   - Library A with folder `/app/epubs`
   - Library B with folder `/var/manga`
-   - Books in each resolve correctly
+   - Books in each resolve correctly via their library ID

 4. **Test frontend**:
-   - Dashboard shows cover images
+   - Dashboard shows cover images (SSR - initial load)
+   - Library switch works (dynamic - uses resolved URLs)
   - Bookshelf shows cover images
   - Downloads work

 5. **Test mobile app** (future):
   - Same JWT auth works for files and covers
+   - `/uploads/library-{id}/...` URLs work
+
+6. **Test device integration**:
+   - Kobo devices can fetch cover images via OPDS
+   - KOReader sync continues to work

 ---