From afc9aa4c92ec3c033410eeb3ec37a0ed79225c34 Mon Sep 17 00:00:00 2001 From: Rob Ede Date: Mon, 13 Dec 2021 05:44:55 +0000 Subject: [PATCH] fix tests and add docs --- actix-http/src/header/map.rs | 4 +- examples/basic.rs | 4 +- examples/uds.rs | 4 +- src/app.rs | 4 +- src/middleware/default_headers.rs | 38 ++++++++---------- src/middleware/mod.rs | 4 +- src/request_data.rs | 2 +- src/resource.rs | 2 +- src/response/customize_responder.rs | 60 +++++++++++++++++++++-------- src/response/responder.rs | 19 +++++---- src/scope.rs | 2 +- 11 files changed, 82 insertions(+), 61 deletions(-) diff --git a/actix-http/src/header/map.rs b/actix-http/src/header/map.rs index 12c8f9462..748410375 100644 --- a/actix-http/src/header/map.rs +++ b/actix-http/src/header/map.rs @@ -333,7 +333,7 @@ impl HeaderMap { } } - /// Inserts a name-value pair into the map. + /// Inserts (overrides) a name-value pair in the map. /// /// If the map already contained this key, the new value is associated with the key and all /// previous values are removed and returned as a `Removed` iterator. The key is not updated; @@ -372,7 +372,7 @@ impl HeaderMap { Removed::new(value) } - /// Inserts a name-value pair into the map. + /// Appends a name-value pair to the map. /// /// If the map already contained this key, the new value is added to the list of values /// currently associated with the key. The key is not updated; this matters for types that can diff --git a/examples/basic.rs b/examples/basic.rs index 0dfc1490f..598d13a40 100644 --- a/examples/basic.rs +++ b/examples/basic.rs @@ -22,14 +22,14 @@ async fn main() -> std::io::Result<()> { HttpServer::new(|| { App::new() - .wrap(middleware::DefaultHeaders::new().insert(("X-Version", "0.2"))) + .wrap(middleware::DefaultHeaders::new().add(("X-Version", "0.2"))) .wrap(middleware::Compress::default()) .wrap(middleware::Logger::default()) .service(index) .service(no_params) .service( web::resource("/resource2/index.html") - .wrap(middleware::DefaultHeaders::new().insert(("X-Version-R2", "0.3"))) + .wrap(middleware::DefaultHeaders::new().add(("X-Version-R2", "0.3"))) .default_service(web::route().to(HttpResponse::MethodNotAllowed)) .route(web::get().to(index_async)), ) diff --git a/examples/uds.rs b/examples/uds.rs index e99ee3822..cf0ffebde 100644 --- a/examples/uds.rs +++ b/examples/uds.rs @@ -26,14 +26,14 @@ async fn main() -> std::io::Result<()> { HttpServer::new(|| { App::new() - .wrap(middleware::DefaultHeaders::new().insert(("X-Version", "0.2"))) + .wrap(middleware::DefaultHeaders::new().add(("X-Version", "0.2"))) .wrap(middleware::Compress::default()) .wrap(middleware::Logger::default()) .service(index) .service(no_params) .service( web::resource("/resource2/index.html") - .wrap(middleware::DefaultHeaders::new().insert(("X-Version-R2", "0.3"))) + .wrap(middleware::DefaultHeaders::new().add(("X-Version-R2", "0.3"))) .default_service(web::route().to(HttpResponse::MethodNotAllowed)) .route(web::get().to(index_async)), ) diff --git a/src/app.rs b/src/app.rs index 64b23f1d4..feb35d7ae 100644 --- a/src/app.rs +++ b/src/app.rs @@ -602,7 +602,7 @@ mod tests { App::new() .wrap( DefaultHeaders::new() - .insert((header::CONTENT_TYPE, HeaderValue::from_static("0001"))), + .add((header::CONTENT_TYPE, HeaderValue::from_static("0001"))), ) .route("/test", web::get().to(HttpResponse::Ok)), ) @@ -623,7 +623,7 @@ mod tests { .route("/test", web::get().to(HttpResponse::Ok)) .wrap( DefaultHeaders::new() - .insert((header::CONTENT_TYPE, HeaderValue::from_static("0001"))), + .add((header::CONTENT_TYPE, HeaderValue::from_static("0001"))), ), ) .await; diff --git a/src/middleware/default_headers.rs b/src/middleware/default_headers.rs index bef4ea0c0..2feb9cdac 100644 --- a/src/middleware/default_headers.rs +++ b/src/middleware/default_headers.rs @@ -30,7 +30,7 @@ use crate::{ /// use actix_web::{web, http, middleware, App, HttpResponse}; /// /// let app = App::new() -/// .wrap(middleware::DefaultHeaders::new().insert("X-Version", "0.2")) +/// .wrap(middleware::DefaultHeaders::new().add(("X-Version", "0.2"))) /// .service( /// web::resource("/test") /// .route(web::get().to(|| HttpResponse::Ok())) @@ -58,12 +58,14 @@ impl DefaultHeaders { /// /// # Panics /// Panics when resolved header name or value is invalid. - pub fn insert(mut self, header: impl TryIntoHeaderPair) -> Self { + #[allow(clippy::should_implement_trait)] + pub fn add(mut self, header: impl TryIntoHeaderPair) -> Self { + // standard header terminology `insert` or `append` for this method would make the behavior + // of this middleware less obvious since it only adds the headers if they are not present + match header.try_into_header_pair() { Ok((key, value)) => Rc::get_mut(&mut self.inner) - .expect( - "DefaultHeaders has been cloned. Add all default headers before cloning.", - ) + .expect("All default headers must be added before cloning.") .headers .append(key, value), Err(err) => panic!("Invalid header: {}", err.into()), @@ -73,7 +75,7 @@ impl DefaultHeaders { } #[doc(hidden)] - #[deprecated(since = "4.0.0", note = "Prefer `insert`.")] + #[deprecated(since = "4.0.0", note = "Prefer `add`.")] pub fn header(self, key: K, value: V) -> Self where HeaderName: TryFrom, @@ -81,7 +83,7 @@ impl DefaultHeaders { HeaderValue: TryFrom, >::Error: Into, { - self.insert(( + self.add(( HeaderName::try_from(key) .map_err(Into::into) .expect("Invalid header name"), @@ -94,18 +96,12 @@ impl DefaultHeaders { /// Adds a default *Content-Type* header if response does not contain one. /// /// Default is `application/octet-stream`. - pub fn insert_content_type(self) -> Self { - self.insert(( + pub fn add_content_type(self) -> Self { + self.add(( CONTENT_TYPE, HeaderValue::from_static("application/octet-stream"), )) } - - #[doc(hidden)] - #[deprecated(since = "4.0.0", note = "Renamed to `insert_content_type`.")] - pub fn add_content_type(self) -> Self { - self.insert_content_type() - } } impl Transform for DefaultHeaders @@ -202,8 +198,8 @@ mod tests { #[actix_rt::test] async fn adding_default_headers() { let mw = DefaultHeaders::new() - .insert(("X-TEST", "0001")) - .insert(("X-TEST-TWO", HeaderValue::from_static("123"))) + .add(("X-TEST", "0001")) + .add(("X-TEST-TWO", HeaderValue::from_static("123"))) .new_transform(ok_service()) .await .unwrap(); @@ -225,7 +221,7 @@ mod tests { )) }; let mw = DefaultHeaders::new() - .insert((CONTENT_TYPE, "0001")) + .add((CONTENT_TYPE, "0001")) .new_transform(srv.into_service()) .await .unwrap(); @@ -237,7 +233,7 @@ mod tests { async fn adding_content_type() { let srv = |req: ServiceRequest| ok(req.into_response(HttpResponse::Ok().finish())); let mw = DefaultHeaders::new() - .insert_content_type() + .add_content_type() .new_transform(srv.into_service()) .await .unwrap(); @@ -253,12 +249,12 @@ mod tests { #[test] #[should_panic] fn invalid_header_name() { - DefaultHeaders::new().insert((":", "hello")); + DefaultHeaders::new().add((":", "hello")); } #[test] #[should_panic] fn invalid_header_value() { - DefaultHeaders::new().insert(("x-test", "\n")); + DefaultHeaders::new().add(("x-test", "\n")); } } diff --git a/src/middleware/mod.rs b/src/middleware/mod.rs index 6984f04b8..42d285580 100644 --- a/src/middleware/mod.rs +++ b/src/middleware/mod.rs @@ -33,7 +33,7 @@ mod tests { let _ = App::new() .wrap(Compat::new(Logger::default())) .wrap(Condition::new(true, DefaultHeaders::new())) - .wrap(DefaultHeaders::new().insert(("X-Test2", "X-Value2"))) + .wrap(DefaultHeaders::new().add(("X-Test2", "X-Value2"))) .wrap(ErrorHandlers::new().handler(StatusCode::FORBIDDEN, |res| { Ok(ErrorHandlerResponse::Response(res)) })) @@ -46,7 +46,7 @@ mod tests { .wrap(ErrorHandlers::new().handler(StatusCode::FORBIDDEN, |res| { Ok(ErrorHandlerResponse::Response(res)) })) - .wrap(DefaultHeaders::new().insert(("X-Test2", "X-Value2"))) + .wrap(DefaultHeaders::new().add(("X-Test2", "X-Value2"))) .wrap(Condition::new(true, DefaultHeaders::new())) .wrap(Compat::new(Logger::default())); diff --git a/src/request_data.rs b/src/request_data.rs index 680f3e566..b685fd0d6 100644 --- a/src/request_data.rs +++ b/src/request_data.rs @@ -17,7 +17,7 @@ use crate::{dev::Payload, error::ErrorInternalServerError, Error, FromRequest, H /// # Mutating Request Data /// Note that since extractors must output owned data, only types that `impl Clone` can use this /// extractor. A clone is taken of the required request data and can, therefore, not be directly -/// mutated in-place. To mutate request data, continue to use [`HttpRequest::extensions_mut`] or +/// mutated in-place. To mutate request data, continue to use [`HttpRequest::req_data_mut`] or /// re-insert the cloned data back into the extensions map. A `DerefMut` impl is intentionally not /// provided to make this potential foot-gun more obvious. /// diff --git a/src/resource.rs b/src/resource.rs index d4682fb92..53104930a 100644 --- a/src/resource.rs +++ b/src/resource.rs @@ -525,7 +525,7 @@ mod tests { .name("test") .wrap( DefaultHeaders::new() - .insert((header::CONTENT_TYPE, HeaderValue::from_static("0001"))), + .add((header::CONTENT_TYPE, HeaderValue::from_static("0001"))), ) .route(web::get().to(HttpResponse::Ok)), ), diff --git a/src/response/customize_responder.rs b/src/response/customize_responder.rs index 9b587a047..2bb29c504 100644 --- a/src/response/customize_responder.rs +++ b/src/response/customize_responder.rs @@ -8,7 +8,9 @@ use actix_http::{ use crate::{BoxError, HttpRequest, HttpResponse, Responder}; -/// Allows overriding status code and headers for a responder. +/// Allows overriding status code and headers for a [`Responder`]. +/// +/// Created by the [`Responder::customize`] method. pub struct CustomizeResponder { inner: CustomizeResponderInner, error: Option, @@ -36,12 +38,15 @@ impl CustomizeResponder { /// Override a status code for the Responder's response. /// + /// # Examples /// ``` - /// use actix_web::{HttpRequest, Responder, http::StatusCode}; + /// use actix_web::{Responder, http::StatusCode, test::TestRequest}; /// - /// fn index(req: HttpRequest) -> impl Responder { - /// "Welcome!".with_status(StatusCode::OK) - /// } + /// let responder = "Welcome!".customize().with_status(StatusCode::ACCEPTED); + /// + /// let request = TestRequest::default().to_http_request(); + /// let response = responder.respond_to(&request); + /// assert_eq!(response.status(), StatusCode::ACCEPTED); /// ``` pub fn with_status(mut self, status: StatusCode) -> Self { if let Some(inner) = self.inner() { @@ -51,24 +56,26 @@ impl CustomizeResponder { self } - /// Insert header to the final response. + /// Insert (override) header in the final response. /// /// Overrides other headers with the same name. + /// See [`HeaderMap::insert`](crate::http::header::HeaderMap::insert). /// + /// Headers added with this method will be inserted before those added + /// with [`append_header`](Self::append_header). As such, header(s) can be overridden with more + /// than one new header by first calling `insert_header` followed by `append_header`. + /// + /// # Examples /// ``` - /// use actix_web::{web, HttpRequest, Responder}; - /// use serde::Serialize; + /// use actix_web::{Responder, test::TestRequest}; /// - /// #[derive(Serialize)] - /// struct MyObj { - /// name: String, - /// } + /// let responder = "Hello world!" + /// .customize() + /// .insert_header(("x-version", "1.2.3")); /// - /// fn index(req: HttpRequest) -> impl Responder { - /// web::Json(MyObj { name: "Name".to_string() }) - /// .with_header(("x-version", "1.2.3")) - /// .with_header(("x-version", "1.2.3")) - /// } + /// let request = TestRequest::default().to_http_request(); + /// let response = responder.respond_to(&request); + /// assert_eq!(response.headers().get("x-version").unwrap(), "1.2.3"); /// ``` pub fn insert_header(mut self, header: impl TryIntoHeaderPair) -> Self { if let Some(inner) = self.inner() { @@ -83,6 +90,25 @@ impl CustomizeResponder { self } + /// Append header to the final response. + /// + /// Unlike [`insert_header`](Self::insert_header), this will not override existing headers. + /// See [`HeaderMap::append`](crate::http::header::HeaderMap::append). + /// + /// Headers added here are appended _after_ additions/overrides from `insert_header`. + /// + /// # Examples + /// ``` + /// use actix_web::{Responder, test::TestRequest}; + /// + /// let responder = "Hello world!" + /// .customize() + /// .append_header(("x-version", "1.2.3")); + /// + /// let request = TestRequest::default().to_http_request(); + /// let response = responder.respond_to(&request); + /// assert_eq!(response.headers().get("x-version").unwrap(), "1.2.3"); + /// ``` pub fn append_header(mut self, header: impl TryIntoHeaderPair) -> Self { if let Some(inner) = self.inner() { match header.try_into_header_pair() { diff --git a/src/response/responder.rs b/src/response/responder.rs index fac3cbeaa..b319e23a5 100644 --- a/src/response/responder.rs +++ b/src/response/responder.rs @@ -21,24 +21,23 @@ pub trait Responder { /// Convert self to `HttpResponse`. fn respond_to(self, req: &HttpRequest) -> HttpResponse; - /// Wraps responder in [`CustomizeResponder`] that allows modification of response details. + /// Wraps responder to allow alteration of its response. /// - /// See [`CustomizeResponder`] docs for more. + /// See [`CustomizeResponder`] docs for its capabilities. /// /// # Examples /// ``` - /// use actix_web::{Responder, test}; - /// - /// let request = test::TestRequest::default().to_http_request(); + /// use actix_web::{Responder, http::StatusCode, test::TestRequest}; /// /// let responder = "Hello world!" /// .customize() - /// .with_status(400) - /// .insert_header(("x-hello", "world")) + /// .with_status(StatusCode::BAD_REQUEST) + /// .insert_header(("x-hello", "world")); /// - /// let response = res.respond_to(&req); - /// assert_eq!(res.status(), 400); - /// assert_eq!(res.headers().get("x-hello").unwrap(), "world"); + /// let request = TestRequest::default().to_http_request(); + /// let response = responder.respond_to(&request); + /// assert_eq!(response.status(), StatusCode::BAD_REQUEST); + /// assert_eq!(response.headers().get("x-hello").unwrap(), "world"); /// ``` #[inline] fn customize(self) -> CustomizeResponder diff --git a/src/scope.rs b/src/scope.rs index eaffdc1b4..c35584770 100644 --- a/src/scope.rs +++ b/src/scope.rs @@ -935,7 +935,7 @@ mod tests { web::scope("app") .wrap( DefaultHeaders::new() - .insert((header::CONTENT_TYPE, HeaderValue::from_static("0001"))), + .add((header::CONTENT_TYPE, HeaderValue::from_static("0001"))), ) .service(web::resource("/test").route(web::get().to(HttpResponse::Ok))), ),