From 5b2612af54595e754529927c3387f75ec54e3325 Mon Sep 17 00:00:00 2001 From: Matt Baer Date: Tue, 3 Mar 2020 11:26:23 -0600 Subject: [PATCH 1/7] Fix `created_at` default val in v4 migration for SQLite This previously used a default timestamp value which caused the migration to fail for SQLite databases. --- db/create.go | 9 +++++++++ migrations/v4.go | 2 +- 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/db/create.go b/db/create.go index c384778..fc1ab99 100644 --- a/db/create.go +++ b/db/create.go @@ -139,6 +139,15 @@ func (c *Column) SetDefault(value string) *Column { return c } +func (c *Column) SetDefaultCurrentTimestamp() *Column { + def := "NOW()" + if c.Dialect == DialectSQLite { + def = "CURRENT_TIMESTAMP" + } + c.Default = OptionalString{Set: true, Value: def} + return c +} + func (c *Column) SetType(t ColumnType) *Column { c.Type = t return c diff --git a/migrations/v4.go b/migrations/v4.go index c075dd8..93bfbc1 100644 --- a/migrations/v4.go +++ b/migrations/v4.go @@ -29,7 +29,7 @@ func oauth(db *datastore) error { SetIfNotExists(true). Column(dialect.Column("state", wf_db.ColumnTypeVarChar, wf_db.OptionalInt{Set: true, Value: 255})). Column(dialect.Column("used", wf_db.ColumnTypeBool, wf_db.UnsetSize)). - Column(dialect.Column("created_at", wf_db.ColumnTypeDateTime, wf_db.UnsetSize).SetDefault("NOW()")). + Column(dialect.Column("created_at", wf_db.ColumnTypeDateTime, wf_db.UnsetSize).SetDefaultCurrentTimestamp()). UniqueConstraint("state"). ToSQL() if err != nil { From f1ffcf96ec6f629078fbc1b298d2aa52b95a4dcc Mon Sep 17 00:00:00 2001 From: Matt Baer Date: Tue, 3 Mar 2020 11:36:30 -0600 Subject: [PATCH 2/7] Remove user_id and remote_user_id constraints in v4&v5 migrations It's not straightforward to remove these constraints in SQLite, so this just skips it entirely. Since both of these migrations are part of the same WF release, this should have minimal impact on admins. --- migrations/v4.go | 2 -- migrations/v5.go | 2 -- 2 files changed, 4 deletions(-) diff --git a/migrations/v4.go b/migrations/v4.go index 93bfbc1..403a3f0 100644 --- a/migrations/v4.go +++ b/migrations/v4.go @@ -18,8 +18,6 @@ func oauth(db *datastore) error { SetIfNotExists(true). Column(dialect.Column("user_id", wf_db.ColumnTypeInteger, wf_db.UnsetSize)). Column(dialect.Column("remote_user_id", wf_db.ColumnTypeInteger, wf_db.UnsetSize)). - UniqueConstraint("user_id"). - UniqueConstraint("remote_user_id"). ToSQL() if err != nil { return err diff --git a/migrations/v5.go b/migrations/v5.go index 94e3944..0dc5129 100644 --- a/migrations/v5.go +++ b/migrations/v5.go @@ -49,8 +49,6 @@ func oauthSlack(db *datastore) error { "access_token", wf_db.ColumnTypeVarChar, wf_db.OptionalInt{Set: true, Value: 512,})), - dialect.DropIndex("remote_user_id", "oauth_users"), - dialect.DropIndex("user_id", "oauth_users"), dialect.CreateUniqueIndex("oauth_users", "oauth_users", "user_id", "provider", "client_id"), } for _, builder := range builders { From bb5da1d3f579b4ffa67ca77797d4908b8147e170 Mon Sep 17 00:00:00 2001 From: Matt Baer Date: Tue, 3 Mar 2020 11:40:56 -0600 Subject: [PATCH 3/7] Break up v5 table ALTERs for SQLite Combining all operations into a single query was causing problems in SQLite. This fixes that by breaking them up into separate queries. It also moves one column length change to only run on MySQL, since SQLite doesn't need it. --- migrations/v5.go | 32 ++++++++++++++++++++++---------- 1 file changed, 22 insertions(+), 10 deletions(-) diff --git a/migrations/v5.go b/migrations/v5.go index 0dc5129..dc404e7 100644 --- a/migrations/v5.go +++ b/migrations/v5.go @@ -20,30 +20,30 @@ func oauthSlack(db *datastore) error { Column( "provider", wf_db.ColumnTypeVarChar, - wf_db.OptionalInt{Set: true, Value: 24,})). + wf_db.OptionalInt{Set: true, Value: 24})), + dialect. + AlterTable("oauth_client_states"). AddColumn(dialect. Column( "client_id", wf_db.ColumnTypeVarChar, - wf_db.OptionalInt{Set: true, Value: 128,})), + wf_db.OptionalInt{Set: true, Value: 128})), dialect. AlterTable("oauth_users"). - ChangeColumn("remote_user_id", - dialect. - Column( - "remote_user_id", - wf_db.ColumnTypeVarChar, - wf_db.OptionalInt{Set: true, Value: 128,})). AddColumn(dialect. Column( "provider", wf_db.ColumnTypeVarChar, - wf_db.OptionalInt{Set: true, Value: 24,})). + wf_db.OptionalInt{Set: true, Value: 24})), + dialect. + AlterTable("oauth_users"). AddColumn(dialect. Column( "client_id", wf_db.ColumnTypeVarChar, - wf_db.OptionalInt{Set: true, Value: 128,})). + wf_db.OptionalInt{Set: true, Value: 128})), + dialect. + AlterTable("oauth_users"). AddColumn(dialect. Column( "access_token", @@ -51,6 +51,18 @@ func oauthSlack(db *datastore) error { wf_db.OptionalInt{Set: true, Value: 512,})), dialect.CreateUniqueIndex("oauth_users", "oauth_users", "user_id", "provider", "client_id"), } + + if dialect != wf_db.DialectSQLite { + // This updates the length of the `remote_user_id` column. It isn't needed for SQLite databases. + builders = append(builders, dialect. + AlterTable("oauth_users"). + ChangeColumn("remote_user_id", + dialect. + Column( + "remote_user_id", + wf_db.ColumnTypeVarChar, + wf_db.OptionalInt{Set: true, Value: 128}))) + } for _, builder := range builders { query, err := builder.ToSQL() if err != nil { From 471ef4d4032469d9bb8b96f381ecd7b5852f590e Mon Sep 17 00:00:00 2001 From: Matt Baer Date: Tue, 3 Mar 2020 11:43:46 -0600 Subject: [PATCH 4/7] Fix "NOT NULL column with NULL" error in v5 SQLite migration Previously, this migration would cause the error: "Cannot add a NOT NULL column with default value NULL". This fixes that by setting the default value for new columns to '' (empty string). It updates the query builder to support this, too. --- db/create.go | 7 +++++-- migrations/v5.go | 11 ++++++----- 2 files changed, 11 insertions(+), 7 deletions(-) diff --git a/db/create.go b/db/create.go index fc1ab99..9032770 100644 --- a/db/create.go +++ b/db/create.go @@ -177,7 +177,11 @@ func (c *Column) String() (string, error) { if c.Default.Set { str.WriteString(" DEFAULT ") - str.WriteString(c.Default.Value) + val := c.Default.Value + if val == "" { + val = "''" + } + str.WriteString(val) } if c.PrimaryKey { @@ -250,4 +254,3 @@ func (b *CreateTableSqlBuilder) ToSQL() (string, error) { return str.String(), nil } - diff --git a/migrations/v5.go b/migrations/v5.go index dc404e7..88ddd28 100644 --- a/migrations/v5.go +++ b/migrations/v5.go @@ -20,35 +20,35 @@ func oauthSlack(db *datastore) error { Column( "provider", wf_db.ColumnTypeVarChar, - wf_db.OptionalInt{Set: true, Value: 24})), + wf_db.OptionalInt{Set: true, Value: 24}).SetDefault("")), dialect. AlterTable("oauth_client_states"). AddColumn(dialect. Column( "client_id", wf_db.ColumnTypeVarChar, - wf_db.OptionalInt{Set: true, Value: 128})), + wf_db.OptionalInt{Set: true, Value: 128}).SetDefault("")), dialect. AlterTable("oauth_users"). AddColumn(dialect. Column( "provider", wf_db.ColumnTypeVarChar, - wf_db.OptionalInt{Set: true, Value: 24})), + wf_db.OptionalInt{Set: true, Value: 24}).SetDefault("")), dialect. AlterTable("oauth_users"). AddColumn(dialect. Column( "client_id", wf_db.ColumnTypeVarChar, - wf_db.OptionalInt{Set: true, Value: 128})), + wf_db.OptionalInt{Set: true, Value: 128}).SetDefault("")), dialect. AlterTable("oauth_users"). AddColumn(dialect. Column( "access_token", wf_db.ColumnTypeVarChar, - wf_db.OptionalInt{Set: true, Value: 512,})), + wf_db.OptionalInt{Set: true, Value: 512}).SetDefault("")), dialect.CreateUniqueIndex("oauth_users", "oauth_users", "user_id", "provider", "client_id"), } @@ -63,6 +63,7 @@ func oauthSlack(db *datastore) error { wf_db.ColumnTypeVarChar, wf_db.OptionalInt{Set: true, Value: 128}))) } + for _, builder := range builders { query, err := builder.ToSQL() if err != nil { From 83b2c5a21ba0dea22ca278b0cf2befbbedafbb08 Mon Sep 17 00:00:00 2001 From: Matt Baer Date: Tue, 3 Mar 2020 11:46:51 -0600 Subject: [PATCH 5/7] Fix unique index on v5 SQLite migration This index needed a unique name in order for this query to succeed. --- migrations/v5.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/migrations/v5.go b/migrations/v5.go index 88ddd28..adc48da 100644 --- a/migrations/v5.go +++ b/migrations/v5.go @@ -49,7 +49,7 @@ func oauthSlack(db *datastore) error { "access_token", wf_db.ColumnTypeVarChar, wf_db.OptionalInt{Set: true, Value: 512}).SetDefault("")), - dialect.CreateUniqueIndex("oauth_users", "oauth_users", "user_id", "provider", "client_id"), + dialect.CreateUniqueIndex("oauth_users_uk", "oauth_users", "user_id", "provider", "client_id"), } if dialect != wf_db.DialectSQLite { From 61ddcff2c035ffd7aacf5a15d14eee5e046759d8 Mon Sep 17 00:00:00 2001 From: Matt Baer Date: Tue, 3 Mar 2020 11:47:38 -0600 Subject: [PATCH 6/7] Add copyright notices to fixed files --- db/create.go | 10 ++++++++++ migrations/v4.go | 10 ++++++++++ migrations/v5.go | 10 ++++++++++ 3 files changed, 30 insertions(+) diff --git a/db/create.go b/db/create.go index 9032770..648f93a 100644 --- a/db/create.go +++ b/db/create.go @@ -1,3 +1,13 @@ +/* + * Copyright © 2019-2020 A Bunch Tell LLC. + * + * This file is part of WriteFreely. + * + * WriteFreely is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License, included + * in the LICENSE file in this source code package. + */ + package db import ( diff --git a/migrations/v4.go b/migrations/v4.go index 403a3f0..fbfe2b5 100644 --- a/migrations/v4.go +++ b/migrations/v4.go @@ -1,3 +1,13 @@ +/* + * Copyright © 2019-2020 A Bunch Tell LLC. + * + * This file is part of WriteFreely. + * + * WriteFreely is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License, included + * in the LICENSE file in this source code package. + */ + package migrations import ( diff --git a/migrations/v5.go b/migrations/v5.go index adc48da..f93d067 100644 --- a/migrations/v5.go +++ b/migrations/v5.go @@ -1,3 +1,13 @@ +/* + * Copyright © 2019-2020 A Bunch Tell LLC. + * + * This file is part of WriteFreely. + * + * WriteFreely is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License, included + * in the LICENSE file in this source code package. + */ + package migrations import ( From 32f3fcb8595945db1766cf78a84265efc2e08a5d Mon Sep 17 00:00:00 2001 From: Matt Baer Date: Tue, 3 Mar 2020 11:48:04 -0600 Subject: [PATCH 7/7] Skip IF [TABLE] NOT EXISTS on v4 migrations We'd like these queries to fail correctly if the tables exist. --- migrations/v4.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/migrations/v4.go b/migrations/v4.go index fbfe2b5..7d73f96 100644 --- a/migrations/v4.go +++ b/migrations/v4.go @@ -25,7 +25,7 @@ func oauth(db *datastore) error { return wf_db.RunTransactionWithOptions(context.Background(), db.DB, &sql.TxOptions{}, func(ctx context.Context, tx *sql.Tx) error { createTableUsersOauth, err := dialect. Table("oauth_users"). - SetIfNotExists(true). + SetIfNotExists(false). Column(dialect.Column("user_id", wf_db.ColumnTypeInteger, wf_db.UnsetSize)). Column(dialect.Column("remote_user_id", wf_db.ColumnTypeInteger, wf_db.UnsetSize)). ToSQL() @@ -34,7 +34,7 @@ func oauth(db *datastore) error { } createTableOauthClientState, err := dialect. Table("oauth_client_states"). - SetIfNotExists(true). + SetIfNotExists(false). Column(dialect.Column("state", wf_db.ColumnTypeVarChar, wf_db.OptionalInt{Set: true, Value: 255})). Column(dialect.Column("used", wf_db.ColumnTypeBool, wf_db.UnsetSize)). Column(dialect.Column("created_at", wf_db.ColumnTypeDateTime, wf_db.UnsetSize).SetDefaultCurrentTimestamp()).