api: Replace bytes::Buf by AsRef<u8> for reading

This allows us to switch back to serde_json::from_slice instead of
serde_json::from_reader, because the latter is significantly slower.

See https://github.com/serde-rs/json/issues/160
This commit is contained in:
Jonas Platte 2021-04-13 21:57:23 +02:00
parent e4ae2a40ee
commit c1693569f1
No known key found for this signature in database
GPG Key ID: CC154DE0E30B7C67
18 changed files with 57 additions and 66 deletions

View File

@ -10,7 +10,6 @@ impl Request {
error_ty: &TokenStream, error_ty: &TokenStream,
ruma_api: &TokenStream, ruma_api: &TokenStream,
) -> TokenStream { ) -> TokenStream {
let bytes = quote! { #ruma_api::exports::bytes };
let http = quote! { #ruma_api::exports::http }; let http = quote! { #ruma_api::exports::http };
let percent_encoding = quote! { #ruma_api::exports::percent_encoding }; let percent_encoding = quote! { #ruma_api::exports::percent_encoding };
let ruma_serde = quote! { #ruma_api::exports::ruma_serde }; let ruma_serde = quote! { #ruma_api::exports::ruma_serde };
@ -159,15 +158,17 @@ impl Request {
RequestBody #body_lifetimes RequestBody #body_lifetimes
as #ruma_serde::Outgoing as #ruma_serde::Outgoing
>::Incoming = { >::Incoming = {
let body = request.into_body(); let body = ::std::convert::AsRef::<[::std::primitive::u8]>::as_ref(
if #bytes::Buf::has_remaining(&body) { request.body(),
#serde_json::from_reader(#bytes::Buf::reader(body))? );
} else {
#serde_json::from_slice(match body {
// If the request body is completely empty, pretend it is an empty JSON // If the request body is completely empty, pretend it is an empty JSON
// object instead. This allows requests with only optional body parameters // object instead. This allows requests with only optional body parameters
// to be deserialized in that case. // to be deserialized in that case.
#serde_json::from_str("{}")? [] => b"{}",
} b => b,
})?
}; };
} }
} else { } else {
@ -184,13 +185,8 @@ impl Request {
} else if let Some(field) = self.newtype_raw_body_field() { } else if let Some(field) = self.newtype_raw_body_field() {
let field_name = field.ident.as_ref().expect("expected field to have an identifier"); let field_name = field.ident.as_ref().expect("expected field to have an identifier");
let parse = quote! { let parse = quote! {
let #field_name = { let #field_name =
let mut reader = #bytes::Buf::reader(request.into_body()); ::std::convert::AsRef::<[u8]>::as_ref(request.body()).to_vec();
let mut vec = ::std::vec::Vec::new();
::std::io::Read::read_to_end(&mut reader, &mut vec)
.expect("reading from a bytes::Buf never fails");
vec
};
}; };
(parse, quote! { #field_name, }) (parse, quote! { #field_name, })

View File

@ -5,7 +5,6 @@ use super::{Response, ResponseField};
impl Response { impl Response {
pub fn expand_incoming(&self, error_ty: &TokenStream, ruma_api: &TokenStream) -> TokenStream { pub fn expand_incoming(&self, error_ty: &TokenStream, ruma_api: &TokenStream) -> TokenStream {
let bytes = quote! { #ruma_api::exports::bytes };
let http = quote! { #ruma_api::exports::http }; let http = quote! { #ruma_api::exports::http };
let ruma_serde = quote! { #ruma_api::exports::ruma_serde }; let ruma_serde = quote! { #ruma_api::exports::ruma_serde };
let serde_json = quote! { #ruma_api::exports::serde_json }; let serde_json = quote! { #ruma_api::exports::serde_json };
@ -25,15 +24,17 @@ impl Response {
ResponseBody ResponseBody
as #ruma_serde::Outgoing as #ruma_serde::Outgoing
>::Incoming = { >::Incoming = {
let body = response.into_body(); let body = ::std::convert::AsRef::<[::std::primitive::u8]>::as_ref(
if #bytes::Buf::has_remaining(&body) { response.body(),
#serde_json::from_reader(#bytes::Buf::reader(body))? );
} else {
#serde_json::from_slice(match body {
// If the response body is completely empty, pretend it is an empty // If the response body is completely empty, pretend it is an empty
// JSON object instead. This allows responses with only optional body // JSON object instead. This allows responses with only optional body
// parameters to be deserialized in that case. // parameters to be deserialized in that case.
#serde_json::from_str("{}")? [] => b"{}",
} b => b,
})?
}; };
} }
} else { } else {
@ -93,11 +94,10 @@ impl Response {
ResponseField::NewtypeRawBody(_) => { ResponseField::NewtypeRawBody(_) => {
new_type_raw_body = Some(quote! { new_type_raw_body = Some(quote! {
#field_name: { #field_name: {
let mut reader = #bytes::Buf::reader(response.into_body()); ::std::convert::AsRef::<[::std::primitive::u8]>::as_ref(
let mut vec = ::std::vec::Vec::new(); response.body(),
::std::io::Read::read_to_end(&mut reader, &mut vec) )
.expect("reading from a bytes::Buf never fails"); .to_vec()
vec
} }
}); });
// skip adding to the vec // skip adding to the vec
@ -120,7 +120,7 @@ impl Response {
impl #ruma_api::IncomingResponse for Response { impl #ruma_api::IncomingResponse for Response {
type EndpointError = #error_ty; type EndpointError = #error_ty;
fn try_from_http_response<T: #bytes::Buf>( fn try_from_http_response<T: ::std::convert::AsRef<[::std::primitive::u8]>>(
response: #http::Response<T>, response: #http::Response<T>,
) -> ::std::result::Result< ) -> ::std::result::Result<
Self, Self,
@ -130,15 +130,17 @@ impl Response {
#extract_response_headers #extract_response_headers
#typed_response_body_decl #typed_response_body_decl
Ok(Self { ::std::result::Result::Ok(Self {
#response_init_fields #response_init_fields
}) })
} else { } else {
match <#error_ty as #ruma_api::EndpointError>::try_from_http_response( match <#error_ty as #ruma_api::EndpointError>::try_from_http_response(
response response
) { ) {
Ok(err) => Err(#ruma_api::error::ServerError::Known(err).into()), ::std::result::Result::Ok(err) => {
Err(response_err) => { Err(#ruma_api::error::ServerError::Known(err).into())
}
::std::result::Result::Err(response_err) => {
Err(#ruma_api::error::ServerError::Unknown(response_err).into()) Err(#ruma_api::error::ServerError::Unknown(response_err).into())
} }
} }

View File

@ -19,7 +19,6 @@ all-features = true
rustdoc-args = ["--cfg", "docsrs"] rustdoc-args = ["--cfg", "docsrs"]
[dependencies] [dependencies]
bytes = "1.0.1"
http = "0.2.2" http = "0.2.2"
percent-encoding = "2.1.0" percent-encoding = "2.1.0"
ruma-api-macros = { version = "=0.17.0-alpha.2", path = "../ruma-api-macros" } ruma-api-macros = { version = "=0.17.0-alpha.2", path = "../ruma-api-macros" }

View File

@ -4,7 +4,6 @@
use std::{error::Error as StdError, fmt}; use std::{error::Error as StdError, fmt};
use bytes::Buf;
use thiserror::Error; use thiserror::Error;
use crate::{EndpointError, OutgoingResponse}; use crate::{EndpointError, OutgoingResponse};
@ -21,7 +20,7 @@ impl OutgoingResponse for Void {
} }
impl EndpointError for Void { impl EndpointError for Void {
fn try_from_http_response<T: Buf>( fn try_from_http_response<T: AsRef<[u8]>>(
_response: http::Response<T>, _response: http::Response<T>,
) -> Result<Self, ResponseDeserializationError> { ) -> Result<Self, ResponseDeserializationError> {
Err(ResponseDeserializationError::none()) Err(ResponseDeserializationError::none())

View File

@ -22,7 +22,6 @@ compile_error!("ruma_api's Cargo features only exist as a workaround are not mea
use std::{convert::TryInto as _, error::Error as StdError}; use std::{convert::TryInto as _, error::Error as StdError};
use bytes::Buf;
use http::Method; use http::Method;
use ruma_identifiers::UserId; use ruma_identifiers::UserId;
@ -204,7 +203,6 @@ pub mod error;
/// It is not considered part of ruma-api's public API. /// It is not considered part of ruma-api's public API.
#[doc(hidden)] #[doc(hidden)]
pub mod exports { pub mod exports {
pub use bytes;
pub use http; pub use http;
pub use percent_encoding; pub use percent_encoding;
pub use ruma_serde; pub use ruma_serde;
@ -246,7 +244,7 @@ pub trait IncomingResponse: Sized {
type EndpointError: EndpointError; type EndpointError: EndpointError;
/// Tries to convert the given `http::Response` into this response type. /// Tries to convert the given `http::Response` into this response type.
fn try_from_http_response<T: Buf>( fn try_from_http_response<T: AsRef<[u8]>>(
response: http::Response<T>, response: http::Response<T>,
) -> Result<Self, FromHttpResponseError<Self::EndpointError>>; ) -> Result<Self, FromHttpResponseError<Self::EndpointError>>;
} }
@ -301,7 +299,9 @@ pub trait IncomingRequest: Sized {
const METADATA: Metadata; const METADATA: Metadata;
/// Tries to turn the given `http::Request` into this request type. /// Tries to turn the given `http::Request` into this request type.
fn try_from_http_request<T: Buf>(req: http::Request<T>) -> Result<Self, FromHttpRequestError>; fn try_from_http_request<T: AsRef<[u8]>>(
req: http::Request<T>,
) -> Result<Self, FromHttpRequestError>;
} }
/// A request type for a Matrix API endpoint, used for sending responses. /// A request type for a Matrix API endpoint, used for sending responses.
@ -319,7 +319,7 @@ pub trait EndpointError: OutgoingResponse + StdError + Sized + 'static {
/// ///
/// This will always return `Err` variant when no `error` field is defined in /// This will always return `Err` variant when no `error` field is defined in
/// the `ruma_api` macro. /// the `ruma_api` macro.
fn try_from_http_response<T: Buf>( fn try_from_http_response<T: AsRef<[u8]>>(
response: http::Response<T>, response: http::Response<T>,
) -> Result<Self, error::ResponseDeserializationError>; ) -> Result<Self, error::ResponseDeserializationError>;
} }

View File

@ -48,7 +48,7 @@ fn request_serde() {
}; };
let http_req = req.clone().try_into_http_request("https://homeserver.tld", None).unwrap(); let http_req = req.clone().try_into_http_request("https://homeserver.tld", None).unwrap();
let req2 = Request::try_from_http_request(http_req.map(std::io::Cursor::new)).unwrap(); let req2 = Request::try_from_http_request(http_req).unwrap();
assert_eq!(req.hello, req2.hello); assert_eq!(req.hello, req2.hello);
assert_eq!(req.world, req2.world); assert_eq!(req.world, req2.world);

View File

@ -2,7 +2,6 @@
use std::convert::TryFrom; use std::convert::TryFrom;
use bytes::Buf;
use http::{header::CONTENT_TYPE, method::Method}; use http::{header::CONTENT_TYPE, method::Method};
use ruma_api::{ use ruma_api::{
error::{FromHttpRequestError, FromHttpResponseError, IntoHttpError, ServerError, Void}, error::{FromHttpRequestError, FromHttpResponseError, IntoHttpError, ServerError, Void},
@ -67,7 +66,7 @@ impl IncomingRequest for Request {
const METADATA: Metadata = METADATA; const METADATA: Metadata = METADATA;
fn try_from_http_request<T: Buf>( fn try_from_http_request<T: AsRef<[u8]>>(
request: http::Request<T>, request: http::Request<T>,
) -> Result<Self, FromHttpRequestError> { ) -> Result<Self, FromHttpRequestError> {
let path_segments: Vec<&str> = request.uri().path()[1..].split('/').collect(); let path_segments: Vec<&str> = request.uri().path()[1..].split('/').collect();
@ -78,7 +77,7 @@ impl IncomingRequest for Request {
TryFrom::try_from(&*decoded)? TryFrom::try_from(&*decoded)?
}; };
let request_body: RequestBody = serde_json::from_reader(request.into_body().reader())?; let request_body: RequestBody = serde_json::from_slice(request.body().as_ref())?;
Ok(Request { room_id: request_body.room_id, room_alias }) Ok(Request { room_id: request_body.room_id, room_alias })
} }
@ -100,7 +99,7 @@ impl Outgoing for Response {
impl IncomingResponse for Response { impl IncomingResponse for Response {
type EndpointError = Void; type EndpointError = Void;
fn try_from_http_response<T: Buf>( fn try_from_http_response<T: AsRef<[u8]>>(
http_response: http::Response<T>, http_response: http::Response<T>,
) -> Result<Self, FromHttpResponseError<Void>> { ) -> Result<Self, FromHttpResponseError<Void>> {
if http_response.status().as_u16() < 400 { if http_response.status().as_u16() < 400 {

View File

@ -1,4 +1,3 @@
use bytes::Buf;
use ruma_api::{ use ruma_api::{
error::{FromHttpResponseError, IntoHttpError, Void}, error::{FromHttpResponseError, IntoHttpError, Void},
ruma_api, IncomingResponse, OutgoingResponse, ruma_api, IncomingResponse, OutgoingResponse,
@ -28,7 +27,7 @@ pub struct Response;
impl IncomingResponse for Response { impl IncomingResponse for Response {
type EndpointError = Void; type EndpointError = Void;
fn try_from_http_response<T: Buf>( fn try_from_http_response<T: AsRef<[u8]>>(
_: http::Response<T>, _: http::Response<T>,
) -> Result<Self, FromHttpResponseError<Void>> { ) -> Result<Self, FromHttpResponseError<Void>> {
todo!() todo!()

View File

@ -17,7 +17,6 @@ edition = "2018"
[dependencies] [dependencies]
assign = "1.1.1" assign = "1.1.1"
bytes = "1.0.1"
http = "0.2.2" http = "0.2.2"
js_int = { version = "0.2.0", features = ["serde"] } js_int = { version = "0.2.0", features = ["serde"] }
maplit = "1.0.2" maplit = "1.0.2"

View File

@ -2,14 +2,13 @@
use std::{collections::BTreeMap, fmt, time::Duration}; use std::{collections::BTreeMap, fmt, time::Duration};
use bytes::Buf;
use ruma_api::{ use ruma_api::{
error::{IntoHttpError, ResponseDeserializationError}, error::{IntoHttpError, ResponseDeserializationError},
EndpointError, OutgoingResponse, EndpointError, OutgoingResponse,
}; };
use ruma_identifiers::RoomVersionId; use ruma_identifiers::RoomVersionId;
use serde::{Deserialize, Serialize}; use serde::{Deserialize, Serialize};
use serde_json::{from_reader as from_json_reader, to_vec as to_json_vec, Value as JsonValue}; use serde_json::{from_slice as from_json_slice, to_vec as to_json_vec, Value as JsonValue};
/// Deserialize and Serialize implementations for ErrorKind. /// Deserialize and Serialize implementations for ErrorKind.
/// Separate module because it's a lot of code. /// Separate module because it's a lot of code.
@ -206,11 +205,11 @@ pub struct Error {
} }
impl EndpointError for Error { impl EndpointError for Error {
fn try_from_http_response<T: Buf>( fn try_from_http_response<T: AsRef<[u8]>>(
response: http::Response<T>, response: http::Response<T>,
) -> Result<Self, ResponseDeserializationError> { ) -> Result<Self, ResponseDeserializationError> {
let status = response.status(); let status = response.status();
let error_body: ErrorBody = from_json_reader(response.into_body().reader())?; let error_body: ErrorBody = from_json_slice(response.body().as_ref())?;
Ok(error_body.into_error(status)) Ok(error_body.into_error(status))
} }
} }

View File

@ -104,7 +104,7 @@ impl ruma_api::IncomingRequest for IncomingRequest {
const METADATA: ruma_api::Metadata = METADATA; const METADATA: ruma_api::Metadata = METADATA;
fn try_from_http_request<T: bytes::Buf>( fn try_from_http_request<T: AsRef<[u8]>>(
request: http::Request<T>, request: http::Request<T>,
) -> Result<Self, ruma_api::error::FromHttpRequestError> { ) -> Result<Self, ruma_api::error::FromHttpRequestError> {
use std::convert::TryFrom; use std::convert::TryFrom;
@ -129,7 +129,7 @@ impl ruma_api::IncomingRequest for IncomingRequest {
let content = { let content = {
let event_type = let event_type =
percent_encoding::percent_decode(path_segments[6].as_bytes()).decode_utf8()?; percent_encoding::percent_decode(path_segments[6].as_bytes()).decode_utf8()?;
let body: Box<RawJsonValue> = serde_json::from_reader(body.reader())?; let body: Box<RawJsonValue> = serde_json::from_slice(body.as_ref())?;
AnyMessageEventContent::from_parts(&event_type, body)? AnyMessageEventContent::from_parts(&event_type, body)?
}; };

View File

@ -77,9 +77,8 @@ mod tests {
http::Request::builder() http::Request::builder()
.method("PUT") .method("PUT")
.uri("https://bar.org/_matrix/client/r0/profile/@foo:bar.org/avatar_url") .uri("https://bar.org/_matrix/client/r0/profile/@foo:bar.org/avatar_url")
.body(std::io::Cursor::new( .body(serde_json::to_vec(&serde_json::json!({ "avatar_url": "" })).unwrap())
serde_json::to_vec(&serde_json::json!({ "avatar_url": "" })).unwrap(), .unwrap(),
)).unwrap(),
).unwrap(), ).unwrap(),
IncomingRequest { user_id, avatar_url: None } if user_id == "@foo:bar.org" IncomingRequest { user_id, avatar_url: None } if user_id == "@foo:bar.org"
); );

View File

@ -108,7 +108,7 @@ impl ruma_api::IncomingRequest for IncomingRequest {
const METADATA: ruma_api::Metadata = METADATA; const METADATA: ruma_api::Metadata = METADATA;
fn try_from_http_request<T: bytes::Buf>( fn try_from_http_request<T: AsRef<[u8]>>(
request: http::Request<T>, request: http::Request<T>,
) -> Result<Self, ruma_api::error::FromHttpRequestError> { ) -> Result<Self, ruma_api::error::FromHttpRequestError> {
use std::convert::TryFrom; use std::convert::TryFrom;

View File

@ -108,7 +108,7 @@ impl ruma_api::IncomingRequest for IncomingRequest {
const METADATA: ruma_api::Metadata = METADATA; const METADATA: ruma_api::Metadata = METADATA;
fn try_from_http_request<T: bytes::Buf>( fn try_from_http_request<T: AsRef<[u8]>>(
request: http::Request<T>, request: http::Request<T>,
) -> Result<Self, ruma_api::error::FromHttpRequestError> { ) -> Result<Self, ruma_api::error::FromHttpRequestError> {
use std::{borrow::Cow, convert::TryFrom}; use std::{borrow::Cow, convert::TryFrom};
@ -136,7 +136,7 @@ impl ruma_api::IncomingRequest for IncomingRequest {
let content = { let content = {
let event_type = let event_type =
percent_encoding::percent_decode(path_segments[6].as_bytes()).decode_utf8()?; percent_encoding::percent_decode(path_segments[6].as_bytes()).decode_utf8()?;
let body: Box<RawJsonValue> = serde_json::from_reader(body.reader())?; let body: Box<RawJsonValue> = serde_json::from_slice(body.as_ref())?;
AnyStateEventContent::from_parts(&event_type, body)? AnyStateEventContent::from_parts(&event_type, body)?
}; };

View File

@ -2,7 +2,6 @@
use std::{collections::BTreeMap, fmt}; use std::{collections::BTreeMap, fmt};
use bytes::Buf;
use ruma_api::{ use ruma_api::{
error::{IntoHttpError, ResponseDeserializationError}, error::{IntoHttpError, ResponseDeserializationError},
EndpointError, OutgoingResponse, EndpointError, OutgoingResponse,
@ -10,7 +9,7 @@ use ruma_api::{
use ruma_serde::Outgoing; use ruma_serde::Outgoing;
use serde::{Deserialize, Serialize}; use serde::{Deserialize, Serialize};
use serde_json::{ use serde_json::{
from_reader as from_json_reader, to_vec as to_json_vec, value::RawValue as RawJsonValue, from_slice as from_json_slice, to_vec as to_json_vec, value::RawValue as RawJsonValue,
Value as JsonValue, Value as JsonValue,
}; };
@ -137,11 +136,11 @@ impl From<MatrixError> for UiaaResponse {
} }
impl EndpointError for UiaaResponse { impl EndpointError for UiaaResponse {
fn try_from_http_response<T: Buf>( fn try_from_http_response<T: AsRef<[u8]>>(
response: http::Response<T>, response: http::Response<T>,
) -> Result<Self, ResponseDeserializationError> { ) -> Result<Self, ResponseDeserializationError> {
if response.status() == http::StatusCode::UNAUTHORIZED { if response.status() == http::StatusCode::UNAUTHORIZED {
Ok(UiaaResponse::AuthResponse(from_json_reader(response.into_body().reader())?)) Ok(UiaaResponse::AuthResponse(from_json_slice(response.body().as_ref())?))
} else { } else {
MatrixError::try_from_http_response(response).map(From::from) MatrixError::try_from_http_response(response).map(From::from)
} }

View File

@ -373,7 +373,9 @@ impl Client {
let hyper_response = client.hyper.request(http_request.map(hyper::Body::from)).await?; let hyper_response = client.hyper.request(http_request.map(hyper::Body::from)).await?;
let (head, body) = hyper_response.into_parts(); let (head, body) = hyper_response.into_parts();
let full_body = hyper::body::aggregate(body).await?; // FIXME: Use aggregate instead of to_bytes once serde_json can parse from a reader at a
// comparable speed as reading from a slice: https://github.com/serde-rs/json/issues/160
let full_body = hyper::body::to_bytes(body).await?;
let full_response = HttpResponse::from_parts(head, full_body); let full_response = HttpResponse::from_parts(head, full_body);
Ok(ruma_api::IncomingResponse::try_from_http_response(full_response)?) Ok(ruma_api::IncomingResponse::try_from_http_response(full_response)?)

View File

@ -6,7 +6,7 @@ pub fn expand_display_as_ref_str(ident: &Ident) -> syn::Result<TokenStream> {
#[automatically_derived] #[automatically_derived]
impl ::std::fmt::Display for #ident { impl ::std::fmt::Display for #ident {
fn fmt(&self, f: &mut ::std::fmt::Formatter<'_>) -> ::std::fmt::Result { fn fmt(&self, f: &mut ::std::fmt::Formatter<'_>) -> ::std::fmt::Result {
f.write_str(<Self as ::std::convert::AsRef<::std::primitive::str>>::as_ref(self)) f.write_str(::std::convert::AsRef::<::std::primitive::str>::as_ref(self))
} }
} }
}) })

View File

@ -13,8 +13,7 @@ pub fn expand_serialize_as_ref_str(ident: &Ident) -> syn::Result<TokenStream> {
where where
S: #ruma_serde::exports::serde::ser::Serializer, S: #ruma_serde::exports::serde::ser::Serializer,
{ {
<Self as ::std::convert::AsRef<::std::primitive::str>>::as_ref(self) ::std::convert::AsRef::<::std::primitive::str>::as_ref(self).serialize(serializer)
.serialize(serializer)
} }
} }
}) })