Migrate to Wire for dependency injection (#32289)

Fixes #30144 Co-authored-by: dsotirakis <sotirakis.dim@gmail.com> Co-authored-by: Marcus Efraimsson <marcus.efraimsson@gmail.com> Co-authored-by: Ida Furjesova <ida.furjesova@grafana.com> Co-authored-by: Jack Westbrook <jack.westbrook@gmail.com> Co-authored-by: Will Browne <wbrowne@users.noreply.github.com> Co-authored-by: Leon Sorokin <leeoniya@gmail.com> Co-authored-by: Andrej Ocenas <mr.ocenas@gmail.com> Co-authored-by: spinillos <selenepinillos@gmail.com> Co-authored-by: Karl Persson <kalle.persson@grafana.com> Co-authored-by: Leonard Gram <leo@xlson.com>
2021-08-25 15:11:22 +02:00
parent e61bc33163
commit 78596a6756
180 changed files with 2384 additions and 2401 deletions
@@ -8,6 +8,7 @@ import (
 	"path"
 	"path/filepath"
 	"strings"
+	"sync"
 	"time"

 	"github.com/go-sql-driver/mysql"
@@ -38,44 +39,62 @@ var (
 // ContextSessionKey is used as key to save values in `context.Context`
 type ContextSessionKey struct{}

-const ServiceName = "SqlStore"
-const InitPriority = registry.High
-
-func init() {
-	ss := &SQLStore{}
-	ss.Register()
-}
-
 type SQLStore struct {
-	Cfg          *setting.Cfg             `inject:""`
-	Bus          bus.Bus                  `inject:""`
-	CacheService *localcache.CacheService `inject:""`
+	Cfg          *setting.Cfg
+	Bus          bus.Bus
+	CacheService *localcache.CacheService

 	dbCfg                       DatabaseConfig
 	engine                      *xorm.Engine
 	log                         log.Logger
 	Dialect                     migrator.Dialect
 	skipEnsureDefaultOrgAndUser bool
+	migrations                  registry.DatabaseMigrator
 }

-// Register registers the SQLStore service with the DI system.
-func (ss *SQLStore) Register() {
+func ProvideService(cfg *setting.Cfg, cacheService *localcache.CacheService, bus bus.Bus, migrations registry.DatabaseMigrator) (*SQLStore, error) {
 	// This change will make xorm use an empty default schema for postgres and
 	// by that mimic the functionality of how it was functioning before
 	// xorm's changes above.
 	xorm.DefaultPostgresSchema = ""
+	s, err := newSQLStore(cfg, cacheService, bus, nil, migrations)
+	if err != nil {
+		return nil, err
+	}

-	registry.Register(&registry.Descriptor{
-		Name:         ServiceName,
-		Instance:     ss,
-		InitPriority: InitPriority,
-	})
+	if err := s.Migrate(); err != nil {
+		return nil, err
+	}
+
+	if err := s.Reset(); err != nil {
+		return nil, err
+	}
+
+	return s, nil
 }

-func (ss *SQLStore) Init() error {
-	ss.log = log.New("sqlstore")
-	if err := ss.initEngine(); err != nil {
-		return errutil.Wrap("failed to connect to database", err)
+func ProvideServiceForTests(migrations registry.DatabaseMigrator) (*SQLStore, error) {
+	return initTestDB(migrations, InitTestDBOpt{EnsureDefaultOrgAndUser: true})
+}
+
+func newSQLStore(cfg *setting.Cfg, cacheService *localcache.CacheService, bus bus.Bus, engine *xorm.Engine,
+	migrations registry.DatabaseMigrator, opts ...InitTestDBOpt) (*SQLStore, error) {
+	ss := &SQLStore{
+		Cfg:                         cfg,
+		Bus:                         bus,
+		CacheService:                cacheService,
+		log:                         log.New("sqlstore"),
+		skipEnsureDefaultOrgAndUser: false,
+		migrations:                  migrations,
+	}
+	for _, opt := range opts {
+		if !opt.EnsureDefaultOrgAndUser {
+			ss.skipEnsureDefaultOrgAndUser = true
+		}
+	}
+
+	if err := ss.initEngine(engine); err != nil {
+		return nil, errutil.Wrap("failed to connect to database", err)
 	}

 	ss.Dialect = migrator.NewDialect(ss.engine)
@@ -84,22 +103,6 @@ func (ss *SQLStore) Init() error {
 	x = ss.engine
 	dialect = ss.Dialect

-	if !ss.dbCfg.SkipMigrations {
-		migrator := migrator.NewMigrator(ss.engine, ss.Cfg)
-		migrations.AddMigrations(migrator)
-
-		for _, descriptor := range registry.GetServices() {
-			sc, ok := descriptor.Instance.(registry.DatabaseMigrator)
-			if ok {
-				sc.AddMigration(migrator)
-			}
-		}
-
-		if err := migrator.Start(); err != nil {
-			return err
-		}
-	}
-
 	// Init repo instances
 	annotations.SetRepository(&SQLAnnotationRepo{})
 	annotations.SetAnnotationCleaner(&AnnotationCleanupService{batchSize: ss.Cfg.AnnotationCleanupJobBatchSize, log: log.New("annotationcleaner")})
@@ -111,17 +114,32 @@ func (ss *SQLStore) Init() error {
 	ss.addPreferencesQueryAndCommandHandlers()
 	ss.addDashboardQueryAndCommandHandlers()

-	if err := ss.Reset(); err != nil {
-		return err
-	}
-	// Make sure the changes are synced, so they get shared with eventual other DB connections
-	if !ss.dbCfg.SkipMigrations {
-		if err := ss.Sync(); err != nil {
-			return err
-		}
+	// if err := ss.Reset(); err != nil {
+	// 	return nil, err
+	// }
+	// // Make sure the changes are synced, so they get shared with eventual other DB connections
+	// // XXX: Why is this only relevant when not skipping migrations?
+	// if !ss.dbCfg.SkipMigrations {
+	// 	if err := ss.Sync(); err != nil {
+	// 		return nil, err
+	// 	}
+	// }
+
+	return ss, nil
+}
+
+// Migrate performs database migrations.
+// Has to be done in a second phase (after initialization), since other services can register migrations during
+// the initialization phase.
+func (ss *SQLStore) Migrate() error {
+	if ss.dbCfg.SkipMigrations {
+		return nil
 	}

-	return nil
+	migrator := migrator.NewMigrator(ss.engine, ss.Cfg)
+	ss.migrations.AddMigration(migrator)
+
+	return migrator.Start()
 }

 // Sync syncs changes to the database.
@@ -278,7 +296,7 @@ func (ss *SQLStore) buildConnectionString() (string, error) {
 }

 // initEngine initializes ss.engine.
-func (ss *SQLStore) initEngine() error {
+func (ss *SQLStore) initEngine(engine *xorm.Engine) error {
 	if ss.engine != nil {
 		sqlog.Debug("Already connected to database")
 		return nil
@@ -323,9 +341,12 @@ func (ss *SQLStore) initEngine() error {
 			}
 		}
 	}
-	engine, err := xorm.NewEngine(ss.dbCfg.Type, connectionString)
-	if err != nil {
-		return err
+	if engine == nil {
+		var err error
+		engine, err = xorm.NewEngine(ss.dbCfg.Type, connectionString)
+		if err != nil {
+			return err
+		}
 	}

 	engine.SetMaxOpenConns(ss.dbCfg.MaxOpenConn)
@@ -406,6 +427,7 @@ type ITestDB interface {
 }

 var testSQLStore *SQLStore
+var testSQLStoreMutex sync.Mutex

 // InitTestDBOpt contains options for InitTestDB.
 type InitTestDBOpt struct {
@@ -413,98 +435,121 @@ type InitTestDBOpt struct {
 	EnsureDefaultOrgAndUser bool
 }

+// InitTestDBWithMigration initializes the test DB given custom migrations.
+func InitTestDBWithMigration(t ITestDB, migration registry.DatabaseMigrator, opts ...InitTestDBOpt) *SQLStore {
+	t.Helper()
+	store, err := initTestDB(migration, opts...)
+	if err != nil {
+		t.Fatalf("failed to initialize sql store: %s", err)
+	}
+	return store
+}
+
 // InitTestDB initializes the test DB.
 func InitTestDB(t ITestDB, opts ...InitTestDBOpt) *SQLStore {
 	t.Helper()
+	store, err := initTestDB(&migrations.OSSMigrations{}, opts...)
+	if err != nil {
+		t.Fatalf("failed to initialize sql store: %s", err)
+	}
+	return store
+}
+
+func initTestDB(migration registry.DatabaseMigrator, opts ...InitTestDBOpt) (*SQLStore, error) {
+	testSQLStoreMutex.Lock()
+	defer testSQLStoreMutex.Unlock()
 	if testSQLStore == nil {
-		testSQLStore = &SQLStore{}
-		testSQLStore.Bus = bus.New()
-		testSQLStore.CacheService = localcache.New(5*time.Minute, 10*time.Minute)
-		testSQLStore.skipEnsureDefaultOrgAndUser = true
-
-		for _, opt := range opts {
-			testSQLStore.skipEnsureDefaultOrgAndUser = !opt.EnsureDefaultOrgAndUser
-		}
-
 		dbType := migrator.SQLite

+		if len(opts) == 0 {
+			opts = []InitTestDBOpt{{EnsureDefaultOrgAndUser: false}}
+		}
+
 		// environment variable present for test db?
 		if db, present := os.LookupEnv("GRAFANA_TEST_DB"); present {
-			t.Logf("Using database type %q", db)
 			dbType = db
 		}

 		// set test db config
-		testSQLStore.Cfg = setting.NewCfg()
-		sec, err := testSQLStore.Cfg.Raw.NewSection("database")
+		cfg := setting.NewCfg()
+		sec, err := cfg.Raw.NewSection("database")
 		if err != nil {
-			t.Fatalf("Failed to create section: %s", err)
+			return nil, err
 		}
 		if _, err := sec.NewKey("type", dbType); err != nil {
-			t.Fatalf("Failed to create key: %s", err)
+			return nil, err
 		}

 		switch dbType {
 		case "mysql":
 			if _, err := sec.NewKey("connection_string", sqlutil.MySQLTestDB().ConnStr); err != nil {
-				t.Fatalf("Failed to create key: %s", err)
+				return nil, err
 			}
 		case "postgres":
 			if _, err := sec.NewKey("connection_string", sqlutil.PostgresTestDB().ConnStr); err != nil {
-				t.Fatalf("Failed to create key: %s", err)
+				return nil, err
 			}
 		default:
 			if _, err := sec.NewKey("connection_string", sqlutil.SQLite3TestDB().ConnStr); err != nil {
-				t.Fatalf("Failed to create key: %s", err)
+				return nil, err
 			}
 		}

 		// useful if you already have a database that you want to use for tests.
 		// cannot just set it on testSQLStore as it overrides the config in Init
 		if _, present := os.LookupEnv("SKIP_MIGRATIONS"); present {
-			t.Log("Skipping database migrations")
 			if _, err := sec.NewKey("skip_migrations", "true"); err != nil {
-				t.Fatalf("Failed to create key: %s", err)
+				return nil, err
 			}
 		}

 		// need to get engine to clean db before we init
-		t.Logf("Creating database connection: %q", sec.Key("connection_string"))
 		engine, err := xorm.NewEngine(dbType, sec.Key("connection_string").String())
 		if err != nil {
-			t.Fatalf("Failed to init test database: %v", err)
+			return nil, err
 		}

-		testSQLStore.Dialect = migrator.NewDialect(engine)
+		engine.DatabaseTZ = time.UTC
+		engine.TZLocation = time.UTC
+
+		testSQLStore, err = newSQLStore(cfg, localcache.New(5*time.Minute, 10*time.Minute), bus.GetBus(), engine, migration, opts...)
+		if err != nil {
+			return nil, err
+		}
+
+		if err := testSQLStore.Migrate(); err != nil {
+			return nil, err
+		}
+
+		if err := dialect.TruncateDBTables(); err != nil {
+			return nil, err
+		}
+
+		if err := testSQLStore.Reset(); err != nil {
+			return nil, err
+		}
+
+		// Make sure the changes are synced, so they get shared with eventual other DB connections
+		// XXX: Why is this only relevant when not skipping migrations?
+		if !testSQLStore.dbCfg.SkipMigrations {
+			if err := testSQLStore.Sync(); err != nil {
+				return nil, err
+			}
+		}

 		// temp global var until we get rid of global vars
 		dialect = testSQLStore.Dialect
-
-		t.Logf("Cleaning DB")
-		if err := dialect.CleanDB(); err != nil {
-			t.Fatalf("Failed to clean test db: %s", err)
-		}
-
-		if err := testSQLStore.Init(); err != nil {
-			t.Fatalf("Failed to init test database: %s", err)
-		}
-		t.Log("Successfully initialized test database")
-
-		testSQLStore.engine.DatabaseTZ = time.UTC
-		testSQLStore.engine.TZLocation = time.UTC
-
-		return testSQLStore
+		return testSQLStore, nil
 	}

-	t.Log("Truncating DB tables")
 	if err := dialect.TruncateDBTables(); err != nil {
-		t.Fatalf("Failed to truncate test db: %s", err)
+		return nil, err
 	}
 	if err := testSQLStore.Reset(); err != nil {
-		t.Fatalf("Failed to reset SQLStore: %s", err)
+		return nil, err
 	}

-	return testSQLStore
+	return testSQLStore, nil
 }

 func IsTestDbMySQL() bool {