Refactor: use InstanceManager for managing ds instances

pkg/datasource/datasource.go

@@ -4,14 +4,14 @@ import (
 	"context"
 	"encoding/json"
 	"errors"
-	"fmt"
 	"time"
 
-	"github.com/alexanderzobnin/grafana-zabbix/pkg/cache"
 	"github.com/alexanderzobnin/grafana-zabbix/pkg/gtime"
 	"github.com/alexanderzobnin/grafana-zabbix/pkg/zabbixapi"
 
 	"github.com/grafana/grafana-plugin-sdk-go/backend"
+	"github.com/grafana/grafana-plugin-sdk-go/backend/datasource"
+	"github.com/grafana/grafana-plugin-sdk-go/backend/instancemgmt"
 	"github.com/grafana/grafana-plugin-sdk-go/backend/log"
 	"github.com/grafana/grafana-plugin-sdk-go/data"
 )
@@ -22,7 +22,7 @@ var (
 )
 
 type ZabbixDatasource struct {
-	datasourceCache *cache.Cache
+	im     instancemgmt.InstanceManager
 	logger log.Logger
 }
 
@@ -37,30 +37,36 @@ type ZabbixDatasourceInstance struct {
 }
 
 func NewZabbixDatasource() *ZabbixDatasource {
+	im := datasource.NewInstanceManager(newZabbixDatasourceInstance)
 	return &ZabbixDatasource{
-		datasourceCache: cache.NewCache(10*time.Minute, 10*time.Minute),
+		im:     im,
 		logger: log.New(),
 	}
 }
 
