0.2.57 - Traliers bug fixes

server/internal/api/api.go

@@ -35,7 +35,6 @@ import (
 	"github.com/ponzischeme89/memby/server/internal/recommend"
 	"github.com/ponzischeme89/memby/server/internal/sonarr"
 	"github.com/ponzischeme89/memby/server/internal/store"
-	"github.com/ponzischeme89/memby/server/internal/trailer"
 )
 
 type Server struct {
@@ -86,7 +85,6 @@ type Server struct {
 	// embyHealth is the reachability probe's live finding, which /v1/status publishes so
 	// a TV can show why playback stopped even if it missed the announcement.
 	embyHealth embyHealth
-	trailers   *trailer.Resolver
 }
 
 // Deps are the collaborators the API needs. A struct rather than positional arguments:
@@ -107,10 +105,6 @@ type Deps struct {
 }
 
 func New(cfg config.Config, deps Deps) *Server {
-	trailerTimeout := cfg.UpstreamTimeout
-	if trailerTimeout <= 0 || trailerTimeout > 8*time.Second {
-		trailerTimeout = 8 * time.Second
-	}
 	return &Server{
 		cfg:         cfg,
 		emby:        deps.Emby,
@@ -125,7 +119,6 @@ func New(cfg config.Config, deps Deps) *Server {
 		syncer:      deps.Syncer,
 		log:         deps.Log,
 		events:      deps.Events,
-		trailers:    trailer.New(&http.Client{Timeout: trailerTimeout}),
 	}
 }
 
@@ -201,6 +194,7 @@ func (s *Server) Routes() http.Handler {
 	v1.Handle("GET /v1/items/{id}/trailer", s.authed(s.handleTrailer))
 	v1.Handle("GET /v1/items/{id}/trailers", s.authed(s.handleTrailers))
 	v1.Handle("POST /v1/items/{id}/trailers/resolve", s.authed(s.handleResolveTrailer))
+	v1.Handle("POST /v1/items/{id}/trailers/report", s.authed(s.handleTrailerReport))
 	v1.Handle("GET /v1/items/{id}/intro", s.authed(s.handleIntro))
 	v1.Handle("GET /v1/items/{id}/trickplay", s.authed(s.handleTrickplay))
 	v1.Handle("GET /v1/items/{id}/trickplay/{frame}", s.authed(s.handleTrickplayFrame))

server/internal/api/auth.go

@@ -69,7 +69,11 @@ func (s *Server) handleLogin(w http.ResponseWriter, r *http.Request) {
 	}
 
 	auth, err := s.emby.Authenticate(
-		r.Context(), req.Username, req.Password, req.DeviceID, req.DeviceName, clientVersion(r),
+		r.Context(), req.Username, req.Password,
+		emby.Credentials{
+			DeviceID: req.DeviceID, DeviceName: req.DeviceName,
+			ClientVersion: clientVersion(r),
+		},
 	)
 	if err != nil {
 		// Never echo Emby's body here: a failed sign-in is the one place a wrong
@@ -250,7 +254,8 @@ func (s *Server) retireEmbyDevice(ctx context.Context, deviceID string) {
 	}
 	if err := s.emby.DeleteDevice(ctx, emby.Credentials{
 		UserID: s.cfg.SyncUserID, Token: s.cfg.SyncAPIKey,
-		DeviceID: "memby-gateway", DeviceName: "Memby Gateway",
+		DeviceID: "memby-gateway", DeviceName: s.gatewayDeviceName(),
+		Gateway: true,
 	}, deviceID); err != nil {
 		s.loggerFor(ctx).Warn("emby device cleanup failed",
 			"removed_device_id", deviceID, "error", err)

server/internal/api/installer_auth.go

@@ -19,13 +19,30 @@ const (
 	installerSessionTTL = 30 * time.Minute
 	adminSessionTTL     = 12 * time.Hour
 	installerDeviceID   = "memby-web-installer"
-	installerDeviceName = "Memby Web Installer"
 
 	// adminRenewWithin is how close to expiry a session must be before an operator's own
 	// request re-issues it. Half the TTL avoids rewriting the cookie on every request.
 	adminRenewWithin = adminSessionTTL / 2
 )
 
+// gatewayDeviceName is what Emby records for a device row the gateway creates for itself.
+// It follows the gateway's client name so one operator-set word covers both halves of how
+// the server identifies itself, and it is deliberately never the product name — Emby's
+// device list is read by whoever runs the server, and an entry called "Memby …" there
+// reads as one of the household's televisions.
+func (s *Server) gatewayDeviceName() string {
+	if name := strings.TrimSpace(s.cfg.GatewayClientName); name != "" {
+		return name
+	}
+	return emby.DefaultGatewayClientName
+}
+
+// installerDeviceName separates the temporary record an admin or installer sign-in
+// creates from the gateway's own, so a password check is recognisable while it exists.
+func (s *Server) installerDeviceName() string {
+	return s.gatewayDeviceName() + " Installer"
+}
+
 func (s *Server) installerSecret() []byte {
 	if s.cfg.ReleasePublishToken == "" {
 		return nil
@@ -192,8 +209,13 @@ func (s *Server) handleInstallLogin(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
+	// Gateway, not a television: this sign-in is the admin console or the web installer
+	// checking a password, so Emby records it under the gateway's own client name.
 	auth, err := s.emby.Authenticate(
-		r.Context(), username, password, installerDeviceID, installerDeviceName, "",
+		r.Context(), username, password,
+		emby.Credentials{
+			DeviceID: installerDeviceID, DeviceName: s.installerDeviceName(), Gateway: true,
+		},
 	)
 	if err != nil {
 		s.loggerFor(r.Context()).Warn("installer Emby authentication failed", "username", username)
@@ -204,13 +226,15 @@ func (s *Server) handleInstallLogin(w http.ResponseWriter, r *http.Request) {
 	// it succeeded, so retire the upstream session immediately and never persist it.
 	if err := s.emby.Logout(r.Context(), emby.Credentials{
 		UserID: auth.User.ID, Token: auth.AccessToken,
-		DeviceID: installerDeviceID, DeviceName: installerDeviceName,
+		DeviceID: installerDeviceID, DeviceName: s.installerDeviceName(),
+		Gateway: true,
 	}); err != nil {
 		s.loggerFor(r.Context()).Warn("installer Emby session cleanup failed", "error", err)
 	}
 	if err := s.emby.DeleteDevice(r.Context(), emby.Credentials{
 		UserID: s.cfg.SyncUserID, Token: s.cfg.SyncAPIKey,
-		DeviceID: "memby-gateway", DeviceName: "Memby Gateway",
+		DeviceID: "memby-gateway", DeviceName: s.gatewayDeviceName(),
+		Gateway: true,
 	}, installerDeviceID); err != nil {
 		s.loggerFor(r.Context()).Error("installer Emby device cleanup failed", "error", err)
 		s.renderAccessLogin(w, r, "Sign-in is temporarily unavailable.",

server/internal/api/logcontext.go

@@ -3,6 +3,7 @@ package api
 import (
 	"context"
 	"log/slog"
+	"net"
 	"net/http"
 	"strings"
 
@@ -173,7 +174,8 @@ func isPlaybackItemPath(path string) bool {
 	// Fetching a subtitle is two segments deep rather than one, and matching its trailing
 	// "search" on its own would claim any future per-item search as playback. A seek
 	// preview is the same shape: the frame number is the last segment, not the word.
-	if strings.Contains(path, "/subtitles/") || strings.Contains(path, "/trickplay") {
+	if strings.Contains(path, "/subtitles/") || strings.Contains(path, "/trickplay") ||
+		strings.Contains(path, "/trailers") {
 		return true
 	}
 	switch path[strings.LastIndex(path, "/")+1:] {
@@ -182,3 +184,28 @@ func isPlaybackItemPath(path string) bool {
 	}
 	return false
 }
+
+// requestClientIP is the viewer-facing address recorded for trailer playback. The first
+// Forwarded address is the original client when the gateway is behind its normal reverse
+// proxy; direct deployments fall back to RemoteAddr. This value is for operational logs,
+// never authentication or access control.
+func requestClientIP(r *http.Request) string {
+	for _, value := range strings.Split(r.Header.Get("X-Forwarded-For"), ",") {
+		if ip := net.ParseIP(strings.TrimSpace(value)); ip != nil {
+			return ip.String()
+		}
+	}
+	if ip := net.ParseIP(strings.TrimSpace(r.Header.Get("X-Real-IP"))); ip != nil {
+		return ip.String()
+	}
+	host, _, err := net.SplitHostPort(strings.TrimSpace(r.RemoteAddr))
+	if err == nil {
+		if ip := net.ParseIP(host); ip != nil {
+			return ip.String()
+		}
+	}
+	if ip := net.ParseIP(strings.TrimSpace(r.RemoteAddr)); ip != nil {
+		return ip.String()
+	}
+	return "unknown"
+}

server/internal/api/logging_test.go

@@ -48,6 +48,8 @@ func TestComponentNamesThePartOfTheAppARouteBelongsTo(t *testing.T) {
 		"/v1/items/42/related":          "details",
 		"/v1/items/42/playback":         "playback",
 		"/v1/items/42/next":             "playback",
+		"/v1/items/42/trailers/resolve": "playback",
+		"/v1/items/42/trailers/report":  "playback",
 		"/v1/items/42/subtitles/search": "playback",
 		"/v1/items/42/trickplay":        "playback",
 		"/v1/items/42/trickplay/12.jpg": "playback",
@@ -69,6 +71,15 @@ func TestComponentNamesThePartOfTheAppARouteBelongsTo(t *testing.T) {
 	}
 }
 
+func TestRequestClientIPPrefersOriginalForwardedAddress(t *testing.T) {
+	request := httptest.NewRequest(http.MethodPost, "/v1/items/42/trailers/report", nil)
+	request.RemoteAddr = "10.0.0.2:41234"
+	request.Header.Set("X-Forwarded-For", "203.0.113.9, 10.0.0.2")
+	if got := requestClientIP(request); got != "203.0.113.9" {
+		t.Fatalf("client ip = %q", got)
+	}
+}
+
 func TestIdentifyNamesTheViewerAndTelevision(t *testing.T) {
 	request := httptest.NewRequest(http.MethodGet, "/v1/home", nil)
 	request.Header.Set("X-Memby-Version", "0.1.60")

server/internal/api/playback_report_test.go

@@ -20,7 +20,7 @@ func TestStoppedPlaybackReportRemainsRetryableWhenEmbyIsDown(t *testing.T) {
 	defer upstream.Close()
 
 	s := &Server{
-		emby: emby.New(upstream.URL, upstream.URL, "Memby test", time.Second),
+		emby: emby.New(upstream.URL, upstream.URL, "MbyATV", "MbyGateway", time.Second),
 		log:  slog.New(slog.NewTextHandler(io.Discard, nil)),
 	}
 	req := httptest.NewRequest(

server/internal/api/preferences.go

@@ -140,6 +140,11 @@ var preferenceCatalogue = []preferenceDefinition{
 		Description: "Roll into the next episode when one finishes.",
 		Kind:        preferenceToggle, Default: true,
 	},
+	{
+		Key: "playNextEpisodePreview", Name: "Next-episode recap or preview", Area: "Playback",
+		Description: "With auto-play on, play a matched YouTube recap or preview two minutes before the episode ends.",
+		Kind:        preferenceToggle, Default: false,
+	},
 	{
 		Key: "showTenMinuteReminder", Name: "Ten-minute reminder", Area: "Playback",
 		Description: "Show the lower-third when ten minutes are left.",

server/internal/api/release_test.go

@@ -282,7 +282,7 @@ func TestInstallerLoginUsesEmbyWithoutCreatingTVSession(t *testing.T) {
 			ReleasePublishToken: "test-release-secret",
 			SyncUserID:          "service-user", SyncAPIKey: "service-token",
 		},
-		emby: emby.New(upstream.URL, upstream.URL, "Memby test", 2*time.Second),
+		emby: emby.New(upstream.URL, upstream.URL, "MbyATV", "MbyGateway", 2*time.Second),
 		log:  slog.New(slog.NewTextHandler(io.Discard, nil)),
 	}
 	form := url.Values{

server/internal/api/trailers.go

@@ -14,7 +14,6 @@ import (
 	"github.com/ponzischeme89/memby/server/internal/cache"
 	"github.com/ponzischeme89/memby/server/internal/emby"
 	"github.com/ponzischeme89/memby/server/internal/store"
-	"github.com/ponzischeme89/memby/server/internal/trailer"
 )
 
 type remoteTrailer struct {
@@ -55,6 +54,7 @@ type trailerPlaybackResponse struct {
 	CandidateID   string `json:"candidateId"`
 	Provider      string `json:"provider"`
 	URL           string `json:"url"`
+	SourceURL     string `json:"sourceUrl,omitempty"`
 	Title         string `json:"title"`
 	ItemID        string `json:"itemId,omitempty"`
 	MediaSourceID string `json:"mediaSourceId,omitempty"`
@@ -62,6 +62,13 @@ type trailerPlaybackResponse struct {
 	PlayMethod    string `json:"playMethod,omitempty"`
 }
 
+type trailerReportRequest struct {
+	CandidateID string `json:"candidateId"`
+	Provider    string `json:"provider"`
+	Phase       string `json:"phase"`
+	Reason      string `json:"reason,omitempty"`
+}
+
 func (s *Server) handleTrailers(w http.ResponseWriter, r *http.Request, sess store.Session) {
 	itemID := strings.TrimSpace(r.PathValue("id"))
 	if itemID == "" {
@@ -107,37 +114,24 @@ func (s *Server) handleResolveTrailer(w http.ResponseWriter, r *http.Request, se
 		s.writeUpstreamError(r.Context(), w, err, "could not inspect trailers")
 		return
 	}
-	resolver := s.trailers
-	if resolver == nil {
-		resolver = trailer.New(nil)
-	}
 	for _, candidate := range s.preferredTrailerCandidates(r.Context(), sess, manifest) {
 		if excluded[candidate.ID] {
-			if candidate.SourceURL != "" {
-				resolver.Invalidate(trailer.Source{Provider: candidate.Provider, URL: candidate.SourceURL})
-			}
 			continue
 		}
 		if len(candidate.LocalItem) > 0 {
 			if resolved, resolveErr := s.resolveLocalTrailer(r.Context(), sess, manifest, candidate); resolveErr == nil {
-				s.rememberTrailerCandidate(r.Context(), sess, itemID, candidate.ID)
 				writeJSON(w, http.StatusOK, resolved)
 				return
 			}
 			continue
 		}
-		resolved, resolveErr := resolver.Resolve(r.Context(), trailer.Source{
-			Provider: candidate.Provider,
-			URL:      candidate.SourceURL,
-		})
-		if resolveErr != nil {
-			continue
-		}
-		s.rememberTrailerCandidate(r.Context(), sess, itemID, candidate.ID)
+		// Remote pages are resolved on the television. YouTube signs direct media URLs for
+		// the resolving IP, so resolving here can make the URL unusable from the viewer's
+		// network and makes provider traffic appear to come from the gateway.
 		writeJSON(w, http.StatusOK, trailerPlaybackResponse{
 			CandidateID: candidate.ID,
 			Provider:    candidate.Provider,
-			URL:         resolved.URL,
+			SourceURL:   candidate.SourceURL,
 			Title:       trailerTitle(manifest.Title, candidate.Name),
 			PlayMethod:  "DirectPlay",
 		})
@@ -146,6 +140,37 @@ func (s *Server) handleResolveTrailer(w http.ResponseWriter, r *http.Request, se
 	writeError(w, http.StatusNotFound, "no playable trailer is available")
 }
 
+func (s *Server) handleTrailerReport(w http.ResponseWriter, r *http.Request, sess store.Session) {
+	itemID := strings.TrimSpace(r.PathValue("id"))
+	var report trailerReportRequest
+	if itemID == "" || json.NewDecoder(http.MaxBytesReader(w, r.Body, 8<<10)).Decode(&report) != nil {
+		writeError(w, http.StatusBadRequest, "invalid trailer report")
+		return
+	}
+	report.CandidateID = strings.TrimSpace(report.CandidateID)
+	report.Provider = strings.ToLower(strings.TrimSpace(report.Provider))
+	report.Phase = strings.ToLower(strings.TrimSpace(report.Phase))
+	if report.CandidateID == "" || report.Provider == "" ||
+		(report.Phase != "started" && report.Phase != "failed" && report.Phase != "completed") {
+		writeError(w, http.StatusBadRequest, "invalid trailer report")
+		return
+	}
+	fields := []any{
+		"item_id", itemID,
+		"provider", report.Provider,
+		"candidate", report.CandidateID,
+		"source_ip", requestClientIP(r),
+	}
+	if reason := strings.TrimSpace(report.Reason); reason != "" {
+		fields = append(fields, "reason", reason)
+	}
+	s.loggerFor(r.Context()).Info("trailer playback "+report.Phase, fields...)
+	if report.Phase == "started" {
+		s.rememberTrailerCandidate(r.Context(), sess, itemID, report.CandidateID)
+	}
+	w.WriteHeader(http.StatusNoContent)
+}
+
 func (s *Server) preferredTrailerCandidates(
 	ctx context.Context,
 	sess store.Session,

server/internal/api/trailers_test.go

@@ -14,17 +14,11 @@ import (
 
 	"github.com/ponzischeme89/memby/server/internal/config"
 	"github.com/ponzischeme89/memby/server/internal/emby"
+	serverlogging "github.com/ponzischeme89/memby/server/internal/logging"
 	"github.com/ponzischeme89/memby/server/internal/store"
-	"github.com/ponzischeme89/memby/server/internal/trailer"
 )
 
-type trailerRoundTripFunc func(*http.Request) (*http.Response, error)
-
-func (fn trailerRoundTripFunc) RoundTrip(request *http.Request) (*http.Response, error) {
-	return fn(request)
-}
-
-func TestResolveTrailerFallsThroughProviders(t *testing.T) {
+func TestResolveTrailerSkipsRejectedProviderOnTheClientBehalf(t *testing.T) {
 	upstream := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 		switch {
 		case strings.HasSuffix(r.URL.Path, "/LocalTrailers"):
@@ -43,35 +37,14 @@ func TestResolveTrailerFallsThroughProviders(t *testing.T) {
 	}))
 	defer upstream.Close()
 
-	resolverClient := &http.Client{Transport: trailerRoundTripFunc(func(request *http.Request) (*http.Response, error) {
-		status := http.StatusOK
-		body := ""
-		headers := http.Header{}
-		switch request.URL.Host {
-		case "trailers.apple.com":
-			status = http.StatusNotFound
-		case "www.youtube.com":
-			body = `{"playabilityStatus":{"status":"OK"},"streamingData":{"formats":[` +
-				`{"url":"https://media.example/trailer.mp4","mimeType":"video/mp4; codecs=avc1,mp4a","height":720}]}}`
-			headers.Set("Content-Type", "application/json")
-		case "media.example":
-			status = http.StatusPartialContent
-			headers.Set("Content-Type", "video/mp4")
-		default:
-			t.Fatalf("unexpected trailer request: %s", request.URL)
-		}
-		return &http.Response{
-			StatusCode: status, Header: headers,
-			Body: io.NopCloser(strings.NewReader(body)), Request: request,
-		}, nil
-	})}
 	server := &Server{
-		cfg:      config.Config{ItemTTL: time.Minute},
-		emby:     emby.New(upstream.URL, upstream.URL, "MbyATV", time.Second),
-		trailers: trailer.New(resolverClient),
-		log:      slog.New(slog.NewTextHandler(io.Discard, nil)),
+		cfg:  config.Config{ItemTTL: time.Minute},
+		emby: emby.New(upstream.URL, upstream.URL, "MbyATV", "MbyGateway", time.Second),
+		log:  slog.New(slog.NewTextHandler(io.Discard, nil)),
 	}
-	request := httptest.NewRequest(http.MethodPost, "/v1/items/film-1/trailers/resolve", bytes.NewBufferString(`{}`))
+	appleID := trailerCandidateID("apple", "https://trailers.apple.com/missing.mov")
+	request := httptest.NewRequest(http.MethodPost, "/v1/items/film-1/trailers/resolve",
+		bytes.NewBufferString(`{"excludedCandidateIds":["`+appleID+`"]}`))
 	request.SetPathValue("id", "film-1")
 	recorder := httptest.NewRecorder()
 	server.handleResolveTrailer(recorder, request, store.Session{EmbyUserID: "user", EmbyToken: "token"})
@@ -82,11 +55,32 @@ func TestResolveTrailerFallsThroughProviders(t *testing.T) {
 	if err := json.Unmarshal(recorder.Body.Bytes(), &response); err != nil {
 		t.Fatal(err)
 	}
-	if response.Provider != "youtube" || response.URL != "https://media.example/trailer.mp4" {
+	if response.Provider != "youtube" || response.SourceURL != "https://youtu.be/dQw4w9WgXcQ" || response.URL != "" {
 		t.Fatalf("unexpected response: %+v", response)
 	}
 }
 
+func TestTrailerStartedLogUsesOriginalClientIP(t *testing.T) {
+	logger, events := serverlogging.NewBuffered(io.Discard, slog.LevelInfo, 10, serverlogging.FormatConsole)
+	server := &Server{log: logger}
+	request := httptest.NewRequest(http.MethodPost, "/v1/items/film-1/trailers/report",
+		bytes.NewBufferString(`{"candidateId":"youtube-1","provider":"youtube","phase":"started"}`))
+	request.SetPathValue("id", "film-1")
+	request.Header.Set("X-Forwarded-For", "203.0.113.42, 10.0.0.2")
+	request, _ = withRequestIdentity(request)
+	identify(request.Context(), store.Session{Username: "viewer", DeviceName: "Lounge TV"})
+	recorder := httptest.NewRecorder()
+	server.handleTrailerReport(recorder, request, store.Session{})
+	if recorder.Code != http.StatusNoContent {
+		t.Fatalf("status = %d", recorder.Code)
+	}
+	page := events.Events(0, 10)
+	if len(page.Events) != 1 || page.Events[0].Attributes["source_ip"] != "203.0.113.42" ||
+		page.Events[0].Message != "trailer playback started" {
+		t.Fatalf("unexpected event: %+v", page.Events)
+	}
+}
+
 func TestRemoteTrailerPriorityPrefersOfficialAppleThenYouTube(t *testing.T) {
 	candidates := []trailerCandidate{
 		{ID: "youtube-other", Provider: "youtube", Priority: remoteTrailerPriority("youtube", "Trailer")},
@@ -99,3 +93,13 @@ func TestRemoteTrailerPriorityPrefersOfficialAppleThenYouTube(t *testing.T) {
 		t.Fatalf("unexpected order: %+v", candidates)
 	}
 }
+
+func TestNextEpisodePreviewPreferenceIsOptional(t *testing.T) {
+	definition, ok := preferenceDefinitionFor("playNextEpisodePreview")
+	if !ok {
+		t.Fatal("playNextEpisodePreview is missing from the preference catalogue")
+	}
+	if definition.Kind != preferenceToggle || definition.Default != false {
+		t.Fatalf("definition = %+v, want an opt-in toggle", definition)
+	}
+}