feat(subsonic): add OpenSubsonic work and movement attributes (#5659)

* feat(subsonic): add Work/Movement response types and tag constants

* feat(subsonic): surface works and movements in Child response

* test(subsonic): verify works/movements JSON serialization

Fix G109 lint: use strconv.ParseInt with bitSize=32 to avoid potential
integer overflow; add JSON serialization test confirming omitempty on
optional sub-fields.

* refactor(subsonic): use number.ParseInt idiom in buildMovements

* refactor(subsonic): move work/movement builders to MediaFile methods

Introduces model.Work and model.Movement types with Works()/Movements()
methods on MediaFile. The Subsonic layer maps them to response types inline
via slice.Map, replacing the deleted buildWorks/buildMovements helpers.

* test(subsonic): cover populated works/movements in response snapshots

* test(subsonic): clarify empty-case name and assert JSON structurally
This commit is contained in:
Deluan Quintão 2026-06-24 09:10:00 -04:00 committed by GitHub
parent e6560ccb40
commit 56f0518830
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
12 changed files with 225 additions and 5 deletions

View File

@ -17,6 +17,7 @@ import (
"github.com/navidrome/navidrome/consts"
"github.com/navidrome/navidrome/utils"
"github.com/navidrome/navidrome/utils/gg"
"github.com/navidrome/navidrome/utils/number"
"github.com/navidrome/navidrome/utils/slice"
)
@ -151,6 +152,55 @@ func (mf MediaFile) String() string {
return mf.Path
}
type Work struct {
Name string
MbzWorkID string
}
type Movement struct {
Name string
Number int32
Count int32
}
func (mf MediaFile) Works() []Work {
names := mf.Tags.Values(TagWork)
if len(names) == 0 {
return nil
}
ids := mf.Tags.Values(TagMusicBrainzWorkID)
works := make([]Work, 0, len(names))
for i, name := range names {
w := Work{Name: name}
if i < len(ids) {
w.MbzWorkID = ids[i]
}
works = append(works, w)
}
return works
}
func (mf MediaFile) Movements() []Movement {
names := mf.Tags.Values(TagMovementName)
if len(names) == 0 {
return nil
}
numbers := mf.Tags.Values(TagMovementNumber)
counts := mf.Tags.Values(TagMovementTotal)
movements := make([]Movement, 0, len(names))
for i, name := range names {
m := Movement{Name: name}
if i < len(numbers) {
m.Number = number.ParseInt[int32](numbers[i])
}
if i < len(counts) {
m.Count = number.ParseInt[int32](counts[i])
}
movements = append(movements, m)
}
return movements
}
// Hash returns a hash of the MediaFile based on its tags and audio properties
func (mf MediaFile) Hash() string {
opts := &hashstructure.HashOptions{

View File

@ -604,6 +604,69 @@ var _ = Describe("MediaFile", func() {
})
var _ = Describe("MediaFile.Works", func() {
It("returns nil when there are no work tags", func() {
mf := MediaFile{}
Expect(mf.Works()).To(BeNil())
})
It("pairs a work name with its MbzWorkID", func() {
mf := MediaFile{Tags: Tags{
TagWork: {"Symphony No. 5"},
TagMusicBrainzWorkID: {"abc-123"},
}}
Expect(mf.Works()).To(Equal([]Work{
{Name: "Symphony No. 5", MbzWorkID: "abc-123"},
}))
})
It("leaves MbzWorkID empty when no id is present", func() {
mf := MediaFile{Tags: Tags{TagWork: {"Symphony No. 5"}}}
Expect(mf.Works()).To(Equal([]Work{
{Name: "Symphony No. 5"},
}))
})
It("pairs by index and ignores extra ids", func() {
mf := MediaFile{Tags: Tags{
TagWork: {"Work A", "Work B"},
TagMusicBrainzWorkID: {"id-a"},
}}
Expect(mf.Works()).To(Equal([]Work{
{Name: "Work A", MbzWorkID: "id-a"},
{Name: "Work B"},
}))
})
})
var _ = Describe("MediaFile.Movements", func() {
It("returns nil when there are no movement tags", func() {
mf := MediaFile{}
Expect(mf.Movements()).To(BeNil())
})
It("builds a movement with name, number and count", func() {
mf := MediaFile{Tags: Tags{
TagMovementName: {"I. Allegro"},
TagMovementNumber: {"1"},
TagMovementTotal: {"4"},
}}
Expect(mf.Movements()).To(Equal([]Movement{
{Name: "I. Allegro", Number: 1, Count: 4},
}))
})
It("non-numeric number/count yields 0", func() {
mf := MediaFile{Tags: Tags{
TagMovementName: {"I. Allegro"},
TagMovementNumber: {"not-a-number"},
}}
Expect(mf.Movements()).To(Equal([]Movement{
{Name: "I. Allegro"},
}))
})
})
var _ = Describe("MediaFile.Hash", func() {
// Guards the upgrade guarantee: converting BPM/BitDepth from int to *int must not change hashes,
// or every file would be spuriously re-imported on the next scan.

View File

@ -192,6 +192,10 @@ const (
TagISRC TagName = "isrc"
TagBPM TagName = "bpm"
TagExplicitStatus TagName = "explicitstatus"
TagWork TagName = "work"
TagMovementName TagName = "movementname"
TagMovementNumber TagName = "movement"
TagMovementTotal TagName = "movementtotal"
// Dates and years
@ -240,6 +244,7 @@ const (
TagMusicBrainzAlbumArtistID TagName = "musicbrainz_albumartistid"
TagMusicBrainzAlbumID TagName = "musicbrainz_albumid"
TagMusicBrainzReleaseGroupID TagName = "musicbrainz_releasegroupid"
TagMusicBrainzWorkID TagName = "musicbrainz_workid"
TagMusicBrainzComposerID TagName = "musicbrainz_composerid"
TagMusicBrainzLyricistID TagName = "musicbrainz_lyricistid"

View File

@ -290,6 +290,12 @@ func osChildFromMediaFile(ctx context.Context, mf model.MediaFile) *responses.Op
}
child.Contributors = contributors
child.ExplicitStatus = mapExplicitStatus(mf.ExplicitStatus)
child.Works = slice.Map(mf.Works(), func(w model.Work) responses.Work {
return responses.Work{Name: w.Name, MusicBrainzId: w.MbzWorkID}
})
child.Movements = slice.Map(mf.Movements(), func(m model.Movement) responses.Movement {
return responses.Movement{Name: m.Name, Number: m.Number, Count: m.Count}
})
return &child
}

View File

@ -2,6 +2,7 @@ package subsonic
import (
"context"
"encoding/json"
"net/http/httptest"
"time"
@ -358,6 +359,52 @@ var _ = Describe("helpers", func() {
Expect(osChild).ToNot(BeNil())
Expect(osChild.Comment).To(Equal("Test Comment"))
})
It("populates works and movements from tags", func() {
mf.Tags = model.Tags{
model.TagWork: {"Symphony No. 5"},
model.TagMusicBrainzWorkID: {"abc-123"},
model.TagMovementName: {"I. Allegro"},
model.TagMovementNumber: {"1"},
model.TagMovementTotal: {"4"},
}
osChild := osChildFromMediaFile(ctx, mf)
Expect(osChild).ToNot(BeNil())
Expect(osChild.Works).To(Equal(responses.Array[responses.Work]{
{Name: "Symphony No. 5", MusicBrainzId: "abc-123"},
}))
Expect(osChild.Movements).To(Equal(responses.Array[responses.Movement]{
{Name: "I. Allegro", Number: 1, Count: 4},
}))
})
It("returns empty works and movements when no classical tags are present", func() {
osChild := osChildFromMediaFile(ctx, mf)
Expect(osChild).ToNot(BeNil())
Expect(osChild.Works).To(BeEmpty())
Expect(osChild.Movements).To(BeEmpty())
})
It("serializes works and movements to spec-compliant JSON", func() {
mf.Tags = model.Tags{
model.TagWork: {"Symphony No. 5"},
model.TagMovementName: {"I. Allegro"},
model.TagMovementNumber: {"1"},
}
osChild := osChildFromMediaFile(ctx, mf)
data, err := json.Marshal(osChild)
Expect(err).ToNot(HaveOccurred())
var got map[string]any
Expect(json.Unmarshal(data, &got)).To(Succeed())
// Required name present; optional musicBrainzId/count omitted (omitempty); number present.
Expect(got).To(HaveKeyWithValue("works", []any{
map[string]any{"name": "Symphony No. 5"},
}))
Expect(got).To(HaveKeyWithValue("movements", []any{
map[string]any{"name": "I. Allegro", "number": float64(1)},
}))
})
})
Context("when legacy clients list is empty", func() {

View File

@ -59,7 +59,9 @@
"explicitStatus": "explicit",
"groupings": [
"Soundtrack"
]
],
"works": [],
"movements": []
}
]
}

View File

@ -171,7 +171,9 @@
"groupings": [
"Soundtrack",
"Live"
]
],
"works": [],
"movements": []
},
{
"id": "2",
@ -217,7 +219,9 @@
"contributors": [],
"displayComposer": "",
"explicitStatus": "",
"groupings": []
"groupings": [],
"works": [],
"movements": []
}
]
}

