diff --git a/cmd/wire_gen.go b/cmd/wire_gen.go index 463ac7a44..c4a797ef4 100644 --- a/cmd/wire_gen.go +++ b/cmd/wire_gen.go @@ -138,7 +138,7 @@ func CreateJellyfinAPIRouter(ctx context.Context) *jellyfin.Router { playlistsPlaylists := playlists.NewPlaylists(dataStore, imageUploadService) sonicSonic := sonic.New(dataStore, manager, matcherMatcher) lyricsLyrics := lyrics.NewLyrics(dataStore, manager) - router := jellyfin.New(dataStore, artworkArtwork, mediaStreamer, transcodeDecider, players, playTracker, playlistsPlaylists, provider, sonicSonic, lyricsLyrics) + router := jellyfin.New(dataStore, artworkArtwork, mediaStreamer, transcodeDecider, players, playTracker, playlistsPlaylists, provider, sonicSonic, lyricsLyrics, broker) return router } diff --git a/model/get_entity.go b/model/get_entity.go index 60972b2e9..3e1a78d1d 100644 --- a/model/get_entity.go +++ b/model/get_entity.go @@ -2,29 +2,26 @@ package model import ( "context" + "errors" ) // TODO: Should the type be encoded in the ID? func GetEntityByID(ctx context.Context, ds DataStore, id string) (any, error) { - ar, err := ds.Artist(ctx).Get(id) - if err == nil { - return ar, nil + getters := []func() (any, error){ + func() (any, error) { return ds.Artist(ctx).Get(id) }, + func() (any, error) { return ds.Album(ctx).Get(id) }, + func() (any, error) { return ds.Playlist(ctx).Get(id) }, + func() (any, error) { return ds.MediaFile(ctx).Get(id) }, + func() (any, error) { return ds.Radio(ctx).Get(id) }, } - al, err := ds.Album(ctx).Get(id) - if err == nil { - return al, nil + for _, get := range getters { + entity, err := get() + if err == nil { + return entity, nil + } + if !errors.Is(err, ErrNotFound) { + return nil, err + } } - pls, err := ds.Playlist(ctx).Get(id) - if err == nil { - return pls, nil - } - mf, err := ds.MediaFile(ctx).Get(id) - if err == nil { - return mf, nil - } - r, err := ds.Radio(ctx).Get(id) - if err == nil { - return r, nil - } - return nil, err + return nil, ErrNotFound } diff --git a/model/get_entity_test.go b/model/get_entity_test.go new file mode 100644 index 000000000..f8a4c9e8e --- /dev/null +++ b/model/get_entity_test.go @@ -0,0 +1,40 @@ +package model_test + +import ( + "context" + + "github.com/navidrome/navidrome/model" + "github.com/navidrome/navidrome/tests" + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" +) + +var _ = Describe("GetEntityByID", func() { + var ds *tests.MockDataStore + var ctx context.Context + + BeforeEach(func() { + ds = &tests.MockDataStore{} + ctx = GinkgoT().Context() + }) + + It("returns the entity matching the id", func() { + ds.Album(ctx).(*tests.MockAlbumRepo).SetData(model.Albums{{ID: "a1", Name: "One"}}) + entity, err := model.GetEntityByID(ctx, ds, "a1") + Expect(err).ToNot(HaveOccurred()) + Expect(entity).To(BeAssignableToTypeOf(&model.Album{})) + Expect(entity.(*model.Album).ID).To(Equal("a1")) + }) + + It("returns ErrNotFound when no entity matches", func() { + _, err := model.GetEntityByID(ctx, ds, "missing") + Expect(err).To(MatchError(model.ErrNotFound)) + }) + + It("propagates unexpected repository errors instead of reporting not-found", func() { + ds.Album(ctx).(*tests.MockAlbumRepo).SetError(true) + _, err := model.GetEntityByID(ctx, ds, "a1") + Expect(err).To(HaveOccurred()) + Expect(err).ToNot(MatchError(model.ErrNotFound)) + }) +}) diff --git a/server/jellyfin/annotations.go b/server/jellyfin/annotations.go index 8f900ee87..f2c2f546e 100644 --- a/server/jellyfin/annotations.go +++ b/server/jellyfin/annotations.go @@ -8,53 +8,41 @@ import ( "github.com/go-chi/chi/v5" "github.com/navidrome/navidrome/model" "github.com/navidrome/navidrome/model/request" + "github.com/navidrome/navidrome/server/events" "github.com/navidrome/navidrome/server/jellyfin/dto" "github.com/navidrome/navidrome/utils/req" ) -// resolveAnnotated finds which annotated repo owns id. Albums and songs 404 when the user can't -// access their library; artists span libraries (library_artist), so have no single LibraryID to -// gate on and rely on list-time scoping. PlaylistRepository.Get enforces playlist visibility. -// When ok is false the response has already been written, so callers must return without writing -// the annotation. -func (api *Router) resolveAnnotated(w http.ResponseWriter, r *http.Request, id string) (repo model.AnnotatedRepository, ok bool) { +// resolveAnnotated finds which annotated repo owns id, returning the resource name used in +// refreshResource events. Albums and songs 404 when the user can't access their library; artists +// span libraries (library_artist), so have no single LibraryID to gate on and rely on list-time +// scoping. PlaylistRepository.Get enforces playlist visibility. When repo is nil the response has +// already been written, so callers must return without writing the annotation. +func (api *Router) resolveAnnotated(w http.ResponseWriter, r *http.Request, id string) (repo model.AnnotatedRepository, resource string) { ctx := r.Context() + entity, err := model.GetEntityByID(ctx, api.ds, id) + if err != nil && !errors.Is(err, model.ErrNotFound) { + api.internalError(w, r, err) + return nil, "" + } u, _ := request.UserFrom(ctx) - if al, err := api.ds.Album(ctx).Get(id); err == nil { - if !u.HasLibraryAccess(al.LibraryID) { - http.Error(w, "Not Found", http.StatusNotFound) - return nil, false + switch e := entity.(type) { + case *model.Album: + if u.HasLibraryAccess(e.LibraryID) { + return api.ds.Album(ctx), "album" } - return api.ds.Album(ctx), true - } else if !errors.Is(err, model.ErrNotFound) { - api.internalError(w, r, err) - return nil, false - } - if _, err := api.ds.Artist(ctx).Get(id); err == nil { - return api.ds.Artist(ctx), true - } else if !errors.Is(err, model.ErrNotFound) { - api.internalError(w, r, err) - return nil, false - } - if mf, err := api.ds.MediaFile(ctx).Get(id); err == nil { - if !u.HasLibraryAccess(mf.LibraryID) { - http.Error(w, "Not Found", http.StatusNotFound) - return nil, false + case *model.Artist: + return api.ds.Artist(ctx), "artist" + case *model.MediaFile: + if u.HasLibraryAccess(e.LibraryID) { + return api.ds.MediaFile(ctx), "song" } - return api.ds.MediaFile(ctx), true - } else if !errors.Is(err, model.ErrNotFound) { - api.internalError(w, r, err) - return nil, false - } - playlistRepo := api.ds.Playlist(ctx) - if _, err := playlistRepo.Get(id); err == nil { - return playlistRepo, true - } else if !errors.Is(err, model.ErrNotFound) { - api.internalError(w, r, err) - return nil, false + case *model.Playlist: + return api.ds.Playlist(ctx), "playlist" } + // Unknown ids, inaccessible-library items and non-annotatable entities (radios) all read as absent. http.Error(w, "Not Found", http.StatusNotFound) - return nil, false + return nil, "" } // getUserItemData returns the caller's play/favorite/rating state for a single item. Jellify @@ -77,14 +65,15 @@ func (api *Router) getUserItemData(w http.ResponseWriter, r *http.Request) { func (api *Router) setFavorite(w http.ResponseWriter, r *http.Request, starred bool) { id := api.resolveItemID(r.Context(), dto.DecodeID(chi.URLParam(r, "itemId"))) - repo, ok := api.resolveAnnotated(w, r, id) - if !ok { + repo, resource := api.resolveAnnotated(w, r, id) + if repo == nil { return } if err := repo.SetStar(starred, id); err != nil { api.internalError(w, r, err) return } + api.broker.SendMessage(r.Context(), (&events.RefreshResource{}).With(resource, id)) encodedID := dto.EncodeID(id) api.ok(w, r, &dto.UserItemDataDto{IsFavorite: starred, Key: encodedID, ItemId: encodedID}) } @@ -96,14 +85,15 @@ func (api *Router) unmarkFavorite(w http.ResponseWriter, r *http.Request) { func (api *Router) setItemRating(w http.ResponseWriter, r *http.Request, rating int) { id := api.resolveItemID(r.Context(), dto.DecodeID(chi.URLParam(r, "itemId"))) - repo, ok := api.resolveAnnotated(w, r, id) - if !ok { + repo, resource := api.resolveAnnotated(w, r, id) + if repo == nil { return } if err := repo.SetRating(rating, id); err != nil { api.internalError(w, r, err) return } + api.broker.SendMessage(r.Context(), (&events.RefreshResource{}).With(resource, id)) encodedID := dto.EncodeID(id) d := &dto.UserItemDataDto{Key: encodedID, ItemId: encodedID} if rating > 0 { diff --git a/server/jellyfin/annotations_test.go b/server/jellyfin/annotations_test.go index 4a7efabd7..9e811a379 100644 --- a/server/jellyfin/annotations_test.go +++ b/server/jellyfin/annotations_test.go @@ -8,6 +8,7 @@ import ( "github.com/navidrome/navidrome/model" "github.com/navidrome/navidrome/model/request" + "github.com/navidrome/navidrome/server/events" "github.com/navidrome/navidrome/server/jellyfin/dto" "github.com/navidrome/navidrome/tests" . "github.com/onsi/ginkgo/v2" @@ -17,6 +18,7 @@ import ( var _ = Describe("Annotations", func() { var api *Router var ds *tests.MockDataStore + var broker *fakeEventBroker // alice has access to library 1 only. ctxUser := func() context.Context { return request.WithUser(context.Background(), model.User{ID: "u1", UserName: "alice", Libraries: model.Libraries{{ID: 1, Name: "Music"}}}) @@ -24,7 +26,8 @@ var _ = Describe("Annotations", func() { BeforeEach(func() { ds = &tests.MockDataStore{} - api = &Router{ds: ds} + broker = &fakeEventBroker{} + api = &Router{ds: ds, broker: broker} }) Describe("markFavorite / unmarkFavorite", func() { @@ -134,6 +137,39 @@ var _ = Describe("Annotations", func() { invoke(api.markFavorite, w, r) Expect(w.Code).To(Equal(http.StatusInternalServerError)) }) + + It("emits a refreshResource event when starring a song", func() { + mfRepo := ds.MediaFile(context.Background()).(*tests.MockMediaFileRepo) + mfRepo.SetData(model.MediaFiles{{ID: "s1", Title: "Song", LibraryID: 1}}) + w := httptest.NewRecorder() + r := httptest.NewRequest("POST", "/Users/u1/FavoriteItems/s1", nil).WithContext(ctxUser()) + r = withChiURLParam(r, "itemId", "s1") + invoke(api.markFavorite, w, r) + Expect(broker.Events).To(HaveLen(1)) + Expect(broker.Events[0].Data(broker.Events[0])).To(Equal(`{"song":["s1"]}`)) + }) + + It("emits a refreshResource event when starring an album", func() { + albumRepo := ds.Album(context.Background()).(*tests.MockAlbumRepo) + albumRepo.SetData(model.Albums{{ID: "a1", Name: "One", LibraryID: 1}}) + w := httptest.NewRecorder() + r := httptest.NewRequest("POST", "/Users/u1/FavoriteItems/"+dto.EncodeID("a1"), nil).WithContext(ctxUser()) + r = withChiURLParam(r, "itemId", dto.EncodeID("a1")) + invoke(api.markFavorite, w, r) + Expect(broker.Events).To(HaveLen(1)) + Expect(broker.Events[0].Data(broker.Events[0])).To(Equal(`{"album":["a1"]}`)) + }) + + It("does not emit an event when the item is not accessible", func() { + albumRepo := ds.Album(context.Background()).(*tests.MockAlbumRepo) + albumRepo.SetData(model.Albums{{ID: "a1", Name: "One", LibraryID: 2}}) + w := httptest.NewRecorder() + r := httptest.NewRequest("POST", "/Users/u1/FavoriteItems/"+dto.EncodeID("a1"), nil).WithContext(ctxUser()) + r = withChiURLParam(r, "itemId", dto.EncodeID("a1")) + invoke(api.markFavorite, w, r) + Expect(w.Code).To(Equal(http.StatusNotFound)) + Expect(broker.Events).To(BeEmpty()) + }) }) Describe("setRating / removeRating", func() { @@ -253,5 +289,31 @@ var _ = Describe("Annotations", func() { Expect(w.Code).To(Equal(http.StatusOK)) Expect(mfRepo.Data["s1"].Rating).To(Equal(0)) }) + + It("emits a refreshResource event when rating a song", func() { + mfRepo := ds.MediaFile(context.Background()).(*tests.MockMediaFileRepo) + mfRepo.SetData(model.MediaFiles{{ID: "s1", Title: "Song", LibraryID: 1}}) + w := httptest.NewRecorder() + r := httptest.NewRequest("POST", "/Users/u1/Items/s1/Rating?Rating=8", nil).WithContext(ctxUser()) + r = withChiURLParam(r, "itemId", "s1") + invoke(api.setRating, w, r) + Expect(broker.Events).To(HaveLen(1)) + Expect(broker.Events[0].Data(broker.Events[0])).To(Equal(`{"song":["s1"]}`)) + }) }) }) + +type fakeEventBroker struct { + http.Handler + Events []events.Event +} + +func (f *fakeEventBroker) SendMessage(_ context.Context, event events.Event) { + f.Events = append(f.Events, event) +} + +func (f *fakeEventBroker) SendBroadcastMessage(_ context.Context, event events.Event) { + f.Events = append(f.Events, event) +} + +var _ events.Broker = (*fakeEventBroker)(nil) diff --git a/server/jellyfin/api.go b/server/jellyfin/api.go index 5e7cad0b8..0f79011c7 100644 --- a/server/jellyfin/api.go +++ b/server/jellyfin/api.go @@ -22,6 +22,7 @@ import ( "github.com/navidrome/navidrome/log" "github.com/navidrome/navidrome/model" "github.com/navidrome/navidrome/server" + "github.com/navidrome/navidrome/server/events" "github.com/navidrome/navidrome/server/jellyfin/dto" "github.com/navidrome/navidrome/utils/cache" ) @@ -38,6 +39,7 @@ type Router struct { provider external.Provider sonic sonic.Engine lyrics lyrics.Lyrics + broker events.Broker lyricsCache cache.SimpleCache[string, model.LyricList] similarFlight singleflight.Group serverIDMu sync.Mutex @@ -47,11 +49,11 @@ type Router struct { func New(ds model.DataStore, artwork artwork.Artwork, streamer stream.MediaStreamer, transcodeDecider stream.TranscodeDecider, players core.Players, scrobbler scrobbler.PlayTracker, playlists playlists.Playlists, provider external.Provider, - sonicSvc sonic.Engine, lyricsSvc lyrics.Lyrics) *Router { + sonicSvc sonic.Engine, lyricsSvc lyrics.Lyrics, broker events.Broker) *Router { r := &Router{ ds: ds, artwork: artwork, streamer: streamer, transcodeDecider: transcodeDecider, players: players, scrobbler: scrobbler, playlists: playlists, provider: provider, - sonic: sonicSvc, lyrics: lyricsSvc, + sonic: sonicSvc, lyrics: lyricsSvc, broker: broker, lyricsCache: cache.NewSimpleCache[string, model.LyricList](cache.Options{ SizeLimit: 1000, DefaultTTL: 5 * time.Minute, diff --git a/server/jellyfin/api_test.go b/server/jellyfin/api_test.go index 62af74794..e8e9cbd4b 100644 --- a/server/jellyfin/api_test.go +++ b/server/jellyfin/api_test.go @@ -18,7 +18,7 @@ import ( var _ = Describe("Router", func() { It("serves the public handshake through the mounted handler", func() { ds := &tests.MockDataStore{} - api := New(ds, nil, nil, nil, nil, nil, nil, nil, nil, nil) + api := New(ds, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil) w := httptest.NewRecorder() r := httptest.NewRequest("GET", "/System/Info/Public", nil) api.ServeHTTP(w, r) @@ -26,7 +26,7 @@ var _ = Describe("Router", func() { }) It("returns 404 JSON for unknown routes", func() { - api := New(&tests.MockDataStore{}, nil, nil, nil, nil, nil, nil, nil, nil, nil) + api := New(&tests.MockDataStore{}, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil) w := httptest.NewRecorder() r := httptest.NewRequest("GET", "/Nonexistent/Route", nil) api.ServeHTTP(w, r) @@ -36,7 +36,7 @@ var _ = Describe("Router", func() { }) It("returns 404 JSON for a known path with an unsupported method", func() { - api := New(&tests.MockDataStore{}, nil, nil, nil, nil, nil, nil, nil, nil, nil) + api := New(&tests.MockDataStore{}, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil) w := httptest.NewRecorder() r := httptest.NewRequest("PATCH", "/System/Info/Public", nil) api.ServeHTTP(w, r) @@ -53,7 +53,7 @@ var _ = Describe("Router", func() { Expect(err).ToNot(HaveOccurred()) fp := &fakePlayers{} - api := New(ds, nil, nil, nil, fp, nil, nil, nil, nil, nil) + api := New(ds, nil, nil, nil, fp, nil, nil, nil, nil, nil, nil) w := httptest.NewRecorder() r := httptest.NewRequest("GET", "/Users/Me", nil) @@ -70,7 +70,7 @@ var _ = Describe("Router", func() { DeferCleanup(configtest.SetupConfig()) conf.Server.AuthRequestLimit = 2 conf.Server.AuthWindowLength = time.Minute - api := New(&tests.MockDataStore{}, nil, nil, nil, nil, nil, nil, nil, nil, nil) + api := New(&tests.MockDataStore{}, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil) login := func() int { w := httptest.NewRecorder() diff --git a/server/jellyfin/e2e/e2e_suite_test.go b/server/jellyfin/e2e/e2e_suite_test.go index e0f7b5bb5..445566387 100644 --- a/server/jellyfin/e2e/e2e_suite_test.go +++ b/server/jellyfin/e2e/e2e_suite_test.go @@ -329,6 +329,7 @@ func setupTestDB() { providerFake, sonicSvc, lyrics.NewLyrics(ds, nil), + events.NoopBroker(), ) } diff --git a/server/jellyfin/routing_test.go b/server/jellyfin/routing_test.go index b0bc92caf..62b52cfff 100644 --- a/server/jellyfin/routing_test.go +++ b/server/jellyfin/routing_test.go @@ -19,7 +19,7 @@ var _ = Describe("Case-insensitive routing", func() { var api *Router BeforeEach(func() { - api = New(&tests.MockDataStore{}, nil, nil, nil, nil, nil, nil, nil, nil, nil) + api = New(&tests.MockDataStore{}, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil) }) It("serves a fully lowercase path directly", func() { diff --git a/server/jellyfin/socket_test.go b/server/jellyfin/socket_test.go index 7d8874476..8c509fe91 100644 --- a/server/jellyfin/socket_test.go +++ b/server/jellyfin/socket_test.go @@ -94,7 +94,7 @@ var _ = Describe("handleSocket", func() { Expect(err).ToNot(HaveOccurred()) token = t - api = New(ds, nil, nil, nil, nil, nil, nil, nil, nil, nil) + api = New(ds, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil) }) It("upgrades when authenticated via the api_key query parameter", func() {