HADOOP-19446. ABFS: [FnsOverBlob][Tests] Add Tests For Negative Scenarios Identified for Delete Operation (#7376)

Contributed by Manika Joshi
Reviewed by Anuj Modi, Anmol Asrani, Manish Bhatt
Signed off by Anuj Modi<anujmodi@apache.org>

hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsBlobClient.java

@@ -1517,8 +1517,24 @@ public class AbfsBlobClient extends AbfsClient {
     final AbfsRestOperation op = getAbfsRestOperation(
         AbfsRestOperationType.DeleteBlob, HTTP_METHOD_DELETE, url,
         requestHeaders);
-    op.execute(tracingContext);
-    return op;
+    try {
+      op.execute(tracingContext);
+      return op;
+    } catch (AzureBlobFileSystemException e) {
+      // If we have no HTTP response, throw the original exception.
+      if (!op.hasResult()) {
+        throw e;
+      }
+      final AbfsRestOperation idempotencyOp = deleteIdempotencyCheckOp(op);
+      if (idempotencyOp.getResult().getStatusCode()
+          == op.getResult().getStatusCode()) {
+        // idempotency did not return different result
+        // throw back the exception
+        throw e;
+      } else {
+        return idempotencyOp;
+      }
+    }
   }
 
   /**

hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/ITestAzureBlobFileSystemDelete.java

@@ -34,6 +34,7 @@ import org.junit.Test;
 import org.mockito.Mockito;
 
 import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.fs.FileAlreadyExistsException;
 import org.apache.hadoop.fs.FileStatus;
 import org.apache.hadoop.fs.FileSystem;
 import org.apache.hadoop.fs.Path;
@@ -55,6 +56,7 @@ import org.apache.hadoop.test.ReflectionUtils;
 
 import static java.net.HttpURLConnection.HTTP_BAD_REQUEST;
 import static java.net.HttpURLConnection.HTTP_FORBIDDEN;
+import static java.net.HttpURLConnection.HTTP_GATEWAY_TIMEOUT;
 import static java.net.HttpURLConnection.HTTP_NOT_FOUND;
 import static java.net.HttpURLConnection.HTTP_OK;
 import static org.apache.hadoop.fs.azurebfs.constants.AbfsHttpConstants.HTTP_METHOD_DELETE;
@@ -196,6 +198,22 @@ public class ITestAzureBlobFileSystemDelete extends
     when(op.isARetriedRequest()).thenReturn(true);
 
     // Case 1: Mock instance of Http Operation response. This will return
+    // HTTP:TIMEOUT
+    AbfsHttpOperation http504Op = mock(AbfsHttpOperation.class);
+    when(http504Op.getStatusCode()).thenReturn(HTTP_GATEWAY_TIMEOUT);
+
+    // Mock delete response to 504
+    when(op.getResult()).thenReturn(http504Op);
+    when(op.hasResult()).thenReturn(true);
+
+    Assertions.assertThat(testClient.deleteIdempotencyCheckOp(op)
+        .getResult()
+        .getStatusCode())
+        .describedAs(
+            "Idempotency check to happen only for HTTP 404 response.")
+        .isEqualTo(HTTP_GATEWAY_TIMEOUT);
+
+    // Case 2: Mock instance of Http Operation response. This will return
     // HTTP:Not Found
     AbfsHttpOperation http404Op = mock(AbfsHttpOperation.class);
     when(http404Op.getStatusCode()).thenReturn(HTTP_NOT_FOUND);
@@ -211,7 +229,7 @@ public class ITestAzureBlobFileSystemDelete extends
             "Delete is considered idempotent by default and should return success.")
         .isEqualTo(HTTP_OK);
 
-    // Case 2: Mock instance of Http Operation response. This will return
+    // Case 3: Mock instance of Http Operation response. This will return
     // HTTP:Bad Request
     AbfsHttpOperation http400Op = mock(AbfsHttpOperation.class);
     when(http400Op.getStatusCode()).thenReturn(HTTP_BAD_REQUEST);
@@ -344,6 +362,123 @@ public class ITestAzureBlobFileSystemDelete extends
     fs.close();
   }
 
+  /**
+   * Test the deletion of file in an implicit directory.
+   *
+   * @throws Exception if an error occurs during the test execution
+   */
+  @Test
+  public void testDeleteFileInImplicitDir() throws Exception {
+    AzureBlobFileSystem fs = getFileSystem();
+    assumeBlobServiceType();
+
+    Path file1 = new Path("/testDir/dir1/file1");
+    Path file2 = new Path("/testDir/dir1/file2");
+    Path implicitDir = file1.getParent();
+
+    createAzCopyFile(file1);
+    createAzCopyFile(file2);
+
+    // Deletion of file with different recursion values
+    fs.delete(file1, false);
+    fs.delete(file2, true);
+
+    Assertions.assertThat(fs.exists(implicitDir))
+        .describedAs("The directory should exist.")
+        .isTrue();
+    Assertions.assertThat(fs.exists(file1))
+        .describedAs("Deleted file should not be present.").isFalse();
+    Assertions.assertThat(fs.exists(file2))
+        .describedAs("Deleted file should not be present.").isFalse();
+    Assertions.assertThat(fs.exists(implicitDir))
+        .describedAs("The parent dir should exist.")
+        .isTrue();
+  }
+
+  /**
+   * Test that the file status of an empty explicit dir
+   * should not exist after its deletion.
+   *
+   * @throws Exception if an error occurs during the test execution
+   */
+  @Test
+  public void testDeleteEmptyExplicitDir() throws Exception {
+    AzureBlobFileSystem fs = getFileSystem();
+
+    Path p1 = new Path("/testDir1/");
+
+    fs.mkdirs(p1);
+    fs.delete(p1, false);
+
+    Assertions.assertThat(fs.exists(p1))
+        .describedAs("The deleted directory should not exist.")
+        .isFalse();
+  }
+
+  /**
+   * Test that deleting a non-empty explicit directory
+   * can only be done with the recursive flag set to true.
+   *
+   * @throws Exception if an error occurs during the test execution
+   */
+  @Test
+  public void testDeleteNonEmptyExplicitDir() throws Exception {
+    AzureBlobFileSystem fs = getFileSystem();
+
+    Path p1 = new Path("/testDir1");
+    Path p2 = new Path("/testDir2");
+
+    fs.mkdirs(p1);
+    fs.mkdirs(p2);
+    fs.create(new Path("/testDir1/f1.txt"));
+    fs.create(new Path("/testDir2/f2.txt"));
+
+    fs.delete(p1, true);
+
+    //Deleting non-empty dir with recursion set as
+    // false returns a FileAlreadyExistsException: 409-DirectoryNotEmpty
+    intercept(FileAlreadyExistsException.class,
+        () -> fs.delete(p2, false));
+
+    Assertions.assertThat(!fs.exists(p1))
+        .describedAs("FileStatus of the deleted directory should not exist.")
+        .isTrue();
+  }
+
+  /**
+   * Assert that deleting a non-existing path
+   * returns a false.
+   *
+   * @throws Exception if an error occurs during the test execution
+   */
+  @Test
+  public void testDeleteNonExistingPath() throws Exception {
+    AzureBlobFileSystem fs = getFileSystem();
+
+    Path p = new Path("/nonExistingPath");
+    Assertions.assertThat(fs.delete(p, true))
+        .describedAs("Delete operation on non-existing path should return false")
+        .isFalse();
+  }
+
+  /**
+   * Test to check test operation returns false
+   * after the file has already been deleted.
+   *
+   * @throws Exception if an error occurs during the test execution
+   */
+  @Test
+  public void testExceptionForDeletedFile() throws Exception {
+    final AzureBlobFileSystem fs = getFileSystem();
+    Path testFile = path("/testFile");
+    fs.create(testFile);
+    fs.delete(testFile, false);
+
+    Assertions.assertThat(fs.delete(testFile, true))
+        .describedAs("Delete operation on deleted path should return false.")
+        .isFalse();
+  }
+
   /**
    * Tests deleting an implicit directory and its contents. The test verifies that after deletion,
    * both the directory and its child file no longer exist.
@@ -359,6 +494,11 @@ public class ITestAzureBlobFileSystemDelete extends
     AbfsBlobClient client = (AbfsBlobClient) fs.getAbfsClient();
     client.deleteBlobPath(new Path("/testDir/dir1"),
             null, getTestTracingContext(fs, true));
+
+    //Deleting non-empty dir with recursion set as
+    // false returns a FileAlreadyExistsException: 409-DirectoryNotEmpty
+    intercept(FileAlreadyExistsException.class,
+        () -> fs.delete(new Path("/testDir/dir1"), false));
     fs.delete(new Path("/testDir/dir1"), true);
     Assertions.assertThat(!fs.exists(new Path("/testDir/dir1")))
             .describedAs("FileStatus of the deleted directory should not exist")