From 021cb71861ae0e9db09281cade07d39ed6438501 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Joly?= Date: Sun, 18 May 2025 21:27:38 +0100 Subject: [PATCH] feat: add a opt-in validations of migrations Some users may want to test downward migrations (#113). This is a proposal for a composable set of checks (called validations), more or less stringent, that users can run in a unit test. More validations could be added in the future. Closes https://github.com/cljoly/rusqlite_migration/issues/113 --- rusqlite_migration/src/lib.rs | 4 + rusqlite_migration/src/tests/mod.rs | 2 +- rusqlite_migration/src/validations/mod.rs | 185 ++++++++++++++++++ ...__error_missing_downward_pretty_print.snap | 6 + ...ation__validations__tests__full_error.snap | 21 ++ ...idations__tests__missing_downward_end.snap | 15 ++ ...tions__tests__missing_downward_middle.snap | 15 ++ ...ations__tests__missing_downward_start.snap | 15 ++ ...sts__multiple_missing_multiple_errors.snap | 23 +++ ...ons__tests__rusqlite_error_conversion.snap | 6 + ...migration__validations__tests__source.snap | 19 ++ rusqlite_migration/src/validations/tests.rs | 120 ++++++++++++ 12 files changed, 430 insertions(+), 1 deletion(-) create mode 100644 rusqlite_migration/src/validations/mod.rs create mode 100644 rusqlite_migration/src/validations/snapshots/rusqlite_migration__validations__tests__error_missing_downward_pretty_print.snap create mode 100644 rusqlite_migration/src/validations/snapshots/rusqlite_migration__validations__tests__full_error.snap create mode 100644 rusqlite_migration/src/validations/snapshots/rusqlite_migration__validations__tests__missing_downward_end.snap create mode 100644 rusqlite_migration/src/validations/snapshots/rusqlite_migration__validations__tests__missing_downward_middle.snap create mode 100644 rusqlite_migration/src/validations/snapshots/rusqlite_migration__validations__tests__missing_downward_start.snap create mode 100644 rusqlite_migration/src/validations/snapshots/rusqlite_migration__validations__tests__multiple_missing_multiple_errors.snap create mode 100644 rusqlite_migration/src/validations/snapshots/rusqlite_migration__validations__tests__rusqlite_error_conversion.snap create mode 100644 rusqlite_migration/src/validations/snapshots/rusqlite_migration__validations__tests__source.snap create mode 100644 rusqlite_migration/src/validations/tests.rs diff --git a/rusqlite_migration/src/lib.rs b/rusqlite_migration/src/lib.rs index 8dd8f8c..973126e 100644 --- a/rusqlite_migration/src/lib.rs +++ b/rusqlite_migration/src/lib.rs @@ -37,6 +37,7 @@ mod builder; pub use builder::MigrationsBuilder; mod errors; +pub mod validations; #[cfg(test)] mod tests; @@ -897,6 +898,9 @@ impl<'m> Migrations<'m> { /// Run upward migrations on a temporary in-memory database from first to last, one by one. /// Convenience method for testing. /// + /// See the [`validations`] module if you want to validate other things as well, like downward + /// migrations. + /// /// # Example /// /// ``` diff --git a/rusqlite_migration/src/tests/mod.rs b/rusqlite_migration/src/tests/mod.rs index 6260a3f..95a368a 100644 --- a/rusqlite_migration/src/tests/mod.rs +++ b/rusqlite_migration/src/tests/mod.rs @@ -17,4 +17,4 @@ mod builder; mod core; -mod helpers; +pub(crate) mod helpers; diff --git a/rusqlite_migration/src/validations/mod.rs b/rusqlite_migration/src/validations/mod.rs new file mode 100644 index 0000000..90bedf1 --- /dev/null +++ b/rusqlite_migration/src/validations/mod.rs @@ -0,0 +1,185 @@ +// SPDX-License-Identifier: Apache-2.0 +// Copyright Clément Joly and contributors. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +//! Run a more complete set of validations (like requiring a downward migration to be present and +//! to apply cleanly). This is useful in a unit test, to validate the migrations. +//! +//! See also [`Migrations::validate`] for simple cases. +//! +//! # Example +//! +//! ``` +//! #[cfg(test)] +//! mod tests { +//! +//! // … Other tests … +//! +//! #[test] +//! fn migrations_test() -> Result<(), dyn Error> { +//! Validations::everything().validate(migrations)?; +//! } +//! } +//! ``` + +use std::fmt::Display; + +use rusqlite::Connection; + +use super::Migrations; + +#[cfg(test)] +mod tests; + +/// Result for validations +pub type Result<'m, T, E = Error> = std::result::Result; + +/// Enum of possible validation errors. +#[derive(Debug, PartialEq)] +#[non_exhaustive] +pub enum Error { + /// Downward migrations were required for every upward migrations, but some are missing. + MissingDownwardMigrations(Vec<(usize, String)>), + /// Underlying rusqlite_migration error. + RusqliteMigration(crate::Error), +} + +impl From for Error { + fn from(value: crate::Error) -> Self { + Error::RusqliteMigration(value) + } +} + +impl From for Error { + fn from(value: rusqlite::Error) -> Self { + Error::from(crate::Error::from(value)) + } +} + +impl std::error::Error for Error { + fn source(&self) -> Option<&(dyn std::error::Error + 'static)> { + match self { + Error::MissingDownwardMigrations(_) => None, + Error::RusqliteMigration(error) => Some(error), + } + } +} + +impl Display for Error { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + Error::RusqliteMigration(e) => write!(f, "underlying rusqlite migration error: {e}"), + Error::MissingDownwardMigrations(vs) => { + write!( + f, + "the following migrations do not have a corresponding downward migration: " + )?; + for (i, v) in vs { + write!(f, "{i}: {v}, ")? + } + Ok(()) + } + } + } +} + +#[derive(PartialEq, Debug)] +enum DownwardCheck { + No, + IfPresent, + Required, +} + +/// Opt-in checks to validate migrations +pub struct Validations { + downward: DownwardCheck, +} + +impl Validations { + /// Apply all possible checks, in their strictest setting. Please note that future versions of + /// the library will add more checks and so this might cause tests to fail when you upgrade the + /// library. + pub fn everything() -> Self { + Self { + downward: DownwardCheck::Required, + } + } + + /// Always validate upward migrations + pub fn upward() -> Self { + Self { + downward: DownwardCheck::No, + } + } + + /// Validate all downwards migrations found. Allow a downward migration to be missing. + pub fn check_downward_if_present(mut self) -> Self { + self.downward = DownwardCheck::IfPresent; + self + } + + /// Validate all downwards migrations found. Error if a downward migration is missing. + pub fn require_downward(mut self) -> Self { + self.downward = DownwardCheck::Required; + self + } + + /// Run the validations + pub fn validate(&self, migrations: &Migrations) -> Result<()> { + // Let’s have all fields in scope, to ensure we don’t forgot to use any flags (or any + // future flags) + let Self { downward } = self; + let mut conn = Connection::open_in_memory()?; + let nbr_migrations = migrations.pending_migrations(&conn)? as usize; + if nbr_migrations == 0 { + log::debug!("no migrations defined, they are deemed valid"); + return Ok(()); + } + + // https://mutants.rs/skip_calls.html#with_capacity + let mut missing_downward_migrations = + Vec::with_capacity(if *downward == DownwardCheck::Required { + nbr_migrations + } else { + 0 + }); + + // Always check upward migrations and check downward ones depending on flags + for i in 1..=nbr_migrations { + log::debug!("Checking migration number {i}"); + migrations.to_version(&mut conn, i)?; + match downward { + DownwardCheck::No => (), + DownwardCheck::Required | DownwardCheck::IfPresent => { + if migrations.ms[i - 1].down.is_some() { + // Revert and reapply, to see if the revert applies cleanly + migrations.to_version(&mut conn, i - 1)?; + migrations.to_version(&mut conn, i)?; + } else if *downward == DownwardCheck::Required { + let m = &migrations.ms[i - 1]; + missing_downward_migrations.push((i, format!("{m:?}"))) + } + } + }; + } + + if missing_downward_migrations.is_empty() { + Ok(()) + } else { + Err(Error::MissingDownwardMigrations( + missing_downward_migrations, + )) + } + } +} diff --git a/rusqlite_migration/src/validations/snapshots/rusqlite_migration__validations__tests__error_missing_downward_pretty_print.snap b/rusqlite_migration/src/validations/snapshots/rusqlite_migration__validations__tests__error_missing_downward_pretty_print.snap new file mode 100644 index 0000000..e5684e9 --- /dev/null +++ b/rusqlite_migration/src/validations/snapshots/rusqlite_migration__validations__tests__error_missing_downward_pretty_print.snap @@ -0,0 +1,6 @@ +--- +source: rusqlite_migration/src/validations/tests.rs +expression: "Validations::everything().validate(&migrations).unwrap_err()" +snapshot_kind: text +--- +the following migrations do not have a corresponding downward migration: 1: M { up: "CREATE TABLE m1(a, b); CREATE TABLE m2(a, b, c);", up_hook: None, down: None, down_hook: None, foreign_key_check: false, comment: None }, diff --git a/rusqlite_migration/src/validations/snapshots/rusqlite_migration__validations__tests__full_error.snap b/rusqlite_migration/src/validations/snapshots/rusqlite_migration__validations__tests__full_error.snap new file mode 100644 index 0000000..af4901e --- /dev/null +++ b/rusqlite_migration/src/validations/snapshots/rusqlite_migration__validations__tests__full_error.snap @@ -0,0 +1,21 @@ +--- +source: rusqlite_migration/src/validations/tests.rs +expression: v +snapshot_kind: text +--- +Err( + RusqliteMigration( + RusqliteError { + query: "Invalid sql", + err: SqliteFailure( + Error { + code: Unknown, + extended_code: 1, + }, + Some( + "near \"Invalid\": syntax error", + ), + ), + }, + ), +) diff --git a/rusqlite_migration/src/validations/snapshots/rusqlite_migration__validations__tests__missing_downward_end.snap b/rusqlite_migration/src/validations/snapshots/rusqlite_migration__validations__tests__missing_downward_end.snap new file mode 100644 index 0000000..a2cfbca --- /dev/null +++ b/rusqlite_migration/src/validations/snapshots/rusqlite_migration__validations__tests__missing_downward_end.snap @@ -0,0 +1,15 @@ +--- +source: rusqlite_migration/src/validations/tests.rs +expression: "Validations::everything().validate(&migrations)" +snapshot_kind: text +--- +Err( + MissingDownwardMigrations( + [ + ( + 6, + "M { up: \"\\n CREATE TABLE fk1(a PRIMARY KEY);\\n CREATE TABLE fk2(\\n a,\\n FOREIGN KEY(a) REFERENCES fk1(a)\\n );\\n INSERT INTO fk1 (a) VALUES ('foo');\\n INSERT INTO fk2 (a) VALUES ('foo');\\n \", up_hook: None, down: None, down_hook: None, foreign_key_check: true, comment: None }", + ), + ], + ), +) diff --git a/rusqlite_migration/src/validations/snapshots/rusqlite_migration__validations__tests__missing_downward_middle.snap b/rusqlite_migration/src/validations/snapshots/rusqlite_migration__validations__tests__missing_downward_middle.snap new file mode 100644 index 0000000..81f33cb --- /dev/null +++ b/rusqlite_migration/src/validations/snapshots/rusqlite_migration__validations__tests__missing_downward_middle.snap @@ -0,0 +1,15 @@ +--- +source: rusqlite_migration/src/validations/tests.rs +expression: "Validations::everything().validate(&migrations)" +snapshot_kind: text +--- +Err( + MissingDownwardMigrations( + [ + ( + 4, + "M { up: \"CREATE TABLE t2(b);\", up_hook: None, down: None, down_hook: None, foreign_key_check: false, comment: None }", + ), + ], + ), +) diff --git a/rusqlite_migration/src/validations/snapshots/rusqlite_migration__validations__tests__missing_downward_start.snap b/rusqlite_migration/src/validations/snapshots/rusqlite_migration__validations__tests__missing_downward_start.snap new file mode 100644 index 0000000..8a18a78 --- /dev/null +++ b/rusqlite_migration/src/validations/snapshots/rusqlite_migration__validations__tests__missing_downward_start.snap @@ -0,0 +1,15 @@ +--- +source: rusqlite_migration/src/validations/tests.rs +expression: "Validations::everything().validate(&migrations)" +snapshot_kind: text +--- +Err( + MissingDownwardMigrations( + [ + ( + 1, + "M { up: \"CREATE TABLE m1(a, b); CREATE TABLE m2(a, b, c);\", up_hook: None, down: None, down_hook: None, foreign_key_check: false, comment: None }", + ), + ], + ), +) diff --git a/rusqlite_migration/src/validations/snapshots/rusqlite_migration__validations__tests__multiple_missing_multiple_errors.snap b/rusqlite_migration/src/validations/snapshots/rusqlite_migration__validations__tests__multiple_missing_multiple_errors.snap new file mode 100644 index 0000000..a2d523b --- /dev/null +++ b/rusqlite_migration/src/validations/snapshots/rusqlite_migration__validations__tests__multiple_missing_multiple_errors.snap @@ -0,0 +1,23 @@ +--- +source: rusqlite_migration/src/validations/tests.rs +expression: "Validations::everything().validate(&migrations)" +snapshot_kind: text +--- +Err( + MissingDownwardMigrations( + [ + ( + 3, + "M { up: \"ALTER TABLE t1 RENAME COLUMN b TO c;\", up_hook: None, down: None, down_hook: None, foreign_key_check: false, comment: None }", + ), + ( + 4, + "M { up: \"CREATE TABLE t2(b);\", up_hook: None, down: None, down_hook: None, foreign_key_check: false, comment: None }", + ), + ( + 6, + "M { up: \"\\n CREATE TABLE fk1(a PRIMARY KEY);\\n CREATE TABLE fk2(\\n a,\\n FOREIGN KEY(a) REFERENCES fk1(a)\\n );\\n INSERT INTO fk1 (a) VALUES ('foo');\\n INSERT INTO fk2 (a) VALUES ('foo');\\n \", up_hook: None, down: None, down_hook: None, foreign_key_check: true, comment: None }", + ), + ], + ), +) diff --git a/rusqlite_migration/src/validations/snapshots/rusqlite_migration__validations__tests__rusqlite_error_conversion.snap b/rusqlite_migration/src/validations/snapshots/rusqlite_migration__validations__tests__rusqlite_error_conversion.snap new file mode 100644 index 0000000..91f6ff3 --- /dev/null +++ b/rusqlite_migration/src/validations/snapshots/rusqlite_migration__validations__tests__rusqlite_error_conversion.snap @@ -0,0 +1,6 @@ +--- +source: rusqlite_migration/src/validations/tests.rs +expression: e +snapshot_kind: text +--- +underlying rusqlite migration error: rusqlite_migrate error: RusqliteError { query: "", err: InvalidQuery } diff --git a/rusqlite_migration/src/validations/snapshots/rusqlite_migration__validations__tests__source.snap b/rusqlite_migration/src/validations/snapshots/rusqlite_migration__validations__tests__source.snap new file mode 100644 index 0000000..41aff59 --- /dev/null +++ b/rusqlite_migration/src/validations/snapshots/rusqlite_migration__validations__tests__source.snap @@ -0,0 +1,19 @@ +--- +source: rusqlite_migration/src/validations/tests.rs +expression: v.unwrap_err().source() +snapshot_kind: text +--- +Some( + RusqliteError { + query: "Invalid sql", + err: SqliteFailure( + Error { + code: Unknown, + extended_code: 1, + }, + Some( + "near \"Invalid\": syntax error", + ), + ), + }, +) diff --git a/rusqlite_migration/src/validations/tests.rs b/rusqlite_migration/src/validations/tests.rs new file mode 100644 index 0000000..62fbef2 --- /dev/null +++ b/rusqlite_migration/src/validations/tests.rs @@ -0,0 +1,120 @@ +// SPDX-License-Identifier: Apache-2.0 +// Copyright Clément Joly and contributors. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +use super::*; +use crate::Migrations; + +use crate::tests::helpers::{ + all_valid_down, m_valid0_up, m_valid11_up, m_valid20_up, m_valid_fk_up, +}; + +#[test] +fn test_empty_migrations() { + let migrations = Migrations::from_slice(&[]); + assert_eq!(Validations::upward().validate(&migrations), Ok(())); + assert_eq!(Validations::everything().validate(&migrations), Ok(())); +} + +#[test] +fn test_missing_downward_start() { + let mut missing_start = all_valid_down(); + missing_start[0] = m_valid0_up(); + + let migrations = Migrations::new(missing_start); + insta::assert_debug_snapshot!(Validations::everything().validate(&migrations)); + + // Additional checks for consistency + assert_eq!(Validations::upward().validate(&migrations), Ok(())); + { + use std::error::Error; + + let v = Validations::everything().validate(&migrations).unwrap_err(); + insta::assert_snapshot!("error_missing_downward_pretty_print", v); + assert!(v.source().is_none()); + } +} + +#[test] +fn test_missing_downward_middle() { + let mut missing_middle = all_valid_down(); + missing_middle[3] = m_valid20_up(); + + let migrations = Migrations::new(missing_middle); + insta::assert_debug_snapshot!(Validations::everything().validate(&migrations)); +} + +#[test] +fn test_missing_downward_end() { + let mut missing_end = all_valid_down(); + let len = missing_end.len(); + missing_end[len - 1] = m_valid_fk_up(); + + let migrations = Migrations::new(missing_end); + insta::assert_debug_snapshot!(Validations::everything().validate(&migrations)); +} + +#[test] +fn test_missing_downward_multiple() { + let mut missing_multiple = all_valid_down(); + let len = missing_multiple.len(); + missing_multiple[2] = m_valid11_up(); + missing_multiple[3] = m_valid20_up(); + missing_multiple[len - 1] = m_valid_fk_up(); + + let migrations = Migrations::new(missing_multiple); + insta::assert_debug_snapshot!( + "multiple_missing_multiple_errors", + Validations::everything().validate(&migrations) + ); +} + +#[test] +fn test_invalid_down_migration() { + use std::error::Error; + + let mut invalid_start = all_valid_down(); + invalid_start[0] = m_valid0_up().down("Invalid sql"); + + let migrations = Migrations::new(invalid_start); + let v = Validations::everything().validate(&migrations); + + insta::assert_debug_snapshot!("full_error", v); + assert_eq!( + Validations::upward() + .require_downward() + .validate(&migrations), + v + ); + assert_eq!( + Validations::upward() + .check_downward_if_present() + .validate(&migrations), + v + ); + assert!(Validations::upward().validate(&migrations).is_ok()); + + insta::assert_debug_snapshot!("source", v.unwrap_err().source()); +} + +#[test] +fn test_rusqlite_error_conversion() { + let rusqlite_e = rusqlite::Error::InvalidQuery; + let e = Error::from(rusqlite_e); + assert!(matches!( + e, + Error::RusqliteMigration(crate::Error::RusqliteError { .. }) + )); + insta::assert_snapshot!(e) +}