Skip to content

Commit a203915

Browse files
committed
Fix info_updates not updating on hidden global mod changes
The triggers managing info_updates where not updating when a hidden global mod/admin was added, but that is wrong because mods/admins of the room *do* see those entries and need to know (via a changed room info_updates when polling for new room info) when the mod list changes, even from a hidden global mod change. This updates the trigger to apply regardless of the visible_mod value.
1 parent 87864f3 commit a203915

File tree

9 files changed

+123
-34
lines changed

9 files changed

+123
-34
lines changed

contrib/upgrade-tests/dump-db.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,7 @@ def dump_rows(table, extra=None, where=None, order="id", skip=set()):
7878
return table.get_string() + "\n"
7979

8080

81-
print(dump_rows("rooms", skip={'created'}))
81+
print(dump_rows("rooms", skip={'created', 'info_updates'}))
8282

8383
TableNotFoundError = pg.errors.UndefinedTable if pg else sqlite3.OperationalError
8484
try:

contrib/upgrade-tests/v0.1.10-expected.txt

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,10 @@
11
rooms:
2-
+----+------------+-------------------+-------------+-------+--------------+--------------+------------------+------+------------+-------+--------+
3-
| id | token | name | description | image | active_users | info_updates | message_sequence | read | accessible | write | upload |
4-
+----+------------+-------------------+-------------+-------+--------------+--------------+------------------+------+------------+-------+--------+
5-
| 1 | round-room | A spherical room! | NULL | NULL | 0 | 1 | 0 | 1 | 1 | 1 | 1 |
6-
| 2 | sudoku | Sudoku lovers | NULL | 3 | 0 | 2 | 1 | 1 | 1 | 1 | 1 |
7-
+----+------------+-------------------+-------------+-------+--------------+--------------+------------------+------+------------+-------+--------+
2+
+----+------------+-------------------+-------------+-------+--------------+------------------+------+------------+-------+--------+
3+
| id | token | name | description | image | active_users | message_sequence | read | accessible | write | upload |
4+
+----+------------+-------------------+-------------+-------+--------------+------------------+------+------------+-------+--------+
5+
| 1 | round-room | A spherical room! | NULL | NULL | 0 | 0 | 1 | 1 | 1 | 1 |
6+
| 2 | sudoku | Sudoku lovers | NULL | 3 | 0 | 1 | 1 | 1 | 1 | 1 |
7+
+----+------------+-------------------+-------------+-------+--------------+------------------+------+------------+-------+--------+
88

99
room_import_hacks:
1010
+------+-------------------+--------------------+

contrib/upgrade-tests/v0.2.0-expected.txt

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,12 @@
11
rooms:
2-
+----+------------+--------------+----------------------------------------------------------------+-------+--------------+--------------+------------------+------+------------+-------+--------+
3-
| id | token | name | description | image | active_users | info_updates | message_sequence | read | accessible | write | upload |
4-
+----+------------+--------------+----------------------------------------------------------------+-------+--------------+--------------+------------------+------+------------+-------+--------+
5-
| 1 | gibberish | Gibberish 🥄 | Delivering your daily dose of gibberish. | 3 | 0 | 3 | 7 | 1 | 1 | 1 | 1 |
6-
| 2 | sudoku | Sudoku | Delivering your daily dose of gib... sudoku. | 1 | 0 | 3 | 5 | 1 | 1 | 1 | 1 |
7-
| 3 | cool-room | Private room | If you're reading these posts then you're too cool for school. | NULL | 0 | 1 | 4 | 0 | 1 | 0 | 0 |
8-
| 4 | empty-room | Empty | We don't go there anymore. | NULL | 0 | 1 | 0 | 1 | 1 | 1 | 1 |
9-
+----+------------+--------------+----------------------------------------------------------------+-------+--------------+--------------+------------------+------+------------+-------+--------+
2+
+----+------------+--------------+----------------------------------------------------------------+-------+--------------+------------------+------+------------+-------+--------+
3+
| id | token | name | description | image | active_users | message_sequence | read | accessible | write | upload |
4+
+----+------------+--------------+----------------------------------------------------------------+-------+--------------+------------------+------+------------+-------+--------+
5+
| 1 | gibberish | Gibberish 🥄 | Delivering your daily dose of gibberish. | 3 | 0 | 7 | 1 | 1 | 1 | 1 |
6+
| 2 | sudoku | Sudoku | Delivering your daily dose of gib... sudoku. | 1 | 0 | 5 | 1 | 1 | 1 | 1 |
7+
| 3 | cool-room | Private room | If you're reading these posts then you're too cool for school. | NULL | 0 | 4 | 0 | 1 | 0 | 0 |
8+
| 4 | empty-room | Empty | We don't go there anymore. | NULL | 0 | 0 | 1 | 1 | 1 | 1 |
9+
+----+------------+--------------+----------------------------------------------------------------+-------+--------------+------------------+------+------------+-------+--------+
1010

