public/grafana
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Chore: Make Cfg field private in SQLStore (#85593)

Browse Source 
* make cfg private in sqlstore

* fix db init in tests

* fix case

* fix folder test init

* fix imports

* make another Cfg private

* remove another Cfg

* remove unused variable

* use store cfg, it has side-effects

* fix mutated cfg in tests
This commit is contained in:
Serge Zaitsev
2024-04-24 10:38:40 +02:00
committed by GitHub
parent f6e472f879
commit 522a98c126
50 changed files with 325 additions and 344 deletions
Download Patch File Download Diff File Expand all files Collapse all files
pkg/services/sqlstore/bulk_test.go
+1 -1
View File
@@ -64,7 +64,7 @@ func TestIntegrationBulkOps(t *testing.T) {
if testing.Short() {
t.Skip("skipping integration test")
}
db := InitTestDB(t)
db, _ := InitTestDB(t)
err := db.engine.Sync(new(bulkTestItem))
require.NoError(t, err)
pkg/services/sqlstore/permissions/dashboard_test.go
+3 -3
View File
@@ -697,13 +697,13 @@ func setupTest(t *testing.T, numFolders, numDashboards int, permissions []access
func setupNestedTest(t *testing.T, usr *user.SignedInUser, perms []accesscontrol.Permission, orgID int64, features featuremgmt.FeatureToggles) db.DB {
t.Helper()
db := sqlstore.InitTestDB(t)
db, cfg := db.InitTestDBWithCfg(t)
// dashboard store commands that should be called.
dashStore, err := database.ProvideDashboardStore(db, db.Cfg, features, tagimpl.ProvideService(db), quotatest.New(false, nil))
dashStore, err := database.ProvideDashboardStore(db, cfg, features, tagimpl.ProvideService(db), quotatest.New(false, nil))
require.NoError(t, err)
folderSvc := folderimpl.ProvideService(mock.New(), bus.ProvideBus(tracing.InitializeTracerForTest()), db.Cfg, dashStore, folderimpl.ProvideDashboardFolderStore(db), db, features, supportbundlestest.NewFakeBundleService(), nil)
folderSvc := folderimpl.ProvideService(mock.New(), bus.ProvideBus(tracing.InitializeTracerForTest()), cfg, dashStore, folderimpl.ProvideDashboardFolderStore(db), db, features, supportbundlestest.NewFakeBundleService(), nil)
// create parent folder
parent, err := folderSvc.Create(context.Background(), &folder.CreateFolderCommand{
pkg/services/sqlstore/permissions/dashboards_bench_test.go
+3 -3
View File
@@ -77,14 +77,14 @@ func setupBenchMark(b *testing.B, usr user.SignedInUser, features featuremgmt.Fe
nestingLevel = folder.MaxNestedFolderDepth
}
store := db.InitTestDB(b)
store, cfg := db.InitTestDBWithCfg(b)
quotaService := quotatest.New(false, nil)
dashboardWriteStore, err := database.ProvideDashboardStore(store, store.Cfg, features, tagimpl.ProvideService(store), quotaService)
dashboardWriteStore, err := database.ProvideDashboardStore(store, cfg, features, tagimpl.ProvideService(store), quotaService)
require.NoError(b, err)
folderSvc := folderimpl.ProvideService(mock.New(), bus.ProvideBus(tracing.InitializeTracerForTest()), store.Cfg, dashboardWriteStore, folderimpl.ProvideDashboardFolderStore(store), store, features, supportbundlestest.NewFakeBundleService(), nil)
folderSvc := folderimpl.ProvideService(mock.New(), bus.ProvideBus(tracing.InitializeTracerForTest()), cfg, dashboardWriteStore, folderimpl.ProvideDashboardFolderStore(store), store, features, supportbundlestest.NewFakeBundleService(), nil)
origNewGuardian := guardian.New
guardian.MockDashboardGuardian(&guardian.FakeDashboardGuardian{CanViewValue: true, CanSaveValue: true})
pkg/services/sqlstore/session_test.go
+2 -2
View File
@@ -12,7 +12,7 @@ import (
)
func TestRetryingDisabled(t *testing.T) {
store := InitTestDB(t)
store, _ := InitTestDB(t)
require.Equal(t, 0, store.dbCfg.QueryRetries)
funcToTest := map[string]func(ctx context.Context, callback DBTransactionFunc) error{
@@ -63,7 +63,7 @@ func TestRetryingDisabled(t *testing.T) {
}
func TestRetryingOnFailures(t *testing.T) {
store := InitTestDB(t)
store, _ := InitTestDB(t)
store.dbCfg.QueryRetries = 5
funcToTest := map[string]func(ctx context.Context, callback DBTransactionFunc) error{
pkg/services/sqlstore/sqlstore.go
+19 -19
View File
@@ -37,7 +37,7 @@ import (
type ContextSessionKey struct{}
type SQLStore struct {
Cfg *setting.Cfg
cfg *setting.Cfg
features featuremgmt.FeatureToggles
sqlxsession *session.SessionDB
@@ -45,7 +45,7 @@ type SQLStore struct {
dbCfg *DatabaseConfig
engine *xorm.Engine
log log.Logger
Dialect migrator.Dialect
dialect migrator.Dialect
skipEnsureDefaultOrgAndUser bool
migrations registry.DatabaseMigrator
tracer tracing.Tracer
@@ -114,7 +114,7 @@ func NewSQLStoreWithoutSideEffects(cfg *setting.Cfg,
func newSQLStore(cfg *setting.Cfg, engine *xorm.Engine,
migrations registry.DatabaseMigrator, bus bus.Bus, tracer tracing.Tracer, opts ...InitTestDBOpt) (*SQLStore, error) {
ss := &SQLStore{
Cfg: cfg,
cfg: cfg,
log: log.New("sqlstore"),
skipEnsureDefaultOrgAndUser: false,
migrations: migrations,
@@ -131,7 +131,7 @@ func newSQLStore(cfg *setting.Cfg, engine *xorm.Engine,
return nil, fmt.Errorf("%v: %w", "failed to connect to database", err)
}
ss.Dialect = migrator.NewDialect(ss.engine.DriverName())
ss.dialect = migrator.NewDialect(ss.engine.DriverName())
// if err := ss.Reset(); err != nil {
// return nil, err
@@ -155,7 +155,7 @@ func (ss *SQLStore) Migrate(isDatabaseLockingEnabled bool) error {
return nil
}
migrator := migrator.NewMigrator(ss.engine, ss.Cfg)
migrator := migrator.NewMigrator(ss.engine, ss.cfg)
ss.migrations.AddMigration(migrator)
return migrator.Start(isDatabaseLockingEnabled, ss.dbCfg.MigrationLockAttemptTimeout)
@@ -178,7 +178,7 @@ func (ss *SQLStore) Quote(value string) string {
// GetDialect return the dialect
func (ss *SQLStore) GetDialect() migrator.Dialect {
return ss.Dialect
return ss.dialect
}
func (ss *SQLStore) GetDBType() core.DbType {
@@ -210,7 +210,7 @@ func (ss *SQLStore) ensureMainOrgAndAdminUser(test bool) error {
var stats stats.SystemUserCountStats
// TODO: Should be able to rename "Count" to "count", for more standard SQL style
// Just have to make sure it gets deserialized properly into models.SystemUserCountStats
rawSQL := `SELECT COUNT(id) AS Count FROM ` + ss.Dialect.Quote("user")
rawSQL := `SELECT COUNT(id) AS Count FROM ` + ss.dialect.Quote("user")
if _, err := sess.SQL(rawSQL).Get(&stats); err != nil {
return fmt.Errorf("could not determine if admin user exists: %w", err)
}
@@ -220,19 +220,19 @@ func (ss *SQLStore) ensureMainOrgAndAdminUser(test bool) error {
}
// ensure admin user
if !ss.Cfg.DisableInitAdminCreation {
if !ss.cfg.DisableInitAdminCreation {
ss.log.Debug("Creating default admin user")
if _, err := ss.createUser(ctx, sess, user.CreateUserCommand{
Login: ss.Cfg.AdminUser,
Email: ss.Cfg.AdminEmail,
Password: user.Password(ss.Cfg.AdminPassword),
Login: ss.cfg.AdminUser,
Email: ss.cfg.AdminEmail,
Password: user.Password(ss.cfg.AdminPassword),
IsAdmin: true,
}); err != nil {
return fmt.Errorf("failed to create admin user: %s", err)
}
ss.log.Info("Created default admin", "user", ss.Cfg.AdminUser)
ss.log.Info("Created default admin", "user", ss.cfg.AdminUser)
}
ss.log.Debug("Creating default org", "name", mainOrgName)
@@ -254,14 +254,14 @@ func (ss *SQLStore) initEngine(engine *xorm.Engine) error {
return nil
}
dbCfg, err := NewDatabaseConfig(ss.Cfg, ss.features)
dbCfg, err := NewDatabaseConfig(ss.cfg, ss.features)
if err != nil {
return err
}
ss.dbCfg = dbCfg
if ss.Cfg.DatabaseInstrumentQueries {
if ss.cfg.DatabaseInstrumentQueries {
ss.dbCfg.Type = WrapDatabaseDriverWithHooks(ss.dbCfg.Type, ss.tracer)
}
@@ -316,7 +316,7 @@ func (ss *SQLStore) initEngine(engine *xorm.Engine) error {
engine.SetConnMaxLifetime(time.Second * time.Duration(ss.dbCfg.ConnMaxLifetime))
// configure sql logging
debugSQL := ss.Cfg.Raw.Section("database").Key("log_queries").MustBool(false)
debugSQL := ss.cfg.Raw.Section("database").Key("log_queries").MustBool(false)
if !debugSQL {
engine.SetLogger(&xorm.DiscardLogger{})
} else {
@@ -425,7 +425,7 @@ func InitTestDBWithMigration(t sqlutil.ITestDB, migration registry.DatabaseMigra
}
// InitTestDB initializes the test DB.
func InitTestDB(t sqlutil.ITestDB, opts ...InitTestDBOpt) *SQLStore {
func InitTestDB(t sqlutil.ITestDB, opts ...InitTestDBOpt) (*SQLStore, *setting.Cfg) {
t.Helper()
features := getFeaturesForTesting(opts...)
cfg := getCfgForTesting(opts...)
@@ -434,7 +434,7 @@ func InitTestDB(t sqlutil.ITestDB, opts ...InitTestDBOpt) *SQLStore {
if err != nil {
t.Fatalf("failed to initialize sql store: %s", err)
}
return store
return store, store.cfg
}
func SetupTestDB() {
@@ -605,9 +605,9 @@ func TestMain(m *testing.M) {
}
// nolint:staticcheck
testSQLStore.Cfg.IsFeatureToggleEnabled = features.IsEnabledGlobally
testSQLStore.cfg.IsFeatureToggleEnabled = features.IsEnabledGlobally
if err := testSQLStore.Dialect.TruncateDBTables(testSQLStore.GetEngine()); err != nil {
if err := testSQLStore.dialect.TruncateDBTables(testSQLStore.GetEngine()); err != nil {
return nil, err
}
if err := testSQLStore.Reset(); err != nil {
pkg/services/sqlstore/sqlstore_test.go
+6 -6
View File
@@ -20,7 +20,7 @@ func TestMain(m *testing.M) {
}
func TestIntegrationIsUniqueConstraintViolation(t *testing.T) {
store := InitTestDB(t)
store, _ := InitTestDB(t)
testCases := []struct {
desc string
@@ -32,12 +32,12 @@ func TestIntegrationIsUniqueConstraintViolation(t *testing.T) {
// Attempt to insert org with provided ID (primary key) twice
now := time.Now()
org := org.Org{Name: "test org primary key violation", Created: now, Updated: now, ID: 42}
err := sess.InsertId(&org, store.Dialect)
err := sess.InsertId(&org, store.dialect)
require.NoError(t, err)
// Provide a different name to avoid unique constraint violation
org.Name = "test org 2"
return sess.InsertId(&org, store.Dialect)
return sess.InsertId(&org, store.dialect)
},
},
{
@@ -46,12 +46,12 @@ func TestIntegrationIsUniqueConstraintViolation(t *testing.T) {
// Attempt to insert org with reserved name
now := time.Now()
org := org.Org{Name: "test org unique constrain violation", Created: now, Updated: now, ID: 43}
err := sess.InsertId(&org, store.Dialect)
err := sess.InsertId(&org, store.dialect)
require.NoError(t, err)
// Provide a different ID to avoid primary key violation
org.ID = 44
return sess.InsertId(&org, store.Dialect)
return sess.InsertId(&org, store.dialect)
},
},
}
@@ -62,7 +62,7 @@ func TestIntegrationIsUniqueConstraintViolation(t *testing.T) {
return tc.f(t, sess)
})
require.Error(t, err)
assert.True(t, store.Dialect.IsUniqueConstraintViolation(err))
assert.True(t, store.dialect.IsUniqueConstraintViolation(err))
})
}
}
pkg/services/sqlstore/transactions_test.go
+2 -2
View File
@@ -13,7 +13,7 @@ func TestIntegrationReuseSessionWithTransaction(t *testing.T) {
if testing.Short() {
t.Skip("skipping integration test")
}
ss := InitTestDB(t)
ss, _ := InitTestDB(t)
t.Run("top level transaction", func(t *testing.T) {
var outerSession *DBSession
@@ -73,7 +73,7 @@ func TestIntegrationPublishAfterCommitWithNestedTransactions(t *testing.T) {
t.Skip("skipping integration test")
}
ss := InitTestDB(t)
ss, _ := InitTestDB(t)
ctx := context.Background()
// On X success
pkg/services/sqlstore/user.go
+10 -10
View File
@@ -16,7 +16,7 @@ import (
const mainOrgName = "Main Org."
func (ss *SQLStore) getOrgIDForNewUser(sess *DBSession, args user.CreateUserCommand) (int64, error) {
if ss.Cfg.AutoAssignOrg && args.OrgID != 0 {
if ss.cfg.AutoAssignOrg && args.OrgID != 0 {
if err := verifyExistingOrg(sess, args.OrgID); err != nil {
return -1, err
}
@@ -114,11 +114,11 @@ func (ss *SQLStore) createUser(ctx context.Context, sess *DBSession, args user.C
Updated: time.Now(),
}
if ss.Cfg.AutoAssignOrg && !usr.IsAdmin {
if ss.cfg.AutoAssignOrg && !usr.IsAdmin {
if len(args.DefaultOrgRole) > 0 {
orgUser.Role = org.RoleType(args.DefaultOrgRole)
} else {
orgUser.Role = org.RoleType(ss.Cfg.AutoAssignOrgRole)
orgUser.Role = org.RoleType(ss.cfg.AutoAssignOrgRole)
}
}
@@ -144,8 +144,8 @@ func verifyExistingOrg(sess *DBSession, orgId int64) error {
func (ss *SQLStore) getOrCreateOrg(sess *DBSession, orgName string) (int64, error) {
var org org.Org
if ss.Cfg.AutoAssignOrg {
has, err := sess.Where("id=?", ss.Cfg.AutoAssignOrgId).Get(&org)
if ss.cfg.AutoAssignOrg {
has, err := sess.Where("id=?", ss.cfg.AutoAssignOrgId).Get(&org)
if err != nil {
return 0, err
}
@@ -154,18 +154,18 @@ func (ss *SQLStore) getOrCreateOrg(sess *DBSession, orgName string) (int64, erro
}
ss.log.Debug("auto assigned organization not found")
if ss.Cfg.AutoAssignOrgId != 1 {
if ss.cfg.AutoAssignOrgId != 1 {
ss.log.Error("Could not create user: organization ID does not exist", "orgID",
ss.Cfg.AutoAssignOrgId)
ss.cfg.AutoAssignOrgId)
return 0, fmt.Errorf("could not create user: organization ID %d does not exist",
ss.Cfg.AutoAssignOrgId)
ss.cfg.AutoAssignOrgId)
}
org.Name = mainOrgName
org.Created = time.Now()
org.Updated = org.Created
org.ID = int64(ss.Cfg.AutoAssignOrgId)
if err := sess.InsertId(&org, ss.Dialect); err != nil {
org.ID = int64(ss.cfg.AutoAssignOrgId)
if err := sess.InsertId(&org, ss.dialect); err != nil {
ss.log.Error("failed to insert organization with provided id", "org_id", org.ID, "err", err)
// ignore failure if for some reason the organization exists
if ss.GetDialect().IsUniqueConstraintViolation(err) {
pkg/services/sqlstore/user_test.go
+4 -4
View File
@@ -14,13 +14,13 @@ func TestIntegrationGetOrCreateOrg(t *testing.T) {
if testing.Short() {
t.Skip("Skipping integration test in short mode")
}
ss := InitTestDB(t)
ss, _ := InitTestDB(t)
err := ss.WithNewDbSession(context.Background(), func(sess *DBSession) error {
// Create the org only:
ss.Cfg.AutoAssignOrg = true
ss.Cfg.DisableInitAdminCreation = true
ss.Cfg.AutoAssignOrgId = 1
ss.cfg.AutoAssignOrg = true
ss.cfg.DisableInitAdminCreation = true
ss.cfg.AutoAssignOrgId = 1
createdOrgID, err := ss.getOrCreateOrg(sess, mainOrgName)
require.NoError(t, err)
require.Equal(t, int64(1), createdOrgID)