Skip to content

Commit b8f16a1

Browse files
zjw1111zjw1111
committed
refactor(fs): return LocalFile create as unique ptr
1 parent 48482e2 commit b8f16a1

8 files changed

Lines changed: 115 additions & 110 deletions

File tree

‎src/paimon/common/fs/file_system_test.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -161,7 +161,7 @@ class FileSystemTest : public ::testing::Test, public ::testing::WithParamInterf
161161
if (file_system == "local") {
162162
std::string data_dir = paimon::test::GetDataDir();
163163
if (data_dir.empty() || data_dir[0] != '/') {
164-
EXPECT_OK_AND_ASSIGN(std::string current_path, PathUtil::GetCurrentPath());
164+
EXPECT_OK_AND_ASSIGN(std::string current_path, PathUtil::GetWorkingDirectory());
165165
data_dir = PathUtil::JoinPath(current_path, data_dir);
166166
}
167167
return data_dir;

‎src/paimon/common/utils/path_util.cpp‎

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@
1919
#include <unistd.h>
2020

2121
#include <cerrno>
22-
#include <climits>
22+
#include <cstdlib>
2323
#include <cstddef>
2424
#include <cstdint>
2525
#include <cstring>
@@ -147,12 +147,15 @@ void PathUtil::TrimLastDelim(std::string* dir_path) noexcept {
147147
}
148148
}
149149