1111
room_import_hacks:
1212
message_metadata:

contrib/upgrade-tests/v0.3.0-pg-expected.txt

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,12 @@
11
rooms:
2-
+----+------------+--------------+----------------------------------------------------------------+-------+--------------+--------------+------------------+------+------------+-------+--------+
3-
| id | token | name | description | image | active_users | info_updates | message_sequence | read | accessible | write | upload |
4-
+----+------------+--------------+----------------------------------------------------------------+-------+--------------+--------------+------------------+------+------------+-------+--------+
5-
| 1 | gibberish | Gibberish 🥄 | Delivering your daily dose of gibberish. | 2 | 0 | 3 | 10 | 1 | 1 | 1 | 1 |
6-
| 2 | sudoku | Sudoku | Delivering your daily dose of gib... sudoku. | 1 | 0 | 2 | 4 | 1 | 1 | 1 | 1 |
7-
| 3 | cool-room | Private room | If you're reading these posts then you're too cool for school. | NULL | 0 | 1 | 4 | 1 | 1 | 1 | 1 |
8-
| 4 | empty-room | Empty | We don't go there anymore. | NULL | 0 | 1 | 0 | 1 | 1 | 1 | 1 |
9-
+----+------------+--------------+----------------------------------------------------------------+-------+--------------+--------------+------------------+------+------------+-------+--------+
2+
+----+------------+--------------+----------------------------------------------------------------+-------+--------------+------------------+------+------------+-------+--------+
3+
| id | token | name | description | image | active_users | message_sequence | read | accessible | write | upload |
4+
+----+------------+--------------+----------------------------------------------------------------+-------+--------------+------------------+------+------------+-------+--------+
5+
| 1 | gibberish | Gibberish 🥄 | Delivering your daily dose of gibberish. | 2 | 0 | 10 | 1 | 1 | 1 | 1 |
6+
| 2 | sudoku | Sudoku | Delivering your daily dose of gib... sudoku. | 1 | 0 | 4 | 1 | 1 | 1 | 1 |
7+
| 3 | cool-room | Private room | If you're reading these posts then you're too cool for school. | NULL | 0 | 4 | 1 | 1 | 1 | 1 |
8+
| 4 | empty-room | Empty | We don't go there anymore. | NULL | 0 | 0 | 1 | 1 | 1 | 1 |
9+
+----+------------+--------------+----------------------------------------------------------------+-------+--------------+------------------+------+------------+-------+--------+
1010

1111
room_import_hacks:
1212
message_metadata:

sogs/migrations/__init__.py

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55

