Skip to content
This repository has been archived by the owner on Apr 4, 2023. It is now read-only.

Provide a sort error handler #371

Merged
merged 2 commits into from
Sep 28, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions http-ui/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -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 } =
Expand Down
64 changes: 63 additions & 1 deletion milli/src/asc_desc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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<AscDescError> 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<SortError> for Error {
fn from(error: SortError) -> Self {
Self::UserError(UserError::SortError(error))
}
}

#[cfg(test)]
mod tests {
use big_s::S;
Expand Down
16 changes: 3 additions & 13 deletions milli/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<String, Value>;

Expand Down Expand Up @@ -61,8 +61,6 @@ pub enum UserError {
InvalidFacetsDistribution { invalid_facets_name: HashSet<String> },
InvalidFilter(pest::error::Error<ParserRule>),
InvalidFilterAttribute(pest::error::Error<ParserRule>),
InvalidSortName { name: String },
InvalidReservedSortName { name: String },
InvalidGeoField { document_id: Value, object: Value },
InvalidSortableAttribute { field: String, valid_fields: HashSet<String> },
SortRankingRuleMissing,
Expand All @@ -74,6 +72,7 @@ pub enum UserError {
PrimaryKeyCannotBeChanged,
PrimaryKeyCannotBeReset,
SerdeJson(serde_json::Error),
SortError(SortError),
UnknownInternalDocumentId { document_id: DocumentId },
}

Expand Down Expand Up @@ -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!(
Expand All @@ -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::<Vec<_>>().join(", ");
Expand Down Expand Up @@ -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)
}
Expand Down
2 changes: 1 addition & 1 deletion milli/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down