use trigger on migrations table

crates/db_schema/src/schema.rs

@@ -1099,6 +1099,7 @@ diesel::allow_tables_to_appear_in_same_query!(
     post_read,
     post_report,
     post_saved,
+    previously_run_sql,
     private_message,
     private_message_report,
     received_activity,

crates/db_schema/src/schema_setup.rs

@@ -19,7 +19,7 @@ use lemmy_utils::error::{LemmyError, LemmyResult};
 use std::time::Instant;
 use tracing::info;
 
-const EMBEDDED_MIGRATIONS: EmbeddedMigrations = embed_migrations!();
+const MIGRATIONS: EmbeddedMigrations = embed_migrations!();
 
 /// This SQL code sets up the `r` schema, which contains things that can be safely dropped and replaced
 /// instead of being changed using migrations. It may not create or modify things outside of the `r` schema
@@ -30,34 +30,10 @@ const REPLACEABLE_SCHEMA: &[&str] = &[
   include_str!("../replaceable_schema/triggers.sql"),
 ];
 
-const REVERT_REPLACEABLE_SCHEMA: &str = "DROP SCHEMA IF EXISTS r CASCADE;";
-
-const LOCK_STATEMENT: &str = "LOCK __diesel_schema_migrations IN SHARE UPDATE EXCLUSIVE MODE;";
-
-struct Migrations;
-
-impl<DB: Backend> MigrationSource<DB> for Migrations {
-  fn migrations(&self) -> diesel::migration::Result<Vec<Box<dyn Migration<DB>>>> {
-    let mut migrations = EMBEDDED_MIGRATIONS.migrations()?;
-    let skipped_migration = if migrations.is_empty() {
-      None
-    } else {
-      Some(migrations.remove(0))
-    };
-
-    debug_assert_eq!(
-      skipped_migration.map(|m| m.name().to_string()),
-      Some("000000000000000_forbid_diesel_cli".to_string())
-    );
-
-    Ok(migrations)
-  }
-}
-
 fn get_pending_migrations(conn: &mut PgConnection) -> LemmyResult<Vec<Box<dyn Migration<Pg>>>> {
   Ok(
     conn
-      .pending_migrations(Migrations)
+      .pending_migrations(MIGRATIONS)
       .map_err(|e| anyhow::anyhow!("Couldn't determine pending migrations: {e}"))?,
   )
 }
@@ -97,14 +73,16 @@ pub fn run(db_url: &str) -> LemmyResult<()> {
     // lemmy_server processes from running this transaction concurrently. This lock does not block
     // `MigrationHarness::pending_migrations` (`SELECT`) or `MigrationHarness::run_migration` (`INSERT`).
     info!("Waiting for lock...");
-    conn.batch_execute(LOCK_STATEMENT)?;
+    conn.batch_execute("LOCK __diesel_schema_migrations IN SHARE UPDATE EXCLUSIVE MODE;")?;
     info!("Running Database migrations (This may take a long time)...");
 
     // Check pending migrations again after locking
     let pending_migrations = get_pending_migrations(conn)?;
 
-    // Run migrations, without stuff from replaceable_schema
+    // Drop `r` schema and disable the trigger that prevents the Diesel CLI from running migrations
-    conn.batch_execute(REVERT_REPLACEABLE_SCHEMA)?;
+    conn.batch_execute(
+      "DROP SCHEMA IF EXISTS r CASCADE; SET LOCAL lemmy.enable_migrations TO 'on';",
+    )?;
 
     for migration in &pending_migrations {
       let name = migration.name();

migrations/000000000000000_forbid_diesel_cli/up.sql

@@ -1,6 +0,0 @@
-DO $$
-BEGIN
-    RAISE 'migrations must be managed using lemmy_server instead of diesel CLI';
-END
-$$;
-

migrations/2024-04-29-012112_forbid_diesel_cli/down.sql

@@ -0,0 +1,2 @@
+DROP FUNCTION forbid_diesel_cli CASCADE;
+

migrations/2024-04-29-012112_forbid_diesel_cli/up.sql

@@ -0,0 +1,29 @@
+-- This trigger prevents using the Diesel CLI to run or revert migrations, so the custom migration runner
+-- can drop and recreate the `r` schema for new migrations.
+--
+-- This migration being seperate from the next migration (created in the same PR) guarantees that the
+-- Diesel CLI will fail to bring the number of pending migrations to 0, which is one of the conditions
+-- required to skip running replaceable_schema.
+--
+-- If the Diesel CLI could run or revert migrations, this scenario would be possible:
+--
+-- Run `diesel migration redo` when the newest migration has a new table with triggers. End up with triggers
+-- being dropped and not replaced because triggers are created outside of up.sql. The custom migration runner
+-- sees that there are no pending migrations and the value in the `previously_run_sql` trigger is correct, so
+-- it doesn't rebuild the `r` schema. There is now incorrect behavior but no error messages.
+CREATE FUNCTION forbid_diesel_cli ()
+    RETURNS TRIGGER
+    LANGUAGE plpgsql
+    AS $$
+BEGIN
+    IF current_setting('lemmy.enable_migrations', TRUE) IS DISTINCT FROM 'on' THEN
+        RAISE 'migrations must be managed using lemmy_server instead of diesel CLI';
+    END IF;
+    RETURN NULL;
+END;
+$$;
+
+CREATE TRIGGER forbid_diesel_cli
+    BEFORE INSERT OR UPDATE OR DELETE OR TRUNCATE ON __diesel_schema_migrations
+    EXECUTE FUNCTION forbid_diesel_cli ();
+