From 932998f5ccdd909a44115f8355731a4557d59535 Mon Sep 17 00:00:00 2001
From: Tamo <tamo@meilisearch.com>
Date: Tue, 7 Sep 2021 11:41:25 +0200
Subject: [PATCH] let the caller decide if they want to return an
 invalidSortName or an invalidCriterionName error

---
 milli/src/criterion.rs | 11 +++++++++--
 milli/src/error.rs     |  8 ++++++++
 2 files changed, 17 insertions(+), 2 deletions(-)

diff --git a/milli/src/criterion.rs b/milli/src/criterion.rs
index 209a71b0d..d91d4a7e1 100644
--- a/milli/src/criterion.rs
+++ b/milli/src/criterion.rs
@@ -52,7 +52,12 @@ impl FromStr for Criterion {
             text => match AscDesc::from_str(text) {
                 Ok(AscDesc::Asc(field)) => Ok(Criterion::Asc(field)),
                 Ok(AscDesc::Desc(field)) => Ok(Criterion::Desc(field)),
-                Err(error) => Err(error.into()),
+                Err(UserError::InvalidAscDescSyntax { name }) => {
+                    Err(UserError::InvalidCriterionName { name }.into())
+                }
+                Err(error) => {
+                    Err(UserError::InvalidCriterionName { name: error.to_string() }.into())
+                }
             },
         }
     }
@@ -76,11 +81,13 @@ impl AscDesc {
 impl FromStr for AscDesc {
     type Err = UserError;
 
+    /// Since we don't know if this was deserialized for a criterion or a sort we just return a
+    /// string and let the caller create his own error
     fn from_str(text: &str) -> Result<AscDesc, Self::Err> {
         match text.rsplit_once(':') {
             Some((field_name, "asc")) => Ok(AscDesc::Asc(field_name.to_string())),
             Some((field_name, "desc")) => Ok(AscDesc::Desc(field_name.to_string())),
-            _ => Err(UserError::InvalidCriterionName { name: text.to_string() }),
+            _ => Err(UserError::InvalidAscDescSyntax { name: text.to_string() }),
         }
     }
 }
diff --git a/milli/src/error.rs b/milli/src/error.rs
index 56028f742..3f473a673 100644
--- a/milli/src/error.rs
+++ b/milli/src/error.rs
@@ -53,11 +53,13 @@ pub enum UserError {
     AttributeLimitReached,
     Csv(csv::Error),
     DocumentLimitReached,
+    InvalidAscDescSyntax { name: String },
     InvalidCriterionName { name: String },
     InvalidDocumentId { document_id: Value },
     InvalidFacetsDistribution { invalid_facets_name: HashSet<String> },
     InvalidFilter(pest::error::Error<ParserRule>),
     InvalidFilterAttribute(pest::error::Error<ParserRule>),
+    InvalidSortName { name: String },
     InvalidSortableAttribute { field: String, valid_fields: HashSet<String> },
     SortRankingRuleMissing,
     InvalidStoreFile,
@@ -216,6 +218,9 @@ impl fmt::Display for UserError {
                 )
             }
             Self::InvalidFilter(error) => error.fmt(f),
+            Self::InvalidAscDescSyntax { name } => {
+                write!(f, "invalid asc/desc syntax for {}", name)
+            }
             Self::InvalidCriterionName { name } => write!(f, "invalid criterion {}", name),
             Self::InvalidDocumentId { document_id } => {
                 let json = serde_json::to_string(document_id).unwrap();
@@ -228,6 +233,9 @@ only composed of alphanumeric characters (a-z A-Z 0-9), hyphens (-) and undersco
                 )
             }
             Self::InvalidFilterAttribute(error) => error.fmt(f),
+            Self::InvalidSortName { name } => {
+                write!(f, "Invalid syntax for the sort parameter: {}", name)
+            }
             Self::InvalidSortableAttribute { field, valid_fields } => {
                 let valid_names =
                     valid_fields.iter().map(AsRef::as_ref).collect::<Vec<_>>().join(", ");