3049: Plural on task query r=Kerollmops a=irevoire

Fixes #3045 and fixes #3046

3063: Update the analytics on the search r=Kerollmops a=irevoire

Partially fix https://github.com/meilisearch/meilisearch/issues/2955

Must be merged after https://github.com/meilisearch/meilisearch/pull/3060

Co-authored-by: Tamo <tamo@meilisearch.com>
This commit is contained in:
bors[bot] 2022-11-16 13:20:40 +00:00 committed by GitHub
commit 7f89e302a2
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
6 changed files with 193 additions and 32 deletions

View File

@ -60,9 +60,9 @@ pub enum Error {
InvalidIndexUid { index_uid: String }, InvalidIndexUid { index_uid: String },
#[error("Task `{0}` not found.")] #[error("Task `{0}` not found.")]
TaskNotFound(TaskId), TaskNotFound(TaskId),
#[error("Query parameters to filter the tasks to delete are missing. Available query parameters are: `uid`, `indexUid`, `status`, `type`.")] #[error("Query parameters to filter the tasks to delete are missing. Available query parameters are: `uids`, `indexUids`, `statuses`, `types`, `beforeEnqueuedAt`, `afterEnqueuedAt`, `beforeStartedAt`, `afterStartedAt`, `beforeFinishedAt`, `afterFinishedAt`.")]
TaskDeletionWithEmptyQuery, TaskDeletionWithEmptyQuery,
#[error("Query parameters to filter the tasks to cancel are missing. Available query parameters are: `uid`, `indexUid`, `status`, `type`.")] #[error("Query parameters to filter the tasks to cancel are missing. Available query parameters are: `uids`, `indexUids`, `statuses`, `types`, `beforeEnqueuedAt`, `afterEnqueuedAt`, `beforeStartedAt`, `afterStartedAt`, `beforeFinishedAt`, `afterFinishedAt`.")]
TaskCancelationWithEmptyQuery, TaskCancelationWithEmptyQuery,
#[error(transparent)] #[error(transparent)]
@ -102,11 +102,11 @@ impl ErrorCode for Error {
Error::IndexAlreadyExists(_) => Code::IndexAlreadyExists, Error::IndexAlreadyExists(_) => Code::IndexAlreadyExists,
Error::SwapDuplicateIndexesFound(_) => Code::DuplicateIndexFound, Error::SwapDuplicateIndexesFound(_) => Code::DuplicateIndexFound,
Error::SwapDuplicateIndexFound(_) => Code::DuplicateIndexFound, Error::SwapDuplicateIndexFound(_) => Code::DuplicateIndexFound,
Error::InvalidTaskDate { .. } => Code::InvalidTaskDate, Error::InvalidTaskDate { .. } => Code::InvalidTaskDateFilter,
Error::InvalidTaskUids { .. } => Code::InvalidTaskUids, Error::InvalidTaskUids { .. } => Code::InvalidTaskUidsFilter,
Error::InvalidTaskStatuses { .. } => Code::InvalidTaskStatuses, Error::InvalidTaskStatuses { .. } => Code::InvalidTaskStatusesFilter,
Error::InvalidTaskTypes { .. } => Code::InvalidTaskTypes, Error::InvalidTaskTypes { .. } => Code::InvalidTaskTypesFilter,
Error::InvalidTaskCanceledBy { .. } => Code::InvalidTaskCanceledBy, Error::InvalidTaskCanceledBy { .. } => Code::InvalidTaskCanceledByFilter,
Error::InvalidIndexUid { .. } => Code::InvalidIndexUid, Error::InvalidIndexUid { .. } => Code::InvalidIndexUid,
Error::TaskNotFound(_) => Code::TaskNotFound, Error::TaskNotFound(_) => Code::TaskNotFound,
Error::TaskDeletionWithEmptyQuery => Code::TaskDeletionWithEmptyQuery, Error::TaskDeletionWithEmptyQuery => Code::TaskDeletionWithEmptyQuery,

View File

@ -467,11 +467,18 @@ pub struct SearchAggregator {
finite_pagination: usize, finite_pagination: usize,
// formatting // formatting
max_attributes_to_retrieve: usize,
max_attributes_to_highlight: usize,
highlight_pre_tag: bool, highlight_pre_tag: bool,
highlight_post_tag: bool, highlight_post_tag: bool,
max_attributes_to_crop: usize,
crop_marker: bool, crop_marker: bool,
show_matches_position: bool, show_matches_position: bool,
crop_length: bool, crop_length: bool,
// facets
facets_sum_of_terms: usize,
facets_total_number_of_facets: usize,
} }
impl SearchAggregator { impl SearchAggregator {
@ -552,16 +559,19 @@ impl SearchAggregator {
for user_agent in other.user_agents.into_iter() { for user_agent in other.user_agents.into_iter() {
self.user_agents.insert(user_agent); self.user_agents.insert(user_agent);
} }
// request // request
self.total_received = self.total_received.saturating_add(other.total_received); self.total_received = self.total_received.saturating_add(other.total_received);
self.total_succeeded = self.total_succeeded.saturating_add(other.total_succeeded); self.total_succeeded = self.total_succeeded.saturating_add(other.total_succeeded);
self.time_spent.append(&mut other.time_spent); self.time_spent.append(&mut other.time_spent);
// sort // sort
self.sort_with_geo_point |= other.sort_with_geo_point; self.sort_with_geo_point |= other.sort_with_geo_point;
self.sort_sum_of_criteria_terms = self.sort_sum_of_criteria_terms =
self.sort_sum_of_criteria_terms.saturating_add(other.sort_sum_of_criteria_terms); self.sort_sum_of_criteria_terms.saturating_add(other.sort_sum_of_criteria_terms);
self.sort_total_number_of_criteria = self.sort_total_number_of_criteria =
self.sort_total_number_of_criteria.saturating_add(other.sort_total_number_of_criteria); self.sort_total_number_of_criteria.saturating_add(other.sort_total_number_of_criteria);
// filter // filter
self.filter_with_geo_radius |= other.filter_with_geo_radius; self.filter_with_geo_radius |= other.filter_with_geo_radius;
self.filter_sum_of_criteria_terms = self.filter_sum_of_criteria_terms =
@ -576,20 +586,34 @@ impl SearchAggregator {
// q // q
self.max_terms_number = self.max_terms_number.max(other.max_terms_number); self.max_terms_number = self.max_terms_number.max(other.max_terms_number);
for (key, value) in other.matching_strategy.into_iter() {
let matching_strategy = self.matching_strategy.entry(key).or_insert(0);
*matching_strategy = matching_strategy.saturating_add(value);
}
// pagination // pagination
self.max_limit = self.max_limit.max(other.max_limit); self.max_limit = self.max_limit.max(other.max_limit);
self.max_offset = self.max_offset.max(other.max_offset); self.max_offset = self.max_offset.max(other.max_offset);
self.finite_pagination += other.finite_pagination; self.finite_pagination += other.finite_pagination;
// formatting
self.max_attributes_to_retrieve =
self.max_attributes_to_retrieve.max(other.max_attributes_to_retrieve);
self.max_attributes_to_highlight =
self.max_attributes_to_highlight.max(other.max_attributes_to_highlight);
self.highlight_pre_tag |= other.highlight_pre_tag; self.highlight_pre_tag |= other.highlight_pre_tag;
self.highlight_post_tag |= other.highlight_post_tag; self.highlight_post_tag |= other.highlight_post_tag;
self.max_attributes_to_crop = self.max_attributes_to_crop.max(other.max_attributes_to_crop);
self.crop_marker |= other.crop_marker; self.crop_marker |= other.crop_marker;
self.show_matches_position |= other.show_matches_position; self.show_matches_position |= other.show_matches_position;
self.crop_length |= other.crop_length; self.crop_length |= other.crop_length;
// facets
self.facets_sum_of_terms =
self.facets_sum_of_terms.saturating_add(other.facets_sum_of_terms);
self.facets_total_number_of_facets =
self.facets_total_number_of_facets.saturating_add(other.facets_total_number_of_facets);
// matching strategy
for (key, value) in other.matching_strategy.into_iter() {
let matching_strategy = self.matching_strategy.entry(key).or_insert(0);
*matching_strategy = matching_strategy.saturating_add(value);
}
} }
pub fn into_event(self, user: &User, event_name: &str) -> Option<Track> { pub fn into_event(self, user: &User, event_name: &str) -> Option<Track> {
@ -622,7 +646,6 @@ impl SearchAggregator {
}, },
"q": { "q": {
"max_terms_number": self.max_terms_number, "max_terms_number": self.max_terms_number,
"most_used_matching_strategy": self.matching_strategy.iter().max_by_key(|(_, v)| *v).map(|(k, _)| json!(k)).unwrap_or_else(|| json!(null)),
}, },
"pagination": { "pagination": {
"max_limit": self.max_limit, "max_limit": self.max_limit,
@ -630,12 +653,21 @@ impl SearchAggregator {
"finite_pagination": self.finite_pagination > self.total_received / 2, "finite_pagination": self.finite_pagination > self.total_received / 2,
}, },
"formatting": { "formatting": {
"max_attributes_to_retrieve": self.max_attributes_to_retrieve,
"max_attributes_to_highlight": self.max_attributes_to_highlight,
"highlight_pre_tag": self.highlight_pre_tag, "highlight_pre_tag": self.highlight_pre_tag,
"highlight_post_tag": self.highlight_post_tag, "highlight_post_tag": self.highlight_post_tag,
"max_attributes_to_crop": self.max_attributes_to_crop,
"crop_marker": self.crop_marker, "crop_marker": self.crop_marker,
"show_matches_position": self.show_matches_position, "show_matches_position": self.show_matches_position,
"crop_length": self.crop_length, "crop_length": self.crop_length,
}, },
"facets": {
"avg_facets_number": format!("{:.2}", self.facets_sum_of_terms as f64 / self.facets_total_number_of_facets as f64),
},
"matching_strategy": {
"most_used_strategy": self.matching_strategy.iter().max_by_key(|(_, v)| *v).map(|(k, _)| json!(k)).unwrap_or_else(|| json!(null)),
}
}); });
Some(Track { Some(Track {

View File

@ -110,13 +110,13 @@ impl Index<'_> {
self.service.get(url).await self.service.get(url).await
} }
pub async fn filtered_tasks(&self, type_: &[&str], status: &[&str]) -> (Value, StatusCode) { pub async fn filtered_tasks(&self, types: &[&str], statuses: &[&str]) -> (Value, StatusCode) {
let mut url = format!("/tasks?indexUids={}", self.uid); let mut url = format!("/tasks?indexUids={}", self.uid);
if !type_.is_empty() { if !types.is_empty() {
let _ = write!(url, "&types={}", type_.join(",")); let _ = write!(url, "&types={}", types.join(","));
} }
if !status.is_empty() { if !statuses.is_empty() {
let _ = write!(url, "&statuses={}", status.join(",")); let _ = write!(url, "&statuses={}", statuses.join(","));
} }
self.service.get(url).await self.service.get(url).await
} }

View File

@ -132,6 +132,10 @@ impl Server {
self.service.get("/tasks").await self.service.get("/tasks").await
} }
pub async fn tasks_filter(&self, filter: Value) -> (Value, StatusCode) {
self.service.get(format!("/tasks?{}", yaup::to_string(&filter).unwrap())).await
}
pub async fn get_dump_status(&self, uid: &str) -> (Value, StatusCode) { pub async fn get_dump_status(&self, uid: &str) -> (Value, StatusCode) {
self.service.get(format!("/dumps/{}/status", uid)).await self.service.get(format!("/dumps/{}/status", uid)).await
} }
@ -144,13 +148,13 @@ impl Server {
self.service.post("/swap-indexes", value).await self.service.post("/swap-indexes", value).await
} }
pub async fn cancel_task(&self, value: Value) -> (Value, StatusCode) { pub async fn cancel_tasks(&self, value: Value) -> (Value, StatusCode) {
self.service self.service
.post(format!("/tasks/cancel?{}", yaup::to_string(&value).unwrap()), json!(null)) .post(format!("/tasks/cancel?{}", yaup::to_string(&value).unwrap()), json!(null))
.await .await
} }
pub async fn delete_task(&self, value: Value) -> (Value, StatusCode) { pub async fn delete_tasks(&self, value: Value) -> (Value, StatusCode) {
self.service.delete(format!("/tasks?{}", yaup::to_string(&value).unwrap())).await self.service.delete(format!("/tasks?{}", yaup::to_string(&value).unwrap())).await
} }

View File

@ -1,4 +1,4 @@
use meili_snap::insta::assert_json_snapshot; use meili_snap::insta::{self, assert_json_snapshot};
use serde_json::json; use serde_json::json;
use time::format_description::well_known::Rfc3339; use time::format_description::well_known::Rfc3339;
use time::OffsetDateTime; use time::OffsetDateTime;
@ -173,6 +173,131 @@ async fn list_tasks_status_and_type_filtered() {
assert_eq!(response["results"].as_array().unwrap().len(), 2); assert_eq!(response["results"].as_array().unwrap().len(), 2);
} }
#[actix_rt::test]
async fn get_task_filter_error() {
let server = Server::new().await;
let (response, code) = server.tasks_filter(json!( { "lol": "pied" })).await;
assert_eq!(code, 400, "{}", response);
insta::assert_json_snapshot!(response, @r###"
{
"message": "Query deserialize error: unknown field `lol`",
"code": "bad_request",
"type": "invalid_request",
"link": "https://docs.meilisearch.com/errors#bad_request"
}
"###);
let (response, code) = server.tasks_filter(json!( { "uids": "pied" })).await;
assert_eq!(code, 400, "{}", response);
insta::assert_json_snapshot!(response, @r###"
{
"message": "Task uid `pied` is invalid. It should only contain numeric characters.",
"code": "invalid_task_uids_filter",
"type": "invalid_request",
"link": "https://docs.meilisearch.com/errors#invalid_task_uids_filter"
}
"###);
let (response, code) = server.tasks_filter(json!( { "from": "pied" })).await;
assert_eq!(code, 400, "{}", response);
insta::assert_json_snapshot!(response, @r###"
{
"message": "Query deserialize error: invalid digit found in string",
"code": "bad_request",
"type": "invalid_request",
"link": "https://docs.meilisearch.com/errors#bad_request"
}
"###);
let (response, code) = server.tasks_filter(json!( { "beforeStartedAt": "pied" })).await;
assert_eq!(code, 400, "{}", response);
insta::assert_json_snapshot!(response, @r###"
{
"message": "Task `beforeStartedAt` `pied` is invalid. It should follow the YYYY-MM-DD or RFC 3339 date-time format.",
"code": "invalid_task_date_filter",
"type": "invalid_request",
"link": "https://docs.meilisearch.com/errors#invalid_task_date_filter"
}
"###);
}
#[actix_rt::test]
async fn delete_task_filter_error() {
let server = Server::new().await;
let (response, code) = server.delete_tasks(json!(null)).await;
assert_eq!(code, 400, "{}", response);
insta::assert_json_snapshot!(response, @r###"
{
"message": "Query parameters to filter the tasks to delete are missing. Available query parameters are: `uids`, `indexUids`, `statuses`, `types`, `beforeEnqueuedAt`, `afterEnqueuedAt`, `beforeStartedAt`, `afterStartedAt`, `beforeFinishedAt`, `afterFinishedAt`.",
"code": "missing_task_filters",
"type": "invalid_request",
"link": "https://docs.meilisearch.com/errors#missing_task_filters"
}
"###);
let (response, code) = server.delete_tasks(json!({ "lol": "pied" })).await;
assert_eq!(code, 400, "{}", response);
insta::assert_json_snapshot!(response, @r###"
{
"message": "Query deserialize error: unknown field `lol`",
"code": "bad_request",
"type": "invalid_request",
"link": "https://docs.meilisearch.com/errors#bad_request"
}
"###);
let (response, code) = server.delete_tasks(json!({ "uids": "pied" })).await;
assert_eq!(code, 400, "{}", response);
insta::assert_json_snapshot!(response, @r###"
{
"message": "Task uid `pied` is invalid. It should only contain numeric characters.",
"code": "invalid_task_uids_filter",
"type": "invalid_request",
"link": "https://docs.meilisearch.com/errors#invalid_task_uids_filter"
}
"###);
}
#[actix_rt::test]
async fn cancel_task_filter_error() {
let server = Server::new().await;
let (response, code) = server.cancel_tasks(json!(null)).await;
assert_eq!(code, 400, "{}", response);
insta::assert_json_snapshot!(response, @r###"
{
"message": "Query parameters to filter the tasks to cancel are missing. Available query parameters are: `uids`, `indexUids`, `statuses`, `types`, `beforeEnqueuedAt`, `afterEnqueuedAt`, `beforeStartedAt`, `afterStartedAt`, `beforeFinishedAt`, `afterFinishedAt`.",
"code": "missing_task_filters",
"type": "invalid_request",
"link": "https://docs.meilisearch.com/errors#missing_task_filters"
}
"###);
let (response, code) = server.cancel_tasks(json!({ "lol": "pied" })).await;
assert_eq!(code, 400, "{}", response);
insta::assert_json_snapshot!(response, @r###"
{
"message": "Query deserialize error: unknown field `lol`",
"code": "bad_request",
"type": "invalid_request",
"link": "https://docs.meilisearch.com/errors#bad_request"
}
"###);
let (response, code) = server.cancel_tasks(json!({ "uids": "pied" })).await;
assert_eq!(code, 400, "{}", response);
insta::assert_json_snapshot!(response, @r###"
{
"message": "Task uid `pied` is invalid. It should only contain numeric characters.",
"code": "invalid_task_uids_filter",
"type": "invalid_request",
"link": "https://docs.meilisearch.com/errors#invalid_task_uids_filter"
}
"###);
}
macro_rules! assert_valid_summarized_task { macro_rules! assert_valid_summarized_task {
($response:expr, $task_type:literal, $index:literal) => {{ ($response:expr, $task_type:literal, $index:literal) => {{
assert_eq!($response.as_object().unwrap().len(), 5); assert_eq!($response.as_object().unwrap().len(), 5);
@ -789,7 +914,7 @@ async fn test_summarized_task_cancelation() {
// to avoid being flaky we're only going to cancel an already finished task :( // to avoid being flaky we're only going to cancel an already finished task :(
index.create(None).await; index.create(None).await;
index.wait_task(0).await; index.wait_task(0).await;
server.cancel_task(json!({ "uids": [0] })).await; server.cancel_tasks(json!({ "uids": [0] })).await;
index.wait_task(1).await; index.wait_task(1).await;
let (task, _) = index.get_task(1).await; let (task, _) = index.get_task(1).await;
assert_json_snapshot!(task, assert_json_snapshot!(task,
@ -822,7 +947,7 @@ async fn test_summarized_task_deletion() {
// to avoid being flaky we're only going to delete an already finished task :( // to avoid being flaky we're only going to delete an already finished task :(
index.create(None).await; index.create(None).await;
index.wait_task(0).await; index.wait_task(0).await;
server.delete_task(json!({ "uids": [0] })).await; server.delete_tasks(json!({ "uids": [0] })).await;
index.wait_task(1).await; index.wait_task(1).await;
let (task, _) = index.get_task(1).await; let (task, _) = index.get_task(1).await;
assert_json_snapshot!(task, assert_json_snapshot!(task,

View File

@ -147,11 +147,11 @@ pub enum Code {
MissingMasterKey, MissingMasterKey,
NoSpaceLeftOnDevice, NoSpaceLeftOnDevice,
DumpNotFound, DumpNotFound,
InvalidTaskDate, InvalidTaskDateFilter,
InvalidTaskStatuses, InvalidTaskStatusesFilter,
InvalidTaskTypes, InvalidTaskTypesFilter,
InvalidTaskCanceledBy, InvalidTaskCanceledByFilter,
InvalidTaskUids, InvalidTaskUidsFilter,
TaskNotFound, TaskNotFound,
TaskDeletionWithEmptyQuery, TaskDeletionWithEmptyQuery,
TaskCancelationWithEmptyQuery, TaskCancelationWithEmptyQuery,
@ -243,19 +243,19 @@ impl Code {
MissingMasterKey => { MissingMasterKey => {
ErrCode::authentication("missing_master_key", StatusCode::UNAUTHORIZED) ErrCode::authentication("missing_master_key", StatusCode::UNAUTHORIZED)
} }
InvalidTaskDate => { InvalidTaskDateFilter => {
ErrCode::invalid("invalid_task_date_filter", StatusCode::BAD_REQUEST) ErrCode::invalid("invalid_task_date_filter", StatusCode::BAD_REQUEST)
} }
InvalidTaskUids => { InvalidTaskUidsFilter => {
ErrCode::invalid("invalid_task_uids_filter", StatusCode::BAD_REQUEST) ErrCode::invalid("invalid_task_uids_filter", StatusCode::BAD_REQUEST)
} }
InvalidTaskStatuses => { InvalidTaskStatusesFilter => {
ErrCode::invalid("invalid_task_statuses_filter", StatusCode::BAD_REQUEST) ErrCode::invalid("invalid_task_statuses_filter", StatusCode::BAD_REQUEST)
} }
InvalidTaskTypes => { InvalidTaskTypesFilter => {
ErrCode::invalid("invalid_task_types_filter", StatusCode::BAD_REQUEST) ErrCode::invalid("invalid_task_types_filter", StatusCode::BAD_REQUEST)
} }
InvalidTaskCanceledBy => { InvalidTaskCanceledByFilter => {
ErrCode::invalid("invalid_task_canceled_by_filter", StatusCode::BAD_REQUEST) ErrCode::invalid("invalid_task_canceled_by_filter", StatusCode::BAD_REQUEST)
} }
TaskNotFound => ErrCode::invalid("task_not_found", StatusCode::NOT_FOUND), TaskNotFound => ErrCode::invalid("task_not_found", StatusCode::NOT_FOUND),