From c7cb816ae155ef3ef2ce9daf904b98a66cd9e578 Mon Sep 17 00:00:00 2001 From: Tamo Date: Mon, 27 Sep 2021 19:07:22 +0200 Subject: [PATCH 1/2] simplify the error handling of the sort syntax for meilisearch --- milli/src/asc_desc.rs | 64 ++++++++++++++++++++++++++++++++++++++++++- milli/src/error.rs | 16 ++--------- milli/src/lib.rs | 2 +- 3 files changed, 67 insertions(+), 15 deletions(-) diff --git a/milli/src/asc_desc.rs b/milli/src/asc_desc.rs index 9a3bda934..5adef782f 100644 --- a/milli/src/asc_desc.rs +++ b/milli/src/asc_desc.rs @@ -6,7 +6,7 @@ use std::str::FromStr; use serde::{Deserialize, Serialize}; use crate::error::is_reserved_keyword; -use crate::CriterionError; +use crate::{CriterionError, Error, UserError}; /// This error type is never supposed to be shown to the end user. /// You must always cast it to a sort error or a criterion error. @@ -139,6 +139,68 @@ impl FromStr for AscDesc { } } +#[derive(Debug)] +pub enum SortError { + InvalidName { name: String }, + ReservedName { name: String }, + ReservedNameForSettings { name: String }, + ReservedNameForFilter { name: String }, +} + +impl From for SortError { + fn from(error: AscDescError) -> Self { + match error { + AscDescError::InvalidSyntax { name } => SortError::InvalidName { name }, + AscDescError::ReservedKeyword { name } if &name == "_geo" => { + SortError::ReservedNameForSettings { name: "_geoPoint".to_string() } + } + AscDescError::ReservedKeyword { name } if name.starts_with("_geoRadius") => { + SortError::ReservedNameForFilter { name: "_geoRadius".to_string() } + } + AscDescError::ReservedKeyword { name } => SortError::ReservedName { name }, + } + } +} + +impl fmt::Display for SortError { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + match self { + Self::InvalidName { name } => { + write!(f, "invalid syntax for the sort parameter {}", name) + } + Self::ReservedName { name } => { + write!( + f, + "{} is a reserved keyword and thus can't be used as a sort expression", + name + ) + } + Self::ReservedNameForSettings { name } => { + write!( + f, + "{} is a reserved keyword and thus can't be used as a sort expression. \ +{} can only be used in the settings", + name, name + ) + } + Self::ReservedNameForFilter { name } => { + write!( + f, + "{} is a reserved keyword and thus can't be used as a sort expression. \ +{} can only be used for filtering at search time", + name, name + ) + } + } + } +} + +impl From for Error { + fn from(error: SortError) -> Self { + Self::UserError(UserError::SortError(error)) + } +} + #[cfg(test)] mod tests { use big_s::S; diff --git a/milli/src/error.rs b/milli/src/error.rs index bd4f02b99..723d5c4c2 100644 --- a/milli/src/error.rs +++ b/milli/src/error.rs @@ -8,7 +8,7 @@ use rayon::ThreadPoolBuildError; use serde_json::{Map, Value}; use crate::search::ParserRule; -use crate::{CriterionError, DocumentId, FieldId}; +use crate::{CriterionError, DocumentId, FieldId, SortError}; pub type Object = Map; @@ -61,8 +61,6 @@ pub enum UserError { InvalidFacetsDistribution { invalid_facets_name: HashSet }, InvalidFilter(pest::error::Error), InvalidFilterAttribute(pest::error::Error), - InvalidSortName { name: String }, - InvalidReservedSortName { name: String }, InvalidGeoField { document_id: Value, object: Value }, InvalidSortableAttribute { field: String, valid_fields: HashSet }, SortRankingRuleMissing, @@ -74,6 +72,7 @@ pub enum UserError { PrimaryKeyCannotBeChanged, PrimaryKeyCannotBeReset, SerdeJson(serde_json::Error), + SortError(SortError), UnknownInternalDocumentId { document_id: DocumentId }, } @@ -227,13 +226,6 @@ impl fmt::Display for UserError { "the document with the id: {} contains an invalid _geo field: {}", document_id, object ), - Self::InvalidReservedSortName { name } => { - write!( - f, - "{} is a reserved keyword and thus can't be used as a sort expression", - name - ) - } Self::InvalidDocumentId { document_id } => { let json = serde_json::to_string(document_id).unwrap(); write!( @@ -245,9 +237,6 @@ 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::>().join(", "); @@ -277,6 +266,7 @@ only composed of alphanumeric characters (a-z A-Z 0-9), hyphens (-) and undersco f.write_str("primary key cannot be reset if the database contains documents") } Self::SerdeJson(error) => error.fmt(f), + Self::SortError(error) => write!(f, "{}", error), Self::UnknownInternalDocumentId { document_id } => { write!(f, "an unknown internal document id have been used ({})", document_id) } diff --git a/milli/src/lib.rs b/milli/src/lib.rs index 8a54bbbdf..bb0a32528 100644 --- a/milli/src/lib.rs +++ b/milli/src/lib.rs @@ -25,7 +25,7 @@ use fxhash::{FxHasher32, FxHasher64}; pub use grenad::CompressionType; use serde_json::{Map, Value}; -pub use self::asc_desc::{AscDesc, AscDescError, Member}; +pub use self::asc_desc::{AscDesc, AscDescError, Member, SortError}; pub use self::criterion::{default_criteria, Criterion, CriterionError}; pub use self::error::{ Error, FieldIdMapMissingEntry, InternalError, SerializationError, UserError, From cc732fe95ef448454cf6eb98243aef75de80590e Mon Sep 17 00:00:00 2001 From: Tamo Date: Tue, 28 Sep 2021 11:15:24 +0200 Subject: [PATCH 2/2] update http-ui to use the sort-error --- http-ui/src/main.rs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/http-ui/src/main.rs b/http-ui/src/main.rs index 1bacdfbed..b76547309 100644 --- a/http-ui/src/main.rs +++ b/http-ui/src/main.rs @@ -22,7 +22,9 @@ use meilisearch_tokenizer::{Analyzer, AnalyzerConfig}; use milli::documents::DocumentBatchReader; use milli::update::UpdateIndexingStep::*; use milli::update::{IndexDocumentsMethod, Setting, UpdateBuilder}; -use milli::{obkv_to_json, CompressionType, FilterCondition, Index, MatchingWords, SearchResult}; +use milli::{ + obkv_to_json, CompressionType, FilterCondition, Index, MatchingWords, SearchResult, SortError, +}; use once_cell::sync::OnceCell; use rayon::ThreadPool; use serde::{Deserialize, Serialize}; @@ -756,7 +758,7 @@ async fn main() -> anyhow::Result<()> { } if let Some(sort) = query.sort { - search.sort_criteria(vec![sort.parse().unwrap()]); + search.sort_criteria(vec![sort.parse().map_err(SortError::from).unwrap()]); } let SearchResult { matching_words, candidates, documents_ids } =