From fde4517339cb6035b7d9682e02382df281199256 Mon Sep 17 00:00:00 2001 From: Deluan Date: Thu, 5 Mar 2026 13:27:02 -0500 Subject: [PATCH] refactor(plugins): rename GetPlaylists, add NotFound error and RetryInterval MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Rename GetPlaylists → GetAvailablePlaylists for clarity. Add PlaylistGeneratorErrorNotFound so plugins can signal a playlist is temporarily unavailable (orchestrator skips it but retries on next discovery). Add RetryInterval on GetAvailablePlaylistsResponse for automatic retry of transient GetPlaylist failures. --- plugins/capabilities/playlist_generator.go | 34 ++++++--- plugins/capabilities/playlist_generator.yaml | 55 +++++++------- .../go/playlistgenerator/playlistgenerator.go | 64 ++++++++++------- .../playlistgenerator_stub.go | 48 ++++++++----- .../src/playlistgenerator.rs | 58 ++++++++------- plugins/playlist_generator.go | 41 ++++++++--- plugins/playlist_generator_test.go | 71 ++++++++++++++++++- .../testdata/test-playlist-generator/main.go | 23 ++++-- 8 files changed, 279 insertions(+), 115 deletions(-) diff --git a/plugins/capabilities/playlist_generator.go b/plugins/capabilities/playlist_generator.go index 5f7cb077c..4d034a7c0 100644 --- a/plugins/capabilities/playlist_generator.go +++ b/plugins/capabilities/playlist_generator.go @@ -2,30 +2,33 @@ package capabilities // PlaylistGenerator provides dynamically-generated playlists (e.g., "Daily Mix", // personalized recommendations). Plugins implementing this capability expose two -// functions: GetPlaylists for lightweight discovery and GetPlaylist for fetching -// the heavy payload (tracks, metadata). +// functions: GetAvailablePlaylists for lightweight discovery and GetPlaylist for +// fetching the heavy payload (tracks, metadata). // //nd:capability name=playlistgenerator required=true type PlaylistGenerator interface { - // GetPlaylists returns the list of playlists this plugin provides. - //nd:export name=nd_playlist_generator_get_playlists - GetPlaylists(GetPlaylistsRequest) (GetPlaylistsResponse, error) + // GetAvailablePlaylists returns the list of playlists this plugin provides. + //nd:export name=nd_playlist_generator_get_available_playlists + GetAvailablePlaylists(GetAvailablePlaylistsRequest) (GetAvailablePlaylistsResponse, error) // GetPlaylist returns the full data for a single playlist (tracks, metadata). //nd:export name=nd_playlist_generator_get_playlist GetPlaylist(GetPlaylistRequest) (GetPlaylistResponse, error) } -// GetPlaylistsRequest is the request for GetPlaylists. -type GetPlaylistsRequest struct{} +// GetAvailablePlaylistsRequest is the request for GetAvailablePlaylists. +type GetAvailablePlaylistsRequest struct{} -// GetPlaylistsResponse is the response for GetPlaylists. -type GetPlaylistsResponse struct { +// GetAvailablePlaylistsResponse is the response for GetAvailablePlaylists. +type GetAvailablePlaylistsResponse struct { // Playlists is the list of playlists provided by this plugin. Playlists []PlaylistInfo `json:"playlists"` - // RefreshInterval is the number of seconds until the next GetPlaylists call. + // RefreshInterval is the number of seconds until the next GetAvailablePlaylists call. // 0 means never re-discover. RefreshInterval int64 `json:"refreshInterval"` + // RetryInterval is the number of seconds before retrying a failed GetPlaylist call. + // 0 means no automatic retry for transient errors. + RetryInterval int64 `json:"retryInterval"` } // PlaylistInfo identifies a plugin playlist and its target user. @@ -56,3 +59,14 @@ type GetPlaylistResponse struct { // 0 means static (never refresh). ValidUntil int64 `json:"validUntil"` } + +// PlaylistGeneratorError represents an error type for playlist generator operations. +type PlaylistGeneratorError string + +const ( + // PlaylistGeneratorErrorNotFound indicates a playlist is currently unavailable. + PlaylistGeneratorErrorNotFound PlaylistGeneratorError = "playlist_generator(not_found)" +) + +// Error implements the error interface for PlaylistGeneratorError. +func (e PlaylistGeneratorError) Error() string { return string(e) } diff --git a/plugins/capabilities/playlist_generator.yaml b/plugins/capabilities/playlist_generator.yaml index 2b4f57a70..519387660 100644 --- a/plugins/capabilities/playlist_generator.yaml +++ b/plugins/capabilities/playlist_generator.yaml @@ -1,12 +1,12 @@ version: v1-draft exports: - nd_playlist_generator_get_playlists: - description: GetPlaylists returns the list of playlists this plugin provides. + nd_playlist_generator_get_available_playlists: + description: GetAvailablePlaylists returns the list of playlists this plugin provides. input: - $ref: '#/components/schemas/GetPlaylistsRequest' + $ref: '#/components/schemas/GetAvailablePlaylistsRequest' contentType: application/json output: - $ref: '#/components/schemas/GetPlaylistsResponse' + $ref: '#/components/schemas/GetAvailablePlaylistsResponse' contentType: application/json nd_playlist_generator_get_playlist: description: GetPlaylist returns the full data for a single playlist (tracks, metadata). @@ -18,6 +18,33 @@ exports: contentType: application/json components: schemas: + GetAvailablePlaylistsRequest: + description: GetAvailablePlaylistsRequest is the request for GetAvailablePlaylists. + properties: {} + GetAvailablePlaylistsResponse: + description: GetAvailablePlaylistsResponse is the response for GetAvailablePlaylists. + properties: + playlists: + type: array + description: Playlists is the list of playlists provided by this plugin. + items: + $ref: '#/components/schemas/PlaylistInfo' + refreshInterval: + type: integer + format: int64 + description: |- + RefreshInterval is the number of seconds until the next GetAvailablePlaylists call. + 0 means never re-discover. + retryInterval: + type: integer + format: int64 + description: |- + RetryInterval is the number of seconds before retrying a failed GetPlaylist call. + 0 means no automatic retry for transient errors. + required: + - playlists + - refreshInterval + - retryInterval GetPlaylistRequest: description: GetPlaylistRequest is the request for GetPlaylist. properties: @@ -53,26 +80,6 @@ components: - name - tracks - validUntil - GetPlaylistsRequest: - description: GetPlaylistsRequest is the request for GetPlaylists. - properties: {} - GetPlaylistsResponse: - description: GetPlaylistsResponse is the response for GetPlaylists. - properties: - playlists: - type: array - description: Playlists is the list of playlists provided by this plugin. - items: - $ref: '#/components/schemas/PlaylistInfo' - refreshInterval: - type: integer - format: int64 - description: |- - RefreshInterval is the number of seconds until the next GetPlaylists call. - 0 means never re-discover. - required: - - playlists - - refreshInterval PlaylistInfo: description: PlaylistInfo identifies a plugin playlist and its target user. properties: diff --git a/plugins/pdk/go/playlistgenerator/playlistgenerator.go b/plugins/pdk/go/playlistgenerator/playlistgenerator.go index 438cff040..15453c13f 100644 --- a/plugins/pdk/go/playlistgenerator/playlistgenerator.go +++ b/plugins/pdk/go/playlistgenerator/playlistgenerator.go @@ -11,6 +11,33 @@ import ( "github.com/navidrome/navidrome/plugins/pdk/go/pdk" ) +// PlaylistGeneratorError represents an error type for playlist generator operations. +type PlaylistGeneratorError string + +const ( + // PlaylistGeneratorErrorNotFound indicates a playlist is currently unavailable. + PlaylistGeneratorErrorNotFound PlaylistGeneratorError = "playlist_generator(not_found)" +) + +// Error implements the error interface for PlaylistGeneratorError. +func (e PlaylistGeneratorError) Error() string { return string(e) } + +// GetAvailablePlaylistsRequest is the request for GetAvailablePlaylists. +type GetAvailablePlaylistsRequest struct { +} + +// GetAvailablePlaylistsResponse is the response for GetAvailablePlaylists. +type GetAvailablePlaylistsResponse struct { + // Playlists is the list of playlists provided by this plugin. + Playlists []PlaylistInfo `json:"playlists"` + // RefreshInterval is the number of seconds until the next GetAvailablePlaylists call. + // 0 means never re-discover. + RefreshInterval int64 `json:"refreshInterval"` + // RetryInterval is the number of seconds before retrying a failed GetPlaylist call. + // 0 means no automatic retry for transient errors. + RetryInterval int64 `json:"retryInterval"` +} + // GetPlaylistRequest is the request for GetPlaylist. type GetPlaylistRequest struct { // ID is the plugin-scoped playlist ID. @@ -32,19 +59,6 @@ type GetPlaylistResponse struct { ValidUntil int64 `json:"validUntil"` } -// GetPlaylistsRequest is the request for GetPlaylists. -type GetPlaylistsRequest struct { -} - -// GetPlaylistsResponse is the response for GetPlaylists. -type GetPlaylistsResponse struct { - // Playlists is the list of playlists provided by this plugin. - Playlists []PlaylistInfo `json:"playlists"` - // RefreshInterval is the number of seconds until the next GetPlaylists call. - // 0 means never re-discover. - RefreshInterval int64 `json:"refreshInterval"` -} - // PlaylistInfo identifies a plugin playlist and its target user. type PlaylistInfo struct { // ID is the plugin-scoped unique identifier for this playlist. @@ -78,23 +92,23 @@ type SongRef struct { // PlaylistGenerator requires all methods to be implemented. // PlaylistGenerator provides dynamically-generated playlists (e.g., "Daily Mix", // personalized recommendations). Plugins implementing this capability expose two -// functions: GetPlaylists for lightweight discovery and GetPlaylist for fetching -// the heavy payload (tracks, metadata). +// functions: GetAvailablePlaylists for lightweight discovery and GetPlaylist for +// fetching the heavy payload (tracks, metadata). type PlaylistGenerator interface { - // GetPlaylists - GetPlaylists returns the list of playlists this plugin provides. - GetPlaylists(GetPlaylistsRequest) (GetPlaylistsResponse, error) + // GetAvailablePlaylists - GetAvailablePlaylists returns the list of playlists this plugin provides. + GetAvailablePlaylists(GetAvailablePlaylistsRequest) (GetAvailablePlaylistsResponse, error) // GetPlaylist - GetPlaylist returns the full data for a single playlist (tracks, metadata). GetPlaylist(GetPlaylistRequest) (GetPlaylistResponse, error) } // Internal implementation holders var ( - playlistsImpl func(GetPlaylistsRequest) (GetPlaylistsResponse, error) - playlistImpl func(GetPlaylistRequest) (GetPlaylistResponse, error) + availablePlaylistsImpl func(GetAvailablePlaylistsRequest) (GetAvailablePlaylistsResponse, error) + playlistImpl func(GetPlaylistRequest) (GetPlaylistResponse, error) ) // Register registers a playlistgenerator implementation. // All methods are required. func Register(impl PlaylistGenerator) { - playlistsImpl = impl.GetPlaylists + availablePlaylistsImpl = impl.GetAvailablePlaylists playlistImpl = impl.GetPlaylist } @@ -102,20 +116,20 @@ func Register(impl PlaylistGenerator) { // The host recognizes this and skips the plugin gracefully. const NotImplementedCode int32 = -2 -//go:wasmexport nd_playlist_generator_get_playlists -func _NdPlaylistGeneratorGetPlaylists() int32 { - if playlistsImpl == nil { +//go:wasmexport nd_playlist_generator_get_available_playlists +func _NdPlaylistGeneratorGetAvailablePlaylists() int32 { + if availablePlaylistsImpl == nil { // Return standard code - host will skip this plugin gracefully return NotImplementedCode } - var input GetPlaylistsRequest + var input GetAvailablePlaylistsRequest if err := pdk.InputJSON(&input); err != nil { pdk.SetError(err) return -1 } - output, err := playlistsImpl(input) + output, err := availablePlaylistsImpl(input) if err != nil { pdk.SetError(err) return -1 diff --git a/plugins/pdk/go/playlistgenerator/playlistgenerator_stub.go b/plugins/pdk/go/playlistgenerator/playlistgenerator_stub.go index b5f95e023..348492a44 100644 --- a/plugins/pdk/go/playlistgenerator/playlistgenerator_stub.go +++ b/plugins/pdk/go/playlistgenerator/playlistgenerator_stub.go @@ -8,6 +8,33 @@ package playlistgenerator +// PlaylistGeneratorError represents an error type for playlist generator operations. +type PlaylistGeneratorError string + +const ( + // PlaylistGeneratorErrorNotFound indicates a playlist is currently unavailable. + PlaylistGeneratorErrorNotFound PlaylistGeneratorError = "playlist_generator(not_found)" +) + +// Error implements the error interface for PlaylistGeneratorError. +func (e PlaylistGeneratorError) Error() string { return string(e) } + +// GetAvailablePlaylistsRequest is the request for GetAvailablePlaylists. +type GetAvailablePlaylistsRequest struct { +} + +// GetAvailablePlaylistsResponse is the response for GetAvailablePlaylists. +type GetAvailablePlaylistsResponse struct { + // Playlists is the list of playlists provided by this plugin. + Playlists []PlaylistInfo `json:"playlists"` + // RefreshInterval is the number of seconds until the next GetAvailablePlaylists call. + // 0 means never re-discover. + RefreshInterval int64 `json:"refreshInterval"` + // RetryInterval is the number of seconds before retrying a failed GetPlaylist call. + // 0 means no automatic retry for transient errors. + RetryInterval int64 `json:"retryInterval"` +} + // GetPlaylistRequest is the request for GetPlaylist. type GetPlaylistRequest struct { // ID is the plugin-scoped playlist ID. @@ -29,19 +56,6 @@ type GetPlaylistResponse struct { ValidUntil int64 `json:"validUntil"` } -// GetPlaylistsRequest is the request for GetPlaylists. -type GetPlaylistsRequest struct { -} - -// GetPlaylistsResponse is the response for GetPlaylists. -type GetPlaylistsResponse struct { - // Playlists is the list of playlists provided by this plugin. - Playlists []PlaylistInfo `json:"playlists"` - // RefreshInterval is the number of seconds until the next GetPlaylists call. - // 0 means never re-discover. - RefreshInterval int64 `json:"refreshInterval"` -} - // PlaylistInfo identifies a plugin playlist and its target user. type PlaylistInfo struct { // ID is the plugin-scoped unique identifier for this playlist. @@ -75,11 +89,11 @@ type SongRef struct { // PlaylistGenerator requires all methods to be implemented. // PlaylistGenerator provides dynamically-generated playlists (e.g., "Daily Mix", // personalized recommendations). Plugins implementing this capability expose two -// functions: GetPlaylists for lightweight discovery and GetPlaylist for fetching -// the heavy payload (tracks, metadata). +// functions: GetAvailablePlaylists for lightweight discovery and GetPlaylist for +// fetching the heavy payload (tracks, metadata). type PlaylistGenerator interface { - // GetPlaylists - GetPlaylists returns the list of playlists this plugin provides. - GetPlaylists(GetPlaylistsRequest) (GetPlaylistsResponse, error) + // GetAvailablePlaylists - GetAvailablePlaylists returns the list of playlists this plugin provides. + GetAvailablePlaylists(GetAvailablePlaylistsRequest) (GetAvailablePlaylistsResponse, error) // GetPlaylist - GetPlaylist returns the full data for a single playlist (tracks, metadata). GetPlaylist(GetPlaylistRequest) (GetPlaylistResponse, error) } diff --git a/plugins/pdk/rust/nd-pdk-capabilities/src/playlistgenerator.rs b/plugins/pdk/rust/nd-pdk-capabilities/src/playlistgenerator.rs index 82aa4d3cd..9e9d3e555 100644 --- a/plugins/pdk/rust/nd-pdk-capabilities/src/playlistgenerator.rs +++ b/plugins/pdk/rust/nd-pdk-capabilities/src/playlistgenerator.rs @@ -18,6 +18,31 @@ fn is_zero_u64(value: &u64) -> bool { *value == 0 } fn is_zero_f32(value: &f32) -> bool { *value == 0.0 } #[allow(dead_code)] fn is_zero_f64(value: &f64) -> bool { *value == 0.0 } +/// PlaylistGeneratorError represents an error type for playlist generator operations. +pub type PlaylistGeneratorError = &'static str; +/// PlaylistGeneratorErrorNotFound indicates a playlist is currently unavailable. +pub const PLAYLIST_GENERATOR_ERROR_NOT_FOUND: PlaylistGeneratorError = "playlist_generator(not_found)"; +/// GetAvailablePlaylistsRequest is the request for GetAvailablePlaylists. +#[derive(Debug, Clone, Default, Serialize, Deserialize)] +#[serde(rename_all = "camelCase")] +pub struct GetAvailablePlaylistsRequest { +} +/// GetAvailablePlaylistsResponse is the response for GetAvailablePlaylists. +#[derive(Debug, Clone, Default, Serialize, Deserialize)] +#[serde(rename_all = "camelCase")] +pub struct GetAvailablePlaylistsResponse { + /// Playlists is the list of playlists provided by this plugin. + #[serde(default)] + pub playlists: Vec, + /// RefreshInterval is the number of seconds until the next GetAvailablePlaylists call. + /// 0 means never re-discover. + #[serde(default)] + pub refresh_interval: i64, + /// RetryInterval is the number of seconds before retrying a failed GetPlaylist call. + /// 0 means no automatic retry for transient errors. + #[serde(default)] + pub retry_interval: i64, +} /// GetPlaylistRequest is the request for GetPlaylist. #[derive(Debug, Clone, Default, Serialize, Deserialize)] #[serde(rename_all = "camelCase")] @@ -47,23 +72,6 @@ pub struct GetPlaylistResponse { #[serde(default)] pub valid_until: i64, } -/// GetPlaylistsRequest is the request for GetPlaylists. -#[derive(Debug, Clone, Default, Serialize, Deserialize)] -#[serde(rename_all = "camelCase")] -pub struct GetPlaylistsRequest { -} -/// GetPlaylistsResponse is the response for GetPlaylists. -#[derive(Debug, Clone, Default, Serialize, Deserialize)] -#[serde(rename_all = "camelCase")] -pub struct GetPlaylistsResponse { - /// Playlists is the list of playlists provided by this plugin. - #[serde(default)] - pub playlists: Vec, - /// RefreshInterval is the number of seconds until the next GetPlaylists call. - /// 0 means never re-discover. - #[serde(default)] - pub refresh_interval: i64, -} /// PlaylistInfo identifies a plugin playlist and its target user. #[derive(Debug, Clone, Default, Serialize, Deserialize)] #[serde(rename_all = "camelCase")] @@ -131,11 +139,11 @@ impl Error { /// PlaylistGenerator requires all methods to be implemented. /// PlaylistGenerator provides dynamically-generated playlists (e.g., "Daily Mix", /// personalized recommendations). Plugins implementing this capability expose two -/// functions: GetPlaylists for lightweight discovery and GetPlaylist for fetching -/// the heavy payload (tracks, metadata). +/// functions: GetAvailablePlaylists for lightweight discovery and GetPlaylist for +/// fetching the heavy payload (tracks, metadata). pub trait PlaylistGenerator { - /// GetPlaylists - GetPlaylists returns the list of playlists this plugin provides. - fn get_playlists(&self, req: GetPlaylistsRequest) -> Result; + /// GetAvailablePlaylists - GetAvailablePlaylists returns the list of playlists this plugin provides. + fn get_available_playlists(&self, req: GetAvailablePlaylistsRequest) -> Result; /// GetPlaylist - GetPlaylist returns the full data for a single playlist (tracks, metadata). fn get_playlist(&self, req: GetPlaylistRequest) -> Result; } @@ -146,11 +154,11 @@ pub trait PlaylistGenerator { macro_rules! register_playlistgenerator { ($plugin_type:ty) => { #[extism_pdk::plugin_fn] - pub fn nd_playlist_generator_get_playlists( - req: extism_pdk::Json<$crate::playlistgenerator::GetPlaylistsRequest> - ) -> extism_pdk::FnResult> { + pub fn nd_playlist_generator_get_available_playlists( + req: extism_pdk::Json<$crate::playlistgenerator::GetAvailablePlaylistsRequest> + ) -> extism_pdk::FnResult> { let plugin = <$plugin_type>::default(); - let result = $crate::playlistgenerator::PlaylistGenerator::get_playlists(&plugin, req.into_inner())?; + let result = $crate::playlistgenerator::PlaylistGenerator::get_available_playlists(&plugin, req.into_inner())?; Ok(extism_pdk::Json(result)) } #[extism_pdk::plugin_fn] diff --git a/plugins/playlist_generator.go b/plugins/playlist_generator.go index 3ae2a4ee4..687bc092b 100644 --- a/plugins/playlist_generator.go +++ b/plugins/playlist_generator.go @@ -3,6 +3,7 @@ package plugins import ( "context" "fmt" + "strings" "sync" "time" @@ -16,14 +17,14 @@ import ( const ( CapabilityPlaylistGenerator Capability = "PlaylistGenerator" - FuncPlaylistGeneratorGetPlaylists = "nd_playlist_generator_get_playlists" - FuncPlaylistGeneratorGetPlaylist = "nd_playlist_generator_get_playlist" + FuncPlaylistGeneratorGetAvailablePlaylists = "nd_playlist_generator_get_available_playlists" + FuncPlaylistGeneratorGetPlaylist = "nd_playlist_generator_get_playlist" ) func init() { registerCapability( CapabilityPlaylistGenerator, - FuncPlaylistGeneratorGetPlaylists, + FuncPlaylistGeneratorGetAvailablePlaylists, FuncPlaylistGeneratorGetPlaylist, ) } @@ -39,6 +40,7 @@ type playlistGeneratorOrchestrator struct { wg sync.WaitGroup refreshTimers map[string]*time.Timer // keyed by playlist DB ID discoveryTimer *time.Timer + retryInterval time.Duration // from last GetAvailablePlaylists response } func newPlaylistGeneratorOrchestrator(pluginName string, p *plugin, ds model.DataStore, parentCtx context.Context) *playlistGeneratorOrchestrator { @@ -54,16 +56,21 @@ func newPlaylistGeneratorOrchestrator(pluginName string, p *plugin, ds model.Dat } } -// discoverAndSync calls GetPlaylists, then GetPlaylist for each, matches tracks, and upserts. +// discoverAndSync calls GetAvailablePlaylists, then GetPlaylist for each, matches tracks, and upserts. func (o *playlistGeneratorOrchestrator) discoverAndSync(ctx context.Context) { - resp, err := callPluginFunction[capabilities.GetPlaylistsRequest, capabilities.GetPlaylistsResponse]( - ctx, o.plugin, FuncPlaylistGeneratorGetPlaylists, capabilities.GetPlaylistsRequest{}, + resp, err := callPluginFunction[capabilities.GetAvailablePlaylistsRequest, capabilities.GetAvailablePlaylistsResponse]( + ctx, o.plugin, FuncPlaylistGeneratorGetAvailablePlaylists, capabilities.GetAvailablePlaylistsRequest{}, ) if err != nil { - log.Error(ctx, "Failed to call GetPlaylists", "plugin", o.pluginName, err) + log.Error(ctx, "Failed to call GetAvailablePlaylists", "plugin", o.pluginName, err) return } + // Store retry interval from response + if resp.RetryInterval > 0 { + o.retryInterval = time.Duration(resp.RetryInterval) * time.Second + } + for _, info := range resp.Playlists { // Resolve username to user ID user, err := o.ds.User(adminContext(ctx)).FindByUsername(info.OwnerUsername) @@ -89,7 +96,20 @@ func (o *playlistGeneratorOrchestrator) syncPlaylist(ctx context.Context, info c ctx, o.plugin, FuncPlaylistGeneratorGetPlaylist, capabilities.GetPlaylistRequest{ID: info.ID}, ) if err != nil { - log.Error(ctx, "Failed to call GetPlaylist", "plugin", o.pluginName, "playlistID", info.ID, err) + if isPlaylistNotFoundError(err) { + log.Info(ctx, "Playlist not found, skipping", "plugin", o.pluginName, "playlistID", info.ID) + // Stop any existing refresh timer for this playlist + if timer, ok := o.refreshTimers[dbID]; ok { + timer.Stop() + delete(o.refreshTimers, dbID) + } + return + } + log.Warn(ctx, "Failed to call GetPlaylist", "plugin", o.pluginName, "playlistID", info.ID, err) + // Schedule retry for transient errors if retryInterval is configured + if o.retryInterval > 0 { + o.schedulePlaylistRefresh(ctx, info, dbID, ownerID, o.retryInterval) + } return } @@ -165,6 +185,11 @@ func (o *playlistGeneratorOrchestrator) scheduleDiscovery(_ context.Context, del }) } +// isPlaylistNotFoundError checks if the error contains a NotFound sentinel from the plugin. +func isPlaylistNotFoundError(err error) bool { + return err != nil && strings.Contains(err.Error(), capabilities.PlaylistGeneratorErrorNotFound.Error()) +} + // stop cancels the context, stops all timers, and waits for in-flight goroutines. func (o *playlistGeneratorOrchestrator) stop() { o.cancel() diff --git a/plugins/playlist_generator_test.go b/plugins/playlist_generator_test.go index 5f474352b..d4b5c4f7c 100644 --- a/plugins/playlist_generator_test.go +++ b/plugins/playlist_generator_test.go @@ -3,8 +3,11 @@ package plugins import ( + "time" + "github.com/navidrome/navidrome/model" "github.com/navidrome/navidrome/model/id" + "github.com/navidrome/navidrome/plugins/capabilities" "github.com/navidrome/navidrome/tests" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" @@ -100,7 +103,7 @@ var _ = Describe("PlaylistGenerator", Ordered, func() { }) }) - Describe("GetPlaylists error handling", func() { + Describe("GetAvailablePlaylists error handling", func() { It("handles plugin errors gracefully", func() { errManager, _ := createTestManagerWithPlugins(map[string]map[string]string{ "test-playlist-generator": {"error": "service unavailable"}, @@ -115,7 +118,7 @@ var _ = Describe("PlaylistGenerator", Ordered, func() { errDS.MockedPlaylist = tests.CreateMockPlaylistRepo() } errPlsRepo := errDS.MockedPlaylist.(*tests.MockPlaylistRepo) - // The orchestrator was started but GetPlaylists returned error, + // The orchestrator was started but GetAvailablePlaylists returned error, // so no playlists should be created Consistently(func() int { return len(errPlsRepo.Data) @@ -123,6 +126,70 @@ var _ = Describe("PlaylistGenerator", Ordered, func() { }) }) + Describe("GetPlaylist NotFound error", func() { + It("skips playlists when plugin returns NotFound", func() { + notFoundManager, _ := createTestManagerWithPlugins(map[string]map[string]string{ + "test-playlist-generator": { + "get_playlist_error": "playlist temporarily unavailable", + "get_playlist_error_type": string(capabilities.PlaylistGeneratorErrorNotFound), + }, + }, "test-playlist-generator"+PackageExtension) + + // Should still have the orchestrator + Expect(notFoundManager.playlistGenerators).To(HaveKey("test-playlist-generator")) + + // No playlists should be created (all returned NotFound) + notFoundDS := notFoundManager.ds.(*tests.MockDataStore) + if notFoundDS.MockedPlaylist == nil { + notFoundDS.MockedPlaylist = tests.CreateMockPlaylistRepo() + } + notFoundPlsRepo := notFoundDS.MockedPlaylist.(*tests.MockPlaylistRepo) + Consistently(func() int { + return len(notFoundPlsRepo.Data) + }, "500ms").Should(Equal(0)) + + // No refresh timers should be scheduled for NotFound playlists + orch := notFoundManager.playlistGenerators["test-playlist-generator"] + Eventually(func() int { + return len(orch.refreshTimers) + }).Should(Equal(0)) + }) + }) + + Describe("GetPlaylist transient error with RetryInterval", func() { + It("stores retryInterval and schedules retry on transient errors", func() { + retryManager, _ := createTestManagerWithPlugins(map[string]map[string]string{ + "test-playlist-generator": { + "get_playlist_error": "temporary failure", + "retry_interval": "60", + }, + }, "test-playlist-generator"+PackageExtension) + + Expect(retryManager.playlistGenerators).To(HaveKey("test-playlist-generator")) + orch := retryManager.playlistGenerators["test-playlist-generator"] + + // retryInterval should be stored from the response + Eventually(func() time.Duration { + return orch.retryInterval + }).Should(Equal(60 * time.Second)) + + // No playlists should be created (GetPlaylist failed) + retryDS := retryManager.ds.(*tests.MockDataStore) + if retryDS.MockedPlaylist == nil { + retryDS.MockedPlaylist = tests.CreateMockPlaylistRepo() + } + retryPlsRepo := retryDS.MockedPlaylist.(*tests.MockPlaylistRepo) + Consistently(func() int { + return len(retryPlsRepo.Data) + }, "500ms").Should(Equal(0)) + + // Refresh timers should be scheduled for transient errors + Eventually(func() int { + return len(orch.refreshTimers) + }).Should(BeNumerically(">=", 1)) + }) + }) + Describe("stop", func() { It("stops the orchestrator when the manager stops", func() { stopManager, _ := createTestManagerWithPlugins(nil, diff --git a/plugins/testdata/test-playlist-generator/main.go b/plugins/testdata/test-playlist-generator/main.go index 830bc0845..f98168b5c 100644 --- a/plugins/testdata/test-playlist-generator/main.go +++ b/plugins/testdata/test-playlist-generator/main.go @@ -3,6 +3,7 @@ package main import ( "fmt" + "strconv" "github.com/navidrome/navidrome/plugins/pdk/go/pdk" pg "github.com/navidrome/navidrome/plugins/pdk/go/playlistgenerator" @@ -14,11 +15,11 @@ func init() { type testPlaylistGenerator struct{} -func (t *testPlaylistGenerator) GetPlaylists(_ pg.GetPlaylistsRequest) (pg.GetPlaylistsResponse, error) { +func (t *testPlaylistGenerator) GetAvailablePlaylists(_ pg.GetAvailablePlaylistsRequest) (pg.GetAvailablePlaylistsResponse, error) { // Check for configured error errMsg, hasErr := pdk.GetConfig("error") if hasErr && errMsg != "" { - return pg.GetPlaylistsResponse{}, fmt.Errorf("%s", errMsg) + return pg.GetAvailablePlaylistsResponse{}, fmt.Errorf("%s", errMsg) } // Get the owner username from config (defaults to "admin") @@ -27,19 +28,33 @@ func (t *testPlaylistGenerator) GetPlaylists(_ pg.GetPlaylistsRequest) (pg.GetPl ownerUsername = u } - return pg.GetPlaylistsResponse{ + resp := pg.GetAvailablePlaylistsResponse{ Playlists: []pg.PlaylistInfo{ {ID: "daily-mix-1", OwnerUsername: ownerUsername}, {ID: "daily-mix-2", OwnerUsername: ownerUsername}, }, RefreshInterval: 0, // No re-discovery in tests - }, nil + } + + // Support configurable retry interval + if ri, ok := pdk.GetConfig("retry_interval"); ok && ri != "" { + if v, err := strconv.ParseInt(ri, 10, 64); err == nil { + resp.RetryInterval = v + } + } + + return resp, nil } func (t *testPlaylistGenerator) GetPlaylist(req pg.GetPlaylistRequest) (pg.GetPlaylistResponse, error) { // Check for configured error errMsg, hasErr := pdk.GetConfig("get_playlist_error") if hasErr && errMsg != "" { + // Check if the error should be typed (e.g., NotFound) + errType, _ := pdk.GetConfig("get_playlist_error_type") + if errType == pg.PlaylistGeneratorErrorNotFound.Error() { + return pg.GetPlaylistResponse{}, fmt.Errorf("%w: %s", pg.PlaylistGeneratorErrorNotFound, errMsg) + } return pg.GetPlaylistResponse{}, fmt.Errorf("%s", errMsg) }