From 7a38fe624f5c8424230aae5c54a4cde35fa43cc7 Mon Sep 17 00:00:00 2001 From: Tamo Date: Mon, 6 Feb 2023 17:50:47 +0100 Subject: [PATCH] throw an error if the top left corner is found below the bottom right corner --- meilisearch/tests/search/errors.rs | 4 ++-- milli/src/index.rs | 21 ++++++++++++++------- milli/src/search/facet/filter.rs | 12 +++++++++++- milli/src/search/mod.rs | 2 +- 4 files changed, 28 insertions(+), 11 deletions(-) diff --git a/meilisearch/tests/search/errors.rs b/meilisearch/tests/search/errors.rs index 2c02dc0a3..3ef342171 100644 --- a/meilisearch/tests/search/errors.rs +++ b/meilisearch/tests/search/errors.rs @@ -540,7 +540,7 @@ async fn filter_reserved_geo_attribute_array() { index.wait_task(1).await; let expected_response = json!({ - "message": "`_geo` is a reserved keyword and thus can't be used as a filter expression. Use the _geoRadius(latitude, longitude, distance) built-in rule to filter on _geo field coordinates.\n1:5 _geo = Glass", + "message": "`_geo` 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.\n1:5 _geo = Glass", "code": "invalid_search_filter", "type": "invalid_request", "link": "https://docs.meilisearch.com/errors#invalid-search-filter" @@ -565,7 +565,7 @@ async fn filter_reserved_geo_attribute_string() { index.wait_task(1).await; let expected_response = json!({ - "message": "`_geo` is a reserved keyword and thus can't be used as a filter expression. Use the _geoRadius(latitude, longitude, distance) built-in rule to filter on _geo field coordinates.\n1:5 _geo = Glass", + "message": "`_geo` 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.\n1:5 _geo = Glass", "code": "invalid_search_filter", "type": "invalid_request", "link": "https://docs.meilisearch.com/errors#invalid-search-filter" diff --git a/milli/src/index.rs b/milli/src/index.rs index 1c0482bca..972ed789e 100644 --- a/milli/src/index.rs +++ b/milli/src/index.rs @@ -1583,20 +1583,27 @@ pub(crate) mod tests { .unwrap(); insta::assert_debug_snapshot!(search_result.candidates, @"RoaringBitmap<[4]>"); + // the requests that doesn't make sense + // try to wrap around the latitude - let search_result = search + let error = search .filter(Filter::from_str("_geoBoundingBox([-80, 0], [80, 0])").unwrap().unwrap()) .execute() - .unwrap(); - insta::assert_debug_snapshot!(search_result.candidates, @"RoaringBitmap<[]>"); + .unwrap_err(); + insta::assert_display_snapshot!(error, @r###" + The top latitude `-80` is below the bottom latitude `80`. + 32:33 _geoBoundingBox([-80, 0], [80, 0]) + "###); - // the request that doesn't make sense // send a top latitude lower than the bottow latitude - let search_result = search + let error = search .filter(Filter::from_str("_geoBoundingBox([-10, 0], [10, 0])").unwrap().unwrap()) .execute() - .unwrap(); - insta::assert_debug_snapshot!(search_result.candidates, @"RoaringBitmap<[]>"); + .unwrap_err(); + insta::assert_display_snapshot!(error, @r###" + The top latitude `-10` is below the bottom latitude `10`. + 32:33 _geoBoundingBox([-10, 0], [10, 0]) + "###); } #[test] diff --git a/milli/src/search/facet/filter.rs b/milli/src/search/facet/filter.rs index 82073c66b..3cf11819f 100644 --- a/milli/src/search/facet/filter.rs +++ b/milli/src/search/facet/filter.rs @@ -27,6 +27,7 @@ enum FilterError<'a> { BadGeo(&'a str), BadGeoLat(f64), BadGeoLng(f64), + BadGeoBoundingBoxTopIsBelowBottom(f64, f64), Reserved(&'a str), TooDeep, } @@ -62,7 +63,8 @@ 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) built-in rule to filter on _geo field coordinates.", 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), } @@ -411,6 +413,14 @@ impl<'a> Filter<'a> { return Err(bottom_right_point[1] .as_external_error(FilterError::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], + ), + ))?; + } // Instead of writing a custom `GeoBoundingBox` filter we're simply going to re-use the range // filter to create the following filter; diff --git a/milli/src/search/mod.rs b/milli/src/search/mod.rs index df59634bb..dc48e04a8 100644 --- a/milli/src/search/mod.rs +++ b/milli/src/search/mod.rs @@ -319,7 +319,7 @@ impl fmt::Debug for Search<'_> { } } -#[derive(Default)] +#[derive(Default, Debug)] pub struct SearchResult { pub matching_words: MatchingWords, pub candidates: RoaringBitmap,