Skip to content

Commit

Permalink
feat: return full foreign_key_check error
Browse files Browse the repository at this point in the history
The `foreign_key_check` pragma returns a list of all the violated
foreign key constraints[^1]:
> The foreign_key_check pragma returns one row output for each foreign
> key violation
Each ForeignKeyCheckError store one of those rows. However, only the
first row is stored.

Store all row returned in a vector in the returned error variant,
`Error::ForeignKeyCheck(…)`.

[^1]: https://www.sqlite.org/pragma.html#pragma_foreign_key_check
  • Loading branch information
cljoly committed Dec 10, 2023
1 parent 950365e commit 4cc4f89
Show file tree
Hide file tree
Showing 10 changed files with 103 additions and 54 deletions.
12 changes: 6 additions & 6 deletions rusqlite_migration/src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ pub enum Error {
/// Something wrong with migration definitions
MigrationDefinition(MigrationDefinitionError),
/// The foreign key check failed
ForeignKeyCheck(ForeignKeyCheckError),
ForeignKeyCheck(Vec<ForeignKeyCheckError>),
/// Error returned by the migration hook
Hook(String),
/// Error returned when loading migrations from directory
Expand Down Expand Up @@ -81,7 +81,7 @@ impl std::error::Error for Error {
Error::RusqliteError { query: _, err } => Some(err),
Error::SpecifiedSchemaVersion(e) => Some(e),
Error::MigrationDefinition(e) => Some(e),
Error::ForeignKeyCheck(e) => Some(e),
Error::ForeignKeyCheck(vec) => Some(vec.get(0)?),
Error::Hook(_) | Error::FileLoad(_) => None,
#[cfg(feature = "async-tokio-rusqlite")]
Error::ConnectionClosed => None,
Expand Down Expand Up @@ -339,18 +339,18 @@ mod tests {
#[test]
fn test_rusqlite_error_fkc() {
assert_ne!(
Error::ForeignKeyCheck(ForeignKeyCheckError {
Error::ForeignKeyCheck(vec![ForeignKeyCheckError {
table: "t1".to_owned(),
rowid: 1,
parent: "t2".to_owned(),
fkid: 3
}),
Error::ForeignKeyCheck(ForeignKeyCheckError {
}]),
Error::ForeignKeyCheck(vec![ForeignKeyCheckError {
table: "t1".to_owned(),
rowid: 3,
parent: "t2".to_owned(),
fkid: 3
},),
}]),
)
}

Expand Down
34 changes: 20 additions & 14 deletions rusqlite_migration/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ limitations under the License.
#![doc = include_str!(concat!(env!("OUT_DIR"), "/readme_for_rustdoc.md"))]

use log::{debug, info, trace, warn};
use rusqlite::{Connection, OptionalExtension, Transaction};
use rusqlite::{Connection, Transaction};

#[cfg(feature = "from-directory")]
use include_dir::Dir;
Expand Down Expand Up @@ -773,20 +773,26 @@ fn set_user_version(conn: &Connection, v: usize) -> Result<()> {
// Validate that no foreign keys are violated
fn validate_foreign_keys(conn: &Connection) -> Result<()> {
let pragma_fk_check = "PRAGMA foreign_key_check";
conn.query_row(pragma_fk_check, [], |row| {
Ok(ForeignKeyCheckError {
table: row.get(0)?,
rowid: row.get(1)?,
parent: row.get(2)?,
fkid: row.get(3)?,
let mut stmt = conn
.prepare_cached(pragma_fk_check)
.map_err(|e| Error::with_sql(e, pragma_fk_check))?;

let fk_errors = stmt
.query_map([], |row| {
Ok(ForeignKeyCheckError {
table: row.get(0)?,
rowid: row.get(1)?,
parent: row.get(2)?,
fkid: row.get(3)?,
})
})
})
.optional()
.map_err(|e| Error::with_sql(e, pragma_fk_check))
.and_then(|o| match o {
Some(e) => Err(Error::ForeignKeyCheck(e)),
None => Ok(()),
})
.map_err(|e| Error::with_sql(e, pragma_fk_check))?
.collect::<Result<Vec<_>, _>>()?;
if !fk_errors.is_empty() {
Err(crate::Error::ForeignKeyCheck(fk_errors))
} else {
Ok(())
}
}

impl<'u> FromIterator<M<'u>> for Migrations<'u> {
Expand Down
1 change: 1 addition & 0 deletions rusqlite_migration/src/tests/helpers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@ pub fn m_invalid_fk() -> M<'static> {
FOREIGN KEY(a) REFERENCES fk1(a) \
); \
INSERT INTO fk2 (a) VALUES ('foo'); \
INSERT INTO fk2 (a) VALUES ('bar'); \
",
)
.foreign_key_check()
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
source: rusqlite_migration/src/tests/synch.rs
expression: "Error::Hook(String::new())"
---
rusqlite_migrate error: Hook("")
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
source: rusqlite_migration/src/tests/synch.rs
expression: "Error::ForeignKeyCheck(vec![ForeignKeyCheckError\n {\n table : String :: new(), rowid : 1, parent : String :: new(), fkid\n : 2,\n }, ForeignKeyCheckError\n {\n table : String :: new(), rowid : 2, parent : String :: new(), fkid\n : 3,\n },])"
---
rusqlite_migrate error: ForeignKeyCheck([ForeignKeyCheckError { table: "", rowid: 1, parent: "", fkid: 2 }, ForeignKeyCheckError { table: "", rowid: 2, parent: "", fkid: 3 }])
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
source: rusqlite_migration/src/tests/synch.rs
expression: "Error::MigrationDefinition(MigrationDefinitionError::NoMigrationsDefined)"
---
rusqlite_migrate error: MigrationDefinition(NoMigrationsDefined)
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
source: rusqlite_migration/src/tests/synch.rs
expression: "Error::RusqliteError {\n query: String::new(),\n err: rusqlite::Error::InvalidQuery,\n}"
---
rusqlite_migrate error: RusqliteError { query: "", err: InvalidQuery }
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
source: rusqlite_migration/src/tests/synch.rs
expression: "Error::SpecifiedSchemaVersion(SchemaVersionError::TargetVersionOutOfRange {\n specified: SchemaVersion::NoneSet,\n highest: SchemaVersion::NoneSet,\n })"
---
rusqlite_migrate error: SpecifiedSchemaVersion(TargetVersionOutOfRange { specified: NoneSet, highest: NoneSet })
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
---
source: rusqlite_migration/src/tests/synch.rs
expression: migrations.validate()
---
Err(
ForeignKeyCheck(
[
ForeignKeyCheckError {
table: "fk2",
rowid: 1,
parent: "fk1",
fkid: 0,
},
ForeignKeyCheckError {
table: "fk2",
rowid: 2,
parent: "fk1",
fkid: 0,
},
],
),
)
63 changes: 29 additions & 34 deletions rusqlite_migration/src/tests/synch.rs
Original file line number Diff line number Diff line change
Expand Up @@ -148,40 +148,38 @@ fn test_migration_definition_error_display() {

#[test]
fn test_error_display() {
let err = Error::SpecifiedSchemaVersion(SchemaVersionError::TargetVersionOutOfRange {
specified: SchemaVersion::NoneSet,
highest: SchemaVersion::NoneSet,
});
assert_eq!(
"rusqlite_migrate error: SpecifiedSchemaVersion(TargetVersionOutOfRange { specified: NoneSet, highest: NoneSet })",
format!("{err}")
);
insta::assert_display_snapshot!(Error::SpecifiedSchemaVersion(
SchemaVersionError::TargetVersionOutOfRange {
specified: SchemaVersion::NoneSet,
highest: SchemaVersion::NoneSet,
}
));

let err = Error::Hook(String::new());
assert_eq!("rusqlite_migrate error: Hook(\"\")", format!("{err}"));
insta::assert_display_snapshot!(Error::Hook(String::new()));

let err = Error::ForeignKeyCheck(ForeignKeyCheckError {
table: String::new(),
rowid: 1,
parent: String::new(),
fkid: 2,
});
assert_eq!("rusqlite_migrate error: ForeignKeyCheck(ForeignKeyCheckError { table: \"\", rowid: 1, parent: \"\", fkid: 2 })", format!("{err}"));
insta::assert_display_snapshot!(Error::ForeignKeyCheck(vec![
ForeignKeyCheckError {
table: String::new(),
rowid: 1,
parent: String::new(),
fkid: 2,
},
ForeignKeyCheckError {
table: String::new(),
rowid: 2,
parent: String::new(),
fkid: 3,
},
]));

let err = Error::MigrationDefinition(MigrationDefinitionError::NoMigrationsDefined);
assert_eq!(
"rusqlite_migrate error: MigrationDefinition(NoMigrationsDefined)",
format!("{err}")
);
insta::assert_display_snapshot!(Error::MigrationDefinition(
MigrationDefinitionError::NoMigrationsDefined
));

let err = Error::RusqliteError {
insta::assert_display_snapshot!(Error::RusqliteError {
query: String::new(),
err: rusqlite::Error::InvalidQuery,
};
assert_eq!(
"rusqlite_migrate error: RusqliteError { query: \"\", err: InvalidQuery }",
format!("{err}")
);
});
}

#[test]
Expand Down Expand Up @@ -240,12 +238,12 @@ fn error_test_source() {
&MigrationDefinitionError::NoMigrationsDefined
);

let err = Error::ForeignKeyCheck(ForeignKeyCheckError {
let err = Error::ForeignKeyCheck(vec![ForeignKeyCheckError {
table: String::new(),
rowid: 1i64,
parent: String::new(),
fkid: 1i64,
});
}]);
assert_eq!(
std::error::Error::source(&err)
.and_then(|e| e.downcast_ref::<ForeignKeyCheckError>())
Expand Down Expand Up @@ -350,10 +348,7 @@ fn valid_fk_check_test() {
#[test]
fn invalid_fk_check_test() {
let migrations = Migrations::new(vec![m_invalid_fk()]);
assert!(matches!(
dbg!(migrations.validate()),
Err(Error::ForeignKeyCheck(_))
));
insta::assert_debug_snapshot!(migrations.validate());
}

#[test]
Expand Down

0 comments on commit 4cc4f89

Please sign in to comment.