mirror of
https://github.com/navidrome/navidrome.git
synced 2026-10-08 02:17:25 +02:00
fix(subsonic): update the playlist changed timestamp when renaming a smart playlist (#6082)
`buildPlaylist` reported `evaluated_at` as `changed` for smart playlists, so a rename or comment edit was invisible to clients until the next evaluation. A never-evaluated smart playlist also reported the current time on every call, which never settled. Report `updated_at` for every playlist. `refreshCounters` now syncs the stamp it writes back onto the model, and `refreshSmartPlaylist` reuses it for `evaluated_at`. `changed` therefore still equals the evaluation time for a just-evaluated playlist, and `validUntil` stays anchored to it. Original Subsonic always bumps `changed` on any playlist update, so this also aligns the behavior with upstream.
This commit is contained in:
parent
afb3a2f881
commit
a7365e119b
5 changed files with 40 additions and 14 deletions
|
|
@ -316,11 +316,12 @@ func (r *playlistRepository) refreshCounters(pls *model.Playlist) error {
|
|||
}
|
||||
|
||||
// Update playlist's total duration, size and count
|
||||
now := time.Now()
|
||||
upd := Update("playlist").
|
||||
Set("duration", res.Duration).
|
||||
Set("size", res.Size).
|
||||
Set("song_count", res.Count).
|
||||
Set("updated_at", time.Now()).
|
||||
Set("updated_at", now).
|
||||
Where(Eq{"id": pls.ID})
|
||||
_, err = r.executeSQL(upd)
|
||||
if err != nil {
|
||||
|
|
@ -329,6 +330,7 @@ func (r *playlistRepository) refreshCounters(pls *model.Playlist) error {
|
|||
pls.SongCount = int(res.Count)
|
||||
pls.Duration = res.Duration
|
||||
pls.Size = int64(res.Size)
|
||||
pls.UpdatedAt = now
|
||||
return nil
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -58,7 +58,8 @@ func (r *playlistRepository) refreshSmartPlaylist(pls *model.Playlist) bool {
|
|||
return false
|
||||
}
|
||||
|
||||
now := time.Now()
|
||||
// Reuse the stamp refreshCounters just wrote, so evaluated_at and updated_at agree
|
||||
now := pls.UpdatedAt
|
||||
updSql := Update(r.tableName).Set("evaluated_at", now).Where(Eq{"id": pls.ID})
|
||||
if _, err = r.executeSQL(updSql); err != nil {
|
||||
log.Error(r.ctx, "Error updating smart playlist", "playlist", pls.Name, "id", pls.ID, err)
|
||||
|
|
|
|||
|
|
@ -45,6 +45,23 @@ var _ = Describe("PlaylistRepository - Smart Playlists", func() {
|
|||
})
|
||||
})
|
||||
|
||||
Context("after an evaluation", func() {
|
||||
It("stamps updated_at and evaluated_at with the same instant", func() {
|
||||
newPls := model.Playlist{Name: "Evaluated", OwnerID: "userid", Rules: rules}
|
||||
Expect(repo.Put(&newPls)).To(Succeed())
|
||||
DeferCleanup(func() { _ = repo.Delete(newPls.ID) })
|
||||
|
||||
refreshed, err := repo.GetWithTracks(newPls.ID, true, false)
|
||||
Expect(err).ToNot(HaveOccurred())
|
||||
|
||||
stored, err := repo.Get(newPls.ID)
|
||||
Expect(err).ToNot(HaveOccurred())
|
||||
Expect(stored.EvaluatedAt).ToNot(BeNil())
|
||||
Expect(stored.UpdatedAt).To(BeTemporally("==", *stored.EvaluatedAt))
|
||||
Expect(refreshed.UpdatedAt).To(BeTemporally("==", stored.UpdatedAt))
|
||||
})
|
||||
})
|
||||
|
||||
Context("invalid rules", func() {
|
||||
It("fails to Put it in the DB", func() {
|
||||
rules = &criteria.Criteria{
|
||||
|
|
|
|||
|
|
@ -5,7 +5,6 @@ import (
|
|||
"errors"
|
||||
"fmt"
|
||||
"net/http"
|
||||
"time"
|
||||
|
||||
"github.com/navidrome/navidrome/conf"
|
||||
"github.com/navidrome/navidrome/log"
|
||||
|
|
@ -133,15 +132,7 @@ func (api *Router) buildPlaylist(ctx context.Context, p model.Playlist) response
|
|||
pls.SongCount = int32(p.SongCount)
|
||||
pls.Duration = int32(p.Duration)
|
||||
pls.Created = p.CreatedAt
|
||||
if p.IsSmartPlaylist() {
|
||||
if p.EvaluatedAt != nil {
|
||||
pls.Changed = *p.EvaluatedAt
|
||||
} else {
|
||||
pls.Changed = time.Now()
|
||||
}
|
||||
} else {
|
||||
pls.Changed = p.UpdatedAt
|
||||
}
|
||||
pls.Changed = p.UpdatedAt
|
||||
|
||||
player, ok := request.PlayerFrom(ctx)
|
||||
if ok && isClientInList(conf.Server.Subsonic.MinimalClients, player.Client) {
|
||||
|
|
|
|||
|
|
@ -220,7 +220,7 @@ var _ = Describe("buildPlaylist", func() {
|
|||
Expect(result.SongCount).To(Equal(int32(10)))
|
||||
Expect(result.Duration).To(Equal(int32(600)))
|
||||
Expect(result.Created).To(Equal(playlist.CreatedAt))
|
||||
Expect(result.Changed).To(Equal(evaluatedAt))
|
||||
Expect(result.Changed).To(Equal(playlist.UpdatedAt))
|
||||
|
||||
// These should not be set
|
||||
Expect(result.Comment).To(BeEmpty())
|
||||
|
|
@ -245,7 +245,7 @@ var _ = Describe("buildPlaylist", func() {
|
|||
Expect(result.SongCount).To(Equal(int32(10)))
|
||||
Expect(result.Duration).To(Equal(int32(600)))
|
||||
Expect(result.Created).To(Equal(playlist.CreatedAt))
|
||||
Expect(result.Changed).To(Equal(*playlist.EvaluatedAt))
|
||||
Expect(result.Changed).To(Equal(playlist.UpdatedAt))
|
||||
Expect(result.Comment).To(Equal("Test comment"))
|
||||
Expect(result.Owner).To(Equal("admin"))
|
||||
Expect(result.Public).To(BeTrue())
|
||||
|
|
@ -271,6 +271,21 @@ var _ = Describe("buildPlaylist", func() {
|
|||
})
|
||||
})
|
||||
|
||||
Context("when it was never evaluated", func() {
|
||||
BeforeEach(func() {
|
||||
playlist.EvaluatedAt = nil
|
||||
player := model.Player{Client: "regular-client"}
|
||||
ctx = request.WithPlayer(ctx, player)
|
||||
})
|
||||
|
||||
It("omits validUntil but still reports changed", func() {
|
||||
result := router.buildPlaylist(ctx, playlist)
|
||||
|
||||
Expect(result.ValidUntil).To(BeNil())
|
||||
Expect(result.Changed).To(Equal(playlist.UpdatedAt))
|
||||
})
|
||||
})
|
||||
|
||||
Context("with a per-playlist refreshDelay", func() {
|
||||
BeforeEach(func() {
|
||||
playlist.Rules.RefreshDelay = 24 * time.Hour
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue