Fix a few things with migrations

src/db.rs

@@ -2,16 +2,26 @@ use crate::migrations::{Migrations, SimpleMigration};
 
 pub fn create_migrations() -> Migrations {
     let mut migs = Migrations::new();
-    migs.add(
+    migs.add(SimpleMigration::new_box(
         1,
-        SimpleMigration::new_box(
+        "CREATE TABLE sections (
-            "CREATE TABLE sections (
                         id INTEGER PRIMARY KEY,
                         name TEXT NOT NULL,
                         interface_id INTEGER NOT NULL
                     );",
-            "DROP TABLE sections;",
+        "DROP TABLE sections;",
-        ),
+    ));
-    );
+    migs.add(SimpleMigration::new_box(
+        2,
+        "INSERT INTO sections (id, name, interface_id)
+                VALUES 
+                    (1, 'Front Yard Middle', 0),
+                    (2, 'Front Yard Left', 1),
+                    (3, 'Front Yard Right', 2),
+                    (4, 'Back Yard Middle', 3),
+                    (5, 'Back Yard Sauna', 4),
+                    (6, 'Garden', 5);",
+        "DELETE FROM sections;",
+    ));
     migs
 }

src/migrations.rs

@@ -1,55 +1,76 @@
+use log::debug;
 use rusqlite::NO_PARAMS;
 use rusqlite::{params, Connection};
 use std::collections::BTreeMap;
 use std::ops::Bound::{Excluded, Unbounded};
 use thiserror::Error;
-use log::debug;
 
 #[derive(Debug, Error)]
 pub enum MigrationError {
     #[error("sql error: {0}")]
     SqlError(#[from] rusqlite::Error),
+    #[error("migration version {0} up failed, sql error: {1}")]
+    MigrationUpFailed(MigrationVersion, rusqlite::Error),
+    #[error("migration version {0} down failed, sql error: {1}")]
+    MigrationDownFailed(MigrationVersion, rusqlite::Error),
     #[error("database version {0} too new to migrate")]
     VersionTooNew(MigrationVersion),
 }
 
 pub type MigrationResult<T> = Result<T, MigrationError>;
 
+pub type MigrationVersion = u32;
+
 pub trait Migration {
+    fn version(&self) -> MigrationVersion;
     fn up(&self, conn: &Connection) -> MigrationResult<()>;
     fn down(&self, conn: &Connection) -> MigrationResult<()>;
 }
 
 pub struct SimpleMigration {
+    pub version: MigrationVersion,
     pub up_sql: String,
     pub down_sql: String,
 }
 
 impl SimpleMigration {
-    pub fn new<T1: ToString, T2: ToString>(up_sql: T1, down_sql: T2) -> Self {
+    pub fn new<T1: ToString, T2: ToString>(
+        version: MigrationVersion,
+        up_sql: T1,
+        down_sql: T2,
+    ) -> Self {
         Self {
+            version,
             up_sql: up_sql.to_string(),
             down_sql: down_sql.to_string(),
         }
     }
 
-    pub fn new_box<T1: ToString, T2: ToString>(up_sql: T1, down_sql: T2) -> Box<dyn Migration> {
+    pub fn new_box<T1: ToString, T2: ToString>(
-        Box::new(Self::new(up_sql, down_sql))
+        version: MigrationVersion,
+        up_sql: T1,
+        down_sql: T2,
+    ) -> Box<dyn Migration> {
+        Box::new(Self::new(version, up_sql, down_sql))
     }
 }
 
 impl Migration for SimpleMigration {
+    fn version(&self) -> MigrationVersion {
+        self.version
+    }
     fn up(&self, conn: &Connection) -> MigrationResult<()> {
-        conn.execute(&self.up_sql, NO_PARAMS)?;
+        conn.execute_batch(&self.up_sql)
+            .map_err(|sql_err| MigrationError::MigrationUpFailed(self.version, sql_err))?;
         Ok(())
     }
     fn down(&self, conn: &Connection) -> MigrationResult<()> {
-        conn.execute(&self.down_sql, NO_PARAMS)?;
+        conn.execute_batch(&self.down_sql)
+            .map_err(|sql_err| MigrationError::MigrationDownFailed(self.version, sql_err))?;
         Ok(())
     }
 }
 
-pub type MigrationVersion = u32;
 pub const NO_MIGRATIONS: MigrationVersion = 0;
 
 pub fn get_db_version(conn: &Connection) -> MigrationResult<MigrationVersion> {
@@ -77,12 +98,15 @@ pub fn set_db_version(conn: &Connection, version: MigrationVersion) -> Migration
         CREATE TABLE IF NOT EXISTS db_version (
             id INTEGER PRIMARY KEY,
             version INTEGER
-        );", NO_PARAMS)?;
+        );",
+        NO_PARAMS,
+    )?;
     conn.execute(
         "
         INSERT OR REPLACE INTO db_version (id, version)
             VALUES (1, ?1);",
-        params![version])?;
+        params![version],
+    )?;
     Ok(())
 }
 
@@ -97,8 +121,9 @@ impl Migrations {
         }
     }
 
-    pub fn add(&mut self, version: MigrationVersion, migration: Box<dyn Migration>) {
+    pub fn add(&mut self, migration: Box<dyn Migration>) {
-        self.migrations.insert(version, migration);
+        assert!(migration.version() != NO_MIGRATIONS, "migration has bad vesion");
+        self.migrations.insert(migration.version(), migration);
     }
 
     pub fn apply(&self, conn: &mut Connection) -> MigrationResult<()> {
@@ -106,16 +131,17 @@ impl Migrations {
         if db_version != 0 && !self.migrations.contains_key(&db_version) {
             return Err(MigrationError::VersionTooNew(db_version));
         }
-        let mig_range = self.migrations.range(
+        let mig_range = self.migrations.range((Excluded(db_version), Unbounded));
-            (Excluded(db_version), Unbounded));
         let mut trans = conn.transaction()?;
-        let mut last_ver: MigrationVersion = 0;
+        let mut last_ver: MigrationVersion = NO_MIGRATIONS;
         for (ver, mig) in mig_range {
             debug!("applying migration version {}", ver);
             mig.up(&mut trans)?;
             last_ver = *ver;
         }
-        set_db_version(&trans, last_ver)?;
+        if last_ver != NO_MIGRATIONS {
+            set_db_version(&trans, last_ver)?;
+        }
         trans.commit()?;
         Ok(())
     }