Skip to content

Commit

Permalink
Implement
Browse files Browse the repository at this point in the history
  • Loading branch information
@kou
kou committed Mar 15, 2024
1 parent 99c8537 commit 78e5a1a
Show file tree
Hide file tree
Showing 4 changed files with 88 additions and 30 deletions.
95 changes: 70 additions & 25 deletions cpp/src/arrow/filesystem/azurefs.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1576,7 +1576,9 @@ class AzureFileSystem::Impl {
if (info.type() == FileType::NotFound) {
return PathNotFound(location);
}
DCHECK_EQ(info.type(), FileType::Directory);
if (info.type() != FileType::Directory) {
return NotADir(location);
}
return Status::OK();
}

Expand Down Expand Up @@ -1803,37 +1805,63 @@ class AzureFileSystem::Impl {
const AzureLocation& location, bool recursive) {
DCHECK(!location.container.empty());
DCHECK(!location.path.empty());
// Non-recursive CreateDir calls require the parent directory to exist.
if (!recursive) {
if (recursive) {
std::vector<AzureLocation> target_locations;
// Recursive CreateDir calls require there is no file in parents.
auto parent = location;
while (!parent.path.empty()) {
ARROW_ASSIGN_OR_RAISE(auto info, GetFileInfo(container_client, parent));
if (info.type() == FileType::File) {
return NotADir(parent);
}
if (info.type() == FileType::NotFound) {
target_locations.push_back(parent);
}
parent = parent.parent();
if (parent.path.empty() && info.type() == FileType::NotFound) {
// parent is container and may not exist
ARROW_ASSIGN_OR_RAISE(info,
GetContainerPropsAsFileInfo(parent, container_client));
if (info.type() == FileType::NotFound) {
try {
container_client.CreateIfNotExists();
} catch (const Storage::StorageException& exception) {
return ExceptionToStatus(exception, "Failed to create directory '",
location.all, "': ", container_client.GetUrl());
}
}
}
}
for (size_t i = target_locations.size(); i > 0; --i) {
const auto& target_location = target_locations[i - 1];
try {
create_if_not_exists(container_client, target_location);
} catch (const Storage::StorageException& exception) {
return ExceptionToStatus(exception, "Failed to create directory '",
location.all, "': ", container_client.GetUrl());
}
}
return Status::OK();
} else {
// Non-recursive CreateDir calls require the parent directory to exist.
auto parent = location.parent();
if (!parent.path.empty()) {
RETURN_NOT_OK(CheckDirExists(container_client, parent));
}
// If the parent location is just the container, we don't need to check if it
// exists because the operation we perform below will fail if the container
// doesn't exist and we can handle that error according to the recursive flag.
}
try {
create_if_not_exists(container_client, location);
return Status::OK();
} catch (const Storage::StorageException& exception) {
if (IsContainerNotFound(exception)) {
try {
if (recursive) {
container_client.CreateIfNotExists();
create_if_not_exists(container_client, location);
return Status::OK();
} else {
auto parent = location.parent();
return PathNotFound(parent);
}
} catch (const Storage::StorageException& second_exception) {
return ExceptionToStatus(second_exception, "Failed to create directory '",
location.all, "': ", container_client.GetUrl());
try {
create_if_not_exists(container_client, location);
return Status::OK();
} catch (const Storage::StorageException& exception) {
if (IsContainerNotFound(exception)) {
auto parent = location.parent();
return PathNotFound(parent);
}
return ExceptionToStatus(exception, "Failed to create directory '", location.all,
"': ", container_client.GetUrl());
}
return ExceptionToStatus(exception, "Failed to create directory '", location.all,
"': ", container_client.GetUrl());
}
}

Expand Down Expand Up @@ -2001,8 +2029,15 @@ class AzureFileSystem::Impl {
bool found_dir_marker_blob = false;
try {
auto list_response = container_client.ListBlobs(options);
if (require_dir_to_exist && list_response.Blobs.empty()) {
return PathNotFound(location);
if (list_response.Blobs.empty()) {
if (require_dir_to_exist) {
return PathNotFound(location);
} else {
ARROW_ASSIGN_OR_RAISE(auto info, GetFileInfo(container_client, location));
if (info.type() == FileType::File) {
return NotADir(location);
}
}
}
for (; list_response.HasPage(); list_response.MoveToNextPage()) {
if (list_response.Blobs.empty()) {
Expand Down Expand Up @@ -2717,6 +2752,16 @@ class AzureFileSystem::Impl {
}
auto dest_blob_client = GetBlobClient(dest.container, dest.path);
auto src_url = GetBlobClient(src.container, src.path).GetUrl();
if (!dest.path.empty()) {
auto dest_parent = dest.parent();
if (!dest_parent.path.empty()) {
auto dest_container_client = GetBlobContainerClient(dest_parent.container);
ARROW_ASSIGN_OR_RAISE(auto info, GetFileInfo(dest_container_client, dest_parent));
if (info.type() == FileType::File) {
return NotADir(dest_parent);
}
}
}
try {
dest_blob_client.CopyFromUri(src_url);
} catch (const Storage::StorageException& exception) {
Expand Down
9 changes: 5 additions & 4 deletions cpp/src/arrow/filesystem/azurefs_test.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -377,10 +377,13 @@ class TestAzureFileSystemGeneric : public ::testing::Test, public GenericFileSys
bool allow_write_file_over_dir() const override { return true; }
bool allow_read_dir_as_file() const override { return true; }
bool allow_move_dir() const override { return false; }
// Azurite doesn't support moving files over containers.
bool allow_move_file() const override { return false; }
bool allow_append_to_file() const override { return true; }
bool have_directory_mtimes() const override { return false; }
bool have_flaky_directory_tree_deletion() const override { return false; }
bool have_file_metadata() const override { return true; }
bool have_file_metadata() const override { return false; }
bool have_default_file_metadata() const override { return true; }

std::shared_ptr<AzureFileSystem> azure_fs_;
std::shared_ptr<FileSystem> fs_;
Expand Down Expand Up @@ -1100,9 +1103,7 @@ class TestAzureFileSystem : public ::testing::Test {

auto path2 = data.Path("directory2");
ASSERT_OK(fs()->OpenOutputStream(path2));
// CreateDir returns OK even if there is already a file or directory at this
// location. Whether or not this is the desired behaviour is debatable.
ASSERT_OK(fs()->CreateDir(path2));
ASSERT_RAISES(IOError, fs()->CreateDir(path2));
AssertFileInfo(fs(), path2, FileType::File);
}

Expand Down
10 changes: 9 additions & 1 deletion cpp/src/arrow/filesystem/test_util.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -385,6 +385,10 @@ void GenericFileSystemTest::TestDeleteFiles(FileSystem* fs) {
}

void GenericFileSystemTest::TestMoveFile(FileSystem* fs) {
if (!allow_move_file()) {
GTEST_SKIP() << "Filesystem doesn't allow moving files";
}

ASSERT_OK(fs->CreateDir("AB/CD"));
ASSERT_OK(fs->CreateDir("EF"));
CreateFile(fs, "abc", "data");
Expand Down Expand Up @@ -922,7 +926,11 @@ void GenericFileSystemTest::TestOpenOutputStream(FileSystem* fs) {
ASSERT_OK_AND_EQ("x-arrow/filesystem-test", got_metadata->Get("Content-Type"));
} else {
if (got_metadata) {
ASSERT_EQ(got_metadata->size(), 0);
if (have_default_file_metadata()) {
ASSERT_GT(got_metadata->size(), 0);
} else {
ASSERT_EQ(got_metadata->size(), 0);
}
}
}

Expand Down
4 changes: 4 additions & 0 deletions cpp/src/arrow/filesystem/test_util.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -168,6 +168,8 @@ class ARROW_TESTING_EXPORT GenericFileSystemTest {
virtual bool allow_write_file_over_dir() const { return false; }
// - Whether the filesystem allows reading a directory
virtual bool allow_read_dir_as_file() const { return false; }
// - Whether the filesystem allows moving a file
virtual bool allow_move_file() const { return true; }
// - Whether the filesystem allows moving a directory
virtual bool allow_move_dir() const { return true; }
// - Whether the filesystem allows moving a directory "over" a non-empty destination
Expand All @@ -182,6 +184,8 @@ class ARROW_TESTING_EXPORT GenericFileSystemTest {
virtual bool have_flaky_directory_tree_deletion() const { return false; }
// - Whether the filesystem stores some metadata alongside files
virtual bool have_file_metadata() const { return false; }
// - Whether the filesystem returns some default metadata alongside files
virtual bool have_default_file_metadata() const { return false; }

void TestEmpty(FileSystem* fs);
void TestNormalizePath(FileSystem* fs);
Expand Down

0 comments on commit 78e5a1a

Please sign in to comment.