Repository: launchbadge/sqlx
File: sqlx-core/src/migrate/migrate.rs (Rust)

Code before:
use crate::error::Error;
use crate::migrate::{AppliedMigration, MigrateError, Migration};
use futures_core::future::BoxFuture;
use std::time::Duration;

pub trait MigrateDatabase {
    // create database in url
    // uses a maintenance database depending on driver
    fn create_database(url: &str) -> BoxFuture<'_, Result<(), Error>>;

    // check if the database in url exists
    // uses a maintenance database depending on driver
    fn database_exists(url: &str) -> BoxFuture<'_, Result<bool, Error>>;

    // drop database in url
    // uses a maintenance database depending on driver
    fn drop_database(url: &str, _force: bool) -> BoxFuture<'_, Result<(), Error>>;
}

// 'e = Executor
pub trait Migrate {
    // ensure migrations table exists
    // will create or migrate it if needed
    fn ensure_migrations_table(&mut self) -> BoxFuture<'_, Result<(), MigrateError>>;

    // Return the version on which the database is dirty or None otherwise.
    // "dirty" means there is a partially applied migration that failed.
    fn dirty_version(&mut self) -> BoxFuture<'_, Result<Option<i64>, MigrateError>>;

    // Return the ordered list of applied migrations
    fn list_applied_migrations(
        &mut self,
    ) -> BoxFuture<'_, Result<Vec<AppliedMigration>, MigrateError>>;

    // Should acquire a database lock so that only one migration process
    // can run at a time. [`Migrate`] will call this function before applying
    // any migrations.
    fn lock(&mut self) -> BoxFuture<'_, Result<(), MigrateError>>;

    // Should release the lock. [`Migrate`] will call this function after all
    // migrations have been run.
    fn unlock(&mut self) -> BoxFuture<'_, Result<(), MigrateError>>;

Review comment (bug):
This is unfortunately a breaking change which would have to wait for 0.8.0.

A backwards-compatible addition would be to add a new method with a default implementation:

```suggestion
    fn drop_database(url: &str) -> BoxFuture<'_, Result<(), Error>>;
    
    fn force_drop_database(url: &str) -> BoxFuture<'_, Result<(), Error>> {
        unimplemented!("Database driver does not support force-dropping a database")
    }
```

Instead of panicking, however, you could add a new variant to `MigrateError`, something like `ForceNotSupported`. That's backwards-compatible since the enum is `#[non_exhaustive]`.

Alternatively, it could just default to calling `.drop_database()` but I feel like if someone passed `--force` and it still failed, they might be confused and/or annoyed.

Code after:
use crate::error::Error;
use crate::migrate::{AppliedMigration, MigrateError, Migration};
use futures_core::future::BoxFuture;
use std::time::Duration;

pub trait MigrateDatabase {
    // create database in url
    // uses a maintenance database depending on driver
    fn create_database(url: &str) -> BoxFuture<'_, Result<(), Error>>;

    // check if the database in url exists
    // uses a maintenance database depending on driver
    fn database_exists(url: &str) -> BoxFuture<'_, Result<bool, Error>>;

    // drop database in url
    // uses a maintenance database depending on driver
    fn drop_database(url: &str) -> BoxFuture<'_, Result<(), Error>>;

    // force drop database in url
    // uses a maintenance database depending on driver
    fn force_drop_database(_url: &str) -> BoxFuture<'_, Result<(), Error>> {
        Box::pin(async { Err(MigrateError::ForceNotSupported)? })
    }
}

// 'e = Executor
pub trait Migrate {
    // ensure migrations table exists
    // will create or migrate it if needed
    fn ensure_migrations_table(&mut self) -> BoxFuture<'_, Result<(), MigrateError>>;

    // Return the version on which the database is dirty or None otherwise.
    // "dirty" means there is a partially applied migration that failed.
    fn dirty_version(&mut self) -> BoxFuture<'_, Result<Option<i64>, MigrateError>>;

    // Return the ordered list of applied migrations
    fn list_applied_migrations(
        &mut self,
    ) -> BoxFuture<'_, Result<Vec<AppliedMigration>, MigrateError>>;

    // Should acquire a database lock so that only one migration process
    // can run at a time. [`Migrate`] will call this function before applying