diff --git a/meilisearch-core/src/settings.rs b/meilisearch-core/src/settings.rs index 157b88251..456b9fead 100644 --- a/meilisearch-core/src/settings.rs +++ b/meilisearch-core/src/settings.rs @@ -29,8 +29,6 @@ pub struct Settings { #[serde(default, deserialize_with = "deserialize_some")] pub synonyms: Option>>>, #[serde(default, deserialize_with = "deserialize_some")] - pub accept_new_fields: Option>, - #[serde(default, deserialize_with = "deserialize_some")] pub attributes_for_faceting: Option>>, } @@ -60,7 +58,6 @@ impl Settings { displayed_attributes: settings.displayed_attributes.into(), stop_words: settings.stop_words.into(), synonyms: settings.synonyms.into(), - accept_new_fields: settings.accept_new_fields.into(), attributes_for_faceting: settings.attributes_for_faceting.into(), }) } @@ -167,7 +164,6 @@ pub struct SettingsUpdate { pub displayed_attributes: UpdateState>, pub stop_words: UpdateState>, pub synonyms: UpdateState>>, - pub accept_new_fields: UpdateState, pub attributes_for_faceting: UpdateState>, } @@ -181,7 +177,6 @@ impl Default for SettingsUpdate { displayed_attributes: UpdateState::Nothing, stop_words: UpdateState::Nothing, synonyms: UpdateState::Nothing, - accept_new_fields: UpdateState::Nothing, attributes_for_faceting: UpdateState::Nothing, } } diff --git a/meilisearch-core/src/update/settings_update.rs b/meilisearch-core/src/update/settings_update.rs index 9aecb23f7..94e337265 100644 --- a/meilisearch-core/src/update/settings_update.rs +++ b/meilisearch-core/src/update/settings_update.rs @@ -68,19 +68,13 @@ pub fn apply_settings_update( UpdateState::Nothing => (), } - match settings.accept_new_fields { - UpdateState::Update(v) => { - schema.set_accept_new_fields(v); - }, - UpdateState::Clear => { - schema.set_accept_new_fields(true); - }, - UpdateState::Nothing => (), - } - match settings.searchable_attributes.clone() { UpdateState::Update(v) => { - schema.update_indexed(v)?; + if v.iter().any(|e| e == "*") || v.is_empty() { + schema.set_all_fields_as_indexed(); + } else { + schema.update_indexed(v)?; + } must_reindex = true; }, UpdateState::Clear => { @@ -90,7 +84,13 @@ pub fn apply_settings_update( UpdateState::Nothing => (), } match settings.displayed_attributes.clone() { - UpdateState::Update(v) => schema.update_displayed(v)?, + UpdateState::Update(v) => { + if v.contains("*") || v.is_empty() { + schema.set_all_fields_as_displayed(); + } else { + schema.update_displayed(v)? + } + }, UpdateState::Clear => { schema.set_all_fields_as_displayed(); }, diff --git a/meilisearch-http/src/routes/setting.rs b/meilisearch-http/src/routes/setting.rs index 3d22af194..8de584c67 100644 --- a/meilisearch-http/src/routes/setting.rs +++ b/meilisearch-http/src/routes/setting.rs @@ -1,6 +1,7 @@ use actix_web::{web, HttpResponse}; use actix_web_macros::{delete, get, post}; use meilisearch_core::settings::{Settings, SettingsUpdate, UpdateState, DEFAULT_RANKING_RULES}; +use meilisearch_schema::Schema; use std::collections::{BTreeMap, BTreeSet, HashSet}; use crate::error::{Error, ResponseError}; @@ -24,8 +25,6 @@ pub fn services(cfg: &mut web::ServiceConfig) { .service(get_displayed) .service(update_displayed) .service(delete_displayed) - .service(get_accept_new_fields) - .service(update_accept_new_fields) .service(get_attributes_for_faceting) .service(delete_attributes_for_faceting) .service(update_attributes_for_faceting); @@ -108,23 +107,8 @@ async fn get_all( _ => vec![], }; - println!("{:?}", attributes_for_faceting); - - let searchable_attributes = schema.as_ref().map(|s| { - s.indexed_name() - .iter() - .map(|s| s.to_string()) - .collect::>() - }); - - let displayed_attributes = schema.as_ref().map(|s| { - s.displayed_name() - .iter() - .map(|s| s.to_string()) - .collect::>() - }); - - let accept_new_fields = schema.map(|s| s.accept_new_fields()); + let searchable_attributes = schema.as_ref().map(get_indexed_attributes); + let displayed_attributes = schema.as_ref().map(get_displayed_attributes); let settings = Settings { ranking_rules: Some(Some(ranking_rules)), @@ -133,7 +117,6 @@ async fn get_all( displayed_attributes: Some(displayed_attributes), stop_words: Some(Some(stop_words)), synonyms: Some(Some(synonyms)), - accept_new_fields: Some(accept_new_fields), attributes_for_faceting: Some(Some(attributes_for_faceting)), }; @@ -158,7 +141,6 @@ async fn delete_all( displayed_attributes: UpdateState::Clear, stop_words: UpdateState::Clear, synonyms: UpdateState::Clear, - accept_new_fields: UpdateState::Clear, attributes_for_faceting: UpdateState::Clear, }; @@ -326,7 +308,7 @@ async fn get_searchable( let reader = data.db.main_read_txn()?; let schema = index.main.schema(&reader)?; let searchable_attributes: Option> = - schema.map(|s| s.indexed_name().iter().map(|i| i.to_string()).collect()); + schema.as_ref().map(get_indexed_attributes); Ok(HttpResponse::Ok().json(searchable_attributes)) } @@ -396,8 +378,7 @@ async fn get_displayed( let schema = index.main.schema(&reader)?; - let displayed_attributes: Option> = - schema.map(|s| s.displayed_name().iter().map(|i| i.to_string()).collect()); + let displayed_attributes = schema.as_ref().map(get_displayed_attributes); Ok(HttpResponse::Ok().json(displayed_attributes)) } @@ -450,52 +431,6 @@ async fn delete_displayed( Ok(HttpResponse::Accepted().json(IndexUpdateResponse::with_id(update_id))) } -#[get( - "/indexes/{index_uid}/settings/accept-new-fields", - wrap = "Authentication::Private" -)] -async fn get_accept_new_fields( - data: web::Data, - path: web::Path, -) -> Result { - let index = data - .db - .open_index(&path.index_uid) - .ok_or(Error::index_not_found(&path.index_uid))?; - let reader = data.db.main_read_txn()?; - - let schema = index.main.schema(&reader)?; - - let accept_new_fields = schema.map(|s| s.accept_new_fields()); - - Ok(HttpResponse::Ok().json(accept_new_fields)) -} - -#[post( - "/indexes/{index_uid}/settings/accept-new-fields", - wrap = "Authentication::Private" -)] -async fn update_accept_new_fields( - data: web::Data, - path: web::Path, - body: web::Json>, -) -> Result { - let index = data - .db - .open_index(&path.index_uid) - .ok_or(Error::index_not_found(&path.index_uid))?; - - let settings = Settings { - accept_new_fields: Some(body.into_inner()), - ..Settings::default() - }; - - let settings = settings.to_update().map_err(Error::bad_request)?; - let update_id = data.db.update_write(|w| index.settings_update(w, settings))?; - - Ok(HttpResponse::Accepted().json(IndexUpdateResponse::with_id(update_id))) -} - #[get( "/indexes/{index_uid}/settings/attributes-for-faceting", wrap = "Authentication::Private" @@ -577,3 +512,25 @@ async fn delete_attributes_for_faceting( Ok(HttpResponse::Accepted().json(IndexUpdateResponse::with_id(update_id))) } + +fn get_indexed_attributes(schema: &Schema) -> Vec { + if schema.is_indexed_all() { + ["*"].iter().map(|s| s.to_string()).collect() + } else { + schema.indexed_name() + .iter() + .map(|s| s.to_string()) + .collect() + } +} + +fn get_displayed_attributes(schema: &Schema) -> HashSet { + if schema.is_displayed_all() { + ["*"].iter().map(|s| s.to_string()).collect() + } else { + schema.displayed_name() + .iter() + .map(|s| s.to_string()) + .collect() + } +} diff --git a/meilisearch-http/tests/common.rs b/meilisearch-http/tests/common.rs index 1906ab44f..a8633bd0f 100644 --- a/meilisearch-http/tests/common.rs +++ b/meilisearch-http/tests/common.rs @@ -112,7 +112,6 @@ impl Server { "longitude", "tags", ], - "acceptNewFields": false, }); server.update_all_settings(body).await; @@ -426,16 +425,6 @@ impl Server { self.delete_request_async(&url).await } - pub async fn get_accept_new_fields(&mut self) -> (Value, StatusCode) { - let url = format!("/indexes/{}/settings/accept-new-fields", self.uid); - self.get_request(&url).await - } - - pub async fn update_accept_new_fields(&mut self, body: Value) { - let url = format!("/indexes/{}/settings/accept-new-fields", self.uid); - self.post_request_async(&url, body).await; - } - pub async fn get_synonyms(&mut self) -> (Value, StatusCode) { let url = format!("/indexes/{}/settings/synonyms", self.uid); self.get_request(&url).await diff --git a/meilisearch-http/tests/search.rs b/meilisearch-http/tests/search.rs index e2e00e219..189e6c453 100644 --- a/meilisearch-http/tests/search.rs +++ b/meilisearch-http/tests/search.rs @@ -1195,15 +1195,21 @@ async fn search_with_differents_attributes_8() { #[actix_rt::test] async fn test_faceted_search_valid() { - let mut server = common::Server::test_server().await; + // set facetting attributes before adding documents + let mut server = common::Server::with_uid("test"); + server.create_index(json!({ "uid": "test" })).await; - // simple tests on attributes with string value let body = json!({ "attributesForFaceting": ["color"] }); - server.update_all_settings(body).await; + let dataset = include_bytes!("assets/test_set.json"); + let body: Value = serde_json::from_slice(dataset).unwrap(); + server.add_or_update_multiple_documents(body).await; + + // simple tests on attributes with string value + let query = json!({ "q": "a", "facetFilters": ["color:green"] @@ -1357,8 +1363,8 @@ async fn test_faceted_search_invalid() { test_post_get_search!(server, query, |response, status_code| { assert_eq!(status_code, 400); - assert_eq!(response["errorCode"], "invalid_facet"); - }); + assert_eq!(response["errorCode"], "invalid_facet"); + }); let body = json!({ "attributesForFaceting": ["color", "tags"] @@ -1373,7 +1379,7 @@ async fn test_faceted_search_invalid() { test_post_get_search!(server, query, |response, status_code| { assert_eq!(status_code, 400); - assert_eq!(response["errorCode"], "invalid_facet"); + assert_eq!(response["errorCode"], "invalid_facet"); }); // [[]] let query = json!({ @@ -1383,7 +1389,7 @@ async fn test_faceted_search_invalid() { test_post_get_search!(server, query, |response, status_code| { assert_eq!(status_code, 400); - assert_eq!(response["errorCode"], "invalid_facet"); + assert_eq!(response["errorCode"], "invalid_facet"); }); // ["color:green", []] @@ -1394,7 +1400,7 @@ async fn test_faceted_search_invalid() { test_post_get_search!(server, query, |response, status_code| { assert_eq!(status_code, 400); - assert_eq!(response["errorCode"], "invalid_facet"); + assert_eq!(response["errorCode"], "invalid_facet"); }); // too much depth @@ -1406,7 +1412,7 @@ async fn test_faceted_search_invalid() { test_post_get_search!(server, query, |response, status_code| { assert_eq!(status_code, 400); - assert_eq!(response["errorCode"], "invalid_facet"); + assert_eq!(response["errorCode"], "invalid_facet"); }); // [["color:green", ["color:blue"]]] @@ -1417,7 +1423,7 @@ async fn test_faceted_search_invalid() { test_post_get_search!(server, query, |response, status_code| { assert_eq!(status_code, 400); - assert_eq!(response["errorCode"], "invalid_facet"); + assert_eq!(response["errorCode"], "invalid_facet"); }); // "color:green" @@ -1428,7 +1434,7 @@ async fn test_faceted_search_invalid() { test_post_get_search!(server, query, |response, status_code| { assert_eq!(status_code, 400); - assert_eq!(response["errorCode"], "invalid_facet"); + assert_eq!(response["errorCode"], "invalid_facet"); }); } diff --git a/meilisearch-http/tests/search_settings.rs b/meilisearch-http/tests/search_settings.rs index 1c23d2942..ae70fce66 100644 --- a/meilisearch-http/tests/search_settings.rs +++ b/meilisearch-http/tests/search_settings.rs @@ -41,7 +41,6 @@ async fn search_with_settings_basic() { ], "stopWords": null, "synonyms": null, - "acceptNewFields": false, }); server.update_all_settings(config).await; @@ -122,7 +121,6 @@ async fn search_with_settings_stop_words() { ], "stopWords": ["ea"], "synonyms": null, - "acceptNewFields": false, }); server.update_all_settings(config).await; @@ -206,7 +204,6 @@ async fn search_with_settings_synonyms() { "exercitation" ] }, - "acceptNewFields": false, }); server.update_all_settings(config).await; @@ -286,7 +283,6 @@ async fn search_with_settings_ranking_rules() { ], "stopWords": null, "synonyms": null, - "acceptNewFields": false, }); server.update_all_settings(config).await; @@ -369,7 +365,6 @@ async fn search_with_settings_searchable_attributes() { "exercitation" ] }, - "acceptNewFields": false, }); server.update_all_settings(config).await; @@ -435,7 +430,6 @@ async fn search_with_settings_displayed_attributes() { ], "stopWords": null, "synonyms": null, - "acceptNewFields": false, }); server.update_all_settings(config).await; @@ -502,7 +496,6 @@ async fn search_with_settings_searchable_attributes_2() { ], "stopWords": null, "synonyms": null, - "acceptNewFields": false, }); server.update_all_settings(config).await; diff --git a/meilisearch-http/tests/settings.rs b/meilisearch-http/tests/settings.rs index 7ddc89905..92114faa3 100644 --- a/meilisearch-http/tests/settings.rs +++ b/meilisearch-http/tests/settings.rs @@ -48,7 +48,6 @@ async fn write_all_and_delete() { "street": ["avenue"], }, "attributesForFaceting": ["name"], - "acceptNewFields": false, }); server.update_all_settings(body.clone()).await; @@ -77,46 +76,11 @@ async fn write_all_and_delete() { "exactness" ], "distinctAttribute": null, - "searchableAttributes": [ - "address", - "id", - "longitude", - "phone", - "age", - "gender", - "latitude", - "email", - "about", - "registered", - "picture", - "isActive", - "tags", - "name", - "color", - "balance" - ], - "displayedAttributes": [ - "address", - "id", - "longitude", - "phone", - "age", - "gender", - "latitude", - "email", - "about", - "registered", - "picture", - "isActive", - "tags", - "name", - "color", - "balance" - ], + "searchableAttributes": ["*"], + "displayedAttributes": ["*"], "stopWords": [], "synonyms": {}, "attributesForFaceting": [], - "acceptNewFields": true, }); assert_json_eq!(expect, response, ordered: false); @@ -168,7 +132,6 @@ async fn write_all_and_update() { "street": ["avenue"], }, "attributesForFaceting": ["name"], - "acceptNewFields": false, }); server.update_all_settings(body.clone()).await; @@ -210,7 +173,6 @@ async fn write_all_and_update() { "street": ["avenue"], }, "attributesForFaceting": ["title"], - "acceptNewFields": false, }); server.update_all_settings(body).await; @@ -248,7 +210,6 @@ async fn write_all_and_update() { "street": ["avenue"], }, "attributesForFaceting": ["title"], - "acceptNewFields": false, }); assert_json_eq!(expected, response, ordered: false); @@ -274,12 +235,11 @@ async fn test_default_settings() { "exactness" ], "distinctAttribute": null, - "searchableAttributes": [], - "displayedAttributes": [], + "searchableAttributes": ["*"], + "displayedAttributes": ["*"], "stopWords": [], "synonyms": {}, "attributesForFaceting": [], - "acceptNewFields": true, }); let (response, _status_code) = server.get_all_settings().await; @@ -308,16 +268,11 @@ async fn test_default_settings_2() { "exactness" ], "distinctAttribute": null, - "searchableAttributes": [ - "id" - ], - "displayedAttributes": [ - "id" - ], + "searchableAttributes": ["*"], + "displayedAttributes": ["*"], "stopWords": [], "synonyms": {}, "attributesForFaceting": [], - "acceptNewFields": true, }); let (response, _status_code) = server.get_all_settings().await; @@ -381,7 +336,6 @@ async fn write_setting_and_update_partial() { "road": ["street", "avenue"], "street": ["avenue"], }, - "acceptNewFields": false, }); server.update_all_settings(body.clone()).await; @@ -427,7 +381,6 @@ async fn write_setting_and_update_partial() { "street": ["avenue"], }, "attributesForFaceting": [], - "acceptNewFields": false, }); let (response, _status_code) = server.get_all_settings().await; @@ -469,17 +422,49 @@ async fn setting_ranking_rules_dont_mess_with_other_settings() { } #[actix_rt::test] -async fn distinct_attribute_recorded_as_known_field() { - let mut server = common::Server::test_server().await; - let body = json!({ - "distinctAttribute": "foobar", - "acceptNewFields": true - }); - server.update_all_settings(body).await; - let document = json!([{"id": 9348127, "foobar": "hello", "foo": "bar"}]); - server.add_or_update_multiple_documents(document).await; - // foobar should not be added to the searchable attributes because it is already known, but "foo" should - let (response, _) = server.get_all_settings().await; - assert!(response["searchableAttributes"].as_array().unwrap().iter().any(|v| v.as_str().unwrap() == "foo")); - assert!(!response["searchableAttributes"].as_array().unwrap().iter().any(|v| v.as_str().unwrap() == "foobar")); +async fn displayed_and_searchable_attributes_reset_to_wildcard() { + let mut server = common::Server::test_server().await; + server.update_all_settings(json!({ "searchableAttributes": ["color"], "displayedAttributes": ["color"] })).await; + let (response, _) = server.get_all_settings().await; + + assert_eq!(response["searchableAttributes"].as_array().unwrap()[0], "color"); + assert_eq!(response["displayedAttributes"].as_array().unwrap()[0], "color"); + + server.delete_searchable_attributes().await; + server.delete_displayed_attributes().await; + + let (response, _) = server.get_all_settings().await; + + assert_eq!(response["searchableAttributes"].as_array().unwrap().len(), 1); + assert_eq!(response["displayedAttributes"].as_array().unwrap().len(), 1); + assert_eq!(response["searchableAttributes"].as_array().unwrap()[0], "*"); + assert_eq!(response["displayedAttributes"].as_array().unwrap()[0], "*"); + + let mut server = common::Server::test_server().await; + server.update_all_settings(json!({ "searchableAttributes": ["color"], "displayedAttributes": ["color"] })).await; + let (response, _) = server.get_all_settings().await; + assert_eq!(response["searchableAttributes"].as_array().unwrap()[0], "color"); + assert_eq!(response["displayedAttributes"].as_array().unwrap()[0], "color"); + + server.update_all_settings(json!({ "searchableAttributes": [], "displayedAttributes": [] })).await; + + let (response, _) = server.get_all_settings().await; + + assert_eq!(response["searchableAttributes"].as_array().unwrap().len(), 1); + assert_eq!(response["displayedAttributes"].as_array().unwrap().len(), 1); + assert_eq!(response["searchableAttributes"].as_array().unwrap()[0], "*"); + assert_eq!(response["displayedAttributes"].as_array().unwrap()[0], "*"); +} + +#[actix_rt::test] +async fn settings_that_contains_wildcard_is_wildcard() { + let mut server = common::Server::test_server().await; + server.update_all_settings(json!({ "searchableAttributes": ["color", "*"], "displayedAttributes": ["color", "*"] })).await; + + let (response, _) = server.get_all_settings().await; + + assert_eq!(response["searchableAttributes"].as_array().unwrap().len(), 1); + assert_eq!(response["displayedAttributes"].as_array().unwrap().len(), 1); + assert_eq!(response["searchableAttributes"].as_array().unwrap()[0], "*"); + assert_eq!(response["displayedAttributes"].as_array().unwrap()[0], "*"); } diff --git a/meilisearch-http/tests/settings_accept_new_fields.rs b/meilisearch-http/tests/settings_accept_new_fields.rs deleted file mode 100644 index 6cc14deed..000000000 --- a/meilisearch-http/tests/settings_accept_new_fields.rs +++ /dev/null @@ -1,349 +0,0 @@ -use assert_json_diff::assert_json_eq; -use serde_json::json; - -mod common; - -#[actix_rt::test] -async fn index_new_fields_default() { - let mut server = common::Server::with_uid("movies"); - let body = json!({ - "uid": "movies", - "primaryKey": "id", - }); - server.create_index(body).await; - - // 1 - Add a document - - let body = json!([{ - "id": 1, - "title": "I'm a legend", - }]); - - server.add_or_replace_multiple_documents(body).await; - - // 2 - Get the complete document - - let expected = json!({ - "id": 1, - "title": "I'm a legend", - }); - - let (response, status_code) = server.get_document(1).await; - assert_eq!(status_code, 200); - assert_json_eq!(response, expected); - - // 3 - Add a document with more fields - - let body = json!([{ - "id": 2, - "title": "I'm not a legend", - "description": "A bad copy of the original movie I'm a lengend" - }]); - - server.add_or_replace_multiple_documents(body).await; - - // 4 - Get the complete document - - let expected = json!({ - "id": 2, - "title": "I'm not a legend", - "description": "A bad copy of the original movie I'm a lengend" - }); - - let (response, status_code) = server.get_document(2).await; - assert_eq!(status_code, 200); - assert_json_eq!(response, expected); -} - -#[actix_rt::test] -async fn index_new_fields_true() { - let mut server = common::Server::with_uid("movies"); - let body = json!({ - "uid": "movies", - "primaryKey": "id", - }); - server.create_index(body).await; - - // 1 - Set indexNewFields = true - - server.update_accept_new_fields(json!(true)).await; - - // 2 - Add a document - - let body = json!([{ - "id": 1, - "title": "I'm a legend", - }]); - - server.add_or_replace_multiple_documents(body).await; - - // 3 - Get the complete document - - let expected = json!({ - "id": 1, - "title": "I'm a legend", - }); - - let (response, status_code) = server.get_document(1).await; - assert_eq!(status_code, 200); - assert_json_eq!(response, expected); - - // 4 - Add a document with more fields - - let body = json!([{ - "id": 2, - "title": "I'm not a legend", - "description": "A bad copy of the original movie I'm a lengend" - }]); - - server.add_or_replace_multiple_documents(body).await; - - // 5 - Get the complete document - - let expected = json!({ - "id": 2, - "title": "I'm not a legend", - "description": "A bad copy of the original movie I'm a lengend" - }); - - let (response, status_code) = server.get_document(2).await; - assert_eq!(status_code, 200); - assert_json_eq!(response, expected); -} - -#[actix_rt::test] -async fn index_new_fields_false() { - let mut server = common::Server::with_uid("movies"); - let body = json!({ - "uid": "movies", - "primaryKey": "id", - }); - server.create_index(body).await; - - // 1 - Set indexNewFields = false - - server.update_accept_new_fields(json!(false)).await; - - // 2 - Add a document - - let body = json!([{ - "id": 1, - "title": "I'm a legend", - }]); - - server.add_or_replace_multiple_documents(body).await; - - // 3 - Get the complete document - - let expected = json!({ - "id": 1, - }); - - let (response, status_code) = server.get_document(1).await; - assert_eq!(status_code, 200); - assert_json_eq!(response, expected); - - // 4 - Add a document with more fields - - let body = json!([{ - "id": 2, - "title": "I'm not a legend", - "description": "A bad copy of the original movie I'm a lengend" - }]); - - server.add_or_replace_multiple_documents(body).await; - - // 5 - Get the complete document - - let expected = json!({ - "id": 2, - }); - - let (response, status_code) = server.get_document(2).await; - assert_eq!(status_code, 200); - assert_json_eq!(response, expected); -} - -#[actix_rt::test] -async fn index_new_fields_true_then_false() { - let mut server = common::Server::with_uid("movies"); - let body = json!({ - "uid": "movies", - "primaryKey": "id", - }); - server.create_index(body).await; - - // 1 - Set indexNewFields = true - - server.update_accept_new_fields(json!(true)).await; - - // 2 - Add a document - - let body = json!([{ - "id": 1, - "title": "I'm a legend", - }]); - - server.add_or_replace_multiple_documents(body).await; - - // 3 - Get the complete document - - let expected = json!({ - "id": 1, - "title": "I'm a legend", - }); - - let (response, status_code) = server.get_document(1).await; - assert_eq!(status_code, 200); - assert_json_eq!(response, expected); - - // 4 - Set indexNewFields = false - - server.update_accept_new_fields(json!(false)).await; - - // 5 - Add a document with more fields - - let body = json!([{ - "id": 2, - "title": "I'm not a legend", - "description": "A bad copy of the original movie I'm a lengend" - }]); - - server.add_or_replace_multiple_documents(body).await; - - // 6 - Get the complete document - - let expected = json!({ - "id": 2, - "title": "I'm not a legend", - }); - - let (response, status_code) = server.get_document(2).await; - assert_eq!(status_code, 200); - assert_json_eq!(response, expected); -} - -#[actix_rt::test] -async fn index_new_fields_false_then_true() { - let mut server = common::Server::with_uid("movies"); - let body = json!({ - "uid": "movies", - "primaryKey": "id", - }); - server.create_index(body).await; - - // 1 - Set indexNewFields = false - - server.update_accept_new_fields(json!(false)).await; - - // 2 - Add a document - - let body = json!([{ - "id": 1, - "title": "I'm a legend", - }]); - - server.add_or_replace_multiple_documents(body).await; - - // 3 - Get the complete document - - let expected = json!({ - "id": 1, - }); - - let (response, status_code) = server.get_document(1).await; - assert_eq!(status_code, 200); - assert_json_eq!(response, expected); - - // 4 - Set indexNewFields = false - - server.update_accept_new_fields(json!(true)).await; - - // 5 - Add a document with more fields - - let body = json!([{ - "id": 2, - "title": "I'm not a legend", - "description": "A bad copy of the original movie I'm a lengend" - }]); - - server.add_or_replace_multiple_documents(body).await; - - // 6 - Get the complete document - - let expected = json!({ - "id": 1, - }); - - let (response, status_code) = server.get_document(1).await; - assert_eq!(status_code, 200); - assert_json_eq!(response, expected); - - let expected = json!({ - "id": 2, - "description": "A bad copy of the original movie I'm a lengend" - }); - - let (response, status_code) = server.get_document(2).await; - assert_eq!(status_code, 200); - assert_json_eq!(response, expected); -} - -// Fix issue https://github.com/meilisearch/MeiliSearch/issues/518 -#[actix_rt::test] -async fn accept_new_fields_does_not_take_into_account_the_primary_key() { - let mut server = common::Server::with_uid("movies"); - - // 1 - Create an index with no primary-key - - let body = json!({ - "uid": "movies", - }); - let (response, status_code) = server.create_index(body).await; - assert_eq!(status_code, 201); - assert_eq!(response["primaryKey"], json!(null)); - - // 2 - Add searchable and displayed attributes as: ["title"] & Set acceptNewFields to false - - let body = json!({ - "searchableAttributes": ["title"], - "displayedAttributes": ["title"], - "acceptNewFields": false, - }); - - server.update_all_settings(body).await; - - // 4 - Add a document - - let body = json!([{ - "id": 1, - "title": "Test", - "comment": "comment test" - }]); - - server.add_or_replace_multiple_documents(body).await; - - // 5 - Get settings, they should not changed - - let (response, _status_code) = server.get_all_settings().await; - - let expected = json!({ - "rankingRules": [ - "typo", - "words", - "proximity", - "attribute", - "wordsPosition", - "exactness", - ], - "distinctAttribute": null, - "searchableAttributes": ["title"], - "displayedAttributes": ["title"], - "stopWords": [], - "synonyms": {}, - "attributesForFaceting": [], - "acceptNewFields": false, - }); - - assert_json_eq!(response, expected, ordered: false); -} diff --git a/meilisearch-http/tests/settings_ranking_rules.rs b/meilisearch-http/tests/settings_ranking_rules.rs index 6da614663..ac9a1e00c 100644 --- a/meilisearch-http/tests/settings_ranking_rules.rs +++ b/meilisearch-http/tests/settings_ranking_rules.rs @@ -171,6 +171,7 @@ async fn write_custom_ranking_and_index_documents() { let expected = json!({ "id": 1, + "name": "Cherry Orr", "color": "green" }); diff --git a/meilisearch-schema/src/schema.rs b/meilisearch-schema/src/schema.rs index 7579234dd..cbeccdffa 100644 --- a/meilisearch-schema/src/schema.rs +++ b/meilisearch-schema/src/schema.rs @@ -1,6 +1,42 @@ use crate::{FieldsMap, FieldId, SResult, Error, IndexedPos}; use serde::{Serialize, Deserialize}; use std::collections::{HashMap, HashSet}; +use std::borrow::Cow; + +#[derive(Clone, Debug, Serialize, Deserialize)] +enum OptionAll { + All, + Some(T), + None, +} + +impl OptionAll { + // replace the value with None and return the previous value + fn take(&mut self) -> OptionAll { + std::mem::replace(self, OptionAll::None) + } + + fn map U>(self, f: F) -> OptionAll { + match self { + OptionAll::Some(x) => OptionAll::Some(f(x)), + OptionAll::All => OptionAll::All, + OptionAll::None => OptionAll::None, + } + } + + pub fn is_all(&self) -> bool { + match self { + OptionAll::All => true, + _ => false, + } + } +} + +impl Default for OptionAll { + fn default() -> OptionAll { + OptionAll::All + } +} #[derive(Clone, Debug, Serialize, Deserialize, Default)] pub struct Schema { @@ -8,20 +44,15 @@ pub struct Schema { primary_key: Option, ranked: HashSet, - displayed: HashSet, + displayed: OptionAll>, - indexed: Vec, + indexed: OptionAll>, indexed_map: HashMap, - - accept_new_fields: bool, } impl Schema { pub fn new() -> Schema { - Schema { - accept_new_fields: true, - ..Default::default() - } + Schema::default() } pub fn with_primary_key(name: &str) -> Schema { @@ -29,21 +60,18 @@ impl Schema { let field_id = fields_map.insert(name).unwrap(); let mut displayed = HashSet::new(); - let mut indexed = Vec::new(); let mut indexed_map = HashMap::new(); displayed.insert(field_id); - indexed.push(field_id); indexed_map.insert(field_id, 0.into()); Schema { fields_map, primary_key: Some(field_id), ranked: HashSet::new(), - displayed, - indexed, + displayed: OptionAll::All, + indexed: OptionAll::All, indexed_map, - accept_new_fields: true, } } @@ -58,10 +86,8 @@ impl Schema { let id = self.insert(name)?; self.primary_key = Some(id); - if self.accept_new_fields { - self.set_indexed(name)?; - self.set_displayed(name)?; - } + self.set_indexed(name)?; + self.set_displayed(name)?; Ok(id) } @@ -92,12 +118,8 @@ impl Schema { Ok(id) } None => { - if self.accept_new_fields { - self.set_indexed(name)?; - self.set_displayed(name) - } else { - self.fields_map.insert(name) - } + self.set_indexed(name)?; + self.set_displayed(name) } } } @@ -110,20 +132,50 @@ impl Schema { self.ranked.iter().filter_map(|a| self.name(*a)).collect() } - pub fn displayed(&self) -> &HashSet { - &self.displayed + pub fn displayed(&self) -> Cow> { + match self.displayed { + OptionAll::Some(ref v) => Cow::Borrowed(v), + OptionAll::All => { + let fields = self + .fields_map + .iter() + .map(|(_, &v)| v) + .collect::>(); + Cow::Owned(fields) + } + OptionAll::None => Cow::Owned(HashSet::new()) + } + } + + pub fn is_displayed_all(&self) -> bool { + self.displayed.is_all() } pub fn displayed_name(&self) -> HashSet<&str> { - self.displayed.iter().filter_map(|a| self.name(*a)).collect() + match self.displayed { + OptionAll::All => self.fields_map.iter().filter_map(|(_, &v)| self.name(v)).collect(), + OptionAll::Some(ref v) => v.iter().filter_map(|a| self.name(*a)).collect(), + OptionAll::None => HashSet::new(), + } } - pub fn indexed(&self) -> &Vec { - &self.indexed + pub fn indexed(&self) -> Cow<[FieldId]> { + match self.indexed { + OptionAll::Some(ref v) => Cow::Borrowed(v), + OptionAll::All => { + let fields = self + .fields_map + .iter() + .map(|(_, &f)| f) + .collect(); + Cow::Owned(fields) + }, + OptionAll::None => Cow::Owned(Vec::new()) + } } pub fn indexed_name(&self) -> Vec<&str> { - self.indexed.iter().filter_map(|a| self.name(*a)).collect() + self.indexed().iter().filter_map(|a| self.name(*a)).collect() } pub fn set_ranked(&mut self, name: &str) -> SResult { @@ -134,18 +186,33 @@ impl Schema { pub fn set_displayed(&mut self, name: &str) -> SResult { let id = self.fields_map.insert(name)?; - self.displayed.insert(id); + self.displayed = match self.displayed.take() { + OptionAll::All => OptionAll::All, + OptionAll::None => { + let mut displayed = HashSet::new(); + displayed.insert(id); + OptionAll::Some(displayed) + }, + OptionAll::Some(mut v) => { + v.insert(id); + OptionAll::Some(v) + } + }; Ok(id) } pub fn set_indexed(&mut self, name: &str) -> SResult<(FieldId, IndexedPos)> { let id = self.fields_map.insert(name)?; + if let Some(indexed_pos) = self.indexed_map.get(&id) { return Ok((id, *indexed_pos)) }; - let pos = self.indexed.len() as u16; - self.indexed.push(id); + 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())) } @@ -159,16 +226,47 @@ 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.remove(&id); + 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::>(); + 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.retain(|x| *x != 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, + } } } @@ -177,20 +275,28 @@ impl Schema { } pub fn is_displayed(&self, id: FieldId) -> bool { - self.displayed.get(&id).is_some() + match self.displayed { + OptionAll::Some(ref v) => v.contains(&id), + OptionAll::All => true, + OptionAll::None => false, + } } pub fn is_indexed(&self, id: FieldId) -> Option<&IndexedPos> { self.indexed_map.get(&id) } + pub fn is_indexed_all(&self) -> bool { + self.indexed.is_all() + } + pub fn indexed_pos_to_field_id>(&self, pos: I) -> Option { - let indexed_pos = pos.into().0 as usize; - if indexed_pos < self.indexed.len() { - Some(self.indexed[indexed_pos as usize]) - } else { - None - } + let indexed_pos = pos.into().0; + self + .indexed_map + .iter() + .find(|(_, &v)| v.0 == indexed_pos) + .map(|(&k, _)| k) } pub fn update_ranked>(&mut self, data: impl IntoIterator) -> SResult<()> { @@ -202,7 +308,13 @@ impl Schema { } pub fn update_displayed>(&mut self, data: impl IntoIterator) -> SResult<()> { - self.displayed.clear(); + self.displayed = match self.displayed.take() { + OptionAll::Some(mut v) => { + v.clear(); + OptionAll::Some(v) + } + _ => OptionAll::Some(HashSet::new()) + }; for name in data { self.set_displayed(name.as_ref())?; } @@ -210,7 +322,13 @@ impl Schema { } pub fn update_indexed>(&mut self, data: Vec) -> SResult<()> { - self.indexed.clear(); + self.indexed = match self.indexed.take() { + OptionAll::Some(mut v) => { + v.clear(); + OptionAll::Some(v) + }, + _ => OptionAll::Some(Vec::new()), + }; self.indexed_map.clear(); for name in data { self.set_indexed(name.as_ref())?; @@ -219,29 +337,16 @@ impl Schema { } pub fn set_all_fields_as_indexed(&mut self) { - self.indexed.clear(); + self.indexed = OptionAll::All; self.indexed_map.clear(); for (_name, id) in self.fields_map.iter() { - let pos = self.indexed.len() as u16; - self.indexed.push(*id); + let pos = self.indexed_map.len() as u16; self.indexed_map.insert(*id, pos.into()); } } pub fn set_all_fields_as_displayed(&mut self) { - self.displayed.clear(); - - for (_name, id) in self.fields_map.iter() { - self.displayed.insert(*id); - } - } - - pub fn accept_new_fields(&self) -> bool { - self.accept_new_fields - } - - pub fn set_accept_new_fields(&mut self, value: bool) { - self.accept_new_fields = value; + self.displayed = OptionAll::All } }