Refactor query parameter deserialisation logic

meilisearch-types/src/deserr/error_messages.rs

@@ -0,0 +1,315 @@
+/*!
+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 deserr::{ErrorKind, IntoValue, ValueKind, ValuePointerRef};
+
+use super::{DeserrJsonError, DeserrQueryParamError};
+use crate::error::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 => "an 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<C: Default + ErrorCode> deserr::DeserializeError for DeserrJsonError<C> {
+    fn error<V: IntoValue>(
+        _self_: Option<Self>,
+        error: deserr::ErrorKind<V>,
+        location: ValuePointerRef,
+    ) -> Result<Self, Self> {
+        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::<Vec<String>>()
+                        .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::<Vec<String>>()
+                        .join(", "),
+                )
+            }
+            ErrorKind::Unexpected { msg } => {
+                let location = location_json_description(location, " at");
+                format!("Invalid value{location}: {msg}")
+            }
+        });
+
+        Err(DeserrJsonError::new(message, C::default().error_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<C: Default + ErrorCode> deserr::DeserializeError for DeserrQueryParamError<C> {
+    fn error<V: IntoValue>(
+        _self_: Option<Self>,
+        error: deserr::ErrorKind<V>,
+        location: ValuePointerRef,
+    ) -> Result<Self, Self> {
+        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::<Vec<String>>()
+                        .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::<Vec<String>>()
+                        .join(", "),
+                )
+            }
+            ErrorKind::Unexpected { msg } => {
+                let location = location_query_param_description(location, " in parameter");
+                format!("Invalid value{location}: {msg}")
+            }
+        });
+
+        Err(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<V: IntoValue>(actual: deserr::Value<V>) -> 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]), @"an 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]), @"an 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");
+    }
+}

meilisearch-types/src/deserr/mod.rs

@@ -0,0 +1,134 @@
+use std::convert::Infallible;
+use std::fmt;
+use std::marker::PhantomData;
+
+use deserr::{DeserializeError, MergeWithError, ValuePointerRef};
+
+use crate::error::deserr_codes::{self, *};
+use crate::error::{
+    unwrap_any, Code, DeserrParseBoolError, DeserrParseIntError, ErrorCode, InvalidTaskDateError,
+    ParseOffsetDateTimeError,
+};
+use crate::index_uid::IndexUidFormatError;
+use crate::tasks::{ParseTaskKindError, ParseTaskStatusError};
+
+pub mod error_messages;
+pub mod query_params;
+
+/// Marker type for the Json format
+pub struct DeserrJson;
+/// Marker type for the Query Parameter format
+pub struct DeserrQueryParam;
+
+pub type DeserrJsonError<C = deserr_codes::BadRequest> = DeserrError<DeserrJson, C>;
+pub type DeserrQueryParamError<C = deserr_codes::BadRequest> = DeserrError<DeserrQueryParam, C>;
+
+/// A request deserialization error.
+///
+/// The first generic paramater is a marker type describing the format of the request: either json (e.g. [`DeserrJson`] or [`DeserrQueryParam`]).
+/// The second generic parameter is the default error code for the deserialization error, in case it is not given.
+pub struct DeserrError<Format, C: Default + ErrorCode> {
+    pub msg: String,
+    pub code: Code,
+    _phantom: PhantomData<(Format, C)>,
+}
+impl<Format, C: Default + ErrorCode> DeserrError<Format, C> {
+    pub fn new(msg: String, code: Code) -> Self {
+        Self { msg, code, _phantom: PhantomData }
+    }
+}
+impl<Format, C: Default + ErrorCode> std::fmt::Debug for DeserrError<Format, C> {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        f.debug_struct("DeserrError").field("msg", &self.msg).field("code", &self.code).finish()
+    }
+}
+
+impl<Format, C: Default + ErrorCode> std::fmt::Display for DeserrError<Format, C> {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        write!(f, "{}", self.msg)
+    }
+}
+
+impl<Format, C: Default + ErrorCode> std::error::Error for DeserrError<Format, C> {}
+impl<Format, C: Default + ErrorCode> ErrorCode for DeserrError<Format, C> {
+    fn error_code(&self) -> Code {
+        self.code
+    }
+}
+
+// For now, we don't accumulate errors. Only one deserialisation error is ever returned at a time.
+impl<Format, C1: Default + ErrorCode, C2: Default + ErrorCode>
+    MergeWithError<DeserrError<Format, C2>> for DeserrError<Format, C1>
+{
+    fn merge(
+        _self_: Option<Self>,
+        other: DeserrError<Format, C2>,
+        _merge_location: ValuePointerRef,
+    ) -> Result<Self, Self> {
+        Err(DeserrError { msg: other.msg, code: other.code, _phantom: PhantomData })
+    }
+}
+
+impl<Format, C: Default + ErrorCode> MergeWithError<Infallible> for DeserrError<Format, C> {
+    fn merge(
+        _self_: Option<Self>,
+        _other: Infallible,
+        _merge_location: ValuePointerRef,
+    ) -> Result<Self, Self> {
+        unreachable!()
+    }
+}
+
+// Implement a convenience function to build a `missing_field` error
+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::<Infallible>(
+                    None,
+                    deserr::ErrorKind::MissingField { field },
+                    location,
+                ));
+                Self { msg: x.msg, code: $err_code.error_code(), _phantom: PhantomData }
+            }
+        }
+    };
+}
+make_missing_field_convenience_builder!(MissingIndexUid, missing_index_uid);
+make_missing_field_convenience_builder!(MissingApiKeyActions, missing_api_key_actions);
+make_missing_field_convenience_builder!(MissingApiKeyExpiresAt, missing_api_key_expires_at);
+make_missing_field_convenience_builder!(MissingApiKeyIndexes, missing_api_key_indexes);
+make_missing_field_convenience_builder!(MissingSwapIndexes, missing_swap_indexes);
+
+// Integrate a sub-error into a [`DeserrError`] by taking its error message but using
+// the default error code (C) from `Self`
+macro_rules! merge_with_error_impl_take_error_message {
+    ($err_type:ty) => {
+        impl<Format, C: Default + ErrorCode> MergeWithError<$err_type> for DeserrError<Format, C>
+        where
+            DeserrError<Format, C>: deserr::DeserializeError,
+        {
+            fn merge(
+                _self_: Option<Self>,
+                other: $err_type,
+                merge_location: ValuePointerRef,
+            ) -> Result<Self, Self> {
+                DeserrError::<Format, C>::error::<Infallible>(
+                    None,
+                    deserr::ErrorKind::Unexpected { msg: other.to_string() },
+                    merge_location,
+                )
+            }
+        }
+    };
+}
+
+// All these errors can be merged into a `DeserrError`
+merge_with_error_impl_take_error_message!(DeserrParseIntError);
+merge_with_error_impl_take_error_message!(DeserrParseBoolError);
+merge_with_error_impl_take_error_message!(uuid::Error);
+merge_with_error_impl_take_error_message!(InvalidTaskDateError);
+merge_with_error_impl_take_error_message!(ParseOffsetDateTimeError);
+merge_with_error_impl_take_error_message!(ParseTaskKindError);
+merge_with_error_impl_take_error_message!(ParseTaskStatusError);
+merge_with_error_impl_take_error_message!(IndexUidFormatError);

meilisearch-types/src/deserr/query_params.rs

@@ -0,0 +1,115 @@
+/*!
+This module provides helper traits, types, and functions to deserialize query parameters.
+
+The source of the problem is that query parameters only give us a string to work with.
+This means `deserr` is never given a sequence or numbers, and thus the default deserialization
+code for common types such as `usize` or `Vec<T>` does not work. To work around it, we create a
+wrapper type called `Param<T>`, which is deserialised using the `from_query_param` method of the trait
+`FromQueryParameter`.
+
+We also use other helper types such as `CS` (i.e. comma-separated) from `serde_cs` as well as
+`StarOr`, `OptionStarOr`, and `OptionStarOrList`.
+*/
+
+use std::convert::Infallible;
+use std::ops::Deref;
+use std::str::FromStr;
+
+use deserr::{DeserializeError, DeserializeFromValue, MergeWithError, ValueKind};
+
+use super::{DeserrParseBoolError, DeserrParseIntError};
+use crate::error::unwrap_any;
+use crate::index_uid::IndexUid;
+use crate::tasks::{Kind, Status};
+
+/// A wrapper type indicating that the inner value should be
+/// deserialised from a query parameter string.
+///
+/// Note that if the field is optional, it is better to use
+/// `Option<Param<T>>` instead of `Param<Option<T>>`.
+#[derive(Default, Debug, Clone, Copy)]
+pub struct Param<T>(pub T);
+
+impl<T> Deref for Param<T> {
+    type Target = T;
+
+    fn deref(&self) -> &Self::Target {
+        &self.0
+    }
+}
+
+impl<T, E> DeserializeFromValue<E> for Param<T>
+where
+    E: DeserializeError + MergeWithError<T::Err>,
+    T: FromQueryParameter,
+{
+    fn deserialize_from_value<V: deserr::IntoValue>(
+        value: deserr::Value<V>,
+        location: deserr::ValuePointerRef,
+    ) -> Result<Self, E> {
+        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(unwrap_any(E::error(
+                None,
+                deserr::ErrorKind::IncorrectValueKind {
+                    actual: value,
+                    accepted: &[ValueKind::String],
+                },
+                location,
+            ))),
+        }
+    }
+}
+
+/// Parse a value from a query parameter string.
+///
+/// This trait is functionally equivalent to `FromStr`.
+/// Having a separate trait trait allows us to return better
+/// deserializatio error messages.
+pub trait FromQueryParameter: Sized {
+    type Err;
+    fn from_query_param(p: &str) -> Result<Self, Self::Err>;
+}
+
+/// Implement `FromQueryParameter` for the given type using its `FromStr`
+/// trait implementation.
+macro_rules! impl_from_query_param_from_str {
+    ($type:ty) => {
+        impl FromQueryParameter for $type {
+            type Err = <$type as FromStr>::Err;
+            fn from_query_param(p: &str) -> Result<Self, Self::Err> {
+                p.parse()
+            }
+        }
+    };
+}
+impl_from_query_param_from_str!(Kind);
+impl_from_query_param_from_str!(Status);
+impl_from_query_param_from_str!(IndexUid);
+
+/// Implement `FromQueryParameter` for the given type using its `FromStr`
+/// trait implementation, replacing the returned error with a struct
+/// that wraps the original query parameter.
+macro_rules! impl_from_query_param_wrap_original_value_in_error {
+    ($type:ty, $err_type:path) => {
+        impl FromQueryParameter for $type {
+            type Err = $err_type;
+            fn from_query_param(p: &str) -> Result<Self, Self::Err> {
+                p.parse().map_err(|_| $err_type(p.to_owned()))
+            }
+        }
+    };
+}
+impl_from_query_param_wrap_original_value_in_error!(usize, DeserrParseIntError);
+impl_from_query_param_wrap_original_value_in_error!(u32, DeserrParseIntError);
+impl_from_query_param_wrap_original_value_in_error!(bool, DeserrParseBoolError);
+
+impl FromQueryParameter for String {
+    type Err = Infallible;
+    fn from_query_param(p: &str) -> Result<Self, Infallible> {
+        Ok(p.to_owned())
+    }
+}