From 30a094cbb2dd75b5a67adc1d93f8733f414aaef8 Mon Sep 17 00:00:00 2001 From: many Date: Wed, 3 Nov 2021 14:25:49 +0100 Subject: [PATCH 1/5] Change lacking errors --- meilisearch-error/src/lib.rs | 33 ++++-- .../src/extractors/authentication/error.rs | 8 +- meilisearch-http/tests/common/index.rs | 104 ++++++++++++++---- meilisearch-http/tests/common/server.rs | 3 +- .../tests/documents/add_documents.rs | 12 +- meilisearch-http/tests/index/create_index.rs | 1 - meilisearch-lib/src/document_formats.rs | 2 +- meilisearch-lib/src/error.rs | 6 +- meilisearch-lib/src/index/error.rs | 2 +- .../src/index_controller/dump_actor/error.rs | 6 +- .../index_controller/dump_actor/loaders/v2.rs | 2 - meilisearch-lib/src/index_controller/error.rs | 2 +- .../index_controller/index_resolver/error.rs | 6 +- .../src/index_controller/updates/error.rs | 2 +- 14 files changed, 128 insertions(+), 61 deletions(-) diff --git a/meilisearch-error/src/lib.rs b/meilisearch-error/src/lib.rs index c6a62aa70..04c6391f9 100644 --- a/meilisearch-error/src/lib.rs +++ b/meilisearch-error/src/lib.rs @@ -53,7 +53,6 @@ pub enum Code { IndexAlreadyExists, IndexNotFound, InvalidIndexUid, - OpenIndex, // invalid state error InvalidState, @@ -70,13 +69,16 @@ pub enum Code { BadParameter, BadRequest, + DatabaseSizeLimitReached, DocumentNotFound, Internal, InvalidGeoField, InvalidRankingRule, + InvalidStore, InvalidToken, MissingAuthorizationHeader, - NotFound, + NoSpaceLeftOnDevice, + DumpNotFound, TaskNotFound, PayloadTooLarge, RetrieveDocument, @@ -100,14 +102,13 @@ impl Code { match self { // index related errors // create index is thrown on internal error while creating an index. - CreateIndex => ErrCode::internal("index_creation_failed", StatusCode::BAD_REQUEST), + CreateIndex => { + ErrCode::internal("index_creation_failed", StatusCode::INTERNAL_SERVER_ERROR) + } 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), - OpenIndex => { - ErrCode::internal("index_not_accessible", StatusCode::INTERNAL_SERVER_ERROR) - } // invalid state error InvalidState => ErrCode::internal("invalid_state", StatusCode::INTERNAL_SERVER_ERROR), @@ -120,6 +121,11 @@ impl Code { // invalid ranking rule InvalidRankingRule => ErrCode::invalid("invalid_request", StatusCode::BAD_REQUEST), + // invalid database + InvalidStore => { + ErrCode::internal("invalid_store_file", StatusCode::INTERNAL_SERVER_ERROR) + } + // invalid document MaxFieldsLimitExceeded => { ErrCode::invalid("max_fields_limit_exceeded", StatusCode::BAD_REQUEST) @@ -136,17 +142,22 @@ impl Code { BadParameter => ErrCode::invalid("bad_parameter", StatusCode::BAD_REQUEST), BadRequest => ErrCode::invalid("bad_request", StatusCode::BAD_REQUEST), + DatabaseSizeLimitReached => ErrCode::internal( + "database_size_limit_reached", + StatusCode::INTERNAL_SERVER_ERROR, + ), DocumentNotFound => ErrCode::invalid("document_not_found", StatusCode::NOT_FOUND), Internal => ErrCode::internal("internal", StatusCode::INTERNAL_SERVER_ERROR), - InvalidGeoField => { - ErrCode::authentication("invalid_geo_field", StatusCode::BAD_REQUEST) - } + InvalidGeoField => ErrCode::invalid("invalid_geo_field", StatusCode::BAD_REQUEST), InvalidToken => ErrCode::authentication("invalid_api_key", StatusCode::FORBIDDEN), 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), + DumpNotFound => ErrCode::invalid("dump_not_found", StatusCode::NOT_FOUND), + NoSpaceLeftOnDevice => { + ErrCode::internal("no_space_left_on_device", StatusCode::INTERNAL_SERVER_ERROR) + } PayloadTooLarge => ErrCode::invalid("payload_too_large", StatusCode::PAYLOAD_TOO_LARGE), RetrieveDocument => { ErrCode::internal("unretrievable_document", StatusCode::BAD_REQUEST) @@ -158,7 +169,7 @@ impl Code { // error related to dump DumpAlreadyInProgress => { - ErrCode::invalid("dump_already_in_progress", StatusCode::CONFLICT) + ErrCode::invalid("dump_already_processing", StatusCode::CONFLICT) } DumpProcessFailed => { ErrCode::internal("dump_process_failed", StatusCode::INTERNAL_SERVER_ERROR) diff --git a/meilisearch-http/src/extractors/authentication/error.rs b/meilisearch-http/src/extractors/authentication/error.rs index 902634045..5ed473b2a 100644 --- a/meilisearch-http/src/extractors/authentication/error.rs +++ b/meilisearch-http/src/extractors/authentication/error.rs @@ -2,14 +2,14 @@ use meilisearch_error::{Code, ErrorCode}; #[derive(Debug, thiserror::Error)] pub enum AuthenticationError { - #[error("You must have an authorization token")] + #[error("The X-MEILI-API-KEY header is missing.")] MissingAuthorizationHeader, - #[error("Invalid API key")] + #[error("The provided API key is invalid.")] InvalidToken(String), // Triggered on configuration error. - #[error("Irretrievable state")] + #[error("An internal error has occurred. `Irretrievable state`.")] IrretrievableState, - #[error("Unknown authentication policy")] + #[error("An internal error has occurred. `Unknown authentication policy`.")] UnknownPolicy, } diff --git a/meilisearch-http/tests/common/index.rs b/meilisearch-http/tests/common/index.rs index cb790ba70..43eac4c07 100644 --- a/meilisearch-http/tests/common/index.rs +++ b/meilisearch-http/tests/common/index.rs @@ -7,6 +7,7 @@ use actix_web::http::StatusCode; use paste::paste; use serde_json::{json, Value}; use tokio::time::sleep; +use urlencoding::encode; use super::service::Service; @@ -14,12 +15,12 @@ macro_rules! make_settings_test_routes { ($($name:ident),+) => { $(paste! { pub async fn [](&self, value: Value) -> (Value, StatusCode) { - let url = format!("/indexes/{}/settings/{}", self.uid, stringify!($name).replace("_", "-")); + let url = format!("/indexes/{}/settings/{}", encode(self.uid.as_ref()).to_string(), stringify!($name).replace("_", "-")); self.service.post(url, value).await } pub async fn [](&self) -> (Value, StatusCode) { - let url = format!("/indexes/{}/settings/{}", self.uid, stringify!($name).replace("_", "-")); + let url = format!("/indexes/{}/settings/{}", encode(self.uid.as_ref()).to_string(), stringify!($name).replace("_", "-")); self.service.get(url).await } })* @@ -34,12 +35,15 @@ pub struct Index<'a> { #[allow(dead_code)] impl Index<'_> { pub async fn get(&self) -> (Value, StatusCode) { - let url = format!("/indexes/{}", self.uid); + let url = format!("/indexes/{}", encode(self.uid.as_ref()).to_string()); self.service.get(url).await } pub async fn load_test_set(&self) -> u64 { - let url = format!("/indexes/{}/documents", self.uid); + let url = format!( + "/indexes/{}/documents", + encode(self.uid.as_ref()).to_string() + ); let (response, code) = self .service .post_str(url, include_str!("../assets/test_set.json")) @@ -62,13 +66,13 @@ impl Index<'_> { let body = json!({ "primaryKey": primary_key, }); - let url = format!("/indexes/{}", self.uid); + let url = format!("/indexes/{}", encode(self.uid.as_ref()).to_string()); self.service.put(url, body).await } pub async fn delete(&self) -> (Value, StatusCode) { - let url = format!("/indexes/{}", self.uid); + let url = format!("/indexes/{}", encode(self.uid.as_ref()).to_string()); self.service.delete(url).await } @@ -78,8 +82,15 @@ impl Index<'_> { primary_key: Option<&str>, ) -> (Value, StatusCode) { let url = match primary_key { - Some(key) => format!("/indexes/{}/documents?primaryKey={}", self.uid, key), - None => format!("/indexes/{}/documents", self.uid), + Some(key) => format!( + "/indexes/{}/documents?primaryKey={}", + encode(self.uid.as_ref()).to_string(), + key + ), + None => format!( + "/indexes/{}/documents", + encode(self.uid.as_ref()).to_string() + ), }; self.service.post(url, documents).await } @@ -90,15 +101,26 @@ impl Index<'_> { primary_key: Option<&str>, ) -> (Value, StatusCode) { let url = match primary_key { - Some(key) => format!("/indexes/{}/documents?primaryKey={}", self.uid, key), - None => format!("/indexes/{}/documents", self.uid), + Some(key) => format!( + "/indexes/{}/documents?primaryKey={}", + encode(self.uid.as_ref()).to_string(), + key + ), + None => format!( + "/indexes/{}/documents", + encode(self.uid.as_ref()).to_string() + ), }; self.service.put(url, documents).await } 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); + let url = format!( + "/indexes/{}/updates/{}", + encode(self.uid.as_ref()).to_string(), + update_id + ); for _ in 0..10 { let (response, status_code) = self.service.get(&url).await; assert_eq!(status_code, 200, "response: {}", response); @@ -113,12 +135,16 @@ impl Index<'_> { } pub async fn get_update(&self, update_id: u64) -> (Value, StatusCode) { - let url = format!("/indexes/{}/updates/{}", self.uid, update_id); + let url = format!( + "/indexes/{}/updates/{}", + encode(self.uid.as_ref()).to_string(), + update_id + ); self.service.get(url).await } pub async fn list_updates(&self) -> (Value, StatusCode) { - let url = format!("/indexes/{}/updates", self.uid); + let url = format!("/indexes/{}/updates", encode(self.uid.as_ref()).to_string()); self.service.get(url).await } @@ -127,12 +153,19 @@ impl Index<'_> { id: u64, _options: Option, ) -> (Value, StatusCode) { - let url = format!("/indexes/{}/documents/{}", self.uid, id); + let url = format!( + "/indexes/{}/documents/{}", + encode(self.uid.as_ref()).to_string(), + id + ); self.service.get(url).await } pub async fn get_all_documents(&self, options: GetAllDocumentsOptions) -> (Value, StatusCode) { - let mut url = format!("/indexes/{}/documents?", self.uid); + let mut url = format!( + "/indexes/{}/documents?", + encode(self.uid.as_ref()).to_string() + ); if let Some(limit) = options.limit { url.push_str(&format!("limit={}&", limit)); } @@ -152,39 +185,58 @@ impl Index<'_> { } pub async fn delete_document(&self, id: u64) -> (Value, StatusCode) { - let url = format!("/indexes/{}/documents/{}", self.uid, id); + let url = format!( + "/indexes/{}/documents/{}", + encode(self.uid.as_ref()).to_string(), + id + ); self.service.delete(url).await } pub async fn clear_all_documents(&self) -> (Value, StatusCode) { - let url = format!("/indexes/{}/documents", self.uid); + let url = format!( + "/indexes/{}/documents", + encode(self.uid.as_ref()).to_string() + ); self.service.delete(url).await } pub async fn delete_batch(&self, ids: Vec) -> (Value, StatusCode) { - let url = format!("/indexes/{}/documents/delete-batch", self.uid); + let url = format!( + "/indexes/{}/documents/delete-batch", + encode(self.uid.as_ref()).to_string() + ); self.service .post(url, serde_json::to_value(&ids).unwrap()) .await } pub async fn settings(&self) -> (Value, StatusCode) { - let url = format!("/indexes/{}/settings", self.uid); + let url = format!( + "/indexes/{}/settings", + encode(self.uid.as_ref()).to_string() + ); self.service.get(url).await } pub async fn update_settings(&self, settings: Value) -> (Value, StatusCode) { - let url = format!("/indexes/{}/settings", self.uid); + let url = format!( + "/indexes/{}/settings", + encode(self.uid.as_ref()).to_string() + ); self.service.post(url, settings).await } pub async fn delete_settings(&self) -> (Value, StatusCode) { - let url = format!("/indexes/{}/settings", self.uid); + let url = format!( + "/indexes/{}/settings", + encode(self.uid.as_ref()).to_string() + ); self.service.delete(url).await } pub async fn stats(&self) -> (Value, StatusCode) { - let url = format!("/indexes/{}/stats", self.uid); + let url = format!("/indexes/{}/stats", encode(self.uid.as_ref()).to_string()); self.service.get(url).await } @@ -209,13 +261,17 @@ impl Index<'_> { } pub async fn search_post(&self, query: Value) -> (Value, StatusCode) { - let url = format!("/indexes/{}/search", self.uid); + let url = format!("/indexes/{}/search", encode(self.uid.as_ref()).to_string()); self.service.post(url, query).await } pub async fn search_get(&self, query: Value) -> (Value, StatusCode) { let params = serde_url_params::to_string(&query).unwrap(); - let url = format!("/indexes/{}/search?{}", self.uid, params); + let url = format!( + "/indexes/{}/search?{}", + encode(self.uid.as_ref()).to_string(), + params + ); self.service.get(url).await } diff --git a/meilisearch-http/tests/common/server.rs b/meilisearch-http/tests/common/server.rs index 82666fc57..ab4127734 100644 --- a/meilisearch-http/tests/common/server.rs +++ b/meilisearch-http/tests/common/server.rs @@ -7,7 +7,6 @@ use meilisearch_lib::options::{IndexerOpts, MaxMemory}; use once_cell::sync::Lazy; use serde_json::Value; use tempfile::TempDir; -use urlencoding::encode; use meilisearch_http::option::Opt; @@ -62,7 +61,7 @@ impl Server { /// Returns a view to an index. There is no guarantee that the index exists. pub fn index(&self, uid: impl AsRef) -> Index<'_> { Index { - uid: encode(uid.as_ref()).to_string(), + uid: uid.as_ref().to_string(), service: &self.service, } } diff --git a/meilisearch-http/tests/documents/add_documents.rs b/meilisearch-http/tests/documents/add_documents.rs index e1747d779..dcecb19d9 100644 --- a/meilisearch-http/tests/documents/add_documents.rs +++ b/meilisearch-http/tests/documents/add_documents.rs @@ -940,25 +940,29 @@ async fn error_document_field_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"); index.create(Some("id")).await; + index + .update_settings(json!({"sortableAttributes": ["_geo"]})) + .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; + index.wait_update_id(1).await; + let (response, code) = index.get_update(1).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."# + r#"The document with the id: `11` contains an invalid _geo field: `foobar`."# ); assert_eq!(response["code"], "invalid_geo_field"); assert_eq!(response["type"], "invalid_request"); diff --git a/meilisearch-http/tests/index/create_index.rs b/meilisearch-http/tests/index/create_index.rs index 6a9fd82ab..2d001517f 100644 --- a/meilisearch-http/tests/index/create_index.rs +++ b/meilisearch-http/tests/index/create_index.rs @@ -89,7 +89,6 @@ 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#!"); diff --git a/meilisearch-lib/src/document_formats.rs b/meilisearch-lib/src/document_formats.rs index 137ecc280..c5b5f7c4e 100644 --- a/meilisearch-lib/src/document_formats.rs +++ b/meilisearch-lib/src/document_formats.rs @@ -25,7 +25,7 @@ impl fmt::Display for PayloadType { #[derive(thiserror::Error, Debug)] pub enum DocumentFormatError { - #[error("Internal error!: {0}")] + #[error("An internal error has occurred. `{0}`.")] Internal(Box), #[error("The `{1}` payload provided is malformed. `{0}`.")] MalformedPayload( diff --git a/meilisearch-lib/src/error.rs b/meilisearch-lib/src/error.rs index 689a8ddae..f245b803b 100644 --- a/meilisearch-lib/src/error.rs +++ b/meilisearch-lib/src/error.rs @@ -36,11 +36,11 @@ impl ErrorCode for MilliError<'_> { match error { // TODO: wait for spec for new error codes. UserError::SerdeJson(_) - | UserError::MaxDatabaseSizeReached - | UserError::InvalidStoreFile - | UserError::NoSpaceLeftOnDevice | UserError::DocumentLimitReached | UserError::UnknownInternalDocumentId { .. } => Code::Internal, + UserError::InvalidStoreFile => Code::InvalidStore, + UserError::NoSpaceLeftOnDevice => Code::NoSpaceLeftOnDevice, + UserError::MaxDatabaseSizeReached => Code::DatabaseSizeLimitReached, UserError::AttributeLimitReached => Code::MaxFieldsLimitExceeded, UserError::InvalidFilter(_) => Code::Filter, UserError::MissingDocumentId { .. } => Code::MissingDocumentId, diff --git a/meilisearch-lib/src/index/error.rs b/meilisearch-lib/src/index/error.rs index 92691cb14..91bf6db93 100644 --- a/meilisearch-lib/src/index/error.rs +++ b/meilisearch-lib/src/index/error.rs @@ -9,7 +9,7 @@ pub type Result = std::result::Result; #[derive(Debug, thiserror::Error)] pub enum IndexError { - #[error("Internal error: {0}")] + #[error("An internal error has occurred. `{0}`.")] Internal(Box), #[error("Document `{0}` not found.")] DocumentNotFound(String), diff --git a/meilisearch-lib/src/index_controller/dump_actor/error.rs b/meilisearch-lib/src/index_controller/dump_actor/error.rs index 49da8c90f..23616f964 100644 --- a/meilisearch-lib/src/index_controller/dump_actor/error.rs +++ b/meilisearch-lib/src/index_controller/dump_actor/error.rs @@ -7,11 +7,11 @@ pub type Result = std::result::Result; #[derive(thiserror::Error, Debug)] pub enum DumpActorError { - #[error("Another dump is already in progress")] + #[error("A dump is already processing. You must wait until the current process is finished before requesting another dump.")] DumpAlreadyRunning, #[error("Dump `{0}` not found.")] DumpDoesNotExist(String), - #[error("Internal error: {0}")] + #[error("An internal error has occurred. `{0}`.")] Internal(Box), #[error("{0}")] IndexResolver(#[from] IndexResolverError), @@ -43,7 +43,7 @@ impl ErrorCode for DumpActorError { fn error_code(&self) -> Code { match self { DumpActorError::DumpAlreadyRunning => Code::DumpAlreadyInProgress, - DumpActorError::DumpDoesNotExist(_) => Code::NotFound, + DumpActorError::DumpDoesNotExist(_) => Code::DumpNotFound, DumpActorError::Internal(_) => Code::Internal, DumpActorError::IndexResolver(e) => e.error_code(), DumpActorError::UpdateLoop(e) => e.error_code(), diff --git a/meilisearch-lib/src/index_controller/dump_actor/loaders/v2.rs b/meilisearch-lib/src/index_controller/dump_actor/loaders/v2.rs index 13d770f6a..ebaa7d3ff 100644 --- a/meilisearch-lib/src/index_controller/dump_actor/loaders/v2.rs +++ b/meilisearch-lib/src/index_controller/dump_actor/loaders/v2.rs @@ -361,7 +361,6 @@ mod compat { "index_already_exists" => Code::IndexAlreadyExists, "index_not_found" => Code::IndexNotFound, "invalid_index_uid" => Code::InvalidIndexUid, - "index_not_accessible" => Code::OpenIndex, "invalid_state" => Code::InvalidState, "missing_primary_key" => Code::MissingPrimaryKey, "primary_key_already_present" => Code::PrimaryKeyAlreadyPresent, @@ -378,7 +377,6 @@ mod compat { "invalid_geo_field" => Code::InvalidGeoField, "invalid_token" => Code::InvalidToken, "missing_authorization_header" => Code::MissingAuthorizationHeader, - "not_found" => Code::NotFound, "payload_too_large" => Code::PayloadTooLarge, "unretrievable_document" => Code::RetrieveDocument, "search_error" => Code::SearchDocuments, diff --git a/meilisearch-lib/src/index_controller/error.rs b/meilisearch-lib/src/index_controller/error.rs index 417bda01b..bc69b534f 100644 --- a/meilisearch-lib/src/index_controller/error.rs +++ b/meilisearch-lib/src/index_controller/error.rs @@ -24,7 +24,7 @@ pub enum IndexControllerError { DumpActor(#[from] DumpActorError), #[error("{0}")] IndexError(#[from] IndexError), - #[error("Internal error: {0}")] + #[error("An internal error has occurred. `{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 9149f801e..f404f0b60 100644 --- a/meilisearch-lib/src/index_controller/index_resolver/error.rs +++ b/meilisearch-lib/src/index_controller/index_resolver/error.rs @@ -19,9 +19,9 @@ pub enum IndexResolverError { UnexistingIndex(String), #[error("A primary key is already present. It's impossible to update it")] ExistingPrimaryKey, - #[error("Internal Error: `{0}`")] + #[error("An internal error has occurred. `{0}`.")] Internal(Box), - #[error("Internal Error: Index uuid `{0}` is already assigned.")] + #[error("The creation of the `{0}` index has failed due to `Index uuid is already assigned`.")] UuidAlreadyExists(Uuid), #[error("{0}")] Milli(#[from] milli::Error), @@ -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::CreateIndex, IndexResolverError::Milli(e) => MilliError(e).error_code(), IndexResolverError::BadlyFormatted(_) => Code::InvalidIndexUid, } diff --git a/meilisearch-lib/src/index_controller/updates/error.rs b/meilisearch-lib/src/index_controller/updates/error.rs index 27260079e..34ffca892 100644 --- a/meilisearch-lib/src/index_controller/updates/error.rs +++ b/meilisearch-lib/src/index_controller/updates/error.rs @@ -16,7 +16,7 @@ pub type Result = std::result::Result; pub enum UpdateLoopError { #[error("Task `{0}` not found.")] UnexistingUpdate(u64), - #[error("Internal error: {0}")] + #[error("An internal error has occurred. `{0}`.")] Internal(Box), #[error( "update store was shut down due to a fatal error, please check your logs for more info." From 06e6eaa7b4b7f6d46851b5356ee4f80698472039 Mon Sep 17 00:00:00 2001 From: many Date: Wed, 3 Nov 2021 16:10:35 +0100 Subject: [PATCH 2/5] Remove useless Facet variant --- meilisearch-error/src/lib.rs | 3 --- meilisearch-lib/src/index_controller/dump_actor/loaders/v2.rs | 2 +- 2 files changed, 1 insertion(+), 4 deletions(-) diff --git a/meilisearch-error/src/lib.rs b/meilisearch-error/src/lib.rs index 04c6391f9..ead2c08c7 100644 --- a/meilisearch-error/src/lib.rs +++ b/meilisearch-error/src/lib.rs @@ -63,7 +63,6 @@ pub enum Code { MissingDocumentId, InvalidDocumentId, - Facet, Filter, Sort, @@ -133,8 +132,6 @@ impl Code { 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), // error related to filters Filter => ErrCode::invalid("invalid_filter", StatusCode::BAD_REQUEST), // error related to sorts diff --git a/meilisearch-lib/src/index_controller/dump_actor/loaders/v2.rs b/meilisearch-lib/src/index_controller/dump_actor/loaders/v2.rs index ebaa7d3ff..62126e91a 100644 --- a/meilisearch-lib/src/index_controller/dump_actor/loaders/v2.rs +++ b/meilisearch-lib/src/index_controller/dump_actor/loaders/v2.rs @@ -367,7 +367,7 @@ mod compat { "invalid_request" => Code::InvalidRankingRule, "max_fields_limit_exceeded" => Code::MaxFieldsLimitExceeded, "missing_document_id" => Code::MissingDocumentId, - "invalid_facet" => Code::Facet, + "invalid_facet" => Code::Filter, "invalid_filter" => Code::Filter, "invalid_sort" => Code::Sort, "bad_parameter" => Code::BadParameter, From b664a46e91ca1f6061c36fde004d3ef2e560b6a7 Mon Sep 17 00:00:00 2001 From: many Date: Wed, 3 Nov 2021 16:11:20 +0100 Subject: [PATCH 3/5] Update milli version --- Cargo.lock | 4 ++-- meilisearch-lib/Cargo.toml | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 4fd864352..10dc0957e 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1804,8 +1804,8 @@ dependencies = [ [[package]] name = "milli" -version = "0.20.0" -source = "git+https://github.com/meilisearch/milli.git?tag=v0.20.0#5a6d22d4ec51dda0aba94b314e1b5a38af9400a2" +version = "0.20.1" +source = "git+https://github.com/meilisearch/milli.git?tag=v0.20.1#3be37b00e7a8ae46757a7629c55222c4e2e6c764" dependencies = [ "bimap", "bincode", diff --git a/meilisearch-lib/Cargo.toml b/meilisearch-lib/Cargo.toml index e33be9c6a..935fc4054 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.20.0" } +milli = { git = "https://github.com/meilisearch/milli.git", tag = "v0.20.1" } mime = "0.3.16" num_cpus = "1.13.0" once_cell = "1.8.0" From b59145385e3d5e5edddf78b1198b5c137d577c86 Mon Sep 17 00:00:00 2001 From: many Date: Thu, 4 Nov 2021 13:38:44 +0100 Subject: [PATCH 4/5] Fix PR comments --- meilisearch-error/src/lib.rs | 6 +- .../tests/documents/add_documents.rs | 29 +++++++ .../tests/settings/get_settings.rs | 75 +++++++++++++++++-- 3 files changed, 103 insertions(+), 7 deletions(-) diff --git a/meilisearch-error/src/lib.rs b/meilisearch-error/src/lib.rs index ead2c08c7..90f45744c 100644 --- a/meilisearch-error/src/lib.rs +++ b/meilisearch-error/src/lib.rs @@ -112,13 +112,15 @@ impl Code { // invalid state error InvalidState => ErrCode::internal("invalid_state", StatusCode::INTERNAL_SERVER_ERROR), // thrown when no primary key has been set - MissingPrimaryKey => ErrCode::invalid("missing_primary_key", StatusCode::BAD_REQUEST), + MissingPrimaryKey => { + ErrCode::invalid("primary_key_inference_failed", StatusCode::BAD_REQUEST) + } // error thrown when trying to set an already existing primary key PrimaryKeyAlreadyPresent => { ErrCode::invalid("index_primary_key_already_exists", StatusCode::BAD_REQUEST) } // invalid ranking rule - InvalidRankingRule => ErrCode::invalid("invalid_request", StatusCode::BAD_REQUEST), + InvalidRankingRule => ErrCode::invalid("invalid_ranking_rule", StatusCode::BAD_REQUEST), // invalid database InvalidStore => { diff --git a/meilisearch-http/tests/documents/add_documents.rs b/meilisearch-http/tests/documents/add_documents.rs index dcecb19d9..23f52f0a9 100644 --- a/meilisearch-http/tests/documents/add_documents.rs +++ b/meilisearch-http/tests/documents/add_documents.rs @@ -997,3 +997,32 @@ async fn error_add_documents_payload_size() { assert_eq!(response, expected_response); assert_eq!(code, 413); } + +#[actix_rt::test] +async fn error_primary_key_inference() { + let server = Server::new().await; + let index = server.index("test"); + + let documents = json!([ + { + "title": "11", + "desc": "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 primary key inference process failed because the engine did not find any fields containing `id` substring in their name. If your document identifier does not contain any `id` substring, you can set the primary key of the index."# + ); + assert_eq!(response["code"], "primary_key_inference_failed"); + assert_eq!(response["type"], "invalid_request"); + assert_eq!( + response["link"], + "https://docs.meilisearch.com/errors#primary_key_inference_failed" + ); +} diff --git a/meilisearch-http/tests/settings/get_settings.rs b/meilisearch-http/tests/settings/get_settings.rs index 37ea6bc82..c8d4a13c2 100644 --- a/meilisearch-http/tests/settings/get_settings.rs +++ b/meilisearch-http/tests/settings/get_settings.rs @@ -63,7 +63,7 @@ async fn get_settings() { } #[actix_rt::test] -async fn update_settings_unknown_field() { +async fn error_update_settings_unknown_field() { let server = Server::new().await; let index = server.index("test"); let (_response, code) = index.update_settings(json!({"foo": 12})).await; @@ -95,10 +95,19 @@ async fn test_partial_update() { } #[actix_rt::test] -async fn delete_settings_unexisting_index() { +async fn error_delete_settings_unexisting_index() { let server = Server::new().await; let index = server.index("test"); - let (_response, code) = index.delete_settings().await; + let (response, code) = index.delete_settings().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); } @@ -164,11 +173,20 @@ async fn update_setting_unexisting_index() { } #[actix_rt::test] -async fn update_setting_unexisting_index_invalid_uid() { +async fn error_update_setting_unexisting_index_invalid_uid() { let server = Server::new().await; let index = server.index("test##! "); let (response, code) = index.update_settings(json!({})).await; - assert_eq!(code, 400, "{}", response); + + let expected_response = json!({ + "message": "`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); } macro_rules! test_setting_routes { @@ -246,3 +264,50 @@ test_setting_routes!( ranking_rules, synonyms ); + +#[actix_rt::test] +async fn error_set_invalid_ranking_rules() { + let server = Server::new().await; + let index = server.index("test"); + index.create(None).await; + + let (_response, _code) = index + .update_settings(json!({ "rankingRules": [ "manyTheFish"]})) + .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#"`manyTheFish` ranking rule is invalid. Valid ranking rules are Words, Typo, Sort, Proximity, Attribute, Exactness and custom ranking rules."# + ); + assert_eq!(response["code"], "invalid_ranking_rule"); + assert_eq!(response["type"], "invalid_request"); + assert_eq!( + response["link"], + "https://docs.meilisearch.com/errors#invalid_ranking_rule" + ); +} + +#[actix_rt::test] +async fn set_and_reset_distinct_attribute_with_dedicated_route() { + let server = Server::new().await; + let index = server.index("test"); + + let (_response, _code) = index.update_distinct_attribute(json!("test")).await; + index.wait_update_id(0).await; + + let (response, _) = index.get_distinct_attribute().await; + + assert_eq!(response, "test"); + + index.update_distinct_attribute(json!(null)).await; + + index.wait_update_id(1).await; + + let (response, _) = index.get_distinct_attribute().await; + + assert_eq!(response, json!(null)); +} From cc6306c0e1c9264411cc981abea0c62a75760ae3 Mon Sep 17 00:00:00 2001 From: many Date: Thu, 4 Nov 2021 14:57:45 +0100 Subject: [PATCH 5/5] Update milli version --- Cargo.lock | 4 ++-- meilisearch-lib/Cargo.toml | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 10dc0957e..9113338e4 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1804,8 +1804,8 @@ dependencies = [ [[package]] name = "milli" -version = "0.20.1" -source = "git+https://github.com/meilisearch/milli.git?tag=v0.20.1#3be37b00e7a8ae46757a7629c55222c4e2e6c764" +version = "0.20.2" +source = "git+https://github.com/meilisearch/milli.git?tag=v0.20.2#a2fc74f010116874c9be01d98a798d30ed718435" dependencies = [ "bimap", "bincode", diff --git a/meilisearch-lib/Cargo.toml b/meilisearch-lib/Cargo.toml index 935fc4054..86d357b52 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.20.1" } +milli = { git = "https://github.com/meilisearch/milli.git", tag = "v0.20.2" } mime = "0.3.16" num_cpus = "1.13.0" once_cell = "1.8.0"