From dd57873f8e1d2d9364f86365cdacc94a42490e67 Mon Sep 17 00:00:00 2001 From: Vivek Kumar Date: Mon, 24 Jul 2023 14:50:07 +0530 Subject: [PATCH] hide fields not in the displayedAttributes list from errors --- milli/src/error.rs | 25 ++++++++++++++++-------- milli/src/index.rs | 20 +++++++++++++++++++ milli/src/search/mod.rs | 6 +++++- milli/src/search/new/mod.rs | 39 ++++++++++++++++--------------------- 4 files changed, 59 insertions(+), 31 deletions(-) diff --git a/milli/src/error.rs b/milli/src/error.rs index 1e8e767d3..e9e1fddd3 100644 --- a/milli/src/error.rs +++ b/milli/src/error.rs @@ -122,22 +122,28 @@ only composed of alphanumeric characters (a-z A-Z 0-9), hyphens (-) and undersco .field, match .valid_fields.is_empty() { true => "This index does not have configured sortable attributes.".to_string(), - false => format!("Available sortable attributes are: `{}`.", - valid_fields.iter().map(AsRef::as_ref).collect::>().join(", ") + false => format!("Available sortable attributes are: `{}{}`.", + valid_fields.iter().map(AsRef::as_ref).collect::>().join(", "), + .hidden_fields.then_some(", <..hidden-attributes>").unwrap_or(""), ), } )] - InvalidSortableAttribute { field: String, valid_fields: BTreeSet }, + InvalidSortableAttribute { field: String, valid_fields: BTreeSet, hidden_fields: bool }, #[error("Attribute `{}` is not facet-searchable. {}", .field, match .valid_fields.is_empty() { true => "This index does not have configured facet-searchable attributes. To make it facet-searchable add it to the `filterableAttributes` index settings.".to_string(), - false => format!("Available facet-searchable attributes are: `{}`. To make it facet-searchable add it to the `filterableAttributes` index settings.", - valid_fields.iter().map(AsRef::as_ref).collect::>().join(", ") + false => format!("Available facet-searchable attributes are: `{}{}`. To make it facet-searchable add it to the `filterableAttributes` index settings.", + valid_fields.iter().map(AsRef::as_ref).collect::>().join(", "), + .hidden_fields.then_some(", <..hidden-attributes>").unwrap_or(""), ), } )] - InvalidFacetSearchFacetName { field: String, valid_fields: BTreeSet }, + InvalidFacetSearchFacetName { + field: String, + valid_fields: BTreeSet, + hidden_fields: bool, + }, #[error("Attribute `{}` is not searchable. Available searchable attributes are: `{}{}`.", .field, .valid_fields.iter().map(AsRef::as_ref).collect::>().join(", "), @@ -340,8 +346,11 @@ fn conditionally_lookup_for_error_message() { ]; for (list, suffix) in messages { - let err = - UserError::InvalidSortableAttribute { field: "name".to_string(), valid_fields: list }; + let err = UserError::InvalidSortableAttribute { + field: "name".to_string(), + valid_fields: list, + hidden_fields: false, + }; assert_eq!(err.to_string(), format!("{} {}", prefix, suffix)); } diff --git a/milli/src/index.rs b/milli/src/index.rs index 0ddfcda94..6f499dee9 100644 --- a/milli/src/index.rs +++ b/milli/src/index.rs @@ -651,6 +651,26 @@ impl Index { } } + /* remove hidden fields */ + pub fn remove_hidden_fields( + &self, + rtxn: &RoTxn, + fields: impl IntoIterator>, + ) -> Result<(BTreeSet, bool)> { + let mut valid_fields = + fields.into_iter().map(|f| f.as_ref().to_string()).collect::>(); + + let fields_len = valid_fields.len(); + + if let Some(dn) = self.displayed_fields(rtxn)? { + let displayable_names = dn.iter().map(|s| s.to_string()).collect(); + valid_fields = &valid_fields & &displayable_names; + } + + let hidden_fields = fields_len > valid_fields.len(); + Ok((valid_fields, hidden_fields)) + } + /* searchable fields */ /// Write the user defined searchable fields and generate the real searchable fields from the specified fields ids map. diff --git a/milli/src/search/mod.rs b/milli/src/search/mod.rs index e1ee37d79..2c78acfdf 100644 --- a/milli/src/search/mod.rs +++ b/milli/src/search/mod.rs @@ -280,9 +280,13 @@ impl<'a> SearchForFacetValues<'a> { let filterable_fields = index.filterable_fields(rtxn)?; if !filterable_fields.contains(&self.facet) { + let (valid_fields, hidden_fields) = + index.remove_hidden_fields(rtxn, filterable_fields)?; + return Err(UserError::InvalidFacetSearchFacetName { field: self.facet.clone(), - valid_fields: filterable_fields.into_iter().collect(), + valid_fields, + hidden_fields, } .into()); } diff --git a/milli/src/search/new/mod.rs b/milli/src/search/new/mod.rs index ad15d8e91..d638061df 100644 --- a/milli/src/search/new/mod.rs +++ b/milli/src/search/new/mod.rs @@ -20,7 +20,7 @@ mod sort; #[cfg(test)] mod tests; -use std::collections::{BTreeSet, HashSet}; +use std::collections::HashSet; use bucket_sort::{bucket_sort, BucketSortOutput}; use charabia::TokenizerBuilder; @@ -108,24 +108,11 @@ impl<'ctx> SearchContext<'ctx> { (None, None) => continue, // The field is not searchable => User error (_fid, Some(false)) => { - let mut valid_fields: BTreeSet<_> = - fids_map.names().map(String::from).collect(); + let (valid_fields, hidden_fields) = match searchable_names { + Some(sn) => self.index.remove_hidden_fields(self.txn, sn)?, + None => self.index.remove_hidden_fields(self.txn, fids_map.names())?, + }; - // Filter by the searchable names - if let Some(sn) = searchable_names { - let searchable_names = sn.iter().map(|s| s.to_string()).collect(); - valid_fields = &valid_fields & &searchable_names; - } - - let searchable_count = valid_fields.len(); - - // Remove hidden fields - if let Some(dn) = self.index.displayed_fields(self.txn)? { - let displayable_names = dn.iter().map(|s| s.to_string()).collect(); - valid_fields = &valid_fields & &displayable_names; - } - - let hidden_fields = searchable_count > valid_fields.len(); let field = field_name.to_string(); return Err(UserError::InvalidSearchableAttribute { field, @@ -590,16 +577,24 @@ fn check_sort_criteria(ctx: &SearchContext, sort_criteria: Option<&Vec> for asc_desc in sort_criteria { match asc_desc.member() { Member::Field(ref field) if !crate::is_faceted(field, &sortable_fields) => { + let (valid_fields, hidden_fields) = + ctx.index.remove_hidden_fields(ctx.txn, sortable_fields)?; + return Err(UserError::InvalidSortableAttribute { field: field.to_string(), - valid_fields: sortable_fields.into_iter().collect(), - })? + valid_fields, + hidden_fields, + })?; } Member::Geo(_) if !sortable_fields.contains("_geo") => { + let (valid_fields, hidden_fields) = + ctx.index.remove_hidden_fields(ctx.txn, sortable_fields)?; + return Err(UserError::InvalidSortableAttribute { field: "_geo".to_string(), - valid_fields: sortable_fields.into_iter().collect(), - })? + valid_fields, + hidden_fields, + })?; } _ => (), }