refactoring: moved compare dashboard version command out of sqlstore, the code in this command did not use any sql operations and was more high level, could be moved out and use existing queries to get the versions

pkg/api/dashboard.go

@@ -11,6 +11,7 @@ import (
 
 	"github.com/grafana/grafana/pkg/api/dtos"
 	"github.com/grafana/grafana/pkg/bus"
+	"github.com/grafana/grafana/pkg/components/dashdiffs"
 	"github.com/grafana/grafana/pkg/components/simplejson"
 	"github.com/grafana/grafana/pkg/log"
 	"github.com/grafana/grafana/pkg/metrics"
@@ -324,8 +325,7 @@ func GetDashboardVersion(c *middleware.Context) Response {
 	return Json(200, dashVersionMeta)
 }
 
-func createCompareDashboardVersionCommand(c *middleware.Context) (*m.CompareDashboardVersionsCommand, error) {
-	cmd := &m.CompareDashboardVersionsCommand{}
+func getDashboardVersionDiffOptions(c *middleware.Context, diffType dashdiffs.DiffType) (*dashdiffs.Options, error) {
 
 	dashId := c.ParamsInt64(":dashboardId")
 	if dashId == 0 {
@@ -347,39 +347,42 @@ func createCompareDashboardVersionCommand(c *middleware.Context) (*m.CompareDash
 		return nil, fmt.Errorf("bad format: second argument is not of type int")
 	}
 
-	cmd.DashboardId = dashId
-	cmd.OrgId = c.OrgId
-	cmd.BaseVersion = BaseVersion
-	cmd.NewVersion = newVersion
+	options := &dashdiffs.Options{}
+	options.DashboardId = dashId
+	options.OrgId = c.OrgId
+	options.BaseVersion = BaseVersion
+	options.NewVersion = newVersion
+	options.DiffType = diffType
 
-	return cmd, nil
+	return options, nil
 }
 
 // CompareDashboardVersions compares dashboards the way the GitHub API does.
 func CompareDashboardVersions(c *middleware.Context) Response {
-	cmd, err := createCompareDashboardVersionCommand(c)
+	options, err := getDashboardVersionDiffOptions(c, dashdiffs.DiffDelta)
+
 	if err != nil {
 		return ApiError(500, err.Error(), err)
 	}
 
-	cmd.DiffType = m.DiffDelta
-
-	if err := bus.Dispatch(&cmd); err != nil {
+	result, err := dashdiffs.GetVersionDiff(options)
+	if err != nil {
 		return ApiError(500, "Unable to compute diff", err)
 	}
 
 	// here the output is already JSON, so we need to unmarshal it into a
 	// map before marshaling the entire response
+
 	deltaMap := make(map[string]interface{})
-	err = json.Unmarshal(cmd.Delta, &deltaMap)
+	err = json.Unmarshal(result.Delta, &deltaMap)
 	if err != nil {
 		return ApiError(500, err.Error(), err)
 	}
 
 	return Json(200, util.DynMap{
 		"meta": util.DynMap{
-			"baseVersion": cmd.BaseVersion,
-			"newVersion":  cmd.NewVersion,
+			"baseVersion": options.BaseVersion,
+			"newVersion":  options.NewVersion,
 		},
 		"delta": deltaMap,
 	})
@@ -388,34 +391,35 @@ func CompareDashboardVersions(c *middleware.Context) Response {
 // CompareDashboardVersionsJSON compares dashboards the way the GitHub API does,
 // returning a human-readable JSON diff.
 func CompareDashboardVersionsJSON(c *middleware.Context) Response {
-	cmd, err := createCompareDashboardVersionCommand(c)
+	options, err := getDashboardVersionDiffOptions(c, dashdiffs.DiffJSON)
+
 	if err != nil {
 		return ApiError(500, err.Error(), err)
 	}
-	cmd.DiffType = m.DiffJSON
 
-	if err := bus.Dispatch(cmd); err != nil {
+	result, err := dashdiffs.GetVersionDiff(options)
+	if err != nil {
 		return ApiError(500, err.Error(), err)
 	}
 
-	return Respond(200, cmd.Delta).Header("Content-Type", "text/html")
+	return Respond(200, result.Delta).Header("Content-Type", "text/html")
 }
 
 // CompareDashboardVersionsBasic compares dashboards the way the GitHub API does,
 // returning a human-readable diff.
 func CompareDashboardVersionsBasic(c *middleware.Context) Response {
-	cmd, err := createCompareDashboardVersionCommand(c)
+	options, err := getDashboardVersionDiffOptions(c, dashdiffs.DiffBasic)
+
 	if err != nil {
 		return ApiError(500, err.Error(), err)
 	}
 
-	cmd.DiffType = m.DiffBasic
-
-	if err := bus.Dispatch(cmd); err != nil {
+	result, err := dashdiffs.GetVersionDiff(options)
+	if err != nil {
 		return ApiError(500, err.Error(), err)
 	}
 
-	return Respond(200, cmd.Delta).Header("Content-Type", "text/html")
+	return Respond(200, result.Delta).Header("Content-Type", "text/html")
 }
 
 // RestoreDashboardVersion restores a dashboard to the given version.

pkg/components/dashdiffs/compare.go

@@ -0,0 +1,124 @@
+package dashdiffs
+
+import (
+	"encoding/json"
+	"errors"
+
+	"github.com/grafana/grafana/pkg/bus"
+	"github.com/grafana/grafana/pkg/components/simplejson"
+	"github.com/grafana/grafana/pkg/models"
+	diff "github.com/yudai/gojsondiff"
+	deltaFormatter "github.com/yudai/gojsondiff/formatter"
+)
+
+type DiffType int
+
+const (
+	DiffJSON DiffType = iota
+	DiffBasic
+	DiffDelta
+)
+
+var (
+	// ErrUnsupportedDiffType occurs when an invalid diff type is used.
+	ErrUnsupportedDiffType = errors.New("dashdiff: unsupported diff type")
+
+	// ErrNilDiff occurs when two compared interfaces are identical.
+	ErrNilDiff = errors.New("dashdiff: diff is nil")
+)
+
+type Options struct {
+	OrgId       int64
+	DashboardId int64
+	BaseVersion int
+	NewVersion  int
+	DiffType    DiffType
+}
+
+type Result struct {
+	Delta []byte `json:"delta"`
+}
+
+// CompareDashboardVersionsCommand computes the JSON diff of two versions,
+// assigning the delta of the diff to the `Delta` field.
+func GetVersionDiff(options *Options) (*Result, error) {
+	baseVersionQuery := models.GetDashboardVersionQuery{
+		DashboardId: options.DashboardId,
+		Version:     options.BaseVersion,
+	}
+
+	if err := bus.Dispatch(&baseVersionQuery); err != nil {
+		return nil, err
+	}
+
+	newVersionQuery := models.GetDashboardVersionQuery{
+		DashboardId: options.DashboardId,
+		Version:     options.NewVersion,
+	}
+
+	if err := bus.Dispatch(&newVersionQuery); err != nil {
+		return nil, err
+	}
+
+	left, jsonDiff, err := getDiff(baseVersionQuery.Result, newVersionQuery.Result)
+	if err != nil {
+		return nil, err
+	}
+
+	result := &Result{}
+
+	switch options.DiffType {
+	case DiffDelta:
+
+		deltaOutput, err := deltaFormatter.NewDeltaFormatter().Format(jsonDiff)
+		if err != nil {
+			return nil, err
+		}
+		result.Delta = []byte(deltaOutput)
+
+	case DiffJSON:
+		jsonOutput, err := NewJSONFormatter(left).Format(jsonDiff)
+		if err != nil {
+			return nil, err
+		}
+		result.Delta = []byte(jsonOutput)
+
+	case DiffBasic:
+		basicOutput, err := NewBasicFormatter(left).Format(jsonDiff)
+		if err != nil {
+			return nil, err
+		}
+		result.Delta = basicOutput
+
+	default:
+		return nil, ErrUnsupportedDiffType
+	}
+
+	return result, nil
+}
+
+// getDiff computes the diff of two dashboard versions.
+func getDiff(originalDash, newDash *models.DashboardVersion) (interface{}, diff.Diff, error) {
+	leftBytes, err := simplejson.NewFromAny(originalDash).Encode()
+	if err != nil {
+		return nil, nil, err
+	}
+
+	rightBytes, err := simplejson.NewFromAny(newDash).Encode()
+	if err != nil {
+		return nil, nil, err
+	}
+
+	jsonDiff, err := diff.New().Compare(leftBytes, rightBytes)
+	if err != nil {
+		return nil, nil, err
+	}
+
+	if !jsonDiff.Modified() {
+		return nil, nil, ErrNilDiff
+	}
+
+	left := make(map[string]interface{})
+	err = json.Unmarshal(leftBytes, &left)
+	return left, jsonDiff, nil
+}

pkg/components/formatter/formatter_basic.go → pkg/components/dashdiffs/formatter_basic.go

@@ -1,4 +1,4 @@
-package formatter
+package dashdiffs
 
 import (
 	"bytes"
@@ -261,7 +261,7 @@ var (
 				line-display="{{ .LineStart }}{{ if .LineEnd }} - {{ .LineEnd }}{{ end }}"
 				switch-view="ctrl.getDiff('html')"
 			/>
-		{{ end }} 
+		{{ end }}
 
 	</div>
 
@@ -286,7 +286,7 @@ var (
 <li class="diff-change-group">
 	<span class="bullet-position-container">
 		<div class="diff-change-item diff-change-title">{{ getChange .Change }} {{ .Key }}</div>
-		
+
 		<div class="diff-change-item">
 			{{ if .Old }}
 				<div class="change list-change diff-label">{{ .Old }}</div>
@@ -312,7 +312,7 @@ var (
 	tplSummary = `{{ define "summary" -}}
 <div class="diff-group-name">
 	<i class="diff-circle diff-circle-{{ getChange .Change }} fa fa-circle-o diff-list-circle"></i>
-	
+
 	{{ if .Count }}
 		<strong>{{ .Count }}</strong>
 	{{ end }}

pkg/components/formatter/formatter_json.go → pkg/components/dashdiffs/formatter_json.go

@@ -1,4 +1,4 @@
-package formatter
+package dashdiffs
 
 import (
 	"bytes"

pkg/models/dashboard_version.go

@@ -7,14 +7,6 @@ import (
 	"github.com/grafana/grafana/pkg/components/simplejson"
 )
 
-type DiffType int
-
-const (
-	DiffJSON DiffType = iota
-	DiffBasic
-	DiffDelta
-)
-
 var (
 	ErrDashboardVersionNotFound = errors.New("Dashboard version not found")
 	ErrNoVersionsForDashboardId = errors.New("No dashboard versions found for the given DashboardId")
@@ -77,18 +69,3 @@ type GetDashboardVersionsQuery struct {
 
 	Result []*DashboardVersionDTO
 }
-
-//
-// Commands
-//
-
-// CompareDashboardVersionsCommand is used to compare two versions.
-type CompareDashboardVersionsCommand struct {
-	OrgId       int64
-	DashboardId int64
-	BaseVersion int
-	NewVersion  int
-	DiffType    DiffType
-
-	Delta []byte `json:"delta"`
-}

pkg/services/sqlstore/dashboard_version.go

@@ -1,80 +1,15 @@
 package sqlstore
 
 import (
-	"encoding/json"
-	"errors"
-
 	"github.com/grafana/grafana/pkg/bus"
-	"github.com/grafana/grafana/pkg/components/formatter"
-	"github.com/grafana/grafana/pkg/components/simplejson"
 	m "github.com/grafana/grafana/pkg/models"
-
-	diff "github.com/yudai/gojsondiff"
-	deltaFormatter "github.com/yudai/gojsondiff/formatter"
-)
-
-var (
-	// ErrUnsupportedDiffType occurs when an invalid diff type is used.
-	ErrUnsupportedDiffType = errors.New("sqlstore: unsupported diff type")
-
-	// ErrNilDiff occurs when two compared interfaces are identical.
-	ErrNilDiff = errors.New("sqlstore: diff is nil")
 )
 
 func init() {
-	bus.AddHandler("sql", CompareDashboardVersionsCommand)
 	bus.AddHandler("sql", GetDashboardVersion)
 	bus.AddHandler("sql", GetDashboardVersions)
 }
 
-// CompareDashboardVersionsCommand computes the JSON diff of two versions,
-// assigning the delta of the diff to the `Delta` field.
-func CompareDashboardVersionsCommand(cmd *m.CompareDashboardVersionsCommand) error {
-	baseVersion, err := getDashboardVersion(cmd.DashboardId, cmd.BaseVersion)
-	if err != nil {
-		return err
-	}
-
-	newVersion, err := getDashboardVersion(cmd.DashboardId, cmd.NewVersion)
-	if err != nil {
-		return err
-	}
-
-	left, jsonDiff, err := getDiff(baseVersion, newVersion)
-	if err != nil {
-		return err
-	}
-
-	switch cmd.DiffType {
-	case m.DiffDelta:
-
-		deltaOutput, err := deltaFormatter.NewDeltaFormatter().Format(jsonDiff)
-		if err != nil {
-			return err
-		}
-		cmd.Delta = []byte(deltaOutput)
-
-	case m.DiffJSON:
-		jsonOutput, err := formatter.NewJSONFormatter(left).Format(jsonDiff)
-		if err != nil {
-			return err
-		}
-		cmd.Delta = []byte(jsonOutput)
-
-	case m.DiffBasic:
-		basicOutput, err := formatter.NewBasicFormatter(left).Format(jsonDiff)
-		if err != nil {
-			return err
-		}
-		cmd.Delta = basicOutput
-
-	default:
-		return ErrUnsupportedDiffType
-	}
-
-	return nil
-}
-
 // GetDashboardVersion gets the dashboard version for the given dashboard ID
 // and version number.
 func GetDashboardVersion(query *m.GetDashboardVersionQuery) error {
@@ -145,29 +80,3 @@ func getDashboard(dashboardId int64) (*m.Dashboard, error) {
 	}
 	return &dashboard, nil
 }
-
-// getDiff computes the diff of two dashboard versions.
-func getDiff(originalDash, newDash *m.DashboardVersion) (interface{}, diff.Diff, error) {
-	leftBytes, err := simplejson.NewFromAny(originalDash).Encode()
-	if err != nil {
-		return nil, nil, err
-	}
-
-	rightBytes, err := simplejson.NewFromAny(newDash).Encode()
-	if err != nil {
-		return nil, nil, err
-	}
-
-	jsonDiff, err := diff.New().Compare(leftBytes, rightBytes)
-	if err != nil {
-		return nil, nil, err
-	}
-
-	if !jsonDiff.Modified() {
-		return nil, nil, ErrNilDiff
-	}
-
-	left := make(map[string]interface{})
-	err = json.Unmarshal(leftBytes, &left)
-	return left, jsonDiff, nil
-}

pkg/services/sqlstore/dashboard_version_test.go

@@ -98,59 +98,3 @@ func TestGetDashboardVersions(t *testing.T) {
 		})
 	})
 }
-
-func TestCompareDashboardVersions(t *testing.T) {
-	Convey("Testing dashboard version comparison", t, func() {
-		InitTestDB(t)
-
-		savedDash := insertTestDashboard("test dash 43", 1, "x")
-		updateTestDashboard(savedDash, map[string]interface{}{
-			"tags": "y",
-		})
-
-		Convey("Compare two versions that are different", func() {
-			query := m.GetDashboardVersionsQuery{DashboardId: savedDash.Id, OrgId: 1}
-
-			err := GetDashboardVersions(&query)
-			So(err, ShouldBeNil)
-			So(len(query.Result), ShouldEqual, 2)
-
-			cmd := m.CompareDashboardVersionsCommand{
-				DashboardId: savedDash.Id,
-				BaseVersion: query.Result[0].Version,
-				NewVersion:  query.Result[1].Version,
-				DiffType:    m.DiffDelta,
-			}
-
-			err = CompareDashboardVersionsCommand(&cmd)
-			So(err, ShouldBeNil)
-			So(cmd.Delta, ShouldNotBeNil)
-		})
-
-		Convey("Compare two versions that are the same", func() {
-			cmd := m.CompareDashboardVersionsCommand{
-				DashboardId: savedDash.Id,
-				BaseVersion: savedDash.Version,
-				NewVersion:  savedDash.Version,
-				DiffType:    m.DiffDelta,
-			}
-
-			err := CompareDashboardVersionsCommand(&cmd)
-			So(err, ShouldNotBeNil)
-			So(cmd.Delta, ShouldBeNil)
-		})
-
-		Convey("Compare two versions that don't exist", func() {
-			cmd := m.CompareDashboardVersionsCommand{
-				DashboardId: savedDash.Id,
-				BaseVersion: 123,
-				NewVersion:  456,
-				DiffType:    m.DiffDelta,
-			}
-
-			err := CompareDashboardVersionsCommand(&cmd)
-			So(err, ShouldNotBeNil)
-			So(cmd.Delta, ShouldBeNil)
-		})
-	})
-}