add test to reproduce #891 bug report

fix bug

meilisearch-core/src/update/documents_addition.rs

@@ -201,12 +201,14 @@ pub fn apply_addition<'a, 'b>(
             };
 
             let old_document = Option::<HashMap<String, Value>>::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);
     }
 

meilisearch-core/src/update/helpers.rs

@@ -20,6 +20,7 @@ pub fn index_value<A>(
 ) -> Option<usize>
 where A: AsRef<[u8]>,
 {
+    println!("indexing value: {}", value);
     match value {
         Value::Null => None,
         Value::Bool(boolean) => {

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;

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"],
+        "displayedAttributes": ["genre", "type"],
-        "searchableAttributes": ["genre"]
+        "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());
 }

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;
 
@@ -521,3 +521,71 @@ async fn test_displayed_attributes_field() {
 
     assert_json_eq!(body, response, ordered: true);
 }
+
+#[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);
+}

meilisearch-schema/src/schema.rs

@@ -43,7 +43,7 @@ pub struct Schema {
     ranked: HashSet<FieldId>,
     displayed: OptionAll<HashSet<FieldId>>,
 
-    indexed: OptionAll<Vec<FieldId>>,
+    indexed: OptionAll<HashSet<FieldId>>,
     indexed_map: HashMap<FieldId, IndexedPos>,
 }
 
@@ -115,8 +115,10 @@ impl Schema {
                 Ok(id)
             }
             None => {
-                self.set_indexed(name)?;
+                let id = self.fields_map.insert(name)?;
-                self.set_displayed(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<FieldId> {
         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> {
+        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())?;
         }