Set mode for Database instead of CollectionDatabase

crates/cli/src/commands/collections.rs

@@ -1,6 +1,6 @@
 use crate::{GlobalArgs, Subcommand};
 use clap::Parser;
-use slumber_core::db::Database;
+use slumber_core::db::{Database, DatabaseMode};
 use std::{path::PathBuf, process::ExitCode};
 
 /// View and modify request collection metadata
@@ -29,14 +29,15 @@ enum CollectionsSubcommand {
 
 impl Subcommand for CollectionsCommand {
     async fn execute(self, _global: GlobalArgs) -> anyhow::Result<ExitCode> {
-        let database = Database::load()?;
         match self.subcommand {
             CollectionsSubcommand::List => {
+                let database = Database::load(DatabaseMode::ReadOnly)?;
                 for path in database.collections()? {
                     println!("{}", path.display());
                 }
             }
             CollectionsSubcommand::Migrate { from, to } => {
+                let database = Database::load(DatabaseMode::ReadWrite)?;
                 database.merge_collections(&from, &to)?;
                 println!("Migrated {} into {}", from.display(), to.display());
             }

crates/cli/src/commands/history.rs

@@ -86,20 +86,16 @@ impl Subcommand for HistoryCommand {
     async fn execute(self, global: GlobalArgs) -> anyhow::Result<ExitCode> {
         match self.subcommand {
             HistorySubcommand::List { recipe, profile } => {
-                let database = Database::load()?.into_collection(
-                    &global.collection_path()?,
-                    DatabaseMode::ReadOnly,
-                )?;
+                let database = Database::load(DatabaseMode::ReadOnly)?
+                    .into_collection(&global.collection_path()?)?;
                 let exchanges =
                     database.get_recipe_requests(profile.into(), &recipe)?;
                 print_list(exchanges);
             }
 
             HistorySubcommand::Get { request, display } => {
-                let database = Database::load()?.into_collection(
-                    &global.collection_path()?,
-                    DatabaseMode::ReadOnly,
-                )?;
+                let database = Database::load(DatabaseMode::ReadOnly)?
+                    .into_collection(&global.collection_path()?)?;
                 let exchange = match request {
                     RecipeOrRequest::Recipe(recipe_id) => database
                         .get_latest_request(ProfileFilter::All, &recipe_id)?
@@ -151,28 +147,21 @@ impl Subcommand for HistoryCommand {
                 }
 
                 // Do the deletion
+                let database = Database::load(DatabaseMode::ReadWrite)?;
                 let deleted = match selection {
-                    RequestSelection::All => {
-                        let database = Database::load()?;
-                        database.delete_all_requests()?
-                    }
+                    RequestSelection::All => database.delete_all_requests()?,
                     RequestSelection::Collection => {
-                        let database = Database::load()?.into_collection(
-                            &global.collection_path()?,
-                            DatabaseMode::ReadWrite,
-                        )?;
+                        let database = database
+                            .into_collection(&global.collection_path()?)?;
                         database.delete_all_requests()?
                     }
                     RequestSelection::Recipe { recipe, profile } => {
-                        let database = Database::load()?.into_collection(
-                            &global.collection_path()?,
-                            DatabaseMode::ReadWrite,
-                        )?;
+                        let database = database
+                            .into_collection(&global.collection_path()?)?;
                         database
                             .delete_recipe_requests(profile.into(), &recipe)?
                     }
                     RequestSelection::Request { request } => {
-                        let database = Database::load()?;
                         database.delete_request(request)?
                     }
                 };
@@ -223,6 +212,8 @@ enum RequestSelection {
         // None -> All profiles
         // Some(None) -> No profile
         // Some(Some("profile1")) -> profile1
+        // It'd be nice if we could load directly into ProfileFilter, but I
+        // couldn't figure out how to set that up with clap
         profile: Option<Option<ProfileId>>,
     },
     /// Select a single request by ID

crates/cli/src/commands/request.rs

@@ -158,8 +158,8 @@ impl BuildRequestCommand {
         // Open DB in readonly. Storing requests in history from the CLI isn't
         // really intuitive, and could have a large perf impact for scripting
         // and large responses
-        let database = Database::load()?
-            .into_collection(&collection_path, DatabaseMode::ReadOnly)?;
+        let database = Database::load(DatabaseMode::ReadOnly)?
+            .into_collection(&collection_path)?;
         let http_engine = HttpEngine::new(&config.http);
 
         // Validate profile ID, so we can provide a good error if it's invalid

crates/cli/src/completions.rs

@@ -9,7 +9,7 @@
 use clap_complete::CompletionCandidate;
 use slumber_core::{
     collection::{Collection, CollectionFile, ProfileId},
-    db::Database,
+    db::{Database, DatabaseMode},
 };
 use std::{ffi::OsStr, ops::Deref};
 
@@ -43,7 +43,7 @@ pub fn complete_recipe(current: &OsStr) -> Vec<CompletionCandidate> {
 
 /// Provide completions for request IDs
 pub fn complete_request_id(current: &OsStr) -> Vec<CompletionCandidate> {
-    let Ok(database) = Database::load() else {
+    let Ok(database) = Database::load(DatabaseMode::ReadOnly) else {
         return Vec::new();
     };
     let Ok(exchanges) = database.get_all_requests() else {

crates/core/src/db.rs

@@ -47,16 +47,18 @@ pub struct Database {
     /// one connection per thread, but the code would be a bit more
     /// complicated.
     connection: Arc<Mutex<Connection>>,
-    // TODO set mode here?
+    /// Read-only or read-write? We use a read-only DB in the CLI to indicate
+    /// to the HTTP engine that requests shouldn't be stored
+    mode: DatabaseMode,
 }
 
 impl Database {
     const FILE: &'static str = "state.sqlite";
 
     /// Load the database. This will perform migrations, but can be called from
     /// anywhere in the app. The migrations will run on first connection, and
-    /// not after that.
-    pub fn load() -> anyhow::Result<Self> {
+    /// not after that. Migrations WILL run on a read-only database!
+    pub fn load(mode: DatabaseMode) -> anyhow::Result<Self> {
         let path = Self::path();
         paths::create_parent(&path)?;
 
@@ -72,6 +74,7 @@ impl Database {
         Self::migrate(&mut connection)?;
         Ok(Self {
             connection: Arc::new(Mutex::new(connection)),
+            mode,
         })
     }
 
@@ -103,6 +106,16 @@ impl Database {
             .context("Error extracting collection data")
     }
 
+    /// Return an error if we are in read-only mode
+    fn ensure_write(&self) -> anyhow::Result<()> {
+        match self.mode {
+            DatabaseMode::ReadOnly => {
+                Err(anyhow!("Database in read-only mode"))
+            }
+            DatabaseMode::ReadWrite => Ok(()),
+        }
+    }
+
     /// Migrate all data for one collection into another, deleting the source
     /// collection
     pub fn merge_collections(
@@ -135,6 +148,7 @@ impl Database {
                 .traced()
         }
 
+        self.ensure_write()?;
         info!(?source, ?target, "Merging database state");
         let connection = self.connection();
 
@@ -204,6 +218,7 @@ impl Database {
         request_id: RequestId,
     ) -> anyhow::Result<usize> {
         info!(%request_id, "Deleting request");
+        self.ensure_write()?;
         self.connection()
             .execute(
                 "DELETE FROM requests_v2 WHERE id = :request_id",
@@ -219,7 +234,6 @@ impl Database {
     pub fn into_collection(
         self,
         path: &Path,
-        mode: DatabaseMode,
     ) -> anyhow::Result<CollectionDatabase> {
         // Convert to canonicalize and make serializable
         let path: CollectionPath = path.try_into()?;
@@ -250,7 +264,6 @@ impl Database {
         Ok(CollectionDatabase {
             collection_id,
             database: self,
-            mode,
         })
     }
 }
@@ -262,7 +275,6 @@ impl Database {
 pub struct CollectionDatabase {
     collection_id: CollectionId,
     database: Database,
-    mode: DatabaseMode,
 }
 
 impl CollectionDatabase {
@@ -282,16 +294,7 @@ impl CollectionDatabase {
 
     /// Is read/write mode enabled for this database?
     pub fn can_write(&self) -> bool {
-        self.mode == DatabaseMode::ReadWrite
-    }
-
-    /// Return an error if we are in read-only mode
-    fn ensure_write(&self) -> anyhow::Result<()> {
-        if self.can_write() {
-            Ok(())
-        } else {
-            Err(anyhow!("Database in read-only mode"))
-        }
+        self.database.mode == DatabaseMode::ReadWrite
     }
 
     /// Get a request by ID, or `None` if it does not exist in history.
@@ -413,7 +416,7 @@ impl CollectionDatabase {
     /// requests that failed to complete (e.g. because of a network error)
     /// should not (and cannot) be stored.
     pub fn insert_exchange(&self, exchange: &Exchange) -> anyhow::Result<()> {
-        self.ensure_write()?;
+        self.database.ensure_write()?;
 
         debug!(
             id = %exchange.id,
@@ -486,7 +489,7 @@ impl CollectionDatabase {
     /// Delete all exchanges for this collection. Return the number of deleted
     /// requests
     pub fn delete_all_requests(&self) -> anyhow::Result<usize> {
-        self.ensure_write()?;
+        self.database.ensure_write()?;
         info!(
             collection_id = %self.collection_id,
             collection_path = ?self.collection_path(),
@@ -509,7 +512,7 @@ impl CollectionDatabase {
         profile_id: ProfileFilter,
         recipe_id: &RecipeId,
     ) -> anyhow::Result<usize> {
-        self.ensure_write()?;
+        self.database.ensure_write()?;
         info!(
             collection_id = %self.collection_id,
             collection_path = ?self.collection_path(),
@@ -586,7 +589,7 @@ impl CollectionDatabase {
         K: Debug + Serialize,
         V: Debug + Serialize,
     {
-        self.ensure_write()?;
+        self.database.ensure_write()?;
 
         debug!(?key, ?value, "Setting UI state");
         self.database
@@ -626,19 +629,25 @@ impl CollectionId {
     }
 }
 
-/// Create an in-memory DB, only for testing
 #[cfg(any(test, feature = "test"))]
 impl crate::test_util::Factory for Database {
     fn factory(_: ()) -> Self {
+        Self::factory(DatabaseMode::ReadWrite)
+    }
+}
+
+#[cfg(any(test, feature = "test"))]
+impl crate::test_util::Factory<DatabaseMode> for Database {
+    fn factory(mode: DatabaseMode) -> Self {
         let mut connection = Connection::open_in_memory().unwrap();
         Self::migrate(&mut connection).unwrap();
         Self {
             connection: Arc::new(Mutex::new(connection)),
+            mode,
         }
     }
 }
 
-/// Create an in-memory DB, only for testing
 #[cfg(any(test, feature = "test"))]
 impl crate::test_util::Factory for CollectionDatabase {
     fn factory(_: ()) -> Self {
@@ -650,8 +659,8 @@ impl crate::test_util::Factory for CollectionDatabase {
 impl crate::test_util::Factory<DatabaseMode> for CollectionDatabase {
     fn factory(mode: DatabaseMode) -> Self {
         use crate::util::paths::get_repo_root;
-        Database::factory(())
-            .into_collection(&get_repo_root().join("slumber.yml"), mode)
+        Database::factory(mode)
+            .into_collection(&get_repo_root().join("slumber.yml"))
             .expect("Error initializing DB collection")
     }
 }

crates/core/src/db/tests.rs

@@ -38,13 +38,11 @@ fn request_db(
     other_collection_path: PathBuf,
 ) -> RequestDb {
     let database = Database::factory(());
-    let collection1 = database
-        .clone()
-        .into_collection(&collection_path, DatabaseMode::ReadWrite)
-        .unwrap();
+    let collection1 =
+        database.clone().into_collection(&collection_path).unwrap();
     let collection2 = database
         .clone()
-        .into_collection(&other_collection_path, DatabaseMode::ReadWrite)
+        .into_collection(&other_collection_path)
         .unwrap();
 
     // We separate requests by 3 columns. Create multiple of each column to
@@ -95,13 +93,11 @@ struct RequestDb {
 #[rstest]
 fn test_merge(collection_path: PathBuf, other_collection_path: PathBuf) {
     let database = Database::factory(());
-    let collection1 = database
-        .clone()
-        .into_collection(&collection_path, DatabaseMode::ReadWrite)
-        .unwrap();
+    let collection1 =
+        database.clone().into_collection(&collection_path).unwrap();
     let collection2 = database
         .clone()
-        .into_collection(&other_collection_path, DatabaseMode::ReadWrite)
+        .into_collection(&other_collection_path)
         .unwrap();
 
     let exchange1 =
@@ -338,13 +334,11 @@ fn test_delete_request(request_db: RequestDb) {
 #[rstest]
 fn test_ui_state(collection_path: PathBuf) {
     let database = Database::factory(());
-    let collection1 = database
-        .clone()
-        .into_collection(&collection_path, DatabaseMode::ReadWrite)
-        .unwrap();
+    let collection1 =
+        database.clone().into_collection(&collection_path).unwrap();
     let collection2 = database
         .clone()
-        .into_collection(Path::new("Cargo.toml"), DatabaseMode::ReadWrite)
+        .into_collection(Path::new("Cargo.toml"))
         .unwrap();
 
     let key_type = "MyKey";

crates/tui/src/lib.rs

@@ -100,8 +100,8 @@ impl Tui {
         // to default, just show an error to the user
         let config = Config::load().reported(&messages_tx).unwrap_or_default();
         // Load a database for this particular collection
-        let database = Database::load()?
-            .into_collection(&collection_path, DatabaseMode::ReadWrite)?;
+        let database = Database::load(DatabaseMode::ReadWrite)?
+            .into_collection(&collection_path)?;
         // Initialize global view context
         TuiContext::init(config);