From a7de61629fe1d11d739166441334f4adb76f59d1 Mon Sep 17 00:00:00 2001 From: Sam Scott Date: Sun, 7 Apr 2024 14:27:09 -0500 Subject: [PATCH] Remove redundant config, add tests --- .github/workflows/ci.yml | 1 - src/actix.rs | 136 +++++++-------------------------------- src/de/mod.rs | 8 ++- tests/test_actix.rs | 24 ++++++- 4 files changed, 52 insertions(+), 117 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 5a478db..373269f 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -43,7 +43,6 @@ jobs: - "" - actix4 - actix3 - - actix2 - warp - axum steps: diff --git a/src/actix.rs b/src/actix.rs index 33c2a33..0be0b9b 100644 --- a/src/actix.rs +++ b/src/actix.rs @@ -13,11 +13,11 @@ use actix_web4 as actix_web; use actix_web::dev::Payload; #[cfg(feature = "actix3")] use actix_web::HttpResponse; -use actix_web::{Error as ActixError, FromRequest, HttpRequest, ResponseError, web}; -use futures::future::{ready, Ready, LocalBoxFuture, FutureExt}; +use actix_web::{web, Error as ActixError, FromRequest, HttpRequest, ResponseError}; +use futures::future::{ready, FutureExt, LocalBoxFuture, Ready}; use futures::StreamExt; -use serde::de::DeserializeOwned; use serde::de; +use serde::de::DeserializeOwned; use std::fmt; use std::fmt::{Debug, Display}; use std::ops::{Deref, DerefMut}; @@ -115,20 +115,14 @@ where #[inline] fn from_request(req: &HttpRequest, _: &mut Payload) -> Self::Future { - let query_config = req.app_data::(); - - let error_handler = query_config.map(|c| c.ehandler.clone()).unwrap_or(None); + let query_config = req.app_data::().unwrap_or(&DEFAULT_CONFIG); - let default_qsconfig = QsConfig::default(); - let qsconfig = query_config - .map(|c| &c.qs_config) - .unwrap_or(&default_qsconfig); - - let res = qsconfig + let res = query_config + .qs_config .deserialize_str::(req.query_string()) .map(|val| Ok(QsQuery(val))) .unwrap_or_else(move |e| { - let e = if let Some(error_handler) = error_handler { + let e = if let Some(error_handler) = &query_config.ehandler { (error_handler)(e, req) } else { e.into() @@ -179,11 +173,17 @@ where /// ); /// } /// ``` +#[derive(Clone)] pub struct QsQueryConfig { ehandler: Option ActixError + Send + Sync>>, qs_config: QsConfig, } +static DEFAULT_CONFIG: QsQueryConfig = QsQueryConfig { + ehandler: None, + qs_config: crate::de::DEFAULT_CONFIG, +}; + impl QsQueryConfig { /// Set custom error handler pub fn error_handler(mut self, f: F) -> Self @@ -210,8 +210,6 @@ impl Default for QsQueryConfig { } } -// QS Form Extractor - #[derive(PartialEq, Eq, PartialOrd, Ord)] /// Extract typed information from from the request's form data. /// @@ -223,8 +221,6 @@ impl Default for QsQueryConfig { /// # use actix_web4 as actix_web; /// # #[cfg(feature = "actix3")] /// # use actix_web3 as actix_web; -/// # #[cfg(feature = "actix2")] -/// # use actix_web2 as actix_web; /// use actix_web::{web, App, HttpResponse}; /// use serde_qs::actix::QsForm; /// @@ -248,13 +244,9 @@ impl Default for QsQueryConfig { /// .route(web::get().to(filter_users))); /// } /// ``` - - #[derive(Debug)] pub struct QsForm(T); - -// let foo: T = QsQuery.into_inner() impl QsForm { /// Unwrap into inner T value pub fn into_inner(self) -> T { @@ -276,99 +268,23 @@ impl DerefMut for QsForm { } } -// private fields on QsQueryConfig prevent its reuse here, so a new struct -// is defined - -/// Form extractor configuration -/// -/// ```rust -/// # #[macro_use] extern crate serde_derive; -/// # #[cfg(feature = "actix4")] -/// # use actix_web4 as actix_web; -/// # #[cfg(feature = "actix3")] -/// # use actix_web3 as actix_web; -/// # #[cfg(feature = "actix2")] -/// # use actix_web2 as actix_web; -/// use actix_web::{error, web, App, FromRequest, HttpResponse}; -/// use serde_qs::actix::QsQuery; -/// use serde_qs::Config as QsConfig; -/// use serde_qs::actix::QsFormConfig; -/// -/// #[derive(Deserialize)] -/// struct Info { -/// username: String, -/// } -/// -/// /// deserialize `Info` from request's payload -/// async fn index(info: QsForm) -> HttpResponse { -/// use serde_qs::actix::QsForm; -/// HttpResponse::Ok().body( -/// format!("Welcome {}!", info.username) -/// ) -/// } -/// -/// fn main() { -/// let qs_config = QsFormConfig::default() -/// .error_handler(|err, req| { // <- create custom error response -/// error::InternalError::from_response( -/// err, HttpResponse::Conflict().finish()).into() -/// }) -/// .qs_config(QsConfig::default()); -/// -/// let app = App::new().service( -/// web::resource("/index.html").app_data(qs_config) -/// .route(web::post().to(index)) -/// ); -/// } -/// ``` - -pub struct QsFormConfig { - ehandler: Option ActixError + Send + Sync>>, - qs_config: QsConfig, -} - -impl QsFormConfig { - /// Set custom error handler - pub fn error_handler(mut self, f: F) -> Self - where - F: Fn(QsError, &HttpRequest) -> ActixError + Send + Sync + 'static, - { - self.ehandler = Some(Arc::new(f)); - self - } - - /// Set custom serialization parameters - pub fn qs_config(mut self, config: QsConfig) -> Self { - self.qs_config = config; - self - } -} - -impl Default for QsFormConfig { - fn default() -> Self { - QsFormConfig { - qs_config: QsConfig::default(), - ehandler: None, - } - } -} - -// -// See: -// - https://github.com/actix/actix-web/blob/master/src/types/form.rs -// - https://github.com/samscott89/serde_qs/blob/main/src/actix.rs impl FromRequest for QsForm where T: DeserializeOwned + Debug, { type Error = ActixError; - // type Config = (); type Future = LocalBoxFuture<'static, Result>; + #[cfg(feature = "actix3")] + type Config = QsQueryConfig; fn from_request(req: &HttpRequest, payload: &mut Payload) -> Self::Future { let mut stream = payload.take(); let req_clone = req.clone(); + let query_config: QsQueryConfig = req + .app_data::() + .unwrap_or(&DEFAULT_CONFIG) + .clone(); async move { let mut bytes = web::BytesMut::new(); @@ -376,20 +292,12 @@ where bytes.extend_from_slice(&item.unwrap()); } - let query_config = req_clone.app_data::().clone(); - let error_handler = query_config.map(|c| c.ehandler.clone()).unwrap_or(None); - - let default_qsconfig = QsConfig::default(); - let qsconfig = query_config - .map(|c| &c.qs_config) - .unwrap_or(&default_qsconfig); - - qsconfig + query_config + .qs_config .deserialize_bytes::(&bytes) .map(|val| Ok(QsForm(val))) .unwrap_or_else(|e| { - let e = if let Some(error_handler) = error_handler { - // e.into() + let e = if let Some(error_handler) = &query_config.ehandler { (error_handler)(e, &req_clone) } else { e.into() diff --git a/src/de/mod.rs b/src/de/mod.rs index 290d843..f882ae7 100644 --- a/src/de/mod.rs +++ b/src/de/mod.rs @@ -80,6 +80,7 @@ use std::collections::btree_map::{BTreeMap, Entry, IntoIter}; /// assert_eq!(map.get("a").unwrap().get("b").unwrap().get("c").unwrap(), "1"); /// ``` /// +#[derive(Clone, Copy)] pub struct Config { /// Specifies the maximum depth key that `serde_qs` will attempt to /// deserialize. Default is 5. @@ -88,9 +89,14 @@ pub struct Config { strict: bool, } +pub const DEFAULT_CONFIG: Config = Config { + max_depth: 5, + strict: true, +}; + impl Default for Config { fn default() -> Self { - Self::new(5, true) + DEFAULT_CONFIG } } diff --git a/tests/test_actix.rs b/tests/test_actix.rs index 792afb2..48ad27f 100644 --- a/tests/test_actix.rs +++ b/tests/test_actix.rs @@ -16,7 +16,7 @@ use actix_web::error::InternalError; use actix_web::http::StatusCode; use actix_web::test::TestRequest; use actix_web::{FromRequest, HttpResponse}; -use qs::actix::{QsQuery, QsQueryConfig}; +use qs::actix::{QsForm, QsQuery, QsQueryConfig}; use qs::Config as QsConfig; use serde::de::Error; @@ -143,3 +143,25 @@ fn test_custom_qs_config() { assert_eq!(s.common.remaining, true); }) } + +#[test] +fn test_form_extractor() { + futures::executor::block_on(async { + let test_data = Query { + foo: 1, + bars: vec![0, 1], + common: CommonParams { + limit: 100, + offset: 50, + remaining: true, + }, + }; + let req = TestRequest::with_uri("/test") + .set_payload(serde_qs::to_string(&test_data).unwrap()) + .to_srv_request(); + let (req, mut pl) = req.into_parts(); + + let s = QsForm::::from_request(&req, &mut pl).await.unwrap(); + assert_eq!(s.into_inner(), test_data); + }) +}