diff --git a/meilisearch-core/src/facets.rs b/meilisearch-core/src/facets.rs index 11135c179..6f5c2e837 100644 --- a/meilisearch-core/src/facets.rs +++ b/meilisearch-core/src/facets.rs @@ -246,7 +246,7 @@ mod test { #[test] fn test_facet_key() { let mut schema = Schema::new(); - let id = schema.insert_and_index("hello").unwrap(); + let id = schema.register_field("hello").unwrap(); let facet_list = [schema.id("hello").unwrap()]; assert_eq!( FacetKey::from_str("hello:12", &schema, &facet_list).unwrap(), @@ -287,7 +287,7 @@ mod test { fn test_parse_facet_array() { use either::Either::{Left, Right}; let mut schema = Schema::new(); - let _id = schema.insert_and_index("hello").unwrap(); + let _id = schema.register_field("hello").unwrap(); let facet_list = [schema.id("hello").unwrap()]; assert_eq!( FacetFilter::from_str("[[\"hello:12\"]]", &schema, &facet_list).unwrap(), diff --git a/meilisearch-core/src/serde/deserializer.rs b/meilisearch-core/src/serde/deserializer.rs index e5e02a4d6..2ef266565 100644 --- a/meilisearch-core/src/serde/deserializer.rs +++ b/meilisearch-core/src/serde/deserializer.rs @@ -55,6 +55,7 @@ pub struct Deserializer<'a> { pub documents_fields: DocumentsFields, pub schema: &'a Schema, pub fields: Option<&'a HashSet>, + pub displayed: bool, } impl<'de, 'a, 'b> de::Deserializer<'de> for &'b mut Deserializer<'a> { @@ -93,7 +94,9 @@ impl<'de, 'a, 'b> de::Deserializer<'de> for &'b mut Deserializer<'a> { }; let is_displayed = self.schema.is_displayed(attr); - if is_displayed && self.fields.map_or(true, |f| f.contains(&attr)) { + // Check if displayed fields were requested, if yes, return only displayed fields, + // else return all fields + if !self.displayed || (is_displayed && self.fields.map_or(true, |f| f.contains(&attr))) { if let Some(attribute_name) = self.schema.name(attr) { let cursor = Cursor::new(value.to_owned()); let ioread = SerdeJsonIoRead::new(cursor); diff --git a/meilisearch-core/src/store/mod.rs b/meilisearch-core/src/store/mod.rs index fa5baa831..4f8e1250a 100644 --- a/meilisearch-core/src/store/mod.rs +++ b/meilisearch-core/src/store/mod.rs @@ -243,6 +243,7 @@ impl Index { documents_fields: self.documents_fields, schema: &schema, fields: attributes.as_ref(), + displayed: true, }; Ok(Option::::deserialize(&mut deserializer)?) diff --git a/meilisearch-core/src/update/documents_addition.rs b/meilisearch-core/src/update/documents_addition.rs index 5603966b0..2dc68c3ba 100644 --- a/meilisearch-core/src/update/documents_addition.rs +++ b/meilisearch-core/src/update/documents_addition.rs @@ -197,6 +197,7 @@ pub fn apply_addition<'a, 'b>( documents_fields: index.documents_fields, schema: &schema, fields: None, + displayed: false, }; let old_document = Option::>::deserialize(&mut deserializer)?; @@ -228,7 +229,7 @@ pub fn apply_addition<'a, 'b>( for (document_id, document) in &documents_additions { // For each key-value pair in the document. for (attribute, value) in document { - let field_id = schema.insert_and_index(&attribute)?; + let field_id = schema.register_field(&attribute)?; index_document( writer, index.documents_fields, diff --git a/meilisearch-http/tests/search.rs b/meilisearch-http/tests/search.rs index 170d9ffaf..92879bb7b 100644 --- a/meilisearch-http/tests/search.rs +++ b/meilisearch-http/tests/search.rs @@ -1780,7 +1780,7 @@ async fn update_documents_with_facet_distribution() { let settings = json!({ "attributesForFaceting": ["genre"], "displayedAttributes": ["genre"], - "searchableAttributes": ["genre"] + "searchableAttributes": ["genre", "type"] }); server.update_all_settings(settings).await; let update1 = json!([ diff --git a/meilisearch-http/tests/settings.rs b/meilisearch-http/tests/settings.rs index 6b125c13a..4b7b591a4 100644 --- a/meilisearch-http/tests/settings.rs +++ b/meilisearch-http/tests/settings.rs @@ -1,5 +1,5 @@ use assert_json_diff::assert_json_eq; -use serde_json::json; +use serde_json::{json, Value}; use std::convert::Into; mod common; @@ -486,15 +486,7 @@ async fn test_displayed_attributes_field() { ], "distinctAttribute": "id", "searchableAttributes": [ - "id", - "name", - "color", - "gender", - "email", - "phone", - "address", - "registered", - "about" + "*" ], "displayedAttributes": [ "age", @@ -520,4 +512,72 @@ async fn test_displayed_attributes_field() { let (response, _status_code) = server.get_all_settings().await; assert_json_eq!(body, response, ordered: true); -} \ No newline at end of file +} + +#[actix_rt::test] +async fn push_documents_after_updating_settings_should_not_change_settings() { + let mut server = common::Server::with_uid("test"); + + let index_body = json!({ + "uid": "test", + "primaryKey": "id", + }); + + let settings_body = json!({ + "rankingRules": [ + "typo", + "words", + "proximity", + "attribute", + "wordsPosition", + "exactness", + "desc(registered)", + "desc(age)", + ], + "distinctAttribute": "id", + "searchableAttributes": [ + "id", + "name", + "color", + "gender", + "email", + "phone", + "address", + "registered", + "about" + ], + "displayedAttributes": [ + "name", + "gender", + "email", + "registered", + "age", + ], + "stopWords": [ + "ad", + "in", + "ut", + ], + "synonyms": { + "road": ["street", "avenue"], + "street": ["avenue"], + }, + "attributesForFaceting": ["name", "color"], + }); + + let dataset = include_bytes!("assets/test_set.json"); + + let documents_body: Value = serde_json::from_slice(dataset).unwrap(); + + server.create_index(index_body).await; + + server.update_all_settings(settings_body.clone()).await; + + server.add_or_replace_multiple_documents(documents_body).await; + + // === check if settings are the same ==== + + let (response, _status_code) = server.get_all_settings().await; + + assert_json_eq!(settings_body, response, ordered: false); +} diff --git a/meilisearch-schema/src/schema.rs b/meilisearch-schema/src/schema.rs index a1992080a..bc73d9ba7 100644 --- a/meilisearch-schema/src/schema.rs +++ b/meilisearch-schema/src/schema.rs @@ -16,14 +16,6 @@ impl OptionAll { std::mem::replace(self, OptionAll::None) } - fn map U>(self, f: F) -> OptionAll { - match self { - OptionAll::Some(x) => OptionAll::Some(f(x)), - OptionAll::All => OptionAll::All, - OptionAll::None => OptionAll::None, - } - } - pub fn is_all(&self) -> bool { matches!(self, OptionAll::All) } @@ -43,8 +35,8 @@ pub struct Schema { ranked: HashSet, displayed: OptionAll>, - indexed: OptionAll>, - indexed_map: HashMap, + indexed: OptionAll>, + fields_position: HashMap, } impl Schema { @@ -68,7 +60,7 @@ impl Schema { ranked: HashSet::new(), displayed: OptionAll::All, indexed: OptionAll::All, - indexed_map, + fields_position: indexed_map, } } @@ -109,14 +101,16 @@ impl Schema { self.fields_map.insert(name) } - pub fn insert_and_index(&mut self, name: &str) -> SResult { + pub fn register_field(&mut self, name: &str) -> SResult { match self.fields_map.id(name) { Some(id) => { Ok(id) } None => { - self.set_indexed(name)?; - self.set_displayed(name) + let id = self.fields_map.insert(name)?; + let pos = self.fields_position.len() as u16; + self.fields_position.insert(id, pos.into()); + Ok(id) } } } @@ -156,18 +150,18 @@ impl Schema { } } - pub fn indexed(&self) -> Cow<[FieldId]> { + pub fn indexed(&self) -> Vec { match self.indexed { - OptionAll::Some(ref v) => Cow::Borrowed(v), + OptionAll::Some(ref v) => v.iter().cloned().collect(), OptionAll::All => { let fields = self - .fields_map - .iter() - .map(|(_, &f)| f) + .fields_position + .keys() + .cloned() .collect(); - Cow::Owned(fields) + fields }, - OptionAll::None => Cow::Owned(Vec::new()) + OptionAll::None => Vec::new() } } @@ -201,15 +195,15 @@ impl Schema { pub fn set_indexed(&mut self, name: &str) -> SResult<(FieldId, IndexedPos)> { let id = self.fields_map.insert(name)?; - if let Some(indexed_pos) = self.indexed_map.get(&id) { + if let OptionAll::Some(ref mut v) = self.indexed { + v.insert(id); + } + + if let Some(indexed_pos) = self.fields_position.get(&id) { return Ok((id, *indexed_pos)) }; - let pos = self.indexed_map.len() as u16; - self.indexed_map.insert(id, pos.into()); - self.indexed = self.indexed.take().map(|mut v| { - v.push(id); - v - }); + let pos = self.fields_position.len() as u16; + self.fields_position.insert(id, pos.into()); Ok((id, pos.into())) } @@ -223,50 +217,6 @@ impl Schema { } } - /// remove field from displayed attributes. If diplayed attributes is OptionAll::All, - /// dipslayed attributes is turned into OptionAll::Some(v) where v is all displayed attributes - /// except name. - pub fn remove_displayed(&mut self, name: &str) { - if let Some(id) = self.fields_map.id(name) { - self.displayed = match self.displayed.take() { - OptionAll::Some(mut v) => { - v.remove(&id); - OptionAll::Some(v) - } - OptionAll::All => { - let displayed = self.fields_map - .iter() - .filter_map(|(key, &value)| { - if key != name { - Some(value) - } else { - None - } - }) - .collect::>(); - OptionAll::Some(displayed) - } - OptionAll::None => OptionAll::None, - }; - } - } - - pub fn remove_indexed(&mut self, name: &str) { - if let Some(id) = self.fields_map.id(name) { - self.indexed_map.remove(&id); - self.indexed = match self.indexed.take() { - // valid because indexed is All and indexed() return the content of - // indexed_map that is already updated - OptionAll::All => OptionAll::Some(self.indexed().into_owned()), - OptionAll::Some(mut v) => { - v.retain(|x| *x != id); - OptionAll::Some(v) - } - OptionAll::None => OptionAll::None, - } - } - } - pub fn is_ranked(&self, id: FieldId) -> bool { self.ranked.get(&id).is_some() } @@ -280,7 +230,17 @@ impl Schema { } pub fn is_indexed(&self, id: FieldId) -> Option<&IndexedPos> { - self.indexed_map.get(&id) + match self.indexed { + OptionAll::All => self.fields_position.get(&id), + OptionAll::Some(ref v) => { + if v.contains(&id) { + self.fields_position.get(&id) + } else { + None + } + } + OptionAll::None => None + } } pub fn is_indexed_all(&self) -> bool { @@ -290,7 +250,7 @@ impl Schema { pub fn indexed_pos_to_field_id>(&self, pos: I) -> Option { let indexed_pos = pos.into().0; self - .indexed_map + .fields_position .iter() .find(|(_, &v)| v.0 == indexed_pos) .map(|(&k, _)| k) @@ -324,9 +284,8 @@ impl Schema { v.clear(); OptionAll::Some(v) }, - _ => OptionAll::Some(Vec::new()), + _ => OptionAll::Some(HashSet::new()), }; - self.indexed_map.clear(); for name in data { self.set_indexed(name.as_ref())?; } @@ -335,11 +294,11 @@ impl Schema { pub fn set_all_fields_as_indexed(&mut self) { self.indexed = OptionAll::All; - self.indexed_map.clear(); + self.fields_position.clear(); for (_name, id) in self.fields_map.iter() { - let pos = self.indexed_map.len() as u16; - self.indexed_map.insert(*id, pos.into()); + let pos = self.fields_position.len() as u16; + self.fields_position.insert(*id, pos.into()); } }