Fix database bugs

- message_views migration code was all sorts of messed up: it wasn't
  applying on sqlite when it was meant to, and it wasn't actually adding
  the filtered field that it was checking to see when migration was
  needed, and didn't have the `seqno` rename.  Also the
  message_details_deleter was missing.

- Moved message_views migration to after seqno updates because it
  depends on `seqno` being there now.

- Added `filtered` column to message_metadata in the schema files.

- "user" wasn't properly quoted in a couple places

- pinned_by had the wrong foreign key on messages.  SQLite tests didn't
  pick this up because FK constraints aren't enforced by default under
  SQLite (which is moronic).

- enable foreign key enforcement on sqlite connections.

Author: jagerman

sogs/db.py

@@ -132,10 +132,10 @@ def database_init():
         migrate_v01x,
         add_new_tables,
         add_new_columns,
-        update_message_views,
         create_message_details_deleter,
         check_for_hacks,
         seqno_etc_updates,
+        update_message_views,
         user_perm_future_updates,
     ):
         with transaction(conn):
@@ -228,7 +228,7 @@ def add_new_tables(conn):
 
 
 def update_message_views(conn):
-    if engine.name != "sqlite":
+    if engine.name == "sqlite":
         if any(x not in metadata.tables['message_metadata'].c for x in ('whisper_to', 'filtered')):
             logging.warning("DB migration: replacing message_metadata/message_details views")
             conn.execute("DROP VIEW IF EXISTS message_metadata")
@@ -240,19 +240,30 @@ def update_message_views(conn):
     FROM messages
         JOIN users uposter ON messages."user" = uposter.id
         LEFT JOIN users uwhisper ON messages.whisper = uwhisper.id
+"""
+            )
+            conn.execute(
+                """
+CREATE TRIGGER message_details_deleter INSTEAD OF DELETE ON message_details
+FOR EACH ROW WHEN OLD.data IS NOT NULL
+BEGIN
+    UPDATE messages SET data = NULL, data_size = NULL, signature = NULL
+        WHERE id = OLD.id;
+END
 """
             )
             conn.execute(
                 """
 CREATE VIEW message_metadata AS
