From 85e9982b434f27604f01817f45de006cddd18376 Mon Sep 17 00:00:00 2001 From: Jorge Pardo Pardo <78924065+J0R6IT0@users.noreply.github.com> Date: Sun, 12 Apr 2026 16:27:58 +0200 Subject: [PATCH] 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 * refactor(plugins): remove hasFilesystemPerm field and use manifest for filesystem permission checks Signed-off-by: Deluan * refactor(plugins): streamline library filesystem access checks in lyrics and scrobbler adapters Signed-off-by: Deluan --------- Signed-off-by: Deluan Co-authored-by: Deluan --- plugins/capabilities/lyrics.yaml | 5 +++ plugins/capabilities/scrobbler.go | 3 ++ plugins/capabilities/scrobbler.yaml | 5 +++ plugins/lyrics_adapter.go | 2 +- plugins/manager_loader.go | 3 +- plugins/manager_plugin.go | 28 +++++++++++++++ plugins/manager_plugin_test.go | 34 +++++++++++++++++++ plugins/manifest.go | 7 ++++ plugins/pdk/go/lyrics/lyrics.go | 3 ++ plugins/pdk/go/lyrics/lyrics_stub.go | 3 ++ plugins/pdk/go/scrobbler/scrobbler.go | 3 ++ plugins/pdk/go/scrobbler/scrobbler_stub.go | 3 ++ .../rust/nd-pdk-capabilities/src/lyrics.rs | 4 +++ .../rust/nd-pdk-capabilities/src/scrobbler.rs | 4 +++ plugins/scrobbler_adapter.go | 16 ++++++--- plugins/scrobbler_adapter_test.go | 34 +++++++++++++++++++ 16 files changed, 150 insertions(+), 7 deletions(-) create mode 100644 plugins/manager_plugin_test.go diff --git a/plugins/capabilities/lyrics.yaml b/plugins/capabilities/lyrics.yaml index e4f88476c..4ac907559 100644 --- a/plugins/capabilities/lyrics.yaml +++ b/plugins/capabilities/lyrics.yaml @@ -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 diff --git a/plugins/capabilities/scrobbler.go b/plugins/capabilities/scrobbler.go index 8091efe50..34cf60015 100644 --- a/plugins/capabilities/scrobbler.go +++ b/plugins/capabilities/scrobbler.go @@ -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. diff --git a/plugins/capabilities/scrobbler.yaml b/plugins/capabilities/scrobbler.yaml index 5de351a5f..f62da1745 100644 --- a/plugins/capabilities/scrobbler.yaml +++ b/plugins/capabilities/scrobbler.yaml @@ -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 diff --git a/plugins/lyrics_adapter.go b/plugins/lyrics_adapter.go index aa9930664..43ebc0e4b 100644 --- a/plugins/lyrics_adapter.go +++ b/plugins/lyrics_adapter.go @@ -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, diff --git a/plugins/manager_loader.go b/plugins/manager_loader.go index 59f48453f..ccda9e4cb 100644 --- a/plugins/manager_loader.go +++ b/plugins/manager_loader.go @@ -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() diff --git a/plugins/manager_plugin.go b/plugins/manager_plugin.go index 08c0073b6..1d4a8c301 100644 --- a/plugins/manager_plugin.go +++ b/plugins/manager_plugin.go @@ -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 +} diff --git a/plugins/manager_plugin_test.go b/plugins/manager_plugin_test.go new file mode 100644 index 000000000..513b8cb8e --- /dev/null +++ b/plugins/manager_plugin_test.go @@ -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()) + }) + }) +}) diff --git a/plugins/manifest.go b/plugins/manifest.go index 375e73e7f..7484718e3 100644 --- a/plugins/manifest.go +++ b/plugins/manifest.go @@ -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 +} diff --git a/plugins/pdk/go/lyrics/lyrics.go b/plugins/pdk/go/lyrics/lyrics.go index 4f5aa6302..188371fee 100644 --- a/plugins/pdk/go/lyrics/lyrics.go +++ b/plugins/pdk/go/lyrics/lyrics.go @@ -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. diff --git a/plugins/pdk/go/lyrics/lyrics_stub.go b/plugins/pdk/go/lyrics/lyrics_stub.go index 1fdf184e5..91eec4997 100644 --- a/plugins/pdk/go/lyrics/lyrics_stub.go +++ b/plugins/pdk/go/lyrics/lyrics_stub.go @@ -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. diff --git a/plugins/pdk/go/scrobbler/scrobbler.go b/plugins/pdk/go/scrobbler/scrobbler.go index c694f59d8..e16bfed4b 100644 --- a/plugins/pdk/go/scrobbler/scrobbler.go +++ b/plugins/pdk/go/scrobbler/scrobbler.go @@ -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. diff --git a/plugins/pdk/go/scrobbler/scrobbler_stub.go b/plugins/pdk/go/scrobbler/scrobbler_stub.go index 6d4afd818..86a71af03 100644 --- a/plugins/pdk/go/scrobbler/scrobbler_stub.go +++ b/plugins/pdk/go/scrobbler/scrobbler_stub.go @@ -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. diff --git a/plugins/pdk/rust/nd-pdk-capabilities/src/lyrics.rs b/plugins/pdk/rust/nd-pdk-capabilities/src/lyrics.rs index 16882abae..fcfe553f8 100644 --- a/plugins/pdk/rust/nd-pdk-capabilities/src/lyrics.rs +++ b/plugins/pdk/rust/nd-pdk-capabilities/src/lyrics.rs @@ -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. diff --git a/plugins/pdk/rust/nd-pdk-capabilities/src/scrobbler.rs b/plugins/pdk/rust/nd-pdk-capabilities/src/scrobbler.rs index 2572712d1..dd42e6803 100644 --- a/plugins/pdk/rust/nd-pdk-capabilities/src/scrobbler.rs +++ b/plugins/pdk/rust/nd-pdk-capabilities/src/scrobbler.rs @@ -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. diff --git a/plugins/scrobbler_adapter.go b/plugins/scrobbler_adapter.go index 874c6603a..4f7cd4661 100644 --- a/plugins/scrobbler_adapter.go +++ b/plugins/scrobbler_adapter.go @@ -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 diff --git a/plugins/scrobbler_adapter_test.go b/plugins/scrobbler_adapter_test.go index ab8dc6f88..0ee229022 100644 --- a/plugins/scrobbler_adapter_test.go +++ b/plugins/scrobbler_adapter_test.go @@ -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() {