Merge branch 'master' into feat-podcast-ux-improvements/5420

This commit is contained in:
Deluan Quintão 2026-08-19 19:49:32 -04:00 committed by GitHub
commit a5e3666754
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
50 changed files with 998 additions and 305 deletions

View File

@ -27,6 +27,9 @@ linters:
disable:
- staticcheck
settings:
errcheck:
exclude-functions:
- (*github.com/zeebo/xxh3.Hasher).Write
gocritic:
disable-all: true
enabled-checks:

View File

@ -59,31 +59,27 @@ var artworkCmd = &cobra.Command{
}
var artworkExplainCmd = &cobra.Command{
Use: "explain <kind> <id>",
Use: "explain [<kind>] <id>",
Short: "Explain why an item's artwork resolved the way it did",
Long: "Explain why an item's artwork resolved the way it did.\n\n" +
"The item can be given as a bare id, a full artwork id (e.g. al-<id>), or a <kind> <id> pair.\n" +
"<kind> is one of: " + kindPrefixes(explainKinds) + ".\n" +
"A disc artwork id is the album id and the disc number, joined by a colon: <albumID>:2",
Args: cobra.ExactArgs(2),
Args: cobra.RangeArgs(1, 2),
Run: func(cmd *cobra.Command, args []string) {
kind, err := parseArtworkKind(args[0], explainKinds)
if err != nil {
log.Fatal(cmd.Context(), err)
}
runExplain(cmd.Context(), kind, args[1])
runExplain(cmd.Context(), args)
},
}
var artworkRefreshCmd = &cobra.Command{
Use: "refresh <kind> <id>...",
Use: "refresh [<kind>] <id>...",
Short: "Clear an item's artwork state and re-resolve it",
Args: cobra.MinimumNArgs(2),
Long: "Clear an item's artwork state and re-resolve it.\n\n" +
"Each item can be given as a bare id, a full artwork id (e.g. al-<id>), or a shared\n" +
"<kind> <id>... leader. <kind> is one of: " + kindPrefixes(artwork.RefreshableKinds) + ".",
Args: cobra.MinimumNArgs(1),
Run: func(cmd *cobra.Command, args []string) {
kind, err := parseArtworkKind(args[0], artwork.RefreshableKinds)
if err != nil {
log.Fatal(cmd.Context(), err)
}
runRefresh(cmd.Context(), kind, args[1:])
runRefresh(cmd.Context(), args)
},
}
@ -499,19 +495,28 @@ func printReprocessPreview(out io.Writer, kinds []model.Kind, matched []int64, t
}
}
func runRefresh(ctx context.Context, kind model.Kind, ids []string) {
func runRefresh(ctx context.Context, args []string) {
defer db.Init(ctx)()
ds, ctx := getAdminContext(ctx)
if failed := refreshItems(ctx, ds, kind, ids, os.Stdout); failed > 0 {
log.Fatal(ctx, "Failed to refresh artwork", "kind", kind, "failed", failed, "total", len(ids))
targets, failures, err := resolveArtworkTargets(ctx, ds, args, artwork.RefreshableKinds)
if err != nil {
log.Fatal(ctx, err)
}
for _, f := range failures {
log.Error(ctx, "Skipping unresolved item", f)
}
failed := refreshItems(ctx, ds, targets, os.Stdout) + len(failures)
if failed > 0 {
log.Fatal(ctx, "Failed to refresh artwork", "failed", failed, "total", len(targets)+len(failures))
}
}
// refreshItems keeps going after a failure — the ids are independent — and returns how many failed.
func refreshItems(ctx context.Context, ds model.DataStore, kind model.Kind, ids []string, out io.Writer) int {
// refreshItems keeps going after a failure — the items are independent — and returns how many failed.
func refreshItems(ctx context.Context, ds model.DataStore, targets []model.ArtworkID, out io.Writer) int {
var failed int
for _, id := range ids {
for _, t := range targets {
kind, id := t.Kind, t.ID
// artwork.Refresh would happily queue an id that does not exist, orphaning a queue row.
if _, err := artworkItemName(ctx, ds, kind, id); err != nil {
log.Error(ctx, "Item not found", "kind", kind, "id", id, err)
@ -544,7 +549,56 @@ func parseArtworkKind(s string, valid []model.Kind) (model.Kind, error) {
if ok && slices.Contains(valid, kind) {
return kind, nil
}
return kind, fmt.Errorf("invalid kind %q, expected one of: %s", s, kindPrefixes(valid))
return kind, invalidKindErr(s, valid)
}
func invalidKindErr(s string, valid []model.Kind) error {
return fmt.Errorf("invalid kind %q, expected one of: %s", s, kindPrefixes(valid))
}
// resolveArtworkTargets resolves explain/refresh positional args into artwork ids, accepting a
// shared "<kind> <id>..." leader or self-describing args (a bare id, or a full artwork id). A
// self-describing arg that cannot be resolved is returned as a failure rather than aborting the
// batch, so refresh can process the resolvable ids; a malformed <kind> leader is a usage error.
func resolveArtworkTargets(ctx context.Context, ds model.DataStore, args []string, valid []model.Kind) ([]model.ArtworkID, []error, error) {
if kind, ok := model.ParseKind(args[0]); ok && len(args) > 1 {
if !slices.Contains(valid, kind) {
return nil, nil, invalidKindErr(args[0], valid)
}
return slice.Map(args[1:], func(id string) model.ArtworkID {
return model.ArtworkID{Kind: kind, ID: id}
}), nil, nil
}
var targets []model.ArtworkID
var failures []error
for _, arg := range args {
target, err := artworkKindAndID(ctx, ds, arg)
if err == nil && !slices.Contains(valid, target.Kind) {
err = invalidKindErr(target.Kind.Prefix(), valid)
}
if err != nil {
failures = append(failures, err)
continue
}
targets = append(targets, target)
}
return targets, failures, nil
}
// artworkKindAndID resolves one self-describing argument: a full artwork id (al-<id>) takes its kind
// from the prefix, a bare id is looked up. Entity ids never start with "<kind>-", so no collision.
func artworkKindAndID(ctx context.Context, ds model.DataStore, arg string) (model.ArtworkID, error) {
if artID, err := model.ParseArtworkID(arg); err == nil && artID.ID != "" {
return model.ArtworkID{Kind: artID.Kind, ID: artID.ID}, nil
}
kind, err := model.GetEntityKindByID(ctx, ds, arg)
if errors.Is(err, model.ErrNotFound) {
return model.ArtworkID{}, fmt.Errorf("could not determine kind for %q; pass an explicit <kind>", arg)
}
if err != nil {
return model.ArtworkID{}, err
}
return model.ArtworkID{Kind: kind, ID: arg}, nil
}
// explainAgents accounts for every configured agent: one the CLI cannot construct (a plugin, or a
@ -721,10 +775,22 @@ func formatTime(t time.Time) string {
return t.Format(time.RFC3339)
}
func runExplain(ctx context.Context, kind model.Kind, id string) {
func runExplain(ctx context.Context, args []string) {
defer db.Init(ctx)()
ds, ctx := getAdminContext(ctx)
targets, failures, err := resolveArtworkTargets(ctx, ds, args, explainKinds)
if err != nil {
log.Fatal(ctx, err)
}
if len(failures) > 0 {
log.Fatal(ctx, failures[0])
}
if len(targets) != 1 {
log.Fatal(ctx, "explain takes a single item; pass one id or a <kind> <id> pair")
}
kind, id := targets[0].Kind, targets[0].ID
name, err := artworkItemName(ctx, ds, kind, id)
if err != nil {
log.Fatal(ctx, "Item not found", "kind", kind, "id", id, err)

View File

@ -54,6 +54,74 @@ var _ = Describe("parseArtworkKind", func() {
})
})
var _ = Describe("resolveArtworkTargets", func() {
var ds *tests.MockDataStore
ctx := context.Background()
BeforeEach(func() {
artists := tests.CreateMockArtistRepo()
artists.SetData(model.Artists{{ID: "artist1"}})
ds = &tests.MockDataStore{MockedArtist: artists}
})
It("accepts the explicit <kind> <id> leader shared by every id", func() {
targets, failures, err := resolveArtworkTargets(ctx, ds, []string{"al", "x", "y"}, explainKinds)
Expect(err).ToNot(HaveOccurred())
Expect(failures).To(BeEmpty())
Expect(targets).To(Equal([]model.ArtworkID{
{Kind: model.KindAlbumArtwork, ID: "x"}, {Kind: model.KindAlbumArtwork, ID: "y"}}))
})
It("rejects an explicit kind the command does not accept as a usage error", func() {
_, _, err := resolveArtworkTargets(ctx, ds, []string{"dc", "x"}, artwork.RefreshableKinds)
Expect(err).To(MatchError(ContainSubstring("invalid kind")))
})
It("resolves a bare id by looking it up across tables", func() {
targets, failures, err := resolveArtworkTargets(ctx, ds, []string{"artist1"}, explainKinds)
Expect(err).ToNot(HaveOccurred())
Expect(failures).To(BeEmpty())
Expect(targets).To(Equal([]model.ArtworkID{{Kind: model.KindArtistArtwork, ID: "artist1"}}))
})
It("reads the kind from a full artwork id prefix without a database lookup", func() {
targets, _, err := resolveArtworkTargets(ctx, ds, []string{"al-realalbum"}, explainKinds)
Expect(err).ToNot(HaveOccurred())
Expect(targets).To(Equal([]model.ArtworkID{{Kind: model.KindAlbumArtwork, ID: "realalbum"}}))
})
It("strips the hash suffix from a full artwork id", func() {
targets, _, err := resolveArtworkTargets(ctx, ds, []string{"al-realalbum_0123456789abcdef"}, explainKinds)
Expect(err).ToNot(HaveOccurred())
Expect(targets).To(Equal([]model.ArtworkID{{Kind: model.KindAlbumArtwork, ID: "realalbum"}}))
})
It("collects a self-describing arg whose kind the command does not accept", func() {
targets, failures, err := resolveArtworkTargets(ctx, ds, []string{"dc-realalbum:2"}, artwork.RefreshableKinds)
Expect(err).ToNot(HaveOccurred())
Expect(targets).To(BeEmpty())
Expect(failures).To(HaveLen(1))
Expect(failures[0]).To(MatchError(ContainSubstring("invalid kind")))
})
It("collects an id that matches nothing and has no kind prefix", func() {
targets, failures, err := resolveArtworkTargets(ctx, ds, []string{"nope"}, explainKinds)
Expect(err).ToNot(HaveOccurred())
Expect(targets).To(BeEmpty())
Expect(failures).To(HaveLen(1))
Expect(failures[0]).To(MatchError(ContainSubstring("could not determine kind")))
})
It("resolves the valid ids and collects the unresolvable ones", func() {
targets, failures, err := resolveArtworkTargets(ctx, ds, []string{"artist1", "nope", "al-realalbum"}, explainKinds)
Expect(err).ToNot(HaveOccurred())
Expect(targets).To(Equal([]model.ArtworkID{
{Kind: model.KindArtistArtwork, ID: "artist1"}, {Kind: model.KindAlbumArtwork, ID: "realalbum"}}))
Expect(failures).To(HaveLen(1))
Expect(failures[0]).To(MatchError(ContainSubstring("could not determine kind")))
})
})
var _ = Describe("explainResult", func() {
It("reports the winning source", func() {
steps := []artwork.TraceStep{{Candidate: "folder", Outcome: "hit", Detail: "/music/a.jpg"}}
@ -337,10 +405,10 @@ var _ = Describe("discArtworkName", func() {
})
var _ = Describe("artwork refresh command", func() {
It("requires at least a kind and one id", func() {
Expect(artworkRefreshCmd.Args(artworkRefreshCmd, []string{"ar"})).To(HaveOccurred())
It("requires at least one argument", func() {
Expect(artworkRefreshCmd.Args(artworkRefreshCmd, []string{})).To(HaveOccurred())
Expect(artworkRefreshCmd.Args(artworkRefreshCmd, []string{"id1"})).ToNot(HaveOccurred())
Expect(artworkRefreshCmd.Args(artworkRefreshCmd, []string{"ar", "id1"})).ToNot(HaveOccurred())
Expect(artworkRefreshCmd.Args(artworkRefreshCmd, []string{"ar", "id1", "id2"})).ToNot(HaveOccurred())
})
})
@ -873,7 +941,8 @@ var _ = Describe("refreshItems", func() {
Expect(art.PutItemArtwork(&model.ItemArtwork{ItemKind: model.KindAlbumArtwork.Prefix(),
ItemID: "al-1", ImageType: model.ImageTypePrimary, Hash: "abc123"})).To(Succeed())
Expect(refreshItems(ctx, ds, model.KindAlbumArtwork, []string{"al-1", "al-3"}, &out)).To(BeZero())
Expect(refreshItems(ctx, ds, []model.ArtworkID{
{Kind: model.KindAlbumArtwork, ID: "al-1"}, {Kind: model.KindAlbumArtwork, ID: "al-3"}}, &out)).To(BeZero())
_, err := art.GetItemArtwork(model.KindAlbumArtwork, "al-1", model.ImageTypePrimary)
Expect(err).To(MatchError(model.ErrNotFound))
@ -884,7 +953,7 @@ var _ = Describe("refreshItems", func() {
})
It("skips an id that does not exist instead of queuing it", func() {
Expect(refreshItems(ctx, ds, model.KindAlbumArtwork, []string{"al-2"}, &out)).To(Equal(1))
Expect(refreshItems(ctx, ds, []model.ArtworkID{{Kind: model.KindAlbumArtwork, ID: "al-2"}}, &out)).To(Equal(1))
_, err := queue.Get(model.KindAlbumArtwork, "al-2", model.ImageTypePrimary)
Expect(err).To(MatchError(model.ErrNotFound), "a typo must not leave an orphan queue row")
@ -892,8 +961,8 @@ var _ = Describe("refreshItems", func() {
})
It("continues past a failing id and counts the failures", func() {
Expect(refreshItems(ctx, ds, model.KindAlbumArtwork,
[]string{"al-1", "al-2", "al-3"}, &out)).To(Equal(1))
Expect(refreshItems(ctx, ds, []model.ArtworkID{{Kind: model.KindAlbumArtwork, ID: "al-1"},
{Kind: model.KindAlbumArtwork, ID: "al-2"}, {Kind: model.KindAlbumArtwork, ID: "al-3"}}, &out)).To(Equal(1))
Expect(out.String()).To(Equal("al/al-1: queued\nal/al-3: queued\n"),
"the ids after a failure are still refreshed")

View File

@ -52,6 +52,12 @@ func Encode(img image.Image) (string, error) {
lin := srgbToLinearTable()
factors := make([][3]float64, xComp*yComp)
linR := make([]float64, w)
linG := make([]float64, w)
linB := make([]float64, w)
rowR := make([]float64, xComp)
rowG := make([]float64, xComp)
rowB := make([]float64, xComp)
for y := range h {
row := src.pix[y*src.stride:]
for x := range w {
@ -60,15 +66,26 @@ func Encode(img image.Image) (string, error) {
if src.straight {
r, g, b = premultiply(r, g, b, row[p+3])
}
lr, lg, lb := lin[r], lin[g], lin[b]
for j := range yComp {
for i := range xComp {
basis := cosX[i][x] * cosY[j][y]
f := &factors[j*xComp+i]
f[0] += basis * lr
f[1] += basis * lg
f[2] += basis * lb
}
linR[x], linG[x], linB[x] = lin[r], lin[g], lin[b]
}
// The basis is separable, so a row costs xComp dot products plus one fold over yComp,
// rather than xComp*yComp multiply-accumulates per pixel.
for i := range xComp {
var sr, sg, sb float64
for x, c := range cosX[i] {
sr += c * linR[x]
sg += c * linG[x]
sb += c * linB[x]
}
rowR[i], rowG[i], rowB[i] = sr, sg, sb
}
for j := range yComp {
cy := cosY[j][y]
for i := range xComp {
f := &factors[j*xComp+i]
f[0] += cy * rowR[i]
f[1] += cy * rowG[i]
f[2] += cy * rowB[i]
}
}
}

View File

@ -106,7 +106,7 @@ var _ = Describe("Acquisition → serve loop", func() {
func(context.Context, cache.Item) (io.Reader, error) {
return nil, errors.New("resize not exercised in e2e")
})
Eventually(func() bool { return imgCache.Available(ctx) }).Should(BeTrue())
Eventually(func() bool { return imgCache.Available(ctx) }, 10*time.Second).Should(BeTrue())
svc = artwork.NewArtwork(ds, imgCache, store, ffm)
worker = artwork.NewWorker(ds, store, agents.GetAgents(ds, nil), ffm, events.NoopBroker(), imgCache)

View File

@ -117,7 +117,7 @@ func setupResolutionHarness() {
func(context.Context, cache.Item) (io.Reader, error) {
return nil, fmt.Errorf("resize not exercised in e2e")
})
Eventually(func() bool { return imgCache.Available(rctx) }).Should(BeTrue())
Eventually(func() bool { return imgCache.Available(rctx) }, 10*time.Second).Should(BeTrue())
rsvc = artwork.NewArtwork(rds, imgCache, rstore, ffm)
rworker = artwork.NewWorker(rds, rstore, agents.GetAgents(rds, nil), ffm, events.NoopBroker(), imgCache)

View File

@ -2,8 +2,6 @@ package artwork
import (
"context"
"crypto/md5"
"encoding/hex"
"fmt"
"slices"
"strconv"
@ -16,6 +14,7 @@ import (
"github.com/navidrome/navidrome/log"
"github.com/navidrome/navidrome/model"
"github.com/navidrome/navidrome/utils/slice"
"github.com/zeebo/xxh3"
)
// StaleAbsentAge is how long an absent state is trusted before a recheck retries it.
@ -66,8 +65,7 @@ func FingerprintInputs() []FingerprintInput {
func ConfigFingerprint() string {
values := slice.Map(FingerprintInputs(), func(i FingerprintInput) string { return i.Value })
raw := fmt.Sprintf("%s|%d", strings.Join(values, "|"), artworkEpoch)
sum := md5.Sum([]byte(raw)) //nolint:gosec // fingerprint, not security-sensitive
return hex.EncodeToString(sum[:])
return fmt.Sprintf("%016x", xxh3.Hash([]byte(raw)))
}
// backfill enqueues artwork resolution for every entity when the config fingerprint changed.

View File

@ -135,7 +135,7 @@ var _ = Describe("Housekeeping", func() {
conf.Server.EnableExternalServices = true
conf.Server.EnableM3UExternalAlbumArt = false
Expect(ConfigFingerprint()).To(Equal("7e537a22febc07d3d5ca40546e88da54"))
Expect(ConfigFingerprint()).To(Equal("7b538a83a870c16d"))
})
It("reports the config inputs it hashes, so a change can be traced to a setting", func() {

View File

@ -2,6 +2,7 @@ package core
import (
"context"
"path/filepath"
. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"
@ -41,10 +42,9 @@ var _ = Describe("common.go", func() {
})
It("returns the absolute path when library exists", func() {
tests.SkipOnWindows("path separator bug (#TBD-path-sep-core)")
ctx := context.Background()
abs := AbsolutePath(ctx, ds, libId, path)
Expect(abs).To(Equal("/library/root/music/file.mp3"))
Expect(abs).To(Equal(filepath.FromSlash("/library/root/music/file.mp3")))
})
It("returns the original path if library not found", func() {

View File

@ -130,12 +130,13 @@ func (s *playlists) Create(ctx context.Context, playlistId string, name string,
if err != nil {
return err
}
if pls.IsSmartPlaylist() {
return model.ErrNotAuthorized
}
// Ownership first: a non-owner must get ErrNotAuthorized, not a read-only conflict.
if !usr.IsAdmin && pls.OwnerID != usr.ID {
return model.ErrNotAuthorized
}
if !pls.TracksEditable() {
return model.ErrPlaylistNotEditable
}
} else {
pls = &model.Playlist{Name: name}
pls.OwnerID = usr.ID
@ -230,14 +231,14 @@ func (s *playlists) checkWritable(ctx context.Context, id string) (*model.Playli
return pls, nil
}
// checkTracksEditable verifies the user can modify tracks (ownership + not smart playlist).
// checkTracksEditable verifies the user owns the playlist and its tracks are editable.
func (s *playlists) checkTracksEditable(ctx context.Context, playlistID string) (*model.Playlist, error) {
pls, err := s.checkWritable(ctx, playlistID)
if err != nil {
return nil, err
}
if pls.IsSmartPlaylist() {
return nil, model.ErrNotAuthorized
if !pls.TracksEditable() {
return nil, model.ErrPlaylistNotEditable
}
return pls, nil
}

View File

@ -102,6 +102,8 @@ var _ = Describe("Playlists", func() {
"pls-2": {ID: "pls-2", Name: "Other's", OwnerID: "other-user"},
"pls-smart": {ID: "pls-smart", Name: "Smart", OwnerID: "user-1",
Rules: &criteria.Criteria{Expression: criteria.Contains{"title": "test"}}},
"pls-synced": {ID: "pls-synced", Name: "Synced", OwnerID: "user-1", Sync: true},
"pls-synced-other": {ID: "pls-synced-other", Name: "Other's Synced", OwnerID: "other-user", Sync: true, Public: true},
}
ps = playlists.NewPlaylists(ds, artwork.NewUploader(ds))
})
@ -145,6 +147,18 @@ var _ = Describe("Playlists", func() {
It("denies replacing tracks on a smart playlist", func() {
ctx = request.WithUser(ctx, model.User{ID: "user-1", IsAdmin: false})
_, err := ps.Create(ctx, "pls-smart", "", []string{"song-1"})
Expect(err).To(MatchError(model.ErrPlaylistNotEditable))
})
It("denies replacing tracks on a synced playlist", func() {
ctx = request.WithUser(ctx, model.User{ID: "user-1", IsAdmin: false})
_, err := ps.Create(ctx, "pls-synced", "", []string{"song-1"})
Expect(err).To(MatchError(model.ErrPlaylistNotEditable))
})
It("denies a non-owner with authorization, not a conflict, on a public synced playlist", func() {
ctx = request.WithUser(ctx, model.User{ID: "user-1", IsAdmin: false})
_, err := ps.Create(ctx, "pls-synced-other", "", []string{"song-1"})
Expect(err).To(MatchError(model.ErrNotAuthorized))
})
})
@ -159,6 +173,7 @@ var _ = Describe("Playlists", func() {
"pls-other": {ID: "pls-other", Name: "Other's", OwnerID: "other-user"},
"pls-smart": {ID: "pls-smart", Name: "Smart", OwnerID: "user-1",
Rules: &criteria.Criteria{Expression: criteria.Contains{"title": "test"}}},
"pls-synced": {ID: "pls-synced", Name: "Synced", OwnerID: "user-1", Sync: true},
}
mockPlsRepo.TracksRepo = mockTracks
ps = playlists.NewPlaylists(ds, artwork.NewUploader(ds))
@ -191,13 +206,13 @@ var _ = Describe("Playlists", func() {
It("denies adding tracks to a smart playlist", func() {
ctx = request.WithUser(ctx, model.User{ID: "user-1", IsAdmin: false})
err := ps.Update(ctx, "pls-smart", nil, nil, nil, []string{"song-1"}, nil)
Expect(err).To(MatchError(model.ErrNotAuthorized))
Expect(err).To(MatchError(model.ErrPlaylistNotEditable))
})
It("denies removing tracks from a smart playlist", func() {
ctx = request.WithUser(ctx, model.User{ID: "user-1", IsAdmin: false})
err := ps.Update(ctx, "pls-smart", nil, nil, nil, nil, []int{0})
Expect(err).To(MatchError(model.ErrNotAuthorized))
Expect(err).To(MatchError(model.ErrPlaylistNotEditable))
})
It("allows metadata updates on a smart playlist", func() {
@ -205,6 +220,18 @@ var _ = Describe("Playlists", func() {
err := ps.Update(ctx, "pls-smart", new("Updated Smart"), nil, nil, nil, nil)
Expect(err).ToNot(HaveOccurred())
})
It("denies adding tracks to a synced playlist", func() {
ctx = request.WithUser(ctx, model.User{ID: "user-1", IsAdmin: false})
err := ps.Update(ctx, "pls-synced", nil, nil, nil, []string{"song-1"}, nil)
Expect(err).To(MatchError(model.ErrPlaylistNotEditable))
})
It("allows metadata updates on a synced playlist", func() {
ctx = request.WithUser(ctx, model.User{ID: "user-1", IsAdmin: false})
err := ps.Update(ctx, "pls-synced", new("Renamed Synced"), nil, nil, nil, nil)
Expect(err).ToNot(HaveOccurred())
})
})
Describe("AddTracks", func() {
@ -216,7 +243,8 @@ var _ = Describe("Playlists", func() {
"pls-1": {ID: "pls-1", Name: "My Playlist", OwnerID: "user-1"},
"pls-smart": {ID: "pls-smart", Name: "Smart", OwnerID: "user-1",
Rules: &criteria.Criteria{Expression: criteria.Contains{"title": "test"}}},
"pls-other": {ID: "pls-other", Name: "Other's", OwnerID: "other-user"},
"pls-other": {ID: "pls-other", Name: "Other's", OwnerID: "other-user"},
"pls-synced": {ID: "pls-synced", Name: "Synced", OwnerID: "user-1", Sync: true},
}
mockPlsRepo.TracksRepo = mockTracks
ps = playlists.NewPlaylists(ds, artwork.NewUploader(ds))
@ -246,7 +274,13 @@ var _ = Describe("Playlists", func() {
It("denies editing smart playlists", func() {
ctx = request.WithUser(ctx, model.User{ID: "user-1", IsAdmin: false})
_, err := ps.AddTracks(ctx, "pls-smart", []string{"song-1"})
Expect(err).To(MatchError(model.ErrNotAuthorized))
Expect(err).To(MatchError(model.ErrPlaylistNotEditable))
})
It("denies editing synced playlists", func() {
ctx = request.WithUser(ctx, model.User{ID: "user-1", IsAdmin: false})
_, err := ps.AddTracks(ctx, "pls-synced", []string{"song-1"})
Expect(err).To(MatchError(model.ErrPlaylistNotEditable))
})
It("returns error when playlist not found", func() {
@ -280,7 +314,7 @@ var _ = Describe("Playlists", func() {
It("denies on smart playlist", func() {
ctx = request.WithUser(ctx, model.User{ID: "user-1", IsAdmin: false})
err := ps.RemoveTracks(ctx, "pls-smart", []string{"track-1"})
Expect(err).To(MatchError(model.ErrNotAuthorized))
Expect(err).To(MatchError(model.ErrPlaylistNotEditable))
})
It("denies non-owner", func() {
@ -314,7 +348,7 @@ var _ = Describe("Playlists", func() {
It("denies on smart playlist", func() {
ctx = request.WithUser(ctx, model.User{ID: "user-1", IsAdmin: false})
err := ps.ReorderTrack(ctx, "pls-smart", 1, 3)
Expect(err).To(MatchError(model.ErrNotAuthorized))
Expect(err).To(MatchError(model.ErrPlaylistNotEditable))
})
})

View File

@ -7,7 +7,6 @@ import (
"runtime"
"testing"
"github.com/navidrome/navidrome/tests"
. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"
)
@ -56,13 +55,16 @@ var _ = Describe("Storage", func() {
Expect(s.(*fakeLocalStorage).u.Path).To(Equal("/tmp"))
})
It("should return a file implementation for a relative folder", func() {
tests.SkipOnWindows("path separator bug (#TBD-path-sep-storage)")
s, err := For("tmp")
Expect(err).ToNot(HaveOccurred())
cwd, _ := os.Getwd()
Expect(s).To(BeAssignableToTypeOf(&fakeLocalStorage{}))
Expect(s.(*fakeLocalStorage).u.Scheme).To(Equal("file"))
Expect(s.(*fakeLocalStorage).u.Path).To(Equal(filepath.Join(cwd, "tmp")))
u := s.(*fakeLocalStorage).u
Expect(u.Scheme).To(Equal("file"))
// On Windows the drive letter lands in u.Host, so re-join it with
// u.Path (as newLocalStorage does) to keep the assertion OS-independent.
got := filepath.Join(u.Host, filepath.FromSlash(u.Path))
Expect(got).To(Equal(filepath.Join(cwd, "tmp")))
})
It("should return error if schema is unregistered", func() {
_, err := For("webdav:///tmp")

View File

@ -5,6 +5,7 @@ import (
"errors"
"io"
"os"
"time"
"github.com/navidrome/navidrome/conf"
"github.com/navidrome/navidrome/conf/configtest"
@ -33,7 +34,7 @@ var _ = Describe("MediaStreamer", func() {
{ID: "123", Path: "tests/fixtures/test.mp3", Suffix: "mp3", BitRate: 128, Duration: 257.0},
})
testCache := stream.NewTranscodingCache()
Eventually(func() bool { return testCache.Available(context.TODO()) }).Should(BeTrue())
Eventually(func() bool { return testCache.Available(context.TODO()) }, 10*time.Second).Should(BeTrue())
streamer = stream.NewMediaStreamer(ds, ffmpeg, testCache)
})
AfterEach(func() {
@ -75,7 +76,7 @@ var _ = Describe("MediaStreamer", func() {
conf.Server.Transcoding.MaxConcurrent = 1
conf.Server.Transcoding.MaxConcurrentPerUser = 0
tightCache := stream.NewTranscodingCache()
Eventually(func() bool { return tightCache.Available(context.TODO()) }).Should(BeTrue())
Eventually(func() bool { return tightCache.Available(context.TODO()) }, 10*time.Second).Should(BeTrue())
tightStreamer := stream.NewMediaStreamer(ds, blockingFFmpeg, tightCache)
userCtx := request.WithUsername(ctx, "alice")
@ -92,7 +93,7 @@ var _ = Describe("MediaStreamer", func() {
conf.Server.Transcoding.MaxConcurrent = 1
conf.Server.Transcoding.MaxConcurrentPerUser = 0
tightCache := stream.NewTranscodingCache()
Eventually(func() bool { return tightCache.Available(context.TODO()) }).Should(BeTrue())
Eventually(func() bool { return tightCache.Available(context.TODO()) }, 10*time.Second).Should(BeTrue())
tightStreamer := stream.NewMediaStreamer(ds, ffmpeg, tightCache)
userCtx := request.WithUsername(ctx, "alice")
@ -112,7 +113,7 @@ var _ = Describe("MediaStreamer", func() {
conf.Server.Transcoding.MaxConcurrent = 1
conf.Server.Transcoding.MaxConcurrentPerUser = 0
tightCache := stream.NewTranscodingCache()
Eventually(func() bool { return tightCache.Available(context.TODO()) }).Should(BeTrue())
Eventually(func() bool { return tightCache.Available(context.TODO()) }, 10*time.Second).Should(BeTrue())
tightStreamer := stream.NewMediaStreamer(ds, ffmpeg, tightCache)
userCtx := request.WithUsername(ctx, "alice")

View File

@ -3,10 +3,11 @@ package model
import "errors"
var (
ErrNotFound = errors.New("data not found")
ErrInvalidAuth = errors.New("invalid authentication")
ErrNotAuthorized = errors.New("not authorized")
ErrExpired = errors.New("access expired")
ErrNotAvailable = errors.New("functionality not available")
ErrValidation = errors.New("validation error")
ErrNotFound = errors.New("data not found")
ErrInvalidAuth = errors.New("invalid authentication")
ErrNotAuthorized = errors.New("not authorized")
ErrExpired = errors.New("access expired")
ErrNotAvailable = errors.New("functionality not available")
ErrValidation = errors.New("validation error")
ErrPlaylistNotEditable = errors.New("playlist tracks are not editable")
)

View File

@ -7,21 +7,36 @@ import (
// TODO: Should the type be encoded in the ID?
func GetEntityByID(ctx context.Context, ds DataStore, id string) (any, error) {
getters := []func() (any, error){
func() (any, error) { return ds.Artist(ctx).Get(id) },
func() (any, error) { return ds.Album(ctx).Get(id) },
func() (any, error) { return ds.Playlist(ctx).Get(id) },
func() (any, error) { return ds.MediaFile(ctx).Get(id) },
func() (any, error) { return ds.Radio(ctx).Get(id) },
entity, _, err := getEntity(ctx, ds, id)
return entity, err
}
// GetEntityKindByID resolves a bare entity id to its artwork Kind, searching the same tables as
// GetEntityByID. It reports ErrNotFound when no entity owns the id.
func GetEntityKindByID(ctx context.Context, ds DataStore, id string) (Kind, error) {
_, kind, err := getEntity(ctx, ds, id)
return kind, err
}
func getEntity(ctx context.Context, ds DataStore, id string) (any, Kind, error) {
getters := []struct {
kind Kind
get func() (any, error)
}{
{KindArtistArtwork, func() (any, error) { return ds.Artist(ctx).Get(id) }},
{KindAlbumArtwork, func() (any, error) { return ds.Album(ctx).Get(id) }},
{KindPlaylistArtwork, func() (any, error) { return ds.Playlist(ctx).Get(id) }},
{KindMediaFileArtwork, func() (any, error) { return ds.MediaFile(ctx).Get(id) }},
{KindRadioArtwork, func() (any, error) { return ds.Radio(ctx).Get(id) }},
}
for _, get := range getters {
entity, err := get()
for _, g := range getters {
entity, err := g.get()
if err == nil {
return entity, nil
return entity, g.kind, nil
}
if !errors.Is(err, ErrNotFound) {
return nil, err
return nil, Kind{}, err
}
}
return nil, ErrNotFound
return nil, Kind{}, ErrNotFound
}

View File

@ -38,3 +38,25 @@ var _ = Describe("GetEntityByID", func() {
Expect(err).ToNot(MatchError(model.ErrNotFound))
})
})
var _ = Describe("GetEntityKindByID", func() {
var ds *tests.MockDataStore
var ctx context.Context
BeforeEach(func() {
ds = &tests.MockDataStore{}
ctx = GinkgoT().Context()
})
It("returns the artwork kind for the matching id", func() {
ds.Album(ctx).(*tests.MockAlbumRepo).SetData(model.Albums{{ID: "a1"}})
kind, err := model.GetEntityKindByID(ctx, ds, "a1")
Expect(err).ToNot(HaveOccurred())
Expect(kind).To(Equal(model.KindAlbumArtwork))
})
It("returns ErrNotFound when no entity matches", func() {
_, err := model.GetEntityKindByID(ctx, ds, "missing")
Expect(err).To(MatchError(model.ErrNotFound))
})
})

View File

@ -2,7 +2,6 @@ package model
import (
"cmp"
"crypto/md5"
"encoding/json"
"fmt"
"iter"
@ -20,6 +19,7 @@ import (
"github.com/navidrome/navidrome/utils/gg"
"github.com/navidrome/navidrome/utils/number"
"github.com/navidrome/navidrome/utils/slice"
"github.com/zeebo/xxh3"
)
type MediaFile struct {
@ -232,7 +232,7 @@ func (mf MediaFile) Hash() string {
ZeroNil: true,
}
hash, _ := hashstructure.Hash(mf, opts)
sum := md5.New()
sum := xxh3.New()
sum.Write(fmt.Appendf(nil, "%d", hash))
sum.Write(mf.Tags.Hash())
sum.Write(mf.Participants.Hash())

View File

@ -715,14 +715,10 @@ var _ = Describe("MediaFile.Movements", func() {
})
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.
// Golden hashes were captured at 46221d516 when those fields were plain ints.
It("keeps hashes identical to the pre-pointer-conversion values", func() {
// Golden hashes computed at 46221d516, when BPM/BitDepth were plain ints — pinning
// them guarantees the pointer conversion cannot trigger a full-library re-import.
Expect(MediaFile{Title: "Song"}.Hash()).To(Equal("1d856ced42cb96db39e354a4bac9a622"))
Expect(MediaFile{Title: "Song", BPM: new(120), BitDepth: new(16)}.Hash()).To(Equal("b2b0b1d1dd7fd767093588e4af3a0689"))
// Pins the hash formula: an accidental change spuriously re-imports every file on the next scan.
It("hashes to a stable value", func() {
Expect(MediaFile{Title: "Song"}.Hash()).To(Equal("05fdf70bb0cbe090"))
Expect(MediaFile{Title: "Song", BPM: new(120), BitDepth: new(16)}.Hash()).To(Equal("b5daf6ac1009a538"))
})
It("changes the hash when a pointer field has a value", func() {
base := MediaFile{Title: "Song"}

View File

@ -2,12 +2,12 @@ package model
import (
"cmp"
"crypto/md5"
"fmt"
"slices"
"strings"
"github.com/navidrome/navidrome/utils/slice"
"github.com/zeebo/xxh3"
)
var (
@ -193,7 +193,7 @@ func (p Participants) Hash() []byte {
flattened = append(flattened, role.String()+":"+strings.Join(ids, "/"))
}
slices.Sort(flattened)
sum := md5.New()
sum := xxh3.New()
sum.Write([]byte(strings.Join(flattened, "|")))
return sum.Sum(nil)
}

View File

@ -42,6 +42,11 @@ func (pls Playlist) IsSmartPlaylist() bool {
return pls.Rules != nil && pls.Rules.Expression != nil
}
// TracksEditable reports whether the track list is user-owned rather than server-managed.
func (pls Playlist) TracksEditable() bool {
return !pls.IsSmartPlaylist() && !pls.Sync
}
// RefreshDelay returns the playlist's own refresh window when set, falling
// back to the global SmartPlaylistRefreshDelay.
func (pls Playlist) RefreshDelay() time.Duration {

View File

@ -73,4 +73,19 @@ var _ = Describe("Playlist", func() {
Expect(pls.RefreshDelay()).To(Equal(5 * time.Second))
})
})
Describe("TracksEditable", func() {
It("is true for a plain playlist", func() {
Expect(model.Playlist{}.TracksEditable()).To(BeTrue())
})
It("is false for a smart playlist", func() {
pls := model.Playlist{Rules: &criteria.Criteria{Expression: criteria.Is{"loved": true}}}
Expect(pls.TracksEditable()).To(BeFalse())
})
It("is false for a synced playlist", func() {
Expect(model.Playlist{Sync: true}.TracksEditable()).To(BeFalse())
})
})
})

View File

@ -2,13 +2,13 @@ package model
import (
"cmp"
"crypto/md5"
"fmt"
"slices"
"strings"
"github.com/navidrome/navidrome/model/id"
"github.com/navidrome/navidrome/utils/slice"
"github.com/zeebo/xxh3"
)
type Tag struct {
@ -117,7 +117,7 @@ func (t Tags) Hash() []byte {
}
ids := t.IDs()
slices.Sort(ids)
sum := md5.New()
sum := xxh3.New()
sum.Write([]byte(strings.Join(ids, "|")))
return sum.Sum(nil)
}

View File

@ -8,7 +8,6 @@ import (
"github.com/navidrome/navidrome/log"
"github.com/navidrome/navidrome/model"
"github.com/navidrome/navidrome/model/request"
"github.com/navidrome/navidrome/tests"
"github.com/navidrome/navidrome/utils/slice"
. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"
@ -137,7 +136,6 @@ var _ = Describe("FolderRepository", func() {
})
It("includes all child folders when querying parent", func() {
tests.SkipOnWindows("path storage (#TBD-path-sep-persistence)")
// Create a parent folder with multiple children
parent := model.NewFolder(testLib, "TestParent/Music")
child1 := model.NewFolder(testLib, "TestParent/Music/Rock/Queen")
@ -159,7 +157,6 @@ var _ = Describe("FolderRepository", func() {
})
It("excludes children from other libraries", func() {
tests.SkipOnWindows("path storage (#TBD-path-sep-persistence)")
// Create parent in testLib
parent := model.NewFolder(testLib, "TestIsolation/Parent")
child := model.NewFolder(testLib, "TestIsolation/Parent/Child")
@ -185,7 +182,6 @@ var _ = Describe("FolderRepository", func() {
})
It("excludes missing children when querying parent", func() {
tests.SkipOnWindows("path storage (#TBD-path-sep-persistence)")
// Create parent and children, mark one as missing
parent := model.NewFolder(testLib, "TestMissingChild/Parent")
child1 := model.NewFolder(testLib, "TestMissingChild/Parent/Child1")
@ -206,7 +202,6 @@ var _ = Describe("FolderRepository", func() {
})
It("handles mix of existing and non-existing target paths", func() {
tests.SkipOnWindows("path storage (#TBD-path-sep-persistence)")
// Create folders for one path but not the other
existingParent := model.NewFolder(testLib, "TestMixed/Exists")
existingChild := model.NewFolder(testLib, "TestMixed/Exists/Child")

View File

@ -2,7 +2,6 @@ package persistence
import (
"context"
"crypto/md5"
"database/sql"
"errors"
"fmt"
@ -23,6 +22,7 @@ import (
"github.com/navidrome/navidrome/utils/hasher"
"github.com/navidrome/navidrome/utils/slice"
"github.com/pocketbase/dbx"
"github.com/zeebo/xxh3"
)
// sqlRepository is the base repository for all SQL repositories. It provides common functions to interact with the DB.
@ -97,6 +97,8 @@ func (r *sqlRepository) registerModel(instance any, filters map[string]filterFun
}
// setSortMappings sets the mappings for the sort fields. If the sort field is not in the map, it will be used as is.
// This applies per comma-separated part, so a key added here also defines that bare name wherever a
// caller uses it inside a sort list.
//
// If PreferSortTags is enabled, it will map the order fields to the corresponding sort expression,
// which gives precedence to sort tags.
@ -147,17 +149,37 @@ func (r sqlRepository) applyOptions(sq SelectBuilder, options ...model.QueryOpti
// TODO Change all sortMappings to have a consistent case
func (r sqlRepository) sortMapping(sort string) string {
if mapping, ok := r.sortMappings[sort]; ok {
if mapping, _, ok := r.lookupSortMapping(sort); ok {
return mapping
}
if mapping, ok := r.sortMappings[toCamelCase(sort)]; ok {
return mapping
// Each part of a comma list is resolved on its own, so a mix of mapped keys and plain columns
// keeps the mappings the recognized parts have.
parts := strings.FieldsFunc(sort, splitFunc(','))
mapped := make([]string, 0, len(parts))
for _, part := range parts {
part = strings.TrimSpace(part)
if partMapping, _, ok := r.lookupSortMapping(part); ok {
part = partMapping
} else {
part = toSnakeCase(part)
}
mapped = append(mapped, part)
}
sort = toSnakeCase(sort)
if mapping, ok := r.sortMappings[sort]; ok {
return mapping
return strings.Join(mapped, ", ")
}
// lookupSortMapping also returns the snake_case form when it had to derive one, so a caller's
// fallback doesn't recompute it: toSnakeCase runs two regexps.
func (r sqlRepository) lookupSortMapping(sort string) (mapping, snakeCased string, ok bool) {
if mapping, ok = r.sortMappings[sort]; ok {
return mapping, sort, true
}
return sort
if mapping, ok = r.sortMappings[toCamelCase(sort)]; ok {
return mapping, "", true
}
snakeCased = toSnakeCase(sort)
mapping, ok = r.sortMappings[snakeCased]
return mapping, snakeCased, ok
}
func (r sqlRepository) buildSortOrder(sort, order string) string {
@ -276,8 +298,8 @@ func (r sqlRepository) visibleLibraryIDs() ([]int, error) {
func (r sqlRepository) seedKey() string {
// Seed keys must be all lowercase, or else SQLite3 will encode it, making it not match the seed
// used in the query. Hashing the user ID and converting it to a hex string will do the trick
userIDHash := md5.Sum([]byte(loggedUser(r.ctx).ID))
return fmt.Sprintf("%s|%x", r.tableName, userIDHash)
userIDHash := xxh3.Hash([]byte(loggedUser(r.ctx).ID))
return fmt.Sprintf("%s|%016x", r.tableName, userIDHash)
}
func (r sqlRepository) resetSeededRandom(options []model.QueryOptions) {

View File

@ -92,14 +92,28 @@ var _ = Describe("sqlRepository", func() {
Expect(sort).To(BeEmpty())
})
It("returns the mapped value when sort key exists", func() {
// Validation only: buildSortOrder resolves the mapping, so mapping here too would hand
// sortMapping its own output and re-map values whose parts are themselves keys.
It("accepts a known sort key without resolving it", func() {
sort, _ := r.sanitizeSort("sort1", "")
Expect(sort).To(Equal("mappedSort1"))
Expect(sort).To(Equal("sort1"))
})
It("is case insensitive", func() {
sort, _ := r.sanitizeSort("Sort1", "")
Expect(sort).To(Equal("mappedSort1"))
Expect(sort).To(Equal("sort1"))
})
It("still resolves the mapping by the time the SQL is built", func() {
Expect(r.buildSortOrder("sort1", "asc")).To(Equal("mappedSort1 asc"))
})
// A mapping whose parts are themselves keys (media_file rated_at = "rating, rated_at")
// must survive the round trip through sanitizeSort and buildSortOrder unduplicated.
It("does not re-map a value whose parts are also keys", func() {
r.sortMappings = map[string]string{"rating": "rating", "rated_at": "rating, rated_at"}
sort, _ := r.sanitizeSort("rated_at", "")
Expect(r.buildSortOrder(sort, "asc")).To(Equal("rating asc, rated_at asc"))
})
It("returns the field if it is a valid field", func() {
@ -135,6 +149,45 @@ var _ = Describe("sqlRepository", func() {
})
})
Describe("sortMapping", func() {
BeforeEach(func() {
r.sortMappings = map[string]string{
"name": "order_album_name, order_album_artist_name",
"recently_added": "album.created_at, album.id",
}
})
It("maps a single key", func() {
Expect(r.sortMapping("recently_added")).To(Equal("album.created_at, album.id"))
})
It("maps every part of a comma list when all of them are known keys", func() {
Expect(r.sortMapping("recently_added, name")).
To(Equal("album.created_at, album.id, order_album_name, order_album_artist_name"))
})
It("resolves the known parts of a mixed list and leaves the rest as columns", func() {
Expect(r.sortMapping("recently_added, play_count")).
To(Equal("album.created_at, album.id, play_count"))
})
// Jellyfin's MusicAlbum SortBy=Runtime,SortName arrives as "duration, name"; duration is a
// plain album column while name is mapped, and the mapping must survive the mix.
It("keeps a mapping when an earlier part is a plain column", func() {
Expect(r.sortMapping("duration, name")).
To(Equal("duration, order_album_name, order_album_artist_name"))
})
It("leaves a raw column list with directions untouched", func() {
Expect(r.sortMapping("starred desc, rating desc")).To(Equal("starred desc, rating desc"))
})
It("does not split an expression on a comma inside its parentheses", func() {
Expect(r.sortMapping("coalesce(name, ''), title")).To(Equal("coalesce(name, ''), title"))
Expect(r.sortMapping("coalesce(nullif(a,''), b) desc, c")).To(Equal("coalesce(nullif(a,''), b) desc, c"))
})
It("keeps a mapping whose value nests commas inside parentheses", func() {
r.sortMappings["max_year"] = "coalesce(nullif(original_date,''), cast(max_year as text)), release_date"
Expect(r.sortMapping("max_year, name")).To(Equal(
"coalesce(nullif(original_date,''), cast(max_year as text)), release_date, " +
"order_album_name, order_album_artist_name"))
})
})
Describe("buildSortOrder", func() {
BeforeEach(func() {
r.sortMappings = map[string]string{}

View File

@ -69,13 +69,11 @@ func (r *sqlRepository) parseRestOptions(ctx context.Context, options ...rest.Qu
func (r sqlRepository) sanitizeSort(sort, order string) (string, string) {
if sort != "" {
sort = toSnakeCase(sort)
if mapped, ok := r.sortMappings[sort]; ok {
sort = mapped
} else {
if !r.isFieldWhiteListed(sort) {
log.Warn(r.ctx, "Ignoring sort not whitelisted", "sort", sort, "table", r.tableName)
sort = ""
}
// Validate only: buildSortOrder resolves the mapping later, and mapping here as well would
// feed sortMapping its own output.
if _, _, known := r.lookupSortMapping(sort); !known && !r.isFieldWhiteListed(sort) {
log.Warn(r.ctx, "Ignoring sort not whitelisted", "sort", sort, "table", r.tableName)
sort = ""
}
}
if order != "" {

View File

@ -23,6 +23,7 @@ func newFolderEntry(job *scanJob, id, path string, info model.FolderUpdateInfo)
path: path,
audioFiles: make(map[string]fs.DirEntry),
imageFiles: make(map[string]fs.DirEntry),
playlistFiles: make(map[string]fs.DirEntry),
albumIDMap: make(map[string]string),
updTime: info.UpdatedAt,
prevHash: info.Hash,
@ -41,7 +42,7 @@ type folderEntry struct {
updTime time.Time // from DB
audioFiles map[string]fs.DirEntry
imageFiles map[string]fs.DirEntry
numPlaylists int
playlistFiles map[string]fs.DirEntry
numSubFolders int
imagesUpdatedAt time.Time
prevHash string // Previous hash from DB
@ -57,7 +58,7 @@ type folderEntry struct {
}
func (f *folderEntry) hasNoFiles() bool {
return len(f.audioFiles) == 0 && len(f.imageFiles) == 0 && f.numPlaylists == 0
return len(f.audioFiles) == 0 && len(f.imageFiles) == 0 && len(f.playlistFiles) == 0
}
func (f *folderEntry) isEmpty() bool {
@ -94,7 +95,7 @@ func (f *folderEntry) toFolder() *model.Folder {
folder := model.NewFolder(f.job.lib, f.path)
folder.NumAudioFiles = len(f.audioFiles)
if playlists.InPath(*folder) {
folder.NumPlaylists = f.numPlaylists
folder.NumPlaylists = len(f.playlistFiles)
}
folder.ImageFiles = slices.Collect(maps.Keys(f.imageFiles))
folder.ImagesUpdatedAt = f.imagesUpdatedAt
@ -108,16 +109,18 @@ func (f *folderEntry) hash() string {
h,
"%s:%d:%d:%s",
f.modTime.UTC(),
f.numPlaylists,
len(f.playlistFiles), // redundant with the loop below, but dropping it re-hashes every folder
f.numSubFolders,
f.imagesUpdatedAt.UTC(),
)
// Sort the keys of audio and image files to ensure consistent hashing
// Sort the keys of audio, image and playlist files to ensure consistent hashing
audioKeys := slices.Collect(maps.Keys(f.audioFiles))
slices.Sort(audioKeys)
imageKeys := slices.Collect(maps.Keys(f.imageFiles))
slices.Sort(imageKeys)
playlistKeys := slices.Collect(maps.Keys(f.playlistFiles))
slices.Sort(playlistKeys)
// Include audio files with their size and modtime
for _, key := range audioKeys {
@ -135,5 +138,14 @@ func (f *folderEntry) hash() string {
}
}
// Include playlist files, so a content edit is detected even when the folder's
// mtime is preserved (rsync -a) or the playlist is not the newest file.
for _, key := range playlistKeys {
_, _ = io.WriteString(h, key)
if info, err := f.playlistFiles[key].Info(); err == nil {
_, _ = fmt.Fprintf(h, ":%d:%s", info.Size(), info.ModTime().UTC().String())
}
}
return hex.EncodeToString(h.Sum(nil))
}

View File

@ -48,6 +48,7 @@ var _ = Describe("folder_entry", func() {
Expect(entry.path).To(Equal(path))
Expect(entry.audioFiles).To(BeEmpty())
Expect(entry.imageFiles).To(BeEmpty())
Expect(entry.playlistFiles).To(BeEmpty())
Expect(entry.albumIDMap).To(BeEmpty())
Expect(entry.updTime).To(Equal(updateInfo.UpdatedAt))
Expect(entry.prevHash).To(Equal(updateInfo.Hash))
@ -95,7 +96,7 @@ var _ = Describe("folder_entry", func() {
})
It("returns false when folder has playlists", func() {
entry.numPlaylists = 1
entry.playlistFiles["list.m3u"] = &fakeDirEntry{name: "list.m3u"}
Expect(entry.hasNoFiles()).To(BeFalse())
})
@ -107,7 +108,7 @@ var _ = Describe("folder_entry", func() {
It("returns false when folder has multiple types of content", func() {
entry.audioFiles["test.mp3"] = &fakeDirEntry{name: "test.mp3"}
entry.imageFiles["cover.jpg"] = &fakeDirEntry{name: "cover.jpg"}
entry.numPlaylists = 2
entry.playlistFiles["list.m3u"] = &fakeDirEntry{name: "list.m3u"}
entry.numSubFolders = 3
Expect(entry.hasNoFiles()).To(BeFalse())
})
@ -149,7 +150,11 @@ var _ = Describe("folder_entry", func() {
"cover.jpg": &fakeDirEntry{name: "cover.jpg"},
"folder.png": &fakeDirEntry{name: "folder.png"},
}
entry.numPlaylists = 3
entry.playlistFiles = map[string]fs.DirEntry{
"list1.m3u": &fakeDirEntry{name: "list1.m3u"},
"list2.m3u": &fakeDirEntry{name: "list2.m3u"},
"list3.m3u": &fakeDirEntry{name: "list3.m3u"},
}
entry.imagesUpdatedAt = time.Now()
})
@ -200,7 +205,10 @@ var _ = Describe("folder_entry", func() {
"z.jpg": &fakeDirEntry{name: "z.jpg"},
"x.png": &fakeDirEntry{name: "x.png"},
}
entry.numPlaylists = 2
entry.playlistFiles = map[string]fs.DirEntry{
"q.m3u": &fakeDirEntry{name: "q.m3u"},
"p.m3u": &fakeDirEntry{name: "p.m3u"},
}
entry.numSubFolders = 3
hash1 := entry.hash()
@ -214,6 +222,10 @@ var _ = Describe("folder_entry", func() {
"x.png": &fakeDirEntry{name: "x.png"},
"z.jpg": &fakeDirEntry{name: "z.jpg"},
}
entry.playlistFiles = map[string]fs.DirEntry{
"p.m3u": &fakeDirEntry{name: "p.m3u"},
"q.m3u": &fakeDirEntry{name: "q.m3u"},
}
hash2 := entry.hash()
Expect(hash1).To(Equal(hash2))
@ -252,10 +264,10 @@ var _ = Describe("folder_entry", func() {
Expect(hash1).ToNot(Equal(hash2))
})
It("produces different hash when playlist count changes", func() {
It("produces different hash when playlist files change", func() {
hash1 := entry.hash()
entry.numPlaylists = 5
entry.playlistFiles["new.m3u"] = &fakeDirEntry{name: "new.m3u"}
hash2 := entry.hash()
Expect(hash1).ToNot(Equal(hash2))
@ -377,6 +389,58 @@ var _ = Describe("folder_entry", func() {
Expect(hash1).ToNot(Equal(hash2))
})
It("produces different hash when playlist file size changes", func() {
baseTime := time.Now()
entry.playlistFiles["list.m3u"] = &fakeDirEntry{
name: "list.m3u",
fileInfo: &fakeFileInfo{name: "list.m3u", size: 1000, modTime: baseTime},
}
hash1 := entry.hash()
entry.playlistFiles["list.m3u"] = &fakeDirEntry{
name: "list.m3u",
fileInfo: &fakeFileInfo{name: "list.m3u", size: 2000, modTime: baseTime},
}
hash2 := entry.hash()
Expect(hash1).ToNot(Equal(hash2))
})
It("produces different hash when playlist file modification time changes", func() {
baseTime := time.Now()
entry.playlistFiles["list.m3u"] = &fakeDirEntry{
name: "list.m3u",
fileInfo: &fakeFileInfo{name: "list.m3u", size: 1000, modTime: baseTime},
}
hash1 := entry.hash()
entry.playlistFiles["list.m3u"] = &fakeDirEntry{
name: "list.m3u",
fileInfo: &fakeFileInfo{name: "list.m3u", size: 1000, modTime: baseTime.Add(1 * time.Hour)},
}
hash2 := entry.hash()
Expect(hash1).ToNot(Equal(hash2))
})
It("produces different hash when a playlist is renamed", func() {
baseTime := time.Now()
entry.playlistFiles["old.m3u"] = &fakeDirEntry{
name: "old.m3u",
fileInfo: &fakeFileInfo{name: "old.m3u", size: 1000, modTime: baseTime},
}
hash1 := entry.hash()
delete(entry.playlistFiles, "old.m3u")
entry.playlistFiles["new.m3u"] = &fakeDirEntry{
name: "new.m3u",
fileInfo: &fakeFileInfo{name: "new.m3u", size: 1000, modTime: baseTime},
}
hash2 := entry.hash()
Expect(hash1).ToNot(Equal(hash2))
})
It("produces valid hex-encoded hash", func() {
hash := entry.hash()
Expect(hash).To(HaveLen(32)) // MD5 hash should be 32 hex characters
@ -421,7 +485,7 @@ var _ = Describe("folder_entry", func() {
})
It("returns true when hash has changed", func() {
entry.numPlaylists = 10 // Change something to change the hash
entry.playlistFiles["list.m3u"] = &fakeDirEntry{name: "list.m3u"} // Change something to change the hash
Expect(entry.isOutdated()).To(BeTrue())
})
@ -445,7 +509,7 @@ var _ = Describe("folder_entry", func() {
It("returns true when full scan condition is not met but hash changed", func() {
entry.updTime = entry.job.lib.LastScanStartedAt.Add(1 * time.Hour)
entry.numPlaylists = 10 // Change hash
entry.playlistFiles["list.m3u"] = &fakeDirEntry{name: "list.m3u"} // Change hash
Expect(entry.isOutdated()).To(BeTrue())
})
})

View File

@ -165,7 +165,7 @@ func (p *phaseFolders) producer() ppl.Producer[*folderEntry] {
log.Trace(p.ctx, "Scanner: Checking folder state", " folder", folder.path, "_updTime", folder.updTime,
"_modTime", folder.modTime, "_lastScanStartedAt", folder.job.lib.LastScanStartedAt,
"numAudioFiles", len(folder.audioFiles), "numImageFiles", len(folder.imageFiles),
"numPlaylists", folder.numPlaylists, "numSubfolders", folder.numSubFolders)
"numPlaylists", len(folder.playlistFiles), "numSubfolders", folder.numSubFolders)
// Check if folder is outdated
if folder.isOutdated() {
@ -492,7 +492,7 @@ func (p *phaseFolders) logFolder(entry *folderEntry) (*folderEntry, error) {
logCall = log.Trace
}
logCall(p.ctx, "Scanner: Completed processing folder",
"audioCount", len(entry.audioFiles), "imageCount", len(entry.imageFiles), "plsCount", entry.numPlaylists,
"audioCount", len(entry.audioFiles), "imageCount", len(entry.imageFiles), "plsCount", len(entry.playlistFiles),
"elapsed", entry.elapsed.Elapsed(), "tracksMissing", len(entry.missingTracks),
"tracksImported", len(entry.tracks), "library", entry.job.lib.Name, consts.Zwsp+"folder", entry.path)
return entry, nil

View File

@ -82,7 +82,7 @@ func walkFolder(ctx context.Context, job *scanJob, currentFolder string, checker
dir := path.Clean(currentFolder)
log.Trace(ctx, "Scanner: Found directory", " path", dir, "audioFiles", maps.Keys(folder.audioFiles),
"images", maps.Keys(folder.imageFiles), "playlists", folder.numPlaylists, "imagesUpdatedAt", folder.imagesUpdatedAt,
"images", maps.Keys(folder.imageFiles), "playlists", len(folder.playlistFiles), "imagesUpdatedAt", folder.imagesUpdatedAt,
"updTime", folder.updTime, "modTime", folder.modTime, "numChildren", len(children))
folder.path = dir
folder.elapsed.Start()
@ -157,7 +157,7 @@ func loadDir(ctx context.Context, job *scanJob, dirPath string, checker *IgnoreC
case model.IsAudioFile(name):
folder.audioFiles[entry.Name()] = entry
case model.IsValidPlaylist(name):
folder.numPlaylists++
folder.playlistFiles[entry.Name()] = entry
case model.IsImageFile(name):
folder.imageFiles[entry.Name()] = entry
folder.imagesUpdatedAt = utils.TimeNewest(folder.imagesUpdatedAt, fileInfo.ModTime(), folder.modTime)

View File

@ -104,7 +104,11 @@ album's tracks — Feishin fetches them this way instead of `ParentId`); `GenreI
genre's albums or tracks — Finamp's genre screen sends it the same way; `/Artists/AlbumArtists`
and `MusicArtist` queries accept it too, matching artists credited on an album of that genre);
`SearchTerm`;
favorites-only (`Filters=IsFavorite` or the standalone `isFavorite=true`); `SortBy`/`SortOrder`;
`Filters` (`IsFavorite`, `IsFavoriteOrLikes`, `IsPlayed`, `IsUnplayed`) and the standalone
`isFavorite`/`isPlayed` booleans it can also be expressed as — `Filters` wins when both are sent, as
in Jellyfin; `Likes`, `Dislikes`, `IsFolder`, `IsNotFolder` and `IsResumable` have no Navidrome
equivalent and are ignored; `SortBy`/`SortOrder` (every recognized key is applied in order, so secondary keys break ties;
unrecognized keys are skipped, and `Random` always sorts alone);
`StartIndex`/`Limit`; and `Ids` (batch fetch by id). `Recursive=false` with a library `ParentId`
returns direct children only (no tracks — no track is a library's direct child).

View File

@ -24,10 +24,6 @@ func (api *Router) getAlbumArtists(w http.ResponseWriter, r *http.Request) {
// when accessible (like queryItems) or all accessible libraries otherwise.
func (api *Router) listArtistsByRole(w http.ResponseWriter, r *http.Request, role model.Role) {
ctx := r.Context()
p := req.Params(r)
opts := model.QueryOptions{Offset: p.IntOr("startindex", 0), Max: p.IntOr("limit", 0)}
applySort(&opts, "MusicArtist", p.StringOr("sortby", ""), p.StringOr("sortorder", ""))
scopeIDs, _, ok := parentIDScope(ctx, r)
if !ok {
http.Error(w, "Not Found", http.StatusNotFound)
@ -38,14 +34,13 @@ func (api *Router) listArtistsByRole(w http.ResponseWriter, r *http.Request, rol
http.Error(w, "Not Found", http.StatusNotFound)
return
}
// Only the fields listArtists reads; /Artists has no favorites filter, so favOnly stays false.
// Finamp's artist tab sends GenreIds when a genre filter is active.
q := itemsQuery{
scopeIDs: scopeIDs,
genreIds: genreIds,
search: searchTerm(p),
fields: dto.ParseFields(p.Strings("fields")...),
}
// This route resolves its own scope, so it shares only the plain query params with /Items.
q := listParams(req.Params(r))
q.scopeIDs = scopeIDs
q.genreIds = genreIds
opts := model.QueryOptions{Offset: q.offset, Max: q.limit}
applySort(&opts, "MusicArtist", q.sortBy, q.sortOrder)
if q.search != "" {
opts.Max = clampLimit(opts.Max, defaultSearchLimit, maxSearchLimit)
}

View File

@ -156,6 +156,29 @@ var _ = Describe("Browsing", func() {
Expect(sql).NotTo(ContainSubstring("library_artist.library_id"))
})
DescribeTable("restricts to favorites",
func(url string, handler func(*Router) http.HandlerFunc) {
artistRepo := ds.Artist(context.Background()).(*tests.MockArtistRepo)
artistRepo.SetData(model.Artists{{ID: testID("ar1"), Name: "Artist"}})
w := httptest.NewRecorder()
r := httptest.NewRequest("GET", url, nil).WithContext(ctxUser(model.Libraries{{ID: 1}}))
invoke(handler(api), w, r)
Expect(w.Code).To(Equal(http.StatusOK))
sql, args, err := artistRepo.Options.Filters.ToSql()
Expect(err).NotTo(HaveOccurred())
Expect(sql).To(ContainSubstring("starred"))
// listArtists always ANDs notMissing, favorites filter or not.
Expect(sql).To(ContainSubstring("missing"))
Expect(args).To(ContainElement(true))
},
Entry("Filters=IsFavorite", "/Artists?Filters=IsFavorite",
func(a *Router) http.HandlerFunc { return a.getArtists }),
Entry("isFavorite=true", "/Artists?isFavorite=true",
func(a *Router) http.HandlerFunc { return a.getArtists }),
Entry("on /Artists/AlbumArtists", "/Artists/AlbumArtists?Filters=IsFavorite",
func(a *Router) http.HandlerFunc { return a.getAlbumArtists }),
)
It("404s a malformed ParentId instead of listing every library's artists", func() {
artistRepo := ds.Artist(context.Background()).(*tests.MockArtistRepo)
artistRepo.SetData(model.Artists{{ID: testID("ar1"), Name: "Artist"}})

View File

@ -11,7 +11,6 @@ import (
_ "image/png"
"io"
"net/http"
"strconv"
"github.com/dustin/go-humanize"
"github.com/navidrome/navidrome/conf"
@ -20,9 +19,20 @@ import (
"github.com/navidrome/navidrome/model"
"github.com/navidrome/navidrome/model/request"
"github.com/navidrome/navidrome/server/imghttp"
"github.com/navidrome/navidrome/utils/req"
_ "golang.org/x/image/webp"
)
// imageSize picks the tighter of Jellyfin's two bounds, because Navidrome resizes on a single
// dimension: reading only MaxWidth serves the full-size original to a client that sent MaxHeight.
func imageSize(maxWidth, maxHeight int) int {
w, h := max(maxWidth, 0), max(maxHeight, 0)
if w == 0 || h == 0 {
return max(w, h)
}
return min(w, h)
}
func (api *Router) getItemImage(w http.ResponseWriter, r *http.Request) {
// Public endpoint, like real Jellyfin's image routes: clients fetch cover URLs without credentials
// and item ids are unguessable, so resolution runs elevated to bypass the visibility filter.
@ -31,7 +41,8 @@ func (api *Router) getItemImage(w http.ResponseWriter, r *http.Request) {
if !ok {
return
}
size, _ := strconv.Atoi(r.URL.Query().Get("maxwidth"))
p := req.Params(r)
size := imageSize(p.IntOr("maxwidth", 0), p.IntOr("maxheight", 0))
artID := api.resolveArtworkID(ctx, itemId)
img, err := api.artwork.GetOrPlaceholder(ctx, artID, size, false)

View File

@ -30,14 +30,16 @@ import (
type fakeArtwork struct {
artwork.Artwork
recvId string
recvCtx context.Context
data []byte
hash string
recvId string
recvSize int
recvCtx context.Context
data []byte
hash string
}
func (f *fakeArtwork) GetOrPlaceholder(ctx context.Context, id string, size int, square bool) (*artwork.Image, error) {
f.recvId = id
f.recvSize = size
f.recvCtx = ctx
data := f.data
if data == nil {
@ -61,6 +63,29 @@ func newImageRequest(itemId string) (*httptest.ResponseRecorder, *http.Request)
}
var _ = Describe("Images", func() {
// Real Jellyfin fits the image inside either bound, so a client that sends only MaxHeight must
// still get a resized image rather than the full-size original.
DescribeTable("derives the requested size from MaxWidth or MaxHeight",
func(query string, wantSize int) {
ds := &tests.MockDataStore{}
ds.Album(context.Background()).(*tests.MockAlbumRepo).SetData(model.Albums{{ID: testID("a1"), Name: "One"}})
fa := &fakeArtwork{}
api := &Router{ds: ds, artwork: fa}
w, r := newImageRequest(dto.EncodeID(testID("a1")))
r.URL.RawQuery = query
api.getItemImage(w, r)
Expect(w.Code).To(Equal(http.StatusOK))
Expect(fa.recvSize).To(Equal(wantSize))
},
Entry("MaxWidth only", "maxwidth=300", 300),
Entry("MaxHeight only", "maxheight=300", 300),
Entry("both, smaller bound wins", "maxwidth=200&maxheight=300", 200),
Entry("both, smaller bound wins regardless of order", "maxwidth=300&maxheight=200", 200),
Entry("neither", "", 0),
)
It("streams album artwork", func() {
ds := &tests.MockDataStore{}
ds.Album(context.Background()).(*tests.MockAlbumRepo).SetData(model.Albums{{ID: testID("a1"), Name: "One"}})

View File

@ -31,6 +31,51 @@ func searchTerm(p *req.Values) string {
return strings.TrimSpace(p.StringOr("searchterm", ""))
}
// itemFilters is the parsed Filters=... list together with the standalone isFavorite/isPlayed params
// clients may send instead. A nil field means the client asked for no filtering on that dimension.
type itemFilters struct {
favorite *bool
played *bool
}
// parseItemFilters reads the standalone params first and lets the Filters list win, matching real
// Jellyfin. Tokens with no Navidrome equivalent (Likes, IsFolder, IsResumable) are dropped.
func parseItemFilters(p *req.Values) itemFilters {
f := itemFilters{favorite: p.BoolPtr("isfavorite"), played: p.BoolPtr("isplayed")}
for token := range strings.SplitSeq(p.StringOr("filters", ""), ",") {
switch strings.TrimSpace(token) {
case "IsFavorite", "IsFavoriteOrLikes":
f.favorite = new(true)
case "IsPlayed":
f.played = new(true)
case "IsUnplayed":
f.played = new(false)
}
}
return f
}
// predicates renders the filters as annotation-column conditions. The negative cases have to match
// NULL as well: annotations are LEFT JOINed, so an item nobody has touched has no row at all.
func (f itemFilters) predicates() []squirrel.Sqlizer {
var out []squirrel.Sqlizer
if f.favorite != nil {
if *f.favorite {
out = append(out, squirrel.Eq{"starred": true})
} else {
out = append(out, squirrel.Or{squirrel.Eq{"starred": nil}, squirrel.Eq{"starred": false}})
}
}
if f.played != nil {
if *f.played {
out = append(out, squirrel.Gt{"play_count": 0})
} else {
out = append(out, squirrel.Or{squirrel.Eq{"play_count": nil}, squirrel.Eq{"play_count": 0}})
}
}
return out
}
func (api *Router) getItems(w http.ResponseWriter, r *http.Request) {
res, err := api.queryItems(r.Context(), r)
if err != nil {
@ -214,7 +259,7 @@ type itemsQuery struct {
sortOrder string
offset int
limit int
favOnly bool
filters itemFilters
// parentId scopes the query. entityParent is the same id only when it names an entity (an artist
// for MusicAlbum, an album for Audio) rather than a library.
parentId string
@ -231,6 +276,19 @@ type itemsQuery struct {
studioIds []string
}
// listParams reads the itemsQuery fields that come straight from query params.
func listParams(p *req.Values) itemsQuery {
return itemsQuery{
fields: dto.ParseFields(p.Strings("fields")...),
search: searchTerm(p),
sortBy: p.StringOr("sortby", ""),
sortOrder: p.StringOr("sortorder", ""),
offset: p.IntOr("startindex", 0),
limit: p.IntOr("limit", 0),
filters: parseItemFilters(p),
}
}
// parseItemsQuery also resolves the entity types (inferring them from the parent when
// IncludeItemTypes is absent) and the library scope. Query keys are read lowercase because
// normalizeQueryKeys folded them (Jellyfin binds case-insensitively). A non-empty id param that
@ -261,24 +319,14 @@ func (api *Router) parseItemsQuery(ctx context.Context, r *http.Request) (itemsQ
if !ok {
return itemsQuery{}, model.ErrNotFound
}
q := itemsQuery{
fields: dto.ParseFields(p.Strings("fields")...),
ids: ids,
rawTypes: p.StringOr("includeitemtypes", ""),
search: searchTerm(p),
sortBy: p.StringOr("sortby", ""),
sortOrder: p.StringOr("sortorder", ""),
offset: p.IntOr("startindex", 0),
limit: p.IntOr("limit", 0),
// Clients express "favorites only" two ways: Filters=IsFavorite and the standalone
// isFavorite=true param (Finamp's "Favourite tracks" widget uses the latter).
favOnly: strings.Contains(p.StringOr("filters", ""), "IsFavorite") || p.BoolOr("isfavorite", false),
parentId: parentId,
genreIds: genreIds,
albumIds: albumIds,
years: parseYears(r),
studioIds: studioIds,
}
q := listParams(p)
q.ids = ids
q.rawTypes = p.StringOr("includeitemtypes", "")
q.parentId = parentId
q.genreIds = genreIds
q.albumIds = albumIds
q.years = parseYears(r)
q.studioIds = studioIds
// An artist's page filters by artist, not ParentId: Finamp sends ParentId=<libraryId> for scoping
// plus AlbumArtistIds/ArtistIds/contributingArtistIds for the artist.
albumArtistScope := firstNonEmpty(p.StringOr("albumartistids", ""), p.StringOr("artistids", ""))
@ -621,8 +669,10 @@ func (api *Router) listAlbums(ctx context.Context, opts model.QueryOptions, q it
if len(q.studioIds) > 0 {
filters = append(filters, filter.ByStudioID(q.studioIds))
}
if q.favOnly {
filters = append(filters, filter.ByStarred().Filters)
// Not on the search path: its first FTS phase selects rowids with no annotation join, so a
// starred/play_count predicate there is "no such column" rather than a filter.
if q.search == "" {
filters = append(filters, q.filters.predicates()...)
}
opts.Filters = filters
opts = filter.ApplyLibraryFilter(opts, q.scopeIDs)
@ -668,8 +718,10 @@ func (api *Router) listSongs(ctx context.Context, opts model.QueryOptions, q ite
if len(q.studioIds) > 0 {
filters = append(filters, filter.ByStudioID(q.studioIds))
}
if q.favOnly {
filters = append(filters, filter.ByStarred().Filters)
// Not on the search path: its first FTS phase selects rowids with no annotation join, so a
// starred/play_count predicate there is "no such column" rather than a filter.
if q.search == "" {
filters = append(filters, q.filters.predicates()...)
}
opts.Filters = filters
opts = filter.ApplyLibraryFilter(opts, q.scopeIDs)
@ -720,14 +772,12 @@ func (api *Router) listArtists(ctx context.Context, opts model.QueryOptions, q i
return materialized(result(slice.Map(artists, toItem), total, opts.Offset)), nil
}
if q.favOnly {
opts.Filters = filter.ArtistsByStarred().Filters
} else {
opts.Filters = notMissing
}
filters := squirrel.And{notMissing}
filters = append(filters, q.filters.predicates()...)
if len(q.genreIds) > 0 {
opts.Filters = squirrel.And{opts.Filters, filter.ArtistsByGenreID(q.genreIds)}
filters = append(filters, filter.ArtistsByGenreID(q.genreIds))
}
opts.Filters = filters
opts = filter.ArtistsByRole(opts, role)
opts = filter.ApplyArtistLibraryFilter(opts, q.scopeIDs)
total, _ := repo.CountAll(model.QueryOptions{Filters: opts.Filters})
@ -752,13 +802,8 @@ func (api *Router) listGenres(ctx context.Context, opts model.QueryOptions) (ite
// listPlaylists lists playlists visible to the current user. Visibility (public or owned) is
// enforced by playlistRepository, not scopeIDs.
func (api *Router) listPlaylists(ctx context.Context, opts model.QueryOptions, q itemsQuery) (itemsResult, error) {
if q.favOnly {
starred := squirrel.Eq{"starred": true}
if opts.Filters == nil {
opts.Filters = starred
} else {
opts.Filters = squirrel.And{opts.Filters, starred}
}
if preds := q.filters.predicates(); len(preds) > 0 {
opts.Filters = squirrel.And(preds)
}
repo := api.ds.Playlist(ctx)
total, err := repo.CountAll(model.QueryOptions{Filters: opts.Filters})
@ -908,18 +953,32 @@ func result(items []dto.BaseItemDto, total, start int) dto.QueryResult {
return dto.QueryResult{Items: items, TotalRecordCount: total, StartIndex: start}
}
// applySort translates Jellyfin's SortBy/SortOrder into a valid model.QueryOptions sort key for the
// item type. Clients send SortBy as a comma-separated fallback list (e.g. "DateCreated,SortName");
// this uses the first recognized key. An unrecognized SortBy is left untouched (the repo's default),
// not passed through raw where it could produce an invalid ORDER BY.
// applySort keeps every recognized SortBy key, so secondary keys break ties as Jellyfin intends.
// Unrecognized keys are skipped, not passed through raw where they could make an invalid ORDER BY.
func applySort(opts *model.QueryOptions, itemType, sortBy, order string) {
var cols []string
for key := range strings.SplitSeq(sortBy, ",") {
if col, ok := sortColumn(itemType, strings.TrimSpace(key)); ok {
opts.Sort = col
col, ok := sortColumn(itemType, strings.TrimSpace(key))
// The repo matches random by exact string equality, so it can only ever sort alone.
if !ok || slices.Contains(cols, col) || (col == "random" && len(cols) > 0) {
continue
}
cols = append(cols, col)
if col == "random" {
break
}
}
if strings.EqualFold(order, "Descending") {
switch {
case len(cols) > 0:
opts.Sort = strings.Join(cols, ", ")
case sortBy != "":
log.Debug("Jellyfin API: no usable SortBy key, falling back to the default order",
"itemType", itemType, "sortBy", sortBy)
}
// Jellyfin allows a per-key SortOrder list, which one Order can't express; honor the first value
// for every key, as Jellyfin does for keys past the end of the list.
first, _, _ := strings.Cut(order, ",")
if strings.EqualFold(first, "Descending") {
opts.Order = "desc"
}
}
@ -941,6 +1000,8 @@ var sortColumnsByType = map[string]map[string]string{
"dateplayed": "play_date",
"communityrating": "rating",
"random": "random",
"runtime": "duration",
"runtimeticks": "duration",
// Finamp's "Latest Releases" sorts by PremiereDate; "year" matches songs' ProductionYear.
"premieredate": "year",
"productionyear": "year",
@ -964,6 +1025,8 @@ var sortColumnsByType = map[string]map[string]string{
"playcount": "play_count",
"dateplayed": "play_date",
"communityrating": "rating",
"runtime": "duration",
"runtimeticks": "duration",
"premieredate": "max_year", "productionyear": "max_year",
},
"MusicGenre": {

View File

@ -327,17 +327,75 @@ var _ = Describe("Items", func() {
Expect(albumRepo.Options.Max).To(Equal(3))
})
It("applies a starred filter when Filters=IsFavorite", func() {
albumRepo := ds.Album(context.Background()).(*tests.MockAlbumRepo)
albumRepo.SetData(model.Albums{{ID: testID("a1"), Name: "One"}})
w := httptest.NewRecorder()
r := httptest.NewRequest("GET", "/Items?IncludeItemTypes=MusicAlbum&Filters=IsFavorite", nil).WithContext(ctxUser())
invoke(api.getItems, w, r)
Expect(w.Code).To(Equal(http.StatusOK))
sql, _, err := albumRepo.Options.Filters.ToSql()
Expect(err).NotTo(HaveOccurred())
Expect(sql).To(ContainSubstring("starred"))
})
DescribeTable("translates the Filters list and its standalone equivalents",
func(query string, wantSQL, notWantSQL []string) {
albumRepo := ds.Album(context.Background()).(*tests.MockAlbumRepo)
albumRepo.SetData(model.Albums{{ID: testID("a1"), Name: "One"}})
w := httptest.NewRecorder()
r := httptest.NewRequest("GET", "/Items?IncludeItemTypes=MusicAlbum&"+query, nil).WithContext(ctxUser())
invoke(api.getItems, w, r)
Expect(w.Code).To(Equal(http.StatusOK))
sql, _, err := albumRepo.Options.Filters.ToSql()
Expect(err).NotTo(HaveOccurred())
for _, want := range wantSQL {
Expect(sql).To(ContainSubstring(want))
}
for _, not := range notWantSQL {
Expect(sql).NotTo(ContainSubstring(not))
}
},
Entry("IsFavorite", "Filters=IsFavorite", []string{"starred"}, nil),
Entry("IsFavorite,IsUnplayed combined", "Filters=IsFavorite,IsUnplayed",
[]string{"starred", "play_count"}, nil),
Entry("IsUnplayed", "Filters=IsUnplayed", []string{"play_count"}, []string{"starred"}),
Entry("IsPlayed", "Filters=IsPlayed", []string{"play_count"}, []string{"starred"}),
Entry("IsFavoriteOrLikes is treated as favorites", "Filters=IsFavoriteOrLikes", []string{"starred"}, nil),
Entry("isPlayed=false", "isPlayed=false", []string{"play_count"}, nil),
Entry("isFavorite=false still filters", "isFavorite=false", []string{"starred"}, nil),
// Jellyfin builds the query from the standalone params, then applies Filters over the top.
Entry("Filters wins over the standalone param", "isFavorite=false&Filters=IsFavorite",
[]string{"starred = "}, nil),
// No Navidrome equivalent: these must be dropped, not half-applied.
Entry("Likes is ignored", "Filters=Likes", nil, []string{"starred", "play_count"}),
Entry("IsResumable is ignored", "Filters=IsResumable", nil, []string{"starred", "play_count"}),
// The artist-parent branch gets notMissing from filter.AlbumsByArtistID, not the default
// branch, so favorites must not be the only predicate left on it.
Entry("keeps missing excluded under an artist parent",
"Filters=IsFavorite&ArtistIds="+dto.EncodeID(testID("ar1")),
[]string{"starred", "missing"}, nil),
)
// Search runs a two-phase FTS query whose first phase has no annotation join, so an
// annotation predicate there is "no such column: starred" -> 500.
DescribeTable("does not push annotation filters into a search",
func(itemType, filters string) {
ds.Album(context.Background()).(*tests.MockAlbumRepo).SetData(model.Albums{{ID: testID("a1"), Name: "One"}})
ds.MediaFile(context.Background()).(*tests.MockMediaFileRepo).SetData(model.MediaFiles{{ID: testID("s1"), Title: "Song"}})
w := httptest.NewRecorder()
r := httptest.NewRequest("GET",
"/Items?IncludeItemTypes="+itemType+"&SearchTerm=one&Filters="+filters, nil).WithContext(ctxUser())
invoke(api.getItems, w, r)
Expect(w.Code).To(Equal(http.StatusOK))
var opts model.QueryOptions
if itemType == "MusicAlbum" {
opts = ds.Album(context.Background()).(*tests.MockAlbumRepo).Options
} else {
opts = ds.MediaFile(context.Background()).(*tests.MockMediaFileRepo).Options
}
if opts.Filters == nil {
return
}
sql, _, err := opts.Filters.ToSql()
Expect(err).NotTo(HaveOccurred())
Expect(sql).NotTo(ContainSubstring("starred"))
Expect(sql).NotTo(ContainSubstring("play_count"))
},
Entry("albums, IsFavorite", "MusicAlbum", "IsFavorite"),
Entry("albums, IsUnplayed", "MusicAlbum", "IsUnplayed"),
Entry("albums, IsPlayed", "MusicAlbum", "IsPlayed"),
Entry("songs, IsFavorite", "Audio", "IsFavorite"),
Entry("songs, IsUnplayed", "Audio", "IsUnplayed"),
)
It("forwards SearchTerm to the repo's Search method", func() {
albumRepo := ds.Album(context.Background()).(*tests.MockAlbumRepo)
@ -601,65 +659,59 @@ var _ = Describe("Items", func() {
})
Describe("sorting", func() {
It("maps SortBy=PlayCount to the play_count column", func() {
albumRepo := ds.Album(context.Background()).(*tests.MockAlbumRepo)
albumRepo.SetData(model.Albums{{ID: testID("a1"), Name: "One"}})
w := httptest.NewRecorder()
r := httptest.NewRequest("GET", "/Items?IncludeItemTypes=MusicAlbum&SortBy=PlayCount", nil).WithContext(ctxUser())
invoke(api.getItems, w, r)
Expect(w.Code).To(Equal(http.StatusOK))
Expect(albumRepo.Options.Sort).To(Equal("play_count"))
})
DescribeTable("translates SortBy into the repo's sort keys",
func(itemType, sortBy, want string) {
albumRepo := ds.Album(context.Background()).(*tests.MockAlbumRepo)
albumRepo.SetData(model.Albums{{ID: testID("a1"), Name: "One"}})
mfRepo := ds.MediaFile(context.Background()).(*tests.MockMediaFileRepo)
mfRepo.SetData(model.MediaFiles{{ID: testID("s1"), Title: "Song"}})
w := httptest.NewRecorder()
r := httptest.NewRequest("GET", "/Items?IncludeItemTypes="+itemType+"&SortBy="+sortBy, nil).WithContext(ctxUser())
invoke(api.getItems, w, r)
Expect(w.Code).To(Equal(http.StatusOK))
got := mfRepo.Options.Sort
if itemType == "MusicAlbum" {
got = albumRepo.Options.Sort
}
Expect(got).To(Equal(want))
},
Entry("PlayCount", "MusicAlbum", "PlayCount", "play_count"),
Entry("DatePlayed", "Audio", "DatePlayed", "play_date"),
Entry("Runtime on albums", "MusicAlbum", "Runtime", "duration"),
Entry("RunTimeTicks alias", "MusicAlbum", "RunTimeTicks", "duration"),
// Finamp leads its track sort with Runtime: unless that resolves, the first recognized
// key is AlbumArtist and the list looks sorted while being sorted by the wrong thing.
Entry("Finamp's Runtime-led track sort", "Audio", "Runtime,AlbumArtist,Album,SortName",
"duration, album_artist, album, title"),
Entry("every recognized key, in order", "MusicAlbum", "DateCreated,SortName", "recently_added, name"),
Entry("a key repeating a column is dropped", "Audio",
"PremiereDate,Album,ParentIndexNumber,IndexNumber,SortName", "year, album, title"),
// random is matched by exact string equality in the repo, so it can never share a sort.
Entry("Random stays alone", "MusicAlbum", "Random,SortName", "random"),
Entry("unrecognized keys are skipped", "Audio", "Runtime,Nonsense,SortName", "duration, title"),
Entry("only the last key recognized", "Audio", "Unknown1,Unknown2,SortName", "title"),
Entry("Finamp's album view is disc+track", "Audio", "ParentIndexNumber,IndexNumber,SortName", "album, title"),
Entry("nothing recognized leaves the repo default", "MusicAlbum", "SeriesSortName", ""),
)
It("maps SortBy=DatePlayed to the play_date column", func() {
mfRepo := ds.MediaFile(context.Background()).(*tests.MockMediaFileRepo)
mfRepo.SetData(model.MediaFiles{{ID: testID("s1"), Title: "Song"}})
w := httptest.NewRecorder()
r := httptest.NewRequest("GET", "/Items?IncludeItemTypes=Audio&SortBy=DatePlayed", nil).WithContext(ctxUser())
invoke(api.getItems, w, r)
Expect(w.Code).To(Equal(http.StatusOK))
Expect(mfRepo.Options.Sort).To(Equal("play_date"))
})
It("uses the first recognized key in a comma-separated SortBy list", func() {
albumRepo := ds.Album(context.Background()).(*tests.MockAlbumRepo)
albumRepo.SetData(model.Albums{{ID: testID("a1"), Name: "One"}})
w := httptest.NewRecorder()
r := httptest.NewRequest("GET", "/Items?IncludeItemTypes=MusicAlbum&SortBy=DateCreated,SortName", nil).WithContext(ctxUser())
invoke(api.getItems, w, r)
Expect(w.Code).To(Equal(http.StatusOK))
Expect(albumRepo.Options.Sort).To(Equal("recently_added"))
})
It("skips unrecognized keys in a comma-separated SortBy list to find one that is", func() {
mfRepo := ds.MediaFile(context.Background()).(*tests.MockMediaFileRepo)
mfRepo.SetData(model.MediaFiles{{ID: testID("s1"), Title: "Song"}})
w := httptest.NewRecorder()
r := httptest.NewRequest("GET", "/Items?IncludeItemTypes=Audio&SortBy=Unknown1,Unknown2,SortName", nil).WithContext(ctxUser())
invoke(api.getItems, w, r)
Expect(w.Code).To(Equal(http.StatusOK))
Expect(mfRepo.Options.Sort).To(Equal("title"))
})
It("maps Finamp's album view SortBy (ParentIndexNumber,IndexNumber) to disc+track order", func() {
mfRepo := ds.MediaFile(context.Background()).(*tests.MockMediaFileRepo)
mfRepo.SetData(model.MediaFiles{{ID: testID("s1"), Title: "Song"}})
w := httptest.NewRecorder()
r := httptest.NewRequest("GET", "/Items?IncludeItemTypes=Audio&SortBy=ParentIndexNumber,IndexNumber,SortName", nil).WithContext(ctxUser())
invoke(api.getItems, w, r)
Expect(w.Code).To(Equal(http.StatusOK))
Expect(mfRepo.Options.Sort).To(Equal("album"))
})
It("leaves Sort at the repo default when no SortBy key is recognized", func() {
albumRepo := ds.Album(context.Background()).(*tests.MockAlbumRepo)
albumRepo.SetData(model.Albums{{ID: testID("a1"), Name: "One"}})
w := httptest.NewRecorder()
r := httptest.NewRequest("GET", "/Items?IncludeItemTypes=MusicAlbum&SortBy=SeriesSortName", nil).WithContext(ctxUser())
invoke(api.getItems, w, r)
Expect(w.Code).To(Equal(http.StatusOK))
Expect(albumRepo.Options.Sort).To(Equal(""))
})
// Jellyfin allows a per-key SortOrder list; we cannot express that through one Order, so
// we honor the first value for all keys, matching Jellyfin's fallback for extra keys.
DescribeTable("reads the first SortOrder value for the whole sort",
func(sortOrder, want string) {
albumRepo := ds.Album(context.Background()).(*tests.MockAlbumRepo)
albumRepo.SetData(model.Albums{{ID: testID("a1"), Name: "One"}})
w := httptest.NewRecorder()
r := httptest.NewRequest("GET",
"/Items?IncludeItemTypes=MusicAlbum&SortBy=Runtime,SortName&SortOrder="+sortOrder, nil).WithContext(ctxUser())
invoke(api.getItems, w, r)
Expect(w.Code).To(Equal(http.StatusOK))
Expect(albumRepo.Options.Order).To(Equal(want))
},
Entry("ascending", "Ascending", ""),
Entry("descending", "Descending", "desc"),
Entry("descending leading a list", "Descending,Ascending", "desc"),
Entry("ascending leading a list", "Ascending,Descending", ""),
)
})
Describe("library scoping", func() {

View File

@ -29,11 +29,11 @@ func playlistsFolder() dto.BaseItemDto {
}
}
// playlistError maps core/playlists write errors to HTTP status: ownership -> 403, missing/invisible
// -> 404 (never revealing another user's private playlist), else -> 500.
// playlistError maps core/playlists write errors to HTTP status: ownership or locked -> 403,
// missing/invisible -> 404 (never revealing another user's private playlist), else -> 500.
func (api *Router) playlistError(w http.ResponseWriter, r *http.Request, err error) {
switch {
case errors.Is(err, model.ErrNotAuthorized):
case errors.Is(err, model.ErrNotAuthorized), errors.Is(err, model.ErrPlaylistNotEditable):
http.Error(w, "Forbidden", http.StatusForbidden)
case errors.Is(err, model.ErrNotFound):
http.Error(w, "Not Found", http.StatusNotFound)
@ -262,7 +262,7 @@ func (api *Router) songIDs(ctx context.Context, opts model.QueryOptions) []strin
}
// addToPlaylist appends items by id, expanding containers into tracks (see expandContainerIDs).
// AddTracks enforces ownership; any error maps to 404.
// AddTracks enforces ownership; a locked playlist maps to 403, any other error to 404.
func (api *Router) addToPlaylist(w http.ResponseWriter, r *http.Request) {
ctx := r.Context()
id, ok := itemIDParam(w, r, "playlistId")
@ -276,6 +276,10 @@ func (api *Router) addToPlaylist(w http.ResponseWriter, r *http.Request) {
}
ids := api.expandContainerIDs(ctx, decoded)
if _, err := api.playlists.AddTracks(ctx, id, ids); err != nil {
if errors.Is(err, model.ErrPlaylistNotEditable) {
http.Error(w, "Forbidden", http.StatusForbidden)
return
}
http.Error(w, "Not Found", http.StatusNotFound)
return
}
@ -284,7 +288,7 @@ func (api *Router) addToPlaylist(w http.ResponseWriter, r *http.Request) {
// removeFromPlaylist removes entries by entryIds — playlist-entry ids (PlaylistItemId), not media
// file ids, since RemoveTracks deletes playlist_tracks rows by that id. RemoveTracks enforces
// ownership; any error maps to 404.
// ownership; a locked playlist maps to 403, any other error to 404.
func (api *Router) removeFromPlaylist(w http.ResponseWriter, r *http.Request) {
ctx := r.Context()
id, ok := itemIDParam(w, r, "playlistId")
@ -302,21 +306,44 @@ func (api *Router) removeFromPlaylist(w http.ResponseWriter, r *http.Request) {
ids = append(ids, entry)
}
if err := api.playlists.RemoveTracks(ctx, id, ids); err != nil {
if errors.Is(err, model.ErrPlaylistNotEditable) {
http.Error(w, "Forbidden", http.StatusForbidden)
return
}
http.Error(w, "Not Found", http.StatusNotFound)
return
}
w.WriteHeader(http.StatusNoContent)
}
// getPlaylistUsers and getPlaylistUser answer client probes (e.g. Finamp) made before allowing
// edits. Navidrome has no per-playlist ACL, so every user is reported CanEdit; ownership is still
// enforced by AddTracks/RemoveTracks.
// Clients probe these before offering edits. Navidrome has no per-playlist ACL, so CanEdit carries
// only editability; ownership is enforced on write, and a lookup error 404s to prevent probing.
func (api *Router) getPlaylistUsers(w http.ResponseWriter, r *http.Request) {
u, _ := request.UserFrom(r.Context())
api.ok(w, r, []dto.PlaylistUserPermissions{{UserId: dto.EncodeID(u.ID), CanEdit: true}})
ctx := r.Context()
id, ok := itemIDParam(w, r, "playlistId")
if !ok {
return
}
pls, err := api.playlists.Get(ctx, id)
if err != nil {
http.Error(w, "Not Found", http.StatusNotFound)
return
}
u, _ := request.UserFrom(ctx)
api.ok(w, r, []dto.PlaylistUserPermissions{{UserId: dto.EncodeID(u.ID), CanEdit: pls.TracksEditable()}})
}
func (api *Router) getPlaylistUser(w http.ResponseWriter, r *http.Request) {
ctx := r.Context()
id, ok := itemIDParam(w, r, "playlistId")
if !ok {
return
}
pls, err := api.playlists.Get(ctx, id)
if err != nil {
http.Error(w, "Not Found", http.StatusNotFound)
return
}
userId := chi.URLParam(r, "userId")
api.ok(w, r, dto.PlaylistUserPermissions{UserId: userId, CanEdit: true})
api.ok(w, r, dto.PlaylistUserPermissions{UserId: userId, CanEdit: pls.TracksEditable()})
}

View File

@ -381,6 +381,15 @@ var _ = Describe("Playlists", func() {
Expect(w.Code).To(Equal(http.StatusNotFound))
})
It("returns 403 when the playlist is not editable (synced/smart), like Jellyfin", func() {
fp.addErr = model.ErrPlaylistNotEditable
w := httptest.NewRecorder()
r := httptest.NewRequest("POST", "/Playlists/"+dto.EncodeID(testID("pl1"))+"/Items?ids="+dto.EncodeID(testID("s1")), nil).WithContext(context.Background())
r = withChiURLParam(r, "playlistId", dto.EncodeID(testID("pl1")))
invoke(api.addToPlaylist, w, r)
Expect(w.Code).To(Equal(http.StatusForbidden))
})
It("passes no ids (not a spurious empty string) when the ids param is absent", func() {
w := httptest.NewRecorder()
r := httptest.NewRequest("POST", "/Playlists/"+dto.EncodeID(testID("pl1"))+"/Items", nil).WithContext(context.Background())
@ -430,6 +439,15 @@ var _ = Describe("Playlists", func() {
Expect(w.Code).To(Equal(http.StatusNotFound))
})
It("returns 403 when the playlist is not editable (synced/smart), like Jellyfin", func() {
fp.removeErr = model.ErrPlaylistNotEditable
w := httptest.NewRecorder()
r := httptest.NewRequest("DELETE", "/Playlists/"+dto.EncodeID(testID("pl1"))+"/Items?entryIds="+dto.EncodePlaylistEntryID("1"), nil).WithContext(context.Background())
r = withChiURLParam(r, "playlistId", dto.EncodeID(testID("pl1")))
invoke(api.removeFromPlaylist, w, r)
Expect(w.Code).To(Equal(http.StatusForbidden))
})
It("passes no ids (not a spurious empty string) when the entryIds param is absent", func() {
w := httptest.NewRecorder()
r := httptest.NewRequest("DELETE", "/Playlists/"+dto.EncodeID(testID("pl1"))+"/Items", nil).WithContext(context.Background())
@ -442,7 +460,8 @@ var _ = Describe("Playlists", func() {
})
Describe("getPlaylistUsers", func() {
It("returns the current user with CanEdit true", func() {
It("returns the current user with CanEdit true for an editable playlist", func() {
fp.getByIDPls = &model.Playlist{ID: testID("pl1")}
w := httptest.NewRecorder()
ctx := request.WithUser(context.Background(), model.User{ID: testID("u1"), UserName: "alice"})
r := httptest.NewRequest("GET", "/Playlists/"+testID("pl1")+"/Users", nil).WithContext(ctx)
@ -453,21 +472,59 @@ var _ = Describe("Playlists", func() {
Expect(json.Unmarshal(w.Body.Bytes(), &res)).To(Succeed())
Expect(res).To(Equal([]dto.PlaylistUserPermissions{{UserId: dto.EncodeID(testID("u1")), CanEdit: true}}))
})
It("reports CanEdit false for a synced playlist", func() {
fp.getByIDPls = &model.Playlist{ID: testID("pl1"), Sync: true}
w := httptest.NewRecorder()
ctx := request.WithUser(context.Background(), model.User{ID: testID("u1"), UserName: "alice"})
r := httptest.NewRequest("GET", "/Playlists/"+testID("pl1")+"/Users", nil).WithContext(ctx)
r = withChiURLParam(r, "playlistId", dto.EncodeID(testID("pl1")))
api.getPlaylistUsers(w, r)
Expect(w.Code).To(Equal(http.StatusOK))
var res []dto.PlaylistUserPermissions
Expect(json.Unmarshal(w.Body.Bytes(), &res)).To(Succeed())
Expect(res[0].CanEdit).To(BeFalse())
})
It("returns 404 when the playlist is not visible", func() {
fp.getByIDErr = model.ErrNotFound
w := httptest.NewRecorder()
ctx := request.WithUser(context.Background(), model.User{ID: testID("u1"), UserName: "alice"})
r := httptest.NewRequest("GET", "/Playlists/"+testID("pl1")+"/Users", nil).WithContext(ctx)
r = withChiURLParam(r, "playlistId", dto.EncodeID(testID("pl1")))
api.getPlaylistUsers(w, r)
Expect(w.Code).To(Equal(http.StatusNotFound))
})
})
Describe("getPlaylistUser", func() {
It("returns CanEdit true for the requested user", func() {
requestUser := func() *httptest.ResponseRecorder {
w := httptest.NewRecorder()
r := httptest.NewRequest("GET", "/Playlists/"+testID("pl1")+"/Users/"+testID("u1"), nil).WithContext(context.Background())
rctx := chi.NewRouteContext()
rctx.URLParams.Add("playlistId", testID("pl1"))
rctx.URLParams.Add("playlistId", dto.EncodeID(testID("pl1")))
rctx.URLParams.Add("userId", testID("u1"))
r = r.WithContext(context.WithValue(r.Context(), chi.RouteCtxKey, rctx))
api.getPlaylistUser(w, r)
return w
}
It("returns CanEdit true for an editable playlist", func() {
fp.getByIDPls = &model.Playlist{ID: testID("pl1")}
w := requestUser()
Expect(w.Code).To(Equal(http.StatusOK))
var res dto.PlaylistUserPermissions
Expect(json.Unmarshal(w.Body.Bytes(), &res)).To(Succeed())
Expect(res).To(Equal(dto.PlaylistUserPermissions{UserId: testID("u1"), CanEdit: true}))
})
It("reports CanEdit false for a synced playlist", func() {
fp.getByIDPls = &model.Playlist{ID: testID("pl1"), Sync: true}
w := requestUser()
Expect(w.Code).To(Equal(http.StatusOK))
var res dto.PlaylistUserPermissions
Expect(json.Unmarshal(w.Body.Bytes(), &res)).To(Succeed())
Expect(res.CanEdit).To(BeFalse())
})
})
})

View File

@ -20,6 +20,20 @@ import (
type restHandler = func(rest.RepositoryConstructor, ...rest.Logger) http.HandlerFunc
// writePlaylistError maps a playlist service error to an HTTP status, or defaultStatus if unknown.
func writePlaylistError(w http.ResponseWriter, err error, defaultStatus int) {
switch {
case errors.Is(err, model.ErrNotFound):
http.Error(w, err.Error(), http.StatusNotFound)
case errors.Is(err, model.ErrNotAuthorized):
http.Error(w, err.Error(), http.StatusForbidden)
case errors.Is(err, model.ErrPlaylistNotEditable):
http.Error(w, err.Error(), http.StatusConflict)
default:
http.Error(w, err.Error(), defaultStatus)
}
}
func playlistTracksHandler(pls playlists.Playlists, handler restHandler, refreshSmartPlaylist func(*http.Request) bool) http.HandlerFunc {
return func(w http.ResponseWriter, r *http.Request) {
plsId := chi.URLParam(r, "playlistId")
@ -111,7 +125,7 @@ func deleteFromPlaylist(pls playlists.Playlists) http.HandlerFunc {
}
if err != nil {
log.Error(r.Context(), "Error deleting tracks from playlist", "playlistId", playlistId, "ids", ids, err)
http.Error(w, err.Error(), http.StatusInternalServerError)
writePlaylistError(w, err, http.StatusInternalServerError)
return
}
writeDeleteManyResponse(w, r, ids)
@ -138,22 +152,22 @@ func addToPlaylist(pls playlists.Playlists) http.HandlerFunc {
}
count, c := 0, 0
if c, err = pls.AddTracks(ctx, playlistId, payload.Ids); err != nil {
http.Error(w, err.Error(), http.StatusBadRequest)
writePlaylistError(w, err, http.StatusBadRequest)
return
}
count += c
if c, err = pls.AddAlbums(ctx, playlistId, payload.AlbumIds); err != nil {
http.Error(w, err.Error(), http.StatusBadRequest)
writePlaylistError(w, err, http.StatusBadRequest)
return
}
count += c
if c, err = pls.AddArtists(ctx, playlistId, payload.ArtistIds); err != nil {
http.Error(w, err.Error(), http.StatusBadRequest)
writePlaylistError(w, err, http.StatusBadRequest)
return
}
count += c
if c, err = pls.AddDiscs(ctx, playlistId, payload.Discs); err != nil {
http.Error(w, err.Error(), http.StatusBadRequest)
writePlaylistError(w, err, http.StatusBadRequest)
return
}
count += c
@ -192,12 +206,8 @@ func reorderItem(pls playlists.Playlists) http.HandlerFunc {
return
}
err = pls.ReorderTrack(ctx, playlistId, id, newPos)
if errors.Is(err, model.ErrNotAuthorized) {
http.Error(w, err.Error(), http.StatusForbidden)
return
}
if err != nil {
http.Error(w, err.Error(), http.StatusBadRequest)
writePlaylistError(w, err, http.StatusBadRequest)
return
}

View File

@ -183,6 +183,20 @@ var _ = Describe("Playlist Tracks Endpoint", func() {
})
})
var _ = Describe("writePlaylistError", func() {
DescribeTable("maps a service error to an HTTP status",
func(err error, expected int) {
w := httptest.NewRecorder()
writePlaylistError(w, err, http.StatusBadRequest)
Expect(w.Code).To(Equal(expected))
},
Entry("not found -> 404", model.ErrNotFound, http.StatusNotFound),
Entry("not authorized -> 403", model.ErrNotAuthorized, http.StatusForbidden),
Entry("not editable -> 409", model.ErrPlaylistNotEditable, http.StatusConflict),
Entry("unrecognized -> default", model.ErrValidation, http.StatusBadRequest),
)
})
type mockPlaylistTrackRepo struct {
model.PlaylistTrackRepository
tracks model.PlaylistTracks

View File

@ -5,12 +5,12 @@ import (
"io"
"io/fs"
"os"
"path"
"path/filepath"
"testing/fstest"
"github.com/navidrome/navidrome/consts"
"github.com/navidrome/navidrome/resources"
"github.com/navidrome/navidrome/tests"
. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"
)
@ -18,13 +18,12 @@ import (
var _ = Describe("Translations", func() {
Describe("I18n files", func() {
It("contains only valid json language files", func() {
tests.SkipOnWindows("path separator bug (#TBD-path-sep-nativeapi)")
fsys := resources.FS()
dir, _ := fsys.Open(consts.I18nFolder)
files, _ := dir.(fs.ReadDirFile).ReadDir(-1)
for _, f := range files {
name := filepath.Base(f.Name())
filePath := filepath.Join(consts.I18nFolder, name)
filePath := path.Join(consts.I18nFolder, name)
file, _ := fsys.Open(filePath)
data, _ := io.ReadAll(file)
var out map[string]any

View File

@ -316,7 +316,8 @@ func mapToSubsonicError(err error) subError {
err = newError(responses.ErrorGeneric, err.Error())
case errors.Is(err, model.ErrNotFound), errors.Is(err, rest.ErrNotFound):
err = newError(responses.ErrorDataNotFound, "data not found")
case errors.Is(err, model.ErrNotAuthorized), errors.Is(err, rest.ErrPermissionDenied):
case errors.Is(err, model.ErrNotAuthorized), errors.Is(err, rest.ErrPermissionDenied),
errors.Is(err, model.ErrPlaylistNotEditable): // Subsonic has no code for "read-only resource"
err = newError(responses.ErrorAuthorizationFail)
case errors.Is(err, stream.ErrTooManyTranscodes):
err = newError(responses.ErrorGeneric, "too many concurrent transcodes, please retry shortly")

View File

@ -313,7 +313,7 @@ func newDummyImageCache(ctx context.Context) cache.FileCache {
func(context.Context, cache.Item) (io.Reader, error) {
return nil, errors.New("resize not exercised in subsonic artwork e2e")
})
Eventually(func() bool { return c.Available(ctx) }).Should(BeTrue())
Eventually(func() bool { return c.Available(ctx) }, 10*time.Second).Should(BeTrue())
return c
}

View File

@ -172,7 +172,7 @@ func buildOSPlaylist(ctx context.Context, p model.Playlist) *responses.OpenSubso
}
} else {
user, ok := request.UserFrom(ctx)
pls.Readonly = !ok || p.OwnerID != user.ID
pls.Readonly = !ok || p.OwnerID != user.ID || !p.TracksEditable()
}
return &pls

View File

@ -111,6 +111,15 @@ var _ = Describe("buildPlaylist", func() {
Expect(result.Public).To(BeTrue())
Expect(result.Readonly).To(BeFalse())
})
It("is read-only for a synced playlist even as owner", func() {
ctx = request.WithUser(ctx, model.User{ID: "1234", UserName: "admin"})
playlist.Sync = true
result := router.buildPlaylist(ctx, playlist)
Expect(result.Readonly).To(BeTrue())
})
})
Context("when minimal clients list is empty", func() {

View File

@ -12,4 +12,4 @@ export const isReadOnly = (ownerId) => {
export const isSmartPlaylist = (pls) => !!pls.rules
export const canChangeTracks = (pls) =>
isWritable(pls.ownerId) && !isSmartPlaylist(pls)
isWritable(pls.ownerId) && !isSmartPlaylist(pls) && !pls.sync

View File

@ -74,5 +74,11 @@ describe('playlistUtils', () => {
const playlist = { ownerId: 'user1', rules: [] }
expect(canChangeTracks(playlist)).toBe(false)
})
it('returns false if playlist is synced', () => {
localStorage.setItem('userId', 'user1')
const playlist = { ownerId: 'user1', sync: true }
expect(canChangeTracks(playlist)).toBe(false)
})
})
})

View File

@ -16,7 +16,7 @@ import {
import AddIcon from '@material-ui/icons/Add'
import { useGetList, useTranslate } from 'react-admin'
import PropTypes from 'prop-types'
import { isWritable } from '../common'
import { canChangeTracks } from '../common'
import { makeStyles } from '@material-ui/core'
const useStyles = makeStyles((theme) => ({
@ -268,8 +268,7 @@ export const SelectPlaylistInput = ({ onChange }) => {
)
const options =
ids &&
ids.map((id) => data[id]).filter((option) => isWritable(option.ownerId))
ids && ids.map((id) => data[id]).filter((option) => canChangeTracks(option))
// Filter playlists based on search text
const filteredOptions =

View File

@ -16,6 +16,7 @@ const mockPlaylists = [
{ id: 'playlist-2', name: 'Jazz Collection', ownerId: 'admin' },
{ id: 'playlist-3', name: 'Electronic Beats', ownerId: 'admin' },
{ id: 'playlist-4', name: 'Chill Vibes', ownerId: 'user2' }, // Not writable by admin
{ id: 'playlist-5', name: 'Synced List', ownerId: 'admin', sync: true },
]
const mockIndexedData = {
@ -27,6 +28,12 @@ const mockIndexedData = {
ownerId: 'admin',
},
'playlist-4': { id: 'playlist-4', name: 'Chill Vibes', ownerId: 'user2' },
'playlist-5': {
id: 'playlist-5',
name: 'Synced List',
ownerId: 'admin',
sync: true,
},
}
const createTestComponent = (
@ -89,6 +96,8 @@ describe('SelectPlaylistInput', () => {
// Should not show playlists not owned by admin (not writable)
expect(screen.queryByText('Chill Vibes')).not.toBeInTheDocument()
// Should not show synced playlists (their tracks are not editable)
expect(screen.queryByText('Synced List')).not.toBeInTheDocument()
})
it('should filter playlists based on search input', async () => {