Merge pull request #5 from zcash/bugfixes-and-improvements

Bugfixes and improvements

Author: str4d

.github/workflows/ci.yml

@@ -47,7 +47,6 @@ jobs:
       - uses: actions/checkout@v4
       - uses: Swatinem/rust-cache@v1
       - run: cargo clippy --all-features -- -D warnings
-      - run: cargo fmt -- --check
 
   fmt:
     name: Rustfmt

Cargo.lock

@@ -15,12 +15,12 @@
 //! use postgres::{Client, NoTls, Transaction};
 //! use schemerz::{Migration, Migrator};
 //! use schemerz_postgres::{PostgresAdapter, PostgresAdapterError, PostgresMigration};
-//! use uuid::Uuid;
+//! use uuid::uuid;
 //!
 //! struct MyExampleMigration;
 //! migration!(
 //!     MyExampleMigration,
-//!     "4885e8ab-dafa-4d76-a565-2dee8b04ef60",
+//!     uuid!("4885e8ab-dafa-4d76-a565-2dee8b04ef60"),
 //!     [],
 //!     "An example migration without dependencies.");
 //!
@@ -61,7 +61,7 @@ use uuid::Uuid;
 use schemerz::{Adapter, Migration};
 
 /// PostgreSQL-specific trait for schema migrations.
