From 675e946b4037b441ec746ad9abb061b8ebaff466 Mon Sep 17 00:00:00 2001 From: Ryan McKinley Date: Sun, 12 Nov 2023 09:29:14 -0800 Subject: [PATCH] SQLX: Remove two unreachable services (#78016) --- pkg/services/apikey/apikeyimpl/apikey.go | 11 +- pkg/services/apikey/apikeyimpl/sqlx_store.go | 184 ------------------ .../apikey/apikeyimpl/sqlx_store_test.go | 17 -- pkg/services/apikey/apikeyimpl/store_test.go | 9 +- pkg/services/apikey/apikeyimpl/xorm_store.go | 4 +- .../apikey/apikeyimpl/xorm_store_test.go | 5 +- .../dashverimpl/sqlx_store.go | 86 -------- .../dashverimpl/sqlx_store_test.go | 18 -- 8 files changed, 9 insertions(+), 325 deletions(-) delete mode 100644 pkg/services/apikey/apikeyimpl/sqlx_store.go delete mode 100644 pkg/services/apikey/apikeyimpl/sqlx_store_test.go delete mode 100644 pkg/services/dashboardversion/dashverimpl/sqlx_store.go delete mode 100644 pkg/services/dashboardversion/dashverimpl/sqlx_store_test.go diff --git a/pkg/services/apikey/apikeyimpl/apikey.go b/pkg/services/apikey/apikeyimpl/apikey.go index 89addfc3beb..9cd6ccbb784 100644 --- a/pkg/services/apikey/apikeyimpl/apikey.go +++ b/pkg/services/apikey/apikeyimpl/apikey.go @@ -5,7 +5,6 @@ import ( "github.com/grafana/grafana/pkg/infra/db" "github.com/grafana/grafana/pkg/services/apikey" - "github.com/grafana/grafana/pkg/services/featuremgmt" "github.com/grafana/grafana/pkg/services/quota" "github.com/grafana/grafana/pkg/setting" ) @@ -15,15 +14,9 @@ type Service struct { } func ProvideService(db db.DB, cfg *setting.Cfg, quotaService quota.Service) (apikey.Service, error) { - s := &Service{} - if cfg.IsFeatureToggleEnabled(featuremgmt.FlagNewDBLibrary) { - s.store = &sqlxStore{ - sess: db.GetSqlxSession(), - cfg: cfg, - } + s := &Service{ + store: &sqlStore{db: db}, } - s.store = &sqlStore{db: db, cfg: cfg} - defaultLimits, err := readQuotaConfig(cfg) if err != nil { return s, err diff --git a/pkg/services/apikey/apikeyimpl/sqlx_store.go b/pkg/services/apikey/apikeyimpl/sqlx_store.go deleted file mode 100644 index 07e2ddf4018..00000000000 --- a/pkg/services/apikey/apikeyimpl/sqlx_store.go +++ /dev/null @@ -1,184 +0,0 @@ -package apikeyimpl - -import ( - "context" - "database/sql" - "errors" - "fmt" - "strings" - "time" - - "github.com/grafana/grafana/pkg/services/accesscontrol" - "github.com/grafana/grafana/pkg/services/apikey" - "github.com/grafana/grafana/pkg/services/quota" - "github.com/grafana/grafana/pkg/services/sqlstore/session" - "github.com/grafana/grafana/pkg/setting" -) - -type sqlxStore struct { - sess *session.SessionDB - cfg *setting.Cfg -} - -func (ss *sqlxStore) GetAPIKeys(ctx context.Context, query *apikey.GetApiKeysQuery) ([]*apikey.APIKey, error) { - var where []string - var args []any - - if query.IncludeExpired { - where = append(where, "org_id=?") - args = append(args, query.OrgID) - } else { - where = append(where, "org_id=? and ( expires IS NULL or expires >= ?)") - args = append(args, query.OrgID, timeNow().Unix()) - } - - where = append(where, "service_account_id IS NULL") - - filter, err := accesscontrol.Filter(query.User, "id", "apikeys:id:", accesscontrol.ActionAPIKeyRead) - if err != nil { - return nil, err - } - where = append(where, filter.Where) - args = append(args, filter.Args...) - - ws := fmt.Sprint(strings.Join(where[:], " AND ")) - qr := fmt.Sprintf(`SELECT * FROM api_key WHERE %s ORDER BY name ASC LIMIT 100`, ws) - keys := make([]*apikey.APIKey, 0) - err = ss.sess.Select(ctx, &keys, qr, args...) - return keys, err -} - -func (ss *sqlxStore) GetAllAPIKeys(ctx context.Context, orgID int64) ([]*apikey.APIKey, error) { - result := make([]*apikey.APIKey, 0) - var err error - if orgID != -1 { - err = ss.sess.Select( - ctx, &result, "SELECT * FROM api_key WHERE service_account_id IS NULL AND org_id = ? ORDER BY name ASC", orgID) - } else { - err = ss.sess.Select( - ctx, &result, "SELECT * FROM api_key WHERE service_account_id IS NULL ORDER BY name ASC") - } - return result, err -} - -func (ss *sqlxStore) CountAPIKeys(ctx context.Context, orgID int64) (int64, error) { - type result struct { - Count int64 - } - r := result{} - err := ss.sess.Get(ctx, &r, `SELECT COUNT(*) AS count FROM api_key WHERE service_account_id IS NULL and org_id = ?`, orgID) - if err != nil { - return 0, err - } - return r.Count, err -} - -func (ss *sqlxStore) DeleteApiKey(ctx context.Context, cmd *apikey.DeleteCommand) error { - res, err := ss.sess.Exec(ctx, "DELETE FROM api_key WHERE id=? and org_id=? and service_account_id IS NULL", cmd.ID, cmd.OrgID) - if err != nil { - return err - } - n, err := res.RowsAffected() - if err == nil && n == 0 { - return apikey.ErrNotFound - } - return err -} - -func (ss *sqlxStore) AddAPIKey(ctx context.Context, cmd *apikey.AddCommand) (*apikey.APIKey, error) { - updated := timeNow() - var expires *int64 = nil - if cmd.SecondsToLive > 0 { - v := updated.Add(time.Second * time.Duration(cmd.SecondsToLive)).Unix() - expires = &v - } else if cmd.SecondsToLive < 0 { - return nil, apikey.ErrInvalidExpiration - } - - _, err := ss.GetApiKeyByName(ctx, &apikey.GetByNameQuery{OrgID: cmd.OrgID, KeyName: cmd.Name}) - // If key with the same orgId and name already exist return err - if !errors.Is(err, apikey.ErrInvalid) { - return nil, apikey.ErrDuplicate - } - isRevoked := false - t := apikey.APIKey{ - OrgID: cmd.OrgID, - Name: cmd.Name, - Role: cmd.Role, - Key: cmd.Key, - Created: updated, - Updated: updated, - Expires: expires, - ServiceAccountId: nil, - IsRevoked: &isRevoked, - } - - t.ID, err = ss.sess.ExecWithReturningId(ctx, - `INSERT INTO api_key (org_id, name, role, "key", created, updated, expires, service_account_id, is_revoked) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?)`, t.OrgID, t.Name, t.Role, t.Key, t.Created, t.Updated, t.Expires, t.ServiceAccountId, t.IsRevoked) - return &t, err -} - -func (ss *sqlxStore) GetApiKeyById(ctx context.Context, query *apikey.GetByIDQuery) (*apikey.APIKey, error) { - var key apikey.APIKey - err := ss.sess.Get(ctx, &key, "SELECT * FROM api_key WHERE id=?", query.ApiKeyID) - if err != nil && errors.Is(err, sql.ErrNoRows) { - return nil, apikey.ErrInvalid - } - return &key, err -} - -func (ss *sqlxStore) GetApiKeyByName(ctx context.Context, query *apikey.GetByNameQuery) (*apikey.APIKey, error) { - var key apikey.APIKey - err := ss.sess.Get(ctx, &key, "SELECT * FROM api_key WHERE org_id=? AND name=?", query.OrgID, query.KeyName) - if err != nil && errors.Is(err, sql.ErrNoRows) { - return nil, apikey.ErrInvalid - } - return &key, err -} - -func (ss *sqlxStore) GetAPIKeyByHash(ctx context.Context, hash string) (*apikey.APIKey, error) { - var key apikey.APIKey - err := ss.sess.Get(ctx, &key, `SELECT * FROM api_key WHERE "key"=?`, hash) - if err != nil && errors.Is(err, sql.ErrNoRows) { - return nil, apikey.ErrInvalid - } - return &key, err -} - -func (ss *sqlxStore) UpdateAPIKeyLastUsedDate(ctx context.Context, tokenID int64) error { - now := timeNow() - _, err := ss.sess.Exec(ctx, `UPDATE api_key SET last_used_at=? WHERE id=?`, &now, tokenID) - return err -} - -func (ss *sqlxStore) Count(ctx context.Context, scopeParams *quota.ScopeParameters) (*quota.Map, error) { - u := "a.Map{} - type result struct { - Count int64 - } - - r := result{} - if err := ss.sess.Get(ctx, &r, `SELECT COUNT(*) AS count FROM api_key`); err != nil { - return u, err - } else { - tag, err := quota.NewTag(apikey.QuotaTargetSrv, apikey.QuotaTarget, quota.GlobalScope) - if err != nil { - return nil, err - } - u.Set(tag, r.Count) - } - - if scopeParams != nil && scopeParams.OrgID != 0 { - if err := ss.sess.Get(ctx, &r, `SELECT COUNT(*) AS count FROM api_key WHERE org_id = ?`, scopeParams.OrgID); err != nil { - return u, err - } else { - tag, err := quota.NewTag(apikey.QuotaTargetSrv, apikey.QuotaTarget, quota.OrgScope) - if err != nil { - return nil, err - } - u.Set(tag, r.Count) - } - } - - return u, nil -} diff --git a/pkg/services/apikey/apikeyimpl/sqlx_store_test.go b/pkg/services/apikey/apikeyimpl/sqlx_store_test.go deleted file mode 100644 index 42435ecfc58..00000000000 --- a/pkg/services/apikey/apikeyimpl/sqlx_store_test.go +++ /dev/null @@ -1,17 +0,0 @@ -package apikeyimpl - -import ( - "testing" - - "github.com/grafana/grafana/pkg/infra/db" - "github.com/grafana/grafana/pkg/setting" -) - -func TestIntegrationSQLxApiKeyDataAccess(t *testing.T) { - if testing.Short() { - t.Skip("skipping integration test") - } - testIntegrationApiKeyDataAccess(t, func(ss db.DB, cfg *setting.Cfg) store { - return &sqlxStore{sess: ss.GetSqlxSession(), cfg: cfg} - }) -} diff --git a/pkg/services/apikey/apikeyimpl/store_test.go b/pkg/services/apikey/apikeyimpl/store_test.go index 6cb6ab1b99c..925e418d404 100644 --- a/pkg/services/apikey/apikeyimpl/store_test.go +++ b/pkg/services/apikey/apikeyimpl/store_test.go @@ -14,10 +14,9 @@ import ( "github.com/grafana/grafana/pkg/services/apikey" "github.com/grafana/grafana/pkg/services/auth/identity" "github.com/grafana/grafana/pkg/services/user" - "github.com/grafana/grafana/pkg/setting" ) -type getStore func(db.DB, *setting.Cfg) store +type getStore func(db.DB) store type getApiKeysTestCase struct { desc string @@ -61,7 +60,7 @@ func testIntegrationApiKeyDataAccess(t *testing.T, fn getStore) { t.Run("Testing API Key data access", func(t *testing.T) { db := db.InitTestDB(t) - ss := fn(db, db.Cfg) + ss := fn(db) t.Run("Given saved api key", func(t *testing.T) { cmd := apikey.AddCommand{OrgID: 1, Name: "hello", Key: "asd"} @@ -200,7 +199,7 @@ func testIntegrationApiKeyDataAccess(t *testing.T, fn getStore) { t.Run("Testing API Key errors", func(t *testing.T) { db := db.InitTestDB(t) - ss := fn(db, db.Cfg) + ss := fn(db) t.Run("Delete non-existing key should return error", func(t *testing.T) { cmd := apikey.DeleteCommand{ID: 1} @@ -255,7 +254,7 @@ func testIntegrationApiKeyDataAccess(t *testing.T, fn getStore) { for _, tt := range tests { t.Run(tt.desc, func(t *testing.T) { db := db.InitTestDB(t, db.InitTestDBOpt{}) - store := fn(db, db.Cfg) + store := fn(db) seedApiKeys(t, store, 10) query := &apikey.GetApiKeysQuery{OrgID: 1, User: tt.user} diff --git a/pkg/services/apikey/apikeyimpl/xorm_store.go b/pkg/services/apikey/apikeyimpl/xorm_store.go index 11c9bd8468a..cef11298473 100644 --- a/pkg/services/apikey/apikeyimpl/xorm_store.go +++ b/pkg/services/apikey/apikeyimpl/xorm_store.go @@ -12,12 +12,10 @@ import ( "github.com/grafana/grafana/pkg/services/apikey" "github.com/grafana/grafana/pkg/services/quota" "github.com/grafana/grafana/pkg/services/sqlstore" - "github.com/grafana/grafana/pkg/setting" ) type sqlStore struct { - db db.DB - cfg *setting.Cfg + db db.DB } // timeNow makes it possible to test usage of time diff --git a/pkg/services/apikey/apikeyimpl/xorm_store_test.go b/pkg/services/apikey/apikeyimpl/xorm_store_test.go index e0b3d268497..06cd3bee148 100644 --- a/pkg/services/apikey/apikeyimpl/xorm_store_test.go +++ b/pkg/services/apikey/apikeyimpl/xorm_store_test.go @@ -4,14 +4,13 @@ import ( "testing" "github.com/grafana/grafana/pkg/infra/db" - "github.com/grafana/grafana/pkg/setting" ) func TestIntegrationXORMApiKeyDataAccess(t *testing.T) { if testing.Short() { t.Skip("skipping integration test") } - testIntegrationApiKeyDataAccess(t, func(ss db.DB, cfg *setting.Cfg) store { - return &sqlStore{db: ss, cfg: cfg} + testIntegrationApiKeyDataAccess(t, func(ss db.DB) store { + return &sqlStore{db: ss} }) } diff --git a/pkg/services/dashboardversion/dashverimpl/sqlx_store.go b/pkg/services/dashboardversion/dashverimpl/sqlx_store.go deleted file mode 100644 index 10be3cb6962..00000000000 --- a/pkg/services/dashboardversion/dashverimpl/sqlx_store.go +++ /dev/null @@ -1,86 +0,0 @@ -package dashverimpl - -import ( - "context" - "database/sql" - "errors" - "strings" - - dashver "github.com/grafana/grafana/pkg/services/dashboardversion" - "github.com/grafana/grafana/pkg/services/sqlstore/session" -) - -type sqlxStore struct { - sess *session.SessionDB -} - -func (ss *sqlxStore) Get(ctx context.Context, query *dashver.GetDashboardVersionQuery) (*dashver.DashboardVersion, error) { - var version dashver.DashboardVersion - qr := `SELECT dashboard_version.* - FROM dashboard_version - LEFT JOIN dashboard ON dashboard.id=dashboard_version.dashboard_id - WHERE dashboard_version.dashboard_id=? AND dashboard_version.version=? AND dashboard.org_id=? - ` - err := ss.sess.Get(ctx, &version, qr, query.DashboardID, query.Version, query.OrgID) - if err != nil && errors.Is(err, sql.ErrNoRows) { - return nil, dashver.ErrDashboardVersionNotFound - } - return &version, err -} - -func (ss *sqlxStore) GetBatch(ctx context.Context, cmd *dashver.DeleteExpiredVersionsCommand, perBatch int, versionsToKeep int) ([]any, error) { - var versionIds []any - versionIdsToDeleteQuery := `SELECT id - FROM dashboard_version, ( - SELECT dashboard_id, count(version) as count, min(version) as min - FROM dashboard_version - GROUP BY dashboard_id - ) AS vtd - WHERE dashboard_version.dashboard_id=vtd.dashboard_id - AND version < vtd.min + vtd.count - ? - LIMIT ?` - err := ss.sess.Get(ctx, &versionIds, versionIdsToDeleteQuery, versionsToKeep, perBatch) - return versionIds, err -} - -// This service is used by cleanup which need to belong to the same transaction -// Here we need to make sure that the transaction is shared between services -func (ss *sqlxStore) DeleteBatch(ctx context.Context, cmd *dashver.DeleteExpiredVersionsCommand, versionIdsToDelete []any) (int64, error) { - var deleted int64 - err := ss.sess.WithTransaction(ctx, func(tx *session.SessionTx) error { - deleteExpiredSQL := `DELETE FROM dashboard_version WHERE id IN (?` + strings.Repeat(",?", len(versionIdsToDelete)-1) + `)` - expiredResponse, err := tx.Exec(ctx, deleteExpiredSQL, versionIdsToDelete...) - if err != nil { - return err - } - deleted, err = expiredResponse.RowsAffected() - return err - }) - return deleted, err -} - -func (ss *sqlxStore) List(ctx context.Context, query *dashver.ListDashboardVersionsQuery) ([]*dashver.DashboardVersion, error) { - var dashboardVersion []*dashver.DashboardVersion - qr := `SELECT dashboard_version.id, - dashboard_version.dashboard_id, - dashboard_version.parent_version, - dashboard_version.restored_from, - dashboard_version.version, - dashboard_version.created, - dashboard_version.created_by, - dashboard_version.message - FROM dashboard_version - LEFT JOIN dashboard ON dashboard.id = dashboard_version.dashboard_id - WHERE dashboard_version.dashboard_id=? AND dashboard.org_id=? - ORDER BY dashboard_version.version DESC - LIMIT ? OFFSET ?` - - err := ss.sess.Select(ctx, &dashboardVersion, qr, query.DashboardID, query.OrgID, query.Limit, query.Start) - if err != nil { - return nil, err - } - if len(dashboardVersion) < 1 { - return nil, dashver.ErrNoVersionsForDashboardID - } - return dashboardVersion, nil -} diff --git a/pkg/services/dashboardversion/dashverimpl/sqlx_store_test.go b/pkg/services/dashboardversion/dashverimpl/sqlx_store_test.go deleted file mode 100644 index 59ffe69e946..00000000000 --- a/pkg/services/dashboardversion/dashverimpl/sqlx_store_test.go +++ /dev/null @@ -1,18 +0,0 @@ -package dashverimpl - -import ( - "testing" - - "github.com/grafana/grafana/pkg/infra/db" -) - -func TestIntegrationSQLxGetDashboardVersion(t *testing.T) { - if testing.Short() { - t.Skip("skipping integration test") - } - testIntegrationGetDashboardVersion(t, func(ss db.DB) store { - return &sqlxStore{ - sess: ss.GetSqlxSession(), - } - }) -}