From 4c46e8a6c420abe407d431a015cfd11ebee4082c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Hugo=20H=C3=A4ggmark?= Date: Mon, 7 Dec 2020 15:08:52 +0100 Subject: [PATCH] Refactor: Adds repository --- pkg/services/librarypanels/api.go | 34 ++++- pkg/services/librarypanels/database_test.go | 82 ------------ pkg/services/librarypanels/librarypanels.go | 24 ++-- .../{database.go => repository.go} | 30 ++++- pkg/services/librarypanels/repository_test.go | 118 ++++++++++++++++++ 5 files changed, 182 insertions(+), 106 deletions(-) delete mode 100644 pkg/services/librarypanels/database_test.go rename pkg/services/librarypanels/{database.go => repository.go} (51%) create mode 100644 pkg/services/librarypanels/repository_test.go diff --git a/pkg/services/librarypanels/api.go b/pkg/services/librarypanels/api.go index 832777aed84..9ed16577387 100644 --- a/pkg/services/librarypanels/api.go +++ b/pkg/services/librarypanels/api.go @@ -6,25 +6,47 @@ import ( "github.com/grafana/grafana/pkg/api/routing" "github.com/grafana/grafana/pkg/middleware" "github.com/grafana/grafana/pkg/models" + "github.com/grafana/grafana/pkg/setting" "github.com/grafana/grafana/pkg/util" ) -func (lps *LibraryPanelService) registerAPIEndpoints() { - if !lps.IsEnabled() { +type LibraryPanelApi interface { + registerAPIEndpoints() + addLibraryPanelEndpoint(c *models.ReqContext, cmd addLibraryPanelCommand) api.Response +} + +type LibraryPanelApiImpl struct { + Cfg *setting.Cfg + RouteRegister routing.RouteRegister + repository LibraryPanelRepository +} + +func NewApi(routeRegister routing.RouteRegister, cfg *setting.Cfg, repository LibraryPanelRepository) LibraryPanelApi { + impl := &LibraryPanelApiImpl{ + Cfg: cfg, + RouteRegister: routeRegister, + repository: repository, + } + + return impl +} + +func (lpa *LibraryPanelApiImpl) registerAPIEndpoints() { + if !lpa.Cfg.IsPanelLibraryEnabled() { return } - lps.RouteRegister.Group("/api/library-panels", func(libraryPanels routing.RouteRegister) { - libraryPanels.Post("/", middleware.ReqSignedIn, binding.Bind(addLibraryPanelCommand{}), api.Wrap(lps.addLibraryPanelEndpoint)) + lpa.RouteRegister.Group("/api/library-panels", func(libraryPanels routing.RouteRegister) { + libraryPanels.Post("/", middleware.ReqSignedIn, binding.Bind(addLibraryPanelCommand{}), api.Wrap(lpa.addLibraryPanelEndpoint)) }) } // addLibraryPanelEndpoint handles POST /api/library-panels. -func (lps *LibraryPanelService) addLibraryPanelEndpoint(c *models.ReqContext, cmd addLibraryPanelCommand) api.Response { +func (lpa *LibraryPanelApiImpl) addLibraryPanelEndpoint(c *models.ReqContext, cmd addLibraryPanelCommand) api.Response { cmd.OrgId = c.SignedInUser.OrgId cmd.SignedInUser = c.SignedInUser - if err := lps.addLibraryPanel(&cmd); err != nil { + if err := lpa.repository.addLibraryPanel(&cmd); err != nil { return api.Error(500, "Failed to create library panel", err) } diff --git a/pkg/services/librarypanels/database_test.go b/pkg/services/librarypanels/database_test.go deleted file mode 100644 index 8ee42ad2548..00000000000 --- a/pkg/services/librarypanels/database_test.go +++ /dev/null @@ -1,82 +0,0 @@ -package librarypanels - -import ( - "testing" - "time" - - "github.com/grafana/grafana/pkg/models" - "github.com/grafana/grafana/pkg/registry" - "github.com/grafana/grafana/pkg/services/sqlstore" - "github.com/grafana/grafana/pkg/setting" - "github.com/stretchr/testify/require" -) - -func TestAddLibraryPanel(t *testing.T) { - t.Run("should fail if library panel already exists", func(t *testing.T) { - lps, user := setupTestEnv(t, models.ROLE_EDITOR) - command := addLibraryPanelCommand{ - OrgId: 1, - FolderId: 1, - SignedInUser: &user, - Title: "Text - Library Panel", - Model: []byte(` - { - "datasource": "${DS_GDEV-TESTDATA}", - "id": 1, - "title": "Text - Library Panel", - "type": "text" - } -`), - } - - noErr := lps.addLibraryPanel(&command) - require.NoError(t, noErr) - - err := lps.addLibraryPanel(&command) - require.Error(t, err) - }) -} - -func setupTestEnv(t *testing.T, orgRole models.RoleType) (LibraryPanelService, models.SignedInUser) { - cfg := setting.NewCfg() - cfg.FeatureToggles = map[string]bool{"panelLibrary": true} - - lps := LibraryPanelService{ - SQLStore: nil, - Cfg: cfg, - } - - overrideServiceFunc := func(d registry.Descriptor) (*registry.Descriptor, bool) { - descriptor := registry.Descriptor{ - Name: "LibraryPanelService", - Instance: &lps, - InitPriority: 0, - } - - return &descriptor, true - } - - registry.RegisterOverride(overrideServiceFunc) - - sqlStore := sqlstore.InitTestDB(t) - lps.SQLStore = sqlStore - - user := models.SignedInUser{ - UserId: 1, - OrgId: 1, - OrgName: "", - OrgRole: orgRole, - Login: "", - Name: "", - Email: "", - ApiKeyId: 0, - OrgCount: 0, - IsGrafanaAdmin: false, - IsAnonymous: false, - HelpFlags1: 0, - LastSeenAt: time.Now(), - Teams: nil, - } - - return lps, user -} diff --git a/pkg/services/librarypanels/librarypanels.go b/pkg/services/librarypanels/librarypanels.go index 3c6821d0017..7545dc3c8aa 100644 --- a/pkg/services/librarypanels/librarypanels.go +++ b/pkg/services/librarypanels/librarypanels.go @@ -14,6 +14,8 @@ type LibraryPanelService struct { Cfg *setting.Cfg `inject:""` SQLStore *sqlstore.SQLStore `inject:""` RouteRegister routing.RouteRegister `inject:""` + api LibraryPanelApi + repository LibraryPanelRepository log log.Logger } @@ -22,27 +24,19 @@ func init() { } // Init initializes the LibraryPanel service -func (lps *LibraryPanelService) Init() error { - lps.log = log.New("librarypanels") - - lps.registerAPIEndpoints() +func (service *LibraryPanelService) Init() error { + service.log = log.New("libraryPanels") + service.repository = NewRepository(service.Cfg, service.SQLStore) + service.api = NewApi(service.RouteRegister, service.Cfg, service.repository) + service.api.registerAPIEndpoints() return nil } -// IsEnabled returns true if the Panel Library feature is enabled for this instance. -func (lps *LibraryPanelService) IsEnabled() bool { - if lps.Cfg == nil { - return false - } - - return lps.Cfg.IsPanelLibraryEnabled() -} - // AddMigration defines database migrations. // If Panel Library is not enabled does nothing. -func (lps *LibraryPanelService) AddMigration(mg *migrator.Migrator) { - if !lps.IsEnabled() { +func (service *LibraryPanelService) AddMigration(mg *migrator.Migrator) { + if !service.Cfg.IsPanelLibraryEnabled() { return } diff --git a/pkg/services/librarypanels/database.go b/pkg/services/librarypanels/repository.go similarity index 51% rename from pkg/services/librarypanels/database.go rename to pkg/services/librarypanels/repository.go index 0fd1dbe14d4..852d47fa3f7 100644 --- a/pkg/services/librarypanels/database.go +++ b/pkg/services/librarypanels/repository.go @@ -4,12 +4,36 @@ import ( "context" "time" + "github.com/grafana/grafana/pkg/setting" + "github.com/grafana/grafana/pkg/services/sqlstore" ) -// AddLibraryPanel function adds a LibraryPanel -func (lps *LibraryPanelService) addLibraryPanel(cmd *addLibraryPanelCommand) error { - return lps.SQLStore.WithTransactionalDbSession(context.Background(), func(session *sqlstore.DBSession) error { +type LibraryPanelRepository interface { + addLibraryPanel(cmd *addLibraryPanelCommand) error +} + +type SQLLibraryPanelRepository struct { + cfg *setting.Cfg + sqlStore *sqlstore.SQLStore +} + +func NewRepository(cfg *setting.Cfg, sqlStore *sqlstore.SQLStore) LibraryPanelRepository { + repository := SQLLibraryPanelRepository{ + cfg: cfg, + sqlStore: sqlStore, + } + + return &repository +} + +// addLibraryPanel function adds a LibraryPanel +func (repo *SQLLibraryPanelRepository) addLibraryPanel(cmd *addLibraryPanelCommand) error { + if !repo.cfg.IsPanelLibraryEnabled() { + return nil + } + + return repo.sqlStore.WithTransactionalDbSession(context.Background(), func(session *sqlstore.DBSession) error { libraryPanel := &LibraryPanel{ OrgId: cmd.OrgId, FolderId: cmd.FolderId, diff --git a/pkg/services/librarypanels/repository_test.go b/pkg/services/librarypanels/repository_test.go new file mode 100644 index 00000000000..66b2828f280 --- /dev/null +++ b/pkg/services/librarypanels/repository_test.go @@ -0,0 +1,118 @@ +package librarypanels + +import ( + "testing" + "time" + + "github.com/grafana/grafana/pkg/registry" + + "github.com/stretchr/testify/require" + + "github.com/grafana/grafana/pkg/models" + "github.com/grafana/grafana/pkg/services/sqlstore" + "github.com/grafana/grafana/pkg/setting" +) + +func TestAddLibraryPanel(t *testing.T) { + t.Run("should fail if library panel already exists", func(t *testing.T) { + repository, user := setupTestEnv(t, true, models.ROLE_EDITOR) + command := addLibraryPanelCommand{ + OrgId: 1, + FolderId: 1, + SignedInUser: &user, + Title: "Text - Library Panel", + Model: []byte(` + { + "datasource": "${DS_GDEV-TESTDATA}", + "id": 1, + "title": "Text - Library Panel", + "type": "text" + } + `), + } + + noErr := repository.addLibraryPanel(&command) + require.NoError(t, noErr) + + err := repository.addLibraryPanel(&command) + require.EqualError(t, err, errLibraryPanelAlreadyAdded.Error()) + }) +} + +func TestAddLibraryPanelWhenFeatureToggleIsOff(t *testing.T) { + t.Run("should return nil", func(t *testing.T) { + repository, user := setupTestEnv(t, false, models.ROLE_EDITOR) + + command := addLibraryPanelCommand{ + OrgId: 1, + FolderId: 1, + SignedInUser: &user, + Title: "Text - Library Panel", + Model: []byte(` + { + "datasource": "${DS_GDEV-TESTDATA}", + "id": 1, + "title": "Text - Library Panel", + "type": "text" + } + `), + } + + err := repository.addLibraryPanel(&command) + require.NoError(t, err) + require.Nil(t, command.Result) + }) +} + +// setupMigration overrides LibraryPanelService so that the AddMigration is run before tests +// this is necessary because our migration is controlled by a feature toggle +func setupMigration(cfg *setting.Cfg) { + lps := LibraryPanelService{ + SQLStore: nil, + Cfg: cfg, + } + + overrideServiceFunc := func(d registry.Descriptor) (*registry.Descriptor, bool) { + descriptor := registry.Descriptor{ + Name: "LibraryPanelService", + Instance: &lps, + InitPriority: 0, + } + + return &descriptor, true + } + + registry.RegisterOverride(overrideServiceFunc) +} + +func setupTestEnv(t *testing.T, featureToggle bool, orgRole models.RoleType) (*SQLLibraryPanelRepository, models.SignedInUser) { + cfg := setting.NewCfg() + cfg.FeatureToggles = map[string]bool{"panelLibrary": featureToggle} + + setupMigration(cfg) + + sqlStore := sqlstore.InitTestDB(t) + repository := &SQLLibraryPanelRepository{ + cfg: cfg, + sqlStore: sqlStore, + } + + user := models.SignedInUser{ + UserId: 1, + OrgId: 1, + OrgName: "", + OrgRole: orgRole, + Login: "", + Name: "", + Email: "", + ApiKeyId: 0, + OrgCount: 0, + IsGrafanaAdmin: false, + IsAnonymous: false, + HelpFlags1: 0, + LastSeenAt: time.Now(), + Teams: nil, + } + + return repository, user +}