nik/navidrome
1
0
Fork 0
mirror of https://github.com/navidrome/navidrome.git synced 2026-08-01 07:21:17 +00:00
Code Issues Packages Projects Releases Wiki Activity

refactor(lyrics): fold batch resolution into the Lyrics interface

Browse Source 
The legacy getLyrics handler type-asserted api.lyrics to a separate
BatchLyrics interface with a fallback loop for the non-batch case. The
assertion always succeeded (the only implementation is *lyricsService),
so the fallback was dead code -- and had it run, its file-outer /
priority-inner order would have violated the configured source priority.

Add GetLyricsForMediaFiles to the Lyrics interface and introduce a
narrow Provider interface (just GetLyrics) for the single-file plugin
contract. The handler now calls GetLyricsForMediaFiles directly, and
LoadLyricsProvider returns the narrower Provider.
This commit is contained in:
Deluan 2026-06-18 20:58:15 -04:00
parent 7889deb410
commit 2e2dc17dd6
4 changed files with 16 additions and 29 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

15
core/lyrics/lyrics.go
View File

@ -9,20 +9,23 @@ import (
"github.com/navidrome/navidrome/model"
)
// Lyrics can fetch lyrics for a media file.
type Lyrics interface {
// Provider fetches lyrics for a single media file. It is the contract
// implemented by individual lyrics sources, such as plugins.
type Provider interface {
GetLyrics(ctx context.Context, mf *model.MediaFile) (model.LyricList, error)
}
// BatchLyrics can resolve lyrics across multiple candidate media files while
// still honoring the configured source priority globally.
type BatchLyrics interface {
// Lyrics resolves lyrics for media files, honoring the configured source
// priority. GetLyricsForMediaFiles preserves that priority across a set of
// duplicate candidates instead of only consulting the first match.
type Lyrics interface {
Provider
GetLyricsForMediaFiles(ctx context.Context, mediaFiles []model.MediaFile) (model.LyricList, error)
}
// PluginLoader discovers and loads lyrics provider plugins.
type PluginLoader interface {
LoadLyricsProvider(name string) (Lyrics, bool)
LoadLyricsProvider(name string) (Provider, bool)
}
type lyricsService struct {

6
core/lyrics/lyrics_test.go
View File

@ -176,10 +176,8 @@ var _ = Describe("sources", func() {
Expect(err).To(BeNil())
svc := lyrics.NewLyrics(nil)
batchSvc, ok := svc.(lyrics.BatchLyrics)
Expect(ok).To(BeTrue())
list, err := batchSvc.GetLyricsForMediaFiles(ctx, []model.MediaFile{
list, err := svc.GetLyricsForMediaFiles(ctx, []model.MediaFile{
{
Lyrics: string(embeddedJSON),
Path: "tests/fixtures/01 Invisible (RED) Edit Version.mp3",
@ -372,7 +370,7 @@ func (m *mockPluginLoader) PluginNames(_ string) []string {
return []string{"test-lyrics-plugin"}
}
func (m *mockPluginLoader) LoadLyricsProvider(name string) (lyrics.Lyrics, bool) {
func (m *mockPluginLoader) LoadLyricsProvider(name string) (lyrics.Provider, bool) {
if m.notFound {
return nil, false
}

2
plugins/manager.go
View File

@ -241,7 +241,7 @@ func (m *Manager) LoadScrobbler(name string) (scrobbler.Scrobbler, bool) {
return loadPlugin(m, name, CapabilityScrobbler, newScrobblerPlugin)
}
func (m *Manager) LoadLyricsProvider(name string) (lyrics.Lyrics, bool) {
func (m *Manager) LoadLyricsProvider(name string) (lyrics.Provider, bool) {
return loadPlugin(m, name, CapabilityLyrics, newLyricsPlugin)
}

22
server/subsonic/media_retrieval.go
View File

@ -10,7 +10,6 @@ import (
"github.com/navidrome/navidrome/conf"
"github.com/navidrome/navidrome/consts"
lyricssvc "github.com/navidrome/navidrome/core/lyrics"
"github.com/navidrome/navidrome/log"
"github.com/navidrome/navidrome/model"
"github.com/navidrome/navidrome/resources"
@ -102,7 +101,7 @@ func (api *Router) GetLyrics(r *http.Request) (*responses.Subsonic, error) {
lyricsResponse := responses.Lyrics{}
response.Lyrics = &lyricsResponse
opts := filter.SongsByArtistTitleWithLyricsFirst(artist, title)
// Search a bounded duplicate window so source-priority fallback can still
// Search a bounded duplicate window so source-priority resolution can still
// reach older matches without turning legacy getLyrics into an unbounded scan.
opts.Max = maxLegacyLyricsCandidates
mediaFiles, err := api.ds.MediaFile(r.Context()).GetAll(opts)
@ -115,22 +114,9 @@ func (api *Router) GetLyrics(r *http.Request) (*responses.Subsonic, error) {
return response, nil
}
var structuredLyrics model.LyricList
if batchLyrics, ok := api.lyrics.(lyricssvc.BatchLyrics); ok {
structuredLyrics, err = batchLyrics.GetLyricsForMediaFiles(r.Context(), mediaFiles)
if err != nil {
return nil, err
}
} else {
for i := range mediaFiles {
structuredLyrics, err = api.lyrics.GetLyrics(r.Context(), &mediaFiles[i])
if err != nil {
return nil, err
}
if len(structuredLyrics) > 0 {
break
}
}
structuredLyrics, err := api.lyrics.GetLyricsForMediaFiles(r.Context(), mediaFiles)
if err != nil {
return nil, err
}
if len(structuredLyrics) == 0 {