Room return info_updates

contrib/upgrade-tests/dump-db.py

@@ -78,7 +78,7 @@ def dump_rows(table, extra=None, where=None, order="id", skip=set()):
     return table.get_string() + "\n"
 
 
-print(dump_rows("rooms", skip={'created'}))
+print(dump_rows("rooms", skip={'created', 'info_updates'}))
 
 TableNotFoundError = pg.errors.UndefinedTable if pg else sqlite3.OperationalError
 try:

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

@@ -1,10 +1,10 @@
 rooms:
-+----+------------+-------------------+-------------+-------+--------------+--------------+------------------+------+------------+-------+--------+
-| id | token      | name              | description | image | active_users | info_updates | message_sequence | read | accessible | write | upload |
-+----+------------+-------------------+-------------+-------+--------------+--------------+------------------+------+------------+-------+--------+
-|  1 | round-room | A spherical room! | NULL        |  NULL |            0 |            1 |                0 |    1 |          1 |     1 |      1 |
-|  2 | sudoku     | Sudoku lovers     | NULL        |     3 |            0 |            2 |                1 |    1 |          1 |     1 |      1 |
-+----+------------+-------------------+-------------+-------+--------------+--------------+------------------+------+------------+-------+--------+
++----+------------+-------------------+-------------+-------+--------------+------------------+------+------------+-------+--------+
+| id | token      | name              | description | image | active_users | message_sequence | read | accessible | write | upload |
++----+------------+-------------------+-------------+-------+--------------+------------------+------+------------+-------+--------+
+|  1 | round-room | A spherical room! | NULL        |  NULL |            0 |                0 |    1 |          1 |     1 |      1 |
+|  2 | sudoku     | Sudoku lovers     | NULL        |     3 |            0 |                1 |    1 |          1 |     1 |      1 |
++----+------------+-------------------+-------------+-------+--------------+------------------+------+------------+-------+--------+
 
 room_import_hacks:
 +------+-------------------+--------------------+

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

@@ -1,12 +1,12 @@
 rooms:
-+----+------------+--------------+----------------------------------------------------------------+-------+--------------+--------------+------------------+------+------------+-------+--------+
-| id | token      | name         | description                                                    | image | active_users | info_updates | message_sequence | read | accessible | write | upload |
-+----+------------+--------------+----------------------------------------------------------------+-------+--------------+--------------+------------------+------+------------+-------+--------+
-|  1 | gibberish  | Gibberish 🥄 | Delivering your daily dose of gibberish.                       |     3 |            0 |            3 |                7 |    1 |          1 |     1 |      1 |
-|  2 | sudoku     | Sudoku       | Delivering your daily dose of gib... sudoku.                   |     1 |            0 |            3 |                5 |    1 |          1 |     1 |      1 |
-|  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 |
-|  4 | empty-room | Empty        | We don't go there anymore.                                     |  NULL |            0 |            1 |                0 |    1 |          1 |     1 |      1 |
-+----+------------+--------------+----------------------------------------------------------------+-------+--------------+--------------+------------------+------+------------+-------+--------+
++----+------------+--------------+----------------------------------------------------------------+-------+--------------+------------------+------+------------+-------+--------+
+| id | token      | name         | description                                                    | image | active_users | message_sequence | read | accessible | write | upload |
++----+------------+--------------+----------------------------------------------------------------+-------+--------------+------------------+------+------------+-------+--------+
+|  1 | gibberish  | Gibberish 🥄 | Delivering your daily dose of gibberish.                       |     3 |            0 |                7 |    1 |          1 |     1 |      1 |
+|  2 | sudoku     | Sudoku       | Delivering your daily dose of gib... sudoku.                   |     1 |            0 |                5 |    1 |          1 |     1 |      1 |
+|  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 |
+|  4 | empty-room | Empty        | We don't go there anymore.                                     |  NULL |            0 |                0 |    1 |          1 |     1 |      1 |
++----+------------+--------------+----------------------------------------------------------------+-------+--------------+------------------+------+------------+-------+--------+
 
 room_import_hacks:
 message_metadata:

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