66
from . import (
77
file_message,
8+
fix_info_update_triggers,
89
import_hacks,
910
message_views,
1011
new_columns,
@@ -46,6 +47,7 @@ def migrate(conn, *, check_only=False):
4647
room_moderators,
4748
user_permissions,
4849
file_message,
50+
fix_info_update_triggers,
4951
import_hacks,
5052
):
5153
changes = False
Lines changed: 87 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,87 @@
1+
import logging
2+
from .exc import DatabaseUpgradeRequired
3+
4+
5+
def migrate(conn, *, check_only):
6+
from .. import db
7+
8+
# Room info_updates triggers for global mods didn't fire for invisible global mods/admins, but
9+
# should (so that other mods/admins notice the change).
10+
has_bad_trigger = db.query(
11+
"""
12+
SELECT COUNT(*) FROM sqlite_master
13+
WHERE type = 'trigger' AND name = :trigger
14+
AND LOWER(sql) LIKE :bad
15+
"""
16+
if db.engine.name == "sqlite"
17+
else """
18+
SELECT COUNT(*) FROM information_schema.triggers
19+
WHERE trigger_name = :trigger
20+
AND LOWER(action_condition) LIKE :bad
21+
""",
22+
trigger='room_metadata_global_mods_insert',
23+
bad='% new.visible_mod%',
24+
dbconn=conn,
25+
).first()[0]
26+
27+
if not has_bad_trigger:
28+
return False
29+
30+
logging.warning("DB migration: fixing global hidden mod room triggers")
31+
if check_only:
32+
raise DatabaseUpgradeRequired("global hidden mod room triggers need to be recreated")
33+
34+
if db.engine.name == "sqlite":
35+
conn.execute("DROP TRIGGER IF EXISTS room_metadata_global_mods_insert")
36+
conn.execute("DROP TRIGGER IF EXISTS room_metadata_global_mods_update")
37+
conn.execute("DROP TRIGGER IF EXISTS room_metadata_global_mods_delete")
38+
conn.execute(
39+
"""
40+
CREATE TRIGGER room_metadata_global_mods_insert AFTER INSERT ON users
41+
FOR EACH ROW WHEN (NEW.admin OR NEW.moderator)
42+
BEGIN
43+
UPDATE rooms SET info_updates = info_updates + 1; -- WHERE everything!
44+
END
45+
"""
46+
)
47+
conn.execute(
48+
"""
49+
CREATE TRIGGER room_metadata_global_mods_update AFTER UPDATE ON users
50+
FOR EACH ROW WHEN (NEW.moderator != OLD.moderator OR NEW.admin != OLD.admin OR NEW.visible_mod != OLD.visible_mod)
51+
BEGIN
52+
UPDATE rooms SET info_updates = info_updates + 1; -- WHERE everything!
53+
END
54+
""" # noqa: E501
55+
)
56+
conn.execute(
57+
"""
58+
CREATE TRIGGER room_metadata_global_mods_delete AFTER DELETE ON users
59+
FOR EACH ROW WHEN (OLD.moderator OR OLD.admin)
60+
BEGIN
61+
UPDATE rooms SET info_updates = info_updates + 1; -- WHERE everything!
62+
END
63+
"""
64+
)
65+
66+
else: # postgresql
67+
conn.execute(
68+
"""
69+
DROP TRIGGER IF EXISTS room_metadata_global_mods_insert ON users;
70+
DROP TRIGGER IF EXISTS room_metadata_global_mods_update ON users;
71+
DROP TRIGGER IF EXISTS room_metadata_global_mods_delete ON users;
72+
73+
CREATE TRIGGER room_metadata_global_mods_insert AFTER INSERT ON users
74+
FOR EACH ROW WHEN (NEW.admin OR NEW.moderator)
75+
EXECUTE PROCEDURE trigger_room_metadata_info_update_all();
76+
77+
CREATE TRIGGER room_metadata_global_mods_update AFTER UPDATE OF moderator, admin, visible_mod ON users
78+
FOR EACH ROW WHEN (NEW.moderator != OLD.moderator OR NEW.admin != OLD.admin OR NEW.visible_mod != OLD.visible_mod)
79+
EXECUTE PROCEDURE trigger_room_metadata_info_update_all();
80+
81+
CREATE TRIGGER room_metadata_global_mods_delete AFTER DELETE ON users
82+
FOR EACH ROW WHEN (OLD.moderator OR OLD.admin)
83+
EXECUTE PROCEDURE trigger_room_metadata_info_update_all();
84+
""" # noqa: E501
85+
)
86+
87+
return True

sogs/schema.pgsql

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -444,15 +444,15 @@ EXECUTE PROCEDURE trigger_room_metadata_info_update_old();
444444
-- because global mod settings affect the permissions of all rooms (and polling clients need to pick
445445
-- up on this).
446446
CREATE TRIGGER room_metadata_global_mods_insert AFTER INSERT ON users
447-
FOR EACH ROW WHEN ((NEW.admin OR NEW.moderator) AND NEW.visible_mod)
447+
FOR EACH ROW WHEN (NEW.admin OR NEW.moderator)
448448
EXECUTE PROCEDURE trigger_room_metadata_info_update_all();
449449

