From e2061312f583628015c20c061914da055a3b7f7f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Torkel=20=C3=96degaard?= Date: Tue, 6 Jun 2017 00:14:40 +0200 Subject: [PATCH] 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 | 50 +++---- pkg/components/dashdiffs/compare.go | 124 ++++++++++++++++++ .../formatter_basic.go | 8 +- .../formatter_json.go | 2 +- pkg/models/dashboard_version.go | 23 ---- pkg/services/sqlstore/dashboard_version.go | 91 ------------- .../sqlstore/dashboard_version_test.go | 56 -------- 7 files changed, 156 insertions(+), 198 deletions(-) create mode 100644 pkg/components/dashdiffs/compare.go rename pkg/components/{formatter => dashdiffs}/formatter_basic.go (99%) rename pkg/components/{formatter => dashdiffs}/formatter_json.go (99%) diff --git a/pkg/api/dashboard.go b/pkg/api/dashboard.go index b0a38c63eb1..736461cabae 100644 --- a/pkg/api/dashboard.go +++ b/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. diff --git a/pkg/components/dashdiffs/compare.go b/pkg/components/dashdiffs/compare.go new file mode 100644 index 00000000000..a559c5b16f9 --- /dev/null +++ b/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 +} diff --git a/pkg/components/formatter/formatter_basic.go b/pkg/components/dashdiffs/formatter_basic.go similarity index 99% rename from pkg/components/formatter/formatter_basic.go rename to pkg/components/dashdiffs/formatter_basic.go index bf460f95f90..ea935604349 100644 --- a/pkg/components/formatter/formatter_basic.go +++ b/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 }} @@ -286,7 +286,7 @@ var (
  • {{ getChange .Change }} {{ .Key }}
    - +
    {{ if .Old }}
    {{ .Old }}
    @@ -312,7 +312,7 @@ var ( tplSummary = `{{ define "summary" -}}
    - + {{ if .Count }} {{ .Count }} {{ end }} diff --git a/pkg/components/formatter/formatter_json.go b/pkg/components/dashdiffs/formatter_json.go similarity index 99% rename from pkg/components/formatter/formatter_json.go rename to pkg/components/dashdiffs/formatter_json.go index 97d9aaf9e42..a2807b15992 100644 --- a/pkg/components/formatter/formatter_json.go +++ b/pkg/components/dashdiffs/formatter_json.go @@ -1,4 +1,4 @@ -package formatter +package dashdiffs import ( "bytes" diff --git a/pkg/models/dashboard_version.go b/pkg/models/dashboard_version.go index 949ad5f1f6a..06b5797e57c 100644 --- a/pkg/models/dashboard_version.go +++ b/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"` -} diff --git a/pkg/services/sqlstore/dashboard_version.go b/pkg/services/sqlstore/dashboard_version.go index bb7502bf0ad..4079cd9ddf7 100644 --- a/pkg/services/sqlstore/dashboard_version.go +++ b/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 -} diff --git a/pkg/services/sqlstore/dashboard_version_test.go b/pkg/services/sqlstore/dashboard_version_test.go index d36db78d9cd..eb7e549ba7c 100644 --- a/pkg/services/sqlstore/dashboard_version_test.go +++ b/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) - }) - }) -}