From baa4644ecee88a6489d44b234a084519f764ab53 Mon Sep 17 00:00:00 2001 From: Caleb Allen Date: Wed, 25 Feb 2026 15:31:05 -0500 Subject: [PATCH] extracted cover art handling from library package --- .opencode/plans/refactoring-catalog.md | 8 +- backend/app.go | 5 +- backend/coverart/coverart.go | 60 +++++++++ backend/coverart/coverart_test.go | 171 +++++++++++++++++++++++++ backend/coverart/handler.go | 51 ++++++++ backend/library/coverart.go | 26 +--- backend/library/coverart_handler.go | 46 ------- backend/library/query.go | 29 ++--- backend/library/rescan.go | 8 +- backend/player/player.go | 15 +-- backend/playlist/playlist.go | 15 +-- 11 files changed, 320 insertions(+), 114 deletions(-) create mode 100644 backend/coverart/coverart.go create mode 100644 backend/coverart/coverart_test.go create mode 100644 backend/coverart/handler.go delete mode 100644 backend/library/coverart_handler.go diff --git a/.opencode/plans/refactoring-catalog.md b/.opencode/plans/refactoring-catalog.md index 3b4d681..7f5c22a 100644 --- a/.opencode/plans/refactoring-catalog.md +++ b/.opencode/plans/refactoring-catalog.md @@ -28,13 +28,9 @@ Prioritized list of architectural improvements identified during a full codebase --- -### 6. Extract `SizedFilename` to a shared utility package +### ~~6. Extract `SizedFilename` to a shared utility package~~ — solved -**Problem:** `library.SizedFilename()` is a small string utility for generating thumbnail filenames. Both `player/player.go` and `playlist/playlist.go` import the entire `library` package solely for this function. - -**Why it matters:** Creates unnecessary coupling — `player` -> `library` and `playlist` -> `library` dependencies exist only for one utility function. - -**Approach:** Move `SizedFilename` to a shared package (e.g., `backend/coverart/` or `backend/fileutil/`). Update the three callers: `library/`, `player/`, and `playlist/`. +Created `backend/coverart/` package with `SizedFilename`, a `ResolveURLs` helper (encapsulates the repeated pattern of resolving filesystem paths to all size-variant URL paths), a `URLs` struct, and a `PathPrefix` constant. Removed `SizedFilename` from `library/coverart.go`. Updated all four callers (`library/query.go`, `player/player.go`, `playlist/playlist.go`, `app.go`) to use `coverart.ResolveURLs`, eliminating the `player` -> `library` and `playlist` -> `library` coupling. Added tests for the new package. --- diff --git a/backend/app.go b/backend/app.go index 283c724..0ece2d6 100644 --- a/backend/app.go +++ b/backend/app.go @@ -14,6 +14,7 @@ import ( "yellowjacket/backend/assets" "yellowjacket/backend/config" + "yellowjacket/backend/coverart" "yellowjacket/backend/database" "yellowjacket/backend/frontendutil" "yellowjacket/backend/library" @@ -90,12 +91,12 @@ func NewYellowJacketApp( yjApp.library = lib // create cover art handler - coverHandler, err := library.NewCoverArtHandler() + coverHandler, err := coverart.NewHandler() if err != nil { return nil, fmt.Errorf("could not create cover art handler: %w", err) } - yjApp.assetHandler.RegisterHandler("/covers/", coverHandler) + yjApp.assetHandler.RegisterHandler(coverart.PathPrefix, coverHandler) // create playlist service yjApp.playlist = playlist.NewService( diff --git a/backend/coverart/coverart.go b/backend/coverart/coverart.go new file mode 100644 index 0000000..21f46b5 --- /dev/null +++ b/backend/coverart/coverart.go @@ -0,0 +1,60 @@ +// Package coverart provides utilities for cover art filenames and URL resolution. +package coverart + +import ( + "fmt" + "path/filepath" + "strings" + + "yellowjacket/backend/system" +) + +// PathPrefix is the URL path prefix for cover art served by the asset handler. +const PathPrefix = "/covers/" + +// URLs holds the resolved URL paths for all cover art size variants. +type URLs struct { + Original string + Small string + Medium string + Large string +} + +// dirName is the subdirectory name under the user data directory +// where cover art files are stored. +const dirName = "covers" + +// CoversDir returns the absolute path to the cover art cache directory. +func CoversDir() (string, error) { + dataDir, err := system.GetUserDataDirPath() + if err != nil { + return "", fmt.Errorf( + "could not get user data directory: %w", err, + ) + } + + return filepath.Join(dataDir, dirName), nil +} + +// SizedFilename derives a sized-variant filename from an original cover art +// filename and a size suffix. +// For example, SizedFilename("a1b2c3d4.jpg", "_sm") returns "a1b2c3d4_sm.jpg". +func SizedFilename(originalFilename, suffix string) string { + ext := filepath.Ext(originalFilename) + name := strings.TrimSuffix(originalFilename, ext) + + return name + suffix + ".jpg" +} + +// ResolveURLs converts a cover art filesystem path into URL paths +// for the original and all size variants (small, medium, large). +func ResolveURLs(filesystemPath string) URLs { + base := filepath.Base(filesystemPath) + + return URLs{ + Original: PathPrefix + base, + Small: PathPrefix + SizedFilename(base, "_sm"), + Medium: PathPrefix + SizedFilename(base, "_md"), + Large: PathPrefix + SizedFilename(base, "_lg"), + } +} diff --git a/backend/coverart/coverart_test.go b/backend/coverart/coverart_test.go new file mode 100644 index 0000000..7e32f2b --- /dev/null +++ b/backend/coverart/coverart_test.go @@ -0,0 +1,171 @@ +package coverart_test + +import ( + "path/filepath" + "strings" + "testing" + + "yellowjacket/backend/coverart" +) + +func TestCoversDir(t *testing.T) { + t.Parallel() + + dir, err := coverart.CoversDir() + if err != nil { + t.Fatalf("CoversDir() returned error: %v", err) + } + + if dir == "" { + t.Fatal("CoversDir() returned empty string") + } + + // The path must end with the "covers" directory name. + if filepath.Base(dir) != "covers" { + t.Errorf( + "CoversDir() = %q, want path ending in %q", + dir, "covers", + ) + } + + // Must be an absolute path. + if !filepath.IsAbs(dir) { + t.Errorf("CoversDir() = %q, want absolute path", dir) + } + + // Must contain the app name somewhere in the path. + if !strings.Contains(dir, "yellowjacket") { + t.Errorf( + "CoversDir() = %q, expected to contain %q", + dir, "yellowjacket", + ) + } +} + +func TestSizedFilename(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + filename string + suffix string + want string + }{ + { + name: "jpg with _sm suffix", + filename: "a1b2c3d4.jpg", + suffix: "_sm", + want: "a1b2c3d4_sm.jpg", + }, + { + name: "jpg with _md suffix", + filename: "a1b2c3d4.jpg", + suffix: "_md", + want: "a1b2c3d4_md.jpg", + }, + { + name: "jpg with _lg suffix", + filename: "a1b2c3d4.jpg", + suffix: "_lg", + want: "a1b2c3d4_lg.jpg", + }, + { + name: "png source outputs jpg", + filename: "abcdef01.png", + suffix: "_sm", + want: "abcdef01_sm.jpg", + }, + { + name: "no extension", + filename: "abcdef01", + suffix: "_md", + want: "abcdef01_md.jpg", + }, + { + name: "empty suffix", + filename: "a1b2c3d4.jpg", + suffix: "", + want: "a1b2c3d4.jpg", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + got := coverart.SizedFilename(tt.filename, tt.suffix) + if got != tt.want { + t.Errorf( + "SizedFilename(%q, %q) = %q, want %q", + tt.filename, tt.suffix, got, tt.want, + ) + } + }) + } +} + +func TestResolveURLs(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + path string + wantOrig string + wantSm string + wantMd string + wantLg string + }{ + { + name: "absolute path", + path: "/home/user/.local/share/yellowjacket/covers/a1b2c3d4.jpg", + wantOrig: "/covers/a1b2c3d4.jpg", + wantSm: "/covers/a1b2c3d4_sm.jpg", + wantMd: "/covers/a1b2c3d4_md.jpg", + wantLg: "/covers/a1b2c3d4_lg.jpg", + }, + { + name: "bare filename", + path: "abcdef01.png", + wantOrig: "/covers/abcdef01.png", + wantSm: "/covers/abcdef01_sm.jpg", + wantMd: "/covers/abcdef01_md.jpg", + wantLg: "/covers/abcdef01_lg.jpg", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + urls := coverart.ResolveURLs(tt.path) + + if urls.Original != tt.wantOrig { + t.Errorf( + "Original = %q, want %q", + urls.Original, tt.wantOrig, + ) + } + + if urls.Small != tt.wantSm { + t.Errorf( + "Small = %q, want %q", + urls.Small, tt.wantSm, + ) + } + + if urls.Medium != tt.wantMd { + t.Errorf( + "Medium = %q, want %q", + urls.Medium, tt.wantMd, + ) + } + + if urls.Large != tt.wantLg { + t.Errorf( + "Large = %q, want %q", + urls.Large, tt.wantLg, + ) + } + }) + } +} diff --git a/backend/coverart/handler.go b/backend/coverart/handler.go new file mode 100644 index 0000000..5b1be5f --- /dev/null +++ b/backend/coverart/handler.go @@ -0,0 +1,51 @@ +package coverart + +import ( + "fmt" + "net/http" + "path/filepath" +) + +// Handler serves cover art images via HTTP. +type Handler struct { + coversDir string +} + +// NewHandler creates an HTTP handler that serves cover art from the +// user data directory. +func NewHandler() (*Handler, error) { + dir, err := CoversDir() + if err != nil { + return nil, fmt.Errorf( + "could not resolve covers directory: %w", err, + ) + } + + return &Handler{coversDir: dir}, nil +} + +// ServeHTTP handles requests for cover art images. +func (h *Handler) ServeHTTP( + w http.ResponseWriter, + r *http.Request, +) { + // Extract filename from path like "/covers/abc123.jpg". + filename := filepath.Base(r.URL.Path) + + // Prevent directory traversal. + if filename == "." || filename == ".." { + http.NotFound(w, r) + + return + } + + // Filenames are content-hashed (SHA-256), so they are immutable. + // Set aggressive cache headers to avoid redundant re-fetches. + w.Header().Set( + "Cache-Control", + "public, max-age=31536000, immutable", + ) + + filePath := filepath.Join(h.coversDir, filename) + http.ServeFile(w, r, filePath) +} diff --git a/backend/library/coverart.go b/backend/library/coverart.go index 9e53ee5..0bb4aea 100644 --- a/backend/library/coverart.go +++ b/backend/library/coverart.go @@ -15,8 +15,8 @@ import ( "golang.org/x/image/draw" + "yellowjacket/backend/coverart" "yellowjacket/backend/metadata" - "yellowjacket/backend/system" ) // thumbnailTier defines a single size tier for generated cover art thumbnails. @@ -80,16 +80,14 @@ func (l *Library) saveCoverArt( saveStart := time.Now() - // Get the data directory for storing cover art. - dataDir, err := system.GetUserDataDirPath() + // Get the covers directory for storing cover art. + coverDir, err := coverart.CoversDir() if err != nil { return "", fmt.Errorf( - "could not get user data directory: %w", err, + "could not resolve covers directory: %w", err, ) } - coverDir := filepath.Join(dataDir, "covers") - // Ensure directory exists. if err := os.MkdirAll(coverDir, 0o755); err != nil { return "", fmt.Errorf( @@ -324,15 +322,13 @@ func encodeAndSaveImage( // _thumb files to _md, and generates any missing sized variants for each // original cover art file. func (l *Library) generateMissingSizedVariants() error { - dataDir, err := system.GetUserDataDirPath() + coverDir, err := coverart.CoversDir() if err != nil { return fmt.Errorf( - "could not get user data directory: %w", err, + "could not resolve covers directory: %w", err, ) } - coverDir := filepath.Join(dataDir, "covers") - entries, err := os.ReadDir(coverDir) if err != nil { return fmt.Errorf( @@ -483,16 +479,6 @@ func (l *Library) migrateLegacyThumbs( return migrated } -// SizedFilename derives a sized-variant filename from an original cover art -// filename and a size suffix. -// For example, SizedFilename("a1b2c3d4.jpg", "_sm") returns "a1b2c3d4_sm.jpg". -func SizedFilename(originalFilename, suffix string) string { - ext := filepath.Ext(originalFilename) - name := strings.TrimSuffix(originalFilename, ext) - - return name + suffix + ".jpg" -} - // extensionFromMIME returns a file extension for common image MIME types. func extensionFromMIME(mimeType string) string { switch mimeType { diff --git a/backend/library/coverart_handler.go b/backend/library/coverart_handler.go deleted file mode 100644 index 0e1ffc3..0000000 --- a/backend/library/coverart_handler.go +++ /dev/null @@ -1,46 +0,0 @@ -package library - -import ( - "fmt" - "net/http" - "path/filepath" - - "yellowjacket/backend/system" -) - -// CoverArtHandler serves cover art images via HTTP. -type CoverArtHandler struct { - coversDir string -} - -// NewCoverArtHandler creates a handler that serves cover art from the user data directory. -func NewCoverArtHandler() (*CoverArtHandler, error) { - dataDir, err := system.GetUserDataDirPath() - if err != nil { - return nil, fmt.Errorf("could not get user data directory: %w", err) - } - - return &CoverArtHandler{ - coversDir: filepath.Join(dataDir, "covers"), - }, nil -} - -// ServeHTTP handles requests for cover art images. -func (h *CoverArtHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { - // Extract filename from path like "/covers/abc123.jpg" - filename := filepath.Base(r.URL.Path) - - // Prevent directory traversal - if filename == "." || filename == ".." { - http.NotFound(w, r) - - return - } - - // Filenames are content-hashed (SHA-256), so they are immutable. - // Set aggressive cache headers to avoid redundant re-fetches. - w.Header().Set("Cache-Control", "public, max-age=31536000, immutable") - - filePath := filepath.Join(h.coversDir, filename) - http.ServeFile(w, r, filePath) -} diff --git a/backend/library/query.go b/backend/library/query.go index 016916d..8738419 100644 --- a/backend/library/query.go +++ b/backend/library/query.go @@ -4,9 +4,10 @@ import ( "database/sql" "errors" "fmt" - "path/filepath" "strconv" "strings" + + "yellowjacket/backend/coverart" ) // Sentinel errors for library queries. @@ -258,14 +259,11 @@ func (l *Library) GetAllAlbums() ([]Album, error) { // Convert filesystem path to URL path for the asset handler. if row.CoverArtPath != "" { - base := filepath.Base(row.CoverArtPath) - album.CoverArtPath = "/covers/" + base - album.CoverArtSmall = "/covers/" + - SizedFilename(base, "_sm") - album.CoverArtMedium = "/covers/" + - SizedFilename(base, "_md") - album.CoverArtLarge = "/covers/" + - SizedFilename(base, "_lg") + urls := coverart.ResolveURLs(row.CoverArtPath) + album.CoverArtPath = urls.Original + album.CoverArtSmall = urls.Small + album.CoverArtMedium = urls.Medium + album.CoverArtLarge = urls.Large } albums = append(albums, album) @@ -345,14 +343,11 @@ func (l *Library) GetAlbumsByArtist( // Convert filesystem path to URL path for the asset handler. if row.CoverArtPath != "" { - base := filepath.Base(row.CoverArtPath) - album.CoverArtPath = "/covers/" + base - album.CoverArtSmall = "/covers/" + - SizedFilename(base, "_sm") - album.CoverArtMedium = "/covers/" + - SizedFilename(base, "_md") - album.CoverArtLarge = "/covers/" + - SizedFilename(base, "_lg") + urls := coverart.ResolveURLs(row.CoverArtPath) + album.CoverArtPath = urls.Original + album.CoverArtSmall = urls.Small + album.CoverArtMedium = urls.Medium + album.CoverArtLarge = urls.Large } albums = append(albums, album) diff --git a/backend/library/rescan.go b/backend/library/rescan.go index 3e4e20d..316d443 100644 --- a/backend/library/rescan.go +++ b/backend/library/rescan.go @@ -6,7 +6,7 @@ import ( "path/filepath" "time" - "yellowjacket/backend/system" + "yellowjacket/backend/coverart" ) // FullRescan clears the queue and player, wipes all library data @@ -182,15 +182,13 @@ func (l *Library) clearLibraryTables() error { // clearCoverArtFiles removes all files from the covers directory. func (l *Library) clearCoverArtFiles() error { - dataDir, err := system.GetUserDataDirPath() + coverDir, err := coverart.CoversDir() if err != nil { return fmt.Errorf( - "could not get user data directory: %w", err, + "could not resolve covers directory: %w", err, ) } - coverDir := filepath.Join(dataDir, "covers") - entries, err := os.ReadDir(coverDir) if err != nil { if os.IsNotExist(err) { diff --git a/backend/player/player.go b/backend/player/player.go index f2cb98a..cb49bae 100644 --- a/backend/player/player.go +++ b/backend/player/player.go @@ -18,10 +18,10 @@ import ( "github.com/TheCodeOfCaleb/beep/v2/speaker" "github.com/wailsapp/wails/v2/pkg/runtime" + "yellowjacket/backend/coverart" "yellowjacket/backend/database" "yellowjacket/backend/database/sql/sqlcgen" "yellowjacket/backend/events" - "yellowjacket/backend/library" "yellowjacket/backend/metadata" "yellowjacket/backend/profiling" ) @@ -868,14 +868,11 @@ func (p *Player) getCurrentTrackInfoLocked() TrackInfo { info.Album = meta.Album if meta.CoverArtPath != "" { - base := filepath.Base(meta.CoverArtPath) - info.CoverArt = "/covers/" + base - info.CoverArtSmall = "/covers/" + - library.SizedFilename(base, "_sm") - info.CoverArtMedium = "/covers/" + - library.SizedFilename(base, "_md") - info.CoverArtLarge = "/covers/" + - library.SizedFilename(base, "_lg") + urls := coverart.ResolveURLs(meta.CoverArtPath) + info.CoverArt = urls.Original + info.CoverArtSmall = urls.Small + info.CoverArtMedium = urls.Medium + info.CoverArtLarge = urls.Large } } else { p.logger.Debug( diff --git a/backend/playlist/playlist.go b/backend/playlist/playlist.go index 95c6532..35a4811 100644 --- a/backend/playlist/playlist.go +++ b/backend/playlist/playlist.go @@ -14,10 +14,10 @@ import ( "github.com/wailsapp/wails/v2/pkg/runtime" + "yellowjacket/backend/coverart" "yellowjacket/backend/database" "yellowjacket/backend/database/sql/sqlcgen" "yellowjacket/backend/events" - "yellowjacket/backend/library" "yellowjacket/backend/system" ) @@ -380,14 +380,11 @@ func trackFromRow( } if coverArtPath != "" { - base := filepath.Base(coverArtPath) - track.CoverArtPath = "/covers/" + base - track.CoverArtSmall = "/covers/" + - library.SizedFilename(base, "_sm") - track.CoverArtMedium = "/covers/" + - library.SizedFilename(base, "_md") - track.CoverArtLarge = "/covers/" + - library.SizedFilename(base, "_lg") + urls := coverart.ResolveURLs(coverArtPath) + track.CoverArtPath = urls.Original + track.CoverArtSmall = urls.Small + track.CoverArtMedium = urls.Medium + track.CoverArtLarge = urls.Large } return track