Merge #2464

2464: Simplify the `star_or` function usage r=ManyTheFish a=Kerollmops

This PR simplifies the usage of the `star_or` function that was originally introduced in #2399. The `serde-cs` https://github.com/naughie/serde-cs/pull/1 PR was merged and was implementing the `IntoIterator` trait on the `CS` types, which makes it possible to directly collect without converting a `CS` into the inner type (vec).

Co-authored-by: Kerollmops <clement@meilisearch.com>

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",
 ]

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"

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

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)

meilisearch-http/src/routes/indexes/search.rs

@@ -3,16 +3,18 @@ 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;
+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,42 +30,26 @@ pub struct SearchQueryGet {
     q: Option<String>,
     offset: Option<usize>,
     limit: Option<usize>,
-    attributes_to_retrieve: Option<String>,
-    attributes_to_crop: Option<String>,
-    #[serde(default = "default_crop_length")]
+    attributes_to_retrieve: Option<CS<StarOr<String>>>,
+    attributes_to_crop: Option<CS<StarOr<String>>>,
+    #[serde(default = "DEFAULT_CROP_LENGTH")]
     crop_length: usize,
-    attributes_to_highlight: Option<String>,
+    attributes_to_highlight: Option<CS<StarOr<String>>>,
     filter: Option<String>,
     sort: Option<String>,
     #[serde(default = "Default::default")]
     show_matches_position: bool,
-    facets: Option<String>,
-    #[serde(default = "default_highlight_pre_tag")]
+    facets: Option<CS<StarOr<String>>>,
+    #[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,
 }
 
 impl From<SearchQueryGet> for SearchQuery {
     fn from(other: SearchQueryGet) -> Self {
-        let attributes_to_retrieve = other
-            .attributes_to_retrieve
-            .map(|attrs| attrs.split(',').map(String::from).collect());
-
-        let attributes_to_crop = other
-            .attributes_to_crop
-            .map(|attrs| attrs.split(',').map(String::from).collect());
-
-        let attributes_to_highlight = other
-            .attributes_to_highlight
-            .map(|attrs| attrs.split(',').map(String::from).collect());
-
-        let facets = other
-            .facets
-            .map(|attrs| attrs.split(',').map(String::from).collect());
-
         let filter = match other.filter {
             Some(f) => match serde_json::from_str(&f) {
                 Ok(v) => Some(v),
@@ -72,20 +58,18 @@ impl From<SearchQueryGet> 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(DEFAULT_SEARCH_LIMIT),
-            attributes_to_retrieve,
-            attributes_to_crop,
+            limit: other.limit.unwrap_or_else(DEFAULT_SEARCH_LIMIT),
+            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,

meilisearch-http/src/routes/mod.rs

@@ -49,7 +49,10 @@ impl<T: FromStr> FromStr for StarOr<T> {
 
 /// Extracts the raw values from the `StarOr` types and
 /// return None if a `StarOr::Star` is encountered.
-pub fn fold_star_or<T>(content: impl IntoIterator<Item = StarOr<T>>) -> Option<Vec<T>> {
+pub fn fold_star_or<T, O>(content: impl IntoIterator<Item = StarOr<T>>) -> Option<O>
+where
+    O: FromIterator<T>,
+{
     content
         .into_iter()
         .map(|value| match value {

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<Vec<_>> = type_.and_then(fold_star_or);
+    let status: Option<Vec<_>> = status.and_then(fold_star_or);
+    let index_uid: Option<Vec<_>> = index_uid.and_then(fold_star_or);
 
     // Then we filter on potential indexes and make sure that the search filter
     // restrictions are also applied.

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};

meilisearch-lib/src/index/search.rs

@@ -20,30 +20,11 @@ use super::index::Index;
 pub type Document = serde_json::Map<String, Value>;
 type MatchesPosition = BTreeMap<String, Vec<MatchBounds>>;
 
-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 = "<em>";
-pub fn default_highlight_pre_tag() -> String {
-    DEFAULT_HIGHLIGHT_PRE_TAG.to_string()
-}
-
-pub const DEFAULT_HIGHLIGHT_POST_TAG: &str = "</em>";
-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 = || "<em>".to_string();
+pub const DEFAULT_HIGHLIGHT_POST_TAG: fn() -> String = || "</em>".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<String>,
     pub offset: Option<usize>,
-    #[serde(default = "default_search_limit")]
+    #[serde(default = "DEFAULT_SEARCH_LIMIT")]
     pub limit: usize,
     pub attributes_to_retrieve: Option<BTreeSet<String>>,
     pub attributes_to_crop: Option<Vec<String>>,
-    #[serde(default = "default_crop_length")]
+    #[serde(default = "DEFAULT_CROP_LENGTH")]
     pub crop_length: usize,
     pub attributes_to_highlight: Option<HashSet<String>>,
     // Default to false
@@ -67,11 +48,11 @@ pub struct SearchQuery {
     pub filter: Option<Value>,
     pub sort: Option<Vec<String>>,
     pub facets: Option<Vec<String>>,
-    #[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,
 }
 

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 {