From 3779053951f338076fcc448daf9bee9ada18cf54 Mon Sep 17 00:00:00 2001 From: Paul Buetow Date: Tue, 19 May 2026 19:23:50 +0300 Subject: Remove nil streamer fallback in api.serveFileResult (DIP) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- player-server/internal/api/handlers_test.go | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) (limited to 'player-server/internal/api/handlers_test.go') 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, -- cgit v1.2.3