916: Considere an empty query search as a placeholder search r=MarinPostma a=qdequele

Fix #856; Relative tracking issue: #729

Co-authored-by: qdequele <quentin@meilisearch.com>
This commit is contained in:
bors[bot] 2020-09-28 13:13:47 +00:00 committed by GitHub
commit 099a0802fc
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 235 additions and 119 deletions

56
Cargo.lock generated
View File

@ -95,7 +95,7 @@ dependencies = [
"serde_urlencoded",
"sha-1 0.9.1",
"slab",
"time 0.2.21",
"time 0.2.22",
]
[[package]]
@ -110,9 +110,9 @@ dependencies = [
[[package]]
name = "actix-router"
version = "0.2.4"
version = "0.2.5"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "9d7a10ca4d94e8c8e7a87c5173aba1b97ba9a6563ca02b0e1cd23531093d3ec8"
checksum = "bbd1f7dbda1645bf7da33554db60891755f6c01c1b2169e2f4c492098d30c235"
dependencies = [
"bytestring",
"http 0.2.1",
@ -266,7 +266,7 @@ dependencies = [
"serde_json",
"serde_urlencoded",
"socket2",
"time 0.2.21",
"time 0.2.22",
"tinyvec",
"url",
]
@ -330,7 +330,7 @@ checksum = "4d25d88fd6b8041580a654f9d0c581a047baee2b3efee13275f2fc392fc75034"
[[package]]
name = "assert-json-diff"
version = "1.0.1"
source = "git+https://github.com/qdequele/assert-json-diff#9012a0c8866d0f2db0ef9a6242e4a19d1e8c67e4"
source = "git+https://github.com/qdequele/assert-json-diff?branch=master#9012a0c8866d0f2db0ef9a6242e4a19d1e8c67e4"
dependencies = [
"serde",
"serde_json",
@ -579,14 +579,16 @@ checksum = "4785bdd1c96b2a846b2bd7cc02e86b6b3dbf14e7e53446c4f54c92a361040822"
[[package]]
name = "chrono"
version = "0.4.15"
version = "0.4.18"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "942f72db697d8767c22d46a598e01f2d3b475501ea43d0db4f16d90259182d0b"
checksum = "d021fddb7bd3e734370acfa4a83f34095571d8570c039f1420d77540f68d5772"
dependencies = [
"libc",
"num-integer",
"num-traits",
"serde",
"time 0.1.44",
"winapi 0.3.9",
]
[[package]]
@ -638,7 +640,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "1373a16a4937bc34efec7b391f9c1500c30b8478a701a4f44c9165cc0475a6e0"
dependencies = [
"percent-encoding 2.1.0",
"time 0.2.21",
"time 0.2.22",
"version_check",
]
@ -1912,7 +1914,8 @@ checksum = "d4fd5641d01c8f18a23da7b6fe29298ff4b55afcccdf78973b24cf3175fee32e"
[[package]]
name = "pest"
version = "2.1.3"
source = "git+https://github.com/pest-parser/pest.git?rev=51fd1d49f1041f7839975664ef71fe15c7dcaf67#51fd1d49f1041f7839975664ef71fe15c7dcaf67"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "10f4872ae94d7b90ae48754df22fd42ad52ce740b8f370b03da4835417403e53"
dependencies = [
"ucd-trie",
]
@ -1920,8 +1923,7 @@ dependencies = [
[[package]]
name = "pest"
version = "2.1.3"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "10f4872ae94d7b90ae48754df22fd42ad52ce740b8f370b03da4835417403e53"
source = "git+https://github.com/pest-parser/pest.git?rev=51fd1d49f1041f7839975664ef71fe15c7dcaf67#51fd1d49f1041f7839975664ef71fe15c7dcaf67"
dependencies = [
"ucd-trie",
]
@ -1962,18 +1964,18 @@ dependencies = [
[[package]]
name = "pin-project"
version = "0.4.23"
version = "0.4.24"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "ca4433fff2ae79342e497d9f8ee990d174071408f28f726d6d83af93e58e48aa"
checksum = "f48fad7cfbff853437be7cf54d7b993af21f53be7f0988cbfe4a51535aa77205"
dependencies = [
"pin-project-internal",
]
[[package]]
name = "pin-project-internal"
version = "0.4.23"
version = "0.4.24"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "2c0e815c3ee9a031fdf5af21c10aa17c573c9c6a566328d99e3936c34e36461f"
checksum = "24c6d293bdd3ca5a1697997854c6cf7855e43fb6a0ba1c47af57a5bcafd158ae"
dependencies = [
"proc-macro2",
"quote",
@ -1982,9 +1984,9 @@ dependencies = [
[[package]]
name = "pin-project-lite"
version = "0.1.7"
version = "0.1.8"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "282adbf10f2698a7a77f8e983a74b2d18176c19a7fd32a45446139ae7b02b715"
checksum = "71f349a4f0e70676ffb2dbafe16d0c992382d02f0a952e3ddf584fc289dac6b3"
[[package]]
name = "pin-utils"
@ -2054,9 +2056,9 @@ checksum = "eba180dafb9038b050a4c280019bbedf9f2467b61e5d892dcad585bb57aadc5a"
[[package]]
name = "proc-macro2"
version = "1.0.21"
version = "1.0.23"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "36e28516df94f3dd551a587da5357459d9b36d945a7c37c3557928c1c2ff2a2c"
checksum = "51ef7cd2518ead700af67bf9d1a658d90b6037d77110fd9c0445429d0ba1c6c9"
dependencies = [
"unicode-xid",
]
@ -2698,9 +2700,9 @@ checksum = "8ea5119cdb4c55b55d432abb513a0429384878c15dde60cc77b1c99de1a95a6a"
[[package]]
name = "structopt"
version = "0.3.17"
version = "0.3.18"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "6cc388d94ffabf39b5ed5fadddc40147cb21e605f53db6f8f36a625d27489ac5"
checksum = "a33f6461027d7f08a13715659b2948e1602c31a3756aeae9378bfe7518c72e82"
dependencies = [
"clap",
"lazy_static",
@ -2709,9 +2711,9 @@ dependencies = [
[[package]]
name = "structopt-derive"
version = "0.4.10"
version = "0.4.11"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "5e2513111825077552a6751dfad9e11ce0fba07d7276a3943a037d7e93e64c5f"
checksum = "c92e775028122a4b3dd55d58f14fc5120289c69bee99df1d117ae30f84b225c9"
dependencies = [
"heck",
"proc-macro-error",
@ -2722,9 +2724,9 @@ dependencies = [
[[package]]
name = "syn"
version = "1.0.41"
version = "1.0.42"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "6690e3e9f692504b941dc6c3b188fd28df054f7fb8469ab40680df52fdcc842b"
checksum = "9c51d92969d209b54a98397e1b91c8ae82d8c87a7bb87df0b29aa2ad81454228"
dependencies = [
"proc-macro2",
"quote",
@ -2848,9 +2850,9 @@ dependencies = [
[[package]]
name = "time"
version = "0.2.21"
version = "0.2.22"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "2c2e31fb28e2a9f01f5ed6901b066c1ba2333c04b64dc61254142bafcb3feb2c"
checksum = "55b7151c9065e80917fbf285d9a5d1432f60db41d170ccafc749a136b41a93af"
dependencies = [
"const_fn",
"libc",

View File

@ -1,9 +1,7 @@
use std::collections::{HashSet, HashMap};
use std::collections::{HashMap, HashSet};
use actix_web::{get, post, web, HttpResponse};
use log::warn;
use actix_web::web;
use actix_web::HttpResponse;
use actix_web::{get, post};
use serde::{Deserialize, Serialize};
use serde_json::Value;
@ -14,11 +12,10 @@ use crate::routes::IndexParam;
use crate::Data;
use meilisearch_core::facets::FacetFilter;
use meilisearch_schema::{Schema, FieldId};
use meilisearch_schema::{FieldId, Schema};
pub fn services(cfg: &mut web::ServiceConfig) {
cfg.service(search_with_post)
.service(search_with_url_query);
cfg.service(search_with_post).service(search_with_url_query);
}
#[derive(Serialize, Deserialize)]
@ -93,7 +90,11 @@ async fn search_with_post(
}
impl SearchQuery {
fn search(&self, index_uid: &str, data: web::Data<Data>) -> Result<SearchResult, ResponseError> {
fn search(
&self,
index_uid: &str,
data: web::Data<Data>,
) -> Result<SearchResult, ResponseError> {
let index = data
.db
.open_index(index_uid)
@ -105,7 +106,12 @@ impl SearchQuery {
.schema(&reader)?
.ok_or(Error::internal("Impossible to retrieve the schema"))?;
let mut search_builder = index.new_search(self.q.clone());
let query = self
.q
.clone()
.and_then(|q| if q.is_empty() { None } else { Some(q) });
let mut search_builder = index.new_search(query);
if let Some(offset) = self.offset {
search_builder.offset(offset);
@ -118,7 +124,8 @@ impl SearchQuery {
let mut restricted_attributes: HashSet<&str>;
match &self.attributes_to_retrieve {
Some(attributes_to_retrieve) => {
let attributes_to_retrieve: HashSet<&str> = attributes_to_retrieve.split(',').collect();
let attributes_to_retrieve: HashSet<&str> =
attributes_to_retrieve.split(',').collect();
if attributes_to_retrieve.contains("*") {
restricted_attributes = available_attributes.clone();
} else {
@ -132,15 +139,22 @@ impl SearchQuery {
}
}
}
},
}
None => {
restricted_attributes = available_attributes.clone();
}
}
if let Some(ref facet_filters) = self.facet_filters {
let attrs = index.main.attributes_for_faceting(&reader)?.unwrap_or_default();
search_builder.add_facet_filters(FacetFilter::from_str(facet_filters, &schema, &attrs)?);
let attrs = index
.main
.attributes_for_faceting(&reader)?
.unwrap_or_default();
search_builder.add_facet_filters(FacetFilter::from_str(
facet_filters,
&schema,
&attrs,
)?);
}
if let Some(facets) = &self.facets_distribution {
@ -148,7 +162,7 @@ impl SearchQuery {
Some(ref attrs) => {
let field_ids = prepare_facet_list(&facets, &schema, attrs)?;
search_builder.add_facets(field_ids);
},
}
None => return Err(FacetCountError::NoFacetSet.into()),
}
}
@ -160,20 +174,23 @@ impl SearchQuery {
for attribute in attributes_to_crop.split(',') {
let mut attribute = attribute.split(':');
let attr = attribute.next();
let length = attribute.next().and_then(|s| s.parse().ok()).unwrap_or(default_length);
let length = attribute
.next()
.and_then(|s| s.parse().ok())
.unwrap_or(default_length);
match attr {
Some("*") => {
for attr in &restricted_attributes {
final_attributes.insert(attr.to_string(), length);
}
},
}
Some(attr) => {
if available_attributes.contains(attr) {
final_attributes.insert(attr.to_string(), length);
} else {
warn!("The attributes {:?} present in attributesToCrop parameter doesn't exist", attr);
}
},
}
None => (),
}
}
@ -215,7 +232,11 @@ impl SearchQuery {
///
/// An error is returned if the array is malformed, or if it contains attributes that are
/// unexisting, or not set as facets.
fn prepare_facet_list(facets: &str, schema: &Schema, facet_attrs: &[FieldId]) -> Result<Vec<(FieldId, String)>, FacetCountError> {
fn prepare_facet_list(
facets: &str,
schema: &Schema,
facet_attrs: &[FieldId],
) -> Result<Vec<(FieldId, String)>, FacetCountError> {
let json_array = serde_json::from_str(facets)?;
match json_array {
Value::Array(vals) => {
@ -243,6 +264,6 @@ fn prepare_facet_list(facets: &str, schema: &Schema, facet_attrs: &[FieldId]) ->
}
Ok(field_ids)
}
bad_val => Err(FacetCountError::unexpected_token(bad_val, &["[String]"]))
bad_val => Err(FacetCountError::unexpected_token(bad_val, &["[String]"])),
}
}

View File

@ -2,10 +2,11 @@ use std::convert::Into;
use serde_json::json;
use serde_json::Value;
use std::sync::Mutex;
use std::cell::RefCell;
use std::sync::Mutex;
#[macro_use] mod common;
#[macro_use]
mod common;
#[actix_rt::test]
async fn placeholder_search_with_limit() {
@ -36,7 +37,12 @@ async fn placeholder_search_with_offset() {
assert_eq!(status_code, 200);
// take results at offset 3 as reference
let lock = expected.lock().unwrap();
lock.replace(response["hits"].as_array().unwrap()[3..6].iter().cloned().collect());
lock.replace(
response["hits"].as_array().unwrap()[3..6]
.iter()
.cloned()
.collect(),
);
});
let expected = expected.into_inner().unwrap().into_inner();
@ -64,11 +70,7 @@ async fn placeholder_search_with_attribute_to_highlight_wildcard() {
test_post_get_search!(server, query, |response, status_code| {
assert_eq!(status_code, 200);
let result = response["hits"]
.as_array()
.unwrap()[0]
.as_object()
.unwrap();
let result = response["hits"].as_array().unwrap()[0].as_object().unwrap();
for value in result.values() {
assert!(value.to_string().find("<em>").is_none());
}
@ -135,11 +137,7 @@ async fn placeholder_search_with_attributes_to_retrieve() {
});
test_post_get_search!(server, query, |response, _status_code| {
let hit = response["hits"]
.as_array()
.unwrap()[0]
.as_object()
.unwrap();
let hit = response["hits"].as_array().unwrap()[0].as_object().unwrap();
assert_eq!(hit.values().count(), 2);
let _ = hit["gender"];
let _ = hit["about"];
@ -166,7 +164,9 @@ async fn placeholder_search_with_filter() {
test_post_get_search!(server, query, |response, _status_code| {
let hits = response["hits"].as_array().unwrap();
let value = Value::String(String::from("bug"));
assert!(hits.iter().all(|v| v["tags"].as_array().unwrap().contains(&value)));
assert!(hits
.iter()
.all(|v| v["tags"].as_array().unwrap().contains(&value)));
});
let query = json!({
@ -176,10 +176,9 @@ async fn placeholder_search_with_filter() {
let hits = response["hits"].as_array().unwrap();
let bug = Value::String(String::from("bug"));
let wontfix = Value::String(String::from("wontfix"));
assert!(hits.iter().all(|v|
v["color"].as_str().unwrap() == "Green" &&
v["tags"].as_array().unwrap().contains(&bug) ||
v["tags"].as_array().unwrap().contains(&wontfix)));
assert!(hits.iter().all(|v| v["color"].as_str().unwrap() == "Green"
&& v["tags"].as_array().unwrap().contains(&bug)
|| v["tags"].as_array().unwrap().contains(&wontfix)));
});
}
@ -257,7 +256,12 @@ async fn placeholder_test_faceted_search_valid() {
.as_array()
.unwrap()
.iter()
.all(|value| value.get("tags").unwrap().as_array().unwrap().contains(&Value::String("bug".to_owned()))));
.all(|value| value
.get("tags")
.unwrap()
.as_array()
.unwrap()
.contains(&Value::String("bug".to_owned()))));
});
// test and: ["color:blue", "tags:bug"]
@ -272,10 +276,13 @@ async fn placeholder_test_faceted_search_valid() {
.as_array()
.unwrap()
.iter()
.all(|value| value
.get("color")
.unwrap() == "blue"
&& value.get("tags").unwrap().as_array().unwrap().contains(&Value::String("bug".to_owned()))));
.all(|value| value.get("color").unwrap() == "blue"
&& value
.get("tags")
.unwrap()
.as_array()
.unwrap()
.contains(&Value::String("bug".to_owned()))));
});
// test or: [["color:blue", "color:green"]]
@ -290,13 +297,8 @@ async fn placeholder_test_faceted_search_valid() {
.as_array()
.unwrap()
.iter()
.all(|value|
value
.get("color")
.unwrap() == "blue"
|| value
.get("color")
.unwrap() == "Green"));
.all(|value| value.get("color").unwrap() == "blue"
|| value.get("color").unwrap() == "Green"));
});
// test and-or: ["tags:bug", ["color:blue", "color:green"]]
let query = json!({
@ -310,20 +312,14 @@ async fn placeholder_test_faceted_search_valid() {
.as_array()
.unwrap()
.iter()
.all(|value|
value
.all(|value| value
.get("tags")
.unwrap()
.as_array()
.unwrap()
.contains(&Value::String("bug".to_owned()))
&& (value
.get("color")
.unwrap() == "blue"
|| value
.get("color")
.unwrap() == "Green")));
&& (value.get("color").unwrap() == "blue"
|| value.get("color").unwrap() == "Green")));
});
}
@ -335,7 +331,10 @@ async fn placeholder_test_faceted_search_invalid() {
let query = json!({
"facetFilters": ["color:blue"]
});
test_post_get_search!(server, query, |_response, status_code| assert_ne!(status_code, 202));
test_post_get_search!(server, query, |_response, status_code| assert_ne!(
status_code,
202
));
let body = json!({
"attributesForFaceting": ["color", "tags"]
@ -346,34 +345,52 @@ async fn placeholder_test_faceted_search_invalid() {
let query = json!({
"facetFilters": []
});
test_post_get_search!(server, query, |_response, status_code| assert_ne!(status_code, 202));
test_post_get_search!(server, query, |_response, status_code| assert_ne!(
status_code,
202
));
// [[]]
let query = json!({
"facetFilters": [[]]
});
test_post_get_search!(server, query, |_response, status_code| assert_ne!(status_code, 202));
test_post_get_search!(server, query, |_response, status_code| assert_ne!(
status_code,
202
));
// ["color:green", []]
let query = json!({
"facetFilters": ["color:green", []]
});
test_post_get_search!(server, query, |_response, status_code| assert_ne!(status_code, 202));
test_post_get_search!(server, query, |_response, status_code| assert_ne!(
status_code,
202
));
// too much depth
// [[[]]]
let query = json!({
"facetFilters": [[[]]]
});
test_post_get_search!(server, query, |_response, status_code| assert_ne!(status_code, 202));
test_post_get_search!(server, query, |_response, status_code| assert_ne!(
status_code,
202
));
// [["color:green", ["color:blue"]]]
let query = json!({
"facetFilters": [["color:green", ["color:blue"]]]
});
test_post_get_search!(server, query, |_response, status_code| assert_ne!(status_code, 202));
test_post_get_search!(server, query, |_response, status_code| assert_ne!(
status_code,
202
));
// "color:green"
let query = json!({
"facetFilters": "color:green"
});
test_post_get_search!(server, query, |_response, status_code| assert_ne!(status_code, 202));
test_post_get_search!(server, query, |_response, status_code| assert_ne!(
status_code,
202
));
}
#[actix_rt::test]
@ -381,9 +398,8 @@ async fn placeholder_test_facet_count() {
let mut server = common::Server::test_server().await;
// test without facet distribution
let query = json!({
});
test_post_get_search!(server, query, |response, _status_code|{
let query = json!({});
test_post_get_search!(server, query, |response, _status_code| {
assert!(response.get("exhaustiveFacetsCount").is_none());
assert!(response.get("facetsDistribution").is_none());
});
@ -392,7 +408,7 @@ async fn placeholder_test_facet_count() {
let query = json!({
"facetsDistribution": ["color"]
});
test_post_get_search!(server, query.clone(), |_response, status_code|{
test_post_get_search!(server, query.clone(), |_response, status_code| {
assert_eq!(status_code, 400);
});
@ -401,52 +417,109 @@ async fn placeholder_test_facet_count() {
});
server.update_all_settings(body).await;
// same as before, but now facets are set:
test_post_get_search!(server, query, |response, _status_code|{
test_post_get_search!(server, query, |response, _status_code| {
println!("{}", response);
assert!(response.get("exhaustiveFacetsCount").is_some());
assert_eq!(response.get("facetsDistribution").unwrap().as_object().unwrap().values().count(), 1);
assert_eq!(
response
.get("facetsDistribution")
.unwrap()
.as_object()
.unwrap()
.values()
.count(),
1
);
});
// searching on color and tags
let query = json!({
"facetsDistribution": ["color", "tags"]
});
test_post_get_search!(server, query, |response, _status_code|{
let facets = response.get("facetsDistribution").unwrap().as_object().unwrap();
test_post_get_search!(server, query, |response, _status_code| {
let facets = response
.get("facetsDistribution")
.unwrap()
.as_object()
.unwrap();
assert_eq!(facets.values().count(), 2);
assert_ne!(!facets.get("color").unwrap().as_object().unwrap().values().count(), 0);
assert_ne!(!facets.get("tags").unwrap().as_object().unwrap().values().count(), 0);
assert_ne!(
!facets
.get("color")
.unwrap()
.as_object()
.unwrap()
.values()
.count(),
0
);
assert_ne!(
!facets
.get("tags")
.unwrap()
.as_object()
.unwrap()
.values()
.count(),
0
);
});
// wildcard
let query = json!({
"facetsDistribution": ["*"]
});
test_post_get_search!(server, query, |response, _status_code|{
assert_eq!(response.get("facetsDistribution").unwrap().as_object().unwrap().values().count(), 2);
test_post_get_search!(server, query, |response, _status_code| {
assert_eq!(
response
.get("facetsDistribution")
.unwrap()
.as_object()
.unwrap()
.values()
.count(),
2
);
});
// wildcard with other attributes:
let query = json!({
"facetsDistribution": ["color", "*"]
});
test_post_get_search!(server, query, |response, _status_code|{
assert_eq!(response.get("facetsDistribution").unwrap().as_object().unwrap().values().count(), 2);
test_post_get_search!(server, query, |response, _status_code| {
assert_eq!(
response
.get("facetsDistribution")
.unwrap()
.as_object()
.unwrap()
.values()
.count(),
2
);
});
// empty facet list
let query = json!({
"facetsDistribution": []
});
test_post_get_search!(server, query, |response, _status_code|{
assert_eq!(response.get("facetsDistribution").unwrap().as_object().unwrap().values().count(), 0);
test_post_get_search!(server, query, |response, _status_code| {
assert_eq!(
response
.get("facetsDistribution")
.unwrap()
.as_object()
.unwrap()
.values()
.count(),
0
);
});
// attr not set as facet passed:
let query = json!({
"facetsDistribution": ["gender"]
});
test_post_get_search!(server, query, |_response, status_code|{
test_post_get_search!(server, query, |_response, status_code| {
assert_eq!(status_code, 400);
});
}
#[actix_rt::test]
@ -475,13 +548,15 @@ async fn placeholder_test_sort() {
"attributesForFaceting": ["color"]
});
server.update_all_settings(body).await;
let query = json!({ });
let query = json!({});
test_post_get_search!(server, query, |response, _status_code| {
let hits = response["hits"].as_array().unwrap();
hits.iter().map(|v| v["age"].as_u64().unwrap()).fold(0, |prev, cur| {
assert!(cur >= prev);
cur
});
hits.iter()
.map(|v| v["age"].as_u64().unwrap())
.fold(0, |prev, cur| {
assert!(cur >= prev);
cur
});
});
let query = json!({
@ -489,9 +564,27 @@ async fn placeholder_test_sort() {
});
test_post_get_search!(server, query, |response, _status_code| {
let hits = response["hits"].as_array().unwrap();
hits.iter().map(|v| v["age"].as_u64().unwrap()).fold(0, |prev, cur| {
assert!(cur >= prev);
cur
});
hits.iter()
.map(|v| v["age"].as_u64().unwrap())
.fold(0, |prev, cur| {
assert!(cur >= prev);
cur
});
});
}
#[actix_rt::test]
async fn placeholder_search_with_empty_query() {
let mut server = common::Server::test_server().await;
let query = json! ({
"q": "",
"limit": 3
});
test_post_get_search!(server, query, |response, status_code| {
eprintln!("{}", response);
assert_eq!(status_code, 200);
assert_eq!(response["hits"].as_array().unwrap().len(), 3);
});
}