From a43765d45473e41ab0fd6a2298b676742142af5f Mon Sep 17 00:00:00 2001 From: Tamo Date: Tue, 14 Feb 2023 13:12:42 +0100 Subject: [PATCH] use the pre-defined deserr extractors --- Cargo.lock | 11 ++- meilisearch-types/Cargo.toml | 2 +- meilisearch-types/src/deserr/mod.rs | 11 +++ meilisearch-types/src/error.rs | 2 +- meilisearch/Cargo.toml | 2 +- meilisearch/src/extractors/json.rs | 78 ------------------- meilisearch/src/extractors/mod.rs | 2 - .../src/extractors/query_parameters.rs | 70 ----------------- meilisearch/src/routes/api_key.rs | 9 +-- meilisearch/src/routes/indexes/documents.rs | 10 +-- meilisearch/src/routes/indexes/mod.rs | 9 +-- meilisearch/src/routes/indexes/search.rs | 7 +- meilisearch/src/routes/indexes/settings.rs | 6 +- meilisearch/src/routes/swap_indexes.rs | 4 +- meilisearch/src/routes/tasks.rs | 8 +- milli/Cargo.toml | 2 +- 16 files changed, 47 insertions(+), 186 deletions(-) delete mode 100644 meilisearch/src/extractors/json.rs delete mode 100644 meilisearch/src/extractors/query_parameters.rs diff --git a/Cargo.lock b/Cargo.lock index 5aa355d73..28ac2ca63 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1113,23 +1113,26 @@ dependencies = [ [[package]] name = "deserr" -version = "0.4.0" +version = "0.4.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "a13eed41ca58d9dc99e2c67e1a5f50507dfa1b123cc4a942c81c49707bd347f0" +checksum = "6eee2844f21cf7fb5693aae1fb8f1658127acfdb2fc072167d68a9152584ae64" dependencies = [ + "actix-http", + "actix-utils", "actix-web", "deserr-internal", "futures", "serde-cs", "serde_json", + "serde_urlencoded", "strsim", ] [[package]] name = "deserr-internal" -version = "0.4.0" +version = "0.4.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "4d5412186d7149542b09319901d28b3c7d1f714a61d0c5d48a50560d09573ae4" +checksum = "c27246f8ca9eeba9dd70d614b664dc43b529251ed7bd9e633131010d340da4b9" dependencies = [ "convert_case 0.5.0", "proc-macro2", diff --git a/meilisearch-types/Cargo.toml b/meilisearch-types/Cargo.toml index 021c44ea0..3ed9464d3 100644 --- a/meilisearch-types/Cargo.toml +++ b/meilisearch-types/Cargo.toml @@ -9,7 +9,7 @@ actix-web = { version = "4.2.1", default-features = false } anyhow = "1.0.65" convert_case = "0.6.0" csv = "1.1.6" -deserr = "0.4.0" +deserr = "0.4.1" either = { version = "1.6.1", features = ["serde"] } enum-iterator = "1.1.3" file-store = { path = "../file-store" } diff --git a/meilisearch-types/src/deserr/mod.rs b/meilisearch-types/src/deserr/mod.rs index 33213c953..ad6f72e2c 100644 --- a/meilisearch-types/src/deserr/mod.rs +++ b/meilisearch-types/src/deserr/mod.rs @@ -38,6 +38,7 @@ impl DeserrError { Self { msg, code, _phantom: PhantomData } } } + impl std::fmt::Debug for DeserrError { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { f.debug_struct("DeserrError").field("msg", &self.msg).field("code", &self.code).finish() @@ -50,6 +51,16 @@ impl std::fmt::Display for DeserrError actix_web::ResponseError for DeserrError { + fn status_code(&self) -> actix_web::http::StatusCode { + self.code.http() + } + + fn error_response(&self) -> actix_web::HttpResponse { + crate::error::ResponseError::from_msg(self.msg.to_string(), self.code).error_response() + } +} + impl std::error::Error for DeserrError {} impl ErrorCode for DeserrError { fn error_code(&self) -> Code { diff --git a/meilisearch-types/src/error.rs b/meilisearch-types/src/error.rs index d83bf7eb9..5c2968b39 100644 --- a/meilisearch-types/src/error.rs +++ b/meilisearch-types/src/error.rs @@ -127,7 +127,7 @@ macro_rules! make_error_codes { } impl Code { /// return the HTTP status code associated with the `Code` - fn http(&self) -> StatusCode { + pub fn http(&self) -> StatusCode { match self { $( Code::$code_ident => StatusCode::$status diff --git a/meilisearch/Cargo.toml b/meilisearch/Cargo.toml index b6410f7cb..b4a87c632 100644 --- a/meilisearch/Cargo.toml +++ b/meilisearch/Cargo.toml @@ -19,7 +19,7 @@ byte-unit = { version = "4.0.14", default-features = false, features = ["std", " bytes = "1.2.1" clap = { version = "4.0.9", features = ["derive", "env"] } crossbeam-channel = "0.5.6" -deserr = "0.4.0" +deserr = "0.4.1" dump = { path = "../dump" } either = "1.8.0" env_logger = "0.9.1" diff --git a/meilisearch/src/extractors/json.rs b/meilisearch/src/extractors/json.rs deleted file mode 100644 index c59af14db..000000000 --- a/meilisearch/src/extractors/json.rs +++ /dev/null @@ -1,78 +0,0 @@ -use std::fmt::Debug; -use std::future::Future; -use std::marker::PhantomData; -use std::pin::Pin; -use std::task::{Context, Poll}; - -use actix_web::dev::Payload; -use actix_web::web::Json; -use actix_web::{FromRequest, HttpRequest}; -use deserr::{DeserializeError, Deserr}; -use futures::ready; -use meilisearch_types::error::{ErrorCode, ResponseError}; - -/// Extractor for typed data from Json request payloads -/// deserialised by deserr. -/// -/// # Extractor -/// To extract typed data from a request body, the inner type `T` must implement the -/// [`deserr::DeserializeFromError`] trait. The inner type `E` must implement the -/// [`ErrorCode`](meilisearch_error::ErrorCode) trait. -#[derive(Debug)] -pub struct ValidatedJson(pub T, PhantomData<*const E>); - -impl ValidatedJson { - pub fn new(data: T) -> Self { - ValidatedJson(data, PhantomData) - } - pub fn into_inner(self) -> T { - self.0 - } -} - -impl FromRequest for ValidatedJson -where - E: DeserializeError + ErrorCode + std::error::Error + 'static, - T: Deserr, -{ - type Error = actix_web::Error; - type Future = ValidatedJsonExtractFut; - - #[inline] - fn from_request(req: &HttpRequest, payload: &mut Payload) -> Self::Future { - ValidatedJsonExtractFut { - fut: Json::::from_request(req, payload), - _phantom: PhantomData, - } - } -} - -pub struct ValidatedJsonExtractFut { - fut: as FromRequest>::Future, - _phantom: PhantomData<*const (T, E)>, -} - -impl Future for ValidatedJsonExtractFut -where - T: Deserr, - E: DeserializeError + ErrorCode + std::error::Error + 'static, -{ - type Output = Result, actix_web::Error>; - - fn poll(self: Pin<&mut Self>, cx: &mut Context<'_>) -> Poll { - let ValidatedJsonExtractFut { fut, .. } = self.get_mut(); - let fut = Pin::new(fut); - - let res = ready!(fut.poll(cx)); - - let res = match res { - Err(err) => Err(err), - Ok(data) => match deserr::deserialize::<_, _, E>(data.into_inner()) { - Ok(data) => Ok(ValidatedJson::new(data)), - Err(e) => Err(ResponseError::from(e).into()), - }, - }; - - Poll::Ready(res) - } -} diff --git a/meilisearch/src/extractors/mod.rs b/meilisearch/src/extractors/mod.rs index f44b0c004..98a22f8c9 100644 --- a/meilisearch/src/extractors/mod.rs +++ b/meilisearch/src/extractors/mod.rs @@ -1,6 +1,4 @@ pub mod payload; #[macro_use] pub mod authentication; -pub mod json; -pub mod query_parameters; pub mod sequential_extractor; diff --git a/meilisearch/src/extractors/query_parameters.rs b/meilisearch/src/extractors/query_parameters.rs deleted file mode 100644 index 39b833062..000000000 --- a/meilisearch/src/extractors/query_parameters.rs +++ /dev/null @@ -1,70 +0,0 @@ -//! A module to parse query parameter with deserr - -use std::marker::PhantomData; -use std::{fmt, ops}; - -use actix_http::Payload; -use actix_utils::future::{err, ok, Ready}; -use actix_web::{FromRequest, HttpRequest}; -use deserr::{DeserializeError, Deserr}; -use meilisearch_types::error::{Code, ErrorCode, ResponseError}; - -#[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Ord)] -pub struct QueryParameter(pub T, PhantomData<*const E>); - -impl QueryParameter { - /// Unwrap into inner `T` value. - pub fn into_inner(self) -> T { - self.0 - } -} - -impl QueryParameter -where - T: Deserr, - E: DeserializeError + ErrorCode + std::error::Error + 'static, -{ - pub fn from_query(query_str: &str) -> Result { - let value = serde_urlencoded::from_str::(query_str) - .map_err(|e| ResponseError::from_msg(e.to_string(), Code::BadRequest))?; - - match deserr::deserialize::<_, _, E>(value) { - Ok(data) => Ok(QueryParameter(data, PhantomData)), - Err(e) => Err(ResponseError::from(e).into()), - } - } -} - -impl ops::Deref for QueryParameter { - type Target = T; - - fn deref(&self) -> &T { - &self.0 - } -} - -impl ops::DerefMut for QueryParameter { - fn deref_mut(&mut self) -> &mut T { - &mut self.0 - } -} - -impl fmt::Display for QueryParameter { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - self.0.fmt(f) - } -} - -impl FromRequest for QueryParameter -where - T: Deserr, - E: DeserializeError + ErrorCode + std::error::Error + 'static, -{ - type Error = actix_web::Error; - type Future = Ready>; - - #[inline] - fn from_request(req: &HttpRequest, _: &mut Payload) -> Self::Future { - QueryParameter::from_query(req.query_string()).map(ok).unwrap_or_else(err) - } -} diff --git a/meilisearch/src/routes/api_key.rs b/meilisearch/src/routes/api_key.rs index 096aa7df0..7514d01f6 100644 --- a/meilisearch/src/routes/api_key.rs +++ b/meilisearch/src/routes/api_key.rs @@ -1,6 +1,7 @@ use std::str; use actix_web::{web, HttpRequest, HttpResponse}; +use deserr::actix_web::{AwebJson, AwebQueryParameter}; use deserr::Deserr; use meilisearch_auth::error::AuthControllerError; use meilisearch_auth::AuthController; @@ -16,8 +17,6 @@ use uuid::Uuid; use super::PAGINATION_DEFAULT_LIMIT; use crate::extractors::authentication::policies::*; use crate::extractors::authentication::GuardedData; -use crate::extractors::json::ValidatedJson; -use crate::extractors::query_parameters::QueryParameter; use crate::extractors::sequential_extractor::SeqHandler; use crate::routes::Pagination; @@ -37,7 +36,7 @@ pub fn configure(cfg: &mut web::ServiceConfig) { pub async fn create_api_key( auth_controller: GuardedData, AuthController>, - body: ValidatedJson, + body: AwebJson, _req: HttpRequest, ) -> Result { let v = body.into_inner(); @@ -68,7 +67,7 @@ impl ListApiKeys { pub async fn list_api_keys( auth_controller: GuardedData, AuthController>, - list_api_keys: QueryParameter, + list_api_keys: AwebQueryParameter, ) -> Result { let paginate = list_api_keys.into_inner().as_pagination(); let page_view = tokio::task::spawn_blocking(move || -> Result<_, AuthControllerError> { @@ -105,7 +104,7 @@ pub async fn get_api_key( pub async fn patch_api_key( auth_controller: GuardedData, AuthController>, - body: ValidatedJson, + body: AwebJson, path: web::Path, ) -> Result { let key = path.into_inner().key; diff --git a/meilisearch/src/routes/indexes/documents.rs b/meilisearch/src/routes/indexes/documents.rs index 90bf70692..fb75cfcf8 100644 --- a/meilisearch/src/routes/indexes/documents.rs +++ b/meilisearch/src/routes/indexes/documents.rs @@ -4,6 +4,7 @@ use actix_web::http::header::CONTENT_TYPE; use actix_web::web::Data; use actix_web::{web, HttpMessage, HttpRequest, HttpResponse}; use bstr::ByteSlice; +use deserr::actix_web::AwebQueryParameter; use deserr::Deserr; use futures::StreamExt; use index_scheduler::IndexScheduler; @@ -33,7 +34,6 @@ use crate::error::PayloadError::ReceivePayload; use crate::extractors::authentication::policies::*; use crate::extractors::authentication::GuardedData; use crate::extractors::payload::Payload; -use crate::extractors::query_parameters::QueryParameter; use crate::extractors::sequential_extractor::SeqHandler; use crate::routes::{PaginationView, SummarizedTaskView, PAGINATION_DEFAULT_LIMIT}; @@ -90,7 +90,7 @@ pub struct GetDocument { pub async fn get_document( index_scheduler: GuardedData, Data>, document_param: web::Path, - params: QueryParameter, + params: AwebQueryParameter, ) -> Result { let DocumentParam { index_uid, document_id } = document_param.into_inner(); let index_uid = IndexUid::try_from(index_uid)?; @@ -139,7 +139,7 @@ pub struct BrowseQuery { pub async fn get_all_documents( index_scheduler: GuardedData, Data>, index_uid: web::Path, - params: QueryParameter, + params: AwebQueryParameter, ) -> Result { let index_uid = IndexUid::try_from(index_uid.into_inner())?; debug!("called with params: {:?}", params); @@ -165,7 +165,7 @@ pub struct UpdateDocumentsQuery { pub async fn add_documents( index_scheduler: GuardedData, Data>, index_uid: web::Path, - params: QueryParameter, + params: AwebQueryParameter, body: Payload, req: HttpRequest, analytics: web::Data, @@ -195,7 +195,7 @@ pub async fn add_documents( pub async fn update_documents( index_scheduler: GuardedData, Data>, index_uid: web::Path, - params: QueryParameter, + params: AwebQueryParameter, body: Payload, req: HttpRequest, analytics: web::Data, diff --git a/meilisearch/src/routes/indexes/mod.rs b/meilisearch/src/routes/indexes/mod.rs index 16b9faa24..2e13d782d 100644 --- a/meilisearch/src/routes/indexes/mod.rs +++ b/meilisearch/src/routes/indexes/mod.rs @@ -2,6 +2,7 @@ use std::convert::Infallible; use actix_web::web::Data; use actix_web::{web, HttpRequest, HttpResponse}; +use deserr::actix_web::{AwebJson, AwebQueryParameter}; use deserr::{DeserializeError, Deserr, ValuePointerRef}; use index_scheduler::IndexScheduler; use log::debug; @@ -20,8 +21,6 @@ use super::{Pagination, SummarizedTaskView, PAGINATION_DEFAULT_LIMIT}; use crate::analytics::Analytics; use crate::extractors::authentication::policies::*; use crate::extractors::authentication::{AuthenticationError, GuardedData}; -use crate::extractors::json::ValidatedJson; -use crate::extractors::query_parameters::QueryParameter; use crate::extractors::sequential_extractor::SeqHandler; pub mod documents; @@ -88,7 +87,7 @@ impl ListIndexes { pub async fn list_indexes( index_scheduler: GuardedData, Data>, - paginate: QueryParameter, + paginate: AwebQueryParameter, ) -> Result { let search_rules = &index_scheduler.filters().search_rules; let indexes: Vec<_> = index_scheduler.indexes()?; @@ -115,7 +114,7 @@ pub struct IndexCreateRequest { pub async fn create_index( index_scheduler: GuardedData, Data>, - body: ValidatedJson, + body: AwebJson, req: HttpRequest, analytics: web::Data, ) -> Result { @@ -180,7 +179,7 @@ pub async fn get_index( pub async fn update_index( index_scheduler: GuardedData, Data>, index_uid: web::Path, - body: ValidatedJson, + body: AwebJson, req: HttpRequest, analytics: web::Data, ) -> Result { diff --git a/meilisearch/src/routes/indexes/search.rs b/meilisearch/src/routes/indexes/search.rs index 50a2ffd74..3fb413c43 100644 --- a/meilisearch/src/routes/indexes/search.rs +++ b/meilisearch/src/routes/indexes/search.rs @@ -1,5 +1,6 @@ use actix_web::web::Data; use actix_web::{web, HttpRequest, HttpResponse}; +use deserr::actix_web::{AwebJson, AwebQueryParameter}; use index_scheduler::IndexScheduler; use log::debug; use meilisearch_auth::IndexSearchRules; @@ -14,8 +15,6 @@ use serde_json::Value; use crate::analytics::{Analytics, SearchAggregator}; use crate::extractors::authentication::policies::*; use crate::extractors::authentication::GuardedData; -use crate::extractors::json::ValidatedJson; -use crate::extractors::query_parameters::QueryParameter; use crate::extractors::sequential_extractor::SeqHandler; use crate::search::{ perform_search, MatchingStrategy, SearchQuery, DEFAULT_CROP_LENGTH, DEFAULT_CROP_MARKER, @@ -150,7 +149,7 @@ fn fix_sort_query_parameters(sort_query: &str) -> Vec { pub async fn search_with_url_query( index_scheduler: GuardedData, Data>, index_uid: web::Path, - params: QueryParameter, + params: AwebQueryParameter, req: HttpRequest, analytics: web::Data, ) -> Result { @@ -184,7 +183,7 @@ pub async fn search_with_url_query( pub async fn search_with_post( index_scheduler: GuardedData, Data>, index_uid: web::Path, - params: ValidatedJson, + params: AwebJson, req: HttpRequest, analytics: web::Data, ) -> Result { diff --git a/meilisearch/src/routes/indexes/settings.rs b/meilisearch/src/routes/indexes/settings.rs index 0c864cc73..d10aec1a2 100644 --- a/meilisearch/src/routes/indexes/settings.rs +++ b/meilisearch/src/routes/indexes/settings.rs @@ -1,5 +1,6 @@ use actix_web::web::Data; use actix_web::{web, HttpRequest, HttpResponse}; +use deserr::actix_web::AwebJson; use index_scheduler::IndexScheduler; use log::debug; use meilisearch_types::deserr::DeserrJsonError; @@ -12,7 +13,6 @@ use serde_json::json; use crate::analytics::Analytics; use crate::extractors::authentication::policies::*; use crate::extractors::authentication::GuardedData; -use crate::extractors::json::ValidatedJson; use crate::routes::SummarizedTaskView; #[macro_export] @@ -68,7 +68,7 @@ macro_rules! make_setting_route { Data, >, index_uid: actix_web::web::Path, - body: $crate::routes::indexes::ValidatedJson, $err_ty>, + body: deserr::actix_web::AwebJson, $err_ty>, req: HttpRequest, $analytics_var: web::Data, ) -> std::result::Result { @@ -468,7 +468,7 @@ generate_configure!( pub async fn update_all( index_scheduler: GuardedData, Data>, index_uid: web::Path, - body: ValidatedJson, DeserrJsonError>, + body: AwebJson, DeserrJsonError>, req: HttpRequest, analytics: web::Data, ) -> Result { diff --git a/meilisearch/src/routes/swap_indexes.rs b/meilisearch/src/routes/swap_indexes.rs index 2070177d9..c4177e900 100644 --- a/meilisearch/src/routes/swap_indexes.rs +++ b/meilisearch/src/routes/swap_indexes.rs @@ -1,5 +1,6 @@ use actix_web::web::Data; use actix_web::{web, HttpRequest, HttpResponse}; +use deserr::actix_web::AwebJson; use deserr::Deserr; use index_scheduler::IndexScheduler; use meilisearch_types::deserr::DeserrJsonError; @@ -14,7 +15,6 @@ use crate::analytics::Analytics; use crate::error::MeilisearchHttpError; use crate::extractors::authentication::policies::*; use crate::extractors::authentication::{AuthenticationError, GuardedData}; -use crate::extractors::json::ValidatedJson; use crate::extractors::sequential_extractor::SeqHandler; pub fn configure(cfg: &mut web::ServiceConfig) { @@ -30,7 +30,7 @@ pub struct SwapIndexesPayload { pub async fn swap_indexes( index_scheduler: GuardedData, Data>, - params: ValidatedJson, DeserrJsonError>, + params: AwebJson, DeserrJsonError>, req: HttpRequest, analytics: web::Data, ) -> Result { diff --git a/meilisearch/src/routes/tasks.rs b/meilisearch/src/routes/tasks.rs index f747320b1..49f3aac66 100644 --- a/meilisearch/src/routes/tasks.rs +++ b/meilisearch/src/routes/tasks.rs @@ -1,5 +1,6 @@ use actix_web::web::Data; use actix_web::{web, HttpRequest, HttpResponse}; +use deserr::actix_web::AwebQueryParameter; use deserr::Deserr; use index_scheduler::{IndexScheduler, Query, TaskId}; use meilisearch_types::deserr::query_params::Param; @@ -23,7 +24,6 @@ use super::SummarizedTaskView; use crate::analytics::Analytics; use crate::extractors::authentication::policies::*; use crate::extractors::authentication::GuardedData; -use crate::extractors::query_parameters::QueryParameter; use crate::extractors::sequential_extractor::SeqHandler; const DEFAULT_LIMIT: u32 = 20; @@ -286,7 +286,7 @@ impl TaskDeletionOrCancelationQuery { async fn cancel_tasks( index_scheduler: GuardedData, Data>, - params: QueryParameter, + params: AwebQueryParameter, req: HttpRequest, analytics: web::Data, ) -> Result { @@ -332,7 +332,7 @@ async fn cancel_tasks( async fn delete_tasks( index_scheduler: GuardedData, Data>, - params: QueryParameter, + params: AwebQueryParameter, req: HttpRequest, analytics: web::Data, ) -> Result { @@ -385,7 +385,7 @@ pub struct AllTasks { async fn get_tasks( index_scheduler: GuardedData, Data>, - params: QueryParameter, + params: AwebQueryParameter, req: HttpRequest, analytics: web::Data, ) -> Result { diff --git a/milli/Cargo.toml b/milli/Cargo.toml index 70658d81d..1752cb3d9 100644 --- a/milli/Cargo.toml +++ b/milli/Cargo.toml @@ -12,7 +12,7 @@ byteorder = "1.4.3" charabia = { version = "0.7.0", default-features = false } concat-arrays = "0.1.2" crossbeam-channel = "0.5.6" -deserr = "0.4.0" +deserr = "0.4.1" either = "1.8.0" flatten-serde-json = { path = "../flatten-serde-json" } fst = "0.4.7"