create and use the error code

meilisearch-types/src/error.rs

@@ -218,6 +218,7 @@ InvalidDocumentGeoField               , InvalidRequest       , BAD_REQUEST ;
 InvalidDocumentId                     , InvalidRequest       , BAD_REQUEST ;
 InvalidDocumentLimit                  , InvalidRequest       , BAD_REQUEST ;
 InvalidDocumentOffset                 , InvalidRequest       , BAD_REQUEST ;
+InvalidDocumentDeleteFilter           , InvalidRequest       , BAD_REQUEST ;
 InvalidIndexLimit                     , InvalidRequest       , BAD_REQUEST ;
 InvalidIndexOffset                    , InvalidRequest       , BAD_REQUEST ;
 InvalidIndexPrimaryKey                , InvalidRequest       , BAD_REQUEST ;

meilisearch/src/analytics/mod.rs

@@ -64,6 +64,7 @@ pub enum DocumentDeletionKind {
     PerDocumentId,
     ClearAll,
     PerBatch,
+    PerFilter,
 }
 
 pub trait Analytics: Sync + Send {

meilisearch/src/error.rs

@@ -20,6 +20,8 @@ pub enum MeilisearchHttpError {
     InvalidContentType(String, Vec<String>),
     #[error("Document `{0}` not found.")]
     DocumentNotFound(String),
+    #[error("Sending an empty filter is forbidden.")]
+    EmptyFilter,
     #[error("Invalid syntax for the filter parameter: `expected {}, found: {1}`.", .0.join(", "))]
     InvalidExpression(&'static [&'static str], Value),
     #[error("A {0} payload is missing.")]
@@ -58,6 +60,7 @@ impl ErrorCode for MeilisearchHttpError {
             MeilisearchHttpError::MissingPayload(_) => Code::MissingPayload,
             MeilisearchHttpError::InvalidContentType(_, _) => Code::InvalidContentType,
             MeilisearchHttpError::DocumentNotFound(_) => Code::DocumentNotFound,
+            MeilisearchHttpError::EmptyFilter => Code::InvalidDocumentDeleteFilter,
             MeilisearchHttpError::InvalidExpression(_, _) => Code::InvalidSearchFilter,
             MeilisearchHttpError::PayloadTooLarge => Code::PayloadTooLarge,
             MeilisearchHttpError::SwapIndexPayloadWrongLength(_) => Code::InvalidSwapIndexes,

meilisearch/src/routes/indexes/documents.rs

@@ -405,8 +405,7 @@ pub async fn delete_documents_batch(
 #[derive(Debug, Deserr)]
 #[deserr(error = DeserrJsonError, rename_all = camelCase, deny_unknown_fields)]
 pub struct DocumentDeletionByFilter {
-    // TODO: Update the error code to something more appropriate
-    #[deserr(error = DeserrJsonError<InvalidDocumentOffset>)]
+    #[deserr(error = DeserrJsonError<InvalidDocumentDeleteFilter>)]
     filter: Value,
 }
 
@@ -417,28 +416,26 @@ pub async fn delete_documents_by_filter(
     req: HttpRequest,
     analytics: web::Data<dyn Analytics>,
 ) -> Result<HttpResponse, ResponseError> {
-    println!("here");
     debug!("called with params: {:?}", body);
     let index_uid = IndexUid::try_from(index_uid.into_inner())?;
     let index_uid = index_uid.into_inner();
     let filter = body.into_inner().filter;
 
-    analytics.delete_documents(DocumentDeletionKind::PerBatch, &req);
-
-    debug!("filter: {:?}", filter);
-
-    // FIXME: spawn_blocking => tamo: but why, it's making zero IO and almost no allocation?
-    // TODO: what should we do in case of an empty filter? Create a task that does nothing (then we should be able to store a None in the task queue)
-    // or refuse the payload with a cool™️  error message 😎
-    let _ = crate::search::parse_filter(&filter)?.expect("You can't send an empty filter");
+    analytics.delete_documents(DocumentDeletionKind::PerFilter, &req);
 
+    // we ensure the filter is well formed before enqueuing it
+    || -> Result<_, ResponseError> {
+        Ok(crate::search::parse_filter(&filter)?.ok_or(MeilisearchHttpError::EmptyFilter)?)
+    }()
+    // and whatever was the error, the error code should always be an InvalidDocumentDeleteFilter
+    .map_err(|err| ResponseError::from_msg(err.message, Code::InvalidDocumentDeleteFilter))?;
     let task = KindWithContent::DocumentDeletionByFilter { index_uid, filter_expr: filter };
 
     let task: SummarizedTaskView =
         tokio::task::spawn_blocking(move || index_scheduler.register(task)).await??.into();
 
     debug!("returns: {:?}", task);
-    return Ok(HttpResponse::Accepted().json(task));
+    Ok(HttpResponse::Accepted().json(task))
 }
 
 pub async fn clear_all_documents(

meilisearch/tests/documents/delete_documents.rs

@@ -156,7 +156,7 @@ async fn delete_document_by_filter() {
     index.wait_task(1).await;
     let (response, code) =
         index.delete_document_by_filter(json!({ "filter": "color = blue"})).await;
-    // snapshot!(code, @"202 Accepted");
+    snapshot!(code, @"202 Accepted");
     snapshot!(json_string!(response, { ".enqueuedAt" => "[date]" }), @r###"
     {
       "taskUid": 2,
@@ -247,14 +247,11 @@ async fn delete_document_by_filter() {
         {
           "id": 0,
           "color": "red"
-        },
-        {
-          "id": 3
         }
       ],
       "offset": 0,
       "limit": 20,
-      "total": 2
+      "total": 1
     }
     "###);
 }

meilisearch/tests/documents/errors.rs

@@ -436,15 +436,39 @@ async fn delete_document_by_filter() {
     }
     "###);
 
+    // send bad payload type
+    let (response, code) = index.delete_document_by_filter(json!({ "filter": true })).await;
+    snapshot!(code, @"400 Bad Request");
+    snapshot!(json_string!(response), @r###"
+    {
+      "message": "Invalid syntax for the filter parameter: `expected String, Array, found: true`.",
+      "code": "invalid_document_delete_filter",
+      "type": "invalid_request",
+      "link": "https://docs.meilisearch.com/errors#invalid_document_delete_filter"
+    }
+    "###);
+
     // send bad filter
     let (response, code) = index.delete_document_by_filter(json!({ "filter": "hello"})).await;
     snapshot!(code, @"400 Bad Request");
     snapshot!(json_string!(response), @r###"
     {
       "message": "Was expecting an operation `=`, `!=`, `>=`, `>`, `<=`, `<`, `IN`, `NOT IN`, `TO`, `EXISTS`, `NOT EXISTS`, `_geoRadius`, or `_geoBoundingBox` at `hello`.\n1:6 hello",
-      "code": "invalid_search_filter",
+      "code": "invalid_document_delete_filter",
       "type": "invalid_request",
-      "link": "https://docs.meilisearch.com/errors#invalid_search_filter"
+      "link": "https://docs.meilisearch.com/errors#invalid_document_delete_filter"
+    }
+    "###);
+
+    // send empty filter
+    let (response, code) = index.delete_document_by_filter(json!({ "filter": ""})).await;
+    snapshot!(code, @"400 Bad Request");
+    snapshot!(json_string!(response), @r###"
+    {
+      "message": "Sending an empty filter is forbidden.",
+      "code": "invalid_document_delete_filter",
+      "type": "invalid_request",
+      "link": "https://docs.meilisearch.com/errors#invalid_document_delete_filter"
     }
     "###);