From 8ec0c4c91349779d390dfa571db860dffc066cbc Mon Sep 17 00:00:00 2001 From: many Date: Thu, 21 Oct 2021 14:42:01 +0200 Subject: [PATCH 1/9] Add bad_request error tests --- .../tests/documents/add_documents.rs | 626 ++++++++++++++---- .../tests/documents/delete_documents.rs | 11 +- .../tests/documents/get_documents.rs | 26 +- meilisearch-http/tests/index/create_index.rs | 60 +- meilisearch-http/tests/index/delete_index.rs | 12 +- meilisearch-http/tests/index/get_index.rs | 15 +- meilisearch-http/tests/index/stats.rs | 16 + meilisearch-http/tests/index/update_index.rs | 30 +- meilisearch-http/tests/search/errors.rs | 419 ++++++++++++ meilisearch-http/tests/updates/mod.rs | 40 +- 10 files changed, 1088 insertions(+), 167 deletions(-) diff --git a/meilisearch-http/tests/documents/add_documents.rs b/meilisearch-http/tests/documents/add_documents.rs index 15f81b5f3..308b41e10 100644 --- a/meilisearch-http/tests/documents/add_documents.rs +++ b/meilisearch-http/tests/documents/add_documents.rs @@ -49,53 +49,9 @@ async fn add_documents_test_json_content_types() { assert_eq!(response, json!({ "updateId": 1 })); } -/// no content type is still supposed to be accepted as json -#[actix_rt::test] -async fn add_documents_test_no_content_types() { - let document = json!([ - { - "id": 1, - "content": "Montagne des Pyrénées", - } - ]); - - let server = Server::new().await; - let app = test::init_service(create_app!( - &server.service.meilisearch, - true, - &server.service.options - )) - .await; - // post - let req = test::TestRequest::post() - .uri("/indexes/dog/documents") - .set_payload(document.to_string()) - .insert_header(("content-type", "application/json")) - .to_request(); - let res = test::call_service(&app, req).await; - let status_code = res.status(); - let body = test::read_body(res).await; - let response: Value = serde_json::from_slice(&body).unwrap_or_default(); - assert_eq!(status_code, 202); - assert_eq!(response, json!({ "updateId": 0 })); - - // put - let req = test::TestRequest::put() - .uri("/indexes/dog/documents") - .set_payload(document.to_string()) - .insert_header(("content-type", "application/json")) - .to_request(); - let res = test::call_service(&app, req).await; - let status_code = res.status(); - let body = test::read_body(res).await; - let response: Value = serde_json::from_slice(&body).unwrap_or_default(); - assert_eq!(status_code, 202); - assert_eq!(response, json!({ "updateId": 1 })); -} - /// any other content-type is must be refused #[actix_rt::test] -async fn add_documents_test_bad_content_types() { +async fn error_add_documents_test_bad_content_types() { let document = json!([ { "id": 1, @@ -127,6 +83,12 @@ async fn add_documents_test_bad_content_types() { r#"The Content-Type "text/plain" is invalid. Accepted values for the Content-Type header are: "application/json", "application/x-ndjson", "text/csv""# ) ); + assert_eq!(response["code"], "invalid_content_type"); + assert_eq!(response["type"], "invalid_request"); + assert_eq!( + response["link"], + "https://docs.meilisearch.com/errors#invalid_content_type" + ); // put let req = test::TestRequest::put() @@ -145,6 +107,244 @@ async fn add_documents_test_bad_content_types() { r#"The Content-Type "text/plain" is invalid. Accepted values for the Content-Type header are: "application/json", "application/x-ndjson", "text/csv""# ) ); + assert_eq!(response["code"], "invalid_content_type"); + assert_eq!(response["type"], "invalid_request"); + assert_eq!( + response["link"], + "https://docs.meilisearch.com/errors#invalid_content_type" + ); +} + +/// missing content-type must be refused +#[actix_rt::test] +async fn error_add_documents_test_no_content_type() { + let document = json!([ + { + "id": 1, + "content": "Leonberg", + } + ]); + + let server = Server::new().await; + let app = test::init_service(create_app!( + &server.service.meilisearch, + true, + &server.service.options + )) + .await; + // post + let req = test::TestRequest::post() + .uri("/indexes/dog/documents") + .set_payload(document.to_string()) + .to_request(); + let res = test::call_service(&app, req).await; + let status_code = res.status(); + let body = test::read_body(res).await; + let response: Value = serde_json::from_slice(&body).unwrap_or_default(); + assert_eq!(status_code, 415); + assert_eq!( + response["message"], + json!( + r#"A Content-Type header is missing. Accepted values for the Content-Type header are: "application/json", "application/x-ndjson", "text/csv""# + ) + ); + assert_eq!(response["code"], "missing_content_type"); + assert_eq!(response["type"], "invalid_request"); + assert_eq!( + response["link"], + "https://docs.meilisearch.com/errors#missing_content_type" + ); + + // put + let req = test::TestRequest::put() + .uri("/indexes/dog/documents") + .set_payload(document.to_string()) + .to_request(); + let res = test::call_service(&app, req).await; + let status_code = res.status(); + let body = test::read_body(res).await; + let response: Value = serde_json::from_slice(&body).unwrap_or_default(); + assert_eq!(status_code, 415); + assert_eq!( + response["message"], + json!( + r#"A Content-Type header is missing. Accepted values for the Content-Type header are: "application/json", "application/x-ndjson", "text/csv""# + ) + ); + assert_eq!(response["code"], "missing_content_type"); + assert_eq!(response["type"], "invalid_request"); + assert_eq!( + response["link"], + "https://docs.meilisearch.com/errors#missing_content_type" + ); +} + +#[actix_rt::test] +async fn error_add_malformed_csv_documents() { + let document = "id, content\n1234, hello, world\n12, hello world"; + + let server = Server::new().await; + let app = test::init_service(create_app!( + &server.service.meilisearch, + true, + &server.service.options + )) + .await; + // post + let req = test::TestRequest::post() + .uri("/indexes/dog/documents") + .set_payload(document.to_string()) + .insert_header(("content-type", "text/plain")) + .to_request(); + let res = test::call_service(&app, req).await; + let status_code = res.status(); + let body = test::read_body(res).await; + let response: Value = serde_json::from_slice(&body).unwrap_or_default(); + assert_eq!(status_code, 400); + assert_eq!( + response["message"], + json!(r#"The csv payload provided is malformed. :syntaxErrorHelper."#) + ); + assert_eq!(response["code"], "malformed_payload"); + assert_eq!(response["type"], "invalid_request"); + assert_eq!( + response["link"], + "https://docs.meilisearch.com/errors#malformed_payload" + ); + + // put + let req = test::TestRequest::put() + .uri("/indexes/dog/documents") + .set_payload(document.to_string()) + .insert_header(("content-type", "text/plain")) + .to_request(); + let res = test::call_service(&app, req).await; + let status_code = res.status(); + let body = test::read_body(res).await; + let response: Value = serde_json::from_slice(&body).unwrap_or_default(); + assert_eq!(status_code, 400); + assert_eq!( + response["message"], + json!(r#"The csv payload provided is malformed. :syntaxErrorHelper."#) + ); + assert_eq!(response["code"], "malformed_payload"); + assert_eq!(response["type"], "invalid_request"); + assert_eq!( + response["link"], + "https://docs.meilisearch.com/errors#malformed_payload" + ); +} + +#[actix_rt::test] +async fn error_add_malformed_json_documents() { + let document = r#"[{"id": 1}, {id: 2}]"#; + + let server = Server::new().await; + let app = test::init_service(create_app!( + &server.service.meilisearch, + true, + &server.service.options + )) + .await; + // post + let req = test::TestRequest::post() + .uri("/indexes/dog/documents") + .set_payload(document.to_string()) + .insert_header(("content-type", "text/plain")) + .to_request(); + let res = test::call_service(&app, req).await; + let status_code = res.status(); + let body = test::read_body(res).await; + let response: Value = serde_json::from_slice(&body).unwrap_or_default(); + assert_eq!(status_code, 400); + assert_eq!( + response["message"], + json!(r#"The json payload provided is malformed. :syntaxErrorHelper."#) + ); + assert_eq!(response["code"], "malformed_payload"); + assert_eq!(response["type"], "invalid_request"); + assert_eq!( + response["link"], + "https://docs.meilisearch.com/errors#malformed_payload" + ); + + // put + let req = test::TestRequest::put() + .uri("/indexes/dog/documents") + .set_payload(document.to_string()) + .insert_header(("content-type", "text/plain")) + .to_request(); + let res = test::call_service(&app, req).await; + let status_code = res.status(); + let body = test::read_body(res).await; + let response: Value = serde_json::from_slice(&body).unwrap_or_default(); + assert_eq!(status_code, 400); + assert_eq!( + response["message"], + json!(r#"The json payload provided is malformed. :syntaxErrorHelper."#) + ); + assert_eq!(response["code"], "malformed_payload"); + assert_eq!(response["type"], "invalid_request"); + assert_eq!( + response["link"], + "https://docs.meilisearch.com/errors#malformed_payload" + ); +} + +#[actix_rt::test] +async fn error_add_malformed_ndjson_documents() { + let document = "{\"id\": 1}\n{id: 2}"; + + let server = Server::new().await; + let app = test::init_service(create_app!( + &server.service.meilisearch, + true, + &server.service.options + )) + .await; + // post + let req = test::TestRequest::post() + .uri("/indexes/dog/documents") + .set_payload(document.to_string()) + .insert_header(("content-type", "text/plain")) + .to_request(); + let res = test::call_service(&app, req).await; + let status_code = res.status(); + let body = test::read_body(res).await; + let response: Value = serde_json::from_slice(&body).unwrap_or_default(); + assert_eq!(status_code, 400); + assert_eq!( + response["message"], + json!(r#"The ndjson payload provided is malformed. :syntaxErrorHelper."#) + ); + assert_eq!(response["code"], "malformed_payload"); + assert_eq!(response["type"], "invalid_request"); + assert_eq!( + response["link"], + "https://docs.meilisearch.com/errors#malformed_payload" + ); + + // put + let req = test::TestRequest::put() + .uri("/indexes/dog/documents") + .set_payload(document.to_string()) + .insert_header(("content-type", "text/plain")) + .to_request(); + let res = test::call_service(&app, req).await; + let status_code = res.status(); + let body = test::read_body(res).await; + let response: Value = serde_json::from_slice(&body).unwrap_or_default(); + assert_eq!(status_code, 400); + assert_eq!( + response["message"], + json!(r#"The ndjson payload provided is malformed. :syntaxErrorHelper."#) + ); + assert_eq!(response["code"], "malformed_payload"); + assert_eq!(response["type"], "invalid_request"); + assert_eq!( + response["link"], + "https://docs.meilisearch.com/errors#malformed_payload" + ); } #[actix_rt::test] @@ -193,19 +393,37 @@ async fn add_documents_no_index_creation() { } #[actix_rt::test] -async fn document_add_create_index_bad_uid() { +async fn error_document_add_create_index_bad_uid() { let server = Server::new().await; let index = server.index("883 fj!"); - let (_response, code) = index.add_documents(json!([]), None).await; + let (response, code) = index.add_documents(json!([]), None).await; + + let expected_response = json!({ + "message": "883 fj! is not a valid index uid. Index uid can be an integer or a string containing only alphanumeric characters, hyphens (-) and underscores (_).", + "code": "invalid_index_uid", + "type": "invalid_request", + "link": "https://docs.meilisearch.com/errors#invalid_index_uid" + }); + + assert_eq!(response, expected_response); assert_eq!(code, 400); } #[actix_rt::test] -async fn document_update_create_index_bad_uid() { +async fn error_document_update_create_index_bad_uid() { let server = Server::new().await; let index = server.index("883 fj!"); let (response, code) = index.update_documents(json!([]), None).await; - assert_eq!(code, 400, "{}", response); + + let expected_response = json!({ + "message": "883 fj! is not a valid index uid. Index uid can be an integer or a string containing only alphanumeric characters, hyphens (-) and underscores (_).", + "code": "invalid_index_uid", + "type": "invalid_request", + "link": "https://docs.meilisearch.com/errors#invalid_index_uid" + }); + + assert_eq!(response, expected_response); + assert_eq!(code, 400); } #[actix_rt::test] @@ -264,60 +482,6 @@ async fn document_update_with_primary_key() { assert_eq!(response["primaryKey"], "primary"); } -#[actix_rt::test] -async fn add_documents_with_primary_key_and_primary_key_already_exists() { - let server = Server::new().await; - let index = server.index("test"); - - index.create(Some("primary")).await; - let documents = json!([ - { - "id": 1, - "content": "foo", - } - ]); - - let (_response, code) = index.add_documents(documents, Some("id")).await; - assert_eq!(code, 202); - - index.wait_update_id(0).await; - - let (response, code) = index.get_update(0).await; - assert_eq!(code, 200); - assert_eq!(response["status"], "failed"); - - let (response, code) = index.get().await; - assert_eq!(code, 200); - assert_eq!(response["primaryKey"], "primary"); -} - -#[actix_rt::test] -async fn update_documents_with_primary_key_and_primary_key_already_exists() { - let server = Server::new().await; - let index = server.index("test"); - - index.create(Some("primary")).await; - let documents = json!([ - { - "id": 1, - "content": "foo", - } - ]); - - let (_response, code) = index.update_documents(documents, Some("id")).await; - assert_eq!(code, 202); - - index.wait_update_id(0).await; - let (response, code) = index.get_update(0).await; - assert_eq!(code, 200); - // Documents without a primary key are not accepted. - assert_eq!(response["status"], "failed"); - - let (response, code) = index.get().await; - assert_eq!(code, 200); - assert_eq!(response["primaryKey"], "primary"); -} - #[actix_rt::test] async fn replace_document() { let server = Server::new().await; @@ -356,25 +520,21 @@ async fn replace_document() { assert_eq!(response.to_string(), r##"{"doc_id":1,"other":"bar"}"##); } -// test broken, see issue milli#92 #[actix_rt::test] -#[ignore] -async fn add_no_documents() { +async fn error_add_no_documents() { let server = Server::new().await; let index = server.index("test"); - let (_response, code) = index.add_documents(json!([]), None).await; - assert_eq!(code, 200); + let (response, code) = index.add_documents(json!([]), None).await; - index.wait_update_id(0).await; - let (response, code) = index.get_update(0).await; - assert_eq!(code, 200); - assert_eq!(response["status"], "processed"); - assert_eq!(response["updateId"], 0); - assert_eq!(response["success"]["DocumentsAddition"]["nb_documents"], 0); + let expected_response = json!({ + "message": "A json payload is missing.", + "code": "missing_payload", + "type": "invalid_request", + "link": "https://docs.meilisearch.com/errors#missing_payload" + }); - let (response, code) = index.get().await; - assert_eq!(code, 200); - assert_eq!(response["primaryKey"], Value::Null); + assert_eq!(response, expected_response); + assert_eq!(code, 400); } #[actix_rt::test] @@ -460,7 +620,7 @@ async fn update_larger_dataset() { } #[actix_rt::test] -async fn add_documents_bad_primary_key() { +async fn error_add_documents_bad_document_id() { let server = Server::new().await; let index = server.index("test"); index.create(Some("docid")).await; @@ -475,10 +635,17 @@ async fn add_documents_bad_primary_key() { let (response, code) = index.get_update(0).await; assert_eq!(code, 200); assert_eq!(response["status"], "failed"); + assert_eq!(response["message"], "Document identifier foo & bar is invalid. A document identifier can be of type integer or string, only composed of alphanumeric characters (a-z A-Z 0-9), hyphens (-) and underscores (_)."); + assert_eq!(response["code"], "invalid_document_id"); + assert_eq!(response["type"], "invalid_request"); + assert_eq!( + response["link"], + "https://docs.meilisearch.com/errors#invalid_document_id" + ); } #[actix_rt::test] -async fn update_documents_bad_primary_key() { +async fn error_update_documents_bad_document_id() { let server = Server::new().await; let index = server.index("test"); index.create(Some("docid")).await; @@ -493,4 +660,225 @@ async fn update_documents_bad_primary_key() { let (response, code) = index.get_update(0).await; assert_eq!(code, 200); assert_eq!(response["status"], "failed"); + assert_eq!(response["message"], "Document identifier foo & bar is invalid. A document identifier can be of type integer or string, only composed of alphanumeric characters (a-z A-Z 0-9), hyphens (-) and underscores (_)."); + assert_eq!(response["code"], "invalid_document_id"); + assert_eq!(response["type"], "invalid_request"); + assert_eq!( + response["link"], + "https://docs.meilisearch.com/errors#invalid_document_id" + ); +} + +#[actix_rt::test] +async fn error_add_documents_missing_document_id() { + let server = Server::new().await; + let index = server.index("test"); + index.create(Some("docid")).await; + let documents = json!([ + { + "id": "11", + "content": "foobar" + } + ]); + index.add_documents(documents, None).await; + index.wait_update_id(0).await; + let (response, code) = index.get_update(0).await; + assert_eq!(code, 200); + assert_eq!(response["status"], "failed"); + assert_eq!( + response["message"], + r#"Document doesn't have a docid attribute: {"id":"11","content":"foobar"}."# + ); + assert_eq!(response["code"], "missing_document_id"); + assert_eq!(response["type"], "invalid_request"); + assert_eq!( + response["link"], + "https://docs.meilisearch.com/errors#missing_document_id" + ); +} + +#[actix_rt::test] +async fn error_update_documents_missing_document_id() { + let server = Server::new().await; + let index = server.index("test"); + index.create(Some("docid")).await; + let documents = json!([ + { + "id": "11", + "content": "foobar" + } + ]); + index.update_documents(documents, None).await; + index.wait_update_id(0).await; + let (response, code) = index.get_update(0).await; + assert_eq!(code, 200); + assert_eq!(response["status"], "failed"); + assert_eq!( + response["message"], + r#"Document doesn't have a docid attribute: {"id":"11","content":"foobar"}."# + ); + assert_eq!(response["code"], "missing_document_id"); + assert_eq!(response["type"], "invalid_request"); + assert_eq!( + response["link"], + "https://docs.meilisearch.com/errors#missing_document_id" + ); +} + +#[actix_rt::test] +async fn error_add_documents_with_primary_key_and_primary_key_already_exists() { + let server = Server::new().await; + let index = server.index("test"); + + index.create(Some("primary")).await; + let documents = json!([ + { + "id": 1, + "content": "foo", + } + ]); + + let (_response, code) = index.add_documents(documents, Some("id")).await; + assert_eq!(code, 202); + + index.wait_update_id(0).await; + + let (response, code) = index.get_update(0).await; + assert_eq!(code, 200); + assert_eq!(response["status"], "failed"); + assert_eq!(response["message"], "Index test already has a primary key."); + assert_eq!(response["code"], "index_primary_key_already_exists"); + assert_eq!(response["type"], "invalid_request"); + assert_eq!( + response["link"], + "https://docs.meilisearch.com/errors#index_primary_key_already_exists" + ); + + let (response, code) = index.get().await; + assert_eq!(code, 200); + assert_eq!(response["primaryKey"], "primary"); +} + +#[actix_rt::test] +async fn error_update_documents_with_primary_key_and_primary_key_already_exists() { + let server = Server::new().await; + let index = server.index("test"); + + index.create(Some("primary")).await; + let documents = json!([ + { + "id": 1, + "content": "foo", + } + ]); + + let (_response, code) = index.update_documents(documents, Some("id")).await; + assert_eq!(code, 202); + + index.wait_update_id(0).await; + let (response, code) = index.get_update(0).await; + assert_eq!(code, 200); + // Documents without a primary key are not accepted. + assert_eq!(response["status"], "failed"); + assert_eq!(response["message"], "Index test already has a primary key."); + assert_eq!(response["code"], "index_primary_key_already_exists"); + assert_eq!(response["type"], "invalid_request"); + assert_eq!( + response["link"], + "https://docs.meilisearch.com/errors#index_primary_key_already_exists" + ); + + let (response, code) = index.get().await; + assert_eq!(code, 200); + assert_eq!(response["primaryKey"], "primary"); +} + +#[actix_rt::test] +async fn error_document_field_limit_reached() { + let server = Server::new().await; + let index = server.index("test"); + + index.create(Some("id")).await; + + let mut big_object = std::collections::HashMap::new(); + big_object.insert("id".to_owned(), "wow"); + for i in 0..65535 { + let key = i.to_string(); + big_object.insert(key, "I am a text!"); + } + + let documents = json!([big_object]); + + let (_response, code) = index.update_documents(documents, Some("id")).await; + assert_eq!(code, 202); + + index.wait_update_id(0).await; + let (response, code) = index.get_update(0).await; + assert_eq!(code, 200); + // Documents without a primary key are not accepted. + assert_eq!(response["status"], "failed"); + assert_eq!( + response["message"], + "A document cannot contain more than 65,535 fields." + ); + assert_eq!(response["code"], "document_fields_limit_reached"); + assert_eq!(response["type"], "invalid_request"); + assert_eq!( + response["link"], + "https://docs.meilisearch.com/errors#document_fields_limit_reached" + ); +} + +#[actix_rt::test] +async fn error_add_documents_invalid_geo_field() { + let server = Server::new().await; + let index = server.index("test"); + index.create(Some("id")).await; + let documents = json!([ + { + "id": "11", + "_geo": "foobar" + } + ]); + index.add_documents(documents, None).await; + index.wait_update_id(0).await; + let (response, code) = index.get_update(0).await; + assert_eq!(code, 200); + assert_eq!(response["status"], "failed"); + assert_eq!( + response["message"], + r#"The document with the id: `11` contains an invalid _geo field: :syntaxErrorHelper:REPLACE_ME."# + ); + assert_eq!(response["code"], "invalid_geo_field"); + assert_eq!(response["type"], "invalid_request"); + assert_eq!( + response["link"], + "https://docs.meilisearch.com/errors#invalid_geo_field" + ); +} + +#[actix_rt::test] +async fn error_add_documents_payload_size() { + let server = Server::new().await; + let index = server.index("test"); + index.create(Some("id")).await; + let document = json!( + { + "id": "11", + "content": "Lorem ipsum dolor sit amet, consectetur adipiscing elit. Donec metus erat, consequat in blandit venenatis, ultricies eu ipsum. Etiam luctus elit et mollis ultrices. Nam turpis risus, dictum non eros in, eleifend feugiat elit. Morbi non dolor pulvinar, sagittis mi sed, ultricies lorem. Nulla ultricies sem metus. Donec at suscipit quam, sed elementum mi. Suspendisse potenti. Fusce pharetra turpis tortor, sed eleifend odio dapibus ut. Nulla facilisi. Suspendisse elementum, dui eget aliquet dignissim, ex tellus aliquam nisl, at eleifend nisl metus tempus diam. Mauris fermentum sollicitudin efficitur. Donec dignissim est vitae elit finibus faucibus" + } + ); + let documents: Vec<_> = (0..16000).into_iter().map(|_| document.clone()).collect(); + let documents = json!(documents); + let (response, code) = index.add_documents(documents, None).await; + + let expected_response = json!({ + "message": "The payload cannot exceed 10MB.", + "code": "payload_too_large", + "type": "invalid_request", + "link": "https://docs.meilisearch.com/errors#payload_too_large" + }); + + assert_eq!(response, expected_response); + assert_eq!(code, 413); } diff --git a/meilisearch-http/tests/documents/delete_documents.rs b/meilisearch-http/tests/documents/delete_documents.rs index eb6fa040b..81125e580 100644 --- a/meilisearch-http/tests/documents/delete_documents.rs +++ b/meilisearch-http/tests/documents/delete_documents.rs @@ -83,10 +83,17 @@ async fn clear_all_documents_empty_index() { } #[actix_rt::test] -async fn delete_batch_unexisting_index() { +async fn error_delete_batch_unexisting_index() { let server = Server::new().await; let (response, code) = server.index("test").delete_batch(vec![]).await; - assert_eq!(code, 404, "{}", response); + let expected_response = json!({ + "message": "Index test not found.", + "code": "index_not_found", + "type": "invalid_request", + "link": "https://docs.meilisearch.com/errors#index_not_found" + }); + assert_eq!(code, 404); + assert_eq!(response, expected_response); } #[actix_rt::test] diff --git a/meilisearch-http/tests/documents/get_documents.rs b/meilisearch-http/tests/documents/get_documents.rs index 14344db35..11f43f2c4 100644 --- a/meilisearch-http/tests/documents/get_documents.rs +++ b/meilisearch-http/tests/documents/get_documents.rs @@ -13,11 +13,20 @@ async fn get_unexisting_index_single_document() { } #[actix_rt::test] -async fn get_unexisting_document() { +async fn error_get_unexisting_document() { let server = Server::new().await; let index = server.index("test"); index.create(None).await; - let (_response, code) = index.get_document(1, None).await; + let (response, code) = index.get_document(1, None).await; + + let expected_response = json!({ + "message": "Document 1 not found.", + "code": "document_not_found", + "type": "invalid_request", + "link": "https://docs.meilisearch.com/errors#document_not_found" + }); + + assert_eq!(response, expected_response); assert_eq!(code, 404); } @@ -47,12 +56,21 @@ async fn get_document() { } #[actix_rt::test] -async fn get_unexisting_index_all_documents() { +async fn error_get_unexisting_index_all_documents() { let server = Server::new().await; - let (_response, code) = server + let (response, code) = server .index("test") .get_all_documents(GetAllDocumentsOptions::default()) .await; + + let expected_response = json!({ + "message": "Index test not found.", + "code": "index_not_found", + "type": "invalid_request", + "link": "https://docs.meilisearch.com/errors#index_not_found" + }); + + assert_eq!(response, expected_response); assert_eq!(code, 404); } diff --git a/meilisearch-http/tests/index/create_index.rs b/meilisearch-http/tests/index/create_index.rs index 4813847ff..e5cfa2fa5 100644 --- a/meilisearch-http/tests/index/create_index.rs +++ b/meilisearch-http/tests/index/create_index.rs @@ -49,28 +49,6 @@ async fn create_index_with_invalid_primary_key() { 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] -async fn create_existing_index() { - let server = Server::new().await; - let index = server.index("test"); - let (_, code) = index.create(Some("primary")).await; - - assert_eq!(code, 201); - - let (_response, code) = index.create(Some("primary")).await; - assert_eq!(code, 400); -} - -#[actix_rt::test] -async fn create_with_invalid_index_uid() { - let server = Server::new().await; - let index = server.index("test test#!"); - let (_, code) = index.create(None).await; - assert_eq!(code, 400); -} - #[actix_rt::test] async fn test_create_multiple_indexes() { let server = Server::new().await; @@ -88,3 +66,41 @@ async fn test_create_multiple_indexes() { assert_eq!(index3.get().await.1, 200); assert_eq!(index4.get().await.1, 404); } + +#[actix_rt::test] +async fn error_create_existing_index() { + let server = Server::new().await; + let index = server.index("test"); + let (_, code) = index.create(Some("primary")).await; + + assert_eq!(code, 201); + + let (response, code) = index.create(Some("primary")).await; + + let expected_response = json!({ + "message": "Index primary already exists.", + "code": "index_already_exists", + "type": "invalid_request", + "link":"https://docs.meilisearch.com/errors#index_already_exists" + }); + + assert_eq!(response, expected_response); + assert_eq!(code, 409); +} + +#[actix_rt::test] +async fn error_create_with_invalid_index_uid() { + let server = Server::new().await; + let index = server.index("test test#!"); + let (response, code) = index.create(None).await; + + let expected_response = json!({ + "message": "test test#! is not a valid index uid. Index uid can be an integer or a string containing only alphanumeric characters, hyphens (-) and underscores (_).", + "code": "invalid_index_uid", + "type": "invalid_request", + "link": "https://docs.meilisearch.com/errors#invalid_index_uid" + }); + + assert_eq!(response, expected_response); + assert_eq!(code, 400); +} diff --git a/meilisearch-http/tests/index/delete_index.rs b/meilisearch-http/tests/index/delete_index.rs index 10a6c0282..84c780650 100644 --- a/meilisearch-http/tests/index/delete_index.rs +++ b/meilisearch-http/tests/index/delete_index.rs @@ -18,11 +18,19 @@ async fn create_and_delete_index() { } #[actix_rt::test] -async fn delete_unexisting_index() { +async fn error_delete_unexisting_index() { let server = Server::new().await; let index = server.index("test"); - let (_response, code) = index.delete().await; + let (response, code) = index.delete().await; + let expected_response = json!({ + "message": "Index test not found.", + "code": "index_not_found", + "type": "invalid_request", + "link": "https://docs.meilisearch.com/errors#index_not_found" + }); + + assert_eq!(response, expected_response); assert_eq!(code, 404); } diff --git a/meilisearch-http/tests/index/get_index.rs b/meilisearch-http/tests/index/get_index.rs index ba26a8e3a..ec35ea283 100644 --- a/meilisearch-http/tests/index/get_index.rs +++ b/meilisearch-http/tests/index/get_index.rs @@ -1,4 +1,5 @@ use crate::common::Server; +use serde_json::json; use serde_json::Value; #[actix_rt::test] @@ -21,15 +22,21 @@ async fn create_and_get_index() { assert_eq!(response.as_object().unwrap().len(), 5); } -// TODO: partial test since we are testing error, and error is not yet fully implemented in -// transplant #[actix_rt::test] -async fn get_unexisting_index() { +async fn error_get_unexisting_index() { let server = Server::new().await; let index = server.index("test"); - let (_response, code) = index.get().await; + let (response, code) = index.get().await; + let expected_response = json!({ + "message": "Index test not found.", + "code": "index_not_found", + "type": "invalid_request", + "link": "https://docs.meilisearch.com/errors#index_not_found" + }); + + assert_eq!(response, expected_response); assert_eq!(code, 404); } diff --git a/meilisearch-http/tests/index/stats.rs b/meilisearch-http/tests/index/stats.rs index 3599e1605..7abbd0cd4 100644 --- a/meilisearch-http/tests/index/stats.rs +++ b/meilisearch-http/tests/index/stats.rs @@ -46,3 +46,19 @@ async fn stats() { assert_eq!(response["fieldDistribution"]["name"], 1); assert_eq!(response["fieldDistribution"]["age"], 1); } + +#[actix_rt::test] +async fn error_get_stats_unexisting_index() { + let server = Server::new().await; + let (response, code) = server.index("test").stats().await; + + let expected_response = json!({ + "message": "Index test not found.", + "code": "index_not_found", + "type": "invalid_request", + "link": "https://docs.meilisearch.com/errors#index_not_found" + }); + + assert_eq!(response, expected_response); + assert_eq!(code, 404); +} diff --git a/meilisearch-http/tests/index/update_index.rs b/meilisearch-http/tests/index/update_index.rs index 454b55fce..5d92a0a16 100644 --- a/meilisearch-http/tests/index/update_index.rs +++ b/meilisearch-http/tests/index/update_index.rs @@ -1,5 +1,6 @@ use crate::common::Server; use chrono::DateTime; +use serde_json::json; #[actix_rt::test] async fn update_primary_key() { @@ -39,26 +40,39 @@ async fn update_nothing() { assert_eq!(response, update); } -// TODO: partial test since we are testing error, amd error is not yet fully implemented in -// transplant #[actix_rt::test] -async fn update_existing_primary_key() { +async fn error_update_existing_primary_key() { let server = Server::new().await; let index = server.index("test"); let (_response, code) = index.create(Some("primary")).await; assert_eq!(code, 201); - let (_update, code) = index.update(Some("primary2")).await; + let (response, code) = index.update(Some("primary2")).await; + let expected_response = json!({ + "message": "Index test already has a primary key.", + "code": "index_primary_key_already_exists", + "type": "invalid_request", + "link": "https://docs.meilisearch.com/errors#index_primary_key_already_exists" + }); + + assert_eq!(response, expected_response); assert_eq!(code, 400); } -// TODO: partial test since we are testing error, amd error is not yet fully implemented in -// transplant #[actix_rt::test] -async fn test_unexisting_index() { +async fn error_update_unexisting_index() { let server = Server::new().await; - let (_response, code) = server.index("test").update(None).await; + let (response, code) = server.index("test").update(None).await; + + let expected_response = json!({ + "message": "Index test not found.", + "code": "index_not_found", + "type": "invalid_request", + "link": "https://docs.meilisearch.com/errors#index_not_found" + }); + + assert_eq!(response, expected_response); assert_eq!(code, 404); } diff --git a/meilisearch-http/tests/search/errors.rs b/meilisearch-http/tests/search/errors.rs index a09e8f76e..d7e4b342e 100644 --- a/meilisearch-http/tests/search/errors.rs +++ b/meilisearch-http/tests/search/errors.rs @@ -1,6 +1,8 @@ use crate::common::Server; use serde_json::json; +use super::DOCUMENTS; + #[actix_rt::test] async fn search_unexisting_index() { let server = Server::new().await; @@ -26,3 +28,420 @@ async fn search_unexisting_parameter() { }) .await; } + +#[actix_rt::test] +async fn filter_invalid_syntax_object() { + let server = Server::new().await; + let index = server.index("test"); + + index + .update_settings(json!({"filterableAttributes": ["title"]})) + .await; + + let documents = DOCUMENTS.clone(); + index.add_documents(documents, None).await; + index.wait_update_id(1).await; + + let expected_response = json!({ + "message": "Invalid syntax for the filter parameter: :syntaxErrorHelper:REPLACE_ME.", + "code": "invalid_filter", + "type": "invalid_request", + "link": "https://docs.meilisearch.com/errors#invalid_filter" + }); + index + .search(json!({"filter": {"title": "Glass"}}), |response, code| { + assert_eq!(response, expected_response); + assert_eq!(code, 400); + }) + .await; +} + +#[actix_rt::test] +async fn filter_invalid_syntax_array() { + let server = Server::new().await; + let index = server.index("test"); + + index + .update_settings(json!({"filterableAttributes": ["title"]})) + .await; + + let documents = DOCUMENTS.clone(); + index.add_documents(documents, None).await; + index.wait_update_id(1).await; + + let expected_response = json!({ + "message": "Invalid syntax for the filter parameter: :syntaxErrorHelper:REPLACE_ME.", + "code": "invalid_filter", + "type": "invalid_request", + "link": "https://docs.meilisearch.com/errors#invalid_filter" + }); + index + .search( + json!({"filter": [[["title = Glass"]]]}), + |response, code| { + assert_eq!(response, expected_response); + assert_eq!(code, 400); + }, + ) + .await; +} + +#[actix_rt::test] +async fn filter_invalid_syntax_string() { + let server = Server::new().await; + let index = server.index("test"); + + index + .update_settings(json!({"filterableAttributes": ["title"]})) + .await; + + let documents = DOCUMENTS.clone(); + index.add_documents(documents, None).await; + index.wait_update_id(1).await; + + let expected_response = json!({ + "message": "Invalid syntax for the filter parameter: :syntaxErrorHelper:REPLACE_ME.", + "code": "invalid_filter", + "type": "invalid_request", + "link": "https://docs.meilisearch.com/errors#invalid_filter" + }); + index + .search( + json!({"filter": "title = Glass XOR title = Glass"}), + |response, code| { + assert_eq!(response, expected_response); + assert_eq!(code, 400); + }, + ) + .await; +} + +#[actix_rt::test] +async fn filter_invalid_attribute_array() { + let server = Server::new().await; + let index = server.index("test"); + + index + .update_settings(json!({"filterableAttributes": ["title"]})) + .await; + + let documents = DOCUMENTS.clone(); + index.add_documents(documents, None).await; + index.wait_update_id(1).await; + + let expected_response = json!({ + "message": "Attribute many is not filterable. Available filterable attributes are: title.", + "code": "invalid_filter", + "type": "invalid_request", + "link": "https://docs.meilisearch.com/errors#invalid_filter" + }); + index + .search(json!({"filter": [["many = Glass"]]}), |response, code| { + assert_eq!(response, expected_response); + assert_eq!(code, 400); + }) + .await; +} + +#[actix_rt::test] +async fn filter_invalid_attribute_string() { + let server = Server::new().await; + let index = server.index("test"); + + index + .update_settings(json!({"filterableAttributes": ["title"]})) + .await; + + let documents = DOCUMENTS.clone(); + index.add_documents(documents, None).await; + index.wait_update_id(1).await; + + let expected_response = json!({ + "message": "Attribute many is not filterable. Available filterable attributes are: title.", + "code": "invalid_filter", + "type": "invalid_request", + "link": "https://docs.meilisearch.com/errors#invalid_filter" + }); + index + .search(json!({"filter": "many = Glass"}), |response, code| { + assert_eq!(response, expected_response); + assert_eq!(code, 400); + }) + .await; +} + +#[actix_rt::test] +async fn filter_reserved_geo_attribute_array() { + let server = Server::new().await; + let index = server.index("test"); + + index + .update_settings(json!({"filterableAttributes": ["title"]})) + .await; + + let documents = DOCUMENTS.clone(); + index.add_documents(documents, None).await; + index.wait_update_id(1).await; + + let expected_response = json!({ + "message": "_geo is a reserved keyword and thus can't be used as a filter expression. Use the _geoRadius(latitude, longitude, distance) built-in rule to filter on _geo field coordinates.", + "code": "invalid_filter", + "type": "invalid_request", + "link": "https://docs.meilisearch.com/errors#invalid_filter" + }); + index + .search(json!({"filter": [["_geo = Glass"]]}), |response, code| { + assert_eq!(response, expected_response); + assert_eq!(code, 400); + }) + .await; +} + +#[actix_rt::test] +async fn filter_reserved_geo_attribute_string() { + let server = Server::new().await; + let index = server.index("test"); + + index + .update_settings(json!({"filterableAttributes": ["title"]})) + .await; + + let documents = DOCUMENTS.clone(); + index.add_documents(documents, None).await; + index.wait_update_id(1).await; + + let expected_response = json!({ + "message": "_geo is a reserved keyword and thus can't be used as a filter expression. Use the _geoRadius(latitude, longitude, distance) built-in rule to filter on _geo field coordinates.", + "code": "invalid_filter", + "type": "invalid_request", + "link": "https://docs.meilisearch.com/errors#invalid_filter" + }); + index + .search(json!({"filter": "_geo = Glass"}), |response, code| { + assert_eq!(response, expected_response); + assert_eq!(code, 400); + }) + .await; +} + +#[actix_rt::test] +async fn filter_reserved_attribute_array() { + let server = Server::new().await; + let index = server.index("test"); + + index + .update_settings(json!({"filterableAttributes": ["title"]})) + .await; + + let documents = DOCUMENTS.clone(); + index.add_documents(documents, None).await; + index.wait_update_id(1).await; + + let expected_response = json!({ + "message": "_geoDistance is a reserved keyword and thus can't be used as a filter expression.", + "code": "invalid_filter", + "type": "invalid_request", + "link": "https://docs.meilisearch.com/errors#invalid_filter" + }); + index + .search( + json!({"filter": [["_geoDistance = Glass"]]}), + |response, code| { + assert_eq!(response, expected_response); + assert_eq!(code, 400); + }, + ) + .await; +} + +#[actix_rt::test] +async fn filter_reserved_attribute_string() { + let server = Server::new().await; + let index = server.index("test"); + + index + .update_settings(json!({"filterableAttributes": ["title"]})) + .await; + + let documents = DOCUMENTS.clone(); + index.add_documents(documents, None).await; + index.wait_update_id(1).await; + + let expected_response = json!({ + "message": "_geoDistance is a reserved keyword and thus can't be used as a filter expression.", + "code": "invalid_filter", + "type": "invalid_request", + "link": "https://docs.meilisearch.com/errors#invalid_filter" + }); + index + .search( + json!({"filter": "_geoDistance = Glass"}), + |response, code| { + assert_eq!(response, expected_response); + assert_eq!(code, 400); + }, + ) + .await; +} + +#[actix_rt::test] +async fn sort_geo_reserved_attribute() { + let server = Server::new().await; + let index = server.index("test"); + + index + .update_settings(json!({"sortableAttributes": ["id"]})) + .await; + + let documents = DOCUMENTS.clone(); + index.add_documents(documents, None).await; + index.wait_update_id(1).await; + + let expected_response = json!({ + "message": "_geo is a reserved keyword and thus can't be used as a filter expression. Use the _geoPoint(latitude, longitude) built-in rule to sort on _geo field coordinates.", + "code": "invalid_sort", + "type": "invalid_request", + "link": "https://docs.meilisearch.com/errors#invalid_sort" + }); + index + .search( + json!({ + "sort": ["_geo:asc"] + }), + |response, code| { + assert_eq!(response, expected_response); + assert_eq!(code, 400); + }, + ) + .await; +} + +#[actix_rt::test] +async fn sort_reserved_attribute() { + let server = Server::new().await; + let index = server.index("test"); + + index + .update_settings(json!({"sortableAttributes": ["id"]})) + .await; + + let documents = DOCUMENTS.clone(); + index.add_documents(documents, None).await; + index.wait_update_id(1).await; + + let expected_response = json!({ + "message": "_geoDistance is a reserved keyword and thus can't be used as a filter expression.", + "code": "invalid_sort", + "type": "invalid_request", + "link": "https://docs.meilisearch.com/errors#invalid_sort" + }); + index + .search( + json!({ + "sort": ["_geoDistance:asc"] + }), + |response, code| { + assert_eq!(response, expected_response); + assert_eq!(code, 400); + }, + ) + .await; +} + +#[actix_rt::test] +async fn sort_unsortable_attribute() { + let server = Server::new().await; + let index = server.index("test"); + + index + .update_settings(json!({"sortableAttributes": ["id"]})) + .await; + + let documents = DOCUMENTS.clone(); + index.add_documents(documents, None).await; + index.wait_update_id(1).await; + + let expected_response = json!({ + "message": "Attribute title is not sortable. Available sortable attributes are: id.", + "code": "invalid_sort", + "type": "invalid_request", + "link": "https://docs.meilisearch.com/errors#invalid_sort" + }); + index + .search( + json!({ + "sort": ["title:asc"] + }), + |response, code| { + assert_eq!(response, expected_response); + assert_eq!(code, 400); + }, + ) + .await; +} + +#[actix_rt::test] +async fn sort_invalid_syntax() { + let server = Server::new().await; + let index = server.index("test"); + + index + .update_settings(json!({"sortableAttributes": ["id"]})) + .await; + + let documents = DOCUMENTS.clone(); + index.add_documents(documents, None).await; + index.wait_update_id(1).await; + + let expected_response = json!({ + "message": "Invalid syntax for the sort parameter: :syntaxErrorHelper:REPLACE_ME.", + "code": "invalid_sort", + "type": "invalid_request", + "link": "https://docs.meilisearch.com/errors#invalid_sort" + }); + index + .search( + json!({ + "sort": ["title"] + }), + |response, code| { + assert_eq!(response, expected_response); + assert_eq!(code, 400); + }, + ) + .await; +} + +#[actix_rt::test] +async fn sort_unset_ranking_rule() { + let server = Server::new().await; + let index = server.index("test"); + + index + .update_settings( + json!({"sortableAttributes": ["id"], "rankingRules": ["proximity", "exactness"]}), + ) + .await; + + let documents = DOCUMENTS.clone(); + index.add_documents(documents, None).await; + index.wait_update_id(1).await; + + let expected_response = json!({ + "message": "The sort ranking rule must be specified in the ranking rules settings to use the sort parameter at search time.", + "code": "invalid_sort", + "type": "invalid_request", + "link": "https://docs.meilisearch.com/errors#invalid_sort" + }); + index + .search( + json!({ + "sort": ["title"] + }), + |response, code| { + assert_eq!(response, expected_response); + assert_eq!(code, 400); + }, + ) + .await; +} diff --git a/meilisearch-http/tests/updates/mod.rs b/meilisearch-http/tests/updates/mod.rs index 00bbf32a8..c4ab1a275 100644 --- a/meilisearch-http/tests/updates/mod.rs +++ b/meilisearch-http/tests/updates/mod.rs @@ -1,18 +1,37 @@ use crate::common::Server; +use serde_json::json; #[actix_rt::test] -async fn get_update_unexisting_index() { +async fn error_get_update_unexisting_index() { let server = Server::new().await; - let (_response, code) = server.index("test").get_update(0).await; + let (response, code) = server.index("test").get_update(0).await; + + let expected_response = json!({ + "message": "Index test not found.", + "code": "index_not_found", + "type": "invalid_request", + "link": "https://docs.meilisearch.com/errors#index_not_found" + }); + + assert_eq!(response, expected_response); assert_eq!(code, 404); } #[actix_rt::test] -async fn get_unexisting_update_status() { +async fn error_get_unexisting_update_status() { let server = Server::new().await; let index = server.index("test"); index.create(None).await; - let (_response, code) = index.get_update(0).await; + let (response, code) = index.get_update(0).await; + + let expected_response = json!({ + "message": "Task 0 not found.", + "code": "task_not_found", + "type": "invalid_request", + "link": "https://docs.meilisearch.com/errors#task_not_found" + }); + + assert_eq!(response, expected_response); assert_eq!(code, 404); } @@ -36,9 +55,18 @@ async fn get_update_status() { } #[actix_rt::test] -async fn list_updates_unexisting_index() { +async fn error_list_updates_unexisting_index() { let server = Server::new().await; - let (_response, code) = server.index("test").list_updates().await; + let (response, code) = server.index("test").list_updates().await; + + let expected_response = json!({ + "message": "Index test not found.", + "code": "index_not_found", + "type": "invalid_request", + "link": "https://docs.meilisearch.com/errors#index_not_found" + }); + + assert_eq!(response, expected_response); assert_eq!(code, 404); } From 61c15b69fb6dfd35676dd4008d29f27e9d2db35e Mon Sep 17 00:00:00 2001 From: many Date: Mon, 25 Oct 2021 14:09:24 +0200 Subject: [PATCH 2/9] Change malformed_payload error --- meilisearch-error/src/lib.rs | 6 +- meilisearch-http/src/error.rs | 3 + .../tests/documents/add_documents.rs | 72 +++++++++++-------- meilisearch-lib/src/document_formats.rs | 2 +- 4 files changed, 49 insertions(+), 34 deletions(-) diff --git a/meilisearch-error/src/lib.rs b/meilisearch-error/src/lib.rs index 41a58b9f5..3a3e2229a 100644 --- a/meilisearch-error/src/lib.rs +++ b/meilisearch-error/src/lib.rs @@ -39,9 +39,9 @@ impl fmt::Display for ErrorType { use ErrorType::*; match self { - InternalError => write!(f, "internal_error"), - InvalidRequestError => write!(f, "invalid_request_error"), - AuthenticationError => write!(f, "authentication_error"), + InternalError => write!(f, "internal"), + InvalidRequestError => write!(f, "invalid_request"), + AuthenticationError => write!(f, "authentication"), } } } diff --git a/meilisearch-http/src/error.rs b/meilisearch-http/src/error.rs index 1f2f4742d..c3c8d5785 100644 --- a/meilisearch-http/src/error.rs +++ b/meilisearch-http/src/error.rs @@ -42,8 +42,11 @@ pub struct ResponseError { #[serde(skip)] code: StatusCode, message: String, + #[serde(rename = "code")] error_code: String, + #[serde(rename = "type")] error_type: String, + #[serde(rename = "link")] error_link: String, } diff --git a/meilisearch-http/tests/documents/add_documents.rs b/meilisearch-http/tests/documents/add_documents.rs index 308b41e10..14f5c6dee 100644 --- a/meilisearch-http/tests/documents/add_documents.rs +++ b/meilisearch-http/tests/documents/add_documents.rs @@ -194,7 +194,7 @@ async fn error_add_malformed_csv_documents() { let req = test::TestRequest::post() .uri("/indexes/dog/documents") .set_payload(document.to_string()) - .insert_header(("content-type", "text/plain")) + .insert_header(("content-type", "text/csv")) .to_request(); let res = test::call_service(&app, req).await; let status_code = res.status(); @@ -203,20 +203,22 @@ async fn error_add_malformed_csv_documents() { assert_eq!(status_code, 400); assert_eq!( response["message"], - json!(r#"The csv payload provided is malformed. :syntaxErrorHelper."#) + json!( + r#"The csv payload provided is malformed. CSV error: record 1 (line: 2, byte: 12): found record with 3 fields, but the previous record has 2 fields."# + ) ); - assert_eq!(response["code"], "malformed_payload"); - assert_eq!(response["type"], "invalid_request"); + assert_eq!(response["code"], json!("malformed_payload")); + assert_eq!(response["type"], json!("invalid_request")); assert_eq!( response["link"], - "https://docs.meilisearch.com/errors#malformed_payload" + json!("https://docs.meilisearch.com/errors#malformed_payload") ); // put let req = test::TestRequest::put() .uri("/indexes/dog/documents") .set_payload(document.to_string()) - .insert_header(("content-type", "text/plain")) + .insert_header(("content-type", "text/csv")) .to_request(); let res = test::call_service(&app, req).await; let status_code = res.status(); @@ -225,13 +227,15 @@ async fn error_add_malformed_csv_documents() { assert_eq!(status_code, 400); assert_eq!( response["message"], - json!(r#"The csv payload provided is malformed. :syntaxErrorHelper."#) + json!( + r#"The csv payload provided is malformed. CSV error: record 1 (line: 2, byte: 12): found record with 3 fields, but the previous record has 2 fields."# + ) ); - assert_eq!(response["code"], "malformed_payload"); - assert_eq!(response["type"], "invalid_request"); + assert_eq!(response["code"], json!("malformed_payload")); + assert_eq!(response["type"], json!("invalid_request")); assert_eq!( response["link"], - "https://docs.meilisearch.com/errors#malformed_payload" + json!("https://docs.meilisearch.com/errors#malformed_payload") ); } @@ -250,7 +254,7 @@ async fn error_add_malformed_json_documents() { let req = test::TestRequest::post() .uri("/indexes/dog/documents") .set_payload(document.to_string()) - .insert_header(("content-type", "text/plain")) + .insert_header(("content-type", "application/json")) .to_request(); let res = test::call_service(&app, req).await; let status_code = res.status(); @@ -259,20 +263,22 @@ async fn error_add_malformed_json_documents() { assert_eq!(status_code, 400); assert_eq!( response["message"], - json!(r#"The json payload provided is malformed. :syntaxErrorHelper."#) + json!( + r#"The json payload provided is malformed. key must be a string at line 1 column 14."# + ) ); - assert_eq!(response["code"], "malformed_payload"); - assert_eq!(response["type"], "invalid_request"); + assert_eq!(response["code"], json!("malformed_payload")); + assert_eq!(response["type"], json!("invalid_request")); assert_eq!( response["link"], - "https://docs.meilisearch.com/errors#malformed_payload" + json!("https://docs.meilisearch.com/errors#malformed_payload") ); // put let req = test::TestRequest::put() .uri("/indexes/dog/documents") .set_payload(document.to_string()) - .insert_header(("content-type", "text/plain")) + .insert_header(("content-type", "application/json")) .to_request(); let res = test::call_service(&app, req).await; let status_code = res.status(); @@ -281,13 +287,15 @@ async fn error_add_malformed_json_documents() { assert_eq!(status_code, 400); assert_eq!( response["message"], - json!(r#"The json payload provided is malformed. :syntaxErrorHelper."#) + json!( + r#"The json payload provided is malformed. key must be a string at line 1 column 14."# + ) ); - assert_eq!(response["code"], "malformed_payload"); - assert_eq!(response["type"], "invalid_request"); + assert_eq!(response["code"], json!("malformed_payload")); + assert_eq!(response["type"], json!("invalid_request")); assert_eq!( response["link"], - "https://docs.meilisearch.com/errors#malformed_payload" + json!("https://docs.meilisearch.com/errors#malformed_payload") ); } @@ -306,7 +314,7 @@ async fn error_add_malformed_ndjson_documents() { let req = test::TestRequest::post() .uri("/indexes/dog/documents") .set_payload(document.to_string()) - .insert_header(("content-type", "text/plain")) + .insert_header(("content-type", "application/x-ndjson")) .to_request(); let res = test::call_service(&app, req).await; let status_code = res.status(); @@ -315,20 +323,22 @@ async fn error_add_malformed_ndjson_documents() { assert_eq!(status_code, 400); assert_eq!( response["message"], - json!(r#"The ndjson payload provided is malformed. :syntaxErrorHelper."#) + json!( + r#"The ndjson payload provided is malformed. key must be a string at line 2 column 2."# + ) ); - assert_eq!(response["code"], "malformed_payload"); - assert_eq!(response["type"], "invalid_request"); + assert_eq!(response["code"], json!("malformed_payload")); + assert_eq!(response["type"], json!("invalid_request")); assert_eq!( response["link"], - "https://docs.meilisearch.com/errors#malformed_payload" + json!("https://docs.meilisearch.com/errors#malformed_payload") ); // put let req = test::TestRequest::put() .uri("/indexes/dog/documents") .set_payload(document.to_string()) - .insert_header(("content-type", "text/plain")) + .insert_header(("content-type", "application/x-ndjson")) .to_request(); let res = test::call_service(&app, req).await; let status_code = res.status(); @@ -337,13 +347,15 @@ async fn error_add_malformed_ndjson_documents() { assert_eq!(status_code, 400); assert_eq!( response["message"], - json!(r#"The ndjson payload provided is malformed. :syntaxErrorHelper."#) + json!( + r#"The ndjson payload provided is malformed. key must be a string at line 2 column 2."# + ) ); - assert_eq!(response["code"], "malformed_payload"); - assert_eq!(response["type"], "invalid_request"); + assert_eq!(response["code"], json!("malformed_payload")); + assert_eq!(response["type"], json!("invalid_request")); assert_eq!( response["link"], - "https://docs.meilisearch.com/errors#malformed_payload" + json!("https://docs.meilisearch.com/errors#malformed_payload") ); } diff --git a/meilisearch-lib/src/document_formats.rs b/meilisearch-lib/src/document_formats.rs index 781ab63a0..1d965d260 100644 --- a/meilisearch-lib/src/document_formats.rs +++ b/meilisearch-lib/src/document_formats.rs @@ -27,7 +27,7 @@ impl fmt::Display for PayloadType { pub enum DocumentFormatError { #[error("Internal error: {0}")] Internal(Box), - #[error("{0}. The {1} payload provided is malformed.")] + #[error("The {1} payload provided is malformed. {0}.")] MalformedPayload( Box, PayloadType, From 7464720426342117e64bf5989156e2f33837a60e Mon Sep 17 00:00:00 2001 From: many Date: Tue, 26 Oct 2021 19:36:48 +0200 Subject: [PATCH 3/9] Fix some errors --- Cargo.lock | 2 +- meilisearch-error/src/lib.rs | 8 +- meilisearch-http/src/error.rs | 6 +- meilisearch-http/tests/common/index.rs | 2 +- .../tests/documents/add_documents.rs | 340 +++++++++++------- .../tests/documents/delete_documents.rs | 2 +- .../tests/documents/get_documents.rs | 4 +- meilisearch-http/tests/index/create_index.rs | 4 +- meilisearch-http/tests/index/delete_index.rs | 2 +- meilisearch-http/tests/index/get_index.rs | 2 +- meilisearch-http/tests/index/stats.rs | 2 +- meilisearch-http/tests/index/update_index.rs | 17 +- meilisearch-http/tests/search/errors.rs | 56 +-- meilisearch-http/tests/updates/mod.rs | 6 +- meilisearch-lib/Cargo.toml | 2 +- meilisearch-lib/src/document_formats.rs | 27 +- meilisearch-lib/src/error.rs | 10 +- meilisearch-lib/src/index/error.rs | 9 +- meilisearch-lib/src/index/updates.rs | 5 +- .../src/index_controller/dump_actor/error.rs | 2 +- .../index_controller/index_resolver/error.rs | 16 +- .../index_resolver/index_store.rs | 2 +- .../index_resolver/uuid_store.rs | 2 +- .../src/index_controller/snapshot.rs | 15 +- .../src/index_controller/updates/error.rs | 23 +- .../src/index_controller/updates/mod.rs | 13 +- .../src/index_controller/updates/store/mod.rs | 2 +- 27 files changed, 347 insertions(+), 234 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index bddde66f4..a323c86de 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1783,7 +1783,7 @@ dependencies = [ [[package]] name = "milli" version = "0.19.0" -source = "git+https://github.com/meilisearch/milli.git?tag=v0.19.0#d7943fe22553b8205b86c32a0f2656d9e42de351" +source = "git+https://github.com/meilisearch/milli.git?branch=update-error-format#3599df77f05f4e89a28c0160411e95a840e0b227" dependencies = [ "bimap", "bincode", diff --git a/meilisearch-error/src/lib.rs b/meilisearch-error/src/lib.rs index 3a3e2229a..8c9ee411c 100644 --- a/meilisearch-error/src/lib.rs +++ b/meilisearch-error/src/lib.rs @@ -62,6 +62,7 @@ pub enum Code { MaxFieldsLimitExceeded, MissingDocumentId, + InvalidDocumentId, Facet, Filter, @@ -76,6 +77,7 @@ pub enum Code { InvalidToken, MissingAuthorizationHeader, NotFound, + TaskNotFound, PayloadTooLarge, RetrieveDocument, SearchDocuments, @@ -99,7 +101,7 @@ impl Code { // index related errors // create index is thrown on internal error while creating an index. CreateIndex => ErrCode::internal("index_creation_failed", StatusCode::BAD_REQUEST), - IndexAlreadyExists => ErrCode::invalid("index_already_exists", StatusCode::BAD_REQUEST), + IndexAlreadyExists => ErrCode::invalid("index_already_exists", StatusCode::CONFLICT), // thrown when requesting an unexisting index IndexNotFound => ErrCode::invalid("index_not_found", StatusCode::NOT_FOUND), InvalidIndexUid => ErrCode::invalid("invalid_index_uid", StatusCode::BAD_REQUEST), @@ -113,7 +115,7 @@ impl Code { MissingPrimaryKey => ErrCode::invalid("missing_primary_key", StatusCode::BAD_REQUEST), // error thrown when trying to set an already existing primary key PrimaryKeyAlreadyPresent => { - ErrCode::invalid("primary_key_already_present", StatusCode::BAD_REQUEST) + ErrCode::invalid("index_primary_key_already_exists", StatusCode::BAD_REQUEST) } // invalid ranking rule InvalidRankingRule => ErrCode::invalid("invalid_request", StatusCode::BAD_REQUEST), @@ -123,6 +125,7 @@ impl Code { ErrCode::invalid("max_fields_limit_exceeded", StatusCode::BAD_REQUEST) } MissingDocumentId => ErrCode::invalid("missing_document_id", StatusCode::BAD_REQUEST), + InvalidDocumentId => ErrCode::invalid("invalid_document_id", StatusCode::BAD_REQUEST), // error related to facets Facet => ErrCode::invalid("invalid_facet", StatusCode::BAD_REQUEST), @@ -142,6 +145,7 @@ impl Code { MissingAuthorizationHeader => { ErrCode::authentication("missing_authorization_header", StatusCode::UNAUTHORIZED) } + TaskNotFound => ErrCode::invalid("task_not_found", StatusCode::NOT_FOUND), NotFound => ErrCode::invalid("not_found", StatusCode::NOT_FOUND), PayloadTooLarge => ErrCode::invalid("payload_too_large", StatusCode::PAYLOAD_TOO_LARGE), RetrieveDocument => { diff --git a/meilisearch-http/src/error.rs b/meilisearch-http/src/error.rs index c3c8d5785..39dace4fc 100644 --- a/meilisearch-http/src/error.rs +++ b/meilisearch-http/src/error.rs @@ -12,11 +12,11 @@ use serde::{Deserialize, Serialize}; #[derive(Debug, thiserror::Error)] pub enum MeilisearchHttpError { #[error("A Content-Type header is missing. Accepted values for the Content-Type header are: {}", - .0.iter().map(|s| format!("\"{}\"", s)).collect::>().join(", "))] + .0.iter().map(|s| format!("`{}`", s)).collect::>().join(", "))] MissingContentType(Vec), #[error( - "The Content-Type \"{0}\" is invalid. Accepted values for the Content-Type header are: {}", - .1.iter().map(|s| format!("\"{}\"", s)).collect::>().join(", ") + "The Content-Type `{0}` is invalid. Accepted values for the Content-Type header are: {}", + .1.iter().map(|s| format!("`{}`", s)).collect::>().join(", ") )] InvalidContentType(String, Vec), } diff --git a/meilisearch-http/tests/common/index.rs b/meilisearch-http/tests/common/index.rs index cb790ba70..2e126e67d 100644 --- a/meilisearch-http/tests/common/index.rs +++ b/meilisearch-http/tests/common/index.rs @@ -99,7 +99,7 @@ impl Index<'_> { pub async fn wait_update_id(&self, update_id: u64) -> Value { // try 10 times to get status, or panic to not wait forever let url = format!("/indexes/{}/updates/{}", self.uid, update_id); - for _ in 0..10 { + for _ in 0..20 { let (response, status_code) = self.service.get(&url).await; assert_eq!(status_code, 200, "response: {}", response); diff --git a/meilisearch-http/tests/documents/add_documents.rs b/meilisearch-http/tests/documents/add_documents.rs index 14f5c6dee..eaeabe417 100644 --- a/meilisearch-http/tests/documents/add_documents.rs +++ b/meilisearch-http/tests/documents/add_documents.rs @@ -80,7 +80,7 @@ async fn error_add_documents_test_bad_content_types() { assert_eq!( response["message"], json!( - r#"The Content-Type "text/plain" is invalid. Accepted values for the Content-Type header are: "application/json", "application/x-ndjson", "text/csv""# + r#"The Content-Type `text/plain` is invalid. Accepted values for the Content-Type header are: `application/json`, `application/x-ndjson`, `text/csv`"# ) ); assert_eq!(response["code"], "invalid_content_type"); @@ -104,7 +104,7 @@ async fn error_add_documents_test_bad_content_types() { assert_eq!( response["message"], json!( - r#"The Content-Type "text/plain" is invalid. Accepted values for the Content-Type header are: "application/json", "application/x-ndjson", "text/csv""# + r#"The Content-Type `text/plain` is invalid. Accepted values for the Content-Type header are: `application/json`, `application/x-ndjson`, `text/csv`"# ) ); assert_eq!(response["code"], "invalid_content_type"); @@ -145,7 +145,7 @@ async fn error_add_documents_test_no_content_type() { assert_eq!( response["message"], json!( - r#"A Content-Type header is missing. Accepted values for the Content-Type header are: "application/json", "application/x-ndjson", "text/csv""# + r#"A Content-Type header is missing. Accepted values for the Content-Type header are: `application/json`, `application/x-ndjson`, `text/csv`"# ) ); assert_eq!(response["code"], "missing_content_type"); @@ -168,7 +168,7 @@ async fn error_add_documents_test_no_content_type() { assert_eq!( response["message"], json!( - r#"A Content-Type header is missing. Accepted values for the Content-Type header are: "application/json", "application/x-ndjson", "text/csv""# + r#"A Content-Type header is missing. Accepted values for the Content-Type header are: `application/json`, `application/x-ndjson`, `text/csv`"# ) ); assert_eq!(response["code"], "missing_content_type"); @@ -204,7 +204,7 @@ async fn error_add_malformed_csv_documents() { assert_eq!( response["message"], json!( - r#"The csv payload provided is malformed. CSV error: record 1 (line: 2, byte: 12): found record with 3 fields, but the previous record has 2 fields."# + r#"The `csv` payload provided is malformed. `CSV error: record 1 (line: 2, byte: 12): found record with 3 fields, but the previous record has 2 fields`."# ) ); assert_eq!(response["code"], json!("malformed_payload")); @@ -228,7 +228,7 @@ async fn error_add_malformed_csv_documents() { assert_eq!( response["message"], json!( - r#"The csv payload provided is malformed. CSV error: record 1 (line: 2, byte: 12): found record with 3 fields, but the previous record has 2 fields."# + r#"The `csv` payload provided is malformed. `CSV error: record 1 (line: 2, byte: 12): found record with 3 fields, but the previous record has 2 fields`."# ) ); assert_eq!(response["code"], json!("malformed_payload")); @@ -264,7 +264,7 @@ async fn error_add_malformed_json_documents() { assert_eq!( response["message"], json!( - r#"The json payload provided is malformed. key must be a string at line 1 column 14."# + r#"The `json` payload provided is malformed. `Couldn't serialize document value: key must be a string at line 1 column 14`."# ) ); assert_eq!(response["code"], json!("malformed_payload")); @@ -288,7 +288,7 @@ async fn error_add_malformed_json_documents() { assert_eq!( response["message"], json!( - r#"The json payload provided is malformed. key must be a string at line 1 column 14."# + r#"The `json` payload provided is malformed. `Couldn't serialize document value: key must be a string at line 1 column 14`."# ) ); assert_eq!(response["code"], json!("malformed_payload")); @@ -324,7 +324,7 @@ async fn error_add_malformed_ndjson_documents() { assert_eq!( response["message"], json!( - r#"The ndjson payload provided is malformed. key must be a string at line 2 column 2."# + r#"The `ndjson` payload provided is malformed. `Couldn't serialize document value: key must be a string at line 1 column 2`."# ) ); assert_eq!(response["code"], json!("malformed_payload")); @@ -348,7 +348,7 @@ async fn error_add_malformed_ndjson_documents() { assert_eq!( response["message"], json!( - r#"The ndjson payload provided is malformed. key must be a string at line 2 column 2."# + r#"The `ndjson` payload provided is malformed. `Couldn't serialize document value: key must be a string at line 1 column 2`."# ) ); assert_eq!(response["code"], json!("malformed_payload")); @@ -359,6 +359,162 @@ async fn error_add_malformed_ndjson_documents() { ); } +#[actix_rt::test] +async fn error_add_missing_payload_csv_documents() { + let document = ""; + + let server = Server::new().await; + let app = test::init_service(create_app!( + &server.service.meilisearch, + true, + &server.service.options + )) + .await; + // post + let req = test::TestRequest::post() + .uri("/indexes/dog/documents") + .set_payload(document.to_string()) + .insert_header(("content-type", "text/csv")) + .to_request(); + let res = test::call_service(&app, req).await; + let status_code = res.status(); + let body = test::read_body(res).await; + let response: Value = serde_json::from_slice(&body).unwrap_or_default(); + assert_eq!(status_code, 400); + assert_eq!(response["message"], json!(r#"A csv payload is missing."#)); + assert_eq!(response["code"], json!("missing_payload")); + assert_eq!(response["type"], json!("invalid_request")); + assert_eq!( + response["link"], + json!("https://docs.meilisearch.com/errors#missing_payload") + ); + + // put + let req = test::TestRequest::put() + .uri("/indexes/dog/documents") + .set_payload(document.to_string()) + .insert_header(("content-type", "text/csv")) + .to_request(); + let res = test::call_service(&app, req).await; + let status_code = res.status(); + let body = test::read_body(res).await; + let response: Value = serde_json::from_slice(&body).unwrap_or_default(); + assert_eq!(status_code, 400); + assert_eq!(response["message"], json!(r#"A csv payload is missing."#)); + assert_eq!(response["code"], json!("missing_payload")); + assert_eq!(response["type"], json!("invalid_request")); + assert_eq!( + response["link"], + json!("https://docs.meilisearch.com/errors#missing_payload") + ); +} + +#[actix_rt::test] +async fn error_add_missing_payload_json_documents() { + let document = ""; + + let server = Server::new().await; + let app = test::init_service(create_app!( + &server.service.meilisearch, + true, + &server.service.options + )) + .await; + // post + let req = test::TestRequest::post() + .uri("/indexes/dog/documents") + .set_payload(document.to_string()) + .insert_header(("content-type", "application/json")) + .to_request(); + let res = test::call_service(&app, req).await; + let status_code = res.status(); + let body = test::read_body(res).await; + let response: Value = serde_json::from_slice(&body).unwrap_or_default(); + assert_eq!(status_code, 400); + assert_eq!(response["message"], json!(r#"A json payload is missing."#)); + assert_eq!(response["code"], json!("missing_payload")); + assert_eq!(response["type"], json!("invalid_request")); + assert_eq!( + response["link"], + json!("https://docs.meilisearch.com/errors#missing_payload") + ); + + // put + let req = test::TestRequest::put() + .uri("/indexes/dog/documents") + .set_payload(document.to_string()) + .insert_header(("content-type", "application/json")) + .to_request(); + let res = test::call_service(&app, req).await; + let status_code = res.status(); + let body = test::read_body(res).await; + let response: Value = serde_json::from_slice(&body).unwrap_or_default(); + assert_eq!(status_code, 400); + assert_eq!(response["message"], json!(r#"A json payload is missing."#)); + assert_eq!(response["code"], json!("missing_payload")); + assert_eq!(response["type"], json!("invalid_request")); + assert_eq!( + response["link"], + json!("https://docs.meilisearch.com/errors#missing_payload") + ); +} + +#[actix_rt::test] +async fn error_add_missing_payload_ndjson_documents() { + let document = ""; + + let server = Server::new().await; + let app = test::init_service(create_app!( + &server.service.meilisearch, + true, + &server.service.options + )) + .await; + // post + let req = test::TestRequest::post() + .uri("/indexes/dog/documents") + .set_payload(document.to_string()) + .insert_header(("content-type", "application/x-ndjson")) + .to_request(); + let res = test::call_service(&app, req).await; + let status_code = res.status(); + let body = test::read_body(res).await; + let response: Value = serde_json::from_slice(&body).unwrap_or_default(); + assert_eq!(status_code, 400); + assert_eq!( + response["message"], + json!(r#"A ndjson payload is missing."#) + ); + assert_eq!(response["code"], json!("missing_payload")); + assert_eq!(response["type"], json!("invalid_request")); + assert_eq!( + response["link"], + json!("https://docs.meilisearch.com/errors#missing_payload") + ); + + // put + let req = test::TestRequest::put() + .uri("/indexes/dog/documents") + .set_payload(document.to_string()) + .insert_header(("content-type", "application/x-ndjson")) + .to_request(); + let res = test::call_service(&app, req).await; + let status_code = res.status(); + let body = test::read_body(res).await; + let response: Value = serde_json::from_slice(&body).unwrap_or_default(); + assert_eq!(status_code, 400); + assert_eq!( + response["message"], + json!(r#"A ndjson payload is missing."#) + ); + assert_eq!(response["code"], json!("missing_payload")); + assert_eq!(response["type"], json!("invalid_request")); + assert_eq!( + response["link"], + json!("https://docs.meilisearch.com/errors#missing_payload") + ); +} + #[actix_rt::test] async fn add_documents_no_index_creation() { let server = Server::new().await; @@ -411,7 +567,7 @@ async fn error_document_add_create_index_bad_uid() { let (response, code) = index.add_documents(json!([]), None).await; let expected_response = json!({ - "message": "883 fj! is not a valid index uid. Index uid can be an integer or a string containing only alphanumeric characters, hyphens (-) and underscores (_).", + "message": "`883 fj!` is not a valid index uid. Index uid can be an integer or a string containing only alphanumeric characters, hyphens (-) and underscores (_).", "code": "invalid_index_uid", "type": "invalid_request", "link": "https://docs.meilisearch.com/errors#invalid_index_uid" @@ -428,7 +584,7 @@ async fn error_document_update_create_index_bad_uid() { let (response, code) = index.update_documents(json!([]), None).await; let expected_response = json!({ - "message": "883 fj! is not a valid index uid. Index uid can be an integer or a string containing only alphanumeric characters, hyphens (-) and underscores (_).", + "message": "`883 fj!` is not a valid index uid. Index uid can be an integer or a string containing only alphanumeric characters, hyphens (-) and underscores (_).", "code": "invalid_index_uid", "type": "invalid_request", "link": "https://docs.meilisearch.com/errors#invalid_index_uid" @@ -646,13 +802,13 @@ async fn error_add_documents_bad_document_id() { index.wait_update_id(0).await; let (response, code) = index.get_update(0).await; assert_eq!(code, 200); - assert_eq!(response["status"], "failed"); - assert_eq!(response["message"], "Document identifier foo & bar is invalid. A document identifier can be of type integer or string, only composed of alphanumeric characters (a-z A-Z 0-9), hyphens (-) and underscores (_)."); - assert_eq!(response["code"], "invalid_document_id"); - assert_eq!(response["type"], "invalid_request"); + assert_eq!(response["status"], json!("failed")); + assert_eq!(response["message"], json!("Document identifier `foo & bar` is invalid. A document identifier can be of type integer or string, only composed of alphanumeric characters (a-z A-Z 0-9), hyphens (-) and underscores (_).")); + assert_eq!(response["code"], json!("invalid_document_id")); + assert_eq!(response["type"], json!("invalid_request")); assert_eq!( response["link"], - "https://docs.meilisearch.com/errors#invalid_document_id" + json!("https://docs.meilisearch.com/errors#invalid_document_id") ); } @@ -671,13 +827,13 @@ async fn error_update_documents_bad_document_id() { index.wait_update_id(0).await; let (response, code) = index.get_update(0).await; assert_eq!(code, 200); - assert_eq!(response["status"], "failed"); - assert_eq!(response["message"], "Document identifier foo & bar is invalid. A document identifier can be of type integer or string, only composed of alphanumeric characters (a-z A-Z 0-9), hyphens (-) and underscores (_)."); - assert_eq!(response["code"], "invalid_document_id"); - assert_eq!(response["type"], "invalid_request"); + assert_eq!(response["status"], json!("failed")); + assert_eq!(response["message"], json!("Document identifier `foo & bar` is invalid. A document identifier can be of type integer or string, only composed of alphanumeric characters (a-z A-Z 0-9), hyphens (-) and underscores (_).")); + assert_eq!(response["code"], json!("invalid_document_id")); + assert_eq!(response["type"], json!("invalid_request")); assert_eq!( response["link"], - "https://docs.meilisearch.com/errors#invalid_document_id" + json!("https://docs.meilisearch.com/errors#invalid_document_id") ); } @@ -699,13 +855,13 @@ async fn error_add_documents_missing_document_id() { assert_eq!(response["status"], "failed"); assert_eq!( response["message"], - r#"Document doesn't have a docid attribute: {"id":"11","content":"foobar"}."# + json!(r#"Document doesn't have a `docid` attribute: `{"id":"11","content":"foobar"}`."#) ); - assert_eq!(response["code"], "missing_document_id"); - assert_eq!(response["type"], "invalid_request"); + assert_eq!(response["code"], json!("missing_document_id")); + assert_eq!(response["type"], json!("invalid_request")); assert_eq!( response["link"], - "https://docs.meilisearch.com/errors#missing_document_id" + json!("https://docs.meilisearch.com/errors#missing_document_id") ); } @@ -727,7 +883,7 @@ async fn error_update_documents_missing_document_id() { assert_eq!(response["status"], "failed"); assert_eq!( response["message"], - r#"Document doesn't have a docid attribute: {"id":"11","content":"foobar"}."# + r#"Document doesn't have a `docid` attribute: `{"id":"11","content":"foobar"}`."# ); assert_eq!(response["code"], "missing_document_id"); assert_eq!(response["type"], "invalid_request"); @@ -737,109 +893,41 @@ async fn error_update_documents_missing_document_id() { ); } -#[actix_rt::test] -async fn error_add_documents_with_primary_key_and_primary_key_already_exists() { - let server = Server::new().await; - let index = server.index("test"); +// #[actix_rt::test] +// async fn error_document_field_limit_reached() { +// let server = Server::new().await; +// let index = server.index("test"); - index.create(Some("primary")).await; - let documents = json!([ - { - "id": 1, - "content": "foo", - } - ]); +// index.create(Some("id")).await; - let (_response, code) = index.add_documents(documents, Some("id")).await; - assert_eq!(code, 202); +// let mut big_object = std::collections::HashMap::new(); +// big_object.insert("id".to_owned(), "wow"); +// for i in 0..65535 { +// let key = i.to_string(); +// big_object.insert(key, "I am a text!"); +// } - index.wait_update_id(0).await; +// let documents = json!([big_object]); - let (response, code) = index.get_update(0).await; - assert_eq!(code, 200); - assert_eq!(response["status"], "failed"); - assert_eq!(response["message"], "Index test already has a primary key."); - assert_eq!(response["code"], "index_primary_key_already_exists"); - assert_eq!(response["type"], "invalid_request"); - assert_eq!( - response["link"], - "https://docs.meilisearch.com/errors#index_primary_key_already_exists" - ); +// let (_response, code) = index.update_documents(documents, Some("id")).await; +// assert_eq!(code, 202); - let (response, code) = index.get().await; - assert_eq!(code, 200); - assert_eq!(response["primaryKey"], "primary"); -} - -#[actix_rt::test] -async fn error_update_documents_with_primary_key_and_primary_key_already_exists() { - let server = Server::new().await; - let index = server.index("test"); - - index.create(Some("primary")).await; - let documents = json!([ - { - "id": 1, - "content": "foo", - } - ]); - - let (_response, code) = index.update_documents(documents, Some("id")).await; - assert_eq!(code, 202); - - index.wait_update_id(0).await; - let (response, code) = index.get_update(0).await; - assert_eq!(code, 200); - // Documents without a primary key are not accepted. - assert_eq!(response["status"], "failed"); - assert_eq!(response["message"], "Index test already has a primary key."); - assert_eq!(response["code"], "index_primary_key_already_exists"); - assert_eq!(response["type"], "invalid_request"); - assert_eq!( - response["link"], - "https://docs.meilisearch.com/errors#index_primary_key_already_exists" - ); - - let (response, code) = index.get().await; - assert_eq!(code, 200); - assert_eq!(response["primaryKey"], "primary"); -} - -#[actix_rt::test] -async fn error_document_field_limit_reached() { - let server = Server::new().await; - let index = server.index("test"); - - index.create(Some("id")).await; - - let mut big_object = std::collections::HashMap::new(); - big_object.insert("id".to_owned(), "wow"); - for i in 0..65535 { - let key = i.to_string(); - big_object.insert(key, "I am a text!"); - } - - let documents = json!([big_object]); - - let (_response, code) = index.update_documents(documents, Some("id")).await; - assert_eq!(code, 202); - - index.wait_update_id(0).await; - let (response, code) = index.get_update(0).await; - assert_eq!(code, 200); - // Documents without a primary key are not accepted. - assert_eq!(response["status"], "failed"); - assert_eq!( - response["message"], - "A document cannot contain more than 65,535 fields." - ); - assert_eq!(response["code"], "document_fields_limit_reached"); - assert_eq!(response["type"], "invalid_request"); - assert_eq!( - response["link"], - "https://docs.meilisearch.com/errors#document_fields_limit_reached" - ); -} +// index.wait_update_id(0).await; +// let (response, code) = index.get_update(0).await; +// assert_eq!(code, 200); +// // Documents without a primary key are not accepted. +// assert_eq!(response["status"], "failed"); +// assert_eq!( +// response["message"], +// "A document cannot contain more than 65,535 fields." +// ); +// assert_eq!(response["code"], "document_fields_limit_reached"); +// assert_eq!(response["type"], "invalid_request"); +// assert_eq!( +// response["link"], +// "https://docs.meilisearch.com/errors#document_fields_limit_reached" +// ); +// } #[actix_rt::test] async fn error_add_documents_invalid_geo_field() { @@ -885,7 +973,7 @@ async fn error_add_documents_payload_size() { let (response, code) = index.add_documents(documents, None).await; let expected_response = json!({ - "message": "The payload cannot exceed 10MB.", + "message": "The provided payload reached the size limit.", "code": "payload_too_large", "type": "invalid_request", "link": "https://docs.meilisearch.com/errors#payload_too_large" diff --git a/meilisearch-http/tests/documents/delete_documents.rs b/meilisearch-http/tests/documents/delete_documents.rs index 81125e580..56210059b 100644 --- a/meilisearch-http/tests/documents/delete_documents.rs +++ b/meilisearch-http/tests/documents/delete_documents.rs @@ -87,7 +87,7 @@ async fn error_delete_batch_unexisting_index() { let server = Server::new().await; let (response, code) = server.index("test").delete_batch(vec![]).await; let expected_response = json!({ - "message": "Index test not found.", + "message": "Index `test` not found.", "code": "index_not_found", "type": "invalid_request", "link": "https://docs.meilisearch.com/errors#index_not_found" diff --git a/meilisearch-http/tests/documents/get_documents.rs b/meilisearch-http/tests/documents/get_documents.rs index 11f43f2c4..6d9a2d150 100644 --- a/meilisearch-http/tests/documents/get_documents.rs +++ b/meilisearch-http/tests/documents/get_documents.rs @@ -20,7 +20,7 @@ async fn error_get_unexisting_document() { let (response, code) = index.get_document(1, None).await; let expected_response = json!({ - "message": "Document 1 not found.", + "message": "Document `1` not found.", "code": "document_not_found", "type": "invalid_request", "link": "https://docs.meilisearch.com/errors#document_not_found" @@ -64,7 +64,7 @@ async fn error_get_unexisting_index_all_documents() { .await; let expected_response = json!({ - "message": "Index test not found.", + "message": "Index `test` not found.", "code": "index_not_found", "type": "invalid_request", "link": "https://docs.meilisearch.com/errors#index_not_found" diff --git a/meilisearch-http/tests/index/create_index.rs b/meilisearch-http/tests/index/create_index.rs index e5cfa2fa5..2d001517f 100644 --- a/meilisearch-http/tests/index/create_index.rs +++ b/meilisearch-http/tests/index/create_index.rs @@ -78,7 +78,7 @@ async fn error_create_existing_index() { let (response, code) = index.create(Some("primary")).await; let expected_response = json!({ - "message": "Index primary already exists.", + "message": "Index `test` already exists.", "code": "index_already_exists", "type": "invalid_request", "link":"https://docs.meilisearch.com/errors#index_already_exists" @@ -95,7 +95,7 @@ async fn error_create_with_invalid_index_uid() { let (response, code) = index.create(None).await; let expected_response = json!({ - "message": "test test#! is not a valid index uid. Index uid can be an integer or a string containing only alphanumeric characters, hyphens (-) and underscores (_).", + "message": "`test test#!` is not a valid index uid. Index uid can be an integer or a string containing only alphanumeric characters, hyphens (-) and underscores (_).", "code": "invalid_index_uid", "type": "invalid_request", "link": "https://docs.meilisearch.com/errors#invalid_index_uid" diff --git a/meilisearch-http/tests/index/delete_index.rs b/meilisearch-http/tests/index/delete_index.rs index 84c780650..c4366af35 100644 --- a/meilisearch-http/tests/index/delete_index.rs +++ b/meilisearch-http/tests/index/delete_index.rs @@ -24,7 +24,7 @@ async fn error_delete_unexisting_index() { let (response, code) = index.delete().await; let expected_response = json!({ - "message": "Index test not found.", + "message": "Index `test` not found.", "code": "index_not_found", "type": "invalid_request", "link": "https://docs.meilisearch.com/errors#index_not_found" diff --git a/meilisearch-http/tests/index/get_index.rs b/meilisearch-http/tests/index/get_index.rs index ec35ea283..4a1fa6692 100644 --- a/meilisearch-http/tests/index/get_index.rs +++ b/meilisearch-http/tests/index/get_index.rs @@ -30,7 +30,7 @@ async fn error_get_unexisting_index() { let (response, code) = index.get().await; let expected_response = json!({ - "message": "Index test not found.", + "message": "Index `test` not found.", "code": "index_not_found", "type": "invalid_request", "link": "https://docs.meilisearch.com/errors#index_not_found" diff --git a/meilisearch-http/tests/index/stats.rs b/meilisearch-http/tests/index/stats.rs index 7abbd0cd4..755366fed 100644 --- a/meilisearch-http/tests/index/stats.rs +++ b/meilisearch-http/tests/index/stats.rs @@ -53,7 +53,7 @@ async fn error_get_stats_unexisting_index() { let (response, code) = server.index("test").stats().await; let expected_response = json!({ - "message": "Index test not found.", + "message": "Index `test` not found.", "code": "index_not_found", "type": "invalid_request", "link": "https://docs.meilisearch.com/errors#index_not_found" diff --git a/meilisearch-http/tests/index/update_index.rs b/meilisearch-http/tests/index/update_index.rs index 5d92a0a16..a22def409 100644 --- a/meilisearch-http/tests/index/update_index.rs +++ b/meilisearch-http/tests/index/update_index.rs @@ -44,14 +44,23 @@ async fn update_nothing() { async fn error_update_existing_primary_key() { let server = Server::new().await; let index = server.index("test"); - let (_response, code) = index.create(Some("primary")).await; + let (_response, code) = index.create(Some("id")).await; assert_eq!(code, 201); - let (response, code) = index.update(Some("primary2")).await; + let documents = json!([ + { + "id": "11", + "content": "foobar" + } + ]); + index.add_documents(documents, None).await; + index.wait_update_id(0).await; + + let (response, code) = index.update(Some("primary")).await; let expected_response = json!({ - "message": "Index test already has a primary key.", + "message": "Index already has a primary key: `id`.", "code": "index_primary_key_already_exists", "type": "invalid_request", "link": "https://docs.meilisearch.com/errors#index_primary_key_already_exists" @@ -67,7 +76,7 @@ async fn error_update_unexisting_index() { let (response, code) = server.index("test").update(None).await; let expected_response = json!({ - "message": "Index test not found.", + "message": "Index `test` not found.", "code": "index_not_found", "type": "invalid_request", "link": "https://docs.meilisearch.com/errors#index_not_found" diff --git a/meilisearch-http/tests/search/errors.rs b/meilisearch-http/tests/search/errors.rs index d7e4b342e..e4dc12f40 100644 --- a/meilisearch-http/tests/search/errors.rs +++ b/meilisearch-http/tests/search/errors.rs @@ -8,10 +8,17 @@ async fn search_unexisting_index() { let server = Server::new().await; let index = server.index("test"); + let expected_response = json!({ + "message": "Index `test` not found.", + "code": "index_not_found", + "type": "invalid_request", + "link": "https://docs.meilisearch.com/errors#index_not_found" + }); + index .search(json!({"q": "hello"}), |response, code| { - assert_eq!(code, 404, "{}", response); - assert_eq!(response["errorCode"], "index_not_found"); + assert_eq!(code, 404); + assert_eq!(response, expected_response); }) .await; } @@ -24,7 +31,7 @@ async fn search_unexisting_parameter() { index .search(json!({"marin": "hello"}), |response, code| { assert_eq!(code, 400, "{}", response); - assert_eq!(response["errorCode"], "bad_request"); + assert_eq!(response["code"], "bad_request"); }) .await; } @@ -43,13 +50,13 @@ async fn filter_invalid_syntax_object() { index.wait_update_id(1).await; let expected_response = json!({ - "message": "Invalid syntax for the filter parameter: :syntaxErrorHelper:REPLACE_ME.", + "message": "Invalid syntax for the filter parameter: ` --> 1:7\n |\n1 | title & Glass\n | ^---\n |\n = expected word`.", "code": "invalid_filter", "type": "invalid_request", "link": "https://docs.meilisearch.com/errors#invalid_filter" }); index - .search(json!({"filter": {"title": "Glass"}}), |response, code| { + .search(json!({"filter": "title & Glass"}), |response, code| { assert_eq!(response, expected_response); assert_eq!(code, 400); }) @@ -70,19 +77,16 @@ async fn filter_invalid_syntax_array() { index.wait_update_id(1).await; let expected_response = json!({ - "message": "Invalid syntax for the filter parameter: :syntaxErrorHelper:REPLACE_ME.", + "message": "Invalid syntax for the filter parameter: ` --> 1:7\n |\n1 | title & Glass\n | ^---\n |\n = expected word`.", "code": "invalid_filter", "type": "invalid_request", "link": "https://docs.meilisearch.com/errors#invalid_filter" }); index - .search( - json!({"filter": [[["title = Glass"]]]}), - |response, code| { - assert_eq!(response, expected_response); - assert_eq!(code, 400); - }, - ) + .search(json!({"filter": [["title & Glass"]]}), |response, code| { + assert_eq!(response, expected_response); + assert_eq!(code, 400); + }) .await; } @@ -100,7 +104,7 @@ async fn filter_invalid_syntax_string() { index.wait_update_id(1).await; let expected_response = json!({ - "message": "Invalid syntax for the filter parameter: :syntaxErrorHelper:REPLACE_ME.", + "message": "Invalid syntax for the filter parameter: ` --> 1:15\n |\n1 | title = Glass XOR title = Glass\n | ^---\n |\n = expected EOI, and, or or`.", "code": "invalid_filter", "type": "invalid_request", "link": "https://docs.meilisearch.com/errors#invalid_filter" @@ -130,7 +134,7 @@ async fn filter_invalid_attribute_array() { index.wait_update_id(1).await; let expected_response = json!({ - "message": "Attribute many is not filterable. Available filterable attributes are: title.", + "message": "Attribute `many` is not filterable. Available filterable attributes are: `title`.", "code": "invalid_filter", "type": "invalid_request", "link": "https://docs.meilisearch.com/errors#invalid_filter" @@ -157,7 +161,7 @@ async fn filter_invalid_attribute_string() { index.wait_update_id(1).await; let expected_response = json!({ - "message": "Attribute many is not filterable. Available filterable attributes are: title.", + "message": "Attribute `many` is not filterable. Available filterable attributes are: `title`.", "code": "invalid_filter", "type": "invalid_request", "link": "https://docs.meilisearch.com/errors#invalid_filter" @@ -184,7 +188,7 @@ async fn filter_reserved_geo_attribute_array() { index.wait_update_id(1).await; let expected_response = json!({ - "message": "_geo is a reserved keyword and thus can't be used as a filter expression. Use the _geoRadius(latitude, longitude, distance) built-in rule to filter on _geo field coordinates.", + "message": "`_geo` is a reserved keyword and thus can't be used as a filter expression. Use the _geoRadius(latitude, longitude, distance) built-in rule to filter on _geo field coordinates.", "code": "invalid_filter", "type": "invalid_request", "link": "https://docs.meilisearch.com/errors#invalid_filter" @@ -211,7 +215,7 @@ async fn filter_reserved_geo_attribute_string() { index.wait_update_id(1).await; let expected_response = json!({ - "message": "_geo is a reserved keyword and thus can't be used as a filter expression. Use the _geoRadius(latitude, longitude, distance) built-in rule to filter on _geo field coordinates.", + "message": "`_geo` is a reserved keyword and thus can't be used as a filter expression. Use the _geoRadius(latitude, longitude, distance) built-in rule to filter on _geo field coordinates.", "code": "invalid_filter", "type": "invalid_request", "link": "https://docs.meilisearch.com/errors#invalid_filter" @@ -238,7 +242,7 @@ async fn filter_reserved_attribute_array() { index.wait_update_id(1).await; let expected_response = json!({ - "message": "_geoDistance is a reserved keyword and thus can't be used as a filter expression.", + "message": "`_geoDistance` is a reserved keyword and thus can't be used as a filter expression.", "code": "invalid_filter", "type": "invalid_request", "link": "https://docs.meilisearch.com/errors#invalid_filter" @@ -268,7 +272,7 @@ async fn filter_reserved_attribute_string() { index.wait_update_id(1).await; let expected_response = json!({ - "message": "_geoDistance is a reserved keyword and thus can't be used as a filter expression.", + "message": "`_geoDistance` is a reserved keyword and thus can't be used as a filter expression.", "code": "invalid_filter", "type": "invalid_request", "link": "https://docs.meilisearch.com/errors#invalid_filter" @@ -298,7 +302,7 @@ async fn sort_geo_reserved_attribute() { index.wait_update_id(1).await; let expected_response = json!({ - "message": "_geo is a reserved keyword and thus can't be used as a filter expression. Use the _geoPoint(latitude, longitude) built-in rule to sort on _geo field coordinates.", + "message": "`_geo` is a reserved keyword and thus can't be used as a sort expression. Use the _geoPoint(latitude, longitude) built-in rule to sort on _geo field coordinates.", "code": "invalid_sort", "type": "invalid_request", "link": "https://docs.meilisearch.com/errors#invalid_sort" @@ -330,7 +334,7 @@ async fn sort_reserved_attribute() { index.wait_update_id(1).await; let expected_response = json!({ - "message": "_geoDistance is a reserved keyword and thus can't be used as a filter expression.", + "message": "`_geoDistance` is a reserved keyword and thus can't be used as a sort expression.", "code": "invalid_sort", "type": "invalid_request", "link": "https://docs.meilisearch.com/errors#invalid_sort" @@ -362,7 +366,7 @@ async fn sort_unsortable_attribute() { index.wait_update_id(1).await; let expected_response = json!({ - "message": "Attribute title is not sortable. Available sortable attributes are: id.", + "message": "Attribute `title` is not sortable. Available sortable attributes are: `id`.", "code": "invalid_sort", "type": "invalid_request", "link": "https://docs.meilisearch.com/errors#invalid_sort" @@ -394,7 +398,7 @@ async fn sort_invalid_syntax() { index.wait_update_id(1).await; let expected_response = json!({ - "message": "Invalid syntax for the sort parameter: :syntaxErrorHelper:REPLACE_ME.", + "message": "Invalid syntax for the sort parameter: expected expression ending by `:asc` or `:desc`, found `title`.", "code": "invalid_sort", "type": "invalid_request", "link": "https://docs.meilisearch.com/errors#invalid_sort" @@ -419,7 +423,7 @@ async fn sort_unset_ranking_rule() { index .update_settings( - json!({"sortableAttributes": ["id"], "rankingRules": ["proximity", "exactness"]}), + json!({"sortableAttributes": ["title"], "rankingRules": ["proximity", "exactness"]}), ) .await; @@ -436,7 +440,7 @@ async fn sort_unset_ranking_rule() { index .search( json!({ - "sort": ["title"] + "sort": ["title:asc"] }), |response, code| { assert_eq!(response, expected_response); diff --git a/meilisearch-http/tests/updates/mod.rs b/meilisearch-http/tests/updates/mod.rs index c4ab1a275..f7bf9450a 100644 --- a/meilisearch-http/tests/updates/mod.rs +++ b/meilisearch-http/tests/updates/mod.rs @@ -7,7 +7,7 @@ async fn error_get_update_unexisting_index() { let (response, code) = server.index("test").get_update(0).await; let expected_response = json!({ - "message": "Index test not found.", + "message": "Index `test` not found.", "code": "index_not_found", "type": "invalid_request", "link": "https://docs.meilisearch.com/errors#index_not_found" @@ -25,7 +25,7 @@ async fn error_get_unexisting_update_status() { let (response, code) = index.get_update(0).await; let expected_response = json!({ - "message": "Task 0 not found.", + "message": "Task `0` not found.", "code": "task_not_found", "type": "invalid_request", "link": "https://docs.meilisearch.com/errors#task_not_found" @@ -60,7 +60,7 @@ async fn error_list_updates_unexisting_index() { let (response, code) = server.index("test").list_updates().await; let expected_response = json!({ - "message": "Index test not found.", + "message": "Index `test` not found.", "code": "index_not_found", "type": "invalid_request", "link": "https://docs.meilisearch.com/errors#index_not_found" diff --git a/meilisearch-lib/Cargo.toml b/meilisearch-lib/Cargo.toml index 429481539..74592419b 100644 --- a/meilisearch-lib/Cargo.toml +++ b/meilisearch-lib/Cargo.toml @@ -30,7 +30,7 @@ lazy_static = "1.4.0" log = "0.4.14" meilisearch-error = { path = "../meilisearch-error" } meilisearch-tokenizer = { git = "https://github.com/meilisearch/tokenizer.git", tag = "v0.2.5" } -milli = { git = "https://github.com/meilisearch/milli.git", tag = "v0.19.0" } +milli = { git = "https://github.com/meilisearch/milli.git", branch = "update-error-format" } mime = "0.3.16" num_cpus = "1.13.0" once_cell = "1.8.0" diff --git a/meilisearch-lib/src/document_formats.rs b/meilisearch-lib/src/document_formats.rs index 1d965d260..9ddbdf09d 100644 --- a/meilisearch-lib/src/document_formats.rs +++ b/meilisearch-lib/src/document_formats.rs @@ -25,9 +25,9 @@ impl fmt::Display for PayloadType { #[derive(thiserror::Error, Debug)] pub enum DocumentFormatError { - #[error("Internal error: {0}")] + #[error("Internal error!: {0}")] Internal(Box), - #[error("The {1} payload provided is malformed. {0}.")] + #[error("The `{1}` payload provided is malformed. `{0}`.")] MalformedPayload( Box, PayloadType, @@ -55,18 +55,18 @@ impl ErrorCode for DocumentFormatError { internal_error!(DocumentFormatError: io::Error); /// reads csv from input and write an obkv batch to writer. -pub fn read_csv(input: impl Read, writer: impl Write + Seek) -> Result<()> { +pub fn read_csv(input: impl Read, writer: impl Write + Seek) -> Result { let writer = BufWriter::new(writer); - DocumentBatchBuilder::from_csv(input, writer) - .map_err(|e| (PayloadType::Csv, e))? - .finish() - .map_err(|e| (PayloadType::Csv, e))?; + let builder = + DocumentBatchBuilder::from_csv(input, writer).map_err(|e| (PayloadType::Csv, e))?; + let document_count = builder.len(); + builder.finish().map_err(|e| (PayloadType::Csv, e))?; - Ok(()) + Ok(document_count) } /// reads jsonl from input and write an obkv batch to writer. -pub fn read_ndjson(input: impl Read, writer: impl Write + Seek) -> Result<()> { +pub fn read_ndjson(input: impl Read, writer: impl Write + Seek) -> Result { let mut reader = BufReader::new(input); let writer = BufWriter::new(writer); @@ -80,19 +80,22 @@ pub fn read_ndjson(input: impl Read, writer: impl Write + Seek) -> Result<()> { buf.clear(); } + let document_count = builder.len(); + builder.finish().map_err(|e| (PayloadType::Ndjson, e))?; - Ok(()) + Ok(document_count) } /// reads json from input and write an obkv batch to writer. -pub fn read_json(input: impl Read, writer: impl Write + Seek) -> Result<()> { +pub fn read_json(input: impl Read, writer: impl Write + Seek) -> Result { let writer = BufWriter::new(writer); let mut builder = DocumentBatchBuilder::new(writer).map_err(|e| (PayloadType::Json, e))?; builder .extend_from_json(input) .map_err(|e| (PayloadType::Json, e))?; + let document_count = builder.len(); builder.finish().map_err(|e| (PayloadType::Json, e))?; - Ok(()) + Ok(document_count) } diff --git a/meilisearch-lib/src/error.rs b/meilisearch-lib/src/error.rs index d29c18d25..689a8ddae 100644 --- a/meilisearch-lib/src/error.rs +++ b/meilisearch-lib/src/error.rs @@ -37,19 +37,17 @@ impl ErrorCode for MilliError<'_> { // TODO: wait for spec for new error codes. UserError::SerdeJson(_) | UserError::MaxDatabaseSizeReached - | UserError::InvalidDocumentId { .. } | UserError::InvalidStoreFile | UserError::NoSpaceLeftOnDevice - | UserError::DocumentLimitReached => Code::Internal, + | UserError::DocumentLimitReached + | UserError::UnknownInternalDocumentId { .. } => Code::Internal, UserError::AttributeLimitReached => Code::MaxFieldsLimitExceeded, UserError::InvalidFilter(_) => Code::Filter, - UserError::InvalidFilterAttribute(_) => Code::Filter, UserError::MissingDocumentId { .. } => Code::MissingDocumentId, + UserError::InvalidDocumentId { .. } => Code::InvalidDocumentId, UserError::MissingPrimaryKey => Code::MissingPrimaryKey, - UserError::PrimaryKeyCannotBeChanged => Code::PrimaryKeyAlreadyPresent, - UserError::PrimaryKeyCannotBeReset => Code::PrimaryKeyAlreadyPresent, + UserError::PrimaryKeyCannotBeChanged(_) => Code::PrimaryKeyAlreadyPresent, UserError::SortRankingRuleMissing => Code::Sort, - UserError::UnknownInternalDocumentId { .. } => Code::DocumentNotFound, UserError::InvalidFacetsDistribution { .. } => Code::BadRequest, UserError::InvalidSortableAttribute { .. } => Code::Sort, UserError::CriterionError(_) => Code::InvalidRankingRule, diff --git a/meilisearch-lib/src/index/error.rs b/meilisearch-lib/src/index/error.rs index 5899b9356..92691cb14 100644 --- a/meilisearch-lib/src/index/error.rs +++ b/meilisearch-lib/src/index/error.rs @@ -11,14 +11,12 @@ pub type Result = std::result::Result; pub enum IndexError { #[error("Internal error: {0}")] Internal(Box), - #[error("Document with id {0} not found.")] + #[error("Document `{0}` not found.")] DocumentNotFound(String), #[error("{0}")] Facet(#[from] FacetError), #[error("{0}")] Milli(#[from] milli::Error), - #[error("A primary key is already present. It's impossible to update it")] - ExistingPrimaryKey, } internal_error!( @@ -35,21 +33,20 @@ impl ErrorCode for IndexError { IndexError::DocumentNotFound(_) => Code::DocumentNotFound, IndexError::Facet(e) => e.error_code(), IndexError::Milli(e) => MilliError(e).error_code(), - IndexError::ExistingPrimaryKey => Code::PrimaryKeyAlreadyPresent, } } } #[derive(Debug, thiserror::Error)] pub enum FacetError { - #[error("Invalid facet expression, expected {}, found: {1}", .0.join(", "))] + #[error("Invalid syntax for the filter parameter: `expected {}, found: {1}`.", .0.join(", "))] InvalidExpression(&'static [&'static str], Value), } impl ErrorCode for FacetError { fn error_code(&self) -> Code { match self { - FacetError::InvalidExpression(_, _) => Code::Facet, + FacetError::InvalidExpression(_, _) => Code::Filter, } } } diff --git a/meilisearch-lib/src/index/updates.rs b/meilisearch-lib/src/index/updates.rs index 772d27d76..1d19b7dd8 100644 --- a/meilisearch-lib/src/index/updates.rs +++ b/meilisearch-lib/src/index/updates.rs @@ -11,7 +11,7 @@ use uuid::Uuid; use crate::index_controller::updates::status::{Failed, Processed, Processing, UpdateResult}; use crate::Update; -use super::error::{IndexError, Result}; +use super::error::Result; use super::index::{Index, IndexMeta}; fn serialize_with_wildcard( @@ -222,9 +222,6 @@ impl Index { match primary_key { Some(primary_key) => { let mut txn = self.write_txn()?; - if self.primary_key(&txn)?.is_some() { - return Err(IndexError::ExistingPrimaryKey); - } let mut builder = UpdateBuilder::new(0).settings(&mut txn, self); builder.set_primary_key(primary_key); builder.execute(|_, _| ())?; diff --git a/meilisearch-lib/src/index_controller/dump_actor/error.rs b/meilisearch-lib/src/index_controller/dump_actor/error.rs index 9831f3931..49da8c90f 100644 --- a/meilisearch-lib/src/index_controller/dump_actor/error.rs +++ b/meilisearch-lib/src/index_controller/dump_actor/error.rs @@ -9,7 +9,7 @@ pub type Result = std::result::Result; pub enum DumpActorError { #[error("Another dump is already in progress")] DumpAlreadyRunning, - #[error("Dump `{0}` not found")] + #[error("Dump `{0}` not found.")] DumpDoesNotExist(String), #[error("Internal error: {0}")] Internal(Box), diff --git a/meilisearch-lib/src/index_controller/index_resolver/error.rs b/meilisearch-lib/src/index_controller/index_resolver/error.rs index 661b9bde3..7e97b8ce5 100644 --- a/meilisearch-lib/src/index_controller/index_resolver/error.rs +++ b/meilisearch-lib/src/index_controller/index_resolver/error.rs @@ -3,6 +3,7 @@ use std::fmt; use meilisearch_error::{Code, ErrorCode}; use tokio::sync::mpsc::error::SendError as MpscSendError; use tokio::sync::oneshot::error::RecvError as OneshotRecvError; +use uuid::Uuid; use crate::{error::MilliError, index::error::IndexError}; @@ -12,17 +13,19 @@ pub type Result = std::result::Result; pub enum IndexResolverError { #[error("{0}")] IndexError(#[from] IndexError), - #[error("Index already exists")] - IndexAlreadyExists, - #[error("Index {0} not found")] + #[error("Index `{0}` already exists.")] + IndexAlreadyExists(String), + #[error("Index `{0}` not found.")] UnexistingIndex(String), #[error("A primary key is already present. It's impossible to update it")] ExistingPrimaryKey, - #[error("Internal Error: {0}")] + #[error("Internal Error: `{0}`")] Internal(Box), + #[error("Internal Error: Index uuid `{0}` is already assigned.")] + UUIdAlreadyExists(Uuid), #[error("{0}")] Milli(#[from] milli::Error), - #[error("Index must have a valid uid; Index uid can be of type integer or string only composed of alphanumeric characters, hyphens (-) and underscores (_).")] + #[error("`{0}` is not a valid index uid. Index uid can be an integer or a string containing only alphanumeric characters, hyphens (-) and underscores (_).")] BadlyFormatted(String), } @@ -53,10 +56,11 @@ impl ErrorCode for IndexResolverError { fn error_code(&self) -> Code { match self { IndexResolverError::IndexError(e) => e.error_code(), - IndexResolverError::IndexAlreadyExists => Code::IndexAlreadyExists, + IndexResolverError::IndexAlreadyExists(_) => Code::IndexAlreadyExists, IndexResolverError::UnexistingIndex(_) => Code::IndexNotFound, IndexResolverError::ExistingPrimaryKey => Code::PrimaryKeyAlreadyPresent, IndexResolverError::Internal(_) => Code::Internal, + IndexResolverError::UUIdAlreadyExists(_) => Code::Internal, IndexResolverError::Milli(e) => MilliError(e).error_code(), IndexResolverError::BadlyFormatted(_) => Code::InvalidIndexUid, } diff --git a/meilisearch-lib/src/index_controller/index_resolver/index_store.rs b/meilisearch-lib/src/index_controller/index_resolver/index_store.rs index dcc024121..302b23c2f 100644 --- a/meilisearch-lib/src/index_controller/index_resolver/index_store.rs +++ b/meilisearch-lib/src/index_controller/index_resolver/index_store.rs @@ -64,7 +64,7 @@ impl IndexStore for MapIndexStore { } let path = self.path.join(format!("{}", uuid)); if path.exists() { - return Err(IndexResolverError::IndexAlreadyExists); + return Err(IndexResolverError::UUIdAlreadyExists(uuid)); } let index_size = self.index_size; diff --git a/meilisearch-lib/src/index_controller/index_resolver/uuid_store.rs b/meilisearch-lib/src/index_controller/index_resolver/uuid_store.rs index f10bad757..94c3ddbb5 100644 --- a/meilisearch-lib/src/index_controller/index_resolver/uuid_store.rs +++ b/meilisearch-lib/src/index_controller/index_resolver/uuid_store.rs @@ -100,7 +100,7 @@ impl HeedUuidStore { let mut txn = env.write_txn()?; if db.get(&txn, &name)?.is_some() { - return Err(IndexResolverError::IndexAlreadyExists); + return Err(IndexResolverError::IndexAlreadyExists(name)); } db.put(&mut txn, &name, uuid.as_bytes())?; diff --git a/meilisearch-lib/src/index_controller/snapshot.rs b/meilisearch-lib/src/index_controller/snapshot.rs index c0d409990..07bf75199 100644 --- a/meilisearch-lib/src/index_controller/snapshot.rs +++ b/meilisearch-lib/src/index_controller/snapshot.rs @@ -222,10 +222,11 @@ mod test { setup(); let mut uuid_store = MockUuidStore::new(); - uuid_store - .expect_snapshot() - .once() - .returning(move |_| Box::pin(err(IndexResolverError::IndexAlreadyExists))); + uuid_store.expect_snapshot().once().returning(move |_| { + Box::pin(err(IndexResolverError::IndexAlreadyExists( + "test".to_string(), + ))) + }); let mut index_store = MockIndexStore::new(); index_store.expect_get().never(); @@ -264,9 +265,9 @@ mod test { let mut indexes = uuids.clone().into_iter().map(|uuid| { let mocker = Mocker::default(); // index returns random error - mocker - .when("snapshot") - .then(|_: &Path| -> IndexResult<()> { Err(IndexError::ExistingPrimaryKey) }); + mocker.when("snapshot").then(|_: &Path| -> IndexResult<()> { + Err(IndexError::DocumentNotFound("1".to_string())) + }); mocker.when("uuid").then(move |_: ()| uuid); Index::faux(mocker) }); diff --git a/meilisearch-lib/src/index_controller/updates/error.rs b/meilisearch-lib/src/index_controller/updates/error.rs index 39a73c7c4..27260079e 100644 --- a/meilisearch-lib/src/index_controller/updates/error.rs +++ b/meilisearch-lib/src/index_controller/updates/error.rs @@ -14,7 +14,7 @@ pub type Result = std::result::Result; #[derive(Debug, thiserror::Error)] #[allow(clippy::large_enum_variant)] pub enum UpdateLoopError { - #[error("Update {0} not found.")] + #[error("Task `{0}` not found.")] UnexistingUpdate(u64), #[error("Internal error: {0}")] Internal(Box), @@ -24,9 +24,8 @@ pub enum UpdateLoopError { FatalUpdateStoreError, #[error("{0}")] DocumentFormatError(#[from] DocumentFormatError), - // TODO: The reference to actix has to go. - #[error("{0}")] - PayloadError(#[from] actix_web::error::PayloadError), + #[error("The provided payload reached the size limit.")] + PayloadTooLarge, #[error("A {0} payload is missing.")] MissingPayload(DocumentAdditionFormat), #[error("{0}")] @@ -48,6 +47,15 @@ impl From for UpdateLoopError { } } +impl From for UpdateLoopError { + fn from(other: actix_web::error::PayloadError) -> Self { + match other { + actix_web::error::PayloadError::Overflow => Self::PayloadTooLarge, + _ => Self::Internal(Box::new(other)), + } + } +} + internal_error!( UpdateLoopError: heed::Error, std::io::Error, @@ -59,14 +67,11 @@ internal_error!( impl ErrorCode for UpdateLoopError { fn error_code(&self) -> Code { match self { - Self::UnexistingUpdate(_) => Code::NotFound, + Self::UnexistingUpdate(_) => Code::TaskNotFound, Self::Internal(_) => Code::Internal, Self::FatalUpdateStoreError => Code::Internal, Self::DocumentFormatError(error) => error.error_code(), - Self::PayloadError(error) => match error { - actix_web::error::PayloadError::Overflow => Code::PayloadTooLarge, - _ => Code::Internal, - }, + Self::PayloadTooLarge => Code::PayloadTooLarge, Self::MissingPayload(_) => Code::MissingPayload, Self::IndexError(e) => e.error_code(), } diff --git a/meilisearch-lib/src/index_controller/updates/mod.rs b/meilisearch-lib/src/index_controller/updates/mod.rs index 07ceed92b..3f2a2ef32 100644 --- a/meilisearch-lib/src/index_controller/updates/mod.rs +++ b/meilisearch-lib/src/index_controller/updates/mod.rs @@ -174,15 +174,18 @@ impl UpdateLoop { } let reader = Cursor::new(buffer); - match format { + let document_count = match format { DocumentAdditionFormat::Json => read_json(reader, &mut *update_file)?, DocumentAdditionFormat::Csv => read_csv(reader, &mut *update_file)?, DocumentAdditionFormat::Ndjson => read_ndjson(reader, &mut *update_file)?, + }; + + if document_count > 0 { + update_file.persist()?; + Ok(()) + } else { + Err(UpdateLoopError::MissingPayload(format)) } - - update_file.persist()?; - - Ok(()) }) .await??; diff --git a/meilisearch-lib/src/index_controller/updates/store/mod.rs b/meilisearch-lib/src/index_controller/updates/store/mod.rs index 81525c3fd..336d648a0 100644 --- a/meilisearch-lib/src/index_controller/updates/store/mod.rs +++ b/meilisearch-lib/src/index_controller/updates/store/mod.rs @@ -731,7 +731,7 @@ mod test { mocker .when::>("handle_update") .once() - .then(|update| Err(update.fail(IndexError::ExistingPrimaryKey))); + .then(|update| Err(update.fail(IndexError::DocumentNotFound("1".to_string())))); Box::pin(ok(Some(Index::faux(mocker)))) }); From 21f35762ca0caf623b1e656399ea3bb0fecac7a8 Mon Sep 17 00:00:00 2001 From: many Date: Thu, 28 Oct 2021 10:57:11 +0200 Subject: [PATCH 4/9] Fix content type test --- meilisearch-http/tests/content_type.rs | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/meilisearch-http/tests/content_type.rs b/meilisearch-http/tests/content_type.rs index 5402a7cd6..45b3b784b 100644 --- a/meilisearch-http/tests/content_type.rs +++ b/meilisearch-http/tests/content_type.rs @@ -8,7 +8,7 @@ use meilisearch_http::create_app; use serde_json::{json, Value}; #[actix_rt::test] -async fn strict_json_bad_content_type() { +async fn error_json_bad_content_type() { let routes = [ // all the POST routes except the dumps that can be created without any body or content-type // and the search that is not a strict json @@ -69,10 +69,10 @@ async fn strict_json_bad_content_type() { assert_eq!( response, json!({ - "message": r#"A Content-Type header is missing. Accepted values for the Content-Type header are: "application/json""#, - "errorCode": "missing_content_type", - "errorType": "invalid_request_error", - "errorLink": "https://docs.meilisearch.com/errors#missing_content_type", + "message": r#"A Content-Type header is missing. Accepted values for the Content-Type header are: `application/json`"#, + "code": "missing_content_type", + "type": "invalid_request", + "link": "https://docs.meilisearch.com/errors#missing_content_type", }), "when calling the route `{}` with no content-type", route, @@ -91,16 +91,16 @@ async fn strict_json_bad_content_type() { let response: Value = serde_json::from_slice(&body).unwrap_or_default(); assert_eq!(status_code, 415); let expected_error_message = format!( - r#"The Content-Type "{}" is invalid. Accepted values for the Content-Type header are: "application/json""#, + r#"The Content-Type `{}` is invalid. Accepted values for the Content-Type header are: `application/json`"#, bad_content_type ); assert_eq!( response, json!({ "message": expected_error_message, - "errorCode": "invalid_content_type", - "errorType": "invalid_request_error", - "errorLink": "https://docs.meilisearch.com/errors#invalid_content_type", + "code": "invalid_content_type", + "type": "invalid_request", + "link": "https://docs.meilisearch.com/errors#invalid_content_type", }), "when calling the route `{}` with a content-type of `{}`", route, From ff0908d3fa15004a76597fe7ee20909f43853dcc Mon Sep 17 00:00:00 2001 From: many Date: Thu, 28 Oct 2021 11:41:59 +0200 Subject: [PATCH 5/9] Ignore errors tests that show unrelated bugs --- Cargo.lock | 2 +- .../tests/documents/add_documents.rs | 62 ++++++++++--------- meilisearch-http/tests/index/create_index.rs | 1 + 3 files changed, 34 insertions(+), 31 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index a323c86de..4edeb10c0 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1783,7 +1783,7 @@ dependencies = [ [[package]] name = "milli" version = "0.19.0" -source = "git+https://github.com/meilisearch/milli.git?branch=update-error-format#3599df77f05f4e89a28c0160411e95a840e0b227" +source = "git+https://github.com/meilisearch/milli.git?branch=update-error-format#ed6db196810f78632758fc386f8a7f5f6cd6f357" dependencies = [ "bimap", "bincode", diff --git a/meilisearch-http/tests/documents/add_documents.rs b/meilisearch-http/tests/documents/add_documents.rs index eaeabe417..fc27765ab 100644 --- a/meilisearch-http/tests/documents/add_documents.rs +++ b/meilisearch-http/tests/documents/add_documents.rs @@ -893,43 +893,45 @@ async fn error_update_documents_missing_document_id() { ); } -// #[actix_rt::test] -// async fn error_document_field_limit_reached() { -// let server = Server::new().await; -// let index = server.index("test"); +#[actix_rt::test] +#[ignore] // // TODO: Fix in an other PR: this does not provoke any error. +async fn error_document_field_limit_reached() { + let server = Server::new().await; + let index = server.index("test"); -// index.create(Some("id")).await; + index.create(Some("id")).await; -// let mut big_object = std::collections::HashMap::new(); -// big_object.insert("id".to_owned(), "wow"); -// for i in 0..65535 { -// let key = i.to_string(); -// big_object.insert(key, "I am a text!"); -// } + let mut big_object = std::collections::HashMap::new(); + big_object.insert("id".to_owned(), "wow"); + for i in 0..65535 { + let key = i.to_string(); + big_object.insert(key, "I am a text!"); + } -// let documents = json!([big_object]); + let documents = json!([big_object]); -// let (_response, code) = index.update_documents(documents, Some("id")).await; -// assert_eq!(code, 202); + let (_response, code) = index.update_documents(documents, Some("id")).await; + assert_eq!(code, 202); -// index.wait_update_id(0).await; -// let (response, code) = index.get_update(0).await; -// assert_eq!(code, 200); -// // Documents without a primary key are not accepted. -// assert_eq!(response["status"], "failed"); -// assert_eq!( -// response["message"], -// "A document cannot contain more than 65,535 fields." -// ); -// assert_eq!(response["code"], "document_fields_limit_reached"); -// assert_eq!(response["type"], "invalid_request"); -// assert_eq!( -// response["link"], -// "https://docs.meilisearch.com/errors#document_fields_limit_reached" -// ); -// } + index.wait_update_id(0).await; + let (response, code) = index.get_update(0).await; + assert_eq!(code, 200); + // Documents without a primary key are not accepted. + assert_eq!(response["status"], "failed"); + assert_eq!( + response["message"], + "A document cannot contain more than 65,535 fields." + ); + assert_eq!(response["code"], "document_fields_limit_reached"); + assert_eq!(response["type"], "invalid_request"); + assert_eq!( + response["link"], + "https://docs.meilisearch.com/errors#document_fields_limit_reached" + ); +} #[actix_rt::test] +#[ignore] // // TODO: Fix in an other PR: this does not provoke any error. async fn error_add_documents_invalid_geo_field() { let server = Server::new().await; let index = server.index("test"); diff --git a/meilisearch-http/tests/index/create_index.rs b/meilisearch-http/tests/index/create_index.rs index 2d001517f..6a9fd82ab 100644 --- a/meilisearch-http/tests/index/create_index.rs +++ b/meilisearch-http/tests/index/create_index.rs @@ -89,6 +89,7 @@ async fn error_create_existing_index() { } #[actix_rt::test] +#[ignore] // TODO: Fix in an other PR: uid returned `test%20test%23%21` instead of `test test#!` async fn error_create_with_invalid_index_uid() { let server = Server::new().await; let index = server.index("test test#!"); From 59636fa68833a4e24ee28a95e1ba6ad2034b4982 Mon Sep 17 00:00:00 2001 From: many Date: Thu, 28 Oct 2021 12:13:51 +0200 Subject: [PATCH 6/9] Pimp error where no document is provided --- .../tests/documents/add_documents.rs | 6 ++-- meilisearch-lib/src/document_formats.rs | 31 +++++++++++++------ .../src/index_controller/updates/mod.rs | 13 +++----- 3 files changed, 30 insertions(+), 20 deletions(-) diff --git a/meilisearch-http/tests/documents/add_documents.rs b/meilisearch-http/tests/documents/add_documents.rs index fc27765ab..4363bd29d 100644 --- a/meilisearch-http/tests/documents/add_documents.rs +++ b/meilisearch-http/tests/documents/add_documents.rs @@ -695,10 +695,10 @@ async fn error_add_no_documents() { let (response, code) = index.add_documents(json!([]), None).await; let expected_response = json!({ - "message": "A json payload is missing.", - "code": "missing_payload", + "message": "The `json` payload must contain at least one document.", + "code": "malformed_payload", "type": "invalid_request", - "link": "https://docs.meilisearch.com/errors#missing_payload" + "link": "https://docs.meilisearch.com/errors#malformed_payload" }); assert_eq!(response, expected_response); diff --git a/meilisearch-lib/src/document_formats.rs b/meilisearch-lib/src/document_formats.rs index 9ddbdf09d..137ecc280 100644 --- a/meilisearch-lib/src/document_formats.rs +++ b/meilisearch-lib/src/document_formats.rs @@ -32,6 +32,8 @@ pub enum DocumentFormatError { Box, PayloadType, ), + #[error("The `{0}` payload must contain at least one document.")] + EmptyPayload(PayloadType), } impl From<(PayloadType, milli::documents::Error)> for DocumentFormatError { @@ -48,6 +50,7 @@ impl ErrorCode for DocumentFormatError { match self { DocumentFormatError::Internal(_) => Code::Internal, DocumentFormatError::MalformedPayload(_, _) => Code::MalformedPayload, + DocumentFormatError::EmptyPayload(_) => Code::MalformedPayload, } } } @@ -55,18 +58,22 @@ impl ErrorCode for DocumentFormatError { internal_error!(DocumentFormatError: io::Error); /// reads csv from input and write an obkv batch to writer. -pub fn read_csv(input: impl Read, writer: impl Write + Seek) -> Result { +pub fn read_csv(input: impl Read, writer: impl Write + Seek) -> Result<()> { let writer = BufWriter::new(writer); let builder = DocumentBatchBuilder::from_csv(input, writer).map_err(|e| (PayloadType::Csv, e))?; - let document_count = builder.len(); + + if builder.len() == 0 { + return Err(DocumentFormatError::EmptyPayload(PayloadType::Csv)); + } + builder.finish().map_err(|e| (PayloadType::Csv, e))?; - Ok(document_count) + Ok(()) } /// reads jsonl from input and write an obkv batch to writer. -pub fn read_ndjson(input: impl Read, writer: impl Write + Seek) -> Result { +pub fn read_ndjson(input: impl Read, writer: impl Write + Seek) -> Result<()> { let mut reader = BufReader::new(input); let writer = BufWriter::new(writer); @@ -80,22 +87,28 @@ pub fn read_ndjson(input: impl Read, writer: impl Write + Seek) -> Result buf.clear(); } - let document_count = builder.len(); + if builder.len() == 0 { + return Err(DocumentFormatError::EmptyPayload(PayloadType::Ndjson)); + } builder.finish().map_err(|e| (PayloadType::Ndjson, e))?; - Ok(document_count) + Ok(()) } /// reads json from input and write an obkv batch to writer. -pub fn read_json(input: impl Read, writer: impl Write + Seek) -> Result { +pub fn read_json(input: impl Read, writer: impl Write + Seek) -> Result<()> { let writer = BufWriter::new(writer); let mut builder = DocumentBatchBuilder::new(writer).map_err(|e| (PayloadType::Json, e))?; builder .extend_from_json(input) .map_err(|e| (PayloadType::Json, e))?; - let document_count = builder.len(); + + if builder.len() == 0 { + return Err(DocumentFormatError::EmptyPayload(PayloadType::Json)); + } + builder.finish().map_err(|e| (PayloadType::Json, e))?; - Ok(document_count) + Ok(()) } diff --git a/meilisearch-lib/src/index_controller/updates/mod.rs b/meilisearch-lib/src/index_controller/updates/mod.rs index 3f2a2ef32..07ceed92b 100644 --- a/meilisearch-lib/src/index_controller/updates/mod.rs +++ b/meilisearch-lib/src/index_controller/updates/mod.rs @@ -174,18 +174,15 @@ impl UpdateLoop { } let reader = Cursor::new(buffer); - let document_count = match format { + match format { DocumentAdditionFormat::Json => read_json(reader, &mut *update_file)?, DocumentAdditionFormat::Csv => read_csv(reader, &mut *update_file)?, DocumentAdditionFormat::Ndjson => read_ndjson(reader, &mut *update_file)?, - }; - - if document_count > 0 { - update_file.persist()?; - Ok(()) - } else { - Err(UpdateLoopError::MissingPayload(format)) } + + update_file.persist()?; + + Ok(()) }) .await??; From cbaca2b5797cd10ace30c83d966a3cc5da744c81 Mon Sep 17 00:00:00 2001 From: many Date: Thu, 28 Oct 2021 15:42:42 +0200 Subject: [PATCH 7/9] Fix PR comments --- Cargo.lock | 2 +- meilisearch-http/tests/common/index.rs | 2 +- meilisearch-lib/src/index_controller/index_resolver/error.rs | 4 ++-- .../src/index_controller/index_resolver/index_store.rs | 2 +- 4 files changed, 5 insertions(+), 5 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 4edeb10c0..44169624d 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1783,7 +1783,7 @@ dependencies = [ [[package]] name = "milli" version = "0.19.0" -source = "git+https://github.com/meilisearch/milli.git?branch=update-error-format#ed6db196810f78632758fc386f8a7f5f6cd6f357" +source = "git+https://github.com/meilisearch/milli.git?branch=update-error-format#9f1e0d2a49447f106277b8a07e0bba65370b47c8" dependencies = [ "bimap", "bincode", diff --git a/meilisearch-http/tests/common/index.rs b/meilisearch-http/tests/common/index.rs index 2e126e67d..cb790ba70 100644 --- a/meilisearch-http/tests/common/index.rs +++ b/meilisearch-http/tests/common/index.rs @@ -99,7 +99,7 @@ impl Index<'_> { pub async fn wait_update_id(&self, update_id: u64) -> Value { // try 10 times to get status, or panic to not wait forever let url = format!("/indexes/{}/updates/{}", self.uid, update_id); - for _ in 0..20 { + for _ in 0..10 { let (response, status_code) = self.service.get(&url).await; assert_eq!(status_code, 200, "response: {}", response); diff --git a/meilisearch-lib/src/index_controller/index_resolver/error.rs b/meilisearch-lib/src/index_controller/index_resolver/error.rs index 7e97b8ce5..9149f801e 100644 --- a/meilisearch-lib/src/index_controller/index_resolver/error.rs +++ b/meilisearch-lib/src/index_controller/index_resolver/error.rs @@ -22,7 +22,7 @@ pub enum IndexResolverError { #[error("Internal Error: `{0}`")] Internal(Box), #[error("Internal Error: Index uuid `{0}` is already assigned.")] - UUIdAlreadyExists(Uuid), + UuidAlreadyExists(Uuid), #[error("{0}")] Milli(#[from] milli::Error), #[error("`{0}` is not a valid index uid. Index uid can be an integer or a string containing only alphanumeric characters, hyphens (-) and underscores (_).")] @@ -60,7 +60,7 @@ impl ErrorCode for IndexResolverError { IndexResolverError::UnexistingIndex(_) => Code::IndexNotFound, IndexResolverError::ExistingPrimaryKey => Code::PrimaryKeyAlreadyPresent, IndexResolverError::Internal(_) => Code::Internal, - IndexResolverError::UUIdAlreadyExists(_) => Code::Internal, + IndexResolverError::UuidAlreadyExists(_) => Code::Internal, IndexResolverError::Milli(e) => MilliError(e).error_code(), IndexResolverError::BadlyFormatted(_) => Code::InvalidIndexUid, } diff --git a/meilisearch-lib/src/index_controller/index_resolver/index_store.rs b/meilisearch-lib/src/index_controller/index_resolver/index_store.rs index 302b23c2f..aa4e68ac8 100644 --- a/meilisearch-lib/src/index_controller/index_resolver/index_store.rs +++ b/meilisearch-lib/src/index_controller/index_resolver/index_store.rs @@ -64,7 +64,7 @@ impl IndexStore for MapIndexStore { } let path = self.path.join(format!("{}", uuid)); if path.exists() { - return Err(IndexResolverError::UUIdAlreadyExists(uuid)); + return Err(IndexResolverError::UuidAlreadyExists(uuid)); } let index_size = self.index_size; From 66f5de9703ab865cd047b94d1fe40d941252822c Mon Sep 17 00:00:00 2001 From: many Date: Thu, 28 Oct 2021 15:56:57 +0200 Subject: [PATCH 8/9] Change missing authrization code --- meilisearch-error/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/meilisearch-error/src/lib.rs b/meilisearch-error/src/lib.rs index 8c9ee411c..26980290d 100644 --- a/meilisearch-error/src/lib.rs +++ b/meilisearch-error/src/lib.rs @@ -141,7 +141,7 @@ impl Code { InvalidGeoField => { ErrCode::authentication("invalid_geo_field", StatusCode::BAD_REQUEST) } - InvalidToken => ErrCode::authentication("invalid_token", StatusCode::FORBIDDEN), + InvalidToken => ErrCode::authentication("invalid_api_key", StatusCode::FORBIDDEN), MissingAuthorizationHeader => { ErrCode::authentication("missing_authorization_header", StatusCode::UNAUTHORIZED) } From 3a29cbf0ae2312d5ab19d46fe6bcc720c67c22af Mon Sep 17 00:00:00 2001 From: many Date: Thu, 28 Oct 2021 15:59:06 +0200 Subject: [PATCH 9/9] Use milli v0.20.0 --- Cargo.lock | 4 ++-- meilisearch-lib/Cargo.toml | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 44169624d..3b40ba1c4 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1782,8 +1782,8 @@ dependencies = [ [[package]] name = "milli" -version = "0.19.0" -source = "git+https://github.com/meilisearch/milli.git?branch=update-error-format#9f1e0d2a49447f106277b8a07e0bba65370b47c8" +version = "0.20.0" +source = "git+https://github.com/meilisearch/milli.git?tag=v0.20.0#5a6d22d4ec51dda0aba94b314e1b5a38af9400a2" dependencies = [ "bimap", "bincode", diff --git a/meilisearch-lib/Cargo.toml b/meilisearch-lib/Cargo.toml index 74592419b..9add8c557 100644 --- a/meilisearch-lib/Cargo.toml +++ b/meilisearch-lib/Cargo.toml @@ -30,7 +30,7 @@ lazy_static = "1.4.0" log = "0.4.14" meilisearch-error = { path = "../meilisearch-error" } meilisearch-tokenizer = { git = "https://github.com/meilisearch/tokenizer.git", tag = "v0.2.5" } -milli = { git = "https://github.com/meilisearch/milli.git", branch = "update-error-format" } +milli = { git = "https://github.com/meilisearch/milli.git", tag = "v0.20.0" } mime = "0.3.16" num_cpus = "1.13.0" once_cell = "1.8.0"