Skip to content

Commit e24cb00

Browse files
committed
Clean up user_permissions view
- drop the "moderator OR admin" clauses from the view: we have triggers that already enforce admin-implies-moderator so the `OR admin` is pointless extra work. - unify the sqlite and postgresql user_permissions view query: the postgresql syntax (with quoted "user" and specifying CROSS JOIN instead of JOIN) is perfectly acceptable for sqlite (and more correct anyway).
1 parent 2e2e5db commit e24cb00

File tree

4 files changed

+24
-22
lines changed

4 files changed

+24
-22
lines changed

sogs/migrations/room_moderators.py

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,10 @@
44

55
def migrate(conn, *, check_only):
66
"""
7-
Adds the room_moderators view and drops/replaces the user_permission_overrides_public_mods
8-
index.
7+
Adds the room_moderators view, along with a couple other optimizations that came at the same
8+
time:
9+
- we drop the user_permissions view (to be recreated in the user_permissions migration code)
10+
- we drop the user_permission_overrides_public_mods index and recreate a tighter index
911
"""
1012

1113
from .. import db
@@ -90,6 +92,7 @@ def migrate(conn, *, check_only):
9092
"""
9193
)
9294

95+
conn.execute("DROP VIEW IF EXISTS user_permissions")
9396
conn.execute("DROP INDEX IF EXISTS user_permission_overrides_public_mods")
9497
conn.execute(
9598
"CREATE INDEX IF NOT EXISTS user_permission_overrides_mods "

sogs/migrations/user_permissions.py

Lines changed: 8 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -17,13 +17,12 @@ def migrate(conn, *, check_only):
1717
if check_only:
1818
raise DatabaseUpgradeRequired("Recreate user_permissions view")
1919

20-
sqlite = db.engine.name == "sqlite"
2120
conn.execute(
22-
f"""
21+
"""
2322
CREATE VIEW user_permissions AS
2423
SELECT
2524
rooms.id AS room,
26-
users.id AS {'user' if sqlite else '"user"'},
25+
users.id AS "user",
2726
users.session_id,
2827
CASE WHEN users.banned THEN TRUE ELSE COALESCE(user_permission_overrides.banned, FALSE) END AS banned,
2928
CASE WHEN users.moderator THEN TRUE ELSE COALESCE(user_permission_overrides.read, rooms.read) END AS read,
@@ -33,19 +32,19 @@ def migrate(conn, *, check_only):
3332
CASE WHEN users.moderator THEN TRUE ELSE COALESCE(user_permission_overrides.moderator, FALSE) END AS moderator,
3433
CASE WHEN users.admin THEN TRUE ELSE COALESCE(user_permission_overrides.admin, FALSE) END AS admin,
3534
-- room_moderator will be TRUE if the user is specifically listed as a moderator of the room
36-
COALESCE(user_permission_overrides.moderator OR user_permission_overrides.admin, FALSE) AS room_moderator,
35+
COALESCE(user_permission_overrides.moderator, FALSE) AS room_moderator,
3736
-- global_moderator will be TRUE if the user is a global moderator/admin (note that this is
3837
-- *not* exclusive of room_moderator: a moderator/admin could be listed in both).
39-
COALESCE(users.moderator OR users.admin, FALSE) as global_moderator,
38+
users.moderator as global_moderator,
4039
-- visible_mod will be TRUE if this mod is a publicly viewable moderator of the room
4140
CASE
42-
WHEN user_permission_overrides.moderator OR user_permission_overrides.admin THEN user_permission_overrides.visible_mod
43-
WHEN users.moderator OR users.admin THEN users.visible_mod
41+
WHEN user_permission_overrides.moderator THEN user_permission_overrides.visible_mod
42+
WHEN users.moderator THEN users.visible_mod
4443
ELSE FALSE
4544
END AS visible_mod
4645
FROM
47-
users {'JOIN' if sqlite else 'CROSS JOIN'} rooms LEFT OUTER JOIN user_permission_overrides ON
48-
(users.id = user_permission_overrides.{'user' if sqlite else '"user"'} AND rooms.id = user_permission_overrides.room)
46+
users CROSS JOIN rooms LEFT OUTER JOIN user_permission_overrides ON
47+
(users.id = user_permission_overrides."user" AND rooms.id = user_permission_overrides.room)
4948
""" # noqa E501
5049
)
5150

