From 8fb7b1d10f2ae21a439da5ae21aecc7612a6c2f9 Mon Sep 17 00:00:00 2001 From: Tamo Date: Mon, 13 Feb 2023 18:45:13 +0100 Subject: [PATCH 1/4] bump deserr --- Cargo.lock | 53 ++++++++++++++----- meilisearch-types/Cargo.toml | 2 +- .../src/deserr/error_messages.rs | 10 ++-- meilisearch-types/src/deserr/mod.rs | 9 ++-- meilisearch-types/src/deserr/query_params.rs | 4 +- meilisearch-types/src/error.rs | 8 +-- meilisearch-types/src/index_uid.rs | 6 +-- meilisearch-types/src/index_uid_pattern.rs | 6 +-- meilisearch-types/src/keys.rs | 16 +++--- meilisearch-types/src/settings.rs | 21 ++++---- meilisearch-types/src/star_or.rs | 14 +++-- meilisearch/Cargo.toml | 2 +- meilisearch/src/extractors/json.rs | 6 +-- .../src/extractors/query_parameters.rs | 6 +-- meilisearch/src/routes/api_key.rs | 4 +- meilisearch/src/routes/indexes/documents.rs | 8 +-- meilisearch/src/routes/indexes/mod.rs | 8 +-- meilisearch/src/routes/indexes/search.rs | 2 +- meilisearch/src/routes/swap_indexes.rs | 4 +- meilisearch/src/routes/tasks.rs | 36 +++++++------ meilisearch/src/search.rs | 6 +-- milli/Cargo.toml | 2 +- milli/src/update/settings.rs | 6 +-- 23 files changed, 137 insertions(+), 102 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index c44891518..5aa355d73 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -36,9 +36,9 @@ dependencies = [ [[package]] name = "actix-http" -version = "3.2.2" +version = "3.3.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "0c83abf9903e1f0ad9973cc4f7b9767fd5a03a583f51a5b7a339e07987cd2724" +checksum = "0070905b2c4a98d184c4e81025253cb192aa8a73827553f38e9410801ceb35bb" dependencies = [ "actix-codec", "actix-rt", @@ -46,7 +46,7 @@ dependencies = [ "actix-tls", "actix-utils", "ahash", - "base64 0.13.1", + "base64 0.21.0", "bitflags", "brotli", "bytes", @@ -68,7 +68,10 @@ dependencies = [ "rand", "sha1", "smallvec", + "tokio", + "tokio-util", "tracing", + "zstd 0.12.3+zstd.1.5.2", ] [[package]] @@ -164,9 +167,9 @@ dependencies = [ [[package]] name = "actix-web" -version = "4.2.1" +version = "4.3.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "d48f7b6534e06c7bfc72ee91db7917d4af6afe23e7d223b51e68fffbb21e96b9" +checksum = "464e0fddc668ede5f26ec1f9557a8d44eda948732f40c6b0ad79126930eb775f" dependencies = [ "actix-codec", "actix-http", @@ -1110,20 +1113,23 @@ dependencies = [ [[package]] name = "deserr" -version = "0.3.0" +version = "0.4.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "28380303ca15ec07e1d5b079baf19cf849b09edad5cab219c1c51b2bd07523de" +checksum = "a13eed41ca58d9dc99e2c67e1a5f50507dfa1b123cc4a942c81c49707bd347f0" dependencies = [ + "actix-web", "deserr-internal", + "futures", "serde-cs", "serde_json", + "strsim", ] [[package]] name = "deserr-internal" -version = "0.3.0" +version = "0.4.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "860928cd8af78d223a3d70dd581f21d7c3de8aa2eecd938e0c0a399ded7c1451" +checksum = "4d5412186d7149542b09319901d28b3c7d1f714a61d0c5d48a50560d09573ae4" dependencies = [ "convert_case 0.5.0", "proc-macro2", @@ -4423,7 +4429,7 @@ dependencies = [ "pbkdf2", "sha1", "time", - "zstd", + "zstd 0.11.2+zstd.1.5.2", ] [[package]] @@ -4432,7 +4438,16 @@ version = "0.11.2+zstd.1.5.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "20cc960326ece64f010d2d2107537f26dc589a6573a316bd5b1dba685fa5fde4" dependencies = [ - "zstd-safe", + "zstd-safe 5.0.2+zstd.1.5.2", +] + +[[package]] +name = "zstd" +version = "0.12.3+zstd.1.5.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "76eea132fb024e0e13fd9c2f5d5d595d8a967aa72382ac2f9d39fcc95afd0806" +dependencies = [ + "zstd-safe 6.0.4+zstd.1.5.4", ] [[package]] @@ -4446,10 +4461,20 @@ dependencies = [ ] [[package]] -name = "zstd-sys" -version = "2.0.5+zstd.1.5.2" +name = "zstd-safe" +version = "6.0.4+zstd.1.5.4" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "edc50ffce891ad571e9f9afe5039c4837bede781ac4bb13052ed7ae695518596" +checksum = "7afb4b54b8910cf5447638cb54bf4e8a65cbedd783af98b98c62ffe91f185543" +dependencies = [ + "libc", + "zstd-sys", +] + +[[package]] +name = "zstd-sys" +version = "2.0.7+zstd.1.5.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "94509c3ba2fe55294d752b79842c530ccfab760192521df74a081a78d2b3c7f5" dependencies = [ "cc", "libc", diff --git a/meilisearch-types/Cargo.toml b/meilisearch-types/Cargo.toml index f62202f76..021c44ea0 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.3.0" +deserr = "0.4.0" either = { version = "1.6.1", features = ["serde"] } enum-iterator = "1.1.3" file-store = { path = "../file-store" } diff --git a/meilisearch-types/src/deserr/error_messages.rs b/meilisearch-types/src/deserr/error_messages.rs index 7e288085d..f17263813 100644 --- a/meilisearch-types/src/deserr/error_messages.rs +++ b/meilisearch-types/src/deserr/error_messages.rs @@ -6,6 +6,8 @@ We try to: 2. Use the correct terms depending on the format of the request (json/query param) 3. Categorise the type of the error (e.g. missing field, wrong value type, unexpected error, etc.) */ +use std::ops::ControlFlow; + use deserr::{ErrorKind, IntoValue, ValueKind, ValuePointerRef}; use super::{DeserrJsonError, DeserrQueryParamError}; @@ -129,7 +131,7 @@ impl deserr::DeserializeError for DeserrJsonError { _self_: Option, error: deserr::ErrorKind, location: ValuePointerRef, - ) -> Result { + ) -> ControlFlow { let mut message = String::new(); message.push_str(&match error { @@ -175,7 +177,7 @@ impl deserr::DeserializeError for DeserrJsonError { } }); - Err(DeserrJsonError::new(message, C::default().error_code())) + ControlFlow::Break(DeserrJsonError::new(message, C::default().error_code())) } } @@ -222,7 +224,7 @@ impl deserr::DeserializeError for DeserrQueryParamError< _self_: Option, error: deserr::ErrorKind, location: ValuePointerRef, - ) -> Result { + ) -> ControlFlow { let mut message = String::new(); message.push_str(&match error { @@ -268,7 +270,7 @@ impl deserr::DeserializeError for DeserrQueryParamError< } }); - Err(DeserrQueryParamError::new(message, C::default().error_code())) + ControlFlow::Break(DeserrQueryParamError::new(message, C::default().error_code())) } } diff --git a/meilisearch-types/src/deserr/mod.rs b/meilisearch-types/src/deserr/mod.rs index c15b2c3a0..49474a9e6 100644 --- a/meilisearch-types/src/deserr/mod.rs +++ b/meilisearch-types/src/deserr/mod.rs @@ -1,6 +1,7 @@ use std::convert::Infallible; use std::fmt; use std::marker::PhantomData; +use std::ops::ControlFlow; use deserr::{DeserializeError, MergeWithError, ValuePointerRef}; @@ -64,8 +65,8 @@ impl _self_: Option, other: DeserrError, _merge_location: ValuePointerRef, - ) -> Result { - Err(DeserrError { msg: other.msg, code: other.code, _phantom: PhantomData }) + ) -> ControlFlow { + ControlFlow::Break(DeserrError { msg: other.msg, code: other.code, _phantom: PhantomData }) } } @@ -74,7 +75,7 @@ impl MergeWithError for DeserrError< _self_: Option, _other: Infallible, _merge_location: ValuePointerRef, - ) -> Result { + ) -> ControlFlow { unreachable!() } } @@ -112,7 +113,7 @@ macro_rules! merge_with_error_impl_take_error_message { _self_: Option, other: $err_type, merge_location: ValuePointerRef, - ) -> Result { + ) -> ControlFlow { DeserrError::::error::( None, deserr::ErrorKind::Unexpected { msg: other.to_string() }, diff --git a/meilisearch-types/src/deserr/query_params.rs b/meilisearch-types/src/deserr/query_params.rs index 28629aa1b..06d83747c 100644 --- a/meilisearch-types/src/deserr/query_params.rs +++ b/meilisearch-types/src/deserr/query_params.rs @@ -15,7 +15,7 @@ use std::convert::Infallible; use std::ops::Deref; use std::str::FromStr; -use deserr::{DeserializeError, DeserializeFromValue, MergeWithError, ValueKind}; +use deserr::{DeserializeError, Deserr, MergeWithError, ValueKind}; use super::{DeserrParseBoolError, DeserrParseIntError}; use crate::error::unwrap_any; @@ -38,7 +38,7 @@ impl Deref for Param { } } -impl DeserializeFromValue for Param +impl Deserr for Param where E: DeserializeError + MergeWithError, T: FromQueryParameter, diff --git a/meilisearch-types/src/error.rs b/meilisearch-types/src/error.rs index 39d9a1551..d83bf7eb9 100644 --- a/meilisearch-types/src/error.rs +++ b/meilisearch-types/src/error.rs @@ -382,10 +382,12 @@ impl ErrorCode for io::Error { } /// Unwrap a result, either its Ok or Err value. -pub fn unwrap_any(any: Result) -> T { +pub fn unwrap_any(any: std::ops::ControlFlow) -> T { + use std::ops::ControlFlow::*; + match any { - Ok(any) => any, - Err(any) => any, + Continue(any) => any, + Break(any) => any, } } diff --git a/meilisearch-types/src/index_uid.rs b/meilisearch-types/src/index_uid.rs index 2f3f6e5df..341ab02cb 100644 --- a/meilisearch-types/src/index_uid.rs +++ b/meilisearch-types/src/index_uid.rs @@ -2,14 +2,14 @@ use std::error::Error; use std::fmt; use std::str::FromStr; -use deserr::DeserializeFromValue; +use deserr::Deserr; use crate::error::{Code, ErrorCode}; /// An index uid is composed of only ascii alphanumeric characters, - and _, between 1 and 400 /// bytes long -#[derive(Debug, Clone, PartialEq, Eq, DeserializeFromValue)] -#[deserr(from(String) = IndexUid::try_from -> IndexUidFormatError)] +#[derive(Debug, Clone, PartialEq, Eq, Deserr)] +#[deserr(try_from(String) = IndexUid::try_from -> IndexUidFormatError)] pub struct IndexUid(String); impl IndexUid { diff --git a/meilisearch-types/src/index_uid_pattern.rs b/meilisearch-types/src/index_uid_pattern.rs index 99537b22f..baf0249e2 100644 --- a/meilisearch-types/src/index_uid_pattern.rs +++ b/meilisearch-types/src/index_uid_pattern.rs @@ -4,7 +4,7 @@ use std::fmt; use std::ops::Deref; use std::str::FromStr; -use deserr::DeserializeFromValue; +use deserr::Deserr; use serde::{Deserialize, Serialize}; use crate::error::{Code, ErrorCode}; @@ -12,8 +12,8 @@ use crate::index_uid::{IndexUid, IndexUidFormatError}; /// An index uid pattern is composed of only ascii alphanumeric characters, - and _, between 1 and 400 /// bytes long and optionally ending with a *. -#[derive(Serialize, Deserialize, DeserializeFromValue, Debug, Clone, PartialEq, Eq, Hash)] -#[deserr(from(&String) = FromStr::from_str -> IndexUidPatternFormatError)] +#[derive(Serialize, Deserialize, Deserr, Debug, Clone, PartialEq, Eq, Hash)] +#[deserr(try_from(&String) = FromStr::from_str -> IndexUidPatternFormatError)] pub struct IndexUidPattern(String); impl IndexUidPattern { diff --git a/meilisearch-types/src/keys.rs b/meilisearch-types/src/keys.rs index 804ee19c6..7478391ba 100644 --- a/meilisearch-types/src/keys.rs +++ b/meilisearch-types/src/keys.rs @@ -2,7 +2,7 @@ use std::convert::Infallible; use std::hash::Hash; use std::str::FromStr; -use deserr::{DeserializeError, DeserializeFromValue, MergeWithError, ValuePointerRef}; +use deserr::{DeserializeError, Deserr, MergeWithError, ValuePointerRef}; use enum_iterator::Sequence; use milli::update::Setting; use serde::{Deserialize, Serialize}; @@ -24,7 +24,7 @@ impl MergeWithError for Dese _self_: Option, other: IndexUidPatternFormatError, merge_location: deserr::ValuePointerRef, - ) -> std::result::Result { + ) -> std::ops::ControlFlow { DeserrError::error::( None, deserr::ErrorKind::Unexpected { msg: other.to_string() }, @@ -33,20 +33,20 @@ impl MergeWithError for Dese } } -#[derive(Debug, DeserializeFromValue)] +#[derive(Debug, Deserr)] #[deserr(error = DeserrJsonError, rename_all = camelCase, deny_unknown_fields)] pub struct CreateApiKey { #[deserr(default, error = DeserrJsonError)] pub description: Option, #[deserr(default, error = DeserrJsonError)] pub name: Option, - #[deserr(default = Uuid::new_v4(), error = DeserrJsonError, from(&String) = Uuid::from_str -> uuid::Error)] + #[deserr(default = Uuid::new_v4(), error = DeserrJsonError, try_from(&String) = Uuid::from_str -> uuid::Error)] pub uid: KeyId, #[deserr(error = DeserrJsonError, missing_field_error = DeserrJsonError::missing_api_key_actions)] pub actions: Vec, #[deserr(error = DeserrJsonError, missing_field_error = DeserrJsonError::missing_api_key_indexes)] pub indexes: Vec, - #[deserr(error = DeserrJsonError, from(Option) = parse_expiration_date -> ParseOffsetDateTimeError, missing_field_error = DeserrJsonError::missing_api_key_expires_at)] + #[deserr(error = DeserrJsonError, try_from(Option) = parse_expiration_date -> ParseOffsetDateTimeError, missing_field_error = DeserrJsonError::missing_api_key_expires_at)] pub expires_at: Option, } @@ -87,7 +87,7 @@ fn deny_immutable_fields_api_key( } } -#[derive(Debug, DeserializeFromValue)] +#[derive(Debug, Deserr)] #[deserr(error = DeserrJsonError, rename_all = camelCase, deny_unknown_fields = deny_immutable_fields_api_key)] pub struct PatchApiKey { #[deserr(default, error = DeserrJsonError)] @@ -182,9 +182,7 @@ fn parse_expiration_date( } } -#[derive( - Copy, Clone, Serialize, Deserialize, Debug, Eq, PartialEq, Hash, Sequence, DeserializeFromValue, -)] +#[derive(Copy, Clone, Serialize, Deserialize, Debug, Eq, PartialEq, Hash, Sequence, Deserr)] #[repr(u8)] pub enum Action { #[serde(rename = "*")] diff --git a/meilisearch-types/src/settings.rs b/meilisearch-types/src/settings.rs index b4ab1eff6..c7023bb10 100644 --- a/meilisearch-types/src/settings.rs +++ b/meilisearch-types/src/settings.rs @@ -3,9 +3,10 @@ use std::convert::Infallible; use std::fmt; use std::marker::PhantomData; use std::num::NonZeroUsize; +use std::ops::ControlFlow; use std::str::FromStr; -use deserr::{DeserializeError, DeserializeFromValue, ErrorKind, MergeWithError, ValuePointerRef}; +use deserr::{DeserializeError, Deserr, ErrorKind, MergeWithError, ValuePointerRef}; use fst::IntoStreamer; use milli::update::Setting; use milli::{Criterion, CriterionError, Index, DEFAULT_VALUES_PER_FACET}; @@ -41,7 +42,7 @@ pub struct Checked; #[derive(Clone, Default, Debug, Serialize, Deserialize, PartialEq, Eq)] pub struct Unchecked; -impl DeserializeFromValue for Unchecked +impl Deserr for Unchecked where E: DeserializeError, { @@ -65,7 +66,7 @@ fn validate_min_word_size_for_typo_setting( Ok(s) } -#[derive(Debug, Clone, Default, Serialize, Deserialize, PartialEq, Eq, DeserializeFromValue)] +#[derive(Debug, Clone, Default, Serialize, Deserialize, PartialEq, Eq, Deserr)] #[serde(deny_unknown_fields, rename_all = "camelCase")] #[deserr(deny_unknown_fields, rename_all = camelCase, validate = validate_min_word_size_for_typo_setting -> DeserrJsonError)] pub struct MinWordSizeTyposSetting { @@ -77,7 +78,7 @@ pub struct MinWordSizeTyposSetting { pub two_typos: Setting, } -#[derive(Debug, Clone, Default, Serialize, Deserialize, PartialEq, Eq, DeserializeFromValue)] +#[derive(Debug, Clone, Default, Serialize, Deserialize, PartialEq, Eq, Deserr)] #[serde(deny_unknown_fields, rename_all = "camelCase")] #[deserr(deny_unknown_fields, rename_all = camelCase, where_predicate = __Deserr_E: deserr::MergeWithError>)] pub struct TypoSettings { @@ -95,7 +96,7 @@ pub struct TypoSettings { pub disable_on_attributes: Setting>, } -#[derive(Debug, Clone, Default, Serialize, Deserialize, PartialEq, Eq, DeserializeFromValue)] +#[derive(Debug, Clone, Default, Serialize, Deserialize, PartialEq, Eq, Deserr)] #[serde(deny_unknown_fields, rename_all = "camelCase")] #[deserr(rename_all = camelCase, deny_unknown_fields)] pub struct FacetingSettings { @@ -104,7 +105,7 @@ pub struct FacetingSettings { pub max_values_per_facet: Setting, } -#[derive(Debug, Clone, Default, Serialize, Deserialize, PartialEq, Eq, DeserializeFromValue)] +#[derive(Debug, Clone, Default, Serialize, Deserialize, PartialEq, Eq, Deserr)] #[serde(deny_unknown_fields, rename_all = "camelCase")] #[deserr(rename_all = camelCase, deny_unknown_fields)] pub struct PaginationSettings { @@ -118,7 +119,7 @@ impl MergeWithError for DeserrJsonError, other: milli::CriterionError, merge_location: ValuePointerRef, - ) -> Result { + ) -> ControlFlow { Self::error::( None, ErrorKind::Unexpected { msg: other.to_string() }, @@ -130,7 +131,7 @@ impl MergeWithError for DeserrJsonError` from a `Settings`. -#[derive(Debug, Clone, Default, Serialize, Deserialize, PartialEq, Eq, DeserializeFromValue)] +#[derive(Debug, Clone, Default, Serialize, Deserialize, PartialEq, Eq, Deserr)] #[serde( deny_unknown_fields, rename_all = "camelCase", @@ -509,8 +510,8 @@ pub fn settings( }) } -#[derive(Debug, Clone, PartialEq, Eq, DeserializeFromValue)] -#[deserr(from(&String) = FromStr::from_str -> CriterionError)] +#[derive(Debug, Clone, PartialEq, Eq, Deserr)] +#[deserr(try_from(&String) = FromStr::from_str -> CriterionError)] pub enum RankingRuleView { /// Sorted by decreasing number of matched query terms. /// Query words at the front of an attribute is considered better than if it was at the back. diff --git a/meilisearch-types/src/star_or.rs b/meilisearch-types/src/star_or.rs index 135f610c4..0f3ef10fb 100644 --- a/meilisearch-types/src/star_or.rs +++ b/meilisearch-types/src/star_or.rs @@ -1,8 +1,9 @@ use std::fmt; use std::marker::PhantomData; +use std::ops::ControlFlow; use std::str::FromStr; -use deserr::{DeserializeError, DeserializeFromValue, MergeWithError, ValueKind}; +use deserr::{DeserializeError, Deserr, MergeWithError, ValueKind}; use serde::de::Visitor; use serde::{Deserialize, Deserializer, Serialize, Serializer}; @@ -111,7 +112,7 @@ where } } -impl DeserializeFromValue for StarOr +impl Deserr for StarOr where T: FromStr, E: DeserializeError + MergeWithError, @@ -191,7 +192,7 @@ where } } -impl DeserializeFromValue for OptionStarOr +impl Deserr for OptionStarOr where E: DeserializeError + MergeWithError, T: FromQueryParameter, @@ -271,7 +272,7 @@ impl OptionStarOrList { } } -impl DeserializeFromValue for OptionStarOrList +impl Deserr for OptionStarOrList where E: DeserializeError + MergeWithError, T: FromQueryParameter, @@ -299,7 +300,10 @@ where Err(e) => { let location = if len_cs > 1 { location.push_index(i) } else { location }; - error = Some(E::merge(error, e, location)?); + error = match E::merge(error, e, location) { + ControlFlow::Continue(e) => Some(e), + ControlFlow::Break(e) => return Err(e), + }; } } } diff --git a/meilisearch/Cargo.toml b/meilisearch/Cargo.toml index dba645665..b6410f7cb 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.3.0" +deserr = "0.4.0" 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 index e2418c9fb..c59af14db 100644 --- a/meilisearch/src/extractors/json.rs +++ b/meilisearch/src/extractors/json.rs @@ -7,7 +7,7 @@ use std::task::{Context, Poll}; use actix_web::dev::Payload; use actix_web::web::Json; use actix_web::{FromRequest, HttpRequest}; -use deserr::{DeserializeError, DeserializeFromValue}; +use deserr::{DeserializeError, Deserr}; use futures::ready; use meilisearch_types::error::{ErrorCode, ResponseError}; @@ -33,7 +33,7 @@ impl ValidatedJson { impl FromRequest for ValidatedJson where E: DeserializeError + ErrorCode + std::error::Error + 'static, - T: DeserializeFromValue, + T: Deserr, { type Error = actix_web::Error; type Future = ValidatedJsonExtractFut; @@ -54,7 +54,7 @@ pub struct ValidatedJsonExtractFut { impl Future for ValidatedJsonExtractFut where - T: DeserializeFromValue, + T: Deserr, E: DeserializeError + ErrorCode + std::error::Error + 'static, { type Output = Result, actix_web::Error>; diff --git a/meilisearch/src/extractors/query_parameters.rs b/meilisearch/src/extractors/query_parameters.rs index 99c76f3aa..39b833062 100644 --- a/meilisearch/src/extractors/query_parameters.rs +++ b/meilisearch/src/extractors/query_parameters.rs @@ -6,7 +6,7 @@ use std::{fmt, ops}; use actix_http::Payload; use actix_utils::future::{err, ok, Ready}; use actix_web::{FromRequest, HttpRequest}; -use deserr::{DeserializeError, DeserializeFromValue}; +use deserr::{DeserializeError, Deserr}; use meilisearch_types::error::{Code, ErrorCode, ResponseError}; #[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Ord)] @@ -21,7 +21,7 @@ impl QueryParameter { impl QueryParameter where - T: DeserializeFromValue, + T: Deserr, E: DeserializeError + ErrorCode + std::error::Error + 'static, { pub fn from_query(query_str: &str) -> Result { @@ -57,7 +57,7 @@ impl fmt::Display for QueryParameter { impl FromRequest for QueryParameter where - T: DeserializeFromValue, + T: Deserr, E: DeserializeError + ErrorCode + std::error::Error + 'static, { type Error = actix_web::Error; diff --git a/meilisearch/src/routes/api_key.rs b/meilisearch/src/routes/api_key.rs index efc591d54..096aa7df0 100644 --- a/meilisearch/src/routes/api_key.rs +++ b/meilisearch/src/routes/api_key.rs @@ -1,7 +1,7 @@ use std::str; use actix_web::{web, HttpRequest, HttpResponse}; -use deserr::DeserializeFromValue; +use deserr::Deserr; use meilisearch_auth::error::AuthControllerError; use meilisearch_auth::AuthController; use meilisearch_types::deserr::query_params::Param; @@ -51,7 +51,7 @@ pub async fn create_api_key( Ok(HttpResponse::Created().json(res)) } -#[derive(DeserializeFromValue, Debug, Clone, Copy)] +#[derive(Deserr, Debug, Clone, Copy)] #[deserr(error = DeserrQueryParamError, rename_all = camelCase, deny_unknown_fields)] pub struct ListApiKeys { #[deserr(default, error = DeserrQueryParamError)] diff --git a/meilisearch/src/routes/indexes/documents.rs b/meilisearch/src/routes/indexes/documents.rs index 0ec1057ae..90bf70692 100644 --- a/meilisearch/src/routes/indexes/documents.rs +++ b/meilisearch/src/routes/indexes/documents.rs @@ -4,7 +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::DeserializeFromValue; +use deserr::Deserr; use futures::StreamExt; use index_scheduler::IndexScheduler; use log::debug; @@ -80,7 +80,7 @@ pub fn configure(cfg: &mut web::ServiceConfig) { ); } -#[derive(Debug, DeserializeFromValue)] +#[derive(Debug, Deserr)] #[deserr(error = DeserrQueryParamError, rename_all = camelCase, deny_unknown_fields)] pub struct GetDocument { #[deserr(default, error = DeserrQueryParamError)] @@ -125,7 +125,7 @@ pub async fn delete_document( Ok(HttpResponse::Accepted().json(task)) } -#[derive(Debug, DeserializeFromValue)] +#[derive(Debug, Deserr)] #[deserr(error = DeserrQueryParamError, rename_all = camelCase, deny_unknown_fields)] pub struct BrowseQuery { #[deserr(default, error = DeserrQueryParamError)] @@ -155,7 +155,7 @@ pub async fn get_all_documents( Ok(HttpResponse::Ok().json(ret)) } -#[derive(Deserialize, Debug, DeserializeFromValue)] +#[derive(Deserialize, Debug, Deserr)] #[deserr(error = DeserrJsonError, rename_all = camelCase, deny_unknown_fields)] pub struct UpdateDocumentsQuery { #[deserr(default, error = DeserrJsonError)] diff --git a/meilisearch/src/routes/indexes/mod.rs b/meilisearch/src/routes/indexes/mod.rs index 2d352bfe5..c087fe202 100644 --- a/meilisearch/src/routes/indexes/mod.rs +++ b/meilisearch/src/routes/indexes/mod.rs @@ -2,7 +2,7 @@ use std::convert::Infallible; use actix_web::web::Data; use actix_web::{web, HttpRequest, HttpResponse}; -use deserr::{DeserializeError, DeserializeFromValue, ValuePointerRef}; +use deserr::{DeserializeError, Deserr, ValuePointerRef}; use index_scheduler::IndexScheduler; use log::debug; use meilisearch_types::deserr::error_messages::immutable_field_error; @@ -73,7 +73,7 @@ impl IndexView { } } -#[derive(DeserializeFromValue, Debug, Clone, Copy)] +#[derive(Deserr, Debug, Clone, Copy)] #[deserr(error = DeserrQueryParamError, rename_all = camelCase, deny_unknown_fields)] pub struct ListIndexes { #[deserr(default, error = DeserrQueryParamError)] @@ -105,7 +105,7 @@ pub async fn list_indexes( Ok(HttpResponse::Ok().json(ret)) } -#[derive(DeserializeFromValue, Debug)] +#[derive(Deserr, Debug)] #[deserr(error = DeserrJsonError, rename_all = camelCase, deny_unknown_fields)] pub struct IndexCreateRequest { #[deserr(error = DeserrJsonError, missing_field_error = DeserrJsonError::missing_index_uid)] @@ -157,7 +157,7 @@ fn deny_immutable_fields_index( } } -#[derive(DeserializeFromValue, Debug)] +#[derive(Deserr, Debug)] #[deserr(error = DeserrJsonError, rename_all = camelCase, deny_unknown_fields = deny_immutable_fields_index)] pub struct UpdateIndexRequest { #[deserr(default, error = DeserrJsonError)] diff --git a/meilisearch/src/routes/indexes/search.rs b/meilisearch/src/routes/indexes/search.rs index 545c69ec5..50a2ffd74 100644 --- a/meilisearch/src/routes/indexes/search.rs +++ b/meilisearch/src/routes/indexes/search.rs @@ -31,7 +31,7 @@ pub fn configure(cfg: &mut web::ServiceConfig) { ); } -#[derive(Debug, deserr::DeserializeFromValue)] +#[derive(Debug, deserr::Deserr)] #[deserr(error = DeserrQueryParamError, rename_all = camelCase, deny_unknown_fields)] pub struct SearchQueryGet { #[deserr(default, error = DeserrQueryParamError)] diff --git a/meilisearch/src/routes/swap_indexes.rs b/meilisearch/src/routes/swap_indexes.rs index 4a7802f2e..2070177d9 100644 --- a/meilisearch/src/routes/swap_indexes.rs +++ b/meilisearch/src/routes/swap_indexes.rs @@ -1,6 +1,6 @@ use actix_web::web::Data; use actix_web::{web, HttpRequest, HttpResponse}; -use deserr::DeserializeFromValue; +use deserr::Deserr; use index_scheduler::IndexScheduler; use meilisearch_types::deserr::DeserrJsonError; use meilisearch_types::error::deserr_codes::InvalidSwapIndexes; @@ -21,7 +21,7 @@ pub fn configure(cfg: &mut web::ServiceConfig) { cfg.service(web::resource("").route(web::post().to(SeqHandler(swap_indexes)))); } -#[derive(DeserializeFromValue, Debug, Clone, PartialEq, Eq)] +#[derive(Deserr, Debug, Clone, PartialEq, Eq)] #[deserr(error = DeserrJsonError, rename_all = camelCase, deny_unknown_fields)] pub struct SwapIndexesPayload { #[deserr(error = DeserrJsonError, missing_field_error = DeserrJsonError::missing_swap_indexes)] diff --git a/meilisearch/src/routes/tasks.rs b/meilisearch/src/routes/tasks.rs index b78be7876..f747320b1 100644 --- a/meilisearch/src/routes/tasks.rs +++ b/meilisearch/src/routes/tasks.rs @@ -1,6 +1,6 @@ use actix_web::web::Data; use actix_web::{web, HttpRequest, HttpResponse}; -use deserr::DeserializeFromValue; +use deserr::Deserr; use index_scheduler::{IndexScheduler, Query, TaskId}; use meilisearch_types::deserr::query_params::Param; use meilisearch_types::deserr::DeserrQueryParamError; @@ -162,7 +162,7 @@ impl From
for DetailsView { } } -#[derive(Debug, DeserializeFromValue)] +#[derive(Debug, Deserr)] #[deserr(error = DeserrQueryParamError, rename_all = camelCase, deny_unknown_fields)] pub struct TasksFilterQuery { #[deserr(default = Param(DEFAULT_LIMIT), error = DeserrQueryParamError)] @@ -181,19 +181,20 @@ pub struct TasksFilterQuery { #[deserr(default, error = DeserrQueryParamError)] pub index_uids: OptionStarOrList, - #[deserr(default, error = DeserrQueryParamError, from(OptionStarOr) = deserialize_date_after -> InvalidTaskDateError)] + #[deserr(default, error = DeserrQueryParamError, try_from(OptionStarOr) = deserialize_date_after -> InvalidTaskDateError)] pub after_enqueued_at: OptionStarOr, - #[deserr(default, error = DeserrQueryParamError, from(OptionStarOr) = deserialize_date_before -> InvalidTaskDateError)] + #[deserr(default, error = DeserrQueryParamError, try_from(OptionStarOr) = deserialize_date_before -> InvalidTaskDateError)] pub before_enqueued_at: OptionStarOr, - #[deserr(default, error = DeserrQueryParamError, from(OptionStarOr) = deserialize_date_after -> InvalidTaskDateError)] + #[deserr(default, error = DeserrQueryParamError, try_from(OptionStarOr) = deserialize_date_after -> InvalidTaskDateError)] pub after_started_at: OptionStarOr, - #[deserr(default, error = DeserrQueryParamError, from(OptionStarOr) = deserialize_date_before -> InvalidTaskDateError)] + #[deserr(default, error = DeserrQueryParamError, try_from(OptionStarOr) = deserialize_date_before -> InvalidTaskDateError)] pub before_started_at: OptionStarOr, - #[deserr(default, error = DeserrQueryParamError, from(OptionStarOr) = deserialize_date_after -> InvalidTaskDateError)] + #[deserr(default, error = DeserrQueryParamError, try_from(OptionStarOr) = deserialize_date_after -> InvalidTaskDateError)] pub after_finished_at: OptionStarOr, - #[deserr(default, error = DeserrQueryParamError, from(OptionStarOr) = deserialize_date_before -> InvalidTaskDateError)] + #[deserr(default, error = DeserrQueryParamError, try_from(OptionStarOr) = deserialize_date_before -> InvalidTaskDateError)] pub before_finished_at: OptionStarOr, } + impl TasksFilterQuery { fn into_query(self) -> Query { Query { @@ -235,7 +236,7 @@ impl TaskDeletionOrCancelationQuery { } } -#[derive(Debug, DeserializeFromValue)] +#[derive(Debug, Deserr)] #[deserr(error = DeserrQueryParamError, rename_all = camelCase, deny_unknown_fields)] pub struct TaskDeletionOrCancelationQuery { #[deserr(default, error = DeserrQueryParamError)] @@ -249,19 +250,20 @@ pub struct TaskDeletionOrCancelationQuery { #[deserr(default, error = DeserrQueryParamError)] pub index_uids: OptionStarOrList, - #[deserr(default, error = DeserrQueryParamError, from(OptionStarOr) = deserialize_date_after -> InvalidTaskDateError)] + #[deserr(default, error = DeserrQueryParamError, try_from(OptionStarOr) = deserialize_date_after -> InvalidTaskDateError)] pub after_enqueued_at: OptionStarOr, - #[deserr(default, error = DeserrQueryParamError, from(OptionStarOr) = deserialize_date_before -> InvalidTaskDateError)] + #[deserr(default, error = DeserrQueryParamError, try_from(OptionStarOr) = deserialize_date_before -> InvalidTaskDateError)] pub before_enqueued_at: OptionStarOr, - #[deserr(default, error = DeserrQueryParamError, from(OptionStarOr) = deserialize_date_after -> InvalidTaskDateError)] + #[deserr(default, error = DeserrQueryParamError, try_from(OptionStarOr) = deserialize_date_after -> InvalidTaskDateError)] pub after_started_at: OptionStarOr, - #[deserr(default, error = DeserrQueryParamError, from(OptionStarOr) = deserialize_date_before -> InvalidTaskDateError)] + #[deserr(default, error = DeserrQueryParamError, try_from(OptionStarOr) = deserialize_date_before -> InvalidTaskDateError)] pub before_started_at: OptionStarOr, - #[deserr(default, error = DeserrQueryParamError, from(OptionStarOr) = deserialize_date_after -> InvalidTaskDateError)] + #[deserr(default, error = DeserrQueryParamError, try_from(OptionStarOr) = deserialize_date_after -> InvalidTaskDateError)] pub after_finished_at: OptionStarOr, - #[deserr(default, error = DeserrQueryParamError, from(OptionStarOr) = deserialize_date_before -> InvalidTaskDateError)] + #[deserr(default, error = DeserrQueryParamError, try_from(OptionStarOr) = deserialize_date_before -> InvalidTaskDateError)] pub before_finished_at: OptionStarOr, } + impl TaskDeletionOrCancelationQuery { fn into_query(self) -> Query { Query { @@ -498,7 +500,7 @@ pub fn deserialize_date_before( #[cfg(test)] mod tests { - use deserr::DeserializeFromValue; + use deserr::Deserr; use meili_snap::snapshot; use meilisearch_types::deserr::DeserrQueryParamError; use meilisearch_types::error::{Code, ResponseError}; @@ -507,7 +509,7 @@ mod tests { fn deserr_query_params(j: &str) -> Result where - T: DeserializeFromValue, + T: Deserr, { let value = serde_urlencoded::from_str::(j) .map_err(|e| ResponseError::from_msg(e.to_string(), Code::BadRequest))?; diff --git a/meilisearch/src/search.rs b/meilisearch/src/search.rs index d4a65cc39..f48563141 100644 --- a/meilisearch/src/search.rs +++ b/meilisearch/src/search.rs @@ -3,7 +3,7 @@ use std::collections::{BTreeMap, BTreeSet, HashSet}; use std::str::FromStr; use std::time::Instant; -use deserr::DeserializeFromValue; +use deserr::Deserr; use either::Either; use meilisearch_types::deserr::DeserrJsonError; use meilisearch_types::error::deserr_codes::*; @@ -29,7 +29,7 @@ pub const DEFAULT_CROP_MARKER: fn() -> String = || "…".to_string(); pub const DEFAULT_HIGHLIGHT_PRE_TAG: fn() -> String = || "".to_string(); pub const DEFAULT_HIGHLIGHT_POST_TAG: fn() -> String = || "".to_string(); -#[derive(Debug, Clone, Default, PartialEq, Eq, DeserializeFromValue)] +#[derive(Debug, Clone, Default, PartialEq, Eq, Deserr)] #[deserr(error = DeserrJsonError, rename_all = camelCase, deny_unknown_fields)] pub struct SearchQuery { #[deserr(default, error = DeserrJsonError)] @@ -74,7 +74,7 @@ impl SearchQuery { } } -#[derive(Debug, Clone, PartialEq, Eq, DeserializeFromValue)] +#[derive(Debug, Clone, PartialEq, Eq, Deserr)] #[deserr(rename_all = camelCase)] pub enum MatchingStrategy { /// Remove query words from last to first diff --git a/milli/Cargo.toml b/milli/Cargo.toml index d9458ca70..70658d81d 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.3.0" +deserr = "0.4.0" either = "1.8.0" flatten-serde-json = { path = "../flatten-serde-json" } fst = "0.4.7" diff --git a/milli/src/update/settings.rs b/milli/src/update/settings.rs index 1646ab5b2..4f4fa25d6 100644 --- a/milli/src/update/settings.rs +++ b/milli/src/update/settings.rs @@ -2,7 +2,7 @@ use std::collections::{BTreeSet, HashMap, HashSet}; use std::result::Result as StdResult; use charabia::{Tokenizer, TokenizerBuilder}; -use deserr::{DeserializeError, DeserializeFromValue}; +use deserr::{DeserializeError, Deserr}; use itertools::Itertools; use serde::{Deserialize, Deserializer, Serialize, Serializer}; use time::OffsetDateTime; @@ -23,9 +23,9 @@ pub enum Setting { NotSet, } -impl DeserializeFromValue for Setting +impl Deserr for Setting where - T: DeserializeFromValue, + T: Deserr, E: DeserializeError, { fn deserialize_from_value( From 769576fd9465ff4c19aa04d04d6f01bd76f237f0 Mon Sep 17 00:00:00 2001 From: Tamo Date: Mon, 13 Feb 2023 19:34:47 +0100 Subject: [PATCH 2/4] get rids of the whole error_message module since it has been integrated into the last version of deserr --- .../src/deserr/error_messages.rs | 330 ------------------ meilisearch-types/src/deserr/mod.rs | 49 ++- meilisearch-types/src/keys.rs | 3 +- meilisearch/src/routes/indexes/mod.rs | 3 +- meilisearch/tests/auth/errors.rs | 4 +- 5 files changed, 48 insertions(+), 341 deletions(-) delete mode 100644 meilisearch-types/src/deserr/error_messages.rs diff --git a/meilisearch-types/src/deserr/error_messages.rs b/meilisearch-types/src/deserr/error_messages.rs deleted file mode 100644 index f17263813..000000000 --- a/meilisearch-types/src/deserr/error_messages.rs +++ /dev/null @@ -1,330 +0,0 @@ -/*! -This module implements the error messages of deserialization errors. - -We try to: -1. Give a human-readable description of where the error originated. -2. Use the correct terms depending on the format of the request (json/query param) -3. Categorise the type of the error (e.g. missing field, wrong value type, unexpected error, etc.) - */ -use std::ops::ControlFlow; - -use deserr::{ErrorKind, IntoValue, ValueKind, ValuePointerRef}; - -use super::{DeserrJsonError, DeserrQueryParamError}; -use crate::error::{Code, ErrorCode}; - -/// Return a description of the given location in a Json, preceded by the given article. -/// e.g. `at .key1[8].key2`. If the location is the origin, the given article will not be -/// included in the description. -pub fn location_json_description(location: ValuePointerRef, article: &str) -> String { - fn rec(location: ValuePointerRef) -> String { - match location { - ValuePointerRef::Origin => String::new(), - ValuePointerRef::Key { key, prev } => rec(*prev) + "." + key, - ValuePointerRef::Index { index, prev } => format!("{}[{index}]", rec(*prev)), - } - } - match location { - ValuePointerRef::Origin => String::new(), - _ => { - format!("{article} `{}`", rec(location)) - } - } -} - -/// Return a description of the list of value kinds for a Json payload. -fn value_kinds_description_json(kinds: &[ValueKind]) -> String { - // Rank each value kind so that they can be sorted (and deduplicated) - // Having a predictable order helps with pattern matching - fn order(kind: &ValueKind) -> u8 { - match kind { - ValueKind::Null => 0, - ValueKind::Boolean => 1, - ValueKind::Integer => 2, - ValueKind::NegativeInteger => 3, - ValueKind::Float => 4, - ValueKind::String => 5, - ValueKind::Sequence => 6, - ValueKind::Map => 7, - } - } - // Return a description of a single value kind, preceded by an article - fn single_description(kind: &ValueKind) -> &'static str { - match kind { - ValueKind::Null => "null", - ValueKind::Boolean => "a boolean", - ValueKind::Integer => "a positive integer", - ValueKind::NegativeInteger => "a negative integer", - ValueKind::Float => "a number", - ValueKind::String => "a string", - ValueKind::Sequence => "an array", - ValueKind::Map => "an object", - } - } - - fn description_rec(kinds: &[ValueKind], count_items: &mut usize, message: &mut String) { - let (msg_part, rest): (_, &[ValueKind]) = match kinds { - [] => (String::new(), &[]), - [ValueKind::Integer | ValueKind::NegativeInteger, ValueKind::Float, rest @ ..] => { - ("a number".to_owned(), rest) - } - [ValueKind::Integer, ValueKind::NegativeInteger, ValueKind::Float, rest @ ..] => { - ("a number".to_owned(), rest) - } - [ValueKind::Integer, ValueKind::NegativeInteger, rest @ ..] => { - ("an integer".to_owned(), rest) - } - [a] => (single_description(a).to_owned(), &[]), - [a, rest @ ..] => (single_description(a).to_owned(), rest), - }; - - if rest.is_empty() { - if *count_items == 0 { - message.push_str(&msg_part); - } else if *count_items == 1 { - message.push_str(&format!(" or {msg_part}")); - } else { - message.push_str(&format!(", or {msg_part}")); - } - } else { - if *count_items == 0 { - message.push_str(&msg_part); - } else { - message.push_str(&format!(", {msg_part}")); - } - - *count_items += 1; - description_rec(rest, count_items, message); - } - } - - let mut kinds = kinds.to_owned(); - kinds.sort_by_key(order); - kinds.dedup(); - - if kinds.is_empty() { - // Should not happen ideally - "a different value".to_owned() - } else { - let mut message = String::new(); - description_rec(kinds.as_slice(), &mut 0, &mut message); - message - } -} - -/// Return the JSON string of the value preceded by a description of its kind -fn value_description_with_kind_json(v: &serde_json::Value) -> String { - match v.kind() { - ValueKind::Null => "null".to_owned(), - kind => { - format!( - "{}: `{}`", - value_kinds_description_json(&[kind]), - serde_json::to_string(v).unwrap() - ) - } - } -} - -impl deserr::DeserializeError for DeserrJsonError { - fn error( - _self_: Option, - error: deserr::ErrorKind, - location: ValuePointerRef, - ) -> ControlFlow { - let mut message = String::new(); - - message.push_str(&match error { - ErrorKind::IncorrectValueKind { actual, accepted } => { - let expected = value_kinds_description_json(accepted); - let received = value_description_with_kind_json(&serde_json::Value::from(actual)); - - let location = location_json_description(location, " at"); - - format!("Invalid value type{location}: expected {expected}, but found {received}") - } - ErrorKind::MissingField { field } => { - let location = location_json_description(location, " inside"); - format!("Missing field `{field}`{location}") - } - ErrorKind::UnknownKey { key, accepted } => { - let location = location_json_description(location, " inside"); - format!( - "Unknown field `{}`{location}: expected one of {}", - key, - accepted - .iter() - .map(|accepted| format!("`{}`", accepted)) - .collect::>() - .join(", ") - ) - } - ErrorKind::UnknownValue { value, accepted } => { - let location = location_json_description(location, " at"); - format!( - "Unknown value `{}`{location}: expected one of {}", - value, - accepted - .iter() - .map(|accepted| format!("`{}`", accepted)) - .collect::>() - .join(", "), - ) - } - ErrorKind::Unexpected { msg } => { - let location = location_json_description(location, " at"); - format!("Invalid value{location}: {msg}") - } - }); - - ControlFlow::Break(DeserrJsonError::new(message, C::default().error_code())) - } -} - -pub fn immutable_field_error(field: &str, accepted: &[&str], code: Code) -> DeserrJsonError { - let msg = format!( - "Immutable field `{field}`: expected one of {}", - accepted - .iter() - .map(|accepted| format!("`{}`", accepted)) - .collect::>() - .join(", ") - ); - - DeserrJsonError::new(msg, code) -} - -/// Return a description of the given location in query parameters, preceded by the -/// given article. e.g. `at key5[2]`. If the location is the origin, the given article -/// will not be included in the description. -pub fn location_query_param_description(location: ValuePointerRef, article: &str) -> String { - fn rec(location: ValuePointerRef) -> String { - match location { - ValuePointerRef::Origin => String::new(), - ValuePointerRef::Key { key, prev } => { - if matches!(prev, ValuePointerRef::Origin) { - key.to_owned() - } else { - rec(*prev) + "." + key - } - } - ValuePointerRef::Index { index, prev } => format!("{}[{index}]", rec(*prev)), - } - } - match location { - ValuePointerRef::Origin => String::new(), - _ => { - format!("{article} `{}`", rec(location)) - } - } -} - -impl deserr::DeserializeError for DeserrQueryParamError { - fn error( - _self_: Option, - error: deserr::ErrorKind, - location: ValuePointerRef, - ) -> ControlFlow { - let mut message = String::new(); - - message.push_str(&match error { - ErrorKind::IncorrectValueKind { actual, accepted } => { - let expected = value_kinds_description_query_param(accepted); - let received = value_description_with_kind_query_param(actual); - - let location = location_query_param_description(location, " for parameter"); - - format!("Invalid value type{location}: expected {expected}, but found {received}") - } - ErrorKind::MissingField { field } => { - let location = location_query_param_description(location, " inside"); - format!("Missing parameter `{field}`{location}") - } - ErrorKind::UnknownKey { key, accepted } => { - let location = location_query_param_description(location, " inside"); - format!( - "Unknown parameter `{}`{location}: expected one of {}", - key, - accepted - .iter() - .map(|accepted| format!("`{}`", accepted)) - .collect::>() - .join(", ") - ) - } - ErrorKind::UnknownValue { value, accepted } => { - let location = location_query_param_description(location, " for parameter"); - format!( - "Unknown value `{}`{location}: expected one of {}", - value, - accepted - .iter() - .map(|accepted| format!("`{}`", accepted)) - .collect::>() - .join(", "), - ) - } - ErrorKind::Unexpected { msg } => { - let location = location_query_param_description(location, " in parameter"); - format!("Invalid value{location}: {msg}") - } - }); - - ControlFlow::Break(DeserrQueryParamError::new(message, C::default().error_code())) - } -} - -/// Return a description of the list of value kinds for query parameters -/// Since query parameters are always treated as strings, we always return -/// "a string" for now. -fn value_kinds_description_query_param(_accepted: &[ValueKind]) -> String { - "a string".to_owned() -} - -fn value_description_with_kind_query_param(actual: deserr::Value) -> String { - match actual { - deserr::Value::Null => "null".to_owned(), - deserr::Value::Boolean(x) => format!("a boolean: `{x}`"), - deserr::Value::Integer(x) => format!("an integer: `{x}`"), - deserr::Value::NegativeInteger(x) => { - format!("an integer: `{x}`") - } - deserr::Value::Float(x) => { - format!("a number: `{x}`") - } - deserr::Value::String(x) => { - format!("a string: `{x}`") - } - deserr::Value::Sequence(_) => "multiple values".to_owned(), - deserr::Value::Map(_) => "multiple parameters".to_owned(), - } -} - -#[cfg(test)] -mod tests { - use deserr::ValueKind; - - use crate::deserr::error_messages::value_kinds_description_json; - - #[test] - fn test_value_kinds_description_json() { - insta::assert_display_snapshot!(value_kinds_description_json(&[]), @"a different value"); - - insta::assert_display_snapshot!(value_kinds_description_json(&[ValueKind::Boolean]), @"a boolean"); - insta::assert_display_snapshot!(value_kinds_description_json(&[ValueKind::Integer]), @"a positive integer"); - insta::assert_display_snapshot!(value_kinds_description_json(&[ValueKind::NegativeInteger]), @"a negative integer"); - insta::assert_display_snapshot!(value_kinds_description_json(&[ValueKind::Integer]), @"a positive integer"); - insta::assert_display_snapshot!(value_kinds_description_json(&[ValueKind::String]), @"a string"); - insta::assert_display_snapshot!(value_kinds_description_json(&[ValueKind::Sequence]), @"an array"); - insta::assert_display_snapshot!(value_kinds_description_json(&[ValueKind::Map]), @"an object"); - - insta::assert_display_snapshot!(value_kinds_description_json(&[ValueKind::Integer, ValueKind::Boolean]), @"a boolean or a positive integer"); - insta::assert_display_snapshot!(value_kinds_description_json(&[ValueKind::Null, ValueKind::Integer]), @"null or a positive integer"); - insta::assert_display_snapshot!(value_kinds_description_json(&[ValueKind::Sequence, ValueKind::NegativeInteger]), @"a negative integer or an array"); - insta::assert_display_snapshot!(value_kinds_description_json(&[ValueKind::Integer, ValueKind::Float]), @"a number"); - insta::assert_display_snapshot!(value_kinds_description_json(&[ValueKind::Integer, ValueKind::Float, ValueKind::NegativeInteger]), @"a number"); - insta::assert_display_snapshot!(value_kinds_description_json(&[ValueKind::Integer, ValueKind::Float, ValueKind::NegativeInteger, ValueKind::Null]), @"null or a number"); - insta::assert_display_snapshot!(value_kinds_description_json(&[ValueKind::Boolean, ValueKind::Integer, ValueKind::Float, ValueKind::NegativeInteger, ValueKind::Null]), @"null, a boolean, or a number"); - insta::assert_display_snapshot!(value_kinds_description_json(&[ValueKind::Null, ValueKind::Boolean, ValueKind::Integer, ValueKind::Float, ValueKind::NegativeInteger, ValueKind::Null]), @"null, a boolean, or a number"); - } -} diff --git a/meilisearch-types/src/deserr/mod.rs b/meilisearch-types/src/deserr/mod.rs index 49474a9e6..33213c953 100644 --- a/meilisearch-types/src/deserr/mod.rs +++ b/meilisearch-types/src/deserr/mod.rs @@ -3,9 +3,10 @@ use std::fmt; use std::marker::PhantomData; use std::ops::ControlFlow; -use deserr::{DeserializeError, MergeWithError, ValuePointerRef}; +use deserr::errors::{JsonError, QueryParamError}; +use deserr::{take_cf_content, DeserializeError, IntoValue, MergeWithError, ValuePointerRef}; -use crate::error::deserr_codes::{self, *}; +use crate::error::deserr_codes::*; use crate::error::{ unwrap_any, Code, DeserrParseBoolError, DeserrParseIntError, ErrorCode, InvalidTaskDateError, ParseOffsetDateTimeError, @@ -13,7 +14,6 @@ use crate::error::{ use crate::index_uid::IndexUidFormatError; use crate::tasks::{ParseTaskKindError, ParseTaskStatusError}; -pub mod error_messages; pub mod query_params; /// Marker type for the Json format @@ -21,8 +21,8 @@ pub struct DeserrJson; /// Marker type for the Query Parameter format pub struct DeserrQueryParam; -pub type DeserrJsonError = DeserrError; -pub type DeserrQueryParamError = DeserrError; +pub type DeserrJsonError = DeserrError; +pub type DeserrQueryParamError = DeserrError; /// A request deserialization error. /// @@ -80,6 +80,45 @@ impl MergeWithError for DeserrError< } } +impl DeserializeError for DeserrJsonError { + fn error( + _self_: Option, + error: deserr::ErrorKind, + location: ValuePointerRef, + ) -> ControlFlow { + ControlFlow::Break(DeserrJsonError::new( + take_cf_content(JsonError::error(None, error, location)).to_string(), + C::default().error_code(), + )) + } +} + +impl DeserializeError for DeserrQueryParamError { + fn error( + _self_: Option, + error: deserr::ErrorKind, + location: ValuePointerRef, + ) -> ControlFlow { + ControlFlow::Break(DeserrQueryParamError::new( + take_cf_content(QueryParamError::error(None, error, location)).to_string(), + C::default().error_code(), + )) + } +} + +pub fn immutable_field_error(field: &str, accepted: &[&str], code: Code) -> DeserrJsonError { + let msg = format!( + "Immutable field `{field}`: expected one of {}", + accepted + .iter() + .map(|accepted| format!("`{}`", accepted)) + .collect::>() + .join(", ") + ); + + DeserrJsonError::new(msg, code) +} + // Implement a convenience function to build a `missing_field` error macro_rules! make_missing_field_convenience_builder { ($err_code:ident, $fn_name:ident) => { diff --git a/meilisearch-types/src/keys.rs b/meilisearch-types/src/keys.rs index 7478391ba..87ed543d6 100644 --- a/meilisearch-types/src/keys.rs +++ b/meilisearch-types/src/keys.rs @@ -11,8 +11,7 @@ use time::macros::{format_description, time}; use time::{Date, OffsetDateTime, PrimitiveDateTime}; use uuid::Uuid; -use crate::deserr::error_messages::immutable_field_error; -use crate::deserr::{DeserrError, DeserrJsonError}; +use crate::deserr::{immutable_field_error, DeserrError, DeserrJsonError}; use crate::error::deserr_codes::*; use crate::error::{unwrap_any, Code, ErrorCode, ParseOffsetDateTimeError}; use crate::index_uid_pattern::{IndexUidPattern, IndexUidPatternFormatError}; diff --git a/meilisearch/src/routes/indexes/mod.rs b/meilisearch/src/routes/indexes/mod.rs index c087fe202..16b9faa24 100644 --- a/meilisearch/src/routes/indexes/mod.rs +++ b/meilisearch/src/routes/indexes/mod.rs @@ -5,9 +5,8 @@ use actix_web::{web, HttpRequest, HttpResponse}; use deserr::{DeserializeError, Deserr, ValuePointerRef}; use index_scheduler::IndexScheduler; use log::debug; -use meilisearch_types::deserr::error_messages::immutable_field_error; use meilisearch_types::deserr::query_params::Param; -use meilisearch_types::deserr::{DeserrJsonError, DeserrQueryParamError}; +use meilisearch_types::deserr::{immutable_field_error, DeserrJsonError, DeserrQueryParamError}; use meilisearch_types::error::deserr_codes::*; use meilisearch_types::error::{unwrap_any, Code, ResponseError}; use meilisearch_types::index_uid::IndexUid; diff --git a/meilisearch/tests/auth/errors.rs b/meilisearch/tests/auth/errors.rs index 0bfef878b..904bb182d 100644 --- a/meilisearch/tests/auth/errors.rs +++ b/meilisearch/tests/auth/errors.rs @@ -138,7 +138,7 @@ async fn create_api_key_bad_expires_at() { snapshot!(code, @"400 Bad Request"); snapshot!(json_string!(response), @r###" { - "message": "Unknown field `expires_at`: expected one of `description`, `name`, `uid`, `actions`, `indexes`, `expiresAt`", + "message": "Unknown field `expires_at`: did you mean `expiresAt`? expected one of `description`, `name`, `uid`, `actions`, `indexes`, `expiresAt`", "code": "bad_request", "type": "invalid_request", "link": "https://docs.meilisearch.com/errors#bad_request" @@ -150,7 +150,7 @@ async fn create_api_key_bad_expires_at() { snapshot!(code, @"400 Bad Request"); snapshot!(json_string!(response), @r###" { - "message": "Unknown field `expires_at`: expected one of `description`, `name`, `uid`, `actions`, `indexes`, `expiresAt`", + "message": "Unknown field `expires_at`: did you mean `expiresAt`? expected one of `description`, `name`, `uid`, `actions`, `indexes`, `expiresAt`", "code": "bad_request", "type": "invalid_request", "link": "https://docs.meilisearch.com/errors#bad_request" From a43765d45473e41ab0fd6a2298b676742142af5f Mon Sep 17 00:00:00 2001 From: Tamo Date: Tue, 14 Feb 2023 13:12:42 +0100 Subject: [PATCH 3/4] 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" From 42a3cdca66eea60c69a5ab04c1a37a8b67a29cc4 Mon Sep 17 00:00:00 2001 From: Tamo Date: Tue, 14 Feb 2023 13:58:33 +0100 Subject: [PATCH 4/4] get rids of the unwrap_any function in favor of take_cf_content --- meilisearch-types/src/deserr/mod.rs | 4 ++-- meilisearch-types/src/deserr/query_params.rs | 5 ++--- meilisearch-types/src/error.rs | 10 ---------- meilisearch-types/src/keys.rs | 4 ++-- meilisearch-types/src/settings.rs | 3 +-- meilisearch-types/src/star_or.rs | 11 +++++------ meilisearch/src/routes/indexes/mod.rs | 4 ++-- 7 files changed, 14 insertions(+), 27 deletions(-) diff --git a/meilisearch-types/src/deserr/mod.rs b/meilisearch-types/src/deserr/mod.rs index ad6f72e2c..3e6ec8b96 100644 --- a/meilisearch-types/src/deserr/mod.rs +++ b/meilisearch-types/src/deserr/mod.rs @@ -8,7 +8,7 @@ use deserr::{take_cf_content, DeserializeError, IntoValue, MergeWithError, Value use crate::error::deserr_codes::*; use crate::error::{ - unwrap_any, Code, DeserrParseBoolError, DeserrParseIntError, ErrorCode, InvalidTaskDateError, + Code, DeserrParseBoolError, DeserrParseIntError, ErrorCode, InvalidTaskDateError, ParseOffsetDateTimeError, }; use crate::index_uid::IndexUidFormatError; @@ -135,7 +135,7 @@ macro_rules! make_missing_field_convenience_builder { ($err_code:ident, $fn_name:ident) => { impl DeserrJsonError<$err_code> { pub fn $fn_name(field: &str, location: ValuePointerRef) -> Self { - let x = unwrap_any(Self::error::( + let x = deserr::take_cf_content(Self::error::( None, deserr::ErrorKind::MissingField { field }, location, diff --git a/meilisearch-types/src/deserr/query_params.rs b/meilisearch-types/src/deserr/query_params.rs index 06d83747c..dded0ea5c 100644 --- a/meilisearch-types/src/deserr/query_params.rs +++ b/meilisearch-types/src/deserr/query_params.rs @@ -18,7 +18,6 @@ use std::str::FromStr; use deserr::{DeserializeError, Deserr, MergeWithError, ValueKind}; use super::{DeserrParseBoolError, DeserrParseIntError}; -use crate::error::unwrap_any; use crate::index_uid::IndexUid; use crate::tasks::{Kind, Status}; @@ -50,9 +49,9 @@ where match value { deserr::Value::String(s) => match T::from_query_param(&s) { Ok(x) => Ok(Param(x)), - Err(e) => Err(unwrap_any(E::merge(None, e, location))), + Err(e) => Err(deserr::take_cf_content(E::merge(None, e, location))), }, - _ => Err(unwrap_any(E::error( + _ => Err(deserr::take_cf_content(E::error( None, deserr::ErrorKind::IncorrectValueKind { actual: value, diff --git a/meilisearch-types/src/error.rs b/meilisearch-types/src/error.rs index 5c2968b39..46368c29e 100644 --- a/meilisearch-types/src/error.rs +++ b/meilisearch-types/src/error.rs @@ -381,16 +381,6 @@ impl ErrorCode for io::Error { } } -/// Unwrap a result, either its Ok or Err value. -pub fn unwrap_any(any: std::ops::ControlFlow) -> T { - use std::ops::ControlFlow::*; - - match any { - Continue(any) => any, - Break(any) => any, - } -} - /// Deserialization when `deserr` cannot parse an API key date. #[derive(Debug)] pub struct ParseOffsetDateTimeError(pub String); diff --git a/meilisearch-types/src/keys.rs b/meilisearch-types/src/keys.rs index 87ed543d6..b2389b238 100644 --- a/meilisearch-types/src/keys.rs +++ b/meilisearch-types/src/keys.rs @@ -13,7 +13,7 @@ use uuid::Uuid; use crate::deserr::{immutable_field_error, DeserrError, DeserrJsonError}; use crate::error::deserr_codes::*; -use crate::error::{unwrap_any, Code, ErrorCode, ParseOffsetDateTimeError}; +use crate::error::{Code, ErrorCode, ParseOffsetDateTimeError}; use crate::index_uid_pattern::{IndexUidPattern, IndexUidPatternFormatError}; pub type KeyId = Uuid; @@ -78,7 +78,7 @@ fn deny_immutable_fields_api_key( "expiresAt" => immutable_field_error(field, accepted, Code::ImmutableApiKeyExpiresAt), "createdAt" => immutable_field_error(field, accepted, Code::ImmutableApiKeyCreatedAt), "updatedAt" => immutable_field_error(field, accepted, Code::ImmutableApiKeyUpdatedAt), - _ => unwrap_any(DeserrJsonError::::error::( + _ => deserr::take_cf_content(DeserrJsonError::::error::( None, deserr::ErrorKind::UnknownKey { key: field, accepted }, location, diff --git a/meilisearch-types/src/settings.rs b/meilisearch-types/src/settings.rs index c7023bb10..0fc7ea6e2 100644 --- a/meilisearch-types/src/settings.rs +++ b/meilisearch-types/src/settings.rs @@ -14,7 +14,6 @@ use serde::{Deserialize, Serialize, Serializer}; use crate::deserr::DeserrJsonError; use crate::error::deserr_codes::*; -use crate::error::unwrap_any; /// The maximimum number of results that the engine /// will be able to return in one search call. @@ -60,7 +59,7 @@ fn validate_min_word_size_for_typo_setting( ) -> Result { if let (Setting::Set(one), Setting::Set(two)) = (s.one_typo, s.two_typos) { if one > two { - return Err(unwrap_any(E::error::(None, ErrorKind::Unexpected { msg: format!("`minWordSizeForTypos` setting is invalid. `oneTypo` and `twoTypos` fields should be between `0` and `255`, and `twoTypos` should be greater or equals to `oneTypo` but found `oneTypo: {one}` and twoTypos: {two}`.") }, location))); + return Err(deserr::take_cf_content(E::error::(None, ErrorKind::Unexpected { msg: format!("`minWordSizeForTypos` setting is invalid. `oneTypo` and `twoTypos` fields should be between `0` and `255`, and `twoTypos` should be greater or equals to `oneTypo` but found `oneTypo: {one}` and twoTypos: {two}`.") }, location))); } } Ok(s) diff --git a/meilisearch-types/src/star_or.rs b/meilisearch-types/src/star_or.rs index 0f3ef10fb..cd26a1fb0 100644 --- a/meilisearch-types/src/star_or.rs +++ b/meilisearch-types/src/star_or.rs @@ -8,7 +8,6 @@ use serde::de::Visitor; use serde::{Deserialize, Deserializer, Serialize, Serializer}; use crate::deserr::query_params::FromQueryParameter; -use crate::error::unwrap_any; /// A type that tries to match either a star (*) or /// any other thing that implements `FromStr`. @@ -128,11 +127,11 @@ where } else { match T::from_str(&v) { Ok(parsed) => Ok(StarOr::Other(parsed)), - Err(e) => Err(unwrap_any(E::merge(None, e, location))), + Err(e) => Err(deserr::take_cf_content(E::merge(None, e, location))), } } } - _ => Err(unwrap_any(E::error::( + _ => Err(deserr::take_cf_content(E::error::( None, deserr::ErrorKind::IncorrectValueKind { actual: value, @@ -206,10 +205,10 @@ where "*" => Ok(OptionStarOr::Star), s => match T::from_query_param(s) { Ok(x) => Ok(OptionStarOr::Other(x)), - Err(e) => Err(unwrap_any(E::merge(None, e, location))), + Err(e) => Err(deserr::take_cf_content(E::merge(None, e, location))), }, }, - _ => Err(unwrap_any(E::error::( + _ => Err(deserr::take_cf_content(E::error::( None, deserr::ErrorKind::IncorrectValueKind { actual: value, @@ -318,7 +317,7 @@ where Ok(OptionStarOrList::List(els)) } } - _ => Err(unwrap_any(E::error::( + _ => Err(deserr::take_cf_content(E::error::( None, deserr::ErrorKind::IncorrectValueKind { actual: value, diff --git a/meilisearch/src/routes/indexes/mod.rs b/meilisearch/src/routes/indexes/mod.rs index 2e13d782d..b58bef0e9 100644 --- a/meilisearch/src/routes/indexes/mod.rs +++ b/meilisearch/src/routes/indexes/mod.rs @@ -9,7 +9,7 @@ use log::debug; use meilisearch_types::deserr::query_params::Param; use meilisearch_types::deserr::{immutable_field_error, DeserrJsonError, DeserrQueryParamError}; use meilisearch_types::error::deserr_codes::*; -use meilisearch_types::error::{unwrap_any, Code, ResponseError}; +use meilisearch_types::error::{Code, ResponseError}; use meilisearch_types::index_uid::IndexUid; use meilisearch_types::milli::{self, FieldDistribution, Index}; use meilisearch_types::tasks::KindWithContent; @@ -147,7 +147,7 @@ fn deny_immutable_fields_index( "uid" => immutable_field_error(field, accepted, Code::ImmutableIndexUid), "createdAt" => immutable_field_error(field, accepted, Code::ImmutableIndexCreatedAt), "updatedAt" => immutable_field_error(field, accepted, Code::ImmutableIndexUpdatedAt), - _ => unwrap_any(DeserrJsonError::::error::( + _ => deserr::take_cf_content(DeserrJsonError::::error::( None, deserr::ErrorKind::UnknownKey { key: field, accepted }, location,