From 9f8720c5df1251b42f4f24d2164b71bcfc6ac6f2 Mon Sep 17 00:00:00 2001 From: Follpvosten Date: Sat, 15 Jan 2022 21:00:15 +0100 Subject: [PATCH 1/3] add cbor response support for actix integration --- integrations/actix-web/Cargo.toml | 1 + integrations/actix-web/src/request.rs | 19 +++++++++++++++---- 2 files changed, 16 insertions(+), 4 deletions(-) diff --git a/integrations/actix-web/Cargo.toml b/integrations/actix-web/Cargo.toml index 8d6135ab..3707b25d 100644 --- a/integrations/actix-web/Cargo.toml +++ b/integrations/actix-web/Cargo.toml @@ -21,6 +21,7 @@ actix-web-actors = "4.0.0-beta.9" async-channel = "1.6.1" futures-util = { version = "0.3.0", default-features = false } serde_json = "1.0.64" +serde_cbor = "0.11" serde_urlencoded = "0.7.0" futures-channel = "0.3.13" thiserror = "1.0.30" diff --git a/integrations/actix-web/src/request.rs b/integrations/actix-web/src/request.rs index 70cdc7a5..04c4c60c 100644 --- a/integrations/actix-web/src/request.rs +++ b/integrations/actix-web/src/request.rs @@ -156,17 +156,28 @@ impl From for GraphQLResponse { impl Responder for GraphQLResponse { type Body = BoxBody; - fn respond_to(self, _req: &HttpRequest) -> HttpResponse { + fn respond_to(self, req: &HttpRequest) -> HttpResponse { let mut res = HttpResponse::build(StatusCode::OK); - res.content_type("application/json"); if self.0.is_ok() { if let Some(cache_control) = self.0.cache_control().value() { - res.append_header(("cache-control", cache_control)); + res.append_header((http::header::CACHE_CONTROL, cache_control)); } } for (name, value) in self.0.http_headers() { res.append_header((name, value)); } - res.body(serde_json::to_string(&self.0).unwrap()) + let accept = req + .headers() + .get(http::header::ACCEPT) + .and_then(|val| val.to_str().ok()); + // TODO: Error handling + // Neither of these branches should potentially panic. + if accept == Some("application/cbor") { + res.content_type("application/cbor"); + res.body(serde_cbor::to_vec(&self.0).unwrap()) + } else { + res.content_type("application/json"); + res.body(serde_json::to_vec(&self.0).unwrap()) + } } } From ce36e736396d7383b78e28425c4277c85910a469 Mon Sep 17 00:00:00 2001 From: Follpvosten Date: Sun, 16 Jan 2022 08:26:56 +0100 Subject: [PATCH 2/3] actix cbor/json error handling; add Content-Length header for cbor --- integrations/actix-web/src/request.rs | 33 +++++++++++++++++++++++---- 1 file changed, 28 insertions(+), 5 deletions(-) diff --git a/integrations/actix-web/src/request.rs b/integrations/actix-web/src/request.rs index 04c4c60c..bbd241f2 100644 --- a/integrations/actix-web/src/request.rs +++ b/integrations/actix-web/src/request.rs @@ -1,4 +1,6 @@ use actix_http::body::BoxBody; +use actix_web::error::JsonPayloadError; +use core::fmt; use std::future::Future; use std::io::{self, ErrorKind}; use std::pin::Pin; @@ -6,7 +8,9 @@ use std::pin::Pin; use actix_http::error::PayloadError; use actix_web::dev::Payload; use actix_web::http::{Method, StatusCode}; -use actix_web::{http, Error, FromRequest, HttpRequest, HttpResponse, Responder, Result}; +use actix_web::{ + http, Error, FromRequest, HttpRequest, HttpResponse, Responder, ResponseError, Result, +}; use futures_util::future::{self, FutureExt}; use futures_util::{StreamExt, TryStreamExt}; @@ -153,6 +157,19 @@ impl From for GraphQLResponse { } } +#[derive(Debug)] +struct CborSerializeError(serde_cbor::Error); +impl fmt::Display for CborSerializeError { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + write!(f, "{}", self.0) + } +} +impl ResponseError for CborSerializeError { + fn status_code(&self) -> StatusCode { + StatusCode::INTERNAL_SERVER_ERROR + } +} + impl Responder for GraphQLResponse { type Body = BoxBody; @@ -170,14 +187,20 @@ impl Responder for GraphQLResponse { .headers() .get(http::header::ACCEPT) .and_then(|val| val.to_str().ok()); - // TODO: Error handling - // Neither of these branches should potentially panic. if accept == Some("application/cbor") { res.content_type("application/cbor"); - res.body(serde_cbor::to_vec(&self.0).unwrap()) + let body = match serde_cbor::to_vec(&self.0) { + Ok(body) => body, + Err(error) => return HttpResponse::from_error(CborSerializeError(error)), + }; + res.append_header((http::header::CONTENT_LENGTH, body.len())); + res.body(body) } else { res.content_type("application/json"); - res.body(serde_json::to_vec(&self.0).unwrap()) + res.body(match serde_json::to_vec(&self.0) { + Ok(body) => body, + Err(error) => return HttpResponse::from_error(JsonPayloadError::Serialize(error)), + }) } } } From 5898f63dacbd0d08a571a92a169bf217d321cf48 Mon Sep 17 00:00:00 2001 From: Follpvosten Date: Mon, 17 Jan 2022 16:09:16 +0100 Subject: [PATCH 3/3] actix-web: make cbor optional feature, add test --- integrations/actix-web/Cargo.toml | 7 ++- integrations/actix-web/src/request.rs | 65 ++++++++++++++----------- integrations/actix-web/tests/graphql.rs | 48 ++++++++++++++++++ 3 files changed, 91 insertions(+), 29 deletions(-) diff --git a/integrations/actix-web/Cargo.toml b/integrations/actix-web/Cargo.toml index 3707b25d..bff1080c 100644 --- a/integrations/actix-web/Cargo.toml +++ b/integrations/actix-web/Cargo.toml @@ -21,11 +21,16 @@ actix-web-actors = "4.0.0-beta.9" async-channel = "1.6.1" futures-util = { version = "0.3.0", default-features = false } serde_json = "1.0.64" -serde_cbor = "0.11" serde_urlencoded = "0.7.0" futures-channel = "0.3.13" thiserror = "1.0.30" +serde_cbor = { version = "0.11.2", optional = true } + +[features] +default = [] +cbor = ["serde_cbor"] [dev-dependencies] actix-rt = "2.2.0" async-mutex = "1.4.0" +serde = { version = "1", features = ["derive"] } diff --git a/integrations/actix-web/src/request.rs b/integrations/actix-web/src/request.rs index bbd241f2..b4639b67 100644 --- a/integrations/actix-web/src/request.rs +++ b/integrations/actix-web/src/request.rs @@ -1,6 +1,5 @@ use actix_http::body::BoxBody; use actix_web::error::JsonPayloadError; -use core::fmt; use std::future::Future; use std::io::{self, ErrorKind}; use std::pin::Pin; @@ -8,9 +7,7 @@ use std::pin::Pin; use actix_http::error::PayloadError; use actix_web::dev::Payload; use actix_web::http::{Method, StatusCode}; -use actix_web::{ - http, Error, FromRequest, HttpRequest, HttpResponse, Responder, ResponseError, Result, -}; +use actix_web::{http, Error, FromRequest, HttpRequest, HttpResponse, Responder, Result}; use futures_util::future::{self, FutureExt}; use futures_util::{StreamExt, TryStreamExt}; @@ -157,16 +154,22 @@ impl From for GraphQLResponse { } } -#[derive(Debug)] -struct CborSerializeError(serde_cbor::Error); -impl fmt::Display for CborSerializeError { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - write!(f, "{}", self.0) +#[cfg(feature = "cbor")] +mod cbor { + use actix_web::{http::StatusCode, ResponseError}; + use core::fmt; + + #[derive(Debug)] + pub struct Error(pub serde_cbor::Error); + impl fmt::Display for Error { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + write!(f, "{}", self.0) + } } -} -impl ResponseError for CborSerializeError { - fn status_code(&self) -> StatusCode { - StatusCode::INTERNAL_SERVER_ERROR + impl ResponseError for Error { + fn status_code(&self) -> StatusCode { + StatusCode::INTERNAL_SERVER_ERROR + } } } @@ -187,20 +190,26 @@ impl Responder for GraphQLResponse { .headers() .get(http::header::ACCEPT) .and_then(|val| val.to_str().ok()); - if accept == Some("application/cbor") { - res.content_type("application/cbor"); - let body = match serde_cbor::to_vec(&self.0) { - Ok(body) => body, - Err(error) => return HttpResponse::from_error(CborSerializeError(error)), - }; - res.append_header((http::header::CONTENT_LENGTH, body.len())); - res.body(body) - } else { - res.content_type("application/json"); - res.body(match serde_json::to_vec(&self.0) { - Ok(body) => body, - Err(error) => return HttpResponse::from_error(JsonPayloadError::Serialize(error)), - }) - } + let (ct, body) = match accept { + // optional cbor support + #[cfg(feature = "cbor")] + // this avoids copy-pasting the mime type + Some(ct @ "application/cbor") => ( + ct, + match serde_cbor::to_vec(&self.0) { + Ok(body) => body, + Err(e) => return HttpResponse::from_error(cbor::Error(e)), + }, + ), + _ => ( + "application/json", + match serde_json::to_vec(&self.0) { + Ok(body) => body, + Err(e) => return HttpResponse::from_error(JsonPayloadError::Serialize(e)), + }, + ), + }; + res.content_type(ct); + res.body(body) } } diff --git a/integrations/actix-web/tests/graphql.rs b/integrations/actix-web/tests/graphql.rs index 820d80ed..3f5c9690 100644 --- a/integrations/actix-web/tests/graphql.rs +++ b/integrations/actix-web/tests/graphql.rs @@ -220,3 +220,51 @@ async fn test_count() { .into_bytes() ); } + +#[cfg(feature = "cbor")] +#[actix_rt::test] +async fn test_cbor() { + let srv = test::init_service( + App::new() + .app_data(Data::new(Schema::new( + AddQueryRoot, + EmptyMutation, + EmptySubscription, + ))) + .service( + web::resource("/") + .guard(guard::Post()) + .to(gql_handle_schema::), + ), + ) + .await; + let response = srv + .call( + test::TestRequest::with_uri("/") + .method(Method::POST) + .set_payload(r#"{"query":"{ add(a: 10, b: 20) }"}"#) + .insert_header((actix_http::header::ACCEPT, "application/cbor")) + .to_request(), + ) + .await + .unwrap(); + assert!(response.status().is_success()); + #[derive(Debug, serde::Deserialize, PartialEq)] + struct Response { + data: ResponseInner, + } + #[derive(Debug, serde::Deserialize, PartialEq)] + struct ResponseInner { + add: i32, + } + let body = actix_web::body::to_bytes(response.into_body()) + .await + .unwrap(); + let response: Response = serde_cbor::from_slice(&body).unwrap(); + assert_eq!( + response, + Response { + data: ResponseInner { add: 30 } + } + ); +}