mirror of
https://github.com/meilisearch/MeiliSearch
synced 2024-11-23 13:24:27 +01:00
Merge #1052
1052: Revert "Merge #1001" r=Kerollmops a=MarinPostma This reverts commit690eab4a25
, reversing changes made to086020e543
. After arbitrage with @curquiza and @eskombro, this fix would introduce a relevancy bug that cannot be circumvented, whereas the previous bug was only a setting bug with a workaround. we need to discuss this issue further to provide a fix that meets our expectations. related to #1050 This will be merged directly in the release branche, as a hotfix Co-authored-by: mpostma <postma.marin@protonmail.com>
This commit is contained in:
commit
4ba5e22f64
@ -1,6 +1,5 @@
|
||||
## v0.16.0
|
||||
|
||||
- Fix settings bug that caused setting to be overwitten (#1001)
|
||||
- Automatically create index on document push if index doesn't exist (#914)
|
||||
- Sort displayedAttributes and facetDistribution (#946)
|
||||
|
||||
|
@ -246,7 +246,7 @@ mod test {
|
||||
#[test]
|
||||
fn test_facet_key() {
|
||||
let mut schema = Schema::new();
|
||||
let id = schema.register_field("hello").unwrap();
|
||||
let id = schema.insert_and_index("hello").unwrap();
|
||||
let facet_list = [schema.id("hello").unwrap()];
|
||||
assert_eq!(
|
||||
FacetKey::from_str("hello:12", &schema, &facet_list).unwrap(),
|
||||
@ -287,7 +287,7 @@ mod test {
|
||||
fn test_parse_facet_array() {
|
||||
use either::Either::{Left, Right};
|
||||
let mut schema = Schema::new();
|
||||
let _id = schema.register_field("hello").unwrap();
|
||||
let _id = schema.insert_and_index("hello").unwrap();
|
||||
let facet_list = [schema.id("hello").unwrap()];
|
||||
assert_eq!(
|
||||
FacetFilter::from_str("[[\"hello:12\"]]", &schema, &facet_list).unwrap(),
|
||||
|
@ -55,7 +55,6 @@ pub struct Deserializer<'a> {
|
||||
pub documents_fields: DocumentsFields,
|
||||
pub schema: &'a Schema,
|
||||
pub fields: Option<&'a HashSet<FieldId>>,
|
||||
pub displayed: bool,
|
||||
}
|
||||
|
||||
impl<'de, 'a, 'b> de::Deserializer<'de> for &'b mut Deserializer<'a> {
|
||||
@ -94,9 +93,7 @@ impl<'de, 'a, 'b> de::Deserializer<'de> for &'b mut Deserializer<'a> {
|
||||
};
|
||||
|
||||
let is_displayed = self.schema.is_displayed(attr);
|
||||
// Check if displayed fields were requested, if yes, return only displayed fields,
|
||||
// else return all fields
|
||||
if !self.displayed || (is_displayed && self.fields.map_or(true, |f| f.contains(&attr))) {
|
||||
if is_displayed && self.fields.map_or(true, |f| f.contains(&attr)) {
|
||||
if let Some(attribute_name) = self.schema.name(attr) {
|
||||
let cursor = Cursor::new(value.to_owned());
|
||||
let ioread = SerdeJsonIoRead::new(cursor);
|
||||
|
@ -243,7 +243,6 @@ impl Index {
|
||||
documents_fields: self.documents_fields,
|
||||
schema: &schema,
|
||||
fields: attributes.as_ref(),
|
||||
displayed: true,
|
||||
};
|
||||
|
||||
Ok(Option::<T>::deserialize(&mut deserializer)?)
|
||||
|
@ -197,7 +197,6 @@ pub fn apply_addition<'a, 'b>(
|
||||
documents_fields: index.documents_fields,
|
||||
schema: &schema,
|
||||
fields: None,
|
||||
displayed: false,
|
||||
};
|
||||
|
||||
let old_document = Option::<HashMap<String, Value>>::deserialize(&mut deserializer)?;
|
||||
@ -229,7 +228,7 @@ pub fn apply_addition<'a, 'b>(
|
||||
for (document_id, document) in &documents_additions {
|
||||
// For each key-value pair in the document.
|
||||
for (attribute, value) in document {
|
||||
let field_id = schema.register_field(&attribute)?;
|
||||
let field_id = schema.insert_and_index(&attribute)?;
|
||||
index_document(
|
||||
writer,
|
||||
index.documents_fields,
|
||||
|
@ -1780,7 +1780,7 @@ async fn update_documents_with_facet_distribution() {
|
||||
let settings = json!({
|
||||
"attributesForFaceting": ["genre"],
|
||||
"displayedAttributes": ["genre"],
|
||||
"searchableAttributes": ["genre", "type"]
|
||||
"searchableAttributes": ["genre"]
|
||||
});
|
||||
server.update_all_settings(settings).await;
|
||||
let update1 = json!([
|
||||
|
@ -1,5 +1,5 @@
|
||||
use assert_json_diff::assert_json_eq;
|
||||
use serde_json::{json, Value};
|
||||
use serde_json::json;
|
||||
use std::convert::Into;
|
||||
mod common;
|
||||
|
||||
@ -486,7 +486,15 @@ async fn test_displayed_attributes_field() {
|
||||
],
|
||||
"distinctAttribute": "id",
|
||||
"searchableAttributes": [
|
||||
"*"
|
||||
"id",
|
||||
"name",
|
||||
"color",
|
||||
"gender",
|
||||
"email",
|
||||
"phone",
|
||||
"address",
|
||||
"registered",
|
||||
"about"
|
||||
],
|
||||
"displayedAttributes": [
|
||||
"age",
|
||||
@ -512,72 +520,4 @@ async fn test_displayed_attributes_field() {
|
||||
let (response, _status_code) = server.get_all_settings().await;
|
||||
|
||||
assert_json_eq!(body, response, ordered: true);
|
||||
}
|
||||
|
||||
#[actix_rt::test]
|
||||
async fn push_documents_after_updating_settings_should_not_change_settings() {
|
||||
let mut server = common::Server::with_uid("test");
|
||||
|
||||
let index_body = json!({
|
||||
"uid": "test",
|
||||
"primaryKey": "id",
|
||||
});
|
||||
|
||||
let settings_body = json!({
|
||||
"rankingRules": [
|
||||
"typo",
|
||||
"words",
|
||||
"proximity",
|
||||
"attribute",
|
||||
"wordsPosition",
|
||||
"exactness",
|
||||
"desc(registered)",
|
||||
"desc(age)",
|
||||
],
|
||||
"distinctAttribute": "id",
|
||||
"searchableAttributes": [
|
||||
"id",
|
||||
"name",
|
||||
"color",
|
||||
"gender",
|
||||
"email",
|
||||
"phone",
|
||||
"address",
|
||||
"registered",
|
||||
"about"
|
||||
],
|
||||
"displayedAttributes": [
|
||||
"name",
|
||||
"gender",
|
||||
"email",
|
||||
"registered",
|
||||
"age",
|
||||
],
|
||||
"stopWords": [
|
||||
"ad",
|
||||
"in",
|
||||
"ut",
|
||||
],
|
||||
"synonyms": {
|
||||
"road": ["street", "avenue"],
|
||||
"street": ["avenue"],
|
||||
},
|
||||
"attributesForFaceting": ["name", "color"],
|
||||
});
|
||||
|
||||
let dataset = include_bytes!("assets/test_set.json");
|
||||
|
||||
let documents_body: Value = serde_json::from_slice(dataset).unwrap();
|
||||
|
||||
server.create_index(index_body).await;
|
||||
|
||||
server.update_all_settings(settings_body.clone()).await;
|
||||
|
||||
server.add_or_replace_multiple_documents(documents_body).await;
|
||||
|
||||
// === check if settings are the same ====
|
||||
|
||||
let (response, _status_code) = server.get_all_settings().await;
|
||||
|
||||
assert_json_eq!(settings_body, response, ordered: false);
|
||||
}
|
||||
}
|
@ -16,6 +16,14 @@ impl<T> OptionAll<T> {
|
||||
std::mem::replace(self, OptionAll::None)
|
||||
}
|
||||
|
||||
fn map<U, F: FnOnce(T) -> U>(self, f: F) -> OptionAll<U> {
|
||||
match self {
|
||||
OptionAll::Some(x) => OptionAll::Some(f(x)),
|
||||
OptionAll::All => OptionAll::All,
|
||||
OptionAll::None => OptionAll::None,
|
||||
}
|
||||
}
|
||||
|
||||
pub fn is_all(&self) -> bool {
|
||||
matches!(self, OptionAll::All)
|
||||
}
|
||||
@ -35,8 +43,8 @@ pub struct Schema {
|
||||
ranked: HashSet<FieldId>,
|
||||
displayed: OptionAll<HashSet<FieldId>>,
|
||||
|
||||
indexed: OptionAll<HashSet<FieldId>>,
|
||||
fields_position: HashMap<FieldId, IndexedPos>,
|
||||
indexed: OptionAll<Vec<FieldId>>,
|
||||
indexed_map: HashMap<FieldId, IndexedPos>,
|
||||
}
|
||||
|
||||
impl Schema {
|
||||
@ -60,7 +68,7 @@ impl Schema {
|
||||
ranked: HashSet::new(),
|
||||
displayed: OptionAll::All,
|
||||
indexed: OptionAll::All,
|
||||
fields_position: indexed_map,
|
||||
indexed_map,
|
||||
}
|
||||
}
|
||||
|
||||
@ -101,16 +109,14 @@ impl Schema {
|
||||
self.fields_map.insert(name)
|
||||
}
|
||||
|
||||
pub fn register_field(&mut self, name: &str) -> SResult<FieldId> {
|
||||
pub fn insert_and_index(&mut self, name: &str) -> SResult<FieldId> {
|
||||
match self.fields_map.id(name) {
|
||||
Some(id) => {
|
||||
Ok(id)
|
||||
}
|
||||
None => {
|
||||
let id = self.fields_map.insert(name)?;
|
||||
let pos = self.fields_position.len() as u16;
|
||||
self.fields_position.insert(id, pos.into());
|
||||
Ok(id)
|
||||
self.set_indexed(name)?;
|
||||
self.set_displayed(name)
|
||||
}
|
||||
}
|
||||
}
|
||||
@ -150,18 +156,18 @@ impl Schema {
|
||||
}
|
||||
}
|
||||
|
||||
pub fn indexed(&self) -> Vec<FieldId> {
|
||||
pub fn indexed(&self) -> Cow<[FieldId]> {
|
||||
match self.indexed {
|
||||
OptionAll::Some(ref v) => v.iter().cloned().collect(),
|
||||
OptionAll::Some(ref v) => Cow::Borrowed(v),
|
||||
OptionAll::All => {
|
||||
let fields = self
|
||||
.fields_position
|
||||
.keys()
|
||||
.cloned()
|
||||
.fields_map
|
||||
.iter()
|
||||
.map(|(_, &f)| f)
|
||||
.collect();
|
||||
fields
|
||||
Cow::Owned(fields)
|
||||
},
|
||||
OptionAll::None => Vec::new()
|
||||
OptionAll::None => Cow::Owned(Vec::new())
|
||||
}
|
||||
}
|
||||
|
||||
@ -195,15 +201,15 @@ impl Schema {
|
||||
pub fn set_indexed(&mut self, name: &str) -> SResult<(FieldId, IndexedPos)> {
|
||||
let id = self.fields_map.insert(name)?;
|
||||
|
||||
if let OptionAll::Some(ref mut v) = self.indexed {
|
||||
v.insert(id);
|
||||
}
|
||||
|
||||
if let Some(indexed_pos) = self.fields_position.get(&id) {
|
||||
if let Some(indexed_pos) = self.indexed_map.get(&id) {
|
||||
return Ok((id, *indexed_pos))
|
||||
};
|
||||
let pos = self.fields_position.len() as u16;
|
||||
self.fields_position.insert(id, pos.into());
|
||||
let pos = self.indexed_map.len() as u16;
|
||||
self.indexed_map.insert(id, pos.into());
|
||||
self.indexed = self.indexed.take().map(|mut v| {
|
||||
v.push(id);
|
||||
v
|
||||
});
|
||||
Ok((id, pos.into()))
|
||||
}
|
||||
|
||||
@ -217,6 +223,50 @@ impl Schema {
|
||||
}
|
||||
}
|
||||
|
||||
/// remove field from displayed attributes. If diplayed attributes is OptionAll::All,
|
||||
/// dipslayed attributes is turned into OptionAll::Some(v) where v is all displayed attributes
|
||||
/// except name.
|
||||
pub fn remove_displayed(&mut self, name: &str) {
|
||||
if let Some(id) = self.fields_map.id(name) {
|
||||
self.displayed = match self.displayed.take() {
|
||||
OptionAll::Some(mut v) => {
|
||||
v.remove(&id);
|
||||
OptionAll::Some(v)
|
||||
}
|
||||
OptionAll::All => {
|
||||
let displayed = self.fields_map
|
||||
.iter()
|
||||
.filter_map(|(key, &value)| {
|
||||
if key != name {
|
||||
Some(value)
|
||||
} else {
|
||||
None
|
||||
}
|
||||
})
|
||||
.collect::<HashSet<_>>();
|
||||
OptionAll::Some(displayed)
|
||||
}
|
||||
OptionAll::None => OptionAll::None,
|
||||
};
|
||||
}
|
||||
}
|
||||
|
||||
pub fn remove_indexed(&mut self, name: &str) {
|
||||
if let Some(id) = self.fields_map.id(name) {
|
||||
self.indexed_map.remove(&id);
|
||||
self.indexed = match self.indexed.take() {
|
||||
// valid because indexed is All and indexed() return the content of
|
||||
// indexed_map that is already updated
|
||||
OptionAll::All => OptionAll::Some(self.indexed().into_owned()),
|
||||
OptionAll::Some(mut v) => {
|
||||
v.retain(|x| *x != id);
|
||||
OptionAll::Some(v)
|
||||
}
|
||||
OptionAll::None => OptionAll::None,
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
pub fn is_ranked(&self, id: FieldId) -> bool {
|
||||
self.ranked.get(&id).is_some()
|
||||
}
|
||||
@ -230,17 +280,7 @@ impl Schema {
|
||||
}
|
||||
|
||||
pub fn is_indexed(&self, id: FieldId) -> Option<&IndexedPos> {
|
||||
match self.indexed {
|
||||
OptionAll::All => self.fields_position.get(&id),
|
||||
OptionAll::Some(ref v) => {
|
||||
if v.contains(&id) {
|
||||
self.fields_position.get(&id)
|
||||
} else {
|
||||
None
|
||||
}
|
||||
}
|
||||
OptionAll::None => None
|
||||
}
|
||||
self.indexed_map.get(&id)
|
||||
}
|
||||
|
||||
pub fn is_indexed_all(&self) -> bool {
|
||||
@ -250,7 +290,7 @@ impl Schema {
|
||||
pub fn indexed_pos_to_field_id<I: Into<IndexedPos>>(&self, pos: I) -> Option<FieldId> {
|
||||
let indexed_pos = pos.into().0;
|
||||
self
|
||||
.fields_position
|
||||
.indexed_map
|
||||
.iter()
|
||||
.find(|(_, &v)| v.0 == indexed_pos)
|
||||
.map(|(&k, _)| k)
|
||||
@ -284,8 +324,9 @@ impl Schema {
|
||||
v.clear();
|
||||
OptionAll::Some(v)
|
||||
},
|
||||
_ => OptionAll::Some(HashSet::new()),
|
||||
_ => OptionAll::Some(Vec::new()),
|
||||
};
|
||||
self.indexed_map.clear();
|
||||
for name in data {
|
||||
self.set_indexed(name.as_ref())?;
|
||||
}
|
||||
@ -294,11 +335,11 @@ impl Schema {
|
||||
|
||||
pub fn set_all_fields_as_indexed(&mut self) {
|
||||
self.indexed = OptionAll::All;
|
||||
self.fields_position.clear();
|
||||
self.indexed_map.clear();
|
||||
|
||||
for (_name, id) in self.fields_map.iter() {
|
||||
let pos = self.fields_position.len() as u16;
|
||||
self.fields_position.insert(*id, pos.into());
|
||||
let pos = self.indexed_map.len() as u16;
|
||||
self.indexed_map.insert(*id, pos.into());
|
||||
}
|
||||
}
|
||||
|
||||
|
Loading…
Reference in New Issue
Block a user