From 0e6300fb49d2d562a30c60dcdcd7009456440d30 Mon Sep 17 00:00:00 2001 From: Kat Yang <69819079+yangkb09@users.noreply.github.com> Date: Fri, 4 Feb 2022 11:53:58 -0500 Subject: [PATCH] Chore: Remove bus from admin (#44920) * Chore: Remove bus from admin * fix test Co-authored-by: Ying WANG --- pkg/api/admin.go | 5 ++--- pkg/api/admin_test.go | 2 ++ pkg/api/api.go | 2 +- pkg/services/sqlstore/mockstore/mockstore.go | 4 ++++ pkg/services/sqlstore/sqlstore.go | 1 + pkg/services/sqlstore/stats.go | 7 +++++-- pkg/services/sqlstore/stats_integration_test.go | 4 ++-- pkg/services/sqlstore/stats_test.go | 2 +- pkg/services/sqlstore/store.go | 1 + 9 files changed, 19 insertions(+), 9 deletions(-) diff --git a/pkg/api/admin.go b/pkg/api/admin.go index 4c6aceec32d..453db021900 100644 --- a/pkg/api/admin.go +++ b/pkg/api/admin.go @@ -5,7 +5,6 @@ import ( "net/http" "github.com/grafana/grafana/pkg/api/response" - "github.com/grafana/grafana/pkg/bus" "github.com/grafana/grafana/pkg/models" ac "github.com/grafana/grafana/pkg/services/accesscontrol" "github.com/grafana/grafana/pkg/setting" @@ -19,10 +18,10 @@ func (hs *HTTPServer) AdminGetSettings(c *models.ReqContext) response.Response { return response.JSON(http.StatusOK, settings) } -func AdminGetStats(c *models.ReqContext) response.Response { +func (hs *HTTPServer) AdminGetStats(c *models.ReqContext) response.Response { statsQuery := models.GetAdminStatsQuery{} - if err := bus.Dispatch(c.Req.Context(), &statsQuery); err != nil { + if err := hs.SQLStore.GetAdminStats(c.Req.Context(), &statsQuery); err != nil { return response.Error(500, "Failed to get admin stats from database", err) } diff --git a/pkg/api/admin_test.go b/pkg/api/admin_test.go index f24aa263306..0ba3db71f2f 100644 --- a/pkg/api/admin_test.go +++ b/pkg/api/admin_test.go @@ -6,6 +6,7 @@ import ( "testing" "github.com/grafana/grafana/pkg/services/accesscontrol" + "github.com/grafana/grafana/pkg/services/sqlstore/mockstore" "github.com/grafana/grafana/pkg/setting" "github.com/stretchr/testify/assert" ) @@ -147,6 +148,7 @@ func TestAdmin_AccessControl(t *testing.T) { sc, hs := setupAccessControlScenarioContext(t, cfg, test.url, test.permissions) sc.resp = httptest.NewRecorder() hs.SettingsProvider = &setting.OSSImpl{Cfg: cfg} + hs.SQLStore = mockstore.NewSQLStoreMock() var err error sc.req, err = http.NewRequest(test.method, test.url, nil) diff --git a/pkg/api/api.go b/pkg/api/api.go index 41cf0eee227..034fb8dd8a2 100644 --- a/pkg/api/api.go +++ b/pkg/api/api.go @@ -463,7 +463,7 @@ func (hs *HTTPServer) registerRoutes() { if hs.Features.IsEnabled(featuremgmt.FlagShowFeatureFlagsInUI) { adminRoute.Get("/settings/features", authorize(reqGrafanaAdmin, ac.EvalPermission(ac.ActionSettingsRead)), hs.Features.HandleGetSettings) } - adminRoute.Get("/stats", authorize(reqGrafanaAdmin, ac.EvalPermission(ac.ActionServerStatsRead)), routing.Wrap(AdminGetStats)) + adminRoute.Get("/stats", authorize(reqGrafanaAdmin, ac.EvalPermission(ac.ActionServerStatsRead)), routing.Wrap(hs.AdminGetStats)) adminRoute.Post("/pause-all-alerts", reqGrafanaAdmin, routing.Wrap(hs.PauseAllAlerts)) if hs.ThumbService != nil { diff --git a/pkg/services/sqlstore/mockstore/mockstore.go b/pkg/services/sqlstore/mockstore/mockstore.go index 429131d2067..909fb442dbe 100644 --- a/pkg/services/sqlstore/mockstore/mockstore.go +++ b/pkg/services/sqlstore/mockstore/mockstore.go @@ -23,6 +23,10 @@ func NewSQLStoreMock() *SQLStoreMock { return &SQLStoreMock{} } +func (m *SQLStoreMock) GetAdminStats(ctx context.Context, query *models.GetAdminStatsQuery) error { + return m.ExpectedError +} + func (m *SQLStoreMock) DeleteExpiredSnapshots(ctx context.Context, cmd *models.DeleteExpiredSnapshotsCommand) error { return m.ExpectedError } diff --git a/pkg/services/sqlstore/sqlstore.go b/pkg/services/sqlstore/sqlstore.go index 70f0497b43b..a3d162d9cfe 100644 --- a/pkg/services/sqlstore/sqlstore.go +++ b/pkg/services/sqlstore/sqlstore.go @@ -115,6 +115,7 @@ func newSQLStore(cfg *setting.Cfg, cacheService *localcache.CacheService, b bus. ss.Bus.SetTransactionManager(ss) // Register handlers + ss.addStatsQueryAndCommandHandlers() ss.addUserQueryAndCommandHandlers() ss.addAlertNotificationUidByIdHandler() ss.addPreferencesQueryAndCommandHandlers() diff --git a/pkg/services/sqlstore/stats.go b/pkg/services/sqlstore/stats.go index d9d34c63dd4..044d1117fe6 100644 --- a/pkg/services/sqlstore/stats.go +++ b/pkg/services/sqlstore/stats.go @@ -13,11 +13,14 @@ func init() { bus.AddHandler("sql", GetSystemStats) bus.AddHandler("sql", GetDataSourceStats) bus.AddHandler("sql", GetDataSourceAccessStats) - bus.AddHandler("sql", GetAdminStats) bus.AddHandler("sql", GetAlertNotifiersUsageStats) bus.AddHandler("sql", GetSystemUserCountStats) } +func (ss *SQLStore) addStatsQueryAndCommandHandlers() { + bus.AddHandler("sql", ss.GetAdminStats) +} + const activeUserTimeLimit = time.Hour * 24 * 30 const dailyActiveUserTimeLimit = time.Hour * 24 @@ -141,7 +144,7 @@ func viewersPermissionsCounterSQL(statName string, isFolder bool, permission mod ) AS ` + statName + `, ` } -func GetAdminStats(ctx context.Context, query *models.GetAdminStatsQuery) error { +func (ss *SQLStore) GetAdminStats(ctx context.Context, query *models.GetAdminStatsQuery) error { now := time.Now() activeEndDate := now.Add(-activeUserTimeLimit) dailyActiveEndDate := now.Add(-dailyActiveUserTimeLimit) diff --git a/pkg/services/sqlstore/stats_integration_test.go b/pkg/services/sqlstore/stats_integration_test.go index a560d07e035..03e89945a10 100644 --- a/pkg/services/sqlstore/stats_integration_test.go +++ b/pkg/services/sqlstore/stats_integration_test.go @@ -12,9 +12,9 @@ import ( ) func TestIntegration_GetAdminStats(t *testing.T) { - InitTestDB(t) + sqlStore := InitTestDB(t) query := models.GetAdminStatsQuery{} - err := GetAdminStats(context.Background(), &query) + err := sqlStore.GetAdminStats(context.Background(), &query) require.NoError(t, err) } diff --git a/pkg/services/sqlstore/stats_test.go b/pkg/services/sqlstore/stats_test.go index b390c8fdef7..758d91703dd 100644 --- a/pkg/services/sqlstore/stats_test.go +++ b/pkg/services/sqlstore/stats_test.go @@ -56,7 +56,7 @@ func TestStatsDataAccess(t *testing.T) { t.Run("Get admin stats should not result in error", func(t *testing.T) { query := models.GetAdminStatsQuery{} - err := GetAdminStats(context.Background(), &query) + err := sqlStore.GetAdminStats(context.Background(), &query) assert.NoError(t, err) }) } diff --git a/pkg/services/sqlstore/store.go b/pkg/services/sqlstore/store.go index 5c49dea0fae..c1a41110a85 100644 --- a/pkg/services/sqlstore/store.go +++ b/pkg/services/sqlstore/store.go @@ -8,6 +8,7 @@ import ( ) type Store interface { + GetAdminStats(ctx context.Context, query *models.GetAdminStatsQuery) error DeleteExpiredSnapshots(ctx context.Context, cmd *models.DeleteExpiredSnapshotsCommand) error CreateDashboardSnapshot(ctx context.Context, cmd *models.CreateDashboardSnapshotCommand) error DeleteDashboardSnapshot(ctx context.Context, cmd *models.DeleteDashboardSnapshotCommand) error