Merge #2006 #2007

2006: chore(http): rename task types r=curquiza a=MarinPostma

Rename
- documentsAddition into documentAddition
- documentsPartial into documentPartial
- documentsDeletion into documentDeletion

close #1999


2007: bug(lib): ignore primary if already set on document addition r=curquiza a=MarinPostma

Ignore the primary key if it is already set on documents updates. Add a test for verify behaviour.

close #2002


Co-authored-by: Marin Postma <postma.marin@protonmail.com>

meilisearch-http/src/task.rs

@@ -13,9 +13,9 @@ enum TaskType {
     IndexCreation,
     IndexUpdate,
     IndexDeletion,
-    DocumentsAddition,
-    DocumentsPartial,
-    DocumentsDeletion,
+    DocumentAddition,
+    DocumentPartial,
+    DocumentDeletion,
     SettingsUpdate,
     ClearAll,
 }
@@ -26,13 +26,13 @@ impl From<TaskContent> for TaskType {
             TaskContent::DocumentAddition {
                 merge_strategy: IndexDocumentsMethod::ReplaceDocuments,
                 ..
-            } => TaskType::DocumentsAddition,
+            } => TaskType::DocumentAddition,
             TaskContent::DocumentAddition {
                 merge_strategy: IndexDocumentsMethod::UpdateDocuments,
                 ..
-            } => TaskType::DocumentsPartial,
+            } => TaskType::DocumentPartial,
             TaskContent::DocumentDeletion(DocumentDeletion::Clear) => TaskType::ClearAll,
-            TaskContent::DocumentDeletion(DocumentDeletion::Ids(_)) => TaskType::DocumentsDeletion,
+            TaskContent::DocumentDeletion(DocumentDeletion::Ids(_)) => TaskType::DocumentDeletion,
             TaskContent::SettingsUpdate { .. } => TaskType::SettingsUpdate,
             TaskContent::IndexDeletion => TaskType::IndexDeletion,
             TaskContent::IndexCreation { .. } => TaskType::IndexCreation,
