From a8577cb47feb88d9c19b48b14a4fd4d513a7e9e6 Mon Sep 17 00:00:00 2001 From: Miklos Vajna Date: Wed, 7 Feb 2024 19:09:33 +0100 Subject: [PATCH] root: simplify database migration We had 2 schema states + migration code between the two. Simplify this to just 2 state, and in the future just migrations have to be added. This is in preparation of adding created & modified columns for passwords, where migration will be needed, which is now much simpler. --- commands/pull.go | 2 + commands/root.go | 91 ++++++++----------------------------------- commands/root_test.go | 37 +----------------- guide/src/hacking.md | 8 ++++ guide/src/news.md | 4 ++ 5 files changed, 32 insertions(+), 110 deletions(-) diff --git a/commands/pull.go b/commands/pull.go index 8d0a134..872df96 100644 --- a/commands/pull.go +++ b/commands/pull.go @@ -26,6 +26,8 @@ func newPullCommand(ctx *Context) *cobra.Command { } ctx.NoWriteBack = true + // Prefer the pull result over the migration result. + ctx.DatabaseMigrated = false return nil }, } diff --git a/commands/root.go b/commands/root.go index 3014098..06efa19 100644 --- a/commands/root.go +++ b/commands/root.go @@ -128,7 +128,6 @@ func openDatabase(ctx *Context) error { if err != nil { return fmt.Errorf("getDatabasePath() failed: %s", err) } - createNew := true if pathExists(ctx.PermanentPath) { Remove(ctx.TempFile.Name()) command := Command("gpg", "--decrypt", "-a", "-o", ctx.TempFile.Name(), ctx.PermanentPath) @@ -136,7 +135,6 @@ func openDatabase(ctx *Context) error { if err != nil { return fmt.Errorf("Command() failed: %s", err) } - createNew = false } ctx.Database, err = sql.Open("sqlite3", ctx.TempFile.Name()) @@ -144,7 +142,7 @@ func openDatabase(ctx *Context) error { return fmt.Errorf("sql.Open() failed: %s", err) } - err = initDatabase(ctx, createNew) + err = initDatabase(ctx) if err != nil { return fmt.Errorf("initDatabase() failed: %s", err) } @@ -152,55 +150,7 @@ func openDatabase(ctx *Context) error { return nil } -func initDatabaseWithVersion(ctx *Context, version int) error { - var statement string - if version == 0 { - statement = `create table passwords ( - machine text not null, - service text not null, - user text not null, - password text not null, - type text not null, - unique(machine, service, user, type) - )` - } else { - statement = `create table passwords ( - id integer primary key autoincrement, - machine text not null, - service text not null, - user text not null, - password text not null, - type text not null, - unique(machine, service, user, type) - )` - } - query, err := ctx.Database.Prepare(statement) - if err != nil { - return fmt.Errorf("db.Prepare() failed: %s", err) - } - _, err = query.Exec() - if err != nil { - return fmt.Errorf("db.Exec() failed: %s", err) - } - - return nil -} - -func initDatabase(ctx *Context, createNew bool) error { - // We need createNew because both an empty db and the first schema was user_version == 0. - if createNew { - initDatabaseWithVersion(ctx, 1) - - query, err := ctx.Database.Prepare("pragma user_version = 1") - if err != nil { - return fmt.Errorf("db.Prepare() failed: %s", err) - } - _, err = query.Exec() - if err != nil { - return fmt.Errorf("db.Exec() failed: %s", err) - } - } - +func initDatabase(ctx *Context) error { var version int rows, err := ctx.Database.Query("pragma user_version") if err != nil { @@ -215,31 +165,24 @@ func initDatabase(ctx *Context, createNew bool) error { } if version < 1 { - statements := []string{`create table passwords_copy( - id integer primary key autoincrement, - machine text not null, - service text not null, - user text not null, - password text not null, - type text not null, - unique(machine, service, user, type) - )`, - "insert into passwords_copy(machine, service, user, password, type) select machine, service, user, password, type from passwords", - "drop table passwords", - "alter table passwords_copy rename to passwords", + query, err := ctx.Database.Prepare(`create table passwords ( + id integer primary key autoincrement, + machine text not null, + service text not null, + user text not null, + password text not null, + type text not null, + unique(machine, service, user, type) + );`) + if err != nil { + return fmt.Errorf("db.Prepare() failed: %s", err) } - for _, statement := range statements { - query, err := ctx.Database.Prepare(statement) - if err != nil { - return fmt.Errorf("db.Prepare() failed: %s", err) - } - _, err = query.Exec() - if err != nil { - return fmt.Errorf("db.Exec() failed: %s", err) - } + _, err = query.Exec() + if err != nil { + return fmt.Errorf("db.Exec() failed: %s", err) } - query, err := ctx.Database.Prepare("pragma user_version = 1") + query, err = ctx.Database.Prepare("pragma user_version = 1") if err != nil { return fmt.Errorf("db.Prepare() failed: %s", err) } diff --git a/commands/root_test.go b/commands/root_test.go index be45061..e2dc7ac 100644 --- a/commands/root_test.go +++ b/commands/root_test.go @@ -41,7 +41,7 @@ func CreateContextForTesting(t *testing.T) Context { t.Cleanup(func() { GenerateTotpCode = oldGenerateTotpCode }) ctx := Context{Database: db} - err = initDatabase(&ctx, true) + err = initDatabase(&ctx) if err != nil { t.Fatalf("initDatabase() = %q, want nil", err) } @@ -182,38 +182,3 @@ func TestGetDatabasePath(t *testing.T) { t.Fatalf("getDatabasePath() = %q, want %q", actual, expected) } } - -func TestMigration(t *testing.T) { - // Create an in-memory database. - db, err := sql.Open("sqlite3", ":memory:") - if err != nil { - t.Fatalf("sql.Open() failed: %s", err) - } - ctx := Context{Database: db} - err = initDatabaseWithVersion(&ctx, 0) - if err != nil { - t.Fatalf("initDatabaseWithVersion() failed: %s", err) - } - - err = initDatabase(&ctx, false) - if err != nil { - t.Fatalf("initDatabase() failed: %s", err) - } - - var actual int - rows, err := db.Query("pragma user_version") - if err != nil { - t.Fatalf("Query() failed: %s", err) - } - defer rows.Close() - for rows.Next() { - err = rows.Scan(&actual) - if err != nil { - t.Fatalf("rows.Scan() failed: %s", err) - } - } - expected := 1 - if actual != expected { - t.Fatalf("initDatabase() = %q, want %q", actual, expected) - } -} diff --git a/guide/src/hacking.md b/guide/src/hacking.md index 96e0803..5fe64a9 100644 --- a/guide/src/hacking.md +++ b/guide/src/hacking.md @@ -17,3 +17,11 @@ Changes to the shell completion can be tested, without restarting the shell usin ```console source <(cpm completion bash) ``` + +## Go debugging + +To run a single test: + +```console +go test -run=TestInsert ./... +``` diff --git a/guide/src/news.md b/guide/src/news.md index 3d136e8..d6a872a 100644 --- a/guide/src/news.md +++ b/guide/src/news.md @@ -1,5 +1,9 @@ # Changelog +## main + +- drop compatibility with cpm < 7.5: update from old versions to 24.2 first + ## 24.2 - create / update: new `-y` switch to generate more secure passwords (3 instead of 0 numeric