From 06e6bcf091bf547595140fe6ca0a9092325bfa1b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Hugo=20H=C3=A4ggmark?= Date: Tue, 16 Feb 2021 13:00:56 +0100 Subject: [PATCH] LibraryPanels: Disconnect before connect during dashboard save (#31235) * LibraryPanels: Disconnect before connect during dashboard save * Tests: fixed test * Chore: updates after PR comments * Chore: changes from context.Background() to c.Context.Req.Context() * Chore: fixes lint issue --- pkg/api/dashboard.go | 4 +- pkg/services/librarypanels/database.go | 31 +++--- pkg/services/librarypanels/librarypanels.go | 8 +- .../librarypanels/librarypanels_test.go | 94 +++++++++++++++++-- 4 files changed, 111 insertions(+), 26 deletions(-) diff --git a/pkg/api/dashboard.go b/pkg/api/dashboard.go index c7ef7ca995e..d72e4cb3853 100644 --- a/pkg/api/dashboard.go +++ b/pkg/api/dashboard.go @@ -149,7 +149,7 @@ func (hs *HTTPServer) GetDashboard(c *models.ReqContext) response.Response { if hs.Cfg.IsPanelLibraryEnabled() { // load library panels JSON for this dashboard - err = hs.LibraryPanelService.LoadLibraryPanelsForDashboard(dash) + err = hs.LibraryPanelService.LoadLibraryPanelsForDashboard(c, dash) if err != nil { return response.Error(500, "Error while loading library panels", err) } @@ -220,7 +220,7 @@ func (hs *HTTPServer) deleteDashboard(c *models.ReqContext) response.Response { if hs.Cfg.IsPanelLibraryEnabled() { // disconnect all library panels for this dashboard - err := hs.LibraryPanelService.DisconnectLibraryPanelsForDashboard(dash) + err := hs.LibraryPanelService.DisconnectLibraryPanelsForDashboard(c, dash) if err != nil { hs.log.Error("Failed to disconnect library panels", "dashboard", dash.Id, "user", c.SignedInUser.UserId, "error", err) } diff --git a/pkg/services/librarypanels/database.go b/pkg/services/librarypanels/database.go index 9a94abc32a3..1e0af02e423 100644 --- a/pkg/services/librarypanels/database.go +++ b/pkg/services/librarypanels/database.go @@ -1,7 +1,6 @@ package librarypanels import ( - "context" "fmt" "time" @@ -42,7 +41,7 @@ func (lps *LibraryPanelService) createLibraryPanel(c *models.ReqContext, cmd cre CreatedBy: c.SignedInUser.UserId, UpdatedBy: c.SignedInUser.UserId, } - err := lps.SQLStore.WithTransactionalDbSession(context.Background(), func(session *sqlstore.DBSession) error { + err := lps.SQLStore.WithTransactionalDbSession(c.Context.Req.Context(), func(session *sqlstore.DBSession) error { if _, err := session.Insert(&libraryPanel); err != nil { if lps.SQLStore.Dialect.IsUniqueConstraintViolation(err) { return errLibraryPanelAlreadyExists @@ -105,7 +104,7 @@ func connectDashboard(session *sqlstore.DBSession, dialect migrator.Dialect, use // connectDashboard adds a connection between a Library Panel and a Dashboard. func (lps *LibraryPanelService) connectDashboard(c *models.ReqContext, uid string, dashboardID int64) error { - err := lps.SQLStore.WithTransactionalDbSession(context.Background(), func(session *sqlstore.DBSession) error { + err := lps.SQLStore.WithTransactionalDbSession(c.Context.Req.Context(), func(session *sqlstore.DBSession) error { return connectDashboard(session, lps.SQLStore.Dialect, c.SignedInUser, uid, dashboardID) }) @@ -114,7 +113,11 @@ func (lps *LibraryPanelService) connectDashboard(c *models.ReqContext, uid strin // connectLibraryPanelsForDashboard adds connections for all Library Panels in a Dashboard. func (lps *LibraryPanelService) connectLibraryPanelsForDashboard(c *models.ReqContext, uids []string, dashboardID int64) error { - err := lps.SQLStore.WithTransactionalDbSession(context.Background(), func(session *sqlstore.DBSession) error { + err := lps.SQLStore.WithTransactionalDbSession(c.Context.Req.Context(), func(session *sqlstore.DBSession) error { + _, err := session.Exec("DELETE FROM library_panel_dashboard WHERE dashboard_id=?", dashboardID) + if err != nil { + return err + } for _, uid := range uids { err := connectDashboard(session, lps.SQLStore.Dialect, c.SignedInUser, uid, dashboardID) if err != nil { @@ -129,7 +132,7 @@ func (lps *LibraryPanelService) connectLibraryPanelsForDashboard(c *models.ReqCo // deleteLibraryPanel deletes a Library Panel. func (lps *LibraryPanelService) deleteLibraryPanel(c *models.ReqContext, uid string) error { - return lps.SQLStore.WithTransactionalDbSession(context.Background(), func(session *sqlstore.DBSession) error { + return lps.SQLStore.WithTransactionalDbSession(c.Context.Req.Context(), func(session *sqlstore.DBSession) error { panel, err := getLibraryPanel(session, uid, c.SignedInUser.OrgId) if err != nil { return err @@ -155,7 +158,7 @@ func (lps *LibraryPanelService) deleteLibraryPanel(c *models.ReqContext, uid str // disconnectDashboard deletes a connection between a Library Panel and a Dashboard. func (lps *LibraryPanelService) disconnectDashboard(c *models.ReqContext, uid string, dashboardID int64) error { - return lps.SQLStore.WithTransactionalDbSession(context.Background(), func(session *sqlstore.DBSession) error { + return lps.SQLStore.WithTransactionalDbSession(c.Context.Req.Context(), func(session *sqlstore.DBSession) error { panel, err := getLibraryPanel(session, uid, c.SignedInUser.OrgId) if err != nil { return err @@ -177,8 +180,8 @@ func (lps *LibraryPanelService) disconnectDashboard(c *models.ReqContext, uid st } // disconnectLibraryPanelsForDashboard deletes connections for all Library Panels in a Dashboard. -func (lps *LibraryPanelService) disconnectLibraryPanelsForDashboard(dashboardID int64, panelCount int64) error { - return lps.SQLStore.WithTransactionalDbSession(context.Background(), func(session *sqlstore.DBSession) error { +func (lps *LibraryPanelService) disconnectLibraryPanelsForDashboard(c *models.ReqContext, dashboardID int64, panelCount int64) error { + return lps.SQLStore.WithTransactionalDbSession(c.Context.Req.Context(), func(session *sqlstore.DBSession) error { result, err := session.Exec("DELETE FROM library_panel_dashboard WHERE dashboard_id=?", dashboardID) if err != nil { return err @@ -214,7 +217,7 @@ func getLibraryPanel(session *sqlstore.DBSession, uid string, orgID int64) (Libr // getLibraryPanel gets a Library Panel. func (lps *LibraryPanelService) getLibraryPanel(c *models.ReqContext, uid string) (LibraryPanelDTO, error) { var libraryPanel LibraryPanelWithMeta - err := lps.SQLStore.WithDbSession(context.Background(), func(session *sqlstore.DBSession) error { + err := lps.SQLStore.WithDbSession(c.Context.Req.Context(), func(session *sqlstore.DBSession) error { var err error libraryPanel, err = getLibraryPanel(session, uid, c.SignedInUser.OrgId) return err @@ -252,7 +255,7 @@ func (lps *LibraryPanelService) getLibraryPanel(c *models.ReqContext, uid string func (lps *LibraryPanelService) getAllLibraryPanels(c *models.ReqContext) ([]LibraryPanelDTO, error) { orgID := c.SignedInUser.OrgId libraryPanels := make([]LibraryPanelWithMeta, 0) - err := lps.SQLStore.WithDbSession(context.Background(), func(session *sqlstore.DBSession) error { + err := lps.SQLStore.WithDbSession(c.Context.Req.Context(), func(session *sqlstore.DBSession) error { sql := sqlStatmentLibrayPanelDTOWithMeta + "WHERE lp.org_id=?" sess := session.SQL(sql, orgID) err := sess.Find(&libraryPanels) @@ -297,7 +300,7 @@ func (lps *LibraryPanelService) getAllLibraryPanels(c *models.ReqContext) ([]Lib // getConnectedDashboards gets all dashboards connected to a Library Panel. func (lps *LibraryPanelService) getConnectedDashboards(c *models.ReqContext, uid string) ([]int64, error) { connectedDashboardIDs := make([]int64, 0) - err := lps.SQLStore.WithDbSession(context.Background(), func(session *sqlstore.DBSession) error { + err := lps.SQLStore.WithDbSession(c.Context.Req.Context(), func(session *sqlstore.DBSession) error { panel, err := getLibraryPanel(session, uid, c.SignedInUser.OrgId) if err != nil { return err @@ -321,9 +324,9 @@ func (lps *LibraryPanelService) getConnectedDashboards(c *models.ReqContext, uid return connectedDashboardIDs, err } -func (lps *LibraryPanelService) getLibraryPanelsForDashboardID(dashboardID int64) (map[string]LibraryPanelDTO, error) { +func (lps *LibraryPanelService) getLibraryPanelsForDashboardID(c *models.ReqContext, dashboardID int64) (map[string]LibraryPanelDTO, error) { libraryPanelMap := make(map[string]LibraryPanelDTO) - err := lps.SQLStore.WithDbSession(context.Background(), func(session *sqlstore.DBSession) error { + err := lps.SQLStore.WithDbSession(c.Context.Req.Context(), func(session *sqlstore.DBSession) error { var libraryPanels []LibraryPanelWithMeta sql := sqlStatmentLibrayPanelDTOWithMeta + "INNER JOIN library_panel_dashboard AS lpd ON lpd.librarypanel_id = lp.id AND lpd.dashboard_id=?" sess := session.SQL(sql, dashboardID) @@ -368,7 +371,7 @@ func (lps *LibraryPanelService) getLibraryPanelsForDashboardID(dashboardID int64 // patchLibraryPanel updates a Library Panel. func (lps *LibraryPanelService) patchLibraryPanel(c *models.ReqContext, cmd patchLibraryPanelCommand, uid string) (LibraryPanelDTO, error) { var dto LibraryPanelDTO - err := lps.SQLStore.WithTransactionalDbSession(context.Background(), func(session *sqlstore.DBSession) error { + err := lps.SQLStore.WithTransactionalDbSession(c.Context.Req.Context(), func(session *sqlstore.DBSession) error { panelInDB, err := getLibraryPanel(session, uid, c.SignedInUser.OrgId) if err != nil { return err diff --git a/pkg/services/librarypanels/librarypanels.go b/pkg/services/librarypanels/librarypanels.go index 02b89206345..62bcf6e7655 100644 --- a/pkg/services/librarypanels/librarypanels.go +++ b/pkg/services/librarypanels/librarypanels.go @@ -45,12 +45,12 @@ func (lps *LibraryPanelService) IsEnabled() bool { // LoadLibraryPanelsForDashboard loops through all panels in dashboard JSON and replaces any library panel JSON // with JSON stored for library panel in db. -func (lps *LibraryPanelService) LoadLibraryPanelsForDashboard(dash *models.Dashboard) error { +func (lps *LibraryPanelService) LoadLibraryPanelsForDashboard(c *models.ReqContext, dash *models.Dashboard) error { if !lps.IsEnabled() { return nil } - libraryPanels, err := lps.getLibraryPanelsForDashboardID(dash.Id) + libraryPanels, err := lps.getLibraryPanelsForDashboardID(c, dash.Id) if err != nil { return err } @@ -194,7 +194,7 @@ func (lps *LibraryPanelService) ConnectLibraryPanelsForDashboard(c *models.ReqCo } // DisconnectLibraryPanelsForDashboard loops through all panels in dashboard JSON and disconnects any library panels from the dashboard. -func (lps *LibraryPanelService) DisconnectLibraryPanelsForDashboard(dash *models.Dashboard) error { +func (lps *LibraryPanelService) DisconnectLibraryPanelsForDashboard(c *models.ReqContext, dash *models.Dashboard) error { if !lps.IsEnabled() { return nil } @@ -216,7 +216,7 @@ func (lps *LibraryPanelService) DisconnectLibraryPanelsForDashboard(dash *models panelCount++ } - return lps.disconnectLibraryPanelsForDashboard(dash.Id, panelCount) + return lps.disconnectLibraryPanelsForDashboard(c, dash.Id, panelCount) } // AddMigration defines database migrations. diff --git a/pkg/services/librarypanels/librarypanels_test.go b/pkg/services/librarypanels/librarypanels_test.go index e49f9526b7b..ff4201c9727 100644 --- a/pkg/services/librarypanels/librarypanels_test.go +++ b/pkg/services/librarypanels/librarypanels_test.go @@ -4,6 +4,7 @@ import ( "context" "encoding/json" "fmt" + "net/http" "testing" "time" @@ -801,7 +802,7 @@ func TestLoadLibraryPanelsForDashboard(t *testing.T) { Data: simplejson.NewFromAny(dashJSON), } - err = sc.service.LoadLibraryPanelsForDashboard(&dash) + err = sc.service.LoadLibraryPanelsForDashboard(sc.reqContext, &dash) require.NoError(t, err) expectedJSON := map[string]interface{}{ "panels": []interface{}{ @@ -898,7 +899,7 @@ func TestLoadLibraryPanelsForDashboard(t *testing.T) { Data: simplejson.NewFromAny(dashJSON), } - err = sc.service.LoadLibraryPanelsForDashboard(&dash) + err = sc.service.LoadLibraryPanelsForDashboard(sc.reqContext, &dash) require.EqualError(t, err, errLibraryPanelHeaderUIDMissing.Error()) }) @@ -943,7 +944,7 @@ func TestLoadLibraryPanelsForDashboard(t *testing.T) { Data: simplejson.NewFromAny(dashJSON), } - err = sc.service.LoadLibraryPanelsForDashboard(&dash) + err = sc.service.LoadLibraryPanelsForDashboard(sc.reqContext, &dash) require.NoError(t, err) expectedJSON := map[string]interface{}{ "panels": []interface{}{ @@ -1258,6 +1259,85 @@ func TestConnectLibraryPanelsForDashboard(t *testing.T) { err = sc.service.ConnectLibraryPanelsForDashboard(sc.reqContext, &dash) require.EqualError(t, err, errLibraryPanelHeaderUIDMissing.Error()) }) + + testScenario(t, "When an admin tries to store a dashboard with unusused/removed library panels, it should disconnect unusused/removed library panels", + func(t *testing.T, sc scenarioContext) { + command := getCreateCommand(1, "Unused Libray Panel") + response := sc.service.createHandler(sc.reqContext, command) + require.Equal(t, 200, response.Status()) + + var unused libraryPanelResult + err := json.Unmarshal(response.Body(), &unused) + require.NoError(t, err) + + sc.reqContext.ReplaceAllParams(map[string]string{":uid": unused.Result.UID, ":dashboardId": "1"}) + response = sc.service.connectHandler(sc.reqContext) + require.Equal(t, 200, response.Status()) + + command = getCreateCommand(1, "Text - Library Panel1") + response = sc.service.createHandler(sc.reqContext, command) + require.Equal(t, 200, response.Status()) + + var existing libraryPanelResult + err = json.Unmarshal(response.Body(), &existing) + require.NoError(t, err) + + dashJSON := map[string]interface{}{ + "panels": []interface{}{ + map[string]interface{}{ + "id": int64(1), + "gridPos": map[string]interface{}{ + "h": 6, + "w": 6, + "x": 0, + "y": 0, + }, + }, + map[string]interface{}{ + "id": int64(2), + "gridPos": map[string]interface{}{ + "h": 6, + "w": 6, + "x": 6, + "y": 0, + }, + "datasource": "${DS_GDEV-TESTDATA}", + "libraryPanel": map[string]interface{}{ + "uid": existing.Result.UID, + "name": existing.Result.Name, + }, + "title": "Text - Library Panel", + "type": "text", + }, + }, + } + dash := models.Dashboard{ + Id: int64(1), + Data: simplejson.NewFromAny(dashJSON), + } + + err = sc.service.ConnectLibraryPanelsForDashboard(sc.reqContext, &dash) + require.NoError(t, err) + + sc.reqContext.ReplaceAllParams(map[string]string{":uid": existing.Result.UID}) + response = sc.service.getConnectedDashboardsHandler(sc.reqContext) + require.Equal(t, 200, response.Status()) + + var existingResult libraryPanelDashboardsResult + err = json.Unmarshal(response.Body(), &existingResult) + require.NoError(t, err) + require.Len(t, existingResult.Result, 1) + require.Equal(t, int64(1), existingResult.Result[0]) + + sc.reqContext.ReplaceAllParams(map[string]string{":uid": unused.Result.UID}) + response = sc.service.getConnectedDashboardsHandler(sc.reqContext) + require.Equal(t, 200, response.Status()) + + var unusedResult libraryPanelDashboardsResult + err = json.Unmarshal(response.Body(), &unusedResult) + require.NoError(t, err) + require.Len(t, unusedResult.Result, 0) + }) } func TestDisconnectLibraryPanelsForDashboard(t *testing.T) { @@ -1309,7 +1389,7 @@ func TestDisconnectLibraryPanelsForDashboard(t *testing.T) { Data: simplejson.NewFromAny(dashJSON), } - err = sc.service.DisconnectLibraryPanelsForDashboard(&dash) + err = sc.service.DisconnectLibraryPanelsForDashboard(sc.reqContext, &dash) require.NoError(t, err) sc.reqContext.ReplaceAllParams(map[string]string{":uid": existing.Result.UID}) @@ -1369,7 +1449,7 @@ func TestDisconnectLibraryPanelsForDashboard(t *testing.T) { Data: simplejson.NewFromAny(dashJSON), } - err = sc.service.DisconnectLibraryPanelsForDashboard(&dash) + err = sc.service.DisconnectLibraryPanelsForDashboard(sc.reqContext, &dash) require.EqualError(t, err, errLibraryPanelHeaderUIDMissing.Error()) }) } @@ -1449,7 +1529,9 @@ func testScenario(t *testing.T, desc string, fn func(t *testing.T, sc scenarioCo t.Run(desc, func(t *testing.T) { t.Cleanup(registry.ClearOverrides) - ctx := macaron.Context{} + ctx := macaron.Context{ + Req: macaron.Request{Request: &http.Request{}}, + } orgID := int64(1) role := models.ROLE_ADMIN