sogs/schema.pgsql

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -374,14 +374,14 @@ SELECT
374374
CASE WHEN users.moderator THEN TRUE ELSE COALESCE(user_permission_overrides.moderator, FALSE) END AS moderator,
375375
CASE WHEN users.admin THEN TRUE ELSE COALESCE(user_permission_overrides.admin, FALSE) END AS admin,
376376
-- room_moderator will be TRUE if the user is specifically listed as a moderator of the room
377-
COALESCE(user_permission_overrides.moderator OR user_permission_overrides.admin, FALSE) AS room_moderator,
377+
COALESCE(user_permission_overrides.moderator, FALSE) AS room_moderator,
378378
-- global_moderator will be TRUE if the user is a global moderator/admin (note that this is
379379
-- *not* exclusive of room_moderator: a moderator/admin could be listed in both).
380-
COALESCE(users.moderator OR users.admin, FALSE) as global_moderator,
380+
users.moderator as global_moderator,
381381
-- visible_mod will be TRUE if this mod is a publicly viewable moderator of the room
382382
CASE
383-
WHEN user_permission_overrides.moderator OR user_permission_overrides.admin THEN user_permission_overrides.visible_mod
384-
WHEN users.moderator OR users.admin THEN users.visible_mod
383+
WHEN user_permission_overrides.moderator THEN user_permission_overrides.visible_mod
384+
WHEN users.moderator THEN users.visible_mod
385385
ELSE FALSE
386386
END AS visible_mod
387387
FROM

sogs/schema.sqlite

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -318,7 +318,7 @@ END;
318318
CREATE VIEW user_permissions AS
319319
SELECT
320320
rooms.id AS room,
321-
users.id AS user,
321+
users.id AS "user",
322322
users.session_id,
323323
CASE WHEN users.banned THEN TRUE ELSE COALESCE(user_permission_overrides.banned, FALSE) END AS banned,
324324
CASE WHEN users.moderator THEN TRUE ELSE COALESCE(user_permission_overrides.read, rooms.read) END AS read,
@@ -328,19 +328,19 @@ SELECT
328328
CASE WHEN users.moderator THEN TRUE ELSE COALESCE(user_permission_overrides.moderator, FALSE) END AS moderator,
329329
CASE WHEN users.admin THEN TRUE ELSE COALESCE(user_permission_overrides.admin, FALSE) END AS admin,
330330
-- room_moderator will be TRUE if the user is specifically listed as a moderator of the room
331-
COALESCE(user_permission_overrides.moderator OR user_permission_overrides.admin, FALSE) AS room_moderator,
331+
COALESCE(user_permission_overrides.moderator, FALSE) AS room_moderator,
332332
-- global_moderator will be TRUE if the user is a global moderator/admin (note that this is
333333
-- *not* exclusive of room_moderator: a moderator/admin could be listed in both).
334-
COALESCE(users.moderator OR users.admin, FALSE) as global_moderator,
334+
users.moderator as global_moderator,
335335
-- visible_mod will be TRUE if this mod is a publicly viewable moderator of the room
336336
CASE
337-
WHEN user_permission_overrides.moderator OR user_permission_overrides.admin THEN user_permission_overrides.visible_mod
338-
WHEN users.moderator OR users.admin THEN users.visible_mod
337+
WHEN user_permission_overrides.moderator THEN user_permission_overrides.visible_mod
338+
WHEN users.moderator THEN users.visible_mod
339339
ELSE FALSE
340340
END AS visible_mod
341341
FROM
342-
users JOIN rooms LEFT OUTER JOIN user_permission_overrides ON
343-
(users.id = user_permission_overrides.user AND rooms.id = user_permission_overrides.room);
342+
users CROSS JOIN rooms LEFT OUTER JOIN user_permission_overrides ON
343+
(users.id = user_permission_overrides."user" AND rooms.id = user_permission_overrides.room);
344344

345345
-- Used to accesses the moderator list for a room. This view is considerably faster than querying
346346
-- the `user_permissions` table for a list of all moderators.

0 commit comments

Comments
 (0)