From 961cbd721b359efb51316f250b43960c10cf785d Mon Sep 17 00:00:00 2001 From: Edward Shen Date: Tue, 31 Dec 2019 23:07:01 -0500 Subject: [PATCH] move route serialization to config.rs --- src/config.rs | 129 +++++++++++++++++++++++++++++++++++++++++++++++++- src/main.rs | 10 ++-- src/routes.rs | 126 +----------------------------------------------- 3 files changed, 134 insertions(+), 131 deletions(-) diff --git a/src/config.rs b/src/config.rs index b9c7559..cfe1bee 100644 --- a/src/config.rs +++ b/src/config.rs @@ -1,7 +1,11 @@ -use crate::{routes::Route, BunBunError}; +use crate::BunBunError; use log::{debug, error, info, trace}; -use serde::{Deserialize, Serialize}; +use serde::{ + de::{Deserializer, Visitor}, + Deserialize, Serialize, Serializer, +}; use std::collections::HashMap; +use std::fmt; use std::fs::{read_to_string, OpenOptions}; use std::io::Write; @@ -22,6 +26,73 @@ pub struct RouteGroup { pub routes: HashMap, } +#[derive(Debug, PartialEq, Clone)] +pub enum Route { + External(String), + Path(String), +} + +/// Serialization of the Route enum needs to be transparent, but since the +/// `#[serde(transparent)]` macro isn't available on enums, so we need to +/// implement it manually. +impl Serialize for Route { + fn serialize(&self, serializer: S) -> Result + where + S: Serializer, + { + match self { + Self::External(s) => serializer.serialize_str(s), + Self::Path(s) => serializer.serialize_str(s), + } + } +} + +/// Deserialization of the route string into the enum requires us to figure out +/// whether or not the string is valid to run as an executable or not. To +/// determine this, we simply check if it exists on disk or assume that it's a +/// web path. This incurs a disk check operation, but since users shouldn't be +/// updating the config that frequently, it should be fine. +impl<'de> Deserialize<'de> for Route { + fn deserialize(deserializer: D) -> Result + where + D: Deserializer<'de>, + { + struct RouteVisitor; + impl<'de> Visitor<'de> for RouteVisitor { + type Value = Route; + + fn expecting(&self, formatter: &mut fmt::Formatter) -> fmt::Result { + formatter.write_str("string") + } + + fn visit_str(self, value: &str) -> Result + where + E: serde::de::Error, + { + // Return early if it's a path, don't go through URL parsing + if std::path::Path::new(value).exists() { + debug!("Parsed {} as a valid local path.", value); + Ok(Route::Path(value.into())) + } else { + debug!("{} does not exist on disk, assuming web path.", value); + Ok(Route::External(value.into())) + } + } + } + + deserializer.deserialize_str(RouteVisitor) + } +} + +impl std::fmt::Display for Route { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + Self::External(s) => write!(f, "raw ({})", s), + Self::Path(s) => write!(f, "file ({})", s), + } + } +} + /// Attempts to read the config file. If it doesn't exist, generate one a /// default config file before attempting to parse it. pub fn read_config(config_file_path: &str) -> Result { @@ -58,6 +129,60 @@ pub fn read_config(config_file_path: &str) -> Result { Ok(serde_yaml::from_str(&config_str)?) } +#[cfg(test)] +mod route { + use super::*; + use serde_yaml::{from_str, to_string}; + use tempfile::NamedTempFile; + + #[test] + fn deserialize_relative_path() { + let tmpfile = NamedTempFile::new_in(".").unwrap(); + let path = format!("{}", tmpfile.path().display()); + let path = path.get(path.rfind(".").unwrap()..).unwrap(); + let path = std::path::Path::new(path); + assert!(path.is_relative()); + let path = path.to_str().unwrap(); + assert_eq!(from_str::(path).unwrap(), Route::Path(path.into())); + } + + #[test] + fn deserialize_absolute_path() { + let tmpfile = NamedTempFile::new().unwrap(); + let path = format!("{}", tmpfile.path().display()); + assert!(tmpfile.path().is_absolute()); + assert_eq!(from_str::(&path).unwrap(), Route::Path(path)); + } + + #[test] + fn deserialize_http_path() { + assert_eq!( + from_str::("http://google.com").unwrap(), + Route::External("http://google.com".into()) + ); + } + + #[test] + fn deserialize_https_path() { + assert_eq!( + from_str::("https://google.com").unwrap(), + Route::External("https://google.com".into()) + ); + } + + #[test] + fn serialize() { + assert_eq!( + &to_string(&Route::External("hello world".into())).unwrap(), + "---\nhello world" + ); + assert_eq!( + &to_string(&Route::Path("hello world".into())).unwrap(), + "---\nhello world" + ); + } +} + #[cfg(test)] mod read_config { use super::*; diff --git a/src/main.rs b/src/main.rs index 5c858cb..47647c0 100644 --- a/src/main.rs +++ b/src/main.rs @@ -1,6 +1,6 @@ #![forbid(unsafe_code)] -use crate::config::{read_config, RouteGroup}; +use crate::config::{read_config, Route, RouteGroup}; use actix_web::{middleware::Logger, App, HttpServer}; use clap::{crate_authors, crate_version, load_yaml, App as ClapApp}; use error::BunBunError; @@ -24,7 +24,7 @@ pub struct State { default_route: Option, groups: Vec, /// Cached, flattened mapping of all routes and their destinations. - routes: HashMap, + routes: HashMap, } #[actix_rt::main] @@ -97,7 +97,7 @@ fn init_logger( /// Generates a hashmap of routes from the data structure created by the config /// file. This should improve runtime performance and is a better solution than /// just iterating over the config object for every hop resolution. -fn cache_routes(groups: &[RouteGroup]) -> HashMap { +fn cache_routes(groups: &[RouteGroup]) -> HashMap { let mut mapping = HashMap::new(); for group in groups { for (kw, dest) in &group.routes { @@ -218,11 +218,11 @@ mod cache_routes { fn generate_external_routes( routes: &[(&str, &str)], - ) -> HashMap { + ) -> HashMap { HashMap::from_iter( routes .iter() - .map(|(k, v)| ((*k).into(), routes::Route::External((*v).into()))), + .map(|(k, v)| ((*k).into(), Route::External((*v).into()))), ) } diff --git a/src/routes.rs b/src/routes.rs index b3e4126..aa9b203 100644 --- a/src/routes.rs +++ b/src/routes.rs @@ -1,4 +1,4 @@ -use crate::{template_args, BunBunError, State}; +use crate::{template_args, BunBunError, Route, State}; use actix_web::web::{Data, Query}; use actix_web::{get, http::header}; use actix_web::{HttpRequest, HttpResponse, Responder}; @@ -6,9 +6,8 @@ use handlebars::Handlebars; use itertools::Itertools; use log::{debug, error}; use percent_encoding::{utf8_percent_encode, AsciiSet, CONTROLS}; -use serde::{de::Visitor, Deserialize, Deserializer, Serialize, Serializer}; +use serde::Deserialize; use std::collections::HashMap; -use std::fmt; use std::path::PathBuf; use std::process::Command; use std::sync::{Arc, RwLock}; @@ -24,73 +23,6 @@ const FRAGMENT_ENCODE_SET: &AsciiSet = &CONTROLS type StateData = Data>>; -#[derive(Debug, PartialEq, Clone)] -pub enum Route { - External(String), - Path(String), -} - -/// Serialization of the Route enum needs to be transparent, but since the -/// `#[serde(transparent)]` macro isn't available on enums, so we need to -/// implement it manually. -impl Serialize for Route { - fn serialize(&self, serializer: S) -> Result - where - S: Serializer, - { - match self { - Self::External(s) => serializer.serialize_str(s), - Self::Path(s) => serializer.serialize_str(s), - } - } -} - -/// Deserialization of the route string into the enum requires us to figure out -/// whether or not the string is valid to run as an executable or not. To -/// determine this, we simply check if it exists on disk or assume that it's a -/// web path. This incurs a disk check operation, but since users shouldn't be -/// updating the config that frequently, it should be fine. -impl<'de> Deserialize<'de> for Route { - fn deserialize(deserializer: D) -> Result - where - D: Deserializer<'de>, - { - struct RouteVisitor; - impl<'de> Visitor<'de> for RouteVisitor { - type Value = Route; - - fn expecting(&self, formatter: &mut fmt::Formatter) -> fmt::Result { - formatter.write_str("string") - } - - fn visit_str(self, value: &str) -> Result - where - E: serde::de::Error, - { - // Return early if it's a path, don't go through URL parsing - if std::path::Path::new(value).exists() { - debug!("Parsed {} as a valid local path.", value); - Ok(Route::Path(value.into())) - } else { - debug!("{} does not exist on disk, assuming web path.", value); - Ok(Route::External(value.into())) - } - } - } - - deserializer.deserialize_str(RouteVisitor) - } -} - -impl std::fmt::Display for Route { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - match self { - Self::External(s) => write!(f, "raw ({})", s), - Self::Path(s) => write!(f, "file ({})", s), - } - } -} - #[get("/")] pub async fn index(data: StateData, req: HttpRequest) -> impl Responder { let data = data.read().unwrap(); @@ -257,60 +189,6 @@ fn resolve_path(path: PathBuf, args: &str) -> Result, BunBunError> { } } -#[cfg(test)] -mod route { - use super::*; - use serde_yaml::{from_str, to_string}; - use tempfile::NamedTempFile; - - #[test] - fn deserialize_relative_path() { - let tmpfile = NamedTempFile::new_in(".").unwrap(); - let path = format!("{}", tmpfile.path().display()); - let path = path.get(path.rfind(".").unwrap()..).unwrap(); - let path = std::path::Path::new(path); - assert!(path.is_relative()); - let path = path.to_str().unwrap(); - assert_eq!(from_str::(path).unwrap(), Route::Path(path.into())); - } - - #[test] - fn deserialize_absolute_path() { - let tmpfile = NamedTempFile::new().unwrap(); - let path = format!("{}", tmpfile.path().display()); - assert!(tmpfile.path().is_absolute()); - assert_eq!(from_str::(&path).unwrap(), Route::Path(path)); - } - - #[test] - fn deserialize_http_path() { - assert_eq!( - from_str::("http://google.com").unwrap(), - Route::External("http://google.com".into()) - ); - } - - #[test] - fn deserialize_https_path() { - assert_eq!( - from_str::("https://google.com").unwrap(), - Route::External("https://google.com".into()) - ); - } - - #[test] - fn serialize() { - assert_eq!( - &to_string(&Route::External("hello world".into())).unwrap(), - "---\nhello world" - ); - assert_eq!( - &to_string(&Route::Path("hello world".into())).unwrap(), - "---\nhello world" - ); - } -} - #[cfg(test)] mod resolve_hop { use super::*;