fix: database migration (#980)

Author: morremeyer

.gitignore

@@ -1,6 +1,4 @@
 tmp
-gorm.db
-gorm.db-journal
 coverage.out
 /backend
 dist/
@@ -17,3 +15,6 @@ package.json
 
 # Debugging binaries
 __debug_bin*
+
+# Database
+/data/**

.pre-commit-config.yaml

@@ -1,7 +1,8 @@
 default_stages: [commit]
 repos:
   - repo: https://github.com/alessandrojcm/commitlint-pre-commit-hook
-    rev: v9.12.0
+    # Waiting for new release, see https://github.com/alessandrojcm/commitlint-pre-commit-hook/issues/152
+    rev: 46ad8d10517771202384ef846ec4a5aa17e6ad26
     hooks:
       - id: commitlint
         stages: [commit-msg]

main.go

@@ -60,7 +60,7 @@ func main() {
 		log.Fatal().Err(err).Msg("Failed to create database directory")
 	}
 
-	err = models.Connect("data/gorm.db?_pragma=foreign_keys(1)")
+	err = models.Connect("data/gorm.db")
 	if err != nil {
 		log.Fatal().Msg(err.Error())
 	}

pkg/controllers/healthz/healthz_test.go

@@ -1,7 +1,6 @@
 package healthz_test
 
 import (
-	"fmt"
 	"net/http"
 	"net/http/httptest"
 	"testing"
@@ -31,7 +30,7 @@ func TestOptions(t *testing.T) {
 }
 
 func TestGet(t *testing.T) {
-	require.Nil(t, models.Connect(fmt.Sprintf("%s?_pragma=foreign_keys(1)", test.TmpFile(t))))
+	require.Nil(t, models.Connect(test.TmpFile(t)))
 
 	t.Parallel()
 	recorder := httptest.NewRecorder()

pkg/controllers/v4/test_suite_test.go

@@ -6,6 +6,7 @@ import (
 	"testing"
 
 	"github.com/envelope-zero/backend/v5/pkg/models"
+	"github.com/envelope-zero/backend/v5/test"
 	"github.com/stretchr/testify/suite"
 )
 
@@ -35,7 +36,7 @@ func (suite *TestSuiteStandard) TearDownTest() {
 
 // SetupTest is called before each test in the suite.
 func (suite *TestSuiteStandard) SetupTest() {
-	err := models.Connect(":memory:?_pragma=foreign_keys(1)")
+	err := models.Connect(test.TmpFile(suite.T()))
 	if err != nil {
 		log.Fatalf("Database initialization failed with: %#v", err)
 	}

pkg/importer/parser/ynab4/parse_test.go

@@ -15,6 +15,7 @@ import (
 	"github.com/envelope-zero/backend/v5/pkg/importer"
 	"github.com/envelope-zero/backend/v5/pkg/importer/parser/ynab4"
 	"github.com/envelope-zero/backend/v5/pkg/models"
+	"github.com/envelope-zero/backend/v5/test"
 	"github.com/shopspring/decimal"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
@@ -28,9 +29,9 @@ func date(year int, month time.Month, day int) time.Time {
 }
 
 // testDB returns an in-memory test database and a function to close it.
-func testDB() (*gorm.DB, func() error) {
+func testDB(t *testing.T) (*gorm.DB, func() error) {
 	// Connect a database
-	err := models.Connect(":memory:?_pragma=foreign_keys(1)")
+	err := models.Connect(test.TmpFile(t))
 	if err != nil {
 		log.Fatalf("Database connection failed with: %#v", err)
 	}
@@ -88,7 +89,7 @@ func TestParse(t *testing.T) {
 	require.Nil(t, err, "Parsing failed", err)
 
 	// Create test database and import
-	db, closeDb := testDB()
+	db, closeDb := testDB(t)
 	defer closeDb()
 
 	b, err := importer.Create(db, r)

pkg/models/database.go

@@ -29,15 +29,39 @@ func Connect(dsn string) error {
 		Logger: gorm_zerolog.New(),
 	}
 
+	// Migration with foreign keys disabled since we're dropping tables
+	// during migration
+	//
+	// sqlite does not support ALTER COLUMN, so tables are copied to a temporary table,
+	// then the table is dropped and recreated
 	db, err := gorm.Open(sqlite.Open(dsn), config)
 	if err != nil {
 		return fmt.Errorf("failed to connect to database: %w", err)
 	}
 
+	err = migrate(db)
+	if err != nil {
+		return err
+	}
+
+	// Close the connection
 	sqlDB, err := db.DB()
 	if err != nil {
 		return fmt.Errorf("failed to get database object: %w", err)
 	}
+	sqlDB.Close()
+
+	// Now, reconnect with foreign keys enabled
+	dsn = fmt.Sprintf("%s?_pragma=foreign_keys(1)", dsn)
+	db, err = gorm.Open(sqlite.Open(dsn), config)
+	if err != nil {
+		return fmt.Errorf("failed to connect to database: %w", err)
+	}
+
+	sqlDB, err = db.DB()
+	if err != nil {
+		return fmt.Errorf("failed to get database object: %w", err)
+	}
 
 	// Get new connections after one hour
 	sqlDB.SetConnMaxLifetime(time.Hour)
@@ -47,11 +71,6 @@ func Connect(dsn string) error {
 	sqlDB.SetMaxIdleConns(1)
 	sqlDB.SetMaxOpenConns(1)
 
-	err = migrate(db)
-	if err != nil {
-		return err
-	}
-
 	// Query callbacks
 	err = db.Callback().Query().After("*").Register("envelope_zero:after_query", queryCallback)
 	if err != nil {

pkg/models/database_test.go

@@ -1,7 +1,7 @@
 package models_test
 
 import (
-	"fmt"
+	"os"
 	"testing"
 
 	"github.com/envelope-zero/backend/v5/pkg/models"
@@ -13,13 +13,38 @@ func TestMigrateWithExistingDB(t *testing.T) {
 	testDB := test.TmpFile(t)
 
 	// Migrate the database once
-	require.Nil(t, models.Connect(fmt.Sprintf("%s?_pragma=foreign_keys(1)", testDB)))
+	require.Nil(t, models.Connect(testDB))
 
 	// Close the connection
 	sqlDB, err := models.DB.DB()
 	require.Nil(t, err)
 	sqlDB.Close()
 
 	// Migrate it again
-	require.Nil(t, models.Connect(fmt.Sprintf("%s?_pragma=foreign_keys(1)", testDB)))
+	require.Nil(t, models.Connect(testDB))
+}
+
+// TestV4V5Migration tests the migration from v4 to v5
+func TestV4V5Migration(t *testing.T) {
+	dbFile := test.TmpFile(t)
+
+	input, err := os.ReadFile("../../testdata/migrations/v4-v5.db")
+	if err != nil {
+		t.Error("Could not read test database")
+	}
+	err = os.WriteFile(dbFile, input, 0o644)
+	if err != nil {
+		t.Error("Could not create temporary copy for database")
+	}
+
+	// Connect to the database
+	require.Nil(t, models.Connect(dbFile))
+
+	// Close the connection
+	sqlDB, err := models.DB.DB()
+	require.Nil(t, err)
+	sqlDB.Close()
+
+	// Reconnect
+	require.Nil(t, models.Connect(dbFile))
 }

pkg/models/test_suite_test.go

@@ -6,6 +6,7 @@ import (
 	"testing"
 
 	"github.com/envelope-zero/backend/v5/pkg/models"
+	"github.com/envelope-zero/backend/v5/test"
 	"github.com/google/uuid"
 	"github.com/stretchr/testify/suite"
 )
@@ -32,7 +33,7 @@ func (suite *TestSuiteStandard) TearDownTest() {
 
 // SetupTest is called before each test in the suite.
 func (suite *TestSuiteStandard) SetupTest() {
-	err := models.Connect(":memory:?_pragma=foreign_keys(1)")
+	err := models.Connect(test.TmpFile(suite.T()))
 	if err != nil {
 		log.Fatalf("Database connection failed with: %#v", err)
 	}