From dbba31077065717bc489bb97800bb16b4b9a69e8 Mon Sep 17 00:00:00 2001 From: Quentin de Quelen Date: Mon, 20 Jan 2020 09:52:24 +0100 Subject: [PATCH] squash me --- Cargo.lock | 2 +- meilisearch-core/src/settings.rs | 109 +++++++++++----- meilisearch-http/src/routes/setting.rs | 82 ++++++++---- meilisearch-http/tests/settings.rs | 171 ++++++++++++++++++++++++- 4 files changed, 297 insertions(+), 67 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 1102873a8..8e26cc940 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -37,7 +37,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" [[package]] name = "assert-json-diff" version = "1.0.1" -source = "git+https://github.com/qdequele/assert-json-diff#0340008d8fc183a490e3ccf81f86a2355a7474e7" +source = "git+https://github.com/qdequele/assert-json-diff#29bdf99315e510abb2824ccb9353b3a7a330cbc4" dependencies = [ "serde 1.0.102 (registry+https://github.com/rust-lang/crates.io-index)", "serde_json 1.0.41 (registry+https://github.com/rust-lang/crates.io-index)", diff --git a/meilisearch-core/src/settings.rs b/meilisearch-core/src/settings.rs index 3a1687060..d3f2bf5f9 100644 --- a/meilisearch-core/src/settings.rs +++ b/meilisearch-core/src/settings.rs @@ -2,7 +2,7 @@ use std::sync::Mutex; use std::collections::{BTreeMap, BTreeSet, HashSet}; use std::str::FromStr; -use serde::{Deserialize, Serialize}; +use serde::{Deserialize, Deserializer, Serialize}; use once_cell::sync::Lazy; static RANKING_RULE_REGEX: Lazy> = Lazy::new(|| { @@ -27,29 +27,7 @@ impl Into for Settings { let settings = self.clone(); let ranking_rules = match settings.ranking_rules { - Some(rules) => { - let mut final_rules = Vec::new(); - for rule in rules { - let parsed_rule = match rule.as_str() { - "_typo" => RankingRule::Typo, - "_words" => RankingRule::Words, - "_proximity" => RankingRule::Proximity, - "_attribute" => RankingRule::Attribute, - "_words_position" => RankingRule::WordsPosition, - "_exact" => RankingRule::Exact, - _ => { - let captures = RANKING_RULE_REGEX.lock().unwrap().captures(&rule).unwrap(); - match captures[1].as_ref() { - "asc" => RankingRule::Asc(captures[2].to_string()), - "dsc" => RankingRule::Dsc(captures[2].to_string()), - _ => continue - } - } - }; - final_rules.push(parsed_rule); - } - Some(final_rules) - } + Some(rules) => Some(RankingRule::from_vec(rules)), None => None }; @@ -65,6 +43,46 @@ impl Into for Settings { } } +#[derive(Default, Clone, Serialize, Deserialize)] +pub struct SettingsComplete { + #[serde(default, deserialize_with = "deserialize_some")] + pub ranking_rules: Option>>, + #[serde(default, deserialize_with = "deserialize_some")] + pub ranking_distinct: Option>, + #[serde(default, deserialize_with = "deserialize_some")] + pub attribute_identifier: Option>, + #[serde(default, deserialize_with = "deserialize_some")] + pub attributes_searchable: Option>>, + #[serde(default, deserialize_with = "deserialize_some")] + pub attributes_displayed: Option>>, + #[serde(default, deserialize_with = "deserialize_some")] + pub stop_words: Option>>, + #[serde(default, deserialize_with = "deserialize_some")] + pub synonyms: Option>>>, +} + +impl Into for SettingsComplete { + fn into(self) -> SettingsUpdate { + let settings = self.clone(); + + let ranking_rules = match settings.ranking_rules { + Some(Some(rules)) => Some(Some(RankingRule::from_vec(rules))), + Some(None) => Some(None), + None => None, + }; + + SettingsUpdate { + ranking_rules: ranking_rules.into(), + ranking_distinct: settings.ranking_distinct.into(), + attribute_identifier: settings.attribute_identifier.into(), + attributes_searchable: settings.attributes_searchable.into(), + attributes_displayed: settings.attributes_displayed.into(), + stop_words: settings.stop_words.into(), + synonyms: settings.synonyms.into(), + } + } +} + #[derive(Debug, Clone, Serialize, Deserialize)] pub enum UpdateState { Update(T), @@ -83,6 +101,16 @@ impl From> for UpdateState { } } +impl From>> for UpdateState { + fn from(opt: Option>) -> UpdateState { + match opt { + Some(Some(t)) => UpdateState::Update(t), + Some(None) => UpdateState::Clear, + None => UpdateState::Nothing, + } + } +} + impl UpdateState { pub fn is_changed(&self) -> bool { match self { @@ -128,15 +156,6 @@ impl ToString for RankingRule { } } -impl RankingRule { - pub fn get_field(&self) -> Option { - match self { - RankingRule::Asc(field) | RankingRule::Dsc(field) => Some((*field).clone()), - _ => None, - } - } -} - impl FromStr for RankingRule { type Err = RankingRuleConversionError; @@ -161,6 +180,22 @@ impl FromStr for RankingRule { } } +impl RankingRule { + pub fn get_field(&self) -> Option { + match self { + RankingRule::Asc(field) | RankingRule::Dsc(field) => Some((*field).clone()), + _ => None, + } + } + + pub fn from_vec(rules: Vec) -> Vec { + rules.iter() + .map(|s| RankingRule::from_str(s.as_str())) + .filter_map(Result::ok) + .collect() + } +} + #[derive(Debug, Clone, Serialize, Deserialize)] pub struct SettingsUpdate { pub ranking_rules: UpdateState>, @@ -185,3 +220,11 @@ impl Default for SettingsUpdate { } } } + +// Any value that is present is considered Some value, including null. +fn deserialize_some<'de, T, D>(deserializer: D) -> Result, D::Error> + where T: Deserialize<'de>, + D: Deserializer<'de> +{ + Deserialize::deserialize(deserializer).map(Some) +} diff --git a/meilisearch-http/src/routes/setting.rs b/meilisearch-http/src/routes/setting.rs index b5a25635d..b001b8fc9 100644 --- a/meilisearch-http/src/routes/setting.rs +++ b/meilisearch-http/src/routes/setting.rs @@ -1,7 +1,7 @@ use std::collections::{BTreeMap, BTreeSet, HashSet}; use serde::{Deserialize, Serialize}; use tide::{Request, Response}; -use meilisearch_core::settings::{SettingsUpdate, UpdateState, Settings}; +use meilisearch_core::settings::{SettingsUpdate, UpdateState, Settings, SettingsComplete}; use crate::error::{ResponseError, SResult}; use crate::helpers::tide::RequestExt; @@ -77,7 +77,7 @@ pub async fn get_all(ctx: Request) -> SResult { pub async fn update_all(mut ctx: Request) -> SResult { ctx.is_allowed(SettingsWrite)?; let index = ctx.index()?; - let settings: Settings = ctx.body_json().await.map_err(ResponseError::bad_request)?; + let settings: SettingsComplete = ctx.body_json().await.map_err(ResponseError::bad_request)?; let db = &ctx.state().db; let mut writer = db.update_write_txn()?; @@ -113,7 +113,7 @@ pub async fn delete_all(ctx: Request) -> SResult { } #[derive(Default, Clone, Serialize, Deserialize)] -pub struct RankingSettings { +pub struct GetRankingSettings { pub ranking_rules: Option>, pub ranking_distinct: Option, } @@ -132,7 +132,7 @@ pub async fn get_ranking(ctx: Request) -> SResult { }; let ranking_distinct = index.main.ranking_distinct(&reader)?; - let settings = RankingSettings { + let settings = GetRankingSettings { ranking_rules, ranking_distinct, }; @@ -140,16 +140,22 @@ pub async fn get_ranking(ctx: Request) -> SResult { Ok(tide::Response::new(200).body_json(&settings).unwrap()) } +#[derive(Default, Clone, Serialize, Deserialize)] +pub struct SetRankingSettings { + pub ranking_rules: Option>>, + pub ranking_distinct: Option>, +} + pub async fn update_ranking(mut ctx: Request) -> SResult { ctx.is_allowed(SettingsWrite)?; let index = ctx.index()?; - let settings: RankingSettings = ctx.body_json().await.map_err(ResponseError::bad_request)?; + let settings: SetRankingSettings = ctx.body_json().await.map_err(ResponseError::bad_request)?; let db = &ctx.state().db; - let settings = Settings { + let settings = SettingsComplete { ranking_rules: settings.ranking_rules, ranking_distinct: settings.ranking_distinct, - .. Settings::default() + .. SettingsComplete::default() }; let mut writer = db.update_write_txn()?; @@ -181,7 +187,7 @@ pub async fn delete_ranking(ctx: Request) -> SResult { } #[derive(Default, Clone, Serialize, Deserialize)] -pub struct RankingRulesSettings { +pub struct GetRankingRulesSettings { pub ranking_rules: Option>, } @@ -198,23 +204,28 @@ pub async fn get_rules(ctx: Request) -> SResult { None => None, }; - let settings = RankingRulesSettings { + let settings = GetRankingRulesSettings { ranking_rules, }; Ok(tide::Response::new(200).body_json(&settings).unwrap()) } +#[derive(Default, Clone, Serialize, Deserialize)] +pub struct SetRankingRulesSettings { + pub ranking_rules: Option>>, +} + pub async fn update_rules(mut ctx: Request) -> SResult { ctx.is_allowed(SettingsWrite)?; let index = ctx.index()?; - let settings: RankingRulesSettings = ctx.body_json().await + let settings: SetRankingRulesSettings = ctx.body_json().await .map_err(ResponseError::bad_request)?; let db = &ctx.state().db; - let settings = Settings { + let settings = SettingsComplete { ranking_rules: settings.ranking_rules, - .. Settings::default() + .. SettingsComplete::default() }; let mut writer = db.update_write_txn()?; @@ -245,7 +256,7 @@ pub async fn delete_rules(ctx: Request) -> SResult { } #[derive(Default, Clone, Serialize, Deserialize)] -pub struct RankingDistinctSettings { +pub struct GetRankingDistinctSettings { pub ranking_distinct: Option, } @@ -256,23 +267,28 @@ pub async fn get_distinct(ctx: Request) -> SResult { let reader = db.main_read_txn()?; let ranking_distinct = index.main.ranking_distinct(&reader)?; - let settings = RankingDistinctSettings { + let settings = GetRankingDistinctSettings { ranking_distinct, }; Ok(tide::Response::new(200).body_json(&settings).unwrap()) } +#[derive(Default, Clone, Serialize, Deserialize)] +pub struct SetRankingDistinctSettings { + pub ranking_distinct: Option>, +} + pub async fn update_distinct(mut ctx: Request) -> SResult { ctx.is_allowed(SettingsWrite)?; let index = ctx.index()?; - let settings: RankingDistinctSettings = ctx.body_json().await + let settings: SetRankingDistinctSettings = ctx.body_json().await .map_err(ResponseError::bad_request)?; let db = &ctx.state().db; - let settings = Settings { + let settings = SettingsComplete { ranking_distinct: settings.ranking_distinct, - .. Settings::default() + .. SettingsComplete::default() }; let mut writer = db.update_write_txn()?; @@ -303,7 +319,7 @@ pub async fn delete_distinct(ctx: Request) -> SResult { } #[derive(Default, Clone, Serialize, Deserialize)] -pub struct AttributesSettings { +pub struct GetAttributesSettings { pub attribute_identifier: Option, pub attributes_searchable: Option>, pub attributes_displayed: Option>, @@ -321,7 +337,7 @@ pub async fn get_attributes(ctx: Request) -> SResult { let attributes_searchable = schema.clone().map(|s| s.get_indexed_name()); let attributes_displayed = schema.clone().map(|s| s.get_displayed_name()); - let settings = AttributesSettings { + let settings = GetAttributesSettings { attribute_identifier, attributes_searchable, attributes_displayed, @@ -330,18 +346,25 @@ pub async fn get_attributes(ctx: Request) -> SResult { Ok(tide::Response::new(200).body_json(&settings).unwrap()) } +#[derive(Default, Clone, Serialize, Deserialize)] +pub struct SetAttributesSettings { + pub attribute_identifier: Option>, + pub attributes_searchable: Option>>, + pub attributes_displayed: Option>>, +} + pub async fn update_attributes(mut ctx: Request) -> SResult { ctx.is_allowed(SettingsWrite)?; let index = ctx.index()?; - let settings: AttributesSettings = ctx.body_json().await + let settings: SetAttributesSettings = ctx.body_json().await .map_err(ResponseError::bad_request)?; let db = &ctx.state().db; - let settings = Settings { + let settings = SettingsComplete { attribute_identifier: settings.attribute_identifier, attributes_searchable: settings.attributes_searchable, attributes_displayed: settings.attributes_displayed, - .. Settings::default() + .. SettingsComplete::default() }; let mut writer = db.update_write_txn()?; @@ -394,7 +417,7 @@ pub async fn get_identifier(ctx: Request) -> SResult { } #[derive(Default, Clone, Serialize, Deserialize)] -pub struct AttributesSearchableSettings { +pub struct GetAttributesSearchableSettings { pub attributes_searchable: Option>, } @@ -408,23 +431,28 @@ pub async fn get_searchable(ctx: Request) -> SResult { let attributes_searchable = schema.map(|s| s.get_indexed_name()); - let settings = AttributesSearchableSettings { + let settings = GetAttributesSearchableSettings { attributes_searchable, }; Ok(tide::Response::new(200).body_json(&settings).unwrap()) } +#[derive(Default, Clone, Serialize, Deserialize)] +pub struct SetAttributesSearchableSettings { + pub attributes_searchable: Option>>, +} + pub async fn update_searchable(mut ctx: Request) -> SResult { ctx.is_allowed(SettingsWrite)?; let index = ctx.index()?; - let settings: AttributesSearchableSettings = ctx.body_json().await + let settings: SetAttributesSearchableSettings = ctx.body_json().await .map_err(ResponseError::bad_request)?; let db = &ctx.state().db; - let settings = Settings { + let settings = SettingsComplete { attributes_searchable: settings.attributes_searchable, - .. Settings::default() + .. SettingsComplete::default() }; let mut writer = db.update_write_txn()?; diff --git a/meilisearch-http/tests/settings.rs b/meilisearch-http/tests/settings.rs index 7646be40d..095220813 100644 --- a/meilisearch-http/tests/settings.rs +++ b/meilisearch-http/tests/settings.rs @@ -11,7 +11,7 @@ use assert_json_diff::assert_json_eq; mod common; #[test] -fn write_all_and_retreive() { +fn write_all_and_delete() { let mut server = common::setup_server().unwrap(); // 1 - Create the index @@ -55,10 +55,6 @@ fn write_all_and_retreive() { "release_date", "rank", ], - "attributes_ranked": [ - "release_date", - "rank", - ], "stop_words": [ "the", "a", @@ -114,10 +110,173 @@ fn write_all_and_retreive() { "attribute_identifier": null, "attributes_searchable": null, "attributes_displayed": null, - "attributes_ranked": null, "stop_words": null, "synonyms": null, }); assert_json_eq!(json, res_value, ordered: false); } + + +#[test] +fn write_all_and_update() { + let mut server = common::setup_server().unwrap(); + + // 1 - Create the index + + let body = json!({ + "uid": "movies", + }).to_string().into_bytes(); + + let req = http::Request::post("/indexes").body(Body::from(body)).unwrap(); + let res = server.simulate(req).unwrap(); + assert_eq!(res.status(), 201); + + // 2 - Send the settings + + let json = json!({ + "ranking_rules": [ + "_typo", + "_words", + "_proximity", + "_attribute", + "_words_position", + "_exact", + "dsc(release_date)", + "dsc(rank)", + ], + "ranking_distinct": "movie_id", + "attribute_identifier": "uid", + "attributes_searchable": [ + "uid", + "movie_id", + "title", + "description", + "poster", + "release_date", + "rank", + ], + "attributes_displayed": [ + "title", + "description", + "poster", + "release_date", + "rank", + ], + "stop_words": [ + "the", + "a", + "an", + ], + "synonyms": { + "wolverine": ["xmen", "logan"], + "logan": ["wolverine"], + } + }); + + let body = json.to_string().into_bytes(); + + let req = http::Request::post("/indexes/movies/settings").body(Body::from(body)).unwrap(); + let res = server.simulate(req).unwrap(); + assert_eq!(res.status(), 202); + + block_on(sleep(Duration::from_secs(1))); + + // 3 - Get all settings and compare to the previous one + + let req = http::Request::get("/indexes/movies/settings").body(Body::empty()).unwrap(); + let res = server.simulate(req).unwrap(); + assert_eq!(res.status(), 200); + + let mut buf = Vec::new(); + block_on(res.into_body().read_to_end(&mut buf)).unwrap(); + let res_value: Value = serde_json::from_slice(&buf).unwrap(); + + assert_json_eq!(json, res_value, ordered: false); + + // 4 - Update all settings + + let json_update = json!({ + "ranking_rules": [ + "_typo", + "_words", + "_proximity", + "_attribute", + "_words_position", + "_exact", + "dsc(release_date)", + ], + "ranking_distinct": null, + "attribute_identifier": "uid", + "attributes_searchable": [ + "title", + "description", + "uid", + ], + "attributes_displayed": [ + "title", + "description", + "release_date", + "rank", + "poster", + ], + "stop_words": [ + ], + "synonyms": { + "wolverine": ["xmen", "logan"], + "logan": ["wolverine", "xmen"], + } + }); + + let body_update = json_update.to_string().into_bytes(); + + let req = http::Request::post("/indexes/movies/settings").body(Body::from(body_update)).unwrap(); + let res = server.simulate(req).unwrap(); + assert_eq!(res.status(), 202); + + block_on(sleep(Duration::from_secs(1))); + + // 5 - Get all settings and check if the content is the same of (4) + + let req = http::Request::get("/indexes/movies/settings").body(Body::empty()).unwrap(); + let res = server.simulate(req).unwrap(); + assert_eq!(res.status(), 200); + + let mut buf = Vec::new(); + block_on(res.into_body().read_to_end(&mut buf)).unwrap(); + let res_value: Value = serde_json::from_slice(&buf).unwrap(); + + let res_expected = json!({ + "ranking_rules": [ + "_typo", + "_words", + "_proximity", + "_attribute", + "_words_position", + "_exact", + "dsc(release_date)", + ], + "ranking_distinct": "movie_id", + "attribute_identifier": "uid", + "attributes_searchable": [ + "title", + "description", + "uid", + ], + "attributes_displayed": [ + "title", + "description", + "release_date", + "rank", + "poster", + ], + "stop_words": [ + ], + "synonyms": { + "wolverine": ["xmen", "logan"], + "logan": ["wolverine", "xmen"], + } + }); + + assert_json_eq!(json_update, res_value, ordered: false); +}