From 5ded070aab59807aa5f541b6113179d236d6c473 Mon Sep 17 00:00:00 2001 From: ying-jeanne <74549700+ying-jeanne@users.noreply.github.com> Date: Thu, 10 Feb 2022 16:58:52 +0800 Subject: [PATCH] dashdiff bus removal (#45175) --- pkg/api/api.go | 2 +- pkg/api/dashboard.go | 34 +++++++++- pkg/api/dashboard_test.go | 70 ++++++++++++-------- pkg/components/dashdiffs/compare.go | 27 +------- pkg/services/sqlstore/mockstore/mockstore.go | 9 ++- 5 files changed, 84 insertions(+), 58 deletions(-) diff --git a/pkg/api/api.go b/pkg/api/api.go index af0d8223747..dba7d43ad0e 100644 --- a/pkg/api/api.go +++ b/pkg/api/api.go @@ -340,7 +340,7 @@ func (hs *HTTPServer) registerRoutes() { dashboardRoute.Put("/uid/:uid/img/:kind/:theme", hs.ThumbService.UpdateThumbnailState) } - dashboardRoute.Post("/calculate-diff", routing.Wrap(CalculateDashboardDiff)) + dashboardRoute.Post("/calculate-diff", routing.Wrap(hs.CalculateDashboardDiff)) dashboardRoute.Post("/trim", routing.Wrap(hs.TrimDashboard)) dashboardRoute.Post("/db", routing.Wrap(hs.PostDashboard)) diff --git a/pkg/api/dashboard.go b/pkg/api/dashboard.go index acf3f9266e1..45a5b577ba8 100644 --- a/pkg/api/dashboard.go +++ b/pkg/api/dashboard.go @@ -555,7 +555,7 @@ func (hs *HTTPServer) GetDashboardVersion(c *models.ReqContext) response.Respons } // POST /api/dashboards/calculate-diff performs diffs on two dashboards -func CalculateDashboardDiff(c *models.ReqContext) response.Response { +func (hs *HTTPServer) CalculateDashboardDiff(c *models.ReqContext) response.Response { apiOptions := dtos.CalculateDiffOptions{} if err := web.Bind(c.Req, &apiOptions); err != nil { return response.Error(http.StatusBadRequest, "bad request data", err) @@ -587,7 +587,37 @@ func CalculateDashboardDiff(c *models.ReqContext) response.Response { }, } - result, err := dashdiffs.CalculateDiff(c.Req.Context(), &options) + baseVersionQuery := models.GetDashboardVersionQuery{ + DashboardId: options.Base.DashboardId, + Version: options.Base.Version, + OrgId: options.OrgId, + } + + if err := hs.SQLStore.GetDashboardVersion(c.Req.Context(), &baseVersionQuery); err != nil { + if errors.Is(err, models.ErrDashboardVersionNotFound) { + return response.Error(404, "Dashboard version not found", err) + } + return response.Error(500, "Unable to compute diff", err) + } + + newVersionQuery := models.GetDashboardVersionQuery{ + DashboardId: options.New.DashboardId, + Version: options.New.Version, + OrgId: options.OrgId, + } + + if err := hs.SQLStore.GetDashboardVersion(c.Req.Context(), &newVersionQuery); err != nil { + if errors.Is(err, models.ErrDashboardVersionNotFound) { + return response.Error(404, "Dashboard version not found", err) + } + return response.Error(500, "Unable to compute diff", err) + } + + baseData := baseVersionQuery.Result.Data + newData := newVersionQuery.Result.Data + + result, err := dashdiffs.CalculateDiff(c.Req.Context(), &options, baseData, newData) + if err != nil { if errors.Is(err, models.ErrDashboardVersionNotFound) { return response.Error(404, "Dashboard version not found", err) diff --git a/pkg/api/dashboard_test.go b/pkg/api/dashboard_test.go index 34126c6f908..3c4f21ec618 100644 --- a/pkg/api/dashboard_test.go +++ b/pkg/api/dashboard_test.go @@ -112,7 +112,6 @@ func TestDashboardAPIEndpoint(t *testing.T) { mockSQLStore := mockstore.NewSQLStoreMock() mockSQLStore.ExpectedDashboard = fakeDash - mockSQLStore.ExpectedDashboardVersion = &models.DashboardVersion{} hs := &HTTPServer{ Cfg: setting.NewCfg(), @@ -212,7 +211,6 @@ func TestDashboardAPIEndpoint(t *testing.T) { mockSQLStore := mockstore.NewSQLStoreMock() mockSQLStore.ExpectedDashboard = fakeDash - mockSQLStore.ExpectedDashboardVersion = &models.DashboardVersion{} hs := &HTTPServer{ Cfg: setting.NewCfg(), @@ -371,7 +369,6 @@ func TestDashboardAPIEndpoint(t *testing.T) { assert.Equal(t, 200, sc.resp.Code) }, mockSQLStore) - mockSQLStore.ExpectedDashboardVersion = &models.DashboardVersion{} loggedInUserScenarioWithRole(t, "When calling GET on", "GET", "/api/dashboards/id/2/versions/1", "/api/dashboards/id/:dashboardId/versions/:id", role, func(sc *scenarioContext) { setUpInner() sc.sqlStore = mockSQLStore @@ -718,21 +715,27 @@ func TestDashboardAPIEndpoint(t *testing.T) { }) t.Run("Given two dashboards being compared", func(t *testing.T) { + dashboardvs := []*models.DashboardVersion{ + { + DashboardId: 1, + Version: 1, + Data: simplejson.NewFromAny(map[string]interface{}{ + "title": "Dash1", + })}, + { + DashboardId: 2, + Version: 2, + Data: simplejson.NewFromAny(map[string]interface{}{ + "title": "Dash2", + })}, + } + sqlmock := mockstore.SQLStoreMock{ExpectedDashboardVersions: dashboardvs} setUp := func() { mockResult := []*models.DashboardAclInfoDTO{} bus.AddHandler("test", func(ctx context.Context, query *models.GetDashboardAclInfoListQuery) error { query.Result = mockResult return nil }) - - bus.AddHandler("test", func(ctx context.Context, query *models.GetDashboardVersionQuery) error { - query.Result = &models.DashboardVersion{ - Data: simplejson.NewFromAny(map[string]interface{}{ - "title": fmt.Sprintf("Dash%d", query.DashboardId), - }), - } - return nil - }) } cmd := dtos.CalculateDiffOptions{ @@ -749,13 +752,12 @@ func TestDashboardAPIEndpoint(t *testing.T) { t.Run("when user does not have permission", func(t *testing.T) { role := models.ROLE_VIEWER - postDiffScenario(t, "When calling POST on", "/api/dashboards/calculate-diff", "/api/dashboards/calculate-diff", cmd, role, func(sc *scenarioContext) { setUp() callPostDashboard(sc) assert.Equal(t, 403, sc.resp.Code) - }) + }, &sqlmock) }) t.Run("when user does have permission", func(t *testing.T) { @@ -766,7 +768,7 @@ func TestDashboardAPIEndpoint(t *testing.T) { callPostDashboard(sc) assert.Equal(t, 200, sc.resp.Code) - }) + }, &sqlmock) }) }) @@ -792,11 +794,12 @@ func TestDashboardAPIEndpoint(t *testing.T) { } mockSQLStore := mockstore.NewSQLStoreMock() mockSQLStore.ExpectedDashboard = fakeDash - mockSQLStore.ExpectedDashboardVersion = &models.DashboardVersion{ - DashboardId: 2, - Version: 1, - Data: fakeDash.Data, - } + mockSQLStore.ExpectedDashboardVersions = []*models.DashboardVersion{ + { + DashboardId: 2, + Version: 1, + Data: fakeDash.Data, + }} restoreDashboardVersionScenario(t, "When calling POST on", "/api/dashboards/id/1/restore", "/api/dashboards/id/:dashboardId/restore", mock, cmd, func(sc *scenarioContext) { callRestoreDashboardVersion(sc) @@ -829,11 +832,12 @@ func TestDashboardAPIEndpoint(t *testing.T) { } mockSQLStore := mockstore.NewSQLStoreMock() mockSQLStore.ExpectedDashboard = fakeDash - mockSQLStore.ExpectedDashboardVersion = &models.DashboardVersion{ - DashboardId: 2, - Version: 1, - Data: fakeDash.Data, - } + mockSQLStore.ExpectedDashboardVersions = []*models.DashboardVersion{ + { + DashboardId: 2, + Version: 1, + Data: fakeDash.Data, + }} restoreDashboardVersionScenario(t, "When calling POST on", "/api/dashboards/id/1/restore", "/api/dashboards/id/:dashboardId/restore", mock, cmd, func(sc *scenarioContext) { callRestoreDashboardVersion(sc) @@ -1051,10 +1055,22 @@ func postDashboardScenario(t *testing.T, desc string, url string, routePattern s }) } -func postDiffScenario(t *testing.T, desc string, url string, routePattern string, cmd dtos.CalculateDiffOptions, role models.RoleType, fn scenarioFunc) { +func postDiffScenario(t *testing.T, desc string, url string, routePattern string, cmd dtos.CalculateDiffOptions, role models.RoleType, fn scenarioFunc, sqlmock sqlstore.Store) { t.Run(fmt.Sprintf("%s %s", desc, url), func(t *testing.T) { defer bus.ClearBusHandlers() + cfg := setting.NewCfg() + hs := HTTPServer{ + Cfg: cfg, + Bus: bus.GetBus(), + ProvisioningService: provisioning.NewProvisioningServiceMock(context.Background()), + Live: newTestLive(t), + QuotaService: "a.QuotaService{Cfg: cfg}, + LibraryPanelService: &mockLibraryPanelService{}, + LibraryElementService: &mockLibraryElementService{}, + SQLStore: sqlmock, + } + sc := setupScenarioContext(t, url) sc.defaultHandler = routing.Wrap(func(c *models.ReqContext) response.Response { c.Req.Body = mockRequestBody(cmd) @@ -1066,7 +1082,7 @@ func postDiffScenario(t *testing.T, desc string, url string, routePattern string } sc.context.OrgRole = role - return CalculateDashboardDiff(c) + return hs.CalculateDashboardDiff(c) }) sc.m.Post(routePattern, sc.defaultHandler) diff --git a/pkg/components/dashdiffs/compare.go b/pkg/components/dashdiffs/compare.go index 0658c88ac44..174c497ea41 100644 --- a/pkg/components/dashdiffs/compare.go +++ b/pkg/components/dashdiffs/compare.go @@ -5,9 +5,7 @@ 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" ) @@ -58,30 +56,7 @@ func ParseDiffType(diff string) DiffType { // CompareDashboardVersionsCommand computes the JSON diff of two versions, // assigning the delta of the diff to the `Delta` field. -func CalculateDiff(ctx context.Context, options *Options) (*Result, error) { - baseVersionQuery := models.GetDashboardVersionQuery{ - DashboardId: options.Base.DashboardId, - Version: options.Base.Version, - OrgId: options.OrgId, - } - - if err := bus.Dispatch(ctx, &baseVersionQuery); err != nil { - return nil, err - } - - newVersionQuery := models.GetDashboardVersionQuery{ - DashboardId: options.New.DashboardId, - Version: options.New.Version, - OrgId: options.OrgId, - } - - if err := bus.Dispatch(ctx, &newVersionQuery); err != nil { - return nil, err - } - - baseData := baseVersionQuery.Result.Data - newData := newVersionQuery.Result.Data - +func CalculateDiff(ctx context.Context, options *Options, baseData, newData *simplejson.Json) (*Result, error) { left, jsonDiff, err := getDiff(baseData, newData) if err != nil { return nil, err diff --git a/pkg/services/sqlstore/mockstore/mockstore.go b/pkg/services/sqlstore/mockstore/mockstore.go index f0f46832689..876674594b9 100644 --- a/pkg/services/sqlstore/mockstore/mockstore.go +++ b/pkg/services/sqlstore/mockstore/mockstore.go @@ -21,7 +21,7 @@ type SQLStoreMock struct { ExpectedPluginSetting *models.PluginSetting ExpectedDashboard *models.Dashboard ExpectedDashboards []*models.Dashboard - ExpectedDashboardVersion *models.DashboardVersion + ExpectedDashboardVersions []*models.DashboardVersion ExpectedDashboardAclInfoList []*models.DashboardAclInfoDTO ExpectedUserOrgList []*models.UserOrgDTO ExpectedOrgListResponse OrgListResponse @@ -339,7 +339,12 @@ func (m *SQLStoreMock) InTransaction(ctx context.Context, fn func(ctx context.Co } func (m *SQLStoreMock) GetDashboardVersion(ctx context.Context, query *models.GetDashboardVersionQuery) error { - query.Result = m.ExpectedDashboardVersion + query.Result = &models.DashboardVersion{} + for _, dashboardversion := range m.ExpectedDashboardVersions { + if dashboardversion.DashboardId == query.DashboardId && dashboardversion.Version == query.Version { + query.Result = dashboardversion + } + } return m.ExpectedError }