From f7cce28cdf2dcb17118e850fe2cd5038368a4b13 Mon Sep 17 00:00:00 2001
From: Will Browne <wbrowne@users.noreply.github.com>
Date: Tue, 7 Jun 2022 17:51:00 +0200
Subject: [PATCH] Plugins: Separate manager read/write components (#50313)

* separate manager read/write

* guarantee consistency in test
---
 pkg/api/http_server.go                         | 4 +++-
 pkg/api/plugins.go                             | 4 ++--
 pkg/plugins/ifaces.go                          | 3 +++
 pkg/plugins/manager/registry/in_memory_test.go | 6 ++++++
 pkg/server/wire.go                             | 1 +
 5 files changed, 15 insertions(+), 3 deletions(-)

diff --git a/pkg/api/http_server.go b/pkg/api/http_server.go
index e663e2e697c..a6475c5aa2b 100644
--- a/pkg/api/http_server.go
+++ b/pkg/api/http_server.go
@@ -107,6 +107,7 @@ type HTTPServer struct {
 	PluginRequestValidator       models.PluginRequestValidator
 	pluginClient                 plugins.Client
 	pluginStore                  plugins.Store
+	pluginManager                plugins.Manager
 	pluginDashboardService       plugindashboards.Service
 	pluginStaticRouteResolver    plugins.StaticRouteResolver
 	pluginErrorResolver          plugins.ErrorResolver
@@ -170,7 +171,7 @@ func ProvideHTTPServer(opts ServerOptions, cfg *setting.Cfg, routeRegister routi
 	cacheService *localcache.CacheService, sqlStore *sqlstore.SQLStore, alertEngine *alerting.AlertEngine,
 	pluginRequestValidator models.PluginRequestValidator, pluginStaticRouteResolver plugins.StaticRouteResolver,
 	pluginDashboardService plugindashboards.Service, pluginStore plugins.Store, pluginClient plugins.Client,
-	pluginErrorResolver plugins.ErrorResolver, settingsProvider setting.Provider,
+	pluginErrorResolver plugins.ErrorResolver, pluginManager plugins.Manager, settingsProvider setting.Provider,
 	dataSourceCache datasources.CacheService, userTokenService models.UserTokenService,
 	cleanUpService *cleanup.CleanUpService, shortURLService shorturls.Service, queryHistoryService queryhistory.Service,
 	thumbService thumbs.Service, remoteCache *remotecache.RemoteCache, provisioningService provisioning.ProvisioningService,
@@ -207,6 +208,7 @@ func ProvideHTTPServer(opts ServerOptions, cfg *setting.Cfg, routeRegister routi
 		SQLStore:                     sqlStore,
 		AlertEngine:                  alertEngine,
 		PluginRequestValidator:       pluginRequestValidator,
+		pluginManager:                pluginManager,
 		pluginClient:                 pluginClient,
 		pluginStore:                  pluginStore,
 		pluginStaticRouteResolver:    pluginStaticRouteResolver,
diff --git a/pkg/api/plugins.go b/pkg/api/plugins.go
index b502ccd91b6..bed48fa2ebb 100644
--- a/pkg/api/plugins.go
+++ b/pkg/api/plugins.go
@@ -358,7 +358,7 @@ func (hs *HTTPServer) InstallPlugin(c *models.ReqContext) response.Response {
 	}
 	pluginID := web.Params(c.Req)[":pluginId"]
 
-	err := hs.pluginStore.Add(c.Req.Context(), pluginID, dto.Version)
+	err := hs.pluginManager.Add(c.Req.Context(), pluginID, dto.Version)
 	if err != nil {
 		var dupeErr plugins.DuplicateError
 		if errors.As(err, &dupeErr) {
@@ -389,7 +389,7 @@ func (hs *HTTPServer) InstallPlugin(c *models.ReqContext) response.Response {
 func (hs *HTTPServer) UninstallPlugin(c *models.ReqContext) response.Response {
 	pluginID := web.Params(c.Req)[":pluginId"]
 
-	err := hs.pluginStore.Remove(c.Req.Context(), pluginID)
+	err := hs.pluginManager.Remove(c.Req.Context(), pluginID)
 	if err != nil {
 		if errors.Is(err, plugins.ErrPluginNotInstalled) {
 			return response.Error(http.StatusNotFound, "Plugin not installed", err)
diff --git a/pkg/plugins/ifaces.go b/pkg/plugins/ifaces.go
index 5d3aa9cb7f3..4971b0913be 100644
--- a/pkg/plugins/ifaces.go
+++ b/pkg/plugins/ifaces.go
@@ -15,6 +15,9 @@ type Store interface {
 	Plugin(ctx context.Context, pluginID string) (PluginDTO, bool)
 	// Plugins returns plugins by their requested type.
 	Plugins(ctx context.Context, pluginTypes ...Type) []PluginDTO
+}
+
+type Manager interface {
 	// Add adds a plugin to the store.
 	Add(ctx context.Context, pluginID, version string) error
 	// Remove removes a plugin from the store.
diff --git a/pkg/plugins/manager/registry/in_memory_test.go b/pkg/plugins/manager/registry/in_memory_test.go
index d2ac98f5088..80e050bb2a1 100644
--- a/pkg/plugins/manager/registry/in_memory_test.go
+++ b/pkg/plugins/manager/registry/in_memory_test.go
@@ -3,6 +3,7 @@ package registry
 import (
 	"context"
 	"fmt"
+	"sort"
 	"testing"
 
 	"github.com/stretchr/testify/require"
@@ -213,6 +214,11 @@ func TestInMemory_Plugins(t *testing.T) {
 				store: tt.mocks.store,
 			}
 			result := i.Plugins(context.Background())
+
+			// to ensure we can compare with expected
+			sort.SliceStable(result, func(i, j int) bool {
+				return result[i].ID < result[j].ID
+			})
 			require.Equal(t, tt.expected, result)
 		})
 	}
diff --git a/pkg/server/wire.go b/pkg/server/wire.go
index 47525d52e52..cd863c600fd 100644
--- a/pkg/server/wire.go
+++ b/pkg/server/wire.go
@@ -143,6 +143,7 @@ var wireBasicSet = wire.NewSet(
 	registry.ProvideService,
 	wire.Bind(new(registry.Service), new(*registry.InMemory)),
 	manager.ProvideService,
+	wire.Bind(new(plugins.Manager), new(*manager.PluginManager)),
 	wire.Bind(new(plugins.Client), new(*manager.PluginManager)),
 	wire.Bind(new(plugins.Store), new(*manager.PluginManager)),
 	wire.Bind(new(plugins.DashboardFileStore), new(*manager.PluginManager)),