fix sqlite3

Instead of using the transaction to call Exec, the database is used which causes the rollback to not revert the new migration row.

LICENSE

@@ -0,0 +1,21 @@
+MIT License
+
+Copyright (c) 2020 Jon Calhoun
+
+Permission is hereby granted, free of charge, to any person obtaining a copy
+of this software and associated documentation files (the "Software"), to deal
+in the Software without restriction, including without limitation the rights
+to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+copies of the Software, and to permit persons to whom the Software is
+furnished to do so, subject to the following conditions:
+
+The above copyright notice and this permission notice shall be included in all
+copies or substantial portions of the Software.
+
+THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
+SOFTWARE.

go.mod

@@ -1,9 +1,9 @@
-module github.com/joncalhoun/migrate
+module gitea.nulo.in/Nulo/go-migrate
 
 go 1.14
 
 require (
 	github.com/jmoiron/sqlx v1.2.0
-	github.com/mattn/go-sqlite3 v2.0.3+incompatible
+	github.com/mattn/go-sqlite3 v1.14.16
 	google.golang.org/appengine v1.6.6 // indirect
 )

go.sum

@@ -6,6 +6,8 @@ github.com/jmoiron/sqlx v1.2.0/go.mod h1:1FEQNm3xlJgrMD+FBdI9+xvCksHtbpVBBw5dYhB
 github.com/lib/pq v1.0.0 h1:X5PMW56eZitiTeO7tKzZxFCSpbFZJtkMMooicw2us9A=
 github.com/lib/pq v1.0.0/go.mod h1:5WUZQaWbwv1U+lTReE5YruASi9Al49XbQIvNi/34Woo=
 github.com/mattn/go-sqlite3 v1.9.0/go.mod h1:FPy6KqzDD04eiIsT53CuJW3U88zkxoIYsOqkbpncsNc=
+github.com/mattn/go-sqlite3 v1.14.16 h1:yOQRA0RpS5PFz/oikGwBEqvAWhWg5ufRz4ETLjwpU1Y=
+github.com/mattn/go-sqlite3 v1.14.16/go.mod h1:2eHXhiwb8IkHr+BDWZGa96P6+rkvnG63S2DGjv9HUNg=
 github.com/mattn/go-sqlite3 v2.0.3+incompatible h1:gXHsfypPkaMZrKbD5209QV9jbUTJKjyR5WD3HYQSd+U=
 github.com/mattn/go-sqlite3 v2.0.3+incompatible/go.mod h1:FPy6KqzDD04eiIsT53CuJW3U88zkxoIYsOqkbpncsNc=
 golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACkg1iLfiJU5Ep61QUkGW8qpdssI0+w=

sqlx.go

@@ -49,6 +49,41 @@ func (s *Sqlx) Migrate(sqlDB *sql.DB, dialect string) error {
 	return nil
 }
 
