Skip to content

Commit ea27471

Browse files
authored
Merge pull request #123 from jagerman/room-return-info-updates
Room return info updates
2 parents fbd6188 + a203915 commit ea27471

File tree

14 files changed

+270
-76
lines changed

14 files changed

+270
-76
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/model/room.py

Lines changed: 22 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1377,9 +1377,8 @@ def set_moderator(self, user: User, *, added_by: User, admin=False, visible=True
13771377
admin=admin,
13781378
visible=visible,
13791379
)
1380-
if u.id in self._perm_cache:
1381-
del self._perm_cache[u.id]
13821380

1381+
self._refresh()
13831382
if u.id in self._perm_cache:
13841383
del self._perm_cache[u.id]
13851384

@@ -1398,19 +1397,21 @@ def remove_moderator(self, user: User, *, removed_by: User, remove_admin_only: b
13981397
if not self.check_admin(removed_by):
13991398
raise BadPermission()
14001399

1401-
query(
1402-
f"""
1403-
UPDATE user_permission_overrides
1404-
SET admin = FALSE
1405-
{', moderator = FALSE, visible_mod = TRUE' if not remove_admin_only else ''}
1406-
WHERE room = :r AND "user" = :u
1407-
""",
1408-
r=self.id,
1409-
u=user.id,
1410-
)
1400+
with db.transaction():
1401+
query(
1402+
f"""
1403+
UPDATE user_permission_overrides
1404+
SET admin = FALSE
1405+
{', moderator = FALSE, visible_mod = TRUE' if not remove_admin_only else ''}
1406+
WHERE room = :r AND "user" = :u
1407+
""",
1408+
r=self.id,
1409+
u=user.id,
1410+
)
14111411

1412-
if user.id in self._perm_cache:
1413-
del self._perm_cache[user.id]
1412+
self._refresh()
1413+
if user.id in self._perm_cache:
1414+
del self._perm_cache[user.id]
14141415

14151416
app.logger.info(f"{removed_by} removed {user} as mod/admin of {self}")
14161417

@@ -1829,7 +1830,7 @@ def pin(self, msg_id: int, admin: User):
18291830
a=admin.id,
18301831
now=time.time(),
18311832
)
1832-
self._pinned = None
1833+
self._refresh()
18331834

18341835
def unpin_all(self, admin: User):
18351836
"""
@@ -1858,8 +1859,9 @@ def unpin_all(self, admin: User):
18581859
if unpinned_files:
18591860
File.reset_expiries(unpinned_files)
18601861

1861-
if count != 0:
1862-
self._pinned = None
1862+
if count != 0:
1863+
self._refresh()
1864+
18631865
return count
18641866

18651867
def unpin(self, msg_id: int, admin: User):
@@ -1887,8 +1889,9 @@ def unpin(self, msg_id: int, admin: User):
18871889
if unpinned_files:
18881890
File.reset_expiries(unpinned_files)
18891891

1890-
if count != 0:
1891-
self._pinned = None
1892+
if count != 0:
1893+
self._refresh()
1894+
18921895
return count
18931896

18941897
@property

sogs/routes/messages.py

Lines changed: 21 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -473,7 +473,10 @@ def message_pin(room, msg_id):
473473
474474
# Return value
475475
476-
On success returns a 200 status code and returns an empty JSON object as response.
476+
On success returns a 200 status code and returns a JSON object as response containing keys:
477+
478+
- `info_updates` -- the new info_updates value of the room; a client can use this to avoid
479+
race conditions with room info polling that might not yet include the updated value(s).
477480
478481
# Error status codes
479482
@@ -483,7 +486,7 @@ def message_pin(room, msg_id):
483486
pinning (e.g. a whisper or deleted post).
484487
"""
485488
room.pin(msg_id, g.user)
486-
return jsonify({})
489+
return jsonify({"info_updates": room.info_updates})
487490

488491

489492
@messages.post("/room/<Room:room>/unpin/<int:msg_id>")
@@ -504,14 +507,20 @@ def message_unpin(room, msg_id):
504507
505508
# Return value
506509
507-
On success returns a 200 status code and returns an empty JSON object as response body.
510+
On success returns a 200 status code and returns an JSON object as response body containing
511+
keys:
512+
513+
- `unpinned` - boolean value indicating whether the message was pinned and has now been unpinned
514+
(true), or was already unpinned (false).
515+
- `info_updates` - the new info_updates value for the room. This value will only change if the
516+
given message was actually pinned (i.e. it does not increment when `unpinned` is false).
508517
509518
# Error status codes
510519
511520
- 403 Forbidden — returned if the invoking user does not have admin permission in this room.
512521
"""
513-
room.unpin(msg_id, g.user)
514-
return jsonify({})
522+
count = room.unpin(msg_id, g.user)
523+
return jsonify({"unpinned": count > 0, "info_updates": room.info_updates})
515524

516525

517526
@messages.post("/room/<Room:room>/unpin/all")
@@ -527,15 +536,18 @@ def message_unpin_all(room):
527536
528537
# Return value
529538
530-
On success returns a 200 status code with an empty JSON object as response body. All pinned
531-
messages have been removed.
539+
On success returns a 200 status code with an JSON object as response body containing keys:
540+
541+
- `unpinned` - count of how many pinned messages were removed.
542+
- `info_updates` - new `info_updates` property for the room. This value is only incremented by
543+
this operation if at least one message was found and unpinned (i.e. if `unpinned > 0`).
532544
533545
# Error status codes
534546
535547
- 403 Forbidden — returned if the invoking user does not have admin permission in this room.
536548
"""
537-
room.unpin_all(g.user)
538-
return jsonify({})
549+
count = room.unpin_all(g.user)
550+
return jsonify({"unpinned": count, "info_updates": room.info_updates})
539551

540552

541553
@messages.put("/room/<Room:room>/reaction/<int:msg_id>/<path:reaction>")

sogs/routes/rooms.py

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -203,7 +203,10 @@ def update_room(room):
203203
204204
# Return value
205205
206-
On success this endpoint returns a 200 status code and an empty json object (`{}`) as the body.
206+
On success this endpoint returns a 200 status code and a json object containing keys:
207+
208+
- `info_updates` -- the new info_updates value of the room; a client can use this to avoid
209+
race conditions with room info polling that might not yet include the updated value(s).
207210
208211
# Error status codes
209212
@@ -276,7 +279,7 @@ def update_room(room):
276279
app.logger.warning("Room update: must include at least one field to update")
277280
abort(http.BAD_REQUEST)
278281

279-
return jsonify({})
282+
return jsonify({"info_updates": room.info_updates})
280283

281284

282285
def addExtraPermInfo(perms):

0 commit comments

Comments
 (0)