From: sigoden <sigoden@gmail.com>
Date: Sat, 4 Nov 2023 09:10:38 +0000 (+0800)
Subject: refactor: improve code quanity (#282)
X-Git-Url: https://git.ozva.co.uk/?a=commitdiff_plain;h=80ac9afe683e90316e1fc2e8cd4c793bc3c8c740;p=ozva-cloud

refactor: improve code quanity (#282)

- rename LogHttp to HttpLogger
---

diff --git a/src/args.rs b/src/args.rs
index 65fd58c..a822d70 100644
--- a/src/args.rs
+++ b/src/args.rs
@@ -8,7 +8,7 @@ use std::net::IpAddr;
 use std::path::{Path, PathBuf};
 
 use crate::auth::AccessControl;
-use crate::log_http::LogHttp;
+use crate::http_logger::HttpLogger;
 use crate::utils::encode_uri;
 
 pub fn build_cli() -> Command {
@@ -260,7 +260,7 @@ pub struct Args {
     pub assets: Option<PathBuf>,
     #[serde(deserialize_with = "deserialize_log_http")]
     #[serde(rename = "log-format")]
-    pub log_http: LogHttp,
+    pub http_logger: HttpLogger,
     pub tls_cert: Option<PathBuf>,
     pub tls_key: Option<PathBuf>,
 }
@@ -361,7 +361,7 @@ impl Args {
         }
 
         if let Some(log_format) = matches.get_one::<String>("log-format") {
-            args.log_http = log_format.parse()?;
+            args.http_logger = log_format.parse()?;
         }
 
         if let Some(assets_path) = matches.get_one::<PathBuf>("assets") {
@@ -468,7 +468,7 @@ where
     AccessControl::new(&rules).map_err(serde::de::Error::custom)
 }
 
-fn deserialize_log_http<'de, D>(deserializer: D) -> Result<LogHttp, D::Error>
+fn deserialize_log_http<'de, D>(deserializer: D) -> Result<HttpLogger, D::Error>
 where
     D: Deserializer<'de>,
 {
diff --git a/src/http_logger.rs b/src/http_logger.rs
new file mode 100644
index 0000000..0b56580
--- /dev/null
+++ b/src/http_logger.rs
@@ -0,0 +1,103 @@
+use std::{collections::HashMap, str::FromStr};
+
+use crate::{auth::get_auth_user, server::Request};
+
+pub const DEFAULT_LOG_FORMAT: &str = r#"$remote_addr "$request" $status"#;
+
+#[derive(Debug)]
+pub struct HttpLogger {
+    elements: Vec<LogElement>,
+}
+
+impl Default for HttpLogger {
+    fn default() -> Self {
+        DEFAULT_LOG_FORMAT.parse().unwrap()
+    }
+}
+
+#[derive(Debug)]
+enum LogElement {
+    Variable(String),
+    Header(String),
+    Literal(String),
+}
+
+impl HttpLogger {
+    pub fn data(&self, req: &Request) -> HashMap<String, String> {
+        let mut data = HashMap::default();
+        for element in self.elements.iter() {
+            match element {
+                LogElement::Variable(name) => match name.as_str() {
+                    "request" => {
+                        data.insert(name.to_string(), format!("{} {}", req.method(), req.uri()));
+                    }
+                    "remote_user" => {
+                        if let Some(user) =
+                            req.headers().get("authorization").and_then(get_auth_user)
+                        {
+                            data.insert(name.to_string(), user);
+                        }
+                    }
+                    _ => {}
+                },
+                LogElement::Header(name) => {
+                    if let Some(value) = req.headers().get(name).and_then(|v| v.to_str().ok()) {
+                        data.insert(name.to_string(), value.to_string());
+                    }
+                }
+                LogElement::Literal(_) => {}
+            }
+        }
+        data
+    }
+    pub fn log(&self, data: &HashMap<String, String>, err: Option<String>) {
+        if self.elements.is_empty() {
+            return;
+        }
+        let mut output = String::new();
+        for element in self.elements.iter() {
+            match element {
+                LogElement::Literal(value) => output.push_str(value.as_str()),
+                LogElement::Header(name) | LogElement::Variable(name) => {
+                    output.push_str(data.get(name).map(|v| v.as_str()).unwrap_or("-"))
+                }
+            }
+        }
+        match err {
+            Some(err) => error!("{} {}", output, err),
+            None => info!("{}", output),
+        }
+    }
+}
+
+impl FromStr for HttpLogger {
+    type Err = anyhow::Error;
+    fn from_str(s: &str) -> Result<Self, Self::Err> {
+        let mut elements = vec![];
+        let mut is_var = false;
+        let mut cache = String::new();
+        for c in format!("{s} ").chars() {
+            if c == '$' {
+                if !cache.is_empty() {
+                    elements.push(LogElement::Literal(cache.to_string()));
+                }
+                cache.clear();
+                is_var = true;
+            } else if is_var && !(c.is_alphanumeric() || c == '_') {
+                if let Some(value) = cache.strip_prefix("$http_") {
+                    elements.push(LogElement::Header(value.replace('_', "-").to_string()));
+                } else if let Some(value) = cache.strip_prefix('$') {
+                    elements.push(LogElement::Variable(value.to_string()));
+                }
+                cache.clear();
+                is_var = false;
+            }
+            cache.push(c);
+        }
+        let cache = cache.trim();
+        if !cache.is_empty() {
+            elements.push(LogElement::Literal(cache.to_string()));
+        }
+        Ok(Self { elements })
+    }
+}
diff --git a/src/log_http.rs b/src/log_http.rs
deleted file mode 100644
index 49e712e..0000000
--- a/src/log_http.rs
+++ /dev/null
@@ -1,103 +0,0 @@
-use std::{collections::HashMap, str::FromStr};
-
-use crate::{auth::get_auth_user, server::Request};
-
-pub const DEFAULT_LOG_FORMAT: &str = r#"$remote_addr "$request" $status"#;
-
-#[derive(Debug)]
-pub struct LogHttp {
-    elements: Vec<LogElement>,
-}
-
-impl Default for LogHttp {
-    fn default() -> Self {
-        DEFAULT_LOG_FORMAT.parse().unwrap()
-    }
-}
-
-#[derive(Debug)]
-enum LogElement {
-    Variable(String),
-    Header(String),
-    Literal(String),
-}
-
-impl LogHttp {
-    pub fn data(&self, req: &Request) -> HashMap<String, String> {
-        let mut data = HashMap::default();
-        for element in self.elements.iter() {
-            match element {
-                LogElement::Variable(name) => match name.as_str() {
-                    "request" => {
-                        data.insert(name.to_string(), format!("{} {}", req.method(), req.uri()));
-                    }
-                    "remote_user" => {
-                        if let Some(user) =
-                            req.headers().get("authorization").and_then(get_auth_user)
-                        {
-                            data.insert(name.to_string(), user);
-                        }
-                    }
-                    _ => {}
-                },
-                LogElement::Header(name) => {
-                    if let Some(value) = req.headers().get(name).and_then(|v| v.to_str().ok()) {
-                        data.insert(name.to_string(), value.to_string());
-                    }
-                }
-                LogElement::Literal(_) => {}
-            }
-        }
-        data
-    }
-    pub fn log(&self, data: &HashMap<String, String>, err: Option<String>) {
-        if self.elements.is_empty() {
-            return;
-        }
-        let mut output = String::new();
-        for element in self.elements.iter() {
-            match element {
-                LogElement::Literal(value) => output.push_str(value.as_str()),
-                LogElement::Header(name) | LogElement::Variable(name) => {
-                    output.push_str(data.get(name).map(|v| v.as_str()).unwrap_or("-"))
-                }
-            }
-        }
-        match err {
-            Some(err) => error!("{} {}", output, err),
-            None => info!("{}", output),
-        }
-    }
-}
-
-impl FromStr for LogHttp {
-    type Err = anyhow::Error;
-    fn from_str(s: &str) -> Result<Self, Self::Err> {
-        let mut elements = vec![];
-        let mut is_var = false;
-        let mut cache = String::new();
-        for c in format!("{s} ").chars() {
-            if c == '$' {
-                if !cache.is_empty() {
-                    elements.push(LogElement::Literal(cache.to_string()));
-                }
-                cache.clear();
-                is_var = true;
-            } else if is_var && !(c.is_alphanumeric() || c == '_') {
-                if let Some(value) = cache.strip_prefix("$http_") {
-                    elements.push(LogElement::Header(value.replace('_', "-").to_string()));
-                } else if let Some(value) = cache.strip_prefix('$') {
-                    elements.push(LogElement::Variable(value.to_string()));
-                }
-                cache.clear();
-                is_var = false;
-            }
-            cache.push(c);
-        }
-        let cache = cache.trim();
-        if !cache.is_empty() {
-            elements.push(LogElement::Literal(cache.to_string()));
-        }
-        Ok(Self { elements })
-    }
-}
diff --git a/src/main.rs b/src/main.rs
index ebc38ee..a3ac7d7 100644
--- a/src/main.rs
+++ b/src/main.rs
@@ -1,6 +1,6 @@
 mod args;
 mod auth;
-mod log_http;
+mod http_logger;
 mod logger;
 mod server;
 mod streamer;
diff --git a/src/server.rs b/src/server.rs
index c9f0b70..95de3dd 100644
--- a/src/server.rs
+++ b/src/server.rs
@@ -98,7 +98,7 @@ impl Server {
         let uri = req.uri().clone();
         let assets_prefix = &self.assets_prefix;
         let enable_cors = self.args.enable_cors;
-        let mut http_log_data = self.args.log_http.data(&req);
+        let mut http_log_data = self.args.http_logger.data(&req);
         if let Some(addr) = addr {
             http_log_data.insert("remote_addr".to_string(), addr.ip().to_string());
         }
@@ -107,7 +107,7 @@ impl Server {
             Ok(res) => {
                 http_log_data.insert("status".to_string(), res.status().as_u16().to_string());
                 if !uri.path().starts_with(assets_prefix) {
-                    self.args.log_http.log(&http_log_data, None);
+                    self.args.http_logger.log(&http_log_data, None);
                 }
                 res
             }
@@ -117,7 +117,7 @@ impl Server {
                 *res.status_mut() = status;
                 http_log_data.insert("status".to_string(), status.as_u16().to_string());
                 self.args
-                    .log_http
+                    .http_logger
                     .log(&http_log_data, Some(err.to_string()));
                 res
             }
diff --git a/tests/http_logger.rs b/tests/http_logger.rs
new file mode 100644
index 0000000..a504f59
--- /dev/null
+++ b/tests/http_logger.rs
@@ -0,0 +1,78 @@
+mod fixtures;
+mod utils;
+
+use diqwest::blocking::WithDigestAuth;
+use fixtures::{port, tmpdir, wait_for_port, Error};
+
+use assert_cmd::prelude::*;
+use assert_fs::fixture::TempDir;
+use rstest::rstest;
+use std::io::Read;
+use std::process::{Command, Stdio};
+
+#[rstest]
+#[case(&["-a", "user:pass@/:rw", "--log-format", "$remote_user"], false)]
+#[case(&["-a", "user:pass@/:rw", "--log-format", "$remote_user"], true)]
+fn log_remote_user(
+    tmpdir: TempDir,
+    port: u16,
+    #[case] args: &[&str],
+    #[case] is_basic: bool,
+) -> Result<(), Error> {
+    let mut child = Command::cargo_bin("dufs")?
+        .arg(tmpdir.path())
+        .arg("-p")
+        .arg(port.to_string())
+        .args(args)
+        .stdout(Stdio::piped())
+        .spawn()?;
+
+    wait_for_port(port);
+
+    let stdout = child.stdout.as_mut().expect("Failed to get stdout");
+
+    let req = fetch!(b"GET", &format!("http://localhost:{port}"));
+
+    let resp = if is_basic {
+        req.basic_auth("user", Some("pass")).send()?
+    } else {
+        req.send_with_digest_auth("user", "pass")?
+    };
+
+    assert_eq!(resp.status(), 200);
+
+    let mut buf = [0; 2048];
+    let buf_len = stdout.read(&mut buf)?;
+    let output = std::str::from_utf8(&buf[0..buf_len])?;
+
+    assert!(output.lines().last().unwrap().ends_with("user"));
+
+    child.kill()?;
+    Ok(())
+}
+
+#[rstest]
+#[case(&["--log-format", ""])]
+fn no_log(tmpdir: TempDir, port: u16, #[case] args: &[&str]) -> Result<(), Error> {
+    let mut child = Command::cargo_bin("dufs")?
+        .arg(tmpdir.path())
+        .arg("-p")
+        .arg(port.to_string())
+        .args(args)
+        .stdout(Stdio::piped())
+        .spawn()?;
+
+    wait_for_port(port);
+
+    let stdout = child.stdout.as_mut().expect("Failed to get stdout");
+
+    let resp = fetch!(b"GET", &format!("http://localhost:{port}")).send()?;
+    assert_eq!(resp.status(), 200);
+
+    let mut buf = [0; 2048];
+    let buf_len = stdout.read(&mut buf)?;
+    let output = std::str::from_utf8(&buf[0..buf_len])?;
+
+    assert_eq!(output.lines().last().unwrap(), "");
+    Ok(())
+}
diff --git a/tests/log_http.rs b/tests/log_http.rs
deleted file mode 100644
index a504f59..0000000
--- a/tests/log_http.rs
+++ /dev/null
@@ -1,78 +0,0 @@
-mod fixtures;
-mod utils;
-
-use diqwest::blocking::WithDigestAuth;
-use fixtures::{port, tmpdir, wait_for_port, Error};
-
-use assert_cmd::prelude::*;
-use assert_fs::fixture::TempDir;
-use rstest::rstest;
-use std::io::Read;
-use std::process::{Command, Stdio};
-
-#[rstest]
-#[case(&["-a", "user:pass@/:rw", "--log-format", "$remote_user"], false)]
-#[case(&["-a", "user:pass@/:rw", "--log-format", "$remote_user"], true)]
-fn log_remote_user(
-    tmpdir: TempDir,
-    port: u16,
-    #[case] args: &[&str],
-    #[case] is_basic: bool,
-) -> Result<(), Error> {
-    let mut child = Command::cargo_bin("dufs")?
-        .arg(tmpdir.path())
-        .arg("-p")
-        .arg(port.to_string())
-        .args(args)
-        .stdout(Stdio::piped())
-        .spawn()?;
-
-    wait_for_port(port);
-
-    let stdout = child.stdout.as_mut().expect("Failed to get stdout");
-
-    let req = fetch!(b"GET", &format!("http://localhost:{port}"));
-
-    let resp = if is_basic {
-        req.basic_auth("user", Some("pass")).send()?
-    } else {
-        req.send_with_digest_auth("user", "pass")?
-    };
-
-    assert_eq!(resp.status(), 200);
-
-    let mut buf = [0; 2048];
-    let buf_len = stdout.read(&mut buf)?;
-    let output = std::str::from_utf8(&buf[0..buf_len])?;
-
-    assert!(output.lines().last().unwrap().ends_with("user"));
-
-    child.kill()?;
-    Ok(())
-}
-
-#[rstest]
-#[case(&["--log-format", ""])]
-fn no_log(tmpdir: TempDir, port: u16, #[case] args: &[&str]) -> Result<(), Error> {
-    let mut child = Command::cargo_bin("dufs")?
-        .arg(tmpdir.path())
-        .arg("-p")
-        .arg(port.to_string())
-        .args(args)
-        .stdout(Stdio::piped())
-        .spawn()?;
-
-    wait_for_port(port);
-
-    let stdout = child.stdout.as_mut().expect("Failed to get stdout");
-
-    let resp = fetch!(b"GET", &format!("http://localhost:{port}")).send()?;
-    assert_eq!(resp.status(), 200);
-
-    let mut buf = [0; 2048];
-    let buf_len = stdout.read(&mut buf)?;
-    let output = std::str::from_utf8(&buf[0..buf_len])?;
-
-    assert_eq!(output.lines().last().unwrap(), "");
-    Ok(())
-}