-pub trait PostgresMigration: Migration {
+pub trait PostgresMigration: Migration<Uuid> {
     /// Apply a migration to the database using a transaction.
     fn up(&self, _transaction: &mut Transaction<'_>) -> Result<(), PostgresError> {
         Ok(())
@@ -127,8 +127,8 @@ impl<'a> PostgresAdapter<'a> {
     }
 }
 
-impl<'a> Adapter for PostgresAdapter<'a> {
-    type MigrationType = dyn PostgresMigration;
+impl<'a> Adapter<Uuid> for PostgresAdapter<'a> {
+    type MigrationType = Box<dyn PostgresMigration>;
 
     type Error = PostgresAdapterError;
 
@@ -176,10 +176,10 @@ mod tests {
     use schemerz::test_schemerz_adapter;
     use schemerz::testing::*;
 
-    impl PostgresMigration for TestMigration {}
+    impl PostgresMigration for TestMigration<Uuid> {}
 
-    impl<'a> TestAdapter for PostgresAdapter<'a> {
-        fn mock(id: Uuid, dependencies: HashSet<Uuid>) -> Box<Self::MigrationType> {
+    impl<'a> TestAdapter<Uuid> for PostgresAdapter<'a> {
+        fn mock(id: Uuid, dependencies: HashSet<Uuid>) -> Self::MigrationType {
             Box::new(TestMigration::new(id, dependencies))
         }
     }
@@ -196,7 +196,13 @@ mod tests {
         adapter
     }
 
+    fn uuid_iter() -> impl Iterator<Item = Uuid> {
+        (0..).map(|v| Uuid::from_fields(v as u32, v, v, &[0; 8]))
+    }
+
     test_schemerz_adapter!(
         let mut conn = build_test_connection(),
-        build_test_adapter(&mut conn));
+        build_test_adapter(&mut conn),
+        uuid_iter(),
+    );
 }

schemerz-postgres/src/lib.rs

@@ -15,12 +15,12 @@
 //! use rusqlite::{params, Connection, Transaction, Error as RusqliteError};
 //! use schemerz::{Migration, Migrator};
 //! use schemerz_rusqlite::{RusqliteAdapter, RusqliteAdapterError, RusqliteMigration};
-//! use uuid::Uuid;
+//! use uuid::uuid;
 //!
 //! struct MyExampleMigration;
 //! migration!(
 //!     MyExampleMigration,
-//!     "4885e8ab-dafa-4d76-a565-2dee8b04ef60",
+//!     uuid!("4885e8ab-dafa-4d76-a565-2dee8b04ef60"),
 //!     [],
 //!     "An example migration without dependencies.");
 //!
@@ -62,7 +62,7 @@ use uuid::Uuid;
 use schemerz::{Adapter, Migration};
 
 /// SQlite-specific trait for schema migrations.
-pub trait RusqliteMigration: Migration {
+pub trait RusqliteMigration: Migration<Uuid> {
     type Error: From<RusqliteError>;
 
     /// Apply a migration to the database using a transaction.
@@ -137,10 +137,11 @@ impl<'a, E> RusqliteAdapter<'a, E> {
     }
 }
 
-impl<'a, E: From<RusqliteError> + Sync + Send + Error + 'static> Adapter
-    for RusqliteAdapter<'a, E>
+impl<'a, E> Adapter<Uuid> for RusqliteAdapter<'a, E>
+where
+    E: From<RusqliteError> + Sync + Send + Error + 'static,
 {
-    type MigrationType = dyn RusqliteMigration<Error = E>;
+    type MigrationType = Box<dyn RusqliteMigration<Error = E>>;
 
     type Error = E;
 
@@ -197,12 +198,12 @@ mod tests {
     use schemerz::test_schemerz_adapter;
     use schemerz::testing::*;
 
-    impl RusqliteMigration for TestMigration {
+    impl RusqliteMigration for TestMigration<Uuid> {
         type Error = RusqliteError;
     }
 
-    impl<'a> TestAdapter for RusqliteAdapter<'a, RusqliteError> {
-        fn mock(id: Uuid, dependencies: HashSet<Uuid>) -> Box<Self::MigrationType> {
+    impl<'a> TestAdapter<Uuid> for RusqliteAdapter<'a, RusqliteError> {
+        fn mock(id: Uuid, dependencies: HashSet<Uuid>) -> Self::MigrationType {
             Box::new(TestMigration::new(id, dependencies))
         }
     }
@@ -217,7 +218,13 @@ mod tests {
         adapter
     }
 
+    fn uuid_iter() -> impl Iterator<Item = Uuid> {
+        (0..).map(|v| Uuid::from_fields(v as u32, v, v, &[0; 8]))
+    }
+
     test_schemerz_adapter!(
         let mut conn = build_test_connection(),
-        build_test_adapter(&mut conn));
+        build_test_adapter(&mut conn),
+        uuid_iter(),
+    );
 }

schemerz-rusqlite/src/lib.rs

@@ -9,8 +9,57 @@ and this library adheres to Rust's notion of
 <!-- next-header -->
 ## [Unreleased]
 
+### Fixed
+- `schemerz::Migrator::{register, register_multiple}` can now register dependent
+  migrations before their dependencies. Previously this would result in a graph
+  with missing edges, leading to some migrations not being applied.
+- `schemerz::Migrator::{up, down}` now reliably run migrations in the correct
+  order.
+
 ### Added
 - `schemerz::test_schemerz_adapter`
+  - Compared to the previous `test_schemer_adapter`, this requires an additional
+    argument with an infinite iterator of valid indices for the tests to use.
+- Blanket implementation of `schemerz::Migration` for the following types:
+  - `Box<T>`
+  - `Rc<T>`
+  - `Arc<T>`
+
+### Changed
+- `schemerz::Migrator` no longer uses `Box` in its API:
+  - `schemerz::Adapter::MigrationType` must now be `Sized`.
+  - `schemerz::Migrator::{register, register_multiple}` now take the migrations
+    without a `Box` wrapper`.
+  - `schemerz::TestAdapter::mock` now returns the migration without a `Box`
+    wrapper.
+- `schemerz::Migration` is now generic over its index type, to make writing
+  tests easier (as they can now use an alternative index type like `usize`).
+  Production migrations should still use `uuid::Uuid` for resiliency.
+  - The following traits and structs now have a generic parameter `I`:
+    - `Migration`
+    - `Adapter`
+    - `testing::TestAdapter`
+    - `testing::TestMigration`
+  - The `Migrator` struct now has an `I: Clone + Display + Hash + Eq` parameter.
+  - The following methods now take `I` as an argument instead of `uuid::Uuid`:
+    - `Migrator::up`
+    - `Migrator::down`
+  - The return types of the following methods now involve `I` instead of
+    `uuid::Uuid`:
+    - `Migration::id`
+    - `Migration::dependencies`
+    - `Adapter::applied_migrations`
+    - `testing::TestAdapter::mock`
+    - `testing::TestMigration::new`
+  - The `schemerz::{DependencyError, MigratorError>` enums now have a generic
+    parameter `I` that replaces uses of `uuid::Uuid`.
+  - The `schemerz::migration` macro now supports an optional initial argument
+    with the index type (which defaults to `uuid::Uuid`).
+  - The individual tests in the `schemerz::testing` module now require an
+    adapter with an `I: Clone + FromStr + Debug + Display + Hash + Eq` bound,
+    and each take an additional argument `T: Iterator<Item = I>`.
+- `schemerz::Migrator::register_multiple` now takes an iterator of migrations
+  instead of a `Vec`.
 
 ### Removed
 - `schemerz::test_schemer_adapter` (use `test_schemerz_adapter` instead).

schemerz/CHANGELOG.md

@@ -19,3 +19,4 @@ daggy = "0.8"
 log = "0.4"
 thiserror = "1.0"
 uuid = { version = "1" }
+indexmap = "1"