From 1639a7338d6b863958b965c16a9a5483bc835da4 Mon Sep 17 00:00:00 2001 From: many Date: Mon, 31 Aug 2020 18:15:07 +0200 Subject: [PATCH] add test to reproduce #891 bug report fix bug --- .../src/update/documents_addition.rs | 2 + meilisearch-core/src/update/helpers.rs | 1 + .../src/update/settings_update.rs | 2 + meilisearch-http/tests/search.rs | 6 +- meilisearch-http/tests/settings.rs | 72 ++++++++++++++++++- meilisearch-schema/src/schema.rs | 53 +++++++++----- 6 files changed, 115 insertions(+), 21 deletions(-) diff --git a/meilisearch-core/src/update/documents_addition.rs b/meilisearch-core/src/update/documents_addition.rs index e6b9db90e..7e7d069d3 100644 --- a/meilisearch-core/src/update/documents_addition.rs +++ b/meilisearch-core/src/update/documents_addition.rs @@ -201,12 +201,14 @@ 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); } diff --git a/meilisearch-core/src/update/helpers.rs b/meilisearch-core/src/update/helpers.rs index 1aad1f505..77e2d5705 100644 --- a/meilisearch-core/src/update/helpers.rs +++ b/meilisearch-core/src/update/helpers.rs @@ -20,6 +20,7 @@ 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 94e337265..7d5bb1d3e 100644 --- a/meilisearch-core/src/update/settings_update.rs +++ b/meilisearch-core/src/update/settings_update.rs @@ -70,9 +70,11 @@ 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 170d9ffaf..6eb46e7f6 100644 --- a/meilisearch-http/tests/search.rs +++ b/meilisearch-http/tests/search.rs @@ -1779,8 +1779,8 @@ async fn update_documents_with_facet_distribution() { server.create_index(body).await; let settings = json!({ "attributesForFaceting": ["genre"], - "displayedAttributes": ["genre"], - "searchableAttributes": ["genre"] + "displayedAttributes": ["genre", "type"], + "searchableAttributes": ["genre", "type"] }); server.update_all_settings(settings).await; let update1 = json!([ @@ -1809,6 +1809,7 @@ 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, @@ -1827,5 +1828,6 @@ 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 6b125c13a..02e35b32d 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; @@ -520,4 +520,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..d35ae4f76 100644 --- a/meilisearch-schema/src/schema.rs +++ b/meilisearch-schema/src/schema.rs @@ -43,7 +43,7 @@ pub struct Schema { ranked: HashSet, displayed: OptionAll>, - indexed: OptionAll>, + indexed: OptionAll>, indexed_map: HashMap, } @@ -115,8 +115,10 @@ impl Schema { Ok(id) } None => { - self.set_indexed(name)?; - self.set_displayed(name) + let id = self.fields_map.insert(name)?; + let pos = self.indexed_map.len() as u16; + self.indexed_map.insert(id, pos.into()); + Ok(id) } } } @@ -156,18 +158,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 + .indexed_map .iter() - .map(|(_, &f)| f) + .map(|(&f, _)| f) .collect(); - Cow::Owned(fields) + fields }, - OptionAll::None => Cow::Owned(Vec::new()) + OptionAll::None => Vec::new() } } @@ -200,16 +202,18 @@ 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| { + v.insert(id); + v + }); if let Some(indexed_pos) = self.indexed_map.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 - }); Ok((id, pos.into())) } @@ -257,7 +261,13 @@ impl Schema { 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::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) @@ -280,7 +290,17 @@ impl Schema { } pub fn is_indexed(&self, id: FieldId) -> Option<&IndexedPos> { - self.indexed_map.get(&id) + match self.indexed { + OptionAll::All => self.indexed_map.get(&id), + OptionAll::Some(ref v) => { + if v.contains(&id) { + self.indexed_map.get(&id) + } else { + None + } + } + OptionAll::None => None + } } pub fn is_indexed_all(&self) -> bool { @@ -324,9 +344,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())?; }