From 7e25d1311f47783df62a4cbc6a3261bf3de5dacb Mon Sep 17 00:00:00 2001 From: Sutou Kouhei Date: Mon, 20 Nov 2023 18:18:58 +0900 Subject: [PATCH] GH-38700: [C++][FS][Azure] Implement `DeleteDir()` --- cpp/src/arrow/filesystem/azurefs.cc | 56 +++++++++++++++++++++++- cpp/src/arrow/filesystem/azurefs_test.cc | 35 +++++++++++++++ 2 files changed, 90 insertions(+), 1 deletion(-) diff --git a/cpp/src/arrow/filesystem/azurefs.cc b/cpp/src/arrow/filesystem/azurefs.cc index bd0e353e4a03a..6bc9e0222ac08 100644 --- a/cpp/src/arrow/filesystem/azurefs.cc +++ b/cpp/src/arrow/filesystem/azurefs.cc @@ -724,6 +724,59 @@ class AzureFileSystem::Impl { return Status::OK(); } + + Status DeleteDir(const AzureLocation& location) { + if (location.container.empty()) { + return Status::Invalid("Cannot delete an empty container"); + } + + if (location.path.empty()) { + auto container_client = + blob_service_client_->GetBlobContainerClient(location.container); + try { + auto response = container_client.Delete(); + if (response.Value.Deleted) { + return Status::OK(); + } else { + return StatusFromErrorResponse( + container_client.GetUrl(), response.RawResponse.get(), + "Failed to delete a container: " + location.container); + } + } catch (const Azure::Storage::StorageException& exception) { + return internal::ExceptionToStatus( + "Failed to delete a container: " + location.container + ": " + + container_client.GetUrl(), + exception); + } + } + + ARROW_ASSIGN_OR_RAISE(auto hierarchical_namespace_enabled, + hierarchical_namespace_.Enabled(location.container)); + if (!hierarchical_namespace_enabled) { + // Without hierarchical namespace enabled Azure blob storage has no directories. + // Therefore we can't, and don't need to delete one. + return Status::OK(); + } + + auto directory_client = + datalake_service_client_->GetFileSystemClient(location.container) + .GetDirectoryClient(location.path); + try { + auto response = directory_client.DeleteRecursive(); + if (response.Value.Deleted) { + return Status::OK(); + } else { + return StatusFromErrorResponse(directory_client.GetUrl(), + response.RawResponse.get(), + "Failed to delete a directory: " + location.path); + } + } catch (const Azure::Storage::StorageException& exception) { + return internal::ExceptionToStatus( + "Failed to delete a directory: " + location.path + ": " + + directory_client.GetUrl(), + exception); + } + } }; const AzureOptions& AzureFileSystem::options() const { return impl_->options(); } @@ -758,7 +811,8 @@ Status AzureFileSystem::CreateDir(const std::string& path, bool recursive) { } Status AzureFileSystem::DeleteDir(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_->DeleteDir(location); } Status AzureFileSystem::DeleteDirContents(const std::string& path, bool missing_dir_ok) { diff --git a/cpp/src/arrow/filesystem/azurefs_test.cc b/cpp/src/arrow/filesystem/azurefs_test.cc index ecf0a19f684eb..207fbf9299b7b 100644 --- a/cpp/src/arrow/filesystem/azurefs_test.cc +++ b/cpp/src/arrow/filesystem/azurefs_test.cc @@ -507,6 +507,41 @@ TEST_F(AzuriteFileSystemTest, CreateDirUri) { ASSERT_RAISES(Invalid, fs_->CreateDir("abfs://" + RandomContainerName(), true)); } +TEST_F(AzuriteFileSystemTest, DeleteContainerDirSuccess) { + auto container_name = RandomContainerName(); + ASSERT_OK(fs_->CreateDir(container_name)); + ASSERT_OK(fs_->DeleteDir(container_name)); +} + +TEST_F(AzuriteFileSystemTest, DeleteDirSuccess) { + const auto path = + internal::ConcatAbstractPath(PreexistingContainerName(), RandomDirectoryName()); + // There is only virtual directory without hierarchical namespace + // support. So the DeleteDir() does nothing. + ASSERT_OK(fs_->DeleteDir(path)); + arrow::fs::AssertFileInfo(fs_.get(), path, FileType::NotFound); +} + +TEST_F(AzureHierarchicalNamespaceFileSystemTest, DeleteDirSuccess) { + const auto parent = + internal::ConcatAbstractPath(PreexistingContainerName(), RandomDirectoryName()); + const auto path = internal::ConcatAbstractPath(parent, "new-sub"); + ASSERT_OK(fs_->CreateDir(path, true)); + ASSERT_OK(fs_->DeleteDir(parent)); + arrow::fs::AssertFileInfo(fs_.get(), path, FileType::NotFound); + arrow::fs::AssertFileInfo(fs_.get(), parent, FileType::NotFound); +} + +TEST_F(AzureHierarchicalNamespaceFileSystemTest, DeleteDirFailureNonexistent) { + const auto path = + internal::ConcatAbstractPath(PreexistingContainerName(), RandomDirectoryName()); + ASSERT_RAISES(IOError, fs_->DeleteDir(path)); +} + +TEST_F(AzuriteFileSystemTest, DeleteDirUri) { + ASSERT_RAISES(Invalid, fs_->DeleteDir("abfs://" + PreexistingContainerPath())); +} + TEST_F(AzuriteFileSystemTest, OpenInputStreamString) { std::shared_ptr stream; ASSERT_OK_AND_ASSIGN(stream, fs_->OpenInputStream(PreexistingObjectPath()));