From dc2e5ceed2672aeb1be52bd2cb21ac0950071d03 Mon Sep 17 00:00:00 2001 From: mpostma Date: Tue, 13 Oct 2020 13:50:42 +0200 Subject: [PATCH] fix bug --- meilisearch-core/src/facets.rs | 4 +- meilisearch-core/src/serde/deserializer.rs | 2 + .../src/update/documents_addition.rs | 4 +- meilisearch-core/src/update/helpers.rs | 1 - .../src/update/settings_update.rs | 2 - meilisearch-http/tests/search.rs | 4 +- meilisearch-http/tests/settings.rs | 10 +- meilisearch-schema/src/schema.rs | 98 ++++--------------- 8 files changed, 26 insertions(+), 99 deletions(-) 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 d4674016b..2ef266565 100644 --- a/meilisearch-core/src/serde/deserializer.rs +++ b/meilisearch-core/src/serde/deserializer.rs @@ -94,6 +94,8 @@ impl<'de, 'a, 'b> de::Deserializer<'de> for &'b mut Deserializer<'a> { }; let is_displayed = self.schema.is_displayed(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()); diff --git a/meilisearch-core/src/update/documents_addition.rs b/meilisearch-core/src/update/documents_addition.rs index 7e7d069d3..2dc68c3ba 100644 --- a/meilisearch-core/src/update/documents_addition.rs +++ b/meilisearch-core/src/update/documents_addition.rs @@ -201,14 +201,12 @@ pub fn apply_addition<'a, 'b>( }; let old_document = Option::>::deserialize(&mut deserializer)?; - println!("old document: {:#?}", old_document); if let Some(old_document) = old_document { for (key, value) in old_document { document.entry(key).or_insert(value); } } } - println!("new document: {:#?}", document); documents_additions.insert(internal_docid, document); } @@ -231,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-core/src/update/helpers.rs b/meilisearch-core/src/update/helpers.rs index 77e2d5705..1aad1f505 100644 --- a/meilisearch-core/src/update/helpers.rs +++ b/meilisearch-core/src/update/helpers.rs @@ -20,7 +20,6 @@ pub fn index_value( ) -> Option where A: AsRef<[u8]>, { - println!("indexing value: {}", value); match value { Value::Null => None, Value::Bool(boolean) => { diff --git a/meilisearch-core/src/update/settings_update.rs b/meilisearch-core/src/update/settings_update.rs index 7d5bb1d3e..94e337265 100644 --- a/meilisearch-core/src/update/settings_update.rs +++ b/meilisearch-core/src/update/settings_update.rs @@ -70,11 +70,9 @@ pub fn apply_settings_update( match settings.searchable_attributes.clone() { UpdateState::Update(v) => { - println!("updating indexed attributes"); if v.iter().any(|e| e == "*") || v.is_empty() { schema.set_all_fields_as_indexed(); } else { - println!("updating indexed"); schema.update_indexed(v)?; } must_reindex = true; diff --git a/meilisearch-http/tests/search.rs b/meilisearch-http/tests/search.rs index 6eb46e7f6..92879bb7b 100644 --- a/meilisearch-http/tests/search.rs +++ b/meilisearch-http/tests/search.rs @@ -1779,7 +1779,7 @@ async fn update_documents_with_facet_distribution() { server.create_index(body).await; let settings = json!({ "attributesForFaceting": ["genre"], - "displayedAttributes": ["genre", "type"], + "displayedAttributes": ["genre"], "searchableAttributes": ["genre", "type"] }); server.update_all_settings(settings).await; @@ -1809,7 +1809,6 @@ async fn update_documents_with_facet_distribution() { "facetsDistribution": ["genre"] }); let (response1, _) = server.search_post(search.clone()).await; - println!("response1: {}", response1); let expected_facet_distribution = json!({ "genre": { "grunge": 1, @@ -1828,6 +1827,5 @@ async fn update_documents_with_facet_distribution() { ]); server.add_or_update_multiple_documents(update2).await; let (response2, _) = server.search_post(search).await; - println!("response2: {}", response2); assert_json_eq!(expected_facet_distribution, response2["facetsDistribution"].clone()); } diff --git a/meilisearch-http/tests/settings.rs b/meilisearch-http/tests/settings.rs index 02e35b32d..4b7b591a4 100644 --- a/meilisearch-http/tests/settings.rs +++ b/meilisearch-http/tests/settings.rs @@ -486,15 +486,7 @@ async fn test_displayed_attributes_field() { ], "distinctAttribute": "id", "searchableAttributes": [ - "id", - "name", - "color", - "gender", - "email", - "phone", - "address", - "registered", - "about" + "*" ], "displayedAttributes": [ "age", diff --git a/meilisearch-schema/src/schema.rs b/meilisearch-schema/src/schema.rs index d35ae4f76..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) } @@ -44,7 +36,7 @@ pub struct Schema { displayed: OptionAll>, indexed: OptionAll>, - indexed_map: HashMap, + 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,15 +101,15 @@ 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 => { let id = self.fields_map.insert(name)?; - 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()); Ok(id) } } @@ -163,9 +155,9 @@ impl Schema { OptionAll::Some(ref v) => v.iter().cloned().collect(), OptionAll::All => { let fields = self - .indexed_map - .iter() - .map(|(&f, _)| f) + .fields_position + .keys() + .cloned() .collect(); fields }, @@ -202,18 +194,16 @@ impl Schema { pub fn set_indexed(&mut self, name: &str) -> SResult<(FieldId, IndexedPos)> { let id = self.fields_map.insert(name)?; - println!("setting {} indexed", name); - self.indexed = self.indexed.take().map(|mut v| { + if let OptionAll::Some(ref mut v) = self.indexed { v.insert(id); - v - }); + } - if let Some(indexed_pos) = self.indexed_map.get(&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()); + let pos = self.fields_position.len() as u16; + self.fields_position.insert(id, pos.into()); Ok((id, pos.into())) } @@ -227,56 +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 => { - let indexed = self.indexed_map - .iter() - .filter_map(|(&i, _)| if i != id { Some(i) } else { None }) - .collect(); - OptionAll::Some(indexed) - }, - 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() } @@ -291,10 +231,10 @@ impl Schema { pub fn is_indexed(&self, id: FieldId) -> Option<&IndexedPos> { match self.indexed { - OptionAll::All => self.indexed_map.get(&id), + OptionAll::All => self.fields_position.get(&id), OptionAll::Some(ref v) => { if v.contains(&id) { - self.indexed_map.get(&id) + self.fields_position.get(&id) } else { None } @@ -310,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) @@ -354,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()); } }