From 772d1f359bb35bbe12bf79ca9b55c635c144a402 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Deluan=20Quint=C3=A3o?= Date: Sun, 25 Jan 2026 16:16:43 -0500 Subject: [PATCH] feat: add similar songs functionality in agents, and Instant Mix (song-based) to UI (#4919) * refactor: rename ArtistRadio to SimilarSongs for clarity and consistency Signed-off-by: Deluan * feat: implement GetSimilarSongsByTrack and related functionality for song similarity retrieval Signed-off-by: Deluan * feat: enhance GetSimilarSongsByTrack to include artist and album details and update tests Signed-off-by: Deluan * feat: enhance song matching by implementing title and artist filtering in loadTracksByTitleAndArtist Signed-off-by: Deluan * test: add unit tests for song matching functionality in provider Signed-off-by: Deluan * refactor: extract song matching functionality into its own file Signed-off-by: Deluan * docs: clarify similarSongsFallback function description in provider.go Signed-off-by: Deluan * refactor: initialize result slice for songs with capacity based on response length Signed-off-by: Deluan * refactor: simplify agent method calls for retrieving images and similar songs Signed-off-by: Deluan * refactor: simplify agent method calls for retrieving images and similar songs Signed-off-by: Deluan * refactor: remove outdated comments in GetSimilarSongs methods Signed-off-by: Deluan * fix: use composite key for song matches to handle duplicates by title and artist Signed-off-by: Deluan * refactor: consolidate expectations setup for similar songs tests Signed-off-by: Deluan * feat: add instant mix action to song context menu and update translations Signed-off-by: Deluan * fix(provider): handle unknown entity types in GetSimilarSongs Signed-off-by: Deluan * refactor: move playSimilar action to playbackActions and streamline song processing Signed-off-by: Deluan * format Signed-off-by: Deluan * feat: enhance instant mix functionality with loading notification and shuffle option Signed-off-by: Deluan * feat: implement fuzzy matching for similar songs based on configurable threshold Signed-off-by: Deluan * refactor: implement track matching with multiple specificity levels Signed-off-by: Deluan * refactor: enhance track matching by implementing unified scoring with specificity levels Signed-off-by: Deluan * feat: enhance deezer top tracks result with album Signed-off-by: Deluan * feat: enhance track matching with fuzzy album similarity for improved scoring Signed-off-by: Deluan * docs: document multi-phase song matching algorithm with detailed scoring and prioritization Signed-off-by: Deluan --------- Signed-off-by: Deluan --- adapters/deezer/client_test.go | 22 + adapters/deezer/deezer.go | 3 +- adapters/lastfm/agent.go | 29 ++ adapters/lastfm/agent_test.go | 48 ++ adapters/lastfm/client.go | 13 + adapters/lastfm/client_test.go | 24 + adapters/lastfm/responses.go | 23 + conf/configuration.go | 2 + core/agents/agents.go | 232 ++++----- core/agents/agents_test.go | 99 ++++ core/agents/interfaces.go | 45 +- core/external/extdata_helper_test.go | 24 + core/external/provider.go | 205 ++------ core/external/provider_artistradio_test.go | 205 -------- core/external/provider_matching.go | 388 +++++++++++++++ .../provider_matching_internal_test.go | 57 +++ core/external/provider_matching_test.go | 460 ++++++++++++++++++ core/external/provider_similarsongs_test.go | 443 +++++++++++++++++ core/external/provider_topsongs_test.go | 6 + resources/i18n/pt-br.json | 4 +- server/subsonic/browsing.go | 2 +- tests/fixtures/lastfm.track.getsimilar.json | 1 + .../lastfm.track.getsimilar.unknown.json | 1 + ui/src/artist/ArtistActions.jsx | 3 +- ui/src/artist/actions.js | 47 +- ui/src/common/SongContextMenu.jsx | 19 + ui/src/common/SongContextMenu.test.jsx | 122 ++++- ui/src/common/playbackActions.js | 76 +++ ui/src/i18n/en.json | 4 +- 29 files changed, 2082 insertions(+), 525 deletions(-) delete mode 100644 core/external/provider_artistradio_test.go create mode 100644 core/external/provider_matching.go create mode 100644 core/external/provider_matching_internal_test.go create mode 100644 core/external/provider_matching_test.go create mode 100644 core/external/provider_similarsongs_test.go create mode 100644 tests/fixtures/lastfm.track.getsimilar.json create mode 100644 tests/fixtures/lastfm.track.getsimilar.unknown.json create mode 100644 ui/src/common/playbackActions.js diff --git a/adapters/deezer/client_test.go b/adapters/deezer/client_test.go index 7e4f7a49..a0e0a5cb 100644 --- a/adapters/deezer/client_test.go +++ b/adapters/deezer/client_test.go @@ -45,6 +45,28 @@ var _ = Describe("client", func() { }) }) + Describe("TopTracks", func() { + It("returns top tracks with artist and album info from a successful request", func() { + f, err := os.Open("tests/fixtures/deezer.artist.top.json") + Expect(err).To(BeNil()) + httpClient.mock("https://api.deezer.com/artist/27/top", http.Response{Body: f, StatusCode: 200}) + + tracks, err := client.getTopTracks(GinkgoT().Context(), 27, 5) + Expect(err).To(BeNil()) + Expect(tracks).To(HaveLen(5)) + + // Verify first track has all expected fields + Expect(tracks[0].Title).To(Equal("Instant Crush (feat. Julian Casablancas)")) + Expect(tracks[0].Artist.Name).To(Equal("Daft Punk")) + Expect(tracks[0].Album.Title).To(Equal("Random Access Memories")) + + // Verify second track + Expect(tracks[1].Title).To(Equal("One More Time")) + Expect(tracks[1].Artist.Name).To(Equal("Daft Punk")) + Expect(tracks[1].Album.Title).To(Equal("Discovery")) + }) + }) + Describe("ArtistBio", func() { BeforeEach(func() { // Mock the JWT token endpoint with a valid JWT that expires in 5 minutes diff --git a/adapters/deezer/deezer.go b/adapters/deezer/deezer.go index 7ec48b38..7272bf9a 100644 --- a/adapters/deezer/deezer.go +++ b/adapters/deezer/deezer.go @@ -135,7 +135,8 @@ func (s *deezerAgent) GetArtistTopSongs(ctx context.Context, _, artistName, _ st res := slice.Map(tracks, func(r Track) agents.Song { return agents.Song{ - Name: r.Title, + Name: r.Title, + Album: r.Album.Title, } }) return res, nil diff --git a/adapters/lastfm/agent.go b/adapters/lastfm/agent.go index e3e53b23..3677be6e 100644 --- a/adapters/lastfm/agent.go +++ b/adapters/lastfm/agent.go @@ -192,6 +192,26 @@ func (l *lastfmAgent) GetArtistTopSongs(ctx context.Context, id, artistName, mbi return res, nil } +func (l *lastfmAgent) GetSimilarSongsByTrack(ctx context.Context, id, name, artist, mbid string, count int) ([]agents.Song, error) { + resp, err := l.callTrackGetSimilar(ctx, name, artist, count) + if err != nil { + return nil, err + } + if len(resp) == 0 { + return nil, agents.ErrNotFound + } + res := make([]agents.Song, 0, len(resp)) + for _, t := range resp { + res = append(res, agents.Song{ + Name: t.Name, + MBID: t.MBID, + Artist: t.Artist.Name, + ArtistMBID: t.Artist.MBID, + }) + } + return res, nil +} + var ( artistOpenGraphQuery = cascadia.MustCompile(`html > head > meta[property="og:image"]`) artistIgnoredImage = "2a96cbd8b46e442fc41c2b86b821562f" // Last.fm artist placeholder image name @@ -290,6 +310,15 @@ func (l *lastfmAgent) callArtistGetTopTracks(ctx context.Context, artistName str return t.Track, nil } +func (l *lastfmAgent) callTrackGetSimilar(ctx context.Context, name, artist string, count int) ([]SimilarTrack, error) { + s, err := l.client.trackGetSimilar(ctx, name, artist, count) + if err != nil { + log.Error(ctx, "Error calling LastFM/track.getSimilar", "track", name, "artist", artist, err) + return nil, err + } + return s.Track, nil +} + func (l *lastfmAgent) getArtistForScrobble(track *model.MediaFile, role model.Role, displayName string) string { if conf.Server.LastFM.ScrobbleFirstArtistOnly && len(track.Participants[role]) > 0 { return track.Participants[role][0].Name diff --git a/adapters/lastfm/agent_test.go b/adapters/lastfm/agent_test.go index fc623840..f2db44fa 100644 --- a/adapters/lastfm/agent_test.go +++ b/adapters/lastfm/agent_test.go @@ -177,6 +177,54 @@ var _ = Describe("lastfmAgent", func() { }) }) + Describe("GetSimilarSongsByTrack", func() { + var agent *lastfmAgent + var httpClient *tests.FakeHttpClient + BeforeEach(func() { + httpClient = &tests.FakeHttpClient{} + client := newClient("API_KEY", "SECRET", "pt", httpClient) + agent = lastFMConstructor(ds) + agent.client = client + }) + + It("returns similar songs", func() { + f, _ := os.Open("tests/fixtures/lastfm.track.getsimilar.json") + httpClient.Res = http.Response{Body: f, StatusCode: 200} + Expect(agent.GetSimilarSongsByTrack(ctx, "123", "Just Can't Get Enough", "Depeche Mode", "", 5)).To(Equal([]agents.Song{ + {Name: "Dreaming of Me", MBID: "027b553e-7c74-3ed4-a95e-1d4fea51f174", Artist: "Depeche Mode", ArtistMBID: "8538e728-ca0b-4321-b7e5-cff6565dd4c0"}, + {Name: "Everything Counts", MBID: "5a5a3ca4-bdb8-4641-a674-9b54b9b319a6", Artist: "Depeche Mode", ArtistMBID: "8538e728-ca0b-4321-b7e5-cff6565dd4c0"}, + {Name: "Don't You Want Me", MBID: "", Artist: "The Human League", ArtistMBID: "7adaabfb-acfb-47bc-8c7c-59471c2f0db8"}, + {Name: "Tainted Love", MBID: "", Artist: "Soft Cell", ArtistMBID: "7fb50287-029d-47cc-825a-235ca28024b2"}, + {Name: "Blue Monday", MBID: "727e84c6-1b56-31dd-a958-a5f46305cec0", Artist: "New Order", ArtistMBID: "f1106b17-dcbb-45f6-b938-199ccfab50cc"}, + })) + Expect(httpClient.RequestCount).To(Equal(1)) + Expect(httpClient.SavedRequest.URL.Query().Get("track")).To(Equal("Just Can't Get Enough")) + Expect(httpClient.SavedRequest.URL.Query().Get("artist")).To(Equal("Depeche Mode")) + }) + + It("returns ErrNotFound when no similar songs found", func() { + f, _ := os.Open("tests/fixtures/lastfm.track.getsimilar.unknown.json") + httpClient.Res = http.Response{Body: f, StatusCode: 200} + _, err := agent.GetSimilarSongsByTrack(ctx, "123", "UnknownTrack", "UnknownArtist", "", 3) + Expect(err).To(MatchError(agents.ErrNotFound)) + Expect(httpClient.RequestCount).To(Equal(1)) + }) + + It("returns an error if Last.fm call fails", func() { + httpClient.Err = errors.New("error") + _, err := agent.GetSimilarSongsByTrack(ctx, "123", "Believe", "Cher", "", 3) + Expect(err).To(HaveOccurred()) + Expect(httpClient.RequestCount).To(Equal(1)) + }) + + It("returns an error if Last.fm call returns an error", func() { + httpClient.Res = http.Response{Body: io.NopCloser(bytes.NewBufferString(lastfmError3)), StatusCode: 200} + _, err := agent.GetSimilarSongsByTrack(ctx, "123", "Believe", "Cher", "", 3) + Expect(err).To(HaveOccurred()) + Expect(httpClient.RequestCount).To(Equal(1)) + }) + }) + Describe("Scrobbling", func() { var agent *lastfmAgent var httpClient *tests.FakeHttpClient diff --git a/adapters/lastfm/client.go b/adapters/lastfm/client.go index 6a24ac80..f15613a1 100644 --- a/adapters/lastfm/client.go +++ b/adapters/lastfm/client.go @@ -95,6 +95,19 @@ func (c *client) artistGetTopTracks(ctx context.Context, name string, limit int) return &response.TopTracks, nil } +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("limit", strconv.Itoa(limit)) + response, err := c.makeRequest(ctx, http.MethodGet, params, false) + if err != nil { + return nil, err + } + return &response.SimilarTracks, nil +} + func (c *client) GetToken(ctx context.Context) (string, error) { params := url.Values{} params.Add("method", "auth.getToken") diff --git a/adapters/lastfm/client_test.go b/adapters/lastfm/client_test.go index 85ec1150..bd319747 100644 --- a/adapters/lastfm/client_test.go +++ b/adapters/lastfm/client_test.go @@ -121,6 +121,30 @@ var _ = Describe("client", func() { }) }) + Describe("trackGetSimilar", func() { + It("returns similar tracks for a successful response", func() { + f, _ := os.Open("tests/fixtures/lastfm.track.getsimilar.json") + httpClient.Res = http.Response{Body: f, StatusCode: 200} + + similar, err := client.trackGetSimilar(context.Background(), "Just Can't Get Enough", "Depeche Mode", 5) + Expect(err).To(BeNil()) + Expect(len(similar.Track)).To(Equal(5)) + Expect(similar.Track[0].Name).To(Equal("Dreaming of Me")) + Expect(similar.Track[0].Artist.Name).To(Equal("Depeche Mode")) + Expect(similar.Track[0].Match).To(Equal(1.0)) + 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("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} + + similar, err := client.trackGetSimilar(context.Background(), "UnknownTrack", "UnknownArtist", 3) + Expect(err).To(BeNil()) + Expect(similar.Track).To(BeEmpty()) + }) + }) + Describe("GetToken", func() { It("returns a token when the request is successful", func() { httpClient.Res = http.Response{ diff --git a/adapters/lastfm/responses.go b/adapters/lastfm/responses.go index 1ceebe76..02674167 100644 --- a/adapters/lastfm/responses.go +++ b/adapters/lastfm/responses.go @@ -5,6 +5,7 @@ type Response struct { SimilarArtists SimilarArtists `json:"similarartists"` TopTracks TopTracks `json:"toptracks"` Album Album `json:"album"` + SimilarTracks SimilarTracks `json:"similartracks"` Error int `json:"error"` Message string `json:"message"` Token string `json:"token"` @@ -59,6 +60,28 @@ type TopTracks struct { Attr Attr `json:"@attr"` } +type SimilarTracks struct { + Track []SimilarTrack `json:"track"` + Attr SimilarAttr `json:"@attr"` +} + +type SimilarTrack struct { + Name string `json:"name"` + MBID string `json:"mbid"` + Match float64 `json:"match"` + Artist SimilarTrackArtist `json:"artist"` +} + +type SimilarTrackArtist struct { + Name string `json:"name"` + MBID string `json:"mbid"` +} + +type SimilarAttr struct { + Artist string `json:"artist"` + Track string `json:"track"` +} + type Session struct { Name string `json:"name"` Key string `json:"key"` diff --git a/conf/configuration.go b/conf/configuration.go index 5d366039..6b735caf 100644 --- a/conf/configuration.go +++ b/conf/configuration.go @@ -57,6 +57,7 @@ type configOptions struct { AutoTranscodeDownload bool DefaultDownsamplingFormat string SearchFullString bool + SimilarSongsMatchThreshold int RecentlyAddedByModTime bool PreferSortTags bool IgnoredArticles string @@ -555,6 +556,7 @@ func setViperDefaults() { viper.SetDefault("autotranscodedownload", false) viper.SetDefault("defaultdownsamplingformat", consts.DefaultDownsamplingFormat) viper.SetDefault("searchfullstring", false) + viper.SetDefault("similarsongsmatchthreshold", 85) viper.SetDefault("recentlyaddedbymodtime", false) viper.SetDefault("prefersorttags", false) viper.SetDefault("ignoredarticles", "The El La Los Las Le Les Os As O A") diff --git a/core/agents/agents.go b/core/agents/agents.go index c82d77b1..ead6dacd 100644 --- a/core/agents/agents.go +++ b/core/agents/agents.go @@ -22,6 +22,8 @@ type PluginLoader interface { LoadMediaAgent(name string) (Interface, bool) } +// Agents is a meta-agent that aggregates multiple built-in and plugin agents. It tries each enabled agent in order +// until one returns valid data. type Agents struct { ds model.DataStore pluginLoader PluginLoader @@ -129,26 +131,14 @@ func (a *Agents) GetArtistMBID(ctx context.Context, id string, name string) (str case consts.VariousArtistsID: return "", nil } - start := time.Now() - for _, enabledAgent := range a.getEnabledAgentNames() { - ag := a.getAgent(enabledAgent) - if ag == nil { - continue - } - if utils.IsCtxDone(ctx) { - break - } + + return callAgentMethod(ctx, a, "GetArtistMBID", func(ag Interface) (string, error) { retriever, ok := ag.(ArtistMBIDRetriever) if !ok { - continue + return "", ErrNotFound } - mbid, err := retriever.GetArtistMBID(ctx, id, name) - if mbid != "" && err == nil { - log.Debug(ctx, "Got MBID", "agent", ag.AgentName(), "artist", name, "mbid", mbid, "elapsed", time.Since(start)) - return mbid, nil - } - } - return "", ErrNotFound + return retriever.GetArtistMBID(ctx, id, name) + }) } func (a *Agents) GetArtistURL(ctx context.Context, id, name, mbid string) (string, error) { @@ -158,26 +148,14 @@ func (a *Agents) GetArtistURL(ctx context.Context, id, name, mbid string) (strin case consts.VariousArtistsID: return "", nil } - start := time.Now() - for _, enabledAgent := range a.getEnabledAgentNames() { - ag := a.getAgent(enabledAgent) - if ag == nil { - continue - } - if utils.IsCtxDone(ctx) { - break - } + + return callAgentMethod(ctx, a, "GetArtistURL", func(ag Interface) (string, error) { retriever, ok := ag.(ArtistURLRetriever) if !ok { - continue + return "", ErrNotFound } - url, err := retriever.GetArtistURL(ctx, id, name, mbid) - if url != "" && err == nil { - log.Debug(ctx, "Got External Url", "agent", ag.AgentName(), "artist", name, "url", url, "elapsed", time.Since(start)) - return url, nil - } - } - return "", ErrNotFound + return retriever.GetArtistURL(ctx, id, name, mbid) + }) } func (a *Agents) GetArtistBiography(ctx context.Context, id, name, mbid string) (string, error) { @@ -187,26 +165,14 @@ func (a *Agents) GetArtistBiography(ctx context.Context, id, name, mbid string) case consts.VariousArtistsID: return "", nil } - start := time.Now() - for _, enabledAgent := range a.getEnabledAgentNames() { - ag := a.getAgent(enabledAgent) - if ag == nil { - continue - } - if utils.IsCtxDone(ctx) { - break - } + + return callAgentMethod(ctx, a, "GetArtistBiography", func(ag Interface) (string, error) { retriever, ok := ag.(ArtistBiographyRetriever) if !ok { - continue + return "", ErrNotFound } - bio, err := retriever.GetArtistBiography(ctx, id, name, mbid) - if err == nil { - log.Debug(ctx, "Got Biography", "agent", ag.AgentName(), "artist", name, "len", len(bio), "elapsed", time.Since(start)) - return bio, nil - } - } - return "", ErrNotFound + return retriever.GetArtistBiography(ctx, id, name, mbid) + }) } // GetSimilarArtists returns similar artists by id, name, and/or mbid. Because some artists returned from an enabled @@ -254,26 +220,14 @@ func (a *Agents) GetArtistImages(ctx context.Context, id, name, mbid string) ([] case consts.VariousArtistsID: return nil, nil } - start := time.Now() - for _, enabledAgent := range a.getEnabledAgentNames() { - ag := a.getAgent(enabledAgent) - if ag == nil { - continue - } - if utils.IsCtxDone(ctx) { - break - } + + return callAgentSliceMethod(ctx, a, "GetArtistImages", func(ag Interface) ([]ExternalImage, error) { retriever, ok := ag.(ArtistImageRetriever) if !ok { - continue + return nil, ErrNotFound } - images, err := retriever.GetArtistImages(ctx, id, name, mbid) - if len(images) > 0 && err == nil { - log.Debug(ctx, "Got Images", "agent", ag.AgentName(), "artist", name, "images", images, "elapsed", time.Since(start)) - return images, nil - } - } - return nil, ErrNotFound + return retriever.GetArtistImages(ctx, id, name, mbid) + }) } // GetArtistTopSongs returns top songs by id, name, and/or mbid. Because some songs returned from an enabled @@ -288,80 +242,127 @@ func (a *Agents) GetArtistTopSongs(ctx context.Context, id, artistName, mbid str overLimit := int(float64(count) * conf.Server.DevExternalArtistFetchMultiplier) - start := time.Now() - for _, enabledAgent := range a.getEnabledAgentNames() { - ag := a.getAgent(enabledAgent) - if ag == nil { - continue - } - if utils.IsCtxDone(ctx) { - break - } + return callAgentSliceMethod(ctx, a, "GetArtistTopSongs", func(ag Interface) ([]Song, error) { retriever, ok := ag.(ArtistTopSongsRetriever) if !ok { - continue + return nil, ErrNotFound } - songs, err := retriever.GetArtistTopSongs(ctx, id, artistName, mbid, overLimit) - if len(songs) > 0 && err == nil { - log.Debug(ctx, "Got Top Songs", "agent", ag.AgentName(), "artist", artistName, "songs", songs, "elapsed", time.Since(start)) - return songs, nil - } - } - return nil, ErrNotFound + return retriever.GetArtistTopSongs(ctx, id, artistName, mbid, overLimit) + }) } func (a *Agents) GetAlbumInfo(ctx context.Context, name, artist, mbid string) (*AlbumInfo, error) { if name == consts.UnknownAlbum { return nil, ErrNotFound } - start := time.Now() - for _, enabledAgent := range a.getEnabledAgentNames() { - ag := a.getAgent(enabledAgent) - if ag == nil { - continue - } - if utils.IsCtxDone(ctx) { - break - } + + return callAgentMethod(ctx, a, "GetAlbumInfo", func(ag Interface) (*AlbumInfo, error) { retriever, ok := ag.(AlbumInfoRetriever) if !ok { - continue + return nil, ErrNotFound } - album, err := retriever.GetAlbumInfo(ctx, name, artist, mbid) - if err == nil { - log.Debug(ctx, "Got Album Info", "agent", ag.AgentName(), "album", name, "artist", artist, - "mbid", mbid, "elapsed", time.Since(start)) - return album, nil - } - } - return nil, ErrNotFound + return retriever.GetAlbumInfo(ctx, name, artist, mbid) + }) } func (a *Agents) GetAlbumImages(ctx context.Context, name, artist, mbid string) ([]ExternalImage, error) { if name == consts.UnknownAlbum { return nil, ErrNotFound } + + return callAgentSliceMethod(ctx, a, "GetAlbumImages", func(ag Interface) ([]ExternalImage, error) { + retriever, ok := ag.(AlbumImageRetriever) + if !ok { + return nil, ErrNotFound + } + return retriever.GetAlbumImages(ctx, name, artist, mbid) + }) +} + +// GetSimilarSongsByTrack returns similar songs for a given track. +func (a *Agents) GetSimilarSongsByTrack(ctx context.Context, id, name, artist, mbid string, count int) ([]Song, error) { + return callAgentSliceMethod(ctx, a, "GetSimilarSongsByTrack", func(ag Interface) ([]Song, error) { + retriever, ok := ag.(SimilarSongsByTrackRetriever) + if !ok { + return nil, ErrNotFound + } + return retriever.GetSimilarSongsByTrack(ctx, id, name, artist, mbid, count) + }) +} + +// GetSimilarSongsByAlbum returns similar songs for a given album. +func (a *Agents) GetSimilarSongsByAlbum(ctx context.Context, id, name, artist, mbid string, count int) ([]Song, error) { + return callAgentSliceMethod(ctx, a, "GetSimilarSongsByAlbum", func(ag Interface) ([]Song, error) { + retriever, ok := ag.(SimilarSongsByAlbumRetriever) + if !ok { + return nil, ErrNotFound + } + return retriever.GetSimilarSongsByAlbum(ctx, id, name, artist, mbid, count) + }) +} + +// GetSimilarSongsByArtist returns similar songs for a given artist. +func (a *Agents) GetSimilarSongsByArtist(ctx context.Context, id, name, mbid string, count int) ([]Song, error) { + switch id { + case consts.UnknownArtistID: + return nil, ErrNotFound + case consts.VariousArtistsID: + return nil, nil + } + + return callAgentSliceMethod(ctx, a, "GetSimilarSongsByArtist", func(ag Interface) ([]Song, error) { + retriever, ok := ag.(SimilarSongsByArtistRetriever) + if !ok { + return nil, ErrNotFound + } + return retriever.GetSimilarSongsByArtist(ctx, id, name, mbid, count) + }) +} + +func callAgentMethod[T comparable](ctx context.Context, agents *Agents, methodName string, fn func(Interface) (T, error)) (T, error) { + var zero T start := time.Now() - for _, enabledAgent := range a.getEnabledAgentNames() { - ag := a.getAgent(enabledAgent) + for _, enabledAgent := range agents.getEnabledAgentNames() { + ag := agents.getAgent(enabledAgent) if ag == nil { continue } if utils.IsCtxDone(ctx) { break } - retriever, ok := ag.(AlbumImageRetriever) - if !ok { + result, err := fn(ag) + if err != nil { + log.Trace(ctx, "Agent method call error", "method", methodName, "agent", ag.AgentName(), "error", err) continue } - images, err := retriever.GetAlbumImages(ctx, name, artist, mbid) - if err != nil { - log.Trace(ctx, "Agent GetAlbumImages failed", "agent", ag.AgentName(), "album", name, "artist", artist, "mbid", mbid, err) + + if result != zero { + log.Debug(ctx, "Got result", "method", methodName, "agent", ag.AgentName(), "elapsed", time.Since(start)) + return result, nil } - if len(images) > 0 && err == nil { - log.Debug(ctx, "Got Album Images", "agent", ag.AgentName(), "album", name, "artist", artist, - "mbid", mbid, "elapsed", time.Since(start)) - return images, nil + } + return zero, ErrNotFound +} + +func callAgentSliceMethod[T any](ctx context.Context, agents *Agents, methodName string, fn func(Interface) ([]T, error)) ([]T, error) { + start := time.Now() + for _, enabledAgent := range agents.getEnabledAgentNames() { + ag := agents.getAgent(enabledAgent) + if ag == nil { + continue + } + if utils.IsCtxDone(ctx) { + break + } + results, err := fn(ag) + if err != nil { + log.Trace(ctx, "Agent method call error", "method", methodName, "agent", ag.AgentName(), "error", err) + continue + } + + if len(results) > 0 { + log.Debug(ctx, "Got results", "method", methodName, "agent", ag.AgentName(), "count", len(results), "elapsed", time.Since(start)) + return results, nil } } return nil, ErrNotFound @@ -376,3 +377,6 @@ var _ ArtistImageRetriever = (*Agents)(nil) var _ ArtistTopSongsRetriever = (*Agents)(nil) var _ AlbumInfoRetriever = (*Agents)(nil) var _ AlbumImageRetriever = (*Agents)(nil) +var _ SimilarSongsByTrackRetriever = (*Agents)(nil) +var _ SimilarSongsByAlbumRetriever = (*Agents)(nil) +var _ SimilarSongsByArtistRetriever = (*Agents)(nil) diff --git a/core/agents/agents_test.go b/core/agents/agents_test.go index 0b7eec28..1d5ae464 100644 --- a/core/agents/agents_test.go +++ b/core/agents/agents_test.go @@ -295,6 +295,72 @@ var _ = Describe("Agents", func() { Expect(mock.Args).To(BeEmpty()) }) }) + + Describe("GetSimilarSongsByTrack", func() { + It("returns on first match", func() { + Expect(ag.GetSimilarSongsByTrack(ctx, "123", "test song", "test artist", "mb123", 2)).To(Equal([]Song{{ + Name: "Similar Song", + MBID: "mbid555", + }})) + Expect(mock.Args).To(HaveExactElements("123", "test song", "test artist", "mb123", 2)) + }) + It("skips the agent if it returns an error", func() { + mock.Err = errors.New("error") + _, err := ag.GetSimilarSongsByTrack(ctx, "123", "test song", "test artist", "mb123", 2) + Expect(err).To(MatchError(ErrNotFound)) + Expect(mock.Args).To(HaveExactElements("123", "test song", "test artist", "mb123", 2)) + }) + It("interrupts if the context is canceled", func() { + cancel() + _, err := ag.GetSimilarSongsByTrack(ctx, "123", "test song", "test artist", "mb123", 2) + Expect(err).To(MatchError(ErrNotFound)) + Expect(mock.Args).To(BeEmpty()) + }) + }) + + Describe("GetSimilarSongsByAlbum", func() { + It("returns on first match", func() { + Expect(ag.GetSimilarSongsByAlbum(ctx, "123", "test album", "test artist", "mb123", 2)).To(Equal([]Song{{ + Name: "Album Similar Song", + MBID: "mbid666", + }})) + Expect(mock.Args).To(HaveExactElements("123", "test album", "test artist", "mb123", 2)) + }) + It("skips the agent if it returns an error", func() { + mock.Err = errors.New("error") + _, err := ag.GetSimilarSongsByAlbum(ctx, "123", "test album", "test artist", "mb123", 2) + Expect(err).To(MatchError(ErrNotFound)) + Expect(mock.Args).To(HaveExactElements("123", "test album", "test artist", "mb123", 2)) + }) + It("interrupts if the context is canceled", func() { + cancel() + _, err := ag.GetSimilarSongsByAlbum(ctx, "123", "test album", "test artist", "mb123", 2) + Expect(err).To(MatchError(ErrNotFound)) + Expect(mock.Args).To(BeEmpty()) + }) + }) + + Describe("GetSimilarSongsByArtist", func() { + It("returns on first match", func() { + Expect(ag.GetSimilarSongsByArtist(ctx, "123", "test artist", "mb123", 2)).To(Equal([]Song{{ + Name: "Artist Similar Song", + MBID: "mbid777", + }})) + Expect(mock.Args).To(HaveExactElements("123", "test artist", "mb123", 2)) + }) + It("skips the agent if it returns an error", func() { + mock.Err = errors.New("error") + _, err := ag.GetSimilarSongsByArtist(ctx, "123", "test artist", "mb123", 2) + Expect(err).To(MatchError(ErrNotFound)) + Expect(mock.Args).To(HaveExactElements("123", "test artist", "mb123", 2)) + }) + It("interrupts if the context is canceled", func() { + cancel() + _, err := ag.GetSimilarSongsByArtist(ctx, "123", "test artist", "mb123", 2) + Expect(err).To(MatchError(ErrNotFound)) + Expect(mock.Args).To(BeEmpty()) + }) + }) }) }) @@ -377,6 +443,39 @@ func (a *mockAgent) GetAlbumInfo(ctx context.Context, name, artist, mbid string) }, nil } +func (a *mockAgent) GetSimilarSongsByTrack(_ context.Context, id, name, artist, mbid string, count int) ([]Song, error) { + a.Args = []interface{}{id, name, artist, mbid, count} + if a.Err != nil { + return nil, a.Err + } + return []Song{{ + Name: "Similar Song", + MBID: "mbid555", + }}, nil +} + +func (a *mockAgent) GetSimilarSongsByAlbum(_ context.Context, id, name, artist, mbid string, count int) ([]Song, error) { + a.Args = []interface{}{id, name, artist, mbid, count} + if a.Err != nil { + return nil, a.Err + } + return []Song{{ + Name: "Album Similar Song", + MBID: "mbid666", + }}, nil +} + +func (a *mockAgent) GetSimilarSongsByArtist(_ context.Context, id, name, mbid string, count int) ([]Song, error) { + a.Args = []interface{}{id, name, mbid, count} + if a.Err != nil { + return nil, a.Err + } + return []Song{{ + Name: "Artist Similar Song", + MBID: "mbid777", + }}, nil +} + type emptyAgent struct { Interface } diff --git a/core/agents/interfaces.go b/core/agents/interfaces.go index 054a14c5..d0805818 100644 --- a/core/agents/interfaces.go +++ b/core/agents/interfaces.go @@ -33,9 +33,13 @@ type ExternalImage struct { } type Song struct { - ID string - Name string - MBID string + ID string + Name string + MBID string + Artist string + ArtistMBID string + Album string + AlbumMBID string } var ( @@ -76,6 +80,41 @@ type ArtistTopSongsRetriever interface { GetArtistTopSongs(ctx context.Context, id, artistName, mbid string, count int) ([]Song, error) } +// SimilarSongsByTrackRetriever provides similar songs based on a specific track +type SimilarSongsByTrackRetriever interface { + // GetSimilarSongsByTrack returns songs similar to the given track. + // Parameters: + // - id: local mediafile ID + // - name: track title + // - artist: artist name + // - mbid: MusicBrainz recording ID (may be empty) + // - count: maximum number of results + GetSimilarSongsByTrack(ctx context.Context, id, name, artist, mbid string, count int) ([]Song, error) +} + +// SimilarSongsByAlbumRetriever provides similar songs based on an album +type SimilarSongsByAlbumRetriever interface { + // GetSimilarSongsByAlbum returns songs similar to tracks on the given album. + // Parameters: + // - id: local album ID + // - name: album name + // - artist: album artist name + // - mbid: MusicBrainz release ID (may be empty) + // - count: maximum number of results + GetSimilarSongsByAlbum(ctx context.Context, id, name, artist, mbid string, count int) ([]Song, error) +} + +// SimilarSongsByArtistRetriever provides similar songs based on an artist +type SimilarSongsByArtistRetriever interface { + // GetSimilarSongsByArtist returns songs similar to the artist's catalog. + // Parameters: + // - id: local artist ID + // - name: artist name + // - mbid: MusicBrainz artist ID (may be empty) + // - count: maximum number of results + GetSimilarSongsByArtist(ctx context.Context, id, name, mbid string, count int) ([]Song, error) +} + var Map map[string]Constructor func Register(name string, init Constructor) { diff --git a/core/external/extdata_helper_test.go b/core/external/extdata_helper_test.go index 29975e5c..f1d92be1 100644 --- a/core/external/extdata_helper_test.go +++ b/core/external/extdata_helper_test.go @@ -282,3 +282,27 @@ func (m *mockAgents) GetAlbumImages(ctx context.Context, name, artist, mbid stri } return nil, args.Error(1) } + +func (m *mockAgents) GetSimilarSongsByTrack(ctx context.Context, id, name, artist, mbid string, count int) ([]agents.Song, error) { + args := m.Called(ctx, id, name, artist, mbid, count) + if args.Get(0) != nil { + return args.Get(0).([]agents.Song), args.Error(1) + } + return nil, args.Error(1) +} + +func (m *mockAgents) GetSimilarSongsByAlbum(ctx context.Context, id, name, artist, mbid string, count int) ([]agents.Song, error) { + args := m.Called(ctx, id, name, artist, mbid, count) + if args.Get(0) != nil { + return args.Get(0).([]agents.Song), args.Error(1) + } + return nil, args.Error(1) +} + +func (m *mockAgents) GetSimilarSongsByArtist(ctx context.Context, id, name, mbid string, count int) ([]agents.Song, error) { + args := m.Called(ctx, id, name, mbid, count) + if args.Get(0) != nil { + return args.Get(0).([]agents.Song), args.Error(1) + } + return nil, args.Error(1) +} diff --git a/core/external/provider.go b/core/external/provider.go index a6eb848a..13a4caf1 100644 --- a/core/external/provider.go +++ b/core/external/provider.go @@ -32,7 +32,7 @@ const ( type Provider interface { UpdateAlbumInfo(ctx context.Context, id string) (*model.Album, error) UpdateArtistInfo(ctx context.Context, id string, count int, includeNotPresent bool) (*model.Artist, error) - ArtistRadio(ctx context.Context, id string, count int) (model.MediaFiles, error) + SimilarSongs(ctx context.Context, id string, count int) (model.MediaFiles, error) TopSongs(ctx context.Context, artist string, count int) (model.MediaFiles, error) ArtistImage(ctx context.Context, id string) (*url.URL, error) AlbumImage(ctx context.Context, id string) (*url.URL, error) @@ -80,6 +80,9 @@ type Agents interface { agents.ArtistSimilarRetriever agents.ArtistTopSongsRetriever agents.ArtistURLRetriever + agents.SimilarSongsByTrackRetriever + agents.SimilarSongsByAlbumRetriever + agents.SimilarSongsByArtistRetriever } func NewProvider(ds model.DataStore, agents Agents) Provider { @@ -256,7 +259,7 @@ func (e *provider) populateArtistInfo(ctx context.Context, artist auxArtist) (au g.Go(func() error { e.callGetImage(ctx, e.ag, &artist); return nil }) g.Go(func() error { e.callGetBiography(ctx, e.ag, &artist); return nil }) g.Go(func() error { e.callGetURL(ctx, e.ag, &artist); return nil }) - g.Go(func() error { e.callGetSimilar(ctx, e.ag, &artist, maxSimilarArtists, true); return nil }) + g.Go(func() error { e.callGetSimilarArtists(ctx, e.ag, &artist, maxSimilarArtists, true); return nil }) _ = g.Wait() if utils.IsCtxDone(ctx) { @@ -275,22 +278,54 @@ func (e *provider) populateArtistInfo(ctx context.Context, artist auxArtist) (au return artist, nil } -func (e *provider) ArtistRadio(ctx context.Context, id string, count int) (model.MediaFiles, error) { +func (e *provider) SimilarSongs(ctx context.Context, id string, count int) (model.MediaFiles, error) { + entity, err := model.GetEntityByID(ctx, e.ds, id) + if err != nil { + return nil, err + } + + var songs []agents.Song + + // Try entity-specific similarity first + switch v := entity.(type) { + case *model.MediaFile: + songs, err = e.ag.GetSimilarSongsByTrack(ctx, v.ID, v.Title, v.Artist, v.MbzRecordingID, count) + case *model.Album: + songs, err = e.ag.GetSimilarSongsByAlbum(ctx, v.ID, v.Name, v.AlbumArtist, v.MbzAlbumID, count) + case *model.Artist: + songs, err = e.ag.GetSimilarSongsByArtist(ctx, v.ID, v.Name, v.MbzArtistID, count) + default: + log.Warn(ctx, "Unknown entity type", "id", id, "type", fmt.Sprintf("%T", entity)) + return nil, model.ErrNotFound + } + + if err == nil && len(songs) > 0 { + return e.matchSongsToLibrary(ctx, songs, count) + } + + // Fallback to existing similar artists + top songs algorithm + return e.similarSongsFallback(ctx, id, count) +} + +// similarSongsFallback uses the original similar artists + top songs algorithm. The idea is to +// get the artist of the given entity, retrieve similar artists, get their top songs, and pick +// a weighted random selection of songs to return as similar songs. +func (e *provider) similarSongsFallback(ctx context.Context, id string, count int) (model.MediaFiles, error) { artist, err := e.getArtist(ctx, id) if err != nil { return nil, err } - e.callGetSimilar(ctx, e.ag, &artist, 15, false) + e.callGetSimilarArtists(ctx, e.ag, &artist, 15, false) if utils.IsCtxDone(ctx) { - log.Warn(ctx, "ArtistRadio call canceled", ctx.Err()) + log.Warn(ctx, "SimilarSongs call canceled", ctx.Err()) return nil, ctx.Err() } weightedSongs := random.NewWeightedChooser[model.MediaFile]() addArtist := func(a model.Artist, weightedSongs *random.WeightedChooser[model.MediaFile], count, artistWeight int) error { if utils.IsCtxDone(ctx) { - log.Warn(ctx, "ArtistRadio call canceled", ctx.Err()) + log.Warn(ctx, "SimilarSongs call canceled", ctx.Err()) return ctx.Err() } @@ -422,21 +457,20 @@ func (e *provider) getMatchingTopSongs(ctx context.Context, agent agents.ArtistT return nil, fmt.Errorf("failed to get top songs for artist %s: %w", artistName, err) } - idMatches, err := e.loadTracksByID(ctx, songs) - if err != nil { - return nil, fmt.Errorf("failed to load tracks by ID: %w", err) - } - mbidMatches, err := e.loadTracksByMBID(ctx, songs) - if err != nil { - return nil, fmt.Errorf("failed to load tracks by MBID: %w", err) - } - titleMatches, err := e.loadTracksByTitle(ctx, songs, artist, idMatches, mbidMatches) - if err != nil { - return nil, fmt.Errorf("failed to load tracks by title: %w", err) + // Enrich songs with artist info if not already present (for top songs, we know the artist) + for i := range songs { + if songs[i].Artist == "" { + songs[i].Artist = artistName + } + if songs[i].ArtistMBID == "" { + songs[i].ArtistMBID = artist.MbzArtistID + } } - log.Trace(ctx, "Top Songs loaded", "name", artistName, "numSongs", len(songs), "numIDMatches", len(idMatches), "numMBIDMatches", len(mbidMatches), "numTitleMatches", len(titleMatches)) - mfs := e.selectTopSongs(songs, idMatches, mbidMatches, titleMatches, count) + mfs, err := e.matchSongsToLibrary(ctx, songs, count) + if err != nil { + return nil, err + } if len(mfs) == 0 { log.Debug(ctx, "No matching top songs found", "name", artistName) @@ -447,137 +481,6 @@ func (e *provider) getMatchingTopSongs(ctx context.Context, agent agents.ArtistT return mfs, nil } -func (e *provider) loadTracksByMBID(ctx context.Context, songs []agents.Song) (map[string]model.MediaFile, error) { - var mbids []string - for _, s := range songs { - if s.MBID != "" { - mbids = append(mbids, s.MBID) - } - } - matches := map[string]model.MediaFile{} - if len(mbids) == 0 { - return matches, nil - } - res, err := e.ds.MediaFile(ctx).GetAll(model.QueryOptions{ - Filters: squirrel.And{ - squirrel.Eq{"mbz_recording_id": mbids}, - squirrel.Eq{"missing": false}, - }, - }) - if err != nil { - return matches, err - } - for _, mf := range res { - if id := mf.MbzRecordingID; id != "" { - if _, ok := matches[id]; !ok { - matches[id] = mf - } - } - } - return matches, nil -} - -func (e *provider) loadTracksByID(ctx context.Context, songs []agents.Song) (map[string]model.MediaFile, error) { - var ids []string - for _, s := range songs { - if s.ID != "" { - ids = append(ids, s.ID) - } - } - matches := map[string]model.MediaFile{} - if len(ids) == 0 { - return matches, nil - } - res, err := e.ds.MediaFile(ctx).GetAll(model.QueryOptions{ - Filters: squirrel.And{ - squirrel.Eq{"media_file.id": ids}, - squirrel.Eq{"missing": false}, - }, - }) - if err != nil { - return matches, err - } - for _, mf := range res { - if _, ok := matches[mf.ID]; !ok { - matches[mf.ID] = mf - } - } - return matches, nil -} - -func (e *provider) loadTracksByTitle(ctx context.Context, songs []agents.Song, artist *auxArtist, idMatches, mbidMatches map[string]model.MediaFile) (map[string]model.MediaFile, error) { - titleMap := map[string]string{} - for _, s := range songs { - // Skip if already matched by ID or MBID - if s.ID != "" && idMatches[s.ID].ID != "" { - continue - } - if s.MBID != "" && mbidMatches[s.MBID].ID != "" { - continue - } - sanitized := str.SanitizeFieldForSorting(s.Name) - titleMap[sanitized] = s.Name - } - matches := map[string]model.MediaFile{} - if len(titleMap) == 0 { - return matches, nil - } - titleFilters := squirrel.Or{} - for sanitized := range titleMap { - titleFilters = append(titleFilters, squirrel.Like{"order_title": sanitized}) - } - - res, err := e.ds.MediaFile(ctx).GetAll(model.QueryOptions{ - Filters: squirrel.And{ - squirrel.Or{ - squirrel.Eq{"artist_id": artist.ID}, - squirrel.Eq{"album_artist_id": artist.ID}, - }, - titleFilters, - squirrel.Eq{"missing": false}, - }, - Sort: "starred desc, rating desc, year asc, compilation asc ", - }) - if err != nil { - return matches, err - } - for _, mf := range res { - sanitized := str.SanitizeFieldForSorting(mf.Title) - if _, ok := matches[sanitized]; !ok { - matches[sanitized] = mf - } - } - return matches, nil -} - -func (e *provider) selectTopSongs(songs []agents.Song, byID, byMBID, byTitle map[string]model.MediaFile, count int) model.MediaFiles { - var mfs model.MediaFiles - for _, t := range songs { - if len(mfs) == count { - break - } - // Try ID match first - if t.ID != "" { - if mf, ok := byID[t.ID]; ok { - mfs = append(mfs, mf) - continue - } - } - // Try MBID match second - if t.MBID != "" { - if mf, ok := byMBID[t.MBID]; ok { - mfs = append(mfs, mf) - continue - } - } - // Fall back to title match - if mf, ok := byTitle[str.SanitizeFieldForSorting(t.Name)]; ok { - mfs = append(mfs, mf) - } - } - return mfs -} - func (e *provider) callGetURL(ctx context.Context, agent agents.ArtistURLRetriever, artist *auxArtist) { artisURL, err := agent.GetArtistURL(ctx, artist.ID, artist.Name(), artist.MbzArtistID) if err != nil { @@ -614,7 +517,7 @@ func (e *provider) callGetImage(ctx context.Context, agent agents.ArtistImageRet } } -func (e *provider) callGetSimilar(ctx context.Context, agent agents.ArtistSimilarRetriever, artist *auxArtist, +func (e *provider) callGetSimilarArtists(ctx context.Context, agent agents.ArtistSimilarRetriever, artist *auxArtist, limit int, includeNotPresent bool) { artistName := artist.Name() similar, err := agent.GetSimilarArtists(ctx, artist.ID, artistName, artist.MbzArtistID, limit) diff --git a/core/external/provider_artistradio_test.go b/core/external/provider_artistradio_test.go deleted file mode 100644 index 18afede6..00000000 --- a/core/external/provider_artistradio_test.go +++ /dev/null @@ -1,205 +0,0 @@ -package external_test - -import ( - "context" - "errors" - - "github.com/Masterminds/squirrel" - "github.com/navidrome/navidrome/core/agents" - . "github.com/navidrome/navidrome/core/external" - "github.com/navidrome/navidrome/model" - "github.com/navidrome/navidrome/tests" - . "github.com/onsi/ginkgo/v2" - . "github.com/onsi/gomega" - "github.com/stretchr/testify/mock" -) - -var _ = Describe("Provider - ArtistRadio", func() { - var ds model.DataStore - var provider Provider - var mockAgent *mockSimilarArtistAgent - var mockTopAgent agents.ArtistTopSongsRetriever - var mockSimilarAgent agents.ArtistSimilarRetriever - var agentsCombined Agents - var artistRepo *mockArtistRepo - var mediaFileRepo *mockMediaFileRepo - var ctx context.Context - - BeforeEach(func() { - ctx = GinkgoT().Context() - - artistRepo = newMockArtistRepo() - mediaFileRepo = newMockMediaFileRepo() - - ds = &tests.MockDataStore{ - MockedArtist: artistRepo, - MockedMediaFile: mediaFileRepo, - } - - mockAgent = &mockSimilarArtistAgent{} - mockTopAgent = mockAgent - mockSimilarAgent = mockAgent - - agentsCombined = &mockAgents{ - topSongsAgent: mockTopAgent, - similarAgent: mockSimilarAgent, - } - - provider = NewProvider(ds, agentsCombined) - }) - - It("returns similar songs from main artist and similar artists", func() { - artist1 := model.Artist{ID: "artist-1", Name: "Artist One"} - similarArtist := model.Artist{ID: "artist-3", Name: "Similar Artist"} - song1 := model.MediaFile{ID: "song-1", Title: "Song One", ArtistID: "artist-1", MbzRecordingID: "mbid-1"} - song2 := model.MediaFile{ID: "song-2", Title: "Song Two", ArtistID: "artist-1", MbzRecordingID: "mbid-2"} - song3 := model.MediaFile{ID: "song-3", Title: "Song Three", ArtistID: "artist-3", MbzRecordingID: "mbid-3"} - - artistRepo.On("Get", "artist-1").Return(&artist1, nil).Maybe() - artistRepo.On("Get", "artist-3").Return(&similarArtist, nil).Maybe() - - artistRepo.On("GetAll", mock.MatchedBy(func(opt model.QueryOptions) bool { - return opt.Max == 1 && opt.Filters != nil - })).Return(model.Artists{artist1}, nil).Once() - - similarAgentsResp := []agents.Artist{ - {Name: "Similar Artist", MBID: "similar-mbid"}, - } - mockAgent.On("GetSimilarArtists", mock.Anything, "artist-1", "Artist One", "", 15). - Return(similarAgentsResp, nil).Once() - - // Mock the three-phase artist lookup: ID (skipped - no IDs), MBID, then Name - // MBID lookup returns empty (no match) - artistRepo.On("GetAll", mock.MatchedBy(func(opt model.QueryOptions) bool { - _, ok := opt.Filters.(squirrel.Eq) - return opt.Max == 0 && ok - })).Return(model.Artists{}, nil).Once() - // Name lookup returns the similar artist - artistRepo.On("GetAll", mock.MatchedBy(func(opt model.QueryOptions) bool { - _, ok := opt.Filters.(squirrel.Or) - return opt.Max == 0 && ok - })).Return(model.Artists{similarArtist}, nil).Once() - - mockAgent.On("GetArtistTopSongs", mock.Anything, "artist-1", "Artist One", "", mock.Anything). - Return([]agents.Song{ - {Name: "Song One", MBID: "mbid-1"}, - {Name: "Song Two", MBID: "mbid-2"}, - }, nil).Once() - - mockAgent.On("GetArtistTopSongs", mock.Anything, "artist-3", "Similar Artist", "", mock.Anything). - Return([]agents.Song{ - {Name: "Song Three", MBID: "mbid-3"}, - }, nil).Once() - - mediaFileRepo.On("GetAll", mock.AnythingOfType("model.QueryOptions")).Return(model.MediaFiles{song1, song2}, nil).Once() - mediaFileRepo.On("GetAll", mock.AnythingOfType("model.QueryOptions")).Return(model.MediaFiles{song3}, nil).Once() - - songs, err := provider.ArtistRadio(ctx, "artist-1", 3) - - Expect(err).ToNot(HaveOccurred()) - Expect(songs).To(HaveLen(3)) - for _, song := range songs { - Expect(song.ID).To(BeElementOf("song-1", "song-2", "song-3")) - } - }) - - It("returns ErrNotFound when artist is not found", func() { - artistRepo.On("Get", "artist-unknown-artist").Return(nil, model.ErrNotFound) - mediaFileRepo.On("Get", "artist-unknown-artist").Return(nil, model.ErrNotFound) - - artistRepo.On("GetAll", mock.MatchedBy(func(opt model.QueryOptions) bool { - return opt.Max == 1 && opt.Filters != nil - })).Return(model.Artists{}, nil).Maybe() - - songs, err := provider.ArtistRadio(ctx, "artist-unknown-artist", 5) - - Expect(err).To(Equal(model.ErrNotFound)) - Expect(songs).To(BeNil()) - }) - - It("returns songs from main artist when GetSimilarArtists returns error", func() { - artist1 := model.Artist{ID: "artist-1", Name: "Artist One"} - song1 := model.MediaFile{ID: "song-1", Title: "Song One", ArtistID: "artist-1", MbzRecordingID: "mbid-1"} - - artistRepo.On("Get", "artist-1").Return(&artist1, nil).Maybe() - artistRepo.On("GetAll", mock.MatchedBy(func(opt model.QueryOptions) bool { - return opt.Max == 1 && opt.Filters != nil - })).Return(model.Artists{artist1}, nil).Maybe() - - mockAgent.On("GetSimilarArtists", mock.Anything, "artist-1", "Artist One", "", 15). - Return(nil, errors.New("error getting similar artists")).Once() - - artistRepo.On("GetAll", mock.MatchedBy(func(opt model.QueryOptions) bool { - return opt.Max == 0 && opt.Filters != nil - })).Return(model.Artists{}, nil).Once() - - mockAgent.On("GetArtistTopSongs", mock.Anything, "artist-1", "Artist One", "", mock.Anything). - Return([]agents.Song{ - {Name: "Song One", MBID: "mbid-1"}, - }, nil).Once() - - mediaFileRepo.On("GetAll", mock.AnythingOfType("model.QueryOptions")).Return(model.MediaFiles{song1}, nil).Once() - - songs, err := provider.ArtistRadio(ctx, "artist-1", 5) - - Expect(err).ToNot(HaveOccurred()) - Expect(songs).To(HaveLen(1)) - Expect(songs[0].ID).To(Equal("song-1")) - }) - - It("returns empty list when GetArtistTopSongs returns error", func() { - artist1 := model.Artist{ID: "artist-1", Name: "Artist One"} - - artistRepo.On("Get", "artist-1").Return(&artist1, nil).Maybe() - artistRepo.On("GetAll", mock.MatchedBy(func(opt model.QueryOptions) bool { - return opt.Max == 1 && opt.Filters != nil - })).Return(model.Artists{artist1}, nil).Maybe() - - mockAgent.On("GetSimilarArtists", mock.Anything, "artist-1", "Artist One", "", 15). - Return([]agents.Artist{}, nil).Once() - - artistRepo.On("GetAll", mock.MatchedBy(func(opt model.QueryOptions) bool { - return opt.Max == 0 && opt.Filters != nil - })).Return(model.Artists{}, nil).Once() - - mockAgent.On("GetArtistTopSongs", mock.Anything, "artist-1", "Artist One", "", mock.Anything). - Return(nil, errors.New("error getting top songs")).Once() - - songs, err := provider.ArtistRadio(ctx, "artist-1", 5) - - Expect(err).ToNot(HaveOccurred()) - Expect(songs).To(BeEmpty()) - }) - - It("respects count parameter", func() { - artist1 := model.Artist{ID: "artist-1", Name: "Artist One"} - song1 := model.MediaFile{ID: "song-1", Title: "Song One", ArtistID: "artist-1", MbzRecordingID: "mbid-1"} - song2 := model.MediaFile{ID: "song-2", Title: "Song Two", ArtistID: "artist-1", MbzRecordingID: "mbid-2"} - - artistRepo.On("Get", "artist-1").Return(&artist1, nil).Maybe() - artistRepo.On("GetAll", mock.MatchedBy(func(opt model.QueryOptions) bool { - return opt.Max == 1 && opt.Filters != nil - })).Return(model.Artists{artist1}, nil).Maybe() - - mockAgent.On("GetSimilarArtists", mock.Anything, "artist-1", "Artist One", "", 15). - Return([]agents.Artist{}, nil).Once() - - artistRepo.On("GetAll", mock.MatchedBy(func(opt model.QueryOptions) bool { - return opt.Max == 0 && opt.Filters != nil - })).Return(model.Artists{}, nil).Once() - - mockAgent.On("GetArtistTopSongs", mock.Anything, "artist-1", "Artist One", "", mock.Anything). - Return([]agents.Song{ - {Name: "Song One", MBID: "mbid-1"}, - {Name: "Song Two", MBID: "mbid-2"}, - }, nil).Once() - - mediaFileRepo.On("GetAll", mock.AnythingOfType("model.QueryOptions")).Return(model.MediaFiles{song1, song2}, nil).Once() - - songs, err := provider.ArtistRadio(ctx, "artist-1", 1) - - Expect(err).ToNot(HaveOccurred()) - Expect(songs).To(HaveLen(1)) - Expect(songs[0].ID).To(BeElementOf("song-1", "song-2")) - }) -}) diff --git a/core/external/provider_matching.go b/core/external/provider_matching.go new file mode 100644 index 00000000..6bf393f7 --- /dev/null +++ b/core/external/provider_matching.go @@ -0,0 +1,388 @@ +package external + +import ( + "context" + "fmt" + + "github.com/Masterminds/squirrel" + "github.com/navidrome/navidrome/conf" + "github.com/navidrome/navidrome/core/agents" + "github.com/navidrome/navidrome/model" + "github.com/navidrome/navidrome/utils/str" + "github.com/xrash/smetrics" +) + +// matchSongsToLibrary matches agent song results to local library tracks using a multi-phase +// matching algorithm that prioritizes accuracy over recall. +// +// # Algorithm Overview +// +// The algorithm matches songs from external agents (Last.fm, Deezer, etc.) to tracks in the +// local music library using three matching strategies in priority order: +// +// 1. Direct ID match: Songs with an ID field are matched directly to MediaFiles by ID +// 2. MusicBrainz Recording ID (MBID) match: Songs with MBID are matched to tracks with +// matching mbz_recording_id +// 3. Title+Artist fuzzy match: Remaining songs are matched using fuzzy string comparison +// with metadata specificity scoring +// +// # Matching Priority +// +// When selecting the final result, matches are prioritized in order: ID > MBID > Title+Artist. +// This ensures that more reliable identifiers take precedence over fuzzy text matching. +// +// # Fuzzy Matching Details +// +// For title+artist matching, the algorithm uses Jaro-Winkler similarity (threshold configurable +// via SimilarSongsMatchThreshold, default 85%). Matches are ranked by: +// +// 1. Title similarity (Jaro-Winkler score, 0.0-1.0) +// 2. Specificity level (0-5, based on metadata precision): +// - Level 5: Title + Artist MBID + Album MBID (most specific) +// - Level 4: Title + Artist MBID + Album name (fuzzy) +// - Level 3: Title + Artist name + Album name (fuzzy) +// - Level 2: Title + Artist MBID +// - Level 1: Title + Artist name +// - Level 0: Title only +// 3. Album similarity (Jaro-Winkler, as final tiebreaker) +// +// # Examples +// +// Example 1 - MBID Priority: +// +// Agent returns: {Name: "Paranoid Android", MBID: "abc-123", Artist: "Radiohead"} +// Library has: [ +// {ID: "t1", Title: "Paranoid Android", MbzRecordingID: "abc-123"}, +// {ID: "t2", Title: "Paranoid Android", Artist: "Radiohead"}, +// ] +// Result: t1 (MBID match takes priority over title+artist) +// +// Example 2 - Specificity Ranking: +// +// Agent returns: {Name: "Enjoy the Silence", Artist: "Depeche Mode", Album: "Violator"} +// Library has: [ +// {ID: "t1", Title: "Enjoy the Silence", Artist: "Depeche Mode", Album: "101"}, // Level 1 +// {ID: "t2", Title: "Enjoy the Silence", Artist: "Depeche Mode", Album: "Violator"}, // Level 3 +// ] +// Result: t2 (Level 3 beats Level 1 due to album match) +// +// Example 3 - Fuzzy Title Matching: +// +// Agent returns: {Name: "Bohemian Rhapsody", Artist: "Queen"} +// Library has: {ID: "t1", Title: "Bohemian Rhapsody - Remastered", Artist: "Queen"} +// With threshold=85%: Match succeeds (similarity ~0.87) +// With threshold=100%: No match (not exact) +// +// # Parameters +// +// - ctx: Context for database operations +// - songs: Slice of agent.Song results from external providers +// - count: Maximum number of matches to return +// +// # Returns +// +// Returns up to 'count' MediaFiles from the library that best match the input songs, +// preserving the original order from the agent. Songs that cannot be matched are skipped. +func (e *provider) matchSongsToLibrary(ctx context.Context, songs []agents.Song, count int) (model.MediaFiles, error) { + idMatches, err := e.loadTracksByID(ctx, songs) + if err != nil { + return nil, fmt.Errorf("failed to load tracks by ID: %w", err) + } + mbidMatches, err := e.loadTracksByMBID(ctx, songs) + if err != nil { + return nil, fmt.Errorf("failed to load tracks by MBID: %w", err) + } + titleMatches, err := e.loadTracksByTitleAndArtist(ctx, songs, idMatches, mbidMatches) + if err != nil { + return nil, fmt.Errorf("failed to load tracks by title: %w", err) + } + + return e.selectBestMatchingSongs(songs, idMatches, mbidMatches, titleMatches, count), nil +} + +// loadTracksByID fetches MediaFiles from the library using direct ID matching. +// It extracts all non-empty ID fields from the input songs and performs a single +// batch query to the database. Returns a map keyed by MediaFile ID for O(1) lookup. +// Only non-missing files are returned. +func (e *provider) loadTracksByID(ctx context.Context, songs []agents.Song) (map[string]model.MediaFile, error) { + var ids []string + for _, s := range songs { + if s.ID != "" { + ids = append(ids, s.ID) + } + } + matches := map[string]model.MediaFile{} + if len(ids) == 0 { + return matches, nil + } + res, err := e.ds.MediaFile(ctx).GetAll(model.QueryOptions{ + Filters: squirrel.And{ + squirrel.Eq{"media_file.id": ids}, + squirrel.Eq{"missing": false}, + }, + }) + if err != nil { + return matches, err + } + for _, mf := range res { + if _, ok := matches[mf.ID]; !ok { + matches[mf.ID] = mf + } + } + return matches, nil +} + +// loadTracksByMBID fetches MediaFiles from the library using MusicBrainz Recording IDs. +// It extracts all non-empty MBID fields from the input songs and performs a single +// batch query against the mbz_recording_id column. Returns a map keyed by MBID for +// O(1) lookup. Only non-missing files are returned. +func (e *provider) loadTracksByMBID(ctx context.Context, songs []agents.Song) (map[string]model.MediaFile, error) { + var mbids []string + for _, s := range songs { + if s.MBID != "" { + mbids = append(mbids, s.MBID) + } + } + matches := map[string]model.MediaFile{} + if len(mbids) == 0 { + return matches, nil + } + res, err := e.ds.MediaFile(ctx).GetAll(model.QueryOptions{ + Filters: squirrel.And{ + squirrel.Eq{"mbz_recording_id": mbids}, + squirrel.Eq{"missing": false}, + }, + }) + if err != nil { + return matches, err + } + for _, mf := range res { + if id := mf.MbzRecordingID; id != "" { + if _, ok := matches[id]; !ok { + matches[id] = mf + } + } + } + return matches, nil +} + +// songQuery represents a normalized query for matching a song to library tracks. +// All string fields are sanitized (lowercased, diacritics removed) for comparison. +// This struct is used internally by loadTracksByTitleAndArtist to group queries by artist. +type songQuery struct { + title string // Sanitized song title + artist string // Sanitized artist name (without articles like "The") + artistMBID string // MusicBrainz Artist ID (optional, for higher specificity matching) + album string // Sanitized album name (optional, for specificity scoring) + albumMBID string // MusicBrainz Album ID (optional, for highest specificity matching) +} + +// matchScore combines title/album similarity with metadata specificity for ranking matches +type matchScore struct { + titleSimilarity float64 // 0.0-1.0 (Jaro-Winkler) + albumSimilarity float64 // 0.0-1.0 (Jaro-Winkler), used as tiebreaker + specificityLevel int // 0-5 (higher = more specific metadata match) +} + +// betterThan returns true if this score beats another. +// Comparison order: title similarity > specificity level > album similarity +func (s matchScore) betterThan(other matchScore) bool { + if s.titleSimilarity != other.titleSimilarity { + return s.titleSimilarity > other.titleSimilarity + } + if s.specificityLevel != other.specificityLevel { + return s.specificityLevel > other.specificityLevel + } + return s.albumSimilarity > other.albumSimilarity +} + +// computeSpecificityLevel determines how well query metadata matches a track (0-5). +// Higher values indicate more specific matches (MBIDs > names > title only). +// Uses fuzzy matching for album names with the same threshold as title matching. +func computeSpecificityLevel(q songQuery, mf model.MediaFile, albumThreshold float64) int { + title := str.SanitizeFieldForSorting(mf.Title) + artist := str.SanitizeFieldForSortingNoArticle(mf.Artist) + album := str.SanitizeFieldForSorting(mf.Album) + + // Level 5: Title + Artist MBID + Album MBID (most specific) + if q.artistMBID != "" && q.albumMBID != "" && + mf.MbzArtistID == q.artistMBID && mf.MbzAlbumID == q.albumMBID { + return 5 + } + // Level 4: Title + Artist MBID + Album name (fuzzy) + if q.artistMBID != "" && q.album != "" && + mf.MbzArtistID == q.artistMBID && similarityRatio(album, q.album) >= albumThreshold { + return 4 + } + // Level 3: Title + Artist name + Album name (fuzzy) + if q.artist != "" && q.album != "" && + artist == q.artist && similarityRatio(album, q.album) >= albumThreshold { + return 3 + } + // Level 2: Title + Artist MBID + if q.artistMBID != "" && mf.MbzArtistID == q.artistMBID { + return 2 + } + // Level 1: Title + Artist name + if q.artist != "" && artist == q.artist { + return 1 + } + // Level 0: Title only match (but for fuzzy, title matched via similarity) + // Check if at least the title matches exactly + if title == q.title { + return 0 + } + return -1 // No exact title match, but could still be a fuzzy match +} + +// loadTracksByTitleAndArtist loads tracks matching by title with optional artist/album filtering. +// Uses a unified scoring approach that combines title similarity (Jaro-Winkler) with +// metadata specificity (MBIDs, album names) for both exact and fuzzy matches. +// Returns a map keyed by "title|artist" for compatibility with selectBestMatchingSongs. +func (e *provider) loadTracksByTitleAndArtist(ctx context.Context, songs []agents.Song, idMatches, mbidMatches map[string]model.MediaFile) (map[string]model.MediaFile, error) { + queries := e.buildTitleQueries(songs, idMatches, mbidMatches) + if len(queries) == 0 { + return map[string]model.MediaFile{}, nil + } + + threshold := float64(conf.Server.SimilarSongsMatchThreshold) / 100.0 + + // Group queries by artist for efficient DB access + byArtist := map[string][]songQuery{} + for _, q := range queries { + if q.artist != "" { + byArtist[q.artist] = append(byArtist[q.artist], q) + } + } + + matches := map[string]model.MediaFile{} + for artist, artistQueries := range byArtist { + // Single DB query per artist - get all their tracks + tracks, err := e.ds.MediaFile(ctx).GetAll(model.QueryOptions{ + Filters: squirrel.And{ + squirrel.Eq{"order_artist_name": artist}, + squirrel.Eq{"missing": false}, + }, + Sort: "starred desc, rating desc, year asc, compilation asc", + }) + if err != nil { + continue + } + + // Find best match for each query using unified scoring + for _, q := range artistQueries { + if mf, found := e.findBestMatch(q, tracks, threshold); found { + key := q.title + "|" + q.artist + if _, exists := matches[key]; !exists { + matches[key] = mf + } + } + } + } + return matches, nil +} + +// findBestMatch finds the best matching track using combined title/album similarity and specificity scoring. +// A track must meet the threshold for title similarity, then the best match is chosen by: +// 1. Highest title similarity +// 2. Highest specificity level +// 3. Highest album similarity (as final tiebreaker) +func (e *provider) findBestMatch(q songQuery, tracks model.MediaFiles, threshold float64) (model.MediaFile, bool) { + var bestMatch model.MediaFile + bestScore := matchScore{titleSimilarity: -1} + found := false + + for _, mf := range tracks { + trackTitle := str.SanitizeFieldForSorting(mf.Title) + titleSim := similarityRatio(q.title, trackTitle) + + if titleSim < threshold { + continue + } + + // Compute album similarity for tiebreaking (0.0 if no album in query) + var albumSim float64 + if q.album != "" { + trackAlbum := str.SanitizeFieldForSorting(mf.Album) + albumSim = similarityRatio(q.album, trackAlbum) + } + + score := matchScore{ + titleSimilarity: titleSim, + albumSimilarity: albumSim, + specificityLevel: computeSpecificityLevel(q, mf, threshold), + } + + if score.betterThan(bestScore) { + bestScore = score + bestMatch = mf + found = true + } + } + return bestMatch, found +} + +func (e *provider) buildTitleQueries(songs []agents.Song, idMatches, mbidMatches map[string]model.MediaFile) []songQuery { + var queries []songQuery + for _, s := range songs { + // Skip if already matched by ID or MBID + if s.ID != "" && idMatches[s.ID].ID != "" { + continue + } + if s.MBID != "" && mbidMatches[s.MBID].ID != "" { + continue + } + queries = append(queries, songQuery{ + title: str.SanitizeFieldForSorting(s.Name), + artist: str.SanitizeFieldForSortingNoArticle(s.Artist), + artistMBID: s.ArtistMBID, + album: str.SanitizeFieldForSorting(s.Album), + albumMBID: s.AlbumMBID, + }) + } + return queries +} + +func (e *provider) selectBestMatchingSongs(songs []agents.Song, byID, byMBID, byTitleArtist map[string]model.MediaFile, count int) model.MediaFiles { + var mfs model.MediaFiles + for _, t := range songs { + if len(mfs) == count { + break + } + // Try ID match first + if t.ID != "" { + if mf, ok := byID[t.ID]; ok { + mfs = append(mfs, mf) + continue + } + } + // Try MBID match second + if t.MBID != "" { + if mf, ok := byMBID[t.MBID]; ok { + mfs = append(mfs, mf) + continue + } + } + // Fall back to title+artist match (composite key preserves duplicate titles) + key := str.SanitizeFieldForSorting(t.Name) + "|" + str.SanitizeFieldForSortingNoArticle(t.Artist) + if mf, ok := byTitleArtist[key]; ok { + mfs = append(mfs, mf) + } + } + return mfs +} + +// similarityRatio calculates the similarity between two strings using Jaro-Winkler algorithm. +// Returns a value between 0.0 (completely different) and 1.0 (identical). +// Jaro-Winkler is well-suited for matching song titles because it gives higher scores +// when strings share a common prefix (e.g., "Song Title" vs "Song Title - Remastered"). +func similarityRatio(a, b string) float64 { + if a == b { + return 1.0 + } + if len(a) == 0 || len(b) == 0 { + return 0.0 + } + // JaroWinkler params: boostThreshold=0.7, prefixSize=4 + return smetrics.JaroWinkler(a, b, 0.7, 4) +} diff --git a/core/external/provider_matching_internal_test.go b/core/external/provider_matching_internal_test.go new file mode 100644 index 00000000..5b9ccea3 --- /dev/null +++ b/core/external/provider_matching_internal_test.go @@ -0,0 +1,57 @@ +package external + +import ( + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" +) + +var _ = Describe("similarityRatio", func() { + It("returns 1.0 for identical strings", func() { + Expect(similarityRatio("hello", "hello")).To(BeNumerically("==", 1.0)) + }) + + It("returns 0.0 for empty strings", func() { + Expect(similarityRatio("", "test")).To(BeNumerically("==", 0.0)) + Expect(similarityRatio("test", "")).To(BeNumerically("==", 0.0)) + }) + + It("returns high similarity for remastered suffix", func() { + // Jaro-Winkler gives ~0.92 for this case + ratio := similarityRatio("paranoid android", "paranoid android remastered") + Expect(ratio).To(BeNumerically(">=", 0.85)) + }) + + It("returns high similarity for suffix additions like (Live)", func() { + // Jaro-Winkler gives ~0.96 for this case + ratio := similarityRatio("bohemian rhapsody", "bohemian rhapsody live") + Expect(ratio).To(BeNumerically(">=", 0.90)) + }) + + It("returns high similarity for 'yesterday' variants (common prefix)", func() { + // Jaro-Winkler gives ~0.90 because of common prefix + ratio := similarityRatio("yesterday", "yesterday once more") + Expect(ratio).To(BeNumerically(">=", 0.85)) + }) + + It("returns low similarity for same suffix", func() { + // Jaro-Winkler gives ~0.70 for this case + ratio := similarityRatio("postman (live)", "taxman (live)") + Expect(ratio).To(BeNumerically("<", 0.85)) + }) + + It("handles unicode characters", func() { + ratio := similarityRatio("dont stop believin", "don't stop believin'") + Expect(ratio).To(BeNumerically(">=", 0.85)) + }) + + It("returns low similarity for completely different strings", func() { + ratio := similarityRatio("abc", "xyz") + Expect(ratio).To(BeNumerically("<", 0.5)) + }) + + It("is symmetric", func() { + ratio1 := similarityRatio("hello world", "hello") + ratio2 := similarityRatio("hello", "hello world") + Expect(ratio1).To(Equal(ratio2)) + }) +}) diff --git a/core/external/provider_matching_test.go b/core/external/provider_matching_test.go new file mode 100644 index 00000000..cb6b33e6 --- /dev/null +++ b/core/external/provider_matching_test.go @@ -0,0 +1,460 @@ +package external_test + +import ( + "context" + + "github.com/Masterminds/squirrel" + "github.com/navidrome/navidrome/conf" + "github.com/navidrome/navidrome/conf/configtest" + "github.com/navidrome/navidrome/core/agents" + . "github.com/navidrome/navidrome/core/external" + "github.com/navidrome/navidrome/model" + "github.com/navidrome/navidrome/tests" + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + "github.com/stretchr/testify/mock" +) + +var _ = Describe("Provider - Song Matching", func() { + var ds model.DataStore + var provider Provider + var agentsCombined *mockAgents + var artistRepo *mockArtistRepo + var mediaFileRepo *mockMediaFileRepo + var albumRepo *mockAlbumRepo + var ctx context.Context + + BeforeEach(func() { + ctx = GinkgoT().Context() + + artistRepo = newMockArtistRepo() + mediaFileRepo = newMockMediaFileRepo() + albumRepo = newMockAlbumRepo() + + ds = &tests.MockDataStore{ + MockedArtist: artistRepo, + MockedMediaFile: mediaFileRepo, + MockedAlbum: albumRepo, + } + + agentsCombined = &mockAgents{} + provider = NewProvider(ds, agentsCombined) + }) + + Describe("matchSongsToLibrary priority matching", func() { + var track model.MediaFile + + BeforeEach(func() { + DeferCleanup(configtest.SetupConfig()) + // Disable fuzzy matching for these tests to avoid unexpected GetAll calls + conf.Server.SimilarSongsMatchThreshold = 100 + + track = model.MediaFile{ID: "track-1", Title: "Test Track", Artist: "Test Artist", MbzRecordingID: ""} + + // Setup for GetEntityByID to return the track + artistRepo.On("Get", "track-1").Return(nil, model.ErrNotFound).Once() + albumRepo.On("Get", "track-1").Return(nil, model.ErrNotFound).Once() + mediaFileRepo.On("Get", "track-1").Return(&track, nil).Once() + }) + + setupExpectations := func(returnedSongs []agents.Song, idMatches, mbidMatches, artistTracks model.MediaFiles) { + agentsCombined.On("GetSimilarSongsByTrack", mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything). + Return(returnedSongs, nil).Once() + + // loadTracksByID + mediaFileRepo.On("GetAll", mock.MatchedBy(func(opt model.QueryOptions) bool { + _, ok := opt.Filters.(squirrel.Eq) + return ok + })).Return(idMatches, nil).Once() + + // loadTracksByMBID + mediaFileRepo.On("GetAll", mock.MatchedBy(func(opt model.QueryOptions) bool { + and, ok := opt.Filters.(squirrel.And) + if !ok || len(and) < 1 { + return false + } + eq, hasEq := and[0].(squirrel.Eq) + if !hasEq { + return false + } + _, hasMBID := eq["mbz_recording_id"] + return hasMBID + })).Return(mbidMatches, nil).Once() + + // loadTracksByTitleAndArtist - now queries by artist name + mediaFileRepo.On("GetAll", mock.MatchedBy(func(opt model.QueryOptions) bool { + and, ok := opt.Filters.(squirrel.And) + if !ok || len(and) < 2 { + return false + } + eq, hasEq := and[0].(squirrel.Eq) + if !hasEq { + return false + } + _, hasArtist := eq["order_artist_name"] + return hasArtist + })).Return(artistTracks, nil).Maybe() + } + + Context("when agent returns artist and album metadata", func() { + It("matches by title + artist MBID + album MBID (highest priority)", func() { + // Song in library with all MBIDs + correctMatch := model.MediaFile{ + ID: "correct-match", Title: "Similar Song", Artist: "Depeche Mode", Album: "Violator", + MbzArtistID: "artist-mbid-123", MbzAlbumID: "album-mbid-456", + } + // Another song with same title but different MBIDs (should NOT match) + wrongMatch := model.MediaFile{ + ID: "wrong-match", Title: "Similar Song", Artist: "Depeche Mode", Album: "Some Other Album", + MbzArtistID: "artist-mbid-123", MbzAlbumID: "different-album-mbid", + } + returnedSongs := []agents.Song{ + {Name: "Similar Song", Artist: "Depeche Mode", ArtistMBID: "artist-mbid-123", Album: "Violator", AlbumMBID: "album-mbid-456"}, + } + + setupExpectations(returnedSongs, model.MediaFiles{}, model.MediaFiles{}, model.MediaFiles{wrongMatch, correctMatch}) + + songs, err := provider.SimilarSongs(ctx, "track-1", 5) + + Expect(err).ToNot(HaveOccurred()) + Expect(songs).To(HaveLen(1)) + Expect(songs[0].ID).To(Equal("correct-match")) + }) + + It("matches by title + artist name + album name when MBIDs unavailable", func() { + // Song in library without MBIDs but with matching artist/album names + correctMatch := model.MediaFile{ + ID: "correct-match", Title: "Similar Song", Artist: "depeche mode", Album: "violator", + } + // Another song with same title but different artist (should NOT match) + wrongMatch := model.MediaFile{ + ID: "wrong-match", Title: "Similar Song", Artist: "Other Artist", Album: "Other Album", + } + + returnedSongs := []agents.Song{ + {Name: "Similar Song", Artist: "Depeche Mode", Album: "Violator"}, // No MBIDs + } + + setupExpectations(returnedSongs, model.MediaFiles{}, model.MediaFiles{}, model.MediaFiles{wrongMatch, correctMatch}) + + songs, err := provider.SimilarSongs(ctx, "track-1", 5) + + Expect(err).ToNot(HaveOccurred()) + Expect(songs).To(HaveLen(1)) + Expect(songs[0].ID).To(Equal("correct-match")) + }) + + It("matches by title + artist only when album info unavailable", func() { + // Song in library with matching artist + correctMatch := model.MediaFile{ + ID: "correct-match", Title: "Similar Song", Artist: "depeche mode", Album: "Some Album", + } + // Another song with same title but different artist + wrongMatch := model.MediaFile{ + ID: "wrong-match", Title: "Similar Song", Artist: "Other Artist", Album: "Other Album", + } + returnedSongs := []agents.Song{ + {Name: "Similar Song", Artist: "Depeche Mode"}, // No album info + } + + setupExpectations(returnedSongs, model.MediaFiles{}, model.MediaFiles{}, model.MediaFiles{wrongMatch, correctMatch}) + + songs, err := provider.SimilarSongs(ctx, "track-1", 5) + + Expect(err).ToNot(HaveOccurred()) + Expect(songs).To(HaveLen(1)) + Expect(songs[0].ID).To(Equal("correct-match")) + }) + + It("does not match songs without artist info", func() { + // Songs without artist info cannot be matched since we query by artist + returnedSongs := []agents.Song{ + {Name: "Similar Song"}, // No artist/album info at all + } + + // No artist to query, so no GetAll calls for title matching + setupExpectations(returnedSongs, model.MediaFiles{}, model.MediaFiles{}, model.MediaFiles{}) + + songs, err := provider.SimilarSongs(ctx, "track-1", 5) + + Expect(err).ToNot(HaveOccurred()) + Expect(songs).To(BeEmpty()) + }) + }) + + Context("when matching multiple songs with the same title but different artists", func() { + It("returns distinct matches for each artist's version (covers scenario)", func() { + // Multiple covers of the same song by different artists + cover1 := model.MediaFile{ + ID: "cover-1", Title: "Yesterday", Artist: "The Beatles", Album: "Help!", + } + cover2 := model.MediaFile{ + ID: "cover-2", Title: "Yesterday", Artist: "Ray Charles", Album: "Greatest Hits", + } + cover3 := model.MediaFile{ + ID: "cover-3", Title: "Yesterday", Artist: "Frank Sinatra", Album: "My Way", + } + + returnedSongs := []agents.Song{ + {Name: "Yesterday", Artist: "The Beatles", Album: "Help!"}, + {Name: "Yesterday", Artist: "Ray Charles", Album: "Greatest Hits"}, + {Name: "Yesterday", Artist: "Frank Sinatra", Album: "My Way"}, + } + + setupExpectations(returnedSongs, model.MediaFiles{}, model.MediaFiles{}, model.MediaFiles{cover1, cover2, cover3}) + + songs, err := provider.SimilarSongs(ctx, "track-1", 5) + + Expect(err).ToNot(HaveOccurred()) + // All three covers should be returned, not just the first one + Expect(songs).To(HaveLen(3)) + // Verify all three different versions are included + ids := []string{songs[0].ID, songs[1].ID, songs[2].ID} + Expect(ids).To(ContainElements("cover-1", "cover-2", "cover-3")) + }) + }) + + Context("when matching multiple songs with different precision levels", func() { + It("prefers more precise matches for each song", func() { + // Library has multiple versions of same song + preciseMatch := model.MediaFile{ + ID: "precise", Title: "Song A", Artist: "Artist One", Album: "Album One", + MbzArtistID: "mbid-1", MbzAlbumID: "album-mbid-1", + } + lessAccurateMatch := model.MediaFile{ + ID: "less-accurate", Title: "Song A", Artist: "Artist One", Album: "Compilation", + MbzArtistID: "mbid-1", + } + artistTwoMatch := model.MediaFile{ + ID: "artist-two", Title: "Song B", Artist: "Artist Two", + } + + returnedSongs := []agents.Song{ + {Name: "Song A", Artist: "Artist One", ArtistMBID: "mbid-1", Album: "Album One", AlbumMBID: "album-mbid-1"}, + {Name: "Song B", Artist: "Artist Two"}, // Different artist + } + + setupExpectations(returnedSongs, model.MediaFiles{}, model.MediaFiles{}, model.MediaFiles{lessAccurateMatch, preciseMatch, artistTwoMatch}) + + songs, err := provider.SimilarSongs(ctx, "track-1", 5) + + Expect(err).ToNot(HaveOccurred()) + Expect(songs).To(HaveLen(2)) + // First song should be the precise match (has all MBIDs) + Expect(songs[0].ID).To(Equal("precise")) + // Second song matches by title + artist + Expect(songs[1].ID).To(Equal("artist-two")) + }) + }) + }) + + Describe("Fuzzy matching fallback", func() { + var track model.MediaFile + + BeforeEach(func() { + DeferCleanup(configtest.SetupConfig()) + track = model.MediaFile{ID: "track-1", Title: "Test Track", Artist: "Test Artist"} + + // Setup for GetEntityByID to return the track + artistRepo.On("Get", "track-1").Return(nil, model.ErrNotFound).Once() + albumRepo.On("Get", "track-1").Return(nil, model.ErrNotFound).Once() + mediaFileRepo.On("Get", "track-1").Return(&track, nil).Once() + }) + + setupFuzzyExpectations := func(returnedSongs []agents.Song, artistTracks model.MediaFiles) { + agentsCombined.On("GetSimilarSongsByTrack", mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything). + Return(returnedSongs, nil).Once() + + // loadTracksByTitleAndArtist now queries by artist in a single pass + // Note: loadTracksByID and loadTracksByMBID return early when no IDs/MBIDs + mediaFileRepo.On("GetAll", mock.MatchedBy(func(opt model.QueryOptions) bool { + and, ok := opt.Filters.(squirrel.And) + if !ok || len(and) < 2 { + return false + } + eq, hasEq := and[0].(squirrel.Eq) + if !hasEq { + return false + } + _, hasArtist := eq["order_artist_name"] + return hasArtist + })).Return(artistTracks, nil).Maybe() + } + + Context("with default threshold (85%)", func() { + It("matches songs with remastered suffix", func() { + conf.Server.SimilarSongsMatchThreshold = 85 + + // Agent returns "Paranoid Android" but library has "Paranoid Android - Remastered" + returnedSongs := []agents.Song{ + {Name: "Paranoid Android", Artist: "Radiohead"}, + } + // Artist catalog has the remastered version (fuzzy match will find it) + artistTracks := model.MediaFiles{ + {ID: "remastered", Title: "Paranoid Android - Remastered", Artist: "Radiohead"}, + } + + setupFuzzyExpectations(returnedSongs, artistTracks) + + songs, err := provider.SimilarSongs(ctx, "track-1", 5) + + Expect(err).ToNot(HaveOccurred()) + Expect(songs).To(HaveLen(1)) + Expect(songs[0].ID).To(Equal("remastered")) + }) + + It("matches songs with live suffix", func() { + conf.Server.SimilarSongsMatchThreshold = 85 + + returnedSongs := []agents.Song{ + {Name: "Bohemian Rhapsody", Artist: "Queen"}, + } + artistTracks := model.MediaFiles{ + {ID: "live", Title: "Bohemian Rhapsody (Live)", Artist: "Queen"}, + } + + setupFuzzyExpectations(returnedSongs, artistTracks) + + songs, err := provider.SimilarSongs(ctx, "track-1", 5) + + Expect(err).ToNot(HaveOccurred()) + Expect(songs).To(HaveLen(1)) + Expect(songs[0].ID).To(Equal("live")) + }) + + It("does not match completely different songs", func() { + conf.Server.SimilarSongsMatchThreshold = 85 + + returnedSongs := []agents.Song{ + {Name: "Yesterday", Artist: "The Beatles"}, + } + // Artist catalog has completely different songs + artistTracks := model.MediaFiles{ + {ID: "different", Title: "Tomorrow Never Knows", Artist: "The Beatles"}, + {ID: "different2", Title: "Here Comes The Sun", Artist: "The Beatles"}, + } + + setupFuzzyExpectations(returnedSongs, artistTracks) + + songs, err := provider.SimilarSongs(ctx, "track-1", 5) + + Expect(err).ToNot(HaveOccurred()) + Expect(songs).To(BeEmpty()) + }) + }) + + Context("with threshold set to 100 (exact match only)", func() { + It("only matches exact titles", func() { + conf.Server.SimilarSongsMatchThreshold = 100 + + returnedSongs := []agents.Song{ + {Name: "Paranoid Android", Artist: "Radiohead"}, + } + // Artist catalog has only remastered version - no exact match + artistTracks := model.MediaFiles{ + {ID: "remastered", Title: "Paranoid Android - Remastered", Artist: "Radiohead"}, + } + + setupFuzzyExpectations(returnedSongs, artistTracks) + + songs, err := provider.SimilarSongs(ctx, "track-1", 5) + + Expect(err).ToNot(HaveOccurred()) + Expect(songs).To(BeEmpty()) + }) + }) + + Context("with lower threshold (75%)", func() { + It("matches more aggressively", func() { + conf.Server.SimilarSongsMatchThreshold = 75 + + returnedSongs := []agents.Song{ + {Name: "Song", Artist: "Artist"}, + } + artistTracks := model.MediaFiles{ + {ID: "extended", Title: "Song (Extended Mix)", Artist: "Artist"}, + } + + setupFuzzyExpectations(returnedSongs, artistTracks) + + songs, err := provider.SimilarSongs(ctx, "track-1", 5) + + Expect(err).ToNot(HaveOccurred()) + Expect(songs).To(HaveLen(1)) + Expect(songs[0].ID).To(Equal("extended")) + }) + }) + + Context("with fuzzy album matching", func() { + It("matches album with (Remaster) suffix", func() { + conf.Server.SimilarSongsMatchThreshold = 85 + + // Agent returns "A Night at the Opera" but library has remastered version + returnedSongs := []agents.Song{ + {Name: "Bohemian Rhapsody", Artist: "Queen", Album: "A Night at the Opera"}, + } + // Library has same album with remaster suffix + correctMatch := model.MediaFile{ + ID: "correct", Title: "Bohemian Rhapsody", Artist: "Queen", Album: "A Night at the Opera (2011 Remaster)", + } + wrongMatch := model.MediaFile{ + ID: "wrong", Title: "Bohemian Rhapsody", Artist: "Queen", Album: "Greatest Hits", + } + + setupFuzzyExpectations(returnedSongs, model.MediaFiles{wrongMatch, correctMatch}) + + songs, err := provider.SimilarSongs(ctx, "track-1", 5) + + Expect(err).ToNot(HaveOccurred()) + Expect(songs).To(HaveLen(1)) + // Should prefer the fuzzy album match (Level 3) over title+artist only (Level 1) + Expect(songs[0].ID).To(Equal("correct")) + }) + + It("matches album with (Deluxe Edition) suffix", func() { + conf.Server.SimilarSongsMatchThreshold = 85 + + returnedSongs := []agents.Song{ + {Name: "Enjoy the Silence", Artist: "Depeche Mode", Album: "Violator"}, + } + correctMatch := model.MediaFile{ + ID: "correct", Title: "Enjoy the Silence", Artist: "Depeche Mode", Album: "Violator (Deluxe Edition)", + } + wrongMatch := model.MediaFile{ + ID: "wrong", Title: "Enjoy the Silence", Artist: "Depeche Mode", Album: "101", + } + + setupFuzzyExpectations(returnedSongs, model.MediaFiles{wrongMatch, correctMatch}) + + songs, err := provider.SimilarSongs(ctx, "track-1", 5) + + Expect(err).ToNot(HaveOccurred()) + Expect(songs).To(HaveLen(1)) + Expect(songs[0].ID).To(Equal("correct")) + }) + + It("prefers exact album match over fuzzy album match", func() { + conf.Server.SimilarSongsMatchThreshold = 85 + + returnedSongs := []agents.Song{ + {Name: "Enjoy the Silence", Artist: "Depeche Mode", Album: "Violator"}, + } + exactMatch := model.MediaFile{ + ID: "exact", Title: "Enjoy the Silence", Artist: "Depeche Mode", Album: "Violator", + } + fuzzyMatch := model.MediaFile{ + ID: "fuzzy", Title: "Enjoy the Silence", Artist: "Depeche Mode", Album: "Violator (Deluxe Edition)", + } + + setupFuzzyExpectations(returnedSongs, model.MediaFiles{fuzzyMatch, exactMatch}) + + songs, err := provider.SimilarSongs(ctx, "track-1", 5) + + Expect(err).ToNot(HaveOccurred()) + Expect(songs).To(HaveLen(1)) + // Both have same title similarity (1.0), so should prefer exact album match (higher specificity via higher album similarity) + Expect(songs[0].ID).To(Equal("exact")) + }) + }) + }) +}) diff --git a/core/external/provider_similarsongs_test.go b/core/external/provider_similarsongs_test.go new file mode 100644 index 00000000..1491d394 --- /dev/null +++ b/core/external/provider_similarsongs_test.go @@ -0,0 +1,443 @@ +package external_test + +import ( + "context" + "errors" + + "github.com/Masterminds/squirrel" + "github.com/navidrome/navidrome/core/agents" + . "github.com/navidrome/navidrome/core/external" + "github.com/navidrome/navidrome/model" + "github.com/navidrome/navidrome/tests" + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + "github.com/stretchr/testify/mock" +) + +var _ = Describe("Provider - SimilarSongs", func() { + var ds model.DataStore + var provider Provider + var mockAgent *mockSimilarArtistAgent + var mockTopAgent agents.ArtistTopSongsRetriever + var mockSimilarAgent agents.ArtistSimilarRetriever + var agentsCombined *mockAgents + var artistRepo *mockArtistRepo + var mediaFileRepo *mockMediaFileRepo + var albumRepo *mockAlbumRepo + var ctx context.Context + + BeforeEach(func() { + ctx = GinkgoT().Context() + + artistRepo = newMockArtistRepo() + mediaFileRepo = newMockMediaFileRepo() + albumRepo = newMockAlbumRepo() + + ds = &tests.MockDataStore{ + MockedArtist: artistRepo, + MockedMediaFile: mediaFileRepo, + MockedAlbum: albumRepo, + } + + mockAgent = &mockSimilarArtistAgent{} + mockTopAgent = mockAgent + mockSimilarAgent = mockAgent + + agentsCombined = &mockAgents{ + topSongsAgent: mockTopAgent, + similarAgent: mockSimilarAgent, + } + + provider = NewProvider(ds, agentsCombined) + }) + + Describe("dispatch by entity type", func() { + Context("when ID is a MediaFile (track)", func() { + It("calls GetSimilarSongsByTrack and returns matched songs", func() { + track := model.MediaFile{ID: "track-1", Title: "Just Can't Get Enough", Artist: "Depeche Mode", MbzRecordingID: "track-mbid"} + matchedSong := model.MediaFile{ID: "matched-1", Title: "Dreaming of Me", Artist: "Depeche Mode"} + + // GetEntityByID tries Artist, Album, Playlist, then MediaFile + artistRepo.On("Get", "track-1").Return(nil, model.ErrNotFound).Once() + albumRepo.On("Get", "track-1").Return(nil, model.ErrNotFound).Once() + mediaFileRepo.On("Get", "track-1").Return(&track, nil).Once() + + agentsCombined.On("GetSimilarSongsByTrack", mock.Anything, "track-1", "Just Can't Get Enough", "Depeche Mode", "track-mbid", 5). + Return([]agents.Song{ + {Name: "Dreaming of Me", MBID: "", Artist: "Depeche Mode", ArtistMBID: "artist-mbid"}, + }, nil).Once() + + // Mock loadTracksByID - no ID matches + mediaFileRepo.On("GetAll", mock.MatchedBy(func(opt model.QueryOptions) bool { + _, ok := opt.Filters.(squirrel.Eq) + return ok + })).Return(model.MediaFiles{}, nil).Once() + + // Mock loadTracksByMBID - no MBID matches (empty MBID means this won't be called) + mediaFileRepo.On("GetAll", mock.MatchedBy(func(opt model.QueryOptions) bool { + and, ok := opt.Filters.(squirrel.And) + if !ok || len(and) < 1 { + return false + } + eq, hasEq := and[0].(squirrel.Eq) + if !hasEq { + return false + } + _, hasMBID := eq["mbz_recording_id"] + return hasMBID + })).Return(model.MediaFiles{}, nil).Maybe() + + // Mock loadTracksByTitleAndArtist - queries by artist name + mediaFileRepo.On("GetAll", mock.MatchedBy(func(opt model.QueryOptions) bool { + and, ok := opt.Filters.(squirrel.And) + if !ok || len(and) < 2 { + return false + } + eq, hasEq := and[0].(squirrel.Eq) + if !hasEq { + return false + } + _, hasArtist := eq["order_artist_name"] + return hasArtist + })).Return(model.MediaFiles{matchedSong}, nil).Maybe() + + songs, err := provider.SimilarSongs(ctx, "track-1", 5) + + Expect(err).ToNot(HaveOccurred()) + Expect(songs).To(HaveLen(1)) + Expect(songs[0].ID).To(Equal("matched-1")) + }) + + It("falls back to artist-based algorithm when GetSimilarSongsByTrack returns empty", func() { + track := model.MediaFile{ID: "track-1", Title: "Track", Artist: "Artist", ArtistID: "artist-1"} + artist := model.Artist{ID: "artist-1", Name: "Artist"} + song := model.MediaFile{ID: "song-1", Title: "Song One", ArtistID: "artist-1", MbzRecordingID: "mbid-1"} + + // GetEntityByID for the initial call tries Artist, Album, Playlist, then MediaFile + artistRepo.On("Get", "track-1").Return(nil, model.ErrNotFound).Once() + albumRepo.On("Get", "track-1").Return(nil, model.ErrNotFound).Once() + mediaFileRepo.On("Get", "track-1").Return(&track, nil).Once() + + agentsCombined.On("GetSimilarSongsByTrack", mock.Anything, "track-1", "Track", "Artist", "", mock.Anything). + Return([]agents.Song{}, nil).Once() + + // Fallback calls getArtist(id) which calls GetEntityByID again - this time it finds the mediafile + // and recursively calls getArtist(v.ArtistID) + artistRepo.On("Get", "track-1").Return(nil, model.ErrNotFound).Once() + albumRepo.On("Get", "track-1").Return(nil, model.ErrNotFound).Once() + mediaFileRepo.On("Get", "track-1").Return(&track, nil).Once() + + // Then it recurses with the artist-1 ID + artistRepo.On("Get", "artist-1").Return(&artist, nil).Maybe() + artistRepo.On("GetAll", mock.MatchedBy(func(opt model.QueryOptions) bool { + return opt.Max == 1 && opt.Filters != nil + })).Return(model.Artists{artist}, nil).Maybe() + + mockAgent.On("GetSimilarArtists", mock.Anything, "artist-1", "Artist", "", 15). + Return([]agents.Artist{}, nil).Once() + + artistRepo.On("GetAll", mock.MatchedBy(func(opt model.QueryOptions) bool { + return opt.Max == 0 && opt.Filters != nil + })).Return(model.Artists{}, nil).Once() + + mockAgent.On("GetArtistTopSongs", mock.Anything, "artist-1", "Artist", "", mock.Anything). + Return([]agents.Song{{Name: "Song One", MBID: "mbid-1"}}, nil).Once() + + mediaFileRepo.On("GetAll", mock.AnythingOfType("model.QueryOptions")).Return(model.MediaFiles{song}, nil).Once() + + songs, err := provider.SimilarSongs(ctx, "track-1", 5) + + Expect(err).ToNot(HaveOccurred()) + Expect(songs).To(HaveLen(1)) + Expect(songs[0].ID).To(Equal("song-1")) + }) + }) + + Context("when ID is an Album", func() { + It("calls GetSimilarSongsByAlbum and returns matched songs", func() { + album := model.Album{ID: "album-1", Name: "Speak & Spell", AlbumArtist: "Depeche Mode", MbzAlbumID: "album-mbid"} + matchedSong := model.MediaFile{ID: "matched-1", Title: "New Life", Artist: "Depeche Mode", MbzRecordingID: "song-mbid"} + + // GetEntityByID tries Artist, Album, Playlist, then MediaFile + artistRepo.On("Get", "album-1").Return(nil, model.ErrNotFound).Once() + albumRepo.On("Get", "album-1").Return(&album, nil).Once() + + agentsCombined.On("GetSimilarSongsByAlbum", mock.Anything, "album-1", "Speak & Spell", "Depeche Mode", "album-mbid", 5). + Return([]agents.Song{ + {Name: "New Life", MBID: "song-mbid", Artist: "Depeche Mode"}, + }, nil).Once() + + // Mock loadTracksByID - no ID matches + mediaFileRepo.On("GetAll", mock.MatchedBy(func(opt model.QueryOptions) bool { + _, ok := opt.Filters.(squirrel.Eq) + return ok + })).Return(model.MediaFiles{}, nil).Once() + + // Mock loadTracksByMBID - MBID match + mediaFileRepo.On("GetAll", mock.MatchedBy(func(opt model.QueryOptions) bool { + and, ok := opt.Filters.(squirrel.And) + if !ok || len(and) < 1 { + return false + } + _, hasEq := and[0].(squirrel.Eq) + return hasEq + })).Return(model.MediaFiles{matchedSong}, nil).Once() + + songs, err := provider.SimilarSongs(ctx, "album-1", 5) + + Expect(err).ToNot(HaveOccurred()) + Expect(songs).To(HaveLen(1)) + Expect(songs[0].ID).To(Equal("matched-1")) + }) + + It("falls back when GetSimilarSongsByAlbum returns ErrNotFound", func() { + album := model.Album{ID: "album-1", Name: "Album", AlbumArtist: "Artist", AlbumArtistID: "artist-1"} + artist := model.Artist{ID: "artist-1", Name: "Artist"} + song := model.MediaFile{ID: "song-1", Title: "Song One", ArtistID: "artist-1", MbzRecordingID: "mbid-1"} + + // GetEntityByID for the initial call tries Artist, Album, Playlist, then MediaFile + artistRepo.On("Get", "album-1").Return(nil, model.ErrNotFound).Once() + albumRepo.On("Get", "album-1").Return(&album, nil).Once() + + agentsCombined.On("GetSimilarSongsByAlbum", mock.Anything, "album-1", "Album", "Artist", "", mock.Anything). + Return(nil, agents.ErrNotFound).Once() + + // Fallback calls getArtist(id) which calls GetEntityByID again - this time it finds the album + // and recursively calls getArtist(v.AlbumArtistID) + artistRepo.On("Get", "album-1").Return(nil, model.ErrNotFound).Once() + albumRepo.On("Get", "album-1").Return(&album, nil).Once() + + // Then it recurses with the artist-1 ID + artistRepo.On("Get", "artist-1").Return(&artist, nil).Maybe() + artistRepo.On("GetAll", mock.MatchedBy(func(opt model.QueryOptions) bool { + return opt.Max == 1 && opt.Filters != nil + })).Return(model.Artists{artist}, nil).Maybe() + + mockAgent.On("GetSimilarArtists", mock.Anything, "artist-1", "Artist", "", 15). + Return([]agents.Artist{}, nil).Once() + + artistRepo.On("GetAll", mock.MatchedBy(func(opt model.QueryOptions) bool { + return opt.Max == 0 && opt.Filters != nil + })).Return(model.Artists{}, nil).Once() + + mockAgent.On("GetArtistTopSongs", mock.Anything, "artist-1", "Artist", "", mock.Anything). + Return([]agents.Song{{Name: "Song One", MBID: "mbid-1"}}, nil).Once() + + mediaFileRepo.On("GetAll", mock.AnythingOfType("model.QueryOptions")).Return(model.MediaFiles{song}, nil).Once() + + songs, err := provider.SimilarSongs(ctx, "album-1", 5) + + Expect(err).ToNot(HaveOccurred()) + Expect(songs).To(HaveLen(1)) + Expect(songs[0].ID).To(Equal("song-1")) + }) + }) + + Context("when ID is an Artist", func() { + It("calls GetSimilarSongsByArtist and returns matched songs", func() { + artist := model.Artist{ID: "artist-1", Name: "Depeche Mode", MbzArtistID: "artist-mbid"} + matchedSong := model.MediaFile{ID: "matched-1", Title: "Enjoy the Silence", Artist: "Depeche Mode", MbzRecordingID: "song-mbid"} + + artistRepo.On("Get", "artist-1").Return(&artist, nil).Once() + agentsCombined.On("GetSimilarSongsByArtist", mock.Anything, "artist-1", "Depeche Mode", "artist-mbid", 5). + Return([]agents.Song{ + {Name: "Enjoy the Silence", MBID: "song-mbid", Artist: "Depeche Mode"}, + }, nil).Once() + + // Mock loadTracksByID - no ID matches + mediaFileRepo.On("GetAll", mock.MatchedBy(func(opt model.QueryOptions) bool { + _, ok := opt.Filters.(squirrel.Eq) + return ok + })).Return(model.MediaFiles{}, nil).Once() + + // Mock loadTracksByMBID - MBID match + mediaFileRepo.On("GetAll", mock.MatchedBy(func(opt model.QueryOptions) bool { + and, ok := opt.Filters.(squirrel.And) + if !ok || len(and) < 1 { + return false + } + _, hasEq := and[0].(squirrel.Eq) + return hasEq + })).Return(model.MediaFiles{matchedSong}, nil).Once() + + songs, err := provider.SimilarSongs(ctx, "artist-1", 5) + + Expect(err).ToNot(HaveOccurred()) + Expect(songs).To(HaveLen(1)) + Expect(songs[0].ID).To(Equal("matched-1")) + }) + }) + }) + + It("returns similar songs from main artist and similar artists", func() { + artist1 := model.Artist{ID: "artist-1", Name: "Artist One"} + similarArtist := model.Artist{ID: "artist-3", Name: "Similar Artist"} + song1 := model.MediaFile{ID: "song-1", Title: "Song One", ArtistID: "artist-1", MbzRecordingID: "mbid-1"} + song2 := model.MediaFile{ID: "song-2", Title: "Song Two", ArtistID: "artist-1", MbzRecordingID: "mbid-2"} + song3 := model.MediaFile{ID: "song-3", Title: "Song Three", ArtistID: "artist-3", MbzRecordingID: "mbid-3"} + + artistRepo.On("Get", "artist-1").Return(&artist1, nil).Maybe() + artistRepo.On("Get", "artist-3").Return(&similarArtist, nil).Maybe() + + artistRepo.On("GetAll", mock.MatchedBy(func(opt model.QueryOptions) bool { + return opt.Max == 1 && opt.Filters != nil + })).Return(model.Artists{artist1}, nil).Once() + + // New similar songs by artist returns ErrNotFound to trigger fallback + agentsCombined.On("GetSimilarSongsByArtist", mock.Anything, "artist-1", "Artist One", "", mock.Anything). + Return(nil, agents.ErrNotFound).Once() + + similarAgentsResp := []agents.Artist{ + {Name: "Similar Artist", MBID: "similar-mbid"}, + } + mockAgent.On("GetSimilarArtists", mock.Anything, "artist-1", "Artist One", "", 15). + Return(similarAgentsResp, nil).Once() + + // Mock the three-phase artist lookup: ID (skipped - no IDs), MBID, then Name + // MBID lookup returns empty (no match) + artistRepo.On("GetAll", mock.MatchedBy(func(opt model.QueryOptions) bool { + _, ok := opt.Filters.(squirrel.Eq) + return opt.Max == 0 && ok + })).Return(model.Artists{}, nil).Once() + // Name lookup returns the similar artist + artistRepo.On("GetAll", mock.MatchedBy(func(opt model.QueryOptions) bool { + _, ok := opt.Filters.(squirrel.Or) + return opt.Max == 0 && ok + })).Return(model.Artists{similarArtist}, nil).Once() + + mockAgent.On("GetArtistTopSongs", mock.Anything, "artist-1", "Artist One", "", mock.Anything). + Return([]agents.Song{ + {Name: "Song One", MBID: "mbid-1"}, + {Name: "Song Two", MBID: "mbid-2"}, + }, nil).Once() + + mockAgent.On("GetArtistTopSongs", mock.Anything, "artist-3", "Similar Artist", "", mock.Anything). + Return([]agents.Song{ + {Name: "Song Three", MBID: "mbid-3"}, + }, nil).Once() + + mediaFileRepo.On("GetAll", mock.AnythingOfType("model.QueryOptions")).Return(model.MediaFiles{song1, song2}, nil).Once() + mediaFileRepo.On("GetAll", mock.AnythingOfType("model.QueryOptions")).Return(model.MediaFiles{song3}, nil).Once() + + songs, err := provider.SimilarSongs(ctx, "artist-1", 3) + + Expect(err).ToNot(HaveOccurred()) + Expect(songs).To(HaveLen(3)) + for _, song := range songs { + Expect(song.ID).To(BeElementOf("song-1", "song-2", "song-3")) + } + }) + + It("returns ErrNotFound when artist is not found", func() { + artistRepo.On("Get", "artist-unknown-artist").Return(nil, model.ErrNotFound) + mediaFileRepo.On("Get", "artist-unknown-artist").Return(nil, model.ErrNotFound) + albumRepo.On("Get", "artist-unknown-artist").Return(nil, model.ErrNotFound) + + artistRepo.On("GetAll", mock.MatchedBy(func(opt model.QueryOptions) bool { + return opt.Max == 1 && opt.Filters != nil + })).Return(model.Artists{}, nil).Maybe() + + songs, err := provider.SimilarSongs(ctx, "artist-unknown-artist", 5) + + Expect(err).To(Equal(model.ErrNotFound)) + Expect(songs).To(BeNil()) + }) + + It("returns songs from main artist when GetSimilarArtists returns error", func() { + artist1 := model.Artist{ID: "artist-1", Name: "Artist One"} + song1 := model.MediaFile{ID: "song-1", Title: "Song One", ArtistID: "artist-1", MbzRecordingID: "mbid-1"} + + artistRepo.On("Get", "artist-1").Return(&artist1, nil).Maybe() + artistRepo.On("GetAll", mock.MatchedBy(func(opt model.QueryOptions) bool { + return opt.Max == 1 && opt.Filters != nil + })).Return(model.Artists{artist1}, nil).Maybe() + + // New similar songs by artist returns ErrNotFound to trigger fallback + agentsCombined.On("GetSimilarSongsByArtist", mock.Anything, "artist-1", "Artist One", "", mock.Anything). + Return(nil, agents.ErrNotFound).Once() + + mockAgent.On("GetSimilarArtists", mock.Anything, "artist-1", "Artist One", "", 15). + Return(nil, errors.New("error getting similar artists")).Once() + + artistRepo.On("GetAll", mock.MatchedBy(func(opt model.QueryOptions) bool { + return opt.Max == 0 && opt.Filters != nil + })).Return(model.Artists{}, nil).Once() + + mockAgent.On("GetArtistTopSongs", mock.Anything, "artist-1", "Artist One", "", mock.Anything). + Return([]agents.Song{ + {Name: "Song One", MBID: "mbid-1"}, + }, nil).Once() + + mediaFileRepo.On("GetAll", mock.AnythingOfType("model.QueryOptions")).Return(model.MediaFiles{song1}, nil).Once() + + songs, err := provider.SimilarSongs(ctx, "artist-1", 5) + + Expect(err).ToNot(HaveOccurred()) + Expect(songs).To(HaveLen(1)) + Expect(songs[0].ID).To(Equal("song-1")) + }) + + It("returns empty list when GetArtistTopSongs returns error", func() { + artist1 := model.Artist{ID: "artist-1", Name: "Artist One"} + + artistRepo.On("Get", "artist-1").Return(&artist1, nil).Maybe() + artistRepo.On("GetAll", mock.MatchedBy(func(opt model.QueryOptions) bool { + return opt.Max == 1 && opt.Filters != nil + })).Return(model.Artists{artist1}, nil).Maybe() + + // New similar songs by artist returns ErrNotFound to trigger fallback + agentsCombined.On("GetSimilarSongsByArtist", mock.Anything, "artist-1", "Artist One", "", mock.Anything). + Return(nil, agents.ErrNotFound).Once() + + mockAgent.On("GetSimilarArtists", mock.Anything, "artist-1", "Artist One", "", 15). + Return([]agents.Artist{}, nil).Once() + + artistRepo.On("GetAll", mock.MatchedBy(func(opt model.QueryOptions) bool { + return opt.Max == 0 && opt.Filters != nil + })).Return(model.Artists{}, nil).Once() + + mockAgent.On("GetArtistTopSongs", mock.Anything, "artist-1", "Artist One", "", mock.Anything). + Return(nil, errors.New("error getting top songs")).Once() + + songs, err := provider.SimilarSongs(ctx, "artist-1", 5) + + Expect(err).ToNot(HaveOccurred()) + Expect(songs).To(BeEmpty()) + }) + + It("respects count parameter", func() { + artist1 := model.Artist{ID: "artist-1", Name: "Artist One"} + song1 := model.MediaFile{ID: "song-1", Title: "Song One", ArtistID: "artist-1", MbzRecordingID: "mbid-1"} + song2 := model.MediaFile{ID: "song-2", Title: "Song Two", ArtistID: "artist-1", MbzRecordingID: "mbid-2"} + + artistRepo.On("Get", "artist-1").Return(&artist1, nil).Maybe() + artistRepo.On("GetAll", mock.MatchedBy(func(opt model.QueryOptions) bool { + return opt.Max == 1 && opt.Filters != nil + })).Return(model.Artists{artist1}, nil).Maybe() + + // New similar songs by artist returns ErrNotFound to trigger fallback + agentsCombined.On("GetSimilarSongsByArtist", mock.Anything, "artist-1", "Artist One", "", mock.Anything). + Return(nil, agents.ErrNotFound).Once() + + mockAgent.On("GetSimilarArtists", mock.Anything, "artist-1", "Artist One", "", 15). + Return([]agents.Artist{}, nil).Once() + + artistRepo.On("GetAll", mock.MatchedBy(func(opt model.QueryOptions) bool { + return opt.Max == 0 && opt.Filters != nil + })).Return(model.Artists{}, nil).Once() + + mockAgent.On("GetArtistTopSongs", mock.Anything, "artist-1", "Artist One", "", mock.Anything). + Return([]agents.Song{ + {Name: "Song One", MBID: "mbid-1"}, + {Name: "Song Two", MBID: "mbid-2"}, + }, nil).Once() + + mediaFileRepo.On("GetAll", mock.AnythingOfType("model.QueryOptions")).Return(model.MediaFiles{song1, song2}, nil).Once() + + songs, err := provider.SimilarSongs(ctx, "artist-1", 1) + + Expect(err).ToNot(HaveOccurred()) + Expect(songs).To(HaveLen(1)) + Expect(songs[0].ID).To(BeElementOf("song-1", "song-2")) + }) +}) diff --git a/core/external/provider_topsongs_test.go b/core/external/provider_topsongs_test.go index 114a35f6..0e513aa3 100644 --- a/core/external/provider_topsongs_test.go +++ b/core/external/provider_topsongs_test.go @@ -7,6 +7,8 @@ import ( _ "github.com/navidrome/navidrome/adapters/lastfm" _ "github.com/navidrome/navidrome/adapters/listenbrainz" _ "github.com/navidrome/navidrome/adapters/spotify" + "github.com/navidrome/navidrome/conf" + "github.com/navidrome/navidrome/conf/configtest" "github.com/navidrome/navidrome/core/agents" . "github.com/navidrome/navidrome/core/external" "github.com/navidrome/navidrome/model" @@ -26,6 +28,10 @@ var _ = Describe("Provider - TopSongs", func() { ) BeforeEach(func() { + DeferCleanup(configtest.SetupConfig()) + // Disable fuzzy matching for these tests to avoid unexpected GetAll calls + conf.Server.SimilarSongsMatchThreshold = 100 + ctx = GinkgoT().Context() artistRepo = newMockArtistRepo() // Use helper mock diff --git a/resources/i18n/pt-br.json b/resources/i18n/pt-br.json index c9917c7b..2697de62 100644 --- a/resources/i18n/pt-br.json +++ b/resources/i18n/pt-br.json @@ -46,7 +46,8 @@ "download": "Baixar", "playNext": "Toca a seguir", "info": "Detalhes", - "showInPlaylist": "Ir para playlist" + "showInPlaylist": "Ir para playlist", + "instantMix": "Mix Instantâneo" } }, "album": { @@ -557,6 +558,7 @@ "transcodingEnabled": "Navidrome está sendo executado com a opção %{config}. Isto permite que potencialmente se execute comandos do sistema pela interface Web. É recomendado que vc mantenha esta opção desabilitada, e só a habilite quando precisar configurar opções de Conversão", "songsAddedToPlaylist": "Música adicionada à playlist |||| %{smart_count} músicas adicionadas à playlist", "noSimilarSongsFound": "Nenhuma música semelhante encontrada", + "startingInstantMix": "Carregando Mix Instantâneo...", "noTopSongsFound": "Nenhuma música mais tocada encontrada", "noPlaylistsAvailable": "Nenhuma playlist", "delete_user_title": "Excluir usuário '%{name}'", diff --git a/server/subsonic/browsing.go b/server/subsonic/browsing.go index 30779e42..63939f6f 100644 --- a/server/subsonic/browsing.go +++ b/server/subsonic/browsing.go @@ -354,7 +354,7 @@ func (api *Router) GetSimilarSongs(r *http.Request) (*responses.Subsonic, error) } count := p.IntOr("count", 50) - songs, err := api.provider.ArtistRadio(ctx, id, count) + songs, err := api.provider.SimilarSongs(ctx, id, count) if err != nil { return nil, err } diff --git a/tests/fixtures/lastfm.track.getsimilar.json b/tests/fixtures/lastfm.track.getsimilar.json new file mode 100644 index 00000000..45041b28 --- /dev/null +++ b/tests/fixtures/lastfm.track.getsimilar.json @@ -0,0 +1 @@ +{"similartracks":{"track":[{"name":"Dreaming of Me","mbid":"027b553e-7c74-3ed4-a95e-1d4fea51f174","match":1.0,"url":"https://www.last.fm/music/Depeche+Mode/_/Dreaming+of+Me","artist":{"name":"Depeche Mode","mbid":"8538e728-ca0b-4321-b7e5-cff6565dd4c0","url":"https://www.last.fm/music/Depeche+Mode"}},{"name":"Everything Counts","mbid":"5a5a3ca4-bdb8-4641-a674-9b54b9b319a6","match":0.892602,"url":"https://www.last.fm/music/Depeche+Mode/_/Everything+Counts","artist":{"name":"Depeche Mode","mbid":"8538e728-ca0b-4321-b7e5-cff6565dd4c0","url":"https://www.last.fm/music/Depeche+Mode"}},{"name":"Don't You Want Me","mbid":"","match":0.491341,"url":"https://www.last.fm/music/The+Human+League/_/Don%27t+You+Want+Me","artist":{"name":"The Human League","mbid":"7adaabfb-acfb-47bc-8c7c-59471c2f0db8","url":"https://www.last.fm/music/The+Human+League"}},{"name":"Tainted Love","mbid":"","match":0.454811,"url":"https://www.last.fm/music/Soft+Cell/_/Tainted+Love","artist":{"name":"Soft Cell","mbid":"7fb50287-029d-47cc-825a-235ca28024b2","url":"https://www.last.fm/music/Soft+Cell"}},{"name":"Blue Monday","mbid":"727e84c6-1b56-31dd-a958-a5f46305cec0","match":0.381057,"url":"https://www.last.fm/music/New+Order/_/Blue+Monday","artist":{"name":"New Order","mbid":"f1106b17-dcbb-45f6-b938-199ccfab50cc","url":"https://www.last.fm/music/New+Order"}}],"@attr":{"artist":"Depeche Mode","track":"Just Can't Get Enough"}}} diff --git a/tests/fixtures/lastfm.track.getsimilar.unknown.json b/tests/fixtures/lastfm.track.getsimilar.unknown.json new file mode 100644 index 00000000..9b879fa0 --- /dev/null +++ b/tests/fixtures/lastfm.track.getsimilar.unknown.json @@ -0,0 +1 @@ +{"similartracks":{"track":[],"@attr":{"track":"UnknownTrack","artist":"UnknownArtist"}}} diff --git a/ui/src/artist/ArtistActions.jsx b/ui/src/artist/ArtistActions.jsx index 8eebe649..0b48f232 100644 --- a/ui/src/artist/ArtistActions.jsx +++ b/ui/src/artist/ArtistActions.jsx @@ -14,7 +14,8 @@ import { import ShuffleIcon from '@material-ui/icons/Shuffle' import PlayArrowIcon from '@material-ui/icons/PlayArrow' import { IoIosRadio } from 'react-icons/io' -import { playShuffle, playSimilar, playTopSongs } from './actions.js' +import { playShuffle, playTopSongs } from './actions.js' +import { playSimilar } from '../common/playbackActions.js' const useStyles = makeStyles((theme) => ({ toolbar: { diff --git a/ui/src/artist/actions.js b/ui/src/artist/actions.js index 6a8fbd9c..94b3adf4 100644 --- a/ui/src/artist/actions.js +++ b/ui/src/artist/actions.js @@ -1,31 +1,6 @@ import subsonic from '../subsonic/index.js' import { playTracks } from '../actions/index.js' - -const mapReplayGain = (song) => { - const { replayGain: rg } = song - if (!rg) { - return song - } - - return { - ...song, - ...(rg.albumGain !== undefined && { rgAlbumGain: rg.albumGain }), - ...(rg.albumPeak !== undefined && { rgAlbumPeak: rg.albumPeak }), - ...(rg.trackGain !== undefined && { rgTrackGain: rg.trackGain }), - ...(rg.trackPeak !== undefined && { rgTrackPeak: rg.trackPeak }), - } -} - -const processSongsForPlayback = (songs) => { - const songData = {} - const ids = [] - songs.forEach((s) => { - const song = mapReplayGain(s) - songData[song.id] = song - ids.push(song.id) - }) - return { songData, ids } -} +import { processSongsForPlayback } from '../common/playbackActions.js' export const playTopSongs = async (dispatch, notify, artistName) => { const res = await subsonic.getTopSongs(artistName, 100) @@ -47,26 +22,6 @@ export const playTopSongs = async (dispatch, notify, artistName) => { dispatch(playTracks(songData, ids)) } -export const playSimilar = async (dispatch, notify, id) => { - const res = await subsonic.getSimilarSongs2(id, 100) - const data = res.json['subsonic-response'] - - if (data.status !== 'ok') { - throw new Error( - `Error fetching similar songs: ${data.error?.message || 'Unknown error'} (Code: ${data.error?.code || 'unknown'})`, - ) - } - - const songs = data.similarSongs2?.song || [] - if (!songs.length) { - notify('message.noSimilarSongsFound', 'warning') - return - } - - const { songData, ids } = processSongsForPlayback(songs) - dispatch(playTracks(songData, ids)) -} - export const playShuffle = async (dataProvider, dispatch, id) => { const res = await dataProvider.getList('song', { pagination: { page: 1, perPage: 500 }, diff --git a/ui/src/common/SongContextMenu.jsx b/ui/src/common/SongContextMenu.jsx index f8b0bba5..ce6604f6 100644 --- a/ui/src/common/SongContextMenu.jsx +++ b/ui/src/common/SongContextMenu.jsx @@ -24,6 +24,7 @@ import { } from '../actions' import { LoveButton } from './LoveButton' import config from '../config' +import { playSimilar } from './playbackActions.js' import { formatBytes } from '../utils' import { useRedirect } from 'react-admin' @@ -86,6 +87,24 @@ export const SongContextMenu = ({ label: translate('resources.song.actions.addToQueue'), action: (record) => dispatch(addTracks({ [record.id]: record })), }, + instantMix: { + enabled: config.enableExternalServices, + label: translate('resources.song.actions.instantMix'), + action: async (record) => { + notify('message.startingInstantMix', { type: 'info' }) + try { + const id = record.mediaFileId || record.id + await playSimilar(dispatch, notify, id, { + seedRecord: record, + shuffle: true, + }) + } catch (e) { + // eslint-disable-next-line no-console + console.error('Error starting instant mix:', e) + notify('ra.page.error', { type: 'warning' }) + } + }, + }, addToPlaylist: { enabled: true, label: translate('resources.song.actions.addToPlaylist'), diff --git a/ui/src/common/SongContextMenu.test.jsx b/ui/src/common/SongContextMenu.test.jsx index a30da859..7172742d 100644 --- a/ui/src/common/SongContextMenu.test.jsx +++ b/ui/src/common/SongContextMenu.test.jsx @@ -3,19 +3,36 @@ import { render, fireEvent, screen, waitFor } from '@testing-library/react' import { TestContext } from 'ra-test' import { describe, it, expect, vi, beforeEach } from 'vitest' import { SongContextMenu } from './SongContextMenu' +import subsonic from '../subsonic' vi.mock('../dataProvider', () => ({ httpClient: vi.fn(), })) -vi.mock('react-redux', () => ({ useDispatch: () => vi.fn() })) +vi.mock('../subsonic', () => ({ + default: { getSimilarSongs2: vi.fn() }, +})) + +vi.mock('../config', () => ({ + default: { + enableDownloads: true, + enableFavourites: true, + enableSharing: true, + enableExternalServices: true, + }, +})) + +const mockDispatch = vi.fn() +vi.mock('react-redux', () => ({ useDispatch: () => mockDispatch })) const getPlaylistsMock = vi.fn() +const mockNotify = vi.fn() vi.mock('react-admin', async (importOriginal) => { const actual = await importOriginal() return { ...actual, + useNotify: () => mockNotify, useRedirect: () => (url) => { window.location.hash = `#${url}` }, @@ -35,6 +52,14 @@ describe('SongContextMenu', () => { getPlaylistsMock.mockResolvedValue({ data: [{ id: 'pl1', name: 'Pl 1' }], }) + subsonic.getSimilarSongs2.mockResolvedValue({ + json: { + 'subsonic-response': { + status: 'ok', + similarSongs2: { song: [{ id: 's1' }] }, + }, + }, + }) }) it('navigates to playlist when selected', async () => { @@ -104,4 +129,99 @@ describe('SongContextMenu', () => { ) expect(mockOnClick).not.toHaveBeenCalled() }) + + describe('Instant Mix action', () => { + it('calls getSimilarSongs2 with song id and shows loading notification', async () => { + render( + + + , + ) + + fireEvent.click(screen.getAllByRole('button')[1]) + await waitFor(() => + screen.getByText(/resources\.song\.actions\.instantMix/), + ) + fireEvent.click(screen.getByText(/resources\.song\.actions\.instantMix/)) + + // Verify loading notification is shown + expect(mockNotify).toHaveBeenCalledWith('message.startingInstantMix', { + type: 'info', + }) + + await waitFor(() => + expect(subsonic.getSimilarSongs2).toHaveBeenCalledWith('song1', 100), + ) + expect(mockDispatch).toHaveBeenCalled() + }) + + it('plays seed song first followed by similar songs', async () => { + const seedRecord = { id: 'song1', title: 'Seed Song', size: 1 } + render( + + + , + ) + + fireEvent.click(screen.getAllByRole('button')[1]) + await waitFor(() => + screen.getByText(/resources\.song\.actions\.instantMix/), + ) + fireEvent.click(screen.getByText(/resources\.song\.actions\.instantMix/)) + + await waitFor(() => expect(mockDispatch).toHaveBeenCalled()) + + // Verify dispatch was called with playTracks action + const dispatchCall = mockDispatch.mock.calls.find( + (call) => call[0]?.type === 'PLAYER_PLAY_TRACKS', + ) + expect(dispatchCall).toBeDefined() + + // Verify seed song is first (id property contains the first song to play) + const { id, data } = dispatchCall[0] + expect(id).toBe('song1') + // Verify seed song data is included + expect(data['song1']).toBeDefined() + }) + + it('uses mediaFileId when available (playlist context)', async () => { + render( + + + , + ) + + fireEvent.click(screen.getAllByRole('button')[1]) + await waitFor(() => + screen.getByText(/resources\.song\.actions\.instantMix/), + ) + fireEvent.click(screen.getByText(/resources\.song\.actions\.instantMix/)) + + await waitFor(() => + expect(subsonic.getSimilarSongs2).toHaveBeenCalledWith( + 'actualSongId', + 100, + ), + ) + + await waitFor(() => expect(mockDispatch).toHaveBeenCalled()) + + // Verify the mediaFileId is used as the seed song id + const dispatchCall = mockDispatch.mock.calls.find( + (call) => call[0]?.type === 'PLAYER_PLAY_TRACKS', + ) + expect(dispatchCall).toBeDefined() + const { id, data } = dispatchCall[0] + expect(id).toBe('actualSongId') + // Verify seed song data is included + expect(data['actualSongId']).toBeDefined() + }) + }) }) diff --git a/ui/src/common/playbackActions.js b/ui/src/common/playbackActions.js new file mode 100644 index 00000000..414dbe41 --- /dev/null +++ b/ui/src/common/playbackActions.js @@ -0,0 +1,76 @@ +import subsonic from '../subsonic/index.js' +import { playTracks } from '../actions/index.js' + +const shuffleArray = (array) => { + const shuffled = [...array] + for (let i = shuffled.length - 1; i > 0; i--) { + const j = Math.floor(Math.random() * (i + 1)) + ;[shuffled[i], shuffled[j]] = [shuffled[j], shuffled[i]] + } + return shuffled +} + +const mapReplayGain = (song) => { + const { replayGain: rg } = song + if (!rg) { + return song + } + + return { + ...song, + ...(rg.albumGain !== undefined && { rgAlbumGain: rg.albumGain }), + ...(rg.albumPeak !== undefined && { rgAlbumPeak: rg.albumPeak }), + ...(rg.trackGain !== undefined && { rgTrackGain: rg.trackGain }), + ...(rg.trackPeak !== undefined && { rgTrackPeak: rg.trackPeak }), + } +} + +export const processSongsForPlayback = (songs) => { + const songData = {} + const ids = [] + songs.forEach((s) => { + const song = mapReplayGain(s) + songData[song.id] = song + ids.push(song.id) + }) + return { songData, ids } +} + +export const playSimilar = async (dispatch, notify, id, options = {}) => { + const { seedRecord = null, shuffle = false } = options + + const res = await subsonic.getSimilarSongs2(id, 100) + const data = res.json['subsonic-response'] + + if (data.status !== 'ok') { + throw new Error( + `Error fetching similar songs: ${data.error?.message || 'Unknown error'} (Code: ${data.error?.code || 'unknown'})`, + ) + } + + let songs = data.similarSongs2?.song || [] + + // Randomize similar songs if requested + if (shuffle) { + songs = shuffleArray(songs) + } + + // If no similar songs found and no seed, show warning + if (!songs.length && !seedRecord) { + notify('message.noSimilarSongsFound', 'warning') + return + } + + const { songData, ids } = processSongsForPlayback(songs) + + // Prepend seed song if provided + if (seedRecord) { + const seedId = seedRecord.mediaFileId || seedRecord.id + // Remove seed from similar songs if it appears there + const filteredIds = ids.filter((songId) => songId !== seedId) + songData[seedId] = mapReplayGain(seedRecord) + dispatch(playTracks(songData, [seedId, ...filteredIds])) + } else { + dispatch(playTracks(songData, ids)) + } +} diff --git a/ui/src/i18n/en.json b/ui/src/i18n/en.json index 11cbbc92..a18c7997 100644 --- a/ui/src/i18n/en.json +++ b/ui/src/i18n/en.json @@ -47,7 +47,8 @@ "shuffleAll": "Shuffle All", "download": "Download", "playNext": "Play Next", - "info": "Get Info" + "info": "Get Info", + "instantMix": "Instant Mix" } }, "album": { @@ -558,6 +559,7 @@ "transcodingEnabled": "Navidrome is currently running with %{config}, making it possible to run system commands from the transcoding settings using the web interface. We recommend to disable it for security reasons and only enable it when configuring Transcoding options.", "songsAddedToPlaylist": "Added 1 song to playlist |||| Added %{smart_count} songs to playlist", "noSimilarSongsFound": "No similar songs found", + "startingInstantMix": "Loading Instant Mix...", "noTopSongsFound": "No top songs found", "noPlaylistsAvailable": "None available", "delete_user_title": "Delete user '%{name}'",