Migrations fixes

sqlx-core/src/migrate/migrator.rs

@@ -5,11 +5,21 @@ use std::collections::{HashMap, HashSet};
 use std::ops::Deref;
 use std::slice;
 
+/// A resolved set of migrations, ready to be run.
+///
+/// Can be constructed statically using `migrate!()` or at runtime using [`Migrator::new()`].
 #[derive(Debug)]
-#[doc(hidden)]
+// Forbids `migrate!()` from constructing this:
+// #[non_exhaustive]
 pub struct Migrator {
+    // NOTE: these fields are semver-exempt and may be changed or removed in any future version.
+    // These have to be public for `migrate!()` to be able to initialize them in an implicitly
+    // const-promotable context. A `const fn` constructor isn't implicitly const-promotable.
+    #[doc(hidden)]
     pub migrations: Cow<'static, [Migration]>,
+    #[doc(hidden)]
     pub ignore_missing: bool,
+    #[doc(hidden)]
     pub locking: bool,
 }
 
@@ -33,6 +43,13 @@ fn validate_applied_migrations(
 }
 
 impl Migrator {
+    #[doc(hidden)]
+    pub const DEFAULT: Migrator = Migrator {
+        migrations: Cow::Borrowed(&[]),
+        ignore_missing: false,
+        locking: true,
+    };
+
     /// Creates a new instance with the given source.
     ///
     /// # Examples
@@ -57,8 +74,7 @@ impl Migrator {
     {
         Ok(Self {
             migrations: Cow::Owned(source.resolve().await.map_err(MigrateError::Source)?),
-            ignore_missing: false,
-            locking: true,
+            ..Self::DEFAULT
         })
     }
 
@@ -68,7 +84,7 @@ impl Migrator {
         self
     }
 
-    /// Specify whether or not to lock database during migration. Defaults to `true`.
+    /// Specify whether or not to lock the database during migration. Defaults to `true`.
     ///
     /// ### Warning
     /// Disabling locking can lead to errors or data loss if multiple clients attempt to apply migrations simultaneously

sqlx-core/src/migrate/mod.rs

@@ -12,3 +12,6 @@ pub use migration::{AppliedMigration, Migration};
 pub use migration_type::MigrationType;
 pub use migrator::Migrator;
 pub use source::MigrationSource;
+
+#[doc(hidden)]
+pub use source::resolve_blocking;

sqlx-core/src/migrate/source.rs

@@ -1,10 +1,11 @@
 use crate::error::BoxDynError;
-use crate::fs;
 use crate::migrate::{Migration, MigrationType};
 use futures_core::future::BoxFuture;
 
 use std::borrow::Cow;
 use std::fmt::Debug;
+use std::fs;
+use std::io;
 use std::path::{Path, PathBuf};
 
 /// In the default implementation, a MigrationSource is a directory which
@@ -28,51 +29,11 @@ pub trait MigrationSource<'s>: Debug {
 impl<'s> MigrationSource<'s> for &'s Path {
     fn resolve(self) -> BoxFuture<'s, Result<Vec<Migration>, BoxDynError>> {
         Box::pin(async move {
-            let mut s = fs::read_dir(self.canonicalize()?).await?;
-            let mut migrations = Vec::new();
-
-            while let Some(entry) = s.next().await? {
-                // std::fs::metadata traverses symlinks
-                if !std::fs::metadata(&entry.path)?.is_file() {
-                    // not a file; ignore
-                    continue;
-                }
-
-                let file_name = entry.file_name.to_string_lossy();
-
-                let parts = file_name.splitn(2, '_').collect::<Vec<_>>();
-
-                if parts.len() != 2 || !parts[1].ends_with(".sql") {
-                    // not of the format: <VERSION>_<DESCRIPTION>.sql; ignore
-                    continue;
-                }
-
-                let version: i64 = parts[0].parse()
-                    .map_err(|_e| {
-                        format!("error parsing migration filename {file_name:?}; expected integer version prefix (e.g. `01_foo.sql`)")
-                    })?;
-
-                let migration_type = MigrationType::from_filename(parts[1]);
-                // remove the `.sql` and replace `_` with ` `
-                let description = parts[1]
-                    .trim_end_matches(migration_type.suffix())
-                    .replace('_', " ")
-                    .to_owned();
-
-                let sql = fs::read_to_string(&entry.path).await?;
-
-                migrations.push(Migration::new(
-                    version,
-                    Cow::Owned(description),
-                    migration_type,
-                    Cow::Owned(sql),
-                ));
-            }
-
-            // ensure that we are sorted by `VERSION ASC`
-            migrations.sort_by_key(|m| m.version);
-
-            Ok(migrations)
+            let canonical = self.canonicalize()?;
+            let migrations_with_paths =
+                crate::rt::spawn_blocking(move || resolve_blocking(canonical)).await?;
+
+            Ok(migrations_with_paths.into_iter().map(|(m, _p)| m).collect())
         })
     }
 }
