refactor(plugins): move PlaylistGenerator orchestrator into plugin closers

Move the PlaylistGenerator orchestrator from a separate map on the Manager
into each plugin's closers list, giving it the same lifecycle as other host
services (Scheduler, TaskQueue, WebSocket). The orchestrator now implements
io.Closer and is created during plugin load, so it starts/stops automatically
with the plugin. This removes the playlistGenerators map, the
startPlaylistGenerators() method, and special-case cleanup code from both
Stop() and unloadPlugin(), simplifying reload and unload paths.
This commit is contained in:
Deluan 2026-03-05 15:37:48 -05:00
parent 9c516cf601
commit 0127b8c607
5 changed files with 45 additions and 60 deletions

View File

@ -66,9 +66,6 @@ type Manager struct {
ds model.DataStore ds model.DataStore
broker events.Broker broker events.Broker
metrics PluginMetricsRecorder metrics PluginMetricsRecorder
// Playlist generator orchestrators (keyed by plugin name)
playlistGenerators map[string]*playlistGeneratorOrchestrator
} }
// GetManager returns a singleton instance of the plugin manager. // 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 { func GetManager(ds model.DataStore, broker events.Broker, m PluginMetricsRecorder) *Manager {
return singleton.GetInstance(func() *Manager { return singleton.GetInstance(func() *Manager {
return &Manager{ return &Manager{
ds: ds, ds: ds,
broker: broker, broker: broker,
metrics: m, metrics: m,
plugins: make(map[string]*plugin), plugins: make(map[string]*plugin),
playlistGenerators: make(map[string]*playlistGeneratorOrchestrator),
} }
}) })
} }
@ -169,9 +165,6 @@ func (m *Manager) Start(ctx context.Context) error {
return fmt.Errorf("loading enabled plugins: %w", err) 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 // Start file watcher if auto-reload is enabled
if conf.Server.Plugins.AutoReload { if conf.Server.Plugins.AutoReload {
if err := m.startWatcher(); err != nil { if err := m.startWatcher(); err != nil {
@ -183,21 +176,6 @@ func (m *Manager) Start(ctx context.Context) error {
return nil 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. // Stop shuts down the plugin manager and releases all resources.
func (m *Manager) Stop() error { func (m *Manager) Stop() error {
// Mark as stopped first to prevent new operations // Mark as stopped first to prevent new operations
@ -208,15 +186,6 @@ func (m *Manager) Stop() error {
m.cancel() 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 // Stop file watcher
m.stopWatcher() m.stopWatcher()
@ -588,17 +557,8 @@ func (m *Manager) unloadPlugin(name string) error {
return fmt.Errorf("plugin %q not found", name) return fmt.Errorf("plugin %q not found", name)
} }
delete(m.plugins, name) delete(m.plugins, name)
// Extract playlist generator orchestrator under lock
orch := m.playlistGenerators[name]
delete(m.playlistGenerators, name)
m.mu.Unlock() m.mu.Unlock()
// Stop playlist generator orchestrator outside lock
if orch != nil {
orch.stop()
}
// Run cleanup functions // Run cleanup functions
err := plugin.Close() err := plugin.Close()
if err != nil { if err != nil {

View File

@ -391,6 +391,14 @@ func (m *Manager) loadPluginWithConfig(p *model.Plugin) error {
// Call plugin init function // Call plugin init function
callPluginInit(ctx, m.plugins[p.ID]) 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 return nil
} }

View File

@ -262,8 +262,9 @@ func (o *playlistGeneratorOrchestrator) stopAllTimers() {
} }
} }
// stop cancels the context and waits for the worker goroutine to finish. // Close cancels the context and waits for the worker goroutine to finish.
func (o *playlistGeneratorOrchestrator) stop() { func (o *playlistGeneratorOrchestrator) Close() error {
o.cancel() o.cancel()
<-o.done <-o.done
return nil
} }

View File

@ -12,6 +12,22 @@ import (
. "github.com/onsi/gomega" . "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 _ = Describe("PlaylistGenerator", Ordered, func() {
var ( var (
pgManager *Manager 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() { 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() { It("discovers and syncs playlists from the plugin", func() {
@ -89,7 +105,7 @@ var _ = Describe("PlaylistGenerator", Ordered, func() {
}, "test-playlist-generator"+PackageExtension) }, "test-playlist-generator"+PackageExtension)
// Should still have the orchestrator (error is logged, not fatal) // 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 // But no playlists created
errPlsRepo := errManager.ds.(*tests.MockDataStore).MockedPlaylist.(*tests.MockPlaylistRepo) errPlsRepo := errManager.ds.(*tests.MockDataStore).MockedPlaylist.(*tests.MockPlaylistRepo)
@ -111,7 +127,7 @@ var _ = Describe("PlaylistGenerator", Ordered, func() {
}, "test-playlist-generator"+PackageExtension) }, "test-playlist-generator"+PackageExtension)
// Should still have the orchestrator // 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) // No playlists should be created (all returned NotFound)
notFoundPlsRepo := notFoundManager.ds.(*tests.MockDataStore).MockedPlaylist.(*tests.MockPlaylistRepo) notFoundPlsRepo := notFoundManager.ds.(*tests.MockDataStore).MockedPlaylist.(*tests.MockPlaylistRepo)
@ -120,7 +136,7 @@ var _ = Describe("PlaylistGenerator", Ordered, func() {
}, "500ms").Should(Equal(0)) }, "500ms").Should(Equal(0))
// No refresh timers should be scheduled for NotFound playlists // No refresh timers should be scheduled for NotFound playlists
orch := notFoundManager.playlistGenerators["test-playlist-generator"] orch := findOrchestrator(notFoundManager, "test-playlist-generator")
Eventually(func() int32 { Eventually(func() int32 {
return orch.refreshTimerCount.Load() return orch.refreshTimerCount.Load()
}).Should(Equal(int32(0))) }).Should(Equal(int32(0)))
@ -136,8 +152,8 @@ var _ = Describe("PlaylistGenerator", Ordered, func() {
}, },
}, "test-playlist-generator"+PackageExtension) }, "test-playlist-generator"+PackageExtension)
Expect(retryManager.playlistGenerators).To(HaveKey("test-playlist-generator")) orch := findOrchestrator(retryManager, "test-playlist-generator")
orch := retryManager.playlistGenerators["test-playlist-generator"] Expect(orch).ToNot(BeNil())
// retryInterval should be stored from the response // retryInterval should be stored from the response
Eventually(func() time.Duration { Eventually(func() time.Duration {
@ -162,11 +178,12 @@ var _ = Describe("PlaylistGenerator", Ordered, func() {
stopManager, _ := createTestManagerWithPlugins(nil, stopManager, _ := createTestManagerWithPlugins(nil,
"test-playlist-generator"+PackageExtension, "test-playlist-generator"+PackageExtension,
) )
Expect(stopManager.playlistGenerators).To(HaveKey("test-playlist-generator")) Expect(findOrchestrator(stopManager, "test-playlist-generator")).ToNot(BeNil())
err := stopManager.Stop() err := stopManager.Stop()
Expect(err).ToNot(HaveOccurred()) 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())
}) })
}) })
}) })

View File

@ -143,11 +143,10 @@ func createTestManagerWithPluginsAndMetrics(pluginConfig map[string]map[string]s
// Create and start manager // Create and start manager
manager := &Manager{ manager := &Manager{
plugins: make(map[string]*plugin), plugins: make(map[string]*plugin),
playlistGenerators: make(map[string]*playlistGeneratorOrchestrator), ds: dataStore,
ds: dataStore, metrics: metrics,
metrics: metrics, subsonicRouter: http.NotFoundHandler(), // Stub router for tests
subsonicRouter: http.NotFoundHandler(), // Stub router for tests
} }
err = manager.Start(GinkgoT().Context()) err = manager.Start(GinkgoT().Context())
Expect(err).ToNot(HaveOccurred()) Expect(err).ToNot(HaveOccurred())