From e0e4a2fa872350c8b2c325c4bca2bc6c45376b88 Mon Sep 17 00:00:00 2001 From: Richard Ramos Date: Wed, 9 Aug 2023 13:23:44 -0400 Subject: [PATCH] refactor: remove unused function and simplify code related to creating db and migrations --- cmd/waku/rest/utils_test.go | 4 +-- waku/persistence/postgres/postgres.go | 35 +++++------------------- waku/persistence/sqlite/sqlite.go | 38 +++++---------------------- waku/persistence/store.go | 4 ++- waku/persistence/utils/db.go | 6 +++-- waku/v2/node/connectedness_test.go | 4 +-- waku/v2/node/wakunode2_test.go | 4 +-- waku/v2/protocol/store/utils_test.go | 4 +-- waku/v2/rendezvous/rendezvous_test.go | 6 ++--- 9 files changed, 29 insertions(+), 76 deletions(-) diff --git a/cmd/waku/rest/utils_test.go b/cmd/waku/rest/utils_test.go index 020eccb4..485519d0 100644 --- a/cmd/waku/rest/utils_test.go +++ b/cmd/waku/rest/utils_test.go @@ -12,10 +12,10 @@ import ( func MemoryDB(t *testing.T) *persistence.DBStore { var db *sql.DB - db, migration, err := sqlite.NewDB(":memory:", false, utils.Logger()) + db, err := sqlite.NewDB(":memory:", false, utils.Logger()) require.NoError(t, err) - dbStore, err := persistence.NewDBStore(utils.Logger(), persistence.WithDB(db), persistence.WithMigrations(migration)) + dbStore, err := persistence.NewDBStore(utils.Logger(), persistence.WithDB(db), persistence.WithMigrations(sqlite.Migrations)) require.NoError(t, err) return dbStore diff --git a/waku/persistence/postgres/postgres.go b/waku/persistence/postgres/postgres.go index 7ce66f96..bda79b54 100644 --- a/waku/persistence/postgres/postgres.go +++ b/waku/persistence/postgres/postgres.go @@ -13,29 +13,6 @@ import ( "go.uber.org/zap" ) -// WithDB is a DBOption that lets you use a postgresql DBStore and run migrations -func WithDB(dburl string, migrate bool, shouldVacuum bool) persistence.DBOption { - return func(d *persistence.DBStore) error { - driverOption := persistence.WithDriver("pgx", dburl) - err := driverOption(d) - if err != nil { - return err - } - - if !migrate { - return nil - } - - migrationOpt := persistence.WithMigrations(Migrate) - err = migrationOpt(d) - if err != nil { - return err - } - - return nil - } -} - func executeVacuum(db *sql.DB, logger *zap.Logger) error { logger.Info("starting PostgreSQL database vacuuming") _, err := db.Exec("VACUUM FULL") @@ -47,20 +24,20 @@ func executeVacuum(db *sql.DB, logger *zap.Logger) error { } // NewDB connects to postgres DB in the specified path -func NewDB(dburl string, shouldVacuum bool, logger *zap.Logger) (*sql.DB, func(*sql.DB) error, error) { +func NewDB(dburl string, shouldVacuum bool, logger *zap.Logger) (*sql.DB, error) { db, err := sql.Open("pgx", dburl) if err != nil { - return nil, nil, err + return nil, err } if shouldVacuum { err := executeVacuum(db, logger) if err != nil { - return nil, nil, err + return nil, err } } - return db, Migrate, nil + return db, nil } func migrationDriver(db *sql.DB) (database.Driver, error) { @@ -69,8 +46,8 @@ func migrationDriver(db *sql.DB) (database.Driver, error) { }) } -// Migrate is the function used for DB migration with postgres driver -func Migrate(db *sql.DB) error { +// Migrations is the function used for DB migration with postgres driver +func Migrations(db *sql.DB) error { migrationDriver, err := migrationDriver(db) if err != nil { return err diff --git a/waku/persistence/sqlite/sqlite.go b/waku/persistence/sqlite/sqlite.go index 517dbcae..c7b845c4 100644 --- a/waku/persistence/sqlite/sqlite.go +++ b/waku/persistence/sqlite/sqlite.go @@ -30,32 +30,6 @@ func addSqliteURLDefaults(dburl string) string { return dburl } -// WithDB is a DBOption that lets you use a sqlite3 DBStore and run migrations -func WithDB(dburl string, migrate bool) persistence.DBOption { - return func(d *persistence.DBStore) error { - driverOption := persistence.WithDriver("sqlite3", addSqliteURLDefaults(dburl), persistence.ConnectionPoolOptions{ - // Disable concurrent access as not supported by the driver - MaxOpenConnections: 1, - }) - err := driverOption(d) - if err != nil { - return err - } - - if !migrate { - return nil - } - - migrationOpt := persistence.WithMigrations(Migrate) - err = migrationOpt(d) - if err != nil { - return err - } - - return nil - } -} - func executeVacuum(db *sql.DB, logger *zap.Logger) error { logger.Info("starting sqlite database vacuuming") _, err := db.Exec("VACUUM") @@ -67,10 +41,10 @@ func executeVacuum(db *sql.DB, logger *zap.Logger) error { } // NewDB creates a sqlite3 DB in the specified path -func NewDB(dburl string, shouldVacuum bool, logger *zap.Logger) (*sql.DB, func(*sql.DB) error, error) { +func NewDB(dburl string, shouldVacuum bool, logger *zap.Logger) (*sql.DB, error) { db, err := sql.Open("sqlite3", addSqliteURLDefaults(dburl)) if err != nil { - return nil, nil, err + return nil, err } // Disable concurrent access as not supported by the driver @@ -79,11 +53,11 @@ func NewDB(dburl string, shouldVacuum bool, logger *zap.Logger) (*sql.DB, func(* if shouldVacuum { err := executeVacuum(db, logger) if err != nil { - return nil, nil, err + return nil, err } } - return db, Migrate, nil + return db, nil } func migrationDriver(db *sql.DB) (database.Driver, error) { @@ -92,8 +66,8 @@ func migrationDriver(db *sql.DB) (database.Driver, error) { }) } -// Migrate is the function used for DB migration with sqlite driver -func Migrate(db *sql.DB) error { +// Migrations is the function used for DB migration with sqlite driver +func Migrations(db *sql.DB) error { migrationDriver, err := migrationDriver(db) if err != nil { return err diff --git a/waku/persistence/store.go b/waku/persistence/store.go index f9332db3..e1a7c22b 100644 --- a/waku/persistence/store.go +++ b/waku/persistence/store.go @@ -121,9 +121,11 @@ func WithRetentionPolicy(maxMessages int, maxDuration time.Duration) DBOption { } } +type MigrationFn func(db *sql.DB) error + // WithMigrations is a DBOption used to determine if migrations should // be executed, and what driver to use -func WithMigrations(migrationFn func(db *sql.DB) error) DBOption { +func WithMigrations(migrationFn MigrationFn) DBOption { return func(d *DBStore) error { d.enableMigrations = true d.migrationFn = migrationFn diff --git a/waku/persistence/utils/db.go b/waku/persistence/utils/db.go index 94593258..96c5a919 100644 --- a/waku/persistence/utils/db.go +++ b/waku/persistence/utils/db.go @@ -49,9 +49,11 @@ func ExtractDBAndMigration(databaseURL string, dbSettings DBSettings, logger *za dbParams := dbURLParts[1] switch dbEngine { case "sqlite3": - db, migrationFn, err = sqlite.NewDB(dbParams, dbSettings.Vacuum, logger) + db, err = sqlite.NewDB(dbParams, dbSettings.Vacuum, logger) + migrationFn = sqlite.Migrations case "postgresql": - db, migrationFn, err = postgres.NewDB(dbURL, dbSettings.Vacuum, logger) + db, err = postgres.NewDB(dbURL, dbSettings.Vacuum, logger) + migrationFn = postgres.Migrations default: err = errors.New("unsupported database engine") } diff --git a/waku/v2/node/connectedness_test.go b/waku/v2/node/connectedness_test.go index 3a199819..098c6790 100644 --- a/waku/v2/node/connectedness_test.go +++ b/waku/v2/node/connectedness_test.go @@ -69,9 +69,9 @@ func TestConnectionStatusChanges(t *testing.T) { err = node2.Start(ctx) require.NoError(t, err) - db, migration, err := sqlite.NewDB(":memory:", false, utils.Logger()) + db, err := sqlite.NewDB(":memory:", false, utils.Logger()) require.NoError(t, err) - dbStore, err := persistence.NewDBStore(utils.Logger(), persistence.WithDB(db), persistence.WithMigrations(migration)) + dbStore, err := persistence.NewDBStore(utils.Logger(), persistence.WithDB(db), persistence.WithMigrations(sqlite.Migrations)) require.NoError(t, err) // Node3: Relay + Store diff --git a/waku/v2/node/wakunode2_test.go b/waku/v2/node/wakunode2_test.go index a38cd978..c62015fe 100644 --- a/waku/v2/node/wakunode2_test.go +++ b/waku/v2/node/wakunode2_test.go @@ -230,9 +230,9 @@ func TestDecoupledStoreFromRelay(t *testing.T) { subs.Unsubscribe() // NODE2: Filter Client/Store - db, migration, err := sqlite.NewDB(":memory:", false, utils.Logger()) + db, err := sqlite.NewDB(":memory:", false, utils.Logger()) require.NoError(t, err) - dbStore, err := persistence.NewDBStore(utils.Logger(), persistence.WithDB(db), persistence.WithMigrations(migration)) + dbStore, err := persistence.NewDBStore(utils.Logger(), persistence.WithDB(db), persistence.WithMigrations(sqlite.Migrations)) require.NoError(t, err) hostAddr2, err := net.ResolveTCPAddr("tcp", "0.0.0.0:0") diff --git a/waku/v2/protocol/store/utils_test.go b/waku/v2/protocol/store/utils_test.go index b9914bf4..a803e06d 100644 --- a/waku/v2/protocol/store/utils_test.go +++ b/waku/v2/protocol/store/utils_test.go @@ -12,10 +12,10 @@ import ( func MemoryDB(t *testing.T) *persistence.DBStore { var db *sql.DB - db, migration, err := sqlite.NewDB(":memory:", false, utils.Logger()) + db, err := sqlite.NewDB(":memory:", false, utils.Logger()) require.NoError(t, err) - dbStore, err := persistence.NewDBStore(utils.Logger(), persistence.WithDB(db), persistence.WithMigrations(migration)) + dbStore, err := persistence.NewDBStore(utils.Logger(), persistence.WithDB(db), persistence.WithMigrations(sqlite.Migrations)) require.NoError(t, err) return dbStore diff --git a/waku/v2/rendezvous/rendezvous_test.go b/waku/v2/rendezvous/rendezvous_test.go index b0350730..19762359 100644 --- a/waku/v2/rendezvous/rendezvous_test.go +++ b/waku/v2/rendezvous/rendezvous_test.go @@ -3,7 +3,6 @@ package rendezvous import ( "context" "crypto/rand" - "database/sql" "fmt" "sync" "testing" @@ -46,11 +45,10 @@ func TestRendezvous(t *testing.T) { host1, err := tests.MakeHost(ctx, port1, rand.Reader) require.NoError(t, err) - var db *sql.DB - db, migration, err := sqlite.NewDB(":memory:", false, utils.Logger()) + db, err := sqlite.NewDB(":memory:", false, utils.Logger()) require.NoError(t, err) - err = migration(db) + err = sqlite.Migrations(db) require.NoError(t, err) rdb := NewDB(ctx, db, utils.Logger())