From e3742a38d4f99c6c855d3be8b1ff1acd7a7b8c6e Mon Sep 17 00:00:00 2001 From: Tamo Date: Thu, 19 Jan 2023 16:45:10 +0100 Subject: [PATCH] improve the error messages for the immutable fields --- .../src/deserr/error_messages.rs | 15 +++++++++- meilisearch-types/src/keys.rs | 30 +++++++++---------- meilisearch/src/routes/indexes/mod.rs | 25 ++++++++-------- meilisearch/tests/auth/api_keys.rs | 6 ++-- 4 files changed, 43 insertions(+), 33 deletions(-) diff --git a/meilisearch-types/src/deserr/error_messages.rs b/meilisearch-types/src/deserr/error_messages.rs index b289e454d..9b46201c4 100644 --- a/meilisearch-types/src/deserr/error_messages.rs +++ b/meilisearch-types/src/deserr/error_messages.rs @@ -9,7 +9,7 @@ We try to: use deserr::{ErrorKind, IntoValue, ValueKind, ValuePointerRef}; use super::{DeserrJsonError, DeserrQueryParamError}; -use crate::error::ErrorCode; +use crate::error::{Code, ErrorCode}; /// Return a description of the given location in a Json, preceded by the given article. /// e.g. `at .key1[8].key2`. If the location is the origin, the given article will not be @@ -179,6 +179,19 @@ impl deserr::DeserializeError for DeserrJsonError { } } +pub fn immutable_field_error(field: &str, accepted: &[&str], code: Code) -> DeserrJsonError { + let msg = format!( + "Immutable field `{field}`: expected one of {}", + accepted + .iter() + .map(|accepted| format!("`{}`", accepted)) + .collect::>() + .join(", ") + ); + + DeserrJsonError::new(msg, code) +} + /// Return a description of the given location in query parameters, preceded by the /// given article. e.g. `at key5[2]`. If the location is the origin, the given article /// will not be included in the description. diff --git a/meilisearch-types/src/keys.rs b/meilisearch-types/src/keys.rs index 7f81e39ac..31561c848 100644 --- a/meilisearch-types/src/keys.rs +++ b/meilisearch-types/src/keys.rs @@ -11,6 +11,7 @@ use time::macros::{format_description, time}; use time::{Date, OffsetDateTime, PrimitiveDateTime}; use uuid::Uuid; +use crate::deserr::error_messages::immutable_field_error; use crate::deserr::DeserrJsonError; use crate::error::deserr_codes::*; use crate::error::{unwrap_any, Code, ParseOffsetDateTimeError}; @@ -57,22 +58,19 @@ fn deny_immutable_fields_api_key( accepted: &[&str], location: ValuePointerRef, ) -> DeserrJsonError { - let mut error = unwrap_any(DeserrJsonError::::error::( - None, - deserr::ErrorKind::UnknownKey { key: field, accepted }, - location, - )); - - error.code = match field { - "uid" => Code::ImmutableApiKeyUid, - "actions" => Code::ImmutableApiKeyActions, - "indexes" => Code::ImmutableApiKeyIndexes, - "expiresAt" => Code::ImmutableApiKeyExpiresAt, - "createdAt" => Code::ImmutableApiKeyCreatedAt, - "updatedAt" => Code::ImmutableApiKeyUpdatedAt, - _ => Code::BadRequest, - }; - error + match field { + "uid" => immutable_field_error(field, accepted, Code::ImmutableApiKeyUid), + "actions" => immutable_field_error(field, accepted, Code::ImmutableApiKeyActions), + "indexes" => immutable_field_error(field, accepted, Code::ImmutableApiKeyIndexes), + "expiresAt" => immutable_field_error(field, accepted, Code::ImmutableApiKeyExpiresAt), + "createdAt" => immutable_field_error(field, accepted, Code::ImmutableApiKeyCreatedAt), + "updatedAt" => immutable_field_error(field, accepted, Code::ImmutableApiKeyUpdatedAt), + _ => unwrap_any(DeserrJsonError::::error::( + None, + deserr::ErrorKind::UnknownKey { key: field, accepted }, + location, + )), + } } #[derive(Debug, DeserializeFromValue)] diff --git a/meilisearch/src/routes/indexes/mod.rs b/meilisearch/src/routes/indexes/mod.rs index d2a842fe3..2d352bfe5 100644 --- a/meilisearch/src/routes/indexes/mod.rs +++ b/meilisearch/src/routes/indexes/mod.rs @@ -5,6 +5,7 @@ use actix_web::{web, HttpRequest, HttpResponse}; use deserr::{DeserializeError, DeserializeFromValue, ValuePointerRef}; use index_scheduler::IndexScheduler; use log::debug; +use meilisearch_types::deserr::error_messages::immutable_field_error; use meilisearch_types::deserr::query_params::Param; use meilisearch_types::deserr::{DeserrJsonError, DeserrQueryParamError}; use meilisearch_types::error::deserr_codes::*; @@ -144,20 +145,18 @@ fn deny_immutable_fields_index( accepted: &[&str], location: ValuePointerRef, ) -> DeserrJsonError { - let mut error = unwrap_any(DeserrJsonError::::error::( - None, - deserr::ErrorKind::UnknownKey { key: field, accepted }, - location, - )); - - error.code = match field { - "uid" => Code::ImmutableIndexUid, - "createdAt" => Code::ImmutableIndexCreatedAt, - "updatedAt" => Code::ImmutableIndexUpdatedAt, - _ => Code::BadRequest, - }; - error + match field { + "uid" => immutable_field_error(field, accepted, Code::ImmutableIndexUid), + "createdAt" => immutable_field_error(field, accepted, Code::ImmutableIndexCreatedAt), + "updatedAt" => immutable_field_error(field, accepted, Code::ImmutableIndexUpdatedAt), + _ => unwrap_any(DeserrJsonError::::error::( + None, + deserr::ErrorKind::UnknownKey { key: field, accepted }, + location, + )), + } } + #[derive(DeserializeFromValue, Debug)] #[deserr(error = DeserrJsonError, rename_all = camelCase, deny_unknown_fields = deny_immutable_fields_index)] pub struct UpdateIndexRequest { diff --git a/meilisearch/tests/auth/api_keys.rs b/meilisearch/tests/auth/api_keys.rs index aa829448b..6ac47e5f7 100644 --- a/meilisearch/tests/auth/api_keys.rs +++ b/meilisearch/tests/auth/api_keys.rs @@ -1394,7 +1394,7 @@ async fn error_patch_api_key_indexes() { let (response, code) = server.patch_api_key(&uid, content).await; meili_snap::snapshot!(meili_snap::json_string!(response, { ".createdAt" => "[ignored]", ".updatedAt" => "[ignored]" }), @r###" { - "message": "Unknown field `indexes`: expected one of `description`, `name`", + "message": "Immutable field `indexes`: expected one of `description`, `name`", "code": "immutable_api_key_indexes", "type": "invalid_request", "link": "https://docs.meilisearch.com/errors#immutable-api-key-indexes" @@ -1471,7 +1471,7 @@ async fn error_patch_api_key_actions() { let (response, code) = server.patch_api_key(&uid, content).await; meili_snap::snapshot!(meili_snap::json_string!(response, { ".createdAt" => "[ignored]", ".updatedAt" => "[ignored]" }), @r###" { - "message": "Unknown field `actions`: expected one of `description`, `name`", + "message": "Immutable field `actions`: expected one of `description`, `name`", "code": "immutable_api_key_actions", "type": "invalid_request", "link": "https://docs.meilisearch.com/errors#immutable-api-key-actions" @@ -1540,7 +1540,7 @@ async fn error_patch_api_key_expiration_date() { let (response, code) = server.patch_api_key(&uid, content).await; meili_snap::snapshot!(meili_snap::json_string!(response, { ".createdAt" => "[ignored]", ".updatedAt" => "[ignored]" }), @r###" { - "message": "Unknown field `expiresAt`: expected one of `description`, `name`", + "message": "Immutable field `expiresAt`: expected one of `description`, `name`", "code": "immutable_api_key_expires_at", "type": "invalid_request", "link": "https://docs.meilisearch.com/errors#immutable-api-key-expires-at"