Merge #2460

2460: Create custom error types for `TaskType`, `TaskStatus`, and `IndexUid` r=Kerollmops a=walterbm

# Pull Request

## What does this PR do?
Fixes #2443 by making the following changes:

- Add custom `TaskTypeError` for `TaskType::from_str` 
- Add custom `TaskStatusError` for `TaskStatus::from_str`
- Add custom `IndexUidFormatError` for `IndexUid::from_str`
- Implement `From<IndexUidFormatError> for IndexResolverError` to convert between errors
- Replace all usages of `IndexUid::new` with `IndexUid::from_str`
    - **NOTE** I am relatively new to Rust and I struggled a lot with this final part. This PR ended up with a messy error conversion which does not seem ideal. Please let me know if you have any suggestions for how to make this better and I'll be happy to make any updates!

## PR checklist
Please check if your PR fulfills the following requirements:
- [x] Does this PR fix an existing issue?
- [x] Have you read the contributing guidelines?
- [x] Have you made sure that the title is accurate and descriptive of the changes?

Thank you so much for contributing to Meilisearch!


Co-authored-by: walter <walter.beller.morales@gmail.com>

meilisearch-http/src/task.rs

@@ -1,4 +1,5 @@
-use std::fmt::Write;
+use std::error::Error;
+use std::fmt::{self, Write};
 use std::str::FromStr;
 use std::write;
 
@@ -39,31 +40,47 @@ impl From<TaskContent> for TaskType {
     }
 }
 
