From ad44164910ce4bc322d2b3e826f5125e649d3857 Mon Sep 17 00:00:00 2001 From: Ryan McKinley Date: Mon, 13 Nov 2023 06:22:22 -0800 Subject: [PATCH] Preferences: Remove SQLX backing store (#78015) --- pkg/services/preference/prefimpl/pref.go | 49 ++++++------- pkg/services/preference/prefimpl/pref_test.go | 40 +++++------ .../preference/prefimpl/sqlx_store.go | 68 ------------------- .../preference/prefimpl/sqlx_store_test.go | 16 ----- 4 files changed, 42 insertions(+), 131 deletions(-) delete mode 100644 pkg/services/preference/prefimpl/sqlx_store.go delete mode 100644 pkg/services/preference/prefimpl/sqlx_store_test.go diff --git a/pkg/services/preference/prefimpl/pref.go b/pkg/services/preference/prefimpl/pref.go index 976a01802a3..cf3bc80a9dc 100644 --- a/pkg/services/preference/prefimpl/pref.go +++ b/pkg/services/preference/prefimpl/pref.go @@ -6,32 +6,34 @@ import ( "time" "github.com/grafana/grafana/pkg/infra/db" - "github.com/grafana/grafana/pkg/services/featuremgmt" pref "github.com/grafana/grafana/pkg/services/preference" "github.com/grafana/grafana/pkg/setting" ) type Service struct { store store - cfg *setting.Cfg - features *featuremgmt.FeatureManager + defaults pref.Preference } -func ProvideService(db db.DB, cfg *setting.Cfg, features *featuremgmt.FeatureManager) pref.Service { - service := &Service{ - cfg: cfg, - features: features, - } - if features.IsEnabled(featuremgmt.FlagNewDBLibrary) { - service.store = &sqlxStore{ - sess: db.GetSqlxSession(), - } - } else { - service.store = &sqlStore{ +func ProvideService(db db.DB, cfg *setting.Cfg) pref.Service { + return &Service{ + store: &sqlStore{ db: db, - } + }, + defaults: prefsFromConfig(cfg), + } +} + +func prefsFromConfig(cfg *setting.Cfg) pref.Preference { + return pref.Preference{ + Theme: cfg.DefaultTheme, + Timezone: cfg.DateFormats.DefaultTimezone, + WeekStart: &cfg.DateFormats.DefaultWeekStart, + HomeDashboardID: 0, + JSONData: &pref.PreferenceJSONData{ + Language: cfg.DefaultLanguage, + }, } - return service } func (s *Service) GetWithDefaults(ctx context.Context, query *pref.GetPreferenceWithDefaultsQuery) (*pref.Preference, error) { @@ -217,16 +219,15 @@ func (s *Service) Patch(ctx context.Context, cmd *pref.PatchPreferenceCommand) e } func (s *Service) GetDefaults() *pref.Preference { - defaults := &pref.Preference{ - Theme: s.cfg.DefaultTheme, - Timezone: s.cfg.DateFormats.DefaultTimezone, - WeekStart: &s.cfg.DateFormats.DefaultWeekStart, + return &pref.Preference{ + Theme: s.defaults.Theme, + Timezone: s.defaults.Timezone, + WeekStart: s.defaults.WeekStart, HomeDashboardID: 0, - JSONData: &pref.PreferenceJSONData{}, + JSONData: &pref.PreferenceJSONData{ + Language: s.defaults.JSONData.Language, + }, } - defaults.JSONData.Language = s.cfg.DefaultLanguage - - return defaults } func (s *Service) DeleteByUser(ctx context.Context, userID int64) error { diff --git a/pkg/services/preference/prefimpl/pref_test.go b/pkg/services/preference/prefimpl/pref_test.go index de666af5be2..aea709d16cf 100644 --- a/pkg/services/preference/prefimpl/pref_test.go +++ b/pkg/services/preference/prefimpl/pref_test.go @@ -8,16 +8,15 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" - "github.com/grafana/grafana/pkg/services/featuremgmt" pref "github.com/grafana/grafana/pkg/services/preference" "github.com/grafana/grafana/pkg/setting" ) func TestGet_empty(t *testing.T) { + cfg := setting.NewCfg() prefService := &Service{ store: newFake(), - cfg: setting.NewCfg(), - features: featuremgmt.WithFeatures(), + defaults: prefsFromConfig(cfg), } preference, err := prefService.Get(context.Background(), &pref.GetPreferenceQuery{}) require.NoError(t, err) @@ -28,14 +27,15 @@ func TestGet_empty(t *testing.T) { } func TestGetDefaults(t *testing.T) { + cfg := setting.NewCfg() + cfg.DefaultLanguage = "en-US" + cfg.DefaultTheme = "light" + cfg.DateFormats.DefaultTimezone = "UTC" prefService := &Service{ store: newFake(), - cfg: setting.NewCfg(), - features: featuremgmt.WithFeatures(), + defaults: prefsFromConfig(cfg), } - prefService.cfg.DefaultLanguage = "en-US" - prefService.cfg.DefaultTheme = "light" - prefService.cfg.DateFormats.DefaultTimezone = "UTC" + weekStart := "" t.Run("GetDefaults", func(t *testing.T) { @@ -74,12 +74,12 @@ func TestGetDefaults(t *testing.T) { } func TestGetWithDefaults_withUserAndOrgPrefs(t *testing.T) { + cfg := setting.NewCfg() + cfg.DefaultLanguage = "en-US" prefService := &Service{ store: newFake(), - cfg: setting.NewCfg(), - features: featuremgmt.WithFeatures(), + defaults: prefsFromConfig(cfg), } - prefService.cfg.DefaultLanguage = "en-US" weekStartOne := "1" weekStartTwo := "2" @@ -163,8 +163,7 @@ func TestGetDefaults_JSONData(t *testing.T) { t.Run("users have precedence over org", func(t *testing.T) { prefService := &Service{ store: newFake(), - cfg: setting.NewCfg(), - features: featuremgmt.WithFeatures(), + defaults: prefsFromConfig(setting.NewCfg()), } insertPrefs(t, prefService.store, @@ -191,8 +190,7 @@ func TestGetDefaults_JSONData(t *testing.T) { t.Run("user JSONData with missing language does not override org preference", func(t *testing.T) { prefService := &Service{ store: newFake(), - cfg: setting.NewCfg(), - features: featuremgmt.WithFeatures(), + defaults: prefsFromConfig(setting.NewCfg()), } insertPrefs(t, prefService.store, @@ -222,8 +220,7 @@ func TestGetDefaults_JSONData(t *testing.T) { t.Run("teams have precedence over org and are read in ascending order", func(t *testing.T) { prefService := &Service{ store: newFake(), - cfg: setting.NewCfg(), - features: featuremgmt.WithFeatures(), + defaults: prefsFromConfig(setting.NewCfg()), } insertPrefs(t, prefService.store, @@ -260,8 +257,7 @@ func TestGetWithDefaults_teams(t *testing.T) { weekStartTwo := "2" prefService := &Service{ store: newFake(), - cfg: setting.NewCfg(), - features: featuremgmt.WithFeatures(), + defaults: prefsFromConfig(setting.NewCfg()), } insertPrefs(t, prefService.store, pref.Preference{ @@ -307,8 +303,7 @@ func TestGetWithDefaults_teams(t *testing.T) { func TestPatch_toCreate(t *testing.T) { prefService := &Service{ store: newFake(), - cfg: setting.NewCfg(), - features: featuremgmt.WithFeatures(), + defaults: prefsFromConfig(setting.NewCfg()), } themeValue := "light" @@ -328,8 +323,7 @@ func TestPatch_toCreate(t *testing.T) { func TestSave(t *testing.T) { prefService := &Service{ store: newFake(), - cfg: setting.NewCfg(), - features: featuremgmt.WithFeatures(), + defaults: prefsFromConfig(setting.NewCfg()), } t.Run("insert", func(t *testing.T) { diff --git a/pkg/services/preference/prefimpl/sqlx_store.go b/pkg/services/preference/prefimpl/sqlx_store.go deleted file mode 100644 index 224537e430f..00000000000 --- a/pkg/services/preference/prefimpl/sqlx_store.go +++ /dev/null @@ -1,68 +0,0 @@ -package prefimpl - -import ( - "context" - "database/sql" - "errors" - "fmt" - "strings" - - pref "github.com/grafana/grafana/pkg/services/preference" - "github.com/grafana/grafana/pkg/services/sqlstore/session" -) - -type sqlxStore struct { - sess *session.SessionDB -} - -func (s *sqlxStore) Get(ctx context.Context, query *pref.Preference) (*pref.Preference, error) { - var prefs pref.Preference - err := s.sess.Get(ctx, &prefs, "SELECT * from preferences WHERE org_id=? AND user_id=? AND team_id=?", query.OrgID, query.UserID, query.TeamID) - if err != nil && errors.Is(err, sql.ErrNoRows) { - return nil, pref.ErrPrefNotFound - } - return &prefs, err -} - -func (s *sqlxStore) List(ctx context.Context, query *pref.Preference) ([]*pref.Preference, error) { - prefs := make([]*pref.Preference, 0) - params := make([]any, 0) - filter := "" - - if len(query.Teams) > 0 { - filter = "(org_id=? AND team_id IN (?" + strings.Repeat(",?", len(query.Teams)-1) + ")) OR " - params = append(params, query.OrgID) - for _, v := range query.Teams { - params = append(params, v) - } - } - - filter += "(org_id=? AND user_id=? AND team_id=0) OR (org_id=? AND team_id=0 AND user_id=0)" - params = append(params, query.OrgID) - params = append(params, query.UserID) - params = append(params, query.OrgID) - err := s.sess.Select(ctx, &prefs, fmt.Sprintf("SELECT * FROM preferences WHERE %s ORDER BY user_id ASC, team_id ASC", filter), params...) - return prefs, err -} - -func (s *sqlxStore) Update(ctx context.Context, cmd *pref.Preference) error { - query := "UPDATE preferences SET org_id=:org_id, user_id=:user_id, team_id=:team_id, version=:version, home_dashboard_id=:home_dashboard_id, " + - "timezone=:timezone, week_start=:week_start, theme=:theme, created=:created, updated=:updated, json_data=:json_data WHERE id=:id" - _, err := s.sess.NamedExec(ctx, query, cmd) - return err -} - -func (s *sqlxStore) Insert(ctx context.Context, cmd *pref.Preference) (int64, error) { - var ID int64 - query := "INSERT INTO preferences (org_id, user_id, team_id, version, home_dashboard_id, timezone, week_start, theme, created, updated, json_data) VALUES " + - "(?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)" - ID, err := s.sess.ExecWithReturningId( - ctx, query, cmd.OrgID, cmd.UserID, cmd.TeamID, cmd.Version, cmd.HomeDashboardID, - cmd.Timezone, cmd.WeekStart, cmd.Theme, cmd.Created, cmd.Updated, cmd.JSONData) - return ID, err -} - -func (s *sqlxStore) DeleteByUser(ctx context.Context, userID int64) error { - _, err := s.sess.Exec(ctx, "DELETE FROM preferences WHERE user_id=?", userID) - return err -} diff --git a/pkg/services/preference/prefimpl/sqlx_store_test.go b/pkg/services/preference/prefimpl/sqlx_store_test.go deleted file mode 100644 index df6027e84ce..00000000000 --- a/pkg/services/preference/prefimpl/sqlx_store_test.go +++ /dev/null @@ -1,16 +0,0 @@ -package prefimpl - -import ( - "testing" - - "github.com/grafana/grafana/pkg/infra/db" -) - -func TestIntegrationSQLxPreferencesDataAccess(t *testing.T) { - if testing.Short() { - t.Skip("skipping integration test") - } - testIntegrationPreferencesDataAccess(t, func(ss db.DB) store { - return &sqlxStore{sess: ss.GetSqlxSession()} - }) -}