Merge pull request #520 from meilisearch/fix-http-issues

Fix http issues

meilisearch-http/src/routes/document.rs

@@ -118,9 +118,9 @@ fn find_primary_key(document: &IndexMap<String, Value>) -> Option<String> {
 }
 
 #[derive(Default, Deserialize)]
-#[serde(deny_unknown_fields)]
+#[serde(rename_all = "camelCase", deny_unknown_fields)]
 struct UpdateDocumentsQuery {
-    document_id: Option<String>,
+    primary_key: Option<String>,
 }
 
 async fn update_multiple_documents(mut ctx: Request<Data>, is_partial: bool) -> SResult<Response> {
@@ -141,11 +141,11 @@ async fn update_multiple_documents(mut ctx: Request<Data>, is_partial: bool) ->
         .ok_or(ResponseError::internal("schema not found"))?;
 
     if schema.primary_key().is_none() {
-        let id = match query.document_id {
+        let id = match query.primary_key {
             Some(id) => id,
             None => match data.first().and_then(|docs| find_primary_key(docs)) {
                 Some(id) => id,
-                None => return Err(ResponseError::bad_request("Could not infer a schema")),
+                None => return Err(ResponseError::bad_request("Could not infer a primary key")),
             },
         };
 

meilisearch-http/src/routes/setting.rs

@@ -1,5 +1,4 @@
 use meilisearch_core::settings::{Settings, SettingsUpdate, UpdateState, DEFAULT_RANKING_RULES};
-use serde::Deserialize;
 use std::collections::{BTreeMap, BTreeSet, HashSet};
 use tide::{Request, Response};
 
@@ -73,36 +72,13 @@ pub async fn get_all(ctx: Request<Data>) -> SResult<Response> {
     Ok(tide::Response::new(200).body_json(&settings).unwrap())
 }
 
-#[derive(Default, Clone, Deserialize)]
-#[serde(rename_all = "camelCase", deny_unknown_fields)]
-pub struct UpdateSettings {
-    pub ranking_rules: Option<Vec<String>>,
-    pub distinct_attribute: Option<String>,
-    pub primary_key: Option<String>,
-    pub searchable_attributes: Option<Vec<String>>,
-    pub displayed_attributes: Option<HashSet<String>>,
-    pub stop_words: Option<BTreeSet<String>>,
-    pub synonyms: Option<BTreeMap<String, Vec<String>>>,
-    pub accept_new_fields: Option<bool>,
-}
-
 pub async fn update_all(mut ctx: Request<Data>) -> SResult<Response> {
     ctx.is_allowed(Private)?;
     let index = ctx.index()?;
-    let settings_update: UpdateSettings =
+    let settings: Settings =
         ctx.body_json().await.map_err(ResponseError::bad_request)?;
     let db = &ctx.state().db;
 
-    let settings = Settings {
-        ranking_rules: Some(settings_update.ranking_rules),
-        distinct_attribute: Some(settings_update.distinct_attribute),
-        searchable_attributes: Some(settings_update.searchable_attributes),
-        displayed_attributes: Some(settings_update.displayed_attributes),
-        stop_words: Some(settings_update.stop_words),
-        synonyms: Some(settings_update.synonyms),
-        accept_new_fields: Some(settings_update.accept_new_fields),
-    };
-
     let mut writer = db.update_write_txn()?;
     let settings = settings.into_update().map_err(ResponseError::bad_request)?;
     let update_id = index.settings_update(&mut writer, settings)?;

meilisearch-http/tests/common.rs

@@ -44,30 +44,23 @@ impl Server {
         }
     }
 
-    fn wait_update_id(&mut self, update_id: u64) {
+    pub fn wait_update_id(&mut self, update_id: u64) {
         loop {
-            let req = http::Request::get(format!("/indexes/{}/updates/{}", self.uid, update_id))
+            let (response, status_code) = self.get_update_status(update_id);
-                .body(Body::empty())
+            assert_eq!(status_code, 200);
-                .unwrap();
-
-            let res = self.mock.simulate(req).unwrap();
-            assert_eq!(res.status(), 200);
-
-            let mut buf = Vec::new();
-            block_on(res.into_body().read_to_end(&mut buf)).unwrap();
-            let response: Value = serde_json::from_slice(&buf).unwrap();
 
             if response["status"] == "processed" || response["status"] == "error" {
                 eprintln!("{:#?}", response);
                 return;
             }
+
             block_on(sleep(Duration::from_secs(1)));
         }
     }
 
-    // // Global Http request GET/POST/DELETE async or sync
+    // Global Http request GET/POST/DELETE async or sync
 
-    fn get_request(&mut self, url: &str) -> (Value, StatusCode) {
+    pub fn get_request(&mut self, url: &str) -> (Value, StatusCode) {
         eprintln!("get_request: {}", url);
         let req = http::Request::get(url).body(Body::empty()).unwrap();
         let res = self.mock.simulate(req).unwrap();
@@ -79,7 +72,7 @@ impl Server {
         (response, status_code)
     }
 
-    fn post_request(&mut self, url: &str, body: Value) -> (Value, StatusCode) {
+    pub fn post_request(&mut self, url: &str, body: Value) -> (Value, StatusCode) {
         eprintln!("post_request: {}", url);
         let body_bytes = body.to_string().into_bytes();
 
@@ -95,7 +88,7 @@ impl Server {
         (response, status_code)
     }
 
-    fn post_request_async(&mut self, url: &str, body: Value) -> (Value, StatusCode) {
+    pub fn post_request_async(&mut self, url: &str, body: Value) -> (Value, StatusCode) {
         eprintln!("post_request_async: {}", url);
         let (response, status_code) = self.post_request(url, body);
         assert_eq!(status_code, 202);
@@ -104,7 +97,7 @@ impl Server {
         (response, status_code)
     }
 
-    fn put_request(&mut self, url: &str, body: Value) -> (Value, StatusCode) {
+    pub fn put_request(&mut self, url: &str, body: Value) -> (Value, StatusCode) {
         eprintln!("put_request: {}", url);
         let body_bytes = body.to_string().into_bytes();
 
@@ -120,7 +113,7 @@ impl Server {
         (response, status_code)
     }
 
-    fn put_request_async(&mut self, url: &str, body: Value) -> (Value, StatusCode) {
+    pub fn put_request_async(&mut self, url: &str, body: Value) -> (Value, StatusCode) {
         eprintln!("put_request_async: {}", url);
         let (response, status_code) = self.put_request(url, body);
         assert!(response["updateId"].as_u64().is_some());
@@ -129,7 +122,7 @@ impl Server {
         (response, status_code)
     }
 
-    fn delete_request(&mut self, url: &str) -> (Value, StatusCode) {
+    pub fn delete_request(&mut self, url: &str) -> (Value, StatusCode) {
         eprintln!("delete_request: {}", url);
         let req = http::Request::delete(url).body(Body::empty()).unwrap();
         let res = self.mock.simulate(req).unwrap();
@@ -141,7 +134,7 @@ impl Server {
         (response, status_code)
     }
 
-    fn delete_request_async(&mut self, url: &str) -> (Value, StatusCode) {
+    pub fn delete_request_async(&mut self, url: &str) -> (Value, StatusCode) {
         eprintln!("delete_request_async: {}", url);
         let (response, status_code) = self.delete_request(url);
         assert!(response["updateId"].as_u64().is_some());
@@ -150,7 +143,7 @@ impl Server {
         (response, status_code)
     }
 
-    // // All Routes
+    // All Routes
 
     pub fn list_indexes(&mut self) -> (Value, StatusCode) {
         self.get_request("/indexes")
@@ -205,6 +198,11 @@ impl Server {
         self.post_request_async(&url, body);
     }
 
+    pub fn add_or_replace_multiple_documents_sync(&mut self, body: Value) -> (Value, StatusCode) {
+        let url = format!("/indexes/{}/documents", self.uid);
+        self.post_request(&url, body)
+    }
+
     pub fn add_or_update_multiple_documents(&mut self, body: Value) {
         let url = format!("/indexes/{}/documents", self.uid);
         self.put_request_async(&url, body);

meilisearch-http/tests/documents_add.rs

@@ -0,0 +1,38 @@
+use serde_json::json;
+
+mod common;
+
+// Test issue https://github.com/meilisearch/MeiliSearch/issues/519
+#[test]
+fn check_add_documents_with_primary_key_param() {
+    let mut server = common::Server::with_uid("movies");
+
+    // 1 - Create the index with no primary_key
+
+    let body = json!({
+        "uid": "movies",
+    });
+    let (response, status_code) = server.create_index(body);
+    assert_eq!(status_code, 201);
+    assert_eq!(response["primaryKey"], json!(null));
+
+    // 2 - Add documents
+
+    let body = json!([{
+      "title": "Test",
+      "comment": "comment test"
+    }]);
+
+    let url = "/indexes/movies/documents?primaryKey=title";
+    let (response, status_code) = server.post_request(&url, body);
+    eprintln!("{:#?}", response);
+    assert_eq!(status_code, 202);
+    let update_id = response["updateId"].as_u64().unwrap();
+    server.wait_update_id(update_id);
+
+    // 3 - Check update sucess
+
+    let (response, status_code) = server.get_update_status(update_id);
+    assert_eq!(status_code, 200);
+    assert_eq!(response["status"], "processed");
+}

meilisearch-http/tests/index.rs

@@ -625,3 +625,35 @@ fn create_index_without_primary_key_and_search() {
     assert_eq!(status_code, 200);
     assert_eq!(response["hits"].as_array().unwrap().len(), 0);
 }
+
+// Test the error message when we push an document update and impossibility to find primary key
+// Test issue https://github.com/meilisearch/MeiliSearch/issues/517
+#[test]
+fn check_add_documents_without_primary_key() {
+    let mut server = common::Server::with_uid("movies");
+
+    // 1 - Create the index with no primary_key
+
+    let body = json!({
+        "uid": "movies",
+    });
+    let (response, status_code) = server.create_index(body);
+    assert_eq!(status_code, 201);
+    assert_eq!(response["primaryKey"], json!(null));
+
+    // 2- Add document
+
+    let body = json!([{
+      "title": "Test",
+      "comment": "comment test"
+    }]);
+
+    let (response, status_code) = server.add_or_replace_multiple_documents_sync(body);
+
+    let expected = json!({
+        "message": "Could not infer a primary key"
+    });
+
+    assert_eq!(status_code, 400);
+    assert_json_eq!(response, expected, ordered: false);
+}

meilisearch-http/tests/search.rs

@@ -644,7 +644,6 @@ fn search_with_settings_basic() {
         "desc(vote_average)"
       ],
       "distinctAttribute": null,
-      "primaryKey": "id",
       "searchableAttributes": [
         "title",
         "tagline",
@@ -751,7 +750,6 @@ fn search_with_settings_stop_words() {
         "desc(vote_average)"
       ],
       "distinctAttribute": null,
-      "primaryKey": "id",
       "searchableAttributes": [
         "title",
         "tagline",
@@ -858,7 +856,6 @@ fn search_with_settings_synonyms() {
         "desc(vote_average)"
       ],
       "distinctAttribute": null,
-      "primaryKey": "id",
       "searchableAttributes": [
         "title",
         "tagline",
@@ -970,7 +967,6 @@ fn search_with_settings_ranking_rules() {
         "desc(popularity)"
       ],
       "distinctAttribute": null,
-      "primaryKey": "id",
       "searchableAttributes": [
         "title",
         "tagline",
@@ -1077,7 +1073,6 @@ fn search_with_settings_searchable_attributes() {
         "desc(vote_average)"
       ],
       "distinctAttribute": null,
-      "primaryKey": "id",
       "searchableAttributes": [
         "tagline",
         "overview",
@@ -1183,7 +1178,6 @@ fn search_with_settings_displayed_attributes() {
         "desc(vote_average)"
       ],
       "distinctAttribute": null,
-      "primaryKey": "id",
       "searchableAttributes": [
         "title",
         "tagline",
@@ -1254,7 +1248,6 @@ fn search_with_settings_searchable_attributes_2() {
         "desc(vote_average)"
       ],
       "distinctAttribute": null,
-      "primaryKey": "id",
       "searchableAttributes": [
         "tagline",
         "overview",

meilisearch-http/tests/settings.rs

@@ -192,6 +192,7 @@ fn write_all_and_update() {
             "exactness",
             "desc(release_date)",
         ],
+        "distinctAttribute": null,
         "searchableAttributes": [
             "title",
             "description",
@@ -204,8 +205,7 @@ fn write_all_and_update() {
             "rank",
             "poster",
         ],
-        "stopWords": [
+        "stopWords": [],
-        ],
         "synonyms": {
             "wolverine": ["xmen", "logan"],
             "logan": ["wolverine", "xmen"],
@@ -321,3 +321,111 @@ fn test_default_settings_2() {
 
     assert_json_eq!(body, response, ordered: false);
 }
+
+// Test issue https://github.com/meilisearch/MeiliSearch/issues/516
+#[test]
+fn write_setting_and_update_partial() {
+    let mut server = common::Server::with_uid("movies");
+    let body = json!({
+        "uid": "movies",
+    });
+    server.create_index(body);
+
+    // 2 - Send the settings
+
+    let body = json!({
+        "searchableAttributes": [
+            "uid",
+            "movie_id",
+            "title",
+            "description",
+            "poster",
+            "release_date",
+            "rank",
+        ],
+        "displayedAttributes": [
+            "title",
+            "description",
+            "poster",
+            "release_date",
+            "rank",
+        ]
+    });
+
+    server.update_all_settings(body.clone());
+
+    // 2 - Send the settings
+
+    let body = json!({
+        "rankingRules": [
+            "typo",
+            "words",
+            "proximity",
+            "attribute",
+            "wordsPosition",
+            "exactness",
+            "desc(release_date)",
+            "desc(rank)",
+        ],
+        "distinctAttribute": "movie_id",
+        "stopWords": [
+            "the",
+            "a",
+            "an",
+        ],
+        "synonyms": {
+            "wolverine": ["xmen", "logan"],
+            "logan": ["wolverine"],
+        },
+        "acceptNewFields": false,
+    });
+
+    server.update_all_settings(body.clone());
+
+    // 2 - Send the settings
+
+    let expected = json!({
+        "rankingRules": [
+            "typo",
+            "words",
+            "proximity",
+            "attribute",
+            "wordsPosition",
+            "exactness",
+            "desc(release_date)",
+            "desc(rank)",
+        ],
+        "distinctAttribute": "movie_id",
+        "searchableAttributes": [
+            "uid",
+            "movie_id",
+            "title",
+            "description",
+            "poster",
+            "release_date",
+            "rank",
+        ],
+        "displayedAttributes": [
+            "title",
+            "description",
+            "poster",
+            "release_date",
+            "rank",
+        ],
+        "stopWords": [
+            "the",
+            "a",
+            "an",
+        ],
+        "synonyms": {
+            "wolverine": ["xmen", "logan"],
+            "logan": ["wolverine"],
+        },
+        "acceptNewFields": false,
+    });
+
+    let (response, _status_code) = server.get_all_settings();
+
+    assert_json_eq!(expected, response, ordered: false);
+
+}

meilisearch-http/tests/settings_accept_new_fields.rs

@@ -288,3 +288,62 @@ fn index_new_fields_false_then_true() {
     assert_eq!(status_code, 200);
     assert_json_eq!(response, expected);
 }
+
+
+// Fix issue https://github.com/meilisearch/MeiliSearch/issues/518
+#[test]
+fn accept_new_fields_does_not_take_into_account_the_primary_key () {
+    let mut server = common::Server::with_uid("movies");
+
+    // 1 - Create an index with no primary-key
+
+    let body = json!({
+        "uid": "movies",
+    });
+    let (response, status_code) = server.create_index(body);
+    assert_eq!(status_code, 201);
+    assert_eq!(response["primaryKey"], json!(null));
+
+    // 2 - Add searchable and displayed attributes as: ["title"] & Set acceptNewFields to false
+
+    let body = json!({
+        "searchableAttributes": ["title"],
+        "displayedAttributes": ["title"],
+        "acceptNewFields": false,
+    });
+
+    server.update_all_settings(body);
+
+    // 4 - Add a document
+
+    let body = json!([{
+      "id": 1,
+      "title": "Test",
+      "comment": "comment test"
+    }]);
+
+    server.add_or_replace_multiple_documents(body);
+
+    // 5 - Get settings, they should not changed
+
+    let (response, _status_code) = server.get_all_settings();
+
+    let expected = json!({
+        "rankingRules": [
+            "typo",
+            "words",
+            "proximity",
+            "attribute",
+            "wordsPosition",
+            "exactness",
+        ],
+        "distinctAttribute": null,
+        "searchableAttributes": ["title"],
+        "displayedAttributes": ["title"],
+        "stopWords": [],
+        "synonyms": {},
+        "acceptNewFields": false,
+    });
+
+    assert_json_eq!(response, expected, ordered: false);
+}

meilisearch-schema/src/schema.rs

@@ -63,8 +63,10 @@ impl Schema {
 
         let id = self.insert(name)?;
         self.primary_key = Some(id);
-        self.set_indexed(name)?;
+        if self.accept_new_fields {
-        self.set_displayed(name)?;
+            self.set_indexed(name)?;
+            self.set_displayed(name)?;
+        }
 
         Ok(id)
     }