diff --git a/meilisearch-types/src/index_uid.rs b/meilisearch-types/src/index_uid.rs index 1a5102355..2f3f6e5df 100644 --- a/meilisearch-types/src/index_uid.rs +++ b/meilisearch-types/src/index_uid.rs @@ -2,11 +2,14 @@ use std::error::Error; use std::fmt; use std::str::FromStr; +use deserr::DeserializeFromValue; + use crate::error::{Code, ErrorCode}; /// An index uid is composed of only ascii alphanumeric characters, - and _, between 1 and 400 /// bytes long -#[derive(Debug, Clone, PartialEq, Eq)] +#[derive(Debug, Clone, PartialEq, Eq, DeserializeFromValue)] +#[deserr(from(String) = IndexUid::try_from -> IndexUidFormatError)] pub struct IndexUid(String); impl IndexUid { diff --git a/meilisearch/src/error.rs b/meilisearch/src/error.rs index 23a101080..9c77f4d3e 100644 --- a/meilisearch/src/error.rs +++ b/meilisearch/src/error.rs @@ -2,7 +2,7 @@ use actix_web as aweb; use aweb::error::{JsonPayloadError, QueryPayloadError}; use meilisearch_types::document_formats::{DocumentFormatError, PayloadType}; use meilisearch_types::error::{Code, ErrorCode, ResponseError}; -use meilisearch_types::index_uid::IndexUidFormatError; +use meilisearch_types::index_uid::{IndexUid, IndexUidFormatError}; use serde_json::Value; use tokio::task::JoinError; @@ -27,7 +27,7 @@ pub enum MeilisearchHttpError { #[error("Two indexes must be given for each swap. The list `{:?}` contains {} indexes.", .0, .0.len() )] - SwapIndexPayloadWrongLength(Vec), + SwapIndexPayloadWrongLength(Vec), #[error(transparent)] IndexUid(#[from] IndexUidFormatError), #[error(transparent)] diff --git a/meilisearch/src/routes/indexes/documents.rs b/meilisearch/src/routes/indexes/documents.rs index 3316ee10b..2b36ba834 100644 --- a/meilisearch/src/routes/indexes/documents.rs +++ b/meilisearch/src/routes/indexes/documents.rs @@ -89,14 +89,17 @@ pub struct GetDocument { pub async fn get_document( index_scheduler: GuardedData, Data>, - path: web::Path, + document_param: web::Path, params: QueryParameter, ) -> Result { + let DocumentParam { index_uid, document_id } = document_param.into_inner(); + let index_uid = IndexUid::try_from(index_uid)?; + let GetDocument { fields } = params.into_inner(); let attributes_to_retrieve = fields.merge_star_and_none(); - let index = index_scheduler.index(&path.index_uid)?; - let document = retrieve_document(&index, &path.document_id, attributes_to_retrieve)?; + let index = index_scheduler.index(&index_uid)?; + let document = retrieve_document(&index, &document_id, attributes_to_retrieve)?; debug!("returns: {:?}", document); Ok(HttpResponse::Ok().json(document)) } @@ -107,10 +110,15 @@ pub async fn delete_document( req: HttpRequest, analytics: web::Data, ) -> Result { + let DocumentParam { index_uid, document_id } = path.into_inner(); + let index_uid = IndexUid::try_from(index_uid)?; + analytics.delete_documents(DocumentDeletionKind::PerDocumentId, &req); - let DocumentParam { document_id, index_uid } = path.into_inner(); - let task = KindWithContent::DocumentDeletion { index_uid, documents_ids: vec![document_id] }; + let task = KindWithContent::DocumentDeletion { + index_uid: index_uid.to_string(), + documents_ids: vec![document_id], + }; let task: SummarizedTaskView = tokio::task::spawn_blocking(move || index_scheduler.register(task)).await??.into(); debug!("returns: {:?}", task); @@ -133,6 +141,7 @@ pub async fn get_all_documents( index_uid: web::Path, params: QueryParameter, ) -> Result { + let index_uid = IndexUid::try_from(index_uid.into_inner())?; debug!("called with params: {:?}", params); let BrowseQuery { limit, offset, fields } = params.into_inner(); let attributes_to_retrieve = fields.merge_star_and_none(); @@ -161,6 +170,8 @@ pub async fn add_documents( req: HttpRequest, analytics: web::Data, ) -> Result { + let index_uid = IndexUid::try_from(index_uid.into_inner())?; + debug!("called with params: {:?}", params); let params = params.into_inner(); @@ -170,7 +181,7 @@ pub async fn add_documents( let task = document_addition( extract_mime_type(&req)?, index_scheduler, - index_uid.into_inner(), + index_uid, params.primary_key, body, IndexDocumentsMethod::ReplaceDocuments, @@ -183,14 +194,15 @@ pub async fn add_documents( pub async fn update_documents( index_scheduler: GuardedData, Data>, - path: web::Path, + index_uid: web::Path, params: QueryParameter, body: Payload, req: HttpRequest, analytics: web::Data, ) -> Result { + let index_uid = IndexUid::try_from(index_uid.into_inner())?; + debug!("called with params: {:?}", params); - let index_uid = path.into_inner(); analytics.update_documents(¶ms, index_scheduler.index(&index_uid).is_err(), &req); @@ -212,7 +224,7 @@ pub async fn update_documents( async fn document_addition( mime_type: Option, index_scheduler: GuardedData, Data>, - index_uid: String, + index_uid: IndexUid, primary_key: Option, mut body: Payload, method: IndexDocumentsMethod, @@ -233,9 +245,6 @@ async fn document_addition( } }; - // is your indexUid valid? - let index_uid = IndexUid::try_from(index_uid)?.into_inner(); - let (uuid, mut update_file) = index_scheduler.create_update_file()?; let temp_file = match tempfile() { @@ -311,7 +320,7 @@ async fn document_addition( documents_count, primary_key, allow_index_creation, - index_uid, + index_uid: index_uid.to_string(), }; let scheduler = index_scheduler.clone(); @@ -329,12 +338,13 @@ async fn document_addition( pub async fn delete_documents( index_scheduler: GuardedData, Data>, - path: web::Path, + index_uid: web::Path, body: web::Json>, req: HttpRequest, analytics: web::Data, ) -> Result { debug!("called with params: {:?}", body); + let index_uid = IndexUid::try_from(index_uid.into_inner())?; analytics.delete_documents(DocumentDeletionKind::PerBatch, &req); @@ -344,7 +354,7 @@ pub async fn delete_documents( .collect(); let task = - KindWithContent::DocumentDeletion { index_uid: path.into_inner(), documents_ids: ids }; + KindWithContent::DocumentDeletion { index_uid: index_uid.to_string(), documents_ids: ids }; let task: SummarizedTaskView = tokio::task::spawn_blocking(move || index_scheduler.register(task)).await??.into(); @@ -354,13 +364,14 @@ pub async fn delete_documents( pub async fn clear_all_documents( index_scheduler: GuardedData, Data>, - path: web::Path, + index_uid: web::Path, req: HttpRequest, analytics: web::Data, ) -> Result { + let index_uid = IndexUid::try_from(index_uid.into_inner())?; analytics.delete_documents(DocumentDeletionKind::ClearAll, &req); - let task = KindWithContent::DocumentClear { index_uid: path.into_inner() }; + let task = KindWithContent::DocumentClear { index_uid: index_uid.to_string() }; let task: SummarizedTaskView = tokio::task::spawn_blocking(move || index_scheduler.register(task)).await??.into(); diff --git a/meilisearch/src/routes/indexes/mod.rs b/meilisearch/src/routes/indexes/mod.rs index d19dc4773..d2a842fe3 100644 --- a/meilisearch/src/routes/indexes/mod.rs +++ b/meilisearch/src/routes/indexes/mod.rs @@ -12,7 +12,7 @@ use meilisearch_types::error::{unwrap_any, Code, ResponseError}; use meilisearch_types::index_uid::IndexUid; use meilisearch_types::milli::{self, FieldDistribution, Index}; use meilisearch_types::tasks::KindWithContent; -use serde::{Deserialize, Serialize}; +use serde::Serialize; use serde_json::json; use time::OffsetDateTime; @@ -49,7 +49,7 @@ pub fn configure(cfg: &mut web::ServiceConfig) { ); } -#[derive(Debug, Serialize, Deserialize, Clone)] +#[derive(Debug, Serialize, Clone)] #[serde(rename_all = "camelCase")] pub struct IndexView { pub uid: String, @@ -108,8 +108,8 @@ pub async fn list_indexes( #[deserr(error = DeserrJsonError, rename_all = camelCase, deny_unknown_fields)] pub struct IndexCreateRequest { #[deserr(error = DeserrJsonError, missing_field_error = DeserrJsonError::missing_index_uid)] - uid: String, - #[deserr(error = DeserrJsonError)] + uid: IndexUid, + #[deserr(default, error = DeserrJsonError)] primary_key: Option, } @@ -120,7 +120,6 @@ pub async fn create_index( analytics: web::Data, ) -> Result { let IndexCreateRequest { primary_key, uid } = body.into_inner(); - let uid = IndexUid::try_from(uid)?.into_inner(); let allow_index_creation = index_scheduler.filters().search_rules.is_index_authorized(&uid); if allow_index_creation { @@ -130,7 +129,7 @@ pub async fn create_index( Some(&req), ); - let task = KindWithContent::IndexCreation { index_uid: uid, primary_key }; + let task = KindWithContent::IndexCreation { index_uid: uid.to_string(), primary_key }; let task: SummarizedTaskView = tokio::task::spawn_blocking(move || index_scheduler.register(task)).await??.into(); @@ -162,7 +161,7 @@ fn deny_immutable_fields_index( #[derive(DeserializeFromValue, Debug)] #[deserr(error = DeserrJsonError, rename_all = camelCase, deny_unknown_fields = deny_immutable_fields_index)] pub struct UpdateIndexRequest { - #[deserr(error = DeserrJsonError)] + #[deserr(default, error = DeserrJsonError)] primary_key: Option, } @@ -170,6 +169,8 @@ pub async fn get_index( index_scheduler: GuardedData, Data>, index_uid: web::Path, ) -> Result { + let index_uid = IndexUid::try_from(index_uid.into_inner())?; + let index = index_scheduler.index(&index_uid)?; let index_view = IndexView::new(index_uid.into_inner(), &index)?; @@ -180,12 +181,13 @@ pub async fn get_index( pub async fn update_index( index_scheduler: GuardedData, Data>, - path: web::Path, + index_uid: web::Path, body: ValidatedJson, req: HttpRequest, analytics: web::Data, ) -> Result { debug!("called with params: {:?}", body); + let index_uid = IndexUid::try_from(index_uid.into_inner())?; let body = body.into_inner(); analytics.publish( "Index Updated".to_string(), @@ -194,7 +196,7 @@ pub async fn update_index( ); let task = KindWithContent::IndexUpdate { - index_uid: path.into_inner(), + index_uid: index_uid.into_inner(), primary_key: body.primary_key, }; @@ -209,6 +211,7 @@ pub async fn delete_index( index_scheduler: GuardedData, Data>, index_uid: web::Path, ) -> Result { + let index_uid = IndexUid::try_from(index_uid.into_inner())?; let task = KindWithContent::IndexDeletion { index_uid: index_uid.into_inner() }; let task: SummarizedTaskView = tokio::task::spawn_blocking(move || index_scheduler.register(task)).await??.into(); @@ -222,6 +225,7 @@ pub async fn get_index_stats( req: HttpRequest, analytics: web::Data, ) -> Result { + let index_uid = IndexUid::try_from(index_uid.into_inner())?; analytics.publish("Stats Seen".to_string(), json!({ "per_index_uid": true }), Some(&req)); let stats = IndexStats::new((*index_scheduler).clone(), index_uid.into_inner())?; diff --git a/meilisearch/src/routes/indexes/search.rs b/meilisearch/src/routes/indexes/search.rs index 6bf5e3dae..545c69ec5 100644 --- a/meilisearch/src/routes/indexes/search.rs +++ b/meilisearch/src/routes/indexes/search.rs @@ -7,6 +7,7 @@ use meilisearch_types::deserr::query_params::Param; use meilisearch_types::deserr::{DeserrJsonError, DeserrQueryParamError}; use meilisearch_types::error::deserr_codes::*; use meilisearch_types::error::ResponseError; +use meilisearch_types::index_uid::IndexUid; use meilisearch_types::serde_cs::vec::CS; use serde_json::Value; @@ -154,6 +155,8 @@ pub async fn search_with_url_query( analytics: web::Data, ) -> Result { debug!("called with params: {:?}", params); + let index_uid = IndexUid::try_from(index_uid.into_inner())?; + let mut query: SearchQuery = params.into_inner().into(); // Tenant token search_rules. @@ -185,6 +188,8 @@ pub async fn search_with_post( req: HttpRequest, analytics: web::Data, ) -> Result { + let index_uid = IndexUid::try_from(index_uid.into_inner())?; + let mut query = params.into_inner(); debug!("search called with params: {:?}", query); diff --git a/meilisearch/src/routes/indexes/settings.rs b/meilisearch/src/routes/indexes/settings.rs index 91c3473fa..0c864cc73 100644 --- a/meilisearch/src/routes/indexes/settings.rs +++ b/meilisearch/src/routes/indexes/settings.rs @@ -41,12 +41,14 @@ macro_rules! make_setting_route { >, index_uid: web::Path, ) -> Result { + let index_uid = IndexUid::try_from(index_uid.into_inner())?; + let new_settings = Settings { $attr: Setting::Reset.into(), ..Default::default() }; let allow_index_creation = index_scheduler.filters().allow_index_creation; - let index_uid = IndexUid::try_from(index_uid.into_inner())?.into_inner(); + let task = KindWithContent::SettingsUpdate { - index_uid, + index_uid: index_uid.to_string(), new_settings: Box::new(new_settings), is_deletion: true, allow_index_creation, @@ -70,6 +72,8 @@ macro_rules! make_setting_route { req: HttpRequest, $analytics_var: web::Data, ) -> std::result::Result { + let index_uid = IndexUid::try_from(index_uid.into_inner())?; + let body = body.into_inner(); $analytics(&body, &req); @@ -83,9 +87,9 @@ macro_rules! make_setting_route { }; let allow_index_creation = index_scheduler.filters().allow_index_creation; - let index_uid = IndexUid::try_from(index_uid.into_inner())?.into_inner(); + let task = KindWithContent::SettingsUpdate { - index_uid, + index_uid: index_uid.to_string(), new_settings: Box::new(new_settings), is_deletion: false, allow_index_creation, @@ -106,6 +110,8 @@ macro_rules! make_setting_route { >, index_uid: actix_web::web::Path, ) -> std::result::Result { + let index_uid = IndexUid::try_from(index_uid.into_inner())?; + let index = index_scheduler.index(&index_uid)?; let rtxn = index.read_txn()?; let settings = settings(&index, &rtxn)?; @@ -466,6 +472,8 @@ pub async fn update_all( req: HttpRequest, analytics: web::Data, ) -> Result { + let index_uid = IndexUid::try_from(index_uid.into_inner())?; + let new_settings = body.into_inner(); analytics.publish( @@ -571,6 +579,8 @@ pub async fn get_all( index_scheduler: GuardedData, Data>, index_uid: web::Path, ) -> Result { + let index_uid = IndexUid::try_from(index_uid.into_inner())?; + let index = index_scheduler.index(&index_uid)?; let rtxn = index.read_txn()?; let new_settings = settings(&index, &rtxn)?; @@ -582,6 +592,8 @@ pub async fn delete_all( index_scheduler: GuardedData, Data>, index_uid: web::Path, ) -> Result { + let index_uid = IndexUid::try_from(index_uid.into_inner())?; + let new_settings = Settings::cleared().into_unchecked(); let allow_index_creation = index_scheduler.filters().allow_index_creation; diff --git a/meilisearch/src/routes/swap_indexes.rs b/meilisearch/src/routes/swap_indexes.rs index 9adbfecdd..4a7802f2e 100644 --- a/meilisearch/src/routes/swap_indexes.rs +++ b/meilisearch/src/routes/swap_indexes.rs @@ -5,6 +5,7 @@ use index_scheduler::IndexScheduler; use meilisearch_types::deserr::DeserrJsonError; use meilisearch_types::error::deserr_codes::InvalidSwapIndexes; use meilisearch_types::error::ResponseError; +use meilisearch_types::index_uid::IndexUid; use meilisearch_types::tasks::{IndexSwap, KindWithContent}; use serde_json::json; @@ -24,7 +25,7 @@ pub fn configure(cfg: &mut web::ServiceConfig) { #[deserr(error = DeserrJsonError, rename_all = camelCase, deny_unknown_fields)] pub struct SwapIndexesPayload { #[deserr(error = DeserrJsonError, missing_field_error = DeserrJsonError::missing_swap_indexes)] - indexes: Vec, + indexes: Vec, } pub async fn swap_indexes( @@ -55,7 +56,7 @@ pub async fn swap_indexes( if !search_rules.is_index_authorized(lhs) || !search_rules.is_index_authorized(rhs) { return Err(AuthenticationError::InvalidToken.into()); } - swaps.push(IndexSwap { indexes: (lhs.clone(), rhs.clone()) }); + swaps.push(IndexSwap { indexes: (lhs.to_string(), rhs.to_string()) }); } let task = KindWithContent::IndexSwap { swaps }; diff --git a/meilisearch/tests/index/create_index.rs b/meilisearch/tests/index/create_index.rs index 6c5adb5c6..884a0b069 100644 --- a/meilisearch/tests/index/create_index.rs +++ b/meilisearch/tests/index/create_index.rs @@ -1,6 +1,7 @@ use actix_web::http::header::ContentType; use actix_web::test; use http::header::ACCEPT_ENCODING; +use meili_snap::{json_string, snapshot}; use serde_json::{json, Value}; use crate::common::encoder::Encoder; @@ -188,13 +189,13 @@ async fn error_create_with_invalid_index_uid() { 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); + snapshot!(code, @"400 Bad Request"); + snapshot!(json_string!(response), @r###" + { + "message": "Invalid value at `.uid`: `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/tests/index/get_index.rs b/meilisearch/tests/index/get_index.rs index 7bd8a0184..6e70484f6 100644 --- a/meilisearch/tests/index/get_index.rs +++ b/meilisearch/tests/index/get_index.rs @@ -1,3 +1,4 @@ +use meili_snap::{json_string, snapshot}; use serde_json::{json, Value}; use crate::common::Server; @@ -182,15 +183,13 @@ async fn get_invalid_index_uid() { let index = server.index("this is not a valid index name"); let (response, code) = index.get().await; - assert_eq!(code, 404); - assert_eq!( - response, - json!( - { - "message": "Index `this is not a valid index name` not found.", - "code": "index_not_found", - "type": "invalid_request", - "link": "https://docs.meilisearch.com/errors#index-not-found" - }) - ); + snapshot!(code, @"400 Bad Request"); + snapshot!(json_string!(response), @r###" + { + "message": "`this is not a valid index name` 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" + } + "###); }