+// Rollback will run all rollbacks using the provided db connection.
+func (s *Sqlx) Rollback(sqlDB *sql.DB, dialect string) error {
+	db := sqlx.NewDb(sqlDB, dialect)
+
+	s.printf("Creating/checking migrations table...\n")
+	err := s.createMigrationTable(db)
+	if err != nil {
+		return err
+	}
+	for i := len(s.Migrations) - 1; i >= 0; i-- {
+		m := s.Migrations[i]
+		if m.Rollback == nil {
+			s.printf("Rollback not provided: %v\n", m.ID)
+			continue
+		}
+		var found string
+		err := db.Get(&found, "SELECT id FROM migrations WHERE id=$1", m.ID)
+		switch err {
+		case sql.ErrNoRows:
+			s.printf("Skipping rollback: %v\n", m.ID)
+			continue
+		case nil:
+			s.printf("Running rollback: %v\n", m.ID)
+			// we need to run the rollback so we continue to code below
+		default:
+			return fmt.Errorf("looking up rollback by id: %w", err)
+		}
+		err = s.runRollback(db, m)
+		if err != nil {
+			return err
+		}
+	}
+	return nil
+}
+
 func (s *Sqlx) printf(format string, a ...interface{}) (n int, err error) {
 	printf := s.Printf
 	if printf == nil {
@@ -72,7 +107,7 @@ func (s *Sqlx) runMigration(db *sqlx.DB, m SqlxMigration) error {
 	if err != nil {
 		return errorf(err)
 	}
-	_, err = db.Exec("INSERT INTO migrations (id) VALUES ($1)", m.ID)
+	_, err = tx.Exec("INSERT INTO migrations (id) VALUES ($1)", m.ID)
 	if err != nil {
 		tx.Rollback()
 		return errorf(err)
@@ -89,48 +124,89 @@ func (s *Sqlx) runMigration(db *sqlx.DB, m SqlxMigration) error {
 	return nil
 }
 
+func (s *Sqlx) runRollback(db *sqlx.DB, m SqlxMigration) error {
+	errorf := func(err error) error { return fmt.Errorf("running rollback: %w", err) }
+
+	tx, err := db.Beginx()
+	if err != nil {
+		return errorf(err)
+	}
+	_, err = tx.Exec("DELETE FROM migrations WHERE id=$1", m.ID)
+	if err != nil {
+		tx.Rollback()
+		return errorf(err)
+	}
+	err = m.Rollback(tx)
+	if err != nil {
+		tx.Rollback()
+		return errorf(err)
+	}
+	err = tx.Commit()
+	if err != nil {
+		return errorf(err)
+	}
+	return nil
+}
+
 // SqlxMigration is a unique ID plus a function that uses a sqlx transaction
 // to perform a database migration step.
 //
 // Note: Long term this could have a Rollback field if we wanted to support
 // that.
 type SqlxMigration struct {
-	ID      string
-	Migrate func(tx *sqlx.Tx) error
+	ID       string
+	Migrate  func(tx *sqlx.Tx) error
+	Rollback func(tx *sqlx.Tx) error
 }
 
 // SqlxQueryMigration will create a SqlxMigration using the provided id and
 // query string. It is a helper function designed to simplify the process of
 // creating migrations that only depending on a SQL query string.
-func SqlxQueryMigration(id, query string) SqlxMigration {
-	m := SqlxMigration{
-		ID: id,
-		Migrate: func(tx *sqlx.Tx) error {
+func SqlxQueryMigration(id, upQuery, downQuery string) SqlxMigration {
+	queryFn := func(query string) func(tx *sqlx.Tx) error {
+		if query == "" {
+			return nil
+		}
+		return func(tx *sqlx.Tx) error {
 			_, err := tx.Exec(query)
 			return err
-		},
+		}
+	}
+
+	m := SqlxMigration{
+		ID:       id,
+		Migrate:  queryFn(upQuery),
+		Rollback: queryFn(downQuery),
 	}
 	return m
 }
 
 // SqlxFileMigration will create a SqlxMigration using the provided file.
-func SqlxFileMigration(id, filename string) SqlxMigration {
-	f, err := os.Open(filename)
-	if err != nil {
-		// We could return a migration that errors when the migration is run, but I
-		// think it makes more sense to panic here.
-		panic(err)
-	}
-	fileBytes, err := ioutil.ReadAll(f)
-	if err != nil {
-		panic(err)
-	}
-	m := SqlxMigration{
-		ID: id,
-		Migrate: func(tx *sqlx.Tx) error {
+func SqlxFileMigration(id, upFile, downFile string) SqlxMigration {
+	fileFn := func(filename string) func(tx *sqlx.Tx) error {
+		if filename == "" {
+			return nil
+		}
+		f, err := os.Open(filename)
+		if err != nil {
+			// We could return a migration that errors when the migration is run, but I
+			// think it makes more sense to panic here.
+			panic(err)
+		}
+		fileBytes, err := ioutil.ReadAll(f)
+		if err != nil {
+			panic(err)
+		}
+		return func(tx *sqlx.Tx) error {
 			_, err := tx.Exec(string(fileBytes))
 			return err
-		},
+		}
+	}
+
+	m := SqlxMigration{
+		ID:       id,
+		Migrate:  fileFn(upFile),
+		Rollback: fileFn(downFile),
 	}
 	return m
 }

sqlx_test.go

@@ -5,7 +5,7 @@ import (
 	"fmt"
 	"testing"
 
-	"github.com/joncalhoun/migrate"
+	"gitea.nulo.in/Nulo/go-migrate"
 	_ "github.com/mattn/go-sqlite3"
 )
 
@@ -33,7 +33,7 @@ func TestSqlx(t *testing.T) {
 				return 0, nil
 			},
 			Migrations: []migrate.SqlxMigration{
-				migrate.SqlxQueryMigration("001_create_courses", createCoursesSql),
+				migrate.SqlxQueryMigration("001_create_courses", createCoursesSql, ""),
 			},
 		}
 		err := migrator.Migrate(db, "sqlite3")
@@ -54,7 +54,7 @@ func TestSqlx(t *testing.T) {
 				return 0, nil
 			},
 			Migrations: []migrate.SqlxMigration{
-				migrate.SqlxQueryMigration("001_create_courses", createCoursesSql),
+				migrate.SqlxQueryMigration("001_create_courses", createCoursesSql, ""),
 			},
 		}
 		err := migrator.Migrate(db, "sqlite3")
@@ -73,8 +73,8 @@ func TestSqlx(t *testing.T) {
 				return 0, nil
 			},
 			Migrations: []migrate.SqlxMigration{
-				migrate.SqlxQueryMigration("001_create_courses", createCoursesSql),
-				migrate.SqlxQueryMigration("002_create_users", createUsersSql),
+				migrate.SqlxQueryMigration("001_create_courses", createCoursesSql, ""),
+				migrate.SqlxQueryMigration("002_create_users", createUsersSql, ""),
 			},
 		}
 		err = migrator.Migrate(db, "sqlite3")
@@ -95,7 +95,7 @@ func TestSqlx(t *testing.T) {
 				return 0, nil
 			},
 			Migrations: []migrate.SqlxMigration{
-				migrate.SqlxFileMigration("001_create_widgets", "testdata/widgets.sql"),
+				migrate.SqlxFileMigration("001_create_widgets", "testdata/widgets.sql", ""),
 			},
 		}
 		err := migrator.Migrate(db, "sqlite3")
@@ -107,16 +107,68 @@ func TestSqlx(t *testing.T) {
 			t.Fatalf("db.Exec() err = %v; want nil", err)
 		}
 	})
+
+	t.Run("rollback", func(t *testing.T) {
+		db := sqliteInMem(t)
+		migrator := migrate.Sqlx{
+			Printf: func(format string, args ...interface{}) (int, error) {
+				t.Logf(format, args...)
+				return 0, nil
+			},
+			Migrations: []migrate.SqlxMigration{
+				migrate.SqlxQueryMigration("001_create_courses", createCoursesSql, dropCoursesSql),
+			},
+		}
+		err := migrator.Migrate(db, "sqlite3")
+		if err != nil {
+			t.Fatalf("Migrate() err = %v; want nil", err)
+		}
+		_, err = db.Exec("INSERT INTO courses (name) VALUES ($1) ", "cor_test")
+		if err != nil {
+			t.Fatalf("db.Exec() err = %v; want nil", err)
+		}
+		err = migrator.Rollback(db, "sqlite3")
+		if err != nil {
+			t.Fatalf("Rollback() err = %v; want nil", err)
+		}
+		var count int
+		err = db.QueryRow("SELECT COUNT(id) FROM courses;").Scan(&count)
+		if err == nil {
+			// Want an error here
+			t.Fatalf("db.QueryRow() err = nil; want table missing error")
+		}
+		// Don't want to test inner workings of lib, so let's just migrate again and verify we have a table now
+		err = migrator.Migrate(db, "sqlite3")
+		if err != nil {
+			t.Fatalf("Migrate() err = %v; want nil", err)
+		}
+		_, err = db.Exec("INSERT INTO courses (name) VALUES ($1) ", "cor_test")
+		if err != nil {
+			t.Fatalf("db.Exec() err = %v; want nil", err)
+		}
+		err = db.QueryRow("SELECT COUNT(*) FROM courses;").Scan(&count)
+		if err != nil {
+			// Want an error here
+			t.Fatalf("db.QueryRow() err = %v; want nil", err)
+		}
+		if count != 1 {
+			t.Fatalf("count = %d; want %d", count, 1)
+		}
+	})
 }
 
-var createCoursesSql = `
+var (
+	createCoursesSql = `
 CREATE TABLE courses (
   id serial PRIMARY KEY,
   name text
 );`
+	dropCoursesSql = `DROP TABLE courses;`
 
-var createUsersSql = `
+	createUsersSql = `
 CREATE TABLE users (
   id serial PRIMARY KEY,
   email text UNIQUE NOT NULL
 );`
+	dropUsersSql = `DROP TABLE users;`
+)

testdata/widgets.down.sql

@@ -0,0 +1,2 @@
+DROP TABLE widgets;
+