@@ -1,12 +1,12 @@
 rooms:
-+----+------------+--------------+----------------------------------------------------------------+-------+--------------+--------------+------------------+------+------------+-------+--------+
-| id | token      | name         | description                                                    | image | active_users | info_updates | message_sequence | read | accessible | write | upload |
-+----+------------+--------------+----------------------------------------------------------------+-------+--------------+--------------+------------------+------+------------+-------+--------+
-|  1 | gibberish  | Gibberish 🥄 | Delivering your daily dose of gibberish.                       |     2 |            0 |            3 |               10 |    1 |          1 |     1 |      1 |
-|  2 | sudoku     | Sudoku       | Delivering your daily dose of gib... sudoku.                   |     1 |            0 |            2 |                4 |    1 |          1 |     1 |      1 |
-|  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 |
-|  4 | empty-room | Empty        | We don't go there anymore.                                     |  NULL |            0 |            1 |                0 |    1 |          1 |     1 |      1 |
-+----+------------+--------------+----------------------------------------------------------------+-------+--------------+--------------+------------------+------+------------+-------+--------+
++----+------------+--------------+----------------------------------------------------------------+-------+--------------+------------------+------+------------+-------+--------+
+| id | token      | name         | description                                                    | image | active_users | message_sequence | read | accessible | write | upload |
++----+------------+--------------+----------------------------------------------------------------+-------+--------------+------------------+------+------------+-------+--------+
+|  1 | gibberish  | Gibberish 🥄 | Delivering your daily dose of gibberish.                       |     2 |            0 |               10 |    1 |          1 |     1 |      1 |
+|  2 | sudoku     | Sudoku       | Delivering your daily dose of gib... sudoku.                   |     1 |            0 |                4 |    1 |          1 |     1 |      1 |
+|  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 |
+|  4 | empty-room | Empty        | We don't go there anymore.                                     |  NULL |            0 |                0 |    1 |          1 |     1 |      1 |
++----+------------+--------------+----------------------------------------------------------------+-------+--------------+------------------+------+------------+-------+--------+
 
 room_import_hacks:
 message_metadata:

sogs/migrations/__init__.py

@@ -5,6 +5,7 @@
 
 from . import (
     file_message,
+    fix_info_update_triggers,
     import_hacks,
     message_views,
     new_columns,
@@ -46,6 +47,7 @@ def migrate(conn, *, check_only=False):
         room_moderators,
         user_permissions,
         file_message,
+        fix_info_update_triggers,
         import_hacks,
     ):
         changes = False

sogs/migrations/fix_info_update_triggers.py

