diff --git a/plugins/manager.go b/plugins/manager.go index 4bf44928a..0c7c91ed8 100644 --- a/plugins/manager.go +++ b/plugins/manager.go @@ -66,9 +66,6 @@ type Manager struct { ds model.DataStore broker events.Broker metrics PluginMetricsRecorder - - // Playlist generator orchestrators (keyed by plugin name) - playlistGenerators map[string]*playlistGeneratorOrchestrator } // GetManager returns a singleton instance of the plugin manager. @@ -76,11 +73,10 @@ type Manager struct { func GetManager(ds model.DataStore, broker events.Broker, m PluginMetricsRecorder) *Manager { return singleton.GetInstance(func() *Manager { return &Manager{ - ds: ds, - broker: broker, - metrics: m, - plugins: make(map[string]*plugin), - playlistGenerators: make(map[string]*playlistGeneratorOrchestrator), + ds: ds, + broker: broker, + metrics: m, + plugins: make(map[string]*plugin), } }) } @@ -169,9 +165,6 @@ func (m *Manager) Start(ctx context.Context) error { return fmt.Errorf("loading enabled plugins: %w", err) } - // Start playlist generators for plugins with the PlaylistGenerator capability - m.startPlaylistGenerators(ctx) - // Start file watcher if auto-reload is enabled if conf.Server.Plugins.AutoReload { if err := m.startWatcher(); err != nil { @@ -183,21 +176,6 @@ func (m *Manager) Start(ctx context.Context) error { return nil } -// startPlaylistGenerators starts orchestrators for all plugins with the PlaylistGenerator capability. -func (m *Manager) startPlaylistGenerators(ctx context.Context) { - m.mu.Lock() - defer m.mu.Unlock() - - for name, p := range m.plugins { - if !hasCapability(p.capabilities, CapabilityPlaylistGenerator) { - continue - } - orch := newPlaylistGeneratorOrchestrator(ctx, name, p, m.ds) - m.playlistGenerators[name] = orch - go orch.run() - } -} - // Stop shuts down the plugin manager and releases all resources. func (m *Manager) Stop() error { // Mark as stopped first to prevent new operations @@ -208,15 +186,6 @@ func (m *Manager) Stop() error { m.cancel() } - // Stop all playlist generator orchestrators - m.mu.Lock() - orchestrators := m.playlistGenerators - m.playlistGenerators = make(map[string]*playlistGeneratorOrchestrator) - m.mu.Unlock() - for _, orch := range orchestrators { - orch.stop() - } - // Stop file watcher m.stopWatcher() @@ -588,17 +557,8 @@ func (m *Manager) unloadPlugin(name string) error { return fmt.Errorf("plugin %q not found", name) } delete(m.plugins, name) - - // Extract playlist generator orchestrator under lock - orch := m.playlistGenerators[name] - delete(m.playlistGenerators, name) m.mu.Unlock() - // Stop playlist generator orchestrator outside lock - if orch != nil { - orch.stop() - } - // Run cleanup functions err := plugin.Close() if err != nil { diff --git a/plugins/manager_loader.go b/plugins/manager_loader.go index ccda9e4cb..e394911f7 100644 --- a/plugins/manager_loader.go +++ b/plugins/manager_loader.go @@ -391,6 +391,14 @@ func (m *Manager) loadPluginWithConfig(p *model.Plugin) error { // Call plugin init function callPluginInit(ctx, m.plugins[p.ID]) + // Start PlaylistGenerator orchestrator if capability is detected + loadedPlugin := m.plugins[p.ID] + if hasCapability(loadedPlugin.capabilities, CapabilityPlaylistGenerator) { + orch := newPlaylistGeneratorOrchestrator(m.ctx, p.ID, loadedPlugin, m.ds) + loadedPlugin.closers = append(loadedPlugin.closers, orch) + go orch.run() + } + return nil } diff --git a/plugins/playlist_generator.go b/plugins/playlist_generator.go index c933f961a..64766bb3a 100644 --- a/plugins/playlist_generator.go +++ b/plugins/playlist_generator.go @@ -262,8 +262,9 @@ func (o *playlistGeneratorOrchestrator) stopAllTimers() { } } -// stop cancels the context and waits for the worker goroutine to finish. -func (o *playlistGeneratorOrchestrator) stop() { +// Close cancels the context and waits for the worker goroutine to finish. +func (o *playlistGeneratorOrchestrator) Close() error { o.cancel() <-o.done + return nil } diff --git a/plugins/playlist_generator_test.go b/plugins/playlist_generator_test.go index dc4187952..2086e0436 100644 --- a/plugins/playlist_generator_test.go +++ b/plugins/playlist_generator_test.go @@ -12,6 +12,22 @@ import ( . "github.com/onsi/gomega" ) +// findOrchestrator finds the playlistGeneratorOrchestrator in a plugin's closers. +func findOrchestrator(m *Manager, pluginName string) *playlistGeneratorOrchestrator { + m.mu.RLock() + defer m.mu.RUnlock() + p, ok := m.plugins[pluginName] + if !ok { + return nil + } + for _, c := range p.closers { + if orch, ok := c.(*playlistGeneratorOrchestrator); ok { + return orch + } + } + return nil +} + var _ = Describe("PlaylistGenerator", Ordered, func() { var ( pgManager *Manager @@ -33,9 +49,9 @@ var _ = Describe("PlaylistGenerator", Ordered, func() { }) }) - Describe("startPlaylistGenerators", func() { + Describe("orchestrator lifecycle", func() { It("creates an orchestrator for the plugin", func() { - Expect(pgManager.playlistGenerators).To(HaveKey("test-playlist-generator")) + Expect(findOrchestrator(pgManager, "test-playlist-generator")).ToNot(BeNil()) }) It("discovers and syncs playlists from the plugin", func() { @@ -89,7 +105,7 @@ var _ = Describe("PlaylistGenerator", Ordered, func() { }, "test-playlist-generator"+PackageExtension) // Should still have the orchestrator (error is logged, not fatal) - Expect(errManager.playlistGenerators).To(HaveKey("test-playlist-generator")) + Expect(findOrchestrator(errManager, "test-playlist-generator")).ToNot(BeNil()) // But no playlists created errPlsRepo := errManager.ds.(*tests.MockDataStore).MockedPlaylist.(*tests.MockPlaylistRepo) @@ -111,7 +127,7 @@ var _ = Describe("PlaylistGenerator", Ordered, func() { }, "test-playlist-generator"+PackageExtension) // Should still have the orchestrator - Expect(notFoundManager.playlistGenerators).To(HaveKey("test-playlist-generator")) + Expect(findOrchestrator(notFoundManager, "test-playlist-generator")).ToNot(BeNil()) // No playlists should be created (all returned NotFound) notFoundPlsRepo := notFoundManager.ds.(*tests.MockDataStore).MockedPlaylist.(*tests.MockPlaylistRepo) @@ -120,7 +136,7 @@ var _ = Describe("PlaylistGenerator", Ordered, func() { }, "500ms").Should(Equal(0)) // No refresh timers should be scheduled for NotFound playlists - orch := notFoundManager.playlistGenerators["test-playlist-generator"] + orch := findOrchestrator(notFoundManager, "test-playlist-generator") Eventually(func() int32 { return orch.refreshTimerCount.Load() }).Should(Equal(int32(0))) @@ -136,8 +152,8 @@ var _ = Describe("PlaylistGenerator", Ordered, func() { }, }, "test-playlist-generator"+PackageExtension) - Expect(retryManager.playlistGenerators).To(HaveKey("test-playlist-generator")) - orch := retryManager.playlistGenerators["test-playlist-generator"] + orch := findOrchestrator(retryManager, "test-playlist-generator") + Expect(orch).ToNot(BeNil()) // retryInterval should be stored from the response Eventually(func() time.Duration { @@ -162,11 +178,12 @@ var _ = Describe("PlaylistGenerator", Ordered, func() { stopManager, _ := createTestManagerWithPlugins(nil, "test-playlist-generator"+PackageExtension, ) - Expect(stopManager.playlistGenerators).To(HaveKey("test-playlist-generator")) + Expect(findOrchestrator(stopManager, "test-playlist-generator")).ToNot(BeNil()) err := stopManager.Stop() Expect(err).ToNot(HaveOccurred()) - Expect(stopManager.playlistGenerators).To(BeEmpty()) + // After Stop(), the plugin is unloaded so findOrchestrator returns nil + Expect(findOrchestrator(stopManager, "test-playlist-generator")).To(BeNil()) }) }) }) diff --git a/plugins/plugins_suite_test.go b/plugins/plugins_suite_test.go index 1e53db664..41adbc133 100644 --- a/plugins/plugins_suite_test.go +++ b/plugins/plugins_suite_test.go @@ -143,11 +143,10 @@ func createTestManagerWithPluginsAndMetrics(pluginConfig map[string]map[string]s // Create and start manager manager := &Manager{ - plugins: make(map[string]*plugin), - playlistGenerators: make(map[string]*playlistGeneratorOrchestrator), - ds: dataStore, - metrics: metrics, - subsonicRouter: http.NotFoundHandler(), // Stub router for tests + plugins: make(map[string]*plugin), + ds: dataStore, + metrics: metrics, + subsonicRouter: http.NotFoundHandler(), // Stub router for tests } err = manager.Start(GinkgoT().Context()) Expect(err).ToNot(HaveOccurred())