View File

@ -114,6 +114,22 @@
"groupings": [
"Soundtrack",
"Live"
],
"works": [
{
"name": "Symphony No. 5",
"musicBrainzId": "mbz-work-1"
},
{
"name": "Encore"
}
],
"movements": [
{
"name": "I. Allegro",
"number": 1,
"count": 4
}
]
},
{
@ -146,7 +162,9 @@
"contributors": [],
"displayComposer": "",
"explicitStatus": "",
"groupings": []
"groupings": [],
"works": [],
"movements": []
}
],
"id": "1",

View File

@ -26,6 +26,9 @@
</contributors>
<groupings>Soundtrack</groupings>
<groupings>Live</groupings>
<works name="Symphony No. 5" musicBrainzId="mbz-work-1"></works>
<works name="Encore"></works>
<movements name="I. Allegro" number="1" count="4"></movements>
</child>
<child id="" isDir="false" title="">
<replayGain trackGain="0" albumGain="0" trackPeak="0" albumPeak="0" baseGain="0" fallbackGain="0"></replayGain>

View File

@ -29,7 +29,9 @@
"contributors": [],
"displayComposer": "",
"explicitStatus": "",
"groupings": []
"groupings": [],
"works": [],
"movements": []
}
],
"id": "",

View File

@ -190,6 +190,8 @@ type OpenSubsonicChild struct {
DisplayComposer string `xml:"displayComposer,attr,omitempty" json:"displayComposer"`
ExplicitStatus string `xml:"explicitStatus,attr,omitempty" json:"explicitStatus"`
Groupings Array[string] `xml:"groupings,omitempty" json:"groupings"`
Works Array[Work] `xml:"works,omitempty" json:"works"`
Movements Array[Movement] `xml:"movements,omitempty" json:"movements"`
}
type Songs struct {
@ -598,6 +600,17 @@ type ItemGenre struct {
Name string `xml:"name,attr" json:"name"`
}
type Work struct {
Name string `xml:"name,attr" json:"name"`
MusicBrainzId string `xml:"musicBrainzId,attr,omitempty" json:"musicBrainzId,omitempty"`
}
type Movement struct {
Name string `xml:"name,attr" json:"name"`
Number int32 `xml:"number,attr,omitempty" json:"number,omitempty"`
Count int32 `xml:"count,attr,omitempty" json:"count,omitempty"`
}
type ReplayGain struct {
TrackGain *float64 `xml:"trackGain,omitempty,attr" json:"trackGain,omitempty"`
AlbumGain *float64 `xml:"albumGain,omitempty,attr" json:"albumGain,omitempty"`

View File

@ -239,6 +239,13 @@ var _ = Describe("Responses", func() {
{Role: "composer", Artist: ArtistID3Ref{Id: "4", Name: "composer2"}},
},
ExplicitStatus: "clean",
Works: []Work{
{Name: "Symphony No. 5", MusicBrainzId: "mbz-work-1"},
{Name: "Encore"},
},
Movements: []Movement{
{Name: "I. Allegro", Number: 1, Count: 4},
},
}
child[1].OpenSubsonicChild = &OpenSubsonicChild{
ReplayGain: ReplayGain{TrackGain: new(0.0), AlbumGain: new(0.0), TrackPeak: new(0.0), AlbumPeak: new(0.0), BaseGain: new(0.0), FallbackGain: new(0.0)},