From 6385acb01f0901d1187a10386a2c8dc2944571e8 Mon Sep 17 00:00:00 2001 From: JordanBrockopp Date: Sat, 17 Jun 2023 07:28:50 -0500 Subject: [PATCH] refactor(database): use engine options --- database/close.go | 4 +- database/close_test.go | 6 +-- database/context.go | 20 +++++----- database/database.go | 79 +++++++++++++++++++++++---------------- database/database_test.go | 35 ++++++++++------- database/driver.go | 2 +- database/ping.go | 6 +-- database/ping_test.go | 6 +-- database/resource.go | 76 ++++++++++++++++++------------------- database/validate.go | 2 +- database/validate_test.go | 20 +++++----- 11 files changed, 140 insertions(+), 116 deletions(-) diff --git a/database/close.go b/database/close.go index 0596b972e..0aff8e51d 100644 --- a/database/close.go +++ b/database/close.go @@ -6,10 +6,10 @@ package database // Close stops and terminates the connection to the database. func (e *engine) Close() error { - e.Logger.Tracef("closing connection to the %s database", e.Driver()) + e.logger.Tracef("closing connection to the %s database", e.Driver()) // capture database/sql database from gorm.io/gorm database - _sql, err := e.Database.DB() + _sql, err := e.client.DB() if err != nil { return err } diff --git a/database/close_test.go b/database/close_test.go index b555e78d5..fe0c55b00 100644 --- a/database/close_test.go +++ b/database/close_test.go @@ -49,15 +49,15 @@ func TestDatabase_Engine_Close(t *testing.T) { name: "failure with invalid gorm database", failure: true, database: &engine{ - Config: &Config{ + config: &config{ Driver: "invalid", }, - Database: &gorm.DB{ + client: &gorm.DB{ Config: &gorm.Config{ ConnPool: nil, }, }, - Logger: logrus.NewEntry(logrus.StandardLogger()), + logger: logrus.NewEntry(logrus.StandardLogger()), }, }, } diff --git a/database/context.go b/database/context.go index d27d73550..fd08cc371 100644 --- a/database/context.go +++ b/database/context.go @@ -43,14 +43,14 @@ func ToContext(c Setter, d Interface) { func FromCLIContext(c *cli.Context) (Interface, error) { logrus.Debug("creating database engine from CLI configuration") - return New(&Config{ - Address: c.String("database.addr"), - CompressionLevel: c.Int("database.compression.level"), - ConnectionLife: c.Duration("database.connection.life"), - ConnectionIdle: c.Int("database.connection.idle"), - ConnectionOpen: c.Int("database.connection.open"), - Driver: c.String("database.driver"), - EncryptionKey: c.String("database.encryption.key"), - SkipCreation: c.Bool("database.skip_creation"), - }) + return New( + WithAddress(c.String("database.addr")), + WithCompressionLevel(c.Int("database.compression.level")), + WithConnectionLife(c.Duration("database.connection.life")), + WithConnectionIdle(c.Int("database.connection.idle")), + WithConnectionOpen(c.Int("database.connection.open")), + WithDriver(c.String("database.driver")), + WithEncryptionKey(c.String("database.encryption.key")), + WithSkipCreation(c.Bool("database.skip_creation")), + ) } diff --git a/database/database.go b/database/database.go index 5fe3de951..c612ba3aa 100644 --- a/database/database.go +++ b/database/database.go @@ -28,8 +28,8 @@ import ( ) type ( - // Config represents the settings required to create the engine that implements the Interface. - Config struct { + // config represents the settings required to create the engine that implements the Interface. + config struct { // specifies the address to use for the database engine Address string // specifies the level of compression to use for the database engine @@ -50,9 +50,12 @@ type ( // engine represents the functionality that implements the Interface. engine struct { - Config *Config - Database *gorm.DB - Logger *logrus.Entry + // gorm.io/gorm database client used in database functions + client *gorm.DB + // engine configuration settings used in database functions + config *config + // sirupsen/logrus logger used in database functions + logger *logrus.Entry build.BuildInterface hook.HookInterface @@ -74,52 +77,64 @@ type ( // // * postgres // * sqlite3 -func New(c *Config) (Interface, error) { +func New(opts ...EngineOpt) (Interface, error) { + // create new database engine + e := new(engine) + + // create new fields + e.client = new(gorm.DB) + e.config = new(config) + e.logger = new(logrus.Entry) + + // apply all provided configuration options + for _, opt := range opts { + err := opt(e) + if err != nil { + return nil, err + } + } + // validate the configuration being provided - err := c.Validate() + err := e.config.Validate() if err != nil { return nil, err } - // create new database engine - e := &engine{ - Config: c, - Database: new(gorm.DB), - Logger: logrus.NewEntry(logrus.StandardLogger()).WithField("database", c.Driver), - } + // update the logger with additional metadata + e.logger = logrus.NewEntry(logrus.StandardLogger()).WithField("database", e.Driver()) - e.Logger.Trace("creating database engine from configuration") + e.logger.Trace("creating database engine from configuration") // process the database driver being provided - switch c.Driver { + switch e.config.Driver { case constants.DriverPostgres: // create the new Postgres database client - e.Database, err = gorm.Open(postgres.Open(e.Config.Address), &gorm.Config{}) + e.client, err = gorm.Open(postgres.Open(e.config.Address), &gorm.Config{}) if err != nil { return nil, err } case constants.DriverSqlite: // create the new Sqlite database client - e.Database, err = gorm.Open(sqlite.Open(e.Config.Address), &gorm.Config{}) + e.client, err = gorm.Open(sqlite.Open(e.config.Address), &gorm.Config{}) if err != nil { return nil, err } default: // handle an invalid database driver being provided - return nil, fmt.Errorf("invalid database driver provided: %s", c.Driver) + return nil, fmt.Errorf("invalid database driver provided: %s", e.Driver()) } // capture database/sql database from gorm.io/gorm database - db, err := e.Database.DB() + db, err := e.client.DB() if err != nil { return nil, err } // set the maximum amount of time a connection may be reused - db.SetConnMaxLifetime(e.Config.ConnectionLife) + db.SetConnMaxLifetime(e.config.ConnectionLife) // set the maximum number of connections in the idle connection pool - db.SetMaxIdleConns(e.Config.ConnectionIdle) + db.SetMaxIdleConns(e.config.ConnectionIdle) // set the maximum number of open connections to the database - db.SetMaxOpenConns(e.Config.ConnectionOpen) + db.SetMaxOpenConns(e.config.ConnectionOpen) // verify connection to the database err = e.Ping() @@ -140,14 +155,14 @@ func New(c *Config) (Interface, error) { // // This function is ONLY intended to be used for testing purposes. func NewTest() (Interface, error) { - return New(&Config{ - Address: "file::memory:?cache=shared", - CompressionLevel: 3, - ConnectionLife: 30 * time.Minute, - ConnectionIdle: 2, - ConnectionOpen: 0, - Driver: "sqlite3", - EncryptionKey: "A1B2C3D4E5G6H7I8J9K0LMNOPQRSTUVW", - SkipCreation: false, - }) + return New( + WithAddress("file::memory:?cache=shared"), + WithCompressionLevel(3), + WithConnectionLife(30*time.Minute), + WithConnectionIdle(2), + WithConnectionOpen(0), + WithDriver("sqlite3"), + WithEncryptionKey("A1B2C3D4E5G6H7I8J9K0LMNOPQRSTUVW"), + WithSkipCreation(false), + ) } diff --git a/database/database_test.go b/database/database_test.go index 6d18421bc..a1cfecc8a 100644 --- a/database/database_test.go +++ b/database/database_test.go @@ -21,12 +21,12 @@ func TestDatabase_New(t *testing.T) { tests := []struct { failure bool name string - config *Config + config *config }{ { name: "failure with postgres", failure: true, - config: &Config{ + config: &config{ Driver: "postgres", Address: "postgres://foo:bar@localhost:5432/vela", CompressionLevel: 3, @@ -40,7 +40,7 @@ func TestDatabase_New(t *testing.T) { { name: "success with sqlite3", failure: false, - config: &Config{ + config: &config{ Driver: "sqlite3", Address: "file::memory:?cache=shared", CompressionLevel: 3, @@ -54,7 +54,7 @@ func TestDatabase_New(t *testing.T) { { name: "failure with invalid config", failure: true, - config: &Config{ + config: &config{ Driver: "postgres", Address: "", CompressionLevel: 3, @@ -68,7 +68,7 @@ func TestDatabase_New(t *testing.T) { { name: "failure with invalid driver", failure: true, - config: &Config{ + config: &config{ Driver: "mysql", Address: "foo:bar@tcp(localhost:3306)/vela?charset=utf8mb4&parseTime=True&loc=Local", CompressionLevel: 3, @@ -84,7 +84,16 @@ func TestDatabase_New(t *testing.T) { // run tests for _, test := range tests { t.Run(test.name, func(t *testing.T) { - _, err := New(test.config) + _, err := New( + WithAddress(test.config.Address), + WithCompressionLevel(test.config.CompressionLevel), + WithConnectionLife(test.config.ConnectionLife), + WithConnectionIdle(test.config.ConnectionIdle), + WithConnectionOpen(test.config.ConnectionOpen), + WithDriver(test.config.Driver), + WithEncryptionKey(test.config.EncryptionKey), + WithSkipCreation(test.config.SkipCreation), + ) if test.failure { if err == nil { @@ -105,7 +114,7 @@ func TestDatabase_New(t *testing.T) { func testPostgres(t *testing.T) (*engine, sqlmock.Sqlmock) { // create the engine with test configuration _engine := &engine{ - Config: &Config{ + config: &config{ CompressionLevel: 3, ConnectionLife: 30 * time.Minute, ConnectionIdle: 2, @@ -114,7 +123,7 @@ func testPostgres(t *testing.T) (*engine, sqlmock.Sqlmock) { EncryptionKey: "A1B2C3D4E5G6H7I8J9K0LMNOPQRSTUVW", SkipCreation: false, }, - Logger: logrus.NewEntry(logrus.StandardLogger()), + logger: logrus.NewEntry(logrus.StandardLogger()), } // create the new mock sql database @@ -129,7 +138,7 @@ func testPostgres(t *testing.T) (*engine, sqlmock.Sqlmock) { _mock.ExpectPing() // create the new mock Postgres database client - _engine.Database, err = gorm.Open( + _engine.client, err = gorm.Open( postgres.New(postgres.Config{Conn: _sql}), &gorm.Config{SkipDefaultTransaction: true}, ) @@ -146,7 +155,7 @@ func testSqlite(t *testing.T) *engine { // create the engine with test configuration _engine := &engine{ - Config: &Config{ + config: &config{ Address: "file::memory:?cache=shared", CompressionLevel: 3, ConnectionLife: 30 * time.Minute, @@ -156,12 +165,12 @@ func testSqlite(t *testing.T) *engine { EncryptionKey: "A1B2C3D4E5G6H7I8J9K0LMNOPQRSTUVW", SkipCreation: false, }, - Logger: logrus.NewEntry(logrus.StandardLogger()), + logger: logrus.NewEntry(logrus.StandardLogger()), } // create the new mock Sqlite database client - _engine.Database, err = gorm.Open( - sqlite.Open(_engine.Config.Address), + _engine.client, err = gorm.Open( + sqlite.Open(_engine.config.Address), &gorm.Config{SkipDefaultTransaction: true}, ) if err != nil { diff --git a/database/driver.go b/database/driver.go index a81662846..1c3130094 100644 --- a/database/driver.go +++ b/database/driver.go @@ -6,5 +6,5 @@ package database // Driver outputs the configured database driver. func (e *engine) Driver() string { - return e.Config.Driver + return e.config.Driver } diff --git a/database/ping.go b/database/ping.go index 7ac1a09c2..80968c152 100644 --- a/database/ping.go +++ b/database/ping.go @@ -11,12 +11,12 @@ import ( // Ping sends a "ping" request with backoff to the database. func (e *engine) Ping() error { - e.Logger.Tracef("sending ping request to the %s database", e.Driver()) + e.logger.Tracef("sending ping request to the %s database", e.Driver()) // create a loop to attempt ping requests 5 times for i := 0; i < 5; i++ { // capture database/sql database from gorm.io/gorm database - _sql, err := e.Database.DB() + _sql, err := e.client.DB() if err != nil { return err } @@ -27,7 +27,7 @@ func (e *engine) Ping() error { // create the duration of time to sleep for before attempting to retry duration := time.Duration(i+1) * time.Second - e.Logger.Warnf("unable to ping %s database - retrying in %v", e.Driver(), duration) + e.logger.Warnf("unable to ping %s database - retrying in %v", e.Driver(), duration) // sleep for loop iteration in seconds time.Sleep(duration) diff --git a/database/ping_test.go b/database/ping_test.go index 7a34ad122..5b2fdb0c9 100644 --- a/database/ping_test.go +++ b/database/ping_test.go @@ -48,15 +48,15 @@ func TestDatabase_Engine_Ping(t *testing.T) { name: "failure with invalid gorm database", failure: true, database: &engine{ - Config: &Config{ + config: &config{ Driver: "invalid", }, - Database: &gorm.DB{ + client: &gorm.DB{ Config: &gorm.Config{ ConnPool: nil, }, }, - Logger: logrus.NewEntry(logrus.StandardLogger()), + logger: logrus.NewEntry(logrus.StandardLogger()), }, }, } diff --git a/database/resource.go b/database/resource.go index 93122eadc..1ff9c929c 100644 --- a/database/resource.go +++ b/database/resource.go @@ -24,9 +24,9 @@ func (e *engine) NewResources() error { // create the database agnostic engine for builds e.BuildInterface, err = build.New( - build.WithClient(e.Database), - build.WithLogger(e.Logger), - build.WithSkipCreation(e.Config.SkipCreation), + build.WithClient(e.client), + build.WithLogger(e.logger), + build.WithSkipCreation(e.config.SkipCreation), ) if err != nil { return err @@ -34,9 +34,9 @@ func (e *engine) NewResources() error { // create the database agnostic engine for hooks e.HookInterface, err = hook.New( - hook.WithClient(e.Database), - hook.WithLogger(e.Logger), - hook.WithSkipCreation(e.Config.SkipCreation), + hook.WithClient(e.client), + hook.WithLogger(e.logger), + hook.WithSkipCreation(e.config.SkipCreation), ) if err != nil { return err @@ -44,10 +44,10 @@ func (e *engine) NewResources() error { // create the database agnostic engine for logs e.LogInterface, err = log.New( - log.WithClient(e.Database), - log.WithCompressionLevel(e.Config.CompressionLevel), - log.WithLogger(e.Logger), - log.WithSkipCreation(e.Config.SkipCreation), + log.WithClient(e.client), + log.WithCompressionLevel(e.config.CompressionLevel), + log.WithLogger(e.logger), + log.WithSkipCreation(e.config.SkipCreation), ) if err != nil { return err @@ -55,10 +55,10 @@ func (e *engine) NewResources() error { // create the database agnostic engine for pipelines e.PipelineInterface, err = pipeline.New( - pipeline.WithClient(e.Database), - pipeline.WithCompressionLevel(e.Config.CompressionLevel), - pipeline.WithLogger(e.Logger), - pipeline.WithSkipCreation(e.Config.SkipCreation), + pipeline.WithClient(e.client), + pipeline.WithCompressionLevel(e.config.CompressionLevel), + pipeline.WithLogger(e.logger), + pipeline.WithSkipCreation(e.config.SkipCreation), ) if err != nil { return err @@ -66,10 +66,10 @@ func (e *engine) NewResources() error { // create the database agnostic engine for repos e.RepoInterface, err = repo.New( - repo.WithClient(e.Database), - repo.WithEncryptionKey(e.Config.EncryptionKey), - repo.WithLogger(e.Logger), - repo.WithSkipCreation(e.Config.SkipCreation), + repo.WithClient(e.client), + repo.WithEncryptionKey(e.config.EncryptionKey), + repo.WithLogger(e.logger), + repo.WithSkipCreation(e.config.SkipCreation), ) if err != nil { return err @@ -77,9 +77,9 @@ func (e *engine) NewResources() error { // create the database agnostic engine for schedules e.ScheduleInterface, err = schedule.New( - schedule.WithClient(e.Database), - schedule.WithLogger(e.Logger), - schedule.WithSkipCreation(e.Config.SkipCreation), + schedule.WithClient(e.client), + schedule.WithLogger(e.logger), + schedule.WithSkipCreation(e.config.SkipCreation), ) if err != nil { return err @@ -89,10 +89,10 @@ func (e *engine) NewResources() error { // // https://pkg.go.dev/github.com/go-vela/server/database/secret#New e.SecretInterface, err = secret.New( - secret.WithClient(e.Database), - secret.WithEncryptionKey(e.Config.EncryptionKey), - secret.WithLogger(e.Logger), - secret.WithSkipCreation(e.Config.SkipCreation), + secret.WithClient(e.client), + secret.WithEncryptionKey(e.config.EncryptionKey), + secret.WithLogger(e.logger), + secret.WithSkipCreation(e.config.SkipCreation), ) if err != nil { return err @@ -100,9 +100,9 @@ func (e *engine) NewResources() error { // create the database agnostic engine for services e.ServiceInterface, err = service.New( - service.WithClient(e.Database), - service.WithLogger(e.Logger), - service.WithSkipCreation(e.Config.SkipCreation), + service.WithClient(e.client), + service.WithLogger(e.logger), + service.WithSkipCreation(e.config.SkipCreation), ) if err != nil { return err @@ -110,9 +110,9 @@ func (e *engine) NewResources() error { // create the database agnostic engine for steps e.StepInterface, err = step.New( - step.WithClient(e.Database), - step.WithLogger(e.Logger), - step.WithSkipCreation(e.Config.SkipCreation), + step.WithClient(e.client), + step.WithLogger(e.logger), + step.WithSkipCreation(e.config.SkipCreation), ) if err != nil { return err @@ -120,10 +120,10 @@ func (e *engine) NewResources() error { // create the database agnostic engine for users e.UserInterface, err = user.New( - user.WithClient(e.Database), - user.WithEncryptionKey(e.Config.EncryptionKey), - user.WithLogger(e.Logger), - user.WithSkipCreation(e.Config.SkipCreation), + user.WithClient(e.client), + user.WithEncryptionKey(e.config.EncryptionKey), + user.WithLogger(e.logger), + user.WithSkipCreation(e.config.SkipCreation), ) if err != nil { return err @@ -131,9 +131,9 @@ func (e *engine) NewResources() error { // create the database agnostic engine for workers e.WorkerInterface, err = worker.New( - worker.WithClient(e.Database), - worker.WithLogger(e.Logger), - worker.WithSkipCreation(e.Config.SkipCreation), + worker.WithClient(e.client), + worker.WithLogger(e.logger), + worker.WithSkipCreation(e.config.SkipCreation), ) if err != nil { return err diff --git a/database/validate.go b/database/validate.go index c16c07cd2..18f07cb84 100644 --- a/database/validate.go +++ b/database/validate.go @@ -13,7 +13,7 @@ import ( ) // Validate verifies the required fields from the provided configuration are populated correctly. -func (c *Config) Validate() error { +func (c *config) Validate() error { logrus.Trace("validating database configuration for engine") // verify a database driver was provided diff --git a/database/validate_test.go b/database/validate_test.go index 356705fe2..60761e215 100644 --- a/database/validate_test.go +++ b/database/validate_test.go @@ -14,12 +14,12 @@ func TestDatabase_Config_Validate(t *testing.T) { tests := []struct { failure bool name string - config *Config + config *config }{ { name: "success with postgres", failure: false, - config: &Config{ + config: &config{ Driver: "postgres", Address: "postgres://foo:bar@localhost:5432/vela", CompressionLevel: 3, @@ -33,7 +33,7 @@ func TestDatabase_Config_Validate(t *testing.T) { { name: "success with sqlite3", failure: false, - config: &Config{ + config: &config{ Driver: "sqlite3", Address: "file::memory:?cache=shared", CompressionLevel: 3, @@ -47,7 +47,7 @@ func TestDatabase_Config_Validate(t *testing.T) { { name: "success with negative compression level", failure: false, - config: &Config{ + config: &config{ Driver: "postgres", Address: "postgres://foo:bar@localhost:5432/vela", CompressionLevel: -1, @@ -61,7 +61,7 @@ func TestDatabase_Config_Validate(t *testing.T) { { name: "failure with empty driver", failure: true, - config: &Config{ + config: &config{ Driver: "", Address: "postgres://foo:bar@localhost:5432/vela", CompressionLevel: 3, @@ -75,7 +75,7 @@ func TestDatabase_Config_Validate(t *testing.T) { { name: "failure with empty address", failure: true, - config: &Config{ + config: &config{ Driver: "postgres", Address: "", CompressionLevel: 3, @@ -89,7 +89,7 @@ func TestDatabase_Config_Validate(t *testing.T) { { name: "failure with invalid address", failure: true, - config: &Config{ + config: &config{ Driver: "postgres", Address: "postgres://foo:bar@localhost:5432/vela/", CompressionLevel: 3, @@ -103,7 +103,7 @@ func TestDatabase_Config_Validate(t *testing.T) { { name: "failure with invalid compression level", failure: true, - config: &Config{ + config: &config{ Driver: "postgres", Address: "postgres://foo:bar@localhost:5432/vela", CompressionLevel: 10, @@ -117,7 +117,7 @@ func TestDatabase_Config_Validate(t *testing.T) { { name: "failure with empty encryption key", failure: true, - config: &Config{ + config: &config{ Driver: "postgres", Address: "postgres://foo:bar@localhost:5432/vela", CompressionLevel: 3, @@ -131,7 +131,7 @@ func TestDatabase_Config_Validate(t *testing.T) { { name: "failure with invalid encryption key", failure: true, - config: &Config{ + config: &config{ Driver: "postgres", Address: "postgres://foo:bar@localhost:5432/vela", CompressionLevel: 3,