feat(plugins): add path to Scrobbler and Lyrics plugin TrackInfo (#5339)

* feat: add Path to TrackInfo struct

* refactor: improve naming to follow the rest of the code

* test: add tests

* fix: actually check for filesystem permission

* refactor: remove library logic from specific plugins

* refactor: move hasFilesystemPermission to a Manifest method

* test(plugins): add unit tests for hasLibraryFilesystemAccess method

Signed-off-by: Deluan <deluan@navidrome.org>

* refactor(plugins): remove hasFilesystemPerm field and use manifest for filesystem permission checks

Signed-off-by: Deluan <deluan@navidrome.org>

* refactor(plugins): streamline library filesystem access checks in lyrics and scrobbler adapters

Signed-off-by: Deluan <deluan@navidrome.org>

---------

Signed-off-by: Deluan <deluan@navidrome.org>
Co-authored-by: Deluan <deluan@navidrome.org>
This commit is contained in:
Jorge Pardo Pardo 2026-04-12 16:27:58 +02:00 committed by GitHub
parent 501c6eaf8f
commit 85e9982b43
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
16 changed files with 150 additions and 7 deletions

View file

@ -102,6 +102,11 @@ components:
mbzReleaseTrackId:
type: string
description: MBZReleaseTrackID is the MusicBrainz release track ID.
path:
type: string
description: |-
Path is the full path to the track file, relative to the library root.
Only included if the plugin has library permission with filesystem access for the track's library.
required:
- id
- title

View file

@ -68,6 +68,9 @@ type TrackInfo struct {
MBZReleaseGroupID string `json:"mbzReleaseGroupId,omitempty"`
// MBZReleaseTrackID is the MusicBrainz release track ID.
MBZReleaseTrackID string `json:"mbzReleaseTrackId,omitempty"`
// Path is the full path to the track file, relative to the library root.
// Only included if the plugin has library permission with filesystem access for the track's library.
Path string `json:"path,omitempty"`
}
// NowPlayingRequest is the request for now playing notification.

View file

@ -128,6 +128,11 @@ components:
mbzReleaseTrackId:
type: string
description: MBZReleaseTrackID is the MusicBrainz release track ID.
path:
type: string
description: |-
Path is the full path to the track file, relative to the library root.
Only included if the plugin has library permission with filesystem access for the track's library.
required:
- id
- title

View file

@ -31,7 +31,7 @@ type LyricsPlugin struct {
// using model.ToLyrics.
func (l *LyricsPlugin) GetLyrics(ctx context.Context, mf *model.MediaFile) (model.LyricList, error) {
req := capabilities.GetLyricsRequest{
Track: mediaFileToTrackInfo(mf),
Track: mediaFileToTrackInfo(l.plugin, mf),
}
resp, err := callPluginFunction[capabilities.GetLyricsRequest, capabilities.GetLyricsResponse](
ctx, l.plugin, FuncLyricsGetLyrics, req,

View file

@ -301,7 +301,7 @@ func (m *Manager) loadPluginWithConfig(p *model.Plugin) error {
}
// Configure filesystem access for library permission
if pkg.Manifest.Permissions != nil && pkg.Manifest.Permissions.Library != nil && pkg.Manifest.Permissions.Library.Filesystem {
if pkg.Manifest.HasLibraryFilesystemPermission() {
adminCtx := adminContext(ctx)
libraries, err := m.ds.Library(adminCtx).GetAll()
if err != nil {
@ -384,6 +384,7 @@ func (m *Manager) loadPluginWithConfig(p *model.Plugin) error {
metrics: m.metrics,
allowedUserIDs: allowedUsers,
allUsers: p.AllUsers,
libraries: newLibraryAccess(allowedLibraries, p.AllLibraries),
}
m.mu.Unlock()

View file

@ -21,6 +21,7 @@ type plugin struct {
metrics PluginMetricsRecorder
allowedUserIDs []string // User IDs this plugin can access (from DB configuration)
allUsers bool // If true, plugin can access all users
libraries libraryAccess
}
// instance creates a new plugin instance for the given context.
@ -47,3 +48,30 @@ func (p *plugin) Close() error {
}
return errors.Join(errs...)
}
func (p *plugin) hasLibraryFilesystemAccess(libID int) bool {
return p.manifest.HasLibraryFilesystemPermission() && p.libraries.contains(libID)
}
// libraryAccess captures the set of libraries a plugin is permitted to see,
// precomputed at load time for O(1) lookup.
type libraryAccess struct {
allLibraries bool
libraryIDSet map[int]struct{}
}
func newLibraryAccess(allowedLibraryIDs []int, allLibraries bool) libraryAccess {
set := make(map[int]struct{}, len(allowedLibraryIDs))
for _, id := range allowedLibraryIDs {
set[id] = struct{}{}
}
return libraryAccess{allLibraries: allLibraries, libraryIDSet: set}
}
func (a libraryAccess) contains(libID int) bool {
if a.allLibraries {
return true
}
_, ok := a.libraryIDSet[libID]
return ok
}

View file

@ -0,0 +1,34 @@
package plugins
import (
. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"
)
var _ = Describe("plugin", func() {
Describe("hasLibraryFilesystemAccess", func() {
fsManifest := &Manifest{
Permissions: &Permissions{
Library: &LibraryPermission{Filesystem: true},
},
}
It("returns false when the manifest does not grant filesystem permission", func() {
p := &plugin{manifest: &Manifest{}, libraries: newLibraryAccess(nil, true)}
Expect(p.hasLibraryFilesystemAccess(1)).To(BeFalse())
})
It("returns true for any library when allLibraries is set", func() {
p := &plugin{manifest: fsManifest, libraries: newLibraryAccess(nil, true)}
Expect(p.hasLibraryFilesystemAccess(1)).To(BeTrue())
Expect(p.hasLibraryFilesystemAccess(42)).To(BeTrue())
})
It("returns true only for libraries in the allowed list", func() {
p := &plugin{manifest: fsManifest, libraries: newLibraryAccess([]int{1, 3}, false)}
Expect(p.hasLibraryFilesystemAccess(1)).To(BeTrue())
Expect(p.hasLibraryFilesystemAccess(3)).To(BeTrue())
Expect(p.hasLibraryFilesystemAccess(2)).To(BeFalse())
})
})
})

View file

@ -86,3 +86,10 @@ func ValidateWithCapabilities(m *Manifest, capabilities []Capability) error {
func (m *Manifest) HasExperimentalThreads() bool {
return m.Experimental != nil && m.Experimental.Threads != nil
}
// HasLibraryFilesystemPermission checks if the manifest grants filesystem permission for libraries.
func (m *Manifest) HasLibraryFilesystemPermission() bool {
return m.Permissions != nil &&
m.Permissions.Library != nil &&
m.Permissions.Library.Filesystem
}

View file

@ -68,6 +68,9 @@ type TrackInfo struct {
MBZReleaseGroupID string `json:"mbzReleaseGroupId,omitempty"`
// MBZReleaseTrackID is the MusicBrainz release track ID.
MBZReleaseTrackID string `json:"mbzReleaseTrackId,omitempty"`
// Path is the full path to the track file, relative to the library root.
// Only included if the plugin has library permission with filesystem access for the track's library.
Path string `json:"path,omitempty"`
}
// Lyrics requires all methods to be implemented.

View file

@ -65,6 +65,9 @@ type TrackInfo struct {
MBZReleaseGroupID string `json:"mbzReleaseGroupId,omitempty"`
// MBZReleaseTrackID is the MusicBrainz release track ID.
MBZReleaseTrackID string `json:"mbzReleaseTrackId,omitempty"`
// Path is the full path to the track file, relative to the library root.
// Only included if the plugin has library permission with filesystem access for the track's library.
Path string `json:"path,omitempty"`
}
// Lyrics requires all methods to be implemented.

View file

@ -92,6 +92,9 @@ type TrackInfo struct {
MBZReleaseGroupID string `json:"mbzReleaseGroupId,omitempty"`
// MBZReleaseTrackID is the MusicBrainz release track ID.
MBZReleaseTrackID string `json:"mbzReleaseTrackId,omitempty"`
// Path is the full path to the track file, relative to the library root.
// Only included if the plugin has library permission with filesystem access for the track's library.
Path string `json:"path,omitempty"`
}
// Scrobbler requires all methods to be implemented.

View file

@ -89,6 +89,9 @@ type TrackInfo struct {
MBZReleaseGroupID string `json:"mbzReleaseGroupId,omitempty"`
// MBZReleaseTrackID is the MusicBrainz release track ID.
MBZReleaseTrackID string `json:"mbzReleaseTrackId,omitempty"`
// Path is the full path to the track file, relative to the library root.
// Only included if the plugin has library permission with filesystem access for the track's library.
Path string `json:"path,omitempty"`
}
// Scrobbler requires all methods to be implemented.

View file

@ -102,6 +102,10 @@ pub struct TrackInfo {
/// MBZReleaseTrackID is the MusicBrainz release track ID.
#[serde(default, skip_serializing_if = "String::is_empty")]
pub mbz_release_track_id: String,
/// Path is the full path to the track file, relative to the library root.
/// Only included if the plugin has library permission with filesystem access for the track's library.
#[serde(default, skip_serializing_if = "String::is_empty")]
pub path: String,
}
/// Error represents an error from a capability method.

View file

@ -122,6 +122,10 @@ pub struct TrackInfo {
/// MBZReleaseTrackID is the MusicBrainz release track ID.
#[serde(default, skip_serializing_if = "String::is_empty")]
pub mbz_release_track_id: String,
/// Path is the full path to the track file, relative to the library root.
/// Only included if the plugin has library permission with filesystem access for the track's library.
#[serde(default, skip_serializing_if = "String::is_empty")]
pub path: String,
}
/// Error represents an error from a capability method.

View file

@ -80,7 +80,7 @@ func (s *ScrobblerPlugin) NowPlaying(ctx context.Context, userId string, track *
username := getUsernameFromContext(ctx)
input := capabilities.NowPlayingRequest{
Username: username,
Track: mediaFileToTrackInfo(track),
Track: mediaFileToTrackInfo(s.plugin, track),
Position: int32(position),
}
@ -93,7 +93,7 @@ func (s *ScrobblerPlugin) Scrobble(ctx context.Context, userId string, sc scrobb
username := getUsernameFromContext(ctx)
input := capabilities.ScrobbleRequest{
Username: username,
Track: mediaFileToTrackInfo(&sc.MediaFile),
Track: mediaFileToTrackInfo(s.plugin, &sc.MediaFile),
Timestamp: sc.TimeStamp.Unix(),
}
@ -109,9 +109,11 @@ func getUsernameFromContext(ctx context.Context) string {
return ""
}
// mediaFileToTrackInfo converts a model.MediaFile to capabilities.TrackInfo
func mediaFileToTrackInfo(mf *model.MediaFile) capabilities.TrackInfo {
return capabilities.TrackInfo{
// mediaFileToTrackInfo converts a model.MediaFile to capabilities.TrackInfo.
// Path is populated only when the plugin is allowed filesystem access to the
// track's library.
func mediaFileToTrackInfo(p *plugin, mf *model.MediaFile) capabilities.TrackInfo {
ti := capabilities.TrackInfo{
ID: mf.ID,
Title: mf.Title,
Album: mf.Album,
@ -127,6 +129,10 @@ func mediaFileToTrackInfo(mf *model.MediaFile) capabilities.TrackInfo {
MBZReleaseGroupID: mf.MbzReleaseGroupID,
MBZReleaseTrackID: mf.MbzReleaseTrackID,
}
if p.hasLibraryFilesystemAccess(mf.LibraryID) {
ti.Path = mf.Path
}
return ti
}
// participantsToArtistRefs converts a ParticipantList to a slice of ArtistRef

View file

@ -240,6 +240,40 @@ var _ = Describe("ScrobblerPlugin", Ordered, func() {
Expect(names).ToNot(ContainElement("test-metadata-agent"))
})
})
Describe("mediaFileToTrackInfo", func() {
var track *model.MediaFile
BeforeEach(func() {
track = &model.MediaFile{
ID: "track-1",
Title: "Test Song",
Path: "/music/test.flac",
LibraryID: 1,
}
})
fsManifest := &Manifest{
Permissions: &Permissions{
Library: &LibraryPermission{Filesystem: true},
},
}
It("includes Path when the plugin has filesystem access to the track's library", func() {
p := &plugin{manifest: fsManifest, libraries: newLibraryAccess([]int{1}, false)}
Expect(mediaFileToTrackInfo(p, track).Path).To(Equal("/music/test.flac"))
})
It("omits Path when the plugin lacks filesystem permission", func() {
p := &plugin{manifest: &Manifest{}, libraries: newLibraryAccess([]int{1}, false)}
Expect(mediaFileToTrackInfo(p, track).Path).To(BeEmpty())
})
It("omits Path when the track's library is not in the allowed set", func() {
p := &plugin{manifest: fsManifest, libraries: newLibraryAccess([]int{2}, false)}
Expect(mediaFileToTrackInfo(p, track).Path).To(BeEmpty())
})
})
})
var _ = Describe("mapScrobblerError", func() {