@@ -82,3 +43,98 @@ impl MigrationSource<'static> for PathBuf {
         Box::pin(async move { self.as_path().resolve().await })
     }
 }
+
+#[derive(thiserror::Error, Debug)]
+#[error("{message}")]
+pub struct ResolveError {
+    message: String,
+    #[source]
+    source: Option<io::Error>,
+}
+
+// FIXME: paths should just be part of `Migration` but we can't add a field backwards compatibly
+// since it's `#[non_exhaustive]`.
+pub fn resolve_blocking(path: PathBuf) -> Result<Vec<(Migration, PathBuf)>, ResolveError> {
+    let mut s = fs::read_dir(&path).map_err(|e| ResolveError {
+        message: format!("error reading migration directory {}: {e}", path.display()),
+        source: Some(e),
+    })?;
+
+    let mut migrations = Vec::new();
+
+    while let Some(res) = s.next() {
+        let entry = res.map_err(|e| ResolveError {
+            message: format!(
+                "error reading contents of migration directory {}: {e}",
+                path.display()
+            ),
+            source: Some(e),
+        })?;
+
+        let entry_path = entry.path();
+
+        let metadata = fs::metadata(&entry_path).map_err(|e| ResolveError {
+            message: format!(
+                "error getting metadata of migration path {}",
+                entry_path.display()
+            ),
+            source: Some(e),
+        })?;
+
+        if !metadata.is_file() {
+            // not a file; ignore
+            continue;
+        }
+
+        let file_name = entry.file_name();
+        // This is arguably the wrong choice,
+        // but it really only matters for parsing the version and description.
+        //
+        // Using `.to_str()` and returning an error if the filename is not UTF-8
+        // would be a breaking change.
+        let file_name = file_name.to_string_lossy();
+
+        let parts = file_name.splitn(2, '_').collect::<Vec<_>>();
+
+        if parts.len() != 2 || !parts[1].ends_with(".sql") {
+            // not of the format: <VERSION>_<DESCRIPTION>.sql; ignore
+            continue;
+        }
+
+        let version: i64 = parts[0].parse()
+            .map_err(|_e| ResolveError {
+                message: format!("error parsing migration filename {file_name:?}; expected integer version prefix (e.g. `01_foo.sql`)"),
+                source: None,
+            })?;
+
+        let migration_type = MigrationType::from_filename(parts[1]);
+        // remove the `.sql` and replace `_` with ` `
+        let description = parts[1]
+            .trim_end_matches(migration_type.suffix())
+            .replace('_', " ")
+            .to_owned();
+
+        let sql = fs::read_to_string(&entry_path).map_err(|e| ResolveError {
+            message: format!(
+                "error reading contents of migration {}: {e}",
+                entry_path.display()
+            ),
+            source: Some(e),
+        })?;
+
+        migrations.push((
+            Migration::new(
+                version,
+                Cow::Owned(description),
+                migration_type,
+                Cow::Owned(sql),
+            ),
+            entry_path,
+        ));
+    }
+
+    // Ensure that we are sorted by version in ascending order.
+    migrations.sort_by_key(|(m, _)| m.version);
+
+    Ok(migrations)
+}