Skip to content

Commit 24f7092

Browse files
committed
FIX: Update GetFileInfo logic to correctly handle directories
1 parent aef990a commit 24f7092

File tree

2 files changed

+12
-21
lines changed

2 files changed

+12
-21
lines changed

cpp/src/arrow/filesystem/gcsfs.cc

+7-6
Original file line numberDiff line numberDiff line change
@@ -354,14 +354,15 @@ class GcsFileSystem::Impl {
354354
std::string canonical = internal::EnsureTrailingSlash(path.object);
355355
auto list_result = client_.ListObjects(path.bucket, gcs::Prefix(canonical));
356356

357-
// Check that the result is valid before determining whether the list is empty.
358-
auto it = list_result.begin();
359-
if (it != list_result.end() && *it) {
360-
// If there is at least one result it indicates this is a directory (at
361-
// least one object exists that starts with "path/")
357+
for (auto&& object_metadata : list_result) {
358+
if (!object_metadata) {
359+
continue;
360+
}
361+
// If there is at least one valid result, it indicates this is a
362+
// directory (at least one object exists that starts with "path/")
362363
return FileInfo(path.full_path, FileType::Directory);
363364
}
364-
// Return the original not-found info if there was no match.
365+
// Return the original not-found info if there was no valid result.
365366
return info;
366367
}
367368

cpp/src/arrow/filesystem/gcsfs_test.cc

+5-15
Original file line numberDiff line numberDiff line change
@@ -644,22 +644,12 @@ TEST_F(GcsIntegrationTest, GetFileInfo) {
644644

645645
// check parent directories are recognized as directories.
646646
AssertFileInfo(fs.get(), PreexistingBucketPath() + "dir/", FileType::Directory);
647-
AssertFileInfo(fs.get(), PreexistingBucketPath() + "dir/foo/", FileType::Directory);
648-
}
649-
650-
TEST_F(GcsIntegrationTest, GetFileInfoWithoutPermission) {
651-
auto options = GcsOptions::Defaults();
652-
// Test with real GCS.
653-
ASSERT_OK_AND_ASSIGN(auto fs, GcsFileSystem::Make(options));
654-
// Without permission, always File typs is FileType::NotFound.
655-
constexpr auto kTextFileName = "dir/foo/bar.txt";
656647

657-
// check this is the File without permission.
658-
AssertFileInfo(fs.get(), PreexistingBucketPath() + kTextFileName, FileType::NotFound);
659-
660-
// check this is the directory without permission.
661-
AssertFileInfo(fs.get(), PreexistingBucketPath() + "dir/", FileType::NotFound);
662-
AssertFileInfo(fs.get(), PreexistingBucketPath() + "dir/foo/", FileType::NotFound);
648+
// check not exists.
649+
AssertFileInfo(fs.get(), PreexistingBucketPath() + "dir/foo/unexpected_dir/",
650+
FileType::NotFound);
651+
AssertFileInfo(fs.get(), PreexistingBucketPath() + "dir/foo/not_bar.txt",
652+
FileType::NotFound);
663653
}
664654

665655
TEST_F(GcsIntegrationTest, GetFileInfoObjectWithNestedStructure) {

0 commit comments

Comments
 (0)