From b40e78a94325658bbbf51de690c02b0bb0082585 Mon Sep 17 00:00:00 2001 From: Carl Bergquist Date: Tue, 15 Jun 2021 16:08:27 +0200 Subject: [PATCH] Instrumentation: add context.Context to the dashboard get flow. (#34955) Signed-off-by: bergquist --- pkg/api/dashboard.go | 15 ++++++------ pkg/api/dashboard_permission.go | 4 ++-- pkg/api/dashboard_permission_test.go | 11 +++++---- pkg/api/dashboard_test.go | 11 +++++---- pkg/services/sqlstore/dashboard.go | 35 +++++++++++++++++----------- pkg/services/sqlstore/star.go | 30 +++++++++++++----------- pkg/services/sqlstore/stars_test.go | 5 ++-- 7 files changed, 63 insertions(+), 48 deletions(-) diff --git a/pkg/api/dashboard.go b/pkg/api/dashboard.go index 95855433c83..91adc0b1eb9 100644 --- a/pkg/api/dashboard.go +++ b/pkg/api/dashboard.go @@ -1,6 +1,7 @@ package api import ( + "context" "encoding/json" "errors" "fmt" @@ -31,7 +32,7 @@ func isDashboardStarredByUser(c *models.ReqContext, dashID int64) (bool, error) } query := models.IsStarredByUserQuery{UserId: c.UserId, DashboardId: dashID} - if err := bus.Dispatch(&query); err != nil { + if err := bus.DispatchCtx(c.Req.Context(), &query); err != nil { return false, err } @@ -70,7 +71,7 @@ func (hs *HTTPServer) TrimDashboard(c *models.ReqContext, cmd models.TrimDashboa func (hs *HTTPServer) GetDashboard(c *models.ReqContext) response.Response { uid := c.Params(":uid") - dash, rsp := getDashboardHelper(c.OrgId, 0, uid) + dash, rsp := getDashboardHelper(c.Req.Context(), c.OrgId, 0, uid) if rsp != nil { return rsp } @@ -135,7 +136,7 @@ func (hs *HTTPServer) GetDashboard(c *models.ReqContext) response.Response { // lookup folder title if dash.FolderId > 0 { query := models.GetDashboardQuery{Id: dash.FolderId, OrgId: c.OrgId} - if err := bus.Dispatch(&query); err != nil { + if err := bus.DispatchCtx(c.Req.Context(), &query); err != nil { if errors.Is(err, models.ErrFolderNotFound) { return response.Error(404, "Folder not found", err) } @@ -196,7 +197,7 @@ func getUserLogin(userID int64) string { return query.Result.Login } -func getDashboardHelper(orgID int64, id int64, uid string) (*models.Dashboard, response.Response) { +func getDashboardHelper(ctx context.Context, orgID int64, id int64, uid string) (*models.Dashboard, response.Response) { var query models.GetDashboardQuery if len(uid) > 0 { @@ -205,7 +206,7 @@ func getDashboardHelper(orgID int64, id int64, uid string) (*models.Dashboard, r query = models.GetDashboardQuery{Id: id, OrgId: orgID} } - if err := bus.Dispatch(&query); err != nil { + if err := bus.DispatchCtx(ctx, &query); err != nil { return nil, response.Error(404, "Dashboard not found", err) } @@ -231,7 +232,7 @@ func (hs *HTTPServer) DeleteDashboardByUID(c *models.ReqContext) response.Respon } func (hs *HTTPServer) deleteDashboard(c *models.ReqContext) response.Response { - dash, rsp := getDashboardHelper(c.OrgId, 0, c.Params(":uid")) + dash, rsp := getDashboardHelper(c.Req.Context(), c.OrgId, 0, c.Params(":uid")) if rsp != nil { return rsp } @@ -625,7 +626,7 @@ func CalculateDashboardDiff(c *models.ReqContext, apiOptions dtos.CalculateDiffO // RestoreDashboardVersion restores a dashboard to the given version. func (hs *HTTPServer) RestoreDashboardVersion(c *models.ReqContext, apiCmd dtos.RestoreDashboardVersionCommand) response.Response { - dash, rsp := getDashboardHelper(c.OrgId, c.ParamsInt64(":dashboardId"), "") + dash, rsp := getDashboardHelper(c.Req.Context(), c.OrgId, c.ParamsInt64(":dashboardId"), "") if rsp != nil { return rsp } diff --git a/pkg/api/dashboard_permission.go b/pkg/api/dashboard_permission.go index cf6e3379b45..4867d68e41c 100644 --- a/pkg/api/dashboard_permission.go +++ b/pkg/api/dashboard_permission.go @@ -13,7 +13,7 @@ import ( func (hs *HTTPServer) GetDashboardPermissionList(c *models.ReqContext) response.Response { dashID := c.ParamsInt64(":dashboardId") - _, rsp := getDashboardHelper(c.OrgId, dashID, "") + _, rsp := getDashboardHelper(c.Req.Context(), c.OrgId, dashID, "") if rsp != nil { return rsp } @@ -57,7 +57,7 @@ func (hs *HTTPServer) UpdateDashboardPermissions(c *models.ReqContext, apiCmd dt dashID := c.ParamsInt64(":dashboardId") - _, rsp := getDashboardHelper(c.OrgId, dashID, "") + _, rsp := getDashboardHelper(c.Req.Context(), c.OrgId, dashID, "") if rsp != nil { return rsp } diff --git a/pkg/api/dashboard_permission_test.go b/pkg/api/dashboard_permission_test.go index 8e0adf8848d..93e148a6148 100644 --- a/pkg/api/dashboard_permission_test.go +++ b/pkg/api/dashboard_permission_test.go @@ -1,6 +1,7 @@ package api import ( + "context" "encoding/json" "fmt" "testing" @@ -65,7 +66,7 @@ func TestDashboardPermissionAPIEndpoint(t *testing.T) { getDashboardQueryResult := models.NewDashboard("Dash") setUp := func() { - bus.AddHandler("test", func(query *models.GetDashboardQuery) error { + bus.AddHandlerCtx("test", func(ctx context.Context, query *models.GetDashboardQuery) error { query.Result = getDashboardQueryResult return nil }) @@ -117,7 +118,7 @@ func TestDashboardPermissionAPIEndpoint(t *testing.T) { setUp := func() { getDashboardQueryResult := models.NewDashboard("Dash") - bus.AddHandler("test", func(query *models.GetDashboardQuery) error { + bus.AddHandlerCtx("test", func(ctx context.Context, query *models.GetDashboardQuery) error { query.Result = getDashboardQueryResult return nil }) @@ -170,7 +171,7 @@ func TestDashboardPermissionAPIEndpoint(t *testing.T) { setUp := func() { getDashboardQueryResult := models.NewDashboard("Dash") - bus.AddHandler("test", func(query *models.GetDashboardQuery) error { + bus.AddHandlerCtx("test", func(ctx context.Context, query *models.GetDashboardQuery) error { query.Result = getDashboardQueryResult return nil }) @@ -241,7 +242,7 @@ func TestDashboardPermissionAPIEndpoint(t *testing.T) { setUp := func() { getDashboardQueryResult := models.NewDashboard("Dash") - bus.AddHandler("test", func(query *models.GetDashboardQuery) error { + bus.AddHandlerCtx("test", func(ctx context.Context, query *models.GetDashboardQuery) error { query.Result = getDashboardQueryResult return nil }) @@ -292,7 +293,7 @@ func TestDashboardPermissionAPIEndpoint(t *testing.T) { setUp := func() { getDashboardQueryResult := models.NewDashboard("Dash") - bus.AddHandler("test", func(query *models.GetDashboardQuery) error { + bus.AddHandlerCtx("test", func(ctx context.Context, query *models.GetDashboardQuery) error { query.Result = getDashboardQueryResult return nil }) diff --git a/pkg/api/dashboard_test.go b/pkg/api/dashboard_test.go index 1b3ee466d41..f07d37ade1c 100644 --- a/pkg/api/dashboard_test.go +++ b/pkg/api/dashboard_test.go @@ -1,6 +1,7 @@ package api import ( + "context" "encoding/json" "errors" "fmt" @@ -110,7 +111,7 @@ func TestDashboardAPIEndpoint(t *testing.T) { state := &testState{} - bus.AddHandler("test", func(query *models.GetDashboardQuery) error { + bus.AddHandlerCtx("test", func(ctx context.Context, query *models.GetDashboardQuery) error { query.Result = fakeDash state.dashQueries = append(state.dashQueries, query) return nil @@ -275,7 +276,7 @@ func TestDashboardAPIEndpoint(t *testing.T) { return nil }) - bus.AddHandler("test", func(query *models.GetDashboardQuery) error { + bus.AddHandlerCtx("test", func(ctx context.Context, query *models.GetDashboardQuery) error { query.Result = fakeDash state.dashQueries = append(state.dashQueries, query) return nil @@ -830,7 +831,7 @@ func TestDashboardAPIEndpoint(t *testing.T) { fakeDash.FolderId = folderID fakeDash.HasAcl = false - bus.AddHandler("test", func(query *models.GetDashboardQuery) error { + bus.AddHandlerCtx("test", func(ctx context.Context, query *models.GetDashboardQuery) error { query.Result = fakeDash return nil }) @@ -878,7 +879,7 @@ func TestDashboardAPIEndpoint(t *testing.T) { fakeDash.Id = 2 fakeDash.HasAcl = false - bus.AddHandler("test", func(query *models.GetDashboardQuery) error { + bus.AddHandlerCtx("test", func(ctx context.Context, query *models.GetDashboardQuery) error { query.Result = fakeDash return nil }) @@ -926,7 +927,7 @@ func TestDashboardAPIEndpoint(t *testing.T) { query.Result = []*models.Dashboard{{}} return nil }) - bus.AddHandler("test", func(query *models.GetDashboardQuery) error { + bus.AddHandlerCtx("test", func(ctx context.Context, query *models.GetDashboardQuery) error { dataValue, err := simplejson.NewJson([]byte(`{"id": 1, "editable": true, "style": "dark"}`)) require.NoError(t, err) query.Result = &models.Dashboard{Id: 1, Data: dataValue} diff --git a/pkg/services/sqlstore/dashboard.go b/pkg/services/sqlstore/dashboard.go index 2e9066cc898..bfe93bd32fb 100644 --- a/pkg/services/sqlstore/dashboard.go +++ b/pkg/services/sqlstore/dashboard.go @@ -28,6 +28,7 @@ var shadowSearchCounter = prometheus.NewCounterVec( func init() { bus.AddHandler("sql", GetDashboard) bus.AddHandler("sql", GetDashboards) + bus.AddHandlerCtx("sql", GetDashboardCtx) bus.AddHandler("sql", DeleteDashboard) bus.AddHandler("sql", SearchDashboards) bus.AddHandler("sql", GetDashboardTags) @@ -230,23 +231,29 @@ func (ss *SQLStore) GetFolderByTitle(orgID int64, title string) (*models.Dashboa // TODO: Remove me func GetDashboard(query *models.GetDashboardQuery) error { - if query.Id == 0 && len(query.Slug) == 0 && len(query.Uid) == 0 { - return models.ErrDashboardIdentifierNotSet - } + return GetDashboardCtx(context.Background(), query) +} - dashboard := models.Dashboard{Slug: query.Slug, OrgId: query.OrgId, Id: query.Id, Uid: query.Uid} - has, err := x.Get(&dashboard) +func GetDashboardCtx(ctx context.Context, query *models.GetDashboardQuery) error { + return withDbSession(ctx, x, func(dbSession *DBSession) error { + if query.Id == 0 && len(query.Slug) == 0 && len(query.Uid) == 0 { + return models.ErrDashboardIdentifierNotSet + } - if err != nil { - return err - } else if !has { - return models.ErrDashboardNotFound - } + dashboard := models.Dashboard{Slug: query.Slug, OrgId: query.OrgId, Id: query.Id, Uid: query.Uid} + has, err := dbSession.Get(&dashboard) - dashboard.SetId(dashboard.Id) - dashboard.SetUid(dashboard.Uid) - query.Result = &dashboard - return nil + if err != nil { + return err + } else if !has { + return models.ErrDashboardNotFound + } + + dashboard.SetId(dashboard.Id) + dashboard.SetUid(dashboard.Uid) + query.Result = &dashboard + return nil + }) } type DashboardSearchProjection struct { diff --git a/pkg/services/sqlstore/star.go b/pkg/services/sqlstore/star.go index b16a8ebc120..cd2cde78eb7 100644 --- a/pkg/services/sqlstore/star.go +++ b/pkg/services/sqlstore/star.go @@ -1,6 +1,8 @@ package sqlstore import ( + "context" + "github.com/grafana/grafana/pkg/bus" "github.com/grafana/grafana/pkg/models" ) @@ -9,24 +11,26 @@ func init() { bus.AddHandler("sql", StarDashboard) bus.AddHandler("sql", UnstarDashboard) bus.AddHandler("sql", GetUserStars) - bus.AddHandler("sql", IsStarredByUser) + bus.AddHandlerCtx("sql", IsStarredByUserCtx) } -func IsStarredByUser(query *models.IsStarredByUserQuery) error { - rawSQL := "SELECT 1 from star where user_id=? and dashboard_id=?" - results, err := x.Query(rawSQL, query.UserId, query.DashboardId) +func IsStarredByUserCtx(ctx context.Context, query *models.IsStarredByUserQuery) error { + return withDbSession(ctx, x, func(dbSession *DBSession) error { + rawSQL := "SELECT 1 from star where user_id=? and dashboard_id=?" + results, err := dbSession.Query(rawSQL, query.UserId, query.DashboardId) - if err != nil { - return err - } + if err != nil { + return err + } + + if len(results) == 0 { + return nil + } + + query.Result = true - if len(results) == 0 { return nil - } - - query.Result = true - - return nil + }) } func StarDashboard(cmd *models.StarDashboardCommand) error { diff --git a/pkg/services/sqlstore/stars_test.go b/pkg/services/sqlstore/stars_test.go index 9f6b0516247..d4bbd3aeb0c 100644 --- a/pkg/services/sqlstore/stars_test.go +++ b/pkg/services/sqlstore/stars_test.go @@ -3,6 +3,7 @@ package sqlstore import ( + "context" "testing" "github.com/grafana/grafana/pkg/models" @@ -24,7 +25,7 @@ func TestUserStarsDataAccess(t *testing.T) { Convey("IsStarredByUser should return true when starred", func() { query := models.IsStarredByUserQuery{UserId: 12, DashboardId: 10} - err := IsStarredByUser(&query) + err := IsStarredByUserCtx(context.Background(), &query) So(err, ShouldBeNil) So(query.Result, ShouldBeTrue) @@ -32,7 +33,7 @@ func TestUserStarsDataAccess(t *testing.T) { Convey("IsStarredByUser should return false when not starred", func() { query := models.IsStarredByUserQuery{UserId: 12, DashboardId: 12} - err := IsStarredByUser(&query) + err := IsStarredByUserCtx(context.Background(), &query) So(err, ShouldBeNil) So(query.Result, ShouldBeFalse)