Merge pull request #778 from MarinPostma/consistent-settings

Make settings more consistent

meilisearch-core/src/store/main.rs

@@ -286,15 +286,15 @@ impl Main {
         Ok(self.main.delete::<_, Str>(writer, RANKING_RULES_KEY)?)
     }
 
-    pub fn distinct_attribute(&self, reader: &heed::RoTxn<MainT>) -> MResult<Option<String>> {
+    pub fn distinct_attribute(&self, reader: &heed::RoTxn<MainT>) -> MResult<Option<FieldId>> {
-        if let Some(value) = self.main.get::<_, Str, Str>(reader, DISTINCT_ATTRIBUTE_KEY)? {
+        if let Some(value) = self.main.get::<_, Str, OwnedType<u16>>(reader, DISTINCT_ATTRIBUTE_KEY)? {
-            return Ok(Some(value.to_owned()))
+            return Ok(Some(FieldId(value.to_owned())))
         }
         return Ok(None)
     }
 
-    pub fn put_distinct_attribute(self, writer: &mut heed::RwTxn<MainT>, value: &str) -> MResult<()> {
+    pub fn put_distinct_attribute(self, writer: &mut heed::RwTxn<MainT>, value: FieldId) -> MResult<()> {
-        Ok(self.main.put::<_, Str, Str>(writer, DISTINCT_ATTRIBUTE_KEY, value)?)
+        Ok(self.main.put::<_, Str, OwnedType<u16>>(writer, DISTINCT_ATTRIBUTE_KEY, &value.0)?)
     }
 
     pub fn delete_distinct_attribute(self, writer: &mut heed::RwTxn<MainT>) -> MResult<bool> {

meilisearch-core/src/update/settings_update.rs

@@ -46,12 +46,6 @@ pub fn apply_settings_update(
         UpdateState::Update(v) => {
             let ranked_field: Vec<&str> = v.iter().filter_map(RankingRule::field).collect();
             schema.update_ranked(&ranked_field)?;
-            for name in ranked_field {
-                if schema.accept_new_fields() {
-                    schema.set_indexed(name.as_ref())?;
-                    schema.set_displayed(name.as_ref())?;
-                }
-            }
             index.main.put_ranking_rules(writer, &v)?;
             must_reindex = true;
         },
@@ -65,7 +59,8 @@ pub fn apply_settings_update(
 
     match settings.distinct_attribute {
         UpdateState::Update(v) => {
-            index.main.put_distinct_attribute(writer, &v)?;
+            let field_id = schema.insert(&v)?;
+            index.main.put_distinct_attribute(writer, field_id)?;
         },
         UpdateState::Clear => {
             index.main.delete_distinct_attribute(writer)?;

meilisearch-http/src/helpers/meilisearch.rs

@@ -139,10 +139,9 @@ impl<'a> SearchBuilder<'a> {
         }
 
         if let Some(field) = self.index.main.distinct_attribute(reader)? {
-            if let Some(field_id) = schema.id(&field) {
             let index = &self.index;
             query_builder.with_distinct(1, move |id| {
-                    match index.document_attribute_bytes(reader, id, field_id) {
+                match index.document_attribute_bytes(reader, id, field) {
                     Ok(Some(bytes)) => {
                         let mut s = SipHasher::new();
                         bytes.hash(&mut s);
@@ -152,7 +151,6 @@ impl<'a> SearchBuilder<'a> {
                 }
             });
         }
-        }
 
         query_builder.set_facet_filter(self.facet_filters);
         query_builder.set_facets(self.facets);

meilisearch-http/src/routes/setting.rs

@@ -89,10 +89,14 @@ async fn get_all(
         .map(|r| r.to_string())
         .collect();
 
-    let distinct_attribute = index.main.distinct_attribute(&reader)?;
 
     let schema = index.main.schema(&reader)?;
 
+    let distinct_attribute = match (index.main.distinct_attribute(&reader)?, &schema) {
+        (Some(id), Some(schema)) => schema.name(id).map(str::to_string),
+        _ => None,
+    };
+
     let attributes_for_faceting = match (&schema, &index.main.attributes_for_faceting(&reader)?) {
         (Some(schema), Some(attrs)) => {
             attrs

meilisearch-http/tests/settings.rs

@@ -455,3 +455,33 @@ async fn attributes_for_faceting_settings() {
     let (response, _status_code) = server.get_request("/indexes/test/settings/attributes-for-faceting").await;
     assert_eq!(response, json!([]));
 }
+
+#[actix_rt::test]
+async fn setting_ranking_rules_dont_mess_with_other_settings() {
+    let mut server = common::Server::test_server().await;
+    let body = json!({
+        "rankingRules": ["asc(foobar)"]
+    });
+    server.update_all_settings(body).await;
+    let (response, _) = server.get_all_settings().await;
+    assert_eq!(response["rankingRules"].as_array().unwrap().len(), 1);
+    assert_eq!(response["rankingRules"].as_array().unwrap().first().unwrap().as_str().unwrap(), "asc(foobar)");
+    assert!(!response["searchableAttributes"].as_array().unwrap().iter().any(|e| e.as_str().unwrap() == "foobar"));
+    assert!(!response["displayedAttributes"].as_array().unwrap().iter().any(|e| e.as_str().unwrap() == "foobar"));
+}
+
+#[actix_rt::test]
+async fn distinct_attribute_recorded_as_known_field() {
+   let mut server = common::Server::test_server().await;
+   let body = json!({
+       "distinctAttribute": "foobar",
+       "acceptNewFields": true
+   });
+   server.update_all_settings(body).await;
+   let document = json!([{"id": 9348127, "foobar": "hello", "foo": "bar"}]);
+   server.add_or_update_multiple_documents(document).await;
+   // foobar should not be added to the searchable attributes because it is already known, but "foo" should
+   let (response, _) = server.get_all_settings().await;
+   assert!(response["searchableAttributes"].as_array().unwrap().iter().any(|v| v.as_str().unwrap() == "foo"));
+   assert!(!response["searchableAttributes"].as_array().unwrap().iter().any(|v| v.as_str().unwrap() == "foobar"));
+}

meilisearch-http/tests/settings_ranking_rules.rs

@@ -173,7 +173,6 @@ async fn write_custom_ranking_and_index_documents() {
 
     let expected = json!({
         "id": 1,
-        "title": "Le Petit Prince",
         "author": "Exupéry"
     });