Merge pull request #2728 from influxdata/ntran/response

feat: handle delete responses

generated_types/protos/influxdata/iox/management/v1/service.proto

@@ -494,5 +494,4 @@ message DeleteRequest {
 }
 
 message DeleteResponse {
-  // NGA todo: define an appropriate response
 }

influxdb_iox_client/src/client/management.rs

@@ -1057,7 +1057,6 @@ impl Client {
                 _ => DeleteError::ServerError(status),
             })?;
 
-        // NGA todo: return a handle to the delete?
         Ok(())
     }
 

predicate/src/delete_predicate.rs

@@ -649,7 +649,6 @@ mod tests {
         assert_eq!(expected, result);
     }
 
-    // NGA todo: check content of error messages
     #[test]
     fn test_parse_delete_negative() {
         // invalid key

src/influxdb_ioxd/http.rs

@@ -282,7 +282,7 @@ impl ApplicationError {
             Self::ParsingLineProtocol { .. } => self.bad_request(),
             Self::ParsingDelete { .. } => self.bad_request(),
             Self::BuildingDeletePredicate { .. } => self.bad_request(),
-            Self::ExecutingDelete { .. } => self.internal_error(),
+            Self::ExecutingDelete { .. } => self.bad_request(),
             Self::ReadingBodyAsGzip { .. } => self.bad_request(),
             Self::ClientHangup { .. } => self.bad_request(),
             Self::RouteNotFound { .. } => self.not_found(),
@@ -1150,6 +1150,44 @@ mod tests {
             "+----------------+--------------+-------+-----------------+----------------------+",
         ];
         assert_batches_eq!(expected, &batches);
+
+        // -------------------
+        // negative tests
+        // Not able to parse _measurement="not_a_table"  (it must be _measurement=\"not_a_table\" to work)
+        let delete_line = r#"{"start":"2021-04-01T14:00:00Z","stop":"2021-04-02T14:00:00Z", "predicate":"_measurement="not_a_table" and location=Boston"}"#;
+        let response = client
+            .post(&format!(
+                "{}/api/v2/delete?bucket={}&org={}",
+                server_url, bucket_name, org_name
+            ))
+            .body(delete_line)
+            .send()
+            .await;
+        check_response(
+            "delete",
+            response,
+            StatusCode::BAD_REQUEST,
+            Some("Unable to parse delete string"),
+        )
+        .await;
+
+        // delete from non-existing table
+        let delete_line = r#"{"start":"2021-04-01T14:00:00Z","stop":"2021-04-02T14:00:00Z", "predicate":"_measurement=not_a_table and location=Boston"}"#;
+        let response = client
+            .post(&format!(
+                "{}/api/v2/delete?bucket={}&org={}",
+                server_url, bucket_name, org_name
+            ))
+            .body(delete_line)
+            .send()
+            .await;
+        check_response(
+            "delete",
+            response,
+            StatusCode::BAD_REQUEST,
+            Some("Cannot delete data from non-existing table"),
+        )
+        .await;
     }
 
     #[tokio::test]
@@ -1607,7 +1645,7 @@ mod tests {
 
             assert_eq!(status, expected_status);
             if let Some(expected_body) = expected_body {
-                assert_eq!(body, expected_body);
+                assert!(body.contains(expected_body));
             }
         } else {
             panic!("Unexpected error response: {:?}", response);

src/influxdb_ioxd/rpc/error.rs

@@ -177,6 +177,12 @@ pub fn default_db_error_handler(error: server::db::Error) -> tonic::Status {
             ),
         }
         .into(),
+        Error::DeleteFromTable { source, table_name } => PreconditionViolation {
+            category: "database".to_string(),
+            subject: "influxdata.com/iox".to_string(),
+            description: format!("Cannot delete data from table: {} : {}", table_name, source),
+        }
+        .into(),
         Error::CatalogError { source } => default_catalog_error_handler(source),
         error => {
             error!(?error, "Unexpected error");

src/influxdb_ioxd/rpc/management.rs

@@ -633,14 +633,13 @@ where
                 }))
             }
             Ok(del_predicate) => {
-                //execute delete
+                // execute delete
                 db.delete(&table_name, Arc::new(del_predicate))
                     .await
                     .map_err(default_db_error_handler)?;
             }
         }
 
-        // NGA todo: return a delete handle with the response?
         Ok(Response::new(DeleteResponse {}))
     }
 }

tests/end_to_end_cases/management_api.rs

@@ -1587,8 +1587,10 @@ async fn test_delete() {
     let pred = "region = west";
     let del = management_client
         .delete(db_name.clone(), table, start, stop, pred)
-        .await;
-    assert!(del.is_err());
+        .await
+        .unwrap_err()
+        .to_string();
+    assert!(del.contains("Cannot delete data from table"));
 
     // Verify both existing tables still have the same data
     // query to verify data deleted