From e405702733f43df31c3d48dae53b194a87ce8d06 Mon Sep 17 00:00:00 2001 From: Filip Bachul Date: Tue, 7 Feb 2023 18:57:27 +0100 Subject: [PATCH] chore: introduce new error ParseGeoError type --- milli/src/search/facet/filter.rs | 93 ++++++++++++++++++++------------ 1 file changed, 60 insertions(+), 33 deletions(-) diff --git a/milli/src/search/facet/filter.rs b/milli/src/search/facet/filter.rs index 3cf11819f..548ed699e 100644 --- a/milli/src/search/facet/filter.rs +++ b/milli/src/search/facet/filter.rs @@ -22,12 +22,30 @@ pub struct Filter<'a> { } #[derive(Debug)] -enum FilterError<'a> { - AttributeNotFilterable { attribute: &'a str, filterable_fields: HashSet }, - BadGeo(&'a str), +enum ParseGeoError { + BadGeo(String), BadGeoLat(f64), BadGeoLng(f64), BadGeoBoundingBoxTopIsBelowBottom(f64, f64), +} + +impl std::error::Error for ParseGeoError {} + +impl Display for ParseGeoError { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + Self::BadGeo(keyword) => write!(f, "`{}` is a reserved keyword and thus can't be used as a filter expression. Use the `_geoRadius(latitude, longitude, distance)` or `_geoBoundingBox([latitude, longitude], [latitude, longitude])` built-in rules to filter on `_geo` field coordinates.", keyword), + Self::BadGeoBoundingBoxTopIsBelowBottom(top, bottom) => write!(f, "The top latitude `{top}` is below the bottom latitude `{bottom}`."), + Self::BadGeoLat(lat) => write!(f, "Bad latitude `{}`. Latitude must be contained between -90 and 90 degrees. ", lat), + Self::BadGeoLng(lng) => write!(f, "Bad longitude `{}`. Longitude must be contained between -180 and 180 degrees. ", lng), + } + } +} + +#[derive(Debug)] +enum FilterError<'a> { + AttributeNotFilterable { attribute: &'a str, filterable_fields: HashSet }, + ParseGeoError(ParseGeoError), Reserved(&'a str), TooDeep, } @@ -44,7 +62,11 @@ impl<'a> Display for FilterError<'a> { attribute, ) } else { - let filterables_list = filterable_fields.iter().map(AsRef::as_ref).collect::>().join(" "); + let filterables_list = filterable_fields + .iter() + .map(AsRef::as_ref) + .collect::>() + .join(" "); write!( f, @@ -53,8 +75,9 @@ impl<'a> Display for FilterError<'a> { filterables_list, ) } - }, - Self::TooDeep => write!(f, + } + Self::TooDeep => write!( + f, "Too many filter conditions, can't process more than {} filters.", MAX_FILTER_DEPTH ), @@ -63,10 +86,7 @@ impl<'a> Display for FilterError<'a> { "`{}` is a reserved keyword and thus can't be used as a filter expression.", keyword ), - Self::BadGeo(keyword) => write!(f, "`{}` is a reserved keyword and thus can't be used as a filter expression. Use the `_geoRadius(latitude, longitude, distance)` or `_geoBoundingBox([latitude, longitude], [latitude, longitude])` built-in rules to filter on `_geo` field coordinates.", keyword), - Self::BadGeoBoundingBoxTopIsBelowBottom(top, bottom) => write!(f, "The top latitude `{top}` is below the bottom latitude `{bottom}`."), - Self::BadGeoLat(lat) => write!(f, "Bad latitude `{}`. Latitude must be contained between -90 and 90 degrees. ", lat), - Self::BadGeoLng(lng) => write!(f, "Bad longitude `{}`. Longitude must be contained between -180 and 180 degrees. ", lng), + Self::ParseGeoError(error) => write!(f, "{}", error), } } } @@ -297,12 +317,13 @@ impl<'a> Filter<'a> { } } else { match fid.value() { - attribute @ "_geo" => { - Err(fid.as_external_error(FilterError::BadGeo(attribute)))? - } - attribute if attribute.starts_with("_geoPoint(") => { - Err(fid.as_external_error(FilterError::BadGeo("_geoPoint")))? - } + attribute @ "_geo" => Err(fid.as_external_error( + FilterError::ParseGeoError(ParseGeoError::BadGeo(attribute.to_owned())), + ))?, + attribute if attribute.starts_with("_geoPoint(") => Err(fid + .as_external_error(FilterError::ParseGeoError( + ParseGeoError::BadGeo("_geoPoint".to_owned()), + )))?, attribute @ "_geoDistance" => { Err(fid.as_external_error(FilterError::Reserved(attribute)))? } @@ -353,14 +374,14 @@ impl<'a> Filter<'a> { let base_point: [f64; 2] = [point[0].parse_finite_float()?, point[1].parse_finite_float()?]; if !(-90.0..=90.0).contains(&base_point[0]) { - return Err( - point[0].as_external_error(FilterError::BadGeoLat(base_point[0])) - )?; + return Err(point[0].as_external_error(FilterError::ParseGeoError( + ParseGeoError::BadGeoLat(base_point[0]), + )))?; } if !(-180.0..=180.0).contains(&base_point[1]) { - return Err( - point[1].as_external_error(FilterError::BadGeoLng(base_point[1])) - )?; + return Err(point[1].as_external_error(FilterError::ParseGeoError( + ParseGeoError::BadGeoLng(base_point[1]), + )))?; } let radius = radius.parse_finite_float()?; let rtree = match index.geo_rtree(rtxn)? { @@ -398,26 +419,32 @@ impl<'a> Filter<'a> { bottom_right_point[1].parse_finite_float()?, ]; if !(-90.0..=90.0).contains(&top_left[0]) { - return Err(top_left_point[0] - .as_external_error(FilterError::BadGeoLat(top_left[0])))?; + return Err(top_left_point[0].as_external_error( + FilterError::ParseGeoError(ParseGeoError::BadGeoLat(top_left[0])), + ))?; } if !(-180.0..=180.0).contains(&top_left[1]) { - return Err(top_left_point[1] - .as_external_error(FilterError::BadGeoLng(top_left[1])))?; + return Err(top_left_point[1].as_external_error( + FilterError::ParseGeoError(ParseGeoError::BadGeoLng(top_left[1])), + ))?; } if !(-90.0..=90.0).contains(&bottom_right[0]) { - return Err(bottom_right_point[0] - .as_external_error(FilterError::BadGeoLat(bottom_right[0])))?; + return Err(bottom_right_point[0].as_external_error( + FilterError::ParseGeoError(ParseGeoError::BadGeoLat(bottom_right[0])), + ))?; } if !(-180.0..=180.0).contains(&bottom_right[1]) { - return Err(bottom_right_point[1] - .as_external_error(FilterError::BadGeoLng(bottom_right[1])))?; + return Err(bottom_right_point[1].as_external_error( + FilterError::ParseGeoError(ParseGeoError::BadGeoLng(bottom_right[1])), + ))?; } if top_left[0] < bottom_right[0] { return Err(bottom_right_point[1].as_external_error( - FilterError::BadGeoBoundingBoxTopIsBelowBottom( - top_left[0], - bottom_right[0], + FilterError::ParseGeoError( + ParseGeoError::BadGeoBoundingBoxTopIsBelowBottom( + top_left[0], + bottom_right[0], + ), ), ))?; }