From 1f0f280286cc9072b23120fe228832c781b927f9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mike=20Schw=C3=B6rer?= Date: Fri, 18 Apr 2025 16:02:14 +0200 Subject: [PATCH] Fix ListChannels(owned) returning channels multiple (if there are deleted subscriptions) [skip-tests] --- scnserver/.idea/sqldialects.xml | 2 + scnserver/db/impl/primary/channels.go | 8 +- scnserver/db/impl/primary/senderNames.go | 2 +- scnserver/db/schema/assets.go | 21 +- scnserver/db/schema/primary_10.ddl | 251 ++++++++++++++++++ .../db/schema/primary_migration_9_10.sql | 10 + scnserver/test/database_test.go | 96 +++++++ 7 files changed, 375 insertions(+), 15 deletions(-) create mode 100644 scnserver/db/schema/primary_10.ddl create mode 100644 scnserver/db/schema/primary_migration_9_10.sql diff --git a/scnserver/.idea/sqldialects.xml b/scnserver/.idea/sqldialects.xml index f8df685..f55707c 100644 --- a/scnserver/.idea/sqldialects.xml +++ b/scnserver/.idea/sqldialects.xml @@ -1,7 +1,9 @@ + + diff --git a/scnserver/db/impl/primary/channels.go b/scnserver/db/impl/primary/channels.go index 21f5f32..0918387 100644 --- a/scnserver/db/impl/primary/channels.go +++ b/scnserver/db/impl/primary/channels.go @@ -68,7 +68,7 @@ func (db *Database) ListChannelsByOwner(ctx db.TxContext, userid models.UserID, order := " ORDER BY channels.timestamp_created ASC, channels.channel_id ASC " - sql := "SELECT channels.*, sub.* FROM channels LEFT JOIN subscriptions AS sub ON channels.channel_id = sub.channel_id AND sub.subscriber_user_id = :subuid WHERE channels.deleted=0 AND owner_user_id = :ouid" + order + sql := "SELECT channels.*, sub.* FROM channels LEFT JOIN subscriptions AS sub ON (channels.channel_id = sub.channel_id AND sub.subscriber_user_id = :subuid AND sub.deleted=0) WHERE channels.deleted=0 AND owner_user_id = :ouid" + order pp := sq.PP{ "ouid": userid, @@ -95,7 +95,7 @@ func (db *Database) ListChannelsBySubscriber(ctx db.TxContext, userid models.Use order := " ORDER BY channels.timestamp_created ASC, channels.channel_id ASC " - sql := "SELECT channels.*, sub.* " + "FROM channels LEFT JOIN subscriptions AS sub on channels.channel_id = sub.channel_id AND sub.subscriber_user_id = :subuid AND sub.deleted=0 WHERE " + cond + order + sql := "SELECT channels.*, sub.* " + "FROM channels LEFT JOIN subscriptions AS sub ON (channels.channel_id = sub.channel_id AND sub.subscriber_user_id = :subuid AND sub.deleted=0) WHERE " + cond + order pp := sq.PP{ "subuid": userid, @@ -121,7 +121,7 @@ func (db *Database) ListChannelsByAccess(ctx db.TxContext, userid models.UserID, order := " ORDER BY channels.timestamp_created ASC, channels.channel_id ASC " - sql := "SELECT channels.*, sub.* " + "FROM channels LEFT JOIN subscriptions AS sub on channels.channel_id = sub.channel_id AND sub.subscriber_user_id = :subuid AND sub.deleted=0 WHERE " + cond + order + sql := "SELECT channels.*, sub.* " + "FROM channels LEFT JOIN subscriptions AS sub ON (channels.channel_id = sub.channel_id AND sub.subscriber_user_id = :subuid AND sub.deleted=0) WHERE " + cond + order pp := sq.PP{ "ouid": userid, @@ -144,7 +144,7 @@ func (db *Database) GetChannel(ctx db.TxContext, userid models.UserID, channelid selectors := "channels.*, sub.*" - join := "LEFT JOIN subscriptions AS sub on channels.channel_id = sub.channel_id AND sub.subscriber_user_id = :subuid AND sub.deleted=0" + join := "LEFT JOIN subscriptions AS sub ON (channels.channel_id = sub.channel_id AND sub.subscriber_user_id = :subuid AND sub.deleted=0)" cond := "channels.channel_id = :cid" if enforceOwner { diff --git a/scnserver/db/impl/primary/senderNames.go b/scnserver/db/impl/primary/senderNames.go index de69c7d..ab2afe1 100644 --- a/scnserver/db/impl/primary/senderNames.go +++ b/scnserver/db/impl/primary/senderNames.go @@ -17,7 +17,7 @@ func (db *Database) ListSenderNames(ctx db.TxContext, userid models.UserID, incl prepParams := sq.PP{"uid": userid} if includeForeignSubscribed { - sqlStr = "SELECT sender_name AS name, MAX(timestamp_real) AS ts_last, MIN(timestamp_real) AS ts_first, COUNT(*) AS count FROM messages LEFT JOIN subscriptions AS subs ON messages.channel_id = subs.channel_id AND subs.deleted=0 WHERE (subs.subscriber_user_id = :uid AND subs.confirmed = 1) AND sender_NAME NOT NULL GROUP BY sender_name ORDER BY ts_last DESC" + sqlStr = "SELECT sender_name AS name, MAX(timestamp_real) AS ts_last, MIN(timestamp_real) AS ts_first, COUNT(*) AS count FROM messages LEFT JOIN subscriptions AS subs ON (messages.channel_id = subs.channel_id AND subs.deleted=0) WHERE (subs.subscriber_user_id = :uid AND subs.confirmed = 1) AND sender_NAME NOT NULL GROUP BY sender_name ORDER BY ts_last DESC" } else { sqlStr = "SELECT sender_name AS name, MAX(timestamp_real) AS ts_last, MIN(timestamp_real) AS ts_first, COUNT(*) AS count FROM messages WHERE sender_user_id = :uid AND sender_NAME NOT NULL GROUP BY sender_name ORDER BY ts_last DESC" } diff --git a/scnserver/db/schema/assets.go b/scnserver/db/schema/assets.go index ad2cc3f..3a6d08e 100644 --- a/scnserver/db/schema/assets.go +++ b/scnserver/db/schema/assets.go @@ -17,16 +17,17 @@ type Def struct { var assets embed.FS var PrimarySchema = map[int]Def{ - 0: {"", "", nil}, - 1: {readDDL("primary_1.ddl"), "f2b2847f32681a7178e405553beea4a324034915a0c5a5dc70b3c6abbcc852f2", nil}, - 2: {readDDL("primary_2.ddl"), "07ed1449114416ed043084a30e0722a5f97bf172161338d2f7106a8dfd387d0a", nil}, - 3: {readDDL("primary_3.ddl"), "65c2125ad0e12d02490cf2275f0067ef3c62a8522edf9a35ee8aa3f3c09b12e8", nil}, - 4: {readDDL("primary_4.ddl"), "cb022156ab0e7aea39dd0c985428c43cae7d60e41ca8e9e5a84c774b3019d2ca", readMig("primary_migration_3_4.sql")}, - 5: {readDDL("primary_5.ddl"), "9d6217ba4a3503cfe090f72569367f95a413bb14e9effe49ffeabbf255bce8dd", readMig("primary_migration_4_5.sql")}, - 6: {readDDL("primary_6.ddl"), "8e83d20bcd008082713f248ae8cd558335a37a37ce90bd8c86e782da640ee160", readMig("primary_migration_5_6.sql")}, - 7: {readDDL("primary_7.ddl"), "90d8dbc460afe025f9b74cda5c16bb8e58b178df275223bd2531907a8d8c36c3", readMig("primary_migration_6_7.sql")}, - 8: {readDDL("primary_8.ddl"), "746f6005c7a573b8816e5993ecd1d949fe2552b0134ba63bab8b4d5b2b5058ad", readMig("primary_migration_7_8.sql")}, - 9: {readDDL("primary_9.ddl"), "0fd1eacb03364153b2a2096106b1a11cc48ae764db52c2896dab9725b55ed188", readMig("primary_migration_8_9.sql")}, + 0: {"", "", nil}, + 1: {readDDL("primary_1.ddl"), "f2b2847f32681a7178e405553beea4a324034915a0c5a5dc70b3c6abbcc852f2", nil}, + 2: {readDDL("primary_2.ddl"), "07ed1449114416ed043084a30e0722a5f97bf172161338d2f7106a8dfd387d0a", nil}, + 3: {readDDL("primary_3.ddl"), "65c2125ad0e12d02490cf2275f0067ef3c62a8522edf9a35ee8aa3f3c09b12e8", nil}, + 4: {readDDL("primary_4.ddl"), "cb022156ab0e7aea39dd0c985428c43cae7d60e41ca8e9e5a84c774b3019d2ca", readMig("primary_migration_3_4.sql")}, + 5: {readDDL("primary_5.ddl"), "9d6217ba4a3503cfe090f72569367f95a413bb14e9effe49ffeabbf255bce8dd", readMig("primary_migration_4_5.sql")}, + 6: {readDDL("primary_6.ddl"), "8e83d20bcd008082713f248ae8cd558335a37a37ce90bd8c86e782da640ee160", readMig("primary_migration_5_6.sql")}, + 7: {readDDL("primary_7.ddl"), "90d8dbc460afe025f9b74cda5c16bb8e58b178df275223bd2531907a8d8c36c3", readMig("primary_migration_6_7.sql")}, + 8: {readDDL("primary_8.ddl"), "746f6005c7a573b8816e5993ecd1d949fe2552b0134ba63bab8b4d5b2b5058ad", readMig("primary_migration_7_8.sql")}, + 9: {readDDL("primary_9.ddl"), "0fd1eacb03364153b2a2096106b1a11cc48ae764db52c2896dab9725b55ed188", readMig("primary_migration_8_9.sql")}, + 10: {readDDL("primary_10.ddl"), "3427b9cc61249e0148e58d5534f2f4e61d3911a47aa1ec6fec434054151d5065", readMig("primary_migration_9_10.sql")}, } var PrimarySchemaVersion = len(PrimarySchema) - 1 diff --git a/scnserver/db/schema/primary_10.ddl b/scnserver/db/schema/primary_10.ddl new file mode 100644 index 0000000..3f7754b --- /dev/null +++ b/scnserver/db/schema/primary_10.ddl @@ -0,0 +1,251 @@ +CREATE TABLE users +( + user_id TEXT NOT NULL, + + username TEXT NULL DEFAULT NULL, + + timestamp_created INTEGER NOT NULL, + timestamp_lastread INTEGER NULL DEFAULT NULL, + timestamp_lastsent INTEGER NULL DEFAULT NULL, + + messages_sent INTEGER NOT NULL DEFAULT '0', + + quota_used INTEGER NOT NULL DEFAULT '0', + quota_used_day TEXT NULL DEFAULT NULL, + + is_pro INTEGER CHECK(is_pro IN (0, 1)) NOT NULL DEFAULT 0, + pro_token TEXT NULL DEFAULT NULL, + + deleted INTEGER CHECK(deleted IN (0, 1)) NOT NULL DEFAULT '0', + + PRIMARY KEY (user_id) +) STRICT; +CREATE UNIQUE INDEX "idx_users_protoken" ON users (pro_token) WHERE pro_token IS NOT NULL AND deleted=0; + + +CREATE TABLE keytokens +( + keytoken_id TEXT NOT NULL, + + timestamp_created INTEGER NOT NULL, + timestamp_lastused INTEGER NULL DEFAULT NULL, + + name TEXT NOT NULL, + + owner_user_id TEXT NOT NULL, + + all_channels INTEGER CHECK(all_channels IN (0, 1)) NOT NULL, + channels TEXT NOT NULL, + token TEXT NOT NULL, + permissions TEXT NOT NULL, + + messages_sent INTEGER NOT NULL DEFAULT '0', + + deleted INTEGER CHECK(deleted IN (0, 1)) NOT NULL DEFAULT '0', + + PRIMARY KEY (keytoken_id) +) STRICT; +CREATE UNIQUE INDEX "idx_keytokens_token" ON keytokens (token); + + +CREATE TABLE clients +( + client_id TEXT NOT NULL, + + user_id TEXT NOT NULL, + type TEXT CHECK(type IN ('ANDROID','IOS','LINUX','MACOS','WINDOWS')) NOT NULL, + fcm_token TEXT NOT NULL, + name TEXT NULL, + + timestamp_created INTEGER NOT NULL, + + agent_model TEXT NOT NULL, + agent_version TEXT NOT NULL, + + deleted INTEGER CHECK(deleted IN (0, 1)) NOT NULL DEFAULT '0', + + PRIMARY KEY (client_id) +) STRICT; +CREATE INDEX "idx_clients_userid" ON clients (user_id); +CREATE INDEX "idx_clients_deleted" ON clients (deleted); +CREATE UNIQUE INDEX "idx_clients_fcmtoken" ON clients (fcm_token) WHERE deleted=0; + + +CREATE TABLE channels +( + channel_id TEXT NOT NULL, + + owner_user_id TEXT NOT NULL, + + internal_name TEXT NOT NULL, + display_name TEXT NOT NULL, + description_name TEXT NULL, + + subscribe_key TEXT NOT NULL, + + timestamp_created INTEGER NOT NULL, + timestamp_lastsent INTEGER NULL DEFAULT NULL, + + messages_sent INTEGER NOT NULL DEFAULT '0', + + deleted INTEGER CHECK(deleted IN (0, 1)) NOT NULL DEFAULT '0', + + PRIMARY KEY (channel_id) +) STRICT; +CREATE UNIQUE INDEX "idx_channels_identity" ON channels (owner_user_id, internal_name) WHERE deleted=0; + +CREATE TABLE subscriptions +( + subscription_id TEXT NOT NULL, + + subscriber_user_id TEXT NOT NULL, + channel_owner_user_id TEXT NOT NULL, + channel_internal_name TEXT NOT NULL, + channel_id TEXT NOT NULL, + + timestamp_created INTEGER NOT NULL, + + confirmed INTEGER CHECK(confirmed IN (0, 1)) NOT NULL, + active INTEGER CHECK(active IN (0, 1)) NOT NULL, + + deleted INTEGER CHECK(deleted IN (0, 1)) NOT NULL DEFAULT '0', + + PRIMARY KEY (subscription_id) +) STRICT; +CREATE UNIQUE INDEX "idx_subscriptions_ref" ON subscriptions (subscriber_user_id, channel_owner_user_id, channel_internal_name) WHERE deleted=0; +CREATE UNIQUE INDEX "idx_subscriptions_ref2" ON subscriptions (subscriber_user_id, channel_owner_user_id, channel_id) WHERE deleted=0; +CREATE UNIQUE INDEX "idx_subscriptions_ref3" ON subscriptions (subscriber_user_id, channel_id) WHERE deleted=0; +CREATE INDEX "idx_subscriptions_chan" ON subscriptions (channel_id); +CREATE INDEX "idx_subscriptions_subuser" ON subscriptions (subscriber_user_id); +CREATE INDEX "idx_subscriptions_ownuser" ON subscriptions (channel_owner_user_id); +CREATE INDEX "idx_subscriptions_tsc" ON subscriptions (timestamp_created); +CREATE INDEX "idx_subscriptions_conf" ON subscriptions (confirmed); + + +CREATE TABLE messages +( + message_id TEXT NOT NULL, + sender_user_id TEXT NOT NULL, + channel_internal_name TEXT NOT NULL, + channel_id TEXT NOT NULL, + channel_owner_user_id TEXT NOT NULL, + sender_ip TEXT NOT NULL, + sender_name TEXT NULL, + + timestamp_real INTEGER NOT NULL, + timestamp_client INTEGER NULL, + + title TEXT NOT NULL, + content TEXT NULL, + priority INTEGER CHECK(priority IN (0, 1, 2)) NOT NULL, + usr_message_id TEXT NULL, + + used_key_id TEXT NOT NULL, + + deleted INTEGER CHECK(deleted IN (0, 1)) NOT NULL DEFAULT '0', + + PRIMARY KEY (message_id) +) STRICT; +CREATE INDEX "idx_messages_channel" ON messages (channel_internal_name COLLATE BINARY); +CREATE INDEX "idx_messages_channel_nc" ON messages (channel_internal_name COLLATE NOCASE); +CREATE UNIQUE INDEX "idx_messages_idempotency" ON messages (sender_user_id, usr_message_id COLLATE BINARY); +CREATE INDEX "idx_messages_senderip" ON messages (sender_ip COLLATE BINARY); +CREATE INDEX "idx_messages_sendername" ON messages (sender_name COLLATE BINARY); +CREATE INDEX "idx_messages_sendername_nc" ON messages (sender_name COLLATE NOCASE); +CREATE INDEX "idx_messages_title" ON messages (title COLLATE BINARY); +CREATE INDEX "idx_messages_title_nc" ON messages (title COLLATE NOCASE); +CREATE INDEX "idx_messages_usedkey" ON messages (sender_user_id, used_key_id); +CREATE INDEX "idx_messages_deleted" ON messages (deleted); + + +CREATE VIRTUAL TABLE messages_fts USING fts5 +( + channel_internal_name, + sender_name, + title, + content, + + tokenize = unicode61, + content = 'messages', + content_rowid = 'rowid' +); + +CREATE TRIGGER fts_insert AFTER INSERT ON messages BEGIN + INSERT INTO messages_fts (rowid, channel_internal_name, sender_name, title, content) VALUES (new.rowid, new.channel_internal_name, new.sender_name, new.title, new.content); +END; + +CREATE TRIGGER fts_update AFTER UPDATE ON messages BEGIN + INSERT INTO messages_fts (messages_fts, rowid, channel_internal_name, sender_name, title, content) VALUES ('delete', old.rowid, old.channel_internal_name, old.sender_name, old.title, old.content); + INSERT INTO messages_fts ( rowid, channel_internal_name, sender_name, title, content) VALUES ( new.rowid, new.channel_internal_name, new.sender_name, new.title, new.content); +END; + +CREATE TRIGGER fts_delete AFTER DELETE ON messages BEGIN + INSERT INTO messages_fts (messages_fts, rowid, channel_internal_name, sender_name, title, content) VALUES ('delete', old.rowid, old.channel_internal_name, old.sender_name, old.title, old.content); +END; + + +CREATE TABLE deliveries +( + delivery_id TEXT NOT NULL, + + message_id TEXT NOT NULL, + receiver_user_id TEXT NOT NULL, + receiver_client_id TEXT NOT NULL, + + timestamp_created INTEGER NOT NULL, + timestamp_finalized INTEGER NULL, + + + status TEXT CHECK(status IN ('RETRY','SUCCESS','FAILED')) NOT NULL, + retry_count INTEGER NOT NULL DEFAULT 0, + next_delivery INTEGER NULL DEFAULT NULL, + + fcm_message_id TEXT NULL, + + deleted INTEGER CHECK(deleted IN (0, 1)) NOT NULL DEFAULT '0', + + PRIMARY KEY (delivery_id) +) STRICT; +CREATE INDEX "idx_deliveries_receiver" ON deliveries (message_id, receiver_client_id); + + +CREATE TABLE compat_ids +( + old INTEGER NOT NULL, + new TEXT NOT NULL, + type TEXT NOT NULL +) STRICT; +CREATE UNIQUE INDEX "idx_compatids_new" ON compat_ids (new); +CREATE UNIQUE INDEX "idx_compatids_old" ON compat_ids (old, type); + + +CREATE TABLE compat_acks +( + user_id TEXT NOT NULL, + message_id TEXT NOT NULL +) STRICT; +CREATE INDEX "idx_compatacks_userid" ON compat_acks (user_id); +CREATE UNIQUE INDEX "idx_compatacks_messageid" ON compat_acks (message_id); +CREATE UNIQUE INDEX "idx_compatacks_userid_messageid" ON compat_acks (user_id, message_id); + + +CREATE TABLE compat_clients +( + client_id TEXT NOT NULL +) STRICT; +CREATE UNIQUE INDEX "idx_compatclient_clientid" ON compat_clients (client_id); + + +CREATE TABLE `meta` +( + meta_key TEXT NOT NULL, + value_int INTEGER NULL, + value_txt TEXT NULL, + value_real REAL NULL, + value_blob BLOB NULL, + + PRIMARY KEY (meta_key) +) STRICT; + + +INSERT INTO meta (meta_key, value_int) VALUES ('schema', 3) \ No newline at end of file diff --git a/scnserver/db/schema/primary_migration_9_10.sql b/scnserver/db/schema/primary_migration_9_10.sql new file mode 100644 index 0000000..9ab8ab1 --- /dev/null +++ b/scnserver/db/schema/primary_migration_9_10.sql @@ -0,0 +1,10 @@ + +-- (only add indizes) +-- + +------------------------------------------------------------------------------------------------------------------------ + +CREATE UNIQUE INDEX "idx_subscriptions_ref2" ON subscriptions (subscriber_user_id, channel_owner_user_id, channel_id) WHERE deleted=0; +CREATE UNIQUE INDEX "idx_subscriptions_ref3" ON subscriptions (subscriber_user_id, channel_id) WHERE deleted=0; + +------------------------------------------------------------------------------------------------------------------------ diff --git a/scnserver/test/database_test.go b/scnserver/test/database_test.go index 127f3f2..4a216f3 100644 --- a/scnserver/test/database_test.go +++ b/scnserver/test/database_test.go @@ -868,3 +868,99 @@ func TestPrimaryDB_Migrate_from_8_to_latest(t *testing.T) { tt.TestFailIfErr(t, err) } } + +func TestPrimaryDB_Migrate_from_9_to_latest(t *testing.T) { + dbf1, dbf2, dbf3, conf, stop := tt.StartSimpleTestspace(t) + defer stop() + + ctx := context.Background() + + tt.AssertAny(dbf1) + tt.AssertAny(dbf2) + tt.AssertAny(dbf3) + tt.AssertAny(conf) + + schemavers := 9 + + { + url := fmt.Sprintf("file:%s", dbf1) + + xdb, err := sqlx.Open("sqlite3", url) + tt.TestFailIfErr(t, err) + + qqdb := sq.NewDB(xdb, sq.DBOptions{}) + + dbschema := schema.PrimarySchema[schemavers] + + _, err = qqdb.Exec(ctx, dbschema.SQL, sq.PP{}) + tt.TestFailIfErr(t, err) + + _, err = qqdb.Exec(ctx, "INSERT INTO meta (meta_key, value_int) VALUES (:key, :val) ON CONFLICT(meta_key) DO UPDATE SET value_int = :val", sq.PP{ + "key": "schema", + "val": schemavers, + }) + + _, err = qqdb.Exec(ctx, "INSERT INTO meta (meta_key, value_txt) VALUES (:key, :val) ON CONFLICT(meta_key) DO UPDATE SET value_txt = :val", sq.PP{ + "key": "schema_hash", + "val": dbschema.Hash, + }) + + { + tctx := simplectx.CreateSimpleContext(ctx, nil) + schemHashDB, err := sq.HashSqliteDatabase(tctx, qqdb) + tt.TestFailIfErr(t, err) + tt.AssertEqual(t, "schemHashDB", dbschema.Hash, schemHashDB) + err = tctx.CommitTransaction() + tt.TestFailIfErr(t, err) + } + + err = qqdb.Exit() + tt.TestFailIfErr(t, err) + } + + { + db1, err := primary.NewPrimaryDatabase(conf) + tt.TestFailIfErr(t, err) + + { + tctx := simplectx.CreateSimpleContext(ctx, nil) + + schema1, err := db1.ReadSchema(tctx) + tt.TestFailIfErr(t, err) + tt.AssertEqual(t, "schema1", schemavers, schema1) + + err = tctx.CommitTransaction() + tt.TestFailIfErr(t, err) + } + + //================================================ + { + err = db1.Migrate(ctx) + tt.TestFailIfErr(t, err) + } + //================================================ + + { + tctx := simplectx.CreateSimpleContext(ctx, nil) + + schema2, err := db1.ReadSchema(tctx) + tt.TestFailIfErr(t, err) + tt.AssertEqual(t, "schema2", schema.PrimarySchemaVersion, schema2) + + err = tctx.CommitTransaction() + tt.TestFailIfErr(t, err) + } + + { + tctx := simplectx.CreateSimpleContext(ctx, nil) + schemHashDB, err := sq.HashSqliteDatabase(tctx, db1.DB()) + tt.TestFailIfErr(t, err) + tt.AssertEqual(t, "schemHashDB", schema.PrimarySchema[schema.PrimarySchemaVersion].Hash, schemHashDB) + err = tctx.CommitTransaction() + tt.TestFailIfErr(t, err) + } + + err = db1.Stop(ctx) + tt.TestFailIfErr(t, err) + } +}