From 93f30e65a91beb80fe19736fa655c1577109795b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Renault?= Date: Wed, 26 Apr 2023 17:09:59 +0200 Subject: [PATCH] Return the correct response JSON object from the facet-search route --- .../src/routes/indexes/facet_search.rs | 46 +------------------ meilisearch/src/search.rs | 26 ++++++++--- milli/src/heed_codec/fst_set_codec.rs | 2 +- milli/src/lib.rs | 2 +- milli/src/search/mod.rs | 10 ++-- 5 files changed, 28 insertions(+), 58 deletions(-) diff --git a/meilisearch/src/routes/indexes/facet_search.rs b/meilisearch/src/routes/indexes/facet_search.rs index 947454d12..3f97e2858 100644 --- a/meilisearch/src/routes/indexes/facet_search.rs +++ b/meilisearch/src/routes/indexes/facet_search.rs @@ -2,16 +2,13 @@ use std::collections::{BTreeSet, HashSet}; use actix_web::web::Data; use actix_web::{web, HttpRequest, HttpResponse}; -use deserr::actix_web::{AwebJson, AwebQueryParameter}; +use deserr::actix_web::AwebJson; use index_scheduler::IndexScheduler; use log::debug; -use meilisearch_types::deserr::query_params::Param; -use meilisearch_types::deserr::{DeserrJsonError, DeserrQueryParamError}; +use meilisearch_types::deserr::DeserrJsonError; use meilisearch_types::error::deserr_codes::*; use meilisearch_types::error::ResponseError; use meilisearch_types::index_uid::IndexUid; -use meilisearch_types::milli::facet; -use meilisearch_types::serde_cs::vec::CS; use serde_json::Value; use crate::analytics::{Analytics, FacetSearchAggregator}; @@ -27,45 +24,6 @@ pub fn configure(cfg: &mut web::ServiceConfig) { cfg.service(web::resource("").route(web::post().to(search))); } -// #[derive(Debug, deserr::Deserr)] -// #[deserr(error = DeserrQueryParamError, rename_all = camelCase, deny_unknown_fields)] -// pub struct FacetSearchQuery { -// #[deserr(default, error = DeserrQueryParamError)] -// facetQuery: Option, -// #[deserr(default = Param(DEFAULT_SEARCH_OFFSET()), error = DeserrQueryParamError)] -// offset: Param, -// #[deserr(default = Param(DEFAULT_SEARCH_LIMIT()), error = DeserrQueryParamError)] -// limit: Param, -// #[deserr(default, error = DeserrQueryParamError)] -// page: Option>, -// #[deserr(default, error = DeserrQueryParamError)] -// hits_per_page: Option>, -// #[deserr(default, error = DeserrQueryParamError)] -// attributes_to_retrieve: Option>, -// #[deserr(default, error = DeserrQueryParamError)] -// attributes_to_crop: Option>, -// #[deserr(default = Param(DEFAULT_CROP_LENGTH()), error = DeserrQueryParamError)] -// crop_length: Param, -// #[deserr(default, error = DeserrQueryParamError)] -// attributes_to_highlight: Option>, -// #[deserr(default, error = DeserrQueryParamError)] -// filter: Option, -// #[deserr(default, error = DeserrQueryParamError)] -// sort: Option, -// #[deserr(default, error = DeserrQueryParamError)] -// show_matches_position: Param, -// #[deserr(default, error = DeserrQueryParamError)] -// facets: Option>, -// #[deserr( default = DEFAULT_HIGHLIGHT_PRE_TAG(), error = DeserrQueryParamError)] -// highlight_pre_tag: String, -// #[deserr( default = DEFAULT_HIGHLIGHT_POST_TAG(), error = DeserrQueryParamError)] -// highlight_post_tag: String, -// #[deserr(default = DEFAULT_CROP_MARKER(), error = DeserrQueryParamError)] -// crop_marker: String, -// #[deserr(default, error = DeserrQueryParamError)] -// matching_strategy: MatchingStrategy, -// } - // TODO improve the error messages #[derive(Debug, Clone, Default, PartialEq, Eq, deserr::Deserr)] #[deserr(error = DeserrJsonError, rename_all = camelCase, deny_unknown_fields)] diff --git a/meilisearch/src/search.rs b/meilisearch/src/search.rs index cd6a0bc54..adf9a01e6 100644 --- a/meilisearch/src/search.rs +++ b/meilisearch/src/search.rs @@ -3,7 +3,6 @@ use std::collections::{BTreeMap, BTreeSet, HashSet}; use std::str::FromStr; use std::time::Instant; -use actix_http::header::q; use deserr::Deserr; use either::Either; use index_scheduler::RoFeatures; @@ -15,7 +14,7 @@ use meilisearch_types::heed::RoTxn; use meilisearch_types::index_uid::IndexUid; use meilisearch_types::milli::score_details::{ScoreDetails, ScoringStrategy}; use meilisearch_types::milli::{ - dot_product_similarity, facet, FacetSearchResult, InternalError, SearchForFacetValue, + dot_product_similarity, FacetValueHit, InternalError, SearchForFacetValue, }; use meilisearch_types::settings::DEFAULT_PAGINATION_MAX_TOTAL_HITS; use meilisearch_types::{milli, Document}; @@ -30,7 +29,6 @@ use serde::Serialize; use serde_json::{json, Value}; use crate::error::MeilisearchHttpError; -use crate::routes::indexes::facet_search::FacetSearchQuery; type MatchesPosition = BTreeMap>; @@ -283,6 +281,14 @@ pub struct FacetStats { pub max: f64, } +#[derive(Serialize, Debug, Clone, PartialEq)] +#[serde(rename_all = "camelCase")] +pub struct FacetSearchResult { + pub hits: Vec, + pub query: Option, + pub processing_time_ms: u128, +} + /// Incorporate search rules in search query pub fn add_search_rules(query: &mut SearchQuery, rules: IndexSearchRules) { query.filter = match (query.filter.take(), rules.filter) { @@ -577,19 +583,25 @@ pub fn perform_search( pub fn perform_facet_search( index: &Index, search_query: SearchQuery, - mut facet_query: Option, + facet_query: Option, facet_name: String, -) -> Result, MeilisearchHttpError> { +) -> Result { let before_search = Instant::now(); let rtxn = index.read_txn()?; let (search, _, _, _) = prepare_search(index, &rtxn, &search_query)?; let mut facet_search = SearchForFacetValue::new(facet_name, search); - if let Some(facet_query) = facet_query.take() { + if let Some(facet_query) = &facet_query { facet_search.query(facet_query); } - facet_search.execute().map_err(Into::into) + let hits = facet_search.execute()?; + + Ok(FacetSearchResult { + hits, + query: facet_query, + processing_time_ms: before_search.elapsed().as_millis(), + }) } fn insert_geo_distance(sorts: &[String], document: &mut Document) { diff --git a/milli/src/heed_codec/fst_set_codec.rs b/milli/src/heed_codec/fst_set_codec.rs index a8d35ebda..551f04678 100644 --- a/milli/src/heed_codec/fst_set_codec.rs +++ b/milli/src/heed_codec/fst_set_codec.rs @@ -1,6 +1,6 @@ -use fst::Set; use std::borrow::Cow; +use fst::Set; use heed::{BytesDecode, BytesEncode}; /// A codec for values of type `Set<&[u8]>`. diff --git a/milli/src/lib.rs b/milli/src/lib.rs index 6f14977ec..84ca0ce97 100644 --- a/milli/src/lib.rs +++ b/milli/src/lib.rs @@ -57,7 +57,7 @@ pub use self::heed_codec::{ }; pub use self::index::Index; pub use self::search::{ - FacetDistribution, FacetSearchResult, Filter, FormatOptions, MatchBounds, MatcherBuilder, + FacetDistribution, FacetValueHit, Filter, FormatOptions, MatchBounds, MatcherBuilder, MatchingWords, Search, SearchForFacetValue, SearchResult, TermsMatchingStrategy, DEFAULT_VALUES_PER_FACET, }; diff --git a/milli/src/search/mod.rs b/milli/src/search/mod.rs index bc88a0ff8..0f71fb172 100644 --- a/milli/src/search/mod.rs +++ b/milli/src/search/mod.rs @@ -257,7 +257,7 @@ impl<'a> SearchForFacetValue<'a> { self } - pub fn execute(&self) -> Result> { + pub fn execute(&self) -> Result> { let index = self.search_query.index; let rtxn = self.search_query.rtxn; @@ -304,7 +304,7 @@ impl<'a> SearchForFacetValue<'a> { }; let count = search_candidates.intersection_len(&docids); if count != 0 { - result.push(FacetSearchResult { value: value.to_string(), count }); + result.push(FacetValueHit { value: value.to_string(), count }); length += 1; } if length >= MAX_NUMBER_OF_FACETS { @@ -327,7 +327,7 @@ impl<'a> SearchForFacetValue<'a> { }; let count = search_candidates.intersection_len(&docids); if count != 0 { - result.push(FacetSearchResult { value: value.to_string(), count }); + result.push(FacetValueHit { value: value.to_string(), count }); length += 1; } if length >= MAX_NUMBER_OF_FACETS { @@ -341,8 +341,8 @@ impl<'a> SearchForFacetValue<'a> { } } -#[derive(Debug, serde::Serialize)] -pub struct FacetSearchResult { +#[derive(Debug, Clone, serde::Serialize, PartialEq)] +pub struct FacetValueHit { /// The original facet value pub value: String, /// The number of documents associated to this facet