-impl FromStr for TaskType {
-    type Err = String;
+#[derive(Debug)]
+pub struct TaskTypeError {
+    invalid_type: String,
+}
 
-    fn from_str(status: &str) -> Result<Self, String> {
-        if status.eq_ignore_ascii_case("indexCreation") {
+impl fmt::Display for TaskTypeError {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        write!(
+            f,
+            "invalid task type `{}`, expecting one of: \
+            indexCreation, indexUpdate, indexDeletion, documentAdditionOrUpdate, \
+            documentDeletion, settingsUpdate, dumpCreation",
+            self.invalid_type
+        )
+    }
+}
+
+impl Error for TaskTypeError {}
+
+impl FromStr for TaskType {
+    type Err = TaskTypeError;
+
+    fn from_str(type_: &str) -> Result<Self, TaskTypeError> {
+        if type_.eq_ignore_ascii_case("indexCreation") {
             Ok(TaskType::IndexCreation)
-        } else if status.eq_ignore_ascii_case("indexUpdate") {
+        } else if type_.eq_ignore_ascii_case("indexUpdate") {
             Ok(TaskType::IndexUpdate)
-        } else if status.eq_ignore_ascii_case("indexDeletion") {
+        } else if type_.eq_ignore_ascii_case("indexDeletion") {
             Ok(TaskType::IndexDeletion)
-        } else if status.eq_ignore_ascii_case("documentAdditionOrUpdate") {
+        } else if type_.eq_ignore_ascii_case("documentAdditionOrUpdate") {
             Ok(TaskType::DocumentAdditionOrUpdate)
-        } else if status.eq_ignore_ascii_case("documentDeletion") {
+        } else if type_.eq_ignore_ascii_case("documentDeletion") {
             Ok(TaskType::DocumentDeletion)
-        } else if status.eq_ignore_ascii_case("settingsUpdate") {
+        } else if type_.eq_ignore_ascii_case("settingsUpdate") {
             Ok(TaskType::SettingsUpdate)
-        } else if status.eq_ignore_ascii_case("dumpCreation") {
+        } else if type_.eq_ignore_ascii_case("dumpCreation") {
             Ok(TaskType::DumpCreation)
         } else {
-            Err(format!(
-                "invalid task type `{}`, expecting one of: \
-                indexCreation, indexUpdate, indexDeletion, documentAdditionOrUpdate, \
-                documentDeletion, settingsUpdate, dumpCreation",
-                status
-            ))
+            Err(TaskTypeError {
+                invalid_type: type_.to_string(),
+            })
         }
     }
 }
@@ -77,10 +94,28 @@ pub enum TaskStatus {
     Failed,
 }
 
-impl FromStr for TaskStatus {
-    type Err = String;
+#[derive(Debug)]
+pub struct TaskStatusError {
+    invalid_status: String,
+}
 
-    fn from_str(status: &str) -> Result<Self, String> {
+impl fmt::Display for TaskStatusError {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        write!(
+            f,
+            "invalid task status `{}`, expecting one of: \
+            enqueued, processing, succeeded, or failed",
+            self.invalid_status,
+        )
+    }
+}
+
+impl Error for TaskStatusError {}
+
+impl FromStr for TaskStatus {
+    type Err = TaskStatusError;
+
+    fn from_str(status: &str) -> Result<Self, TaskStatusError> {
         if status.eq_ignore_ascii_case("enqueued") {
             Ok(TaskStatus::Enqueued)
         } else if status.eq_ignore_ascii_case("processing") {
@@ -90,11 +125,9 @@ impl FromStr for TaskStatus {
         } else if status.eq_ignore_ascii_case("failed") {
             Ok(TaskStatus::Failed)
         } else {
-            Err(format!(
-                "invalid task status `{}`, expecting one of: \
-                enqueued, processing, succeeded, or failed",
-                status,
-            ))
+            Err(TaskStatusError {
+                invalid_status: status.to_string(),
+            })
         }
     }
 }

meilisearch-lib/src/index_controller/mod.rs

@@ -3,6 +3,7 @@ use std::collections::BTreeMap;
 use std::fmt;
 use std::io::Cursor;
 use std::path::{Path, PathBuf};
+use std::str::FromStr;
 use std::sync::Arc;
 use std::time::Duration;
 
@@ -23,6 +24,7 @@ use crate::dump::{self, load_dump, DumpHandler};
 use crate::index::{
     Checked, Document, IndexMeta, IndexStats, SearchQuery, SearchResult, Settings, Unchecked,
 };
+use crate::index_resolver::error::IndexResolverError;
 use crate::options::{IndexerOpts, SchedulerConfig};
 use crate::snapshot::{load_snapshot, SnapshotService};
 use crate::tasks::error::TaskError;
@@ -356,7 +358,7 @@ where
     }
 
     pub async fn register_update(&self, uid: String, update: Update) -> Result<Task> {
-        let index_uid = IndexUid::new(uid)?;
+        let index_uid = IndexUid::from_str(&uid).map_err(IndexResolverError::from)?;
         let content = match update {
             Update::DeleteDocuments(ids) => TaskContent::DocumentDeletion {
                 index_uid,

meilisearch-lib/src/index_resolver/mod.rs

@@ -3,6 +3,8 @@ pub mod index_store;
 pub mod meta_store;
 
 use std::convert::{TryFrom, TryInto};
+use std::error::Error;
+use std::fmt;
 use std::path::Path;
 use std::str::FromStr;
 use std::sync::Arc;
@@ -52,18 +54,6 @@ pub fn create_index_resolver(
 }
 
 impl IndexUid {
-    pub fn new(uid: String) -> Result<Self> {
-        if !uid
-            .chars()
-            .all(|x| x.is_ascii_alphanumeric() || x == '-' || x == '_')
-            || !(1..=400).contains(&uid.len())
-        {
-            Err(IndexResolverError::BadlyFormatted(uid))
-        } else {
-            Ok(Self(uid))
-        }
-    }
-
     pub fn new_unchecked(s: impl AsRef<str>) -> Self {
         Self(s.as_ref().to_string())
     }
@@ -87,18 +77,54 @@ impl std::ops::Deref for IndexUid {
 }
 
 impl TryInto<IndexUid> for String {
-    type Error = IndexResolverError;
+    type Error = IndexUidFormatError;
 
-    fn try_into(self) -> Result<IndexUid> {
-        IndexUid::new(self)
+    fn try_into(self) -> std::result::Result<IndexUid, IndexUidFormatError> {
+        IndexUid::from_str(&self)
+    }
+}
+
+#[derive(Debug)]
+pub struct IndexUidFormatError {
+    invalid_uid: String,
+}
+
+impl fmt::Display for IndexUidFormatError {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        write!(
+            f,
+            "invalid index uid `{}`, the uid must be an integer \
+            or a string containing only alphanumeric characters \
+            a-z A-Z 0-9, hyphens - and underscores _.",
+            self.invalid_uid,
+        )
+    }
+}
+
+impl Error for IndexUidFormatError {}
+
+impl From<IndexUidFormatError> for IndexResolverError {
+    fn from(error: IndexUidFormatError) -> Self {
+        Self::BadlyFormatted(error.invalid_uid)
     }
 }
 
 impl FromStr for IndexUid {
-    type Err = IndexResolverError;
+    type Err = IndexUidFormatError;
 
-    fn from_str(s: &str) -> Result<IndexUid> {
-        IndexUid::new(s.to_string())
+    fn from_str(uid: &str) -> std::result::Result<IndexUid, IndexUidFormatError> {
+        if !uid
+            .chars()
+            .all(|x| x.is_ascii_alphanumeric() || x == '-' || x == '_')
+            || uid.is_empty()
+            || uid.len() > 400
+        {
+            Err(IndexUidFormatError {
+                invalid_uid: uid.to_string(),
+            })
+        } else {
+            Ok(IndexUid(uid.to_string()))
+        }
     }
 }