@@ -0,0 +1,87 @@
+import logging
+from .exc import DatabaseUpgradeRequired
+
+
+def migrate(conn, *, check_only):
+    from .. import db
+
+    # Room info_updates triggers for global mods didn't fire for invisible global mods/admins, but
+    # should (so that other mods/admins notice the change).
+    has_bad_trigger = db.query(
+        """
+        SELECT COUNT(*) FROM sqlite_master
+            WHERE type = 'trigger' AND name = :trigger
+            AND LOWER(sql) LIKE :bad
+        """
+        if db.engine.name == "sqlite"
+        else """
+        SELECT COUNT(*) FROM information_schema.triggers
+            WHERE trigger_name = :trigger
+            AND LOWER(action_condition) LIKE :bad
+        """,
+        trigger='room_metadata_global_mods_insert',
+        bad='% new.visible_mod%',
+        dbconn=conn,
+    ).first()[0]
+
+    if not has_bad_trigger:
+        return False
+
+    logging.warning("DB migration: fixing global hidden mod room triggers")
+    if check_only:
+        raise DatabaseUpgradeRequired("global hidden mod room triggers need to be recreated")
+
+    if db.engine.name == "sqlite":
+        conn.execute("DROP TRIGGER IF EXISTS room_metadata_global_mods_insert")
+        conn.execute("DROP TRIGGER IF EXISTS room_metadata_global_mods_update")
+        conn.execute("DROP TRIGGER IF EXISTS room_metadata_global_mods_delete")
+        conn.execute(
+            """
+CREATE TRIGGER room_metadata_global_mods_insert AFTER INSERT ON users
+FOR EACH ROW WHEN (NEW.admin OR NEW.moderator)
+BEGIN
+    UPDATE rooms SET info_updates = info_updates + 1; -- WHERE everything!
+END
+"""
+        )
+        conn.execute(
+            """
+CREATE TRIGGER room_metadata_global_mods_update AFTER UPDATE ON users
+FOR EACH ROW WHEN (NEW.moderator != OLD.moderator OR NEW.admin != OLD.admin OR NEW.visible_mod != OLD.visible_mod)
+BEGIN
+    UPDATE rooms SET info_updates = info_updates + 1; -- WHERE everything!
+END
+"""  # noqa: E501
+        )
+        conn.execute(
+            """
+CREATE TRIGGER room_metadata_global_mods_delete AFTER DELETE ON users
+FOR EACH ROW WHEN (OLD.moderator OR OLD.admin)
+BEGIN
+    UPDATE rooms SET info_updates = info_updates + 1; -- WHERE everything!
+END
+"""
+        )
+
+    else:  # postgresql
+        conn.execute(
+            """
+DROP TRIGGER IF EXISTS room_metadata_global_mods_insert ON users;
+DROP TRIGGER IF EXISTS room_metadata_global_mods_update ON users;
+DROP TRIGGER IF EXISTS room_metadata_global_mods_delete ON users;
+
+CREATE TRIGGER room_metadata_global_mods_insert AFTER INSERT ON users
+FOR EACH ROW WHEN (NEW.admin OR NEW.moderator)
+EXECUTE PROCEDURE trigger_room_metadata_info_update_all();
+
+CREATE TRIGGER room_metadata_global_mods_update AFTER UPDATE OF moderator, admin, visible_mod ON users
+FOR EACH ROW WHEN (NEW.moderator != OLD.moderator OR NEW.admin != OLD.admin OR NEW.visible_mod != OLD.visible_mod)
+EXECUTE PROCEDURE trigger_room_metadata_info_update_all();
+
+CREATE TRIGGER room_metadata_global_mods_delete AFTER DELETE ON users
+FOR EACH ROW WHEN (OLD.moderator OR OLD.admin)
+EXECUTE PROCEDURE trigger_room_metadata_info_update_all();
+"""  # noqa: E501
+        )
+
+    return True

sogs/model/room.py

@@ -1374,9 +1374,8 @@ def set_moderator(self, user: User, *, added_by: User, admin=False, visible=True
                     admin=admin,
                     visible=visible,
                 )
-                if u.id in self._perm_cache:
-                    del self._perm_cache[u.id]
 
+                self._refresh()
                 if u.id in self._perm_cache:
                     del self._perm_cache[u.id]
 
@@ -1395,19 +1394,21 @@ def remove_moderator(self, user: User, *, removed_by: User, remove_admin_only: b
         if not self.check_admin(removed_by):
             raise BadPermission()
 
-        query(
-            f"""
-            UPDATE user_permission_overrides
-            SET admin = FALSE
-                {', moderator = FALSE, visible_mod = TRUE' if not remove_admin_only else ''}
-            WHERE room = :r AND "user" = :u
-            """,
-            r=self.id,
-            u=user.id,
-        )
+        with db.transaction():
+            query(
+                f"""
+                UPDATE user_permission_overrides
+                SET admin = FALSE
+                    {', moderator = FALSE, visible_mod = TRUE' if not remove_admin_only else ''}
+                WHERE room = :r AND "user" = :u
+                """,
+                r=self.id,
+                u=user.id,
+            )
 
-        if user.id in self._perm_cache:
-            del self._perm_cache[user.id]
+            self._refresh()
+            if user.id in self._perm_cache:
+                del self._perm_cache[user.id]
 
         app.logger.info(f"{removed_by} removed {user} as mod/admin of {self}")
 
@@ -1826,7 +1827,7 @@ def pin(self, msg_id: int, admin: User):
                 a=admin.id,
                 now=time.time(),
             )
