From ecfe720b48193536587a95cf4d1c965a1c2a7ef2 Mon Sep 17 00:00:00 2001 From: Pam Harris Date: Thu, 5 Dec 2024 21:31:13 -0700 Subject: [PATCH 1/5] Fix file list for root directory --- src/Util/Casacore.cc | 25 ++++++++++++++++--------- 1 file changed, 16 insertions(+), 9 deletions(-) diff --git a/src/Util/Casacore.cc b/src/Util/Casacore.cc index 4699f7fdc..fdacd46d5 100644 --- a/src/Util/Casacore.cc +++ b/src/Util/Casacore.cc @@ -103,34 +103,41 @@ casacore::String GetResolvedFilename(const string& root_dir, const string& direc // (absolute pathname with symlinks resolved) casacore::String resolved_filename; casacore::Path path(root_dir); - path.append(directory); - casacore::File cc_directory(path); + if (directory != ".") { + path.append(directory); + } + casacore::File cc_file(path); - if (!cc_directory.exists()) { + // Check directory for error messages + if (!cc_file.exists()) { message = "Directory " + directory + " does not exist."; - } else if (!cc_directory.isReadable()) { + } else if (!cc_file.isReadable()) { message = "Directory " + directory + " is not readable."; } else { + // Check file path.append(file); - casacore::File cc_file(path); - + cc_file = casacore::File(path); if (!cc_file.exists()) { message = "File " + file + " does not exist."; } else if (!cc_file.isReadable()) { message = "File " + file + " is not readable."; } else { try { - resolved_filename = cc_file.path().resolvedName(); + resolved_filename = path.resolvedName(); } catch (const casacore::AipsError& err) { try { - resolved_filename = cc_file.path().absoluteName(); + resolved_filename = path.absoluteName(); + // Workaround for casacore parsing bug when path is "/" + if (resolved_filename.empty()) { + resolved_filename = path.originalName(); + } } catch (const casacore::AipsError& err) { - // return empty string message = "Cannot resolve file path."; } } } } + return resolved_filename; } From bf6d64a7fd1a80a23a59ae88e3459b13e8cae5f9 Mon Sep 17 00:00:00 2001 From: Pam Harris Date: Thu, 5 Dec 2024 22:30:12 -0700 Subject: [PATCH 2/5] Improve casacore path bug fix --- src/Util/Casacore.cc | 25 +++++++++++-------------- 1 file changed, 11 insertions(+), 14 deletions(-) diff --git a/src/Util/Casacore.cc b/src/Util/Casacore.cc index fdacd46d5..60f2576d3 100644 --- a/src/Util/Casacore.cc +++ b/src/Util/Casacore.cc @@ -6,6 +6,7 @@ #include "Casacore.h" +#include #include #include @@ -99,16 +100,14 @@ bool IsSubdirectory(string folder, string top_folder) { } casacore::String GetResolvedFilename(const string& root_dir, const string& directory, const string& file, string& message) { - // Given directory (relative to root folder) and file, return resolved path and filename - // (absolute pathname with symlinks resolved) + // Given directory (relative to root directory) and file, return resolved file path. + // Check if file path exists and is readable. casacore::String resolved_filename; casacore::Path path(root_dir); - if (directory != ".") { - path.append(directory); - } + path.append(directory); casacore::File cc_file(path); - // Check directory for error messages + // Check directory if (!cc_file.exists()) { message = "Directory " + directory + " does not exist."; } else if (!cc_file.isReadable()) { @@ -125,14 +124,12 @@ casacore::String GetResolvedFilename(const string& root_dir, const string& direc try { resolved_filename = path.resolvedName(); } catch (const casacore::AipsError& err) { - try { - resolved_filename = path.absoluteName(); - // Workaround for casacore parsing bug when path is "/" - if (resolved_filename.empty()) { - resolved_filename = path.originalName(); - } - } catch (const casacore::AipsError& err) { - message = "Cannot resolve file path."; + // resolvedName() calls absoluteName(), which returns an empty path due to parsing bug to remove dots (., ..) from path, + // resulting in AipsError. Workaround just sets expanded name used for exists() and isReadable(). + if (path.absoluteName().empty()) { + resolved_filename = path.expandedName(); + } else { + message = err.getMesg(); } } } From 1c604f5e233d3358f7b5eec307858da74c5bd79e Mon Sep 17 00:00:00 2001 From: Pam Harris Date: Thu, 5 Dec 2024 22:43:20 -0700 Subject: [PATCH 3/5] Update changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index ee9cc75bc..51fa3fd10 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,6 +19,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 * Fix matched polygon region approximation crash ([#1383](https://github.com/CARTAvis/carta-backend/issues/1383)). * Fix save image/export regions bug which could cause directory overwrite or deletion ([#1377](https://github.com/CARTAvis/carta-backend/issues/1377)). * Fix bug in cache slicer transformation which affects some images with rotated axes ([#1389](https://github.com/CARTAvis/carta-backend/pull/1389)). +* Fix bug accessing top (root) folder of file browser ([#2354](https://github.com/CARTAvis/carta-frontend/issues/2354)). ### Changed * Move the loader cache to separate files ([#1021](https://github.com/CARTAvis/carta-backend/issues/1021)). From d3915058b19b008f35a22d40033bde2bb171b9fa Mon Sep 17 00:00:00 2001 From: Pam Harris Date: Thu, 5 Dec 2024 22:46:20 -0700 Subject: [PATCH 4/5] Remove unneeded include --- src/Util/Casacore.cc | 1 - 1 file changed, 1 deletion(-) diff --git a/src/Util/Casacore.cc b/src/Util/Casacore.cc index 60f2576d3..cea925380 100644 --- a/src/Util/Casacore.cc +++ b/src/Util/Casacore.cc @@ -6,7 +6,6 @@ #include "Casacore.h" -#include #include #include From b49832a20e5e6be9039fa9f9cfdd6b154a73db1b Mon Sep 17 00:00:00 2001 From: Pam Harris Date: Fri, 6 Dec 2024 14:00:04 -0700 Subject: [PATCH 5/5] Add test for file list for default top level folder --- test/TestFileList.cc | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/test/TestFileList.cc b/test/TestFileList.cc index 9fc292dbb..4596070b1 100644 --- a/test/TestFileList.cc +++ b/test/TestFileList.cc @@ -47,6 +47,22 @@ class FileListTest : public ::testing::Test { } } } + + void TestFileListSubdirectories(const std::string& top_level_folder, const std::string& starting_folder, + const CARTA::FileListRequest& request, bool expected_success = true) { + std::shared_ptr file_list_handler = std::make_shared(top_level_folder, starting_folder); + CARTA::FileListResponse response; + FileListHandler::ResultMsg result_msg; + file_list_handler->OnFileListRequest(request, response, result_msg); + + EXPECT_EQ(response.success(), expected_success); + if (!response.success()) { + return; + } + // Expect no image files, non-zero subdirectories + EXPECT_EQ(response.files_size(), 0); + EXPECT_GT(response.subdirectories_size(), 0); + } }; TEST_F(FileListTest, SetTopLevelFolder) { @@ -64,6 +80,11 @@ TEST_F(FileListTest, SetTopLevelFolder) { auto request4 = Message::FileListRequest("."); TestFileList(abs_path, "", request4); + + // Request file list for default top folder "/" + // 0 image files, > 0 subdirectories + auto request5 = Message::FileListRequest(""); + TestFileListSubdirectories("/", "", request5); } TEST_F(FileListTest, SetStartingFolder) {