From 8c7e4cd6d55e0b52d7f78748227dbeca7b99d0e2 Mon Sep 17 00:00:00 2001 From: Rob Ede Date: Fri, 24 Dec 2021 10:30:51 +0000 Subject: [PATCH] fix payload documentation --- actix-http/src/h1/payload.rs | 59 +++++++++++++++++++++--------------- actix-http/src/payload.rs | 2 +- actix-http/src/test.rs | 2 +- awc/CHANGES.md | 3 ++ awc/src/test.rs | 5 +-- src/response/builder.rs | 36 +++++++++------------- src/test/test_request.rs | 21 +++++++------ 7 files changed, 69 insertions(+), 59 deletions(-) diff --git a/actix-http/src/h1/payload.rs b/actix-http/src/h1/payload.rs index dac954d8f..4d031c15a 100644 --- a/actix-http/src/h1/payload.rs +++ b/actix-http/src/h1/payload.rs @@ -1,9 +1,12 @@ //! Payload stream -use std::cell::RefCell; -use std::collections::VecDeque; -use std::pin::Pin; -use std::rc::{Rc, Weak}; -use std::task::{Context, Poll, Waker}; + +use std::{ + cell::RefCell, + collections::VecDeque, + pin::Pin, + rc::{Rc, Weak}, + task::{Context, Poll, Waker}, +}; use bytes::Bytes; use futures_core::Stream; @@ -22,39 +25,32 @@ pub enum PayloadStatus { /// Buffered stream of bytes chunks /// -/// Payload stores chunks in a vector. First chunk can be received with `.readany()` method. -/// Payload stream is not thread safe. Payload does not notify current task when new data -/// is available. +/// Payload stores chunks in a vector. First chunk can be received with `poll_next`. Payload does +/// not notify current task when new data is available. /// -/// Payload stream can be used as `Response` body stream. +/// Payload can be used as `Response` body stream. #[derive(Debug)] pub struct Payload { inner: Rc>, } impl Payload { - /// Create payload stream. + /// Creates a payload stream. /// - /// This method construct two objects responsible for bytes stream - /// generation. - /// - /// * `PayloadSender` - *Sender* side of the stream - /// - /// * `Payload` - *Receiver* side of the stream + /// This method construct two objects responsible for bytes stream generation: + /// - `PayloadSender` - *Sender* side of the stream + /// - `Payload` - *Receiver* side of the stream pub fn create(eof: bool) -> (PayloadSender, Payload) { let shared = Rc::new(RefCell::new(Inner::new(eof))); ( - PayloadSender { - inner: Rc::downgrade(&shared), - }, + PayloadSender::new(Rc::downgrade(&shared)), Payload { inner: shared }, ) } - /// Create empty payload - #[doc(hidden)] - pub fn empty() -> Payload { + /// Creates an empty payload. + pub(crate) fn empty() -> Payload { Payload { inner: Rc::new(RefCell::new(Inner::new(true))), } @@ -86,7 +82,7 @@ impl Stream for Payload { self: Pin<&mut Self>, cx: &mut Context<'_>, ) -> Poll>> { - self.inner.borrow_mut().readany(cx) + Pin::new(&mut *self.inner.borrow_mut()).poll_next(cx) } } @@ -96,6 +92,10 @@ pub struct PayloadSender { } impl PayloadSender { + fn new(inner: Weak>) -> Self { + Self { inner } + } + #[inline] pub fn set_error(&mut self, err: PayloadError) { if let Some(shared) = self.inner.upgrade() { @@ -219,7 +219,10 @@ impl Inner { self.len } - fn readany(&mut self, cx: &mut Context<'_>) -> Poll>> { + fn poll_next( + mut self: Pin<&mut Self>, + cx: &mut Context<'_>, + ) -> Poll>> { if let Some(data) = self.items.pop_front() { self.len -= data.len(); self.need_read = self.len < MAX_BUFFER_SIZE; @@ -249,12 +252,18 @@ impl Inner { #[cfg(test)] mod tests { + use std::panic::{RefUnwindSafe, UnwindSafe}; + use actix_utils::future::poll_fn; - use static_assertions::assert_impl_all; + use static_assertions::{assert_impl_all, assert_not_impl_any}; use super::*; assert_impl_all!(Payload: Unpin); + assert_not_impl_any!(Payload: Send, Sync, UnwindSafe, RefUnwindSafe); + + assert_impl_all!(Inner: Unpin, Send, Sync); + assert_not_impl_any!(Inner: UnwindSafe, RefUnwindSafe); #[actix_rt::test] async fn test_unread_data() { diff --git a/actix-http/src/payload.rs b/actix-http/src/payload.rs index 408767ba0..4a2107c0f 100644 --- a/actix-http/src/payload.rs +++ b/actix-http/src/payload.rs @@ -23,7 +23,7 @@ pin_project_lite::pin_project! { H1 { payload: crate::h1::Payload }, H2 { payload: crate::h2::Payload }, Stream { #[pin] payload: S }, -} + } } impl From for Payload { diff --git a/actix-http/src/test.rs b/actix-http/src/test.rs index ea80345fe..1f76498ef 100644 --- a/actix-http/src/test.rs +++ b/actix-http/src/test.rs @@ -120,7 +120,7 @@ impl TestRequest { } /// Set request payload. - pub fn set_payload>(&mut self, data: B) -> &mut Self { + pub fn set_payload(&mut self, data: impl Into) -> &mut Self { let mut payload = crate::h1::Payload::empty(); payload.unread_data(data.into()); parts(&mut self.0).payload = Some(payload.into()); diff --git a/awc/CHANGES.md b/awc/CHANGES.md index b5144b7a2..b8decdc32 100644 --- a/awc/CHANGES.md +++ b/awc/CHANGES.md @@ -3,8 +3,11 @@ ## Unreleased - 2021-xx-xx - Rename `Connector::{ssl => openssl}`. [#2503] - Improve `Client` instantiation efficiency when using `openssl` by only building connectors once. [#2503] +- `ClientResponse` is no longer `Unpin`. [#2545] +- `impl Stream` for `ClientResponse` no longer requires the body type be `Unpin`. [#2545] [#2503]: https://github.com/actix/actix-web/pull/2503 +[#2545]: https://github.com/actix/actix-web/pull/2545 ## 3.0.0-beta.14 - 2021-12-17 diff --git a/awc/src/test.rs b/awc/src/test.rs index 1b41efc93..96ae1f0a1 100644 --- a/awc/src/test.rs +++ b/awc/src/test.rs @@ -65,7 +65,7 @@ impl TestResponse { /// Set response's payload pub fn set_payload>(mut self, data: B) -> Self { - let mut payload = h1::Payload::empty(); + let (_, mut payload) = h1::Payload::create(true); payload.unread_data(data.into()); self.payload = Some(payload.into()); self @@ -90,7 +90,8 @@ impl TestResponse { if let Some(pl) = self.payload { ClientResponse::new(head, pl) } else { - ClientResponse::new(head, h1::Payload::empty().into()) + let (_, payload) = h1::Payload::create(true); + ClientResponse::new(head, payload.into()) } } } diff --git a/src/response/builder.rs b/src/response/builder.rs index b500ab331..93d8ab567 100644 --- a/src/response/builder.rs +++ b/src/response/builder.rs @@ -429,9 +429,12 @@ mod tests { use actix_http::body; use super::*; - use crate::http::{ - header::{self, HeaderValue, CONTENT_TYPE}, - StatusCode, + use crate::{ + http::{ + header::{self, HeaderValue, CONTENT_TYPE}, + StatusCode, + }, + test::assert_body_eq, }; #[test] @@ -472,32 +475,23 @@ mod tests { #[actix_rt::test] async fn test_json() { - let resp = HttpResponse::Ok().json(vec!["v1", "v2", "v3"]); - let ct = resp.headers().get(CONTENT_TYPE).unwrap(); + let res = HttpResponse::Ok().json(vec!["v1", "v2", "v3"]); + let ct = res.headers().get(CONTENT_TYPE).unwrap(); assert_eq!(ct, HeaderValue::from_static("application/json")); - assert_eq!( - body::to_bytes(resp.into_body()).await.unwrap().as_ref(), - br#"["v1","v2","v3"]"# - ); + assert_body_eq!(res, br#"["v1","v2","v3"]"#); - let resp = HttpResponse::Ok().json(&["v1", "v2", "v3"]); - let ct = resp.headers().get(CONTENT_TYPE).unwrap(); + let res = HttpResponse::Ok().json(&["v1", "v2", "v3"]); + let ct = res.headers().get(CONTENT_TYPE).unwrap(); assert_eq!(ct, HeaderValue::from_static("application/json")); - assert_eq!( - body::to_bytes(resp.into_body()).await.unwrap().as_ref(), - br#"["v1","v2","v3"]"# - ); + assert_body_eq!(res, br#"["v1","v2","v3"]"#); // content type override - let resp = HttpResponse::Ok() + let res = HttpResponse::Ok() .insert_header((CONTENT_TYPE, "text/json")) .json(&vec!["v1", "v2", "v3"]); - let ct = resp.headers().get(CONTENT_TYPE).unwrap(); + let ct = res.headers().get(CONTENT_TYPE).unwrap(); assert_eq!(ct, HeaderValue::from_static("text/json")); - assert_eq!( - body::to_bytes(resp.into_body()).await.unwrap().as_ref(), - br#"["v1","v2","v3"]"# - ); + assert_body_eq!(res, br#"["v1","v2","v3"]"#); } #[actix_rt::test] diff --git a/src/test/test_request.rs b/src/test/test_request.rs index fd3355ef3..5c4de9084 100644 --- a/src/test/test_request.rs +++ b/src/test/test_request.rs @@ -174,25 +174,28 @@ impl TestRequest { } /// Set request payload. - pub fn set_payload>(mut self, data: B) -> Self { + pub fn set_payload(mut self, data: impl Into) -> Self { self.req.set_payload(data); self } - /// Serialize `data` to a URL encoded form and set it as the request payload. The `Content-Type` - /// header is set to `application/x-www-form-urlencoded`. - pub fn set_form(mut self, data: &T) -> Self { - let bytes = serde_urlencoded::to_string(data) + /// Serialize `data` to a URL encoded form and set it as the request payload. + /// + /// The `Content-Type` header is set to `application/x-www-form-urlencoded`. + pub fn set_form(mut self, data: impl Serialize) -> Self { + let bytes = serde_urlencoded::to_string(&data) .expect("Failed to serialize test data as a urlencoded form"); self.req.set_payload(bytes); self.req.insert_header(ContentType::form_url_encoded()); self } - /// Serialize `data` to JSON and set it as the request payload. The `Content-Type` header is - /// set to `application/json`. - pub fn set_json(mut self, data: &T) -> Self { - let bytes = serde_json::to_string(data).expect("Failed to serialize test data to json"); + /// Serialize `data` to JSON and set it as the request payload. + /// + /// The `Content-Type` header is set to `application/json`. + pub fn set_json(mut self, data: impl Serialize) -> Self { + let bytes = + serde_json::to_string(&data).expect("Failed to serialize test data to json"); self.req.set_payload(bytes); self.req.insert_header(ContentType::json()); self