sqldb: separate migration execution from construction

This commit separates the execution of SQL and in-code migrations
from their construction. This change is necessary because,
currently, the SQL schema is migrated during the construction
phase in the lncfg package. However, migrations are typically
executed when individual stores are constructed within the
configuration builder.

config_builder.go

@@ -932,10 +932,10 @@ type DatabaseInstances struct {
 	// the btcwallet's loader.
 	WalletDB btcwallet.LoaderOption
 
-	// NativeSQLStore is a pointer to a native SQL store that can be used
+	// NativeSQLStore holds a reference to the native SQL store that can
-	// for native SQL queries for tables that already support it. This may
+	// be used for native SQL queries for tables that already support it.
-	// be nil if the use-native-sql flag was not set.
+	// This may be nil if the use-native-sql flag was not set.
-	NativeSQLStore *sqldb.BaseDB
+	NativeSQLStore sqldb.DB
 }
 
 // DefaultDatabaseBuilder is a type that builds the default database backends
@@ -1079,6 +1079,19 @@ func (d *DefaultDatabaseBuilder) BuildDatabase(
 
 	// Instantiate a native SQL invoice store if the flag is set.
 	if d.cfg.DB.UseNativeSQL {
+		// We need to apply all migrations to the native SQL store
+		// before we can use it.
+		err := dbs.NativeSQLStore.ApplyAllMigrations(
+			ctx, sqldb.GetMigrations(),
+		)
+		if err != nil {
+			cleanUp()
+			err := fmt.Errorf("unable to apply migrations: %w", err)
+			d.logger.Error(err)
+
+			return nil, nil, err
+		}
+
 		// KV invoice db resides in the same database as the channel
 		// state DB. Let's query the database to see if we have any
 		// invoices there. If we do, we won't allow the user to start
@@ -1107,10 +1120,11 @@ func (d *DefaultDatabaseBuilder) BuildDatabase(
 			return nil, nil, err
 		}
 
+		baseDB := dbs.NativeSQLStore.GetBaseDB()
 		executor := sqldb.NewTransactionExecutor(
-			dbs.NativeSQLStore,
+			baseDB,
 			func(tx *sql.Tx) invoices.SQLInvoiceQueries {
-				return dbs.NativeSQLStore.WithTx(tx)
+				return baseDB.WithTx(tx)
 			},
 		)
 

lncfg/db.go

@@ -231,10 +231,10 @@ type DatabaseBackends struct {
 	// the underlying wallet database from.
 	WalletDB btcwallet.LoaderOption
 
-	// NativeSQLStore is a pointer to a native SQL store that can be used
+	// NativeSQLStore holds a reference to the native SQL store that can
-	// for native SQL queries for tables that already support it. This may
+	// be used for native SQL queries for tables that already support it.
-	// be nil if the use-native-sql flag was not set.
+	// This may be nil if the use-native-sql flag was not set.
-	NativeSQLStore *sqldb.BaseDB
+	NativeSQLStore sqldb.DB
 
 	// Remote indicates whether the database backends are remote, possibly
 	// replicated instances or local bbolt or sqlite backed databases.
@@ -449,17 +449,17 @@ func (db *DB) GetBackends(ctx context.Context, chanDBPath,
 		}
 		closeFuncs[NSWalletDB] = postgresWalletBackend.Close
 
-		var nativeSQLStore *sqldb.BaseDB
+		var nativeSQLStore sqldb.DB
 		if db.UseNativeSQL {
 			nativePostgresStore, err := sqldb.NewPostgresStore(
-				db.Postgres, sqldb.GetMigrations(),
+				db.Postgres,
 			)
 			if err != nil {
 				return nil, fmt.Errorf("error opening "+
 					"native postgres store: %v", err)
 			}
 
-			nativeSQLStore = nativePostgresStore.BaseDB
+			nativeSQLStore = nativePostgresStore
 			closeFuncs[PostgresBackend] = nativePostgresStore.Close
 		}
 
@@ -571,19 +571,18 @@ func (db *DB) GetBackends(ctx context.Context, chanDBPath,
 		}
 		closeFuncs[NSWalletDB] = sqliteWalletBackend.Close
 
-		var nativeSQLStore *sqldb.BaseDB
+		var nativeSQLStore sqldb.DB
 		if db.UseNativeSQL {
 			nativeSQLiteStore, err := sqldb.NewSqliteStore(
 				db.Sqlite,
 				path.Join(chanDBPath, SqliteNativeDBName),
-				sqldb.GetMigrations(),
 			)
 			if err != nil {
 				return nil, fmt.Errorf("error opening "+
 					"native SQLite store: %v", err)
 			}
 
-			nativeSQLStore = nativeSQLiteStore.BaseDB
+			nativeSQLStore = nativeSQLiteStore
 			closeFuncs[SqliteBackend] = nativeSQLiteStore.Close
 		}
 

sqldb/interfaces.go

@@ -355,6 +355,18 @@ func (t *TransactionExecutor[Q]) ExecTx(ctx context.Context,
 	)
 }
 
+// DB is an interface that represents a generic SQL database. It provides
+// methods to apply migrations and access the underlying database connection.
+type DB interface {
+	// GetBaseDB returns the underlying BaseDB instance.
+	GetBaseDB() *BaseDB
+
+	// ApplyAllMigrations applies all migrations to the database including
+	// both sqlc and custom in-code migrations.
+	ApplyAllMigrations(ctx context.Context,
+		customMigrations []MigrationConfig) error
+}
+
 // BaseDB is the base database struct that each implementation can embed to
 // gain some common functionality.
 type BaseDB struct {

sqldb/migrations_test.go

@@ -314,16 +314,19 @@ func TestCustomMigration(t *testing.T) {
 			for i := 0; i < 3; i++ {
 				db, err = NewSqliteStore(&SqliteConfig{
 					SkipMigrations: false,
-				}, dbFileName, test.migrations)
+				}, dbFileName)
-				if db != nil {
+				require.NoError(t, err)
-					dbToCleanup := db.DB
-					t.Cleanup(func() {
-						require.NoError(
-							t, dbToCleanup.Close(),
-						)
-					})
-				}
 
+				dbToCleanup := db.DB
+				t.Cleanup(func() {
+					require.NoError(
+						t, dbToCleanup.Close(),
+					)
+				})
+
+				err = db.ApplyAllMigrations(
+					ctxb, test.migrations,
+				)
 				if test.expectedSuccess {
 					require.NoError(t, err)
 				} else {
@@ -333,7 +336,7 @@ func TestCustomMigration(t *testing.T) {
 					// so we can read versions.
 					db, err = NewSqliteStore(&SqliteConfig{
 						SkipMigrations: true,
-					}, dbFileName, nil)
+					}, dbFileName)
 					require.NoError(t, err)
 				}
 
@@ -399,8 +402,12 @@ func TestCustomMigration(t *testing.T) {
 			// are idempotent.
 			for i := 0; i < 3; i++ {
 				cfg.SkipMigrations = false
-				db, err = NewPostgresStore(cfg, test.migrations)
+				db, err = NewPostgresStore(cfg)
+				require.NoError(t, err)
 
+				err = db.ApplyAllMigrations(
+					ctxb, test.migrations,
+				)
 				if test.expectedSuccess {
 					require.NoError(t, err)
 				} else {
@@ -409,7 +416,7 @@ func TestCustomMigration(t *testing.T) {
 					// Also repoen the DB without migrations
 					// so we can read versions.
 					cfg.SkipMigrations = true
-					db, err = NewPostgresStore(cfg, nil)
+					db, err = NewPostgresStore(cfg)
 					require.NoError(t, err)
 				}
 

sqldb/no_sqlite.go

@@ -2,7 +2,15 @@
 
 package sqldb
 
-import "fmt"
+import (
+	"context"
+	"fmt"
+)
+
+var (
+	// Make sure SqliteStore implements the DB interface.
+	_ DB = (*SqliteStore)(nil)
+)
 
 // SqliteStore is a database store implementation that uses a sqlite backend.
 type SqliteStore struct {
@@ -16,3 +24,17 @@ type SqliteStore struct {
 func NewSqliteStore(cfg *SqliteConfig, dbPath string) (*SqliteStore, error) {
 	return nil, fmt.Errorf("SQLite backend not supported in WebAssembly")
 }
+
+// GetBaseDB returns the underlying BaseDB instance for the SQLite store.
+// It is a trivial helper method to comply with the sqldb.DB interface.
+func (s *SqliteStore) GetBaseDB() *BaseDB {
+	return s.BaseDB
+}
+
+// ApplyAllMigrations applies both the SQLC and custom in-code migrations to
+// the SQLite database.
+func (s *SqliteStore) ApplyAllMigrations(context.Context,
+	[]MigrationConfig) error {
+
+	return fmt.Errorf("SQLite backend not supported in WebAssembly")
+}

sqldb/postgres.go

@@ -36,6 +36,9 @@ var (
 
 	// Make sure PostgresStore implements the MigrationExecutor interface.
 	_ MigrationExecutor = (*PostgresStore)(nil)
+
+	// Make sure PostgresStore implements the DB interface.
+	_ DB = (*PostgresStore)(nil)
 )
 
 // replacePasswordInDSN takes a DSN string and returns it with the password
@@ -89,9 +92,7 @@ type PostgresStore struct {
 
 // NewPostgresStore creates a new store that is backed by a Postgres database
 // backend.
-func NewPostgresStore(cfg *PostgresConfig, migrations []MigrationConfig) (
+func NewPostgresStore(cfg *PostgresConfig) (*PostgresStore, error) {
-	*PostgresStore, error) {
-
 	sanitizedDSN, err := replacePasswordInDSN(cfg.Dsn)
 	if err != nil {
 		return nil, err
@@ -130,25 +131,32 @@ func NewPostgresStore(cfg *PostgresConfig, migrations []MigrationConfig) (
 
 	queries := sqlc.New(db)
 
-	s := &PostgresStore{
+	return &PostgresStore{
 		cfg: cfg,
 		BaseDB: &BaseDB{
 			DB:      db,
 			Queries: queries,
 		},
-	}
+	}, nil
+}
+
+// GetBaseDB returns the underlying BaseDB instance for the Postgres store.
+// It is a trivial helper method to comply with the sqldb.DB interface.
+func (s *PostgresStore) GetBaseDB() *BaseDB {
+	return s.BaseDB
+}
+
+// ApplyAllMigrations applies both the SQLC and custom in-code migrations to the
+// Postgres database.
+func (s *PostgresStore) ApplyAllMigrations(ctx context.Context,
+	migrations []MigrationConfig) error {
 
 	// Execute migrations unless configured to skip them.
-	if !cfg.SkipMigrations {
+	if s.cfg.SkipMigrations {
-		err := ApplyMigrations(
+		return nil
-			context.Background(), s.BaseDB, s, migrations,
-		)
-		if err != nil {
-			return nil, err
-		}
 	}
 
-	return s, nil
+	return ApplyMigrations(ctx, s.BaseDB, s, migrations)
 }
 
 // ExecuteMigrations runs migrations for the Postgres database, depending on the

sqldb/postgres_fixture.go

@@ -148,9 +148,13 @@ func NewTestPostgresDB(t *testing.T, fixture *TestPgFixture) *PostgresStore {
 	require.NoError(t, err)
 
 	cfg := fixture.GetConfig(dbName)
-	store, err := NewPostgresStore(cfg, GetMigrations())
+	store, err := NewPostgresStore(cfg)
 	require.NoError(t, err)
 
+	require.NoError(t, store.ApplyAllMigrations(
+		context.Background(), GetMigrations()),
+	)
+
 	return store
 }
 
@@ -172,7 +176,7 @@ func NewTestPostgresDBWithVersion(t *testing.T, fixture *TestPgFixture,
 
 	storeCfg := fixture.GetConfig(dbName)
 	storeCfg.SkipMigrations = true
-	store, err := NewPostgresStore(storeCfg, GetMigrations())
+	store, err := NewPostgresStore(storeCfg)
 	require.NoError(t, err)
 
 	err = store.ExecuteMigrations(TargetVersion(version))

sqldb/sqlite.go

@@ -38,6 +38,9 @@ var (
 
 	// Make sure SqliteStore implements the MigrationExecutor interface.
 	_ MigrationExecutor = (*SqliteStore)(nil)
+
+	// Make sure SqliteStore implements the DB interface.
+	_ DB = (*SqliteStore)(nil)
 )
 
 // SqliteStore is a database store implementation that uses a sqlite backend.
@@ -49,9 +52,7 @@ type SqliteStore struct {
 
 // NewSqliteStore attempts to open a new sqlite database based on the passed
 // config.
-func NewSqliteStore(cfg *SqliteConfig, dbPath string,
+func NewSqliteStore(cfg *SqliteConfig, dbPath string) (*SqliteStore, error) {
-	migrations []MigrationConfig) (*SqliteStore, error) {
-
 	// The set of pragma options are accepted using query options. For now
 	// we only want to ensure that foreign key constraints are properly
 	// enforced.
@@ -138,17 +139,26 @@ func NewSqliteStore(cfg *SqliteConfig, dbPath string,
 		},
 	}
 
+	return s, nil
+}
+
+// GetBaseDB returns the underlying BaseDB instance for the SQLite store.
+// It is a trivial helper method to comply with the sqldb.DB interface.
+func (s *SqliteStore) GetBaseDB() *BaseDB {
+	return s.BaseDB
+}
+
+// ApplyAllMigrations applies both the SQLC and custom in-code migrations to the
+// SQLite database.
+func (s *SqliteStore) ApplyAllMigrations(ctx context.Context,
+	migrations []MigrationConfig) error {
+
 	// Execute migrations unless configured to skip them.
-	if !cfg.SkipMigrations {
+	if s.cfg.SkipMigrations {
-		err := ApplyMigrations(
+		return nil
-			context.Background(), s.BaseDB, s, migrations,
-		)
-		if err != nil {
-			return nil, err
-		}
 	}
 
-	return s, nil
+	return ApplyMigrations(ctx, s.BaseDB, s, migrations)
 }
 
 // ExecuteMigrations runs migrations for the sqlite database, depending on the
@@ -181,9 +191,13 @@ func NewTestSqliteDB(t *testing.T) *SqliteStore {
 	dbFileName := filepath.Join(t.TempDir(), "tmp.db")
 	sqlDB, err := NewSqliteStore(&SqliteConfig{
 		SkipMigrations: false,
-	}, dbFileName, GetMigrations())
+	}, dbFileName)
 	require.NoError(t, err)
 
+	require.NoError(t, sqlDB.ApplyAllMigrations(
+		context.Background(), GetMigrations()),
+	)
+
 	t.Cleanup(func() {
 		require.NoError(t, sqlDB.DB.Close())
 	})
@@ -204,7 +218,7 @@ func NewTestSqliteDBWithVersion(t *testing.T, version uint) *SqliteStore {
 	dbFileName := filepath.Join(t.TempDir(), "tmp.db")
 	sqlDB, err := NewSqliteStore(&SqliteConfig{
 		SkipMigrations: true,
-	}, dbFileName, nil)
+	}, dbFileName)
 	require.NoError(t, err)
 
 	err = sqlDB.ExecuteMigrations(TargetVersion(version))