diff --git a/adapters/lastfm/client.go b/adapters/lastfm/client.go index e468aa638..7b2601cf9 100644 --- a/adapters/lastfm/client.go +++ b/adapters/lastfm/client.go @@ -49,6 +49,12 @@ type client struct { hc httpDoer } +// escapePlus works around Last.fm decoding artist.* and track.* params twice, turning "+" into a space. +// album.getInfo decodes only once, so it must not use this. +func escapePlus(s string) string { + return strings.ReplaceAll(s, "+", "%2B") +} + func (c *client) albumGetInfo(ctx context.Context, name string, artist string, mbid string, lang string) (*Album, error) { params := url.Values{} params.Add("method", "album.getInfo") @@ -66,7 +72,7 @@ func (c *client) albumGetInfo(ctx context.Context, name string, artist string, m func (c *client) artistGetInfo(ctx context.Context, name string, lang string) (*Artist, error) { params := url.Values{} params.Add("method", "artist.getInfo") - params.Add("artist", name) + params.Add("artist", escapePlus(name)) params.Add("lang", lang) response, err := c.makeRequest(ctx, http.MethodGet, params, false) if err != nil { @@ -78,7 +84,7 @@ func (c *client) artistGetInfo(ctx context.Context, name string, lang string) (* func (c *client) artistGetSimilar(ctx context.Context, name string, limit int) (*SimilarArtists, error) { params := url.Values{} params.Add("method", "artist.getSimilar") - params.Add("artist", name) + params.Add("artist", escapePlus(name)) params.Add("limit", strconv.Itoa(limit)) response, err := c.makeRequest(ctx, http.MethodGet, params, false) if err != nil { @@ -90,7 +96,7 @@ func (c *client) artistGetSimilar(ctx context.Context, name string, limit int) ( func (c *client) artistGetTopTracks(ctx context.Context, name string, limit int) (*TopTracks, error) { params := url.Values{} params.Add("method", "artist.getTopTracks") - params.Add("artist", name) + params.Add("artist", escapePlus(name)) params.Add("limit", strconv.Itoa(limit)) response, err := c.makeRequest(ctx, http.MethodGet, params, false) if err != nil { @@ -102,8 +108,8 @@ func (c *client) artistGetTopTracks(ctx context.Context, name string, limit int) func (c *client) trackGetSimilar(ctx context.Context, name, artist string, limit int) (*SimilarTracks, error) { params := url.Values{} params.Add("method", "track.getSimilar") - params.Add("track", name) - params.Add("artist", artist) + params.Add("track", escapePlus(name)) + params.Add("artist", escapePlus(artist)) params.Add("limit", strconv.Itoa(limit)) response, err := c.makeRequest(ctx, http.MethodGet, params, false) if err != nil { diff --git a/adapters/lastfm/client_test.go b/adapters/lastfm/client_test.go index 271ae1419..f72c84b91 100644 --- a/adapters/lastfm/client_test.go +++ b/adapters/lastfm/client_test.go @@ -35,6 +35,15 @@ var _ = Describe("client", func() { Expect(album.Name).To(Equal("Believe")) Expect(httpClient.SavedRequest.URL.String()).To(Equal(apiBaseUrl + "?album=Believe&api_key=API_KEY&artist=U2&format=json&lang=pt&mbid=mbid-1234&method=album.getInfo")) }) + + It("does not double-encode plus signs", func() { + f, _ := os.Open("tests/fixtures/lastfm.album.getinfo.json") + httpClient.Res = http.Response{Body: f, StatusCode: 200} + + _, err := client.albumGetInfo(context.Background(), "Lungs", "Florence + the Machine", "", "en") + Expect(err).ToNot(HaveOccurred()) + Expect(httpClient.SavedRequest.URL.Query().Get("artist")).To(Equal("Florence + the Machine")) + }) }) Describe("artistGetInfo", func() { @@ -48,6 +57,15 @@ var _ = Describe("client", func() { Expect(httpClient.SavedRequest.URL.String()).To(Equal(apiBaseUrl + "?api_key=API_KEY&artist=U2&format=json&lang=pt&method=artist.getInfo")) }) + It("double-encodes plus signs in the artist name", func() { + f, _ := os.Open("tests/fixtures/lastfm.artist.getinfo.json") + httpClient.Res = http.Response{Body: f, StatusCode: 200} + + _, err := client.artistGetInfo(context.Background(), "Florence + the Machine", "en") + Expect(err).ToNot(HaveOccurred()) + Expect(httpClient.SavedRequest.URL.Query().Get("artist")).To(Equal("Florence %2B the Machine")) + }) + It("fails if Last.fm returns an http status != 200", func() { httpClient.Res = http.Response{ Body: io.NopCloser(bytes.NewBufferString(`Internal Server Error`)), @@ -107,6 +125,15 @@ var _ = Describe("client", func() { Expect(len(similar.Artists)).To(Equal(2)) Expect(httpClient.SavedRequest.URL.String()).To(Equal(apiBaseUrl + "?api_key=API_KEY&artist=U2&format=json&limit=2&method=artist.getSimilar")) }) + + It("double-encodes plus signs in the artist name", func() { + f, _ := os.Open("tests/fixtures/lastfm.artist.getsimilar.json") + httpClient.Res = http.Response{Body: f, StatusCode: 200} + + _, err := client.artistGetSimilar(context.Background(), "+44", 2) + Expect(err).ToNot(HaveOccurred()) + Expect(httpClient.SavedRequest.URL.Query().Get("artist")).To(Equal("%2B44")) + }) }) Describe("artistGetTopTracks", func() { @@ -119,6 +146,15 @@ var _ = Describe("client", func() { Expect(len(top.Track)).To(Equal(2)) Expect(httpClient.SavedRequest.URL.String()).To(Equal(apiBaseUrl + "?api_key=API_KEY&artist=U2&format=json&limit=2&method=artist.getTopTracks")) }) + + It("double-encodes plus signs in the artist name", func() { + f, _ := os.Open("tests/fixtures/lastfm.artist.gettoptracks.json") + httpClient.Res = http.Response{Body: f, StatusCode: 200} + + _, err := client.artistGetTopTracks(context.Background(), "C+C Music Factory", 2) + Expect(err).ToNot(HaveOccurred()) + Expect(httpClient.SavedRequest.URL.Query().Get("artist")).To(Equal("C%2BC Music Factory")) + }) }) Describe("trackGetSimilar", func() { @@ -135,6 +171,17 @@ var _ = Describe("client", func() { Expect(httpClient.SavedRequest.URL.String()).To(Equal(apiBaseUrl + "?api_key=API_KEY&artist=Depeche+Mode&format=json&limit=5&method=track.getSimilar&track=Just+Can%27t+Get+Enough")) }) + It("double-encodes plus signs in the track and artist names", func() { + f, _ := os.Open("tests/fixtures/lastfm.track.getsimilar.json") + httpClient.Res = http.Response{Body: f, StatusCode: 200} + + _, err := client.trackGetSimilar(context.Background(), "1+1", "Queen + Paul Rodgers", 5) + Expect(err).ToNot(HaveOccurred()) + query := httpClient.SavedRequest.URL.Query() + Expect(query.Get("track")).To(Equal("1%2B1")) + Expect(query.Get("artist")).To(Equal("Queen %2B Paul Rodgers")) + }) + It("returns empty list when no similar tracks found", func() { f, _ := os.Open("tests/fixtures/lastfm.track.getsimilar.unknown.json") httpClient.Res = http.Response{Body: f, StatusCode: 200} diff --git a/cmd/root.go b/cmd/root.go index c4e360010..f632b0425 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -86,6 +86,7 @@ func runNavidrome(ctx context.Context) { g.Go(startSignaller(ctx)) g.Go(startScheduler(ctx)) g.Go(startPlaybackServer(ctx)) + g.Go(startJellyfinDiscovery(ctx)) g.Go(schedulePeriodicBackup(ctx)) g.Go(startInsightsCollector(ctx)) g.Go(scheduleDBAnalyzer(ctx)) @@ -343,6 +344,18 @@ func startInsightsCollector(ctx context.Context) func() error { } } +// startJellyfinDiscovery never returns an error: a discovery failure must not stop the server. +func startJellyfinDiscovery(ctx context.Context) func() error { + return func() error { + if !conf.Server.Jellyfin.Enabled || !conf.Server.Jellyfin.AutoDiscovery { + log.Debug("Jellyfin auto-discovery is DISABLED") + return nil + } + CreateJellyfinDiscovery().Serve(ctx) + return nil + } +} + // startPlaybackServer starts the Navidrome playback server, if configured. // It is responsible for the Jukebox functionality func startPlaybackServer(ctx context.Context) func() error { diff --git a/cmd/wire_gen.go b/cmd/wire_gen.go index fd04c44c5..d6bf4da07 100644 --- a/cmd/wire_gen.go +++ b/cmd/wire_gen.go @@ -168,6 +168,13 @@ func CreateListenBrainzRouter() *listenbrainz.Router { return router } +func CreateJellyfinDiscovery() *jellyfin.Discovery { + sqlDB := db.Db() + dataStore := persistence.New(sqlDB) + discovery := jellyfin.NewDiscovery(dataStore) + return discovery +} + func CreateInsights() metrics.Insights { sqlDB := db.Db() dataStore := persistence.New(sqlDB) @@ -249,7 +256,7 @@ func getPluginManager() *plugins.Manager { // wire_injectors.go: -var allProviders = wire.NewSet(core.Set, artwork.Set, server.New, subsonic.New, jellyfin.New, nativeapi.New, public.New, persistence.New, lastfm.NewRouter, listenbrainz.NewRouter, events.GetBroker, scanner.GetInstance, scanner.GetWatcher, metrics.GetPrometheusInstance, db.Db, plugins.GetManager, sonic.New, wire.Bind(new(agents.PluginLoader), new(*plugins.Manager)), wire.Bind(new(scrobbler.PluginLoader), new(*plugins.Manager)), wire.Bind(new(lyrics.PluginLoader), new(*plugins.Manager)), wire.Bind(new(sonic.PluginLoader), new(*plugins.Manager)), wire.Bind(new(sonic.Engine), new(*sonic.Sonic)), wire.Bind(new(nativeapi.PluginManager), new(*plugins.Manager)), wire.Bind(new(core.PluginUnloader), new(*plugins.Manager)), wire.Bind(new(plugins.PluginMetricsRecorder), new(metrics.Metrics)), wire.Bind(new(core.Watcher), new(scanner.Watcher)), wire.Bind(new(playlists.ImageUploadService), new(artwork.Uploader))) +var allProviders = wire.NewSet(core.Set, artwork.Set, server.New, subsonic.New, jellyfin.New, jellyfin.NewDiscovery, nativeapi.New, public.New, persistence.New, lastfm.NewRouter, listenbrainz.NewRouter, events.GetBroker, scanner.GetInstance, scanner.GetWatcher, metrics.GetPrometheusInstance, db.Db, plugins.GetManager, sonic.New, wire.Bind(new(agents.PluginLoader), new(*plugins.Manager)), wire.Bind(new(scrobbler.PluginLoader), new(*plugins.Manager)), wire.Bind(new(lyrics.PluginLoader), new(*plugins.Manager)), wire.Bind(new(sonic.PluginLoader), new(*plugins.Manager)), wire.Bind(new(sonic.Engine), new(*sonic.Sonic)), wire.Bind(new(nativeapi.PluginManager), new(*plugins.Manager)), wire.Bind(new(core.PluginUnloader), new(*plugins.Manager)), wire.Bind(new(plugins.PluginMetricsRecorder), new(metrics.Metrics)), wire.Bind(new(core.Watcher), new(scanner.Watcher)), wire.Bind(new(playlists.ImageUploadService), new(artwork.Uploader))) func GetPluginManager(ctx context.Context) *plugins.Manager { manager := getPluginManager() diff --git a/cmd/wire_injectors.go b/cmd/wire_injectors.go index a54d4ae1b..527617959 100644 --- a/cmd/wire_injectors.go +++ b/cmd/wire_injectors.go @@ -36,6 +36,7 @@ var allProviders = wire.NewSet( server.New, subsonic.New, jellyfin.New, + jellyfin.NewDiscovery, nativeapi.New, public.New, persistence.New, @@ -108,6 +109,12 @@ func CreateListenBrainzRouter() *listenbrainz.Router { )) } +func CreateJellyfinDiscovery() *jellyfin.Discovery { + panic(wire.Build( + allProviders, + )) +} + func CreateInsights() metrics.Insights { panic(wire.Build( allProviders, diff --git a/conf/configuration.go b/conf/configuration.go index e549e2a61..4812f2515 100644 --- a/conf/configuration.go +++ b/conf/configuration.go @@ -235,6 +235,7 @@ type jellyfinOptions struct { // ExposedPublicUsers is a comma-separated list of usernames to advertise on the unauthenticated // GET /Users/Public, so Jellyfin clients can show a login user-picker. Empty exposes no users. ExposedPublicUsers string + AutoDiscovery bool // MaxConcurrentStreams bounds how many collection responses can stream at once. Each holds a DB // cursor — and its pooled connection — for the whole client-paced response, so without a bound // enough slow clients would take the entire pool and stall the scanner, scrobbles and the UI. @@ -1087,6 +1088,7 @@ func setViperDefaults() { viper.SetDefault("listenbrainz.trackalgorithm", consts.DefaultListenBrainzTrackAlgorithm) viper.SetDefault("jellyfin.enabled", false) viper.SetDefault("jellyfin.servername", "") + viper.SetDefault("jellyfin.autodiscovery", false) viper.SetDefault("enablescrobblehistory", true) viper.SetDefault("httpheaders.frameoptions", "DENY") viper.SetDefault("backup.path", "") diff --git a/core/artwork/e2e/artist_test.go b/core/artwork/e2e/artist_test.go index 5a9396cc3..4a6959d95 100644 --- a/core/artwork/e2e/artist_test.go +++ b/core/artwork/e2e/artist_test.go @@ -90,6 +90,28 @@ var _ = Describe("Artist artwork resolution", func() { }) }) + When("the artist's only album folder has no images of its own", func() { + // Artist/ + // ├── backdrop1.jpg + // ├── folder.jpg ← matched by folder.* + // ├── logo.png + // └── Album/ + // ├── 01 - Track.mp3 + // └── 02 - Track.mp3 + It("resolves the artist folder, not the library root", func() { + conf.Server.ArtistArtPriority = "folder.*, artist.*, album/artist.*" + setLayout(fstest.MapFS{ + "Artist/Album/01 - Track.mp3": trackFile(1, "Track 1", map[string]any{"albumartist": "Artist", "album": "Album"}), + "Artist/Album/02 - Track.mp3": trackFile(2, "Track 2", map[string]any{"albumartist": "Artist", "album": "Album"}), + "Artist/backdrop1.jpg": smallPNG("backdrop"), + "Artist/folder.jpg": smallPNG("artist-folder"), + "Artist/logo.png": smallPNG("logo"), + }) + scan() + expectArtistFolder(soleArtist(), "Artist/folder.jpg") + }) + }) + When("the artist's only album has its tracks in disc subfolders", func() { // Artist/ // ├── artist.jpg ← wins (artist.* before album/artist.*) diff --git a/core/library.go b/core/library.go index d905e00cb..6df0e95b5 100644 --- a/core/library.go +++ b/core/library.go @@ -2,7 +2,6 @@ package core import ( "context" - "errors" "fmt" "io/fs" "os" @@ -307,14 +306,7 @@ func (r *libraryRepositoryWrapper) mapError(err error) error { } } - switch { - case errors.Is(err, model.ErrNotFound): - return rest.ErrNotFound - case errors.Is(err, model.ErrNotAuthorized): - return rest.ErrPermissionDenied - default: - return err - } + return err } func (r *libraryRepositoryWrapper) validateLibrary(library *model.Library) error { diff --git a/core/playlists/playlists.go b/core/playlists/playlists.go index 9ae8f09cb..1f8cc9581 100644 --- a/core/playlists/playlists.go +++ b/core/playlists/playlists.go @@ -32,6 +32,7 @@ type Playlists interface { // Track management AddTracks(ctx context.Context, playlistID string, ids []string) (int, error) + InsertTracks(ctx context.Context, playlistID string, ids []string, pos int) (int, error) AddAlbums(ctx context.Context, playlistID string, albumIds []string) (int, error) AddArtists(ctx context.Context, playlistID string, artistIds []string) (int, error) AddDiscs(ctx context.Context, playlistID string, discs []model.DiscID) (int, error) @@ -268,6 +269,22 @@ func (s *playlists) AddTracks(ctx context.Context, playlistID string, ids []stri return s.ds.Playlist(ctx).Tracks(playlistID, false).Add(ids) } +// InsertTracks adds tracks before the 1-based position pos; a position past the end appends. +func (s *playlists) InsertTracks(ctx context.Context, playlistID string, ids []string, pos int) (int, error) { + if _, err := s.checkTracksEditable(ctx, playlistID); err != nil { + return 0, err + } + var count int + // Immediate: a deferred tx that reads before shifting fails at once with SQLITE_BUSY under + // concurrent writers instead of waiting for the lock. + err := s.ds.WithTxImmediate(func(tx model.DataStore) error { + var err error + count, err = tx.Playlist(ctx).Tracks(playlistID, false).Insert(ids, pos) + return err + }) + return count, err +} + func (s *playlists) AddAlbums(ctx context.Context, playlistID string, albumIds []string) (int, error) { if _, err := s.checkTracksEditable(ctx, playlistID); err != nil { return 0, err @@ -302,7 +319,7 @@ func (s *playlists) ReorderTrack(ctx context.Context, playlistID string, pos int if _, err := s.checkTracksEditable(ctx, playlistID); err != nil { return err } - return s.ds.WithTx(func(tx model.DataStore) error { + return s.ds.WithTxImmediate(func(tx model.DataStore) error { return tx.Playlist(ctx).Tracks(playlistID, false).Reorder(pos, newPos) }) } diff --git a/core/playlists/playlists_test.go b/core/playlists/playlists_test.go index dd182b3f1..dd6213555 100644 --- a/core/playlists/playlists_test.go +++ b/core/playlists/playlists_test.go @@ -324,6 +324,43 @@ var _ = Describe("Playlists", func() { }) }) + Describe("InsertTracks", func() { + var mockTracks *tests.MockPlaylistTrackRepo + + BeforeEach(func() { + mockTracks = &tests.MockPlaylistTrackRepo{AddCount: 2} + mockPlsRepo.Data = map[string]*model.Playlist{ + "pls-1": {ID: "pls-1", Name: "My Playlist", OwnerID: "user-1"}, + "pls-smart": {ID: "pls-smart", Name: "Smart", OwnerID: "user-1", + Rules: &criteria.Criteria{Expression: criteria.Contains{"title": "test"}}}, + } + mockPlsRepo.TracksRepo = mockTracks + ps = playlists.NewPlaylists(ds, artwork.NewUploader(ds)) + }) + + It("inserts the tracks at the given position for the owner", func() { + ctx = request.WithUser(ctx, model.User{ID: "user-1", IsAdmin: false}) + count, err := ps.InsertTracks(ctx, "pls-1", []string{"song-1", "song-2"}, 3) + Expect(err).ToNot(HaveOccurred()) + Expect(count).To(Equal(2)) + Expect(mockTracks.AddedIds).To(Equal([]string{"song-1", "song-2"})) + Expect(mockTracks.InsertPos).To(Equal(3)) + }) + + It("denies non-owner, non-admin", func() { + ctx = request.WithUser(ctx, model.User{ID: "other-user", IsAdmin: false}) + _, err := ps.InsertTracks(ctx, "pls-1", []string{"song-1"}, 1) + Expect(err).To(MatchError(model.ErrNotAuthorized)) + Expect(mockTracks.AddedIds).To(BeEmpty()) + }) + + It("denies editing smart playlists", func() { + ctx = request.WithUser(ctx, model.User{ID: "user-1", IsAdmin: false}) + _, err := ps.InsertTracks(ctx, "pls-smart", []string{"song-1"}, 1) + Expect(err).To(MatchError(model.ErrPlaylistNotEditable)) + }) + }) + Describe("ReorderTrack", func() { var mockTracks *tests.MockPlaylistTrackRepo diff --git a/core/playlists/rest_adapter.go b/core/playlists/rest_adapter.go index ca7cfb0cc..45642c819 100644 --- a/core/playlists/rest_adapter.go +++ b/core/playlists/rest_adapter.go @@ -2,7 +2,6 @@ package playlists import ( "context" - "errors" "reflect" "strings" @@ -41,15 +40,7 @@ func (r *playlistRepositoryWrapper) Update(id string, entity any, cols ...string } func (r *playlistRepositoryWrapper) Delete(id string) error { - err := r.service.Delete(r.ctx, id) - switch { - case errors.Is(err, model.ErrNotFound): - return rest.ErrNotFound - case errors.Is(err, model.ErrNotAuthorized): - return rest.ErrPermissionDenied - default: - return err - } + return r.service.Delete(r.ctx, id) } func (s *playlists) TracksRepository(ctx context.Context, playlistId string, refreshSmartPlaylist bool) rest.Repository { @@ -92,14 +83,7 @@ func (s *playlists) savePlaylist(ctx context.Context, pls *model.Playlist) (stri func (s *playlists) updatePlaylistEntity(ctx context.Context, id string, entity *model.Playlist, cols ...string) error { current, err := s.checkWritable(ctx, id) if err != nil { - switch { - case errors.Is(err, model.ErrNotFound): - return rest.ErrNotFound - case errors.Is(err, model.ErrNotAuthorized): - return rest.ErrPermissionDenied - default: - return err - } + return err } sent := sentFields(cols) diff --git a/core/stream/decider.go b/core/stream/decider.go index 3c6b01e05..886898206 100644 --- a/core/stream/decider.go +++ b/core/stream/decider.go @@ -23,6 +23,7 @@ type TranscodeDecider interface { CreateTranscodeParams(decision *TranscodeDecision) (string, error) ResolveRequestFromToken(ctx context.Context, token string, mf *model.MediaFile, offset int) (Request, error) ResolveRequest(ctx context.Context, mf *model.MediaFile, reqFormat string, reqBitRate int, offset int) Request + ResolveClientRequest(ctx context.Context, mf *model.MediaFile, clientInfo *ClientInfo, offset int) Request } func NewTranscodeDecider(ds model.DataStore, ff ffmpeg.FFmpeg) TranscodeDecider { diff --git a/core/stream/legacy_client.go b/core/stream/legacy_client.go index 652e42eba..5b732c3e2 100644 --- a/core/stream/legacy_client.go +++ b/core/stream/legacy_client.go @@ -73,37 +73,8 @@ func (s *deciderService) ResolveRequest(ctx context.Context, mf *model.MediaFile } clientInfo := buildLegacyClientInfo(mf, reqFormat, reqBitRate, playerMaxBitRate) - - // Apply server-side player transcoding override before making the decision - if trc, ok := request.TranscodingFrom(ctx); ok && trc.TargetFormat != "" { - clientInfo = applyServerOverride(ctx, clientInfo, &trc) - } else if player, ok := request.PlayerFrom(ctx); ok { - modified := *clientInfo - if modified.CapBitrate(player.MaxBitRate) { - clientInfo = &modified - log.Debug(ctx, "Applied player MaxBitRate cap", "playerMaxBitRate", player.MaxBitRate, "client", clientInfo.Name) - } - } - - decision, err := s.MakeDecision(ctx, mf, clientInfo, TranscodeOptions{SkipProbe: true}) - if err != nil { - log.Error(ctx, "Error making transcode decision, falling back to raw", "id", mf.ID, err) - req.Format = "raw" - return req - } - - if decision.CanDirectPlay { - req.Format = "raw" - return req - } - - if decision.CanTranscode { - req.Format = decision.TargetFormat - req.BitRate = decision.TargetBitrate - req.SampleRate = decision.TargetSampleRate - req.BitDepth = decision.TargetBitDepth - req.Channels = decision.TargetChannels - return req + if resolved, ok := s.resolve(ctx, mf, clientInfo, offset); ok { + return resolved } // No compatible profile for the requested format — retry with DefaultDownsamplingFormat @@ -119,3 +90,47 @@ func (s *deciderService) ResolveRequest(ctx context.Context, mf *model.MediaFile req.Format = "raw" return req } + +// ResolveClientRequest resolves a stream request for a client that declared its own direct play +// and transcoding profiles, falling back to raw when none fits. +func (s *deciderService) ResolveClientRequest(ctx context.Context, mf *model.MediaFile, clientInfo *ClientInfo, offset int) Request { + if req, ok := s.resolve(ctx, mf, clientInfo, offset); ok { + return req + } + return Request{Format: "raw", Offset: offset} +} + +// resolve applies the server-side player overrides to clientInfo and maps the decision to a +// Request. ok is false when no profile fits. +func (s *deciderService) resolve(ctx context.Context, mf *model.MediaFile, clientInfo *ClientInfo, offset int) (Request, bool) { + req := Request{Offset: offset} + if trc, ok := request.TranscodingFrom(ctx); ok && trc.TargetFormat != "" { + clientInfo = applyServerOverride(ctx, clientInfo, &trc) + } else if player, ok := request.PlayerFrom(ctx); ok { + modified := *clientInfo + if modified.CapBitrate(player.MaxBitRate) { + clientInfo = &modified + log.Debug(ctx, "Applied player MaxBitRate cap", "playerMaxBitRate", player.MaxBitRate, "client", clientInfo.Name) + } + } + + decision, err := s.MakeDecision(ctx, mf, clientInfo, TranscodeOptions{SkipProbe: true}) + if err != nil { + log.Error(ctx, "Error making transcode decision, falling back to raw", "id", mf.ID, err) + req.Format = "raw" + return req, true + } + switch { + case decision.CanDirectPlay: + req.Format = "raw" + case decision.CanTranscode: + req.Format = decision.TargetFormat + req.BitRate = decision.TargetBitrate + req.SampleRate = decision.TargetSampleRate + req.BitDepth = decision.TargetBitDepth + req.Channels = decision.TargetChannels + default: + return req, false + } + return req, true +} diff --git a/core/stream/legacy_client_test.go b/core/stream/legacy_client_test.go index bc8405976..ad3417faf 100644 --- a/core/stream/legacy_client_test.go +++ b/core/stream/legacy_client_test.go @@ -414,3 +414,72 @@ var _ = Describe("ResolveRequest", func() { }) }) }) + +var _ = Describe("ResolveClientRequest", func() { + var ( + svc TranscodeDecider + ctx context.Context + ) + mp3Target := []Profile{{Container: "mp3", AudioCodec: "mp3", Protocol: ProtocolHTTP}} + + BeforeEach(func() { + ctx = GinkgoT().Context() + ds := &tests.MockDataStore{ + MockedProperty: &tests.MockedPropertyRepo{}, + MockedTranscoding: &tests.MockTranscodingRepo{}, + } + auth.Init(ds) + svc = NewTranscodeDecider(ds, tests.NewMockFFmpeg("")) + }) + + It("direct plays a source matching a profile through container aliases", func() { + mf := withProbe(&model.MediaFile{ID: "1", Suffix: "m4a", Codec: "AAC", BitRate: 256, Channels: 2, SampleRate: 44100}) + ci := &ClientInfo{ + DirectPlayProfiles: []DirectPlayProfile{{Containers: []string{"mp4"}, AudioCodecs: []string{"aac"}}}, + TranscodingProfiles: mp3Target, + } + + req := svc.ResolveClientRequest(ctx, mf, ci, 5) + + Expect(req.Format).To(Equal("raw")) + Expect(req.Offset).To(Equal(5)) + }) + + It("transcodes to the client's transcoding profile when no direct play profile matches", func() { + mf := withProbe(&model.MediaFile{ID: "1", Suffix: "flac", Codec: "FLAC", BitRate: 1000, Channels: 2, SampleRate: 44100, BitDepth: new(16)}) + ci := &ClientInfo{ + DirectPlayProfiles: []DirectPlayProfile{{Containers: []string{"mp3"}}}, + TranscodingProfiles: mp3Target, + } + + Expect(svc.ResolveClientRequest(ctx, mf, ci, 0).Format).To(Equal("mp3")) + }) + + It("transcodes a direct-playable source over the bitrate cap to the client's target", func() { + mf := withProbe(&model.MediaFile{ID: "1", Suffix: "flac", Codec: "FLAC", BitRate: 1000, Channels: 2, SampleRate: 44100, BitDepth: new(16)}) + ci := &ClientInfo{ + MaxAudioBitrate: 128, + DirectPlayProfiles: []DirectPlayProfile{{Containers: []string{"flac"}}}, + TranscodingProfiles: mp3Target, + } + + req := svc.ResolveClientRequest(ctx, mf, ci, 0) + + Expect(req.Format).To(Equal("mp3")) + Expect(req.BitRate).To(Equal(128)) + }) + + It("applies the player's MaxBitRate cap", func() { + mf := withProbe(&model.MediaFile{ID: "1", Suffix: "flac", Codec: "FLAC", BitRate: 1000, Channels: 2, SampleRate: 44100, BitDepth: new(16)}) + ctx = request.WithPlayer(ctx, model.Player{MaxBitRate: 192}) + ci := &ClientInfo{ + DirectPlayProfiles: []DirectPlayProfile{{Containers: []string{"flac"}}}, + TranscodingProfiles: mp3Target, + } + + req := svc.ResolveClientRequest(ctx, mf, ci, 0) + + Expect(req.Format).To(Equal("mp3")) + Expect(req.BitRate).To(Equal(192)) + }) +}) diff --git a/go.mod b/go.mod index 32db6ab1f..9a693d651 100644 --- a/go.mod +++ b/go.mod @@ -9,7 +9,7 @@ require ( github.com/Masterminds/squirrel v1.5.4 github.com/andybalholm/cascadia v1.3.5 github.com/bmatcuk/doublestar/v4 v4.10.0 - github.com/deluan/rest v0.0.0-20211102003136-6260bc399cbf + github.com/deluan/rest v0.0.0-20260913134927-47b21f30cc12 github.com/deluan/sanitize v0.0.0-20241120162836-fdfd8fdfaa55 github.com/dexterlb/mpvipc v0.0.0-20260722094525-0cf47d745b36 github.com/djherbis/atime v1.1.0 diff --git a/go.sum b/go.sum index 5ed48d77a..4cda9a2f3 100644 --- a/go.sum +++ b/go.sum @@ -31,8 +31,8 @@ github.com/decred/dcrd/dcrec/secp256k1/v4 v4.4.1 h1:5RVFMOWjMyRy8cARdy79nAmgYw3h github.com/decred/dcrd/dcrec/secp256k1/v4 v4.4.1/go.mod h1:ZXNYxsqcloTdSy/rNShjYzMhyjf0LaoftYK0p+A3h40= github.com/deluan/go-taglib v0.0.0-20260913142955-d55e0c9353cb h1:CGVY6RtDsqaleUFogGP03m3a/9OKi3ZMDr5nhm51Emk= github.com/deluan/go-taglib v0.0.0-20260913142955-d55e0c9353cb/go.mod h1:QGxQ4Z1IWyY9w56xNEFjYAaWE8uSxA/gneQ7RPcFJrY= -github.com/deluan/rest v0.0.0-20211102003136-6260bc399cbf h1:tb246l2Zmpt/GpF9EcHCKTtwzrd0HGfEmoODFA/qnk4= -github.com/deluan/rest v0.0.0-20211102003136-6260bc399cbf/go.mod h1:tSgDythFsl0QgS/PFWfIZqcJKnkADWneY80jaVRlqK8= +github.com/deluan/rest v0.0.0-20260913134927-47b21f30cc12 h1:x4N/tx0XC9zcnipk5rpdI4qaG9JmovHW5HL97XYnO1I= +github.com/deluan/rest v0.0.0-20260913134927-47b21f30cc12/go.mod h1:tSgDythFsl0QgS/PFWfIZqcJKnkADWneY80jaVRlqK8= github.com/deluan/sanitize v0.0.0-20241120162836-fdfd8fdfaa55 h1:wSCnggTs2f2ji6nFwQmfwgINcmSMj0xF0oHnoyRSPe4= github.com/deluan/sanitize v0.0.0-20241120162836-fdfd8fdfaa55/go.mod h1:ZNCLJfehvEf34B7BbLKjgpsL9lyW7q938w/GY1XgV4E= github.com/dexterlb/mpvipc v0.0.0-20260722094525-0cf47d745b36 h1:KtPfdSST6e0vJbMzMmVqPa5mO1u8vMBlybRCW2ieXpA= diff --git a/model/errors.go b/model/errors.go index 9e56f9501..b3f07565f 100644 --- a/model/errors.go +++ b/model/errors.go @@ -1,11 +1,15 @@ package model -import "errors" +import ( + "errors" + + "github.com/deluan/rest" +) var ( - ErrNotFound = errors.New("data not found") + ErrNotFound = rest.ErrNotFound ErrInvalidAuth = errors.New("invalid authentication") - ErrNotAuthorized = errors.New("not authorized") + ErrNotAuthorized = rest.ErrPermissionDenied ErrExpired = errors.New("access expired") ErrNotAvailable = errors.New("functionality not available") ErrValidation = errors.New("validation error") diff --git a/model/playlist.go b/model/playlist.go index abd11b8c4..f320d845b 100644 --- a/model/playlist.go +++ b/model/playlist.go @@ -184,6 +184,7 @@ type PlaylistTrackRepository interface { GetAlbumIDs(options ...QueryOptions) ([]string, error) GetMediaFileIDs(options ...QueryOptions) ([]string, error) Add(mediaFileIds []string) (int, error) + Insert(mediaFileIds []string, pos int) (int, error) AddAlbums(albumIds []string) (int, error) AddArtists(artistIds []string) (int, error) AddDiscs(discs []DiscID) (int, error) diff --git a/persistence/player_repository.go b/persistence/player_repository.go index 353b0444f..a2b079d53 100644 --- a/persistence/player_repository.go +++ b/persistence/player_repository.go @@ -2,7 +2,6 @@ package persistence import ( "context" - "errors" . "github.com/Masterminds/squirrel" "github.com/deluan/rest" @@ -127,11 +126,7 @@ func (r *playerRepository) Save(entity any) (string, error) { if !r.isPermitted(t) { return "", rest.ErrPermissionDenied } - id, err := r.put(t.ID, t) - if errors.Is(err, model.ErrNotFound) { - return "", rest.ErrNotFound - } - return id, err + return r.put(t.ID, t) } func (r *playerRepository) Update(id string, entity any, cols ...string) error { diff --git a/persistence/playlist_repository.go b/persistence/playlist_repository.go index bc6bffd25..cd3e48d18 100644 --- a/persistence/playlist_repository.go +++ b/persistence/playlist_repository.go @@ -416,9 +416,6 @@ func (r *playlistRepository) Update(id string, entity any, cols ...string) error pls.ID = id pls.UpdatedAt = time.Now() _, err := r.put(id, pls, append(cols, "updatedAt")...) - if errors.Is(err, model.ErrNotFound) { - return rest.ErrNotFound - } return err } diff --git a/persistence/playlist_track_repository.go b/persistence/playlist_track_repository.go index cf1b8f3fa..847316868 100644 --- a/persistence/playlist_track_repository.go +++ b/persistence/playlist_track_repository.go @@ -195,6 +195,29 @@ func (r *playlistTrackRepository) Add(mediaFileIds []string) (int, error) { return len(mediaFileIds), r.playlistRepo.addTracks(r.playlistId, int(res.Max.Int32+1), mediaFileIds) } +// Insert adds tracks before the 1-based position pos, shifting the following entries down; a +// position past the end appends. Callers must run it in a transaction. +func (r *playlistTrackRepository) Insert(mediaFileIds []string, pos int) (int, error) { + if len(mediaFileIds) == 0 { + return 0, nil + } + pos = max(pos, 1) + n := len(mediaFileIds) + // Negate while shifting, so no intermediate row hits the unique (playlist_id, id) index. + _, err := r.executeSQL(Expr(`UPDATE playlist_tracks SET id = -(id + ?) WHERE playlist_id = ? AND id >= ?`, n, r.playlistId, pos)) + if err != nil { + return 0, err + } + res, err := r.executeSQL(Expr(`UPDATE playlist_tracks SET id = -id WHERE playlist_id = ? AND id < 0`, r.playlistId)) + if err != nil { + return 0, err + } + if res == 0 { + return r.Add(mediaFileIds) + } + return n, r.playlistRepo.addTracks(r.playlistId, pos, mediaFileIds) +} + func (r *playlistTrackRepository) addMediaFileIds(cond Sqlizer) (int, error) { sq := Select("id").From("media_file").Where(cond).OrderBy("album_artist, album, release_date, disc_number, track_number") var ids []string @@ -247,8 +270,18 @@ func (r *playlistTrackRepository) DeleteAll() error { return r.playlistRepo.renumber(r.playlistId) } -// Reorder moves a track from pos to newPos, shifting other tracks accordingly. +// Reorder moves a track from pos to newPos, shifting other tracks accordingly. newPos is clamped +// to the playlist; a pos outside it is ErrNotFound, since shifting around it would leave a gap. func (r *playlistTrackRepository) Reorder(pos int, newPos int) error { + var res struct{ Max sql.NullInt32 } + if err := r.queryOne(r.newSelect().Columns("max(id) as max").Where(Eq{"playlist_id": r.playlistId}), &res); err != nil { + return err + } + last := int(res.Max.Int32) + if pos < 1 || pos > last { + return model.ErrNotFound + } + newPos = min(max(newPos, 1), last) if pos == newPos { return nil } diff --git a/persistence/playlist_track_repository_test.go b/persistence/playlist_track_repository_test.go index 88ddbff48..e0360adb4 100644 --- a/persistence/playlist_track_repository_test.go +++ b/persistence/playlist_track_repository_test.go @@ -78,6 +78,105 @@ var _ = Describe("PlaylistTrackRepository", func() { }) }) + Describe("Insert", func() { + var tracks model.PlaylistTrackRepository + + BeforeEach(func() { + ctx := log.NewContext(GinkgoT().Context()) + ctx = request.WithUser(ctx, model.User{ID: "userid", UserName: "userid", IsAdmin: true}) + plsRepo := NewPlaylistRepository(ctx, GetDBXBuilder()) + pls := model.Playlist{Name: "Insert", OwnerID: "userid", OwnerName: "userid"} + Expect(plsRepo.Put(&pls)).To(Succeed()) + DeferCleanup(func() { Expect(plsRepo.Delete(pls.ID)).To(Succeed()) }) + + tracks = plsRepo.Tracks(pls.ID, false) + Expect(tracks.Add([]string{songDayInALife.ID, songRadioactivity.ID})).To(Equal(2)) + }) + + order := func() []string { + ids, err := tracks.GetMediaFileIDs(model.QueryOptions{Sort: "id"}) + Expect(err).ToNot(HaveOccurred()) + return ids + } + + DescribeTable("inserts before a 1-based position, keeping the new tracks' order", + func(pos int, want func() []string) { + Expect(tracks.Insert([]string{songComeTogether.ID, songAntenna.ID}, pos)).To(Equal(2)) + Expect(order()).To(Equal(want())) + Expect(tracks.CountAll()).To(Equal(int64(4))) + }, + Entry("in the middle", 2, func() []string { + return []string{songDayInALife.ID, songComeTogether.ID, songAntenna.ID, songRadioactivity.ID} + }), + Entry("at the start, for zero or less", 0, func() []string { + return []string{songComeTogether.ID, songAntenna.ID, songDayInALife.ID, songRadioactivity.ID} + }), + Entry("at the end, past the last position", 9, func() []string { + return []string{songDayInALife.ID, songRadioactivity.ID, songComeTogether.ID, songAntenna.ID} + }), + ) + + It("renumbers positions contiguously", func() { + Expect(tracks.Insert([]string{songComeTogether.ID}, 1)).To(Equal(1)) + all, err := tracks.GetAll(model.QueryOptions{Sort: "id"}) + Expect(err).ToNot(HaveOccurred()) + Expect([]string{all[0].ID, all[1].ID, all[2].ID}).To(Equal([]string{"1", "2", "3"})) + }) + }) + + Describe("Reorder", func() { + var tracks model.PlaylistTrackRepository + + BeforeEach(func() { + ctx := log.NewContext(GinkgoT().Context()) + ctx = request.WithUser(ctx, model.User{ID: "userid", UserName: "userid", IsAdmin: true}) + plsRepo := NewPlaylistRepository(ctx, GetDBXBuilder()) + pls := model.Playlist{Name: "Reorder", OwnerID: "userid", OwnerName: "userid"} + Expect(plsRepo.Put(&pls)).To(Succeed()) + DeferCleanup(func() { Expect(plsRepo.Delete(pls.ID)).To(Succeed()) }) + + tracks = plsRepo.Tracks(pls.ID, false) + Expect(tracks.Add([]string{songDayInALife.ID, songRadioactivity.ID, songComeTogether.ID})).To(Equal(3)) + }) + + rows := func() ([]string, []string) { + all, err := tracks.GetAll(model.QueryOptions{Sort: "id"}) + Expect(err).ToNot(HaveOccurred()) + var ids, songs []string + for _, t := range all { + ids = append(ids, t.ID) + songs = append(songs, t.MediaFileID) + } + return ids, songs + } + + DescribeTable("clamps the destination to the playlist", + func(newPos int, want func() []string) { + Expect(tracks.Reorder(1, newPos)).To(Succeed()) + ids, songs := rows() + Expect(ids).To(Equal([]string{"1", "2", "3"})) + Expect(songs).To(Equal(want())) + }, + Entry("past the end moves to the end", 9, func() []string { + return []string{songRadioactivity.ID, songComeTogether.ID, songDayInALife.ID} + }), + Entry("below 1 stays first", -4, func() []string { + return []string{songDayInALife.ID, songRadioactivity.ID, songComeTogether.ID} + }), + ) + + DescribeTable("rejects a source position outside the playlist, leaving rows untouched", + func(pos int) { + Expect(tracks.Reorder(pos, 1)).To(MatchError(model.ErrNotFound)) + ids, songs := rows() + Expect(ids).To(Equal([]string{"1", "2", "3"})) + Expect(songs).To(Equal([]string{songDayInALife.ID, songRadioactivity.ID, songComeTogether.ID})) + }, + Entry("past the end", 4), + Entry("zero", 0), + ) + }) + Describe("Delete", func() { var tracks model.PlaylistTrackRepository const numTracks = deleteChunkSize*2 + 1 diff --git a/persistence/radio_repository.go b/persistence/radio_repository.go index e042ee6bf..4584ceaed 100644 --- a/persistence/radio_repository.go +++ b/persistence/radio_repository.go @@ -2,7 +2,6 @@ package persistence import ( "context" - "errors" "time" . "github.com/Masterminds/squirrel" @@ -47,7 +46,7 @@ func (r *radioRepository) Delete(id string) error { return rest.ErrPermissionDenied } - return r.delete(Eq{"id": id}) + return r.deleteByID(id) } func (r *radioRepository) Get(id string) (*model.Radio, error) { @@ -132,9 +131,6 @@ func (r *radioRepository) Save(entity any) (string, error) { return "", rest.ErrPermissionDenied } err := r.Put(t) - if errors.Is(err, model.ErrNotFound) { - return "", rest.ErrNotFound - } return t.ID, err } @@ -144,11 +140,7 @@ func (r *radioRepository) Update(id string, entity any, cols ...string) error { if !r.isPermitted() { return rest.ErrPermissionDenied } - err := r.Put(t, cols...) - if errors.Is(err, model.ErrNotFound) { - return rest.ErrNotFound - } - return err + return r.Put(t, cols...) } var _ model.RadioRepository = (*radioRepository)(nil) diff --git a/persistence/radio_repository_test.go b/persistence/radio_repository_test.go index a958f715d..8d4b685d9 100644 --- a/persistence/radio_repository_test.go +++ b/persistence/radio_repository_test.go @@ -52,6 +52,10 @@ var _ = Describe("RadioRepository", func() { _, err = repo.Get(radioWithHomePage.ID) Expect(err).To(MatchError(model.ErrNotFound)) }) + + It("errors when missing", func() { + Expect(repo.Delete("notanid")).To(MatchError(model.ErrNotFound)) + }) }) Describe("Get", func() { diff --git a/persistence/share_repository.go b/persistence/share_repository.go index bcf33790d..0cf23068a 100644 --- a/persistence/share_repository.go +++ b/persistence/share_repository.go @@ -2,7 +2,6 @@ package persistence import ( "context" - "errors" "fmt" "strings" "time" @@ -178,11 +177,7 @@ func (r *shareRepository) Save(entity any) (string, error) { } s.CreatedAt = time.Now() s.UpdatedAt = time.Now() - id, err := r.put(s.ID, s) - if errors.Is(err, model.ErrNotFound) { - return "", rest.ErrNotFound - } - return id, err + return r.put(s.ID, s) } func (r *shareRepository) CountAll(options ...model.QueryOptions) (int64, error) { diff --git a/persistence/sql_base_repository.go b/persistence/sql_base_repository.go index 5530d2568..9248f2a26 100644 --- a/persistence/sql_base_repository.go +++ b/persistence/sql_base_repository.go @@ -614,12 +614,20 @@ func (r sqlRepository) put(id string, m any, colsToUpdate ...string) (newId stri } func (r sqlRepository) delete(cond Sqlizer) error { - del := Delete(r.tableName).Where(cond) - _, err := r.executeSQL(del) - if errors.Is(err, sql.ErrNoRows) { + _, err := r.executeSQL(Delete(r.tableName).Where(cond)) + return err +} + +// deleteByID is for single-item deletes that must report a missing row; delete succeeds silently. +func (r sqlRepository) deleteByID(id string) error { + count, err := r.executeSQL(Delete(r.tableName).Where(Eq{"id": id})) + if err != nil { + return err + } + if count == 0 { return model.ErrNotFound } - return err + return nil } func (r sqlRepository) logSQL(sql string, args dbx.Params, err error, rowsAffected int64, start time.Time) { diff --git a/persistence/transcoding_repository.go b/persistence/transcoding_repository.go index 96fd3efdb..fdf67806d 100644 --- a/persistence/transcoding_repository.go +++ b/persistence/transcoding_repository.go @@ -2,7 +2,6 @@ package persistence import ( "context" - "errors" . "github.com/Masterminds/squirrel" "github.com/deluan/rest" @@ -91,11 +90,7 @@ func (r *transcodingRepository) Save(entity any) (string, error) { return "", rest.ErrPermissionDenied } t := entity.(*model.Transcoding) - id, err := r.put(t.ID, t) - if errors.Is(err, model.ErrNotFound) { - return "", rest.ErrNotFound - } - return id, err + return r.put(t.ID, t) } func (r *transcodingRepository) Update(id string, entity any, cols ...string) error { @@ -105,9 +100,6 @@ func (r *transcodingRepository) Update(id string, entity any, cols ...string) er t := entity.(*model.Transcoding) t.ID = id _, err := r.put(id, t) - if errors.Is(err, model.ErrNotFound) { - return rest.ErrNotFound - } return err } @@ -115,11 +107,7 @@ func (r *transcodingRepository) Delete(id string) error { if !loggedUser(r.ctx).IsAdmin { return rest.ErrPermissionDenied } - err := r.delete(Eq{"id": id}) - if errors.Is(err, model.ErrNotFound) { - return rest.ErrNotFound - } - return err + return r.deleteByID(id) } var _ model.TranscodingRepository = (*transcodingRepository)(nil) diff --git a/persistence/transcoding_repository_test.go b/persistence/transcoding_repository_test.go index 73250163c..3be46bae3 100644 --- a/persistence/transcoding_repository_test.go +++ b/persistence/transcoding_repository_test.go @@ -65,6 +65,11 @@ var _ = Describe("TranscodingRepository", func() { Expect(err).To(MatchError(model.ErrNotFound)) }) + It("returns not found when deleting a missing transcoding", func() { + err := adminRepo.(*transcodingRepository).Delete("does-not-exist") + Expect(err).To(MatchError(model.ErrNotFound)) + }) + It("reads the Command field via the REST Read method", func() { tr := &model.Transcoding{ID: "adminread", Name: "temp", TargetFormat: "test_format", DefaultBitRate: 64, Command: "ffmpeg -secret"} Expect(adminRepo.Put(tr)).To(Succeed()) diff --git a/persistence/user_repository.go b/persistence/user_repository.go index 9de37876b..9399bcb53 100644 --- a/persistence/user_repository.go +++ b/persistence/user_repository.go @@ -244,11 +244,7 @@ func (r *userRepository) Read(id string) (any, error) { if !usr.IsAdmin && usr.ID != id { return nil, rest.ErrPermissionDenied } - usr, err := r.Get(id) - if errors.Is(err, model.ErrNotFound) { - return nil, rest.ErrNotFound - } - return usr, err + return r.Get(id) } func (r *userRepository) ReadAll(options ...rest.QueryOptions) (any, error) { @@ -314,11 +310,7 @@ func (r *userRepository) Update(id string, entity any, _ ...string) error { if err := validateScrobbleFilter(u); err != nil { return err } - err := r.Put(u) - if errors.Is(err, model.ErrNotFound) { - return rest.ErrNotFound - } - return err + return r.Put(u) } func validatePasswordChange(newUser *model.User, logged *model.User) error { @@ -393,11 +385,7 @@ func (r *userRepository) Delete(id string) error { if !usr.IsAdmin { return rest.ErrPermissionDenied } - err := r.delete(Eq{"id": id}) - if errors.Is(err, model.ErrNotFound) { - return rest.ErrNotFound - } - if err != nil { + if err := r.deleteByID(id); err != nil { return err } diff --git a/persistence/user_repository_test.go b/persistence/user_repository_test.go index dc519d0a1..9a51b17dd 100644 --- a/persistence/user_repository_test.go +++ b/persistence/user_repository_test.go @@ -230,6 +230,14 @@ var _ = Describe("UserRepository", func() { }) }) + Describe("Delete", func() { + It("returns not found for a missing user", func() { + adminCtx := request.WithUser(log.NewContext(GinkgoT().Context()), adminUser) + adminRepo := NewUserRepository(adminCtx, GetDBXBuilder()).(*userRepository) + Expect(adminRepo.Delete("does-not-exist")).To(MatchError(model.ErrNotFound)) + }) + }) + Describe("ReadAll name filter", func() { var adminRepo model.ResourceRepository diff --git a/release/linux/postinstall.sh b/release/linux/postinstall.sh index ed39fb127..007114ef5 100644 --- a/release/linux/postinstall.sh +++ b/release/linux/postinstall.sh @@ -13,15 +13,16 @@ if [ ! -f /etc/navidrome/navidrome.toml ]; then printf "MusicFolder = \"/opt/navidrome/music\"\n" >> /etc/navidrome/navidrome.toml fi +# Older versions created these folders as root when this script ran `navidrome`. They were created empty, +# so no -R. Real dirs only, never following links: navidrome owns /var/lib/navidrome and could plant symlinks. +find /var/lib/navidrome/cache /var/lib/navidrome/artwork /var/lib/navidrome/plugins -maxdepth 0 -type d -user root -exec chown -h navidrome:navidrome {} + 2>/dev/null + postinstall_flag="/var/lib/navidrome/.installed" if [ ! -f "$postinstall_flag" ]; then # The primary reason why this would fail is if the service was already installed AND # someone manually removed the .installed flag. In this case, ignore the error navidrome service install --user navidrome --working-directory /var/lib/navidrome --configfile /etc/navidrome/navidrome.toml || : - # Any `navidrome` command will make a cache. Make sure that this is properly owned by the Navidrome user - # and not by root - chown navidrome:navidrome /var/lib/navidrome/cache touch "$postinstall_flag" else navidrome service stop --configfile /etc/navidrome/navidrome.toml && navidrome service start --configfile /etc/navidrome/navidrome.toml diff --git a/resources/i18n/zh-Hans.json b/resources/i18n/zh-Hans.json index 24f55357a..cae42460b 100644 --- a/resources/i18n/zh-Hans.json +++ b/resources/i18n/zh-Hans.json @@ -93,7 +93,8 @@ "shuffle": "随机播放", "addToPlaylist": "添加到歌单", "download": "下载", - "info": "查看信息" + "info": "查看信息", + "refresh": "刷新元数据" }, "lists": { "all": "全部", @@ -155,11 +156,13 @@ "currentPassword": "当前密码", "newPassword": "新密码", "token": "令牌", - "libraries": "媒体库" + "libraries": "媒体库", + "scrobbleFilter": "个性化记录过滤器" }, "helperTexts": { "name": "名称的更改将在下次登录时生效", - "libraries": "为此用户选择指定媒体库,留空则使用默认媒体库" + "libraries": "为此用户选择指定媒体库,留空则使用默认媒体库", + "scrobbleFilter": "符合这些智能歌单规则的歌曲不会发送到 Last.fm、ListenBrainz 或个性化记录插件。使用与智能歌单相同的 JSON 语法和行为。例如:{\"all\":[{\"lt\":{\"rating\":4}}]}。留空则记录所有播放。本地播放次数不受影响。" }, "notifications": { "created": "用户已创建", @@ -167,7 +170,8 @@ "deleted": "用户已删除" }, "validation": { - "librariesRequired": "至少为非管理员用户选择一个媒体库" + "librariesRequired": "至少为非管理员用户选择一个媒体库", + "invalidScrobbleFilter": "必须是有效的智能歌单规则。不支持数量限制(limit)、偏移量(offset)和刷新延迟(refresh delay)。" }, "message": { "listenBrainzToken": "输入您的 ListenBrainz 用户令牌", @@ -196,6 +200,9 @@ "targetFormat": "目标格式", "defaultBitRate": "默认比特率", "command": "命令" + }, + "choices": { + "noDefaultBitRate": "无" } }, "playlist": { @@ -393,6 +400,7 @@ "invalidJson": "配置必须是有效的 JSON" }, "messages": { + "idHelp": "由插件文件名派生的插件 ID。在 Agents 等配置选项中引用此插件时使用。", "configHelp": "使用键值对配置插件。如果插件无需配置则留空。", "configValidationError": "配置验证失败:", "schemaRenderError": "无法渲染配置表单。此插件的 schema 定义可能无效。", @@ -401,10 +409,10 @@ "allUsersHelp": "启用时,插件将可以访问所有用户,包括将来创建的。", "noUsers": "未选择用户", "permissionReason": "原因", - "usersRequired": "此插件需要访问用户信息。请选择允许此插件访问的用户, 或启用 '允许所有用户'。", + "usersRequired": "此插件需要访问用户信息。请选择允许此插件访问的用户,或启用 '允许所有用户'。", "allLibrariesHelp": "启用时,插件将可以访问所有媒体库,包括将来创建的。", "noLibraries": "未选择媒体库", - "librariesRequired": "此插件需要访问媒体库信息。请选择允许此插件访问的媒体库, 或启用 '允许所有媒体库'。", + "librariesRequired": "此插件需要访问媒体库信息。请选择允许此插件访问的媒体库,或启用 '允许所有媒体库'。", "allowWriteAccessHelp": "启用时,插件将可以修改媒体库目录中的文件。默认情况下,插件仅拥有只读权限。", "requiredHosts": "必需的主机" }, @@ -566,6 +574,7 @@ "coverRemoved": "封面已移除", "coverUploadError": "上传封面时出错", "coverRemoveError": "移除封面时出错", + "metadataRefreshStarted": "正在后台刷新元数据", "note": "注意", "transcodingDisabled": "出于安全原因,从 Web 界面更改转码配置的功能已被禁用。要更改(编辑或新增)转码选项,请在启用 %{config} 选项的情况下重新启动服务器。", "transcodingEnabled": "Navidrome 当前与 %{config} 一起使用,可以通过从 Web 界面配置转码选项来执行任意命令。建议禁用此选项,并且仅在需要配置转码选项时启用此功能。", @@ -598,7 +607,7 @@ "shareOriginalFormat": "分享原始格式", "shareDialogTitle": "分享 %{resource} '%{name}'", "shareBatchDialogTitle": "分享 %{smart_count} 项 %{resource}", - "shareCopyToClipboard": "复制到剪切板: Ctrl+C, Enter", + "shareCopyToClipboard": "复制到剪切板: Ctrl+C,Enter", "shareSuccess": "URL 已复制: %{url}", "shareFailure": "URL 复制失败: %{url}", "downloadDialogTitle": "下载 %{resource} '%{name}' (%{size})", diff --git a/server/jellyfin/README.md b/server/jellyfin/README.md index 5b9a2dba4..893642cdd 100644 --- a/server/jellyfin/README.md +++ b/server/jellyfin/README.md @@ -22,6 +22,8 @@ Enabled = true ServerName = "My Music Server" # Optional: usernames to show in the client login user-picker (default: none). See "Public user list". ExposedPublicUsers = "alice, bob" +# Optional: answer LAN auto-discovery broadcasts on UDP 7359 (default: false). See "Auto discovery". +AutoDiscovery = true # Optional: max collection responses streaming at once (default: half the DB connection pool, # min 2). Each streaming response holds a DB connection for its whole duration; excess requests # queue rather than fail. @@ -34,6 +36,7 @@ or via environment variables: ND_JELLYFIN_ENABLED=true ND_JELLYFIN_SERVERNAME="My Music Server" ND_JELLYFIN_EXPOSEDPUBLICUSERS="alice,bob" +ND_JELLYFIN_AUTODISCOVERY=true ``` Once enabled, the API is mounted at: @@ -46,6 +49,25 @@ All the paths below are relative to that base URL (e.g. `System/Info/Public` mea `http://localhost:4533/jellyfin/System/Info/Public`). Routes are matched **case-insensitively**, since real Jellyfin clients (and `jellyfin-apiclient-python`) send mixed-case paths. +## Auto discovery + +With `AutoDiscovery = true`, Navidrome answers the Jellyfin LAN discovery broadcast +(`who is JellyfinServer?` on UDP port 7359), so clients list the server without a typed URL. +It is off by default because a real Jellyfin server on the same host owns that port. If the port +is taken, Navidrome logs a warning and keeps running without discovery. + +The advertised address is `BaseURL` when it includes a host. Otherwise it is the bind `Address` +when that is a specific IP, or else the local IP that faces the requesting client, plus `Port`. With a +unix socket `Address` there is no port to advertise, so discovery only starts when `BaseURL` has a host. + +Discovery answers on all IPv4 interfaces, but the advertised address follows `BaseURL`, `Address` and +`Port`. If `Address` is a loopback or a single interface IP and `BaseURL` has no host, clients on other +networks get an address they cannot reach. Set `BaseURL` to the address clients should use. + +Docker: publish the port (`-p 7359:7359/udp`). In bridge mode the server only sees its container +IP, so also set `ND_BASEURL` to the LAN address (for example `http://192.168.1.10:4533`), or use +host networking. Keep UDP 7359 on the LAN: never forward it from the internet. + ## Authentication Jellyfin clients authenticate with `POST /Users/AuthenticateByName` using the user's Navidrome @@ -123,13 +145,13 @@ returns direct children only (no tracks — no track is a library's direct child | Users | `GET UserViews`, `GET Users/{userId}/Views`, `GET Users/Me`, `GET Users/{userId}` | | Browsing | `GET Items`, `GET Users/{userId}/Items`, `GET Items/{itemId}`, `GET Users/{userId}/Items/{itemId}`, `GET Users/{userId}/Items/Latest`, `DELETE Items/{itemId}` (playlists only) | | Artists / genres | `GET Artists`, `GET Artists/AlbumArtists`, `GET Genres`, `GET MusicGenres` | -| Similar / mixes | `GET Artists/{itemId}/Similar`, `GET Items/{itemId}/Similar`, `GET Items/{itemId}/InstantMix` | +| Similar / mixes | `GET Artists/{itemId}/Similar`, `GET Items/{itemId}/Similar`, `GET {Items,Songs,Albums,Artists,Playlists}/{itemId}/InstantMix`, `GET Artists/InstantMix?id=`, `GET MusicGenres/InstantMix?id=` | | Images | `GET Items/{itemId}/Images/{type}[/{index}]` (public), `POST`/`DELETE Items/{itemId}/Images/{type}` (playlist cover, authenticated) | | Favorites / ratings for songs, albums, artists, and playlists | `POST`/`DELETE UserFavoriteItems/{itemId}`, `POST`/`DELETE Users/{userId}/FavoriteItems/{itemId}`, `POST`/`DELETE Users/{userId}/Items/{itemId}/Rating`, `GET UserItems/{itemId}/UserData`, `GET Users/{userId}/Items/{itemId}/UserData` | -| Streaming | `GET Audio/{itemId}/stream[.{container}]`, `GET Audio/{itemId}/universal`, `GET Audio/{itemId}/main.m3u8`, `GET Items/{itemId}/File`, `GET Items/{itemId}/Download`, `GET`/`POST Items/{itemId}/PlaybackInfo` | +| Streaming | `GET Audio/{itemId}/stream[.{container}]`, `GET Audio/{itemId}/universal`, `GET Audio/{itemId}/main.m3u8`, `GET Items/{itemId}/File`, `GET Items/{itemId}/Download`, `GET`/`POST Items/{itemId}/PlaybackInfo` (`HEAD` too on stream, universal, File, Download and images; a transcode HEAD answers without starting it) | | Lyrics | `GET Audio/{itemId}/Lyrics` | -| Playback reporting | `POST Sessions/Playing`, `POST Sessions/Playing/Progress`, `POST Sessions/Playing/Stopped`, `POST Sessions/Capabilities[/Full]` | -| Playlists | `POST Playlists`, `GET Playlists/{playlistId}`, `POST Playlists/{playlistId}` (rename / visibility / replace tracks), `GET Playlists/{playlistId}/Items`, `POST`/`DELETE Playlists/{playlistId}/Items`, `GET Playlists/{playlistId}/Users[/{userId}]` | +| Playback reporting | `POST Sessions/Playing`, `POST Sessions/Playing/Progress`, `POST Sessions/Playing/Stopped`, `POST Sessions/Playing/Ping` (no-op), `POST Sessions/Capabilities[/Full]` | +| Playlists | `POST Playlists`, `GET Playlists/{playlistId}`, `POST Playlists/{playlistId}` (rename / visibility / replace tracks), `GET Playlists/{playlistId}/Items`, `POST`/`DELETE Playlists/{playlistId}/Items` (`POST` honors `position`), `POST Playlists/{playlistId}/Items/{entryId}/Move/{newIndex}`, `GET Playlists/{playlistId}/Users[/{userId}]` | | Real-time | `GET socket` (WebSocket; keeps clients like Finamp from 404-loop-reconnecting) | | AudioMuse-AI (see below) | `GET AudioMuseAI/info`, `GET AudioMuseAI/health`, `GET AudioMuseAI/similar_tracks`, `GET AudioMuseAI/find_path` | @@ -350,7 +372,7 @@ make test PKG=./server/jellyfin/... just a list badge): browse lists advertise it from embedded lyrics only (the `"[]"` sentinel check — the column is never `""` post-scan), while `PlaybackInfo` runs the full pipeline per track so sidecar/plugin lyrics also light up. Feishin additionally requires server version - ≥ 10.9 — the reason `jellyfinVersion` is 10.9.11. + ≥ 10.9 (`jellyfinVersion` advertises 12.1.0). Concurrent misses on the same track share one pipeline invocation (`SimpleCache.GetWithLoader` is singleflighted), and the load runs detached from the request context with a one-minute bound, so a cancelled request or hung plugin can't fail or pin the load for other waiters. diff --git a/server/jellyfin/api.go b/server/jellyfin/api.go index 971f7794a..eddd14f66 100644 --- a/server/jellyfin/api.go +++ b/server/jellyfin/api.go @@ -3,7 +3,6 @@ package jellyfin import ( "encoding/json" "net/http" - "sync" "time" "github.com/go-chi/chi/v5" @@ -41,7 +40,6 @@ type Router struct { broker events.Broker lyricsCache cache.SimpleCache[string, model.LyricList] similarFlight singleflight.Group - serverIDMu sync.Mutex serverIDVal string } @@ -93,6 +91,8 @@ func (api *Router) routes() http.Handler { conf.Server.DevArtworkThrottleBacklogTimeout)) r.Get("/items/{itemId}/images/{type}", api.getItemImage) r.Get("/items/{itemId}/images/{type}/{index}", api.getItemImage) + r.Head("/items/{itemId}/images/{type}", api.getItemImage) + r.Head("/items/{itemId}/images/{type}/{index}", api.getItemImage) }) inner.Group(func(r chi.Router) { @@ -145,6 +145,12 @@ func (api *Router) routes() http.Handler { r.Get("/items/{itemId}/similar", api.getSimilarItems) r.Get("/albums/{itemId}/similar", api.getSimilarAlbums) r.Get("/items/{itemId}/instantmix", api.getInstantMix) + r.Get("/songs/{itemId}/instantmix", api.getInstantMix) + r.Get("/albums/{itemId}/instantmix", api.getInstantMix) + r.Get("/artists/{itemId}/instantmix", api.getInstantMix) + r.Get("/playlists/{itemId}/instantmix", api.getInstantMix) + r.Get("/artists/instantmix", api.getInstantMixByQuery) + r.Get("/musicgenres/instantmix", api.getInstantMixByQuery) r.Get("/genres", api.getGenres) r.Get("/musicgenres", api.getGenres) r.Get("/studios", api.getStudios) @@ -155,6 +161,7 @@ func (api *Router) routes() http.Handler { r.Post("/playlists/{playlistId}", api.updatePlaylist) r.Post("/playlists/{playlistId}/items", api.addToPlaylist) r.Delete("/playlists/{playlistId}/items", api.removeFromPlaylist) + r.Post("/playlists/{playlistId}/items/{entryId}/move/{newIndex}", api.movePlaylistItem) r.Get("/playlists/{playlistId}/users", api.getPlaylistUsers) r.Get("/playlists/{playlistId}/users/{userId}", api.getPlaylistUser) @@ -165,7 +172,11 @@ func (api *Router) routes() http.Handler { r.Get("/audio/{itemId}/stream", api.streamAudio) r.Get("/audio/{itemId}/stream.{container}", api.streamAudio) - r.Get("/audio/{itemId}/universal", api.streamAudio) + r.Get("/audio/{itemId}/universal", api.streamUniversal) + // Fintunes probes these with HEAD for the content type before playing or downloading. + r.Head("/audio/{itemId}/stream", api.streamAudio) + r.Head("/audio/{itemId}/stream.{container}", api.streamAudio) + r.Head("/audio/{itemId}/universal", api.streamUniversal) r.Get("/audio/{itemId}/main.m3u8", api.streamHls) r.Get("/items/{itemId}/playbackinfo", api.getPlaybackInfo) r.Post("/items/{itemId}/playbackinfo", api.getPlaybackInfo) @@ -174,12 +185,15 @@ func (api *Router) routes() http.Handler { // /Audio/{id}/stream; /Download reuses the direct-play handler as Jellyfin serves the same file. r.Get("/items/{itemId}/file", api.streamFile) r.Get("/items/{itemId}/download", api.streamFile) + r.Head("/items/{itemId}/file", api.streamFile) + r.Head("/items/{itemId}/download", api.streamFile) r.Post("/sessions/playing", api.reportPlaybackStart) r.Post("/sessions/playing/progress", api.reportPlaybackProgress) r.Post("/sessions/playing/stopped", api.reportPlaybackStopped) - r.Post("/sessions/capabilities", api.postCapabilities) - r.Post("/sessions/capabilities/full", api.postCapabilities) + r.Post("/sessions/playing/ping", api.acknowledge) + r.Post("/sessions/capabilities", api.acknowledge) + r.Post("/sessions/capabilities/full", api.acknowledge) // Real-time clients (e.g. Finamp) open this right after login; without it they 404-loop-reconnect. r.Get("/socket", api.handleSocket) @@ -213,8 +227,7 @@ func (api *Router) ok(w http.ResponseWriter, r *http.Request, payload any) { api.writeItems(w, r, materialized(p)) return case dto.BaseItemDto: - p.ServerId = api.serverID(r.Context()) - payload = p + payload = stampItem(p, api.serverID(r.Context()), requestFields(r)) } w.Header().Set("Content-Type", "application/json; charset=utf-8") if err := json.NewEncoder(w).Encode(payload); err != nil { diff --git a/server/jellyfin/auth.go b/server/jellyfin/auth.go index e7070d341..ba4fe40f1 100644 --- a/server/jellyfin/auth.go +++ b/server/jellyfin/auth.go @@ -42,12 +42,13 @@ func (api *Router) authenticateByName(w http.ResponseWriter, r *http.Request) { return } - // SessionInfo is omitted, not partially filled: a stub {Id, UserId} could fail a strict client's - // parse, and Finamp's login doesn't need it (its AuthenticationResult.sessionInfo is nullable). + a := parseMediaBrowserAuth(r) + serverID := api.serverID(ctx) api.ok(w, r, dto.AuthenticationResult{ - User: userToDto(usr, api.serverName(), api.serverID(ctx)), + User: userToDto(usr, serverName(), serverID), + SessionInfo: dto.NewSessionInfo(usr, a.Client, a.DeviceId, a.Device, a.Version, serverID), AccessToken: token, - ServerId: api.serverID(ctx), + ServerId: serverID, }) } diff --git a/server/jellyfin/auth_test.go b/server/jellyfin/auth_test.go index 577677643..0821614d6 100644 --- a/server/jellyfin/auth_test.go +++ b/server/jellyfin/auth_test.go @@ -48,10 +48,54 @@ var _ = Describe("AuthenticateByName", func() { Expect(res.User.Policy.EnableAllFolders).To(BeTrue()) Expect(res.User.Policy.EnableMediaPlayback).To(BeTrue()) Expect(res.User.Configuration).ToNot(BeNil()) + }) - // Ours is a partial SessionInfo; a strict client may fail to parse it, and Finamp's - // login doesn't require it, so it should be omitted entirely rather than sent partial. - Expect(res.SessionInfo).To(BeNil()) + Describe("SessionInfo", func() { + login := func(authHeader string) map[string]any { + w := httptest.NewRecorder() + r := httptest.NewRequest("POST", "/Users/AuthenticateByName", + strings.NewReader(`{"Username":"alice","Pw":"secret"}`)) + r.Header.Set("Authorization", authHeader) + api.authenticateByName(w, r) + Expect(w.Code).To(Equal(http.StatusOK)) + var raw map[string]any + Expect(json.Unmarshal(w.Body.Bytes(), &raw)).To(Succeed()) + Expect(raw).To(HaveKey("SessionInfo")) + return raw["SessionInfo"].(map[string]any) + } + const jellybox = `MediaBrowser Client="JellyBox", Device="Mac", DeviceId="dev-1", Version="2.1"` + + It("has the fields strict clients require", func() { + s := login(jellybox) + Expect(s["Id"]).To(And(BeAssignableToTypeOf(""), Not(BeEmpty()))) + Expect(s["UserId"]).To(Equal(dto.EncodeID(testID("u1")))) + Expect(s["LastActivityDate"]).To(And(BeAssignableToTypeOf(""), Not(BeEmpty()))) + for _, k := range []string{"SupportsRemoteControl", "SupportsMediaControl", "HasCustomDeviceName"} { + Expect(s[k]).To(BeAssignableToTypeOf(false), k) + } + ps, ok := s["PlayState"].(map[string]any) + Expect(ok).To(BeTrue()) + for _, k := range []string{"CanSeek", "IsPaused", "IsMuted"} { + Expect(ps[k]).To(BeAssignableToTypeOf(false), k) + } + }) + + It("describes the calling client", func() { + s := login(jellybox) + Expect(s).To(HaveKeyWithValue("UserName", "alice")) + Expect(s).To(HaveKeyWithValue("Client", "JellyBox")) + Expect(s).To(HaveKeyWithValue("DeviceName", "Mac")) + Expect(s).To(HaveKeyWithValue("DeviceId", "dev-1")) + Expect(s).To(HaveKeyWithValue("ApplicationVersion", "2.1")) + Expect(s).To(HaveKeyWithValue("IsActive", true)) + }) + + It("keeps the same Id for the same device, and a new one for another device", func() { + first := login(jellybox)["Id"] + Expect(login(jellybox)["Id"]).To(Equal(first)) + other := login(`MediaBrowser Client="JellyBox", Device="Mac", DeviceId="dev-2", Version="2.1"`) + Expect(other["Id"]).ToNot(Equal(first)) + }) }) It("records the login time, like the web UI login does", func() { diff --git a/server/jellyfin/discovery.go b/server/jellyfin/discovery.go new file mode 100644 index 000000000..d111287a6 --- /dev/null +++ b/server/jellyfin/discovery.go @@ -0,0 +1,110 @@ +package jellyfin + +import ( + "context" + "encoding/json" + "net" + "strconv" + "strings" + + "github.com/navidrome/navidrome/conf" + "github.com/navidrome/navidrome/consts" + "github.com/navidrome/navidrome/core/publicurl" + "github.com/navidrome/navidrome/log" + "github.com/navidrome/navidrome/model" + "github.com/navidrome/navidrome/model/request" + "github.com/navidrome/navidrome/utils/gg" +) + +// Jellyfin clients broadcast this query text to this UDP port. +const ( + discoveryPort = 7359 + discoveryQuery = "who is jellyfinserver?" +) + +type discoveryInfo struct { + Address string `json:"Address"` + Id string `json:"Id"` + Name string `json:"Name"` + EndpointAddress *string `json:"EndpointAddress"` +} + +// Discovery answers LAN auto-discovery broadcasts with the same identity the Router reports. +type Discovery struct { + ds model.DataStore + serverIDVal string +} + +func NewDiscovery(ds model.DataStore) *Discovery { + return &Discovery{ds: ds} +} + +func (d *Discovery) serverID(ctx context.Context) string { + return resolveServerID(ctx, d.ds, &d.serverIDVal) +} + +// Serve runs until ctx is done. A failed bind is only logged: discovery is best-effort. +func (d *Discovery) Serve(ctx context.Context) { + if !hasAdvertisableAddress() { + log.Warn(ctx, "Jellyfin API: auto-discovery is off, a unix socket server needs a BaseURL with a host to advertise") + return + } + // udp4 only: a dual-stack bind can share the port with another server and never get a packet. + conn, err := net.ListenPacket("udp4", net.JoinHostPort("0.0.0.0", strconv.Itoa(discoveryPort))) + if err != nil { + log.Warn(ctx, "Jellyfin API: auto-discovery is off, the UDP port is unavailable. Is another Jellyfin server running?", "port", discoveryPort, err) + return + } + log.Info(ctx, "Jellyfin API: listening for auto-discovery broadcasts", "port", discoveryPort) + d.ServeOn(ctx, conn) +} + +// ServeOn answers discovery queries on conn until ctx is done, then closes conn. +func (d *Discovery) ServeOn(ctx context.Context, conn net.PacketConn) { + defer conn.Close() + stop := context.AfterFunc(ctx, func() { _ = conn.Close() }) + defer stop() + buf := make([]byte, 1024) + for { + n, remote, err := conn.ReadFrom(buf) + if err != nil { + if ctx.Err() == nil { + log.Error(ctx, "Jellyfin API: auto-discovery listener stopped", err) + } + return + } + if !strings.Contains(strings.ToLower(string(buf[:n])), discoveryQuery) { + continue + } + info := discoveryInfo{Address: discoveryAddress(ctx, remote), Id: d.serverID(ctx), Name: serverName()} + res, _ := json.Marshal(info) + log.Debug(ctx, "Jellyfin API: answering auto-discovery request", "from", remote.String(), "address", info.Address) + if _, err := conn.WriteTo(res, remote); err != nil { + log.Debug(ctx, "Jellyfin API: could not answer auto-discovery request", "to", remote.String(), err) + } + } +} + +// Behind a unix socket nothing listens on Port, so only a BaseURL host gives clients an address. +func hasAdvertisableAddress() bool { + return conf.Server.BaseHost != "" || !strings.HasPrefix(conf.Server.Address, "unix:") +} + +func discoveryAddress(ctx context.Context, remote net.Addr) string { + scheme := gg.If(conf.Server.TLSEnabled(), "https", "http") + host := net.JoinHostPort(localIPFor(remote), strconv.Itoa(conf.Server.Port)) + return publicurl.AbsoluteURL(request.WithServerAddress(ctx, scheme, host), consts.URLPathJellyfinAPI, nil) +} + +// On a multi-homed host, only the interface that routes to the requester is reachable by it. +func localIPFor(remote net.Addr) string { + if ip := parseIP(conf.Server.Address); ip.IsValid() && !ip.IsUnspecified() { + return ip.String() + } + c, err := net.Dial("udp", remote.String()) + if err != nil { + return conf.Server.Address + } + defer c.Close() + return c.LocalAddr().(*net.UDPAddr).IP.String() +} diff --git a/server/jellyfin/discovery_test.go b/server/jellyfin/discovery_test.go new file mode 100644 index 000000000..bcd11fe76 --- /dev/null +++ b/server/jellyfin/discovery_test.go @@ -0,0 +1,175 @@ +package jellyfin + +import ( + "context" + "encoding/json" + "errors" + "net" + "os" + "time" + + "github.com/navidrome/navidrome/conf" + "github.com/navidrome/navidrome/conf/configtest" + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" +) + +type failingConn struct { + net.PacketConn + closed bool +} + +func (c *failingConn) ReadFrom([]byte) (int, net.Addr, error) { + return 0, nil, errors.New("read failed") +} + +func (c *failingConn) Close() error { + c.closed = true + return nil +} + +var _ = Describe("Discovery", func() { + var d *Discovery + + BeforeEach(func() { + DeferCleanup(configtest.SetupConfig()) + conf.Server.Jellyfin.ServerName = "Test Server" + conf.Server.Address = "0.0.0.0" + conf.Server.Port = 4533 + conf.Server.BaseHost = "" + conf.Server.BaseScheme = "" + conf.Server.BasePath = "" + conf.Server.TLSCert = "" + conf.Server.TLSKey = "" + d = &Discovery{} + }) + + DescribeTable("discoveryAddress", + func(setup func(), expected string) { + setup() + remote := &net.UDPAddr{IP: net.ParseIP("127.0.0.1"), Port: 50000} + Expect(discoveryAddress(context.Background(), remote)).To(Equal(expected)) + }, + Entry("uses the BaseURL host and scheme when set", func() { + conf.Server.BaseScheme = "https" + conf.Server.BaseHost = "music.example.com" + conf.Server.BasePath = "/nd" + }, "https://music.example.com/nd/jellyfin"), + Entry("uses a specific bind Address with the Port", func() { + conf.Server.Address = "192.168.1.10" + }, "http://192.168.1.10:4533/jellyfin"), + Entry("falls back to the interface facing the requester when Address is unspecified", func() {}, + "http://127.0.0.1:4533/jellyfin"), + Entry("falls back to the interface facing the requester when Address is empty", func() { + conf.Server.Address = "" + }, "http://127.0.0.1:4533/jellyfin"), + Entry("advertises https when TLS is configured", func() { + conf.Server.TLSCert = "/path/cert.pem" + conf.Server.TLSKey = "/path/key.pem" + }, "https://127.0.0.1:4533/jellyfin"), + Entry("advertises http when only the TLS cert is configured", func() { + conf.Server.TLSCert = "/path/cert.pem" + }, "http://127.0.0.1:4533/jellyfin"), + Entry("keeps a path-only BaseURL as the path prefix", func() { + conf.Server.BasePath = "/music" + }, "http://127.0.0.1:4533/music/jellyfin"), + Entry("does not double the slash when BasePath has a trailing slash", func() { + conf.Server.BasePath = "/music/" + }, "http://127.0.0.1:4533/music/jellyfin"), + ) + + DescribeTable("hasAdvertisableAddress", + func(address, baseHost string, expected bool) { + conf.Server.Address = address + conf.Server.BaseHost = baseHost + Expect(hasAdvertisableAddress()).To(Equal(expected)) + }, + Entry("TCP listener", "0.0.0.0", "", true), + Entry("unix socket without a BaseURL host", "unix:/tmp/navidrome.sock", "", false), + Entry("unix socket behind a proxy named by BaseURL", "unix:/tmp/navidrome.sock", "music.example.com", true), + ) + + It("closes the connection when the read loop fails", func() { + fake := &failingConn{} + d.ServeOn(context.Background(), fake) + Expect(fake.closed).To(BeTrue()) + }) + + Describe("ServeOn", func() { + var ( + server net.PacketConn + client net.PacketConn + cancel context.CancelFunc + done chan struct{} + ) + + BeforeEach(func() { + var err error + server, err = net.ListenPacket("udp4", "127.0.0.1:0") + Expect(err).ToNot(HaveOccurred()) + client, err = net.ListenPacket("udp4", "127.0.0.1:0") + Expect(err).ToNot(HaveOccurred()) + DeferCleanup(client.Close) + + var ctx context.Context + ctx, cancel = context.WithCancel(context.Background()) + done = make(chan struct{}) + go func() { + defer close(done) + d.ServeOn(ctx, server) + }() + DeferCleanup(func() { + cancel() + Eventually(done).Should(BeClosed()) + }) + }) + + send := func(msg string) { + _, err := client.WriteTo([]byte(msg), server.LocalAddr()) + Expect(err).ToNot(HaveOccurred()) + } + receive := func() ([]byte, error) { + Expect(client.SetReadDeadline(time.Now().Add(time.Second))).To(Succeed()) + buf := make([]byte, 1024) + n, _, err := client.ReadFrom(buf) + return buf[:n], err + } + + It("answers the discovery query with the server identity", func() { + send("who is JellyfinServer?") + res, err := receive() + Expect(err).ToNot(HaveOccurred()) + + var info discoveryInfo + Expect(json.Unmarshal(res, &info)).To(Succeed()) + Expect(info.Address).To(Equal("http://127.0.0.1:4533/jellyfin")) + Expect(info.Id).To(Equal(d.serverID(context.Background()))) + Expect(info.Name).To(Equal("Test Server")) + Expect(string(res)).To(ContainSubstring(`"EndpointAddress":null`)) + }) + + It("matches the query case-insensitively", func() { + send("WHO IS JELLYFINSERVER?") + _, err := receive() + Expect(err).ToNot(HaveOccurred()) + }) + + // The loop is serial, so a reply to the first packet would arrive before the second's. + It("ignores unrelated packets", func() { + send("who is PlexServer?") + send("who is JellyfinServer?") + _, err := receive() + Expect(err).ToNot(HaveOccurred()) + Expect(client.SetReadDeadline(time.Now().Add(50 * time.Millisecond))).To(Succeed()) + _, _, err = client.ReadFrom(make([]byte, 1024)) + Expect(errors.Is(err, os.ErrDeadlineExceeded)).To(BeTrue()) + }) + + It("stops and closes the socket when the context is cancelled", func() { + cancel() + Eventually(done).Should(BeClosed()) + _, _, err := server.ReadFrom(make([]byte, 1)) + Expect(err).To(HaveOccurred()) + }) + }) +}) diff --git a/server/jellyfin/dto/dto.go b/server/jellyfin/dto/dto.go index 58a6b919e..21e3903f2 100644 --- a/server/jellyfin/dto/dto.go +++ b/server/jellyfin/dto/dto.go @@ -49,20 +49,23 @@ type BaseItemDto struct { // PlaylistItemId identifies an entry within a playlist listing (GET /Playlists/{id}/Items), // distinct from Id so a song appearing more than once can be removed by occurrence // (DELETE .../Items?EntryIds=...) rather than by song id. - PlaylistItemId string `json:"PlaylistItemId,omitempty"` - Type string `json:"Type"` - IsFolder bool `json:"IsFolder"` - MediaType string `json:"MediaType,omitempty"` - CollectionType string `json:"CollectionType,omitempty"` - LocationType string `json:"LocationType,omitempty"` - HasLyrics bool `json:"HasLyrics,omitempty"` - SortName string `json:"SortName,omitempty"` - Path string `json:"Path,omitempty"` - ParentId string `json:"ParentId,omitempty"` - RunTimeTicks int64 `json:"RunTimeTicks,omitempty"` - IndexNumber *int `json:"IndexNumber,omitempty"` - ParentIndexNumber *int `json:"ParentIndexNumber,omitempty"` - ProductionYear *int `json:"ProductionYear,omitempty"` + PlaylistItemId string `json:"PlaylistItemId,omitempty"` + Type string `json:"Type"` + IsFolder bool `json:"IsFolder"` + MediaType string `json:"MediaType,omitempty"` + CollectionType string `json:"CollectionType,omitempty"` + LocationType string `json:"LocationType,omitempty"` + HasLyrics *bool `json:"HasLyrics,omitempty"` + // ChannelId is always null for music, but Jellyfin emits it on every item and clients may require it. + ChannelId *string `json:"ChannelId"` + Tags []string `json:"Tags,omitzero"` + SortName string `json:"SortName,omitempty"` + Path string `json:"Path,omitempty"` + ParentId string `json:"ParentId,omitempty"` + RunTimeTicks int64 `json:"RunTimeTicks,omitempty"` + IndexNumber *int `json:"IndexNumber,omitempty"` + ParentIndexNumber *int `json:"ParentIndexNumber,omitempty"` + ProductionYear *int `json:"ProductionYear,omitempty"` // PremiereDate is the ISO 8601 release date; Finamp sorts "Latest Releases" by it client-side. PremiereDate *string `json:"PremiereDate,omitempty"` // DateCreated is the ISO 8601 date the item was added to the library; clients show it as @@ -73,17 +76,17 @@ type BaseItemDto struct { AlbumArtist string `json:"AlbumArtist,omitempty"` AlbumArtists []NameGuidPair `json:"AlbumArtists,omitempty"` AlbumPrimaryImageTag string `json:"AlbumPrimaryImageTag,omitempty"` - Artists []string `json:"Artists,omitempty"` + Artists []string `json:"Artists,omitzero"` ArtistItems []NameGuidPair `json:"ArtistItems,omitempty"` - Genres []string `json:"Genres,omitempty"` - GenreItems []NameGuidPair `json:"GenreItems,omitempty"` + Genres []string `json:"Genres,omitzero"` + GenreItems []NameGuidPair `json:"GenreItems,omitzero"` Studios []NameGuidPair `json:"Studios,omitempty"` NormalizationGain *float64 `json:"NormalizationGain,omitempty"` AlbumNormalizationGain *float64 `json:"AlbumNormalizationGain,omitempty"` ChildCount *int `json:"ChildCount,omitempty"` SongCount *int `json:"SongCount,omitempty"` AlbumCount *int `json:"AlbumCount,omitempty"` - ImageTags map[string]string `json:"ImageTags,omitempty"` + ImageTags map[string]string `json:"ImageTags"` // ImageBlurHashes is keyed by image type (e.g. "Primary") then image tag. Finamp uses it as a // de-dup key for image downloads (and a placeholder); absent, it warns the server isn't // calculating blurhashes. @@ -199,14 +202,40 @@ type UserConfiguration struct { CastReceiverId string `json:"CastReceiverId"` } +// SessionInfo mirrors real Jellyfin's SessionInfoDto. JellyBox requires Id and PlayState, and Finamp +// requires UserId, LastActivityDate and the bools, so none of those may be omitted. type SessionInfo struct { - Id string `json:"Id"` - UserId string `json:"UserId"` + Id string `json:"Id"` + UserId string `json:"UserId"` + UserName string `json:"UserName"` + Client string `json:"Client"` + DeviceId string `json:"DeviceId"` + DeviceName string `json:"DeviceName"` + ApplicationVersion string `json:"ApplicationVersion"` + ServerId string `json:"ServerId"` + LastActivityDate string `json:"LastActivityDate"` + IsActive bool `json:"IsActive"` + SupportsMediaControl bool `json:"SupportsMediaControl"` + SupportsRemoteControl bool `json:"SupportsRemoteControl"` + HasCustomDeviceName bool `json:"HasCustomDeviceName"` + PlayableMediaTypes []string `json:"PlayableMediaTypes"` + SupportedCommands []string `json:"SupportedCommands"` + AdditionalUsers []any `json:"AdditionalUsers"` + NowPlayingQueue []any `json:"NowPlayingQueue"` + PlayState PlayerStateInfo `json:"PlayState"` +} + +type PlayerStateInfo struct { + CanSeek bool `json:"CanSeek"` + IsPaused bool `json:"IsPaused"` + IsMuted bool `json:"IsMuted"` + RepeatMode string `json:"RepeatMode"` + PlaybackOrder string `json:"PlaybackOrder"` } type AuthenticationResult struct { User *UserDto `json:"User"` - SessionInfo *SessionInfo `json:"SessionInfo,omitempty"` + SessionInfo *SessionInfo `json:"SessionInfo"` AccessToken string `json:"AccessToken"` ServerId string `json:"ServerId"` } diff --git a/server/jellyfin/dto/mappers.go b/server/jellyfin/dto/mappers.go index 1ebf66ca1..bb17b40c7 100644 --- a/server/jellyfin/dto/mappers.go +++ b/server/jellyfin/dto/mappers.go @@ -7,6 +7,7 @@ import ( "github.com/navidrome/navidrome/conf" "github.com/navidrome/navidrome/model" + "github.com/navidrome/navidrome/model/id" "github.com/navidrome/navidrome/utils/slice" ) @@ -46,17 +47,24 @@ func premiereDate(date string, year int) *string { } d = fmt.Sprintf("%04d-01-01", year) } - s := d + "T00:00:00Z" + parsed, err := time.Parse(time.DateOnly, d) + if err != nil { + return nil + } + s := jellyfinDate(&parsed) return &s } -// jellyfinDate formats t as the ISO 8601 string clients expect, or "" for the zero time so the +// Dates use .NET's round-trip layout, 7 fractional digits and all: Manet rejects plain RFC3339. +const jellyfinDateLayout = "2006-01-02T15:04:05.0000000Z07:00" + +// jellyfinDate formats t as the date string clients expect, or "" for the zero time so the // field is omitted rather than sent as a meaningless epoch. func jellyfinDate(t *time.Time) string { if t == nil || t.IsZero() { return "" } - return t.UTC().Format(time.RFC3339) + return t.UTC().Format(jellyfinDateLayout) } // channelLayout maps a channel count to the label Jellyfin clients expect on a MediaStream. @@ -127,8 +135,7 @@ func UserData(a model.Annotations, itemID string) *UserItemDataDto { r := float64(a.Rating) * 2 // Navidrome 0-5 -> Jellyfin 0-10 d.Rating = &r } - if a.PlayDate != nil { - s := a.PlayDate.UTC().Format(time.RFC3339) + if s := jellyfinDate(a.PlayDate); s != "" { d.LastPlayedDate = &s } return d @@ -146,7 +153,7 @@ func SongToBaseItem(mf model.MediaFile, fields Fields) BaseItemDto { MediaType: "Audio", IsFolder: false, LocationType: "FileSystem", - HasLyrics: mf.HasEmbeddedLyrics(), + HasLyrics: new(mf.HasEmbeddedLyrics()), ParentId: albumID, Album: mf.Album, AlbumId: albumID, @@ -251,6 +258,7 @@ func AlbumToBaseItem(al model.Album, fields Fields) BaseItemDto { Id: EncodeID(al.ID), Type: "MusicAlbum", IsFolder: true, + LocationType: "FileSystem", ParentId: EncodeID(al.AlbumArtistID), AlbumArtist: al.AlbumArtist, Album: al.Name, @@ -266,6 +274,10 @@ func AlbumToBaseItem(al model.Album, fields Fields) BaseItemDto { if tag != "" { item.ImageTags = map[string]string{"Primary": tag} } + item.Artists = []string{} + if al.AlbumArtist != "" { + item.Artists = append(item.Artists, al.AlbumArtist) + } if al.AlbumArtistID != "" { item.AlbumArtists = []NameGuidPair{{Name: al.AlbumArtist, Id: EncodeID(al.AlbumArtistID)}} item.ArtistItems = item.AlbumArtists @@ -321,6 +333,26 @@ func ArtistToBaseItem(ar model.Artist, fields Fields) BaseItemDto { return item } +// LibraryToBaseItem maps a library to the CollectionFolder item clients browse as a top-level node. +// Manet keeps no library, and so syncs nothing, unless it carries the fields Jellyfin sends here. +func LibraryToBaseItem(lib model.Library) BaseItemDto { + id := EncodeLibraryID(lib.ID) + return BaseItemDto{ + Id: id, + Name: lib.Name, + SortName: lib.Name, + Type: "CollectionFolder", + CollectionType: "music", + IsFolder: true, + Path: lib.Path, + LocationType: "FileSystem", + DateCreated: jellyfinDate(&lib.CreatedAt), + ChildCount: new(lib.TotalAlbums), + UserData: &UserItemDataDto{Key: id, ItemId: id}, + BackdropImageTags: []string{}, + } +} + func GenreToBaseItem(g model.Genre) BaseItemDto { return BaseItemDto{ Name: g.Name, @@ -354,6 +386,7 @@ func PlaylistToBaseItem(p model.Playlist, fields Fields) BaseItemDto { MediaType: "Audio", ChildCount: new(p.SongCount), RunTimeTicks: TicksFromSeconds(p.Duration), + DateCreated: jellyfinDate(&p.CreatedAt), ImageBlurHashes: blurs, PrimaryImageAspectRatio: ratio, BackdropImageTags: []string{}, @@ -362,6 +395,10 @@ func PlaylistToBaseItem(p model.Playlist, fields Fields) BaseItemDto { if tag != "" { item.ImageTags = map[string]string{"Primary": tag} } + // Playlists have no sort tag; the repository orders them by name. + if fields.Has("SortName") { + item.SortName = p.Name + } return item } @@ -410,3 +447,25 @@ func LyricDtoFromLyrics(mf model.MediaFile, lyrics model.Lyrics) LyricDto { } return d } + +// NewSessionInfo's Id is stable per client install, as Jellyfin reuses a device's session across logins. +func NewSessionInfo(u *model.User, client, deviceID, deviceName, version, serverID string) *SessionInfo { + now := time.Now() + return &SessionInfo{ + Id: EncodeID(id.NewHash(client, deviceID)), + UserId: EncodeID(u.ID), + UserName: u.UserName, + Client: client, + DeviceId: deviceID, + DeviceName: deviceName, + ApplicationVersion: version, + ServerId: serverID, + LastActivityDate: jellyfinDate(&now), + IsActive: true, + PlayableMediaTypes: []string{"Audio"}, + SupportedCommands: []string{}, + AdditionalUsers: []any{}, + NowPlayingQueue: []any{}, + PlayState: PlayerStateInfo{RepeatMode: "RepeatNone", PlaybackOrder: "Default"}, + } +} diff --git a/server/jellyfin/dto/mappers_test.go b/server/jellyfin/dto/mappers_test.go index 576482e61..6dc177363 100644 --- a/server/jellyfin/dto/mappers_test.go +++ b/server/jellyfin/dto/mappers_test.go @@ -104,10 +104,10 @@ var _ = Describe("mappers", func() { }) It("sets HasLyrics from the media file's lyrics", func() { - Expect(SongToBaseItem(mf, nil).HasLyrics).To(BeTrue()) - Expect(SongToBaseItem(model.MediaFile{ID: testID("s2"), Title: "No Lyrics"}, nil).HasLyrics).To(BeFalse()) + Expect(*SongToBaseItem(mf, nil).HasLyrics).To(BeTrue()) + Expect(*SongToBaseItem(model.MediaFile{ID: testID("s2"), Title: "No Lyrics"}, nil).HasLyrics).To(BeFalse()) // "[]" is the no-lyrics sentinel, not a truthy value. - Expect(SongToBaseItem(model.MediaFile{ID: testID("s3"), Title: "Empty Lyrics", Lyrics: "[]"}, nil).HasLyrics).To(BeFalse()) + Expect(*SongToBaseItem(model.MediaFile{ID: testID("s3"), Title: "Empty Lyrics", Lyrics: "[]"}, nil).HasLyrics).To(BeFalse()) }) }) @@ -120,7 +120,7 @@ var _ = Describe("mappers", func() { It("sets DateCreated from the media file's CreatedAt", func() { mf := model.MediaFile{ID: testID("s1"), Title: "Song", CreatedAt: time.Date(2024, 1, 15, 10, 30, 0, 0, time.UTC)} - Expect(SongToBaseItem(mf, nil).DateCreated).To(Equal("2024-01-15T10:30:00Z")) + Expect(SongToBaseItem(mf, nil).DateCreated).To(Equal("2024-01-15T10:30:00.0000000Z")) }) It("omits DateCreated when CreatedAt is the zero time", func() { @@ -282,7 +282,7 @@ var _ = Describe("mappers", func() { mf.PlayDate = &playDate item := SongToBaseItem(mf, nil) Expect(item.UserData.LastPlayedDate).NotTo(BeNil()) - Expect(*item.UserData.LastPlayedDate).To(Equal(playDate.Format(time.RFC3339))) + Expect(*item.UserData.LastPlayedDate).To(Equal("2023-05-17T12:30:00.0000000Z")) }) It("maps an album to a MusicAlbum folder item", func() { @@ -299,6 +299,9 @@ var _ = Describe("mappers", func() { Expect(*item.ChildCount).To(Equal(10)) Expect(item.ImageTags).To(HaveKeyWithValue("Primary", testID("alb-1"))) Expect(item.ImageBlurHashes).To(BeNil()) + // Jellyfin sends both on every album; Manet stops its sync on an album without them. + Expect(item.Artists).To(Equal([]string{"AA"})) + Expect(item.LocationType).To(Equal("FileSystem")) Expect(item.Genres).To(Equal([]string{"genre 1", "genre 2"})) Expect(item.GenreItems).To(Equal([]NameGuidPair{{Id: EncodeID(testID("1")), Name: "genre 1"}, {Id: EncodeID(testID("2")), Name: "genre 2"}})) }) @@ -426,22 +429,22 @@ var _ = Describe("mappers", func() { It("serializes a full date", func() { mf := model.MediaFile{ID: testID("s1"), Title: "Song", Date: "2007-02-01", Year: 2007} item := SongToBaseItem(mf, nil) - Expect(*item.PremiereDate).To(Equal("2007-02-01T00:00:00Z")) + Expect(*item.PremiereDate).To(Equal("2007-02-01T00:00:00.0000000Z")) }) It("pads a year-only date so clients can parse it", func() { mf := model.MediaFile{ID: testID("s1"), Title: "Song", Date: "2007", Year: 2007} - Expect(*SongToBaseItem(mf, nil).PremiereDate).To(Equal("2007-01-01T00:00:00Z")) + Expect(*SongToBaseItem(mf, nil).PremiereDate).To(Equal("2007-01-01T00:00:00.0000000Z")) }) It("pads a year-month date", func() { mf := model.MediaFile{ID: testID("s1"), Title: "Song", Date: "2007-02"} - Expect(*SongToBaseItem(mf, nil).PremiereDate).To(Equal("2007-02-01T00:00:00Z")) + Expect(*SongToBaseItem(mf, nil).PremiereDate).To(Equal("2007-02-01T00:00:00.0000000Z")) }) It("falls back to the year when no date tag exists", func() { mf := model.MediaFile{ID: testID("s1"), Title: "Song", Year: 1999} - Expect(*SongToBaseItem(mf, nil).PremiereDate).To(Equal("1999-01-01T00:00:00Z")) + Expect(*SongToBaseItem(mf, nil).PremiereDate).To(Equal("1999-01-01T00:00:00.0000000Z")) }) It("is omitted when the track has no date at all", func() { @@ -449,8 +452,8 @@ var _ = Describe("mappers", func() { }) It("is set on albums from their date, falling back to MaxYear", func() { - Expect(*AlbumToBaseItem(model.Album{ID: testID("a1"), Date: "2013-09-06"}, nil).PremiereDate).To(Equal("2013-09-06T00:00:00Z")) - Expect(*AlbumToBaseItem(model.Album{ID: testID("a2"), MaxYear: 2013}, nil).PremiereDate).To(Equal("2013-01-01T00:00:00Z")) + Expect(*AlbumToBaseItem(model.Album{ID: testID("a1"), Date: "2013-09-06"}, nil).PremiereDate).To(Equal("2013-09-06T00:00:00.0000000Z")) + Expect(*AlbumToBaseItem(model.Album{ID: testID("a2"), MaxYear: 2013}, nil).PremiereDate).To(Equal("2013-01-01T00:00:00.0000000Z")) Expect(AlbumToBaseItem(model.Album{ID: testID("a3")}, nil).PremiereDate).To(BeNil()) }) }) @@ -475,6 +478,13 @@ var _ = Describe("mappers", func() { Expect(item.ImageBlurHashes).To(BeNil()) }) + It("emits DateCreated and SortName on playlists, like albums and artists", func() { + p := model.Playlist{ID: testID("pl-1"), Name: "Chill", CreatedAt: time.Date(2026, 7, 1, 12, 0, 0, 0, time.UTC)} + Expect(PlaylistToBaseItem(p, nil).DateCreated).To(Equal("2026-07-01T12:00:00.0000000Z")) + Expect(PlaylistToBaseItem(p, nil).SortName).To(BeEmpty()) + Expect(PlaylistToBaseItem(p, ParseFields("SortName")).SortName).To(Equal("Chill")) + }) + It("changes the playlist image tag when the cover content changes", func() { p := model.Playlist{ID: testID("pl-1"), Name: "Chill"} p.ImageHash = "1111111111111111" diff --git a/server/jellyfin/e2e/browsing_test.go b/server/jellyfin/e2e/browsing_test.go index 832b69f25..68d87cd25 100644 --- a/server/jellyfin/e2e/browsing_test.go +++ b/server/jellyfin/e2e/browsing_test.go @@ -1,6 +1,7 @@ package e2e import ( + "encoding/json" "net/http" "slices" "sort" @@ -110,10 +111,51 @@ var _ = Describe("Browsing", func() { Expect(q.Items).To(BeEmpty()) }) - It("defaults to albums when IncludeItemTypes is unrecognized", func() { + // Manet syncs collections as Boxset, and took albums coming back instead as a sync failure. + DescribeTable("returns nothing for a type it does not serve", + func(itemType string) { + q := queryResult(get("/Items?IncludeItemTypes=" + itemType + "&Recursive=true")) + Expect(q.TotalRecordCount).To(Equal(0)) + Expect(q.Items).To(BeEmpty()) + }, + Entry("a Jellyfin kind Navidrome has none of", "Boxset"), + ) + + It("treats a name Jellyfin does not know as an absent IncludeItemTypes", func() { q := queryResult(get("/Items?IncludeItemTypes=Nonsense&Recursive=true")) - Expect(q.TotalRecordCount).To(Equal(5)) + Expect(q.TotalRecordCount).To(Equal(queryResult(get("/Items?Recursive=true")).TotalRecordCount)) + Expect(q.TotalRecordCount).To(BeNumerically(">", 0)) }) + + // A strict client (Manet) fails its whole sync on the first item missing any of these. + DescribeTable("sends the keys Jellyfin puts on every item", + func(itemType string, fields string, nonNull ...string) { + var body struct{ Items []map[string]json.RawMessage } + res := get("/Items?IncludeItemTypes=" + itemType + "&Fields=" + fields + "&Recursive=true") + Expect(json.Unmarshal(res.Body.Bytes(), &body)).To(Succeed()) + Expect(body.Items).ToNot(BeEmpty()) + for _, it := range body.Items { + Expect(it).To(HaveKey("ChannelId"), "ChannelId is null, but always present") + for _, k := range nonNull { + Expect(it).To(HaveKey(k)) + Expect(string(it[k])).ToNot(Equal("null"), k) + } + } + }, + Entry("songs", "Audio", "Genres,Tags", "ImageTags", "HasLyrics", "Genres", "GenreItems", "Tags"), + Entry("albums", "MusicAlbum", "Genres", "ImageTags", "Genres", "GenreItems"), + Entry("artists", "MusicArtist", "Genres", "ImageTags", "Genres", "GenreItems"), + ) + + It("sends MediaType Unknown on items without one, as Jellyfin always emits it", func() { + q := queryResult(get("/Items?IncludeItemTypes=MusicAlbum&Recursive=true")) + Expect(q.Items).ToNot(BeEmpty()) + for _, it := range q.Items { + Expect(it.MediaType).To(Equal("Unknown")) + } + Expect(queryResult(get("/Items?IncludeItemTypes=Audio&Recursive=true")).Items[0].MediaType).To(Equal("Audio")) + }) + }) Describe("ParentId browsing", func() { diff --git a/server/jellyfin/e2e/discovery_test.go b/server/jellyfin/e2e/discovery_test.go new file mode 100644 index 000000000..0126ef3c6 --- /dev/null +++ b/server/jellyfin/e2e/discovery_test.go @@ -0,0 +1,50 @@ +package e2e + +import ( + "context" + "encoding/json" + "net" + "time" + + "github.com/navidrome/navidrome/server/jellyfin" + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" +) + +var _ = Describe("Auto discovery", func() { + BeforeEach(func() { setupTestDB() }) + + It("advertises the same Id and Name as /System/Info/Public", func() { + server, err := net.ListenPacket("udp4", "127.0.0.1:0") + Expect(err).ToNot(HaveOccurred()) + client, err := net.ListenPacket("udp4", "127.0.0.1:0") + Expect(err).ToNot(HaveOccurred()) + DeferCleanup(client.Close) + + ctx, cancel := context.WithCancel(context.Background()) + done := make(chan struct{}) + go func() { + defer close(done) + jellyfin.NewDiscovery(ds).ServeOn(ctx, server) + }() + DeferCleanup(func() { + cancel() + Eventually(done).Should(BeClosed()) + }) + + _, err = client.WriteTo([]byte("who is JellyfinServer?"), server.LocalAddr()) + Expect(err).ToNot(HaveOccurred()) + Expect(client.SetReadDeadline(time.Now().Add(time.Second))).To(Succeed()) + buf := make([]byte, 1024) + n, _, err := client.ReadFrom(buf) + Expect(err).ToNot(HaveOccurred()) + var reply map[string]any + Expect(json.Unmarshal(buf[:n], &reply)).To(Succeed()) + + var pub map[string]any + parseInto(rawReq("GET", "/System/Info/Public", ""), &pub) + Expect(reply["Id"]).To(Equal(pub["Id"])) + Expect(reply["Name"]).To(Equal(pub["ServerName"])) + Expect(reply["Address"]).To(HaveSuffix("/jellyfin")) + }) +}) diff --git a/server/jellyfin/e2e/images_test.go b/server/jellyfin/e2e/images_test.go index 7fb85cba8..d1c3c132b 100644 --- a/server/jellyfin/e2e/images_test.go +++ b/server/jellyfin/e2e/images_test.go @@ -41,6 +41,10 @@ var _ = Describe("Item images", func() { Expect(u.IsAdmin).To(BeTrue()) }) + It("answers HEAD without authentication", func() { + Expect(rawReq("HEAD", "/Items/"+enc(albumID("Abbey Road"))+"/Images/Primary", "").Code).To(Equal(http.StatusOK)) + }) + It("serves images without authentication (public route)", func() { id := albumID("IV") w := rawReq("GET", "/Items/"+enc(id)+"/Images/Primary", "") diff --git a/server/jellyfin/e2e/lyrics_test.go b/server/jellyfin/e2e/lyrics_test.go index a49019bbf..d52a3b00f 100644 --- a/server/jellyfin/e2e/lyrics_test.go +++ b/server/jellyfin/e2e/lyrics_test.go @@ -58,14 +58,14 @@ var _ = Describe("Lyrics", func() { }) Describe("HasLyrics badge", func() { - It("is true for a track with embedded lyrics and omitted/false otherwise", func() { + It("is true for a track with embedded lyrics and false otherwise", func() { var stairway dto.BaseItemDto parseInto(get("/Items/"+enc(songID("Stairway To Heaven"))), &stairway) - Expect(stairway.HasLyrics).To(BeTrue()) + Expect(*stairway.HasLyrics).To(BeTrue()) var soWhat dto.BaseItemDto parseInto(get("/Items/"+enc(songID("So What"))), &soWhat) - Expect(soWhat.HasLyrics).To(BeFalse()) + Expect(*soWhat.HasLyrics).To(BeFalse()) }) }) }) diff --git a/server/jellyfin/e2e/playlists_test.go b/server/jellyfin/e2e/playlists_test.go index 50c2e17f2..174ba8660 100644 --- a/server/jellyfin/e2e/playlists_test.go +++ b/server/jellyfin/e2e/playlists_test.go @@ -19,6 +19,7 @@ var _ = Describe("Playlists", func() { playlistItems := func(plID string) dto.QueryResult { return queryResult(get("/Playlists/" + enc(plID) + "/Items")) } + order := func(plID string) []string { return names(playlistItems(plID).Items) } Describe("create", func() { It("creates an empty playlist", func() { @@ -102,6 +103,36 @@ var _ = Describe("Playlists", func() { Expect(playlistItems(plID).TotalRecordCount).To(BeZero()) }) + Describe("at a position", func() { + var plID string + addAt := func(position string, ids ...string) { + url := "/Playlists/" + enc(plID) + "/Items?position=" + position + for _, id := range ids { + url += "&ids=" + enc(id) + } + Expect(post(url, "").Code).To(Equal(http.StatusNoContent)) + } + BeforeEach(func() { + plID = createPlaylist("Insert", []string{enc(songID("Come Together")), enc(songID("Something"))}) + }) + + DescribeTable("inserts at Jellyfin's zero-based position", + func(position string, want []string) { + addAt(position, songID("So What"), songID("Help!")) + Expect(order(plID)).To(Equal(want)) + }, + Entry("in the middle", "1", []string{"Come Together", "So What", "Help!", "Something"}), + Entry("first, for zero or less", "-3", []string{"So What", "Help!", "Come Together", "Something"}), + Entry("last, past the end", "9", []string{"Come Together", "Something", "So What", "Help!"}), + ) + + It("keeps an expanded album's track order", func() { + albumOrder := order(createPlaylist("Album", []string{enc(albumID("Abbey Road"))})) + addAt("1", albumID("Abbey Road")) + Expect(order(plID)).To(Equal(append(append([]string{"Come Together"}, albumOrder...), "Something"))) + }) + }) + It("removes an entry by its PlaylistItemId", func() { plID := createPlaylist("Remove", []string{enc(songID("Come Together")), enc(songID("Something"))}) entryID := playlistItems(plID).Items[0].PlaylistItemId @@ -118,6 +149,58 @@ var _ = Describe("Playlists", func() { }) }) + Describe("move", func() { + move := func(u model.User, plID, entryID string, newIndex string) int { + return jReq(u, "POST", "/Playlists/"+enc(plID)+"/Items/"+entryID+"/Move/"+newIndex, "").Code + } + var plID string + var entries []dto.BaseItemDto + BeforeEach(func() { + plID = createPlaylist("Move", []string{enc(songID("Come Together")), enc(songID("Something")), enc(songID("So What"))}) + entries = playlistItems(plID).Items + }) + + DescribeTable("moves an entry to Jellyfin's zero-based index", + func(entry int, newIndex string, want []string) { + Expect(move(adminUser, plID, entries[entry].PlaylistItemId, newIndex)).To(Equal(http.StatusNoContent)) + Expect(order(plID)).To(Equal(want)) + var positions []string + for _, it := range playlistItems(plID).Items { + positions = append(positions, it.PlaylistItemId) + } + Expect(positions).To(Equal([]string{dto.EncodePlaylistEntryID("1"), dto.EncodePlaylistEntryID("2"), dto.EncodePlaylistEntryID("3")})) + }, + Entry("towards the end", 0, "2", []string{"Something", "So What", "Come Together"}), + Entry("towards the start", 2, "0", []string{"So What", "Come Together", "Something"}), + Entry("to the end, past the last index", 0, "99", []string{"Something", "So What", "Come Together"}), + Entry("to the end, for the largest int", 0, "9223372036854775807", []string{"Something", "So What", "Come Together"}), + ) + + It("ignores an entry that is not in the playlist", func() { + Expect(move(adminUser, plID, dto.EncodePlaylistEntryID("42"), "0")).To(Equal(http.StatusNoContent)) + Expect(order(plID)).To(Equal([]string{"Come Together", "Something", "So What"})) + }) + + It("404s on a malformed entry id", func() { + Expect(move(adminUser, plID, enc(songID("So What")), "0")).To(Equal(http.StatusNotFound)) + }) + + It("hides another user's private playlist", func() { + Expect(move(regularUser, plID, entries[0].PlaylistItemId, "2")).To(Equal(http.StatusNotFound)) + Expect(order(plID)).To(Equal([]string{"Come Together", "Something", "So What"})) + }) + + It("forbids a non-owner on a public playlist", func() { + Expect(post("/Playlists/"+enc(plID), `{"Name":"Move","IsPublic":true}`).Code).To(Equal(http.StatusNoContent)) + Expect(move(regularUser, plID, entries[0].PlaylistItemId, "2")).To(Equal(http.StatusForbidden)) + Expect(order(plID)).To(Equal([]string{"Come Together", "Something", "So What"})) + }) + + It("rejects a negative index", func() { + Expect(move(adminUser, plID, entries[0].PlaylistItemId, "-1")).To(Equal(http.StatusBadRequest)) + }) + }) + Describe("users", func() { It("reports the current user as an editor", func() { plID := createPlaylist("Perms", nil) diff --git a/server/jellyfin/e2e/sessions_test.go b/server/jellyfin/e2e/sessions_test.go index 2057b43c6..c5c42cb2b 100644 --- a/server/jellyfin/e2e/sessions_test.go +++ b/server/jellyfin/e2e/sessions_test.go @@ -37,6 +37,10 @@ var _ = Describe("Sessions", func() { Expect(mf.PlayCount).To(BeNumerically(">=", 1)) }) + It("acknowledges a playback session ping", func() { + Expect(post("/Sessions/Playing/Ping?playSessionId=abc", "").Code).To(Equal(http.StatusNoContent)) + }) + It("does not count a brief play stopped before the threshold", func() { // Regression: Finamp sends a Stopped report on every track switch, so an immediate skip // (1 second in) must not mark the track played. Seeded tracks are >= 120s, so the 50% diff --git a/server/jellyfin/e2e/similar_test.go b/server/jellyfin/e2e/similar_test.go index 5f1a88a21..06054f8bf 100644 --- a/server/jellyfin/e2e/similar_test.go +++ b/server/jellyfin/e2e/similar_test.go @@ -150,4 +150,33 @@ var _ = Describe("Similar", func() { Expect(w.Code).To(Equal(http.StatusNotFound)) }) }) + + Describe("type-specific InstantMix routes", func() { + BeforeEach(func() { + providerFake.similarSongs = model.MediaFiles{{ID: testID("x1"), Title: "Mix Song", LibraryID: 1}} + }) + + It("leads a song mix with the seed on /Songs/{id}/InstantMix", func() { + q := queryResult(get("/Songs/" + enc(songID("So What")) + "/InstantMix")) + Expect(names(q.Items)).To(Equal([]string{"So What", "Mix Song"})) + }) + + DescribeTable("returns the provider's mix", + func(path func() string) { + q := queryResult(get(path())) + Expect(names(q.Items)).To(Equal([]string{"Mix Song"})) + }, + Entry("Albums/{id}", func() string { return "/Albums/" + enc(albumID("Abbey Road")) + "/InstantMix" }), + Entry("Artists/{id}", func() string { return "/Artists/" + enc(artistID("Miles Davis")) + "/InstantMix" }), + Entry("Playlists/{id}", func() string { + return "/Playlists/" + enc(createPlaylist("Seed", []string{enc(songID("So What"))})) + "/InstantMix" + }), + Entry("Artists/InstantMix?id=", func() string { return "/Artists/InstantMix?id=" + enc(artistID("Miles Davis")) }), + Entry("MusicGenres/InstantMix?id=", func() string { return "/MusicGenres/InstantMix?id=" + enc(genreID("Jazz")) }), + ) + + It("404s a malformed id query param", func() { + Expect(get("/Artists/InstantMix?id=not-a-valid-id").Code).To(Equal(http.StatusNotFound)) + }) + }) }) diff --git a/server/jellyfin/e2e/streaming_test.go b/server/jellyfin/e2e/streaming_test.go index 67208aed3..9b7380185 100644 --- a/server/jellyfin/e2e/streaming_test.go +++ b/server/jellyfin/e2e/streaming_test.go @@ -1,6 +1,7 @@ package e2e import ( + "fmt" "net/http" "strings" @@ -45,6 +46,19 @@ var _ = Describe("Streaming", func() { It("returns 404 for an unknown track", func() { Expect(get("/Audio/" + enc(testID("nope")) + "/stream").Code).To(Equal(http.StatusNotFound)) }) + + DescribeTable("answers HEAD (Fintunes probes the type before playing or downloading)", + func(path string) { + w := jReq(adminUser, "HEAD", fmt.Sprintf(path, enc(songID("Help!"))), "") + Expect(w.Code).To(Equal(http.StatusOK)) + Expect(w.Header().Get("Content-Type")).ToNot(BeEmpty()) + }, + Entry("stream", "/Audio/%s/stream"), + Entry("stream.{container}", "/Audio/%s/stream.mp3"), + Entry("universal", "/Audio/%s/universal"), + Entry("File", "/Items/%s/File"), + Entry("Download", "/Items/%s/Download"), + ) }) Describe("GET /Audio/{id}/main.m3u8 (Finamp transcoding mode)", func() { diff --git a/server/jellyfin/images.go b/server/jellyfin/images.go index 9f1b9ce1a..0c9e0380c 100644 --- a/server/jellyfin/images.go +++ b/server/jellyfin/images.go @@ -23,14 +23,17 @@ import ( _ "golang.org/x/image/webp" ) -// imageSize picks the tighter of Jellyfin's two bounds, because Navidrome resizes on a single -// dimension: reading only MaxWidth serves the full-size original to a client that sent MaxHeight. -func imageSize(maxWidth, maxHeight int) int { - w, h := max(maxWidth, 0), max(maxHeight, 0) - if w == 0 || h == 0 { - return max(w, h) +// imageSize reduces Jellyfin's size params to Navidrome's single bound. Width/Height/Max* are +// bounds, so the tightest wins; Fill* must cover its box, so its larger side is the bound. +func imageSize(p *req.Values) int { + fill := max(p.IntOr("fillwidth", 0), p.IntOr("fillheight", 0)) + size := 0 + for _, v := range []int{p.IntOr("width", 0), p.IntOr("height", 0), p.IntOr("maxwidth", 0), p.IntOr("maxheight", 0), fill} { + if v > 0 && (size == 0 || v < size) { + size = v + } } - return min(w, h) + return size } func (api *Router) getItemImage(w http.ResponseWriter, r *http.Request) { @@ -41,8 +44,7 @@ func (api *Router) getItemImage(w http.ResponseWriter, r *http.Request) { if !ok { return } - p := req.Params(r) - size := imageSize(p.IntOr("maxwidth", 0), p.IntOr("maxheight", 0)) + size := imageSize(req.Params(r)) artID := api.resolveArtworkID(ctx, itemId) img, err := api.artwork.GetOrPlaceholder(ctx, artID, size, false) diff --git a/server/jellyfin/images_test.go b/server/jellyfin/images_test.go index 2f32e6ec9..b7b262895 100644 --- a/server/jellyfin/images_test.go +++ b/server/jellyfin/images_test.go @@ -65,7 +65,7 @@ func newImageRequest(itemId string) (*httptest.ResponseRecorder, *http.Request) var _ = Describe("Images", func() { // Real Jellyfin fits the image inside either bound, so a client that sends only MaxHeight must // still get a resized image rather than the full-size original. - DescribeTable("derives the requested size from MaxWidth or MaxHeight", + DescribeTable("derives the requested size from the Jellyfin size params", func(query string, wantSize int) { ds := &tests.MockDataStore{} ds.Album(context.Background()).(*tests.MockAlbumRepo).SetData(model.Albums{{ID: testID("a1"), Name: "One"}}) @@ -84,6 +84,15 @@ var _ = Describe("Images", func() { Entry("both, smaller bound wins", "maxwidth=200&maxheight=300", 200), Entry("both, smaller bound wins regardless of order", "maxwidth=300&maxheight=200", 200), Entry("neither", "", 0), + Entry("Width only", "width=300", 300), + Entry("Height only", "height=300", 300), + Entry("Width capped by MaxWidth", "width=500&maxwidth=300", 300), + Entry("FillWidth only", "fillwidth=300", 300), + Entry("FillHeight only", "fillheight=300", 300), + Entry("both fill sides, larger one covers the box", "fillwidth=200&fillheight=300", 300), + Entry("fill smaller than MaxWidth wins", "maxwidth=500&fillwidth=300&fillheight=300", 300), + Entry("MaxWidth smaller than fill wins", "maxwidth=200&fillwidth=300&fillheight=300", 200), + Entry("non-positive values are ignored", "fillwidth=0&maxwidth=-5&height=250", 250), ) It("streams album artwork", func() { diff --git a/server/jellyfin/items.go b/server/jellyfin/items.go index d16c5e57e..124d17920 100644 --- a/server/jellyfin/items.go +++ b/server/jellyfin/items.go @@ -219,11 +219,40 @@ func (api *Router) writeItemsArray(w http.ResponseWriter, r *http.Request, res i api.streamResult(w, r, res, streamItemsArray) } -// streamResult stamps every item's ServerId (constant per request, so it's set here rather than in -// each mapper). The cursor opens before the first byte, so a failed open is still a clean 500. +// stampItem fills in what real Jellyfin puts on every item it returns, so a client that requires a +// key never meets an item without it: ServerId, MediaType, ImageTags, and the Fields-gated lists. +func stampItem(it dto.BaseItemDto, serverID string, fields dto.Fields) dto.BaseItemDto { + it.ServerId = serverID + if it.MediaType == "" { + it.MediaType = "Unknown" + } + if it.ImageTags == nil { + it.ImageTags = map[string]string{} + } + if fields.Has("Genres") { + if it.Genres == nil { + it.Genres = []string{} + } + if it.GenreItems == nil { + it.GenreItems = []dto.NameGuidPair{} + } + } + if fields.Has("Tags") && it.Tags == nil { + it.Tags = []string{} + } + return it +} + +// requestFields parses the Fields param, which gates what stampItem and the mappers attach. +func requestFields(r *http.Request) dto.Fields { + return dto.ParseFields(req.Params(r).Strings("fields")...) +} + +// streamResult stamps every item (see stampItem). The cursor opens before the first byte, so a +// failed open is still a clean 500. func (api *Router) streamResult(w http.ResponseWriter, r *http.Request, res itemsResult, write func(io.Writer, iter.Seq2[dto.BaseItemDto, error]) error) { - sid := api.serverID(r.Context()) + sid, fields := api.serverID(r.Context()), requestFields(r) seq, err := res.seq() if err != nil { api.internalError(w, r, err) @@ -235,8 +264,7 @@ func (api *Router) streamResult(w http.ResponseWriter, r *http.Request, res item yield(dto.BaseItemDto{}, err) return } - it.ServerId = sid - if !yield(it, nil) { + if !yield(stampItem(it, sid, fields), nil) { return } } @@ -321,7 +349,7 @@ func (api *Router) parseItemsQuery(ctx context.Context, r *http.Request) (itemsQ } q := listParams(p) q.ids = ids - q.rawTypes = p.StringOr("includeitemtypes", "") + q.rawTypes = knownItemKinds(p.StringOr("includeitemtypes", "")) q.parentId = parentId q.genreIds = genreIds q.albumIds = albumIds @@ -380,7 +408,7 @@ func (api *Router) queryItems(ctx context.Context, r *http.Request) (itemsResult case len(q.ids) > 0: return materialized(api.itemsByIDs(ctx, q.ids, q.fields)), nil // A ManualPlaylistsFolder query asks for the synthetic "playlists library" container, not real items. - case strings.Contains(q.rawTypes, "ManualPlaylistsFolder"): + case strings.Contains(strings.ToLower(q.rawTypes), "manualplaylistsfolder"): return materialized(result([]dto.BaseItemDto{playlistsFolder()}, 1, 0)), nil } if repo, ok := api.playlistTracksRepo(ctx, q); ok { @@ -559,23 +587,49 @@ func parseYears(r *http.Request) []int { return years } -// parseTypes returns the recognized entries in IncludeItemTypes in order, defaulting to -// {"MusicAlbum"} when none are recognized (so ParentId= browses that artist's albums). +// supportedTypes maps lowercased IncludeItemTypes names (Jellyfin binds them case-insensitively) +// to the item types Navidrome serves. +var supportedTypes = map[string]string{ + "audio": "Audio", "musicartist": "MusicArtist", "musicalbum": "MusicAlbum", "musicgenre": "MusicGenre", "playlist": "Playlist", +} + +// jellyfinItemKinds lists Jellyfin's BaseItemKind names, lowercased. +var jellyfinItemKinds = map[string]bool{ + "aggregatefolder": true, "audio": true, "audiobook": true, "basepluginfolder": true, "book": true, + "boxset": true, "channel": true, "channelfolderitem": true, "collectionfolder": true, "episode": true, + "folder": true, "genre": true, "manualplaylistsfolder": true, "movie": true, "livetvchannel": true, + "livetvprogram": true, "musicalbum": true, "musicartist": true, "musicgenre": true, "musicvideo": true, + "person": true, "photo": true, "photoalbum": true, "playlist": true, "playlistsfolder": true, + "program": true, "recording": true, "season": true, "series": true, "studio": true, "trailer": true, + "tvchannel": true, "tvprogram": true, "userrootfolder": true, "userview": true, "video": true, "year": true, +} + +// knownItemKinds drops IncludeItemTypes entries that aren't BaseItemKind names, as Jellyfin's binder +// does, so an all-unknown list (JellyBox sends "music") behaves like an absent one. +func knownItemKinds(types string) string { + var known []string + for t := range strings.SplitSeq(types, ",") { + if t = strings.TrimSpace(t); jellyfinItemKinds[strings.ToLower(t)] { + known = append(known, t) + } + } + return strings.Join(known, ",") +} + +// parseTypes returns the supported entries in IncludeItemTypes in order. Only an absent param +// defaults to albums, so ParentId= still browses that artist's albums. func parseTypes(types string) []string { + if strings.TrimSpace(types) == "" { + return []string{"MusicAlbum"} + } var recognized []string for t := range strings.SplitSeq(types, ",") { - t = strings.TrimSpace(t) - switch t { - case "Audio", "MusicArtist", "MusicAlbum", "MusicGenre", "Playlist": - recognized = append(recognized, t) + if name, ok := supportedTypes[strings.ToLower(strings.TrimSpace(t))]; ok { + recognized = append(recognized, name) } } // Dedupe: a repeated type would duplicate items in the merge and spawn a redundant query. - recognized = slice.Unique(recognized) - if len(recognized) == 0 { - return []string{"MusicAlbum"} - } - return recognized + return slice.Unique(recognized) } // paginate applies StartIndex/Limit to an in-memory item list, for the multi-type merge path only @@ -828,14 +882,8 @@ func (api *Router) resolveItemByID(ctx context.Context, id string, fields dto.Fi // Finamp resolves a /UserViews entry (Id=library id) by fetching it as a plain item; without this // the home screen and library tabs 404. if libID, err := strconv.Atoi(id); err == nil && u.HasLibraryAccess(libID) { - for _, lib := range u.Libraries { - if lib.ID == libID { - return libraryView(lib), true - } - } - // Admin bypass: Libraries is empty but all access is granted, so fetch the real library. if lib, err := api.ds.Library(ctx).Get(libID); err == nil { - return libraryView(*lib), true + return dto.LibraryToBaseItem(*lib), true } } if al, err := api.ds.Album(ctx).Get(id); err == nil { diff --git a/server/jellyfin/items_test.go b/server/jellyfin/items_test.go index 17180c36a..a8dfb7bb2 100644 --- a/server/jellyfin/items_test.go +++ b/server/jellyfin/items_test.go @@ -74,6 +74,18 @@ var _ = Describe("Items", func() { Expect(res.Items[0].Id).To(Equal(dto.EncodeID(testID("s1")))) }) + It("ignores IncludeItemTypes names that aren't Jellyfin item kinds", func() { + ds.Album(context.Background()).(*tests.MockAlbumRepo).SetData(model.Albums{{ID: testID("a1"), Name: "One"}}) + ds.MediaFile(context.Background()).(*tests.MockMediaFileRepo).SetData(model.MediaFiles{{ID: testID("s1"), Title: "Song", AlbumID: testID("a1")}}) + w := httptest.NewRecorder() + r := httptest.NewRequest("GET", "/Items?ParentId="+dto.EncodeID(testID("a1"))+"&IncludeItemTypes=music", nil).WithContext(ctxUser()) + invoke(api.getItems, w, r) + var res dto.QueryResult + Expect(json.Unmarshal(w.Body.Bytes(), &res)).To(Succeed()) + Expect(res.Items).To(HaveLen(1)) + Expect(res.Items[0].Id).To(Equal(dto.EncodeID(testID("s1")))) + }) + It("lists a playlist's tracks when ParentId is a playlist, whatever the type", func() { fp.getPls = &model.Playlist{ID: testID("pl1"), Tracks: model.PlaylistTracks{ {ID: "1", MediaFileID: testID("s1"), PlaylistID: testID("pl1"), MediaFile: model.MediaFile{ID: testID("s1")}}, @@ -981,6 +993,7 @@ var _ = Describe("Items", func() { It("resolves a library-view id (from /UserViews) as a CollectionFolder item", func() { w := httptest.NewRecorder() libs := model.Libraries{{ID: 1, Name: "Music Library"}} + ds.Library(context.Background()).(*tests.MockLibraryRepo).SetData(libs) r := httptest.NewRequest("GET", "/Items/"+dto.EncodeLibraryID(1), nil).WithContext(ctxUserWithLibraries(libs)) r = withChiURLParam(r, "itemId", dto.EncodeLibraryID(1)) invoke(api.getItem, w, r) @@ -1124,6 +1137,25 @@ var _ = Describe("Items", func() { It("dedupes repeated types, preserving first-seen order", func() { Expect(parseTypes("Audio,MusicAlbum,Audio")).To(Equal([]string{"Audio", "MusicAlbum"})) }) + + It("matches type names case-insensitively, like Jellyfin's enum binding", func() { + Expect(parseTypes("musicalbum, AUDIO")).To(Equal([]string{"MusicAlbum", "Audio"})) + }) + + It("returns no types for real Jellyfin kinds Navidrome has none of", func() { + Expect(parseTypes("Boxset")).To(BeEmpty()) + Expect(parseTypes("BoxSet,Movie")).To(BeEmpty()) + }) + + It("drops names that aren't Jellyfin item kinds, case-insensitively", func() { + Expect(knownItemKinds("music")).To(BeEmpty()) + Expect(knownItemKinds("music, audio,MUSICVIDEO")).To(Equal("audio,MUSICVIDEO")) + }) + + It("defaults to albums only when IncludeItemTypes is absent", func() { + Expect(parseTypes("")).To(Equal([]string{"MusicAlbum"})) + Expect(parseTypes("Nonsense")).To(BeEmpty()) + }) }) Describe("decodeFilterParam", func() { diff --git a/server/jellyfin/library.go b/server/jellyfin/library.go index b21603843..9f1f7ee73 100644 --- a/server/jellyfin/library.go +++ b/server/jellyfin/library.go @@ -7,7 +7,6 @@ import ( "github.com/Masterminds/squirrel" "github.com/go-chi/chi/v5" - "github.com/navidrome/navidrome/model" "github.com/navidrome/navidrome/model/request" "github.com/navidrome/navidrome/server/jellyfin/dto" "github.com/navidrome/navidrome/utils/req" @@ -74,16 +73,3 @@ func libraryScopeFilter(scope []int) squirrel.Sqlizer { } return squirrel.Eq{"library_tag.library_id": scope} } - -// libraryView builds the CollectionFolder BaseItemDto representing a library as a top-level node. -// Shared by getUserViews and getItem, since Finamp fetches a UserView's id as a plain item. -func libraryView(lib model.Library) dto.BaseItemDto { - return dto.BaseItemDto{ - Id: dto.EncodeLibraryID(lib.ID), - Name: lib.Name, - Type: "CollectionFolder", - CollectionType: "music", - IsFolder: true, - BackdropImageTags: []string{}, - } -} diff --git a/server/jellyfin/playlists.go b/server/jellyfin/playlists.go index ac790a821..1052a398f 100644 --- a/server/jellyfin/playlists.go +++ b/server/jellyfin/playlists.go @@ -4,7 +4,9 @@ import ( "context" "encoding/json" "errors" + "math" "net/http" + "strconv" "strings" "github.com/go-chi/chi/v5" @@ -261,8 +263,8 @@ func (api *Router) songIDs(ctx context.Context, opts model.QueryOptions) []strin return slice.Map(mfs, func(mf model.MediaFile) string { return mf.ID }) } -// addToPlaylist appends items by id, expanding containers into tracks (see expandContainerIDs). -// AddTracks enforces ownership; a locked playlist maps to 403, any other error to 404. +// addToPlaylist adds items by id (containers expand to tracks), inserting at the zero-based position +// when given. Core enforces ownership: a locked playlist maps to 403, any other error to 404. func (api *Router) addToPlaylist(w http.ResponseWriter, r *http.Request) { ctx := r.Context() id, ok := itemIDParam(w, r, "playlistId") @@ -275,7 +277,13 @@ func (api *Router) addToPlaylist(w http.ResponseWriter, r *http.Request) { return } ids := api.expandContainerIDs(ctx, decoded) - if _, err := api.playlists.AddTracks(ctx, id, ids); err != nil { + var err error + if position, perr := req.Params(r).Int64("position"); perr == nil { + _, err = api.playlists.InsertTracks(ctx, id, ids, insertPosition(position)) + } else { + _, err = api.playlists.AddTracks(ctx, id, ids) + } + if err != nil { if errors.Is(err, model.ErrPlaylistNotEditable) { http.Error(w, "Forbidden", http.StatusForbidden) return @@ -286,6 +294,12 @@ func (api *Router) addToPlaylist(w http.ResponseWriter, r *http.Request) { w.WriteHeader(http.StatusNoContent) } +// insertPosition maps Jellyfin's zero-based position to a 1-based one, clamped in int64 first so +// it can't wrap on 32-bit builds. +func insertPosition(position int64) int { + return int(min(max(position, 0), math.MaxInt32-1) + 1) +} + // removeFromPlaylist removes entries by entryIds — playlist-entry ids (PlaylistItemId), not media // file ids, since RemoveTracks deletes playlist_tracks rows by that id. RemoveTracks enforces // ownership; a locked playlist maps to 403, any other error to 404. @@ -316,6 +330,38 @@ func (api *Router) removeFromPlaylist(w http.ResponseWriter, r *http.Request) { w.WriteHeader(http.StatusNoContent) } +// movePlaylistItem moves an entry to Jellyfin's zero-based newIndex. Reorder clamps past-the-end +// indexes and rejects unknown entries, which Jellyfin treats as a no-op. +func (api *Router) movePlaylistItem(w http.ResponseWriter, r *http.Request) { + ctx := r.Context() + id, ok := itemIDParam(w, r, "playlistId") + if !ok { + return + } + entry, ok := dto.DecodePlaylistEntryID(chi.URLParam(r, "entryId")) + if !ok { + http.Error(w, "Not Found", http.StatusNotFound) + return + } + newIndex, err := strconv.Atoi(chi.URLParam(r, "newIndex")) + if err != nil || newIndex < 0 { + http.Error(w, "Bad Request", http.StatusBadRequest) + return + } + // Resolve first, so a missing or hidden playlist is still a 404 below the unknown-entry no-op. + if _, err := api.playlists.Get(ctx, id); err != nil { + api.playlistError(w, r, err) + return + } + pos, _ := strconv.Atoi(entry) + err = api.playlists.ReorderTrack(ctx, id, pos, min(newIndex, math.MaxInt32-1)+1) + if err != nil && !errors.Is(err, model.ErrNotFound) { + api.playlistError(w, r, err) + return + } + w.WriteHeader(http.StatusNoContent) +} + // Clients probe these before offering edits. Navidrome has no per-playlist ACL, so CanEdit carries // only editability; ownership is enforced on write, and a lookup error 404s to prevent probing. func (api *Router) getPlaylistUsers(w http.ResponseWriter, r *http.Request) { diff --git a/server/jellyfin/playlists_test.go b/server/jellyfin/playlists_test.go index 270f5fe08..ae0a5da3c 100644 --- a/server/jellyfin/playlists_test.go +++ b/server/jellyfin/playlists_test.go @@ -5,6 +5,7 @@ import ( "encoding/json" "errors" "io" + "math" "net/http" "net/http/httptest" "strings" @@ -133,6 +134,17 @@ func (f *fakePlaylists) RemoveImage(_ context.Context, playlistID string) error return f.removeImageErr } +var _ = DescribeTable("insertPosition", + func(position int64, want int) { + Expect(insertPosition(position)).To(Equal(want)) + }, + Entry("zero-based index becomes a 1-based position", int64(2), 3), + Entry("negative prepends", int64(-5), 1), + Entry("beyond int32 stays past the end instead of wrapping", int64(1)<<32+1, math.MaxInt32), + Entry("largest int64 stays past the end", int64(math.MaxInt64), math.MaxInt32), + Entry("smallest int64 prepends", int64(math.MinInt64), 1), +) + var _ = Describe("Playlists", func() { var api *Router var fp *fakePlaylists diff --git a/server/jellyfin/sessions.go b/server/jellyfin/sessions.go index a145e7359..e60b1bf5b 100644 --- a/server/jellyfin/sessions.go +++ b/server/jellyfin/sessions.go @@ -113,8 +113,8 @@ func (api *Router) reportPlaybackStopped(w http.ResponseWriter, r *http.Request) w.WriteHeader(http.StatusNoContent) } -// postCapabilities acknowledges Jellyfin session-capability negotiation. -// Navidrome doesn't track per-session client capabilities, so this is a no-op. -func (api *Router) postCapabilities(w http.ResponseWriter, _ *http.Request) { +// acknowledge answers requests Navidrome keeps no state for: session capabilities (not tracked per +// session) and playback pings (transcodes live only as long as their stream request). +func (api *Router) acknowledge(w http.ResponseWriter, _ *http.Request) { w.WriteHeader(http.StatusNoContent) } diff --git a/server/jellyfin/sessions_test.go b/server/jellyfin/sessions_test.go index 05151c39a..f2f04497c 100644 --- a/server/jellyfin/sessions_test.go +++ b/server/jellyfin/sessions_test.go @@ -152,12 +152,12 @@ var _ = Describe("Sessions", func() { }) }) - Describe("postCapabilities", func() { + Describe("acknowledge", func() { It("returns 204 No Content and does not touch the scrobbler", func() { w := httptest.NewRecorder() r := authed(httptest.NewRequest("POST", "/Sessions/Capabilities", strings.NewReader(`{"SupportsMediaControl":true}`))) - api.postCapabilities(w, r) + api.acknowledge(w, r) Expect(w.Code).To(Equal(http.StatusNoContent)) Expect(pt.reported).To(BeEmpty()) diff --git a/server/jellyfin/similar.go b/server/jellyfin/similar.go index 82d12f52b..1bd887f43 100644 --- a/server/jellyfin/similar.go +++ b/server/jellyfin/similar.go @@ -111,11 +111,26 @@ func (api *Router) getSimilarAlbums(w http.ResponseWriter, r *http.Request) { // a track seed leads its own mix; provider errors and unknown seeds degrade to seed-only/empty // results, never a 404 the client would surface as an error. func (api *Router) getInstantMix(w http.ResponseWriter, r *http.Request) { - ctx := r.Context() id, ok := itemIDParam(w, r, "itemId") if !ok { return } + api.instantMix(w, r, id) +} + +// getInstantMixByQuery serves the legacy Artists/InstantMix and MusicGenres/InstantMix forms, +// which take the seed as ?id= instead of a path segment. +func (api *Router) getInstantMixByQuery(w http.ResponseWriter, r *http.Request) { + id, ok := dto.DecodeID(req.Params(r).StringOr("id", "")) + if !ok { + http.Error(w, "Not Found", http.StatusNotFound) + return + } + api.instantMix(w, r, id) +} + +func (api *Router) instantMix(w http.ResponseWriter, r *http.Request, id string) { + ctx := r.Context() limit := clampLimit(req.Params(r).IntOr("limit", 0), defaultSimilarLimit, maxInstantMixLimit) // Genre ids don't resolve via GetEntityByID, so a not-found entity is fine: it is just "not a diff --git a/server/jellyfin/stream.go b/server/jellyfin/stream.go index 87660056c..b9809bd29 100644 --- a/server/jellyfin/stream.go +++ b/server/jellyfin/stream.go @@ -1,8 +1,10 @@ package jellyfin import ( + "cmp" "fmt" "math" + "mime" "net/http" "net/url" "slices" @@ -10,6 +12,7 @@ import ( "strings" "github.com/go-chi/chi/v5" + "github.com/navidrome/navidrome/core/stream" "github.com/navidrome/navidrome/log" "github.com/navidrome/navidrome/model" "github.com/navidrome/navidrome/model/request" @@ -65,16 +68,14 @@ func (api *Router) getPlaybackInfo(w http.ResponseWriter, r *http.Request) { api.ok(w, r, dto.PlaybackInfoResponse{MediaSources: []dto.MediaSourceInfo{src}, PlaySessionId: dto.EncodeID(mf.ID)}) } -// streamAudio serves /Audio/{itemId}/stream[.container] and /Audio/{itemId}/universal, -// reusing the same transcode-decision + streaming pipeline as the Subsonic /stream endpoint. +// streamAudio serves /Audio/{itemId}/stream[.container], reusing the same transcode-decision + +// streaming pipeline as the Subsonic /stream endpoint. func (api *Router) streamAudio(w http.ResponseWriter, r *http.Request) { mf, ok := api.mediaFileForRequest(w, r) if !ok { return } - ctx := r.Context() p := req.Params(r) - format := p.StringOr("container", "") if format == "" { // The /stream.{container} route form carries the format as a path segment, not a query param. @@ -84,17 +85,69 @@ func (api *Router) streamAudio(w http.ResponseWriter, r *http.Request) { // Jellyfin's audioCodec param names the target codec when no container is given. format = p.StringOr("audiocodec", "") } + api.serveAudio(w, r, mf, format) +} + +// streamUniversal serves /Audio/{itemId}/universal, where Container lists the "container|codec" +// entries the client direct-plays, and TranscodingContainer/AudioCodec name the fallback target. +func (api *Router) streamUniversal(w http.ResponseWriter, r *http.Request) { + mf, ok := api.mediaFileForRequest(w, r) + if !ok { + return + } + p := req.Params(r) + var streamReq stream.Request if p.BoolOr("static", false) { + streamReq = api.transcodeDecider.ResolveRequest(r.Context(), mf, "raw", 0, 0) + } else { + streamReq = api.transcodeDecider.ResolveClientRequest(r.Context(), mf, universalClientInfo(p), 0) + } + api.serveStream(w, r, mf, streamReq) +} + +func universalClientInfo(p *req.Values) *stream.ClientInfo { + ci := &stream.ClientInfo{Name: "jellyfin-universal", MaxAudioBitrate: bitRateParam(p)} + for entry := range strings.SplitSeq(p.StringOr("container", ""), ",") { + container, codec, _ := strings.Cut(strings.TrimSpace(entry), "|") + if container == "" { + continue + } + profile := stream.DirectPlayProfile{Containers: []string{container}, Protocols: []string{stream.ProtocolHTTP}} + if codec != "" { + profile.AudioCodecs = []string{codec} + } + ci.DirectPlayProfiles = append(ci.DirectPlayProfiles, profile) + } + codec := p.StringOr("audiocodec", "") + if container := cmp.Or(p.StringOr("transcodingcontainer", ""), codec); container != "" { + ci.TranscodingProfiles = []stream.Profile{{Container: container, AudioCodec: cmp.Or(codec, container), Protocol: stream.ProtocolHTTP}} + ci.MaxTranscodingAudioBitrate = ci.MaxAudioBitrate + } + return ci +} + +// bitRateParam reads Jellyfin's bits/sec bitrate params as the kbps the stream package expects. +func bitRateParam(p *req.Values) int { + return cmp.Or(p.IntOr("audiobitrate", 0), p.IntOr("maxstreamingbitrate", 0)) / 1000 +} + +func (api *Router) serveAudio(w http.ResponseWriter, r *http.Request, mf *model.MediaFile, format string) { + if req.Params(r).BoolOr("static", false) { format = "raw" } + api.serveStream(w, r, mf, api.transcodeDecider.ResolveRequest(r.Context(), mf, format, bitRateParam(req.Params(r)), 0)) +} - // Bitrate params are bits/sec by Jellyfin convention; ResolveRequest expects kbps. - bitRate := p.IntOr("audiobitrate", 0) / 1000 - if bitRate == 0 { - bitRate = p.IntOr("maxstreamingbitrate", 0) / 1000 +func (api *Router) serveStream(w http.ResponseWriter, r *http.Request, mf *model.MediaFile, streamReq stream.Request) { + ctx := r.Context() + // A probe must not start a transcode. Omitting Content-Length is what tells a client (Fintunes) + // this is not direct play; direct play falls through to Serve, which answers HEAD itself. + if r.Method == http.MethodHead && streamReq.Format != "" && streamReq.Format != "raw" { + w.Header().Set("Content-Type", mime.TypeByExtension("."+streamReq.Format)) + w.Header().Set("Accept-Ranges", "none") + w.WriteHeader(http.StatusOK) + return } - - streamReq := api.transcodeDecider.ResolveRequest(ctx, mf, format, bitRate, 0) s, err := api.streamer.NewStream(ctx, mf, streamReq) if err != nil { api.internalError(w, r, err) @@ -160,15 +213,5 @@ func (api *Router) streamFile(w http.ResponseWriter, r *http.Request) { if !ok { return } - ctx := r.Context() - streamReq := api.transcodeDecider.ResolveRequest(ctx, mf, "raw", 0, 0) - s, err := api.streamer.NewStream(ctx, mf, streamReq) - if err != nil { - api.internalError(w, r, err) - return - } - defer s.Close() - if _, err := s.Serve(ctx, w, r); err != nil { - log.Error(ctx, "Jellyfin API: error streaming", "id", mf.ID, err) - } + api.serveStream(w, r, mf, api.transcodeDecider.ResolveRequest(r.Context(), mf, "raw", 0, 0)) } diff --git a/server/jellyfin/stream_test.go b/server/jellyfin/stream_test.go index d6220d33f..1a6d1badc 100644 --- a/server/jellyfin/stream_test.go +++ b/server/jellyfin/stream_test.go @@ -244,6 +244,75 @@ var _ = Describe("Stream", func() { }) }) + Describe("HEAD requests", func() { + head := func(query string) *httptest.ResponseRecorder { + ds.MediaFile(context.Background()).(*tests.MockMediaFileRepo).SetData(model.MediaFiles{ + {ID: testID("s1"), Title: "Song", Suffix: "flac", LibraryID: 1}, + }) + streamer.content = "audio-bytes" + w := httptest.NewRecorder() + r := httptest.NewRequest("HEAD", "/Audio/"+dto.EncodeID(testID("s1"))+"/stream?"+query, nil).WithContext(ctxUser()) + r = withChiURLParam(r, "itemId", dto.EncodeID(testID("s1"))) + invoke(api.streamAudio, w, r) + return w + } + + It("answers a transcode with the target type and no length, without starting it", func() { + w := head("audioCodec=mp3") + Expect(w.Code).To(Equal(http.StatusOK)) + Expect(w.Header().Get("Content-Type")).To(Equal("audio/mpeg")) + Expect(w.Header().Get("Content-Length")).To(BeEmpty()) + Expect(w.Body.String()).To(BeEmpty()) + Expect(streamer.invoked).To(BeFalse()) + }) + + It("answers direct play through the streamer, without a body", func() { + w := head("static=true") + Expect(w.Code).To(Equal(http.StatusOK)) + Expect(streamer.invoked).To(BeTrue()) + Expect(w.Body.String()).To(BeEmpty()) + }) + }) + + Describe("streamUniversal", func() { + universal := func(query string) { + ds.MediaFile(context.Background()).(*tests.MockMediaFileRepo).SetData(model.MediaFiles{ + {ID: testID("s1"), Suffix: "mp3", LibraryID: 1}, + }) + w := httptest.NewRecorder() + r := httptest.NewRequest("GET", "/Audio/"+dto.EncodeID(testID("s1"))+"/universal?"+query, nil).WithContext(ctxUser()) + r = withChiURLParam(r, "itemId", dto.EncodeID(testID("s1"))) + invoke(api.streamUniversal, w, r) + Expect(w.Code).To(Equal(http.StatusOK)) + } + + It("turns Container into direct play profiles and TranscodingContainer into the target", func() { + universal("Container=mp3,m4a|aac&TranscodingContainer=m4a&AudioCodec=aac&MaxStreamingBitrate=128000") + Expect(decider.client.DirectPlayProfiles).To(Equal([]stream.DirectPlayProfile{ + {Containers: []string{"mp3"}, Protocols: []string{stream.ProtocolHTTP}}, + {Containers: []string{"m4a"}, AudioCodecs: []string{"aac"}, Protocols: []string{stream.ProtocolHTTP}}, + })) + Expect(decider.client.TranscodingProfiles).To(Equal([]stream.Profile{ + {Container: "m4a", AudioCodec: "aac", Protocol: stream.ProtocolHTTP}, + })) + Expect(decider.client.MaxAudioBitrate).To(Equal(128)) + Expect(decider.client.MaxTranscodingAudioBitrate).To(Equal(128)) + }) + + It("uses AudioCodec as the target when no TranscodingContainer is given", func() { + universal("Container=mp3&AudioCodec=aac") + Expect(decider.client.TranscodingProfiles).To(Equal([]stream.Profile{ + {Container: "aac", AudioCodec: "aac", Protocol: stream.ProtocolHTTP}, + })) + }) + + It("serves the file as is for static=true", func() { + universal("static=true&Container=ogg") + Expect(decider.req.Format).To(Equal("raw")) + Expect(decider.client).To(BeNil()) + }) + }) + Describe("streamHls", func() { BeforeEach(func() { ds.MediaFile(context.Background()).(*tests.MockMediaFileRepo).SetData(model.MediaFiles{ @@ -364,6 +433,7 @@ var _ = Describe("Stream", func() { type fakeTranscodeDecider struct { invoked bool req stream.Request + client *stream.ClientInfo } func (f *fakeTranscodeDecider) MakeDecision(context.Context, *model.MediaFile, *stream.ClientInfo, stream.TranscodeOptions) (*stream.TranscodeDecision, error) { @@ -384,6 +454,13 @@ func (f *fakeTranscodeDecider) ResolveRequest(_ context.Context, _ *model.MediaF return f.req } +func (f *fakeTranscodeDecider) ResolveClientRequest(_ context.Context, _ *model.MediaFile, ci *stream.ClientInfo, offset int) stream.Request { + f.invoked = true + f.client = ci + f.req = stream.Request{Offset: offset} + return f.req +} + // fakeMediaStreamer is a local test double for stream.MediaStreamer: it records whether // NewStream was invoked and, on success, returns a real (non-seekable) *stream.Stream backed // by an in-memory reader, so streamAudio's call to Stream.Serve exercises real code. diff --git a/server/jellyfin/system.go b/server/jellyfin/system.go index ccc1277df..35ade5ff1 100644 --- a/server/jellyfin/system.go +++ b/server/jellyfin/system.go @@ -10,6 +10,7 @@ import ( "net/netip" "path" "strings" + "sync" "github.com/google/uuid" "github.com/navidrome/navidrome/conf" @@ -20,40 +21,41 @@ import ( "github.com/navidrome/navidrome/server/jellyfin/dto" ) -// jellyfinVersion is the Jellyfin API version advertised in the handshake. Clients feature-gate -// on it, so it must stay a real Jellyfin release, not Navidrome's own version. 10.9+ is required -// for Feishin to use the server lyrics endpoint. -const jellyfinVersion = "10.9.11" +// jellyfinVersion is the Jellyfin API version advertised in the handshake. Clients and SDKs gate on +// it (Streamyfin and the Android apps refuse < 10.10), and some parsers need exactly three parts. +const jellyfinVersion = "12.1.0" -func (api *Router) serverName() string { +func serverName() string { if conf.Server.Jellyfin.ServerName != "" { return conf.Server.Jellyfin.ServerName } return fmt.Sprintf("Navidrome %s", consts.Version) } -// serverID returns a stable Id that survives restarts, get-or-created in the Property table. -// Jellyfin clients cache ServerId across sessions, so a per-process value would break -// re-authentication. api.ds is nil only in unit tests; New() always sets it. -// -// The mutex serializes first-boot resolution so concurrent requests can't persist different -// UUIDs. Only a successful read or persisted id is cached; a transient failure yields a -// temporary id and retries on the next request rather than pinning a value. func (api *Router) serverID(ctx context.Context) string { - api.serverIDMu.Lock() - defer api.serverIDMu.Unlock() - if api.serverIDVal != "" { - return api.serverIDVal + return resolveServerID(ctx, api.ds, &api.serverIDVal) +} + +// Package-level: the Router and Discovery are separate objects and must not persist different ids. +var serverIDMu sync.Mutex + +// Clients cache ServerId across sessions, so it is persisted. Only a successful read or write is +// cached: a transient failure yields a temporary id and retries on the next call. +func resolveServerID(ctx context.Context, ds model.DataStore, cached *string) string { + serverIDMu.Lock() + defer serverIDMu.Unlock() + if *cached != "" { + return *cached } - if api.ds == nil { - api.serverIDVal = newServerID() - return api.serverIDVal + if ds == nil { + *cached = newServerID() + return *cached } - id, err := api.ds.Property(ctx).Get(consts.JellyfinServerIDKey) + id, err := ds.Property(ctx).Get(consts.JellyfinServerIDKey) switch { case errors.Is(err, model.ErrNotFound): id = newServerID() - if err := api.ds.Property(ctx).Put(consts.JellyfinServerIDKey, id); err != nil { + if err := ds.Property(ctx).Put(consts.JellyfinServerIDKey, id); err != nil { log.Error(ctx, "Jellyfin API: could not persist server id", err) return id } @@ -62,8 +64,8 @@ func (api *Router) serverID(ctx context.Context) string { return newServerID() } // Ids persisted before this change are dashed; normalize on read rather than rewriting the DB. - api.serverIDVal = strings.ReplaceAll(id, "-", "") - return api.serverIDVal + *cached = strings.ReplaceAll(id, "-", "") + return *cached } // newServerID returns a UUID in Jellyfin's no-dash GUID form (Guid.ToString("N")). @@ -75,7 +77,7 @@ func newServerID() string { func (api *Router) publicInfo(r *http.Request) dto.PublicSystemInfo { return dto.PublicSystemInfo{ LocalAddress: localAddress(r), - ServerName: api.serverName(), + ServerName: serverName(), Version: jellyfinVersion, ProductName: "Jellyfin Server", Id: api.serverID(r.Context()), @@ -107,7 +109,7 @@ func (api *Router) getSystemInfo(w http.ResponseWriter, r *http.Request) { func (api *Router) ping(w http.ResponseWriter, r *http.Request) { w.Header().Set("Content-Type", "text/plain; charset=utf-8") w.WriteHeader(http.StatusOK) - _, _ = w.Write([]byte(api.serverName())) + _, _ = w.Write([]byte(serverName())) } // getEndpointInfo answers /System/Endpoint, which Finamp's connection test uses to pick between a diff --git a/server/jellyfin/system_test.go b/server/jellyfin/system_test.go index 4b3ac240e..c9368f3b5 100644 --- a/server/jellyfin/system_test.go +++ b/server/jellyfin/system_test.go @@ -7,6 +7,7 @@ import ( "net" "net/http" "net/http/httptest" + "sync" "github.com/navidrome/navidrome/conf" "github.com/navidrome/navidrome/conf/configtest" @@ -173,6 +174,17 @@ var _ = Describe("System", func() { Expect(second.serverID(ctx)).To(Equal(id)) }) + It("resolves one id when a Router and a Discovery race on first boot", func() { + r, d := &Router{ds: ds}, NewDiscovery(ds) + ids := make([]string, 2) + var wg sync.WaitGroup + wg.Go(func() { ids[0] = r.serverID(ctx) }) + wg.Go(func() { ids[1] = d.serverID(ctx) }) + wg.Wait() + Expect(ids[0]).ToNot(BeEmpty()) + Expect(ids[1]).To(Equal(ids[0])) + }) + It("memoizes the id across repeated calls on the same Router", func() { r := &Router{ds: ds} id := r.serverID(ctx) diff --git a/server/jellyfin/users.go b/server/jellyfin/users.go index bbc60c892..dddcd99da 100644 --- a/server/jellyfin/users.go +++ b/server/jellyfin/users.go @@ -13,10 +13,20 @@ import ( // getUserViews returns one CollectionFolder view per accessible library, so clients browse each // library as its own top-level view rather than one aggregate. func (api *Router) getUserViews(w http.ResponseWriter, r *http.Request) { - u, _ := request.UserFrom(r.Context()) - views := make([]dto.BaseItemDto, 0, len(u.Libraries)) - for _, lib := range u.Libraries { - views = append(views, libraryView(lib)) + ctx := r.Context() + u, _ := request.UserFrom(ctx) + // u.Libraries comes from a projection without counts or stats, and clients hide a library that + // looks empty, so the rows are re-read in full here. + libs, err := api.ds.Library(ctx).GetAll() + if err != nil { + api.internalError(w, r, err) + return + } + views := make([]dto.BaseItemDto, 0, len(libs)) + for _, lib := range libs { + if u.HasLibraryAccess(lib.ID) { + views = append(views, dto.LibraryToBaseItem(lib)) + } } api.ok(w, r, dto.QueryResult{Items: views, TotalRecordCount: len(views), StartIndex: 0}) } @@ -24,7 +34,7 @@ func (api *Router) getUserViews(w http.ResponseWriter, r *http.Request) { func (api *Router) getCurrentUser(w http.ResponseWriter, r *http.Request) { ctx := r.Context() u, _ := request.UserFrom(ctx) - api.ok(w, r, userToDto(&u, api.serverName(), api.serverID(ctx))) + api.ok(w, r, userToDto(&u, serverName(), api.serverID(ctx))) } // getPublicUsers advertises the users named in Jellyfin.ExposedPublicUsers for a client login diff --git a/server/jellyfin/users_test.go b/server/jellyfin/users_test.go index 3e53d598f..c6bb99f71 100644 --- a/server/jellyfin/users_test.go +++ b/server/jellyfin/users_test.go @@ -5,6 +5,7 @@ import ( "encoding/json" "net/http" "net/http/httptest" + "time" "github.com/navidrome/navidrome/conf" "github.com/navidrome/navidrome/conf/configtest" @@ -18,8 +19,14 @@ import ( var _ = Describe("Users", func() { var api *Router + // The repo holds the full rows; the user carries the id/name-only copy its projection returns. authedWithLibraries := func(r *http.Request, libs model.Libraries) *http.Request { - ctx := request.WithUser(context.Background(), model.User{ID: testID("u1"), UserName: "alice", Libraries: libs}) + api.ds.Library(context.Background()).(*tests.MockLibraryRepo).SetData(libs) + stripped := make(model.Libraries, len(libs)) + for i, lib := range libs { + stripped[i] = model.Library{ID: lib.ID, Name: lib.Name} + } + ctx := request.WithUser(context.Background(), model.User{ID: testID("u1"), UserName: "alice", Libraries: stripped}) return r.WithContext(ctx) } BeforeEach(func() { api = &Router{ds: &tests.MockDataStore{}} }) @@ -45,6 +52,25 @@ var _ = Describe("Users", func() { Expect(res.Items[1].Name).To(Equal("Podcasts")) }) + // Manet keeps no library, and so syncs no artists/albums/tracks, when these are missing. + It("describes the library like Jellyfin's CollectionFolder", func() { + created := time.Date(2026, 7, 1, 12, 0, 0, 0, time.UTC) + libs := model.Libraries{{ID: 1, Name: "Music", Path: "/music", TotalAlbums: 42, CreatedAt: created}} + w := httptest.NewRecorder() + api.getUserViews(w, authedWithLibraries(httptest.NewRequest("GET", "/UserViews", nil), libs)) + var res dto.QueryResult + Expect(json.Unmarshal(w.Body.Bytes(), &res)).To(Succeed()) + + item := res.Items[0] + Expect(*item.ChildCount).To(Equal(42)) + Expect(item.DateCreated).To(Equal("2026-07-01T12:00:00.0000000Z")) + Expect(item.SortName).To(Equal("Music")) + Expect(item.Path).To(Equal("/music")) + Expect(item.LocationType).To(Equal("FileSystem")) + Expect(item.UserData).ToNot(BeNil()) + Expect(item.UserData.ItemId).To(Equal(dto.EncodeLibraryID(1))) + }) + It("returns a single view for a user with one library", func() { libs := model.Libraries{{ID: 1, Name: "Music"}} w := httptest.NewRecorder() diff --git a/server/nativeapi/missing.go b/server/nativeapi/missing.go index 0ad9bb0cc..de6595e00 100644 --- a/server/nativeapi/missing.go +++ b/server/nativeapi/missing.go @@ -6,7 +6,6 @@ import ( "maps" "net/http" - "github.com/Masterminds/squirrel" "github.com/deluan/rest" "github.com/navidrome/navidrome/core" "github.com/navidrome/navidrome/log" @@ -46,17 +45,14 @@ func (r *missingRepository) parseOptions(options []rest.QueryOptions) rest.Query } func (r *missingRepository) Read(id string) (any, error) { - all, err := r.mfRepo.GetAll(model.QueryOptions{Filters: squirrel.And{ - squirrel.Eq{"id": id}, - squirrel.Eq{"missing": true}, - }}) + mf, err := r.mfRepo.Get(id) if err != nil { return nil, err } - if len(all) == 0 { + if !mf.Missing { return nil, model.ErrNotFound } - return all[0], nil + return mf, nil } func (r *missingRepository) EntityName() string { diff --git a/server/nativeapi/missing_test.go b/server/nativeapi/missing_test.go new file mode 100644 index 000000000..4da550c0d --- /dev/null +++ b/server/nativeapi/missing_test.go @@ -0,0 +1,56 @@ +package nativeapi + +import ( + "bytes" + "net/http" + "net/http/httptest" + "time" + + "github.com/navidrome/navidrome/conf" + "github.com/navidrome/navidrome/conf/configtest" + "github.com/navidrome/navidrome/core/auth" + "github.com/navidrome/navidrome/model" + "github.com/navidrome/navidrome/server" + "github.com/navidrome/navidrome/tests" + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" +) + +var _ = Describe("Missing Files Endpoint", func() { + var router http.Handler + var token string + + BeforeEach(func() { + DeferCleanup(configtest.SetupConfig()) + conf.Server.SessionTimeout = time.Minute + conf.Server.EnableSharing = false + + mfRepo := tests.CreateMockMediaFileRepo() + mfRepo.SetData(model.MediaFiles{ + {ID: "missing-1", Title: "Gone", Missing: true}, + {ID: "present-1", Title: "Here"}, + }) + userRepo := tests.CreateMockUserRepo() + ds := &tests.MockDataStore{MockedMediaFile: mfRepo, MockedUser: userRepo, MockedProperty: &tests.MockedPropertyRepo{}} + auth.Init(ds) + + user := model.User{ID: "user-1", UserName: "user", NewPassword: "pass"} + Expect(userRepo.Put(&user)).To(Succeed()) + var err error + token, err = auth.CreateToken(&user) + Expect(err).ToNot(HaveOccurred()) + + router = server.JWTVerifier(New(ds, nil, nil, nil, tests.NewMockLibraryService(), tests.NewMockUserService(), nil, nil, nil, nil)) + }) + + DescribeTable("GET /missing/{id}", + func(id string, status int) { + w := httptest.NewRecorder() + router.ServeHTTP(w, createAuthenticatedRequest(http.MethodGet, "/missing/"+id, &bytes.Buffer{}, token)) + Expect(w.Code).To(Equal(status), w.Body.String()) + }, + Entry("returns a missing file", "missing-1", http.StatusOK), + Entry("returns 404 for a file that is not missing", "present-1", http.StatusNotFound), + Entry("returns 404 for an unknown id", "unknown", http.StatusNotFound), + ) +}) diff --git a/server/nativeapi/playlists_test.go b/server/nativeapi/playlists_test.go index 9abcc477f..4a6f5d03d 100644 --- a/server/nativeapi/playlists_test.go +++ b/server/nativeapi/playlists_test.go @@ -192,6 +192,7 @@ var _ = Describe("writePlaylistError", func() { }, Entry("not found -> 404", model.ErrNotFound, http.StatusNotFound), Entry("not authorized -> 403", model.ErrNotAuthorized, http.StatusForbidden), + Entry("rest permission denied -> 403", rest.ErrPermissionDenied, http.StatusForbidden), Entry("not editable -> 409", model.ErrPlaylistNotEditable, http.StatusConflict), Entry("unrecognized -> default", model.ErrValidation, http.StatusBadRequest), ) diff --git a/server/subsonic/api.go b/server/subsonic/api.go index 029046c39..deedc46c7 100644 --- a/server/subsonic/api.go +++ b/server/subsonic/api.go @@ -9,7 +9,6 @@ import ( "regexp" "strconv" - "github.com/deluan/rest" "github.com/go-chi/chi/v5" "github.com/navidrome/navidrome/conf" "github.com/navidrome/navidrome/core" @@ -302,10 +301,9 @@ func mapToSubsonicError(err error) subError { err = newError(responses.ErrorMissingParameter, err.Error()) case errors.Is(err, req.ErrInvalidParam): err = newError(responses.ErrorGeneric, err.Error()) - case errors.Is(err, model.ErrNotFound), errors.Is(err, rest.ErrNotFound): + case errors.Is(err, model.ErrNotFound): err = newError(responses.ErrorDataNotFound, "data not found") - case errors.Is(err, model.ErrNotAuthorized), errors.Is(err, rest.ErrPermissionDenied), - errors.Is(err, model.ErrPlaylistNotEditable): // Subsonic has no code for "read-only resource" + case errors.Is(err, model.ErrNotAuthorized), errors.Is(err, model.ErrPlaylistNotEditable): // Subsonic has no code for "read-only resource" err = newError(responses.ErrorAuthorizationFail) case errors.Is(err, stream.ErrTooManyTranscodes): err = newError(responses.ErrorGeneric, "too many concurrent transcodes, please retry shortly") diff --git a/server/subsonic/e2e/subsonic_radio_test.go b/server/subsonic/e2e/subsonic_radio_test.go index cd778fa79..d51cf6b14 100644 --- a/server/subsonic/e2e/subsonic_radio_test.go +++ b/server/subsonic/e2e/subsonic_radio_test.go @@ -130,4 +130,12 @@ var _ = Describe("Internet Radio Endpoints", Ordered, func() { Expect(resp.InternetRadioStations).ToNot(BeNil()) Expect(resp.InternetRadioStations.Radios).To(BeEmpty()) }) + + It("deleteInternetRadioStation returns not found for a missing station", func() { + resp := doReq("deleteInternetRadioStation", "id", radioID) + + Expect(resp.Status).To(Equal(responses.StatusFailed)) + Expect(resp.Error).ToNot(BeNil()) + Expect(resp.Error.Code).To(Equal(responses.ErrorDataNotFound)) + }) }) diff --git a/server/subsonic/transcode_test.go b/server/subsonic/transcode_test.go index 0f3c24832..1f2fdfcef 100644 --- a/server/subsonic/transcode_test.go +++ b/server/subsonic/transcode_test.go @@ -625,6 +625,10 @@ func (m *mockTranscodeDecision) ResolveRequest(_ context.Context, _ *model.Media return stream.Request{Format: "raw"} } +func (m *mockTranscodeDecision) ResolveClientRequest(context.Context, *model.MediaFile, *stream.ClientInfo, int) stream.Request { + return stream.Request{Format: "raw"} +} + func (m *mockTranscodeDecision) CreateTranscodeParams(_ *stream.TranscodeDecision) (string, error) { return m.token, m.tokenErr } diff --git a/tests/mock_library_repo.go b/tests/mock_library_repo.go index 1a16a7e0b..0f7af2aab 100644 --- a/tests/mock_library_repo.go +++ b/tests/mock_library_repo.go @@ -172,11 +172,7 @@ func (m *MockLibraryRepo) Count(options ...rest.QueryOptions) (int64, error) { func (m *MockLibraryRepo) Read(id string) (any, error) { idInt, _ := strconv.Atoi(id) - mf, err := m.Get(idInt) - if errors.Is(err, model.ErrNotFound) { - return nil, rest.ErrNotFound - } - return mf, err + return m.Get(idInt) } func (m *MockLibraryRepo) ReadAll(options ...rest.QueryOptions) (any, error) { diff --git a/tests/mock_mediafile_repo.go b/tests/mock_mediafile_repo.go index 74225cd9b..a365bd2bd 100644 --- a/tests/mock_mediafile_repo.go +++ b/tests/mock_mediafile_repo.go @@ -302,11 +302,7 @@ func (m *MockMediaFileRepo) Count(...rest.QueryOptions) (int64, error) { } func (m *MockMediaFileRepo) Read(id string) (any, error) { - mf, err := m.Get(id) - if errors.Is(err, model.ErrNotFound) { - return nil, rest.ErrNotFound - } - return mf, err + return m.Get(id) } func (m *MockMediaFileRepo) ReadAll(...rest.QueryOptions) (any, error) { diff --git a/tests/mock_playlist_track_repo.go b/tests/mock_playlist_track_repo.go index 83751bb28..5666e7cc7 100644 --- a/tests/mock_playlist_track_repo.go +++ b/tests/mock_playlist_track_repo.go @@ -13,6 +13,7 @@ type MockPlaylistTrackRepo struct { DeletedIds []string Reordered bool AddCount int + InsertPos int Err error AlbumIDs []string // stubbed result for GetAlbumIDs, ignoring options } @@ -89,6 +90,11 @@ func (m *MockPlaylistTrackRepo) Add(ids []string) (int, error) { return m.AddCount, nil } +func (m *MockPlaylistTrackRepo) Insert(ids []string, pos int) (int, error) { + m.InsertPos = pos + return m.Add(ids) +} + func (m *MockPlaylistTrackRepo) AddAlbums(_ []string) (int, error) { if m.Err != nil { return 0, m.Err diff --git a/tests/mock_plugin_repo.go b/tests/mock_plugin_repo.go index 5d22c26aa..e65d56def 100644 --- a/tests/mock_plugin_repo.go +++ b/tests/mock_plugin_repo.go @@ -69,11 +69,7 @@ func (m *MockPluginRepo) Get(id string) (*model.Plugin, error) { } func (m *MockPluginRepo) Read(id string) (any, error) { - p, err := m.Get(id) - if errors.Is(err, model.ErrNotFound) { - return nil, rest.ErrNotFound - } - return p, err + return m.Get(id) } func (m *MockPluginRepo) Put(p *model.Plugin) error { diff --git a/ui/src/album/AlbumDetails.jsx b/ui/src/album/AlbumDetails.jsx index f55d94ffc..7fbc479db 100644 --- a/ui/src/album/AlbumDetails.jsx +++ b/ui/src/album/AlbumDetails.jsx @@ -31,6 +31,7 @@ import { SizeField, useAlbumsPerPage, } from '../common' +import { useDateLocale } from '../i18n/useDateLocale' import { formatFullDate, intersperse } from '../utils' import AlbumExternalLinks from './AlbumExternalLinks' import { SafeHTML } from '../common/SafeHTML' @@ -151,6 +152,7 @@ export const Details = (props) => { const isXsmall = useMediaQuery((theme) => theme.breakpoints.down('xs')) const translate = useTranslate() const record = useRecordContext(props) + const locale = useDateLocale() // Create an array of detail elements let details = [] @@ -161,12 +163,13 @@ export const Details = (props) => { // Calculate date related fields const yearRange = formatRange(record, 'year') - const date = record.date ? formatFullDate(record.date) : yearRange + const date = record.date ? formatFullDate(record.date, locale) : yearRange const originalDate = record.originalDate - ? formatFullDate(record.originalDate) + ? formatFullDate(record.originalDate, locale) : formatRange(record, 'originalYear') - const releaseDate = record?.releaseDate && formatFullDate(record.releaseDate) + const releaseDate = + record?.releaseDate && formatFullDate(record.releaseDate, locale) const dateToUse = originalDate || date const isOriginalDate = originalDate && dateToUse !== date diff --git a/ui/src/common/DateField.jsx b/ui/src/common/DateField.jsx index dce24a2b9..dac9cff08 100644 --- a/ui/src/common/DateField.jsx +++ b/ui/src/common/DateField.jsx @@ -1,12 +1,14 @@ import React from 'react' import { isDateSet } from '../utils/validations' import { DateField as RADateField } from 'react-admin' +import { useDateLocale } from '../i18n/useDateLocale' export const DateField = (props) => { const { record, source } = props + const locale = useDateLocale() const value = record?.[source] if (!isDateSet(value)) return null - return + return } DateField.defaultProps = { diff --git a/ui/src/common/DateField.test.jsx b/ui/src/common/DateField.test.jsx new file mode 100644 index 000000000..47d56756b --- /dev/null +++ b/ui/src/common/DateField.test.jsx @@ -0,0 +1,32 @@ +import React from 'react' +import { render, screen } from '@testing-library/react' +import { describe, it, expect, beforeEach, vi } from 'vitest' +import { DateField } from './DateField' + +vi.mock('react-admin', async (importOriginal) => ({ + ...(await importOriginal()), + useLocale: vi.fn(), +})) + +describe('', () => { + const record = { id: '1', updatedAt: '2026-09-17T14:30:00Z' } + + beforeEach(async () => { + vi.clearAllMocks() + vi.spyOn(navigator, 'languages', 'get').mockReturnValue([]) + const { useLocale } = await import('react-admin') + vi.mocked(useLocale).mockReturnValue('de') + }) + + it('formats the date using the selected language', () => { + render() + expect(screen.getByText('17.9.2026')).toBeInTheDocument() + }) + + it('renders nothing when the date is not set', () => { + const { container } = render( + , + ) + expect(container).toBeEmptyDOMElement() + }) +}) diff --git a/ui/src/common/LoveButton.jsx b/ui/src/common/LoveButton.jsx index 492ba95b3..128b375c2 100644 --- a/ui/src/common/LoveButton.jsx +++ b/ui/src/common/LoveButton.jsx @@ -6,6 +6,8 @@ import IconButton from '@material-ui/core/IconButton' import { makeStyles } from '@material-ui/core/styles' import clsx from 'clsx' import { useToggleLove } from './useToggleLove' +import { useDateLocale } from '../i18n/useDateLocale' +import { formatDateTime } from '../utils/formatters' import { useRecordContext } from 'react-admin' import config from '../config' import { isDateSet } from '../utils/validations' @@ -40,6 +42,7 @@ export const LoveButton = ({ const record = useRecordContext({ record: recordProp }) || {} const classes = useStyles({ color, visible, loved: record.starred }) const [toggleLove, loading] = useToggleLove(resource, record) + const locale = useDateLocale() const handleToggleLove = useCallback( (e) => { @@ -61,7 +64,7 @@ export const LoveButton = ({ className={clsx(classes.love, className)} title={ isDateSet(record.starredAt) - ? new Date(record.starredAt).toLocaleString() + ? formatDateTime(record.starredAt, locale) : undefined } {...rest} diff --git a/ui/src/common/RatingField.jsx b/ui/src/common/RatingField.jsx index f92b0d948..f892b475a 100644 --- a/ui/src/common/RatingField.jsx +++ b/ui/src/common/RatingField.jsx @@ -6,6 +6,8 @@ import { isDateSet } from '../utils/validations' import StarBorderIcon from '@material-ui/icons/StarBorder' import clsx from 'clsx' import { useRating } from './useRating' +import { useDateLocale } from '../i18n/useDateLocale' +import { formatDateTime } from '../utils/formatters' import { useRecordContext } from 'react-admin' const useStyles = makeStyles({ @@ -32,6 +34,7 @@ export const RatingField = ({ const record = useRecordContext(rest) || {} const [rate, rating] = useRating(resource, record) const classes = useStyles({ color, visible }) + const locale = useDateLocale() const stopPropagation = (e) => { e.stopPropagation() @@ -50,7 +53,7 @@ export const RatingField = ({ onClick={(e) => stopPropagation(e)} title={ isDateSet(record.ratedAt) - ? new Date(record.ratedAt).toLocaleString() + ? formatDateTime(record.ratedAt, locale) : undefined } > diff --git a/ui/src/i18n/useDateLocale.js b/ui/src/i18n/useDateLocale.js new file mode 100644 index 000000000..c9298a473 --- /dev/null +++ b/ui/src/i18n/useDateLocale.js @@ -0,0 +1,14 @@ +import { useLocale } from 'react-admin' + +// Our language codes are mostly region-less ("en"), and Intl reads a bare "en" +// as en-US. Borrow the region from the browser when it speaks the same language. +const resolveDateLocale = (locale, browserLocales = []) => { + if (!locale || locale.includes('-')) return locale + const base = locale.toLowerCase() + return ( + browserLocales.find((l) => l.toLowerCase().split('-')[0] === base) || locale + ) +} + +export const useDateLocale = () => + resolveDateLocale(useLocale(), navigator.languages) diff --git a/ui/src/i18n/useDateLocale.test.js b/ui/src/i18n/useDateLocale.test.js new file mode 100644 index 000000000..c0ea5ab40 --- /dev/null +++ b/ui/src/i18n/useDateLocale.test.js @@ -0,0 +1,45 @@ +import { renderHook } from '@testing-library/react-hooks' +import { describe, it, expect, beforeEach, vi } from 'vitest' +import { useDateLocale } from './useDateLocale' + +vi.mock('react-admin', () => ({ + useLocale: vi.fn(), +})) + +describe('useDateLocale', () => { + beforeEach(() => { + vi.clearAllMocks() + }) + + const renderWith = async (locale, browserLocales) => { + const { useLocale } = await import('react-admin') + vi.mocked(useLocale).mockReturnValue(locale) + vi.spyOn(navigator, 'languages', 'get').mockReturnValue(browserLocales) + return renderHook(() => useDateLocale()).result + } + + it('adds the region from the browser when the language has none', async () => { + const result = await renderWith('en', ['en-GB', 'fr-FR']) + expect(result.current).toEqual('en-GB') + }) + + it('falls back to the language when no browser entry matches', async () => { + const result = await renderWith('de', ['en-US', 'fr-FR']) + expect(result.current).toEqual('de') + }) + + it('keeps a language that already carries a region or script', async () => { + expect((await renderWith('pt-br', ['pt-PT'])).current).toEqual('pt-br') + expect((await renderWith('zh-Hans', ['zh-TW'])).current).toEqual('zh-Hans') + }) + + it('matches the browser language case-insensitively', async () => { + const result = await renderWith('pt', ['PT-PT']) + expect(result.current).toEqual('PT-PT') + }) + + it('returns undefined when there is no language', async () => { + const result = await renderWith(undefined, ['en-GB']) + expect(result.current).toBeUndefined() + }) +}) diff --git a/ui/src/user/UserList.jsx b/ui/src/user/UserList.jsx index 4faf31785..7922ca08d 100644 --- a/ui/src/user/UserList.jsx +++ b/ui/src/user/UserList.jsx @@ -9,6 +9,8 @@ import { } from 'react-admin' import { useMediaQuery } from '@material-ui/core' import { List, DateField } from '../common' +import { useDateLocale } from '../i18n/useDateLocale' +import { formatDateTime } from '../utils/formatters' const UserFilter = (props) => ( @@ -18,6 +20,7 @@ const UserFilter = (props) => ( const UserList = (props) => { const isXsmall = useMediaQuery((theme) => theme.breakpoints.down('xs')) + const locale = useDateLocale() return ( { record.userName} secondaryText={(record) => - record.lastLoginAt && new Date(record.lastLoginAt).toLocaleString() + record.lastLoginAt && formatDateTime(record.lastLoginAt, locale) } tertiaryText={(record) => (record.isAdmin ? '[admin]️' : '')} /> diff --git a/ui/src/utils/formatters.js b/ui/src/utils/formatters.js index cfcb84b05..f7c59c195 100644 --- a/ui/src/utils/formatters.js +++ b/ui/src/utils/formatters.js @@ -95,6 +95,9 @@ export const formatFullDate = (date, locale) => { return new Date(date).toLocaleDateString(locale, options) } +export const formatDateTime = (value, locale) => + new Date(value).toLocaleString(locale) + export const formatNumber = (value, locale) => { if (value === null || value === undefined) return '0' return value.toLocaleString(locale)