From e571587a8ccc71b29caf52642704c8727d96bdc3 Mon Sep 17 00:00:00 2001 From: Nikolay Kim Date: Thu, 23 Nov 2017 15:17:16 -0800 Subject: [PATCH] refactor logger middleware --- examples/basic.rs | 2 +- src/middlewares/logger.rs | 244 ++++++++++++++++++++++---------------- 2 files changed, 145 insertions(+), 101 deletions(-) diff --git a/examples/basic.rs b/examples/basic.rs index 8ffedec16..bf740dc23 100644 --- a/examples/basic.rs +++ b/examples/basic.rs @@ -50,7 +50,7 @@ fn main() { HttpServer::new( Application::default("/") // enable logger - //.middleware(middlewares::Logger::default()) + .middleware(middlewares::Logger::default()) // register simple handle r, handle all methods .handler("/index.html", index) // with path parameters diff --git a/src/middlewares/logger.rs b/src/middlewares/logger.rs index 9b2e9a627..219380754 100644 --- a/src/middlewares/logger.rs +++ b/src/middlewares/logger.rs @@ -21,12 +21,17 @@ use middlewares::{Middleware, Started, Finished}; /// ```ignore /// %a %t "%r" %s %b "%{Referrer}i" "%{User-Agent}i" %T /// ``` -/// ```rust,ignore +/// ```rust +/// extern crate actix_web; +/// use actix_web::Application; +/// use actix_web::middlewares::Logger; /// -/// let app = Application::default("/") -/// .middleware(Logger::default()) -/// .middleware(Logger::new("%a %{User-Agent}i")) -/// .finish() +/// fn main() { +/// let app = Application::default("/") +/// .middleware(Logger::default()) +/// .middleware(Logger::new("%a %{User-Agent}i")) +/// .finish(); +/// } /// ``` /// /// ## Format @@ -84,72 +89,13 @@ impl Logger { fn log(&self, req: &mut HttpRequest, resp: &HttpResponse) { let entry_time = req.extensions().get::().unwrap().0; - let response_time = time::now() - entry_time; - let response_time_ms = (response_time.num_seconds() * 1000) as f64 + - (response_time.num_nanoseconds().unwrap_or(0) as f64) / 1000000.0; - { - let render = |fmt: &mut Formatter, text: &FormatText| { - match *text { - FormatText::Str(ref string) => fmt.write_str(string), - FormatText::Percent => "%".fmt(fmt), - FormatText::RequestLine => { - if req.query_string().is_empty() { - fmt.write_fmt(format_args!( - "{} {} {:?}", - req.method(), req.path(), req.version())) - } else { - fmt.write_fmt(format_args!( - "{} {}?{} {:?}", - req.method(), req.path(), req.query_string(), req.version())) - } - }, - FormatText::ResponseStatus => resp.status().as_u16().fmt(fmt), - FormatText::ResponseSize => resp.response_size().fmt(fmt), - FormatText::Pid => unsafe{libc::getpid().fmt(fmt)}, - FormatText::Time => - fmt.write_fmt(format_args!("{:.6}", response_time_ms/1000.0)), - FormatText::TimeMillis => - fmt.write_fmt(format_args!("{:.6}", response_time_ms)), - FormatText::RemoteAddr => { - if let Some(addr) = req.remote() { - addr.fmt(fmt) - } else { - "-".fmt(fmt) - } - } - FormatText::RequestTime => { - entry_time.strftime("[%d/%b/%Y:%H:%M:%S %z]") - .unwrap() - .fmt(fmt) - } - FormatText::RequestHeader(ref name) => { - let s = if let Some(val) = req.headers().get(name) { - if let Ok(s) = val.to_str() { s } else { "-" } - } else { - "-" - }; - fmt.write_fmt(format_args!("{}", s)) - } - FormatText::ResponseHeader(ref name) => { - let s = if let Some(val) = resp.headers().get(name) { - if let Ok(s) = val.to_str() { s } else { "-" } - } else { - "-" - }; - fmt.write_fmt(format_args!("{}", s)) - } - FormatText::EnvironHeader(ref name) => { - if let Ok(val) = env::var(name) { - fmt.write_fmt(format_args!("{}", val)) - } else { - "-".fmt(fmt) - } - } - } - }; - - info!("{}", self.format.display_with(&render)); - } + let render = |fmt: &mut Formatter| { + for unit in &self.format.0 { + unit.render(fmt, req, resp, entry_time)?; + } + Ok(()) + }; + info!("{}", FormatDisplay(&render)); } } @@ -232,27 +178,6 @@ impl Format { } } -pub(crate) trait ContextDisplay<'a> { - type Item; - type Display: fmt::Display; - fn display_with(&'a self, - render: &'a Fn(&mut Formatter, &Self::Item) -> Result<(), fmt::Error>) - -> Self::Display; -} - -impl<'a> ContextDisplay<'a> for Format { - type Item = FormatText; - type Display = FormatDisplay<'a>; - fn display_with(&'a self, - render: &'a Fn(&mut Formatter, &FormatText) -> Result<(), fmt::Error>) - -> FormatDisplay<'a> { - FormatDisplay { - format: self, - render: render, - } - } -} - /// A string of text to be logged. This is either one of the data /// fields supported by the `Logger`, or a custom `String`. #[doc(hidden)] @@ -273,17 +198,136 @@ pub enum FormatText { EnvironHeader(String), } -pub(crate) struct FormatDisplay<'a> { - format: &'a Format, - render: &'a Fn(&mut Formatter, &FormatText) -> Result<(), fmt::Error>, +impl FormatText { + + fn render(&self, fmt: &mut Formatter, + req: &HttpRequest, + resp: &HttpResponse, + entry_time: time::Tm) -> Result<(), fmt::Error> + { + match *self { + FormatText::Str(ref string) => fmt.write_str(string), + FormatText::Percent => "%".fmt(fmt), + FormatText::RequestLine => { + if req.query_string().is_empty() { + fmt.write_fmt(format_args!( + "{} {} {:?}", + req.method(), req.path(), req.version())) + } else { + fmt.write_fmt(format_args!( + "{} {}?{} {:?}", + req.method(), req.path(), req.query_string(), req.version())) + } + }, + FormatText::ResponseStatus => resp.status().as_u16().fmt(fmt), + FormatText::ResponseSize => resp.response_size().fmt(fmt), + FormatText::Pid => unsafe{libc::getpid().fmt(fmt)}, + FormatText::Time => { + let response_time = time::now() - entry_time; + let response_time = (response_time.num_seconds() * 1000) as f64 + + (response_time.num_nanoseconds().unwrap_or(0) as f64) / 1000000000.0; + + fmt.write_fmt(format_args!("{:.6}", response_time)) + }, + FormatText::TimeMillis => { + let response_time = time::now() - entry_time; + let response_time_ms = (response_time.num_seconds() * 1000) as f64 + + (response_time.num_nanoseconds().unwrap_or(0) as f64) / 1000000.0; + + fmt.write_fmt(format_args!("{:.6}", response_time_ms)) + }, + FormatText::RemoteAddr => { + if let Some(addr) = req.remote() { + addr.fmt(fmt) + } else { + "-".fmt(fmt) + } + } + FormatText::RequestTime => { + entry_time.strftime("[%d/%b/%Y:%H:%M:%S %z]") + .unwrap() + .fmt(fmt) + } + FormatText::RequestHeader(ref name) => { + let s = if let Some(val) = req.headers().get(name) { + if let Ok(s) = val.to_str() { s } else { "-" } + } else { + "-" + }; + fmt.write_fmt(format_args!("{}", s)) + } + FormatText::ResponseHeader(ref name) => { + let s = if let Some(val) = resp.headers().get(name) { + if let Ok(s) = val.to_str() { s } else { "-" } + } else { + "-" + }; + fmt.write_fmt(format_args!("{}", s)) + } + FormatText::EnvironHeader(ref name) => { + if let Ok(val) = env::var(name) { + fmt.write_fmt(format_args!("{}", val)) + } else { + "-".fmt(fmt) + } + } + } + } } +pub(crate) struct FormatDisplay<'a>( + &'a Fn(&mut Formatter) -> Result<(), fmt::Error>); + impl<'a> fmt::Display for FormatDisplay<'a> { fn fmt(&self, fmt: &mut Formatter) -> Result<(), fmt::Error> { - let Format(ref format) = *self.format; - for unit in format { - (self.render)(fmt, unit)?; - } - Ok(()) + (self.0)(fmt) + } +} + +#[cfg(test)] +mod tests { + use Body; + use super::*; + use time; + use http::{Method, Version, StatusCode}; + use http::header::{self, HeaderMap}; + + #[test] + fn test_default_logger() { + let format = Format::default(); + + let mut headers = HeaderMap::new(); + headers.insert(header::USER_AGENT, header::HeaderValue::from_static("ACTIX-WEB")); + let req = HttpRequest::new( + Method::GET, "/".to_owned(), Version::HTTP_11, headers, String::new()); + let resp = HttpResponse::builder(StatusCode::OK) + .force_close().body(Body::Empty).unwrap(); + let entry_time = time::now(); + + let render = |fmt: &mut Formatter| { + for unit in format.0.iter() { + unit.render(fmt, &req, &resp, entry_time)?; + } + Ok(()) + }; + let s = format!("{}", FormatDisplay(&render)); + assert!(s.contains("GET / HTTP/1.1")); + assert!(s.contains("200 0")); + assert!(s.contains("ACTIX-WEB")); + + let req = HttpRequest::new( + Method::GET, "/?test".to_owned(), Version::HTTP_11, HeaderMap::new(), String::new()); + let resp = HttpResponse::builder(StatusCode::OK) + .force_close().body(Body::Empty).unwrap(); + let entry_time = time::now(); + + let render = |fmt: &mut Formatter| { + for unit in format.0.iter() { + unit.render(fmt, &req, &resp, entry_time)?; + } + Ok(()) + }; + let s = format!("{}", FormatDisplay(&render)); + assert!(s.contains("GET /?test HTTP/1.1")); } }