From 749cbd91ffe0ccfc9a206888f2a126e28d5bc575 Mon Sep 17 00:00:00 2001 From: John O'Keefe Date: Thu, 26 Feb 2026 21:30:53 -0500 Subject: [PATCH] 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. --- cover_image_serving_plan.md | 271 +++++++++++++++++++++--------------- 1 file changed, 162 insertions(+), 109 deletions(-) diff --git a/cover_image_serving_plan.md b/cover_image_serving_plan.md index 76d3ba3..c2fad00 100644 --- a/cover_image_serving_plan.md +++ b/cover_image_serving_plan.md @@ -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,58 +238,59 @@ 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()}) + return c.JSON(http.StatusNotFound, map[string]string{"error": "file not found"}) } - - 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"}) - } - + + // 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) c.Response().Header().Set("Cache-Control", "public, max-age=86400") return c.File(fullPath) @@ -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 ---