diff --git a/core/playlists/playlists.go b/core/playlists/playlists.go index 4ba6e76fd..4ee35eb8e 100644 --- a/core/playlists/playlists.go +++ b/core/playlists/playlists.go @@ -163,8 +163,10 @@ func (s *playlists) Update(ctx context.Context, playlistID string, return err } // Plugin playlists allow public toggle and cover art, but block name/comment changes - if pls.IsPluginPlaylist() && (name != nil || comment != nil) { - return model.ErrNotAuthorized + if pls.IsPluginPlaylist() { + if (name != nil && *name != pls.Name) || (comment != nil && *comment != pls.Comment) { + return model.ErrNotAuthorized + } } return s.ds.WithTxImmediate(func(tx model.DataStore) error { repo := tx.Playlist(ctx) diff --git a/core/playlists/rest_adapter.go b/core/playlists/rest_adapter.go index 33073e7da..818d8a5cb 100644 --- a/core/playlists/rest_adapter.go +++ b/core/playlists/rest_adapter.go @@ -3,6 +3,7 @@ package playlists import ( "context" "errors" + "slices" "github.com/deluan/rest" "github.com/navidrome/navidrome/model" @@ -94,7 +95,16 @@ func (s *playlists) updatePlaylistEntity(ctx context.Context, id string, entity return rest.ErrPermissionDenied } // Plugin playlists allow public and ownership changes, but block name/comment - if current.IsPluginPlaylist() && (entity.Name != current.Name || entity.Comment != current.Comment) { + if current.IsPluginPlaylist() && slices.ContainsFunc(cols, func(c string) bool { + switch c { + case "name": + return entity.Name != current.Name + case "comment": + return entity.Comment != current.Comment + default: + return false + } + }) { return rest.ErrPermissionDenied } // Apply ownership change (admin only) diff --git a/core/playlists/rest_adapter_test.go b/core/playlists/rest_adapter_test.go index 05f4b3be2..825c53656 100644 --- a/core/playlists/rest_adapter_test.go +++ b/core/playlists/rest_adapter_test.go @@ -110,7 +110,7 @@ var _ = Describe("REST Adapter", func() { ctx = request.WithUser(ctx, model.User{ID: "user-1", IsAdmin: false}) repo = ps.NewRepository(ctx).(rest.Persistable) pls := &model.Playlist{Name: "Changed Name", Comment: ""} - err := repo.Update("pls-plugin", pls) + err := repo.Update("pls-plugin", pls, "name", "comment") Expect(err).To(Equal(rest.ErrPermissionDenied)) }) @@ -122,7 +122,7 @@ var _ = Describe("REST Adapter", func() { ctx = request.WithUser(ctx, model.User{ID: "user-1", IsAdmin: false}) repo = ps.NewRepository(ctx).(rest.Persistable) pls := &model.Playlist{Name: "Plugin PL", Comment: "new comment"} - err := repo.Update("pls-plugin", pls) + err := repo.Update("pls-plugin", pls, "name", "comment") Expect(err).To(Equal(rest.ErrPermissionDenied)) }) @@ -134,7 +134,7 @@ var _ = Describe("REST Adapter", func() { ctx = request.WithUser(ctx, model.User{ID: "user-1", IsAdmin: false}) repo = ps.NewRepository(ctx).(rest.Persistable) pls := &model.Playlist{Name: "Plugin PL", Public: true} - err := repo.Update("pls-plugin", pls) + err := repo.Update("pls-plugin", pls, "public") Expect(err).ToNot(HaveOccurred()) }) diff --git a/plugins/playlist_provider.go b/plugins/playlist_provider.go index a1e894873..9ab62a0da 100644 --- a/plugins/playlist_provider.go +++ b/plugins/playlist_provider.go @@ -119,10 +119,8 @@ func (p *playlistSyncer) discoverAndSync() { return } - // Store retry interval from response - if resp.RetryInterval > 0 { - p.retryInterval.Store(int64(time.Duration(resp.RetryInterval) * time.Second)) - } + // Store retry interval from response (including 0, which disables retries) + p.retryInterval.Store(int64(time.Duration(resp.RetryInterval) * time.Second)) resolvedUsers := map[string]string{} // username -> userID cache for _, info := range resp.Playlists { @@ -150,9 +148,12 @@ func (p *playlistSyncer) discoverAndSync() { p.syncPlaylist(info, dbID, ownerID) } - // Schedule re-discovery if RefreshInterval > 0 + // Schedule re-discovery if RefreshInterval > 0, otherwise cancel any existing timer if resp.RefreshInterval > 0 { p.scheduleDiscovery(time.Duration(resp.RefreshInterval) * time.Second) + } else if p.discoveryTimer != nil { + p.discoveryTimer.Stop() + p.discoveryTimer = nil } } @@ -165,12 +166,7 @@ func (p *playlistSyncer) syncPlaylist(info capabilities.PlaylistInfo, dbID strin if err != nil { if isPlaylistNotFoundError(err) { log.Info(ctx, "Playlist not found, skipping", "plugin", p.pluginName, "playlistID", info.ID) - // Stop any existing refresh timer for this playlist - if timer, ok := p.refreshTimers[dbID]; ok { - timer.Stop() - delete(p.refreshTimers, dbID) - p.refreshTimerCount.Store(int32(len(p.refreshTimers))) - } + p.cancelRefreshTimer(dbID) return } log.Warn(ctx, "Failed to call GetPlaylist", "plugin", p.pluginName, "playlistID", info.ID, err) @@ -218,7 +214,7 @@ func (p *playlistSyncer) syncPlaylist(info capabilities.PlaylistInfo, dbID strin log.Info(ctx, "Synced plugin playlist", "plugin", p.pluginName, "playlistID", info.ID, "name", resp.Name, "tracks", len(matched), "owner", ownerID) - // Schedule refresh if ValidUntil > 0 + // Schedule refresh if ValidUntil > 0, otherwise cancel any stale timer if resp.ValidUntil > 0 { validUntil := time.Unix(resp.ValidUntil, 0) delay := time.Until(validUntil) @@ -226,6 +222,17 @@ func (p *playlistSyncer) syncPlaylist(info capabilities.PlaylistInfo, dbID strin delay = 1 * time.Second // Already expired, refresh soon } p.schedulePlaylistRefresh(info, dbID, ownerID, delay) + } else { + p.cancelRefreshTimer(dbID) + } +} + +// cancelRefreshTimer stops and removes the refresh timer for the given playlist DB ID, if any. +func (p *playlistSyncer) cancelRefreshTimer(dbID string) { + if timer, ok := p.refreshTimers[dbID]; ok { + timer.Stop() + delete(p.refreshTimers, dbID) + p.refreshTimerCount.Store(int32(len(p.refreshTimers))) } }