Schema compare tests

user/manager_postgres_schema.go

@@ -96,7 +96,7 @@ const (
 )
 
 const (
-	postgresCurrentSchemaVersion = 8
+	postgresCurrentSchemaVersion = 9
 )
 
 const (
@@ -135,5 +135,7 @@ var (
 	postgresMigrations = map[int]schema.MigrateFunc{
 		6: schema.AsMigrateFunc(postgresMigrate6To7UpdateQueries),
 		7: schema.AsMigrateFunc(postgresMigrate7To8UpdateQueries),
+		// 8 -> 9 repairs a SQLite-only foreign key defect; nothing to do on Postgres
+		8: schema.NopMigrateFunc,
 	}
 )

user/manager_sqlite_schema.go

@@ -114,7 +114,7 @@ const (
 )
 
 const (
-	sqliteCurrentSchemaVersion = 8
+	sqliteCurrentSchemaVersion = 9
 )
 
 // Schema migrations for SQLite
@@ -347,6 +347,23 @@ 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 SELECT * FROM user_phone_old;
+		DROP TABLE user_phone_old;
+	`
 )
 
 var (
@@ -362,6 +379,7 @@ var (
 		5: schema.AsMigrateFunc(sqliteMigrate5To6UpdateQueries),
 		6: schema.AsMigrateFunc(sqliteMigrate6To7UpdateQueries),
 		7: schema.AsMigrateFunc(sqliteMigrate7To8UpdateQueries),
+		8: schema.AsMigrateFunc(sqliteMigrate8To9UpdateQueries),
 	}
 )
 

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) {
@@ -1796,6 +1875,56 @@ 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
+
+	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) {