150-
Result<std::string> PathUtil::GetCurrentPath() noexcept {
151-
char cwd[PATH_MAX];
152-
if (getcwd(cwd, sizeof(cwd)) != nullptr) {
153-
return std::string(cwd);
150+
Result<std::string> PathUtil::GetWorkingDirectory() noexcept {
151+
char* path = getcwd(nullptr, 0);
152+
if (path != nullptr) {
153+
std::string ret(path);
154+
free(path);
155+
return ret;
154156
}
155-
return Status::IOError(fmt::format("get current path failed, ec: {}", std::strerror(errno)));
157+
return Status::IOError(
158+
fmt::format("get working directory failed, ec: {}", std::strerror(errno)));
156159
}
157160

158161
Result<std::string> PathUtil::CreateTempPath(const std::string& path) noexcept {

‎src/paimon/common/utils/path_util.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ class PAIMON_EXPORT PathUtil {
4444
static std::string GetParentDirPath(const std::string& path) noexcept;
4545
static std::string GetName(const std::string& path) noexcept;
4646
static void TrimLastDelim(std::string* dir_path) noexcept;
47-
static Result<std::string> GetCurrentPath() noexcept;
47+
static Result<std::string> GetWorkingDirectory() noexcept;
4848
static Result<std::string> CreateTempPath(const std::string& path) noexcept;
4949
static Result<Path> ToPath(const std::string& path) noexcept;
5050
static Result<std::string> NormalizePath(const std::string& path) noexcept;

‎src/paimon/common/utils/path_util_test.cpp‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -91,8 +91,8 @@ TEST(PathUtilsTest, TestTrimLastDelim) {
9191
}
9292
}
9393

94-
TEST(PathUtilsTest, TestGetCurrentPath) {
95-
ASSERT_OK_AND_ASSIGN(std::string current_path, PathUtil::GetCurrentPath());
94+
TEST(PathUtilsTest, TestGetWorkingDirectory) {
95+
ASSERT_OK_AND_ASSIGN(std::string current_path, PathUtil::GetWorkingDirectory());
9696
ASSERT_FALSE(current_path.empty());
9797
ASSERT_EQ(current_path[0], '/');
9898
}

‎src/paimon/fs/local/local_file.cpp‎

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -38,10 +38,10 @@ namespace paimon {
3838
PAIMON_RETURN_NOT_OK(hook_->Try(path_)); \
3939
}
4040

41-
Result<LocalFile> LocalFile::Create(const std::string& path_string) {
41+
Result<std::unique_ptr<LocalFile>> LocalFile::Create(const std::string& path_string) {
4242
if (path_string.empty()) {
43-
PAIMON_ASSIGN_OR_RAISE(std::string current_path, PathUtil::GetCurrentPath());
44-
return LocalFile(current_path);
43+
PAIMON_ASSIGN_OR_RAISE(std::string current_path, PathUtil::GetWorkingDirectory());
44+
return std::unique_ptr<LocalFile>(new LocalFile(current_path));
4545
}
4646

4747
// local file system does not support path_string with scheme, e.g., "file:/tmp" will be
@@ -51,10 +51,11 @@ Result<LocalFile> LocalFile::Create(const std::string& path_string) {
5151
return Status::Invalid(fmt::format("invalid scheme {} for local file system", path.scheme));
5252
}
5353
if (path.path.empty() || path.path[0] != '/') {
54-
PAIMON_ASSIGN_OR_RAISE(std::string current_path, PathUtil::GetCurrentPath());
55-
return LocalFile(PathUtil::JoinPath(current_path, path.path));
54+
PAIMON_ASSIGN_OR_RAISE(std::string current_path, PathUtil::GetWorkingDirectory());
55+
return std::unique_ptr<LocalFile>(
56+
new LocalFile(PathUtil::JoinPath(current_path, path.path)));
5657
}
57-
return LocalFile(path.path);
58+
return std::unique_ptr<LocalFile>(new LocalFile(path.path));
5859
}
5960

6061
LocalFile::LocalFile(const std::string& path) : path_(path), hook_(IOHook::GetInstance()) {}

‎src/paimon/fs/local/local_file.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ class LocalFileStatus;
3636

3737
class LocalFile {
3838
public:
39-
static Result<LocalFile> Create(const std::string& path_string);
39+
static Result<std::unique_ptr<LocalFile>> Create(const std::string& path_string);
4040
~LocalFile() = default;
4141

4242
Result<bool> Exists() const;

‎src/paimon/fs/local/local_file_system.cpp‎

Lines changed: 33 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -31,17 +31,17 @@
3131
namespace paimon {
3232

3333
Result<bool> LocalFileSystem::Exists(const std::string& path) const {
34-
PAIMON_ASSIGN_OR_RAISE(LocalFile file, LocalFile::Create(path));
35-
return file.Exists();
34+
PAIMON_ASSIGN_OR_RAISE(std::unique_ptr<LocalFile> file, LocalFile::Create(path));
35+
return file->Exists();
3636
}
3737

3838
Result<std::unique_ptr<InputStream>> LocalFileSystem::Open(const std::string& path) const {
3939
PAIMON_ASSIGN_OR_RAISE(bool is_exist, Exists(path));
4040
if (!is_exist) {
4141
return Status::NotExist(fmt::format("File '{}' not exists", path));
4242
}
43-
PAIMON_ASSIGN_OR_RAISE(LocalFile file, LocalFile::Create(path));
44-
PAIMON_ASSIGN_OR_RAISE(std::unique_ptr<LocalInputStream> in, LocalInputStream::Create(file));
43+
PAIMON_ASSIGN_OR_RAISE(std::unique_ptr<LocalFile> file, LocalFile::Create(path));
44+
PAIMON_ASSIGN_OR_RAISE(std::unique_ptr<LocalInputStream> in, LocalInputStream::Create(*file));
4545
return in;
4646
}
4747

@@ -52,16 +52,17 @@ Result<std::unique_ptr<OutputStream>> LocalFileSystem::Create(const std::string&
5252
return Status::Invalid(
5353
fmt::format("do not allow overwrite, but the file {} already exists", path));
5454
}
55-
PAIMON_ASSIGN_OR_RAISE(LocalFile file, LocalFile::Create(path));
56-
LocalFile parent = file.GetParentFile();
55+
PAIMON_ASSIGN_OR_RAISE(std::unique_ptr<LocalFile> file, LocalFile::Create(path));
56+
LocalFile parent = file->GetParentFile();
5757
PAIMON_RETURN_NOT_OK(Mkdirs(parent.GetPath()));
58-
PAIMON_ASSIGN_OR_RAISE(std::unique_ptr<LocalOutputStream> out, LocalOutputStream::Create(file));
58+
PAIMON_ASSIGN_OR_RAISE(std::unique_ptr<LocalOutputStream> out,
59+
LocalOutputStream::Create(*file));
5960
return out;
6061
}
6162

6263
Status LocalFileSystem::Mkdirs(const std::string& path) const {
63-
PAIMON_ASSIGN_OR_RAISE(LocalFile file, LocalFile::Create(path));
64-
return MkdirsInternal(file);
64+
PAIMON_ASSIGN_OR_RAISE(std::unique_ptr<LocalFile> file, LocalFile::Create(path));
65+
return MkdirsInternal(*file);
6566
}
6667

6768
Status LocalFileSystem::MkdirsInternal(const LocalFile& file) const {
@@ -96,33 +97,33 @@ Status LocalFileSystem::MkdirsInternal(const LocalFile& file) const {
9697
}
9798

9899
Result<std::unique_ptr<FileStatus>> LocalFileSystem::GetFileStatus(const std::string& path) const {
99-
PAIMON_ASSIGN_OR_RAISE(LocalFile file, LocalFile::Create(path));
100-
PAIMON_ASSIGN_OR_RAISE(bool is_exist, file.Exists());
100+
PAIMON_ASSIGN_OR_RAISE(std::unique_ptr<LocalFile> file, LocalFile::Create(path));
101+
PAIMON_ASSIGN_OR_RAISE(bool is_exist, file->Exists());
101102
if (is_exist) {
102-
return file.GetFileStatus();
103+
return file->GetFileStatus();
103104
} else {
104105
return Status::NotExist(
105106
fmt::format("File {} does not exist or the user running "
106107
"Paimon has insufficient permissions to access it.",
107-
file.GetPath()));
108+
file->GetPath()));
108109
}
109110
}
110111

111112
Status LocalFileSystem::ListDir(
112113
const std::string& directory,
113114
std::vector<std::unique_ptr<BasicFileStatus>>* file_status_list) const {
114-
PAIMON_ASSIGN_OR_RAISE(LocalFile file, LocalFile::Create(directory));
115-
PAIMON_ASSIGN_OR_RAISE(bool is_exist, file.Exists());
115+
PAIMON_ASSIGN_OR_RAISE(std::unique_ptr<LocalFile> file, LocalFile::Create(directory));
116+
PAIMON_ASSIGN_OR_RAISE(bool is_exist, file->Exists());
116117
if (!is_exist) {
117118
return Status::OK();
118119
}
119-
PAIMON_ASSIGN_OR_RAISE(bool is_file, file.IsFile());
120+
PAIMON_ASSIGN_OR_RAISE(bool is_file, file->IsFile());
120121
if (is_file) {
121122
return Status::IOError(
122-
fmt::format("file {} already exists and is not a directory", file.GetPath()));
123+
fmt::format("file {} already exists and is not a directory", file->GetPath()));
123124
} else {
124125
std::vector<std::string> file_list;
125-
PAIMON_RETURN_NOT_OK(file.List(&file_list));
126+
PAIMON_RETURN_NOT_OK(file->List(&file_list));
126127
file_status_list->reserve(file_status_list->size() + file_list.size());
127128
for (const auto& f : file_list) {
128129
Result<std::unique_ptr<FileStatus>> file_status =
@@ -140,18 +141,18 @@ Status LocalFileSystem::ListDir(
140141

141142
Status LocalFileSystem::ListFileStatus(
142143
const std::string& path, std::vector<std::unique_ptr<FileStatus>>* file_status_list) const {
143-
PAIMON_ASSIGN_OR_RAISE(LocalFile file, LocalFile::Create(path));
144-
PAIMON_ASSIGN_OR_RAISE(bool is_exist, file.Exists());
144+
PAIMON_ASSIGN_OR_RAISE(std::unique_ptr<LocalFile> file, LocalFile::Create(path));
145+
PAIMON_ASSIGN_OR_RAISE(bool is_exist, file->Exists());
145146
if (!is_exist) {
146147
return Status::OK();
147148
}
148-
PAIMON_ASSIGN_OR_RAISE(bool is_file, file.IsFile());
149+
PAIMON_ASSIGN_OR_RAISE(bool is_file, file->IsFile());
149150
if (is_file) {
150-
PAIMON_ASSIGN_OR_RAISE(std::unique_ptr<FileStatus> file_status, file.GetFileStatus());
151+
PAIMON_ASSIGN_OR_RAISE(std::unique_ptr<FileStatus> file_status, file->GetFileStatus());
151152
file_status_list->emplace_back(std::move(file_status));
152153
} else {
153154
std::vector<std::string> file_list;
154-
PAIMON_RETURN_NOT_OK(file.List(&file_list));
155+
PAIMON_RETURN_NOT_OK(file->List(&file_list));
155156
file_status_list->reserve(file_status_list->size() + file_list.size());
156157
for (const auto& f : file_list) {
157158
Result<std::unique_ptr<FileStatus>> file_status =
@@ -167,12 +168,12 @@ Status LocalFileSystem::ListFileStatus(
167168
}
168169

169170
Status LocalFileSystem::Delete(const std::string& path, bool recursive) const {
170-
PAIMON_ASSIGN_OR_RAISE(LocalFile file, LocalFile::Create(path));
171-
PAIMON_ASSIGN_OR_RAISE(bool is_file, file.IsFile());
171+
PAIMON_ASSIGN_OR_RAISE(std::unique_ptr<LocalFile> file, LocalFile::Create(path));
172+
PAIMON_ASSIGN_OR_RAISE(bool is_file, file->IsFile());
172173
if (is_file) {
173-
return file.Delete();
174+
return file->Delete();
174175
}
175-
return Delete(file, recursive);
176+
return Delete(*file, recursive);
176177
}
177178

178179
Status LocalFileSystem::Delete(const LocalFile& f, bool recursive) const {
@@ -202,17 +203,17 @@ Status LocalFileSystem::Rename(const std::string& src, const std::string& dst) c
202203
if (is_dst_exist) {
203204
return Status::Invalid(err_msg, "dst file already exist");
204205
}
205-
PAIMON_ASSIGN_OR_RAISE(LocalFile src_file, LocalFile::Create(src));
206-
PAIMON_ASSIGN_OR_RAISE(bool is_file, src_file.IsFile());
206+
PAIMON_ASSIGN_OR_RAISE(std::unique_ptr<LocalFile> src_file, LocalFile::Create(src));
207+
PAIMON_ASSIGN_OR_RAISE(bool is_file, src_file->IsFile());
207208
std::string new_file_name = dst;
208209

209210
if (is_file && new_file_name[new_file_name.length() - 1] == '/') {
210211
return Status::Invalid(err_msg, "src file is not a dir");
211212
}
212-
PAIMON_ASSIGN_OR_RAISE(LocalFile dst_file, LocalFile::Create(dst));
213-
auto parent = dst_file.GetParentFile();
213+
PAIMON_ASSIGN_OR_RAISE(std::unique_ptr<LocalFile> dst_file, LocalFile::Create(dst));
214+
auto parent = dst_file->GetParentFile();
214215
PAIMON_RETURN_NOT_OK(Mkdirs(parent.GetPath()));
215-
if (::rename(src_file.GetPath().c_str(), dst_file.GetPath().c_str()) != 0) {
216+
if (::rename(src_file->GetPath().c_str(), dst_file->GetPath().c_str()) != 0) {
216217
int32_t cur_errno = errno;
217218
return Status::IOError(err_msg, std::strerror(cur_errno));
218219
}

0 commit comments

Comments
 (0)