fix(unit-tests): do not use cache=shared for in-memory SQLite (#971)

This commit is contained in:
Alessandro (Ale) Segala
2025-09-21 17:32:41 -07:00
committed by GitHub
parent ffe18db2fb
commit 549d219f44
3 changed files with 35 additions and 21 deletions

View File

@@ -1,6 +1,7 @@
package bootstrap
import (
"database/sql"
"errors"
"fmt"
"log/slog"
@@ -140,6 +141,7 @@ func connectDatabase() (db *gorm.DB, err error) {
var dialector gorm.Dialector
// Choose the correct database provider
var onConnFn func(conn *sql.DB)
switch common.EnvConfig.DbProvider {
case common.DbProviderSqlite:
if common.EnvConfig.DbConnectionString == "" {
@@ -148,7 +150,7 @@ func connectDatabase() (db *gorm.DB, err error) {
sqliteutil.RegisterSqliteFunctions()
connString, dbPath, err := parseSqliteConnectionString(common.EnvConfig.DbConnectionString)
connString, dbPath, isMemoryDB, err := parseSqliteConnectionString(common.EnvConfig.DbConnectionString)
if err != nil {
return nil, err
}
@@ -159,6 +161,14 @@ func connectDatabase() (db *gorm.DB, err error) {
return nil, err
}
if isMemoryDB {
// For in-memory SQLite databases, we must limit to 1 open connection at the same time, or they won't see the whole data
// The other workaround, of using shared caches, doesn't work well with multiple write transactions trying to happen at once
onConnFn = func(conn *sql.DB) {
conn.SetMaxOpenConns(1)
}
}
dialector = sqlite.Open(connString)
case common.DbProviderPostgres:
if common.EnvConfig.DbConnectionString == "" {
@@ -176,6 +186,16 @@ func connectDatabase() (db *gorm.DB, err error) {
})
if err == nil {
slog.Info("Connected to database", slog.String("provider", string(common.EnvConfig.DbProvider)))
if onConnFn != nil {
conn, err := db.DB()
if err != nil {
slog.Warn("Failed to get database connection, will retry in 3s", slog.Int("attempt", i), slog.String("provider", string(common.EnvConfig.DbProvider)), slog.Any("error", err))
time.Sleep(3 * time.Second)
}
onConnFn(conn)
}
return db, nil
}
@@ -188,18 +208,18 @@ func connectDatabase() (db *gorm.DB, err error) {
return nil, err
}
func parseSqliteConnectionString(connString string) (parsedConnString string, dbPath string, err error) {
func parseSqliteConnectionString(connString string) (parsedConnString string, dbPath string, isMemoryDB bool, err error) {
if !strings.HasPrefix(connString, "file:") {
connString = "file:" + connString
}
// Check if we're using an in-memory database
isMemoryDB := isSqliteInMemory(connString)
isMemoryDB = isSqliteInMemory(connString)
// Parse the connection string
connStringUrl, err := url.Parse(connString)
if err != nil {
return "", "", fmt.Errorf("failed to parse SQLite connection string: %w", err)
return "", "", false, fmt.Errorf("failed to parse SQLite connection string: %w", err)
}
// Convert options for the old SQLite driver to the new one
@@ -208,7 +228,7 @@ func parseSqliteConnectionString(connString string) (parsedConnString string, db
// Add the default and required params
err = addSqliteDefaultParameters(connStringUrl, isMemoryDB)
if err != nil {
return "", "", fmt.Errorf("invalid SQLite connection string: %w", err)
return "", "", false, fmt.Errorf("invalid SQLite connection string: %w", err)
}
// Get the absolute path to the database
@@ -217,10 +237,10 @@ func parseSqliteConnectionString(connString string) (parsedConnString string, db
idx := strings.IndexRune(parsedConnString, '?')
dbPath, err = filepath.Abs(parsedConnString[len("file:"):idx])
if err != nil {
return "", "", fmt.Errorf("failed to determine absolute path to the database: %w", err)
return "", "", false, fmt.Errorf("failed to determine absolute path to the database: %w", err)
}
return parsedConnString, dbPath, nil
return parsedConnString, dbPath, isMemoryDB, nil
}
// The official C implementation of SQLite allows some additional properties in the connection string
@@ -272,11 +292,6 @@ func addSqliteDefaultParameters(connStringUrl *url.URL, isMemoryDB bool) error {
qs = make(url.Values, 2)
}
// If the database is in-memory, we must ensure that cache=shared is set
if isMemoryDB {
qs["cache"] = []string{"shared"}
}
// Check if the database is read-only or immutable
isReadOnly := false
if len(qs["mode"]) > 0 {

View File

@@ -205,7 +205,7 @@ func TestAddSqliteDefaultParameters(t *testing.T) {
name: "in-memory database",
input: "file::memory:",
isMemoryDB: true,
expected: "file::memory:?_pragma=busy_timeout%282500%29&_pragma=foreign_keys%281%29&_pragma=journal_mode%28MEMORY%29&_txlock=immediate&cache=shared",
expected: "file::memory:?_pragma=busy_timeout%282500%29&_pragma=foreign_keys%281%29&_pragma=journal_mode%28MEMORY%29&_txlock=immediate",
},
{
name: "read-only database with mode=ro",
@@ -249,12 +249,6 @@ func TestAddSqliteDefaultParameters(t *testing.T) {
isMemoryDB: false,
expected: "file:test.db?_pragma=busy_timeout%283000%29&_pragma=foreign_keys%281%29&_pragma=journal_mode%28TRUNCATE%29&_pragma=synchronous%28NORMAL%29&_txlock=immediate",
},
{
name: "in-memory database with cache already set",
input: "file::memory:?cache=private",
isMemoryDB: true,
expected: "file::memory:?_pragma=busy_timeout%282500%29&_pragma=foreign_keys%281%29&_pragma=journal_mode%28MEMORY%29&_txlock=immediate&cache=shared",
},
{
name: "database with mode=rw (not read-only)",
input: "file:test.db?mode=rw",

View File

@@ -36,7 +36,7 @@ func NewDatabaseForTest(t *testing.T) *gorm.DB {
// Connect to a new in-memory SQL database
db, err := gorm.Open(
sqlite.Open("file:"+dbName+"?mode=memory&cache=shared"),
sqlite.Open("file:"+dbName+"?mode=memory"),
&gorm.Config{
TranslateError: true,
Logger: logger.New(
@@ -52,9 +52,14 @@ func NewDatabaseForTest(t *testing.T) *gorm.DB {
})
require.NoError(t, err, "Failed to connect to test database")
// Perform migrations with the embedded migrations
sqlDB, err := db.DB()
require.NoError(t, err, "Failed to get sql.DB")
// For in-memory SQLite databases, we must limit to 1 open connection at the same time, or they won't see the whole data
// The other workaround, of using shared caches, doesn't work well with multiple write transactions trying to happen at once
sqlDB.SetMaxOpenConns(1)
// Perform migrations with the embedded migrations
driver, err := sqliteMigrate.WithInstance(sqlDB, &sqliteMigrate.Config{
NoTxWrap: true,
})