diff --git a/Cargo.lock b/Cargo.lock index 757a2a9e8..75102b14e 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1707,8 +1707,8 @@ dependencies = [ [[package]] name = "milli" -version = "0.4.0" -source = "git+https://github.com/meilisearch/milli.git?tag=v0.4.0#3bd4cf94cc60733393b94021fca77eb100bfe17a" +version = "0.4.1" +source = "git+https://github.com/meilisearch/milli.git?tag=v0.4.1#a67ccfdf3ac093b51bdf5ada3621fd6663897497" dependencies = [ "bstr", "byteorder", diff --git a/meilisearch-http/Cargo.toml b/meilisearch-http/Cargo.toml index 70ab30d34..5e49bba32 100644 --- a/meilisearch-http/Cargo.toml +++ b/meilisearch-http/Cargo.toml @@ -51,7 +51,7 @@ main_error = "0.1.0" meilisearch-error = { path = "../meilisearch-error" } meilisearch-tokenizer = { git = "https://github.com/meilisearch/Tokenizer.git", tag = "v0.2.2" } memmap = "0.7.0" -milli = { git = "https://github.com/meilisearch/milli.git", tag = "v0.4.0" } +milli = { git = "https://github.com/meilisearch/milli.git", tag = "v0.4.1" } mime = "0.3.16" once_cell = "1.5.2" oxidized-json-checker = "0.3.2" diff --git a/meilisearch-http/src/data/mod.rs b/meilisearch-http/src/data/mod.rs index 9f8a688bc..48dfcfa06 100644 --- a/meilisearch-http/src/data/mod.rs +++ b/meilisearch-http/src/data/mod.rs @@ -5,7 +5,7 @@ use sha2::Digest; use crate::index::{Checked, Settings}; use crate::index_controller::{ - DumpInfo, IndexController, IndexMetadata, IndexSettings, IndexStats, Stats, + error::Result, DumpInfo, IndexController, IndexMetadata, IndexSettings, IndexStats, Stats, }; use crate::option::Opt; @@ -79,15 +79,15 @@ impl Data { Ok(Data { inner }) } - pub async fn settings(&self, uid: String) -> anyhow::Result> { + pub async fn settings(&self, uid: String) -> Result> { self.index_controller.settings(uid).await } - pub async fn list_indexes(&self) -> anyhow::Result> { + pub async fn list_indexes(&self) -> Result> { self.index_controller.list_indexes().await } - pub async fn index(&self, uid: String) -> anyhow::Result { + pub async fn index(&self, uid: String) -> Result { self.index_controller.get_index(uid).await } @@ -95,7 +95,7 @@ impl Data { &self, uid: String, primary_key: Option, - ) -> anyhow::Result { + ) -> Result { let settings = IndexSettings { uid: Some(uid), primary_key, @@ -105,19 +105,19 @@ impl Data { Ok(meta) } - pub async fn get_index_stats(&self, uid: String) -> anyhow::Result { + pub async fn get_index_stats(&self, uid: String) -> Result { Ok(self.index_controller.get_index_stats(uid).await?) } - pub async fn get_all_stats(&self) -> anyhow::Result { + pub async fn get_all_stats(&self) -> Result { Ok(self.index_controller.get_all_stats().await?) } - pub async fn create_dump(&self) -> anyhow::Result { + pub async fn create_dump(&self) -> Result { Ok(self.index_controller.create_dump().await?) } - pub async fn dump_status(&self, uid: String) -> anyhow::Result { + pub async fn dump_status(&self, uid: String) -> Result { Ok(self.index_controller.dump_info(uid).await?) } diff --git a/meilisearch-http/src/data/search.rs b/meilisearch-http/src/data/search.rs index 1a998b997..5ad8d4a07 100644 --- a/meilisearch-http/src/data/search.rs +++ b/meilisearch-http/src/data/search.rs @@ -2,13 +2,10 @@ use serde_json::{Map, Value}; use super::Data; use crate::index::{SearchQuery, SearchResult}; +use crate::index_controller::error::Result; impl Data { - pub async fn search( - &self, - index: String, - search_query: SearchQuery, - ) -> anyhow::Result { + pub async fn search(&self, index: String, search_query: SearchQuery) -> Result { self.index_controller.search(index, search_query).await } @@ -18,7 +15,7 @@ impl Data { offset: usize, limit: usize, attributes_to_retrieve: Option>, - ) -> anyhow::Result>> { + ) -> Result>> { self.index_controller .documents(index, offset, limit, attributes_to_retrieve) .await @@ -29,7 +26,7 @@ impl Data { index: String, document_id: String, attributes_to_retrieve: Option>, - ) -> anyhow::Result> { + ) -> Result> { self.index_controller .document(index, document_id, attributes_to_retrieve) .await diff --git a/meilisearch-http/src/data/updates.rs b/meilisearch-http/src/data/updates.rs index 23949aa86..6b29d46b1 100644 --- a/meilisearch-http/src/data/updates.rs +++ b/meilisearch-http/src/data/updates.rs @@ -3,7 +3,7 @@ use milli::update::{IndexDocumentsMethod, UpdateFormat}; use super::Data; use crate::index::{Checked, Settings}; -use crate::index_controller::{IndexMetadata, IndexSettings, UpdateStatus}; +use crate::index_controller::{error::Result, IndexMetadata, IndexSettings, UpdateStatus}; impl Data { pub async fn add_documents( @@ -13,7 +13,7 @@ impl Data { format: UpdateFormat, stream: Payload, primary_key: Option, - ) -> anyhow::Result { + ) -> Result { let update_status = self .index_controller .add_documents(index, method, format, stream, primary_key) @@ -26,7 +26,7 @@ impl Data { index: String, settings: Settings, create: bool, - ) -> anyhow::Result { + ) -> Result { let update = self .index_controller .update_settings(index, settings, create) @@ -34,7 +34,7 @@ impl Data { Ok(update) } - pub async fn clear_documents(&self, index: String) -> anyhow::Result { + pub async fn clear_documents(&self, index: String) -> Result { let update = self.index_controller.clear_documents(index).await?; Ok(update) } @@ -43,7 +43,7 @@ impl Data { &self, index: String, document_ids: Vec, - ) -> anyhow::Result { + ) -> Result { let update = self .index_controller .delete_documents(index, document_ids) @@ -51,16 +51,16 @@ impl Data { Ok(update) } - pub async fn delete_index(&self, index: String) -> anyhow::Result<()> { + pub async fn delete_index(&self, index: String) -> Result<()> { self.index_controller.delete_index(index).await?; Ok(()) } - pub async fn get_update_status(&self, index: String, uid: u64) -> anyhow::Result { + pub async fn get_update_status(&self, index: String, uid: u64) -> Result { self.index_controller.update_status(index, uid).await } - pub async fn get_updates_status(&self, index: String) -> anyhow::Result> { + pub async fn get_updates_status(&self, index: String) -> Result> { self.index_controller.all_update_status(index).await } @@ -69,7 +69,7 @@ impl Data { uid: String, primary_key: Option, new_uid: Option, - ) -> anyhow::Result { + ) -> Result { let settings = IndexSettings { uid: new_uid, primary_key, diff --git a/meilisearch-http/src/error.rs b/meilisearch-http/src/error.rs index 07bd96fb9..7d56de738 100644 --- a/meilisearch-http/src/error.rs +++ b/meilisearch-http/src/error.rs @@ -1,321 +1,182 @@ -use std::error; +use std::error::Error; use std::fmt; use actix_web as aweb; use actix_web::body::Body; use actix_web::dev::BaseHttpResponseBuilder; -use actix_web::error::{JsonPayloadError, QueryPayloadError}; -use actix_web::http::Error as HttpError; use actix_web::http::StatusCode; +use aweb::error::{JsonPayloadError, QueryPayloadError}; use meilisearch_error::{Code, ErrorCode}; -use serde::ser::{Serialize, SerializeStruct, Serializer}; +use milli::UserError; +use serde::{Deserialize, Serialize}; -#[derive(Debug)] -pub struct ResponseError { - inner: Box, +#[derive(Debug, thiserror::Error)] +pub enum AuthenticationError { + #[error("you must have an authorization token")] + MissingAuthorizationHeader, + #[error("invalid API key")] + InvalidToken(String), } -impl error::Error for ResponseError {} - -impl ErrorCode for ResponseError { +impl ErrorCode for AuthenticationError { fn error_code(&self) -> Code { - self.inner.error_code() + match self { + AuthenticationError::MissingAuthorizationHeader => Code::MissingAuthorizationHeader, + AuthenticationError::InvalidToken(_) => Code::InvalidToken, + } } } +#[derive(Debug, Serialize, Deserialize, Clone)] +#[serde(rename_all = "camelCase")] +pub struct ResponseError { + #[serde(skip)] + code: StatusCode, + message: String, + error_code: String, + error_type: String, + error_link: String, +} + impl fmt::Display for ResponseError { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - self.inner.fmt(f) + self.message.fmt(f) } } -// TODO: remove this when implementing actual error handling -impl From for ResponseError { - fn from(other: anyhow::Error) -> ResponseError { - ResponseError { - inner: Box::new(Error::NotFound(other.to_string())), +impl From for ResponseError +where + T: ErrorCode, +{ + fn from(other: T) -> Self { + Self { + code: other.http_status(), + message: other.to_string(), + error_code: other.error_name(), + error_type: other.error_type(), + error_link: other.error_url(), } } } -impl From for ResponseError { - fn from(error: Error) -> ResponseError { - ResponseError { - inner: Box::new(error), - } - } -} - -impl From for ResponseError { - fn from(err: FacetCountError) -> ResponseError { - ResponseError { - inner: Box::new(err), - } - } -} - -impl Serialize for ResponseError { - fn serialize(&self, serializer: S) -> Result - where - S: Serializer, - { - let struct_name = "ResponseError"; - let field_count = 4; - - let mut state = serializer.serialize_struct(struct_name, field_count)?; - state.serialize_field("message", &self.to_string())?; - state.serialize_field("errorCode", &self.error_name())?; - state.serialize_field("errorType", &self.error_type())?; - state.serialize_field("errorLink", &self.error_url())?; - state.end() - } -} - impl aweb::error::ResponseError for ResponseError { fn error_response(&self) -> aweb::BaseHttpResponse { let json = serde_json::to_vec(self).unwrap(); - BaseHttpResponseBuilder::new(self.status_code()).body(json) + BaseHttpResponseBuilder::new(self.status_code()) + .content_type("application/json") + .body(json) } fn status_code(&self) -> StatusCode { - self.http_status() + self.code + } +} + +macro_rules! internal_error { + ($target:ty : $($other:path), *) => { + $( + impl From<$other> for $target { + fn from(other: $other) -> Self { + Self::Internal(Box::new(other)) + } + } + )* } } #[derive(Debug)] -pub enum Error { - BadParameter(String, String), - BadRequest(String), - CreateIndex(String), - DocumentNotFound(String), - IndexNotFound(String), - IndexAlreadyExists(String), - Internal(String), - InvalidIndexUid, - InvalidToken(String), - MissingAuthorizationHeader, - NotFound(String), - OpenIndex(String), - RetrieveDocument(u32, String), - SearchDocuments(String), - PayloadTooLarge, - UnsupportedMediaType, - DumpAlreadyInProgress, - DumpProcessFailed(String), +pub struct MilliError<'a>(pub &'a milli::Error); + +impl Error for MilliError<'_> {} + +impl fmt::Display for MilliError<'_> { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + self.0.fmt(f) + } } -impl error::Error for Error {} - -impl ErrorCode for Error { +impl ErrorCode for MilliError<'_> { fn error_code(&self) -> Code { - use Error::*; + match self.0 { + milli::Error::InternalError(_) => Code::Internal, + milli::Error::IoError(_) => Code::Internal, + milli::Error::UserError(ref error) => { + match error { + // TODO: wait for spec for new error codes. + | UserError::Csv(_) + | UserError::SerdeJson(_) + | UserError::MaxDatabaseSizeReached + | UserError::InvalidCriterionName { .. } + | UserError::InvalidDocumentId { .. } + | UserError::InvalidStoreFile + | UserError::NoSpaceLeftOnDevice + | UserError::DocumentLimitReached => Code::Internal, + UserError::AttributeLimitReached => Code::MaxFieldsLimitExceeded, + UserError::InvalidFilter(_) => Code::Filter, + UserError::InvalidFilterAttribute(_) => Code::Filter, + UserError::MissingDocumentId { .. } => Code::MissingDocumentId, + UserError::MissingPrimaryKey => Code::MissingPrimaryKey, + UserError::PrimaryKeyCannotBeChanged => Code::PrimaryKeyAlreadyPresent, + UserError::PrimaryKeyCannotBeReset => Code::PrimaryKeyAlreadyPresent, + UserError::UnknownInternalDocumentId { .. } => Code::DocumentNotFound, + } + } + } + } +} + +impl fmt::Display for PayloadError { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { match self { - BadParameter(_, _) => Code::BadParameter, - BadRequest(_) => Code::BadRequest, - CreateIndex(_) => Code::CreateIndex, - DocumentNotFound(_) => Code::DocumentNotFound, - IndexNotFound(_) => Code::IndexNotFound, - IndexAlreadyExists(_) => Code::IndexAlreadyExists, - Internal(_) => Code::Internal, - InvalidIndexUid => Code::InvalidIndexUid, - InvalidToken(_) => Code::InvalidToken, - MissingAuthorizationHeader => Code::MissingAuthorizationHeader, - NotFound(_) => Code::NotFound, - OpenIndex(_) => Code::OpenIndex, - RetrieveDocument(_, _) => Code::RetrieveDocument, - SearchDocuments(_) => Code::SearchDocuments, - PayloadTooLarge => Code::PayloadTooLarge, - UnsupportedMediaType => Code::UnsupportedMediaType, - _ => unreachable!() - //DumpAlreadyInProgress => Code::DumpAlreadyInProgress, - //DumpProcessFailed(_) => Code::DumpProcessFailed, + PayloadError::Json(e) => e.fmt(f), + PayloadError::Query(e) => e.fmt(f), } } } #[derive(Debug)] -pub enum FacetCountError { - AttributeNotSet(String), - SyntaxError(String), - UnexpectedToken { - found: String, - expected: &'static [&'static str], - }, - NoFacetSet, +pub enum PayloadError { + Json(JsonPayloadError), + Query(QueryPayloadError), } -impl error::Error for FacetCountError {} +impl Error for PayloadError {} -impl ErrorCode for FacetCountError { +impl ErrorCode for PayloadError { fn error_code(&self) -> Code { - Code::BadRequest - } -} - -impl FacetCountError { - pub fn unexpected_token( - found: impl ToString, - expected: &'static [&'static str], - ) -> FacetCountError { - let found = found.to_string(); - FacetCountError::UnexpectedToken { expected, found } - } -} - -impl From for FacetCountError { - fn from(other: serde_json::error::Error) -> FacetCountError { - FacetCountError::SyntaxError(other.to_string()) - } -} - -impl fmt::Display for FacetCountError { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - use FacetCountError::*; - match self { - AttributeNotSet(attr) => write!(f, "Attribute {} is not set as facet", attr), - SyntaxError(msg) => write!(f, "Syntax error: {}", msg), - UnexpectedToken { expected, found } => { - write!(f, "Unexpected {} found, expected {:?}", found, expected) - } - NoFacetSet => write!(f, "Can't perform facet count, as no facet is set"), + PayloadError::Json(err) => match err { + JsonPayloadError::Overflow => Code::PayloadTooLarge, + JsonPayloadError::ContentType => Code::UnsupportedMediaType, + JsonPayloadError::Payload(aweb::error::PayloadError::Overflow) => Code::PayloadTooLarge, + JsonPayloadError::Deserialize(_) + | JsonPayloadError::Payload(_) => Code::BadRequest, + JsonPayloadError::Serialize(_) => Code::Internal, + _ => Code::Internal, + }, + PayloadError::Query(err) => match err { + QueryPayloadError::Deserialize(_) => Code::BadRequest, + _ => Code::Internal, + }, } } } -impl Error { - pub fn internal(err: impl fmt::Display) -> Error { - Error::Internal(err.to_string()) - } - - pub fn bad_request(err: impl fmt::Display) -> Error { - Error::BadRequest(err.to_string()) - } - - pub fn missing_authorization_header() -> Error { - Error::MissingAuthorizationHeader - } - - pub fn invalid_token(err: impl fmt::Display) -> Error { - Error::InvalidToken(err.to_string()) - } - - pub fn not_found(err: impl fmt::Display) -> Error { - Error::NotFound(err.to_string()) - } - - pub fn index_not_found(err: impl fmt::Display) -> Error { - Error::IndexNotFound(err.to_string()) - } - - pub fn document_not_found(err: impl fmt::Display) -> Error { - Error::DocumentNotFound(err.to_string()) - } - - pub fn bad_parameter(param: impl fmt::Display, err: impl fmt::Display) -> Error { - Error::BadParameter(param.to_string(), err.to_string()) - } - - pub fn open_index(err: impl fmt::Display) -> Error { - Error::OpenIndex(err.to_string()) - } - - pub fn create_index(err: impl fmt::Display) -> Error { - Error::CreateIndex(err.to_string()) - } - - pub fn invalid_index_uid() -> Error { - Error::InvalidIndexUid - } - - pub fn retrieve_document(doc_id: u32, err: impl fmt::Display) -> Error { - Error::RetrieveDocument(doc_id, err.to_string()) - } - - pub fn search_documents(err: impl fmt::Display) -> Error { - Error::SearchDocuments(err.to_string()) - } - - pub fn dump_conflict() -> Error { - Error::DumpAlreadyInProgress - } - - pub fn dump_failed(message: String) -> Error { - Error::DumpProcessFailed(message) +impl From for PayloadError { + fn from(other: JsonPayloadError) -> Self { + Self::Json(other) } } -impl fmt::Display for Error { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - match self { - Self::BadParameter(param, err) => write!(f, "Url parameter {} error: {}", param, err), - Self::BadRequest(err) => f.write_str(err), - Self::CreateIndex(err) => write!(f, "Impossible to create index; {}", err), - Self::DocumentNotFound(document_id) => write!(f, "Document with id {} not found", document_id), - Self::IndexNotFound(index_uid) => write!(f, "Index {} not found", index_uid), - Self::IndexAlreadyExists(index_uid) => write!(f, "Index {} already exists", index_uid), - Self::Internal(err) => f.write_str(err), - Self::InvalidIndexUid => f.write_str("Index must have a valid uid; Index uid can be of type integer or string only composed of alphanumeric characters, hyphens (-) and underscores (_)."), - Self::InvalidToken(err) => write!(f, "Invalid API key: {}", err), - Self::MissingAuthorizationHeader => f.write_str("You must have an authorization token"), - Self::NotFound(err) => write!(f, "{} not found", err), - Self::OpenIndex(err) => write!(f, "Impossible to open index; {}", err), - Self::RetrieveDocument(id, err) => write!(f, "Impossible to retrieve the document with id: {}; {}", id, err), - Self::SearchDocuments(err) => write!(f, "Impossible to search documents; {}", err), - Self::PayloadTooLarge => f.write_str("Payload too large"), - Self::UnsupportedMediaType => f.write_str("Unsupported media type"), - Self::DumpAlreadyInProgress => f.write_str("Another dump is already in progress"), - Self::DumpProcessFailed(message) => write!(f, "Dump process failed: {}", message), - } +impl From for PayloadError { + fn from(other: QueryPayloadError) -> Self { + Self::Query(other) } } -impl From for Error { - fn from(err: std::io::Error) -> Error { - Error::Internal(err.to_string()) - } -} - -impl From for Error { - fn from(err: HttpError) -> Error { - Error::Internal(err.to_string()) - } -} - -impl From for Error { - fn from(err: serde_json::error::Error) -> Error { - Error::Internal(err.to_string()) - } -} - -impl From for Error { - fn from(err: JsonPayloadError) -> Error { - match err { - JsonPayloadError::Deserialize(err) => { - Error::BadRequest(format!("Invalid JSON: {}", err)) - } - JsonPayloadError::Overflow => Error::PayloadTooLarge, - JsonPayloadError::ContentType => Error::UnsupportedMediaType, - JsonPayloadError::Payload(err) => { - Error::BadRequest(format!("Problem while decoding the request: {}", err)) - } - e => Error::Internal(format!("Unexpected Json error: {}", e)), - } - } -} - -impl From for Error { - fn from(err: QueryPayloadError) -> Error { - match err { - QueryPayloadError::Deserialize(err) => { - Error::BadRequest(format!("Invalid query parameters: {}", err)) - } - e => Error::Internal(format!("Unexpected query payload error: {}", e)), - } - } -} - -pub fn payload_error_handler>(err: E) -> ResponseError { - let error: Error = err.into(); - error.into() +pub fn payload_error_handler(err: E) -> ResponseError +where + E: Into, +{ + err.into().into() } diff --git a/meilisearch-http/src/helpers/authentication.rs b/meilisearch-http/src/helpers/authentication.rs index 54d5488f4..dddf57138 100644 --- a/meilisearch-http/src/helpers/authentication.rs +++ b/meilisearch-http/src/helpers/authentication.rs @@ -9,7 +9,7 @@ use futures::future::{ok, Future, Ready}; use futures::ready; use pin_project::pin_project; -use crate::error::{Error, ResponseError}; +use crate::error::{AuthenticationError, ResponseError}; use crate::Data; #[derive(Clone, Copy)] @@ -117,7 +117,8 @@ where AuthProj::NoHeader(req) => { match req.take() { Some(req) => { - let response = ResponseError::from(Error::MissingAuthorizationHeader); + let response = + ResponseError::from(AuthenticationError::MissingAuthorizationHeader); let response = response.error_response(); let response = req.into_response(response); Poll::Ready(Ok(response)) @@ -134,7 +135,8 @@ where .get("X-Meili-API-Key") .map(|h| h.to_str().map(String::from).unwrap_or_default()) .unwrap_or_default(); - let response = ResponseError::from(Error::InvalidToken(bad_token)); + let response = + ResponseError::from(AuthenticationError::InvalidToken(bad_token)); let response = response.error_response(); let response = req.into_response(response); Poll::Ready(Ok(response)) diff --git a/meilisearch-http/src/index/dump.rs b/meilisearch-http/src/index/dump.rs index 13e6cbc02..52a5cabd9 100644 --- a/meilisearch-http/src/index/dump.rs +++ b/meilisearch-http/src/index/dump.rs @@ -3,7 +3,7 @@ use std::io::{BufRead, BufReader, Write}; use std::path::Path; use std::sync::Arc; -use anyhow::{bail, Context}; +use anyhow::{Context, bail}; use heed::RoTxn; use indexmap::IndexMap; use milli::update::{IndexDocumentsMethod, UpdateFormat::JsonStream}; @@ -11,6 +11,7 @@ use serde::{Deserialize, Serialize}; use crate::option::IndexerOpts; +use super::error::Result; use super::{update_handler::UpdateHandler, Index, Settings, Unchecked}; #[derive(Serialize, Deserialize)] @@ -23,7 +24,7 @@ const META_FILE_NAME: &str = "meta.json"; const DATA_FILE_NAME: &str = "documents.jsonl"; impl Index { - pub fn dump(&self, path: impl AsRef) -> anyhow::Result<()> { + pub fn dump(&self, path: impl AsRef) -> Result<()> { // acquire write txn make sure any ongoing write is finished before we start. let txn = self.env.write_txn()?; @@ -33,7 +34,7 @@ impl Index { Ok(()) } - fn dump_documents(&self, txn: &RoTxn, path: impl AsRef) -> anyhow::Result<()> { + fn dump_documents(&self, txn: &RoTxn, path: impl AsRef) -> Result<()> { let document_file_path = path.as_ref().join(DATA_FILE_NAME); let mut document_file = File::create(&document_file_path)?; @@ -60,7 +61,7 @@ impl Index { Ok(()) } - fn dump_meta(&self, txn: &RoTxn, path: impl AsRef) -> anyhow::Result<()> { + fn dump_meta(&self, txn: &RoTxn, path: impl AsRef) -> Result<()> { let meta_file_path = path.as_ref().join(META_FILE_NAME); let mut meta_file = File::create(&meta_file_path)?; @@ -86,6 +87,7 @@ impl Index { .as_ref() .file_name() .with_context(|| format!("invalid dump index: {}", src.as_ref().display()))?; + let dst_dir_path = dst.as_ref().join("indexes").join(dir_name); create_dir_all(&dst_dir_path)?; diff --git a/meilisearch-http/src/index/error.rs b/meilisearch-http/src/index/error.rs new file mode 100644 index 000000000..b9bf71a3b --- /dev/null +++ b/meilisearch-http/src/index/error.rs @@ -0,0 +1,52 @@ +use std::error::Error; + +use meilisearch_error::{Code, ErrorCode}; +use serde_json::Value; + +use crate::error::MilliError; + +pub type Result = std::result::Result; + +#[derive(Debug, thiserror::Error)] +pub enum IndexError { + #[error("internal error: {0}")] + Internal(Box), + #[error("document with id {0} not found.")] + DocumentNotFound(String), + #[error("error with facet: {0}")] + Facet(#[from] FacetError), + #[error("{0}")] + Milli(#[from] milli::Error), +} + +internal_error!( + IndexError: std::io::Error, + heed::Error, + fst::Error, + serde_json::Error +); + +impl ErrorCode for IndexError { + fn error_code(&self) -> Code { + match self { + IndexError::Internal(_) => Code::Internal, + IndexError::DocumentNotFound(_) => Code::DocumentNotFound, + IndexError::Facet(e) => e.error_code(), + IndexError::Milli(e) => MilliError(e).error_code(), + } + } +} + +#[derive(Debug, thiserror::Error)] +pub enum FacetError { + #[error("invalid facet expression, 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, + } + } +} diff --git a/meilisearch-http/src/index/mod.rs b/meilisearch-http/src/index/mod.rs index 56958760a..ca1518a2e 100644 --- a/meilisearch-http/src/index/mod.rs +++ b/meilisearch-http/src/index/mod.rs @@ -5,19 +5,24 @@ use std::ops::Deref; use std::path::Path; use std::sync::Arc; -use anyhow::{bail, Context}; use heed::{EnvOpenOptions, RoTxn}; use milli::obkv_to_json; +use serde::{de::Deserializer, Deserialize}; use serde_json::{Map, Value}; use crate::helpers::EnvSizer; +use error::Result; + pub use search::{SearchQuery, SearchResult, DEFAULT_SEARCH_LIMIT}; -use serde::{de::Deserializer, Deserialize}; pub use updates::{Checked, Facets, Settings, Unchecked}; +use self::error::IndexError; + +pub mod error; +pub mod update_handler; + mod dump; mod search; -pub mod update_handler; mod updates; pub type Document = Map; @@ -33,7 +38,7 @@ impl Deref for Index { } } -pub fn deserialize_some<'de, T, D>(deserializer: D) -> Result, D::Error> +pub fn deserialize_some<'de, T, D>(deserializer: D) -> std::result::Result, D::Error> where T: Deserialize<'de>, D: Deserializer<'de>, @@ -42,7 +47,7 @@ where } impl Index { - pub fn open(path: impl AsRef, size: usize) -> anyhow::Result { + pub fn open(path: impl AsRef, size: usize) -> Result { create_dir_all(&path)?; let mut options = EnvOpenOptions::new(); options.map_size(size); @@ -50,12 +55,12 @@ impl Index { Ok(Index(Arc::new(index))) } - pub fn settings(&self) -> anyhow::Result> { + pub fn settings(&self) -> Result> { let txn = self.read_txn()?; self.settings_txn(&txn) } - pub fn settings_txn(&self, txn: &RoTxn) -> anyhow::Result> { + pub fn settings_txn(&self, txn: &RoTxn) -> Result> { let displayed_attributes = self .displayed_fields(&txn)? .map(|fields| fields.into_iter().map(String::from).collect()); @@ -64,10 +69,7 @@ impl Index { .searchable_fields(&txn)? .map(|fields| fields.into_iter().map(String::from).collect()); - let faceted_attributes = self - .faceted_fields(&txn)? - .into_iter() - .collect(); + let faceted_attributes = self.faceted_fields(&txn)?.into_iter().collect(); let criteria = self .criteria(&txn)? @@ -77,7 +79,7 @@ impl Index { let stop_words = self .stop_words(&txn)? - .map(|stop_words| -> anyhow::Result> { + .map(|stop_words| -> Result> { Ok(stop_words.stream().into_strs()?.into_iter().collect()) }) .transpose()? @@ -114,7 +116,7 @@ impl Index { offset: usize, limit: usize, attributes_to_retrieve: Option>, - ) -> anyhow::Result>> { + ) -> Result>> { let txn = self.read_txn()?; let fields_ids_map = self.fields_ids_map(&txn)?; @@ -138,7 +140,7 @@ impl Index { &self, doc_id: String, attributes_to_retrieve: Option>, - ) -> anyhow::Result> { + ) -> Result> { let txn = self.read_txn()?; let fields_ids_map = self.fields_ids_map(&txn)?; @@ -149,18 +151,18 @@ impl Index { let internal_id = self .external_documents_ids(&txn)? .get(doc_id.as_bytes()) - .with_context(|| format!("Document with id {} not found", doc_id))?; + .ok_or_else(|| IndexError::DocumentNotFound(doc_id.clone()))?; let document = self .documents(&txn, std::iter::once(internal_id))? .into_iter() .next() - .map(|(_, d)| d); + .map(|(_, d)| d) + .ok_or(IndexError::DocumentNotFound(doc_id))?; - match document { - Some(document) => Ok(obkv_to_json(&fields_to_display, &fields_ids_map, document)?), - None => bail!("Document with id {} not found", doc_id), - } + let document = obkv_to_json(&fields_to_display, &fields_ids_map, document)?; + + Ok(document) } pub fn size(&self) -> u64 { @@ -172,7 +174,7 @@ impl Index { txn: &heed::RoTxn, attributes_to_retrieve: &Option>, fields_ids_map: &milli::FieldsIdsMap, - ) -> anyhow::Result> { + ) -> Result> { let mut displayed_fields_ids = match self.displayed_fields_ids(&txn)? { Some(ids) => ids.into_iter().collect::>(), None => fields_ids_map.iter().map(|(id, _)| id).collect(), diff --git a/meilisearch-http/src/index/search.rs b/meilisearch-http/src/index/search.rs index 01650c9f5..0a658ec30 100644 --- a/meilisearch-http/src/index/search.rs +++ b/meilisearch-http/src/index/search.rs @@ -2,7 +2,6 @@ use std::borrow::Cow; use std::collections::{BTreeMap, BTreeSet, HashSet}; use std::time::Instant; -use anyhow::bail; use either::Either; use heed::RoTxn; use indexmap::IndexMap; @@ -11,6 +10,9 @@ use milli::{FilterCondition, FieldId, FieldsIdsMap, MatchingWords}; use serde::{Deserialize, Serialize}; use serde_json::Value; +use crate::index::error::FacetError; + +use super::error::Result; use super::Index; pub type Document = IndexMap; @@ -71,7 +73,7 @@ struct FormatOptions { } impl Index { - pub fn perform_search(&self, query: SearchQuery) -> anyhow::Result { + pub fn perform_search(&self, query: SearchQuery) -> Result { let before_search = Instant::now(); let rtxn = self.read_txn()?; @@ -96,7 +98,7 @@ impl Index { candidates, .. } = search.execute()?; - let mut documents = Vec::new(); + let fields_ids_map = self.fields_ids_map(&rtxn).unwrap(); let displayed_ids = self @@ -158,7 +160,11 @@ impl Index { let formatter = Formatter::new(&stop_words, (String::from(""), String::from(""))); - for (_id, obkv) in self.documents(&rtxn, documents_ids)? { + let mut documents = Vec::new(); + + let documents_iter = self.documents(&rtxn, documents_ids)?; + + for (_id, obkv) in documents_iter { let document = make_document(&to_retrieve_ids, &fields_ids_map, obkv)?; let formatted = format_fields( &fields_ids_map, @@ -167,6 +173,7 @@ impl Index { &matching_words, &formatted_options, )?; + let hit = SearchHit { document, formatted, @@ -182,7 +189,9 @@ impl Index { if fields.iter().all(|f| f != "*") { facet_distribution.facets(fields); } - Some(facet_distribution.candidates(candidates).execute()?) + let distribution = facet_distribution.candidates(candidates).execute()?; + + Some(distribution) } None => None, }; @@ -326,7 +335,7 @@ fn make_document( attributes_to_retrieve: &BTreeSet, field_ids_map: &FieldsIdsMap, obkv: obkv::KvReader, -) -> anyhow::Result { +) -> Result { let mut document = Document::new(); for attr in attributes_to_retrieve { if let Some(value) = obkv.get(*attr) { @@ -351,7 +360,7 @@ fn format_fields>( formatter: &Formatter, matching_words: &impl Matcher, formatted_options: &BTreeMap, -) -> anyhow::Result { +) -> Result { let mut document = Document::new(); for (id, format) in formatted_options { @@ -514,15 +523,14 @@ impl<'a, A: AsRef<[u8]>> Formatter<'a, A> { } } -fn parse_filter( - facets: &Value, - index: &Index, - txn: &RoTxn, -) -> anyhow::Result> { +fn parse_filter(facets: &Value, index: &Index, txn: &RoTxn) -> Result> { match facets { - Value::String(expr) => Ok(Some(FilterCondition::from_str(txn, index, expr)?)), + Value::String(expr) => { + let condition = FilterCondition::from_str(txn, index, expr)?; + Ok(Some(condition)) + } Value::Array(arr) => parse_filter_array(txn, index, arr), - v => bail!("Invalid facet expression, expected Array, found: {:?}", v), + v => Err(FacetError::InvalidExpression(&["Array"], v.clone()).into()), } } @@ -530,7 +538,7 @@ fn parse_filter_array( txn: &RoTxn, index: &Index, arr: &[Value], -) -> anyhow::Result> { +) -> Result> { let mut ands = Vec::new(); for value in arr { match value { @@ -540,15 +548,18 @@ fn parse_filter_array( for value in arr { match value { Value::String(s) => ors.push(s.clone()), - v => bail!("Invalid facet expression, expected String, found: {:?}", v), + v => { + return Err(FacetError::InvalidExpression(&["String"], v.clone()).into()) + } } } ands.push(Either::Left(ors)); } - v => bail!( - "Invalid facet expression, expected String or [String], found: {:?}", - v - ), + v => { + return Err( + FacetError::InvalidExpression(&["String", "[String]"], v.clone()).into(), + ) + } } } diff --git a/meilisearch-http/src/index/update_handler.rs b/meilisearch-http/src/index/update_handler.rs index 63a074abb..9896c9391 100644 --- a/meilisearch-http/src/index/update_handler.rs +++ b/meilisearch-http/src/index/update_handler.rs @@ -1,7 +1,6 @@ use std::fs::File; use crate::index::Index; -use anyhow::Result; use grenad::CompressionType; use milli::update::UpdateBuilder; use rayon::ThreadPool; @@ -87,7 +86,7 @@ impl UpdateHandler { match result { Ok(result) => Ok(meta.process(result)), - Err(e) => Err(meta.fail(e.to_string())), + Err(e) => Err(meta.fail(e.into())), } } } diff --git a/meilisearch-http/src/index/updates.rs b/meilisearch-http/src/index/updates.rs index ce327520e..6ace7e375 100644 --- a/meilisearch-http/src/index/updates.rs +++ b/meilisearch-http/src/index/updates.rs @@ -1,4 +1,4 @@ -use std::collections::{BTreeSet, BTreeMap, HashSet}; +use std::collections::{BTreeMap, BTreeSet, HashSet}; use std::io; use std::marker::PhantomData; use std::num::NonZeroUsize; @@ -10,9 +10,13 @@ use serde::{Deserialize, Serialize, Serializer}; use crate::index_controller::UpdateResult; +use super::error::Result; use super::{deserialize_some, Index}; -fn serialize_with_wildcard(field: &Option>>, s: S) -> Result +fn serialize_with_wildcard( + field: &Option>>, + s: S, +) -> std::result::Result where S: Serializer, { @@ -174,7 +178,7 @@ impl Index { content: Option, update_builder: UpdateBuilder, primary_key: Option<&str>, - ) -> anyhow::Result { + ) -> Result { let mut txn = self.write_txn()?; let result = self.update_documents_txn( &mut txn, @@ -196,13 +200,12 @@ impl Index { content: Option, update_builder: UpdateBuilder, primary_key: Option<&str>, - ) -> anyhow::Result { + ) -> Result { info!("performing document addition"); // Set the primary key if not set already, ignore if already set. if let (None, Some(primary_key)) = (self.primary_key(txn)?, primary_key) { - let mut builder = UpdateBuilder::new(0) - .settings(txn, &self); + let mut builder = UpdateBuilder::new(0).settings(txn, &self); builder.set_primary_key(primary_key.to_string()); builder.execute(|_, _| ())?; } @@ -228,18 +231,16 @@ impl Index { Ok(UpdateResult::DocumentsAddition(addition)) } - pub fn clear_documents(&self, update_builder: UpdateBuilder) -> anyhow::Result { + pub fn clear_documents(&self, update_builder: UpdateBuilder) -> Result { // We must use the write transaction of the update here. let mut wtxn = self.write_txn()?; let builder = update_builder.clear_documents(&mut wtxn, self); - match builder.execute() { - Ok(_count) => wtxn - .commit() - .and(Ok(UpdateResult::Other)) - .map_err(Into::into), - Err(e) => Err(e.into()), - } + let _count = builder.execute()?; + + wtxn.commit() + .and(Ok(UpdateResult::Other)) + .map_err(Into::into) } pub fn update_settings_txn<'a, 'b>( @@ -247,7 +248,7 @@ impl Index { txn: &mut heed::RwTxn<'a, 'b>, settings: &Settings, update_builder: UpdateBuilder, - ) -> anyhow::Result { + ) -> Result { // We must use the write transaction of the update here. let mut builder = update_builder.settings(txn, self); @@ -309,7 +310,7 @@ impl Index { &self, settings: &Settings, update_builder: UpdateBuilder, - ) -> anyhow::Result { + ) -> Result { let mut txn = self.write_txn()?; let result = self.update_settings_txn(&mut txn, settings, update_builder)?; txn.commit()?; @@ -320,7 +321,7 @@ impl Index { &self, document_ids: &[String], update_builder: UpdateBuilder, - ) -> anyhow::Result { + ) -> Result { let mut txn = self.write_txn()?; let mut builder = update_builder.delete_documents(&mut txn, self)?; @@ -329,13 +330,10 @@ impl Index { builder.delete_external_id(id); }); - match builder.execute() { - Ok(deleted) => txn - .commit() - .and(Ok(UpdateResult::DocumentDeletion { deleted })) - .map_err(Into::into), - Err(e) => Err(e.into()), - } + let deleted = builder.execute()?; + txn.commit() + .and(Ok(UpdateResult::DocumentDeletion { deleted })) + .map_err(Into::into) } } diff --git a/meilisearch-http/src/index_controller/dump_actor/actor.rs b/meilisearch-http/src/index_controller/dump_actor/actor.rs index c78079de6..4924df947 100644 --- a/meilisearch-http/src/index_controller/dump_actor/actor.rs +++ b/meilisearch-http/src/index_controller/dump_actor/actor.rs @@ -10,7 +10,8 @@ use tokio::sync::{mpsc, oneshot, RwLock}; use update_actor::UpdateActorHandle; use uuid_resolver::UuidResolverHandle; -use super::{DumpError, DumpInfo, DumpMsg, DumpResult, DumpStatus, DumpTask}; +use super::error::{DumpActorError, Result}; +use super::{DumpInfo, DumpMsg, DumpStatus, DumpTask}; use crate::index_controller::{update_actor, uuid_resolver}; pub const CONCURRENT_DUMP_MSG: usize = 10; @@ -95,14 +96,14 @@ where } } - async fn handle_create_dump(&self, ret: oneshot::Sender>) { + async fn handle_create_dump(&self, ret: oneshot::Sender>) { let uid = generate_uid(); let info = DumpInfo::new(uid.clone(), DumpStatus::InProgress); let _lock = match self.lock.try_lock() { Some(lock) => lock, None => { - ret.send(Err(DumpError::DumpAlreadyRunning)) + ret.send(Err(DumpActorError::DumpAlreadyRunning)) .expect("Dump actor is dead"); return; } @@ -147,10 +148,10 @@ where }; } - async fn handle_dump_info(&self, uid: String) -> DumpResult { + async fn handle_dump_info(&self, uid: String) -> Result { match self.dump_infos.read().await.get(&uid) { Some(info) => Ok(info.clone()), - _ => Err(DumpError::DumpDoesNotExist(uid)), + _ => Err(DumpActorError::DumpDoesNotExist(uid)), } } } diff --git a/meilisearch-http/src/index_controller/dump_actor/error.rs b/meilisearch-http/src/index_controller/dump_actor/error.rs new file mode 100644 index 000000000..ed693d5f3 --- /dev/null +++ b/meilisearch-http/src/index_controller/dump_actor/error.rs @@ -0,0 +1,52 @@ +use meilisearch_error::{Code, ErrorCode}; + +use crate::index_controller::update_actor::error::UpdateActorError; +use crate::index_controller::uuid_resolver::error::UuidResolverError; + +pub type Result = std::result::Result; + +#[derive(thiserror::Error, Debug)] +pub enum DumpActorError { + #[error("dump already running")] + DumpAlreadyRunning, + #[error("dump `{0}` does not exist")] + DumpDoesNotExist(String), + #[error("internal error: {0}")] + Internal(Box), + #[error("error while dumping uuids: {0}")] + UuidResolver(#[from] UuidResolverError), + #[error("error while dumping updates: {0}")] + UpdateActor(#[from] UpdateActorError), +} + +macro_rules! internal_error { + ($($other:path), *) => { + $( + impl From<$other> for DumpActorError { + fn from(other: $other) -> Self { + Self::Internal(Box::new(other)) + } + } + )* + } +} + +internal_error!( + heed::Error, + std::io::Error, + tokio::task::JoinError, + serde_json::error::Error, + tempfile::PersistError +); + +impl ErrorCode for DumpActorError { + fn error_code(&self) -> Code { + match self { + DumpActorError::DumpAlreadyRunning => Code::DumpAlreadyInProgress, + DumpActorError::DumpDoesNotExist(_) => Code::DocumentNotFound, + DumpActorError::Internal(_) => Code::Internal, + DumpActorError::UuidResolver(e) => e.error_code(), + DumpActorError::UpdateActor(e) => e.error_code(), + } + } +} diff --git a/meilisearch-http/src/index_controller/dump_actor/handle_impl.rs b/meilisearch-http/src/index_controller/dump_actor/handle_impl.rs index ab91aeae6..db11fb8fc 100644 --- a/meilisearch-http/src/index_controller/dump_actor/handle_impl.rs +++ b/meilisearch-http/src/index_controller/dump_actor/handle_impl.rs @@ -3,7 +3,8 @@ use std::path::Path; use actix_web::web::Bytes; use tokio::sync::{mpsc, oneshot}; -use super::{DumpActor, DumpActorHandle, DumpInfo, DumpMsg, DumpResult}; +use super::error::Result; +use super::{DumpActor, DumpActorHandle, DumpInfo, DumpMsg}; #[derive(Clone)] pub struct DumpActorHandleImpl { @@ -12,14 +13,14 @@ pub struct DumpActorHandleImpl { #[async_trait::async_trait] impl DumpActorHandle for DumpActorHandleImpl { - async fn create_dump(&self) -> DumpResult { + async fn create_dump(&self) -> Result { let (ret, receiver) = oneshot::channel(); let msg = DumpMsg::CreateDump { ret }; let _ = self.sender.send(msg).await; receiver.await.expect("IndexActor has been killed") } - async fn dump_info(&self, uid: String) -> DumpResult { + async fn dump_info(&self, uid: String) -> Result { let (ret, receiver) = oneshot::channel(); let msg = DumpMsg::DumpInfo { ret, uid }; let _ = self.sender.send(msg).await; diff --git a/meilisearch-http/src/index_controller/dump_actor/message.rs b/meilisearch-http/src/index_controller/dump_actor/message.rs index dff9f5954..6c9dded9f 100644 --- a/meilisearch-http/src/index_controller/dump_actor/message.rs +++ b/meilisearch-http/src/index_controller/dump_actor/message.rs @@ -1,13 +1,14 @@ use tokio::sync::oneshot; -use super::{DumpInfo, DumpResult}; +use super::error::Result; +use super::DumpInfo; pub enum DumpMsg { CreateDump { - ret: oneshot::Sender>, + ret: oneshot::Sender>, }, DumpInfo { uid: String, - ret: oneshot::Sender>, + ret: oneshot::Sender>, }, } diff --git a/meilisearch-http/src/index_controller/dump_actor/mod.rs b/meilisearch-http/src/index_controller/dump_actor/mod.rs index 66f081e87..057b8ebaf 100644 --- a/meilisearch-http/src/index_controller/dump_actor/mod.rs +++ b/meilisearch-http/src/index_controller/dump_actor/mod.rs @@ -3,11 +3,10 @@ use std::path::{Path, PathBuf}; use anyhow::Context; use chrono::{DateTime, Utc}; -use log::{error, info, warn}; +use log::{info, warn}; #[cfg(test)] use mockall::automock; use serde::{Deserialize, Serialize}; -use thiserror::Error; use tokio::fs::create_dir_all; use loaders::v1::MetadataV1; @@ -18,39 +17,28 @@ pub use handle_impl::*; pub use message::DumpMsg; use super::{update_actor::UpdateActorHandle, uuid_resolver::UuidResolverHandle}; +use crate::index_controller::dump_actor::error::DumpActorError; use crate::{helpers::compression, option::IndexerOpts}; +use error::Result; mod actor; +pub mod error; mod handle_impl; mod loaders; mod message; const META_FILE_NAME: &str = "metadata.json"; -pub type DumpResult = std::result::Result; - -#[derive(Error, Debug)] -pub enum DumpError { - #[error("error with index: {0}")] - Error(#[from] anyhow::Error), - #[error("Heed error: {0}")] - HeedError(#[from] heed::Error), - #[error("dump already running")] - DumpAlreadyRunning, - #[error("dump `{0}` does not exist")] - DumpDoesNotExist(String), -} - #[async_trait::async_trait] #[cfg_attr(test, automock)] pub trait DumpActorHandle { /// Start the creation of a dump /// Implementation: [handle_impl::DumpActorHandleImpl::create_dump] - async fn create_dump(&self) -> DumpResult; + async fn create_dump(&self) -> Result; /// Return the status of an already created dump /// Implementation: [handle_impl::DumpActorHandleImpl::dump_status] - async fn dump_info(&self, uid: String) -> DumpResult; + async fn dump_info(&self, uid: String) -> Result; } #[derive(Debug, Serialize, Deserialize)] @@ -175,7 +163,7 @@ where U: UuidResolverHandle + Send + Sync + Clone + 'static, P: UpdateActorHandle + Send + Sync + Clone + 'static, { - async fn run(self) -> anyhow::Result<()> { + async fn run(self) -> Result<()> { info!("Performing dump."); create_dir_all(&self.path).await?; @@ -196,9 +184,10 @@ where .dump(uuids, temp_dump_path.clone()) .await?; - let dump_path = tokio::task::spawn_blocking(move || -> anyhow::Result { + let dump_path = tokio::task::spawn_blocking(move || -> Result { let temp_dump_file = tempfile::NamedTempFile::new_in(&self.path)?; - compression::to_tar_gz(temp_dump_path, temp_dump_file.path())?; + compression::to_tar_gz(temp_dump_path, temp_dump_file.path()) + .map_err(|e| DumpActorError::Internal(e.into()))?; let dump_path = self.path.join(self.uid).with_extension("dump"); temp_dump_file.persist(&dump_path)?; diff --git a/meilisearch-http/src/index_controller/error.rs b/meilisearch-http/src/index_controller/error.rs new file mode 100644 index 000000000..c01eb24c0 --- /dev/null +++ b/meilisearch-http/src/index_controller/error.rs @@ -0,0 +1,40 @@ +use meilisearch_error::Code; +use meilisearch_error::ErrorCode; + +use crate::index::error::IndexError; + +use super::dump_actor::error::DumpActorError; +use super::index_actor::error::IndexActorError; +use super::update_actor::error::UpdateActorError; +use super::uuid_resolver::error::UuidResolverError; + +pub type Result = std::result::Result; + +#[derive(Debug, thiserror::Error)] +pub enum IndexControllerError { + #[error("missing index uid")] + MissingUid, + #[error("index resolution error: {0}")] + Uuid(#[from] UuidResolverError), + #[error("error with index: {0}")] + IndexActor(#[from] IndexActorError), + #[error("error with update: {0}")] + UpdateActor(#[from] UpdateActorError), + #[error("error with dump: {0}")] + DumpActor(#[from] DumpActorError), + #[error("error with index: {0}")] + IndexError(#[from] IndexError), +} + +impl ErrorCode for IndexControllerError { + fn error_code(&self) -> Code { + match self { + IndexControllerError::MissingUid => Code::InvalidIndexUid, + IndexControllerError::Uuid(e) => e.error_code(), + IndexControllerError::IndexActor(e) => e.error_code(), + IndexControllerError::UpdateActor(e) => e.error_code(), + IndexControllerError::DumpActor(e) => e.error_code(), + IndexControllerError::IndexError(e) => e.error_code(), + } + } +} diff --git a/meilisearch-http/src/index_controller/index_actor/actor.rs b/meilisearch-http/src/index_controller/index_actor/actor.rs index c35e685a8..f7b1a0981 100644 --- a/meilisearch-http/src/index_controller/index_actor/actor.rs +++ b/meilisearch-http/src/index_controller/index_actor/actor.rs @@ -19,7 +19,8 @@ use crate::index_controller::{ }; use crate::option::IndexerOpts; -use super::{IndexError, IndexMeta, IndexMsg, IndexResult, IndexSettings, IndexStore}; +use super::error::{IndexActorError, Result}; +use super::{IndexMeta, IndexMsg, IndexSettings, IndexStore}; pub const CONCURRENT_INDEX_MSG: usize = 10; @@ -30,7 +31,7 @@ pub struct IndexActor { } impl IndexActor { - pub fn new(receiver: mpsc::Receiver, store: S) -> IndexResult { + pub fn new(receiver: mpsc::Receiver, store: S) -> anyhow::Result { let options = IndexerOpts::default(); let update_handler = UpdateHandler::new(&options)?; let update_handler = Arc::new(update_handler); @@ -137,20 +138,21 @@ impl IndexActor { } } - async fn handle_search(&self, uuid: Uuid, query: SearchQuery) -> anyhow::Result { + async fn handle_search(&self, uuid: Uuid, query: SearchQuery) -> Result { let index = self .store .get(uuid) .await? - .ok_or(IndexError::UnexistingIndex)?; - spawn_blocking(move || index.perform_search(query)).await? + .ok_or(IndexActorError::UnexistingIndex)?; + let result = spawn_blocking(move || index.perform_search(query)).await??; + Ok(result) } async fn handle_create_index( &self, uuid: Uuid, primary_key: Option, - ) -> IndexResult { + ) -> Result { let index = self.store.create(uuid, primary_key).await?; let meta = spawn_blocking(move || IndexMeta::new(&index)).await??; Ok(meta) @@ -161,7 +163,7 @@ impl IndexActor { uuid: Uuid, meta: Processing, data: Option, - ) -> IndexResult> { + ) -> Result> { debug!("Processing update {}", meta.id()); let update_handler = self.update_handler.clone(); let index = match self.store.get(uuid).await? { @@ -172,12 +174,12 @@ impl IndexActor { Ok(spawn_blocking(move || update_handler.handle_update(meta, data, index)).await?) } - async fn handle_settings(&self, uuid: Uuid) -> IndexResult> { + async fn handle_settings(&self, uuid: Uuid) -> Result> { let index = self .store .get(uuid) .await? - .ok_or(IndexError::UnexistingIndex)?; + .ok_or(IndexActorError::UnexistingIndex)?; let result = spawn_blocking(move || index.settings()).await??; Ok(result) } @@ -188,12 +190,12 @@ impl IndexActor { offset: usize, limit: usize, attributes_to_retrieve: Option>, - ) -> IndexResult> { + ) -> Result> { let index = self .store .get(uuid) .await? - .ok_or(IndexError::UnexistingIndex)?; + .ok_or(IndexActorError::UnexistingIndex)?; let result = spawn_blocking(move || index.retrieve_documents(offset, limit, attributes_to_retrieve)) .await??; @@ -206,12 +208,12 @@ impl IndexActor { uuid: Uuid, doc_id: String, attributes_to_retrieve: Option>, - ) -> IndexResult { + ) -> Result { let index = self .store .get(uuid) .await? - .ok_or(IndexError::UnexistingIndex)?; + .ok_or(IndexActorError::UnexistingIndex)?; let result = spawn_blocking(move || index.retrieve_document(doc_id, attributes_to_retrieve)) @@ -220,7 +222,7 @@ impl IndexActor { Ok(result) } - async fn handle_delete(&self, uuid: Uuid) -> IndexResult<()> { + async fn handle_delete(&self, uuid: Uuid) -> Result<()> { let index = self.store.delete(uuid).await?; if let Some(index) = index { @@ -237,13 +239,13 @@ impl IndexActor { Ok(()) } - async fn handle_get_meta(&self, uuid: Uuid) -> IndexResult { + async fn handle_get_meta(&self, uuid: Uuid) -> Result { match self.store.get(uuid).await? { Some(index) => { let meta = spawn_blocking(move || IndexMeta::new(&index)).await??; Ok(meta) } - None => Err(IndexError::UnexistingIndex), + None => Err(IndexActorError::UnexistingIndex), } } @@ -251,23 +253,22 @@ impl IndexActor { &self, uuid: Uuid, index_settings: IndexSettings, - ) -> IndexResult { + ) -> Result { let index = self .store .get(uuid) .await? - .ok_or(IndexError::UnexistingIndex)?; + .ok_or(IndexActorError::UnexistingIndex)?; let result = spawn_blocking(move || match index_settings.primary_key { Some(primary_key) => { let mut txn = index.write_txn()?; if index.primary_key(&txn)?.is_some() { - return Err(IndexError::ExistingPrimaryKey); + return Err(IndexActorError::ExistingPrimaryKey); } let mut builder = UpdateBuilder::new(0).settings(&mut txn, &index); builder.set_primary_key(primary_key); - builder.execute(|_, _| ()) - .map_err(|e| IndexError::Internal(e.to_string()))?; + builder.execute(|_, _| ())?; let meta = IndexMeta::new_txn(&index, &txn)?; txn.commit()?; Ok(meta) @@ -282,7 +283,7 @@ impl IndexActor { Ok(result) } - async fn handle_snapshot(&self, uuid: Uuid, mut path: PathBuf) -> IndexResult<()> { + async fn handle_snapshot(&self, uuid: Uuid, mut path: PathBuf) -> Result<()> { use tokio::fs::create_dir_all; path.push("indexes"); @@ -294,7 +295,7 @@ impl IndexActor { create_dir_all(&index_path).await?; index_path.push("data.mdb"); - spawn_blocking(move || -> anyhow::Result<()> { + spawn_blocking(move || -> Result<()> { // Get write txn to wait for ongoing write transaction before snapshot. let _txn = index.write_txn()?; index @@ -310,12 +311,12 @@ impl IndexActor { /// Create a `documents.jsonl` and a `settings.json` in `path/uid/` with a dump of all the /// documents and all the settings. - async fn handle_dump(&self, uuid: Uuid, path: PathBuf) -> IndexResult<()> { + async fn handle_dump(&self, uuid: Uuid, path: PathBuf) -> Result<()> { let index = self .store .get(uuid) .await? - .ok_or(IndexError::UnexistingIndex)?; + .ok_or(IndexActorError::UnexistingIndex)?; let path = path.join(format!("indexes/index-{}/", uuid)); fs::create_dir_all(&path).await?; @@ -325,20 +326,19 @@ impl IndexActor { Ok(()) } - async fn handle_get_stats(&self, uuid: Uuid) -> IndexResult { + async fn handle_get_stats(&self, uuid: Uuid) -> Result { let index = self .store .get(uuid) .await? - .ok_or(IndexError::UnexistingIndex)?; + .ok_or(IndexActorError::UnexistingIndex)?; spawn_blocking(move || { let rtxn = index.read_txn()?; Ok(IndexStats { size: index.size(), - number_of_documents: index.number_of_documents(&rtxn) - .map_err(|e| IndexError::Internal(e.to_string()))?, + number_of_documents: index.number_of_documents(&rtxn)?, is_indexing: None, fields_distribution: index.fields_distribution(&rtxn)?, }) diff --git a/meilisearch-http/src/index_controller/index_actor/error.rs b/meilisearch-http/src/index_controller/index_actor/error.rs new file mode 100644 index 000000000..244797234 --- /dev/null +++ b/meilisearch-http/src/index_controller/index_actor/error.rs @@ -0,0 +1,48 @@ +use meilisearch_error::{Code, ErrorCode}; + +use crate::{error::MilliError, index::error::IndexError}; + +pub type Result = std::result::Result; + +#[derive(thiserror::Error, Debug)] +pub enum IndexActorError { + #[error("index error: {0}")] + IndexError(#[from] IndexError), + #[error("index already exists")] + IndexAlreadyExists, + #[error("index doesn't exists")] + UnexistingIndex, + #[error("existing primary key")] + ExistingPrimaryKey, + #[error("internal Index Error: {0}")] + Internal(Box), + #[error("{0}")] + Milli(#[from] milli::Error), +} + +macro_rules! internal_error { + ($($other:path), *) => { + $( + impl From<$other> for IndexActorError { + fn from(other: $other) -> Self { + Self::Internal(Box::new(other)) + } + } + )* + } +} + +internal_error!(heed::Error, tokio::task::JoinError, std::io::Error); + +impl ErrorCode for IndexActorError { + fn error_code(&self) -> Code { + match self { + IndexActorError::IndexError(e) => e.error_code(), + IndexActorError::IndexAlreadyExists => Code::IndexAlreadyExists, + IndexActorError::UnexistingIndex => Code::IndexNotFound, + IndexActorError::ExistingPrimaryKey => Code::PrimaryKeyAlreadyPresent, + IndexActorError::Internal(_) => Code::Internal, + IndexActorError::Milli(e) => MilliError(e).error_code(), + } + } +} diff --git a/meilisearch-http/src/index_controller/index_actor/handle_impl.rs b/meilisearch-http/src/index_controller/index_actor/handle_impl.rs index 6bf83c647..231a3a44b 100644 --- a/meilisearch-http/src/index_controller/index_actor/handle_impl.rs +++ b/meilisearch-http/src/index_controller/index_actor/handle_impl.rs @@ -12,7 +12,8 @@ use crate::{ index_controller::{Failed, Processed}, }; -use super::{IndexActor, IndexActorHandle, IndexMeta, IndexMsg, IndexResult, MapIndexStore}; +use super::error::Result; +use super::{IndexActor, IndexActorHandle, IndexMeta, IndexMsg, MapIndexStore}; #[derive(Clone)] pub struct IndexActorHandleImpl { @@ -21,11 +22,7 @@ pub struct IndexActorHandleImpl { #[async_trait::async_trait] impl IndexActorHandle for IndexActorHandleImpl { - async fn create_index( - &self, - uuid: Uuid, - primary_key: Option, - ) -> IndexResult { + async fn create_index(&self, uuid: Uuid, primary_key: Option) -> Result { let (ret, receiver) = oneshot::channel(); let msg = IndexMsg::CreateIndex { ret, @@ -41,7 +38,7 @@ impl IndexActorHandle for IndexActorHandleImpl { uuid: Uuid, meta: Processing, data: Option, - ) -> anyhow::Result> { + ) -> Result> { let (ret, receiver) = oneshot::channel(); let msg = IndexMsg::Update { ret, @@ -53,14 +50,14 @@ impl IndexActorHandle for IndexActorHandleImpl { Ok(receiver.await.expect("IndexActor has been killed")?) } - async fn search(&self, uuid: Uuid, query: SearchQuery) -> IndexResult { + async fn search(&self, uuid: Uuid, query: SearchQuery) -> Result { let (ret, receiver) = oneshot::channel(); let msg = IndexMsg::Search { uuid, query, ret }; let _ = self.sender.send(msg).await; Ok(receiver.await.expect("IndexActor has been killed")?) } - async fn settings(&self, uuid: Uuid) -> IndexResult> { + async fn settings(&self, uuid: Uuid) -> Result> { let (ret, receiver) = oneshot::channel(); let msg = IndexMsg::Settings { uuid, ret }; let _ = self.sender.send(msg).await; @@ -73,7 +70,7 @@ impl IndexActorHandle for IndexActorHandleImpl { offset: usize, limit: usize, attributes_to_retrieve: Option>, - ) -> IndexResult> { + ) -> Result> { let (ret, receiver) = oneshot::channel(); let msg = IndexMsg::Documents { uuid, @@ -91,7 +88,7 @@ impl IndexActorHandle for IndexActorHandleImpl { uuid: Uuid, doc_id: String, attributes_to_retrieve: Option>, - ) -> IndexResult { + ) -> Result { let (ret, receiver) = oneshot::channel(); let msg = IndexMsg::Document { uuid, @@ -103,25 +100,21 @@ impl IndexActorHandle for IndexActorHandleImpl { Ok(receiver.await.expect("IndexActor has been killed")?) } - async fn delete(&self, uuid: Uuid) -> IndexResult<()> { + async fn delete(&self, uuid: Uuid) -> Result<()> { let (ret, receiver) = oneshot::channel(); let msg = IndexMsg::Delete { uuid, ret }; let _ = self.sender.send(msg).await; Ok(receiver.await.expect("IndexActor has been killed")?) } - async fn get_index_meta(&self, uuid: Uuid) -> IndexResult { + async fn get_index_meta(&self, uuid: Uuid) -> Result { let (ret, receiver) = oneshot::channel(); let msg = IndexMsg::GetMeta { uuid, ret }; let _ = self.sender.send(msg).await; Ok(receiver.await.expect("IndexActor has been killed")?) } - async fn update_index( - &self, - uuid: Uuid, - index_settings: IndexSettings, - ) -> IndexResult { + async fn update_index(&self, uuid: Uuid, index_settings: IndexSettings) -> Result { let (ret, receiver) = oneshot::channel(); let msg = IndexMsg::UpdateIndex { uuid, @@ -132,21 +125,21 @@ impl IndexActorHandle for IndexActorHandleImpl { Ok(receiver.await.expect("IndexActor has been killed")?) } - async fn snapshot(&self, uuid: Uuid, path: PathBuf) -> IndexResult<()> { + async fn snapshot(&self, uuid: Uuid, path: PathBuf) -> Result<()> { let (ret, receiver) = oneshot::channel(); let msg = IndexMsg::Snapshot { uuid, path, ret }; let _ = self.sender.send(msg).await; Ok(receiver.await.expect("IndexActor has been killed")?) } - async fn dump(&self, uuid: Uuid, path: PathBuf) -> IndexResult<()> { + async fn dump(&self, uuid: Uuid, path: PathBuf) -> Result<()> { let (ret, receiver) = oneshot::channel(); let msg = IndexMsg::Dump { uuid, path, ret }; let _ = self.sender.send(msg).await; Ok(receiver.await.expect("IndexActor has been killed")?) } - async fn get_index_stats(&self, uuid: Uuid) -> IndexResult { + async fn get_index_stats(&self, uuid: Uuid) -> Result { let (ret, receiver) = oneshot::channel(); let msg = IndexMsg::GetStats { uuid, ret }; let _ = self.sender.send(msg).await; diff --git a/meilisearch-http/src/index_controller/index_actor/message.rs b/meilisearch-http/src/index_controller/index_actor/message.rs index e7304d56c..415b90e4b 100644 --- a/meilisearch-http/src/index_controller/index_actor/message.rs +++ b/meilisearch-http/src/index_controller/index_actor/message.rs @@ -3,10 +3,11 @@ use std::path::PathBuf; use tokio::sync::oneshot; use uuid::Uuid; +use super::error::Result as IndexResult; use crate::index::{Checked, Document, SearchQuery, SearchResult, Settings}; use crate::index_controller::{Failed, IndexStats, Processed, Processing}; -use super::{IndexMeta, IndexResult, IndexSettings}; +use super::{IndexMeta, IndexSettings}; #[allow(clippy::large_enum_variant)] pub enum IndexMsg { @@ -24,7 +25,7 @@ pub enum IndexMsg { Search { uuid: Uuid, query: SearchQuery, - ret: oneshot::Sender>, + ret: oneshot::Sender>, }, Settings { uuid: Uuid, diff --git a/meilisearch-http/src/index_controller/index_actor/mod.rs b/meilisearch-http/src/index_controller/index_actor/mod.rs index b54a676b0..4085dc0bd 100644 --- a/meilisearch-http/src/index_controller/index_actor/mod.rs +++ b/meilisearch-http/src/index_controller/index_actor/mod.rs @@ -5,7 +5,6 @@ use chrono::{DateTime, Utc}; #[cfg(test)] use mockall::automock; use serde::{Deserialize, Serialize}; -use thiserror::Error; use uuid::Uuid; use actor::IndexActor; @@ -16,16 +15,16 @@ use store::{IndexStore, MapIndexStore}; use crate::index::{Checked, Document, Index, SearchQuery, SearchResult, Settings}; use crate::index_controller::{Failed, IndexStats, Processed, Processing}; +use error::Result; use super::IndexSettings; mod actor; +pub mod error; mod handle_impl; mod message; mod store; -pub type IndexResult = std::result::Result; - #[derive(Debug, Serialize, Deserialize, Clone)] #[serde(rename_all = "camelCase")] pub struct IndexMeta { @@ -35,18 +34,14 @@ pub struct IndexMeta { } impl IndexMeta { - fn new(index: &Index) -> IndexResult { + fn new(index: &Index) -> Result { let txn = index.read_txn()?; Self::new_txn(index, &txn) } - fn new_txn(index: &Index, txn: &heed::RoTxn) -> IndexResult { - let created_at = index - .created_at(&txn) - .map_err(|e| IndexError::Internal(e.to_string()))?; - let updated_at = index - .updated_at(&txn) - .map_err(|e| IndexError::Internal(e.to_string()))?; + fn new_txn(index: &Index, txn: &heed::RoTxn) -> Result { + let created_at = index.created_at(&txn)?; + let updated_at = index.updated_at(&txn)?; let primary_key = index.primary_key(&txn)?.map(String::from); Ok(Self { created_at, @@ -56,50 +51,18 @@ impl IndexMeta { } } -#[derive(Error, Debug)] -pub enum IndexError { - #[error("index already exists")] - IndexAlreadyExists, - #[error("Index doesn't exists")] - UnexistingIndex, - #[error("Existing primary key")] - ExistingPrimaryKey, - #[error("Internal Index Error: {0}")] - Internal(String), -} - -macro_rules! internal_error { - ($($other:path), *) => { - $( - impl From<$other> for IndexError { - fn from(other: $other) -> Self { - Self::Internal(other.to_string()) - } - } - )* - } -} - -internal_error!( - anyhow::Error, - heed::Error, - tokio::task::JoinError, - std::io::Error -); - #[async_trait::async_trait] #[cfg_attr(test, automock)] pub trait IndexActorHandle { - async fn create_index(&self, uuid: Uuid, primary_key: Option) - -> IndexResult; + async fn create_index(&self, uuid: Uuid, primary_key: Option) -> Result; async fn update( &self, uuid: Uuid, meta: Processing, data: Option, - ) -> anyhow::Result>; - async fn search(&self, uuid: Uuid, query: SearchQuery) -> IndexResult; - async fn settings(&self, uuid: Uuid) -> IndexResult>; + ) -> Result>; + async fn search(&self, uuid: Uuid, query: SearchQuery) -> Result; + async fn settings(&self, uuid: Uuid) -> Result>; async fn documents( &self, @@ -107,23 +70,19 @@ pub trait IndexActorHandle { offset: usize, limit: usize, attributes_to_retrieve: Option>, - ) -> IndexResult>; + ) -> Result>; async fn document( &self, uuid: Uuid, doc_id: String, attributes_to_retrieve: Option>, - ) -> IndexResult; - async fn delete(&self, uuid: Uuid) -> IndexResult<()>; - async fn get_index_meta(&self, uuid: Uuid) -> IndexResult; - async fn update_index( - &self, - uuid: Uuid, - index_settings: IndexSettings, - ) -> IndexResult; - async fn snapshot(&self, uuid: Uuid, path: PathBuf) -> IndexResult<()>; - async fn dump(&self, uuid: Uuid, path: PathBuf) -> IndexResult<()>; - async fn get_index_stats(&self, uuid: Uuid) -> IndexResult; + ) -> Result; + async fn delete(&self, uuid: Uuid) -> Result<()>; + async fn get_index_meta(&self, uuid: Uuid) -> Result; + async fn update_index(&self, uuid: Uuid, index_settings: IndexSettings) -> Result; + async fn snapshot(&self, uuid: Uuid, path: PathBuf) -> Result<()>; + async fn dump(&self, uuid: Uuid, path: PathBuf) -> Result<()>; + async fn get_index_stats(&self, uuid: Uuid) -> Result; } #[cfg(test)] @@ -135,11 +94,7 @@ mod test { #[async_trait::async_trait] /// Useful for passing around an `Arc` in tests. impl IndexActorHandle for Arc { - async fn create_index( - &self, - uuid: Uuid, - primary_key: Option, - ) -> IndexResult { + async fn create_index(&self, uuid: Uuid, primary_key: Option) -> Result { self.as_ref().create_index(uuid, primary_key).await } @@ -148,15 +103,15 @@ mod test { uuid: Uuid, meta: Processing, data: Option, - ) -> anyhow::Result> { + ) -> Result> { self.as_ref().update(uuid, meta, data).await } - async fn search(&self, uuid: Uuid, query: SearchQuery) -> IndexResult { + async fn search(&self, uuid: Uuid, query: SearchQuery) -> Result { self.as_ref().search(uuid, query).await } - async fn settings(&self, uuid: Uuid) -> IndexResult> { + async fn settings(&self, uuid: Uuid) -> Result> { self.as_ref().settings(uuid).await } @@ -166,7 +121,7 @@ mod test { offset: usize, limit: usize, attributes_to_retrieve: Option>, - ) -> IndexResult> { + ) -> Result> { self.as_ref() .documents(uuid, offset, limit, attributes_to_retrieve) .await @@ -177,17 +132,17 @@ mod test { uuid: Uuid, doc_id: String, attributes_to_retrieve: Option>, - ) -> IndexResult { + ) -> Result { self.as_ref() .document(uuid, doc_id, attributes_to_retrieve) .await } - async fn delete(&self, uuid: Uuid) -> IndexResult<()> { + async fn delete(&self, uuid: Uuid) -> Result<()> { self.as_ref().delete(uuid).await } - async fn get_index_meta(&self, uuid: Uuid) -> IndexResult { + async fn get_index_meta(&self, uuid: Uuid) -> Result { self.as_ref().get_index_meta(uuid).await } @@ -195,19 +150,19 @@ mod test { &self, uuid: Uuid, index_settings: IndexSettings, - ) -> IndexResult { + ) -> Result { self.as_ref().update_index(uuid, index_settings).await } - async fn snapshot(&self, uuid: Uuid, path: PathBuf) -> IndexResult<()> { + async fn snapshot(&self, uuid: Uuid, path: PathBuf) -> Result<()> { self.as_ref().snapshot(uuid, path).await } - async fn dump(&self, uuid: Uuid, path: PathBuf) -> IndexResult<()> { + async fn dump(&self, uuid: Uuid, path: PathBuf) -> Result<()> { self.as_ref().dump(uuid, path).await } - async fn get_index_stats(&self, uuid: Uuid) -> IndexResult { + async fn get_index_stats(&self, uuid: Uuid) -> Result { self.as_ref().get_index_stats(uuid).await } } diff --git a/meilisearch-http/src/index_controller/index_actor/store.rs b/meilisearch-http/src/index_controller/index_actor/store.rs index 39a6e64a6..2cfda61b5 100644 --- a/meilisearch-http/src/index_controller/index_actor/store.rs +++ b/meilisearch-http/src/index_controller/index_actor/store.rs @@ -8,16 +8,16 @@ use tokio::sync::RwLock; use tokio::task::spawn_blocking; use uuid::Uuid; -use super::{IndexError, IndexResult}; +use super::error::{IndexActorError, Result}; use crate::index::Index; type AsyncMap = Arc>>; #[async_trait::async_trait] pub trait IndexStore { - async fn create(&self, uuid: Uuid, primary_key: Option) -> IndexResult; - async fn get(&self, uuid: Uuid) -> IndexResult>; - async fn delete(&self, uuid: Uuid) -> IndexResult>; + async fn create(&self, uuid: Uuid, primary_key: Option) -> Result; + async fn get(&self, uuid: Uuid) -> Result>; + async fn delete(&self, uuid: Uuid) -> Result>; } pub struct MapIndexStore { @@ -40,7 +40,7 @@ impl MapIndexStore { #[async_trait::async_trait] impl IndexStore for MapIndexStore { - async fn create(&self, uuid: Uuid, primary_key: Option) -> IndexResult { + async fn create(&self, uuid: Uuid, primary_key: Option) -> Result { // We need to keep the lock until we are sure the db file has been opened correclty, to // ensure that another db is not created at the same time. let mut lock = self.index_store.write().await; @@ -50,19 +50,18 @@ impl IndexStore for MapIndexStore { } let path = self.path.join(format!("index-{}", uuid)); if path.exists() { - return Err(IndexError::IndexAlreadyExists); + return Err(IndexActorError::IndexAlreadyExists); } let index_size = self.index_size; - let index = spawn_blocking(move || -> IndexResult { + let index = spawn_blocking(move || -> Result { let index = Index::open(path, index_size)?; if let Some(primary_key) = primary_key { let mut txn = index.write_txn()?; let mut builder = UpdateBuilder::new(0).settings(&mut txn, &index); builder.set_primary_key(primary_key); - builder.execute(|_, _| ()) - .map_err(|e| IndexError::Internal(e.to_string()))?; + builder.execute(|_, _| ())?; txn.commit()?; } @@ -75,7 +74,7 @@ impl IndexStore for MapIndexStore { Ok(index) } - async fn get(&self, uuid: Uuid) -> IndexResult> { + async fn get(&self, uuid: Uuid) -> Result> { let guard = self.index_store.read().await; match guard.get(&uuid) { Some(index) => Ok(Some(index.clone())), @@ -95,7 +94,7 @@ impl IndexStore for MapIndexStore { } } - async fn delete(&self, uuid: Uuid) -> IndexResult> { + async fn delete(&self, uuid: Uuid) -> Result> { let db_path = self.path.join(format!("index-{}", uuid)); fs::remove_dir_all(db_path).await?; let index = self.index_store.write().await.remove(&uuid); diff --git a/meilisearch-http/src/index_controller/mod.rs b/meilisearch-http/src/index_controller/mod.rs index 0c801558b..b51a6c5b0 100644 --- a/meilisearch-http/src/index_controller/mod.rs +++ b/meilisearch-http/src/index_controller/mod.rs @@ -4,7 +4,6 @@ use std::sync::Arc; use std::time::Duration; use actix_web::web::{Bytes, Payload}; -use anyhow::bail; use chrono::{DateTime, Utc}; use futures::stream::StreamExt; use log::info; @@ -20,15 +19,18 @@ use index_actor::IndexActorHandle; use snapshot::{load_snapshot, SnapshotService}; use update_actor::UpdateActorHandle; pub use updates::*; -use uuid_resolver::{UuidResolverError, UuidResolverHandle}; +use uuid_resolver::{error::UuidResolverError, UuidResolverHandle}; use crate::index::{Checked, Document, SearchQuery, SearchResult, Settings}; use crate::option::Opt; +use error::Result; use self::dump_actor::load_dump; +use self::error::IndexControllerError; mod dump_actor; -mod index_actor; +pub mod error; +pub mod index_actor; mod snapshot; mod update_actor; mod updates; @@ -151,7 +153,7 @@ impl IndexController { format: milli::update::UpdateFormat, payload: Payload, primary_key: Option, - ) -> anyhow::Result { + ) -> Result { let perform_update = |uuid| async move { let meta = UpdateMeta::DocumentsAddition { method, @@ -189,7 +191,7 @@ impl IndexController { } } - pub async fn clear_documents(&self, uid: String) -> anyhow::Result { + pub async fn clear_documents(&self, uid: String) -> Result { let uuid = self.uuid_resolver.get(uid).await?; let meta = UpdateMeta::ClearDocuments; let (_, receiver) = mpsc::channel(1); @@ -201,7 +203,7 @@ impl IndexController { &self, uid: String, documents: Vec, - ) -> anyhow::Result { + ) -> Result { let uuid = self.uuid_resolver.get(uid).await?; let meta = UpdateMeta::DeleteDocuments { ids: documents }; let (_, receiver) = mpsc::channel(1); @@ -214,7 +216,7 @@ impl IndexController { uid: String, settings: Settings, create: bool, - ) -> anyhow::Result { + ) -> Result { let perform_udpate = |uuid| async move { let meta = UpdateMeta::Settings(settings.into_unchecked()); // Nothing so send, drop the sender right away, as not to block the update actor. @@ -236,12 +238,9 @@ impl IndexController { } } - pub async fn create_index( - &self, - index_settings: IndexSettings, - ) -> anyhow::Result { + pub async fn create_index(&self, index_settings: IndexSettings) -> Result { let IndexSettings { uid, primary_key } = index_settings; - let uid = uid.ok_or_else(|| anyhow::anyhow!("Can't create an index without a uid."))?; + let uid = uid.ok_or(IndexControllerError::MissingUid)?; let uuid = Uuid::new_v4(); let meta = self.index_handle.create_index(uuid, primary_key).await?; self.uuid_resolver.insert(uid.clone(), uuid).await?; @@ -255,26 +254,26 @@ impl IndexController { Ok(meta) } - pub async fn delete_index(&self, uid: String) -> anyhow::Result<()> { + pub async fn delete_index(&self, uid: String) -> Result<()> { let uuid = self.uuid_resolver.delete(uid).await?; self.update_handle.delete(uuid).await?; self.index_handle.delete(uuid).await?; Ok(()) } - pub async fn update_status(&self, uid: String, id: u64) -> anyhow::Result { + pub async fn update_status(&self, uid: String, id: u64) -> Result { let uuid = self.uuid_resolver.get(uid).await?; let result = self.update_handle.update_status(uuid, id).await?; Ok(result) } - pub async fn all_update_status(&self, uid: String) -> anyhow::Result> { + pub async fn all_update_status(&self, uid: String) -> Result> { let uuid = self.uuid_resolver.get(uid).await?; let result = self.update_handle.get_all_updates_status(uuid).await?; Ok(result) } - pub async fn list_indexes(&self) -> anyhow::Result> { + pub async fn list_indexes(&self) -> Result> { let uuids = self.uuid_resolver.list().await?; let mut ret = Vec::new(); @@ -293,7 +292,7 @@ impl IndexController { Ok(ret) } - pub async fn settings(&self, uid: String) -> anyhow::Result> { + pub async fn settings(&self, uid: String) -> Result> { let uuid = self.uuid_resolver.get(uid.clone()).await?; let settings = self.index_handle.settings(uuid).await?; Ok(settings) @@ -305,7 +304,7 @@ impl IndexController { offset: usize, limit: usize, attributes_to_retrieve: Option>, - ) -> anyhow::Result> { + ) -> Result> { let uuid = self.uuid_resolver.get(uid.clone()).await?; let documents = self .index_handle @@ -319,7 +318,7 @@ impl IndexController { uid: String, doc_id: String, attributes_to_retrieve: Option>, - ) -> anyhow::Result { + ) -> Result { let uuid = self.uuid_resolver.get(uid.clone()).await?; let document = self .index_handle @@ -331,10 +330,10 @@ impl IndexController { pub async fn update_index( &self, uid: String, - index_settings: IndexSettings, - ) -> anyhow::Result { + mut index_settings: IndexSettings, + ) -> Result { if index_settings.uid.is_some() { - bail!("Can't change the index uid.") + index_settings.uid.take(); } let uuid = self.uuid_resolver.get(uid.clone()).await?; @@ -348,13 +347,13 @@ impl IndexController { Ok(meta) } - pub async fn search(&self, uid: String, query: SearchQuery) -> anyhow::Result { + pub async fn search(&self, uid: String, query: SearchQuery) -> Result { let uuid = self.uuid_resolver.get(uid).await?; let result = self.index_handle.search(uuid, query).await?; Ok(result) } - pub async fn get_index(&self, uid: String) -> anyhow::Result { + pub async fn get_index(&self, uid: String) -> Result { let uuid = self.uuid_resolver.get(uid.clone()).await?; let meta = self.index_handle.get_index_meta(uuid).await?; let meta = IndexMetadata { @@ -366,11 +365,11 @@ impl IndexController { Ok(meta) } - pub async fn get_uuids_size(&self) -> anyhow::Result { + pub async fn get_uuids_size(&self) -> Result { Ok(self.uuid_resolver.get_size().await?) } - pub async fn get_index_stats(&self, uid: String) -> anyhow::Result { + pub async fn get_index_stats(&self, uid: String) -> Result { let uuid = self.uuid_resolver.get(uid).await?; let update_infos = self.update_handle.get_info().await?; let mut stats = self.index_handle.get_index_stats(uuid).await?; @@ -379,7 +378,7 @@ impl IndexController { Ok(stats) } - pub async fn get_all_stats(&self) -> anyhow::Result { + pub async fn get_all_stats(&self) -> Result { let update_infos = self.update_handle.get_info().await?; let mut database_size = self.get_uuids_size().await? + update_infos.size; let mut last_update: Option> = None; @@ -405,11 +404,11 @@ impl IndexController { }) } - pub async fn create_dump(&self) -> anyhow::Result { + pub async fn create_dump(&self) -> Result { Ok(self.dump_handle.create_dump().await?) } - pub async fn dump_info(&self, uid: String) -> anyhow::Result { + pub async fn dump_info(&self, uid: String) -> Result { Ok(self.dump_handle.dump_info(uid).await?) } } diff --git a/meilisearch-http/src/index_controller/snapshot.rs b/meilisearch-http/src/index_controller/snapshot.rs index daef7d582..9f0c5c0ba 100644 --- a/meilisearch-http/src/index_controller/snapshot.rs +++ b/meilisearch-http/src/index_controller/snapshot.rs @@ -142,9 +142,11 @@ mod test { use super::*; use crate::index_controller::index_actor::MockIndexActorHandle; use crate::index_controller::update_actor::{ - MockUpdateActorHandle, UpdateActorHandleImpl, UpdateError, + error::UpdateActorError, MockUpdateActorHandle, UpdateActorHandleImpl, + }; + use crate::index_controller::uuid_resolver::{ + error::UuidResolverError, MockUuidResolverHandle, }; - use crate::index_controller::uuid_resolver::{MockUuidResolverHandle, UuidResolverError}; #[actix_rt::test] async fn test_normal() { @@ -224,7 +226,7 @@ mod test { update_handle .expect_snapshot() // abitrary error - .returning(|_, _| Box::pin(err(UpdateError::UnexistingUpdate(0)))); + .returning(|_, _| Box::pin(err(UpdateActorError::UnexistingUpdate(0)))); let snapshot_path = tempfile::tempdir_in(".").unwrap(); let snapshot_service = SnapshotService::new( diff --git a/meilisearch-http/src/index_controller/update_actor/actor.rs b/meilisearch-http/src/index_controller/update_actor/actor.rs index eebbf6247..41995727a 100644 --- a/meilisearch-http/src/index_controller/update_actor/actor.rs +++ b/meilisearch-http/src/index_controller/update_actor/actor.rs @@ -13,7 +13,8 @@ use tokio::io::AsyncWriteExt; use tokio::sync::mpsc; use uuid::Uuid; -use super::{PayloadData, Result, UpdateError, UpdateMsg, UpdateStore, UpdateStoreInfo}; +use super::error::{Result, UpdateActorError}; +use super::{PayloadData, UpdateMsg, UpdateStore, UpdateStoreInfo}; use crate::index_controller::index_actor::IndexActorHandle; use crate::index_controller::{UpdateMeta, UpdateStatus}; @@ -172,7 +173,8 @@ where if copy(&mut checker, &mut sink()).is_err() || checker.finish().is_err() { // The json file is invalid, we use Serde to get a nice error message: file.seek(SeekFrom::Start(0))?; - let _: serde_json::Value = serde_json::from_reader(file)?; + let _: serde_json::Value = serde_json::from_reader(file) + .map_err(|e| UpdateActorError::InvalidPayload(Box::new(e)))?; } Some(uuid) } else { @@ -200,9 +202,9 @@ where async fn handle_get_update(&self, uuid: Uuid, id: u64) -> Result { let store = self.store.clone(); tokio::task::spawn_blocking(move || { - let result = store - .meta(uuid, id)? - .ok_or(UpdateError::UnexistingUpdate(id))?; + let result = store + .meta(uuid, id)? + .ok_or(UpdateActorError::UnexistingUpdate(id))?; Ok(result) }) .await? @@ -230,7 +232,7 @@ where let index_handle = self.index_handle.clone(); let update_store = self.store.clone(); - tokio::task::spawn_blocking(move || -> anyhow::Result<()> { + tokio::task::spawn_blocking(move || -> Result<()> { update_store.dump(&uuids, path.to_path_buf(), index_handle)?; Ok(()) }) @@ -241,7 +243,7 @@ where async fn handle_get_info(&self) -> Result { let update_store = self.store.clone(); - let info = tokio::task::spawn_blocking(move || -> anyhow::Result { + let info = tokio::task::spawn_blocking(move || -> Result { let info = update_store.get_info()?; Ok(info) }) diff --git a/meilisearch-http/src/index_controller/update_actor/error.rs b/meilisearch-http/src/index_controller/update_actor/error.rs new file mode 100644 index 000000000..a95f1eafa --- /dev/null +++ b/meilisearch-http/src/index_controller/update_actor/error.rs @@ -0,0 +1,56 @@ +use std::error::Error; + +use meilisearch_error::{Code, ErrorCode}; + +use crate::index_controller::index_actor::error::IndexActorError; + +pub type Result = std::result::Result; + +#[derive(Debug, thiserror::Error)] +#[allow(clippy::large_enum_variant)] +pub enum UpdateActorError { + #[error("update {0} doesn't exist.")] + UnexistingUpdate(u64), + #[error("internal error processing update: {0}")] + Internal(Box), + #[error("error with index: {0}")] + IndexActor(#[from] IndexActorError), + #[error( + "update store was shut down due to a fatal error, please check your logs for more info." + )] + FatalUpdateStoreError, + #[error("invalid payload: {0}")] + InvalidPayload(Box), +} + +impl From> for UpdateActorError { + fn from(_: tokio::sync::mpsc::error::SendError) -> Self { + Self::FatalUpdateStoreError + } +} + +impl From for UpdateActorError { + fn from(_: tokio::sync::oneshot::error::RecvError) -> Self { + Self::FatalUpdateStoreError + } +} + +internal_error!( + UpdateActorError: heed::Error, + std::io::Error, + serde_json::Error, + actix_http::error::PayloadError, + tokio::task::JoinError +); + +impl ErrorCode for UpdateActorError { + fn error_code(&self) -> Code { + match self { + UpdateActorError::UnexistingUpdate(_) => Code::NotFound, + UpdateActorError::Internal(_) => Code::Internal, + UpdateActorError::IndexActor(e) => e.error_code(), + UpdateActorError::FatalUpdateStoreError => Code::Internal, + UpdateActorError::InvalidPayload(_) => Code::BadRequest, + } + } +} diff --git a/meilisearch-http/src/index_controller/update_actor/handle_impl.rs b/meilisearch-http/src/index_controller/update_actor/handle_impl.rs index 7844bf855..125c63401 100644 --- a/meilisearch-http/src/index_controller/update_actor/handle_impl.rs +++ b/meilisearch-http/src/index_controller/update_actor/handle_impl.rs @@ -6,10 +6,8 @@ use uuid::Uuid; use crate::index_controller::{IndexActorHandle, UpdateStatus}; -use super::{ - PayloadData, Result, UpdateActor, UpdateActorHandle, UpdateError, UpdateMeta, UpdateMsg, - UpdateStoreInfo, -}; +use super::error::Result; +use super::{PayloadData, UpdateActor, UpdateActorHandle, UpdateMeta, UpdateMsg, UpdateStoreInfo}; #[derive(Clone)] pub struct UpdateActorHandleImpl { @@ -48,72 +46,42 @@ where async fn get_all_updates_status(&self, uuid: Uuid) -> Result> { let (ret, receiver) = oneshot::channel(); let msg = UpdateMsg::ListUpdates { uuid, ret }; - self.sender - .send(msg) - .await - .map_err(|_| UpdateError::FatalUpdateStoreError)?; - receiver - .await - .map_err(|_| UpdateError::FatalUpdateStoreError)? + self.sender.send(msg).await?; + receiver.await? } async fn update_status(&self, uuid: Uuid, id: u64) -> Result { let (ret, receiver) = oneshot::channel(); let msg = UpdateMsg::GetUpdate { uuid, id, ret }; - self.sender - .send(msg) - .await - .map_err(|_| UpdateError::FatalUpdateStoreError)?; - receiver - .await - .map_err(|_| UpdateError::FatalUpdateStoreError)? + self.sender.send(msg).await?; + receiver.await? } async fn delete(&self, uuid: Uuid) -> Result<()> { let (ret, receiver) = oneshot::channel(); let msg = UpdateMsg::Delete { uuid, ret }; - self.sender - .send(msg) - .await - .map_err(|_| UpdateError::FatalUpdateStoreError)?; - receiver - .await - .map_err(|_| UpdateError::FatalUpdateStoreError)? + self.sender.send(msg).await?; + receiver.await? } async fn snapshot(&self, uuids: HashSet, path: PathBuf) -> Result<()> { let (ret, receiver) = oneshot::channel(); let msg = UpdateMsg::Snapshot { uuids, path, ret }; - self.sender - .send(msg) - .await - .map_err(|_| UpdateError::FatalUpdateStoreError)?; - receiver - .await - .map_err(|_| UpdateError::FatalUpdateStoreError)? + self.sender.send(msg).await?; + receiver.await? } async fn dump(&self, uuids: HashSet, path: PathBuf) -> Result<()> { let (ret, receiver) = oneshot::channel(); let msg = UpdateMsg::Dump { uuids, path, ret }; - self.sender - .send(msg) - .await - .map_err(|_| UpdateError::FatalUpdateStoreError)?; - receiver - .await - .map_err(|_| UpdateError::FatalUpdateStoreError)? + self.sender.send(msg).await?; + receiver.await? } async fn get_info(&self) -> Result { let (ret, receiver) = oneshot::channel(); let msg = UpdateMsg::GetInfo { ret }; - self.sender - .send(msg) - .await - .map_err(|_| UpdateError::FatalUpdateStoreError)?; - receiver - .await - .map_err(|_| UpdateError::FatalUpdateStoreError)? + self.sender.send(msg).await?; + receiver.await? } async fn update( @@ -129,12 +97,7 @@ where meta, ret, }; - self.sender - .send(msg) - .await - .map_err(|_| UpdateError::FatalUpdateStoreError)?; - receiver - .await - .map_err(|_| UpdateError::FatalUpdateStoreError)? + self.sender.send(msg).await?; + receiver.await? } } diff --git a/meilisearch-http/src/index_controller/update_actor/message.rs b/meilisearch-http/src/index_controller/update_actor/message.rs index 37df2af32..6b8a0f73f 100644 --- a/meilisearch-http/src/index_controller/update_actor/message.rs +++ b/meilisearch-http/src/index_controller/update_actor/message.rs @@ -4,7 +4,8 @@ use std::path::PathBuf; use tokio::sync::{mpsc, oneshot}; use uuid::Uuid; -use super::{PayloadData, Result, UpdateMeta, UpdateStatus, UpdateStoreInfo}; +use super::error::Result; +use super::{PayloadData, UpdateMeta, UpdateStatus, UpdateStoreInfo}; pub enum UpdateMsg { Update { diff --git a/meilisearch-http/src/index_controller/update_actor/mod.rs b/meilisearch-http/src/index_controller/update_actor/mod.rs index b854cca70..cb40311b2 100644 --- a/meilisearch-http/src/index_controller/update_actor/mod.rs +++ b/meilisearch-http/src/index_controller/update_actor/mod.rs @@ -1,62 +1,29 @@ -mod actor; -mod handle_impl; -mod message; -pub mod store; - use std::{collections::HashSet, path::PathBuf}; use actix_http::error::PayloadError; -use thiserror::Error; use tokio::sync::mpsc; use uuid::Uuid; use crate::index_controller::{UpdateMeta, UpdateStatus}; use actor::UpdateActor; +use error::Result; use message::UpdateMsg; pub use handle_impl::UpdateActorHandleImpl; pub use store::{UpdateStore, UpdateStoreInfo}; -pub type Result = std::result::Result; +mod actor; +pub mod error; +mod handle_impl; +mod message; +pub mod store; + type PayloadData = std::result::Result; #[cfg(test)] use mockall::automock; -#[derive(Debug, Error)] -pub enum UpdateError { - #[error("Update {0} doesn't exist.")] - UnexistingUpdate(u64), - #[error("Internal error processing update: {0}")] - Internal(String), - #[error( - "Update store was shut down due to a fatal error, please check your logs for more info." - )] - FatalUpdateStoreError, -} - -macro_rules! internal_error { - ($($other:path), *) => { - $( - impl From<$other> for UpdateError { - fn from(other: $other) -> Self { - Self::Internal(other.to_string()) - } - } - )* - } -} - -internal_error!( - heed::Error, - std::io::Error, - serde_json::Error, - PayloadError, - tokio::task::JoinError, - anyhow::Error -); - #[async_trait::async_trait] #[cfg_attr(test, automock(type Data=Vec;))] pub trait UpdateActorHandle { diff --git a/meilisearch-http/src/index_controller/update_actor/store/dump.rs b/meilisearch-http/src/index_controller/update_actor/store/dump.rs index e7f36a2a1..7c46f98fa 100644 --- a/meilisearch-http/src/index_controller/update_actor/store/dump.rs +++ b/meilisearch-http/src/index_controller/update_actor/store/dump.rs @@ -9,7 +9,7 @@ use heed::{EnvOpenOptions, RoTxn}; use serde::{Deserialize, Serialize}; use uuid::Uuid; -use super::{State, UpdateStore}; +use super::{Result, State, UpdateStore}; use crate::index_controller::{ index_actor::IndexActorHandle, update_actor::store::update_uuid_to_file_path, Enqueued, UpdateStatus, @@ -27,7 +27,7 @@ impl UpdateStore { uuids: &HashSet, path: PathBuf, handle: impl IndexActorHandle, - ) -> anyhow::Result<()> { + ) -> Result<()> { let state_lock = self.state.write(); state_lock.swap(State::Dumping); @@ -52,7 +52,7 @@ impl UpdateStore { txn: &RoTxn, uuids: &HashSet, path: impl AsRef, - ) -> anyhow::Result<()> { + ) -> Result<()> { let dump_data_path = path.as_ref().join("data.jsonl"); let mut dump_data_file = File::create(dump_data_path)?; @@ -71,7 +71,7 @@ impl UpdateStore { uuids: &HashSet, mut file: &mut File, dst_path: impl AsRef, - ) -> anyhow::Result<()> { + ) -> Result<()> { let pendings = self.pending_queue.iter(txn)?.lazily_decode_data(); for pending in pendings { @@ -103,7 +103,7 @@ impl UpdateStore { txn: &RoTxn, uuids: &HashSet, mut file: &mut File, - ) -> anyhow::Result<()> { + ) -> Result<()> { let updates = self.updates.iter(txn)?.lazily_decode_data(); for update in updates { @@ -175,7 +175,7 @@ async fn dump_indexes( uuids: &HashSet, handle: impl IndexActorHandle, path: impl AsRef, -) -> anyhow::Result<()> { +) -> Result<()> { for uuid in uuids { handle.dump(*uuid, path.as_ref().to_owned()).await?; } diff --git a/meilisearch-http/src/index_controller/update_actor/store/mod.rs b/meilisearch-http/src/index_controller/update_actor/store/mod.rs index e7b719fc9..0bdf53aee 100644 --- a/meilisearch-http/src/index_controller/update_actor/store/mod.rs +++ b/meilisearch-http/src/index_controller/update_actor/store/mod.rs @@ -23,9 +23,10 @@ use uuid::Uuid; use codec::*; +use super::error::Result; use super::UpdateMeta; +use crate::helpers::EnvSizer; use crate::index_controller::{index_actor::CONCURRENT_INDEX_MSG, updates::*, IndexActorHandle}; -use crate::{helpers::EnvSizer, index_controller::index_actor::IndexResult}; #[allow(clippy::upper_case_acronyms)] type BEU64 = U64; @@ -269,11 +270,8 @@ impl UpdateStore { } _ => { let _update_id = self.next_update_id_raw(wtxn, index_uuid)?; - self.updates.put( - wtxn, - &(index_uuid, update.id()), - &update, - )?; + self.updates + .put(wtxn, &(index_uuid, update.id()), &update)?; } } Ok(()) @@ -282,10 +280,7 @@ impl UpdateStore { /// Executes the user provided function on the next pending update (the one with the lowest id). /// This is asynchronous as it let the user process the update with a read-only txn and /// only writing the result meta to the processed-meta store *after* it has been processed. - fn process_pending_update( - &self, - index_handle: impl IndexActorHandle, - ) -> anyhow::Result> { + fn process_pending_update(&self, index_handle: impl IndexActorHandle) -> Result> { // Create a read transaction to be able to retrieve the pending update in order. let rtxn = self.env.read_txn()?; let first_meta = self.pending_queue.first(&rtxn)?; @@ -320,7 +315,7 @@ impl UpdateStore { index_handle: impl IndexActorHandle, index_uuid: Uuid, global_id: u64, - ) -> anyhow::Result> { + ) -> Result> { let content_path = content.map(|uuid| update_uuid_to_file_path(&self.path, uuid)); let update_id = processing.id(); @@ -337,7 +332,7 @@ impl UpdateStore { let result = match handle.block_on(index_handle.update(index_uuid, processing.clone(), file)) { Ok(result) => result, - Err(e) => Err(processing.fail(e.to_string())), + Err(e) => Err(processing.fail(e.into())), }; // Once the pending update have been successfully processed @@ -352,11 +347,8 @@ impl UpdateStore { Err(res) => res.into(), }; - self.updates.put( - &mut wtxn, - &(index_uuid, update_id), - &result, - )?; + self.updates + .put(&mut wtxn, &(index_uuid, update_id), &result)?; wtxn.commit()?; @@ -368,7 +360,7 @@ impl UpdateStore { } /// List the updates for `index_uuid`. - pub fn list(&self, index_uuid: Uuid) -> anyhow::Result> { + pub fn list(&self, index_uuid: Uuid) -> Result> { let mut update_list = BTreeMap::::new(); let txn = self.env.read_txn()?; @@ -437,7 +429,7 @@ impl UpdateStore { } /// Delete all updates for an index from the update store. - pub fn delete_all(&self, index_uuid: Uuid) -> anyhow::Result<()> { + pub fn delete_all(&self, index_uuid: Uuid) -> Result<()> { let mut txn = self.env.write_txn()?; // Contains all the content file paths that we need to be removed if the deletion was successful. let mut uuids_to_remove = Vec::new(); @@ -488,7 +480,7 @@ impl UpdateStore { uuids: &HashSet, path: impl AsRef, handle: impl IndexActorHandle + Clone, - ) -> anyhow::Result<()> { + ) -> Result<()> { let state_lock = self.state.write(); state_lock.swap(State::Snapshoting); @@ -535,13 +527,13 @@ impl UpdateStore { while let Some(res) = stream.next().await { res?; } - Ok(()) as IndexResult<()> + Ok(()) as Result<()> })?; Ok(()) } - pub fn get_info(&self) -> anyhow::Result { + pub fn get_info(&self) -> Result { let mut size = self.env.size(); let txn = self.env.read_txn()?; for entry in self.pending_queue.iter(&txn)? { @@ -573,7 +565,7 @@ fn update_uuid_to_file_path(root: impl AsRef, uuid: Uuid) -> PathBuf { #[cfg(test)] mod test { use super::*; - use crate::index_controller::{index_actor::MockIndexActorHandle, UpdateResult}; + use crate::index_controller::{UpdateResult, index_actor::{MockIndexActorHandle, error::IndexActorError}}; use futures::future::ok; @@ -652,7 +644,7 @@ mod test { if processing.id() == 0 { Box::pin(ok(Ok(processing.process(UpdateResult::Other)))) } else { - Box::pin(ok(Err(processing.fail(String::from("err"))))) + Box::pin(ok(Err(processing.fail(IndexActorError::ExistingPrimaryKey.into())))) } }); @@ -703,18 +695,10 @@ mod test { let txn = store.env.read_txn().unwrap(); assert!(store.pending_queue.first(&txn).unwrap().is_none()); - let update = store - .updates - .get(&txn, &(uuid, 0)) - .unwrap() - .unwrap(); + let update = store.updates.get(&txn, &(uuid, 0)).unwrap().unwrap(); assert!(matches!(update, UpdateStatus::Processed(_))); - let update = store - .updates - .get(&txn, &(uuid, 1)) - .unwrap() - .unwrap(); + let update = store.updates.get(&txn, &(uuid, 1)).unwrap().unwrap(); assert!(matches!(update, UpdateStatus::Failed(_))); } diff --git a/meilisearch-http/src/index_controller/updates.rs b/meilisearch-http/src/index_controller/updates.rs index abeec413e..13ad9f7e5 100644 --- a/meilisearch-http/src/index_controller/updates.rs +++ b/meilisearch-http/src/index_controller/updates.rs @@ -3,9 +3,7 @@ use milli::update::{DocumentAdditionResult, IndexDocumentsMethod, UpdateFormat}; use serde::{Deserialize, Serialize}; use uuid::Uuid; -use crate::index::{Settings, Unchecked}; - -pub type UpdateError = String; +use crate::{error::ResponseError, index::{Settings, Unchecked}}; #[derive(Debug, Clone, Serialize, Deserialize)] pub enum UpdateResult { @@ -25,7 +23,7 @@ pub enum UpdateMeta { }, ClearDocuments, DeleteDocuments { - ids: Vec + ids: Vec, }, Settings(Settings), } @@ -116,7 +114,7 @@ impl Processing { } } - pub fn fail(self, error: UpdateError) -> Failed { + pub fn fail(self, error: ResponseError) -> Failed { Failed { from: self, error, @@ -143,12 +141,12 @@ impl Aborted { } } -#[derive(Debug, Serialize, Deserialize, Clone)] +#[derive(Debug, Serialize, Deserialize)] #[serde(rename_all = "camelCase")] pub struct Failed { #[serde(flatten)] pub from: Processing, - pub error: UpdateError, + pub error: ResponseError, pub failed_at: DateTime, } @@ -162,7 +160,7 @@ impl Failed { } } -#[derive(Debug, Serialize, Deserialize, Clone)] +#[derive(Debug, Serialize, Deserialize)] #[serde(tag = "status", rename_all = "camelCase")] pub enum UpdateStatus { Processing(Processing), diff --git a/meilisearch-http/src/index_controller/uuid_resolver/actor.rs b/meilisearch-http/src/index_controller/uuid_resolver/actor.rs index 74158ce04..efad15c92 100644 --- a/meilisearch-http/src/index_controller/uuid_resolver/actor.rs +++ b/meilisearch-http/src/index_controller/uuid_resolver/actor.rs @@ -4,7 +4,7 @@ use log::{info, warn}; use tokio::sync::mpsc; use uuid::Uuid; -use super::{Result, UuidResolveMsg, UuidResolverError, UuidStore}; +use super::{error::UuidResolverError, Result, UuidResolveMsg, UuidStore}; pub struct UuidResolverActor { inbox: mpsc::Receiver, diff --git a/meilisearch-http/src/index_controller/uuid_resolver/error.rs b/meilisearch-http/src/index_controller/uuid_resolver/error.rs new file mode 100644 index 000000000..3d7fb8444 --- /dev/null +++ b/meilisearch-http/src/index_controller/uuid_resolver/error.rs @@ -0,0 +1,34 @@ +use meilisearch_error::{Code, ErrorCode}; + +pub type Result = std::result::Result; + +#[derive(Debug, thiserror::Error)] +pub enum UuidResolverError { + #[error("name already exist.")] + NameAlreadyExist, + #[error("index \"{0}\" doesn't exist.")] + UnexistingIndex(String), + #[error("badly formatted index uid: {0}")] + BadlyFormatted(String), + #[error("internal error resolving index uid: {0}")] + Internal(Box), +} + +internal_error!( + UuidResolverError: heed::Error, + uuid::Error, + std::io::Error, + tokio::task::JoinError, + serde_json::Error +); + +impl ErrorCode for UuidResolverError { + fn error_code(&self) -> Code { + match self { + UuidResolverError::NameAlreadyExist => Code::IndexAlreadyExists, + UuidResolverError::UnexistingIndex(_) => Code::IndexNotFound, + UuidResolverError::BadlyFormatted(_) => Code::InvalidIndexUid, + UuidResolverError::Internal(_) => Code::Internal, + } + } +} diff --git a/meilisearch-http/src/index_controller/uuid_resolver/handle_impl.rs b/meilisearch-http/src/index_controller/uuid_resolver/handle_impl.rs index af710dd87..1296264e0 100644 --- a/meilisearch-http/src/index_controller/uuid_resolver/handle_impl.rs +++ b/meilisearch-http/src/index_controller/uuid_resolver/handle_impl.rs @@ -12,7 +12,7 @@ pub struct UuidResolverHandleImpl { } impl UuidResolverHandleImpl { - pub fn new(path: impl AsRef) -> anyhow::Result { + pub fn new(path: impl AsRef) -> Result { let (sender, reveiver) = mpsc::channel(100); let store = HeedUuidStore::new(path)?; let actor = UuidResolverActor::new(reveiver, store); @@ -32,7 +32,7 @@ impl UuidResolverHandle for UuidResolverHandleImpl { .expect("Uuid resolver actor has been killed")?) } - async fn delete(&self, name: String) -> anyhow::Result { + async fn delete(&self, name: String) -> Result { let (ret, receiver) = oneshot::channel(); let msg = UuidResolveMsg::Delete { uid: name, ret }; let _ = self.sender.send(msg).await; @@ -41,7 +41,7 @@ impl UuidResolverHandle for UuidResolverHandleImpl { .expect("Uuid resolver actor has been killed")?) } - async fn list(&self) -> anyhow::Result> { + async fn list(&self) -> Result> { let (ret, receiver) = oneshot::channel(); let msg = UuidResolveMsg::List { ret }; let _ = self.sender.send(msg).await; @@ -50,7 +50,7 @@ impl UuidResolverHandle for UuidResolverHandleImpl { .expect("Uuid resolver actor has been killed")?) } - async fn insert(&self, name: String, uuid: Uuid) -> anyhow::Result<()> { + async fn insert(&self, name: String, uuid: Uuid) -> Result<()> { let (ret, receiver) = oneshot::channel(); let msg = UuidResolveMsg::Insert { ret, name, uuid }; let _ = self.sender.send(msg).await; diff --git a/meilisearch-http/src/index_controller/uuid_resolver/mod.rs b/meilisearch-http/src/index_controller/uuid_resolver/mod.rs index 3c3b5fd06..da6c1264d 100644 --- a/meilisearch-http/src/index_controller/uuid_resolver/mod.rs +++ b/meilisearch-http/src/index_controller/uuid_resolver/mod.rs @@ -1,4 +1,5 @@ mod actor; +pub mod error; mod handle_impl; mod message; pub mod store; @@ -6,10 +7,10 @@ pub mod store; use std::collections::HashSet; use std::path::PathBuf; -use thiserror::Error; use uuid::Uuid; use actor::UuidResolverActor; +use error::Result; use message::UuidResolveMsg; use store::UuidStore; @@ -21,48 +22,14 @@ pub use store::HeedUuidStore; const UUID_STORE_SIZE: usize = 1_073_741_824; //1GiB -pub type Result = std::result::Result; - #[async_trait::async_trait] #[cfg_attr(test, automock)] pub trait UuidResolverHandle { async fn get(&self, name: String) -> Result; - async fn insert(&self, name: String, uuid: Uuid) -> anyhow::Result<()>; - async fn delete(&self, name: String) -> anyhow::Result; - async fn list(&self) -> anyhow::Result>; + async fn insert(&self, name: String, uuid: Uuid) -> Result<()>; + async fn delete(&self, name: String) -> Result; + async fn list(&self) -> Result>; async fn snapshot(&self, path: PathBuf) -> Result>; async fn get_size(&self) -> Result; async fn dump(&self, path: PathBuf) -> Result>; } - -#[derive(Debug, Error)] -pub enum UuidResolverError { - #[error("Name already exist.")] - NameAlreadyExist, - #[error("Index \"{0}\" doesn't exist.")] - UnexistingIndex(String), - #[error("Badly formatted index uid: {0}")] - BadlyFormatted(String), - #[error("Internal error resolving index uid: {0}")] - Internal(String), -} - -macro_rules! internal_error { - ($($other:path), *) => { - $( - impl From<$other> for UuidResolverError { - fn from(other: $other) -> Self { - Self::Internal(other.to_string()) - } - } - )* - } -} - -internal_error!( - heed::Error, - uuid::Error, - std::io::Error, - tokio::task::JoinError, - serde_json::Error -); diff --git a/meilisearch-http/src/index_controller/uuid_resolver/store.rs b/meilisearch-http/src/index_controller/uuid_resolver/store.rs index bab223bb3..f02d22d7f 100644 --- a/meilisearch-http/src/index_controller/uuid_resolver/store.rs +++ b/meilisearch-http/src/index_controller/uuid_resolver/store.rs @@ -8,7 +8,7 @@ use heed::{CompactionOption, Database, Env, EnvOpenOptions}; use serde::{Deserialize, Serialize}; use uuid::Uuid; -use super::{Result, UuidResolverError, UUID_STORE_SIZE}; +use super::{error::UuidResolverError, Result, UUID_STORE_SIZE}; use crate::helpers::EnvSizer; #[derive(Serialize, Deserialize)] @@ -39,7 +39,7 @@ pub struct HeedUuidStore { } impl HeedUuidStore { - pub fn new(path: impl AsRef) -> anyhow::Result { + pub fn new(path: impl AsRef) -> Result { let path = path.as_ref().join(UUIDS_DB_PATH); create_dir_all(&path)?; let mut options = EnvOpenOptions::new(); @@ -153,7 +153,7 @@ impl HeedUuidStore { Ok(uuids) } - pub fn load_dump(src: impl AsRef, dst: impl AsRef) -> anyhow::Result<()> { + pub fn load_dump(src: impl AsRef, dst: impl AsRef) -> Result<()> { let uuid_resolver_path = dst.as_ref().join(UUIDS_DB_PATH); std::fs::create_dir_all(&uuid_resolver_path)?; diff --git a/meilisearch-http/src/lib.rs b/meilisearch-http/src/lib.rs index a815a3659..25c29ac11 100644 --- a/meilisearch-http/src/lib.rs +++ b/meilisearch-http/src/lib.rs @@ -1,4 +1,5 @@ pub mod data; +#[macro_use] pub mod error; pub mod helpers; mod index; diff --git a/meilisearch-http/src/main.rs b/meilisearch-http/src/main.rs index 2787d8b78..9f3e62356 100644 --- a/meilisearch-http/src/main.rs +++ b/meilisearch-http/src/main.rs @@ -30,6 +30,7 @@ async fn main() -> Result<(), MainError> { .into(), ); } + #[cfg(all(not(debug_assertions), feature = "analytics"))] if !opt.no_analytics { let logger = diff --git a/meilisearch-http/src/routes/document.rs b/meilisearch-http/src/routes/document.rs index 4f211bf09..9019bdb7c 100644 --- a/meilisearch-http/src/routes/document.rs +++ b/meilisearch-http/src/routes/document.rs @@ -61,15 +61,10 @@ async fn get_document( ) -> Result { let index = path.index_uid.clone(); let id = path.document_id.clone(); - match data + let document = data .retrieve_document(index, id, None as Option>) - .await - { - Ok(document) => Ok(HttpResponse::Ok().json(document)), - Err(e) => { - Ok(HttpResponse::BadRequest().json(serde_json::json!({ "error": e.to_string() }))) - } - } + .await?; + Ok(HttpResponse::Ok().json(document)) } #[delete( @@ -80,17 +75,10 @@ async fn delete_document( data: web::Data, path: web::Path, ) -> Result { - match data + let update_status = data .delete_documents(path.index_uid.clone(), vec![path.document_id.clone()]) - .await - { - Ok(update_status) => Ok( - HttpResponse::Accepted().json(serde_json::json!({ "updateId": update_status.id() })) - ), - Err(e) => { - Ok(HttpResponse::BadRequest().json(serde_json::json!({ "error": e.to_string() }))) - } - } + .await?; + Ok(HttpResponse::Accepted().json(serde_json::json!({ "updateId": update_status.id() }))) } #[derive(Deserialize)] @@ -118,20 +106,15 @@ async fn get_all_documents( Some(names) }); - match data + let documents = data .retrieve_documents( path.index_uid.clone(), params.offset.unwrap_or(DEFAULT_RETRIEVE_DOCUMENTS_OFFSET), params.limit.unwrap_or(DEFAULT_RETRIEVE_DOCUMENTS_LIMIT), attributes_to_retrieve, ) - .await - { - Ok(documents) => Ok(HttpResponse::Ok().json(documents)), - Err(e) => { - Ok(HttpResponse::BadRequest().json(serde_json::json!({ "error": e.to_string() }))) - } - } + .await?; + Ok(HttpResponse::Ok().json(documents)) } #[derive(Deserialize)] @@ -149,7 +132,7 @@ async fn add_documents( params: web::Query, body: Payload, ) -> Result { - let addition_result = data + let update_status = data .add_documents( path.into_inner().index_uid, IndexDocumentsMethod::ReplaceDocuments, @@ -157,16 +140,9 @@ async fn add_documents( body, params.primary_key.clone(), ) - .await; + .await?; - match addition_result { - Ok(update_status) => Ok( - HttpResponse::Accepted().json(serde_json::json!({ "updateId": update_status.id() })) - ), - Err(e) => { - Ok(HttpResponse::BadRequest().json(serde_json::json!({ "error": e.to_string() }))) - } - } + Ok(HttpResponse::Accepted().json(serde_json::json!({ "updateId": update_status.id() }))) } /// Default route for adding documents, this should return an error and redirect to the documentation @@ -200,7 +176,7 @@ async fn update_documents( params: web::Query, body: web::Payload, ) -> Result { - let addition_result = data + let update = data .add_documents( path.into_inner().index_uid, IndexDocumentsMethod::UpdateDocuments, @@ -208,16 +184,9 @@ async fn update_documents( body, params.primary_key.clone(), ) - .await; + .await?; - match addition_result { - Ok(update) => { - Ok(HttpResponse::Accepted().json(serde_json::json!({ "updateId": update.id() }))) - } - Err(e) => { - Ok(HttpResponse::BadRequest().json(serde_json::json!({ "error": e.to_string() }))) - } - } + Ok(HttpResponse::Accepted().json(serde_json::json!({ "updateId": update.id() }))) } #[post( @@ -238,14 +207,8 @@ async fn delete_documents( }) .collect(); - match data.delete_documents(path.index_uid.clone(), ids).await { - Ok(update_status) => Ok( - HttpResponse::Accepted().json(serde_json::json!({ "updateId": update_status.id() })) - ), - Err(e) => { - Ok(HttpResponse::BadRequest().json(serde_json::json!({ "error": e.to_string() }))) - } - } + let update_status = data.delete_documents(path.index_uid.clone(), ids).await?; + Ok(HttpResponse::Accepted().json(serde_json::json!({ "updateId": update_status.id() }))) } /// delete all documents @@ -254,12 +217,6 @@ async fn clear_all_documents( data: web::Data, path: web::Path, ) -> Result { - match data.clear_documents(path.index_uid.clone()).await { - Ok(update_status) => Ok( - HttpResponse::Accepted().json(serde_json::json!({ "updateId": update_status.id() })) - ), - Err(e) => { - Ok(HttpResponse::BadRequest().json(serde_json::json!({ "error": e.to_string() }))) - } - } + let update_status = data.clear_documents(path.index_uid.clone()).await?; + Ok(HttpResponse::Accepted().json(serde_json::json!({ "updateId": update_status.id() }))) } diff --git a/meilisearch-http/src/routes/index.rs b/meilisearch-http/src/routes/index.rs index efe2ef711..137b4dbbd 100644 --- a/meilisearch-http/src/routes/index.rs +++ b/meilisearch-http/src/routes/index.rs @@ -3,9 +3,9 @@ use actix_web::{web, HttpResponse}; use chrono::{DateTime, Utc}; use serde::{Deserialize, Serialize}; +use super::{IndexParam, UpdateStatusResponse}; use crate::error::ResponseError; use crate::helpers::Authentication; -use super::{UpdateStatusResponse, IndexParam}; use crate::Data; pub fn services(cfg: &mut web::ServiceConfig) { @@ -20,12 +20,8 @@ pub fn services(cfg: &mut web::ServiceConfig) { #[get("/indexes", wrap = "Authentication::Private")] async fn list_indexes(data: web::Data) -> Result { - match data.list_indexes().await { - Ok(indexes) => Ok(HttpResponse::Ok().json(indexes)), - Err(e) => { - Ok(HttpResponse::BadRequest().json(serde_json::json!({ "error": e.to_string() }))) - } - } + let indexes = data.list_indexes().await?; + Ok(HttpResponse::Ok().json(indexes)) } #[get("/indexes/{index_uid}", wrap = "Authentication::Private")] @@ -33,12 +29,8 @@ async fn get_index( data: web::Data, path: web::Path, ) -> Result { - match data.index(path.index_uid.clone()).await { - Ok(meta) => Ok(HttpResponse::Ok().json(meta)), - Err(e) => { - Ok(HttpResponse::BadRequest().json(serde_json::json!({ "error": e.to_string() }))) - } - } + let meta = data.index(path.index_uid.clone()).await?; + Ok(HttpResponse::Ok().json(meta)) } #[derive(Debug, Deserialize)] @@ -54,12 +46,8 @@ async fn create_index( body: web::Json, ) -> Result { let body = body.into_inner(); - match data.create_index(body.uid, body.primary_key).await { - Ok(meta) => Ok(HttpResponse::Ok().json(meta)), - Err(e) => { - Ok(HttpResponse::BadRequest().json(serde_json::json!({ "error": e.to_string() }))) - } - } + let meta = data.create_index(body.uid, body.primary_key).await?; + Ok(HttpResponse::Ok().json(meta)) } #[derive(Debug, Deserialize)] @@ -86,15 +74,10 @@ async fn update_index( body: web::Json, ) -> Result { let body = body.into_inner(); - match data + let meta = data .update_index(path.into_inner().index_uid, body.primary_key, body.uid) - .await - { - Ok(meta) => Ok(HttpResponse::Ok().json(meta)), - Err(e) => { - Ok(HttpResponse::BadRequest().json(serde_json::json!({ "error": e.to_string() }))) - } - } + .await?; + Ok(HttpResponse::Ok().json(meta)) } #[delete("/indexes/{index_uid}", wrap = "Authentication::Private")] @@ -102,12 +85,8 @@ async fn delete_index( data: web::Data, path: web::Path, ) -> Result { - match data.delete_index(path.index_uid.clone()).await { - Ok(_) => Ok(HttpResponse::NoContent().finish()), - Err(e) => { - Ok(HttpResponse::BadRequest().json(serde_json::json!({ "error": e.to_string() }))) - } - } + data.delete_index(path.index_uid.clone()).await?; + Ok(HttpResponse::NoContent().finish()) } #[derive(Deserialize)] @@ -125,18 +104,11 @@ async fn get_update_status( path: web::Path, ) -> Result { let params = path.into_inner(); - let result = data + let meta = data .get_update_status(params.index_uid, params.update_id) - .await; - match result { - Ok(meta) => { - let meta = UpdateStatusResponse::from(meta); - Ok(HttpResponse::Ok().json(meta)) - }, - Err(e) => { - Ok(HttpResponse::BadRequest().json(serde_json::json!({ "error": e.to_string() }))) - } - } + .await?; + let meta = UpdateStatusResponse::from(meta); + Ok(HttpResponse::Ok().json(meta)) } #[get("/indexes/{index_uid}/updates", wrap = "Authentication::Private")] @@ -144,18 +116,11 @@ async fn get_all_updates_status( data: web::Data, path: web::Path, ) -> Result { - let result = data.get_updates_status(path.into_inner().index_uid).await; - match result { - Ok(metas) => { - let metas = metas - .into_iter() - .map(UpdateStatusResponse::from) - .collect::>(); + let metas = data.get_updates_status(path.into_inner().index_uid).await?; + let metas = metas + .into_iter() + .map(UpdateStatusResponse::from) + .collect::>(); - Ok(HttpResponse::Ok().json(metas)) - }, - Err(e) => { - Ok(HttpResponse::BadRequest().json(serde_json::json!({ "error": e.to_string() }))) - } - } + Ok(HttpResponse::Ok().json(metas)) } diff --git a/meilisearch-http/src/routes/mod.rs b/meilisearch-http/src/routes/mod.rs index 4c5ebbe21..f4581ebcb 100644 --- a/meilisearch-http/src/routes/mod.rs +++ b/meilisearch-http/src/routes/mod.rs @@ -4,6 +4,7 @@ use actix_web::{get, HttpResponse}; use chrono::{DateTime, Utc}; use serde::{Deserialize, Serialize}; +use crate::error::ResponseError; use crate::index::{Settings, Unchecked}; use crate::index_controller::{UpdateMeta, UpdateResult, UpdateStatus}; @@ -24,17 +25,19 @@ pub enum UpdateType { Customs, DocumentsAddition { #[serde(skip_serializing_if = "Option::is_none")] - number: Option + number: Option, }, DocumentsPartial { #[serde(skip_serializing_if = "Option::is_none")] - number: Option + number: Option, }, DocumentsDeletion { #[serde(skip_serializing_if = "Option::is_none")] - number: Option + number: Option, + }, + Settings { + settings: Settings, }, - Settings { settings: Settings }, } impl From<&UpdateStatus> for UpdateType { @@ -60,9 +63,9 @@ impl From<&UpdateStatus> for UpdateType { } } UpdateMeta::ClearDocuments => UpdateType::ClearAll, - UpdateMeta::DeleteDocuments { ids } => { - UpdateType::DocumentsDeletion { number: Some(ids.len()) } - } + UpdateMeta::DeleteDocuments { ids } => UpdateType::DocumentsDeletion { + number: Some(ids.len()), + }, UpdateMeta::Settings(settings) => UpdateType::Settings { settings: settings.clone(), }, @@ -87,10 +90,8 @@ pub struct FailedUpdateResult { pub update_id: u64, #[serde(rename = "type")] pub update_type: UpdateType, - pub error: String, - pub error_type: String, - pub error_code: String, - pub error_link: String, + #[serde(flatten)] + pub response: ResponseError, pub duration: f64, // in seconds pub enqueued_at: DateTime, pub processed_at: DateTime, @@ -179,13 +180,13 @@ impl From for UpdateStatusResponse { // necessary since chrono::duration don't expose a f64 secs method. let duration = Duration::from_millis(duration as u64).as_secs_f64(); + let update_id = failed.id(); + let response = failed.error; + let content = FailedUpdateResult { - update_id: failed.id(), + update_id, update_type, - error: failed.error, - error_type: String::from("todo"), - error_code: String::from("todo"), - error_link: String::from("todo"), + response, duration, enqueued_at: failed.from.from.enqueued_at, processed_at: failed.failed_at, diff --git a/meilisearch-http/src/routes/search.rs b/meilisearch-http/src/routes/search.rs index 36f5bdf4d..e8b197caa 100644 --- a/meilisearch-http/src/routes/search.rs +++ b/meilisearch-http/src/routes/search.rs @@ -1,9 +1,8 @@ use std::collections::{BTreeSet, HashSet}; -use std::convert::{TryFrom, TryInto}; use actix_web::{get, post, web, HttpResponse}; -use serde_json::Value; use serde::Deserialize; +use serde_json::Value; use crate::error::ResponseError; use crate::helpers::Authentication; @@ -30,10 +29,8 @@ pub struct SearchQueryGet { facet_distributions: Option, } -impl TryFrom for SearchQuery { - type Error = anyhow::Error; - - fn try_from(other: SearchQueryGet) -> anyhow::Result { +impl From for SearchQuery { + fn from(other: SearchQueryGet) -> Self { let attributes_to_retrieve = other .attributes_to_retrieve .map(|attrs| attrs.split(',').map(String::from).collect::>()); @@ -51,16 +48,14 @@ impl TryFrom for SearchQuery { .map(|attrs| attrs.split(',').map(String::from).collect::>()); let filter = match other.filter { - Some(f) => { - match serde_json::from_str(&f) { - Ok(v) => Some(v), - _ => Some(Value::String(f)), - } + Some(f) => match serde_json::from_str(&f) { + Ok(v) => Some(v), + _ => Some(Value::String(f)), }, None => None, }; - Ok(Self { + Self { q: other.q, offset: other.offset, limit: other.limit.unwrap_or(DEFAULT_SEARCH_LIMIT), @@ -71,7 +66,7 @@ impl TryFrom for SearchQuery { filter, matches: other.matches, facet_distributions, - }) + } } } @@ -81,21 +76,9 @@ async fn search_with_url_query( path: web::Path, params: web::Query, ) -> Result { - let query: SearchQuery = match params.into_inner().try_into() { - Ok(q) => q, - Err(e) => { - return Ok( - HttpResponse::BadRequest().json(serde_json::json!({ "error": e.to_string() })) - ) - } - }; - let search_result = data.search(path.into_inner().index_uid, query).await; - match search_result { - Ok(docs) => Ok(HttpResponse::Ok().json(docs)), - Err(e) => { - Ok(HttpResponse::BadRequest().json(serde_json::json!({ "error": e.to_string() }))) - } - } + let query = params.into_inner().into(); + let search_result = data.search(path.into_inner().index_uid, query).await?; + Ok(HttpResponse::Ok().json(search_result)) } #[post("/indexes/{index_uid}/search", wrap = "Authentication::Public")] @@ -106,11 +89,6 @@ async fn search_with_post( ) -> Result { let search_result = data .search(path.into_inner().index_uid, params.into_inner()) - .await; - match search_result { - Ok(docs) => Ok(HttpResponse::Ok().json(docs)), - Err(e) => { - Ok(HttpResponse::BadRequest().json(serde_json::json!({ "error": e.to_string() }))) - } - } + .await?; + Ok(HttpResponse::Ok().json(search_result)) } diff --git a/meilisearch-http/src/routes/settings/mod.rs b/meilisearch-http/src/routes/settings.rs similarity index 66% rename from meilisearch-http/src/routes/settings/mod.rs rename to meilisearch-http/src/routes/settings.rs index 45d49132c..10ce7292e 100644 --- a/meilisearch-http/src/routes/settings/mod.rs +++ b/meilisearch-http/src/routes/settings.rs @@ -26,14 +26,8 @@ macro_rules! make_setting_route { $attr: Some(None), ..Default::default() }; - match data.update_settings(index_uid.into_inner(), settings, false).await { - Ok(update_status) => { - Ok(HttpResponse::Accepted().json(serde_json::json!({ "updateId": update_status.id() }))) - } - Err(e) => { - Ok(HttpResponse::BadRequest().json(serde_json::json!({ "error": e.to_string() }))) - } - } + let update_status = data.update_settings(index_uid.into_inner(), settings, false).await?; + Ok(HttpResponse::Accepted().json(serde_json::json!({ "updateId": update_status.id() }))) } #[actix_web::post($route, wrap = "Authentication::Private")] @@ -47,14 +41,8 @@ macro_rules! make_setting_route { ..Default::default() }; - match data.update_settings(index_uid.into_inner(), settings, true).await { - Ok(update_status) => { - Ok(HttpResponse::Accepted().json(serde_json::json!({ "updateId": update_status.id() }))) - } - Err(e) => { - Ok(HttpResponse::BadRequest().json(serde_json::json!({ "error": e.to_string() }))) - } - } + let update_status = data.update_settings(index_uid.into_inner(), settings, true).await?; + Ok(HttpResponse::Accepted().json(serde_json::json!({ "updateId": update_status.id() }))) } #[actix_web::get($route, wrap = "Authentication::Private")] @@ -62,12 +50,8 @@ macro_rules! make_setting_route { data: actix_web::web::Data, index_uid: actix_web::web::Path, ) -> std::result::Result { - match data.settings(index_uid.into_inner()).await { - Ok(settings) => Ok(HttpResponse::Ok().json(settings.$attr)), - Err(e) => { - Ok(HttpResponse::BadRequest().json(serde_json::json!({ "error": e.to_string() }))) - } - } + let settings = data.settings(index_uid.into_inner()).await?; + Ok(HttpResponse::Ok().json(settings.$attr)) } } }; @@ -148,17 +132,11 @@ async fn update_all( body: web::Json>, ) -> Result { let settings = body.into_inner().check(); - match data + let update_result = data .update_settings(index_uid.into_inner(), settings, true) - .await - { - Ok(update_result) => Ok( - HttpResponse::Accepted().json(serde_json::json!({ "updateId": update_result.id() })) - ), - Err(e) => { - Ok(HttpResponse::BadRequest().json(serde_json::json!({ "error": e.to_string() }))) - } - } + .await?; + let json = serde_json::json!({ "updateId": update_result.id() }); + Ok(HttpResponse::Accepted().json(json)) } #[get("/indexes/{index_uid}/settings", wrap = "Authentication::Private")] @@ -166,12 +144,8 @@ async fn get_all( data: web::Data, index_uid: web::Path, ) -> Result { - match data.settings(index_uid.into_inner()).await { - Ok(settings) => Ok(HttpResponse::Ok().json(settings)), - Err(e) => { - Ok(HttpResponse::BadRequest().json(serde_json::json!({ "error": e.to_string() }))) - } - } + let settings = data.settings(index_uid.into_inner()).await?; + Ok(HttpResponse::Ok().json(settings)) } #[delete("/indexes/{index_uid}/settings", wrap = "Authentication::Private")] @@ -180,15 +154,9 @@ async fn delete_all( index_uid: web::Path, ) -> Result { let settings = Settings::cleared(); - match data + let update_result = data .update_settings(index_uid.into_inner(), settings, false) - .await - { - Ok(update_result) => Ok( - HttpResponse::Accepted().json(serde_json::json!({ "updateId": update_result.id() })) - ), - Err(e) => { - Ok(HttpResponse::BadRequest().json(serde_json::json!({ "error": e.to_string() }))) - } - } + .await?; + let json = serde_json::json!({ "updateId": update_result.id() }); + Ok(HttpResponse::Accepted().json(json)) } diff --git a/meilisearch-http/src/routes/settings/distinct_attributes.rs b/meilisearch-http/src/routes/settings/distinct_attributes.rs deleted file mode 100644 index 7b991f861..000000000 --- a/meilisearch-http/src/routes/settings/distinct_attributes.rs +++ /dev/null @@ -1,36 +0,0 @@ -use crate::make_update_delete_routes; -use actix_web::{web, HttpResponse, get}; - -use crate::error::{Error, ResponseError}; -use crate::helpers::Authentication; -use crate::Data; - -#[get( - "/indexes/{index_uid}/settings/distinct-attribute", - wrap = "Authentication::Private" -)] -async fn get( - data: web::Data, - index_uid: web::Path, -) -> Result { - let index = data - .db - .load() - .open_index(&index_uid.as_ref()) - .ok_or(Error::index_not_found(&index_uid.as_ref()))?; - let reader = data.db.load().main_read_txn()?; - let distinct_attribute_id = index.main.distinct_attribute(&reader)?; - let schema = index.main.schema(&reader)?; - let distinct_attribute = match (schema, distinct_attribute_id) { - (Some(schema), Some(id)) => schema.name(id).map(str::to_string), - _ => None, - }; - - Ok(HttpResponse::Ok().json(distinct_attribute)) -} - -make_update_delete_routes!( - "/indexes/{index_uid}/settings/distinct-attribute", - String, - distinct_attribute -); diff --git a/meilisearch-http/src/routes/settings/ranking_rules.rs b/meilisearch-http/src/routes/settings/ranking_rules.rs deleted file mode 100644 index e0872954a..000000000 --- a/meilisearch-http/src/routes/settings/ranking_rules.rs +++ /dev/null @@ -1,23 +0,0 @@ -use crate::make_update_delete_routes; -use actix_web::{web, HttpResponse, get}; - -use crate::error::{Error, ResponseError}; -use crate::helpers::Authentication; -use crate::Data; - -#[get( - "/indexes/{index_uid}/settings/ranking-rules", - wrap = "Authentication::Private" -)] -async fn get( - data: web::Data, - index_uid: web::Path, -) -> Result { - todo!() -} - -make_update_delete_routes!( - "/indexes/{index_uid}/settings/ranking-rules", - Vec, - ranking_rules -); diff --git a/meilisearch-http/src/routes/settings/synonyms.rs b/meilisearch-http/src/routes/settings/synonyms.rs deleted file mode 100644 index e5b5b2afd..000000000 --- a/meilisearch-http/src/routes/settings/synonyms.rs +++ /dev/null @@ -1,43 +0,0 @@ -use std::collections::BTreeMap; - -use actix_web::{web, HttpResponse, get}; -use indexmap::IndexMap; - -use crate::error::{Error, ResponseError}; -use crate::helpers::Authentication; -use crate::make_update_delete_routes; -use crate::Data; - -#[get( - "/indexes/{index_uid}/settings/synonyms", - wrap = "Authentication::Private" -)] -async fn get( - data: web::Data, - index_uid: web::Path, -) -> Result { - let index = data - .db - .load() - .open_index(&index_uid.as_ref()) - .ok_or(Error::index_not_found(&index_uid.as_ref()))?; - - let reader = data.db.load().main_read_txn()?; - - let synonyms_list = index.main.synonyms(&reader)?; - - let mut synonyms = IndexMap::new(); - let index_synonyms = &index.synonyms; - for synonym in synonyms_list { - let list = index_synonyms.synonyms(&reader, synonym.as_bytes())?; - synonyms.insert(synonym, list); - } - - Ok(HttpResponse::Ok().json(synonyms)) -} - -make_update_delete_routes!( - "/indexes/{index_uid}/settings/synonyms", - BTreeMap>, - synonyms -); diff --git a/meilisearch-http/tests/documents/delete_documents.rs b/meilisearch-http/tests/documents/delete_documents.rs index d9b97d68d..d5794e40c 100644 --- a/meilisearch-http/tests/documents/delete_documents.rs +++ b/meilisearch-http/tests/documents/delete_documents.rs @@ -6,7 +6,7 @@ use crate::common::{GetAllDocumentsOptions, Server}; async fn delete_one_document_unexisting_index() { let server = Server::new().await; let (_response, code) = server.index("test").delete_document(0).await; - assert_eq!(code, 400); + assert_eq!(code, 404); } #[actix_rt::test] @@ -33,14 +33,14 @@ async fn delete_one_document() { index.wait_update_id(1).await; let (_response, code) = index.get_document(0, None).await; - assert_eq!(code, 400); + assert_eq!(code, 404); } #[actix_rt::test] async fn clear_all_documents_unexisting_index() { let server = Server::new().await; let (_response, code) = server.index("test").clear_all_documents().await; - assert_eq!(code, 400); + assert_eq!(code, 404); } #[actix_rt::test] @@ -86,7 +86,7 @@ async fn clear_all_documents_empty_index() { async fn delete_batch_unexisting_index() { let server = Server::new().await; let (_response, code) = server.index("test").delete_batch(vec![]).await; - assert_eq!(code, 400); + assert_eq!(code, 404); } #[actix_rt::test] diff --git a/meilisearch-http/tests/documents/get_documents.rs b/meilisearch-http/tests/documents/get_documents.rs index 24809532e..945bd6b5c 100644 --- a/meilisearch-http/tests/documents/get_documents.rs +++ b/meilisearch-http/tests/documents/get_documents.rs @@ -9,7 +9,7 @@ use serde_json::json; async fn get_unexisting_index_single_document() { let server = Server::new().await; let (_response, code) = server.index("test").get_document(1, None).await; - assert_eq!(code, 400); + assert_eq!(code, 404); } #[actix_rt::test] @@ -18,7 +18,7 @@ async fn get_unexisting_document() { let index = server.index("test"); index.create(None).await; let (_response, code) = index.get_document(1, None).await; - assert_eq!(code, 400); + assert_eq!(code, 404); } #[actix_rt::test] @@ -53,7 +53,7 @@ async fn get_unexisting_index_all_documents() { .index("test") .get_all_documents(GetAllDocumentsOptions::default()) .await; - assert_eq!(code, 400); + assert_eq!(code, 404); } #[actix_rt::test] diff --git a/meilisearch-http/tests/index/create_index.rs b/meilisearch-http/tests/index/create_index.rs index a61524d04..e65908cb2 100644 --- a/meilisearch-http/tests/index/create_index.rs +++ b/meilisearch-http/tests/index/create_index.rs @@ -70,5 +70,5 @@ async fn test_create_multiple_indexes() { assert_eq!(index1.get().await.1, 200); assert_eq!(index2.get().await.1, 200); assert_eq!(index3.get().await.1, 200); - assert_eq!(index4.get().await.1, 400); + assert_eq!(index4.get().await.1, 404); } diff --git a/meilisearch-http/tests/index/delete_index.rs b/meilisearch-http/tests/index/delete_index.rs index 0d1a7c820..b0f067b24 100644 --- a/meilisearch-http/tests/index/delete_index.rs +++ b/meilisearch-http/tests/index/delete_index.rs @@ -12,7 +12,7 @@ async fn create_and_delete_index() { assert_eq!(code, 204); - assert_eq!(index.get().await.1, 400); + assert_eq!(index.get().await.1, 404); } #[actix_rt::test] @@ -21,5 +21,5 @@ async fn delete_unexisting_index() { let index = server.index("test"); let (_response, code) = index.delete().await; - assert_eq!(code, 400); + assert_eq!(code, 404); } diff --git a/meilisearch-http/tests/index/get_index.rs b/meilisearch-http/tests/index/get_index.rs index ce4d7b792..a6b22509e 100644 --- a/meilisearch-http/tests/index/get_index.rs +++ b/meilisearch-http/tests/index/get_index.rs @@ -21,7 +21,7 @@ async fn create_and_get_index() { assert_eq!(response.as_object().unwrap().len(), 5); } -// TODO: partial test since we are testing error, amd error is not yet fully implemented in +// 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() { @@ -30,7 +30,7 @@ async fn get_unexisting_index() { let (_response, code) = index.get().await; - assert_eq!(code, 400); + assert_eq!(code, 404); } #[actix_rt::test] diff --git a/meilisearch-http/tests/index/update_index.rs b/meilisearch-http/tests/index/update_index.rs index 9c78ccb16..c7d910b59 100644 --- a/meilisearch-http/tests/index/update_index.rs +++ b/meilisearch-http/tests/index/update_index.rs @@ -60,5 +60,5 @@ async fn update_existing_primary_key() { async fn test_unexisting_index() { let server = Server::new().await; let (_response, code) = server.index("test").update(None).await; - assert_eq!(code, 400); + assert_eq!(code, 404); } diff --git a/meilisearch-http/tests/settings/get_settings.rs b/meilisearch-http/tests/settings/get_settings.rs index a03d907a7..804167517 100644 --- a/meilisearch-http/tests/settings/get_settings.rs +++ b/meilisearch-http/tests/settings/get_settings.rs @@ -5,7 +5,7 @@ use serde_json::json; async fn get_settings_unexisting_index() { let server = Server::new().await; let (_response, code) = server.index("test").settings().await; - assert_eq!(code, 400) + assert_eq!(code, 404) } #[actix_rt::test] @@ -65,7 +65,7 @@ async fn delete_settings_unexisting_index() { let server = Server::new().await; let index = server.index("test"); let (_response, code) = index.delete_settings().await; - assert_eq!(code, 400); + assert_eq!(code, 404); } #[actix_rt::test] @@ -152,7 +152,7 @@ macro_rules! test_setting_routes { .map(|c| if c == '_' { '-' } else { c }) .collect::()); let (_response, code) = server.service.get(url).await; - assert_eq!(code, 400); + assert_eq!(code, 404); } #[actix_rt::test] @@ -178,7 +178,7 @@ macro_rules! test_setting_routes { .map(|c| if c == '_' { '-' } else { c }) .collect::()); let (_response, code) = server.service.delete(url).await; - assert_eq!(code, 400); + assert_eq!(code, 404); } } )* diff --git a/meilisearch-http/tests/stats/mod.rs b/meilisearch-http/tests/stats/mod.rs index 7b9ab10b0..5a065d1d2 100644 --- a/meilisearch-http/tests/stats/mod.rs +++ b/meilisearch-http/tests/stats/mod.rs @@ -39,8 +39,6 @@ async fn stats() { assert_eq!(response["indexes"]["test"]["numberOfDocuments"], 0); assert!(response["indexes"]["test"]["isIndexing"] == false); - let last_update = response["lastUpdate"].as_str().unwrap(); - let documents = json!([ { "id": 1, @@ -66,7 +64,7 @@ async fn stats() { assert_eq!(code, 200); assert!(response["databaseSize"].as_u64().unwrap() > 0); - assert!(response["lastUpdate"].as_str().unwrap() > last_update); + assert!(response.get("lastUpdate").is_some()); assert_eq!(response["indexes"]["test"]["numberOfDocuments"], 2); assert!(response["indexes"]["test"]["isIndexing"] == false); assert_eq!(response["indexes"]["test"]["fieldsDistribution"]["id"], 2); diff --git a/meilisearch-http/tests/updates/mod.rs b/meilisearch-http/tests/updates/mod.rs index 6ce5ca381..00bbf32a8 100644 --- a/meilisearch-http/tests/updates/mod.rs +++ b/meilisearch-http/tests/updates/mod.rs @@ -4,7 +4,7 @@ use crate::common::Server; async fn get_update_unexisting_index() { let server = Server::new().await; let (_response, code) = server.index("test").get_update(0).await; - assert_eq!(code, 400); + assert_eq!(code, 404); } #[actix_rt::test] @@ -13,7 +13,7 @@ async fn get_unexisting_update_status() { let index = server.index("test"); index.create(None).await; let (_response, code) = index.get_update(0).await; - assert_eq!(code, 400); + assert_eq!(code, 404); } #[actix_rt::test] @@ -39,7 +39,7 @@ async fn get_update_status() { async fn list_updates_unexisting_index() { let server = Server::new().await; let (_response, code) = server.index("test").list_updates().await; - assert_eq!(code, 400); + assert_eq!(code, 404); } #[actix_rt::test]