extracted cover art handling from library package
This commit is contained in:
@@ -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.
|
||||
|
||||
---
|
||||
|
||||
|
||||
+3
-2
@@ -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(
|
||||
|
||||
@@ -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"),
|
||||
}
|
||||
}
|
||||
@@ -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,
|
||||
)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
@@ -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)
|
||||
}
|
||||
@@ -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 {
|
||||
|
||||
@@ -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)
|
||||
}
|
||||
+12
-17
@@ -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)
|
||||
|
||||
@@ -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) {
|
||||
|
||||
@@ -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(
|
||||
|
||||
@@ -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
|
||||
|
||||
Reference in New Issue
Block a user