-// NewZabbixDatasourceInstance returns an initialized zabbix datasource instance
+// newZabbixDatasourceInstance returns an initialized zabbix datasource instance
-func NewZabbixDatasourceInstance(dsInfo *backend.DataSourceInstanceSettings) (*ZabbixDatasourceInstance, error) {
+func newZabbixDatasourceInstance(settings backend.DataSourceInstanceSettings) (instancemgmt.Instance, error) {
-	zabbixAPI, err := zabbixapi.New(dsInfo.URL)
+	logger := log.New()
+	logger.Debug("Initializing new data source instance")
+
+	zabbixAPI, err := zabbixapi.New(settings.URL)
 	if err != nil {
+		logger.Error("Error initializing Zabbix API", "error", err)
 		return nil, err
 	}
 
-	zabbixSettings, err := readZabbixSettings(dsInfo)
+	zabbixSettings, err := readZabbixSettings(&settings)
 	if err != nil {
+		logger.Error("Error parsing Zabbix settings", "error", err)
 		return nil, err
 	}
 
 	return &ZabbixDatasourceInstance{
-		dsInfo:     dsInfo,
+		dsInfo:     &settings,
 		zabbixAPI:  zabbixAPI,
 		Settings:   zabbixSettings,
 		queryCache: NewDatasourceCache(zabbixSettings.CacheTTL, 10*time.Minute),
-		logger:     log.New(),
+		logger:     logger,
 	}, nil
 }
 
@@ -68,7 +74,7 @@ func NewZabbixDatasourceInstance(dsInfo *backend.DataSourceInstanceSettings) (*Z
 func (ds *ZabbixDatasource) CheckHealth(ctx context.Context, req *backend.CheckHealthRequest) (*backend.CheckHealthResult, error) {
 	res := &backend.CheckHealthResult{}
 
-	dsInstance, err := ds.GetDatasource(req.PluginContext)
+	dsInstance, err := ds.getDSInstance(req.PluginContext)
 	if err != nil {
 		res.Status = backend.HealthStatusError
 		res.Message = "Error getting datasource instance"
@@ -92,7 +98,7 @@ func (ds *ZabbixDatasource) CheckHealth(ctx context.Context, req *backend.CheckH
 func (ds *ZabbixDatasource) QueryData(ctx context.Context, req *backend.QueryDataRequest) (*backend.QueryDataResponse, error) {
 	qdr := backend.NewQueryDataResponse()
 
-	zabbixDS, err := ds.GetDatasource(req.PluginContext)
+	zabbixDS, err := ds.getDSInstance(req.PluginContext)
 	if err != nil {
 		return nil, err
 	}
@@ -121,32 +127,13 @@ func (ds *ZabbixDatasource) QueryData(ctx context.Context, req *backend.QueryDat
 	return qdr, nil
 }
 
-// GetDatasource Returns cached datasource or creates new one
+// getDSInstance Returns cached datasource or creates new one
-func (ds *ZabbixDatasource) GetDatasource(pluginContext backend.PluginContext) (*ZabbixDatasourceInstance, error) {
+func (ds *ZabbixDatasource) getDSInstance(pluginContext backend.PluginContext) (*ZabbixDatasourceInstance, error) {
-	dsSettings := pluginContext.DataSourceInstanceSettings
+	instance, err := ds.im.Get(pluginContext)
-	dsKey := fmt.Sprintf("%d-%d", pluginContext.OrgID, dsSettings.ID)
-	// Get hash to check if settings changed
-	dsInfoHash := HashDatasourceInfo(dsSettings)
-
-	if cachedData, ok := ds.datasourceCache.Get(dsKey); ok {
-		if cachedDS, ok := cachedData.(*ZabbixDatasourceInstance); ok {
-			cachedDSHash := HashDatasourceInfo(cachedDS.dsInfo)
-			if cachedDSHash == dsInfoHash {
-				return cachedDS, nil
-			}
-			ds.logger.Debug("Data source settings changed", "org", pluginContext.OrgID, "id", dsSettings.ID, "name", dsSettings.Name)
-		}
-	}
-
-	ds.logger.Debug("Initializing data source", "org", pluginContext.OrgID, "id", dsSettings.ID, "name", dsSettings.Name)
-	dsInstance, err := NewZabbixDatasourceInstance(pluginContext.DataSourceInstanceSettings)
 	if err != nil {
-		ds.logger.Error("Error initializing datasource", "error", err)
 		return nil, err
 	}
-
+	return instance.(*ZabbixDatasourceInstance), nil
-	ds.datasourceCache.Set(dsKey, dsInstance)
-	return dsInstance, nil
 }
 
 func readZabbixSettings(dsInstanceSettings *backend.DataSourceInstanceSettings) (*ZabbixDatasourceSettings, error) {

pkg/datasource/datasource_cache.go

@@ -3,11 +3,9 @@ package datasource
 import (
 	"crypto/sha1"
 	"encoding/hex"
-	"encoding/json"
 	"time"
 
 	"github.com/alexanderzobnin/grafana-zabbix/pkg/cache"
-	"github.com/grafana/grafana-plugin-sdk-go/backend"
 )
 
 // DatasourceCache is a cache for datasource instance.
@@ -40,12 +38,3 @@ func HashString(text string) string {
 	hash.Write([]byte(text))
 	return hex.EncodeToString(hash.Sum(nil))
 }
-
-// HashDatasourceInfo converts the given datasource info to hash string
-func HashDatasourceInfo(dsInfo *backend.DataSourceInstanceSettings) string {
-	digester := sha1.New()
-	if err := json.NewEncoder(digester).Encode(dsInfo); err != nil {
-		panic(err) // This shouldn't be possible but just in case DatasourceInfo changes
-	}
-	return hex.EncodeToString(digester.Sum(nil))
-}

pkg/datasource/datasource_cache_test.go

@@ -1,100 +0,0 @@
-package datasource
-
-import (
-	"testing"
-
-	"github.com/grafana/grafana-plugin-sdk-go/backend"
-	"gotest.tools/assert"
-)
-
-func TestHashDatasourceInfo(t *testing.T) {
-	tests := []struct {
-		name         string
-		dsInfoBefore *backend.DataSourceInstanceSettings
-		dsInfoAfter  *backend.DataSourceInstanceSettings
-		equal        bool
-	}{
-		{
-			name: "Same datasource settings",
-			dsInfoBefore: &backend.DataSourceInstanceSettings{
-				ID:       1,
-				Name:     "Zabbix",
-				URL:      "https://localhost:3306/zabbix/api_jsonrpc.php",
-				JSONData: []byte("{}"),
-				DecryptedSecureJSONData: map[string]string{
-					"username": "grafanaZabbixUser",
-					"password": "$uper$ecr3t!!!",
-				},
-			},
-			dsInfoAfter: &backend.DataSourceInstanceSettings{
-				ID:       1,
-				Name:     "Zabbix",
-				URL:      "https://localhost:3306/zabbix/api_jsonrpc.php",
-				JSONData: []byte("{}"),
-				DecryptedSecureJSONData: map[string]string{
-					"username": "grafanaZabbixUser",
-					"password": "$uper$ecr3t!!!",
-				},
-			},
-			equal: true,
-		},
-		{
-			name: "Password changed",
-			dsInfoBefore: &backend.DataSourceInstanceSettings{
-				ID:       1,
-				Name:     "Zabbix",
-				URL:      "https://localhost:3306/zabbix/api_jsonrpc.php",
-				JSONData: []byte("{}"),
-				DecryptedSecureJSONData: map[string]string{
-					"username": "grafanaZabbixUser",
-					"password": "$uper$ecr3t!!!",
-				},
-			},
-			dsInfoAfter: &backend.DataSourceInstanceSettings{
-				ID:       1,
-				Name:     "Zabbix",
-				URL:      "https://localhost:3306/zabbix/api_jsonrpc.php",
-				JSONData: []byte("{}"),
-				DecryptedSecureJSONData: map[string]string{
-					"username": "grafanaZabbixUser",
-					"password": "new$uper$ecr3t!!!",
-				},
-			},
-			equal: false,
-		},
-	}
-	for _, tt := range tests {
-		t.Run(tt.name, func(t *testing.T) {
-			before := HashDatasourceInfo(tt.dsInfoBefore)
-			after := HashDatasourceInfo(tt.dsInfoAfter)
-			got := before == after
-			assert.Equal(t, tt.equal, got)
-		})
-	}
-}
-
-func BenchmarkHashDatasourceInfo(b *testing.B) {
-	benches := []struct {
-		name   string
-		dsInfo *backend.DataSourceInstanceSettings
-	}{
-		{
-			"Normal Datasource Info",
-			&backend.DataSourceInstanceSettings{
-				ID:       4,
-				Name:     "MyZabbixDatasource",
-				URL:      "https://localhost:3306/zabbix/api_jsonrpc.php",
-				JSONData: []byte(`{ "addThresholds": true, "disableReadOnlyUsersAck": true }`),
-				DecryptedSecureJSONData: map[string]string{
-					"username": "grafanaZabbixUser",
-					"password": "$uper$ecr3t!!!",
-				},
-			},
-		},
-	}
-	for _, bt := range benches {
-		b.Run(bt.name, func(b *testing.B) {
-			HashDatasourceInfo(bt.dsInfo)
-		})
-	}
-}

pkg/datasource/datasource_test.go

@@ -1,38 +1,32 @@
 package datasource
 
 import (
-	"fmt"
 	"testing"
 
-	"github.com/alexanderzobnin/grafana-zabbix/pkg/cache"
 	"github.com/grafana/grafana-plugin-sdk-go/backend"
-	"github.com/grafana/grafana-plugin-sdk-go/backend/log"
 	"gotest.tools/assert"
 )
 
 func TestZabbixBackend_getCachedDatasource(t *testing.T) {
-	basicDsSettings := &backend.DataSourceInstanceSettings{
+	basicDsSettings := backend.DataSourceInstanceSettings{
 		ID:       1,
 		Name:     "TestDatasource",
 		URL:      "http://zabbix.org/zabbix",
 		JSONData: []byte("{}"),
 	}
 
-	modifiedDatasourceSettings := &backend.DataSourceInstanceSettings{
+	modifiedDatasourceSettings := backend.DataSourceInstanceSettings{
 		ID:       1,
 		Name:     "TestDatasource",
 		URL:      "http://another.zabbix.org/zabbix",
 		JSONData: []byte("{}"),
 	}
-	modifiedDatasource, _ := NewZabbixDatasourceInstance(modifiedDatasourceSettings)
+	modifiedDatasource, _ := newZabbixDatasourceInstance(modifiedDatasourceSettings)
 
-	basicDS, _ := NewZabbixDatasourceInstance(basicDsSettings)
+	basicDS, _ := newZabbixDatasourceInstance(basicDsSettings)
-	dsCache := cache.NewCache(cache.NoExpiration, cache.NoExpiration)
-	dsCache.Set("1-1", basicDS)
 
 	tests := []struct {
 		name          string
-		cache         *cache.Cache
 		pluginContext backend.PluginContext
 		want          *ZabbixDatasourceInstance
 	}{
@@ -40,47 +34,34 @@ func TestZabbixBackend_getCachedDatasource(t *testing.T) {
 			name: "Uncached Datasource (nothing in cache)",
 			pluginContext: backend.PluginContext{
 				OrgID:                      1,
-				DataSourceInstanceSettings: basicDsSettings,
+				DataSourceInstanceSettings: &basicDsSettings,
 			},
-			want: basicDS,
+			want: basicDS.(*ZabbixDatasourceInstance),
 		},
 		{
 			name: "Cached Datasource",
-			cache: dsCache,
 			pluginContext: backend.PluginContext{
 				OrgID:                      1,
-				DataSourceInstanceSettings: basicDsSettings,
+				DataSourceInstanceSettings: &basicDsSettings,
 			},
-			want: basicDS,
+			want: basicDS.(*ZabbixDatasourceInstance),
 		},
 		{
 			name: "Cached then modified",
-			cache: dsCache,
 			pluginContext: backend.PluginContext{
 				OrgID:                      1,
-				DataSourceInstanceSettings: modifiedDatasourceSettings,
+				DataSourceInstanceSettings: &modifiedDatasourceSettings,
 			},
-			want: modifiedDatasource,
+			want: modifiedDatasource.(*ZabbixDatasourceInstance),
 		},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			if tt.cache == nil {
+			ds := NewZabbixDatasource()
-				tt.cache = cache.NewCache(cache.NoExpiration, cache.NoExpiration)
+			got, _ := ds.getDSInstance(tt.pluginContext)
-			}
-			ds := &ZabbixDatasource{
-				datasourceCache: tt.cache,
-				logger:          log.New(),
-			}
-			got, _ := ds.GetDatasource(tt.pluginContext)
 
 			// Only checking the URL, being the easiest value to, and guarantee equality for
 			assert.Equal(t, tt.want.zabbixAPI.GetUrl().String(), got.zabbixAPI.GetUrl().String())
-
-			// Ensure the datasource is in the cache
-			cacheds, ok := tt.cache.Get(fmt.Sprint("1-", tt.pluginContext.DataSourceInstanceSettings.ID))
-			assert.Equal(t, true, ok)
-			assert.Equal(t, got, cacheds)
 		})
 	}
 }

pkg/datasource/resource_handler.go

@@ -40,7 +40,7 @@ func (ds *ZabbixDatasource) ZabbixAPIHandler(rw http.ResponseWriter, req *http.R
 	}
 
 	pluginCxt := httpadapter.PluginConfigFromContext(req.Context())
-	dsInstance, err := ds.GetDatasource(pluginCxt)
+	dsInstance, err := ds.getDSInstance(pluginCxt)
 	if err != nil {
 		ds.logger.Error("Error loading datasource", "error", err)
 		writeError(rw, http.StatusInternalServerError, err)