From e07e251ada64a7eb861c7648ba982241a63160a4 Mon Sep 17 00:00:00 2001 From: Dessalines Date: Fri, 20 Jun 2025 03:49:44 -0400 Subject: [PATCH] Removing an existing deferrable constraint, and fail test if any constraint is deferrable. (#5806) * Removing an existing deferrable constraint. - Also adding a check to make sure the final dump contains no DEFERs. * Spelling error --- crates/db_schema_file/src/diff_check.rs | 11 +++++++++-- crates/db_schema_file/src/schema_setup.rs | 2 ++ .../down.sql | 3 +++ .../up.sql | 6 ++++++ 4 files changed, 20 insertions(+), 2 deletions(-) create mode 100644 migrations/2025-06-18-130813_community_instance_not_deferrable/down.sql create mode 100644 migrations/2025-06-18-130813_community_instance_not_deferrable/up.sql diff --git a/crates/db_schema_file/src/diff_check.rs b/crates/db_schema_file/src/diff_check.rs index 41638f4d7..55ded6f3a 100644 --- a/crates/db_schema_file/src/diff_check.rs +++ b/crates/db_schema_file/src/diff_check.rs @@ -13,7 +13,7 @@ use std::{ /// Returns almost all things currently in the database, represented as SQL statements that would /// recreate them. -pub fn get_dump() -> String { +pub(crate) fn get_dump() -> String { let db_url = SETTINGS.get_database_url(); let output = Command::new("pg_dump") .args([ @@ -58,7 +58,7 @@ pub fn get_dump() -> String { /// not `dumps[1]` to `dumps[0]`. This requires the two `dumps` elements being in an order that fits /// with `label_of_change_from_0_to_1`. This does not necessarily match the order in which the dumps /// were created. -pub fn check_dump_diff(dumps: [&str; 2], label_of_change_from_0_to_1: &str) { +pub(crate) fn check_dump_diff(dumps: [&str; 2], label_of_change_from_0_to_1: &str) { let [sorted_statements_in_0, sorted_statements_in_1] = dumps.map(|dump| { dump .split("\n\n") @@ -195,6 +195,13 @@ fn display_change([before, after]: [&str; 2]) -> impl Iterator { }) } +/// Makes sure the after dump does not contain any DEFERRABLE constraints. +pub(crate) fn deferr_constraint_check(dump: &str) { + if dump.contains(" DEFERR") { + panic!("Schema should not have DEFER constraints.") + } +} + // `#[cfg(test)]` would be redundant here mod tests { #[test] diff --git a/crates/db_schema_file/src/schema_setup.rs b/crates/db_schema_file/src/schema_setup.rs index b8984c974..ac90ec986 100644 --- a/crates/db_schema_file/src/schema_setup.rs +++ b/crates/db_schema_file/src/schema_setup.rs @@ -234,6 +234,8 @@ pub fn run(options: Options) -> LemmyResult { let after = diff_check::get_dump(); diff_check::check_dump_diff([&before, &after], "The code in crates/db_schema/replaceable_schema incorrectly created or modified things outside of the `r` schema, causing these changes to be left behind after dropping the schema:"); + + diff_check::deferr_constraint_check(&after); } run_replaceable_schema(&mut conn)?; diff --git a/migrations/2025-06-18-130813_community_instance_not_deferrable/down.sql b/migrations/2025-06-18-130813_community_instance_not_deferrable/down.sql new file mode 100644 index 000000000..f5971025a --- /dev/null +++ b/migrations/2025-06-18-130813_community_instance_not_deferrable/down.sql @@ -0,0 +1,3 @@ +ALTER TABLE community + ALTER CONSTRAINT community_instance_id_fkey DEFERRABLE INITIALLY DEFERRED; + diff --git a/migrations/2025-06-18-130813_community_instance_not_deferrable/up.sql b/migrations/2025-06-18-130813_community_instance_not_deferrable/up.sql new file mode 100644 index 000000000..738f617e5 --- /dev/null +++ b/migrations/2025-06-18-130813_community_instance_not_deferrable/up.sql @@ -0,0 +1,6 @@ +-- We should remove existing deferrable constraints, as they're potentially dangerous. +-- +-- This is the only one I could find after doing a DB dump. +ALTER TABLE community + ALTER CONSTRAINT community_instance_id_fkey NOT DEFERRABLE; +