summaryrefslogtreecommitdiff
path: root/player-server
diff options
context:
space:
mode:
authorPaul Buetow <paul@buetow.org>2026-05-19 19:23:50 +0300
committerPaul Buetow <paul@buetow.org>2026-05-19 19:23:50 +0300
commit3779053951f338076fcc448daf9bee9ada18cf54 (patch)
tree50bbb2ccb68d9eab90b0ffdd5897560daec0bd17 /player-server
parent3ce06a482fafd884013c9a7a827d9ccb183a05da (diff)
Remove nil streamer fallback in api.serveFileResult (DIP)
The serveFileResult handler used to fabricate a default MediaStreamer at request time when s.streamer was nil. That silently masked wiring mistakes and violated the Dependency Inversion Principle — the handler was inventing its own dependency instead of demanding one from the caller. Removed the fallback so the handler now uses s.streamer directly. Construction-time validation of deps.MediaStreamer was added to api.NewServerWithLogger in commit 4215db6, which makes the runtime nil case unreachable. Mirrors the explicit-deps pattern set in commits 622827c (http.Client in podcast service) and 92edb83 (TokenManager in auth service). Test helpers updated to inject a service.NewMediaStreamer(nil) when one isn't otherwise supplied so the existing suite still constructs Servers through the validating constructor. Refs agent task 6a. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Diffstat (limited to 'player-server')
-rw-r--r--player-server/internal/api/handlers.go8
-rw-r--r--player-server/internal/api/handlers_playback_test.go3
-rw-r--r--player-server/internal/api/handlers_podcast_test.go3
-rw-r--r--player-server/internal/api/handlers_test.go14
-rw-r--r--player-server/internal/api/integration_test.go3
5 files changed, 22 insertions, 9 deletions
diff --git a/player-server/internal/api/handlers.go b/player-server/internal/api/handlers.go
index a9c832f..f04ec19 100644
--- a/player-server/internal/api/handlers.go
+++ b/player-server/internal/api/handlers.go
@@ -162,10 +162,12 @@ func (s *Server) serveDetach(w http.ResponseWriter, r *http.Request) {
// ------------------------------------------------------------------
func (s *Server) serveFileResult(w http.ResponseWriter, r *http.Request, res *service.FileResult, attachment bool) {
+ // s.streamer is required at construction time (see NewServerWithLogger),
+ // so it is guaranteed non-nil here. We previously fell back to a default
+ // streamer when nil, which silently hid wiring mistakes and violated the
+ // Dependency Inversion Principle by letting the handler decide its own
+ // dependency.
streamer := s.streamer
- if streamer == nil {
- streamer = service.NewMediaStreamer(nil)
- }
stream, err := streamer.Open(r.Context(), res, attachment)
if err != nil {
diff --git a/player-server/internal/api/handlers_playback_test.go b/player-server/internal/api/handlers_playback_test.go
index 7febab0..869b0d1 100644
--- a/player-server/internal/api/handlers_playback_test.go
+++ b/player-server/internal/api/handlers_playback_test.go
@@ -38,7 +38,8 @@ func newPlaybackTestServer(t *testing.T, store repository.Store, sm auth.Session
Auth: authSvc,
PlaybackHints: hintSvc,
},
- StaticFS: fs,
+ StaticFS: fs,
+ MediaStreamer: service.NewMediaStreamer(nil),
})
if err != nil {
t.Fatalf("NewServer: %v", err)
diff --git a/player-server/internal/api/handlers_podcast_test.go b/player-server/internal/api/handlers_podcast_test.go
index 4138d97..7dcdfe6 100644
--- a/player-server/internal/api/handlers_podcast_test.go
+++ b/player-server/internal/api/handlers_podcast_test.go
@@ -68,7 +68,8 @@ func newPodcastTestServer(t *testing.T, store repository.Store, hasher auth.Hash
Auth: authSvc,
Podcast: podcastSvc,
},
- StaticFS: fs,
+ StaticFS: fs,
+ MediaStreamer: service.NewMediaStreamer(nil),
})
if err != nil {
t.Fatalf("NewServer: %v", err)
diff --git a/player-server/internal/api/handlers_test.go b/player-server/internal/api/handlers_test.go
index 6279895..5fbd4c9 100644
--- a/player-server/internal/api/handlers_test.go
+++ b/player-server/internal/api/handlers_test.go
@@ -65,9 +65,17 @@ func newTestServer(t *testing.T, store repository.Store, hasher auth.Hasher, sm
if len(streamer) > 0 {
mediaStreamer = streamer[0]
}
- // NewServer now returns an error when required deps (e.g. Config) are
- // missing. Tests always pass a non-nil Config, so a failure here indicates
- // a programming mistake in the test setup itself.
+ if mediaStreamer == nil {
+ // NewServer now requires a non-nil MediaStreamer at construction
+ // (see api.NewServerWithLogger). Tests that don't exercise streaming
+ // still need one, so we fall back to the default in-process streamer
+ // (nil remuxer means remux requests will error, which is fine for
+ // non-streaming tests).
+ mediaStreamer = service.NewMediaStreamer(nil)
+ }
+ // NewServer now returns an error when required deps (e.g. Config,
+ // MediaStreamer) are missing. Tests always pass non-nil values, so a
+ // failure here indicates a programming mistake in the test setup itself.
srv, err := NewServer(ServerDeps{
Store: store,
Hasher: hasher,
diff --git a/player-server/internal/api/integration_test.go b/player-server/internal/api/integration_test.go
index 2f51bd2..9344552 100644
--- a/player-server/internal/api/integration_test.go
+++ b/player-server/internal/api/integration_test.go
@@ -82,7 +82,8 @@ func newIntegrationServer(t *testing.T) *integrationEnv {
Auth: authSvc,
Podcast: &integrationPodcastService{},
},
- StaticFS: http.FS(staticFS),
+ StaticFS: http.FS(staticFS),
+ MediaStreamer: service.NewMediaStreamer(nil),
})
if err != nil {
t.Fatalf("NewServer: %v", err)