Merge #3401

3401: improve the error messages for the immutable fields r=dureuill a=irevoire

Fix https://github.com/meilisearch/meilisearch/issues/3400

Co-authored-by: Tamo <tamo@meilisearch.com>

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<C: Default + ErrorCode> deserr::DeserializeError for DeserrJsonError<C> {
     }
 }
 
+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::<Vec<String>>()
+            .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.

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::<BadRequest>::error::<Infallible>(
+    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::<BadRequest>::error::<Infallible>(
             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
+        )),
+    }
 }
 
 #[derive(Debug, DeserializeFromValue)]

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::<BadRequest>::error::<Infallible>(
+    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::<BadRequest>::error::<Infallible>(
             None,
             deserr::ErrorKind::UnknownKey { key: field, accepted },
             location,
-    ));
-
-    error.code = match field {
-        "uid" => Code::ImmutableIndexUid,
-        "createdAt" => Code::ImmutableIndexCreatedAt,
-        "updatedAt" => Code::ImmutableIndexUpdatedAt,
-        _ => Code::BadRequest,
-    };
-    error
+        )),
+    }
 }
+
 #[derive(DeserializeFromValue, Debug)]
 #[deserr(error = DeserrJsonError, rename_all = camelCase, deny_unknown_fields = deny_immutable_fields_index)]
 pub struct UpdateIndexRequest {

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"