342: Let the caller decide what kind of error they want to returns when parsing `AscDesc` r=Kerollmops a=irevoire

This is one possible fix for #339 
We would then need to patch these lines https://github.com/meilisearch/MeiliSearch/blob/main/meilisearch-http/src/index/search.rs#L110-L114 to return the error we want.

Another solution would be to add a parameter to the `from_str` to specify which context we are in.

Co-authored-by: Tamo <tamo@meilisearch.com>
This commit is contained in:
bors[bot] 2021-09-08 14:18:57 +00:00 committed by GitHub
commit b22aac92ac
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 17 additions and 2 deletions

View File

@ -52,7 +52,12 @@ impl FromStr for Criterion {
text => match AscDesc::from_str(text) { text => match AscDesc::from_str(text) {
Ok(AscDesc::Asc(field)) => Ok(Criterion::Asc(field)), Ok(AscDesc::Asc(field)) => Ok(Criterion::Asc(field)),
Ok(AscDesc::Desc(field)) => Ok(Criterion::Desc(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 { impl FromStr for AscDesc {
type Err = UserError; 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> { fn from_str(text: &str) -> Result<AscDesc, Self::Err> {
match text.rsplit_once(':') { match text.rsplit_once(':') {
Some((field_name, "asc")) => Ok(AscDesc::Asc(field_name.to_string())), Some((field_name, "asc")) => Ok(AscDesc::Asc(field_name.to_string())),
Some((field_name, "desc")) => Ok(AscDesc::Desc(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() }),
} }
} }
} }

View File

@ -53,11 +53,13 @@ pub enum UserError {
AttributeLimitReached, AttributeLimitReached,
Csv(csv::Error), Csv(csv::Error),
DocumentLimitReached, DocumentLimitReached,
InvalidAscDescSyntax { name: String },
InvalidCriterionName { name: String }, InvalidCriterionName { name: String },
InvalidDocumentId { document_id: Value }, InvalidDocumentId { document_id: Value },
InvalidFacetsDistribution { invalid_facets_name: HashSet<String> }, InvalidFacetsDistribution { invalid_facets_name: HashSet<String> },
InvalidFilter(pest::error::Error<ParserRule>), InvalidFilter(pest::error::Error<ParserRule>),
InvalidFilterAttribute(pest::error::Error<ParserRule>), InvalidFilterAttribute(pest::error::Error<ParserRule>),
InvalidSortName { name: String },
InvalidSortableAttribute { field: String, valid_fields: HashSet<String> }, InvalidSortableAttribute { field: String, valid_fields: HashSet<String> },
SortRankingRuleMissing, SortRankingRuleMissing,
InvalidStoreFile, InvalidStoreFile,
@ -216,6 +218,9 @@ impl fmt::Display for UserError {
) )
} }
Self::InvalidFilter(error) => error.fmt(f), 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::InvalidCriterionName { name } => write!(f, "invalid criterion {}", name),
Self::InvalidDocumentId { document_id } => { Self::InvalidDocumentId { document_id } => {
let json = serde_json::to_string(document_id).unwrap(); 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::InvalidFilterAttribute(error) => error.fmt(f),
Self::InvalidSortName { name } => {
write!(f, "Invalid syntax for the sort parameter: {}", name)
}
Self::InvalidSortableAttribute { field, valid_fields } => { Self::InvalidSortableAttribute { field, valid_fields } => {
let valid_names = let valid_names =
valid_fields.iter().map(AsRef::as_ref).collect::<Vec<_>>().join(", "); valid_fields.iter().map(AsRef::as_ref).collect::<Vec<_>>().join(", ");