Change DB binary format

This requires wiping out all data in the local DB
LucasPickering · Mar 17, 2024 · 8b10f66 · 8b10f66
1 parent 5558374
commit 8b10f66
Show file tree

Hide file tree

Showing 14 changed files with 120 additions and 77 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -11,6 +11,8 @@
 
 - Fix prompt in TUI always rendering as sensitive ([#108](https://github.com/LucasPickering/slumber/issues/108))
 - Fix content type identification for extended JSON MIME types ([#103](https://github.com/LucasPickering/slumber/issues/103))
+- Use named records in binary blobs in the local DB
+  - This required wiping out existing binary blobs, meaning **all request history and UI state will be lost on upgrade**
 
 ## [0.13.1] - 2024-03-07
 

diff --git a/Cargo.lock b/Cargo.lock
diff --git a/Cargo.toml b/Cargo.toml
@@ -13,6 +13,7 @@ rust-version = "1.74.0"
 [dependencies]
 anyhow = {version = "^1.0.75", features = ["backtrace"]}
 async-trait = "^0.1.73"
+bytes = { version = "1.5.0", features = ["serde"] }
 chrono = {version = "^0.4.31", default-features = false, features = ["clock", "serde", "std"]}
 clap = {version = "^4.4.2", features = ["derive"]}
 cli-clipboard = "0.4.0"
@@ -43,7 +44,7 @@ thiserror = "^1.0.48"
 tokio = {version = "^1.32.0", default-features = false, features = ["full"]}
 tracing = "^0.1.37"
 tracing-subscriber = {version = "^0.3.17", default-features = false, features = ["env-filter", "fmt", "registry"]}
-url = "^2.5.0"
+url = { version = "^2.5.0", features = ["serde"] }
 uuid = {version = "^1.4.1", default-features = false, features = ["serde", "v4"]}
 
 [dev-dependencies]

diff --git a/src/cli/request.rs b/src/cli/request.rs
@@ -5,7 +5,7 @@ use crate::{
     db::Database,
     http::{HttpEngine, RecipeOptions, RequestBuilder},
     template::{Prompt, Prompter, TemplateContext},
-    util::ResultExt,
+    util::{MaybeStr, ResultExt},
     GlobalArgs,
 };
 use anyhow::{anyhow, Context};
@@ -137,7 +137,7 @@ impl Subcommand for RequestCommand {
                 println!("{}", HeaderDisplay(&record.response.headers));
             }
             if !self.no_body {
-                print!("{}", record.response.body.text());
+                print!("{}", MaybeStr(record.response.body.bytes()));
             }
 
             if self.exit_status && status.as_u16() >= 400 {
@@ -200,7 +200,7 @@ impl<'a> Display for HeaderDisplay<'a> {
                 f,
                 "{}: {}",
                 key_style.apply_to(key),
-                value.to_str().unwrap_or("<invalid utf-8>")
+                MaybeStr(value.as_bytes()),
             )?;
         }
         Ok(())

diff --git a/src/db.rs b/src/db.rs
@@ -120,6 +120,11 @@ impl Database {
                 )",
             )
             .down("DROP TABLE ui_state"),
+            // This is a sledgehammer migration. Added when we switch from
+            // rmp_serde::to_vec to rmp_serde::to_vec_named. This affected the
+            // serialization of all binary blobs, so there's no easy way to
+            // migrate it all. It's easiest just to wipe it all out.
+            M::up("DELETE FROM requests; DELETE FROM ui_state;").down(""),
         ]);
         migrations.to_latest(connection)?;
         Ok(())
@@ -519,7 +524,7 @@ struct Bytes<T>(T);
 
 impl<T: Serialize> ToSql for Bytes<T> {
     fn to_sql(&self) -> rusqlite::Result<ToSqlOutput<'_>> {
-        let bytes = rmp_serde::to_vec(&self.0).map_err(|err| {
+        let bytes = rmp_serde::to_vec_named(&self.0).map_err(|err| {
             rusqlite::Error::ToSqlConversionFailure(Box::new(err))
         })?;
         Ok(ToSqlOutput::Owned(bytes.into()))

diff --git a/src/factory.rs b/src/factory.rs
@@ -23,7 +23,7 @@ factori!(Request, {
         profile_id = None,
         recipe_id = "recipe1".into(),
         method = Method::GET,
-        url = "/url".into(),
+        url = "/url".parse().unwrap(),
         headers = HeaderMap::new(),
         query = IndexMap::new(),
         body = None,

diff --git a/src/http.rs b/src/http.rs
@@ -46,6 +46,7 @@ use crate::{
     template::TemplateContext, util::ResultExt,
 };
 use anyhow::Context;
+use bytes::Bytes;
 use chrono::Utc;
 use futures::future;
 use indexmap::IndexMap;
@@ -56,6 +57,7 @@ use reqwest::{
 use std::collections::HashSet;
 use tokio::try_join;
 use tracing::{debug, info, info_span};
+use url::Url;
 
 const USER_AGENT: &str =
     concat!(env!("CARGO_PKG_NAME"), "/", env!("CARGO_PKG_VERSION"));
@@ -201,7 +203,7 @@ impl HttpEngine {
         // Convert to reqwest's request format
         let mut request_builder = self
             .client
-            .request(request.method.clone(), &request.url)
+            .request(request.method.clone(), request.url.clone())
             .query(&request.query)
             .headers(request.headers.clone());
 
@@ -226,7 +228,7 @@ impl HttpEngine {
         let headers = response.headers().clone();
 
         // Pre-resolve the content, so we get all the async work done
-        let body = response.text().await?.into();
+        let body = response.bytes().await?.into();
 
         Ok(Response {
             status,
@@ -335,12 +337,14 @@ impl RequestBuilder {
         })
     }
 
-    async fn render_url(&self) -> anyhow::Result<String> {
-        self.recipe
-            .url
-            .render(&self.template_context)
-            .await
-            .context("Error rendering URL")
+    async fn render_url(&self) -> anyhow::Result<Url> {
+        // Shitty try block
+        async {
+            let url = self.recipe.url.render(&self.template_context).await?;
+            url.parse().map_err(anyhow::Error::from)
+        }
+        .await
+        .context("Error rendering URL")
     }
 
     async fn render_headers(&self) -> anyhow::Result<HeaderMap> {
@@ -404,13 +408,16 @@ impl RequestBuilder {
             .collect::<IndexMap<String, String>>())
     }
 
-    async fn render_body(&self) -> anyhow::Result<Option<String>> {
+    async fn render_body(&self) -> anyhow::Result<Option<Bytes>> {
         match &self.recipe.body {
-            Some(body) => Ok(Some(
-                body.render(&self.template_context)
+            Some(body) => {
+                let body = body
+                    .render(&self.template_context)
                     .await
-                    .context("Error rendering body")?,
-            )),
+                    .context("Error rendering body")?
+                    .into();
+                Ok(Some(body))
+            }
             None => Ok(None),
         }
     }

diff --git a/src/http/parse.rs b/src/http/parse.rs
@@ -36,7 +36,7 @@ pub trait ResponseContent: Debug + Display {
     fn content_type(&self) -> ContentType;
 
     /// Parse the response body as this type
-    fn parse(body: &str) -> anyhow::Result<Self>
+    fn parse(body: &[u8]) -> anyhow::Result<Self>
     where
         Self: Sized;
 
@@ -62,8 +62,8 @@ impl ResponseContent for Json {
         ContentType::Json
     }
 
-    fn parse(body: &str) -> anyhow::Result<Self> {
-        Ok(Self(serde_json::from_str(body)?))
+    fn parse(body: &[u8]) -> anyhow::Result<Self> {
+        Ok(Self(serde_json::from_slice(body)?))
     }
 
     fn prettify(&self) -> String {
@@ -86,7 +86,7 @@ impl ContentType {
     /// object.
     pub fn parse_content(
         self,
-        content: &str,
+        content: &[u8],
     ) -> anyhow::Result<Box<dyn ResponseContent>> {
         match self {
             Self::Json => Ok(Box::new(Json::parse(content)?)),
@@ -111,7 +111,8 @@ impl ContentType {
     pub(super) fn parse_response(
         response: &Response,
     ) -> anyhow::Result<Box<dyn ResponseContent>> {
-        Self::from_response(response)?.parse_content(response.body.text())
+        let content_type = Self::from_response(response)?;
+        content_type.parse_content(response.body.bytes())
     }
 
     /// Parse the content type from a file's extension
@@ -128,10 +129,10 @@ impl ContentType {
     /// Parse the content type from a response's `Content-Type` header
     pub fn from_response(response: &Response) -> anyhow::Result<Self> {
         // If the header value isn't utf-8, we're hosed
-        let header_value =
-            std::str::from_utf8(response.content_type().ok_or_else(|| {
-                anyhow!("Response has no content-type header")
-            })?)
+        let header_value = response
+            .content_type()
+            .ok_or_else(|| anyhow!("Response has no content-type header"))?;
+        let header_value = std::str::from_utf8(header_value)
             .context("content-type header is not valid utf-8")?;
         Self::from_header(header_value)
     }

diff --git a/src/http/record.rs b/src/http/record.rs
@@ -6,6 +6,7 @@ use crate::{
     util::ResultExt,
 };
 use anyhow::Context;
+use bytes::Bytes;
 use chrono::{DateTime, Duration, Utc};
 use derive_more::{Display, From};
 use indexmap::IndexMap;
@@ -16,6 +17,7 @@ use reqwest::{
 use serde::{Deserialize, Serialize};
 use std::fmt::Debug;
 use thiserror::Error;
+use url::Url;
 use uuid::Uuid;
 
 /// An error that can occur while *building* a request
@@ -100,13 +102,12 @@ pub struct Request {
 
     #[serde(with = "serde_method")]
     pub method: Method,
-    pub url: String,
+    pub url: Url,
     #[serde(with = "serde_header_map")]
     pub headers: HeaderMap,
     pub query: IndexMap<String, String>,
-    /// Text body content. At some point we'll support other formats (binary,
-    /// streaming from file, etc.)
-    pub body: Option<String>,
+    /// Body content as bytes. This should be decoded as needed
+    pub body: Option<Bytes>,
 }
 
 /// A resolved HTTP response, with all content loaded and ready to be displayed
@@ -145,22 +146,38 @@ impl Response {
     }
 }
 
-/// HTTP response body. Right now we store as text only, but at some point
-/// should add support for binary responses
+/// HTTP response body. Content is stored as bytes to support non-text content.
+/// Should be converted to text only as needed
 #[derive(Default, From, Serialize, Deserialize)]
-pub struct Body(String);
+pub struct Body(Bytes);
 
 impl Body {
-    pub fn new(text: String) -> Self {
-        Self(text)
+    pub fn new(bytes: Bytes) -> Self {
+        Self(bytes)
     }
 
-    pub fn text(&self) -> &str {
+    /// Raw content bytes
+    pub fn bytes(&self) -> &[u8] {
         &self.0
     }
 
-    pub fn into_text(self) -> String {
-        self.0
+    /// Owned raw content bytes
+    pub fn into_bytes(self) -> Vec<u8> {
+        self.0.into()
+    }
+}
+
+#[cfg(test)]
+impl From<String> for Body {
+    fn from(value: String) -> Self {
+        Self(value.into())
+    }
+}
+
+#[cfg(test)]
+impl From<&str> for Body {
+    fn from(value: &str) -> Self {
+        value.to_owned().into()
     }
 }
 
@@ -173,12 +190,6 @@ impl Debug for Body {
     }
 }
 
-impl From<&str> for Body {
-    fn from(value: &str) -> Self {
-        Body::new(value.into())
-    }
-}
-
 /// Serialization/deserialization for [reqwest::Method]
 mod serde_method {
     use super::*;

diff --git a/src/template/error.rs b/src/template/error.rs
@@ -75,6 +75,12 @@ pub enum ChainError {
     /// generated by our code so we don't need any extra context.
     #[error(transparent)]
     Database(anyhow::Error),
+    /// Chain source produced non-UTF-8 bytes
+    #[error("Error decoding content as UTF-8")]
+    InvalidUtf8 {
+        #[source]
+        error: FromUtf8Error,
+    },
     /// The chain ID is valid, but the corresponding recipe has no successful
     /// response
     #[error("No response available")]
@@ -104,12 +110,6 @@ pub enum ChainError {
         #[source]
         error: io::Error,
     },
-    #[error("Error decoding output for {command:?}")]
-    CommandInvalidUtf8 {
-        command: Vec<String>,
-        #[source]
-        error: FromUtf8Error,
-    },
     #[error("Error reading from file `{path}`")]
     File {
         path: PathBuf,