From ef3bcd65ab412bc722b139bde3534de5b5bb4eed Mon Sep 17 00:00:00 2001
From: qdequele <quentin@dequelen.me>
Date: Tue, 10 Mar 2020 15:35:19 +0100
Subject: [PATCH] fix comments from review

---
 meilisearch-core/src/error.rs           |  2 +-
 meilisearch-core/src/serde/mod.rs       |  2 +-
 meilisearch-http/src/routes/document.rs |  9 ++++-----
 meilisearch-http/src/routes/index.rs    | 14 +++++++-------
 meilisearch-http/src/routes/setting.rs  | 17 +++++++----------
 5 files changed, 20 insertions(+), 24 deletions(-)

diff --git a/meilisearch-core/src/error.rs b/meilisearch-core/src/error.rs
index 0e4cef035..eca70843f 100644
--- a/meilisearch-core/src/error.rs
+++ b/meilisearch-core/src/error.rs
@@ -87,7 +87,7 @@ impl fmt::Display for Error {
         match self {
             Io(e) => write!(f, "{}", e),
             IndexAlreadyExists => write!(f, "index already exists"),
-            MissingPrimaryKey => write!(f, "schema cannot be built without primary key"),
+            MissingPrimaryKey => write!(f, "schema cannot be built without a primary key"),
             SchemaMissing => write!(f, "this index does not have a schema"),
             WordIndexMissing => write!(f, "this index does not have a word index"),
             MissingDocumentId => write!(f, "document id is missing"),
diff --git a/meilisearch-core/src/serde/mod.rs b/meilisearch-core/src/serde/mod.rs
index 2605c4035..46352c0f5 100644
--- a/meilisearch-core/src/serde/mod.rs
+++ b/meilisearch-core/src/serde/mod.rs
@@ -57,7 +57,7 @@ impl fmt::Display for SerializerError {
                 f.write_str("serialized document does not have an id according to the schema")
             }
             SerializerError::InvalidDocumentIdType => {
-                f.write_str("documents primary keys can be of type integer or string only composed of alphanumeric characters, hyphens (-) and underscores (_).")
+                f.write_str("a document primary key can be of type integer or string only composed of alphanumeric characters, hyphens (-) and underscores (_).")
             }
             SerializerError::Zlmdb(e) => write!(f, "heed related error: {}", e),
             SerializerError::SerdeJson(e) => write!(f, "serde json error: {}", e),
diff --git a/meilisearch-http/src/routes/document.rs b/meilisearch-http/src/routes/document.rs
index 1eb7b66c1..a3e59ca05 100644
--- a/meilisearch-http/src/routes/document.rs
+++ b/meilisearch-http/src/routes/document.rs
@@ -149,11 +149,10 @@ async fn update_multiple_documents(mut ctx: Request<Data>, is_partial: bool) ->
             },
         };
 
-        if schema.set_primary_key(&id).is_ok() {
-            let mut writer = db.main_write_txn()?;
-            index.main.put_schema(&mut writer, &schema)?;
-            writer.commit()?;
-        }
+        let mut writer = db.main_write_txn()?;
+        schema.set_primary_key(&id).map_err(ResponseError::bad_request)?;
+        index.main.put_schema(&mut writer, &schema)?;
+        writer.commit()?;
     }
 
     let mut document_addition = if is_partial {
diff --git a/meilisearch-http/src/routes/index.rs b/meilisearch-http/src/routes/index.rs
index a4b11f049..5666ff0da 100644
--- a/meilisearch-http/src/routes/index.rs
+++ b/meilisearch-http/src/routes/index.rs
@@ -180,9 +180,8 @@ pub async fn create_index(mut ctx: Request<Data>) -> SResult<Response> {
 
     if let Some(id) = body.primary_key.clone() {
         if let Some(mut schema) = created_index.main.schema(&mut writer)? {
-            if let Ok(_) = schema.set_primary_key(&id) {
-                created_index.main.put_schema(&mut writer, &schema)?;
-            }
+            schema.set_primary_key(&id).map_err(ResponseError::bad_request)?;
+            created_index.main.put_schema(&mut writer, &schema)?;
         }
     }
 
@@ -239,13 +238,14 @@ pub async fn update_index(mut ctx: Request<Data>) -> SResult<Response> {
             match schema.primary_key() {
                 Some(_) => {
                     return Err(ResponseError::bad_request(
-                        "The index primary key cannot be updated",
+                        "The primary key cannot be updated",
                     ));
                 }
                 None => {
-                    if let Ok(_) = schema.set_primary_key(&id) {
-                        index.main.put_schema(&mut writer, &schema)?;
-                    }
+                    schema
+                        .set_primary_key(&id)
+                        .map_err(ResponseError::bad_request)?;
+                    index.main.put_schema(&mut writer, &schema)?;
                 }
             }
         }
diff --git a/meilisearch-http/src/routes/setting.rs b/meilisearch-http/src/routes/setting.rs
index 97d0ed1b3..703607c32 100644
--- a/meilisearch-http/src/routes/setting.rs
+++ b/meilisearch-http/src/routes/setting.rs
@@ -45,29 +45,26 @@ pub async fn get_all(ctx: Request<Data>) -> SResult<Response> {
     let schema = index.main.schema(&reader)?;
 
     let searchable_attributes = schema.clone().map(|s| {
-        let attrs = s
-            .indexed_name()
+        s.indexed_name()
             .iter()
             .map(|s| (*s).to_string())
-            .collect::<Vec<String>>();
-        Some(attrs)
+            .collect::<Vec<String>>()
     });
 
     let displayed_attributes = schema.clone().map(|s| {
-        let attrs = s
-            .displayed_name()
+        s.displayed_name()
             .iter()
             .map(|s| (*s).to_string())
-            .collect::<HashSet<String>>();
-        Some(attrs)
+            .collect::<HashSet<String>>()
     });
+
     let accept_new_fields = schema.map(|s| s.accept_new_fields());
 
     let settings = Settings {
         ranking_rules: Some(Some(ranking_rules)),
         distinct_attribute: Some(distinct_attribute),
-        searchable_attributes,
-        displayed_attributes,
+        searchable_attributes: Some(searchable_attributes),
+        displayed_attributes: Some(displayed_attributes),
         stop_words: Some(Some(stop_words)),
         synonyms: Some(Some(synonyms)),
         accept_new_fields: Some(accept_new_fields),