diff --git a/model/metadata/persistent_ids.go b/model/metadata/persistent_ids.go index 70dfe0532..db315dc6b 100644 --- a/model/metadata/persistent_ids.go +++ b/model/metadata/persistent_ids.go @@ -12,88 +12,85 @@ import ( "github.com/navidrome/navidrome/model" "github.com/navidrome/navidrome/model/id" "github.com/navidrome/navidrome/utils" - "github.com/navidrome/navidrome/utils/slice" "github.com/navidrome/navidrome/utils/str" ) type hashFunc = func(...string) string -// createGetPID returns a function that calculates the persistent ID for a given spec, getting the referenced values from the metadata -// The spec is a pipe-separated list of fields, where each field is a comma-separated list of attributes -// Attributes can be either tags or some processed values like folder, albumid, albumartistid, etc. -// For each field, it gets all its attributes values and concatenates them, then hashes the result. -// If a field is empty, it is skipped and the function looks for the next field. -type getPIDFunc = func(mf model.MediaFile, md Metadata, spec string, prependLibId bool) string - -func createGetPID(hash hashFunc) getPIDFunc { - var getPID getPIDFunc - getAttr := func(mf model.MediaFile, md Metadata, attr string, prependLibId bool, spec string) string { - attr = strings.TrimSpace(strings.ToLower(attr)) - switch attr { - case "albumid": - if spec == conf.Server.PID.Album { - log.Error("Recursive PID definition detected, ignoring `albumid`", "spec", spec) - return "" +// computePID calculates the persistent ID for a given spec. The spec is a +// pipe-separated list of fields, where each field is a comma-separated list of +// attributes. Attributes can be either tags or processed values like folder, +// albumid, albumartistid, etc. For each field, it gets all its attribute values +// and concatenates them, then hashes the result. If a field is empty, it is +// skipped and the function looks for the next field. +// +// Taking hash as a parameter (instead of closing over it in a factory) keeps +// mf on the stack: closing over mf would force the whole ~1KB MediaFile to the +// heap on every call. +func computePID(mf model.MediaFile, md Metadata, spec string, prependLibId bool, hash hashFunc) string { + switch spec { + case "track_legacy": + return legacyTrackID(mf, prependLibId) + case "album_legacy": + return legacyAlbumID(mf, md, prependLibId) + } + pid := "" + fields := strings.SplitSeq(spec, "|") + for field := range fields { + attributes := strings.Split(field, ",") + values := make([]string, len(attributes)) + hasValue := false + for i, attr := range attributes { + v := getPIDAttr(mf, md, attr, prependLibId, spec, hash) + if v != "" { + hasValue = true } - return getPID(mf, md, conf.Server.PID.Album, prependLibId) - case "folder": - return filepath.Dir(mf.Path) - case "albumartistid": - return hash(str.Clear(strings.ToLower(mf.AlbumArtist))) - case "title": - return mf.Title - case "album": - return str.Clear(strings.ToLower(md.String(model.TagAlbum))) + values[i] = v + } + if hasValue { + pid += strings.Join(values, "\\") + break } - return md.String(model.TagName(attr)) } - getPID = func(mf model.MediaFile, md Metadata, spec string, prependLibId bool) string { - pid := "" - fields := strings.SplitSeq(spec, "|") - for field := range fields { - attributes := strings.Split(field, ",") - hasValue := false - values := slice.Map(attributes, func(attr string) string { - v := getAttr(mf, md, attr, prependLibId, spec) - if v != "" { - hasValue = true - } - return v - }) - if hasValue { - pid += strings.Join(values, "\\") - break - } - } - if prependLibId { - pid = fmt.Sprintf("%d\\%s", mf.LibraryID, pid) - } - return hash(pid) + if prependLibId { + pid = fmt.Sprintf("%d\\%s", mf.LibraryID, pid) } + return hash(pid) +} - return func(mf model.MediaFile, md Metadata, spec string, prependLibId bool) string { - switch spec { - case "track_legacy": - return legacyTrackID(mf, prependLibId) - case "album_legacy": - return legacyAlbumID(mf, md, prependLibId) +func getPIDAttr(mf model.MediaFile, md Metadata, attr string, prependLibId bool, spec string, hash hashFunc) string { + attr = strings.TrimSpace(strings.ToLower(attr)) + switch attr { + case "albumid": + if spec == conf.Server.PID.Album { + log.Error("Recursive PID definition detected, ignoring `albumid`", "spec", spec) + return "" } - return getPID(mf, md, spec, prependLibId) + return computePID(mf, md, conf.Server.PID.Album, prependLibId, hash) + case "folder": + return filepath.Dir(mf.Path) + case "albumartistid": + return hash(str.Clear(strings.ToLower(mf.AlbumArtist))) + case "title": + return mf.Title + case "album": + return str.Clear(strings.ToLower(md.String(model.TagAlbum))) } + return md.String(model.TagName(attr)) } func (md Metadata) trackPID(mf model.MediaFile) string { - return createGetPID(id.NewHash)(mf, md, conf.Server.PID.Track, true) + return computePID(mf, md, conf.Server.PID.Track, true, id.NewHash) } func (md Metadata) albumID(mf model.MediaFile, pidConf string) string { - return createGetPID(id.NewHash)(mf, md, pidConf, true) + return computePID(mf, md, pidConf, true, id.NewHash) } // BFR Must be configurable? func (md Metadata) artistID(name string) string { mf := model.MediaFile{AlbumArtist: name} - return createGetPID(id.NewHash)(mf, md, "albumartistid", false) + return computePID(mf, md, "albumartistid", false, id.NewHash) } func (md Metadata) mapTrackTitle() string { diff --git a/model/metadata/persistent_ids_test.go b/model/metadata/persistent_ids_test.go index 9f1dacbd4..47f5ca63f 100644 --- a/model/metadata/persistent_ids_test.go +++ b/model/metadata/persistent_ids_test.go @@ -12,15 +12,16 @@ import ( var _ = Describe("getPID", func() { var ( - md Metadata - mf model.MediaFile - sum hashFunc - getPID getPIDFunc + md Metadata + mf model.MediaFile + sum hashFunc ) + getPID := func(mf model.MediaFile, md Metadata, spec string, prependLibId bool) string { + return computePID(mf, md, spec, prependLibId, sum) + } BeforeEach(func() { sum = func(s ...string) string { return "(" + strings.Join(s, ",") + ")" } - getPID = createGetPID(sum) }) Context("attributes are tags", func() { diff --git a/server/subsonic/helpers.go b/server/subsonic/helpers.go index ffa10898e..74d57ade4 100644 --- a/server/subsonic/helpers.go +++ b/server/subsonic/helpers.go @@ -217,7 +217,7 @@ func childFromMediaFile(ctx context.Context, mf model.MediaFile) responses.Child child.Path = fakePath(mf) } child.DiscNumber = int32(mf.DiscNumber) - child.Created = &mf.BirthTime + child.Created = P(mf.BirthTime) child.AlbumId = mf.AlbumID child.ArtistId = mf.ArtistID child.Type = "music" @@ -407,9 +407,12 @@ func buildDiscSubtitles(a model.Album) []responses.DiscTitle { return nil } var discTitles []responses.DiscTitle + // Hoist UpdatedAt to a single stack-local so &updatedAt doesn't force the + // whole model.Album parameter onto the heap. + updatedAt := a.UpdatedAt for num, title := range a.Discs { artID := model.NewArtworkID(model.KindDiscArtwork, - model.DiscArtworkID(a.ID, num), &a.UpdatedAt) + model.DiscArtworkID(a.ID, num), &updatedAt) discTitles = append(discTitles, responses.DiscTitle{ Disc: int32(num), Title: title,