diff --git a/core/playlists/import.go b/core/playlists/import.go index 9d3ecabc5..bafb870cd 100644 --- a/core/playlists/import.go +++ b/core/playlists/import.go @@ -8,7 +8,6 @@ import ( "os" "path/filepath" "strings" - "time" "github.com/navidrome/navidrome/conf" "github.com/navidrome/navidrome/log" @@ -187,7 +186,7 @@ func (s *playlists) updatePlaylist(ctx context.Context, newPls *model.Playlist, newPls.OwnerID = pls.OwnerID newPls.Public = pls.Public newPls.UploadedImage = pls.UploadedImage // Preserve manual upload - newPls.EvaluatedAt = &time.Time{} + newPls.EvaluatedAt = nil // force re-evaluation on next read } else { log.Info(ctx, "Adding synced playlist", "playlist", newPls.Name, "path", newPls.Path, "owner", owner.UserName) newPls.OwnerID = owner.ID diff --git a/core/playlists/rest_adapter.go b/core/playlists/rest_adapter.go index 3f886aadd..f34524e27 100644 --- a/core/playlists/rest_adapter.go +++ b/core/playlists/rest_adapter.go @@ -135,6 +135,7 @@ func (s *playlists) applyContentUpdate(ctx context.Context, current, entity *mod } if rulesChanged { current.Rules = entity.Rules + current.EvaluatedAt = nil // force re-evaluation on next read } if sent("sync") && current.Path != "" && current.Sync != entity.Sync { current.Sync = entity.Sync diff --git a/core/playlists/rest_adapter_test.go b/core/playlists/rest_adapter_test.go index 79d72d147..58a327bde 100644 --- a/core/playlists/rest_adapter_test.go +++ b/core/playlists/rest_adapter_test.go @@ -314,6 +314,38 @@ var _ = Describe("REST Adapter", func() { Expect(mockPlsRepo.Last.Public).To(BeTrue()) }) + It("resets EvaluatedAt when rules change", func() { + evaluatedAt := time.Now().Add(-1 * time.Hour) + mockPlsRepo.Data["smart-reset"] = &model.Playlist{ + ID: "smart-reset", + Name: "Smart", + OwnerID: "user-1", + Rules: &criteria.Criteria{Expression: criteria.Is{"genre": "Rock"}}, + EvaluatedAt: &evaluatedAt, + } + repo = ps.NewRepository(ctx).(rest.Persistable) + newRules := &criteria.Criteria{Expression: criteria.Is{"genre": "Jazz"}} + err := repo.Update("smart-reset", &model.Playlist{Rules: newRules}, "rules") + Expect(err).ToNot(HaveOccurred()) + Expect(mockPlsRepo.Last.EvaluatedAt).To(BeNil()) + }) + + It("keeps EvaluatedAt when rules are not changed", func() { + evaluatedAt := time.Now().Add(-1 * time.Hour) + mockPlsRepo.Data["smart-keep"] = &model.Playlist{ + ID: "smart-keep", + Name: "Smart", + OwnerID: "user-1", + Rules: &criteria.Criteria{Expression: criteria.Is{"genre": "Rock"}}, + EvaluatedAt: &evaluatedAt, + } + repo = ps.NewRepository(ctx).(rest.Persistable) + err := repo.Update("smart-keep", &model.Playlist{Name: "Renamed Smart"}, "name") + Expect(err).ToNot(HaveOccurred()) + Expect(mockPlsRepo.Last.EvaluatedAt).ToNot(BeNil()) + Expect(*mockPlsRepo.Last.EvaluatedAt).To(BeTemporally("~", evaluatedAt, time.Second)) + }) + It("updates name and rules together (smart-playlist Edit form)", func() { mockPlsRepo.Data["smart-edit"] = &model.Playlist{ ID: "smart-edit", diff --git a/model/criteria/criteria.go b/model/criteria/criteria.go index 8c3d183a9..5d7dc3826 100644 --- a/model/criteria/criteria.go +++ b/model/criteria/criteria.go @@ -4,9 +4,12 @@ package criteria import ( "encoding/json" "errors" + "fmt" "slices" + "time" "github.com/navidrome/navidrome/log" + "github.com/navidrome/navidrome/utils" ) type Expression interface { @@ -20,6 +23,7 @@ type Criteria struct { Limit int LimitPercent int Offset int + RefreshDelay time.Duration // 0 = use conf.Server.SmartPlaylistRefreshDelay } // EffectiveLimit resolves the effective limit for a query. If a fixed Limit is @@ -83,6 +87,7 @@ func (c Criteria) MarshalJSON() ([]byte, error) { Limit int `json:"limit,omitempty"` LimitPercent int `json:"limitPercent,omitempty"` Offset int `json:"offset,omitempty"` + RefreshDelay string `json:"refreshDelay,omitempty"` }{ Sort: c.Sort, Order: c.Order, @@ -90,6 +95,9 @@ func (c Criteria) MarshalJSON() ([]byte, error) { LimitPercent: c.LimitPercent, Offset: c.Offset, } + if c.RefreshDelay > 0 { + aux.RefreshDelay = utils.FormatDuration(c.RefreshDelay) + } switch rules := c.Expression.(type) { case Any: aux.Any = rules @@ -110,6 +118,7 @@ func (c *Criteria) UnmarshalJSON(data []byte) error { Limit int `json:"limit"` LimitPercent int `json:"limitPercent"` Offset int `json:"offset"` + RefreshDelay string `json:"refreshDelay"` } if err := json.Unmarshal(data, &aux); err != nil { return err @@ -131,6 +140,14 @@ func (c *Criteria) UnmarshalJSON(data []byte) error { c.Limit = aux.Limit c.Offset = aux.Offset + if aux.RefreshDelay != "" { + d, err := utils.ParseDuration(aux.RefreshDelay) + if err != nil { + return fmt.Errorf("invalid refreshDelay: %w", err) + } + c.RefreshDelay = d + } + // Clamp LimitPercent to [0, 100] if aux.LimitPercent < 0 { log.Warn("limitPercent value out of range, clamping to 0", "value", aux.LimitPercent) diff --git a/model/criteria/criteria_test.go b/model/criteria/criteria_test.go index 7f214e703..5e653150a 100644 --- a/model/criteria/criteria_test.go +++ b/model/criteria/criteria_test.go @@ -3,6 +3,7 @@ package criteria import ( "bytes" "encoding/json" + "time" "github.com/google/uuid" . "github.com/onsi/ginkgo/v2" @@ -255,6 +256,71 @@ var _ = Describe("Criteria", func() { }) }) + Describe("refreshDelay", func() { + newCriteria := func(extra string) []byte { + return []byte(`{"all":[{"is":{"loved":true}}]` + extra + `}`) + } + + It("unmarshals a valid refreshDelay", func() { + var c Criteria + gomega.Expect(json.Unmarshal(newCriteria(`,"refreshDelay":"1d"`), &c)).To(gomega.Succeed()) + gomega.Expect(c.RefreshDelay).To(gomega.Equal(24 * time.Hour)) + }) + + It("supports week units", func() { + var c Criteria + gomega.Expect(json.Unmarshal(newCriteria(`,"refreshDelay":"1w"`), &c)).To(gomega.Succeed()) + gomega.Expect(c.RefreshDelay).To(gomega.Equal(7 * 24 * time.Hour)) + }) + + It("leaves RefreshDelay zero when absent", func() { + var c Criteria + gomega.Expect(json.Unmarshal(newCriteria(``), &c)).To(gomega.Succeed()) + gomega.Expect(c.RefreshDelay).To(gomega.BeZero()) + }) + + It("rejects an invalid refreshDelay", func() { + var c Criteria + err := json.Unmarshal(newCriteria(`,"refreshDelay":"tomorrow"`), &c) + gomega.Expect(err).To(gomega.MatchError(gomega.ContainSubstring("refreshDelay"))) + }) + + It("rejects a negative refreshDelay", func() { + var c Criteria + err := json.Unmarshal(newCriteria(`,"refreshDelay":"-1h"`), &c) + gomega.Expect(err).To(gomega.MatchError(gomega.ContainSubstring("refreshDelay"))) + }) + + It("marshals RefreshDelay back as a duration string", func() { + c := Criteria{ + Expression: All{Is{"loved": true}}, + RefreshDelay: 24 * time.Hour, + } + j, err := json.Marshal(c) + gomega.Expect(err).ToNot(gomega.HaveOccurred()) + gomega.Expect(string(j)).To(gomega.ContainSubstring(`"refreshDelay":"1d"`)) + }) + + It("omits refreshDelay from JSON when zero", func() { + c := Criteria{Expression: All{Is{"loved": true}}} + j, err := json.Marshal(c) + gomega.Expect(err).ToNot(gomega.HaveOccurred()) + gomega.Expect(string(j)).ToNot(gomega.ContainSubstring("refreshDelay")) + }) + + It("round-trips through marshal and unmarshal", func() { + c := Criteria{ + Expression: All{Is{"loved": true}}, + RefreshDelay: 36 * time.Hour, + } + j, err := json.Marshal(c) + gomega.Expect(err).ToNot(gomega.HaveOccurred()) + var c2 Criteria + gomega.Expect(json.Unmarshal(j, &c2)).To(gomega.Succeed()) + gomega.Expect(c2.RefreshDelay).To(gomega.Equal(36 * time.Hour)) + }) + }) + Context("with child playlists", func() { var ( topLevelInPlaylistID string diff --git a/model/playlist.go b/model/playlist.go index 40adb8d0a..185f6f942 100644 --- a/model/playlist.go +++ b/model/playlist.go @@ -6,6 +6,7 @@ import ( "strconv" "time" + "github.com/navidrome/navidrome/conf" "github.com/navidrome/navidrome/consts" "github.com/navidrome/navidrome/model/criteria" ) @@ -39,6 +40,15 @@ func (pls Playlist) IsSmartPlaylist() bool { return pls.Rules != nil && pls.Rules.Expression != nil } +// RefreshDelay returns the playlist's own refresh window when set, falling +// back to the global SmartPlaylistRefreshDelay. +func (pls Playlist) RefreshDelay() time.Duration { + if pls.IsSmartPlaylist() && pls.Rules.RefreshDelay > 0 { + return pls.Rules.RefreshDelay + } + return conf.Server.SmartPlaylistRefreshDelay +} + func (pls Playlist) MediaFiles() MediaFiles { if len(pls.Tracks) == 0 { return nil diff --git a/model/playlist_test.go b/model/playlist_test.go index 9ed24f00f..d936129ce 100644 --- a/model/playlist_test.go +++ b/model/playlist_test.go @@ -1,7 +1,12 @@ package model_test import ( + "time" + + "github.com/navidrome/navidrome/conf" + "github.com/navidrome/navidrome/conf/configtest" "github.com/navidrome/navidrome/model" + "github.com/navidrome/navidrome/model/criteria" "github.com/navidrome/navidrome/tests" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" @@ -43,4 +48,29 @@ var _ = Describe("Playlist", func() { Expect(pls.ToM3U8()).To(Equal(expected)) }) }) + + Describe("RefreshDelay", func() { + BeforeEach(func() { + DeferCleanup(configtest.SetupConfig()) + conf.Server.SmartPlaylistRefreshDelay = 5 * time.Second + }) + + It("returns the global config value when rules have no refreshDelay", func() { + pls := model.Playlist{Rules: &criteria.Criteria{Expression: criteria.All{criteria.Is{"loved": true}}}} + Expect(pls.RefreshDelay()).To(Equal(5 * time.Second)) + }) + + It("returns the per-playlist value when set", func() { + pls := model.Playlist{Rules: &criteria.Criteria{ + Expression: criteria.All{criteria.Is{"loved": true}}, + RefreshDelay: 24 * time.Hour, + }} + Expect(pls.RefreshDelay()).To(Equal(24 * time.Hour)) + }) + + It("returns the global value for non-smart playlists", func() { + pls := model.Playlist{} + Expect(pls.RefreshDelay()).To(Equal(5 * time.Second)) + }) + }) }) diff --git a/persistence/smart_playlist_repository.go b/persistence/smart_playlist_repository.go index 54f316152..f7c41597d 100644 --- a/persistence/smart_playlist_repository.go +++ b/persistence/smart_playlist_repository.go @@ -4,7 +4,6 @@ import ( "time" . "github.com/Masterminds/squirrel" - "github.com/navidrome/navidrome/conf" "github.com/navidrome/navidrome/log" "github.com/navidrome/navidrome/model" ) @@ -77,7 +76,7 @@ func (r *playlistRepository) shouldRefreshSmartPlaylist(pls *model.Playlist, usr if !pls.IsSmartPlaylist() { return false } - if pls.EvaluatedAt != nil && time.Since(*pls.EvaluatedAt) < conf.Server.SmartPlaylistRefreshDelay { + if pls.EvaluatedAt != nil && time.Since(*pls.EvaluatedAt) < pls.RefreshDelay() { return false } if pls.OwnerID != usr.ID { diff --git a/persistence/smart_playlist_repository_test.go b/persistence/smart_playlist_repository_test.go index 7bc705385..8e3ae488f 100644 --- a/persistence/smart_playlist_repository_test.go +++ b/persistence/smart_playlist_repository_test.go @@ -147,6 +147,50 @@ var _ = Describe("PlaylistRepository - Smart Playlists", func() { Expect(*nestedPlsAfterParentGet.EvaluatedAt).To(Equal(*nestedPlsRead.EvaluatedAt)) }) }) + + Context("per-playlist refreshDelay", func() { + BeforeEach(func() { + DeferCleanup(configtest.SetupConfig()) + }) + + It("does NOT refresh when the per-playlist delay has not elapsed, even if global has", func() { + conf.Server.SmartPlaylistRefreshDelay = -1 * time.Second + evaluatedAt := time.Now().Add(-1 * time.Hour) + + rules := &criteria.Criteria{ + Expression: criteria.All{criteria.Contains{"title": "Day"}}, + RefreshDelay: 24 * time.Hour, + } + pls := model.Playlist{Name: "Frozen Daily", OwnerID: "userid", Rules: rules, EvaluatedAt: &evaluatedAt} + Expect(repo.Put(&pls)).To(Succeed()) + DeferCleanup(func() { _ = repo.Delete(pls.ID) }) + + got, err := repo.GetWithTracks(pls.ID, true, false) + Expect(err).ToNot(HaveOccurred()) + // Not re-evaluated: EvaluatedAt unchanged, no tracks materialized + Expect(*got.EvaluatedAt).To(BeTemporally("~", evaluatedAt, time.Second)) + Expect(got.Tracks).To(BeEmpty()) + }) + + It("refreshes when the per-playlist delay has elapsed, even if global has not", func() { + conf.Server.SmartPlaylistRefreshDelay = 1 * time.Hour + evaluatedAt := time.Now().Add(-10 * time.Minute) + + rules := &criteria.Criteria{ + Expression: criteria.All{criteria.Contains{"title": "Day"}}, + RefreshDelay: 5 * time.Minute, + } + pls := model.Playlist{Name: "Fast Refresh", OwnerID: "userid", Rules: rules, EvaluatedAt: &evaluatedAt} + Expect(repo.Put(&pls)).To(Succeed()) + DeferCleanup(func() { _ = repo.Delete(pls.ID) }) + + got, err := repo.GetWithTracks(pls.ID, true, false) + Expect(err).ToNot(HaveOccurred()) + Expect(*got.EvaluatedAt).To(BeTemporally("~", time.Now(), 2*time.Second)) + Expect(got.Tracks).To(HaveLen(1)) + Expect(got.Tracks[0].MediaFileID).To(Equal(songDayInALife.ID)) + }) + }) }) }) diff --git a/server/subsonic/playlists.go b/server/subsonic/playlists.go index 7101f9f15..c58fb9ab9 100644 --- a/server/subsonic/playlists.go +++ b/server/subsonic/playlists.go @@ -168,7 +168,7 @@ func buildOSPlaylist(ctx context.Context, p model.Playlist) *responses.OpenSubso pls.Readonly = true if p.EvaluatedAt != nil { - pls.ValidUntil = new(p.EvaluatedAt.Add(conf.Server.SmartPlaylistRefreshDelay)) + pls.ValidUntil = new(p.EvaluatedAt.Add(p.RefreshDelay())) } } else { user, ok := request.UserFrom(ctx) diff --git a/server/subsonic/playlists_test.go b/server/subsonic/playlists_test.go index 697dd5852..f0a2f8ac5 100644 --- a/server/subsonic/playlists_test.go +++ b/server/subsonic/playlists_test.go @@ -248,6 +248,20 @@ var _ = Describe("buildPlaylist", func() { Expect(result.OpenSubsonicPlaylist).To(BeNil()) }) }) + + Context("with a per-playlist refreshDelay", func() { + BeforeEach(func() { + playlist.Rules.RefreshDelay = 24 * time.Hour + player := model.Player{Client: "regular-client"} + ctx = request.WithPlayer(ctx, player) + }) + + It("computes validUntil from the playlist's own delay", func() { + result := router.buildPlaylist(ctx, playlist) + expected := evaluatedAt.Add(24 * time.Hour) + Expect(result.ValidUntil).To(Equal(&expected)) + }) + }) }) Describe("annotation leakage", func() { diff --git a/utils/time.go b/utils/time.go index c1e949589..b3f1a98fb 100644 --- a/utils/time.go +++ b/utils/time.go @@ -1,6 +1,12 @@ package utils -import "time" +import ( + "fmt" + "regexp" + "strconv" + "strings" + "time" +) func TimeNewest(times ...time.Time) time.Time { newest := time.Time{} @@ -11,3 +17,59 @@ func TimeNewest(times ...time.Time) time.Time { } return newest } + +var durationDayWeekRe = regexp.MustCompile(`-?\d+(?:\.\d+)?[dw]`) + +// ParseDuration is time.ParseDuration extended with d (24h) and w (168h) units. +// Negative durations are rejected. +func ParseDuration(s string) (time.Duration, error) { + expanded := durationDayWeekRe.ReplaceAllStringFunc(s, func(match string) string { + value, err := strconv.ParseFloat(match[:len(match)-1], 64) + if err != nil { + return match + } + hours := value * 24 + if match[len(match)-1] == 'w' { + hours = value * 24 * 7 + } + return strconv.FormatFloat(hours, 'f', -1, 64) + "h" + }) + d, err := time.ParseDuration(expanded) + if err != nil { + return 0, fmt.Errorf("invalid duration %q: %w", s, err) + } + if d < 0 { + return 0, fmt.Errorf("negative duration not allowed: %q", s) + } + return d, nil +} + +// FormatDuration renders whole w/d multiples with those units, falling back to +// time.Duration.String for the sub-day remainder, so ParseDuration round-trips. +func FormatDuration(d time.Duration) string { + if d < 24*time.Hour { + return formatSubDay(d) + } + var b strings.Builder + weekDuration := 7 * 24 * time.Hour + if weeks := d / weekDuration; weeks > 0 { + b.WriteString(strconv.Itoa(int(weeks)) + "w") + d %= weekDuration + } + dayDuration := 24 * time.Hour + if days := d / dayDuration; days > 0 { + b.WriteString(strconv.Itoa(int(days)) + "d") + d %= dayDuration + } + if d > 0 { + b.WriteString(formatSubDay(d)) + } + return b.String() +} + +func formatSubDay(d time.Duration) string { + if d >= time.Hour && d%time.Hour == 0 { + return strconv.Itoa(int(d/time.Hour)) + "h" + } + return d.String() +} diff --git a/utils/time_test.go b/utils/time_test.go index f89f0d2be..8460b98f9 100644 --- a/utils/time_test.go +++ b/utils/time_test.go @@ -26,3 +26,74 @@ var _ = Describe("TimeNewest", func() { Expect(utils.TimeNewest(t1, t2, t3)).To(Equal(t2)) }) }) + +var _ = Describe("ParseDuration", func() { + DescribeTable("parses valid durations", + func(input string, expected time.Duration) { + d, err := utils.ParseDuration(input) + Expect(err).ToNot(HaveOccurred()) + Expect(d).To(Equal(expected)) + }, + Entry("standard Go units", "90m", 90*time.Minute), + Entry("hours", "12h", 12*time.Hour), + Entry("days", "1d", 24*time.Hour), + Entry("weeks", "1w", 7*24*time.Hour), + Entry("multiple days", "3d", 72*time.Hour), + Entry("mixed day and hours", "1d12h", 36*time.Hour), + Entry("mixed week, day and hours", "1w2d3h", (7*24+2*24+3)*time.Hour), + Entry("fractional days", "0.5d", 12*time.Hour), + ) + + DescribeTable("rejects invalid durations", + func(input string) { + _, err := utils.ParseDuration(input) + Expect(err).To(HaveOccurred()) + }, + Entry("empty string", ""), + Entry("not a duration", "tomorrow"), + Entry("bare number", "42"), + Entry("unit only", "d"), + Entry("unknown unit", "5y"), + ) + + DescribeTable("rejects negative durations", + func(input string) { + _, err := utils.ParseDuration(input) + Expect(err).To(MatchError(ContainSubstring("negative duration"))) + }, + Entry("negative days", "-1d"), + Entry("negative weeks", "-0.5w"), + Entry("negative Go units", "-30m"), + ) +}) + +var _ = Describe("FormatDuration", func() { + DescribeTable("formats durations using the largest whole units", + func(input time.Duration, expected string) { + Expect(utils.FormatDuration(input)).To(Equal(expected)) + }, + Entry("whole weeks", 7*24*time.Hour, "1w"), + Entry("whole days", 24*time.Hour, "1d"), + Entry("multiple days", 72*time.Hour, "3d"), + Entry("day and hours", 36*time.Hour, "1d12h"), + Entry("week, day and hours", (7*24+2*24+3)*time.Hour, "1w2d3h"), + Entry("hours only", 12*time.Hour, "12h"), + Entry("sub-hour", 90*time.Minute, "1h30m0s"), + Entry("zero", time.Duration(0), "0s"), + ) + + DescribeTable("round-trips through ParseDuration", + func(input string) { + d, err := utils.ParseDuration(input) + Expect(err).ToNot(HaveOccurred()) + formatted := utils.FormatDuration(d) + d2, err := utils.ParseDuration(formatted) + Expect(err).ToNot(HaveOccurred()) + Expect(d2).To(Equal(d)) + }, + Entry("1d", "1d"), + Entry("1w", "1w"), + Entry("1d12h", "1d12h"), + Entry("90m", "90m"), + ) +})