diff --git a/meilisearch-http/src/extractors/jayson.rs b/meilisearch-http/src/extractors/jayson.rs index 3af7e4876..20a03dfb2 100644 --- a/meilisearch-http/src/extractors/jayson.rs +++ b/meilisearch-http/src/extractors/jayson.rs @@ -1,7 +1,7 @@ use actix_web::{dev::Payload, web::Json, FromRequest, HttpRequest}; use futures::ready; use jayson::{DeserializeError, DeserializeFromValue}; -use meilisearch_types::error::{Code, ErrorCode, ResponseError}; +use meilisearch_types::error::{ErrorCode, ResponseError}; use std::{ fmt::Debug, future::Future, @@ -34,7 +34,7 @@ where E: DeserializeError + ErrorCode + 'static, T: DeserializeFromValue, { - type Error = ResponseError; + type Error = actix_web::Error; type Future = ValidatedJsonExtractFut; #[inline] @@ -56,7 +56,7 @@ where T: DeserializeFromValue, E: DeserializeError + ErrorCode + 'static, { - type Output = Result, ResponseError>; + type Output = Result, actix_web::Error>; fn poll(self: Pin<&mut Self>, cx: &mut Context<'_>) -> Poll { let this = self.get_mut(); @@ -64,13 +64,10 @@ where let res = ready!(Pin::new(&mut this.fut).poll(cx)); let res = match res { - Err(err) => Err(ResponseError::from_msg( - format!("{err}"), - Code::MalformedPayload, - )), + Err(err) => Err(err), Ok(data) => match jayson::deserialize::<_, _, E>(data.into_inner()) { Ok(data) => Ok(ValidatedJson::new(data)), - Err(e) => Err(e.into()), + Err(e) => Err(ResponseError::from(e).into()), }, }; diff --git a/meilisearch-http/src/routes/indexes/settings.rs b/meilisearch-http/src/routes/indexes/settings.rs index c7a4a9fcd..2eaa4bbee 100644 --- a/meilisearch-http/src/routes/indexes/settings.rs +++ b/meilisearch-http/src/routes/indexes/settings.rs @@ -1,5 +1,9 @@ use log::debug; +use crate::analytics::Analytics; +use crate::extractors::authentication::{policies::*, GuardedData}; +use crate::extractors::jayson::ValidatedJson; +use crate::task::SummarizedTaskView; use actix_web::{web, HttpRequest, HttpResponse}; use meilisearch_lib::index::Settings; use meilisearch_lib::index_controller::Update; @@ -7,11 +11,6 @@ use meilisearch_lib::MeiliSearch; use meilisearch_types::error::{MeiliDeserError, ResponseError}; use serde_json::json; -use crate::analytics::Analytics; -use crate::extractors::authentication::{policies::*, GuardedData}; -use crate::extractors::jayson::ValidatedJson; -use crate::task::SummarizedTaskView; - #[macro_export] macro_rules! make_setting_route { ($route:literal, $update_verb:ident, $type:ty, $attr:ident, $camelcase_attr:literal, $analytics_var:ident, $analytics:expr) => { @@ -55,7 +54,10 @@ macro_rules! make_setting_route { pub async fn update( meilisearch: GuardedData, MeiliSearch>, index_uid: actix_web::web::Path, - body: actix_web::web::Json>, + body: crate::extractors::jayson::ValidatedJson< + Option<$type>, + meilisearch_types::error::MeiliDeserError, + >, req: HttpRequest, $analytics_var: web::Data, ) -> std::result::Result { diff --git a/meilisearch-lib/src/index/mod.rs b/meilisearch-lib/src/index/mod.rs index 0250947c2..e08a079e4 100644 --- a/meilisearch-lib/src/index/mod.rs +++ b/meilisearch-lib/src/index/mod.rs @@ -36,7 +36,7 @@ pub mod test { use super::error::Result; use super::index::Index; use super::Document; - use super::{Checked, IndexMeta, IndexStats, SearchQuery, SearchResult, Settings}; + use super::{IndexMeta, IndexStats, SearchQuery, SearchResult, Settings}; use crate::update_file_store::UpdateFileStore; #[derive(Clone)] diff --git a/meilisearch-lib/src/index/updates.rs b/meilisearch-lib/src/index/updates.rs index d1bb11d40..1636f6a6d 100644 --- a/meilisearch-lib/src/index/updates.rs +++ b/meilisearch-lib/src/index/updates.rs @@ -67,6 +67,7 @@ pub struct TypoSettings { #[cfg_attr(test, derive(proptest_derive::Arbitrary))] #[derive(Debug, Clone, Default, Serialize, Deserialize, PartialEq, DeserializeFromValue)] +#[jayson(rename_all = camelCase, deny_unknown_fields)] #[serde(deny_unknown_fields)] #[serde(rename_all = "camelCase")] pub struct FacetingSettings { @@ -79,6 +80,7 @@ pub struct FacetingSettings { #[derive( Debug, Clone, Default, Serialize, Deserialize, PartialEq, jayson::DeserializeFromValue, )] +#[jayson(rename_all = camelCase, deny_unknown_fields)] #[serde(deny_unknown_fields)] #[serde(rename_all = "camelCase")] pub struct PaginationSettings { @@ -92,10 +94,10 @@ pub struct PaginationSettings { #[derive( Debug, Clone, Default, Serialize, Deserialize, PartialEq, jayson::DeserializeFromValue, )] +#[jayson(rename_all = camelCase, deny_unknown_fields)] #[serde(deny_unknown_fields)] #[serde(rename_all = "camelCase")] #[cfg_attr(test, derive(proptest_derive::Arbitrary))] -#[jayson(rename_all = camelCase, deny_unknown_fields)] pub struct Settings { #[serde( default, @@ -122,7 +124,7 @@ pub struct Settings { #[cfg_attr(test, proptest(strategy = "test::setting_strategy()"))] pub sortable_attributes: Setting>, #[serde(default, skip_serializing_if = "Setting::is_not_set")] - #[cfg_attr(test, proptest(strategy = "test::setting_strategy()"))] + #[cfg_attr(test, proptest(strategy = "test::criteria_setting_strategy()"))] #[jayson(needs_predicate)] pub ranking_rules: Setting>, #[serde(default, skip_serializing_if = "Setting::is_not_set")] @@ -162,9 +164,7 @@ impl Settings { } } } -pub fn map_searchable_or_displayed_attributes( - setting: Setting>, -) -> Setting> { +fn map_searchable_or_displayed_attributes(setting: Setting>) -> Setting> { match setting { Setting::Set(fields) => { if fields.iter().any(|f| f == "*") { @@ -422,10 +422,33 @@ pub fn apply_settings_to_builder(settings: &Settings, builder: &mut milli::updat #[cfg(test)] pub(crate) mod test { + use super::*; + use milli::Criterion; use proptest::prelude::*; - use super::*; + fn criteria_strategy() -> impl Strategy> { + proptest::collection::vec( + prop_oneof![ + Just(Criterion::Words), + Just(Criterion::Typo), + Just(Criterion::Proximity), + Just(Criterion::Attribute), + Just(Criterion::Sort), + Just(Criterion::Exactness), + any::().prop_map(Criterion::Asc), + any::().prop_map(Criterion::Desc), + ], + 0..100, + ) + } + pub(super) fn criteria_setting_strategy() -> impl Strategy>> { + prop_oneof![ + Just(Setting::NotSet), + Just(Setting::Reset), + criteria_strategy().prop_map(Setting::Set), + ] + } pub(super) fn setting_strategy() -> impl Strategy> { prop_oneof![ Just(Setting::NotSet), @@ -451,11 +474,10 @@ pub(crate) mod test { pagination: Setting::NotSet, }; - let checked = settings.clone().check(); - assert_eq!(settings.displayed_attributes, checked.displayed_attributes); + assert_eq!(settings.displayed_attributes, settings.displayed_attributes); assert_eq!( settings.searchable_attributes, - checked.searchable_attributes + settings.searchable_attributes ); // test wildcard @@ -474,8 +496,7 @@ pub(crate) mod test { pagination: Setting::NotSet, }; - let checked = settings.check(); - assert_eq!(checked.displayed_attributes, Setting::Reset); - assert_eq!(checked.searchable_attributes, Setting::Reset); + assert_eq!(settings.displayed_attributes, Setting::Reset); + assert_eq!(settings.searchable_attributes, Setting::Reset); } }