-        self._pinned = None
+            self._refresh()
 
     def unpin_all(self, admin: User):
         """
@@ -1855,8 +1856,9 @@ def unpin_all(self, admin: User):
             if unpinned_files:
                 File.reset_expiries(unpinned_files)
 
-        if count != 0:
-            self._pinned = None
+            if count != 0:
+                self._refresh()
+
         return count
 
     def unpin(self, msg_id: int, admin: User):
@@ -1884,8 +1886,9 @@ def unpin(self, msg_id: int, admin: User):
             if unpinned_files:
                 File.reset_expiries(unpinned_files)
 
-        if count != 0:
-            self._pinned = None
+            if count != 0:
+                self._refresh()
+
         return count
 
     @property

sogs/routes/messages.py

@@ -473,7 +473,10 @@ def message_pin(room, msg_id):
 
     # Return value
 
-    On success returns a 200 status code and returns an empty JSON object as response.
+    On success returns a 200 status code and returns a JSON object as response containing keys:
+
+    - `info_updates` -- the new info_updates value of the room; a client can use this to avoid
+      race conditions with room info polling that might not yet include the updated value(s).
 
     # Error status codes
 
@@ -483,7 +486,7 @@ def message_pin(room, msg_id):
       pinning (e.g. a whisper or deleted post).
     """
     room.pin(msg_id, g.user)
-    return jsonify({})
+    return jsonify({"info_updates": room.info_updates})
 
 
 @messages.post("/room/<Room:room>/unpin/<int:msg_id>")
@@ -504,14 +507,20 @@ def message_unpin(room, msg_id):
 
     # Return value
 
-    On success returns a 200 status code and returns an empty JSON object as response body.
+    On success returns a 200 status code and returns an JSON object as response body containing
+    keys:
+
+    - `unpinned` - boolean value indicating whether the message was pinned and has now been unpinned
+      (true), or was already unpinned (false).
+    - `info_updates` - the new info_updates value for the room.  This value will only change if the
+      given message was actually pinned (i.e. it does not increment when `unpinned` is false).
 
     # Error status codes
 
     - 403 Forbidden — returned if the invoking user does not have admin permission in this room.
     """
-    room.unpin(msg_id, g.user)
-    return jsonify({})
+    count = room.unpin(msg_id, g.user)
+    return jsonify({"unpinned": count > 0, "info_updates": room.info_updates})
 
 
 @messages.post("/room/<Room:room>/unpin/all")
@@ -527,15 +536,18 @@ def message_unpin_all(room):
 
     # Return value
 
-    On success returns a 200 status code with an empty JSON object as response body.  All pinned
-    messages have been removed.
+    On success returns a 200 status code with an JSON object as response body containing keys:
+
+    - `unpinned` - count of how many pinned messages were removed.
+    - `info_updates` - new `info_updates` property for the room.  This value is only incremented by
+      this operation if at least one message was found and unpinned (i.e. if `unpinned > 0`).
 
     # Error status codes
 
     - 403 Forbidden — returned if the invoking user does not have admin permission in this room.
     """
-    room.unpin_all(g.user)
-    return jsonify({})
+    count = room.unpin_all(g.user)
+    return jsonify({"unpinned": count, "info_updates": room.info_updates})
 
 
 @messages.put("/room/<Room:room>/reaction/<int:msg_id>/<path:reaction>")

sogs/routes/rooms.py

@@ -203,7 +203,10 @@ def update_room(room):
 
     # Return value
 
-    On success this endpoint returns a 200 status code and an empty json object (`{}`) as the body.
+    On success this endpoint returns a 200 status code and a json object containing keys:
+
+    - `info_updates` -- the new info_updates value of the room; a client can use this to avoid
+      race conditions with room info polling that might not yet include the updated value(s).
 
     # Error status codes
 
@@ -276,7 +279,7 @@ def update_room(room):
             app.logger.warning("Room update: must include at least one field to update")
             abort(http.BAD_REQUEST)
 
-    return jsonify({})
+    return jsonify({"info_updates": room.info_updates})
 
 
 def addExtraPermInfo(perms):