summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
Diffstat
-rw-r--r--player-server/cmd/player/main.go6
-rw-r--r--player-server/internal/api/handlers.go38
-rw-r--r--player-server/internal/scanner/scanner.go65
-rw-r--r--player-server/internal/scanner/scanner_test.go23
-rw-r--r--player-server/internal/service/service.go54
-rw-r--r--player-server/internal/thumb/maker.go104
-rw-r--r--player-server/internal/thumb/maker_test.go102
7 files changed, 322 insertions, 70 deletions
diff --git a/player-server/cmd/player/main.go b/player-server/cmd/player/main.go
index 1cfb4f7..3e8046f 100644
--- a/player-server/cmd/player/main.go
+++ b/player-server/cmd/player/main.go
@@ -94,13 +94,17 @@ func wireDeps(cfg *internal.Config, store repository.Store, logger *slog.Logger,
// free of direct os.Stat calls and makes the dependency easy to swap
// out in tests or alternate deployments (e.g. object storage).
thumbResolver := thumb.NewFSResolver()
+ // thumb.FSMaker encapsulates the "create .thumbnails dir + invoke
+ // generator + warn-on-failure" policy so the scanner only
+ // orchestrates the scan and does not own thumbnail layout policy.
+ thumbMaker := thumb.NewFSMaker(thumbGen, nil, logger)
helper := service.NewAccessHelper(store)
browser := service.NewPodcastBrowseService(store, cfg.MediaRoot)
mediaSvc := service.NewMediaServiceWithDeps(store, clk, cfg.MediaRoot, thumbGen, prober, browser, thumbResolver)
playbackHintSvc := service.NewPlaybackHintsService(helper)
- fsScanner := scanner.NewFSScannerWithLogger(store, prober, thumbGen, clk, cfg.MediaRoot, logger)
+ fsScanner := scanner.NewFSScannerWithMaker(store, prober, thumbMaker, clk, cfg.MediaRoot, logger)
adminSvc := service.NewAdminServiceWithLogger(store, clk, hasher, fsScanner, cfg.MediaRoot, appCtx, logger)
progressSvc := service.NewProgressService(store, clk)
diff --git a/player-server/internal/api/handlers.go b/player-server/internal/api/handlers.go
index 08fdbeb..9cffa50 100644
--- a/player-server/internal/api/handlers.go
+++ b/player-server/internal/api/handlers.go
@@ -65,28 +65,26 @@ func forbidden(w http.ResponseWriter, message string) {
writeError(w, http.StatusForbidden, message)
}
-// handleError maps service sentinel errors to the appropriate HTTP status
-// and writes a JSON error response. It falls back to 500 for unknown errors.
+// HTTPStatuser is implemented by service errors that know their own HTTP
+// status. Sentinels in internal/service implement this so handleError can
+// dispatch without an ever-growing switch (OCP): adding a new sentinel only
+// requires defining its status alongside the sentinel itself, with no edit
+// required here.
+type HTTPStatuser interface {
+ HTTPStatus() int
+}
+
+// handleError dispatches service errors to an HTTP response. If any error in
+// the chain implements HTTPStatuser, that status is used together with the
+// wrapped error's message (so callers that add context via fmt.Errorf("%w: …")
+// keep that context in the body). Unrecognised errors fall back to 500.
func handleError(w http.ResponseWriter, err error) {
- switch {
- case errors.Is(err, service.ErrNotFound),
- errors.Is(err, service.ErrShareNotFound),
- errors.Is(err, service.ErrMediaNotFound):
- notFound(w)
- case errors.Is(err, service.ErrForbidden):
- forbidden(w, "forbidden")
- case errors.Is(err, service.ErrAlreadyBootstrapped):
- forbidden(w, "bootstrap already complete")
- case errors.Is(err, service.ErrInvalidCredentials):
- writeJSON(w, http.StatusUnauthorized, map[string]string{"error": "invalid credentials"})
- case errors.Is(err, service.ErrUnsupportedExtension),
- errors.Is(err, service.ErrInvalidFeed),
- errors.Is(err, service.ErrCannotDeleteSelf),
- errors.Is(err, service.ErrEmptySetForCover):
- badRequest(w, err.Error())
- default:
- writeJSON(w, http.StatusInternalServerError, map[string]string{"error": err.Error()})
+ var statuser HTTPStatuser
+ if errors.As(err, &statuser) {
+ writeError(w, statuser.HTTPStatus(), err.Error())
+ return
}
+ writeJSON(w, http.StatusInternalServerError, map[string]string{"error": err.Error()})
}
func readJSON(r *http.Request, dst interface{}) error {
diff --git a/player-server/internal/scanner/scanner.go b/player-server/internal/scanner/scanner.go
index 0f6e50b..a669a6b 100644
--- a/player-server/internal/scanner/scanner.go
+++ b/player-server/internal/scanner/scanner.go
@@ -26,10 +26,15 @@ type Scanner interface {
}
// FSScanner recursively scans media root for sets and media files.
+//
+// The scanner does not own thumbnail policy: it delegates path derivation,
+// directory creation, generator invocation, and failure-tolerant warning
+// to a thumb.Maker. This keeps SRP intact — FSScanner orchestrates the
+// scan, thumb.Maker decides how thumbnails get produced on disk.
type FSScanner struct {
store repository.ScannerStore
prober probe.Prober
- thumbGen thumb.Generator
+ thumbMkr thumb.Maker
clock clock.Clock
mediaRoot string
fs FS
@@ -38,19 +43,35 @@ type FSScanner struct {
}
// NewFSScanner creates a filesystem scanner with injected dependencies.
+// A default thumb.FSMaker is constructed from thumbGen so existing callers
+// keep working without having to know about the Maker interface.
func NewFSScanner(store repository.ScannerStore, prober probe.Prober, thumbGen thumb.Generator, clk clock.Clock, mediaRoot string) *FSScanner {
return NewFSScannerWithLogger(store, prober, thumbGen, clk, mediaRoot, slog.Default())
}
// NewFSScannerWithLogger creates a filesystem scanner with an injected logger.
+// Like NewFSScanner this wraps thumbGen in a default thumb.FSMaker; callers
+// that want a custom Maker should use NewFSScannerWithMaker instead.
func NewFSScannerWithLogger(store repository.ScannerStore, prober probe.Prober, thumbGen thumb.Generator, clk clock.Clock, mediaRoot string, logger *slog.Logger) *FSScanner {
if logger == nil {
logger = slog.Default()
}
+ maker := thumb.NewFSMaker(thumbGen, nil, logger)
+ return NewFSScannerWithMaker(store, prober, maker, clk, mediaRoot, logger)
+}
+
+// NewFSScannerWithMaker creates a filesystem scanner with an explicit
+// thumb.Maker. Production wiring (cmd/player/main.go) prefers this form
+// so the Maker can be constructed once and shared with any other
+// component that needs to produce thumbnails consistently.
+func NewFSScannerWithMaker(store repository.ScannerStore, prober probe.Prober, maker thumb.Maker, clk clock.Clock, mediaRoot string, logger *slog.Logger) *FSScanner {
+ if logger == nil {
+ logger = slog.Default()
+ }
return &FSScanner{
store: store,
prober: prober,
- thumbGen: thumbGen,
+ thumbMkr: maker,
clock: clk,
mediaRoot: mediaRoot,
fs: osFS{},
@@ -204,43 +225,15 @@ func (s *FSScanner) gatherCoverImages(setPath string) map[string]string {
return coverImages
}
-// thumbnailForVideo generates a thumbnail for a video file inside the set's .thumbnails directory.
-// The destination directory + filename are derived via internal/thumb so the
-// layout convention stays in lock-step with importers (service.generateThumbnail,
-// writeService.RegenerateThumbnail).
-func (s *FSScanner) thumbnailForVideo(ctx context.Context, path, setPath string, duration float64) (string, error) {
- thumbDir := thumb.ThumbnailDir(setPath)
- if err := s.fs.MkdirAll(thumbDir, 0o755); err != nil {
- return "", fmt.Errorf("mkdir thumbnails %q: %w", thumbDir, err)
- }
- thumbnailPath := thumb.ThumbnailPathFor(path, setPath)
- if err := s.thumbGen.Generate(ctx, path, thumbnailPath, duration); err != nil {
- s.log().Warn("scanner skipping thumbnail", "path", path, "err", err)
- return "", nil
- }
- return thumbnailPath, nil
-}
-
-// thumbnailForImage generates a thumbnail for an image file inside the set's .thumbnails directory.
-// See thumbnailForVideo for the shared path-derivation contract.
-func (s *FSScanner) thumbnailForImage(ctx context.Context, path, setPath string) (string, error) {
- thumbDir := thumb.ThumbnailDir(setPath)
- if err := s.fs.MkdirAll(thumbDir, 0o755); err != nil {
- return "", fmt.Errorf("mkdir thumbnails %q: %w", thumbDir, err)
- }
- thumbnailPath := thumb.ThumbnailPathFor(path, setPath)
- if err := s.thumbGen.Generate(ctx, path, thumbnailPath, 0); err != nil {
- s.log().Warn("scanner skipping thumbnail", "path", path, "err", err)
- return "", nil
- }
- return thumbnailPath, nil
-}
-
// buildThumbnailPath resolves the thumbnail path for a new media file.
+// Video and image thumbnails are produced via thumb.Maker so the scanner
+// stays out of mkdir / path-derivation / generator policy. Audio uses a
+// nearby cover image when one was discovered during gatherCoverImages.
+// SVG images are served as-is (vector — no raster thumbnail makes sense).
func (s *FSScanner) buildThumbnailPath(ctx context.Context, path, setPath string, mediaType model.MediaType, coverImages map[string]string, meta *model.Metadata) (string, error) {
switch mediaType {
case model.MediaTypeVideo:
- return s.thumbnailForVideo(ctx, path, setPath, meta.Duration)
+ return s.thumbMkr.MakeVideo(ctx, path, setPath, meta.Duration)
case model.MediaTypeAudio:
return findCoverImage(path, coverImages, setPath), nil
case model.MediaTypeImage:
@@ -248,7 +241,7 @@ func (s *FSScanner) buildThumbnailPath(ctx context.Context, path, setPath string
if ext == ".svg" {
return path, nil
}
- thumbPath, err := s.thumbnailForImage(ctx, path, setPath)
+ thumbPath, err := s.thumbMkr.MakeImage(ctx, path, setPath)
if err != nil {
return "", err
}
diff --git a/player-server/internal/scanner/scanner_test.go b/player-server/internal/scanner/scanner_test.go
index 0738709..853bd89 100644
--- a/player-server/internal/scanner/scanner_test.go
+++ b/player-server/internal/scanner/scanner_test.go
@@ -111,16 +111,37 @@ func (m *mockFS) WalkDir(root string, walkFn fs.WalkDirFunc) error {
return nil
}
+// newTestScanner builds an FSScanner around the in-memory mockFS while
+// wrapping the test Generator in a thumb.FSMaker. The Maker is configured
+// with the same mockFS so MkdirAll respects the test's injected error
+// (mfs.mkdirErr), preserving the previous behaviour where the scanner
+// itself called fs.MkdirAll directly.
func newTestScanner(store repository.ScannerStore, prober probe.Prober, gen thumb.Generator, clk clock.Clock, filesystem FS) *FSScanner {
+ var maker thumb.Maker
+ if gen != nil {
+ maker = thumb.NewFSMaker(gen, makerFSFromScannerFS{fs: filesystem}, nil)
+ }
return &FSScanner{
store: store,
prober: prober,
- thumbGen: gen,
+ thumbMkr: maker,
clock: clk,
fs: filesystem,
}
}
+// makerFSFromScannerFS adapts the scanner test's FS to thumb.MakerFS so the
+// Maker's MkdirAll honours the same in-memory error injection the scanner
+// previously honoured directly.
+type makerFSFromScannerFS struct{ fs FS }
+
+func (a makerFSFromScannerFS) MkdirAll(path string, perm os.FileMode) error {
+ if a.fs == nil {
+ return nil
+ }
+ return a.fs.MkdirAll(path, perm)
+}
+
func TestFSScanner_Scan(t *testing.T) {
now := time.Date(2024, 1, 1, 0, 0, 0, 0, time.UTC)
clk := &clock.MockClock{T: now}
diff --git a/player-server/internal/service/service.go b/player-server/internal/service/service.go
index b4ae127..beeb5aa 100644
--- a/player-server/internal/service/service.go
+++ b/player-server/internal/service/service.go
@@ -5,24 +5,54 @@ import (
"context"
"errors"
"io"
+ "net/http"
"time"
"codeberg.org/snonux/player/internal/model"
)
-// Sentinel errors returned by the service layer.
+// apiError is a sentinel error that knows its own HTTP status. It implements
+// the api.HTTPStatuser interface (which is satisfied structurally — no import
+// of the api package is needed here) so that api.handleError can dispatch
+// based on the sentinel's own metadata instead of an ever-growing switch.
+//
+// Sentinels are declared as *apiError pointers, which makes errors.Is work via
+// pointer equality even when the error is wrapped with fmt.Errorf("%w: …").
+type apiError struct {
+ msg string
+ status int
+}
+
+// Error returns the sentinel's human-readable message.
+func (e *apiError) Error() string { return e.msg }
+
+// HTTPStatus returns the HTTP status code this sentinel should map to.
+func (e *apiError) HTTPStatus() int { return e.status }
+
+// Sentinel errors returned by the service layer. Each one carries the HTTP
+// status that the API layer should emit, so handleError stays open for
+// extension (new sentinel) but closed for modification.
+//
+// Where the previous handleError emitted a fixed message that differed from
+// the sentinel's text (e.g. "forbidden" instead of "access denied"), the
+// sentinel text now matches that message so the dispatcher can use the
+// error chain's own text uniformly.
var (
- ErrNotFound = errors.New("not found")
- ErrForbidden = errors.New("access denied")
- ErrShareNotFound = errors.New("share not found")
- ErrShareExpired = errors.New("share expired")
- ErrMediaNotFound = errors.New("media not found")
- ErrUnsupportedExtension = errors.New("unsupported file extension")
- ErrEmptySetForCover = errors.New("no media files available for cover")
- ErrAlreadyBootstrapped = errors.New("already bootstrapped")
- ErrInvalidCredentials = errors.New("invalid credentials")
- ErrInvalidFeed = errors.New("invalid feed")
- ErrCannotDeleteSelf = errors.New("cannot delete self")
+ ErrNotFound = &apiError{msg: "not found", status: http.StatusNotFound}
+ ErrForbidden = &apiError{msg: "forbidden", status: http.StatusForbidden}
+ ErrShareNotFound = &apiError{msg: "share not found", status: http.StatusNotFound}
+ ErrMediaNotFound = &apiError{msg: "media not found", status: http.StatusNotFound}
+ ErrUnsupportedExtension = &apiError{msg: "unsupported file extension", status: http.StatusBadRequest}
+ ErrEmptySetForCover = &apiError{msg: "no media files available for cover", status: http.StatusBadRequest}
+ ErrAlreadyBootstrapped = &apiError{msg: "bootstrap already complete", status: http.StatusForbidden}
+ ErrInvalidCredentials = &apiError{msg: "invalid credentials", status: http.StatusUnauthorized}
+ ErrInvalidFeed = &apiError{msg: "invalid feed", status: http.StatusBadRequest}
+ ErrCannotDeleteSelf = &apiError{msg: "cannot delete self", status: http.StatusBadRequest}
+
+ // ErrShareExpired is handled directly by share handlers (not via
+ // handleError); it stays a plain sentinel because no dispatch metadata
+ // is needed.
+ ErrShareExpired = errors.New("share expired")
)
// MediaQueryFilter defines query parameters for listing media from the API layer.
diff --git a/player-server/internal/thumb/maker.go b/player-server/internal/thumb/maker.go
new file mode 100644
index 0000000..52b15db
--- /dev/null
+++ b/player-server/internal/thumb/maker.go
@@ -0,0 +1,104 @@
+package thumb
+
+import (
+ "context"
+ "fmt"
+ "log/slog"
+ "os"
+)
+
+// Maker creates thumbnail files for media items. The scanner uses it to
+// off-load thumbnail creation policy (mkdir of the .thumbnails directory,
+// canonical thumbnail path derivation, generator invocation, and the
+// failure-tolerant warn-and-continue behaviour) so the scanner only
+// orchestrates the higher-level scan and does not own thumbnail policy.
+//
+// MakeVideo / MakeImage return the resolved thumbnail path on success, or
+// an empty string (and nil error) when the underlying generator failed —
+// matching the scanner's pre-extraction "skip-and-continue" semantics.
+// A non-nil error is reserved for genuine filesystem problems such as
+// failing to create the .thumbnails directory.
+type Maker interface {
+ // MakeVideo produces a thumbnail for a video file. duration is the
+ // video duration in seconds and is forwarded to the underlying
+ // Generator so it can pick a sensible frame.
+ MakeVideo(ctx context.Context, srcPath, parent string, duration float64) (string, error)
+ // MakeImage produces a thumbnail for an image file. No duration is
+ // applicable so 0 is passed to the underlying Generator.
+ MakeImage(ctx context.Context, srcPath, parent string) (string, error)
+}
+
+// MakerFS is the small filesystem surface FSMaker needs. It mirrors the
+// scanner's FS interface for the single operation FSMaker performs
+// (creating the .thumbnails directory) so tests can inject an in-memory
+// fake without depending on the scanner package.
+type MakerFS interface {
+ MkdirAll(path string, perm os.FileMode) error
+}
+
+// osMakerFS delegates MkdirAll to the standard library; it is used as
+// the default when NewFSMaker is called without an explicit MakerFS.
+type osMakerFS struct{}
+
+func (osMakerFS) MkdirAll(path string, perm os.FileMode) error {
+ return os.MkdirAll(path, perm)
+}
+
+// FSMaker is the production Maker implementation. It wraps a Generator
+// (which performs the actual frame extraction) together with a small
+// filesystem dependency for directory creation, and a logger so the
+// "skipped thumbnail" warning is consistent with the scanner's previous
+// behaviour.
+type FSMaker struct {
+ gen Generator
+ fs MakerFS
+ logger *slog.Logger
+}
+
+var _ Maker = (*FSMaker)(nil)
+
+// NewFSMaker constructs an FSMaker around gen. A nil fs defaults to the
+// real OS filesystem; a nil logger defaults to slog.Default(). gen must
+// not be nil — callers always have a Generator available in production
+// and tests can pass MockGenerator.
+func NewFSMaker(gen Generator, fs MakerFS, logger *slog.Logger) *FSMaker {
+ if fs == nil {
+ fs = osMakerFS{}
+ }
+ if logger == nil {
+ logger = slog.Default()
+ }
+ return &FSMaker{gen: gen, fs: fs, logger: logger}
+}
+
+// MakeVideo creates a thumbnail for a video file inside parent/.thumbnails/.
+// The path layout mirrors ThumbnailPathFor so importers, the scanner, and
+// the resolver all agree on where thumbnails live. Generator failures are
+// logged and swallowed so a single bad file does not abort the scan.
+func (m *FSMaker) MakeVideo(ctx context.Context, srcPath, parent string, duration float64) (string, error) {
+ return m.make(ctx, srcPath, parent, duration)
+}
+
+// MakeImage creates a thumbnail for an image file. Duration is irrelevant
+// for static images so 0 is forwarded to the Generator.
+func (m *FSMaker) MakeImage(ctx context.Context, srcPath, parent string) (string, error) {
+ return m.make(ctx, srcPath, parent, 0)
+}
+
+// make is the shared implementation behind MakeVideo / MakeImage. It
+// ensures the .thumbnails directory exists, derives the canonical
+// thumbnail path via ThumbnailPathFor, and delegates the actual frame
+// extraction to the Generator. Generator errors are logged and reported
+// as ("", nil) so the scanner can continue with the next file.
+func (m *FSMaker) make(ctx context.Context, srcPath, parent string, duration float64) (string, error) {
+ dir := ThumbnailDir(parent)
+ if err := m.fs.MkdirAll(dir, 0o755); err != nil {
+ return "", fmt.Errorf("mkdir thumbnails %q: %w", dir, err)
+ }
+ thumbnailPath := ThumbnailPathFor(srcPath, parent)
+ if err := m.gen.Generate(ctx, srcPath, thumbnailPath, duration); err != nil {
+ m.logger.Warn("thumb maker skipping thumbnail", "path", srcPath, "err", err)
+ return "", nil
+ }
+ return thumbnailPath, nil
+}
diff --git a/player-server/internal/thumb/maker_test.go b/player-server/internal/thumb/maker_test.go
new file mode 100644
index 0000000..cf99c67
--- /dev/null
+++ b/player-server/internal/thumb/maker_test.go
@@ -0,0 +1,102 @@
+package thumb
+
+import (
+ "context"
+ "errors"
+ "os"
+ "path/filepath"
+ "testing"
+)
+
+// recordingFS is a tiny MakerFS used to confirm FSMaker invokes MkdirAll
+// with the canonical .thumbnails directory and surfaces filesystem errors.
+type recordingFS struct {
+ calls []string
+ mkdirErr error
+}
+
+func (r *recordingFS) MkdirAll(path string, _ os.FileMode) error {
+ r.calls = append(r.calls, path)
+ return r.mkdirErr
+}
+
+func TestFSMaker_MakeVideo_DelegatesToGenerator(t *testing.T) {
+ var gotInput, gotOutput string
+ var gotDuration float64
+ gen := &MockGenerator{
+ GenerateFunc: func(_ context.Context, in, out string, dur float64) error {
+ gotInput = in
+ gotOutput = out
+ gotDuration = dur
+ return nil
+ },
+ }
+ fs := &recordingFS{}
+ m := NewFSMaker(gen, fs, nil)
+
+ got, err := m.MakeVideo(context.Background(), "/set/movie.mp4", "/set", 12.5)
+ if err != nil {
+ t.Fatalf("unexpected error: %v", err)
+ }
+ wantPath := filepath.Join("/set", DirName, "movie.jpg")
+ if got != wantPath {
+ t.Fatalf("path = %q, want %q", got, wantPath)
+ }
+ if gotInput != "/set/movie.mp4" {
+ t.Fatalf("input = %q", gotInput)
+ }
+ if gotOutput != wantPath {
+ t.Fatalf("output = %q, want %q", gotOutput, wantPath)
+ }
+ if gotDuration != 12.5 {
+ t.Fatalf("duration = %v", gotDuration)
+ }
+ if len(fs.calls) != 1 || fs.calls[0] != filepath.Join("/set", DirName) {
+ t.Fatalf("mkdir calls = %v", fs.calls)
+ }
+}
+
+func TestFSMaker_MakeImage_PassesZeroDuration(t *testing.T) {
+ var gotDuration float64
+ gen := &MockGenerator{
+ GenerateFunc: func(_ context.Context, _, _ string, dur float64) error {
+ gotDuration = dur
+ return nil
+ },
+ }
+ m := NewFSMaker(gen, &recordingFS{}, nil)
+
+ if _, err := m.MakeImage(context.Background(), "/p/img.png", "/p"); err != nil {
+ t.Fatalf("unexpected error: %v", err)
+ }
+ if gotDuration != 0 {
+ t.Fatalf("duration = %v, want 0", gotDuration)
+ }
+}
+
+func TestFSMaker_GeneratorError_SwallowedAsEmptyPath(t *testing.T) {
+ gen := &MockGenerator{
+ GenerateFunc: func(_ context.Context, _, _ string, _ float64) error {
+ return errors.New("ffmpeg boom")
+ },
+ }
+ m := NewFSMaker(gen, &recordingFS{}, nil)
+
+ got, err := m.MakeVideo(context.Background(), "/s/v.mp4", "/s", 1)
+ if err != nil {
+ t.Fatalf("expected generator error to be swallowed, got %v", err)
+ }
+ if got != "" {
+ t.Fatalf("expected empty path on generator failure, got %q", got)
+ }
+}
+
+func TestFSMaker_MkdirErrorPropagates(t *testing.T) {
+ fs := &recordingFS{mkdirErr: errors.New("readonly fs")}
+ m := NewFSMaker(&MockGenerator{}, fs, nil)
+
+ _, err := m.MakeImage(context.Background(), "/s/x.jpg", "/s")
+ if err == nil {
+ t.Fatal("expected mkdir error to propagate")
+ }
+}
generated by cgit v1.2.3 (git 2.25.1) at 2026-08-07 05:15:54 +0000