Repository: grafana/grafana-plugin-sdk-go (go)

Issue:
SQLUtil: Nullable bool should be handled automatically 
**What would you like to be added**:
I'd like SQLUtil to have a default converter for nullable bool. 

**Why is this needed**:
Without a converter for this, go/sql returns an error. Each consumer (ds plugin) should have to implement its own default converter for this.


Pull request:
SQLUtil: Add default converter for nullable bool
**What this PR does / why we need it**:
Nullable bool aren't handled by SQLUtil.

**Which issue(s) this PR fixes**:
Fixes #368

**Special notes for your reviewer**:
This problem was found when developing the Redshift data source. It's temporary solved by providing a custom converter in the plugin. However, external plugins shouldn't have to provide default converters IMO.  


Patch:
diff --git a/backend/common.go b/backend/common.go
index a19addd..c2fd69e 100644
--- a/backend/common.go
+++ b/backend/common.go
@@ -91,6 +91,16 @@ func (s *DataSourceInstanceSettings) HTTPClientOptions() (httpclient.Options, er
 		return httpclient.Options{}, err
 	}
 
+	if s.BasicAuthEnabled {
+		httpSettings.BasicAuthEnabled = s.BasicAuthEnabled
+		httpSettings.BasicAuthUser = s.BasicAuthUser
+		httpSettings.BasicAuthPassword = s.DecryptedSecureJSONData["basicAuthPassword"]
+	} else if s.User != "" {
+		httpSettings.BasicAuthEnabled = true
+		httpSettings.BasicAuthUser = s.User
+		httpSettings.BasicAuthPassword = s.DecryptedSecureJSONData["password"]
+	}
+
 	return httpSettings.HTTPClientOptions(), nil
 }
 
