Skip to content

Commit

Permalink
Don't store CLI requests in history
Browse files Browse the repository at this point in the history
Storing requests is probably unintuitive because you wouldn't really expect CLI requests to appear in the TUI history menu. The CLI also is going to handle more bulk and large responses because of scripting.
  • Loading branch information
LucasPickering committed Dec 8, 2024
1 parent bae4982 commit e70a62f
Show file tree
Hide file tree
Showing 6 changed files with 92 additions and 17 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,10 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/),

## [Unreleased] - ReleaseDate

### Breaking

- Don't store CLI requests in history

### Changes

- Wrap long error messages in response pane
Expand Down
5 changes: 3 additions & 2 deletions crates/cli/src/commands/history.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ use clap_complete::ArgValueCompleter;
use dialoguer::console::Style;
use slumber_core::{
collection::{CollectionFile, ProfileId, RecipeId},
db::Database,
db::{Database, DatabaseMode},
http::{Exchange, ExchangeSummary, RequestId},
util::{format_byte_size, format_duration, format_time, MaybeStr},
};
Expand Down Expand Up @@ -62,7 +62,8 @@ impl Subcommand for HistoryCommand {
it may change or be removed at any time"
);
let collection_path = CollectionFile::try_path(None, global.file)?;
let database = Database::load()?.into_collection(&collection_path)?;
let database = Database::load()?
.into_collection(&collection_path, DatabaseMode::ReadOnly)?;

