From 28927e0bc3e1a8b2ad753a608d378c8b22570e66 Mon Sep 17 00:00:00 2001 From: Lucas Pickering Date: Wed, 4 Dec 2024 20:23:18 -0500 Subject: [PATCH] Replace JSONPath queries with commands --- Cargo.lock | 2 +- Cargo.toml | 1 - crates/core/Cargo.toml | 2 +- crates/core/src/db/convert.rs | 4 +- crates/core/src/db/migrations.rs | 4 +- crates/core/src/http.rs | 4 +- crates/core/src/http/content_type.rs | 10 - crates/core/src/http/models.rs | 76 +--- crates/core/src/template.rs | 6 +- crates/core/src/template/render.rs | 5 +- crates/tui/Cargo.toml | 2 +- crates/tui/src/http.rs | 142 ++----- crates/tui/src/lib.rs | 24 +- crates/tui/src/message.rs | 12 +- crates/tui/src/test_util.rs | 40 +- crates/tui/src/util.rs | 46 +- crates/tui/src/view/common/text_box.rs | 12 +- .../tui/src/view/component/queryable_body.rs | 394 +++++++----------- .../tui/src/view/component/response_view.rs | 95 ++--- crates/tui/src/view/component/root.rs | 14 +- 20 files changed, 321 insertions(+), 574 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 8ecb71e4..3a48b133 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2479,8 +2479,8 @@ dependencies = [ "reqwest", "rstest", "serde", - "serde_json_path", "serde_yaml", + "shellish_parse", "slumber_config", "slumber_core", "strum", diff --git a/Cargo.toml b/Cargo.toml index a144cce2..0a48da69 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -33,7 +33,6 @@ reqwest = {version = "0.12.5", default-features = false} rstest = {version = "0.21.0", default-features = false} serde = {version = "1.0.204", default-features = false} serde_json = {version = "1.0.120", default-features = false, features = ["preserve_order"]} -serde_json_path = "0.7.1" serde_test = "1.0.176" serde_yaml = {version = "0.9.0", default-features = false} slumber_cli = {path = "./crates/cli", version = "2.3.0"} diff --git a/crates/core/Cargo.toml b/crates/core/Cargo.toml index 70c908ac..b10922e7 100644 --- a/crates/core/Cargo.toml +++ b/crates/core/Cargo.toml @@ -29,7 +29,7 @@ rusqlite = {version = "0.31.0", default-features = false, features = ["bundled", rusqlite_migration = "1.2.0" serde = {workspace = true, features = ["derive"]} serde_json = {workspace = true} -serde_json_path = {workspace = true} +serde_json_path = "0.7.1" serde_path_to_error = "0.1.16" serde_yaml = {workspace = true} strum = {workspace = true, features = ["derive"]} diff --git a/crates/core/src/db/convert.rs b/crates/core/src/db/convert.rs index b99e2e6b..afd8ef33 100644 --- a/crates/core/src/db/convert.rs +++ b/crates/core/src/db/convert.rs @@ -209,13 +209,13 @@ impl<'a, 'b> TryFrom<&'a Row<'b>> for Exchange { .get::<_, Option>>("request_body")? .map(|wrap| wrap.0), }), - response: ResponseRecord { + response: Arc::new(ResponseRecord { status: row.get::<_, SqlWrap>("status_code")?.0, headers: row .get::<_, SqlWrap>("response_headers")? .0, body: row.get::<_, SqlWrap>("response_body")?.0.into(), - }, + }), }) } } diff --git a/crates/core/src/db/migrations.rs b/crates/core/src/db/migrations.rs index 7971b1b6..cff751ec 100644 --- a/crates/core/src/db/migrations.rs +++ b/crates/core/src/db/migrations.rs @@ -128,7 +128,7 @@ fn migrate_requests_v2_up(transaction: &Transaction) -> HookResult { end_time: row.get("end_time")?, // Deserialize from bytes request: Arc::new(row.get::<_, ByteEncoded<_>>("request")?.0), - response: row.get::<_, ByteEncoded<_>>("response")?.0, + response: Arc::new(row.get::<_, ByteEncoded<_>>("response")?.0), }; Ok((collection_id, exchange)) } @@ -430,7 +430,7 @@ mod tests { ":start_time": &exchange.start_time, ":end_time": &exchange.end_time, ":request": &ByteEncoded(&*exchange.request), - ":response": &ByteEncoded(&exchange.response), + ":response": &ByteEncoded(&*exchange.response), ":status_code": exchange.response.status.as_u16(), }, ) diff --git a/crates/core/src/http.rs b/crates/core/src/http.rs index bd4a9060..22f935b1 100644 --- a/crates/core/src/http.rs +++ b/crates/core/src/http.rs @@ -375,7 +375,7 @@ impl RequestTicket { let exchange = Exchange { id, request: self.record, - response, + response: response.into(), start_time, end_time, }; @@ -1362,7 +1362,7 @@ mod tests { .to_str() .unwrap(); assert_eq!( - exchange.response, + *exchange.response, ResponseRecord { status: StatusCode::OK, headers: header_map([ diff --git a/crates/core/src/http/content_type.rs b/crates/core/src/http/content_type.rs index 4cb62209..5355c15f 100644 --- a/crates/core/src/http/content_type.rs +++ b/crates/core/src/http/content_type.rs @@ -134,11 +134,6 @@ pub trait ResponseContent: Debug + Display + Send + Sync { where Self: Sized; - /// Prettify a parsed body into something the user will really like. Once - /// a response is parsed, prettification is infallible. Could be slow - /// though! - fn prettify(&self) -> String; - /// Convert the content to JSON. JSON is the common language used for /// querying internally, so everything needs to be convertible to/from JSON. fn to_json(&self) -> Cow<'_, serde_json::Value>; @@ -161,11 +156,6 @@ impl ResponseContent for Json { Ok(Self(serde_json::from_slice(body)?)) } - fn prettify(&self) -> String { - // serde_json can't fail serializing its own Value type - serde_json::to_string_pretty(&self.0).unwrap() - } - fn to_json(&self) -> Cow<'_, serde_json::Value> { Cow::Borrowed(&self.0) } diff --git a/crates/core/src/http/models.rs b/crates/core/src/http/models.rs index 4db8f179..dc6d9002 100644 --- a/crates/core/src/http/models.rs +++ b/crates/core/src/http/models.rs @@ -5,10 +5,7 @@ use crate::{ collection::{Authentication, ProfileId, RecipeBody, RecipeId}, - http::{ - cereal, - content_type::{ContentType, ResponseContent}, - }, + http::{cereal, content_type::ContentType}, template::Template, }; use anyhow::Context; @@ -183,7 +180,7 @@ pub struct Exchange { /// What we said. Use an Arc so the view can hang onto it. pub request: Arc, /// What we heard - pub response: ResponseRecord, + pub response: Arc, /// When was the request sent to the server? pub start_time: DateTime, /// When did we finish receiving the *entire* response? @@ -423,7 +420,7 @@ impl crate::test_util::Factory<(RequestRecord, ResponseRecord)> for Exchange { Self { id: request.id, request: request.into(), - response, + response: response.into(), start_time: Utc::now(), end_time: Utc::now(), } @@ -450,20 +447,10 @@ pub struct ResponseRecord { } impl ResponseRecord { - /// Stored the parsed form of this request's body - pub fn set_parsed_body(&mut self, body: Box) { - self.body.parsed = Some(body); - } - /// Get the content type of the response body, according to the /// `Content-Type` header pub fn content_type(&self) -> Option { - // If we've parsed the body, we'll have the content type present. If - // not, check the header now - self.body - .parsed() - .map(|content| content.content_type()) - .or_else(|| ContentType::from_headers(&self.headers).ok()) + ContentType::from_headers(&self.headers).ok() } /// Get a suggested file name for the content of this response. First we'll @@ -496,64 +483,41 @@ impl ResponseRecord { } } -pub enum ParseMode { - Immediate, - Background { - callback: Box) + Send>, - }, -} - /// HTTP response body. Content is stored as bytes because it may not /// necessarily be valid UTF-8. Converted to text only as needed. +/// +/// The generic type is to make this usable with references to bodies. In most +/// cases you can just use the default. #[derive(Default, Deserialize)] -#[serde(from = "Bytes")] // Can't use into=Bytes because that requires cloning -pub struct ResponseBody { +#[serde(bound = "T: From", from = "Bytes")] // Can't use into=Bytes because that requires cloning +pub struct ResponseBody { /// Raw body - data: Bytes, - /// For responses of a known content type, we can parse the body into a - /// real data structure. This is populated manually; Call - /// [ResponseRecord::parse_body] to set the parsed body. This uses a lock - /// so it can be parsed and populated in a background thread. - #[serde(skip)] - parsed: Option>, + data: T, } -impl ResponseBody { - pub fn new(data: Bytes) -> Self { - Self { - data, - parsed: Default::default(), - } +impl> ResponseBody { + pub fn new(data: T) -> Self { + Self { data } } /// Raw content bytes - pub fn bytes(&self) -> &Bytes { + pub fn bytes(&self) -> &T { &self.data } /// Owned raw content bytes - pub fn into_bytes(self) -> Bytes { + pub fn into_bytes(self) -> T { self.data } /// Get bytes as text, if valid UTF-8 pub fn text(&self) -> Option<&str> { - std::str::from_utf8(&self.data).ok() + std::str::from_utf8(self.data.as_ref()).ok() } /// Get body size, in bytes pub fn size(&self) -> usize { - self.bytes().len() - } - - /// Get the parsed version of this body. Must haved call - /// [ResponseRecord::parse_body] first to actually do the parse. Parsing has - /// to be done on the parent because we don't have access to the - /// `Content-Type` header here, which tells us how to parse. - /// - /// Return `None` if parsing either hasn't happened yet, or failed. - pub fn parsed(&self) -> Option<&dyn ResponseContent> { - self.parsed.as_deref() + self.data.as_ref().len() } } @@ -566,9 +530,9 @@ impl Debug for ResponseBody { } } -impl From for ResponseBody { - fn from(bytes: Bytes) -> Self { - Self::new(bytes) +impl> From for ResponseBody { + fn from(data: Bytes) -> Self { + Self { data: data.into() } } } diff --git a/crates/core/src/template.rs b/crates/core/src/template.rs index ce189285..05f521cf 100644 --- a/crates/core/src/template.rs +++ b/crates/core/src/template.rs @@ -539,7 +539,7 @@ mod tests { response: ResponseRecord { body: "not json!".into(), ..ResponseRecord::factory(()) - }, + }.into(), ..Exchange::factory(RecipeId::from("recipe1")) }), "content type not provided", @@ -561,7 +561,7 @@ mod tests { response: ResponseRecord { body: "not json!".into(), ..ResponseRecord::factory(()) - }, + }.into(), ..Exchange::factory(RecipeId::from("recipe1")) }), "Parsing response: expected ident at line 1 column 2", @@ -583,7 +583,7 @@ mod tests { response: ResponseRecord { body: "[1, 2]".into(), ..ResponseRecord::factory(()) - }, + }.into(), ..Exchange::factory(RecipeId::from("recipe1")) }), "No results from JSONPath query", diff --git a/crates/core/src/template/render.rs b/crates/core/src/template/render.rs index d811a5a0..ddcbc952 100644 --- a/crates/core/src/template/render.rs +++ b/crates/core/src/template/render.rs @@ -539,7 +539,10 @@ impl<'a> ChainTemplateSource<'a> { ChainRequestTrigger::Always => send_request().await?, }; - Ok(exchange.response) + // Safe because we just constructed the exchange and haven't shared it + let response = + Arc::into_inner(exchange.response).expect("Arc never shared"); + Ok(response) } /// Extract the specified component bytes from the response. For headers, diff --git a/crates/tui/Cargo.toml b/crates/tui/Cargo.toml index bbe9057d..51b7cba2 100644 --- a/crates/tui/Cargo.toml +++ b/crates/tui/Cargo.toml @@ -26,8 +26,8 @@ persisted = {version = "0.3.1", features = ["serde"]} ratatui = {workspace = true, features = ["crossterm", "underline-color", "unstable-widget-ref"]} reqwest = {workspace = true} serde = {workspace = true} -serde_json_path = {workspace = true} serde_yaml = {workspace = true} +shellish_parse = "2.2.0" slumber_config = {workspace = true} slumber_core = {workspace = true} strum = {workspace = true} diff --git a/crates/tui/src/http.rs b/crates/tui/src/http.rs index b88a314c..653ea8d0 100644 --- a/crates/tui/src/http.rs +++ b/crates/tui/src/http.rs @@ -1,7 +1,5 @@ //! Types for managing HTTP state in the TUI -use crate::message::{Message, MessageSender}; -use anyhow::{bail, Context}; use chrono::{DateTime, TimeDelta, Utc}; use itertools::Itertools; use reqwest::StatusCode; @@ -9,18 +7,16 @@ use slumber_core::{ collection::{ProfileId, RecipeId}, db::CollectionDatabase, http::{ - content_type::ResponseContent, Exchange, ExchangeSummary, - RequestBuildError, RequestError, RequestId, RequestRecord, - ResponseRecord, + Exchange, ExchangeSummary, RequestBuildError, RequestError, RequestId, + RequestRecord, }, - util::ResultTraced, }; use std::{ collections::{hash_map::Entry, HashMap}, fmt::Debug, sync::Arc, }; -use tokio::task::{self, JoinHandle}; +use tokio::task::JoinHandle; use tracing::{error, warn}; /// Simple in-memory "database" for request state. This serves a few purposes: @@ -39,19 +35,13 @@ use tracing::{error, warn}; pub struct RequestStore { database: CollectionDatabase, requests: HashMap, - /// A helper to parse response bodies. See trait definition - parser: Box, } impl RequestStore { - pub fn new( - database: CollectionDatabase, - parser: impl 'static + ResponseParser, - ) -> Self { + pub fn new(database: CollectionDatabase) -> Self { Self { database, requests: Default::default(), - parser: Box::new(parser), } } @@ -127,7 +117,7 @@ impl RequestStore { /// Mark a request as successful, i.e. we received a response pub fn response(&mut self, exchange: Exchange) { - let response_state = RequestState::response(exchange, &*self.parser); + let response_state = RequestState::response(exchange); // Use replace just to help catch bugs self.replace(response_state.id(), |state| { // This indicates a bug or race condition (e.g. request cancelled as @@ -217,12 +207,10 @@ impl RequestStore { ) -> anyhow::Result> { let request = match self.requests.entry(id) { Entry::Occupied(entry) => Some(entry.into_mut()), - Entry::Vacant(entry) => { - self.database.get_request(id)?.map(|exchange| { - entry - .insert(RequestState::response(exchange, &*self.parser)) - }) - } + Entry::Vacant(entry) => self + .database + .get_request(id)? + .map(|exchange| entry.insert(RequestState::response(exchange))), }; Ok(request.map(|r| &*r)) } @@ -243,9 +231,7 @@ impl RequestStore { .entry(exchange.id) // This is expensive because it parses the body, so avoid it if // the record is already cached - .or_insert_with(|| { - RequestState::response(exchange, &*self.parser) - }); + .or_insert_with(|| RequestState::response(exchange)); } // Now that the know the most recent completed record is in our local @@ -294,23 +280,6 @@ impl RequestStore { Ok(iter) } - /// Set the parsed body for a response. This is part of the callback chain - /// after a body has been parsed in a background task - pub fn set_parsed_body( - &mut self, - id: RequestId, - body: Box, - ) -> anyhow::Result<()> { - let Some(request_state) = self.requests.get_mut(&id) else { - bail!("Request not in store") - }; - let RequestState::Response { exchange } = request_state else { - bail!("Request is not complete") - }; - exchange.response.set_parsed_body(body); - Ok(()) - } - /// Replace a request state in the store with new state, by mapping it /// through a function. This assumes the request state is supposed to be in /// the state, so it logs a message if it isn't (panics in debug mode). This @@ -482,12 +451,8 @@ impl RequestState { } } - /// Create a request state from a completed response. This will trigger - /// parsing of the response in a background task, so the call is expensive - /// but not blocking. - fn response(mut exchange: Exchange, parser: &dyn ResponseParser) -> Self { - // Pre-parse the body so the view doesn't have to do it - parser.parse(exchange.id, &mut exchange.response); + /// Create a request state from a completed response + fn response(exchange: Exchange) -> Self { Self::Response { exchange } } } @@ -697,57 +662,10 @@ impl From<&RequestState> for RequestStateSummary { } } -/// An abstraction for defining how response bodies should be parsed. In the TUI -/// we use background threads, but in tests we do it inline for simplicity. -pub trait ResponseParser: Debug { - fn parse(&self, request_id: RequestId, response: &mut ResponseRecord); -} - -/// Parser response bodies in background threads, to prevent blocking the UI. -#[derive(Debug)] -pub struct BackgroundResponseParser { - messages_tx: MessageSender, -} - -impl BackgroundResponseParser { - pub fn new(messages_tx: MessageSender) -> Self { - Self { messages_tx } - } -} - -impl ResponseParser for BackgroundResponseParser { - fn parse(&self, request_id: RequestId, response: &mut ResponseRecord) { - if let Some(content_type) = response.content_type() { - // Bytes are cheaply clonable - let body = response.body.bytes().clone(); - let messages_tx = self.messages_tx.clone(); - task::spawn_blocking(move || { - let Ok(parsed) = content_type - .parse_content(&body) - .with_context(|| { - format!( - "Error parsing response body \ - for request {request_id}" - ) - }) - .traced() - else { - return; - }; - - messages_tx.send(Message::ParseResponseBodyComplete { - request_id, - body: parsed, - }); - }); - } - } -} - #[cfg(test)] mod tests { use super::*; - use crate::test_util::{harness, TestHarness, TestResponseParser}; + use crate::test_util::{harness, TestHarness}; use anyhow::anyhow; use chrono::Utc; use rstest::rstest; @@ -765,17 +683,14 @@ mod tests { }; use tokio::time; - const PARSER: TestResponseParser = TestResponseParser; - #[rstest] fn test_get() { - let mut store = - RequestStore::new(CollectionDatabase::factory(()), PARSER); + let mut store = RequestStore::new(CollectionDatabase::factory(())); let exchange = Exchange::factory(()); let id = exchange.id; store .requests - .insert(exchange.id, RequestState::response(exchange, &PARSER)); + .insert(exchange.id, RequestState::response(exchange)); // This is a bit jank, but since we can't clone exchanges, the only way // to get the value back for comparison is to access the map directly @@ -787,8 +702,7 @@ mod tests { #[rstest] #[tokio::test] async fn test_life_cycle_success() { - let mut store = - RequestStore::new(CollectionDatabase::factory(()), PARSER); + let mut store = RequestStore::new(CollectionDatabase::factory(())); let exchange = Exchange::factory(()); let id = exchange.id; @@ -824,10 +738,7 @@ mod tests { #[rstest] #[tokio::test] async fn test_life_cycle_build_error() { - let mut store = RequestStore::new( - CollectionDatabase::factory(()), - TestResponseParser, - ); + let mut store = RequestStore::new(CollectionDatabase::factory(())); let exchange = Exchange::factory(()); let id = exchange.id; let profile_id = &exchange.request.profile_id; @@ -856,8 +767,7 @@ mod tests { #[rstest] #[tokio::test] async fn test_life_cycle_request_error() { - let mut store = - RequestStore::new(CollectionDatabase::factory(()), PARSER); + let mut store = RequestStore::new(CollectionDatabase::factory(())); let exchange = Exchange::factory(()); let id = exchange.id; let profile_id = &exchange.request.profile_id; @@ -887,8 +797,7 @@ mod tests { #[rstest] #[tokio::test] async fn test_life_cycle_cancel() { - let mut store = - RequestStore::new(CollectionDatabase::factory(()), PARSER); + let mut store = RequestStore::new(CollectionDatabase::factory(())); let exchange = Exchange::factory(()); let id = exchange.id; let profile_id = &exchange.request.profile_id; @@ -936,10 +845,9 @@ mod tests { // it so we can ensure the store *isn't* going to the DB for it let present_exchange = Exchange::factory(()); let present_id = present_exchange.id; - store.requests.insert( - present_id, - RequestState::response(present_exchange, &PARSER), - ); + store + .requests + .insert(present_id, RequestState::response(present_exchange)); let missing_exchange = Exchange::factory(()); let missing_id = missing_exchange.id; @@ -989,7 +897,7 @@ mod tests { assert_eq!( store.load_latest(Some(&profile_id), &recipe_id).unwrap(), - Some(&RequestState::response(expected_exchange, &PARSER)) + Some(&RequestState::response(expected_exchange)) ); // Non-match @@ -1017,7 +925,7 @@ mod tests { let mut store = harness.request_store.borrow_mut(); store .requests - .insert(exchange.id, RequestState::response(exchange, &PARSER)); + .insert(exchange.id, RequestState::response(exchange)); let loaded = store.load_latest(Some(&profile_id), &recipe_id).unwrap(); assert_eq!(loaded.map(RequestState::id), Some(request_id)); } @@ -1044,7 +952,7 @@ mod tests { let response_id = exchange.id; store .requests - .insert(exchange.id, RequestState::response(exchange, &PARSER)); + .insert(exchange.id, RequestState::response(exchange)); let building_id = RequestId::new(); store.start( diff --git a/crates/tui/src/lib.rs b/crates/tui/src/lib.rs index 8a5c5dbf..05e1db8b 100644 --- a/crates/tui/src/lib.rs +++ b/crates/tui/src/lib.rs @@ -18,8 +18,8 @@ mod view; use crate::{ context::TuiContext, - http::{BackgroundResponseParser, RequestState, RequestStore}, - message::{Message, MessageSender, RequestConfig}, + http::{RequestState, RequestStore}, + message::{Callback, Message, MessageSender, RequestConfig}, util::{ clear_event_buffer, delete_temp_file, get_editor_command, get_viewer_command, save_file, signals, ResultReported, @@ -121,10 +121,7 @@ impl Tui { // `Tui`. let terminal = initialize_terminal()?; - let request_store = RequestStore::new( - database.clone(), - BackgroundResponseParser::new(messages_tx.clone()), - ); + let request_store = RequestStore::new(database.clone()); let app = Tui { terminal, @@ -329,17 +326,6 @@ impl Tui { self.view.open_modal(confirm); } - Message::ParseResponseBodyComplete { request_id, body } => { - self.request_store - .set_parsed_body(request_id, body) - .with_context(|| { - format!( - "Error storing parsed response body \ - for request {request_id}" - ) - })?; - } - Message::TemplatePreview { template, on_complete, @@ -650,9 +636,7 @@ impl Tui { &self, template: Template, profile_id: Option, - on_complete: Box< - dyn 'static + Send + Sync + FnOnce(Vec), - >, + on_complete: Callback>, ) -> anyhow::Result<()> { let context = self.template_context(profile_id, true)?; let messages_tx = self.messages_tx(); diff --git a/crates/tui/src/message.rs b/crates/tui/src/message.rs index 5382fc1d..343d7f6a 100644 --- a/crates/tui/src/message.rs +++ b/crates/tui/src/message.rs @@ -8,8 +8,8 @@ use slumber_config::Action; use slumber_core::{ collection::{Collection, ProfileId, RecipeId}, http::{ - content_type::ResponseContent, BuildOptions, Exchange, - RequestBuildError, RequestError, RequestId, RequestRecord, + BuildOptions, Exchange, RequestBuildError, RequestError, RequestId, + RequestRecord, }, template::{Prompt, Prompter, Select, Template, TemplateChunk}, util::ResultTraced, @@ -119,14 +119,6 @@ pub enum Message { /// Send an informational notification to the user Notify(String), - /// Register a parsed response body. Parsing is performed in a background - /// thread so it doesn't block the UI - ParseResponseBodyComplete { - request_id: RequestId, - #[debug(skip)] - body: Box, - }, - /// Show a prompt to the user, asking for some input. Use the included /// channel to return the value. PromptStart(Prompt), diff --git a/crates/tui/src/test_util.rs b/crates/tui/src/test_util.rs index 9d79c5f7..b5b768c0 100644 --- a/crates/tui/src/test_util.rs +++ b/crates/tui/src/test_util.rs @@ -2,7 +2,7 @@ use crate::{ context::TuiContext, - http::{RequestStore, ResponseParser}, + http::RequestStore, message::{Message, MessageSender}, view::ViewContext, }; @@ -14,10 +14,7 @@ use ratatui::{ }; use rstest::fixture; use slumber_core::{ - collection::Collection, - db::CollectionDatabase, - http::{RequestId, ResponseRecord}, - test_util::Factory, + collection::Collection, db::CollectionDatabase, test_util::Factory, }; use std::{cell::RefCell, rc::Rc, sync::Arc}; use tokio::sync::mpsc::{self, UnboundedReceiver}; @@ -30,10 +27,8 @@ pub fn harness() -> TestHarness { let messages_tx: MessageSender = messages_tx.into(); let collection = Collection::factory(()).into(); let database = CollectionDatabase::factory(()); - let request_store = Rc::new(RefCell::new(RequestStore::new( - database.clone(), - TestResponseParser, - ))); + let request_store = + Rc::new(RefCell::new(RequestStore::new(database.clone()))); ViewContext::init( Arc::clone(&collection), database.clone(), @@ -138,33 +133,6 @@ impl TestTerminal { } } -/// Parse response bodies inline, for simplicity. Maybe not using the main -/// code path for tests is bad practice, but IMO it's not worth the effort -/// to make the background parser work in tests. -#[derive(Debug)] -pub struct TestResponseParser; - -impl TestResponseParser { - /// A helper for manually parsing a response body in tests - pub fn parse_body(response: &mut ResponseRecord) { - // Request ID is never used, so we can just pass a random one in - Self.parse(RequestId::new(), response); - } -} - -impl ResponseParser for TestResponseParser { - fn parse(&self, _: RequestId, response: &mut ResponseRecord) { - let Some(content_type) = response.content_type() else { - return; - }; - let Ok(parsed) = content_type.parse_content(response.body.bytes()) - else { - return; - }; - response.set_parsed_body(parsed); - } -} - /// Assert that the event queue matches the given list of patterns. Each event /// can optionally include a conditional expression to apply additional /// assertions. diff --git a/crates/tui/src/util.rs b/crates/tui/src/util.rs index 549c05a0..c3612156 100644 --- a/crates/tui/src/util.rs +++ b/crates/tui/src/util.rs @@ -3,7 +3,7 @@ use crate::{ message::{Message, MessageSender}, view::Confirm, }; -use anyhow::Context; +use anyhow::{anyhow, bail, Context}; use bytes::Bytes; use crossterm::event; use editor_command::EditorBuilder; @@ -16,11 +16,11 @@ use std::{ env, io, ops::Deref, path::{Path, PathBuf}, - process::Command, + process::{Command, Stdio}, time::Duration, }; use tokio::{fs::OpenOptions, io::AsyncWriteExt, sync::oneshot}; -use tracing::{debug, error, info, warn}; +use tracing::{debug, debug_span, error, info, warn}; use uuid::Uuid; /// Extension trait for [Result] @@ -231,6 +231,46 @@ pub fn get_viewer_command(file: &Path) -> anyhow::Result { }) } +/// Run a shellish command, optionally piping some stdin to it +pub async fn run_command( + command: &str, + stdin: Option<&[u8]>, +) -> anyhow::Result> { + let _ = debug_span!("Command", command).entered(); + + let mut parsed = shellish_parse::parse(command, true)?; + let mut tokens = parsed.drain(..); + let program = tokens.next().ok_or_else(|| anyhow!("Command is empty"))?; + let args = tokens; + let mut process = tokio::process::Command::new(program) + .args(args) + .stdin(Stdio::piped()) + .stdout(Stdio::piped()) + .stderr(Stdio::piped()) + .spawn()?; + + if let Some(stdin) = stdin { + process + .stdin + .as_mut() + .expect("Process missing stdin") + .write_all(stdin) + .await?; + } + let output = process.wait_with_output().await?; + debug!( + status = ?output.status, + stdout = %String::from_utf8_lossy(&output.stdout), + stderr = %String::from_utf8_lossy(&output.stderr), + "Command complete" + ); + if !output.status.success() { + let stderr = std::str::from_utf8(&output.stderr).unwrap_or_default(); + bail!("{}", stderr); + } + Ok(output.stdout) +} + /// Ask the user for some text input and wait for a response. Return `None` if /// the prompt is closed with no input. async fn prompt( diff --git a/crates/tui/src/view/common/text_box.rs b/crates/tui/src/view/common/text_box.rs index 2c73701e..089d5911 100644 --- a/crates/tui/src/view/common/text_box.rs +++ b/crates/tui/src/view/common/text_box.rs @@ -40,6 +40,9 @@ pub struct TextBox { // State state: TextState, + /// Text box has some related error. This is set externally by + /// [Self::set_error], and cleared whenever text changes + has_error: bool, on_change_debounce: Option, // Callbacks @@ -148,6 +151,11 @@ impl TextBox { self.submit(); } + /// Enable error state, to show something invalid to the user + pub fn set_error(&mut self) { + self.has_error = true; + } + /// Check if the current input text is valid. Always returns true if there /// is no validator fn is_valid(&self) -> bool { @@ -208,6 +216,7 @@ impl TextBox { /// Call parent's on_change callback. Should be called whenever text /// _content_ is changed fn change(&mut self) { + self.has_error = false; // Clear existing error for the new text let is_valid = self.is_valid(); if let Some(debounce) = &self.on_change_debounce { if self.is_valid() { @@ -296,9 +305,10 @@ impl Draw for TextBox { }; // Draw the text - let style = if self.is_valid() { + let style = if self.is_valid() && !self.has_error { styles.text_box.text } else { + // Invalid and error state look the same styles.text_box.invalid }; frame.render_widget(Paragraph::new(text).style(style), metadata.area()); diff --git a/crates/tui/src/view/component/queryable_body.rs b/crates/tui/src/view/component/queryable_body.rs index 33ca84db..d12cb5a7 100644 --- a/crates/tui/src/view/component/queryable_body.rs +++ b/crates/tui/src/view/component/queryable_body.rs @@ -2,6 +2,8 @@ use crate::{ context::TuiContext, + message::Message, + util::run_command, view::{ common::{ text_box::TextBox, @@ -10,79 +12,48 @@ use crate::{ context::UpdateContext, draw::{Draw, DrawMetadata}, event::{Child, Event, EventHandler, Update}, - state::{Identified, StateCell}, + state::Identified, util::{highlight, str_to_text}, Component, ViewContext, }, }; -use anyhow::Context; use persisted::PersistedContainer; use ratatui::{ layout::{Constraint, Layout}, text::Text, Frame, }; -use serde_json_path::JsonPath; use slumber_config::Action; use slumber_core::{ - http::{content_type::ContentType, query::Query, ResponseBody}, - util::{MaybeStr, ResultTraced}, + http::{content_type::ContentType, ResponseBody, ResponseRecord}, + util::MaybeStr, }; -use std::cell::{Cell, Ref}; +use std::sync::Arc; -/// Display response body as text, with a query box to filter it if the body has -/// been parsed. The query state can be persisted by persisting this entire -/// container. +/// Display response body as text, with a query box to run commands on the body. +/// The query state can be persisted by persisting this entire container. #[derive(Debug)] pub struct QueryableBody { - /// Visible text state. This needs to be in a cell because it's initialized - /// from the body passed in via props - state: StateCell, - /// Store whether the body can be queried. True only if it's a recognized - /// and parsed format - query_available: Cell, + response: Arc, + /// Are we currently typing in the query box? query_focused: bool, - /// Expression used to filter the content of the body down - query: Option, + /// Shell command used to transform the content body + query_command: Option, /// Where the user enters their body query query_text_box: Component, /// Filtered text display text_window: Component, -} - -#[derive(Clone)] -pub struct QueryableBodyProps<'a> { - /// Type of the body content; include for syntax highlighting - pub content_type: Option, - /// Body content. Theoretically this component isn't specific to responses, - /// but that's the only place where it's used currently so we specifically - /// accept a response body. By keeping it 90% agnostic (i.e. not accepting - /// a full response), it makes it easier to adapt in the future if we want - /// to make request bodies queryable as well. - pub body: &'a ResponseBody, -} -#[derive(Clone, Debug, PartialEq)] -struct StateKey { - /// Sometimes the parsing takes a little bit. We want to make sure the body - /// is regenerated after parsing completes - is_parsed: bool, - query: Option, -} - -#[derive(Debug)] -struct State { - text: Identified>, - is_parsed: bool, - is_binary: bool, + /// Data that can update as the query changes + state: TextState, } impl QueryableBody { /// Create a new body, optionally loading the query text from the /// persistence DB. This is optional because not all callers use the query /// box, or want to persist the value. - pub fn new() -> Self { + pub fn new(response: Arc) -> Self { let input_engine = &TuiContext::get().input_engine; let binding = input_engine.binding_display(Action::Search); @@ -90,76 +61,71 @@ impl QueryableBody { move || ViewContext::push_event(Event::new_local(callback)) }; let text_box = TextBox::default() - .placeholder(format!("{binding} to query body")) - .placeholder_focused("Query with JSONPath (ex: $.results)") - .validator(|text| JsonPath::parse(text).is_ok()) + .placeholder(format!("{binding} to filter")) + .placeholder_focused("Enter command (ex: `jq .results`)") // Callback trigger an events, so we can modify our own state .on_click(send_local(QueryCallback::Focus)) .on_change(send_local(QueryCallback::Change), true) .on_cancel(send_local(QueryCallback::Cancel)) .on_submit(send_local(QueryCallback::Submit)); + let state = init_state(response.content_type(), &response.body); + Self { - state: Default::default(), - query_available: Cell::new(false), + response, query_focused: false, - query: Default::default(), + query_command: None, query_text_box: text_box.into(), text_window: Default::default(), + state, } } - /// Get visible body text. Return an owned value because that's what all - /// consumers need anyway, and it makes the API simpler. Return `None` if: - /// - Text isn't initialized yet - /// - Body is binary - /// - Body has not been parsed, either because it's too large or not a known - /// content type - /// - /// Note that in the last case, we _could_ return the body, but it's going - /// to be the same content as what's in the request store so we can avoid - /// the clone by returning `None` instead. - pub fn parsed_text(&self) -> Option { - let state = self.state.get()?; - if state.is_binary || !state.is_parsed { + /// If a query command has been applied, get the visible text. Otherwise, + /// return `None` to indicate the response's original body can be used. + /// Return an owned value because we have to join the text to a string. + pub fn modified_text(&self) -> Option { + if self.query_command.is_none() { None } else { - Some(state.text.to_string()) + Some(self.state.text.to_string()) } } /// Get visible body text - pub fn visible_text(&self) -> Option> { - self.state - .get() - .map(|state| Ref::map(state, |state| &*state.text)) + pub fn visible_text(&self) -> &Text { + &self.state.text } + /// Update query command based on the current text in the box, and start + /// a task to run the command fn update_query(&mut self) { - let text = self.query_text_box.data().text(); - self.query = if text.is_empty() { - None + let command = self.query_text_box.data().text(); + if command.is_empty() { + // Reset to initial body + self.query_command = None; + self.state = + init_state(self.response.content_type(), &self.response.body); } else { - text.parse() - // Log the error, then throw it away - .with_context(|| format!("Error parsing query {text:?}")) - .traced() - .ok() - }; - } -} - -impl Default for QueryableBody { - fn default() -> Self { - Self::new() + self.query_command = Some(command.to_owned()); + + // Spawn the command in the background because it could be slow. + // Clone is cheap because Bytes uses refcounting + let body = self.response.body.bytes().clone(); + let messages_tx = ViewContext::messages_tx(); + let command = command.to_owned(); + tokio::spawn(async move { + let result = run_command(&command, Some(&body)).await; + messages_tx + .send(Message::Local(Box::new(QueryComplete(result)))); + }); + } } } impl EventHandler for QueryableBody { fn update(&mut self, _: &mut UpdateContext, event: Event) -> Update { if let Some(Action::Search) = event.action() { - if self.query_available.get() { - self.query_focused = true; - } + self.query_focused = true; } else if let Some(callback) = event.local::() { match callback { QueryCallback::Focus => self.query_focused = true, @@ -167,10 +133,7 @@ impl EventHandler for QueryableBody { QueryCallback::Cancel => { // Reset text to whatever was submitted last self.query_text_box.data_mut().set_text( - self.query - .as_ref() - .map(Query::to_string) - .unwrap_or_default(), + self.query_command.clone().unwrap_or_default(), ); self.query_focused = false; } @@ -179,6 +142,22 @@ impl EventHandler for QueryableBody { self.query_focused = false; } } + } else if let Some(QueryComplete(result)) = + event.local::() + { + match result { + Ok(stdout) => { + self.state = init_state( + // Assume the output has the same content type + self.response.content_type(), + &ResponseBody::new(stdout), + ); + } + // Trigger error state. We DON'T want to show a modal here + // because it's incredibly annoying. Maybe there should be a + // way to open the error though? + Err(_) => self.query_text_box.data_mut().set_error(), + } } else { return Update::Propagate(event); } @@ -193,36 +172,16 @@ impl EventHandler for QueryableBody { } } -impl<'a> Draw> for QueryableBody { - fn draw( - &self, - frame: &mut Frame, - props: QueryableBodyProps, - metadata: DrawMetadata, - ) { - // Body can only be queried if it's been parsed - let query_available = props.body.parsed().is_some(); - self.query_available.set(query_available); - - let [body_area, query_area] = Layout::vertical([ - Constraint::Min(0), - Constraint::Length(if query_available { 1 } else { 0 }), - ]) - .areas(metadata.area()); - - // Draw the body - let body = props.body; - let state_key = StateKey { - query: self.query.clone(), - is_parsed: props.body.parsed().is_some(), - }; - let state = self.state.get_or_update(&state_key, || { - init_state(props.content_type, body, self.query.as_ref()) - }); +impl Draw for QueryableBody { + fn draw(&self, frame: &mut Frame, _: (), metadata: DrawMetadata) { + let [body_area, query_area] = + Layout::vertical([Constraint::Min(0), Constraint::Length(1)]) + .areas(metadata.area()); + self.text_window.draw( frame, TextWindowProps { - text: &state.text, + text: &self.state.text, margins: ScrollbarMargins { bottom: 2, // Extra margin to jump over the search box ..Default::default() @@ -233,10 +192,8 @@ impl<'a> Draw> for QueryableBody { true, ); - if query_available { - self.query_text_box - .draw(frame, (), query_area, self.query_focused); - } + self.query_text_box + .draw(frame, (), query_area, self.query_focused); } } @@ -253,6 +210,12 @@ impl PersistedContainer for QueryableBody { } } +#[derive(Debug)] +struct TextState { + /// The full body, which we need to track for launching commands + text: Identified>, +} + /// All callback events from the query text box #[derive(Copy, Clone, Debug)] enum QueryCallback { @@ -262,69 +225,42 @@ enum QueryCallback { Submit, } +/// Callback event from executing the query command. Value is command stdout +#[derive(Debug)] +struct QueryComplete(anyhow::Result>); + /// Calculate display text based on current body/query -fn init_state( +fn init_state>( content_type: Option, - body: &ResponseBody, - query: Option<&Query>, -) -> State { + body: &ResponseBody, +) -> TextState { if TuiContext::get().config.http.is_large(body.size()) { // For bodies over the "large" size, skip prettification and // highlighting because it's slow. We could try to push this work - // into a background thread instead, but there's way to kill those + // into a background thread instead, but there's no way to kill those // threads so we could end up piling up a lot of work. It also burns // a lot of CPU, regardless of where it's run // // We don't show a hint to the user in this case because it's not // worth the screen real estate if let Some(text) = body.text() { - State { + TextState { text: str_to_text(text).into(), - is_parsed: false, - is_binary: false, } } else { // Showing binary content is a bit of a novelty, there's not much // value in it. For large bodies it's not worth the CPU cycles let text: Text = "".into(); - State { - text: text.into(), - is_parsed: false, - is_binary: true, - } + TextState { text: text.into() } } + } else if let Some(text) = body.text() { + let text = highlight::highlight_if(content_type, str_to_text(text)); + TextState { text: text.into() } } else { - // Query and prettify text if possible. This involves a lot of cloning - // because it makes stuff easier. If it becomes a bottleneck on large - // responses it's fixable. - if let Some(parsed) = body.parsed() { - // Body is a known content type so we parsed it - apply a query - // if necessary and prettify the output - let text = query - .map(|query| query.query_content(parsed).prettify()) - .unwrap_or_else(|| parsed.prettify()); - let text = highlight::highlight_if(content_type, text.into()); - State { - text: text.into(), - is_parsed: true, - is_binary: false, - } - } else if let Some(text) = body.text() { - // Body is textual but hasn't been parsed. Just show the plain text - State { - text: str_to_text(text).into(), - is_parsed: false, - is_binary: false, - } - } else { - // Content is binary, show a textual representation of it - let text: Text = format!("{:#}", MaybeStr(body.bytes())).into(); - State { - text: text.into(), - is_parsed: false, - is_binary: true, - } - } + // Content is binary, show a textual representation of it + let text: Text = + format!("{:#}", MaybeStr(body.bytes().as_ref())).into(); + TextState { text: text.into() } } } @@ -333,9 +269,7 @@ mod tests { use super::*; use crate::{ context::TuiContext, - test_util::{ - harness, terminal, TestHarness, TestResponseParser, TestTerminal, - }, + test_util::{harness, terminal, TestHarness, TestTerminal}, view::{ test_util::TestComponent, util::persistence::{DatabasePersistedStore, PersistedLazy}, @@ -347,7 +281,10 @@ mod tests { use reqwest::StatusCode; use rstest::{fixture, rstest}; use serde::Serialize; - use slumber_core::{http::ResponseRecord, test_util::header_map}; + use slumber_core::{ + assert_matches, + http::{ResponseBody, ResponseRecord}, + }; const TEXT: &[u8] = b"{\"greeting\":\"hello\"}"; @@ -358,96 +295,67 @@ mod tests { } #[fixture] - fn json_response() -> ResponseRecord { - let mut response = ResponseRecord { + fn response() -> Arc { + ResponseRecord { status: StatusCode::OK, - headers: header_map([("Content-Type", "application/json")]), + // Note: do NOT set the content-type header here. All it does is + // enable syntax highlighting, which makes buffer assertions hard + headers: Default::default(), body: ResponseBody::new(TEXT.into()), - }; - TestResponseParser::parse_body(&mut response); - response - } - - /// Render an unparsed body with no query box - #[rstest] - fn test_unparsed( - harness: TestHarness, - #[with(30, 2)] terminal: TestTerminal, - ) { - let body = ResponseBody::new(TEXT.into()); - let component = TestComponent::new( - &harness, - &terminal, - QueryableBody::new(), - QueryableBodyProps { - content_type: None, - body: &body, - }, - ); - - // Assert state - let data = component.data(); - assert_eq!(data.parsed_text().as_deref(), None); - assert!(!data.query_available.get()); - assert_eq!(data.query, None); - - // Assert view - terminal.assert_buffer_lines([ - vec![gutter("1"), " {\"greeting\":\"hello\"} ".into()], - vec![gutter(" "), " ".into()], - ]); + } + .into() } /// Render a parsed body with query text box #[rstest] #[tokio::test] async fn test_parsed( - harness: TestHarness, - #[with(32, 5)] terminal: TestTerminal, - json_response: ResponseRecord, + mut harness: TestHarness, + #[with(26, 3)] terminal: TestTerminal, + response: Arc, ) { let mut component = TestComponent::new( &harness, &terminal, - QueryableBody::new(), - QueryableBodyProps { - content_type: None, - body: &json_response.body, - }, + QueryableBody::new(response), + (), ); // Assert initial state/view let data = component.data(); - assert!(data.query_available.get()); - assert_eq!(data.query, None); + assert_eq!(data.query_command, None); assert_eq!( - data.parsed_text().as_deref(), - Some("{\n \"greeting\": \"hello\"\n}") + data.modified_text().as_deref(), + Some("{\"greeting\":\"hello\"}") ); let styles = &TuiContext::get().styles.text_box; terminal.assert_buffer_lines([ - vec![gutter("1"), " { ".into()], - vec![gutter("2"), " \"greeting\": \"hello\"".into()], - vec![gutter("3"), " } ".into()], - vec![gutter(" "), " ".into()], + vec![gutter("1"), " {\"greeting\":\"hello\"}".into()], + vec![gutter(" "), " ".into()], vec![ Span::styled( - "/ to query body", + "/ to filter", styles.text.patch(styles.placeholder), ), - Span::styled(" ", styles.text), + Span::styled(" ", styles.text), ], ]); // Type something into the query box component.send_key(KeyCode::Char('/')).assert_empty(); - component.send_text("$.greeting").assert_empty(); + component.send_text("head -c 1").assert_empty(); component.send_key(KeyCode::Enter).assert_empty(); + // Wait for the command to finish, pass results back to the component + let event = assert_matches!( + harness.pop_message_wait().await, + Message::Local(event) => event, + ); + component.update_draw(Event::Local(event)).assert_empty(); // Make sure state updated correctly let data = component.data(); - assert_eq!(data.query, Some("$.greeting".parse().unwrap())); - assert_eq!(data.parsed_text().as_deref(), Some("[\n \"hello\"\n]")); + assert_eq!(data.query_command.as_deref(), Some("head -c 1")); + assert_eq!(data.modified_text().as_deref(), Some("{")); assert!(!data.query_focused); // Cancelling out of the text box should reset the query value @@ -455,37 +363,33 @@ mod tests { component.send_text("more text").assert_empty(); component.send_key(KeyCode::Esc).assert_empty(); let data = component.data(); - assert_eq!(data.query, Some("$.greeting".parse().unwrap())); - assert_eq!(data.query_text_box.data().text(), "$.greeting"); + assert_eq!(data.query_command.as_deref(), Some("head -c 1")); + assert_eq!(data.query_text_box.data().text(), "head -c 1"); assert!(!data.query_focused); // Check the view again terminal.assert_buffer_lines([ - vec![gutter("1"), " [ ".into()], - vec![gutter("2"), " \"hello\" ".into()], - vec![gutter("3"), " ] ".into()], - vec![gutter(" "), " ".into()], - vec![Span::styled( - "$.greeting ", - styles.text, - )], + vec![gutter("1"), " { ".into()], + vec![gutter(" "), " ".into()], + vec![Span::styled("head -c 1 ", styles.text)], ]); } /// Render a parsed body with query text box, and load initial query from /// the DB. This tests the `PersistedContainer` implementation #[rstest] - fn test_persistence( + #[tokio::test] + async fn test_persistence( harness: TestHarness, #[with(30, 4)] terminal: TestTerminal, - json_response: ResponseRecord, + response: Arc, ) { #[derive(Debug, Serialize, PersistedKey)] #[persisted(String)] struct Key; // Add initial query to the DB - DatabasePersistedStore::store_persisted(&Key, &"$.greeting".to_owned()); + DatabasePersistedStore::store_persisted(&Key, &"head -n 1".to_owned()); // We already have another test to check that querying works via typing // in the box, so we just need to make sure state is initialized @@ -493,12 +397,12 @@ mod tests { let component = TestComponent::new( &harness, &terminal, - PersistedLazy::new(Key, QueryableBody::new()), - QueryableBodyProps { - content_type: None, - body: &json_response.body, - }, + PersistedLazy::new(Key, QueryableBody::new(response)), + (), + ); + assert_eq!( + component.data().query_command.as_deref(), + Some("head -n 1") ); - assert_eq!(component.data().query, Some("$.greeting".parse().unwrap())); } } diff --git a/crates/tui/src/view/component/response_view.rs b/crates/tui/src/view/component/response_view.rs index 169e27ff..6b6bdce5 100644 --- a/crates/tui/src/view/component/response_view.rs +++ b/crates/tui/src/view/component/response_view.rs @@ -4,7 +4,7 @@ use crate::{ message::Message, view::{ common::{actions::ActionsModal, header_table::HeaderTable}, - component::queryable_body::{QueryableBody, QueryableBodyProps}, + component::queryable_body::QueryableBody, context::UpdateContext, draw::{Draw, DrawMetadata, Generate, ToStringGenerate}, event::{Child, Event, EventHandler, Update}, @@ -22,6 +22,7 @@ use slumber_core::{ collection::RecipeId, http::{RequestId, ResponseRecord}, }; +use std::sync::Arc; use strum::{EnumCount, EnumIter}; /// Display response body @@ -36,7 +37,7 @@ pub struct ResponseBodyView { pub struct ResponseBodyViewProps<'a> { pub request_id: RequestId, pub recipe_id: &'a RecipeId, - pub response: &'a ResponseRecord, + pub response: &'a Arc, } /// Items in the actions popup menu for the Body @@ -75,9 +76,7 @@ struct ResponseQueryPersistedKey(RecipeId); impl ResponseBodyView { fn with_body(&self, f: impl Fn(&Text)) { if let Some(state) = self.state.get() { - if let Some(body) = state.body.data().visible_text() { - f(&body) - } + f(state.body.data().visible_text()) } } } @@ -112,7 +111,7 @@ impl EventHandler for ResponseBodyView { // This will trigger a modal to ask the user for a path ViewContext::send_message(Message::SaveResponseBody { request_id: state.request_id, - data: state.body.data().parsed_text(), + data: state.body.data().modified_text(), }); } } @@ -139,25 +138,16 @@ impl<'a> Draw> for ResponseBodyView { props: ResponseBodyViewProps, metadata: DrawMetadata, ) { - let response = &props.response; let state = self.state.get_or_update(&props.request_id, || State { request_id: props.request_id, body: PersistedLazy::new( ResponseQueryPersistedKey(props.recipe_id.clone()), - QueryableBody::new(), + QueryableBody::new(Arc::clone(props.response)), ) .into(), }); - state.body.draw( - frame, - QueryableBodyProps { - content_type: response.content_type(), - body: &response.body, - }, - metadata.area(), - true, - ); + state.body.draw(frame, (), metadata.area(), true); } } @@ -189,36 +179,21 @@ impl<'a> Draw> for ResponseHeadersView { mod tests { use super::*; use crate::{ - test_util::{ - harness, terminal, TestHarness, TestResponseParser, TestTerminal, - }, + test_util::{harness, terminal, TestHarness, TestTerminal}, view::test_util::TestComponent, }; - use indexmap::indexmap; + use crossterm::event::KeyCode; use rstest::rstest; - use slumber_core::{ - assert_matches, - http::Exchange, - test_util::{header_map, Factory}, - }; + use slumber_core::{assert_matches, http::Exchange, test_util::Factory}; /// Test "Copy Body" menu action #[rstest] - #[case::json_body( + #[case::text_body( ResponseRecord { - headers: header_map(indexmap! {"content-type" => "application/json"}), body: br#"{"hello":"world"}"#.to_vec().into(), ..ResponseRecord::factory(()) }, - "{\n \"hello\": \"world\"\n}", - )] - #[case::unparsed_text_body( - ResponseRecord { - headers: header_map(indexmap! {"content-type" => "text/plain"}), - body: b"hello!".to_vec().into(), - ..ResponseRecord::factory(()) - }, - "hello!", + "{\"hello\":\"world\"}", )] #[case::binary_body( ResponseRecord { @@ -234,11 +209,10 @@ mod tests { #[case] response: ResponseRecord, #[case] expected_body: &str, ) { - let mut exchange = Exchange { - response, + let exchange = Exchange { + response: response.into(), ..Exchange::factory(()) }; - TestResponseParser::parse_body(&mut exchange.response); let mut component = TestComponent::new( &harness, &terminal, @@ -263,21 +237,13 @@ mod tests { /// Test "Save Body as File" menu action #[rstest] - #[case::json_body( + #[case::text_body( ResponseRecord { - headers: header_map(indexmap! {"content-type" => "application/json"}), - body: br#"{"hello":"world"}"#.to_vec().into(), - ..ResponseRecord::factory(()) - }, - Some("{\n \"hello\": \"world\"\n}"), - )] - #[case::unparsed_text_body( - ResponseRecord { - headers: header_map(indexmap! {"content-type" => "text/plain"}), body: b"hello!".to_vec().into(), ..ResponseRecord::factory(()) }, None, + None, )] #[case::binary_body( ResponseRecord { @@ -285,19 +251,28 @@ mod tests { ..ResponseRecord::factory(()) }, None, + None, + )] + #[case::queried_body( + ResponseRecord { + body: b"hello!".to_vec().into(), + ..ResponseRecord::factory(()) + }, + Some("head -c 4"), + Some("hell"), )] #[tokio::test] async fn test_save_file( mut harness: TestHarness, terminal: TestTerminal, #[case] response: ResponseRecord, + #[case] query: Option<&str>, #[case] expected_body: Option<&str>, ) { - let mut exchange = Exchange { - response, + let exchange = Exchange { + response: response.into(), ..Exchange::factory(()) }; - TestResponseParser::parse_body(&mut exchange.response); let mut component = TestComponent::new( &harness, &terminal, @@ -309,6 +284,20 @@ mod tests { }, ); + if let Some(query) = query { + // Type something into the query box + component.send_key(KeyCode::Char('/')).assert_empty(); + component.send_text(query).assert_empty(); + component.send_key(KeyCode::Enter).assert_empty(); + // Wait for the command to finish, pass results back to the + // component + let event = assert_matches!( + harness.pop_message_wait().await, + Message::Local(event) => event, + ); + component.update_draw(Event::Local(event)).assert_empty(); + } + component .update_draw(Event::new_local(BodyMenuAction::SaveBody)) .assert_empty(); diff --git a/crates/tui/src/view/component/root.rs b/crates/tui/src/view/component/root.rs index 0e1a893a..524fb529 100644 --- a/crates/tui/src/view/component/root.rs +++ b/crates/tui/src/view/component/root.rs @@ -265,9 +265,7 @@ impl PersistedContainer for SelectedRequestId { mod tests { use super::*; use crate::{ - test_util::{ - harness, terminal, TestHarness, TestResponseParser, TestTerminal, - }, + test_util::{harness, terminal, TestHarness, TestTerminal}, view::{ test_util::TestComponent, util::persistence::DatabasePersistedStore, }, @@ -277,14 +275,12 @@ mod tests { use rstest::rstest; use slumber_core::{assert_matches, http::Exchange, test_util::Factory}; - const PARSER: TestResponseParser = TestResponseParser; - /// Test that, on first render, the view loads the most recent historical /// request for the first recipe+profile #[rstest] fn test_preload_request(harness: TestHarness, terminal: TestTerminal) { // Add a request into the DB that we expect to preload - let request_store = RequestStore::new(harness.database.clone(), PARSER); + let request_store = RequestStore::new(harness.database.clone()); let collection = Collection::factory(()); let profile_id = collection.first_profile_id(); let recipe_id = collection.first_recipe_id(); @@ -318,7 +314,7 @@ mod tests { harness: TestHarness, terminal: TestTerminal, ) { - let request_store = RequestStore::new(harness.database.clone(), PARSER); + let request_store = RequestStore::new(harness.database.clone()); let collection = Collection::factory(()); let recipe_id = collection.first_recipe_id(); let profile_id = collection.first_profile_id(); @@ -365,7 +361,7 @@ mod tests { harness: TestHarness, terminal: TestTerminal, ) { - let request_store = RequestStore::new(harness.database.clone(), PARSER); + let request_store = RequestStore::new(harness.database.clone()); let collection = Collection::factory(()); let recipe_id = collection.first_recipe_id(); let profile_id = collection.first_profile_id(); @@ -401,7 +397,7 @@ mod tests { #[rstest] fn test_edit_collection(mut harness: TestHarness, terminal: TestTerminal) { - let request_store = RequestStore::new(harness.database.clone(), PARSER); + let request_store = RequestStore::new(harness.database.clone()); let root = Root::new(&harness.collection); let mut component = TestComponent::new( &harness,