450450
CREATE TRIGGER room_metadata_global_mods_update AFTER UPDATE OF moderator, admin, visible_mod ON users
451-
FOR EACH ROW WHEN ((NEW.moderator != OLD.moderator OR NEW.admin != OLD.admin) AND (NEW.visible_mod OR OLD.visible_mod))
451+
FOR EACH ROW WHEN (NEW.moderator != OLD.moderator OR NEW.admin != OLD.admin OR NEW.visible_mod != OLD.visible_mod)
452452
EXECUTE PROCEDURE trigger_room_metadata_info_update_all();
453453

454454
CREATE TRIGGER room_metadata_global_mods_delete AFTER DELETE ON users
455-
FOR EACH ROW WHEN ((OLD.moderator OR OLD.admin) AND OLD.visible_mod)
455+
FOR EACH ROW WHEN (OLD.moderator OR OLD.admin)
456456
EXECUTE PROCEDURE trigger_room_metadata_info_update_all();
457457

458458

sogs/schema.sqlite

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -372,17 +372,17 @@ END;
372372
-- because global mod settings affect the permissions of all rooms (and polling clients need to pick
373373
-- up on this).
374374
CREATE TRIGGER room_metadata_global_mods_insert AFTER INSERT ON users
375-
FOR EACH ROW WHEN (NEW.admin OR NEW.moderator) AND NEW.visible_mod
375+
FOR EACH ROW WHEN (NEW.admin OR NEW.moderator)
376376
BEGIN
377377
UPDATE rooms SET info_updates = info_updates + 1; -- WHERE everything!
378378
END;
379379
CREATE TRIGGER room_metadata_global_mods_update AFTER UPDATE ON users
380-
FOR EACH ROW WHEN (NEW.moderator != OLD.moderator OR NEW.admin != OLD.admin) AND (NEW.visible_mod OR OLD.visible_mod)
380+
FOR EACH ROW WHEN (NEW.moderator != OLD.moderator OR NEW.admin != OLD.admin OR NEW.visible_mod != OLD.visible_mod)
381381
BEGIN
382382
UPDATE rooms SET info_updates = info_updates + 1; -- WHERE everything!
383383
END;
384384
CREATE TRIGGER room_metadata_global_mods_delete AFTER DELETE ON users
385-
FOR EACH ROW WHEN (OLD.moderator OR OLD.admin) AND OLD.visible_mod
385+
FOR EACH ROW WHEN (OLD.moderator OR OLD.admin)
386386
BEGIN
387387
UPDATE rooms SET info_updates = info_updates + 1; -- WHERE everything!
388388
END;

tests/test_room_routes.py

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ def test_list(client, room, room2, user, user2, admin, mod, global_mod, global_a
3131
"token": "room2",
3232
"name": "Room 2",
3333
"description": "Test suite testing room2",
34-
"info_updates": 0,
34+
"info_updates": 2,
3535
"message_sequence": 0,
3636
"created": room2.created,
3737
"active_users": 0,
@@ -52,7 +52,7 @@ def test_list(client, room, room2, user, user2, admin, mod, global_mod, global_a
5252
"token": "test-room",
5353
"name": "Test room",
5454
"description": "Test suite testing room",
55-
"info_updates": 2,
55+
"info_updates": 4,
5656
"message_sequence": 0,
5757
"created": room.created,
5858
"active_users": 0,
@@ -193,7 +193,7 @@ def test_updates(client, room, user, user2, mod, admin, global_mod, global_admin
193193
"token": "test-room",
194194
"name": "Test room",
195195
"description": "Test suite testing room",
196-
"info_updates": 2,
196+
"info_updates": 4,
197197
"message_sequence": 0,
198198
"created": room.created,
199199
"active_users": 0,
@@ -372,14 +372,14 @@ def test_polling(client, room, user, user2, mod, admin, global_mod, global_admin
372372
r = sogs_get(client, "/room/test-room", user)
373373
assert r.status_code == 200
374374
info_up = r.json['info_updates']
375-
assert info_up == 2
375+
assert info_up == 4
376376

377377
basic = {'token': 'test-room', 'active_users': 0, 'read': True, 'write': True, 'upload': True}
378378
details = {
379379
"token": "test-room",
380380
"name": "Test room",
381381
"description": "Test suite testing room",
382-
"info_updates": 2,
382+
"info_updates": 4,
383383
"message_sequence": 0,
384384
"created": room.created,
385385
"active_users": 0,

0 commit comments

Comments
 (0)