Merge #1746

1746: Do not commit transaction on failed updates r=irevoire a=Kerollmops

This PR fixes MeiliSearch that was always committing the transactions even when an update was invalid and the whole transaction should have been trashed. It was the source of a bug where an invalid update (with an invalid primary key) was creating an index with the specified primary key and should instead have failed and done nothing on the server.

Fixes #1735.

Co-authored-by: Kerollmops <clement@meilisearch.com>

meilisearch-http/tests/index/create_index.rs

@@ -1,5 +1,5 @@
 use crate::common::Server;
-use serde_json::Value;
+use serde_json::{json, Value};
 
 #[actix_rt::test]
 async fn create_index_no_primary_key() {
@@ -33,6 +33,22 @@ async fn create_index_with_primary_key() {
     assert_eq!(response.as_object().unwrap().len(), 5);
 }
 
+#[actix_rt::test]
+async fn create_index_with_invalid_primary_key() {
+    let document = json!([ { "id": 2, "title": "Pride and Prejudice" } ]);
+
+    let server = Server::new().await;
+    let index = server.index("movies");
+    let (_response, code) = index.add_documents(document, Some("title")).await;
+    assert_eq!(code, 202);
+
+    index.wait_update_id(0).await;
+
+    let (response, code) = index.get().await;
+    assert_eq!(code, 200);
+    assert_eq!(response["primaryKey"], Value::Null);
+}
+
 // TODO: partial test since we are testing error, amd error is not yet fully implemented in
 // transplant
 #[actix_rt::test]

meilisearch-lib/src/index/updates.rs

@@ -202,7 +202,9 @@ impl Index {
                     Ok(UpdateResult::DocumentDeletion { deleted })
                 }
             };
+            if result.is_ok() {
                 txn.commit()?;
+            }
             result
         })();
 

meilisearch-lib/src/index_controller/mod.rs

@@ -276,8 +276,6 @@ impl IndexController {
                     let index = self.index_resolver.create_index(name, None).await?;
                     let update_result =
                         UpdateMsg::update(&self.update_sender, index.uuid, update).await?;
-                    // ignore if index creation fails now, since it may already have been created
-
                     Ok(update_result)
                 } else {
                     Err(IndexResolverError::UnexistingIndex(name).into())