diff --git a/cmd/wire_gen.go b/cmd/wire_gen.go index 78846f631..f4760fa81 100644 --- a/cmd/wire_gen.go +++ b/cmd/wire_gen.go @@ -71,7 +71,7 @@ func CreateNativeAPIRouter(ctx context.Context) *nativeapi.Router { watcher := scanner.GetWatcher(dataStore, modelScanner) library := core.NewLibrary(dataStore, modelScanner, watcher, broker) maintenance := core.NewMaintenance(dataStore) - router := nativeapi.New(dataStore, share, playlists, insights, library, maintenance) + router := nativeapi.New(dataStore, share, playlists, insights, library, maintenance, manager) return router } @@ -194,7 +194,7 @@ func getPluginManager() *plugins.Manager { // wire_injectors.go: -var allProviders = wire.NewSet(core.Set, artwork.Set, server.New, subsonic.New, nativeapi.New, public.New, persistence.New, lastfm.NewRouter, listenbrainz.NewRouter, events.GetBroker, scanner.New, scanner.GetWatcher, metrics.GetPrometheusInstance, db.Db, plugins.GetManager, wire.Bind(new(agents.PluginLoader), new(*plugins.Manager)), wire.Bind(new(scrobbler.PluginLoader), new(*plugins.Manager)), wire.Bind(new(core.Watcher), new(scanner.Watcher))) +var allProviders = wire.NewSet(core.Set, artwork.Set, server.New, subsonic.New, nativeapi.New, public.New, persistence.New, lastfm.NewRouter, listenbrainz.NewRouter, events.GetBroker, scanner.New, scanner.GetWatcher, metrics.GetPrometheusInstance, db.Db, plugins.GetManager, wire.Bind(new(agents.PluginLoader), new(*plugins.Manager)), wire.Bind(new(scrobbler.PluginLoader), new(*plugins.Manager)), wire.Bind(new(nativeapi.PluginManager), new(*plugins.Manager)), wire.Bind(new(core.Watcher), new(scanner.Watcher))) func GetPluginManager(ctx context.Context) *plugins.Manager { manager := getPluginManager() diff --git a/cmd/wire_injectors.go b/cmd/wire_injectors.go index 7a90d8bf4..15973c497 100644 --- a/cmd/wire_injectors.go +++ b/cmd/wire_injectors.go @@ -44,6 +44,7 @@ var allProviders = wire.NewSet( plugins.GetManager, wire.Bind(new(agents.PluginLoader), new(*plugins.Manager)), wire.Bind(new(scrobbler.PluginLoader), new(*plugins.Manager)), + wire.Bind(new(nativeapi.PluginManager), new(*plugins.Manager)), wire.Bind(new(core.Watcher), new(scanner.Watcher)), ) diff --git a/plugins/manager.go b/plugins/manager.go index 8ef8875cb..8912ec69a 100644 --- a/plugins/manager.go +++ b/plugins/manager.go @@ -117,13 +117,6 @@ func GetManager() *Manager { }) } -// IsConfigured returns true if the manager has been configured with a DataStore. -// This is useful for API handlers to know if they should use the manager or fall back -// to direct DB operations (e.g., in test environments). -func (m *Manager) IsConfigured() bool { - return m.ds != nil -} - // adminContext returns a context with admin privileges for DB operations. func adminContext(ctx context.Context) context.Context { return request.WithUser(ctx, model.User{IsAdmin: true}) diff --git a/server/nativeapi/config_test.go b/server/nativeapi/config_test.go index d9c722955..7edda34d2 100644 --- a/server/nativeapi/config_test.go +++ b/server/nativeapi/config_test.go @@ -29,7 +29,7 @@ var _ = Describe("Config API", func() { conf.Server.DevUIShowConfig = true // Enable config endpoint for tests ds = &tests.MockDataStore{} auth.Init(ds) - nativeRouter := New(ds, nil, nil, nil, core.NewMockLibraryService(), nil) + nativeRouter := New(ds, nil, nil, nil, core.NewMockLibraryService(), nil, nil) router = server.JWTVerifier(nativeRouter) // Create test users diff --git a/server/nativeapi/library_test.go b/server/nativeapi/library_test.go index 950338492..df5e7b16e 100644 --- a/server/nativeapi/library_test.go +++ b/server/nativeapi/library_test.go @@ -30,7 +30,7 @@ var _ = Describe("Library API", func() { DeferCleanup(configtest.SetupConfig()) ds = &tests.MockDataStore{} auth.Init(ds) - nativeRouter := New(ds, nil, nil, nil, core.NewMockLibraryService(), nil) + nativeRouter := New(ds, nil, nil, nil, core.NewMockLibraryService(), nil, nil) router = server.JWTVerifier(nativeRouter) // Create test users diff --git a/server/nativeapi/native_api.go b/server/nativeapi/native_api.go index 1cd340694..9b4051d12 100644 --- a/server/nativeapi/native_api.go +++ b/server/nativeapi/native_api.go @@ -20,18 +20,27 @@ import ( "github.com/navidrome/navidrome/server" ) -type Router struct { - http.Handler - ds model.DataStore - share core.Share - playlists core.Playlists - insights metrics.Insights - libs core.Library - maintenance core.Maintenance +// PluginManager defines the interface for plugin management operations. +// This interface is used by the API handlers to enable/disable plugins and update configuration. +type PluginManager interface { + EnablePlugin(ctx context.Context, id string) error + DisablePlugin(ctx context.Context, id string) error + UpdatePluginConfig(ctx context.Context, id, configJSON string) error } -func New(ds model.DataStore, share core.Share, playlists core.Playlists, insights metrics.Insights, libraryService core.Library, maintenance core.Maintenance) *Router { - r := &Router{ds: ds, share: share, playlists: playlists, insights: insights, libs: libraryService, maintenance: maintenance} +type Router struct { + http.Handler + ds model.DataStore + share core.Share + playlists core.Playlists + insights metrics.Insights + libs core.Library + maintenance core.Maintenance + pluginManager PluginManager +} + +func New(ds model.DataStore, share core.Share, playlists core.Playlists, insights metrics.Insights, libraryService core.Library, maintenance core.Maintenance, pluginManager PluginManager) *Router { + r := &Router{ds: ds, share: share, playlists: playlists, insights: insights, libs: libraryService, maintenance: maintenance, pluginManager: pluginManager} r.Handler = r.routes() return r } diff --git a/server/nativeapi/native_api_song_test.go b/server/nativeapi/native_api_song_test.go index b52042643..e90379f68 100644 --- a/server/nativeapi/native_api_song_test.go +++ b/server/nativeapi/native_api_song_test.go @@ -95,7 +95,7 @@ var _ = Describe("Song Endpoints", func() { mfRepo.SetData(testSongs) // Create the native API router and wrap it with the JWTVerifier middleware - nativeRouter := New(ds, nil, nil, nil, core.NewMockLibraryService(), nil) + nativeRouter := New(ds, nil, nil, nil, core.NewMockLibraryService(), nil, nil) router = server.JWTVerifier(nativeRouter) w = httptest.NewRecorder() }) diff --git a/server/nativeapi/plugin.go b/server/nativeapi/plugin.go index 243e692b2..94615fdf7 100644 --- a/server/nativeapi/plugin.go +++ b/server/nativeapi/plugin.go @@ -11,7 +11,6 @@ import ( "github.com/navidrome/navidrome/conf" "github.com/navidrome/navidrome/log" "github.com/navidrome/navidrome/model" - "github.com/navidrome/navidrome/plugins" "github.com/navidrome/navidrome/server" ) @@ -52,7 +51,6 @@ func (api *Router) updatePlugin(w http.ResponseWriter, r *http.Request) { id := chi.URLParam(r, "id") ctx := r.Context() repo := api.ds.Plugin(ctx) - manager := plugins.GetManager() // Get existing plugin to verify it exists plugin, err := repo.Get(id) @@ -78,64 +76,39 @@ func (api *Router) updatePlugin(w http.ResponseWriter, r *http.Request) { return } - // If manager is configured, use it to properly load/unload plugins - // Otherwise, fall back to direct DB operations (e.g., in tests) - if manager.IsConfigured() { - // Handle config update first (if provided) - if req.Config != nil { - // Validate JSON if not empty - if *req.Config != "" && !isValidJSON(*req.Config) { - http.Error(w, "Invalid JSON in config field", http.StatusBadRequest) - return - } - if err := manager.UpdatePluginConfig(ctx, id, *req.Config); err != nil { - log.Error(ctx, "Error updating plugin config", "id", id, err) - http.Error(w, "Error updating plugin configuration: "+err.Error(), http.StatusInternalServerError) - return - } - } - - // Handle enable/disable - if req.Enabled != nil { - if *req.Enabled { - if err := manager.EnablePlugin(ctx, id); err != nil { - log.Error(ctx, "Error enabling plugin", "id", id, err) - // Refresh plugin from DB to get the error - plugin, _ = repo.Get(id) - w.Header().Set("Content-Type", "application/json") - w.WriteHeader(http.StatusUnprocessableEntity) - _ = json.NewEncoder(w).Encode(plugin) - return - } - } else { - if err := manager.DisablePlugin(ctx, id); err != nil { - log.Error(ctx, "Error disabling plugin", "id", id, err) - http.Error(w, "Error disabling plugin: "+err.Error(), http.StatusInternalServerError) - return - } - } - } - } else { - // Fallback: direct DB operations (for tests or when manager is not started) - if req.Config != nil { - if *req.Config != "" && !isValidJSON(*req.Config) { - http.Error(w, "Invalid JSON in config field", http.StatusBadRequest) - return - } - plugin.Config = *req.Config - } - if req.Enabled != nil { - plugin.Enabled = *req.Enabled - } - if err := repo.Put(plugin); err != nil { - if errors.Is(err, rest.ErrPermissionDenied) { - http.Error(w, "Access denied: admin privileges required", http.StatusForbidden) - return - } - log.Error(ctx, "Error updating plugin", "id", id, err) - http.Error(w, "Internal server error", http.StatusInternalServerError) + // Handle config update first (if provided) + if req.Config != nil { + // Validate JSON if not empty + if *req.Config != "" && !isValidJSON(*req.Config) { + http.Error(w, "Invalid JSON in config field", http.StatusBadRequest) return } + if err := api.pluginManager.UpdatePluginConfig(ctx, id, *req.Config); err != nil { + log.Error(ctx, "Error updating plugin config", "id", id, err) + http.Error(w, "Error updating plugin configuration: "+err.Error(), http.StatusInternalServerError) + return + } + } + + // Handle enable/disable + if req.Enabled != nil { + if *req.Enabled { + if err := api.pluginManager.EnablePlugin(ctx, id); err != nil { + log.Error(ctx, "Error enabling plugin", "id", id, err) + // Refresh plugin from DB to get the error + plugin, _ = repo.Get(id) + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(http.StatusUnprocessableEntity) + _ = json.NewEncoder(w).Encode(plugin) + return + } + } else { + if err := api.pluginManager.DisablePlugin(ctx, id); err != nil { + log.Error(ctx, "Error disabling plugin", "id", id, err) + http.Error(w, "Error disabling plugin: "+err.Error(), http.StatusInternalServerError) + return + } + } } // Refresh and return updated plugin diff --git a/server/nativeapi/plugin_test.go b/server/nativeapi/plugin_test.go index 69aaebc39..5efc4c454 100644 --- a/server/nativeapi/plugin_test.go +++ b/server/nativeapi/plugin_test.go @@ -2,6 +2,7 @@ package nativeapi import ( "bytes" + "context" "encoding/json" "net/http" "net/http/httptest" @@ -21,6 +22,7 @@ import ( var _ = Describe("Plugin API", func() { var ds *tests.MockDataStore + var mockManager *tests.MockPluginManager var router http.Handler var adminUser, regularUser model.User var testPlugin1, testPlugin2 model.Plugin @@ -29,8 +31,9 @@ var _ = Describe("Plugin API", func() { DeferCleanup(configtest.SetupConfig()) conf.Server.Plugins.Enabled = true ds = &tests.MockDataStore{} + mockManager = &tests.MockPluginManager{} auth.Init(ds) - nativeRouter := New(ds, nil, nil, nil, core.NewMockLibraryService(), nil) + nativeRouter := New(ds, nil, nil, nil, core.NewMockLibraryService(), nil, mockManager) router = server.JWTVerifier(nativeRouter) // Create test users @@ -153,6 +156,14 @@ var _ = Describe("Plugin API", func() { Describe("PUT /api/plugin/{id}", func() { It("updates plugin enabled state", func() { + // Configure mock to update the repo when EnablePlugin is called + mockManager.EnablePluginFn = func(ctx context.Context, id string) error { + adminCtx := request.WithUser(ctx, adminUser) + p, _ := ds.Plugin(adminCtx).Get(id) + p.Enabled = true + return ds.Plugin(adminCtx).Put(p) + } + body := bytes.NewBufferString(`{"enabled":true}`) req := httptest.NewRequest("PUT", "/plugin/test-plugin-1", body) req.Header.Set(consts.UIAuthorizationHeader, "Bearer "+adminToken) @@ -167,9 +178,18 @@ var _ = Describe("Plugin API", func() { err := json.Unmarshal(w.Body.Bytes(), &plugin) Expect(err).ToNot(HaveOccurred()) Expect(plugin.Enabled).To(BeTrue()) + Expect(mockManager.EnablePluginCalls).To(ContainElement("test-plugin-1")) }) It("updates plugin config with valid JSON", func() { + // Configure mock to update the repo when UpdatePluginConfig is called + mockManager.UpdatePluginConfigFn = func(ctx context.Context, id, configJSON string) error { + adminCtx := request.WithUser(ctx, adminUser) + p, _ := ds.Plugin(adminCtx).Get(id) + p.Config = configJSON + return ds.Plugin(adminCtx).Put(p) + } + body := bytes.NewBufferString(`{"config":"{\"key\":\"value\"}"}`) req := httptest.NewRequest("PUT", "/plugin/test-plugin-1", body) req.Header.Set(consts.UIAuthorizationHeader, "Bearer "+adminToken) @@ -184,6 +204,8 @@ var _ = Describe("Plugin API", func() { err := json.Unmarshal(w.Body.Bytes(), &plugin) Expect(err).ToNot(HaveOccurred()) Expect(plugin.Config).To(Equal(`{"key":"value"}`)) + Expect(mockManager.UpdatePluginConfigCalls).To(HaveLen(1)) + Expect(mockManager.UpdatePluginConfigCalls[0].ConfigJSON).To(Equal(`{"key":"value"}`)) }) It("rejects invalid JSON in config field", func() { @@ -200,6 +222,14 @@ var _ = Describe("Plugin API", func() { }) It("allows empty config", func() { + // Configure mock to update the repo when UpdatePluginConfig is called + mockManager.UpdatePluginConfigFn = func(ctx context.Context, id, configJSON string) error { + adminCtx := request.WithUser(ctx, adminUser) + p, _ := ds.Plugin(adminCtx).Get(id) + p.Config = configJSON + return ds.Plugin(adminCtx).Put(p) + } + body := bytes.NewBufferString(`{"config":""}`) req := httptest.NewRequest("PUT", "/plugin/test-plugin-1", body) req.Header.Set(consts.UIAuthorizationHeader, "Bearer "+adminToken) diff --git a/tests/mock_plugin_manager.go b/tests/mock_plugin_manager.go new file mode 100644 index 000000000..67ac8065d --- /dev/null +++ b/tests/mock_plugin_manager.go @@ -0,0 +1,56 @@ +package tests + +import ( + "context" +) + +// MockPluginManager is a mock implementation of plugins.PluginManager for testing. +// It implements EnablePlugin, DisablePlugin, and UpdatePluginConfig methods. +type MockPluginManager struct { + // EnablePluginFn is called when EnablePlugin is invoked. If nil, returns EnableError. + EnablePluginFn func(ctx context.Context, id string) error + // DisablePluginFn is called when DisablePlugin is invoked. If nil, returns DisableError. + DisablePluginFn func(ctx context.Context, id string) error + // UpdatePluginConfigFn is called when UpdatePluginConfig is invoked. If nil, returns ConfigError. + UpdatePluginConfigFn func(ctx context.Context, id, configJSON string) error + + // Default errors to return when Fn callbacks are not set + EnableError error + DisableError error + ConfigError error + + // Track calls for assertions + EnablePluginCalls []string + DisablePluginCalls []string + UpdatePluginConfigCalls []struct { + ID string + ConfigJSON string + } +} + +func (m *MockPluginManager) EnablePlugin(ctx context.Context, id string) error { + m.EnablePluginCalls = append(m.EnablePluginCalls, id) + if m.EnablePluginFn != nil { + return m.EnablePluginFn(ctx, id) + } + return m.EnableError +} + +func (m *MockPluginManager) DisablePlugin(ctx context.Context, id string) error { + m.DisablePluginCalls = append(m.DisablePluginCalls, id) + if m.DisablePluginFn != nil { + return m.DisablePluginFn(ctx, id) + } + return m.DisableError +} + +func (m *MockPluginManager) UpdatePluginConfig(ctx context.Context, id, configJSON string) error { + m.UpdatePluginConfigCalls = append(m.UpdatePluginConfigCalls, struct { + ID string + ConfigJSON string + }{ID: id, ConfigJSON: configJSON}) + if m.UpdatePluginConfigFn != nil { + return m.UpdatePluginConfigFn(ctx, id, configJSON) + } + return m.ConfigError +}