Procházet zdrojové kódy

Add support for collecting existing schemata and confirming that migrations apply as expected.

Kestrel před 6 dny
rodič
revize
9f9d9fd6fc

+ 76 - 2
microrm/src/db.rs

@@ -3,7 +3,7 @@ use libsqlite3_sys as sq;
 use std::{
     cell::{Cell, RefCell},
     collections::{HashMap, VecDeque},
-    ffi::{CStr, CString},
+    ffi::{c_void, CStr, CString},
     pin::Pin,
     sync::{Arc, Condvar, Mutex, Weak},
 };
@@ -23,7 +23,7 @@ fn check_rcode<'a>(sql: impl FnOnce() -> Option<&'a str>, rcode: i32) -> Result<
 }
 
 /// Internal struct that stores several pieces of connection-relevant data.
-pub struct ConnectionData {
+struct ConnectionData {
     sqlite: *mut sq::sqlite3,
     stmts: RefCell<HashMap<u64, Statement>>,
 }
@@ -380,6 +380,80 @@ impl<'l, 'd: 'l> Drop for Transaction<'l, 'd> {
     }
 }
 
+#[derive(Debug)]
+pub(crate) struct RawSchemaRow {
+    pub type_: String,
+    pub name: String,
+    pub tbl_name: String,
+    pub rootpage: String,
+    pub sql: String,
+}
+
+pub(crate) fn get_raw_schema(lease: &mut ConnectionLease) -> DBResult<Vec<RawSchemaRow>> {
+    let mut schema_entries: Vec<RawSchemaRow> = vec![].into();
+
+    unsafe extern "C" fn rowcb(
+        entries: *mut c_void,
+        ncols: i32,
+        rowdata: *mut *mut i8,
+        _cols: *mut *mut i8,
+    ) -> i32 {
+        assert_eq!(ncols, 5);
+        let rows = std::slice::from_raw_parts(rowdata, ncols as usize);
+        let entries = (entries as *mut Vec<RawSchemaRow>).as_mut().unwrap();
+
+        let extract_value = |idx: usize| {
+            if !rows[idx].is_null() {
+                CStr::from_ptr(rows[idx]).to_str().map(ToString::to_string)
+            } else {
+                Ok(String::new())
+            }
+        };
+
+        let row = (|| {
+            DBResult::Ok(RawSchemaRow {
+                type_: extract_value(0)?,
+                name: extract_value(1)?,
+                tbl_name: extract_value(2)?,
+                rootpage: extract_value(3)?,
+                sql: extract_value(4)?,
+            })
+        })();
+
+        match row {
+            Ok(row) => {
+                // log::trace!("should save extracted row {row:?}");
+                entries.push(row);
+                // log::trace!("saved");
+                0
+            },
+            Err(err) => {
+                log::error!("error while getting raw schema: {err:?}");
+                1
+            },
+        }
+    }
+
+    unsafe {
+        let sql = "SELECT * FROM sqlite_schema";
+        let c_sql = CString::new(sql)?;
+        let entries_mut = &mut schema_entries;
+        let entries_ptr = std::ptr::from_mut(entries_mut);
+        check_rcode(
+            || Some(sql),
+            sq::sqlite3_exec(
+                lease.conn.sqlite,
+                c_sql.as_ptr(),
+                Some(rowcb),
+                entries_ptr as *mut c_void,
+                std::ptr::null_mut(),
+            ),
+        )?;
+    }
+
+    Ok(schema_entries)
+}
+
 struct Statement {
     #[allow(unused)]
     sqlite: *mut sq::sqlite3,

+ 3 - 0
microrm/src/lib.rs

@@ -288,6 +288,9 @@ pub enum Error {
     LockError(String),
     /// An empty database was found when an existing one was expected.
     EmptyDatabase,
+    /// The database schema is in an inconsistent state, probably either from corruption or an
+    /// incomplete migration.
+    ConsistencyError(String),
 }
 
 impl std::fmt::Display for Error {

+ 1 - 0
microrm/src/schema.rs

@@ -29,6 +29,7 @@ pub mod relation;
 pub mod index;
 
 mod build;
+mod check;
 mod collect;
 pub(crate) mod meta;
 

+ 3 - 1
microrm/src/schema/build.rs

@@ -56,8 +56,10 @@ impl TableInfo {
             ))
         });
 
