From 32bae38ffdad359d1d4afdaee0b37012b261f688 Mon Sep 17 00:00:00 2001 From: Deluan Date: Wed, 14 Jan 2026 10:23:39 -0500 Subject: [PATCH] feat: introduce ArtistRef struct for better artist representation and update track metadata handling Signed-off-by: Deluan --- plugins/capabilities/metadata_agent.go | 10 ---- plugins/capabilities/scrobbler.go | 22 ++++++--- plugins/capabilities/scrobbler.yaml | 36 ++++++++++---- plugins/cmd/ndpgen/internal/parser.go | 48 +++++++++++-------- plugins/pdk/go/scrobbler/scrobbler.go | 22 ++++++--- plugins/pdk/go/scrobbler/scrobbler_stub.go | 22 ++++++--- .../rust/nd-pdk-capabilities/src/scrobbler.rs | 30 ++++++++---- plugins/scrobbler_adapter.go | 17 ++++++- plugins/scrobbler_adapter_test.go | 8 ++++ plugins/testdata/test-scrobbler/main.go | 12 ++++- 10 files changed, 159 insertions(+), 68 deletions(-) diff --git a/plugins/capabilities/metadata_agent.go b/plugins/capabilities/metadata_agent.go index 29e11da88..fbe89a2be 100644 --- a/plugins/capabilities/metadata_agent.go +++ b/plugins/capabilities/metadata_agent.go @@ -90,16 +90,6 @@ type SimilarArtistsRequest struct { Limit int32 `json:"limit"` } -// ArtistRef is a reference to an artist with name and optional MBID. -type ArtistRef struct { - // ID is the internal Navidrome artist ID (if known). - ID string `json:"id,omitempty"` - // Name is the artist name. - Name string `json:"name"` - // MBID is the MusicBrainz ID for the artist. - MBID string `json:"mbid,omitempty"` -} - // SimilarArtistsResponse is the response for GetSimilarArtists. type SimilarArtistsResponse struct { // Artists is the list of similar artists. diff --git a/plugins/capabilities/scrobbler.go b/plugins/capabilities/scrobbler.go index 5ba091bff..300652cb3 100644 --- a/plugins/capabilities/scrobbler.go +++ b/plugins/capabilities/scrobbler.go @@ -28,6 +28,16 @@ type IsAuthorizedRequest struct { Username string `json:"username"` } +// ArtistRef is a reference to an artist with name and optional MBID. +type ArtistRef struct { + // ID is the internal Navidrome artist ID (if known). + ID string `json:"id,omitempty"` + // Name is the artist name. + Name string `json:"name"` + // MBID is the MusicBrainz ID for the artist. + MBID string `json:"mbid,omitempty"` +} + // TrackInfo contains track metadata for scrobbling. type TrackInfo struct { // ID is the internal Navidrome track ID. @@ -36,10 +46,14 @@ type TrackInfo struct { Title string `json:"title"` // Album is the album name. Album string `json:"album"` - // Artist is the track artist. + // Artist is the formatted artist name for display (e.g., "Artist1 • Artist2"). Artist string `json:"artist"` - // AlbumArtist is the album artist. + // AlbumArtist is the formatted album artist name for display. AlbumArtist string `json:"albumArtist"` + // Artists is the list of track artists. + Artists []ArtistRef `json:"artists"` + // AlbumArtists is the list of album artists. + AlbumArtists []ArtistRef `json:"albumArtists"` // Duration is the track duration in seconds. Duration float32 `json:"duration"` // TrackNumber is the track number on the album. @@ -50,12 +64,8 @@ type TrackInfo struct { MBZRecordingID string `json:"mbzRecordingId,omitempty"` // MBZAlbumID is the MusicBrainz album/release ID. MBZAlbumID string `json:"mbzAlbumId,omitempty"` - // MBZArtistID is the MusicBrainz artist ID. - MBZArtistID string `json:"mbzArtistId,omitempty"` // MBZReleaseGroupID is the MusicBrainz release group ID. MBZReleaseGroupID string `json:"mbzReleaseGroupId,omitempty"` - // MBZAlbumArtistID is the MusicBrainz album artist ID. - MBZAlbumArtistID string `json:"mbzAlbumArtistId,omitempty"` // MBZReleaseTrackID is the MusicBrainz release track ID. MBZReleaseTrackID string `json:"mbzReleaseTrackId,omitempty"` } diff --git a/plugins/capabilities/scrobbler.yaml b/plugins/capabilities/scrobbler.yaml index b4dd2d1fa..d8f47c951 100644 --- a/plugins/capabilities/scrobbler.yaml +++ b/plugins/capabilities/scrobbler.yaml @@ -20,6 +20,20 @@ exports: contentType: application/json components: schemas: + ArtistRef: + description: ArtistRef is a reference to an artist with name and optional MBID. + properties: + id: + type: string + description: ID is the internal Navidrome artist ID (if known). + name: + type: string + description: Name is the artist name. + mbid: + type: string + description: MBID is the MusicBrainz ID for the artist. + required: + - name IsAuthorizedRequest: description: IsAuthorizedRequest is the request for authorization check. properties: @@ -76,10 +90,20 @@ components: description: Album is the album name. artist: type: string - description: Artist is the track artist. + description: Artist is the formatted artist name for display (e.g., "Artist1 • Artist2"). albumArtist: type: string - description: AlbumArtist is the album artist. + description: AlbumArtist is the formatted album artist name for display. + artists: + type: array + description: Artists is the list of track artists. + items: + $ref: '#/components/schemas/ArtistRef' + albumArtists: + type: array + description: AlbumArtists is the list of album artists. + items: + $ref: '#/components/schemas/ArtistRef' duration: type: number format: float @@ -98,15 +122,9 @@ components: mbzAlbumId: type: string description: MBZAlbumID is the MusicBrainz album/release ID. - mbzArtistId: - type: string - description: MBZArtistID is the MusicBrainz artist ID. mbzReleaseGroupId: type: string description: MBZReleaseGroupID is the MusicBrainz release group ID. - mbzAlbumArtistId: - type: string - description: MBZAlbumArtistID is the MusicBrainz album artist ID. mbzReleaseTrackId: type: string description: MBZReleaseTrackID is the MusicBrainz release track ID. @@ -116,6 +134,8 @@ components: - album - artist - albumArtist + - artists + - albumArtists - duration - trackNumber - discNumber diff --git a/plugins/cmd/ndpgen/internal/parser.go b/plugins/cmd/ndpgen/internal/parser.go index 63bc81380..4cb28f8d4 100644 --- a/plugins/cmd/ndpgen/internal/parser.go +++ b/plugins/cmd/ndpgen/internal/parser.go @@ -64,9 +64,14 @@ func ParseCapabilities(dir string) ([]Capability, error) { return nil, fmt.Errorf("reading directory: %w", err) } - var capabilities []Capability fset := token.NewFileSet() + // First pass: collect all structs and type aliases from all files in the package + sharedStructMap := make(map[string]StructDef) + sharedAliasMap := make(map[string]TypeAlias) + var allConstGroups []ConstGroup + + var goFiles []string for _, entry := range entries { if entry.IsDir() || !strings.HasSuffix(entry.Name(), ".go") { continue @@ -77,11 +82,29 @@ func ParseCapabilities(dir string) ([]Capability, error) { entry.Name() == "doc.go" { continue } + goFiles = append(goFiles, filepath.Join(dir, entry.Name())) + } - path := filepath.Join(dir, entry.Name()) - parsed, err := parseCapabilityFile(fset, path) + for _, path := range goFiles { + f, err := parser.ParseFile(fset, path, nil, parser.ParseComments) if err != nil { - return nil, fmt.Errorf("parsing %s: %w", entry.Name(), err) + return nil, fmt.Errorf("parsing %s for types: %w", filepath.Base(path), err) + } + for _, s := range parseStructs(f) { + sharedStructMap[s.Name] = s + } + for _, a := range parseTypeAliases(f) { + sharedAliasMap[a.Name] = a + } + allConstGroups = append(allConstGroups, parseConstGroups(f)...) + } + + // Second pass: parse capabilities using the shared type maps + var capabilities []Capability + for _, path := range goFiles { + parsed, err := parseCapabilityFile(fset, path, sharedStructMap, sharedAliasMap, allConstGroups) + if err != nil { + return nil, fmt.Errorf("parsing %s: %w", filepath.Base(path), err) } capabilities = append(capabilities, parsed...) } @@ -90,27 +113,12 @@ func ParseCapabilities(dir string) ([]Capability, error) { } // parseCapabilityFile parses a single Go source file and extracts capabilities. -func parseCapabilityFile(fset *token.FileSet, path string) ([]Capability, error) { +func parseCapabilityFile(fset *token.FileSet, path string, structMap map[string]StructDef, aliasMap map[string]TypeAlias, allConstGroups []ConstGroup) ([]Capability, error) { f, err := parser.ParseFile(fset, path, nil, parser.ParseComments) if err != nil { return nil, err } - // First pass: collect all struct definitions in the file - allStructs := parseStructs(f) - structMap := make(map[string]StructDef) - for _, s := range allStructs { - structMap[s.Name] = s - } - - // Collect type aliases and consts - allTypeAliases := parseTypeAliases(f) - aliasMap := make(map[string]TypeAlias) - for _, a := range allTypeAliases { - aliasMap[a.Name] = a - } - allConstGroups := parseConstGroups(f) - var capabilities []Capability for _, decl := range f.Decls { diff --git a/plugins/pdk/go/scrobbler/scrobbler.go b/plugins/pdk/go/scrobbler/scrobbler.go index 7f57698d1..258b1b4c1 100644 --- a/plugins/pdk/go/scrobbler/scrobbler.go +++ b/plugins/pdk/go/scrobbler/scrobbler.go @@ -26,6 +26,16 @@ const ( // Error implements the error interface for ScrobblerError. func (e ScrobblerError) Error() string { return string(e) } +// ArtistRef is a reference to an artist with name and optional MBID. +type ArtistRef struct { + // ID is the internal Navidrome artist ID (if known). + ID string `json:"id,omitempty"` + // Name is the artist name. + Name string `json:"name"` + // MBID is the MusicBrainz ID for the artist. + MBID string `json:"mbid,omitempty"` +} + // IsAuthorizedRequest is the request for authorization check. type IsAuthorizedRequest struct { // Username is the username of the user. @@ -60,10 +70,14 @@ type TrackInfo struct { Title string `json:"title"` // Album is the album name. Album string `json:"album"` - // Artist is the track artist. + // Artist is the formatted artist name for display (e.g., "Artist1 • Artist2"). Artist string `json:"artist"` - // AlbumArtist is the album artist. + // AlbumArtist is the formatted album artist name for display. AlbumArtist string `json:"albumArtist"` + // Artists is the list of track artists. + Artists []ArtistRef `json:"artists"` + // AlbumArtists is the list of album artists. + AlbumArtists []ArtistRef `json:"albumArtists"` // Duration is the track duration in seconds. Duration float32 `json:"duration"` // TrackNumber is the track number on the album. @@ -74,12 +88,8 @@ type TrackInfo struct { MBZRecordingID string `json:"mbzRecordingId,omitempty"` // MBZAlbumID is the MusicBrainz album/release ID. MBZAlbumID string `json:"mbzAlbumId,omitempty"` - // MBZArtistID is the MusicBrainz artist ID. - MBZArtistID string `json:"mbzArtistId,omitempty"` // MBZReleaseGroupID is the MusicBrainz release group ID. MBZReleaseGroupID string `json:"mbzReleaseGroupId,omitempty"` - // MBZAlbumArtistID is the MusicBrainz album artist ID. - MBZAlbumArtistID string `json:"mbzAlbumArtistId,omitempty"` // MBZReleaseTrackID is the MusicBrainz release track ID. MBZReleaseTrackID string `json:"mbzReleaseTrackId,omitempty"` } diff --git a/plugins/pdk/go/scrobbler/scrobbler_stub.go b/plugins/pdk/go/scrobbler/scrobbler_stub.go index b3b665113..f2fc584ad 100644 --- a/plugins/pdk/go/scrobbler/scrobbler_stub.go +++ b/plugins/pdk/go/scrobbler/scrobbler_stub.go @@ -23,6 +23,16 @@ const ( // Error implements the error interface for ScrobblerError. func (e ScrobblerError) Error() string { return string(e) } +// ArtistRef is a reference to an artist with name and optional MBID. +type ArtistRef struct { + // ID is the internal Navidrome artist ID (if known). + ID string `json:"id,omitempty"` + // Name is the artist name. + Name string `json:"name"` + // MBID is the MusicBrainz ID for the artist. + MBID string `json:"mbid,omitempty"` +} + // IsAuthorizedRequest is the request for authorization check. type IsAuthorizedRequest struct { // Username is the username of the user. @@ -57,10 +67,14 @@ type TrackInfo struct { Title string `json:"title"` // Album is the album name. Album string `json:"album"` - // Artist is the track artist. + // Artist is the formatted artist name for display (e.g., "Artist1 • Artist2"). Artist string `json:"artist"` - // AlbumArtist is the album artist. + // AlbumArtist is the formatted album artist name for display. AlbumArtist string `json:"albumArtist"` + // Artists is the list of track artists. + Artists []ArtistRef `json:"artists"` + // AlbumArtists is the list of album artists. + AlbumArtists []ArtistRef `json:"albumArtists"` // Duration is the track duration in seconds. Duration float32 `json:"duration"` // TrackNumber is the track number on the album. @@ -71,12 +85,8 @@ type TrackInfo struct { MBZRecordingID string `json:"mbzRecordingId,omitempty"` // MBZAlbumID is the MusicBrainz album/release ID. MBZAlbumID string `json:"mbzAlbumId,omitempty"` - // MBZArtistID is the MusicBrainz artist ID. - MBZArtistID string `json:"mbzArtistId,omitempty"` // MBZReleaseGroupID is the MusicBrainz release group ID. MBZReleaseGroupID string `json:"mbzReleaseGroupId,omitempty"` - // MBZAlbumArtistID is the MusicBrainz album artist ID. - MBZAlbumArtistID string `json:"mbzAlbumArtistId,omitempty"` // MBZReleaseTrackID is the MusicBrainz release track ID. MBZReleaseTrackID string `json:"mbzReleaseTrackId,omitempty"` } diff --git a/plugins/pdk/rust/nd-pdk-capabilities/src/scrobbler.rs b/plugins/pdk/rust/nd-pdk-capabilities/src/scrobbler.rs index 1d879b749..7a777496d 100644 --- a/plugins/pdk/rust/nd-pdk-capabilities/src/scrobbler.rs +++ b/plugins/pdk/rust/nd-pdk-capabilities/src/scrobbler.rs @@ -12,6 +12,20 @@ pub const SCROBBLER_ERROR_NOT_AUTHORIZED: ScrobblerError = "scrobbler(not_author pub const SCROBBLER_ERROR_RETRY_LATER: ScrobblerError = "scrobbler(retry_later)"; /// ScrobblerErrorUnrecoverable indicates an unrecoverable error. pub const SCROBBLER_ERROR_UNRECOVERABLE: ScrobblerError = "scrobbler(unrecoverable)"; +/// ArtistRef is a reference to an artist with name and optional MBID. +#[derive(Debug, Clone, Default, Serialize, Deserialize)] +#[serde(rename_all = "camelCase")] +pub struct ArtistRef { + /// ID is the internal Navidrome artist ID (if known). + #[serde(default, skip_serializing_if = "String::is_empty")] + pub id: String, + /// Name is the artist name. + #[serde(default)] + pub name: String, + /// MBID is the MusicBrainz ID for the artist. + #[serde(default, skip_serializing_if = "String::is_empty")] + pub mbid: String, +} /// IsAuthorizedRequest is the request for authorization check. #[derive(Debug, Clone, Default, Serialize, Deserialize)] #[serde(rename_all = "camelCase")] @@ -61,12 +75,18 @@ pub struct TrackInfo { /// Album is the album name. #[serde(default)] pub album: String, - /// Artist is the track artist. + /// Artist is the formatted artist name for display (e.g., "Artist1 • Artist2"). #[serde(default)] pub artist: String, - /// AlbumArtist is the album artist. + /// AlbumArtist is the formatted album artist name for display. #[serde(default)] pub album_artist: String, + /// Artists is the list of track artists. + #[serde(default)] + pub artists: Vec, + /// AlbumArtists is the list of album artists. + #[serde(default)] + pub album_artists: Vec, /// Duration is the track duration in seconds. #[serde(default)] pub duration: f32, @@ -82,15 +102,9 @@ pub struct TrackInfo { /// MBZAlbumID is the MusicBrainz album/release ID. #[serde(default, skip_serializing_if = "String::is_empty")] pub mbz_album_id: String, - /// MBZArtistID is the MusicBrainz artist ID. - #[serde(default, skip_serializing_if = "String::is_empty")] - pub mbz_artist_id: String, /// MBZReleaseGroupID is the MusicBrainz release group ID. #[serde(default, skip_serializing_if = "String::is_empty")] pub mbz_release_group_id: String, - /// MBZAlbumArtistID is the MusicBrainz album artist ID. - #[serde(default, skip_serializing_if = "String::is_empty")] - pub mbz_album_artist_id: String, /// MBZReleaseTrackID is the MusicBrainz release track ID. #[serde(default, skip_serializing_if = "String::is_empty")] pub mbz_release_track_id: String, diff --git a/plugins/scrobbler_adapter.go b/plugins/scrobbler_adapter.go index 4ade70d72..874c6603a 100644 --- a/plugins/scrobbler_adapter.go +++ b/plugins/scrobbler_adapter.go @@ -117,18 +117,31 @@ func mediaFileToTrackInfo(mf *model.MediaFile) capabilities.TrackInfo { Album: mf.Album, Artist: mf.Artist, AlbumArtist: mf.AlbumArtist, + Artists: participantsToArtistRefs(mf.Participants[model.RoleArtist]), + AlbumArtists: participantsToArtistRefs(mf.Participants[model.RoleAlbumArtist]), Duration: mf.Duration, TrackNumber: int32(mf.TrackNumber), DiscNumber: int32(mf.DiscNumber), MBZRecordingID: mf.MbzRecordingID, MBZAlbumID: mf.MbzAlbumID, - MBZArtistID: mf.MbzArtistID, MBZReleaseGroupID: mf.MbzReleaseGroupID, - MBZAlbumArtistID: mf.MbzAlbumArtistID, MBZReleaseTrackID: mf.MbzReleaseTrackID, } } +// participantsToArtistRefs converts a ParticipantList to a slice of ArtistRef +func participantsToArtistRefs(participants model.ParticipantList) []capabilities.ArtistRef { + refs := make([]capabilities.ArtistRef, len(participants)) + for i, p := range participants { + refs[i] = capabilities.ArtistRef{ + ID: p.ID, + Name: p.Name, + MBID: p.MbzArtistID, + } + } + return refs +} + // mapScrobblerError converts plugin errors to scrobbler errors based on error message, as errors are returned as // strings from plugins. func mapScrobblerError(err error) error { diff --git a/plugins/scrobbler_adapter_test.go b/plugins/scrobbler_adapter_test.go index 9421837b1..05fc11757 100644 --- a/plugins/scrobbler_adapter_test.go +++ b/plugins/scrobbler_adapter_test.go @@ -117,6 +117,10 @@ var _ = Describe("ScrobblerPlugin", Ordered, func() { Duration: 180, TrackNumber: 1, DiscNumber: 1, + Participants: model.Participants{ + model.RoleArtist: {{Artist: model.Artist{ID: "artist-1", Name: "Test Artist"}}}, + model.RoleAlbumArtist: {{Artist: model.Artist{ID: "album-artist-1", Name: "Test Album Artist"}}}, + }, } err := s.NowPlaying(ctxWithUser(), "user-1", track, 30) @@ -150,6 +154,10 @@ var _ = Describe("ScrobblerPlugin", Ordered, func() { Duration: 180, TrackNumber: 1, DiscNumber: 1, + Participants: model.Participants{ + model.RoleArtist: {{Artist: model.Artist{ID: "artist-1", Name: "Test Artist"}}}, + model.RoleAlbumArtist: {{Artist: model.Artist{ID: "album-artist-1", Name: "Test Album Artist"}}}, + }, }, TimeStamp: time.Now(), } diff --git a/plugins/testdata/test-scrobbler/main.go b/plugins/testdata/test-scrobbler/main.go index defb37316..d9c142d51 100644 --- a/plugins/testdata/test-scrobbler/main.go +++ b/plugins/testdata/test-scrobbler/main.go @@ -29,7 +29,11 @@ func (t *testScrobbler) NowPlaying(input scrobbler.NowPlayingRequest) error { } // Log the now playing (for potential debugging) - pdk.Log(pdk.LogInfo, "NowPlaying: "+input.Track.Title+" by "+input.Track.Artist) + artistName := "" + if len(input.Track.Artists) > 0 { + artistName = input.Track.Artists[0].Name + } + pdk.Log(pdk.LogInfo, "NowPlaying: "+input.Track.Title+" by "+artistName) return nil } @@ -41,7 +45,11 @@ func (t *testScrobbler) Scrobble(input scrobbler.ScrobbleRequest) error { } // Log the scrobble (for potential debugging) - pdk.Log(pdk.LogInfo, "Scrobble: "+input.Track.Title+" by "+input.Track.Artist) + artistName := "" + if len(input.Track.Artists) > 0 { + artistName = input.Track.Artists[0].Name + } + pdk.Log(pdk.LogInfo, "Scrobble: "+input.Track.Title+" by "+artistName) return nil }