From 74315b4ea83573e695585166f65d590511178d83 Mon Sep 17 00:00:00 2001 From: Louis Dureuil Date: Wed, 12 Jul 2023 10:08:29 +0200 Subject: [PATCH 1/4] Fixes #3911 --- milli/src/search/new/mod.rs | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/milli/src/search/new/mod.rs b/milli/src/search/new/mod.rs index 034b279ad..5da344014 100644 --- a/milli/src/search/new/mod.rs +++ b/milli/src/search/new/mod.rs @@ -85,7 +85,12 @@ impl<'ctx> SearchContext<'ctx> { let searchable_names = self.index.searchable_fields(self.txn)?; let mut restricted_fids = Vec::new(); + let mut contains_wildcard = false; for field_name in searchable_attributes { + if field_name == "*" { + contains_wildcard = true; + continue; + } let searchable_contains_name = searchable_names.as_ref().map(|sn| sn.iter().any(|name| name == field_name)); let fid = match (fids_map.id(field_name), searchable_contains_name) { @@ -132,7 +137,7 @@ impl<'ctx> SearchContext<'ctx> { restricted_fids.push(fid); } - self.restricted_fids = Some(restricted_fids); + self.restricted_fids = (!contains_wildcard).then_some(restricted_fids); Ok(()) } From 4310928803940181ae6cfd8a598d03910c5c525b Mon Sep 17 00:00:00 2001 From: Louis Dureuil Date: Wed, 12 Jul 2023 10:08:56 +0200 Subject: [PATCH 2/4] Fixes #3912 --- milli/src/search/new/mod.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/milli/src/search/new/mod.rs b/milli/src/search/new/mod.rs index 5da344014..26f992be2 100644 --- a/milli/src/search/new/mod.rs +++ b/milli/src/search/new/mod.rs @@ -104,8 +104,10 @@ impl<'ctx> SearchContext<'ctx> { } .into()) } + // The field is not searchable, but the searchableAttributes are set to * => ignore field + (None, None) => continue, // The field is not searchable => User error - _otherwise => { + (_fid, Some(false)) => { let mut valid_fields: BTreeSet<_> = fids_map.names().map(String::from).collect(); From 16c8437b28a8ba18f479300307b81235cb05d1d7 Mon Sep 17 00:00:00 2001 From: Louis Dureuil Date: Wed, 12 Jul 2023 10:36:55 +0200 Subject: [PATCH 3/4] Update tests --- meilisearch/tests/search/errors.rs | 51 +++++++++++++- .../tests/search/restrict_searchable.rs | 70 +++++++++++++++++++ 2 files changed, 120 insertions(+), 1 deletion(-) diff --git a/meilisearch/tests/search/errors.rs b/meilisearch/tests/search/errors.rs index 3d34de0fd..0745aa7d6 100644 --- a/meilisearch/tests/search/errors.rs +++ b/meilisearch/tests/search/errors.rs @@ -968,9 +968,12 @@ async fn sort_unset_ranking_rule() { async fn search_on_unknown_field() { let server = Server::new().await; let index = server.index("test"); + index.update_settings_searchable_attributes(json!(["id", "title"])).await; + index.wait_task(0).await; + let documents = DOCUMENTS.clone(); index.add_documents(documents, None).await; - index.wait_task(0).await; + index.wait_task(1).await; index .search( @@ -989,3 +992,49 @@ async fn search_on_unknown_field() { ) .await; } + +#[actix_rt::test] +async fn search_on_unknown_field_plus_joker() { + let server = Server::new().await; + let index = server.index("test"); + index.update_settings_searchable_attributes(json!(["id", "title"])).await; + index.wait_task(0).await; + + let documents = DOCUMENTS.clone(); + index.add_documents(documents, None).await; + index.wait_task(1).await; + + index + .search( + json!({"q": "Captain Marvel", "attributesToSearchOn": ["*", "unknown"]}), + |response, code| { + snapshot!(code, @"400 Bad Request"); + snapshot!(json_string!(response), @r###" + { + "message": "Attribute `unknown` is not searchable. Available searchable attributes are: `id, title`.", + "code": "invalid_search_attributes_to_search_on", + "type": "invalid_request", + "link": "https://docs.meilisearch.com/errors#invalid_search_attributes_to_search_on" + } + "###); + }, + ) + .await; + + index + .search( + json!({"q": "Captain Marvel", "attributesToSearchOn": ["unknown", "*"]}), + |response, code| { + snapshot!(code, @"400 Bad Request"); + snapshot!(json_string!(response), @r###" + { + "message": "Attribute `unknown` is not searchable. Available searchable attributes are: `id, title`.", + "code": "invalid_search_attributes_to_search_on", + "type": "invalid_request", + "link": "https://docs.meilisearch.com/errors#invalid_search_attributes_to_search_on" + } + "###); + }, + ) + .await; +} diff --git a/meilisearch/tests/search/restrict_searchable.rs b/meilisearch/tests/search/restrict_searchable.rs index f119acea5..6e41c5c55 100644 --- a/meilisearch/tests/search/restrict_searchable.rs +++ b/meilisearch/tests/search/restrict_searchable.rs @@ -49,6 +49,76 @@ async fn simple_search_on_title() { .await; } +#[actix_rt::test] +async fn search_no_searchable_attribute_set() { + let server = Server::new().await; + let index = index_with_documents(&server, &SIMPLE_SEARCH_DOCUMENTS).await; + + index + .search( + json!({"q": "Captain Marvel", "attributesToSearchOn": ["unknown"]}), + |response, code| { + snapshot!(code, @"200 OK"); + snapshot!(response["hits"].as_array().unwrap().len(), @"0"); + }, + ) + .await; + + index.update_settings_searchable_attributes(json!(["*"])).await; + index.wait_task(1).await; + + index + .search( + json!({"q": "Captain Marvel", "attributesToSearchOn": ["unknown"]}), + |response, code| { + snapshot!(code, @"200 OK"); + snapshot!(response["hits"].as_array().unwrap().len(), @"0"); + }, + ) + .await; + + index.update_settings_searchable_attributes(json!(["description", "*", "title"])).await; + index.wait_task(2).await; + + index + .search( + json!({"q": "Captain Marvel", "attributesToSearchOn": ["unknown"]}), + |response, code| { + snapshot!(code, @"200 OK"); + snapshot!(response["hits"].as_array().unwrap().len(), @"0"); + }, + ) + .await; +} + +#[actix_rt::test] +async fn search_on_all_attributes() { + let server = Server::new().await; + let index = index_with_documents(&server, &SIMPLE_SEARCH_DOCUMENTS).await; + + index + .search(json!({"q": "Captain Marvel", "attributesToSearchOn": ["*"]}), |response, code| { + snapshot!(code, @"200 OK"); + snapshot!(response["hits"].as_array().unwrap().len(), @"3"); + }) + .await; +} + +#[actix_rt::test] +async fn search_on_all_attributes_restricted_set() { + let server = Server::new().await; + let index = index_with_documents(&server, &SIMPLE_SEARCH_DOCUMENTS).await; + index.update_settings_searchable_attributes(json!(["title"])).await; + index.wait_task(1).await; + + index + .search(json!({"q": "Captain Marvel", "attributesToSearchOn": ["*"]}), |response, code| { + snapshot!(code, @"200 OK"); + snapshot!(response["hits"].as_array().unwrap().len(), @"2"); + }) + .await; +} + #[actix_rt::test] async fn simple_prefix_search_on_title() { let server = Server::new().await; From 183f23f40d4152a5387d6bb5e41e12de8bc563bd Mon Sep 17 00:00:00 2001 From: Louis Dureuil Date: Wed, 12 Jul 2023 16:06:15 +0200 Subject: [PATCH 4/4] More relevant test Co-authored-by: Many the fish --- meilisearch/tests/search/restrict_searchable.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/meilisearch/tests/search/restrict_searchable.rs b/meilisearch/tests/search/restrict_searchable.rs index 6e41c5c55..b589ccfb7 100644 --- a/meilisearch/tests/search/restrict_searchable.rs +++ b/meilisearch/tests/search/restrict_searchable.rs @@ -77,15 +77,15 @@ async fn search_no_searchable_attribute_set() { ) .await; - index.update_settings_searchable_attributes(json!(["description", "*", "title"])).await; + index.update_settings_searchable_attributes(json!(["*"])).await; index.wait_task(2).await; index .search( - json!({"q": "Captain Marvel", "attributesToSearchOn": ["unknown"]}), + json!({"q": "Captain Marvel", "attributesToSearchOn": ["unknown", "title"]}), |response, code| { snapshot!(code, @"200 OK"); - snapshot!(response["hits"].as_array().unwrap().len(), @"0"); + snapshot!(response["hits"].as_array().unwrap().len(), @"2"); }, ) .await;