From 10d3b367dc7e68f8ae15778a39901e6811449ad6 Mon Sep 17 00:00:00 2001 From: Kerollmops Date: Thu, 2 Jun 2022 10:48:02 +0200 Subject: [PATCH 1/4] Simplify the const default values --- meilisearch-http/src/routes/indexes/search.rs | 14 +++---- meilisearch-lib/src/index/mod.rs | 3 +- meilisearch-lib/src/index/search.rs | 39 +++++-------------- meilisearch-lib/src/index_controller/mod.rs | 8 ++-- 4 files changed, 22 insertions(+), 42 deletions(-) diff --git a/meilisearch-http/src/routes/indexes/search.rs b/meilisearch-http/src/routes/indexes/search.rs index 869bc4931..eb4ee6d34 100644 --- a/meilisearch-http/src/routes/indexes/search.rs +++ b/meilisearch-http/src/routes/indexes/search.rs @@ -3,8 +3,8 @@ use log::debug; use meilisearch_auth::IndexSearchRules; use meilisearch_error::ResponseError; use meilisearch_lib::index::{ - default_crop_length, default_crop_marker, default_highlight_post_tag, - default_highlight_pre_tag, SearchQuery, DEFAULT_SEARCH_LIMIT, + SearchQuery, DEFAULT_CROP_LENGTH, DEFAULT_CROP_MARKER, DEFAULT_HIGHLIGHT_POST_TAG, + DEFAULT_HIGHLIGHT_PRE_TAG, DEFAULT_SEARCH_LIMIT, }; use meilisearch_lib::MeiliSearch; use serde::Deserialize; @@ -30,7 +30,7 @@ pub struct SearchQueryGet { limit: Option, attributes_to_retrieve: Option, attributes_to_crop: Option, - #[serde(default = "default_crop_length")] + #[serde(default = "DEFAULT_CROP_LENGTH")] crop_length: usize, attributes_to_highlight: Option, filter: Option, @@ -38,11 +38,11 @@ pub struct SearchQueryGet { #[serde(default = "Default::default")] show_matches_position: bool, facets: Option, - #[serde(default = "default_highlight_pre_tag")] + #[serde(default = "DEFAULT_HIGHLIGHT_PRE_TAG")] highlight_pre_tag: String, - #[serde(default = "default_highlight_post_tag")] + #[serde(default = "DEFAULT_HIGHLIGHT_POST_TAG")] highlight_post_tag: String, - #[serde(default = "default_crop_marker")] + #[serde(default = "DEFAULT_CROP_MARKER")] crop_marker: String, } @@ -77,7 +77,7 @@ impl From for SearchQuery { Self { q: other.q, offset: other.offset, - limit: other.limit.unwrap_or(DEFAULT_SEARCH_LIMIT), + limit: other.limit.unwrap_or_else(DEFAULT_SEARCH_LIMIT), attributes_to_retrieve, attributes_to_crop, crop_length: other.crop_length, diff --git a/meilisearch-lib/src/index/mod.rs b/meilisearch-lib/src/index/mod.rs index b46d97849..e6c831a01 100644 --- a/meilisearch-lib/src/index/mod.rs +++ b/meilisearch-lib/src/index/mod.rs @@ -1,6 +1,5 @@ pub use search::{ - default_crop_length, default_crop_marker, default_highlight_post_tag, - default_highlight_pre_tag, SearchQuery, SearchResult, DEFAULT_CROP_LENGTH, DEFAULT_CROP_MARKER, + SearchQuery, SearchResult, DEFAULT_CROP_LENGTH, DEFAULT_CROP_MARKER, DEFAULT_HIGHLIGHT_POST_TAG, DEFAULT_HIGHLIGHT_PRE_TAG, DEFAULT_SEARCH_LIMIT, }; pub use updates::{apply_settings_to_builder, Checked, Facets, Settings, Unchecked}; diff --git a/meilisearch-lib/src/index/search.rs b/meilisearch-lib/src/index/search.rs index 097a91570..91a46600f 100644 --- a/meilisearch-lib/src/index/search.rs +++ b/meilisearch-lib/src/index/search.rs @@ -20,30 +20,11 @@ use super::index::Index; pub type Document = serde_json::Map; type MatchesPosition = BTreeMap>; -pub const DEFAULT_SEARCH_LIMIT: usize = 20; -const fn default_search_limit() -> usize { - DEFAULT_SEARCH_LIMIT -} - -pub const DEFAULT_CROP_LENGTH: usize = 10; -pub const fn default_crop_length() -> usize { - DEFAULT_CROP_LENGTH -} - -pub const DEFAULT_CROP_MARKER: &str = "…"; -pub fn default_crop_marker() -> String { - DEFAULT_CROP_MARKER.to_string() -} - -pub const DEFAULT_HIGHLIGHT_PRE_TAG: &str = ""; -pub fn default_highlight_pre_tag() -> String { - DEFAULT_HIGHLIGHT_PRE_TAG.to_string() -} - -pub const DEFAULT_HIGHLIGHT_POST_TAG: &str = ""; -pub fn default_highlight_post_tag() -> String { - DEFAULT_HIGHLIGHT_POST_TAG.to_string() -} +pub const DEFAULT_SEARCH_LIMIT: fn() -> usize = || 20; +pub const DEFAULT_CROP_LENGTH: fn() -> usize = || 10; +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(); /// The maximimum number of results that the engine /// will be able to return in one search call. @@ -54,11 +35,11 @@ pub const HARD_RESULT_LIMIT: usize = 1000; pub struct SearchQuery { pub q: Option, pub offset: Option, - #[serde(default = "default_search_limit")] + #[serde(default = "DEFAULT_SEARCH_LIMIT")] pub limit: usize, pub attributes_to_retrieve: Option>, pub attributes_to_crop: Option>, - #[serde(default = "default_crop_length")] + #[serde(default = "DEFAULT_CROP_LENGTH")] pub crop_length: usize, pub attributes_to_highlight: Option>, // Default to false @@ -67,11 +48,11 @@ pub struct SearchQuery { pub filter: Option, pub sort: Option>, pub facets: Option>, - #[serde(default = "default_highlight_pre_tag")] + #[serde(default = "DEFAULT_HIGHLIGHT_PRE_TAG")] pub highlight_pre_tag: String, - #[serde(default = "default_highlight_post_tag")] + #[serde(default = "DEFAULT_HIGHLIGHT_POST_TAG")] pub highlight_post_tag: String, - #[serde(default = "default_crop_marker")] + #[serde(default = "DEFAULT_CROP_MARKER")] pub crop_marker: String, } diff --git a/meilisearch-lib/src/index_controller/mod.rs b/meilisearch-lib/src/index_controller/mod.rs index ecca9ac63..7eb4f985b 100644 --- a/meilisearch-lib/src/index_controller/mod.rs +++ b/meilisearch-lib/src/index_controller/mod.rs @@ -652,7 +652,7 @@ mod test { use crate::index::error::Result as IndexResult; use crate::index::Index; use crate::index::{ - default_crop_marker, default_highlight_post_tag, default_highlight_pre_tag, + DEFAULT_CROP_MARKER, DEFAULT_HIGHLIGHT_POST_TAG, DEFAULT_HIGHLIGHT_PRE_TAG, }; use crate::index_resolver::index_store::MockIndexStore; use crate::index_resolver::meta_store::MockIndexMetaStore; @@ -692,9 +692,9 @@ mod test { filter: None, sort: None, facets: None, - highlight_pre_tag: default_highlight_pre_tag(), - highlight_post_tag: default_highlight_post_tag(), - crop_marker: default_crop_marker(), + highlight_pre_tag: DEFAULT_HIGHLIGHT_PRE_TAG(), + highlight_post_tag: DEFAULT_HIGHLIGHT_POST_TAG(), + crop_marker: DEFAULT_CROP_MARKER(), }; let result = SearchResult { From 64b5b2e1f817f919bf2eb89285c92ac5530760f8 Mon Sep 17 00:00:00 2001 From: Kerollmops Date: Thu, 2 Jun 2022 11:14:46 +0200 Subject: [PATCH 2/4] Use serde-cs::CS with StarOr to reduce the logic duplication --- meilisearch-http/src/routes/indexes/search.rs | 23 +++++++++++-------- meilisearch-http/src/routes/mod.rs | 5 +++- meilisearch-http/src/routes/tasks.rs | 6 ++--- 3 files changed, 20 insertions(+), 14 deletions(-) diff --git a/meilisearch-http/src/routes/indexes/search.rs b/meilisearch-http/src/routes/indexes/search.rs index eb4ee6d34..2f3f4a83b 100644 --- a/meilisearch-http/src/routes/indexes/search.rs +++ b/meilisearch-http/src/routes/indexes/search.rs @@ -8,11 +8,13 @@ use meilisearch_lib::index::{ }; use meilisearch_lib::MeiliSearch; use serde::Deserialize; +use serde_cs::vec::CS; use serde_json::Value; use crate::analytics::{Analytics, SearchAggregator}; use crate::extractors::authentication::{policies::*, GuardedData}; use crate::extractors::sequential_extractor::SeqHandler; +use crate::routes::{fold_star_or, StarOr}; pub fn configure(cfg: &mut web::ServiceConfig) { cfg.service( @@ -28,16 +30,16 @@ pub struct SearchQueryGet { q: Option, offset: Option, limit: Option, - attributes_to_retrieve: Option, - attributes_to_crop: Option, + attributes_to_retrieve: Option>>, + attributes_to_crop: Option>>, #[serde(default = "DEFAULT_CROP_LENGTH")] crop_length: usize, - attributes_to_highlight: Option, + attributes_to_highlight: Option>>, filter: Option, sort: Option, #[serde(default = "Default::default")] show_matches_position: bool, - facets: Option, + facets: Option>>, #[serde(default = "DEFAULT_HIGHLIGHT_PRE_TAG")] highlight_pre_tag: String, #[serde(default = "DEFAULT_HIGHLIGHT_POST_TAG")] @@ -50,19 +52,20 @@ impl From for SearchQuery { fn from(other: SearchQueryGet) -> Self { let attributes_to_retrieve = other .attributes_to_retrieve - .map(|attrs| attrs.split(',').map(String::from).collect()); + .map(CS::into_inner) + .and_then(fold_star_or); let attributes_to_crop = other .attributes_to_crop - .map(|attrs| attrs.split(',').map(String::from).collect()); + .map(CS::into_inner) + .and_then(fold_star_or); let attributes_to_highlight = other .attributes_to_highlight - .map(|attrs| attrs.split(',').map(String::from).collect()); + .map(CS::into_inner) + .and_then(fold_star_or); - let facets = other - .facets - .map(|attrs| attrs.split(',').map(String::from).collect()); + let facets = other.facets.map(CS::into_inner).and_then(fold_star_or); let filter = match other.filter { Some(f) => match serde_json::from_str(&f) { diff --git a/meilisearch-http/src/routes/mod.rs b/meilisearch-http/src/routes/mod.rs index 1b37396e9..a438d12d7 100644 --- a/meilisearch-http/src/routes/mod.rs +++ b/meilisearch-http/src/routes/mod.rs @@ -49,7 +49,10 @@ impl FromStr for StarOr { /// Extracts the raw values from the `StarOr` types and /// return None if a `StarOr::Star` is encountered. -pub fn fold_star_or(content: impl IntoIterator>) -> Option> { +pub fn fold_star_or(content: impl IntoIterator>) -> Option +where + O: FromIterator, +{ content .into_iter() .map(|value| match value { diff --git a/meilisearch-http/src/routes/tasks.rs b/meilisearch-http/src/routes/tasks.rs index 0ab4678b7..b13c04dc7 100644 --- a/meilisearch-http/src/routes/tasks.rs +++ b/meilisearch-http/src/routes/tasks.rs @@ -81,9 +81,9 @@ async fn get_tasks( // We first transform a potential indexUid=* into a "not specified indexUid filter" // for every one of the filters: type, status, and indexUid. - let type_ = type_.map(CS::into_inner).and_then(fold_star_or); - let status = status.map(CS::into_inner).and_then(fold_star_or); - let index_uid = index_uid.map(CS::into_inner).and_then(fold_star_or); + let type_: Option> = type_.map(CS::into_inner).and_then(fold_star_or); + let status: Option> = status.map(CS::into_inner).and_then(fold_star_or); + let index_uid: Option> = index_uid.map(CS::into_inner).and_then(fold_star_or); // Then we filter on potential indexes and make sure that the search filter // restrictions are also applied. From 277a0a79677432c7b91a31d3126b15e3b70b37ec Mon Sep 17 00:00:00 2001 From: Kerollmops Date: Mon, 6 Jun 2022 10:17:33 +0200 Subject: [PATCH 3/4] Bump serde-cs to simplify our usage of the star_or function --- Cargo.lock | 4 +-- meilisearch-http/Cargo.toml | 2 +- .../src/routes/indexes/documents.rs | 4 +-- meilisearch-http/src/routes/indexes/search.rs | 29 ++++--------------- meilisearch-http/src/routes/tasks.rs | 6 ++-- 5 files changed, 13 insertions(+), 32 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 317ae620a..a1be24517 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3078,9 +3078,9 @@ dependencies = [ [[package]] name = "serde-cs" -version = "0.2.2" +version = "0.2.3" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "18d5b0435c9139761fbe5abeb1283234bcfbde88fadc2ae432579648fbce72ad" +checksum = "8202c9f3f58762d274952790ff8a1f1f625b5664f75e5dc1952c8dcacc64a925" dependencies = [ "serde", ] diff --git a/meilisearch-http/Cargo.toml b/meilisearch-http/Cargo.toml index b9771afa2..ac4fed7b1 100644 --- a/meilisearch-http/Cargo.toml +++ b/meilisearch-http/Cargo.toml @@ -62,7 +62,7 @@ rustls = "0.20.4" rustls-pemfile = "0.3.0" segment = { version = "0.2.0", optional = true } serde = { version = "1.0.136", features = ["derive"] } -serde-cs = "0.2.2" +serde-cs = "0.2.3" serde_json = { version = "1.0.79", features = ["preserve_order"] } sha2 = "0.10.2" siphasher = "0.3.10" diff --git a/meilisearch-http/src/routes/indexes/documents.rs b/meilisearch-http/src/routes/indexes/documents.rs index f506e587c..b5f578a56 100644 --- a/meilisearch-http/src/routes/indexes/documents.rs +++ b/meilisearch-http/src/routes/indexes/documents.rs @@ -99,7 +99,7 @@ pub async fn get_document( let index = path.index_uid.clone(); let id = path.document_id.clone(); let GetDocument { fields } = params.into_inner(); - let attributes_to_retrieve = fields.map(CS::into_inner).and_then(fold_star_or); + let attributes_to_retrieve = fields.and_then(fold_star_or); let document = meilisearch .document(index, id, attributes_to_retrieve) @@ -143,7 +143,7 @@ pub async fn get_all_documents( offset, fields, } = params.into_inner(); - let attributes_to_retrieve = fields.map(CS::into_inner).and_then(fold_star_or); + let attributes_to_retrieve = fields.and_then(fold_star_or); let (total, documents) = meilisearch .documents(path.into_inner(), offset, limit, attributes_to_retrieve) diff --git a/meilisearch-http/src/routes/indexes/search.rs b/meilisearch-http/src/routes/indexes/search.rs index 2f3f4a83b..4eaa65b9d 100644 --- a/meilisearch-http/src/routes/indexes/search.rs +++ b/meilisearch-http/src/routes/indexes/search.rs @@ -50,23 +50,6 @@ pub struct SearchQueryGet { impl From for SearchQuery { fn from(other: SearchQueryGet) -> Self { - let attributes_to_retrieve = other - .attributes_to_retrieve - .map(CS::into_inner) - .and_then(fold_star_or); - - let attributes_to_crop = other - .attributes_to_crop - .map(CS::into_inner) - .and_then(fold_star_or); - - let attributes_to_highlight = other - .attributes_to_highlight - .map(CS::into_inner) - .and_then(fold_star_or); - - let facets = other.facets.map(CS::into_inner).and_then(fold_star_or); - let filter = match other.filter { Some(f) => match serde_json::from_str(&f) { Ok(v) => Some(v), @@ -75,20 +58,18 @@ impl From for SearchQuery { None => None, }; - let sort = other.sort.map(|attr| fix_sort_query_parameters(&attr)); - Self { q: other.q, offset: other.offset, limit: other.limit.unwrap_or_else(DEFAULT_SEARCH_LIMIT), - attributes_to_retrieve, - attributes_to_crop, + attributes_to_retrieve: other.attributes_to_retrieve.and_then(fold_star_or), + attributes_to_crop: other.attributes_to_crop.and_then(fold_star_or), crop_length: other.crop_length, - attributes_to_highlight, + attributes_to_highlight: other.attributes_to_highlight.and_then(fold_star_or), filter, - sort, + sort: other.sort.map(|attr| fix_sort_query_parameters(&attr)), show_matches_position: other.show_matches_position, - facets, + facets: other.facets.and_then(fold_star_or), highlight_pre_tag: other.highlight_pre_tag, highlight_post_tag: other.highlight_post_tag, crop_marker: other.crop_marker, diff --git a/meilisearch-http/src/routes/tasks.rs b/meilisearch-http/src/routes/tasks.rs index b13c04dc7..14716ff6b 100644 --- a/meilisearch-http/src/routes/tasks.rs +++ b/meilisearch-http/src/routes/tasks.rs @@ -81,9 +81,9 @@ async fn get_tasks( // We first transform a potential indexUid=* into a "not specified indexUid filter" // for every one of the filters: type, status, and indexUid. - let type_: Option> = type_.map(CS::into_inner).and_then(fold_star_or); - let status: Option> = status.map(CS::into_inner).and_then(fold_star_or); - let index_uid: Option> = index_uid.map(CS::into_inner).and_then(fold_star_or); + let type_: Option> = type_.and_then(fold_star_or); + let status: Option> = status.and_then(fold_star_or); + let index_uid: Option> = index_uid.and_then(fold_star_or); // Then we filter on potential indexes and make sure that the search filter // restrictions are also applied. From e5b760c59a256fdd3877a493a8341b76eb2a48c8 Mon Sep 17 00:00:00 2001 From: Kerollmops Date: Mon, 6 Jun 2022 10:44:46 +0200 Subject: [PATCH 4/4] Fix the segment analytics tests --- meilisearch-http/src/analytics/segment_analytics.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/meilisearch-http/src/analytics/segment_analytics.rs b/meilisearch-http/src/analytics/segment_analytics.rs index 20df96942..562b99c16 100644 --- a/meilisearch-http/src/analytics/segment_analytics.rs +++ b/meilisearch-http/src/analytics/segment_analytics.rs @@ -426,10 +426,10 @@ impl SearchAggregator { ret.max_limit = query.limit; ret.max_offset = query.offset.unwrap_or_default(); - ret.highlight_pre_tag = query.highlight_pre_tag != DEFAULT_HIGHLIGHT_PRE_TAG; - ret.highlight_post_tag = query.highlight_post_tag != DEFAULT_HIGHLIGHT_POST_TAG; - ret.crop_marker = query.crop_marker != DEFAULT_CROP_MARKER; - ret.crop_length = query.crop_length != DEFAULT_CROP_LENGTH; + ret.highlight_pre_tag = query.highlight_pre_tag != DEFAULT_HIGHLIGHT_PRE_TAG(); + ret.highlight_post_tag = query.highlight_post_tag != DEFAULT_HIGHLIGHT_POST_TAG(); + ret.crop_marker = query.crop_marker != DEFAULT_CROP_MARKER(); + ret.crop_length = query.crop_length != DEFAULT_CROP_LENGTH(); ret.show_matches_position = query.show_matches_position; ret