@@ -56,7 +56,7 @@ enum TaskStatus {
 #[allow(clippy::large_enum_variant)]
 enum TaskDetails {
     #[serde(rename_all = "camelCase")]
-    DocumentsAddition {
+    DocumentAddition {
         received_documents: usize,
         indexed_documents: Option<u64>,
     },
@@ -123,21 +123,21 @@ impl From<Task> for TaskView {
                 documents_count,
                 ..
             } => {
-                let details = TaskDetails::DocumentsAddition {
+                let details = TaskDetails::DocumentAddition {
                     received_documents: documents_count,
                     indexed_documents: None,
                 };
 
                 let task_type = match merge_strategy {
-                    IndexDocumentsMethod::UpdateDocuments => TaskType::DocumentsPartial,
-                    IndexDocumentsMethod::ReplaceDocuments => TaskType::DocumentsAddition,
+                    IndexDocumentsMethod::UpdateDocuments => TaskType::DocumentPartial,
+                    IndexDocumentsMethod::ReplaceDocuments => TaskType::DocumentAddition,
                     _ => unreachable!("Unexpected document merge strategy."),
                 };
 
                 (task_type, Some(details))
             }
             TaskContent::DocumentDeletion(DocumentDeletion::Ids(ids)) => (
-                TaskType::DocumentsDeletion,
+                TaskType::DocumentDeletion,
                 Some(TaskDetails::DocumentDeletion {
                     received_document_ids: ids.len(),
                     deleted_documents: None,
@@ -181,7 +181,7 @@ impl From<Task> for TaskView {
                             indexed_documents: num,
                             ..
                         },
-                        Some(TaskDetails::DocumentsAddition {
+                        Some(TaskDetails::DocumentAddition {
                             ref mut indexed_documents,
                             ..
                         }),

meilisearch-http/tests/documents/add_documents.rs

@@ -563,7 +563,7 @@ async fn add_documents_no_index_creation() {
     assert_eq!(code, 200);
     assert_eq!(response["status"], "succeeded");
     assert_eq!(response["uid"], 0);
-    assert_eq!(response["type"], "documentsAddition");
+    assert_eq!(response["type"], "documentAddition");
     assert_eq!(response["details"]["receivedDocuments"], 1);
     assert_eq!(response["details"]["indexedDocuments"], 1);
 
@@ -633,7 +633,7 @@ async fn document_addition_with_primary_key() {
     assert_eq!(code, 200);
     assert_eq!(response["status"], "succeeded");
     assert_eq!(response["uid"], 0);
-    assert_eq!(response["type"], "documentsAddition");
+    assert_eq!(response["type"], "documentAddition");
     assert_eq!(response["details"]["receivedDocuments"], 1);
     assert_eq!(response["details"]["indexedDocuments"], 1);
 
@@ -662,7 +662,7 @@ async fn document_update_with_primary_key() {
     assert_eq!(code, 200);
     assert_eq!(response["status"], "succeeded");
     assert_eq!(response["uid"], 0);
-    assert_eq!(response["type"], "documentsPartial");
+    assert_eq!(response["type"], "documentPartial");
     assert_eq!(response["details"]["indexedDocuments"], 1);
     assert_eq!(response["details"]["receivedDocuments"], 1);
 
@@ -775,7 +775,7 @@ async fn add_larger_dataset() {
     let (response, code) = index.get_task(update_id).await;
     assert_eq!(code, 200);
     assert_eq!(response["status"], "succeeded");
-    assert_eq!(response["type"], "documentsAddition");
+    assert_eq!(response["type"], "documentAddition");
     assert_eq!(response["details"]["indexedDocuments"], 77);
     assert_eq!(response["details"]["receivedDocuments"], 77);
     let (response, code) = index
@@ -797,7 +797,7 @@ async fn update_larger_dataset() {
     index.wait_task(0).await;
     let (response, code) = index.get_task(0).await;
     assert_eq!(code, 200);
-    assert_eq!(response["type"], "documentsPartial");
+    assert_eq!(response["type"], "documentPartial");
     assert_eq!(response["details"]["indexedDocuments"], 77);
     let (response, code) = index
         .get_all_documents(GetAllDocumentsOptions {
@@ -1032,3 +1032,26 @@ async fn error_primary_key_inference() {
 
     assert_eq!(response["error"], expected_error);
 }
+
+#[actix_rt::test]
+async fn add_documents_with_primary_key_twice() {
+    let server = Server::new().await;
+    let index = server.index("test");
+
+    let documents = json!([
+        {
+            "title": "11",
+            "desc": "foobar"
+        }
+    ]);
+
+    index.add_documents(documents.clone(), Some("title")).await;
+    index.wait_task(0).await;
+    let (response, _code) = index.get_task(0).await;
+    assert_eq!(response["status"], "succeeded");
+
+    index.add_documents(documents, Some("title")).await;
+    index.wait_task(1).await;
+    let (response, _code) = index.get_task(1).await;
+    assert_eq!(response["status"], "succeeded");
+}

meilisearch-http/tests/tasks/mod.rs

@@ -117,13 +117,13 @@ async fn test_summarized_task_view() {
     assert_valid_summarized_task!(response, "settingsUpdate", "test");
 
     let (response, _) = index.update_documents(json!([{"id": 1}]), None).await;
-    assert_valid_summarized_task!(response, "documentsPartial", "test");
+    assert_valid_summarized_task!(response, "documentPartial", "test");
 
     let (response, _) = index.add_documents(json!([{"id": 1}]), None).await;
-    assert_valid_summarized_task!(response, "documentsAddition", "test");
+    assert_valid_summarized_task!(response, "documentAddition", "test");
 
     let (response, _) = index.delete_document(1).await;
-    assert_valid_summarized_task!(response, "documentsDeletion", "test");
+    assert_valid_summarized_task!(response, "documentDeletion", "test");
 
     let (response, _) = index.clear_all_documents().await;
     assert_valid_summarized_task!(response, "clearAll", "test");

meilisearch-lib/src/index/updates.rs

@@ -237,8 +237,10 @@ impl Index {
         let mut txn = self.write_txn()?;
 
         if let Some(primary_key) = primary_key {
+            if self.primary_key(&txn)?.is_none() {
                 self.update_primary_key_txn(&mut txn, primary_key)?;
             }
+        }
 
         let indexing_callback = |indexing_step| debug!("update: {:?}", indexing_step);