diff --git a/persistence/playlist_repository.go b/persistence/playlist_repository.go index bf8d6d5a8..bc6bffd25 100644 --- a/persistence/playlist_repository.go +++ b/persistence/playlist_repository.go @@ -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 } diff --git a/persistence/smart_playlist_repository.go b/persistence/smart_playlist_repository.go index 65ae4656b..9d2ac9590 100644 --- a/persistence/smart_playlist_repository.go +++ b/persistence/smart_playlist_repository.go @@ -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) diff --git a/persistence/smart_playlist_repository_test.go b/persistence/smart_playlist_repository_test.go index ddc155fab..6f8684d5c 100644 --- a/persistence/smart_playlist_repository_test.go +++ b/persistence/smart_playlist_repository_test.go @@ -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{ diff --git a/server/subsonic/playlists.go b/server/subsonic/playlists.go index 774a9c430..e64fc9e82 100644 --- a/server/subsonic/playlists.go +++ b/server/subsonic/playlists.go @@ -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) { diff --git a/server/subsonic/playlists_test.go b/server/subsonic/playlists_test.go index f18f33b47..c7775c0fa 100644 --- a/server/subsonic/playlists_test.go +++ b/server/subsonic/playlists_test.go @@ -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