diff --git a/backend/http_settings.go b/backend/http_settings.go
index 4a22e13..3470685 100644
--- a/backend/http_settings.go
+++ b/backend/http_settings.go
@@ -23,6 +23,7 @@ type HTTPSettings struct {
 	KeepAlive             time.Duration
 	TLSHandshakeTimeout   time.Duration
 	ExpectContinueTimeout time.Duration
+	MaxConnsPerHost       int
 	MaxIdleConns          int
 	MaxIdleConnsPerHost   int
 	IdleConnTimeout       time.Duration
@@ -57,6 +58,7 @@ func (s *HTTPSettings) HTTPClientOptions() httpclient.Options {
 		KeepAlive:             s.KeepAlive,
 		TLSHandshakeTimeout:   s.TLSHandshakeTimeout,
 		ExpectContinueTimeout: s.ExpectContinueTimeout,
+		MaxConnsPerHost:       s.MaxConnsPerHost,
 		MaxIdleConns:          s.MaxIdleConns,
 		MaxIdleConnsPerHost:   s.MaxIdleConnsPerHost,
 		IdleConnTimeout:       s.IdleConnTimeout,
@@ -101,8 +103,10 @@ func parseHTTPSettings(jsonData json.RawMessage, secureJSONData map[string]strin
 	}
 
 	var dat map[string]interface{}
-	if err := json.Unmarshal(jsonData, &dat); err != nil {
-		return nil, err
+	if jsonData != nil {
+		if err := json.Unmarshal(jsonData, &dat); err != nil {
+			return nil, err
+		}
 	}
 
 	if v, exists := dat["access"]; exists {
@@ -169,6 +173,14 @@ func parseHTTPSettings(jsonData json.RawMessage, secureJSONData map[string]strin
 		s.ExpectContinueTimeout = httpclient.DefaultTimeoutOptions.ExpectContinueTimeout
 	}
 
+	if v, exists := dat["httpMaxConnsPerHost"]; exists {
+		if iv, ok := v.(float64); ok {
+			s.MaxConnsPerHost = int(iv)
+		}
+	} else {
+		s.MaxConnsPerHost = httpclient.DefaultTimeoutOptions.MaxConnsPerHost
+	}
+
 	if v, exists := dat["httpMaxIdleConns"]; exists {
 		if iv, ok := v.(float64); ok {
 			s.MaxIdleConns = int(iv)
diff --git a/backend/httpclient/http_client.go b/backend/httpclient/http_client.go
index 16b6396..bb80d61 100644
--- a/backend/httpclient/http_client.go
+++ b/backend/httpclient/http_client.go
@@ -70,6 +70,7 @@ func GetTransport(opts ...Options) (http.RoundTripper, error) {
 		ResponseHeaderTimeout: clientOpts.Timeouts.Timeout,
 		TLSHandshakeTimeout:   clientOpts.Timeouts.TLSHandshakeTimeout,
 		ExpectContinueTimeout: clientOpts.Timeouts.ExpectContinueTimeout,
+		MaxConnsPerHost:       clientOpts.Timeouts.MaxConnsPerHost,
 		MaxIdleConns:          clientOpts.Timeouts.MaxIdleConns,
 		MaxIdleConnsPerHost:   clientOpts.Timeouts.MaxIdleConnsPerHost,
 		IdleConnTimeout:       clientOpts.Timeouts.IdleConnTimeout,
diff --git a/backend/httpclient/options.go b/backend/httpclient/options.go
index de5004e..ec5600d 100644
--- a/backend/httpclient/options.go
+++ b/backend/httpclient/options.go
@@ -72,6 +72,7 @@ type TimeoutOptions struct {
 	KeepAlive             time.Duration
 	TLSHandshakeTimeout   time.Duration
 	ExpectContinueTimeout time.Duration
+	MaxConnsPerHost       int
 	MaxIdleConns          int
 	MaxIdleConnsPerHost   int
 	IdleConnTimeout       time.Duration
@@ -84,6 +85,7 @@ var DefaultTimeoutOptions = TimeoutOptions{
 	KeepAlive:             30 * time.Second,
 	TLSHandshakeTimeout:   10 * time.Second,
 	ExpectContinueTimeout: 1 * time.Second,
+	MaxConnsPerHost:       0,
 	MaxIdleConns:          100,
 	MaxIdleConnsPerHost:   100,
 	IdleConnTimeout:       90 * time.Second,
diff --git a/build/common.go b/build/common.go
index da15aa2..77961a4 100644
--- a/build/common.go
+++ b/build/common.go
@@ -2,16 +2,15 @@ package build
 
 import (
 	"bufio"
-	"encoding/json"
 	"fmt"
 	"io/ioutil"
 	"log"
 	"os"
-	"path"
 	"path/filepath"
 	"runtime"
 	"strings"
 
+	"github.com/grafana/grafana-plugin-sdk-go/internal"
 	"github.com/magefile/mage/mg"
 	"github.com/magefile/mage/sh"
 )
@@ -31,7 +30,7 @@ var exname string
 
 func getExecutableName(os string, arch string) (string, error) {
 	if exname == "" {
-		exename, err := getExecutableFromPluginJSON()
+		exename, err := internal.GetExecutableFromPluginJSON("src")
 		if err != nil {
 			return "", err
 		}
@@ -46,29 +45,6 @@ func getExecutableName(os string, arch string) (string, error) {
 	return exeName, nil
 }
 
-func getValueFromJSON(fpath string, key string) (string, error) {
-	byteValue, err := ioutil.ReadFile(fpath)
-	if err != nil {
-		return "", err
-	}
-
-	var result map[string]interface{}
-	err = json.Unmarshal(byteValue, &result)
-	if err != nil {
-		return "", err
-	}
-	executable := result[key]
-	name, ok := executable.(string)
-	if !ok || name == "" {
-		return "", fmt.Errorf("plugin.json is missing: %s", key)
-	}
-	return name, nil
-}
-
-func getExecutableFromPluginJSON() (string, error) {
-	return getValueFromJSON(path.Join("src", "plugin.json"), "executable")
-}
-
 func buildBackend(cfg Config) error {
 	cfg, err := beforeBuild(cfg)
 	if err != nil {
@@ -104,7 +80,7 @@ func buildBackend(cfg Config) error {
 	}
 
 	info := getBuildInfoFromEnvironment()
-	version, err := getValueFromJSON("package.json", "version")
+	version, err := internal.GetStringValueFromJSON("package.json", "version")
 	if err == nil && len(version) > 0 {
 		info.Version = version
 	}
diff --git a/data/frame.go b/data/frame.go
index a87069c..2a15e78 100644
--- a/data/frame.go
+++ b/data/frame.go
@@ -206,6 +206,17 @@ func (f *Frame) SetMeta(m *FrameMeta) *Frame {
 	return f
 }
 
+// SetIsWideTimeSeries sets the Frame's Meta's IsWideTimeSeries attribute to b and returns the Frame.
+// It will create Meta if it is missing.
+func (f *Frame) SetIsWideTimeSeries(b bool) *Frame {
+	if f.Meta != nil {
+		f.Meta.IsWideTimeSeries = b
+	} else {
+		f.Meta = &FrameMeta{IsWideTimeSeries: b}
+	}
+	return f
+}
+
 // Rows returns the number of rows in the frame.
 func (f *Frame) Rows() int {
 	if len(f.Fields) > 0 {
diff --git a/data/frame_meta.go b/data/frame_meta.go
index 1e10374..c08ecab 100644
--- a/data/frame_meta.go
+++ b/data/frame_meta.go
@@ -35,6 +35,9 @@ type FrameMeta struct {
 	// ExecutedQueryString is the raw query sent to the underlying system. All macros and templating
 	// have been applied.  When metadata contains this value, it will be shown in the query inspector.
 	ExecutedQueryString string `json:"executedQueryString,omitempty"`
+
+	// IsWideTimeSeries is true when the Frame is a wide data frame.
+	IsWideTimeSeries bool `json:"isWideTimeSeries,omitempty"`
 }
 
 // Should be kept in sync with grafana/packages/grafana-data/src/types/data.ts#PreferredVisualisationType
diff --git a/data/sqlutil/converter.go b/data/sqlutil/converter.go
index 768c52a..5133de8 100644
--- a/data/sqlutil/converter.go
+++ b/data/sqlutil/converter.go
@@ -286,6 +286,25 @@ var (
 			},
 		},
 	}
+
+	// NullBoolConverter creates a *bool using the scan type of `sql.NullBool`
+	NullBoolConverter = Converter{
+		Name:          "nullable bool converter",
+		InputScanType: reflect.TypeOf(sql.NullBool{}),
+		InputTypeName: "BOOLEAN",
+		FrameConverter: FrameConverter{
+			FieldType: data.FieldTypeNullableBool,
+			ConverterFunc: func(n interface{}) (interface{}, error) {
+				v := n.(*sql.NullBool)
+
+				if !v.Valid {
+					return (*bool)(nil), nil
+				}
+
+				return &v.Bool, nil
+			},
+		},
+	}
 )
 
 // NullConverters is a map of data type names (from reflect.TypeOf(...).String()) to converters
@@ -296,4 +315,5 @@ var NullConverters = map[reflect.Type]Converter{
 	reflect.TypeOf(int32(0)):    NullInt32Converter,
 	reflect.TypeOf(""):          NullStringConverter,
 	reflect.TypeOf(time.Time{}): NullTimeConverter,
+	reflect.TypeOf(false):       NullBoolConverter,
 }
diff --git a/data/time_series.go b/data/time_series.go
index 006ebe8..06b27aa 100644
--- a/data/time_series.go
+++ b/data/time_series.go
@@ -219,6 +219,7 @@ func LongToWide(longFrame *Frame, fillMissing *FillMissing) (*Frame, error) {
 
 	wideFrame := NewFrame(longFrame.Name, NewField(longFrame.Fields[tsSchema.TimeIndex].Name, nil, []time.Time{}))
 	wideFrame.Meta = longFrame.Meta
+	wideFrame.SetIsWideTimeSeries(true)
 
 	sortKeys := make([]string, len(tsSchema.FactorIndices))
 	for i, v := range tsSchema.FactorIndices { // set dimension key order for final sort
diff --git a/internal/standalone/standalone.go b/internal/standalone/standalone.go
index 07300b8..79297f4 100644
--- a/internal/standalone/standalone.go
+++ b/internal/standalone/standalone.go
@@ -1,7 +1,6 @@
 package standalone
 
 import (
-	"encoding/json"
 	"flag"
 	"fmt"
 	"io/ioutil"
@@ -13,6 +12,8 @@ import (
 	"strconv"
 	"strings"
 	"time"
+
+	"github.com/grafana/grafana-plugin-sdk-go/internal"
 )
 
 type Args struct {
@@ -114,25 +115,15 @@ func findAndKillCurrentPlugin(dir string) {
 		}
 	}()
 
-	var pluginJSON map[string]interface{}
-	pjson, err := ioutil.ReadFile(filepath.Join(dir, "plugin.json"))
-	if err != nil {
-		return
-	}
-	err = json.Unmarshal(pjson, &pluginJSON)
+	exeprefix, err := internal.GetExecutableFromPluginJSON(dir)
 	if err != nil {
-		return
-	}
-	exeprefix, ok := pluginJSON["executable"]
-	if !ok {
-		fmt.Printf("missing executable form plugin.json")
+		fmt.Printf("missing executable in plugin.json (standalone)")
 		return
 	}
 
-	arg1 := exeprefix.(string)
-	out, err := exec.Command("pgrep", arg1).Output()
+	out, err := exec.Command("pgrep", exeprefix).Output()
 	if err != nil {
-		fmt.Printf("error running pgrep")
+		fmt.Printf("error running pgrep: %s (%s)", err.Error(), exeprefix)
 		return
 	}
 	for _, txt := range strings.Fields(string(out)) {
diff --git a/internal/utils.go b/internal/utils.go
new file mode 100644
index 0000000..b99bd1d
--- /dev/null
+++ b/internal/utils.go
@@ -0,0 +1,43 @@
+package internal
+
+import (
+	"encoding/json"
+	"fmt"
+	"io/ioutil"
+	"path"
+	"strings"
+)
+
+func GetStringValueFromJSON(fpath string, key string) (string, error) {
+	byteValue, err := ioutil.ReadFile(fpath)
+	if err != nil {
+		return "", err
+	}
+
+	var result map[string]interface{}
+	err = json.Unmarshal(byteValue, &result)
+	if err != nil {
+		return "", err
+	}
+	executable := result[key]
+	name, ok := executable.(string)
+	if !ok || name == "" {
+		return "", fmt.Errorf("plugin.json is missing: %s", key)
+	}
+	return name, nil
+}
+
+func GetExecutableFromPluginJSON(dir string) (string, error) {
+	exe, err := GetStringValueFromJSON(path.Join(dir, "plugin.json"), "executable")
+	if err != nil {
+		// In app plugins, the exe may be nested
+		exe, err2 := GetStringValueFromJSON(path.Join(dir, "datasource", "plugin.json"), "executable")
+		if err2 == nil {
+			if !strings.HasPrefix(exe, "../") {
+				return "", fmt.Errorf("datasouce should reference executable in root folder")
+			}
+			return exe[3:], nil
+		}
+	}
+	return exe, err
+}


Test patch:
diff --git a/backend/common_test.go b/backend/common_test.go
new file mode 100644
index 0000000..ce4440f
--- /dev/null
+++ b/backend/common_test.go
@@ -0,0 +1,64 @@
+package backend
+
+import (
+	"testing"
+
+	"github.com/grafana/grafana-plugin-sdk-go/backend/httpclient"
+	"github.com/stretchr/testify/assert"
+)
+
+func TestDataSourceInstanceSettings(t *testing.T) {
+	t.Run("HTTPClientOptions() should translate basic auth settings as expected", func(t *testing.T) {
+		tcs := []struct {
+			instanceSettings      *DataSourceInstanceSettings
+			expectedClientOptions httpclient.Options
+		}{
+			{
+				instanceSettings:      &DataSourceInstanceSettings{},
+				expectedClientOptions: httpclient.Options{},
+			},
+			{
+				instanceSettings: &DataSourceInstanceSettings{
+					User:             "user",
+					JSONData:         []byte("{}"),
+					BasicAuthEnabled: true,
+					BasicAuthUser:    "buser",
+					DecryptedSecureJSONData: map[string]string{
+						"basicAuthPassword": "bpwd",
+						"password":          "pwd",
+					},
+				},
+				expectedClientOptions: httpclient.Options{
+					BasicAuth: &httpclient.BasicAuthOptions{
+						User:     "buser",
+						Password: "bpwd",
+					},
+				},
+			},
+			{
+				instanceSettings: &DataSourceInstanceSettings{
+					User:             "user",
+					JSONData:         []byte("{}"),
+					BasicAuthEnabled: false,
+					BasicAuthUser:    "buser",
+					DecryptedSecureJSONData: map[string]string{
+						"basicAuthPassword": "bpwd",
+						"password":          "pwd",
+					},
+				},
+				expectedClientOptions: httpclient.Options{
+					BasicAuth: &httpclient.BasicAuthOptions{
+						User:     "user",
+						Password: "pwd",
+					},
+				},
+			},
+		}
+
+		for _, tc := range tcs {
+			opts, err := tc.instanceSettings.HTTPClientOptions()
+			assert.NoError(t, err)
+			assert.Equal(t, tc.expectedClientOptions.BasicAuth, opts.BasicAuth)
+		}
+	})
+}
diff --git a/backend/http_settings_test.go b/backend/http_settings_test.go
index c5a29ba..a0711d1 100644
--- a/backend/http_settings_test.go
+++ b/backend/http_settings_test.go
@@ -28,6 +28,7 @@ func TestParseHTTPSettings(t *testing.T) {
 			"httpKeepAlive": 11,
 			"httpTLSHandshakeTimeout": 12,
 			"httpExpectContinueTimeout": 13,
+			"httpMaxConnsPerHost": 20,
 			"httpMaxIdleConns": 14,
 			"httpMaxIdleConnsPerHost": 16,
 			"httpIdleConnTimeout": 15,
@@ -72,6 +73,7 @@ func TestParseHTTPSettings(t *testing.T) {
 			KeepAlive:             11 * time.Second,
 			TLSHandshakeTimeout:   12 * time.Second,
 			ExpectContinueTimeout: 13 * time.Second,
+			MaxConnsPerHost:       20,
 			MaxIdleConns:          14,
 			MaxIdleConnsPerHost:   16,
 			IdleConnTimeout:       15 * time.Second,
@@ -110,6 +112,7 @@ func TestParseHTTPSettings(t *testing.T) {
 					KeepAlive:             11 * time.Second,
 					TLSHandshakeTimeout:   12 * time.Second,
 					ExpectContinueTimeout: 13 * time.Second,
+					MaxConnsPerHost:       20,
 					MaxIdleConns:          14,
 					MaxIdleConnsPerHost:   16,
 					IdleConnTimeout:       15 * time.Second,
diff --git a/backend/httpclient/http_client_test.go b/backend/httpclient/http_client_test.go
index 5ccd589..e421a22 100644
--- a/backend/httpclient/http_client_test.go
+++ b/backend/httpclient/http_client_test.go
@@ -24,6 +24,7 @@ func TestNewClient(t *testing.T) {
 				KeepAlive:             2 * time.Second,
 				TLSHandshakeTimeout:   3 * time.Second,
 				ExpectContinueTimeout: 4 * time.Second,
+				MaxConnsPerHost:       10,
 				MaxIdleConns:          5,
 				MaxIdleConnsPerHost:   7,
 				IdleConnTimeout:       6 * time.Second,
@@ -39,6 +40,7 @@ func TestNewClient(t *testing.T) {
 		require.NotNil(t, transport)
 		require.Equal(t, 3*time.Second, transport.TLSHandshakeTimeout)
 		require.Equal(t, 4*time.Second, transport.ExpectContinueTimeout)
+		require.Equal(t, 10, transport.MaxConnsPerHost)
 		require.Equal(t, 5, transport.MaxIdleConns)
 		require.Equal(t, 7, transport.MaxIdleConnsPerHost)
 		require.Equal(t, 6*time.Second, transport.IdleConnTimeout)
diff --git a/backend/httpclient/provider_test.go b/backend/httpclient/provider_test.go
index 7f8a0b1..a3fbd5b 100644
--- a/backend/httpclient/provider_test.go
+++ b/backend/httpclient/provider_test.go
@@ -46,6 +46,7 @@ func TestProvider(t *testing.T) {
 					KeepAlive:             2 * time.Second,
 					TLSHandshakeTimeout:   3 * time.Second,
 					ExpectContinueTimeout: 4 * time.Second,
+					MaxConnsPerHost:       10,
 					MaxIdleConns:          5,
 					MaxIdleConnsPerHost:   7,
 					IdleConnTimeout:       6 * time.Second,
@@ -108,6 +109,7 @@ func TestProvider(t *testing.T) {
 				KeepAlive:             2 * time.Second,
 				TLSHandshakeTimeout:   3 * time.Second,
 				ExpectContinueTimeout: 4 * time.Second,
+				MaxConnsPerHost:       10,
 				MaxIdleConns:          5,
 				MaxIdleConnsPerHost:   7,
 				IdleConnTimeout:       6 * time.Second,
@@ -153,6 +155,7 @@ func TestProvider(t *testing.T) {
 			require.NotNil(t, transport)
 			require.Equal(t, 3*time.Second, ctx.transport.TLSHandshakeTimeout)
 			require.Equal(t, 4*time.Second, ctx.transport.ExpectContinueTimeout)
+			require.Equal(t, 10, ctx.transport.MaxConnsPerHost)
 			require.Equal(t, 5, ctx.transport.MaxIdleConns)
 			require.Equal(t, 7, ctx.transport.MaxIdleConnsPerHost)
 			require.Equal(t, 6*time.Second, ctx.transport.IdleConnTimeout)
diff --git a/data/sqlutil/converter_test.go b/data/sqlutil/converter_test.go
index ec87aab..000f2ca 100644
--- a/data/sqlutil/converter_test.go
+++ b/data/sqlutil/converter_test.go
@@ -87,6 +87,17 @@ func TestDefaultConverter(t *testing.T) {
 				},
 			},
 		},
+		{
+			Name:     "nullable bool",
+			Type:     reflect.TypeOf(false),
+			Nullable: true,
+			Expected: sqlutil.Converter{
+				InputScanType: reflect.TypeOf(sql.NullBool{}),
+				FrameConverter: sqlutil.FrameConverter{
+					FieldType: data.FieldTypeBool.NullableType(),
+				},
+			},
+		},
 	}
 
 	for i, v := range suite {
diff --git a/data/time_series_test.go b/data/time_series_test.go
index e8931e1..6ffd3c5 100644
--- a/data/time_series_test.go
+++ b/data/time_series_test.go
@@ -1006,6 +1006,7 @@ func TestLongToWide(t *testing.T) {
 	}
 	for i := range tests {
 		tt := tests[i]
+		tt.wideFrame.Meta = &data.FrameMeta{IsWideTimeSeries: true}
 		t.Run(tt.name, func(t *testing.T) {
 			frame, err := data.LongToWide(tt.longFrame, tt.tsFillMissing)
 			tt.Err(t, err)
@@ -1064,6 +1065,7 @@ func TestLongToWideBool(t *testing.T) {
 	}
 	for i := range tests {
 		tt := tests[i]
+		tt.wideFrame.Meta = &data.FrameMeta{IsWideTimeSeries: true}
 		t.Run(tt.name, func(t *testing.T) {
 			frame, err := data.LongToWide(tt.longFrame, tt.tsFillMissing)
 			tt.Err(t, err)