diff --git a/db/test/schema.go b/db/test/schema.go new file mode 100644 index 00000000..c3afc09f --- /dev/null +++ b/db/test/schema.go @@ -0,0 +1,185 @@ +package dbtest + +import ( + "database/sql" + "fmt" + "sort" + "strings" + "testing" + + "github.com/stretchr/testify/require" +) + +// Querier is the subset of *sql.DB / *db.DB needed to introspect a schema. +type Querier interface { + Query(query string, args ...any) (*sql.Rows, error) +} + +// SQLiteSchema returns a normalized, comparable description of the database schema: tables +// with their columns, named indexes, and foreign keys. Column order, declared type spelling +// (INT vs INTEGER) and default values are not part of the description, so the schema produced +// by a migration chain can be compared to a freshly created one. +func SQLiteSchema(t testing.TB, d Querier) string { + t.Helper() + lines := make([]string, 0) + for _, table := range sqliteTables(t, d) { + lines = append(lines, "table "+table) + lines = append(lines, sqliteColumns(t, d, table)...) + lines = append(lines, sqliteForeignKeys(t, d, table)...) + lines = append(lines, sqliteIndexes(t, d, table)...) + } + return strings.Join(lines, "\n") +} + +// PostgresSchema is SQLiteSchema's PostgreSQL counterpart, describing the current schema's +// tables, columns, constraints and indexes in a normalized, comparable way. +func PostgresSchema(t testing.TB, d Querier) string { + t.Helper() + lines := make([]string, 0) + for _, table := range postgresTables(t, d) { + lines = append(lines, "table "+table) + lines = append(lines, postgresColumns(t, d, table)...) + } + lines = append(lines, postgresConstraints(t, d)...) + lines = append(lines, postgresIndexes(t, d)...) + return strings.Join(lines, "\n") +} + +func sqliteTables(t testing.TB, d Querier) []string { + t.Helper() + return queryStrings(t, d, `SELECT name FROM sqlite_master WHERE type = 'table' AND name NOT LIKE 'sqlite_%' ORDER BY name`) +} + +func sqliteColumns(t testing.TB, d Querier, table string) []string { + t.Helper() + rows, err := d.Query(fmt.Sprintf(`PRAGMA table_info(%q)`, table)) + require.Nil(t, err) + defer rows.Close() + lines := make([]string, 0) + for rows.Next() { + var cid, notNull, pk int + var name, typ string + var dflt sql.NullString + require.Nil(t, rows.Scan(&cid, &name, &typ, ¬Null, &dflt, &pk)) + typ = strings.ToUpper(typ) + if typ == "INT" { // INT and INTEGER are the same affinity; migrations spell them inconsistently + typ = "INTEGER" + } + lines = append(lines, fmt.Sprintf(" col %s %s notnull=%d pk=%d", name, typ, notNull, pk)) + } + require.Nil(t, rows.Err()) + sort.Strings(lines) + return lines +} + +func sqliteForeignKeys(t testing.TB, d Querier, table string) []string { + t.Helper() + rows, err := d.Query(fmt.Sprintf(`PRAGMA foreign_key_list(%q)`, table)) + require.Nil(t, err) + defer rows.Close() + lines := make([]string, 0) + for rows.Next() { + var id, seq int + var refTable, from, onUpdate, onDelete, match string + var to sql.NullString // NULL when referencing the parent's primary key implicitly + require.Nil(t, rows.Scan(&id, &seq, &refTable, &from, &to, &onUpdate, &onDelete, &match)) + lines = append(lines, fmt.Sprintf(" fk %s -> %s(%s) on_delete=%s", from, refTable, to.String, onDelete)) + } + require.Nil(t, rows.Err()) + sort.Strings(lines) + return lines +} + +func sqliteIndexes(t testing.TB, d Querier, table string) []string { + t.Helper() + rows, err := d.Query(fmt.Sprintf(`PRAGMA index_list(%q)`, table)) + require.Nil(t, err) + type index struct { + name string + unique, partial int + } + indexes := make([]index, 0) + for rows.Next() { + var seq, unique, partial int + var name, origin string + require.Nil(t, rows.Scan(&seq, &name, &unique, &origin, &partial)) + // Skip auto-indexes backing PRIMARY KEY/UNIQUE table constraints; those are described + // by the column and constraint listings already + if strings.HasPrefix(name, "sqlite_autoindex_") { + continue + } + indexes = append(indexes, index{name, unique, partial}) + } + require.Nil(t, rows.Err()) + require.Nil(t, rows.Close()) + lines := make([]string, 0, len(indexes)) + for _, idx := range indexes { + cols := queryStrings(t, d, fmt.Sprintf(`SELECT name FROM pragma_index_info(%q) ORDER BY seqno`, idx.name)) + lines = append(lines, fmt.Sprintf(" index %s unique=%d partial=%d cols=(%s)", idx.name, idx.unique, idx.partial, strings.Join(cols, ","))) + } + sort.Strings(lines) + return lines +} + +func postgresTables(t testing.TB, d Querier) []string { + t.Helper() + return queryStrings(t, d, `SELECT table_name FROM information_schema.tables WHERE table_schema = current_schema() AND table_type = 'BASE TABLE' ORDER BY table_name`) +} + +func postgresColumns(t testing.TB, d Querier, table string) []string { + t.Helper() + rows, err := d.Query(`SELECT column_name, data_type, is_nullable FROM information_schema.columns WHERE table_schema = current_schema() AND table_name = $1 ORDER BY column_name`, table) + require.Nil(t, err) + defer rows.Close() + lines := make([]string, 0) + for rows.Next() { + var name, typ, nullable string + require.Nil(t, rows.Scan(&name, &typ, &nullable)) + lines = append(lines, fmt.Sprintf(" col %s %s nullable=%s", name, typ, nullable)) + } + require.Nil(t, rows.Err()) + return lines +} + +func postgresConstraints(t testing.TB, d Querier) []string { + t.Helper() + return queryStrings(t, d, ` + SELECT 'constraint ' || conrelid::regclass::text || ': ' || pg_get_constraintdef(oid) + FROM pg_constraint + WHERE connamespace = current_schema()::regnamespace + ORDER BY 1 + `) +} + +func postgresIndexes(t testing.TB, d Querier) []string { + t.Helper() + rows, err := d.Query(`SELECT indexname, indexdef, schemaname FROM pg_indexes WHERE schemaname = current_schema() ORDER BY indexname`) + require.Nil(t, err) + defer rows.Close() + lines := make([]string, 0) + for rows.Next() { + var name, def, schema string + require.Nil(t, rows.Scan(&name, &def, &schema)) + // The index definition qualifies the table with the (test-specific) schema name; strip + // it so snapshots from different test schemas compare equal + def = strings.ReplaceAll(def, schema+".", "") + lines = append(lines, "index "+def) + } + require.Nil(t, rows.Err()) + return lines +} + +func queryStrings(t testing.TB, d Querier, query string) []string { + t.Helper() + rows, err := d.Query(query) + require.Nil(t, err) + defer rows.Close() + values := make([]string, 0) + for rows.Next() { + var value string + require.Nil(t, rows.Scan(&value)) + values = append(values, value) + } + require.Nil(t, rows.Err()) + return values +} diff --git a/docs/releases.md b/docs/releases.md index 925d1c77..dbab6f5e 100644 --- a/docs/releases.md +++ b/docs/releases.md @@ -2066,6 +2066,7 @@ and the [ntfy Android app](https://github.com/binwiederhier/ntfy-android/release * Fix Twilio phone calls and phone number verifications failing silently when Twilio rejected the request, and move the Twilio integration into its own `twilio` package * Move the Prometheus metrics into a dedicated `metrics` package * Message cache databases from ntfy older than v1.10.0 (November 2021) can no longer be migrated; upgrade via an older ntfy version first, or delete the cache database +* Fix `user_phone` table in the SQLite user database referencing a dropped table after the v2.14 schema migration; repaired automatically by a new migration ### ntfy iOS app v1.8.0 (UNRELEASED) diff --git a/message/cache_postgres_test.go b/message/cache_postgres_test.go new file mode 100644 index 00000000..c8535862 --- /dev/null +++ b/message/cache_postgres_test.go @@ -0,0 +1,76 @@ +package message_test + +import ( + "testing" + + "github.com/stretchr/testify/require" + dbtest "heckel.io/ntfy/v2/db/test" + "heckel.io/ntfy/v2/message" + "heckel.io/ntfy/v2/model" +) + +func TestPostgresStore_Migration_From14(t *testing.T) { + // A pre-framework database at version 14: full v14 schema, version tracked in the + // hand-rolled schema_version table, and no idx_message_attachment_expires yet + testDB := dbtest.CreateTestPostgres(t) + _, err := testDB.Exec(` + CREATE TABLE message ( + id BIGSERIAL PRIMARY KEY, + mid TEXT NOT NULL, + sequence_id TEXT NOT NULL, + time BIGINT NOT NULL, + event TEXT NOT NULL, + expires BIGINT NOT NULL, + topic TEXT NOT NULL, + message TEXT NOT NULL, + title TEXT NOT NULL, + priority INT NOT NULL, + tags TEXT NOT NULL, + click TEXT NOT NULL, + icon TEXT NOT NULL, + actions TEXT NOT NULL, + attachment_name TEXT NOT NULL, + attachment_type TEXT NOT NULL, + attachment_size BIGINT NOT NULL, + attachment_expires BIGINT NOT NULL, + attachment_url TEXT NOT NULL, + attachment_deleted BOOLEAN NOT NULL DEFAULT FALSE, + sender TEXT NOT NULL, + user_id TEXT NOT NULL, + content_type TEXT NOT NULL, + encoding TEXT NOT NULL, + published BOOLEAN NOT NULL DEFAULT FALSE + ); + CREATE INDEX idx_message_mid ON message (mid); + CREATE INDEX idx_message_sequence_id ON message (sequence_id); + CREATE INDEX idx_message_topic_published_time ON message (topic, published, time, id); + CREATE INDEX idx_message_published_expires ON message (published, expires); + CREATE INDEX idx_message_sender_attachment_expires ON message (sender, attachment_expires) WHERE user_id = ''; + CREATE INDEX idx_message_user_id_attachment_expires ON message (user_id, attachment_expires); + CREATE TABLE message_stats (key TEXT PRIMARY KEY, value BIGINT); + INSERT INTO message_stats (key, value) VALUES ('messages', 0); + CREATE TABLE schema_version (store TEXT PRIMARY KEY, version INT NOT NULL); + INSERT INTO schema_version (store, version) VALUES ('message', 14); + `) + require.Nil(t, err) + store, err := message.NewPostgresStore(testDB, 0, 0) + require.Nil(t, err) + // The 14 -> 15 step ran: version bumped, partial index created + var version int + require.Nil(t, testDB.QueryRow(`SELECT version FROM schema_version WHERE store = 'message'`).Scan(&version)) + require.Equal(t, 15, version) + var indexCount int + require.Nil(t, testDB.QueryRow(`SELECT COUNT(*) FROM pg_indexes WHERE indexname = 'idx_message_attachment_expires' AND schemaname = current_schema()`).Scan(&indexCount)) + require.Equal(t, 1, indexCount) + // And the store works + require.Nil(t, store.AddMessage(model.NewDefaultMessage("mytopic", "hi there"))) + messages, err := store.Messages("mytopic", model.SinceAllMessages, false) + require.Nil(t, err) + require.Len(t, messages, 1) + + // The migrated database must be structurally identical to a freshly created one + freshDB := dbtest.CreateTestPostgres(t) + _, err = message.NewPostgresStore(freshDB, 0, 0) + require.Nil(t, err) + require.Equal(t, dbtest.PostgresSchema(t, freshDB), dbtest.PostgresSchema(t, testDB)) +} diff --git a/message/cache_sqlite_test.go b/message/cache_sqlite_test.go index 47519163..3180de02 100644 --- a/message/cache_sqlite_test.go +++ b/message/cache_sqlite_test.go @@ -9,6 +9,7 @@ import ( _ "github.com/mattn/go-sqlite3" // SQLite driver "github.com/stretchr/testify/require" + dbtest "heckel.io/ntfy/v2/db/test" "heckel.io/ntfy/v2/message" "heckel.io/ntfy/v2/model" ) @@ -50,6 +51,19 @@ func TestSqliteStore_Migration_From1(t *testing.T) { s := newSqliteTestStoreFromFile(t, filename, "") checkSqliteSchemaVersion(t, filename) + // The migrated database must be structurally identical to a freshly created one + freshFile := newSqliteTestStoreFile(t) + fresh, err := message.NewSQLiteStore(freshFile, "", time.Hour, 0, 0, false) + require.Nil(t, err) + t.Cleanup(func() { fresh.Close() }) + freshDB, err := sql.Open("sqlite3", freshFile) + require.Nil(t, err) + defer freshDB.Close() + migratedDB, err := sql.Open("sqlite3", filename) + require.Nil(t, err) + defer migratedDB.Close() + require.Equal(t, dbtest.SQLiteSchema(t, freshDB), dbtest.SQLiteSchema(t, migratedDB)) + // Add delayed message delayedMessage := model.NewDefaultMessage("mytopic", "some delayed message") delayedMessage.Time = time.Now().Add(time.Minute).Unix() diff --git a/message/cache_test.go b/message/cache_test.go index 04838abd..059a1f62 100644 --- a/message/cache_test.go +++ b/message/cache_test.go @@ -36,60 +36,6 @@ func newTestPostgresStore(t *testing.T) *message.Cache { return store } -func TestPostgresStore_Migration_From14(t *testing.T) { - // A pre-framework database at version 14: full v14 schema, version tracked in the - // hand-rolled schema_version table, and no idx_message_attachment_expires yet - testDB := dbtest.CreateTestPostgres(t) - _, err := testDB.Exec(` - CREATE TABLE message ( - id BIGSERIAL PRIMARY KEY, - mid TEXT NOT NULL, - sequence_id TEXT NOT NULL, - time BIGINT NOT NULL, - event TEXT NOT NULL, - expires BIGINT NOT NULL, - topic TEXT NOT NULL, - message TEXT NOT NULL, - title TEXT NOT NULL, - priority INT NOT NULL, - tags TEXT NOT NULL, - click TEXT NOT NULL, - icon TEXT NOT NULL, - actions TEXT NOT NULL, - attachment_name TEXT NOT NULL, - attachment_type TEXT NOT NULL, - attachment_size BIGINT NOT NULL, - attachment_expires BIGINT NOT NULL, - attachment_url TEXT NOT NULL, - attachment_deleted BOOLEAN NOT NULL DEFAULT FALSE, - sender TEXT NOT NULL, - user_id TEXT NOT NULL, - content_type TEXT NOT NULL, - encoding TEXT NOT NULL, - published BOOLEAN NOT NULL DEFAULT FALSE - ); - CREATE TABLE message_stats (key TEXT PRIMARY KEY, value BIGINT); - INSERT INTO message_stats (key, value) VALUES ('messages', 0); - CREATE TABLE schema_version (store TEXT PRIMARY KEY, version INT NOT NULL); - INSERT INTO schema_version (store, version) VALUES ('message', 14); - `) - require.Nil(t, err) - store, err := message.NewPostgresStore(testDB, 0, 0) - require.Nil(t, err) - // The 14 -> 15 step ran: version bumped, partial index created - var version int - require.Nil(t, testDB.QueryRow(`SELECT version FROM schema_version WHERE store = 'message'`).Scan(&version)) - require.Equal(t, 15, version) - var indexCount int - require.Nil(t, testDB.QueryRow(`SELECT COUNT(*) FROM pg_indexes WHERE indexname = 'idx_message_attachment_expires' AND schemaname = current_schema()`).Scan(&indexCount)) - require.Equal(t, 1, indexCount) - // And the store works - require.Nil(t, store.AddMessage(model.NewDefaultMessage("mytopic", "hi there"))) - messages, err := store.Messages("mytopic", model.SinceAllMessages, false) - require.Nil(t, err) - require.Len(t, messages, 1) -} - func forEachBackend(t *testing.T, f func(t *testing.T, s *message.Cache)) { t.Run("sqlite", func(t *testing.T) { f(t, newSqliteTestStore(t)) diff --git a/user/manager_postgres_schema.go b/user/manager_postgres_schema.go index 1038fb38..d1e585e8 100644 --- a/user/manager_postgres_schema.go +++ b/user/manager_postgres_schema.go @@ -96,7 +96,7 @@ const ( ) const ( - postgresCurrentSchemaVersion = 8 + postgresCurrentSchemaVersion = 9 ) const ( @@ -135,5 +135,6 @@ var ( postgresMigrations = map[int]schema.MigrateFunc{ 6: schema.AsMigrateFunc(postgresMigrate6To7UpdateQueries), 7: schema.AsMigrateFunc(postgresMigrate7To8UpdateQueries), + 8: schema.NopMigrateFunc, // 8 -> 9 repairs a SQLite-only foreign key defect; nothing to do on Postgres } ) diff --git a/user/manager_sqlite_schema.go b/user/manager_sqlite_schema.go index b64c6f31..fa8952d6 100644 --- a/user/manager_sqlite_schema.go +++ b/user/manager_sqlite_schema.go @@ -114,7 +114,7 @@ const ( ) const ( - sqliteCurrentSchemaVersion = 8 + sqliteCurrentSchemaVersion = 9 ) // Schema migrations for SQLite @@ -347,6 +347,25 @@ const ( CREATE UNIQUE INDEX idx_user_stripe_subscription_id ON user (stripe_subscription_id); CREATE UNIQUE INDEX idx_user_token ON user_token (token); ` + + // 8 -> 9: Repair the user_phone foreign key. The 5 -> 6 migration renamed user to + // user_old, which rewrote user_phone's REFERENCES clause to user_old -- a table that was + // then dropped (the rebuilt tables got correct fresh foreign keys; user_phone was the only + // child table not rebuilt). Rebuilding user_phone re-points the foreign key at user; on + // healthy databases the rebuild is a harmless no-op schema-wise. + sqliteMigrate8To9UpdateQueries = ` + ALTER TABLE user_phone RENAME TO user_phone_old; + CREATE TABLE user_phone ( + user_id TEXT NOT NULL, + phone_number TEXT NOT NULL, + PRIMARY KEY (user_id, phone_number), + FOREIGN KEY (user_id) REFERENCES user (id) ON DELETE CASCADE + ); + INSERT INTO user_phone (user_id, phone_number) + SELECT user_id, phone_number FROM user_phone_old + WHERE user_id IN (SELECT id FROM user); -- Drop orphaned rows that the broken foreign key failed to cascade-delete + DROP TABLE user_phone_old; + ` ) var ( @@ -362,6 +381,7 @@ var ( 5: schema.AsMigrateFunc(sqliteMigrate5To6UpdateQueries), 6: schema.AsMigrateFunc(sqliteMigrate6To7UpdateQueries), 7: schema.AsMigrateFunc(sqliteMigrate7To8UpdateQueries), + 8: schema.AsMigrateFunc(sqliteMigrate8To9UpdateQueries), } ) diff --git a/user/manager_test.go b/user/manager_test.go index a3104c79..686ff903 100644 --- a/user/manager_test.go +++ b/user/manager_test.go @@ -1567,6 +1567,83 @@ func TestToFromSQLWildcard(t *testing.T) { require.Equal(t, "foo", fromSQLWildcard(toSQLWildcard("foo"))) } +// testPostgresV6Schema is the PostgreSQL schema exactly as created by the version that first +// shipped Postgres support (schema version 6), taken from the code at that time; used to +// verify the migration chain from its oldest supported version. +const testPostgresV6Schema = ` + CREATE TABLE IF NOT EXISTS tier ( + id TEXT PRIMARY KEY, + code TEXT NOT NULL, + name TEXT NOT NULL, + messages_limit BIGINT NOT NULL, + messages_expiry_duration BIGINT NOT NULL, + emails_limit BIGINT NOT NULL, + calls_limit BIGINT NOT NULL, + reservations_limit BIGINT NOT NULL, + attachment_file_size_limit BIGINT NOT NULL, + attachment_total_size_limit BIGINT NOT NULL, + attachment_expiry_duration BIGINT NOT NULL, + attachment_bandwidth_limit BIGINT NOT NULL, + stripe_monthly_price_id TEXT, + stripe_yearly_price_id TEXT, + UNIQUE(code), + UNIQUE(stripe_monthly_price_id), + UNIQUE(stripe_yearly_price_id) + ); + CREATE TABLE IF NOT EXISTS "user" ( + id TEXT PRIMARY KEY, + tier_id TEXT REFERENCES tier(id), + user_name TEXT NOT NULL UNIQUE, + pass TEXT NOT NULL, + role TEXT NOT NULL CHECK (role IN ('anonymous', 'admin', 'user')), + prefs JSONB NOT NULL DEFAULT '{}', + sync_topic TEXT NOT NULL, + provisioned BOOLEAN NOT NULL, + stats_messages BIGINT NOT NULL DEFAULT 0, + stats_emails BIGINT NOT NULL DEFAULT 0, + stats_calls BIGINT NOT NULL DEFAULT 0, + stripe_customer_id TEXT UNIQUE, + stripe_subscription_id TEXT UNIQUE, + stripe_subscription_status TEXT, + stripe_subscription_interval TEXT, + stripe_subscription_paid_until BIGINT, + stripe_subscription_cancel_at BIGINT, + created BIGINT NOT NULL, + deleted BIGINT + ); + CREATE TABLE IF NOT EXISTS user_access ( + user_id TEXT NOT NULL REFERENCES "user"(id) ON DELETE CASCADE, + topic TEXT NOT NULL, + read BOOLEAN NOT NULL, + write BOOLEAN NOT NULL, + owner_user_id TEXT REFERENCES "user"(id) ON DELETE CASCADE, + provisioned BOOLEAN NOT NULL, + PRIMARY KEY (user_id, topic) + ); + CREATE TABLE IF NOT EXISTS user_token ( + user_id TEXT NOT NULL REFERENCES "user"(id) ON DELETE CASCADE, + token TEXT NOT NULL UNIQUE, + label TEXT NOT NULL, + last_access BIGINT NOT NULL, + last_origin TEXT NOT NULL, + expires BIGINT NOT NULL, + provisioned BOOLEAN NOT NULL, + PRIMARY KEY (user_id, token) + ); + CREATE TABLE IF NOT EXISTS user_phone ( + user_id TEXT NOT NULL REFERENCES "user"(id) ON DELETE CASCADE, + phone_number TEXT NOT NULL, + PRIMARY KEY (user_id, phone_number) + ); + CREATE TABLE IF NOT EXISTS schema_version ( + store TEXT PRIMARY KEY, + version INT NOT NULL + ); + INSERT INTO "user" (id, user_name, pass, role, sync_topic, provisioned, created) + VALUES ('u_everyone', '*', '', 'anonymous', '', false, EXTRACT(EPOCH FROM NOW())::BIGINT) + ON CONFLICT (id) DO NOTHING; +` + func TestMigrationFrom1(t *testing.T) { filename := filepath.Join(t.TempDir(), "user.db") db, err := sql.Open("sqlite3", filename) @@ -1650,6 +1727,8 @@ func TestMigrationFrom1(t *testing.T) { require.Equal(t, 1, len(everyoneGrants)) require.Equal(t, "stats", everyoneGrants[0].TopicPattern) require.Equal(t, PermissionRead, everyoneGrants[0].Permission) + + checkMigratedSqliteSchema(t, filename) } func TestMigrationFrom4(t *testing.T) { @@ -1741,12 +1820,15 @@ func TestMigrationFrom4(t *testing.T) { `) require.Nil(t, err) - // Insert a few ACL entries + // Insert a few ACL entries, and phone numbers: one for a live user, one orphaned (its user + // is gone; the broken pre-v9 foreign key never cascade-deleted it) _, err = db.Exec(` BEGIN; INSERT INTO user_access (user_id, topic, read, write) values ('u_everyone', 'mytopic_', 1, 1); INSERT INTO user_access (user_id, topic, read, write) values ('u_everyone', 'up%', 1, 1); INSERT INTO user_access (user_id, topic, read, write) values ('u_everyone', 'down_%', 1, 1); + INSERT INTO user_phone (user_id, phone_number) VALUES ('u_everyone', '+12223334444'); + INSERT INTO user_phone (user_id, phone_number) VALUES ('u_gone', '+15556667777'); COMMIT; `) require.Nil(t, err) @@ -1796,6 +1878,68 @@ func TestMigrationFrom4(t *testing.T) { require.Nil(t, a.Authorize(nil, "up123", PermissionRead)) require.Nil(t, a.Authorize(nil, "up", PermissionRead)) // % matches 0 or more characters + + // The 8 -> 9 repair kept the live user's phone number and dropped the orphaned row + phoneNumbers := make([]string, 0) + rows, err = db.Query(`SELECT phone_number FROM user_phone ORDER BY phone_number`) + require.Nil(t, err) + for rows.Next() { + var phoneNumber string + require.Nil(t, rows.Scan(&phoneNumber)) + phoneNumbers = append(phoneNumbers, phoneNumber) + } + require.Nil(t, rows.Close()) + require.Equal(t, []string{"+12223334444"}, phoneNumbers) + + checkMigratedSqliteSchema(t, filename) +} + +// TestMigrationFrom6Postgres tests the Postgres migration chain from its oldest supported +// version (6, the version PostgreSQL support first shipped with). +func TestMigrationFrom6Postgres(t *testing.T) { + testDB := dbtest.CreateTestPostgres(t) + _, err := testDB.Exec(testPostgresV6Schema) + require.Nil(t, err) + _, err = testDB.Exec(`INSERT INTO schema_version (store, version) VALUES ('user', 6)`) + require.Nil(t, err) + // Create manager to trigger migration + a, err := NewPostgresManager(testDB, &Config{DefaultAccess: PermissionDenyAll, BcryptCost: bcrypt.MinCost, QueueWriterInterval: DefaultUserStatsQueueWriterInterval}) + require.Nil(t, err) + var version int + require.Nil(t, testDB.QueryRow(`SELECT version FROM schema_version WHERE store = 'user'`).Scan(&version)) + require.Equal(t, postgresCurrentSchemaVersion, version) + // The manager works against the migrated schema + require.Nil(t, a.AddUser("phil", "mypass", RoleUser, false)) + u, err := a.User("phil") + require.Nil(t, err) + require.Nil(t, a.AddEmail(u.ID, "phil@example.com")) + // The migrated database must be structurally identical to a freshly created one + freshDB := dbtest.CreateTestPostgres(t) + _, err = NewPostgresManager(freshDB, &Config{DefaultAccess: PermissionDenyAll, BcryptCost: bcrypt.MinCost, QueueWriterInterval: DefaultUserStatsQueueWriterInterval}) + require.Nil(t, err) + require.Equal(t, dbtest.PostgresSchema(t, freshDB), dbtest.PostgresSchema(t, testDB)) +} + +// checkMigratedSqliteSchema verifies that a migrated database is structurally identical to a +// freshly created one (this pins, among other things, that the foreign keys of the tables +// rebuilt in migration 5 -> 6 still point at "user", not at a dropped "user_old"), and that +// its data passes SQLite's foreign key consistency check. +func checkMigratedSqliteSchema(t *testing.T, filename string) { + t.Helper() + freshFile := filepath.Join(t.TempDir(), "fresh.db") + fresh := newTestManagerFromFile(t, freshFile, "", PermissionDenyAll, bcrypt.MinCost, DefaultUserStatsQueueWriterInterval) + defer fresh.Close() + freshDB, err := sql.Open("sqlite3", freshFile) + require.Nil(t, err) + defer freshDB.Close() + migratedDB, err := sql.Open("sqlite3", filename) + require.Nil(t, err) + defer migratedDB.Close() + require.Equal(t, dbtest.SQLiteSchema(t, freshDB), dbtest.SQLiteSchema(t, migratedDB)) + rows, err := migratedDB.Query(`PRAGMA foreign_key_check`) + require.Nil(t, err) + defer rows.Close() + require.False(t, rows.Next(), "foreign_key_check reported violations in the migrated database") } func checkSchemaVersion(t *testing.T, d *db.DB) { diff --git a/webpush/store_test.go b/webpush/store_test.go index 2323de56..65dd817f 100644 --- a/webpush/store_test.go +++ b/webpush/store_test.go @@ -30,11 +30,14 @@ const ( warned_at INT NOT NULL DEFAULT 0 ); CREATE UNIQUE INDEX idx_endpoint ON subscription (endpoint); + CREATE INDEX idx_subscriber_ip ON subscription (subscriber_ip); CREATE TABLE subscription_topic ( subscription_id TEXT NOT NULL, topic TEXT NOT NULL, - PRIMARY KEY (subscription_id, topic) + PRIMARY KEY (subscription_id, topic), + FOREIGN KEY (subscription_id) REFERENCES subscription (id) ON DELETE CASCADE ); + CREATE INDEX idx_topic ON subscription_topic (topic); CREATE TABLE schemaVersion (id INT PRIMARY KEY, version INT NOT NULL); INSERT INTO schemaVersion VALUES (1, 1); ` @@ -49,16 +52,58 @@ const ( updated_at BIGINT NOT NULL, warned_at BIGINT NOT NULL DEFAULT 0 ); + CREATE INDEX idx_webpush_subscriber_ip ON webpush_subscription (subscriber_ip); + CREATE INDEX idx_webpush_updated_at ON webpush_subscription (updated_at); + CREATE INDEX idx_webpush_user_id ON webpush_subscription (user_id); CREATE TABLE webpush_subscription_topic ( subscription_id TEXT NOT NULL REFERENCES webpush_subscription (id) ON DELETE CASCADE, topic TEXT NOT NULL, PRIMARY KEY (subscription_id, topic) ); + CREATE INDEX idx_webpush_topic ON webpush_subscription_topic (topic); CREATE TABLE schema_version (store TEXT PRIMARY KEY, version INT NOT NULL); INSERT INTO schema_version (store, version) VALUES ('webpush', 1); ` ) +// TestStoreSchemaEquivalence verifies that a database adopted from the pre-framework layout is +// structurally identical to a freshly created one: same tables, columns, indexes and keys. +func TestStoreSchemaEquivalence(t *testing.T) { + t.Run("sqlite", func(t *testing.T) { + freshFile := filepath.Join(t.TempDir(), "fresh.db") + fresh, err := webpush.NewSQLiteStore(freshFile, "") + require.Nil(t, err) + defer fresh.Close() + migratedFile := filepath.Join(t.TempDir(), "migrated.db") + d, err := sql.Open("sqlite3", migratedFile) + require.Nil(t, err) + _, err = d.Exec(testPreFrameworkSQLiteSchema) + require.Nil(t, err) + require.Nil(t, d.Close()) + migrated, err := webpush.NewSQLiteStore(migratedFile, "") + require.Nil(t, err) + defer migrated.Close() + freshDB, err := sql.Open("sqlite3", freshFile) + require.Nil(t, err) + defer freshDB.Close() + migratedDB, err := sql.Open("sqlite3", migratedFile) + require.Nil(t, err) + defer migratedDB.Close() + require.Equal(t, dbtest.SQLiteSchema(t, freshDB), dbtest.SQLiteSchema(t, migratedDB)) + }) + t.Run("postgres", func(t *testing.T) { + freshDB := dbtest.CreateTestPostgres(t) + _, err := webpush.NewPostgresStore(freshDB) + require.Nil(t, err) + migratedDB := dbtest.CreateTestPostgres(t) + _, err = migratedDB.Exec(testPreFrameworkPostgresSchema) + require.Nil(t, err) + _, err = webpush.NewPostgresStore(migratedDB) + require.Nil(t, err) + require.Equal(t, dbtest.PostgresSchema(t, freshDB), dbtest.PostgresSchema(t, migratedDB)) + }) +} + func TestStoreSQLiteOpensExistingDatabase(t *testing.T) { filename := filepath.Join(t.TempDir(), "webpush.db") d, err := sql.Open("sqlite3", filename)