+        // note the use of double quotes here to ensure sqlite's normalization doesn't modify this
+        // query when stored in sqlite_schema
         format!(
-            "create table `{}` (`id` integer primary key{}{}{});",
+            "CREATE TABLE \"{}\" (`id` integer primary key{}{}{})",
             self.table_name,
             columns.collect::<String>(),
             fkeys.collect::<String>(),

+ 74 - 0
microrm/src/schema/check.rs

@@ -0,0 +1,74 @@
+use crate::prelude::*;
+use crate::schema;
+use crate::{ConnectionLease, DBResult};
+
+pub fn check_schema<S: schema::Schema>(lease: &mut ConnectionLease) -> DBResult<()> {
+    let raw_schema = crate::db::get_raw_schema(lease)?;
+    let generated = schema::build::generate_from_schema::<S>();
+    let metagenerated = schema::build::generate_single_entity_table::<schema::meta::MicrormMeta>();
+
+    let mut tablecount = 0;
+    let mut indexcount = 0;
+
+    for rs in raw_schema {
+        if rs.type_ == "table" {
+            // confirm generated SQL is the same
+            if rs.tbl_name == "microrm_meta" {
+                if metagenerated != rs.sql {
+                    log::error!("Metadata schema inconsistent between DB and schema");
+                    return Err(crate::Error::ConsistencyError(format!(
+                        "Table SQL mismatch for metadata"
+                    )));
+                }
+            } else if Some(&rs.sql) != generated.table_queries().get(&rs.tbl_name) {
+                log::error!("Mismatch in SQL for {}", rs.tbl_name);
+                log::trace!(
+                    "Should be: {:?}",
+                    generated.table_queries().get(&rs.tbl_name)
+                );
+                log::trace!("Found in DB: {:?}", rs.sql);
+                return Err(crate::Error::ConsistencyError(format!(
+                    "Table SQL mismatch for {}",
+                    rs.tbl_name
+                )));
+            } else {
+                tablecount += 1;
+            }
+        } else if rs.type_ == "index" {
+            if rs.name.starts_with("sqlite_autoindex") {
+                continue;
+            }
+
+            indexcount += 1;
+            todo!("index handling")
+        } else {
+            log::warn!(
+                "unknown sqlite schema element type {} for {}",
+                rs.type_,
+                rs.name
+            );
+        }
+    }
+
+    if tablecount != generated.table_queries().len() {
+        log::error!(
+            "Incorrect number of tables in DB schema: should be {}, is {}",
+            generated.table_queries().len(),
+            tablecount
+        );
+        Err(crate::Error::ConsistencyError(format!(
+            "Table count mismatch"
+        )))
+    } else if indexcount != generated.index_queries().len() {
+        log::error!(
+            "Incorrect number of indicies in DB schema: should be {}, is {}",
+            generated.index_queries().len(),
+            indexcount
+        );
+        Err(crate::Error::ConsistencyError(format!(
+            "Index count mismatch"
+        )))
+    } else {
+        Ok(())
+    }
+}

+ 14 - 2
microrm/src/schema/migration.rs

@@ -411,6 +411,7 @@ fn migration_helper<A: SchemaList>(lease: &mut ConnectionLease) -> DBResult<()>
     }
 
     let built = generate_from_schema::<A::Head>();
+    log::trace!("checking if head schema is present");
     match built.check(lease) {
         Some(true) => Ok(()),
         Some(false) => {
@@ -429,16 +430,27 @@ fn migration_helper<A: SchemaList>(lease: &mut ConnectionLease) -> DBResult<()>
 
             <MigrateTo<A> as MigratableItem<MigrateFrom<A>>>::run_migration(&mut context)?;
 
-            context.finish()
+            context.finish()?;
+
+            schema::check::check_schema::<A::Head>(lease)?;
+
+            Ok(())
         },
         None => Err(Error::EmptyDatabase),
     }
 }
 
 pub fn run_migration<A: SchemaList>(lease: &mut ConnectionLease) -> DBResult<()> {
+    lease.execute_raw_sql("PRAGMA foreign_keys=OFF")?;
     let mut upgrade_txn = crate::db::Transaction::new(lease, "_microrm_migration")?;
     // find the earliest matching schema, and then run each migration in sequence if needed
     migration_helper::<A>(upgrade_txn.lease())?;
 
-    upgrade_txn.commit()
+    upgrade_txn
+        .lease()
+        .execute_raw_sql("PRAGMA foreign_key_check")?;
+
+    upgrade_txn.commit()?;
+
+    lease.execute_raw_sql("PRAGMA foreign_keys=ON")
 }