match self.subcommand {
HistorySubcommand::List { recipe, profile } => {
Expand Down
10 changes: 7 additions & 3 deletions crates/cli/src/commands/request.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ use itertools::Itertools;
use slumber_config::Config;
use slumber_core::{
collection::{Collection, CollectionFile, ProfileId, RecipeId},
db::{CollectionDatabase, Database},
db::{CollectionDatabase, Database, DatabaseMode},
http::{BuildOptions, HttpEngine, RequestSeed, RequestTicket},
template::{Prompt, Prompter, Select, TemplateContext, TemplateError},
util::ResultTraced,
Expand Down Expand Up @@ -163,9 +163,13 @@ impl BuildRequestCommand {
trigger_dependencies: bool,
) -> anyhow::Result<(CollectionDatabase, RequestTicket)> {
let collection_path = CollectionFile::try_path(None, global.file)?;
let database = Database::load()?.into_collection(&collection_path)?;
let collection = Collection::load(&collection_path)?;
let config = Config::load()?;
let collection = Collection::load(&collection_path)?;
// 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 http_engine = HttpEngine::new(&config.http);

// Validate profile ID, so we can provide a good error if it's invalid
Expand Down
79 changes: 71 additions & 8 deletions crates/core/src/db.rs
Original file line number Diff line number Diff line change
Expand Up @@ -175,6 +175,7 @@ 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()?;
Expand Down Expand Up @@ -205,6 +206,7 @@ impl Database {
Ok(CollectionDatabase {
collection_id,
database: self,
mode,
})
}
}
Expand All @@ -216,6 +218,7 @@ impl Database {
pub struct CollectionDatabase {
collection_id: CollectionId,
database: Database,
mode: DatabaseMode,
}

impl CollectionDatabase {
Expand All @@ -233,6 +236,20 @@ impl CollectionDatabase {
.map(PathBuf::from)
}

/// 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"))
}
}

/// Get a request by ID, or `None` if it does not exist in history.
pub fn get_request(
&self,
Expand Down Expand Up @@ -307,6 +324,8 @@ 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()?;

debug!(
id = %exchange.id,
url = %exchange.request.url,
Expand Down Expand Up @@ -453,6 +472,8 @@ impl CollectionDatabase {
K: Debug + Serialize,
V: Debug + Serialize,
{
self.ensure_write()?;

debug!(?key, ?value, "Setting UI state");
self.database
.connection()
Expand Down Expand Up @@ -507,17 +528,31 @@ impl crate::test_util::Factory for Database {
#[cfg(any(test, feature = "test"))]
impl crate::test_util::Factory for CollectionDatabase {
fn factory(_: ()) -> Self {
Self::factory(DatabaseMode::ReadWrite)
}
}

#[cfg(any(test, feature = "test"))]
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"))
.into_collection(&get_repo_root().join("slumber.yml"), mode)
.expect("Error initializing DB collection")
}
}

/// Is the database read-only or read/write?
#[derive(Copy, Clone, Debug, PartialEq)]
pub enum DatabaseMode {
ReadOnly,
ReadWrite,
}

#[cfg(test)]
mod tests {
use super::*;
use crate::{test_util::Factory, util::paths::get_repo_root};
use crate::{assert_err, test_util::Factory, util::paths::get_repo_root};
use itertools::Itertools;
use std::collections::HashMap;

Expand All @@ -526,8 +561,14 @@ mod tests {
let database = Database::factory(());
let path1 = get_repo_root().join("slumber.yml");
let path2 = get_repo_root().join("README.md"); // Has to be a real file
let collection1 = database.clone().into_collection(&path1).unwrap();
let collection2 = database.clone().into_collection(&path2).unwrap();
let collection1 = database
.clone()
.into_collection(&path1, DatabaseMode::ReadWrite)
.unwrap();
let collection2 = database
.clone()
.into_collection(&path2, DatabaseMode::ReadWrite)
.unwrap();

let exchange1 =
Exchange::factory((Some("profile1".into()), "recipe1".into()));
Expand Down Expand Up @@ -598,11 +639,17 @@ mod tests {
let database = Database::factory(());
let collection1 = database
.clone()
.into_collection(&get_repo_root().join("slumber.yml"))
.into_collection(
&get_repo_root().join("slumber.yml"),
DatabaseMode::ReadWrite,
)
.unwrap();
let collection2 = database
.clone()
.into_collection(&get_repo_root().join("README.md"))
.into_collection(
&get_repo_root().join("README.md"),
DatabaseMode::ReadWrite,
)
.unwrap();

let exchange2 = Exchange::factory(());
Expand Down Expand Up @@ -736,11 +783,14 @@ mod tests {
let database = Database::factory(());
let collection1 = database
.clone()
.into_collection(Path::new("../../slumber.yml"))
.into_collection(
Path::new("../../slumber.yml"),
DatabaseMode::ReadWrite,
)
.unwrap();
let collection2 = database
.clone()
.into_collection(Path::new("Cargo.toml"))
.into_collection(Path::new("Cargo.toml"), DatabaseMode::ReadWrite)
.unwrap();

let key_type = "MyKey";
Expand All @@ -757,4 +807,17 @@ mod tests {
Some("value2".into())
);
}

#[test]
fn test_readonly_mode() {
let database = CollectionDatabase::factory(DatabaseMode::ReadOnly);
assert_err!(
database.insert_exchange(&Exchange::factory(())),
"Database in read-only mode"
);
assert_err!(
database.set_ui("MyKey", "key1", "value1"),
"Database in read-only mode"
);
}
}
6 changes: 4 additions & 2 deletions crates/core/src/http.rs
Original file line number Diff line number Diff line change
Expand Up @@ -380,8 +380,10 @@ impl RequestTicket {
end_time,
};

// Error here should *not* kill the request
let _ = database.insert_exchange(&exchange);
if database.can_write() {
// Error here should *not* kill the request
let _ = database.insert_exchange(&exchange);
}
Ok(exchange)
}

Expand Down
5 changes: 3 additions & 2 deletions crates/tui/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ use ratatui::{prelude::CrosstermBackend, Terminal};
use slumber_config::{Action, Config};
use slumber_core::{
collection::{Collection, CollectionFile, ProfileId},
db::{CollectionDatabase, Database},
db::{CollectionDatabase, Database, DatabaseMode},
http::{RequestId, RequestSeed},
template::{Prompter, Template, TemplateChunk, TemplateContext},
};
Expand Down Expand Up @@ -101,7 +101,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)?;
let database = Database::load()?
.into_collection(&collection_path, DatabaseMode::ReadWrite)?;
// Initialize global view context
TuiContext::init(config);

Expand Down

0 comments on commit e70a62f

Please sign in to comment.