From 8e0bbd61eba6274d0bae8858137bfa3d5c405ea1 Mon Sep 17 00:00:00 2001 From: Dull Bananas Date: Fri, 17 May 2024 21:21:57 +0000 Subject: [PATCH] Revert "Revert "diff_checker (partial)"" This reverts commit d4bdda5d11216f4acd13b9585d2392ca8c252a73. --- Cargo.lock | 12 +++++ crates/db_schema/Cargo.toml | 1 + crates/db_schema/src/schema_setup.rs | 48 +++++++++++++++---- .../src/schema_setup/diff_checker.rs | 0 crates/db_schema/src/utils.rs | 2 +- src/lib.rs | 2 +- 6 files changed, 53 insertions(+), 12 deletions(-) create mode 100644 crates/db_schema/src/schema_setup/diff_checker.rs diff --git a/Cargo.lock b/Cargo.lock index 29bdde002..caa20959e 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2833,6 +2833,7 @@ dependencies = [ "serde_json", "serde_with", "serial_test", + "string-interner", "strum", "strum_macros", "tokio", @@ -5228,6 +5229,17 @@ dependencies = [ "pin-project-lite", ] +[[package]] +name = "string-interner" +version = "0.17.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "1c6a0d765f5807e98a091107bae0a56ea3799f66a5de47b2c84c94a39c09974e" +dependencies = [ + "cfg-if", + "hashbrown 0.14.3", + "serde", +] + [[package]] name = "string_cache" version = "0.8.7" diff --git a/crates/db_schema/Cargo.toml b/crates/db_schema/Cargo.toml index a0654f063..c54a6a8be 100644 --- a/crates/db_schema/Cargo.toml +++ b/crates/db_schema/Cargo.toml @@ -85,6 +85,7 @@ moka.workspace = true [dev-dependencies] serial_test = { workspace = true } pretty_assertions = { workspace = true } +string-interner = { version = "0.17.0", features = ["inline-more", "backends"] } [package.metadata.cargo-machete] ignored = ["strum"] diff --git a/crates/db_schema/src/schema_setup.rs b/crates/db_schema/src/schema_setup.rs index a0abd1fcc..b042d8e19 100644 --- a/crates/db_schema/src/schema_setup.rs +++ b/crates/db_schema/src/schema_setup.rs @@ -1,3 +1,6 @@ +#[cfg(test)] +mod diff_checker; + use crate::schema::previously_run_sql; use anyhow::{anyhow, Context}; use diesel::{ @@ -17,6 +20,8 @@ use diesel_migrations::MigrationHarness; use lemmy_utils::error::{LemmyError, LemmyResult}; use std::time::Instant; use tracing::info; +use lemmy_utils::settings::SETTINGS; +use std::collections::BTreeMap; // In production, include migrations in the binary #[cfg(not(debug_assertions))] @@ -96,20 +101,40 @@ impl<'a, T: MigrationSource> MigrationSource for MigrationSourceRef<&'a } #[derive(Default)] -pub struct Options { +struct DiffChecker { + /// Maps a migration name to the schema that exists before the migration is applied + schema_before: BTreeMap, + /// Stores strings +} + +#[derive(Default)] +struct Schema { + indexes: BTreeMap +} + +#[derive(Default)] +pub struct Options<'a> { enable_forbid_diesel_cli_trigger: bool, revert: bool, revert_amount: Option, redo_after_revert: bool, + #[cfg(test)] + diff_checker: Option<&'a mut diff_checker::DiffChecker>, } -impl Options { +impl<'a> Options<'a> { #[cfg(test)] fn enable_forbid_diesel_cli_trigger(mut self) -> Self { self.enable_forbid_diesel_cli_trigger = true; self } + #[cfg(test)] + fn diff_checker(mut self, diff_checker: &'a mut diff_checker::DiffChecker) -> Self { + self.diff_checker = Some(diff_checker); + self + } + pub fn revert(mut self, amount: Option) -> Self { self.revert = true; self.revert_amount = amount; @@ -122,14 +147,15 @@ impl Options { } } -pub fn run(db_url: &str, options: Options) -> LemmyResult<()> { +pub fn run(options: Options) -> LemmyResult<()> { + let db_url = SETTINGS.get_database_url(); + // Migrations don't support async connection, and this function doesn't need to be async let mut conn = PgConnection::establish(db_url).with_context(|| "Error connecting to database")?; let new_sql = REPLACEABLE_SCHEMA.join("\n"); let migration_source = get_migration_source(); - let migration_source_ref = MigrationSourceRef(&migration_source); // If possible, skip locking the migrations table and recreating the "r" schema, so @@ -228,24 +254,26 @@ mod tests { #[test] #[serial] fn test_schema_setup() -> LemmyResult<()> { - let url = SETTINGS.get_database_url(); - let mut conn = PgConnection::establish(&url)?; + let db_url = SETTINGS.get_database_url(); + let mut conn = PgConnection::establish(&db_url)?; + let diff_checker = DiffChecker::default(); + let options = || Options::default().diff_checker(&mut diff_checker); // Start with consistent state by dropping everything conn.batch_execute("DROP OWNED BY CURRENT_USER;")?; // Run and revert all migrations, ensuring there's no mistakes in any down.sql file - run(&url, Options::default())?; - run(&url, Options::default().revert(None))?; + run(options())?; + run(options().revert(None))?; // TODO also don't drop r, and maybe just directly call the migrationharness method here assert!(matches!( - run(&url, Options::default().enable_forbid_diesel_cli_trigger()), + run(options().enable_forbid_diesel_cli_trigger()), Err(e) if e.to_string().contains("lemmy_server") )); // Previous run shouldn't stop this one from working - run(&url, Options::default())?; + run(options())?; Ok(()) } diff --git a/crates/db_schema/src/schema_setup/diff_checker.rs b/crates/db_schema/src/schema_setup/diff_checker.rs new file mode 100644 index 000000000..e69de29bb diff --git a/crates/db_schema/src/utils.rs b/crates/db_schema/src/utils.rs index ade500d2c..11809255f 100644 --- a/crates/db_schema/src/utils.rs +++ b/crates/db_schema/src/utils.rs @@ -427,7 +427,7 @@ pub async fn build_db_pool() -> LemmyResult { })) .build()?; - crate::schema_setup::run(&db_url, Default::default())?; + crate::schema_setup::run(Default::default())?; Ok(pool) } diff --git a/src/lib.rs b/src/lib.rs index 377ad946c..1010f91ca 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -133,7 +133,7 @@ pub async fn start_lemmy_server(args: CmdArgs) -> LemmyResult<()> { MigrationSubcommand::Run => schema_setup::Options::default(), }; - schema_setup::run(&SETTINGS.get_database_url(), options)?; + schema_setup::run(options)?; return Ok(()); }