+ 45 - 16
microrm/tests/migration.rs

@@ -10,14 +10,10 @@ mod schema1 {
 
     #[derive(Entity)]
     pub struct KVEntity {
+        #[key]
         pub k: usize,
         pub v: String,
     }
-
-    #[derive(Default, Schema)]
-    pub struct Schema {
-        pub kv: microrm::IDMap<KVEntity>,
-    }
 }
 
 mod schema2 {
@@ -25,17 +21,28 @@ mod schema2 {
 
     #[derive(Entity)]
     pub struct KVEntity {
+        #[key]
         pub k: String,
         pub v: String,
     }
+}
 
-    #[derive(Default, Schema)]
-    pub struct Schema {
-        pub kv: microrm::IDMap<KVEntity>,
-    }
+#[derive(Default, Schema)]
+pub struct Schema1 {
+    pub kv: microrm::IDMap<schema1::KVEntity>,
+}
+
+#[derive(Default, Schema)]
+pub struct Schema2 {
+    pub kv: microrm::IDMap<schema2::KVEntity>,
+}
+
+#[derive(Default, Schema)]
+pub struct Schema2b {
+    pub kv: microrm::IDMap<schema2::KVEntity>,
 }
 
-impl MigratableItem<schema1::Schema> for schema2::Schema {
+impl MigratableItem<Schema1> for Schema2 {
     fn run_migration(ctx: &mut MigrationContext) -> microrm::DBResult<()>
     where
         Self: Sized,
@@ -44,7 +51,7 @@ impl MigratableItem<schema1::Schema> for schema2::Schema {
 
         let ipkv2 = ctx.in_progress::<schema2::KVEntity>()?;
 
-        for kv in schema1::Schema::default().kv.get(ctx.lease())? {
+        for kv in Schema1::default().kv.get(ctx.lease())? {
             ipkv2.insert_matching(
                 ctx.lease(),
                 schema2::KVEntity {
@@ -68,11 +75,21 @@ impl MigratableItem<schema1::Schema> for schema2::Schema {
     }
 }
 
-type Schemas = (schema1::Schema, schema2::Schema);
+impl MigratableItem<Schema1> for Schema2b {
+    fn run_migration(_ctx: &mut MigrationContext) -> microrm::DBResult<()>
+    where
+        Self: Sized,
+    {
+        // incorrectly, don't do anything. this should be caught.
+        Ok(())
+    }
+}
+
+type Schemas = (Schema1, Schema2);
 
 #[test]
 fn run_empty_migration() {
-    let (pool, _db): (_, schema2::Schema) = common::open_test_db!();
+    let (pool, _db): (_, Schema2) = common::open_test_db!();
 
     let mut lease = pool.acquire().unwrap();
     run_migration::<Schemas>(&mut lease).expect("migration result");
@@ -80,7 +97,7 @@ fn run_empty_migration() {
 
 #[test]
 fn run_simple_migration() {
-    let (pool, _db): (_, schema1::Schema) = common::open_test_db!();
+    let (pool, _db): (_, Schema1) = common::open_test_db!();
 
     let mut lease = pool.acquire().unwrap();
     run_migration::<Schemas>(&mut lease).expect("migration result");
@@ -88,7 +105,7 @@ fn run_simple_migration() {
 
 #[test]
 fn check_simple_migration() {
-    let (pool, db): (_, schema1::Schema) = common::open_test_db!();
+    let (pool, db): (_, Schema1) = common::open_test_db!();
     let mut lease = pool.acquire().unwrap();
 
     db.kv
@@ -103,7 +120,7 @@ fn check_simple_migration() {
 
     run_migration::<Schemas>(&mut lease).expect("migration result");
 
-    let db2 = schema2::Schema::default();
+    let db2 = Schema2::default();
     let migrated = db2
         .kv
         .with(schema2::KVEntity::K, "42")
@@ -124,3 +141,15 @@ fn check_simple_migration() {
     assert_eq!(migrated.k, "s");
     assert_eq!(migrated.v, "v");
 }
+
+#[test]
+fn check_incorrect_migration() {
+    let (pool, _db): (_, Schema1) = common::open_test_db!();
+    let mut lease = pool.acquire().unwrap();
+
+    let Err(microrm::Error::ConsistencyError(_msg)) =
+        run_migration::<(Schema1, Schema2b)>(&mut lease)
+    else {
+        panic!("incorrect migration succeeded");
+    };
+}