From 433b8610932f61f7c7cba400d28b7966fdf709ef Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Hugo=20H=C3=A4ggmark?= Date: Mon, 21 Dec 2020 09:40:27 +0100 Subject: [PATCH] PanelLibrary: Adds get and getAll to the api (#29772) * PanelLibrary: Adds get to the API * Refactor: adds tests for get and getAll and cleans up other tests * Refactor: changed name on DTO * Update pkg/services/librarypanels/api.go Co-authored-by: Arve Knudsen * Update pkg/services/librarypanels/database.go --- pkg/api/common.go | 7 + pkg/services/librarypanels/api.go | 35 +- pkg/services/librarypanels/database.go | 58 +++- .../librarypanels/librarypanels_test.go | 303 +++++++++++++----- pkg/services/librarypanels/models.go | 4 +- 5 files changed, 305 insertions(+), 102 deletions(-) diff --git a/pkg/api/common.go b/pkg/api/common.go index f3420354ff8..cbb44ec3d97 100644 --- a/pkg/api/common.go +++ b/pkg/api/common.go @@ -19,6 +19,8 @@ type Response interface { WriteTo(ctx *models.ReqContext) // Status gets the response's status. Status() int + // Body gets the response's body. + Body() []byte } type NormalResponse struct { @@ -48,6 +50,11 @@ func (r *NormalResponse) Status() int { return r.status } +// Body gets the response's body. +func (r *NormalResponse) Body() []byte { + return r.body +} + func (r *NormalResponse) WriteTo(ctx *models.ReqContext) { if r.err != nil { ctx.Logger.Error(r.errMessage, "error", r.err, "remote_addr", ctx.RemoteAddr()) diff --git a/pkg/services/librarypanels/api.go b/pkg/services/librarypanels/api.go index a7875e91be6..73f0eb3e12e 100644 --- a/pkg/services/librarypanels/api.go +++ b/pkg/services/librarypanels/api.go @@ -17,15 +17,16 @@ func (lps *LibraryPanelService) registerAPIEndpoints() { } lps.RouteRegister.Group("/api/library-panels", func(libraryPanels routing.RouteRegister) { - libraryPanels.Post("/", middleware.ReqSignedIn, binding.Bind(addLibraryPanelCommand{}), api.Wrap(lps.createHandler)) + libraryPanels.Post("/", middleware.ReqSignedIn, binding.Bind(createLibraryPanelCommand{}), api.Wrap(lps.createHandler)) libraryPanels.Delete("/:panelId", middleware.ReqSignedIn, api.Wrap(lps.deleteHandler)) + libraryPanels.Get("/:panelId", middleware.ReqSignedIn, api.Wrap(lps.getHandler)) + libraryPanels.Get("/", middleware.ReqSignedIn, api.Wrap(lps.getAllHandler)) }) } // createHandler handles POST /api/library-panels. -func (lps *LibraryPanelService) createHandler(c *models.ReqContext, cmd addLibraryPanelCommand) api.Response { +func (lps *LibraryPanelService) createHandler(c *models.ReqContext, cmd createLibraryPanelCommand) api.Response { panel, err := lps.createLibraryPanel(c, cmd) - if err != nil { if errors.Is(err, errLibraryPanelAlreadyAdded) { return api.Error(400, errLibraryPanelAlreadyAdded.Error(), err) @@ -33,13 +34,12 @@ func (lps *LibraryPanelService) createHandler(c *models.ReqContext, cmd addLibra return api.Error(500, "Failed to create library panel", err) } - return api.JSON(200, util.DynMap{"panel": panel}) + return api.JSON(200, util.DynMap{"result": panel}) } -// deleteHandler handles DELETE /api/library-panels/:panelId. +// deleteHandler handles DELETE /api/library-panels/:panelId func (lps *LibraryPanelService) deleteHandler(c *models.ReqContext) api.Response { err := lps.deleteLibraryPanel(c, c.ParamsInt64(":panelId")) - if err != nil { if errors.Is(err, errLibraryPanelNotFound) { return api.Error(404, errLibraryPanelNotFound.Error(), err) @@ -49,3 +49,26 @@ func (lps *LibraryPanelService) deleteHandler(c *models.ReqContext) api.Response return api.Success("Library panel deleted") } + +// getHandler handles GET /api/library-panels/:panelId +func (lps *LibraryPanelService) getHandler(c *models.ReqContext) api.Response { + libraryPanel, err := lps.getLibraryPanel(c, c.ParamsInt64(":panelId")) + if err != nil { + if errors.Is(err, errLibraryPanelNotFound) { + return api.Error(404, errLibraryPanelNotFound.Error(), err) + } + return api.Error(500, "Failed to get library panel", err) + } + + return api.JSON(200, util.DynMap{"result": libraryPanel}) +} + +// getAllHandler handles GET /api/library-panels/ +func (lps *LibraryPanelService) getAllHandler(c *models.ReqContext) api.Response { + libraryPanels, err := lps.getAllLibraryPanels(c) + if err != nil { + return api.Error(500, "Failed to get library panels", err) + } + + return api.JSON(200, util.DynMap{"result": libraryPanels}) +} diff --git a/pkg/services/librarypanels/database.go b/pkg/services/librarypanels/database.go index 1702373aa1a..3c68f9fddb5 100644 --- a/pkg/services/librarypanels/database.go +++ b/pkg/services/librarypanels/database.go @@ -2,6 +2,7 @@ package librarypanels import ( "context" + "fmt" "time" "github.com/grafana/grafana/pkg/models" @@ -9,8 +10,8 @@ import ( "github.com/grafana/grafana/pkg/services/sqlstore" ) -// createLibraryPanel function adds a Library Panel -func (lps *LibraryPanelService) createLibraryPanel(c *models.ReqContext, cmd addLibraryPanelCommand) (LibraryPanel, error) { +// createLibraryPanel adds a Library Panel +func (lps *LibraryPanelService) createLibraryPanel(c *models.ReqContext, cmd createLibraryPanelCommand) (LibraryPanel, error) { libraryPanel := LibraryPanel{ OrgID: c.SignedInUser.OrgId, FolderID: cmd.FolderID, @@ -23,9 +24,9 @@ func (lps *LibraryPanelService) createLibraryPanel(c *models.ReqContext, cmd add CreatedBy: c.SignedInUser.UserId, UpdatedBy: c.SignedInUser.UserId, } - err := lps.SQLStore.WithTransactionalDbSession(context.Background(), func(session *sqlstore.DBSession) error { - if res, err := session.Query("SELECT 1 from library_panel WHERE org_id=? and folder_id=? and title=?", c.SignedInUser.OrgId, cmd.FolderID, cmd.Title); err != nil { + if res, err := session.Query("SELECT 1 FROM library_panel WHERE org_id=? AND folder_id=? AND title=?", + c.SignedInUser.OrgId, cmd.FolderID, cmd.Title); err != nil { return err } else if len(res) == 1 { return errLibraryPanelAlreadyAdded @@ -40,13 +41,11 @@ func (lps *LibraryPanelService) createLibraryPanel(c *models.ReqContext, cmd add return libraryPanel, err } -// deleteLibraryPanel function deletes a Library Panel +// deleteLibraryPanel deletes a Library Panel func (lps *LibraryPanelService) deleteLibraryPanel(c *models.ReqContext, panelID int64) error { orgID := c.SignedInUser.OrgId - - err := lps.SQLStore.WithTransactionalDbSession(context.Background(), func(session *sqlstore.DBSession) error { + return lps.SQLStore.WithTransactionalDbSession(context.Background(), func(session *sqlstore.DBSession) error { result, err := session.Exec("DELETE FROM library_panel WHERE id=? and org_id=?", panelID, orgID) - if err != nil { return err } @@ -59,6 +58,47 @@ func (lps *LibraryPanelService) deleteLibraryPanel(c *models.ReqContext, panelID return nil }) +} - return err +// getLibraryPanel gets a Library Panel. +func (lps *LibraryPanelService) getLibraryPanel(c *models.ReqContext, panelID int64) (LibraryPanel, error) { + orgID := c.SignedInUser.OrgId + var libraryPanel LibraryPanel + err := lps.SQLStore.WithDbSession(context.Background(), func(session *sqlstore.DBSession) error { + libraryPanels := make([]LibraryPanel, 0) + err := session.SQL("SELECT * FROM library_panel WHERE id=? and org_id=?", panelID, orgID).Find(&libraryPanels) + if err != nil { + return err + } + + if len(libraryPanels) == 0 { + return errLibraryPanelNotFound + } + if len(libraryPanels) > 1 { + return fmt.Errorf("found %d panels, while expecting at most one", len(libraryPanels)) + } + + libraryPanel = libraryPanels[0] + + return nil + }) + + return libraryPanel, err +} + +// getAllLibraryPanels gets all library panels. +func (lps *LibraryPanelService) getAllLibraryPanels(c *models.ReqContext) ([]LibraryPanel, error) { + orgID := c.SignedInUser.OrgId + libraryPanels := make([]LibraryPanel, 0) + + err := lps.SQLStore.WithDbSession(context.Background(), func(session *sqlstore.DBSession) error { + err := session.SQL("SELECT * FROM library_panel WHERE org_id=?", orgID).Find(&libraryPanels) + if err != nil { + return err + } + + return nil + }) + + return libraryPanels, err } diff --git a/pkg/services/librarypanels/librarypanels_test.go b/pkg/services/librarypanels/librarypanels_test.go index bf526b2fbe5..edcc3af14ad 100644 --- a/pkg/services/librarypanels/librarypanels_test.go +++ b/pkg/services/librarypanels/librarypanels_test.go @@ -1,6 +1,7 @@ package librarypanels import ( + "encoding/json" "testing" "time" @@ -15,78 +16,167 @@ import ( ) func TestCreateLibraryPanel(t *testing.T) { - libraryPanelScenario(t, "When an admin tries to create a library panel", "then it should fail if library panel already exists", func(t *testing.T) { - lps, context := setupTestEnv(t, models.ROLE_ADMIN, map[string]string{}) - command := getCreateCommand(1, "Text - Library Panel") + testScenario(t, "When an admin tries to create a library panel that already exists, it should fail", + func(t *testing.T, sc scenarioContext) { + command := getCreateCommand(1, "Text - Library Panel") - response := lps.createHandler(&context, command) - require.Equal(t, 200, response.Status()) + response := sc.service.createHandler(sc.reqContext, command) + require.Equal(t, 200, response.Status()) - response = lps.createHandler(&context, command) - require.Equal(t, 400, response.Status()) - }) + response = sc.service.createHandler(sc.reqContext, command) + require.Equal(t, 400, response.Status()) + }) } func TestDeleteLibraryPanel(t *testing.T) { - libraryPanelScenario(t, "When an admin tries to delete a library panel that does not exist", "then it should fail", func(t *testing.T) { - lps, context := setupTestEnv(t, models.ROLE_ADMIN, map[string]string{":panelId": "74"}) - - response := lps.deleteHandler(&context) - require.Equal(t, 404, response.Status()) - }) - - libraryPanelScenario(t, "When an admin tries to delete a library panel that exists", "then it should succeed", func(t *testing.T) { - lps, context := setupTestEnv(t, models.ROLE_ADMIN, map[string]string{":panelId": "1"}) - command := getCreateCommand(1, "Text - Library Panel") - - response := lps.createHandler(&context, command) - require.Equal(t, 200, response.Status()) - - response = lps.deleteHandler(&context) - require.Equal(t, 200, response.Status()) - }) - - libraryPanelScenario(t, "When an admin tries to delete a library panel in another org", "then it should fail", func(t *testing.T) { - params := map[string]string{":panelId": "1"} - lps, context := setupTestEnv(t, models.ROLE_ADMIN, params) - command := getCreateCommand(1, "Text - Library Panel") - - response := lps.createHandler(&context, command) - require.Equal(t, 200, response.Status()) - - user := getTestUser(models.ROLE_ADMIN, 2) - context = getTestContext(user, params) - - response = lps.deleteHandler(&context) - require.Equal(t, 404, response.Status()) - }) -} - -func libraryPanelScenario(t *testing.T, when string, then string, fn func(t *testing.T)) { - t.Run(when, func(t *testing.T) { - t.Run(then, func(t *testing.T) { - fn(t) - t.Cleanup(registry.ClearOverrides) + testScenario(t, "When an admin tries to delete a library panel that does not exist, it should fail", + func(t *testing.T, sc scenarioContext) { + response := sc.service.deleteHandler(sc.reqContext) + require.Equal(t, 404, response.Status()) + }) + + testScenario(t, "When an admin tries to delete a library panel that exists, it should succeed", + func(t *testing.T, sc scenarioContext) { + command := getCreateCommand(1, "Text - Library Panel") + response := sc.service.createHandler(sc.reqContext, command) + require.Equal(t, 200, response.Status()) + response = sc.service.deleteHandler(sc.reqContext) + require.Equal(t, 200, response.Status()) + }, scenarioConfig{params: map[string]string{":panelId": "1"}}) + + testScenario(t, "When an admin tries to delete a library panel in another org, it should fail", + func(t *testing.T, sc scenarioContext) { + command := getCreateCommand(1, "Text - Library Panel") + response := sc.service.createHandler(sc.reqContext, command) + require.Equal(t, 200, response.Status()) + + sc.reqContext.SignedInUser.OrgId = 2 + sc.reqContext.SignedInUser.OrgRole = models.ROLE_ADMIN + response = sc.service.deleteHandler(sc.reqContext) + require.Equal(t, 404, response.Status()) + }, scenarioConfig{ + params: map[string]string{":panelId": "1"}, }) - }) } -func setupTestEnv(t *testing.T, orgRole models.RoleType, params macaron.Params) (LibraryPanelService, models.ReqContext) { - cfg := setting.NewCfg() - cfg.FeatureToggles = map[string]bool{"panelLibrary": true} // Everything in this service is behind the feature toggle "panelLibrary" +func TestGetLibraryPanel(t *testing.T) { + testScenario(t, "When an admin tries to get a library panel that does not exist, it should fail", + func(t *testing.T, sc scenarioContext) { + response := sc.service.getHandler(sc.reqContext) + require.Equal(t, 404, response.Status()) + }, scenarioConfig{params: map[string]string{":panelId": "74"}}) - // Because the LibraryPanelService is behind a feature toggle we need to override the service in the registry - // with a Cfg that contains the feature toggle so that Migrations are run properly - service := overrideLibraryPanelServiceInRegistry(cfg) + testScenario(t, "When an admin tries to get a library panel that exists, it should succeed and return correct result", + func(t *testing.T, sc scenarioContext) { + command := getCreateCommand(1, "Text - Library Panel") + response := sc.service.createHandler(sc.reqContext, command) + require.Equal(t, 200, response.Status()) - sqlStore := sqlstore.InitTestDB(t) - // We need to assign SQLStore after the override and migrations are done - service.SQLStore = sqlStore + response = sc.service.getHandler(sc.reqContext) + require.Equal(t, 200, response.Status()) - user := getTestUser(orgRole, 1) - context := getTestContext(user, params) + var result libraryPanelResult + err := json.Unmarshal(response.Body(), &result) + require.NoError(t, err) + require.Equal(t, int64(1), result.Result.FolderID) + require.Equal(t, "Text - Library Panel", result.Result.Title) + }, scenarioConfig{params: map[string]string{":panelId": "1"}}) - return service, context + testScenario(t, "When an admin tries to get a library panel that exists in an other org, it should fail", + func(t *testing.T, sc scenarioContext) { + command := getCreateCommand(1, "Text - Library Panel") + + response := sc.service.createHandler(sc.reqContext, command) + require.Equal(t, 200, response.Status()) + + sc.reqContext.SignedInUser.OrgId = 2 + sc.reqContext.SignedInUser.OrgRole = models.ROLE_ADMIN + + response = sc.service.getHandler(sc.reqContext) + require.Equal(t, 404, response.Status()) + }, scenarioConfig{params: map[string]string{":panelId": "1"}}) +} + +func TestGetAllLibraryPanels(t *testing.T) { + testScenario(t, "When an admin tries to get all library panels and none exists, it should return none", + func(t *testing.T, sc scenarioContext) { + response := sc.service.getAllHandler(sc.reqContext) + require.Equal(t, 200, response.Status()) + + var result libraryPanelsResult + err := json.Unmarshal(response.Body(), &result) + require.NoError(t, err) + require.NotNil(t, result.Result) + require.Equal(t, 0, len(result.Result)) + }) + + testScenario(t, "When an admin tries to get all library panels and two exist, it should work", + func(t *testing.T, sc scenarioContext) { + command := getCreateCommand(1, "Text - Library Panel") + response := sc.service.createHandler(sc.reqContext, command) + require.Equal(t, 200, response.Status()) + + command = getCreateCommand(1, "Text - Library Panel2") + response = sc.service.createHandler(sc.reqContext, command) + require.Equal(t, 200, response.Status()) + + response = sc.service.getAllHandler(sc.reqContext) + require.Equal(t, 200, response.Status()) + + var result libraryPanelsResult + err := json.Unmarshal(response.Body(), &result) + require.NoError(t, err) + require.Equal(t, 2, len(result.Result)) + require.Equal(t, int64(1), result.Result[0].FolderID) + require.Equal(t, "Text - Library Panel", result.Result[0].Title) + require.Equal(t, int64(1), result.Result[1].FolderID) + require.Equal(t, "Text - Library Panel2", result.Result[1].Title) + }) + + testScenario(t, "When an admin tries to get all library panels in a different org, none should be returned", + func(t *testing.T, sc scenarioContext) { + command := getCreateCommand(1, "Text - Library Panel") + + response := sc.service.createHandler(sc.reqContext, command) + require.Equal(t, 200, response.Status()) + + response = sc.service.getAllHandler(sc.reqContext) + require.Equal(t, 200, response.Status()) + + var result libraryPanelsResult + err := json.Unmarshal(response.Body(), &result) + require.NoError(t, err) + require.Equal(t, 1, len(result.Result)) + require.Equal(t, int64(1), result.Result[0].FolderID) + require.Equal(t, "Text - Library Panel", result.Result[0].Title) + + sc.reqContext.SignedInUser.OrgId = 2 + sc.reqContext.SignedInUser.OrgRole = models.ROLE_ADMIN + + response = sc.service.getAllHandler(sc.reqContext) + require.Equal(t, 200, response.Status()) + + result = libraryPanelsResult{} + err = json.Unmarshal(response.Body(), &result) + require.NoError(t, err) + require.NotNil(t, result.Result) + require.Equal(t, 0, len(result.Result)) + }) +} + +type libraryPanel struct { + ID int64 `json:"ID"` + OrgID int64 `json:"OrgID"` + FolderID int64 `json:"FolderID"` + Title string `json:"Title"` +} + +type libraryPanelResult struct { + Result libraryPanel `json:"result"` +} + +type libraryPanelsResult struct { + Result []libraryPanel `json:"result"` } func overrideLibraryPanelServiceInRegistry(cfg *setting.Cfg) LibraryPanelService { @@ -110,31 +200,8 @@ func overrideLibraryPanelServiceInRegistry(cfg *setting.Cfg) LibraryPanelService return lps } -func getTestUser(orgRole models.RoleType, orgID int64) models.SignedInUser { - user := models.SignedInUser{ - UserId: 1, - OrgId: orgID, - OrgRole: orgRole, - LastSeenAt: time.Now(), - } - - return user -} - -func getTestContext(user models.SignedInUser, params macaron.Params) models.ReqContext { - macronContext := macaron.Context{} - macronContext.ReplaceAllParams(params) - - context := models.ReqContext{ - Context: ¯onContext, - SignedInUser: &user, - } - - return context -} - -func getCreateCommand(folderID int64, title string) addLibraryPanelCommand { - command := addLibraryPanelCommand{ +func getCreateCommand(folderID int64, title string) createLibraryPanelCommand { + command := createLibraryPanelCommand{ FolderID: folderID, Title: title, Model: []byte(` @@ -149,3 +216,69 @@ func getCreateCommand(folderID int64, title string) addLibraryPanelCommand { return command } + +type scenarioContext struct { + ctx *macaron.Context + service *LibraryPanelService + reqContext *models.ReqContext + user models.SignedInUser +} + +type scenarioConfig struct { + params map[string]string + orgID int64 + role models.RoleType +} + +// testScenario is a wrapper around t.Run performing common setup for library panel tests. +// It takes your real test function as a callback. +func testScenario(t *testing.T, desc string, fn func(t *testing.T, sc scenarioContext), cfgs ...scenarioConfig) { + t.Helper() + + t.Run(desc, func(t *testing.T) { + t.Cleanup(registry.ClearOverrides) + + ctx := macaron.Context{} + orgID := int64(1) + role := models.ROLE_ADMIN + for _, cfg := range cfgs { + if len(cfg.params) > 0 { + ctx.ReplaceAllParams(cfg.params) + } + if cfg.orgID != 0 { + orgID = cfg.orgID + } + if cfg.role != "" { + role = cfg.role + } + } + + cfg := setting.NewCfg() + // Everything in this service is behind the feature toggle "panelLibrary" + cfg.FeatureToggles = map[string]bool{"panelLibrary": true} + // Because the LibraryPanelService is behind a feature toggle, we need to override the service in the registry + // with a Cfg that contains the feature toggle so migrations are run properly + service := overrideLibraryPanelServiceInRegistry(cfg) + + // We need to assign SQLStore after the override and migrations are done + sqlStore := sqlstore.InitTestDB(t) + service.SQLStore = sqlStore + + user := models.SignedInUser{ + UserId: 1, + OrgId: orgID, + OrgRole: role, + LastSeenAt: time.Now(), + } + sc := scenarioContext{ + user: user, + ctx: &ctx, + service: &service, + reqContext: &models.ReqContext{ + Context: &ctx, + SignedInUser: &user, + }, + } + fn(t, sc) + }) +} diff --git a/pkg/services/librarypanels/models.go b/pkg/services/librarypanels/models.go index bc15d2570a3..07e7d9815d5 100644 --- a/pkg/services/librarypanels/models.go +++ b/pkg/services/librarypanels/models.go @@ -30,8 +30,8 @@ var ( // Commands -// addLibraryPanelCommand is the command for adding a LibraryPanel -type addLibraryPanelCommand struct { +// createLibraryPanelCommand is the command for adding a LibraryPanel +type createLibraryPanelCommand struct { FolderID int64 `json:"folderId"` Title string `json:"title"` Model json.RawMessage `json:"model"`