Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

morph.go: ban global variables #34

Merged
merged 5 commits into from
Jan 13, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 2 additions & 9 deletions drivers/mysql/mysql.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,13 +16,6 @@ import (

const driverName = "mysql"
const defaultMigrationMaxSize = 10 * 1 << 20 // 10 MB
var defaultConfig = &Config{
Config: drivers.Config{
MigrationsTable: "db_migrations",
StatementTimeoutInSecs: 60,
MigrationMaxSize: defaultMigrationMaxSize,
},
}

// add here any custom driver configuration
var configParams = []string{
Expand All @@ -44,7 +37,7 @@ type mysql struct {
}

func WithInstance(dbInstance *sql.DB, config *Config) (drivers.Driver, error) {
driverConfig := mergeConfigs(config, defaultConfig)
driverConfig := mergeConfigs(config, getDefaultConfig())

conn, err := dbInstance.Conn(context.Background())
if err != nil {
Expand All @@ -69,7 +62,7 @@ func Open(connURL string) (drivers.Driver, error) {
return nil, &drivers.AppError{Driver: driverName, OrigErr: err, Message: "failed to sanitize url from custom parameters"}
}

driverConfig, err := mergeConfigWithParams(customParams, defaultConfig)
driverConfig, err := mergeConfigWithParams(customParams, getDefaultConfig())
if err != nil {
return nil, &drivers.AppError{Driver: driverName, OrigErr: err, Message: "failed to merge custom params to driver config"}
}
Expand Down
23 changes: 21 additions & 2 deletions drivers/mysql/mysql_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -95,11 +95,20 @@ func (suite *MysqlTestSuite) TestOpen() {
})

suite.T().Run("when connURL is valid and bare uses default configuration", func(t *testing.T) {
connectedDriver, teardown := suite.InitializeDriver(testConnURL)
connectedDriver, teardown := suite.InitializeDriver(defaultConnURL)
agnivade marked this conversation as resolved.
Show resolved Hide resolved
defer teardown()

defaultConfig := getDefaultConfig()
cfg := &Config{
Config: drivers.Config{
MigrationsTable: defaultConfig.MigrationsTable,
StatementTimeoutInSecs: defaultConfig.StatementTimeoutInSecs,
MigrationMaxSize: defaultConfig.MigrationMaxSize,
},
closeDBonClose: true, // we have created DB from DSN
}
mysqlDriver := connectedDriver.(*mysql)
suite.Assert().EqualValues(defaultConfig, mysqlDriver.config)
suite.Assert().EqualValues(cfg, mysqlDriver.config)
agnivade marked this conversation as resolved.
Show resolved Hide resolved
})

suite.T().Run("when connURL is valid can override migrations table", func(t *testing.T) {
Expand Down Expand Up @@ -136,6 +145,8 @@ func (suite *MysqlTestSuite) TestOpen() {
}

func (suite *MysqlTestSuite) TestCreateSchemaTableIfNotExists() {
defaultConfig := getDefaultConfig()

suite.T().Run("it errors when connection is missing", func(t *testing.T) {
driver := &mysql{}

Expand Down Expand Up @@ -186,6 +197,8 @@ func (suite *MysqlTestSuite) TestCreateSchemaTableIfNotExists() {
}

func (suite *MysqlTestSuite) TestLock() {
defaultConfig := getDefaultConfig()

connectedDriver, teardown := suite.InitializeDriver(testConnURL)
defer teardown()

Expand All @@ -203,6 +216,8 @@ func (suite *MysqlTestSuite) TestLock() {
}

func (suite *MysqlTestSuite) TestUnlock() {
defaultConfig := getDefaultConfig()

connectedDriver, teardown := suite.InitializeDriver(testConnURL)
defer teardown()

Expand All @@ -222,6 +237,8 @@ func (suite *MysqlTestSuite) TestUnlock() {
}

func (suite *MysqlTestSuite) TestAppliedMigrations() {
defaultConfig := getDefaultConfig()

connectedDriver, teardown := suite.InitializeDriver(testConnURL)
defer teardown()

Expand All @@ -244,6 +261,8 @@ func (suite *MysqlTestSuite) TestAppliedMigrations() {
}

func (suite *MysqlTestSuite) TestApply() {
defaultConfig := getDefaultConfig()

testData := []struct {
Scenario string
PendingMigrations []*models.Migration
Expand Down
11 changes: 11 additions & 0 deletions drivers/mysql/utils.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package mysql

import (
"github.com/go-morph/morph/drivers"
mysqlDriver "github.com/go-sql-driver/mysql"
)

Expand All @@ -21,3 +22,13 @@ func extractDatabaseNameFromURL(conn string) (string, error) {

return cfg.DBName, nil
}

func getDefaultConfig() *Config {
return &Config{
Config: drivers.Config{
MigrationsTable: "db_migrations",
StatementTimeoutInSecs: 60,
MigrationMaxSize: defaultMigrationMaxSize,
},
}
}
13 changes: 3 additions & 10 deletions drivers/postgres/postgres.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,14 +14,7 @@ import (
)

var (
driverName = "postgres"
defaultConfig = &Config{
Config: drivers.Config{
MigrationsTable: "db_migrations",
StatementTimeoutInSecs: 60,
MigrationMaxSize: defaultMigrationMaxSize,
},
}
driverName = "postgres"
defaultMigrationMaxSize = 10 * 1 << 20 // 10 MB
configParams = []string{
"x-migration-max-size",
Expand All @@ -44,7 +37,7 @@ type postgres struct {
}

func WithInstance(dbInstance *sql.DB, config *Config) (drivers.Driver, error) {
driverConfig := mergeConfigs(config, defaultConfig)
driverConfig := mergeConfigs(config, getDefaultConfig())

conn, err := dbInstance.Conn(context.Background())
if err != nil {
Expand Down Expand Up @@ -77,7 +70,7 @@ func Open(connURL string) (drivers.Driver, error) {
return nil, &drivers.AppError{Driver: driverName, OrigErr: err, Message: "failed to sanitize url from custom parameters"}
}

driverConfig, err := mergeConfigWithParams(customParams, defaultConfig)
driverConfig, err := mergeConfigWithParams(customParams, getDefaultConfig())
if err != nil {
return nil, &drivers.AppError{Driver: driverName, OrigErr: err, Message: "failed to merge custom params to driver config"}
}
Expand Down
20 changes: 19 additions & 1 deletion drivers/postgres/postgres_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -91,8 +91,20 @@ func (suite *PostgresTestSuite) TestOpen() {
connectedDriver, teardown := suite.InitializeDriver(testConnURL)
defer teardown()

defaultConfig := getDefaultConfig()
cfg := &Config{
Config: drivers.Config{
MigrationsTable: defaultConfig.MigrationsTable,
StatementTimeoutInSecs: defaultConfig.StatementTimeoutInSecs,
MigrationMaxSize: defaultConfig.MigrationMaxSize,
},
databaseName: databaseName,
schemaName: "public",
closeDBonClose: true, // we have created DB from DSN
}

pgDriver := connectedDriver.(*postgres)
suite.Assert().EqualValues(defaultConfig, pgDriver.config)
suite.Assert().EqualValues(cfg, pgDriver.config)
})

suite.T().Run("when connURL is valid can override migrations table", func(t *testing.T) {
Expand Down Expand Up @@ -130,6 +142,8 @@ func (suite *PostgresTestSuite) TestOpen() {
}

func (suite *PostgresTestSuite) TestCreateSchemaTableIfNotExists() {
defaultConfig := getDefaultConfig()

suite.T().Run("it errors when connection is missing", func(t *testing.T) {
driver := &postgres{}

Expand Down Expand Up @@ -225,6 +239,8 @@ func (suite *PostgresTestSuite) TestUnlock() {
}

func (suite *PostgresTestSuite) TestAppliedMigrations() {
defaultConfig := getDefaultConfig()

connectedDriver, teardown := suite.InitializeDriver(testConnURL)
defer teardown()

Expand All @@ -247,6 +263,8 @@ func (suite *PostgresTestSuite) TestAppliedMigrations() {
}

func (suite *PostgresTestSuite) TestApply() {
defaultConfig := getDefaultConfig()

testData := []struct {
Scenario string
PendingMigrations []*models.Migration
Expand Down
16 changes: 15 additions & 1 deletion drivers/postgres/utils.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,10 @@
package postgres

import "net/url"
import (
"net/url"

"github.com/go-morph/morph/drivers"
)

func extractDatabaseNameFromURL(URL string) (string, error) {
uri, err := url.Parse(URL)
Expand All @@ -10,3 +14,13 @@ func extractDatabaseNameFromURL(URL string) (string, error) {

return uri.Path[1:], nil
}

func getDefaultConfig() *Config {
return &Config{
Config: drivers.Config{
MigrationsTable: "db_migrations",
StatementTimeoutInSecs: 60,
MigrationMaxSize: defaultMigrationMaxSize,
},
}
}
8 changes: 3 additions & 5 deletions morph.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,10 +42,6 @@ type Config struct {

type EngineOption func(*Morph)

var defaultConfig = &Config{
Logger: log.New(os.Stderr, "", log.LstdFlags), // add default logger
}

func WithLogger(logger *log.Logger) EngineOption {
return func(m *Morph) {
m.config.Logger = logger
Expand Down Expand Up @@ -82,7 +78,9 @@ func WithLock(key string) EngineOption {
// New creates a new instance of the migrations engine from an existing db instance and a migrations source.
func New(ctx context.Context, driver drivers.Driver, source sources.Source, options ...EngineOption) (*Morph, error) {
engine := &Morph{
config: defaultConfig,
config: &Config{
Logger: log.New(os.Stderr, "", log.LstdFlags), // add default logger
},
source: source,
driver: driver,
}
Expand Down