apacheGH-38703: [C++][FS][Azure] Implement DeleteFile() (apache#39840)

### Rationale for this change `DeleteFile()` API isn't implemented yet. ### What changes are included in this PR? Implement `DeleteFile()` by the "Delete Blob" API: https://learn.microsoft.com/en-us/rest/api/storageservices/delete-blob ### Are these changes tested? I tested the modification by creating a file via the web browser on our internal ADLS, then ran a sample program that deleted the file. I added three regression tests to cover the use case scenarios of: * A valid delete attempt, where "valid" means that the file exists and is indeed a file * An intentional failure where a file delete is attempted, but the file does not exist * An intentional failure where a file delete is attempted, but the target is a container * An intentional failure where a file delete is attempted, but the target is a directory ### Are there any user-facing changes? Yes. * Closes: apache#38703 Lead-authored-by: av8or1 <[email protected]> Co-authored-by: jerry.adair <[email protected]> Co-authored-by: Sutou Kouhei <[email protected]> Signed-off-by: Sutou Kouhei <[email protected]>
dgreiss · Feb 17, 2024 · 75f0a72 · 75f0a72
1 parent c33cb3f
commit 75f0a72
Show file tree

Hide file tree

Showing 2 changed files with 54 additions and 2 deletions.
diff --git a/cpp/src/arrow/filesystem/azurefs.cc b/cpp/src/arrow/filesystem/azurefs.cc
@@ -1690,6 +1690,26 @@ class AzureFileSystem::Impl {
     }
   }
 
+  Status DeleteFile(const AzureLocation& location) {
+    RETURN_NOT_OK(ValidateFileLocation(location));
+    auto file_client = datalake_service_client_->GetFileSystemClient(location.container)
+                           .GetFileClient(location.path);
+    try {
+      auto response = file_client.Delete();
+      // Only the "*IfExists" functions ever set Deleted to false.
+      // All the others either succeed or throw an exception.
+      DCHECK(response.Value.Deleted);
+    } catch (const Storage::StorageException& exception) {
+      if (exception.ErrorCode == "FilesystemNotFound" ||
+          exception.ErrorCode == "PathNotFound") {
+        return PathNotFound(location);
+      }
+      return ExceptionToStatus(exception, "Failed to delete a file: ", location.path,
+                               ": ", file_client.GetUrl());
+    }
+    return Status::OK();
+  }
+
   Status CopyFile(const AzureLocation& src, const AzureLocation& dest) {
     RETURN_NOT_OK(ValidateFileLocation(src));
     RETURN_NOT_OK(ValidateFileLocation(dest));
@@ -1875,7 +1895,8 @@ Status AzureFileSystem::DeleteRootDirContents() {
 }
 
 Status AzureFileSystem::DeleteFile(const std::string& path) {
-  return Status::NotImplemented("The Azure FileSystem is not fully implemented");
+  ARROW_ASSIGN_OR_RAISE(auto location, AzureLocation::FromString(path));
+  return impl_->DeleteFile(location);
 }
 
 Status AzureFileSystem::Move(const std::string& src, const std::string& dest) {

diff --git a/cpp/src/arrow/filesystem/azurefs_test.cc b/cpp/src/arrow/filesystem/azurefs_test.cc
@@ -1382,6 +1382,38 @@ TEST_F(TestAzuriteFileSystem, DeleteDirContentsFailureNonexistent) {
   this->TestDeleteDirContentsFailureNonexistent();
 }
 
+TEST_F(TestAzuriteFileSystem, DeleteFileSuccess) {
+  const auto container_name = PreexistingData::RandomContainerName(rng_);
+  ASSERT_OK(fs()->CreateDir(container_name));
+  const auto file_name = ConcatAbstractPath(container_name, "abc");
+  CreateFile(fs(), file_name, "data");
+  arrow::fs::AssertFileInfo(fs(), file_name, FileType::File);
+  ASSERT_OK(fs()->DeleteFile(file_name));
+  arrow::fs::AssertFileInfo(fs(), file_name, FileType::NotFound);
+}
+
+TEST_F(TestAzuriteFileSystem, DeleteFileFailureNonexistent) {
+  const auto container_name = PreexistingData::RandomContainerName(rng_);
+  ASSERT_OK(fs()->CreateDir(container_name));
+  const auto nonexistent_file_name = ConcatAbstractPath(container_name, "nonexistent");
+  ASSERT_RAISES(IOError, fs()->DeleteFile(nonexistent_file_name));
+}
+
+TEST_F(TestAzuriteFileSystem, DeleteFileFailureContainer) {
+  const auto container_name = PreexistingData::RandomContainerName(rng_);
+  ASSERT_OK(fs()->CreateDir(container_name));
+  arrow::fs::AssertFileInfo(fs(), container_name, FileType::Directory);
+  ASSERT_RAISES(IOError, fs()->DeleteFile(container_name));
+}
+
+TEST_F(TestAzuriteFileSystem, DeleteFileFailureDirectory) {
+  const auto directory_name =
+      ConcatAbstractPath(PreexistingData::RandomContainerName(rng_), "directory");
+  ASSERT_OK(fs()->CreateDir(directory_name));
+  arrow::fs::AssertFileInfo(fs(), directory_name, FileType::Directory);
+  ASSERT_RAISES(IOError, fs()->DeleteFile(directory_name));
+}
+
 TEST_F(TestAzuriteFileSystem, CopyFileSuccessDestinationNonexistent) {
   auto data = SetUpPreexistingData();
   const auto destination_path = data.ContainerPath("copy-destionation");
@@ -1868,6 +1900,5 @@ TEST_F(TestAzuriteFileSystem, OpenInputFileClosed) {
   ASSERT_RAISES(Invalid, stream->ReadAt(1, 1));
   ASSERT_RAISES(Invalid, stream->Seek(2));
 }
-
 }  // namespace fs
 }  // namespace arrow