nik/navidrome
1
0
Fork 0
mirror of https://github.com/navidrome/navidrome.git synced 2026-05-03 06:51:16 +00:00
Code Issues Packages Projects Releases Wiki Activity

refactor(plugins): update plugin instance creation to accept context for cancellation support

Browse Source 
Signed-off-by: Deluan <deluan@navidrome.org>
This commit is contained in:
Deluan 2025-12-31 10:16:56 -05:00
parent 8d586f7425
commit d6b412acde
11 changed files with 57 additions and 50 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
plugins/host_artwork_test.go
View File

@ -111,7 +111,7 @@ var _ = Describe("ArtworkService", Ordered, func() {
p := manager.plugins["test-artwork"]
manager.mu.RUnlock()
instance, err := p.instance()
instance, err := p.instance(ctx)
if err != nil {
return "", err
}

2
plugins/host_cache_test.go
View File

@ -408,7 +408,7 @@ var _ = Describe("CacheService Integration", Ordered, func() {
p := manager.plugins["test-cache-plugin"]
manager.mu.RUnlock()
instance, err := p.instance()
instance, err := p.instance(ctx)
if err != nil {
return nil, err
}

2
plugins/host_kvstore_test.go
View File

@ -434,7 +434,7 @@ var _ = Describe("KVStoreService Integration", Ordered, func() {
p := manager.plugins["test-kvstore"]
manager.mu.RUnlock()
instance, err := p.instance()
instance, err := p.instance(ctx)
if err != nil {
return nil, err
}

2
plugins/host_library_test.go
View File

@ -363,7 +363,7 @@ var _ = Describe("LibraryService Integration", Ordered, func() {
p := manager.plugins["test-library"]
manager.mu.RUnlock()
instance, err := p.instance()
instance, err := p.instance(ctx)
if err != nil {
return nil, err
}

2
plugins/host_scheduler_test.go
View File

@ -273,7 +273,7 @@ var _ = Describe("SchedulerService", Ordered, func() {
Expect(testService.GetScheduleCount()).To(Equal(1))
// Create a plugin instance
instance, err := plugin.instance()
instance, err := plugin.instance(GinkgoT().Context())
Expect(err).ToNot(HaveOccurred())
defer instance.Close(GinkgoT().Context())

6
plugins/host_subsonicapi_test.go
View File

@ -130,7 +130,7 @@ var _ = Describe("SubsonicAPI Host Function", Ordered, func() {
})
It("successfully calls the ping endpoint", func() {
instance, err := plugin.instance()
instance, err := plugin.instance(GinkgoT().Context())
Expect(err).ToNot(HaveOccurred())
defer instance.Close(GinkgoT().Context())
@ -149,7 +149,7 @@ var _ = Describe("SubsonicAPI Host Function", Ordered, func() {
})
It("adds required parameters (c, f, v) to the request", func() {
instance, err := plugin.instance()
instance, err := plugin.instance(GinkgoT().Context())
Expect(err).ToNot(HaveOccurred())
defer instance.Close(GinkgoT().Context())
@ -166,7 +166,7 @@ var _ = Describe("SubsonicAPI Host Function", Ordered, func() {
})
It("returns error when username is missing", func() {
instance, err := plugin.instance()
instance, err := plugin.instance(GinkgoT().Context())
Expect(err).ToNot(HaveOccurred())
defer instance.Close(GinkgoT().Context())

13
plugins/manager_call.go
View File

@ -27,13 +27,15 @@ func callPluginFunctionNoOutput[I any](ctx context.Context, plugin *plugin, func
// callPluginFunction is a helper to call a plugin function with input and output types.
// It handles JSON marshalling/unmarshalling and error checking.
// The context is used for cancellation - if cancelled during the call, the plugin
// instance will be terminated and context.Canceled or context.DeadlineExceeded will be returned.
func callPluginFunction[I any, O any](ctx context.Context, plugin *plugin, funcName string, input I) (O, error) {
start := time.Now()
var result O
// Create plugin instance
p, err := plugin.instance()
// Create plugin instance with context for cancellation support
p, err := plugin.instance(ctx)
if err != nil {
return result, fmt.Errorf("failed to create plugin: %w", err)
}
@ -50,8 +52,13 @@ func callPluginFunction[I any, O any](ctx context.Context, plugin *plugin, funcN
}
startCall := time.Now()
exit, output, err := p.Call(funcName, inputBytes)
exit, output, err := p.CallWithContext(ctx, funcName, inputBytes)
if err != nil {
// If context was cancelled, return that error instead of the plugin error
if ctx.Err() != nil {
log.Debug(ctx, "Plugin call cancelled", "plugin", plugin.name, "function", funcName, "pluginDuration", time.Since(startCall))
return result, ctx.Err()
}
log.Trace(ctx, "Plugin call failed", "plugin", plugin.name, "function", funcName, "pluginDuration", time.Since(startCall), "navidromeDuration", startCall.Sub(start), err)
return result, fmt.Errorf("plugin call failed: %w", err)
}

6
plugins/manager_loader.go
View File

@ -269,8 +269,10 @@ func (m *Manager) loadPluginWithConfig(name, ndpPath, configJSON string) error {
// Compile the plugin with all host functions
extismConfig := extism.PluginConfig{
EnableWasi: true,
RuntimeConfig: wazero.NewRuntimeConfig().WithCompilationCache(m.cache),
EnableWasi: true,
RuntimeConfig: wazero.NewRuntimeConfig().
WithCompilationCache(m.cache).
WithCloseOnContextDone(true),
}
compiled, err := extism.NewCompiledPlugin(m.ctx, pluginManifest, extismConfig, hostFunctions)
if err != nil {

7
plugins/manager_plugin.go
View File

@ -20,8 +20,11 @@ type plugin struct {
closers []io.Closer // Cleanup functions to call on unload
}
func (p *plugin) instance() (*extism.Plugin, error) {
instance, err := p.compiled.Instance(context.Background(), extism.PluginInstanceConfig{
// instance creates a new plugin instance for the given context.
// The context is used for cancellation - if cancelled during a call,
// the module will be terminated and the instance becomes unusable.
func (p *plugin) instance(ctx context.Context) (*extism.Plugin, error) {
instance, err := p.compiled.Instance(ctx, extism.PluginInstanceConfig{
ModuleConfig: wazero.NewModuleConfig().WithSysWalltime().WithRandSource(rand.Reader),
})
if err != nil {

39
plugins/metadata_agent_test.go
View File

@ -1,8 +1,6 @@
package plugins
import (
"context"
"github.com/navidrome/navidrome/core/agents"
. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"
@ -10,10 +8,8 @@ import (
var _ = Describe("MetadataAgent", Ordered, func() {
var agent agents.Interface
var ctx context.Context
BeforeAll(func() {
ctx = GinkgoT().Context()
// Load the agent via shared manager
var ok bool
agent, ok = testManager.LoadMediaAgent("test-metadata-agent")
@ -29,7 +25,7 @@ var _ = Describe("MetadataAgent", Ordered, func() {
Describe("GetArtistMBID", func() {
It("returns the MBID from the plugin", func() {
retriever := agent.(agents.ArtistMBIDRetriever)
mbid, err := retriever.GetArtistMBID(ctx, "artist-1", "The Beatles")
mbid, err := retriever.GetArtistMBID(GinkgoT().Context(), "artist-1", "The Beatles")
Expect(err).ToNot(HaveOccurred())
Expect(mbid).To(Equal("test-mbid-The Beatles"))
})
@ -38,7 +34,7 @@ var _ = Describe("MetadataAgent", Ordered, func() {
Describe("GetArtistURL", func() {
It("returns the URL from the plugin", func() {
retriever := agent.(agents.ArtistURLRetriever)
url, err := retriever.GetArtistURL(ctx, "artist-1", "The Beatles", "some-mbid")
url, err := retriever.GetArtistURL(GinkgoT().Context(), "artist-1", "The Beatles", "some-mbid")
Expect(err).ToNot(HaveOccurred())
Expect(url).To(Equal("https://test.example.com/artist/The Beatles"))
})
@ -47,7 +43,7 @@ var _ = Describe("MetadataAgent", Ordered, func() {
Describe("GetArtistBiography", func() {
It("returns the biography from the plugin", func() {
retriever := agent.(agents.ArtistBiographyRetriever)
bio, err := retriever.GetArtistBiography(ctx, "artist-1", "The Beatles", "some-mbid")
bio, err := retriever.GetArtistBiography(GinkgoT().Context(), "artist-1", "The Beatles", "some-mbid")
Expect(err).ToNot(HaveOccurred())
Expect(bio).To(Equal("Biography for The Beatles"))
})
@ -56,7 +52,7 @@ var _ = Describe("MetadataAgent", Ordered, func() {
Describe("GetArtistImages", func() {
It("returns images from the plugin", func() {
retriever := agent.(agents.ArtistImageRetriever)
images, err := retriever.GetArtistImages(ctx, "artist-1", "The Beatles", "some-mbid")
images, err := retriever.GetArtistImages(GinkgoT().Context(), "artist-1", "The Beatles", "some-mbid")
Expect(err).ToNot(HaveOccurred())
Expect(images).To(HaveLen(2))
Expect(images[0].URL).To(Equal("https://test.example.com/images/The Beatles/large.jpg"))
@ -69,7 +65,7 @@ var _ = Describe("MetadataAgent", Ordered, func() {
Describe("GetSimilarArtists", func() {
It("returns similar artists from the plugin", func() {
retriever := agent.(agents.ArtistSimilarRetriever)
artists, err := retriever.GetSimilarArtists(ctx, "artist-1", "The Beatles", "some-mbid", 3)
artists, err := retriever.GetSimilarArtists(GinkgoT().Context(), "artist-1", "The Beatles", "some-mbid", 3)
Expect(err).ToNot(HaveOccurred())
Expect(artists).To(HaveLen(3))
Expect(artists[0].Name).To(Equal("The Beatles Similar A"))
@ -81,7 +77,7 @@ var _ = Describe("MetadataAgent", Ordered, func() {
Describe("GetArtistTopSongs", func() {
It("returns top songs from the plugin", func() {
retriever := agent.(agents.ArtistTopSongsRetriever)
songs, err := retriever.GetArtistTopSongs(ctx, "artist-1", "The Beatles", "some-mbid", 3)
songs, err := retriever.GetArtistTopSongs(GinkgoT().Context(), "artist-1", "The Beatles", "some-mbid", 3)
Expect(err).ToNot(HaveOccurred())
Expect(songs).To(HaveLen(3))
Expect(songs[0].Name).To(Equal("The Beatles Song 1"))
@ -93,7 +89,7 @@ var _ = Describe("MetadataAgent", Ordered, func() {
Describe("GetAlbumInfo", func() {
It("returns album info from the plugin", func() {
retriever := agent.(agents.AlbumInfoRetriever)
info, err := retriever.GetAlbumInfo(ctx, "Abbey Road", "The Beatles", "album-mbid")
info, err := retriever.GetAlbumInfo(GinkgoT().Context(), "Abbey Road", "The Beatles", "album-mbid")
Expect(err).ToNot(HaveOccurred())
Expect(info.Name).To(Equal("Abbey Road"))
Expect(info.MBID).To(Equal("test-album-mbid-Abbey Road"))
@ -105,7 +101,7 @@ var _ = Describe("MetadataAgent", Ordered, func() {
Describe("GetAlbumImages", func() {
It("returns album images from the plugin", func() {
retriever := agent.(agents.AlbumImageRetriever)
images, err := retriever.GetAlbumImages(ctx, "Abbey Road", "The Beatles", "album-mbid")
images, err := retriever.GetAlbumImages(GinkgoT().Context(), "Abbey Road", "The Beatles", "album-mbid")
Expect(err).ToNot(HaveOccurred())
Expect(images).To(HaveLen(1))
Expect(images[0].URL).To(Equal("https://test.example.com/albums/Abbey Road/cover.jpg"))
@ -119,12 +115,9 @@ var _ = Describe("MetadataAgent error handling", Ordered, func() {
var (
errorManager *Manager
errorAgent agents.Interface
ctx context.Context
)
BeforeAll(func() {
ctx = GinkgoT().Context()
// Create manager with error injection config
errorManager, _ = createTestManager(map[string]map[string]string{
"test-metadata-agent": {
@ -140,56 +133,56 @@ var _ = Describe("MetadataAgent error handling", Ordered, func() {
It("returns error from GetArtistMBID", func() {
retriever := errorAgent.(agents.ArtistMBIDRetriever)
_, err := retriever.GetArtistMBID(ctx, "artist-1", "Test")
_, err := retriever.GetArtistMBID(GinkgoT().Context(), "artist-1", "Test")
Expect(err).To(HaveOccurred())
Expect(err.Error()).To(ContainSubstring("simulated plugin error"))
})
It("returns error from GetArtistURL", func() {
retriever := errorAgent.(agents.ArtistURLRetriever)
_, err := retriever.GetArtistURL(ctx, "artist-1", "Test", "mbid")
_, err := retriever.GetArtistURL(GinkgoT().Context(), "artist-1", "Test", "mbid")
Expect(err).To(HaveOccurred())
Expect(err.Error()).To(ContainSubstring("simulated plugin error"))
})
It("returns error from GetArtistBiography", func() {
retriever := errorAgent.(agents.ArtistBiographyRetriever)
_, err := retriever.GetArtistBiography(ctx, "artist-1", "Test", "mbid")
_, err := retriever.GetArtistBiography(GinkgoT().Context(), "artist-1", "Test", "mbid")
Expect(err).To(HaveOccurred())
Expect(err.Error()).To(ContainSubstring("simulated plugin error"))
})
It("returns error from GetArtistImages", func() {
retriever := errorAgent.(agents.ArtistImageRetriever)
_, err := retriever.GetArtistImages(ctx, "artist-1", "Test", "mbid")
_, err := retriever.GetArtistImages(GinkgoT().Context(), "artist-1", "Test", "mbid")
Expect(err).To(HaveOccurred())
Expect(err.Error()).To(ContainSubstring("simulated plugin error"))
})
It("returns error from GetSimilarArtists", func() {
retriever := errorAgent.(agents.ArtistSimilarRetriever)
_, err := retriever.GetSimilarArtists(ctx, "artist-1", "Test", "mbid", 5)
_, err := retriever.GetSimilarArtists(GinkgoT().Context(), "artist-1", "Test", "mbid", 5)
Expect(err).To(HaveOccurred())
Expect(err.Error()).To(ContainSubstring("simulated plugin error"))
})
It("returns error from GetArtistTopSongs", func() {
retriever := errorAgent.(agents.ArtistTopSongsRetriever)
_, err := retriever.GetArtistTopSongs(ctx, "artist-1", "Test", "mbid", 5)
_, err := retriever.GetArtistTopSongs(GinkgoT().Context(), "artist-1", "Test", "mbid", 5)
Expect(err).To(HaveOccurred())
Expect(err.Error()).To(ContainSubstring("simulated plugin error"))
})
It("returns error from GetAlbumInfo", func() {
retriever := errorAgent.(agents.AlbumInfoRetriever)
_, err := retriever.GetAlbumInfo(ctx, "Album", "Artist", "mbid")
_, err := retriever.GetAlbumInfo(GinkgoT().Context(), "Album", "Artist", "mbid")
Expect(err).To(HaveOccurred())
Expect(err.Error()).To(ContainSubstring("simulated plugin error"))
})
It("returns error from GetAlbumImages", func() {
retriever := errorAgent.(agents.AlbumImageRetriever)
_, err := retriever.GetAlbumImages(ctx, "Album", "Artist", "mbid")
_, err := retriever.GetAlbumImages(GinkgoT().Context(), "Album", "Artist", "mbid")
Expect(err).To(HaveOccurred())
Expect(err.Error()).To(ContainSubstring("simulated plugin error"))
})

26
plugins/scrobbler_adapter_test.go
View File

@ -14,18 +14,20 @@ import (
. "github.com/onsi/gomega"
)
// ctxWithUser returns a fresh context with the test user.
// Must be called within each test, not in BeforeAll, because the context
// from BeforeAll gets cancelled before tests run.
func ctxWithUser() context.Context {
return request.WithUser(GinkgoT().Context(), model.User{ID: "user-1", UserName: "testuser"})
}
var _ = Describe("ScrobblerPlugin", Ordered, func() {
var (
scrobblerManager *Manager
s scrobbler.Scrobbler
ctx context.Context
)
BeforeAll(func() {
ctx = GinkgoT().Context()
// Add user to context for username extraction
ctx = request.WithUser(ctx, model.User{ID: "user-1", UserName: "testuser"})
// Load the scrobbler via a new manager with the test-scrobbler plugin
scrobblerManager, _ = createTestManagerWithPlugins(nil, "test-scrobbler"+PackageExtension)
@ -52,7 +54,7 @@ var _ = Describe("ScrobblerPlugin", Ordered, func() {
Describe("IsAuthorized", func() {
It("returns true when plugin is configured to authorize", func() {
result := s.IsAuthorized(ctx, "user-1")
result := s.IsAuthorized(ctxWithUser(), "user-1")
Expect(result).To(BeTrue())
})
@ -64,7 +66,7 @@ var _ = Describe("ScrobblerPlugin", Ordered, func() {
sc, ok := manager.LoadScrobbler("test-scrobbler")
Expect(ok).To(BeTrue())
result := sc.IsAuthorized(ctx, "user-1")
result := sc.IsAuthorized(ctxWithUser(), "user-1")
Expect(result).To(BeFalse())
})
})
@ -82,7 +84,7 @@ var _ = Describe("ScrobblerPlugin", Ordered, func() {
DiscNumber: 1,
}
err := s.NowPlaying(ctx, "user-1", track, 30)
err := s.NowPlaying(ctxWithUser(), "user-1", track, 30)
Expect(err).ToNot(HaveOccurred())
})
@ -95,7 +97,7 @@ var _ = Describe("ScrobblerPlugin", Ordered, func() {
Expect(ok).To(BeTrue())
track := &model.MediaFile{ID: "track-1", Title: "Test Song"}
err := sc.NowPlaying(ctx, "user-1", track, 30)
err := sc.NowPlaying(ctxWithUser(), "user-1", track, 30)
Expect(err).To(HaveOccurred())
Expect(err).To(MatchError(scrobbler.ErrRetryLater))
})
@ -117,7 +119,7 @@ var _ = Describe("ScrobblerPlugin", Ordered, func() {
TimeStamp: time.Now(),
}
err := s.Scrobble(ctx, "user-1", sc)
err := s.Scrobble(ctxWithUser(), "user-1", sc)
Expect(err).ToNot(HaveOccurred())
})
@ -133,7 +135,7 @@ var _ = Describe("ScrobblerPlugin", Ordered, func() {
MediaFile: model.MediaFile{ID: "track-1", Title: "Test Song"},
TimeStamp: time.Now(),
}
err := sc.Scrobble(ctx, "user-1", scrobble)
err := sc.Scrobble(ctxWithUser(), "user-1", scrobble)
Expect(err).To(HaveOccurred())
Expect(err).To(MatchError(scrobbler.ErrNotAuthorized))
})
@ -150,7 +152,7 @@ var _ = Describe("ScrobblerPlugin", Ordered, func() {
MediaFile: model.MediaFile{ID: "track-1", Title: "Test Song"},
TimeStamp: time.Now(),
}
err := sc.Scrobble(ctx, "user-1", scrobble)
err := sc.Scrobble(ctxWithUser(), "user-1", scrobble)
Expect(err).To(HaveOccurred())
Expect(err).To(MatchError(scrobbler.ErrUnrecoverable))
})