-SELECT id, room, "user", session_id, posted, edited, updated, filtered, whisper_to,
+SELECT id, room, "user", session_id, posted, edited, seqno, filtered, whisper_to,
         length(data) AS data_unpadded, data_size, length(signature) as signature_length
     FROM message_details
 """
             )
 
             return True
-        # else: don't worry about this for postgresql because initial pg support had the fix
+
+    # else: don't worry about this for postgresql because initial pg support had the fix
 
     return False
 
@@ -342,7 +353,7 @@ def seqno_etc_updates(conn):
             """
 CREATE TABLE pinned_messages (
     room INTEGER NOT NULL REFERENCES rooms(id) ON DELETE CASCADE,
-    message INTEGER NOT NULL REFERENCES rooms(id) ON DELETE CASCADE,
+    message INTEGER NOT NULL REFERENCES messages(id) ON DELETE CASCADE,
     pinned_by INTEGER NOT NULL REFERENCES users(id),
     pinned_at FLOAT NOT NULL DEFAULT ((julianday('now') - 2440587.5)*86400.0), /* unix epoch when pinned */
     PRIMARY KEY(room, message)
@@ -425,7 +436,7 @@ def seqno_etc_updates(conn):
             """
 CREATE TABLE pinned_messages (
     room BIGINT NOT NULL REFERENCES rooms ON DELETE CASCADE,
-    message BIGINT NOT NULL REFERENCES rooms ON DELETE CASCADE,
+    message BIGINT NOT NULL REFERENCES messages ON DELETE CASCADE,
     pinned_by BIGINT NOT NULL REFERENCES users,
     pinned_at FLOAT NOT NULL DEFAULT (extract(epoch from now())),
     PRIMARY KEY(room, message)
@@ -627,6 +638,10 @@ def sqlite_fix_connect(dbapi_connection, connection_record):
             # disable pysqlite's emitting of the BEGIN statement entirely.
             # also stops it from emitting COMMIT before any DDL.
             dbapi_connection.isolation_level = None
+            # Enforce foreign keys.  It is very sad that this is not default.
+            cursor = dbapi_connection.cursor()
+            cursor.execute("PRAGMA foreign_keys=ON")
+            cursor.close()
 
         @sqlalchemy.event.listens_for(engine, "begin")
         def do_begin(conn):

sogs/model/room.py

@@ -525,7 +525,7 @@ def get_messages_for(
             # - anything directed to us specifically
             # - anything we sent (i.e. outbound whispers)
             # - non-whispers
-            "whisper_mods OR whisper = :user OR user = :user OR whisper IS NULL"
+            'whisper_mods OR whisper = :user OR "user" = :user OR whisper IS NULL'
             if mod
             # For a regular user we want to see:
             # - anything with whisper_to sent to us

sogs/schema.pgsql

@@ -17,7 +17,6 @@ CREATE TABLE rooms (
     upload BOOLEAN NOT NULL DEFAULT TRUE, /* Whether file uploads are allowed by default */
     CHECK(token SIMILAR TO '[a-zA-Z0-9_-]+')
 );
-CREATE INDEX rooms_token ON rooms(token);
 
 -- Trigger to expire an old room image attachment when the room image is changed
 CREATE OR REPLACE FUNCTION trigger_room_image_expiry()
@@ -90,8 +89,8 @@ EXECUTE PROCEDURE trigger_messages_insert_history();
 
 CREATE TABLE pinned_messages (
     room BIGINT NOT NULL REFERENCES rooms ON DELETE CASCADE,
-    message BIGINT NOT NULL REFERENCES rooms ON DELETE CASCADE,
-    pinned_by BIGINT NOT NULL REFERENCES users,
+    message BIGINT NOT NULL REFERENCES messages ON DELETE CASCADE,
+    pinned_by BIGINT NOT NULL, /* foreign key to users(id) */
     pinned_at FLOAT NOT NULL DEFAULT (extract(epoch from now())),
     PRIMARY KEY(room, message)
 );
@@ -154,6 +153,7 @@ CREATE INDEX users_last_active ON users(last_active);
 ALTER TABLE messages ADD CONSTRAINT messages_user_fk FOREIGN KEY ("user") REFERENCES users;
 ALTER TABLE messages ADD CONSTRAINT messages_whisper_fk FOREIGN KEY (whisper) REFERENCES users;
 ALTER TABLE files ADD CONSTRAINT files_uploader_fk FOREIGN KEY (uploader) REFERENCES users;
+ALTER TABLE pinned_messages ADD CONSTRAINT pinned_messages_pinned_by FOREIGN KEY (pinned_by) REFERENCES users;
 
 -- Create a trigger to maintain the implication "admin implies moderator"
 CREATE OR REPLACE FUNCTION trigger_user_admins_are_mods()
@@ -194,7 +194,7 @@ EXECUTE PROCEDURE trigger_message_details_deleter();
 -- View of `messages` that is useful for manually inspecting table contents by only returning the
 -- length (rather than raw bytes) for data/signature.
 CREATE VIEW message_metadata AS
-SELECT id, room, "user", session_id, posted, edited, seqno, whisper_to,
+SELECT id, room, "user", session_id, posted, edited, seqno, filtered, whisper_to,
         length(data) AS data_unpadded, data_size, length(signature) as signature_length
     FROM message_details;
 

sogs/schema.sqlite

@@ -81,7 +81,7 @@ END;
 
 CREATE TABLE pinned_messages (
     room INTEGER NOT NULL REFERENCES rooms(id) ON DELETE CASCADE,
-    message INTEGER NOT NULL REFERENCES rooms(id) ON DELETE CASCADE,
+    message INTEGER NOT NULL REFERENCES messages(id) ON DELETE CASCADE,
     pinned_by INTEGER NOT NULL REFERENCES users(id),
     pinned_at FLOAT NOT NULL DEFAULT ((julianday('now') - 2440587.5)*86400.0), /* unix epoch when pinned */
     PRIMARY KEY(room, message)
@@ -165,7 +165,7 @@ END;
 -- View of `messages` that is useful for manually inspecting table contents by only returning the
 -- length (rather than raw bytes) for data/signature.
 CREATE VIEW message_metadata AS
-SELECT id, room, user, session_id, posted, edited, seqno, whisper_to,
+SELECT id, room, user, session_id, posted, edited, seqno, filtered, whisper_to,
         length(data) AS data_unpadded, data_size, length(signature) as signature_length
     FROM message_details;