From 1b6744233e1ebd71b688a5ac4f6b4d35fc79963a Mon Sep 17 00:00:00 2001
From: Sergey Yakubov <sergey.yakubov@desy.de>
Date: Fri, 15 Dec 2017 13:43:33 +0100
Subject: [PATCH] read data from file finished#
---
common/cpp/include/common/file_info.h | 1 +
common/cpp/include/system_wrappers/io.h | 5 +-
.../cpp/include/system_wrappers/system_io.h | 2 +-
common/cpp/src/system_io.cpp | 44 +++++++++++---
common/cpp/src/system_io_linux.cpp | 53 ++++++++++++-----
tests/common/cpp/include/testing.h | 2 +
tests/common/cpp/src/testing.cpp | 14 ++++-
.../read_file_content/CMakeLists.txt | 2 +-
.../read_file_content/read_file_content.cpp | 7 ++-
.../read_folder_content.cpp | 1 -
worker/api/cpp/src/folder_data_broker.cpp | 2 +-
.../api/cpp/unittests/test_folder_broker.cpp | 58 ++++++++++---------
12 files changed, 132 insertions(+), 59 deletions(-)
diff --git a/common/cpp/include/common/file_info.h b/common/cpp/include/common/file_info.h
index baa44781e..a08967c1f 100644
--- a/common/cpp/include/common/file_info.h
+++ b/common/cpp/include/common/file_info.h
@@ -11,6 +11,7 @@ struct FileInfo {
std::string base_name;
std::string relative_path;
std::chrono::system_clock::time_point modify_date;
+ uint64_t size;
};
typedef std::vector<uint8_t> FileData;
diff --git a/common/cpp/include/system_wrappers/io.h b/common/cpp/include/system_wrappers/io.h
index f8e8d686d..059b376c5 100644
--- a/common/cpp/include/system_wrappers/io.h
+++ b/common/cpp/include/system_wrappers/io.h
@@ -1,6 +1,8 @@
#ifndef HIDRA2_SYSTEM_WRAPPERS__IO_H
#define HIDRA2_SYSTEM_WRAPPERS__IO_H
+#include <cinttypes>
+
#include <string>
#include <vector>
#include <chrono>
@@ -20,10 +22,11 @@ enum class IOErrors {
IOErrors IOErrorFromErrno();
+
class IO {
public:
- virtual FileData GetDataFromFile(const std::string& fname, IOErrors* err) = 0;
+ virtual FileData GetDataFromFile(const std::string& fname, uint64_t fsize, IOErrors* err) = 0;
virtual int open(const char* __file, int __oflag) = 0;
virtual int close(int __fd) = 0;
diff --git a/common/cpp/include/system_wrappers/system_io.h b/common/cpp/include/system_wrappers/system_io.h
index 9cb2214e5..8bee93684 100644
--- a/common/cpp/include/system_wrappers/system_io.h
+++ b/common/cpp/include/system_wrappers/system_io.h
@@ -7,7 +7,7 @@ namespace hidra2 {
class SystemIO final : public IO {
public:
- FileData GetDataFromFile(const std::string& fname, IOErrors* err);
+ FileData GetDataFromFile(const std::string& fname, uint64_t fsize, IOErrors* err) override;
int open(const char* __file, int __oflag);
int close(int __fd);
ssize_t read(int __fd, void* buf, size_t count);
diff --git a/common/cpp/src/system_io.cpp b/common/cpp/src/system_io.cpp
index b70669981..6f012a9f9 100644
--- a/common/cpp/src/system_io.cpp
+++ b/common/cpp/src/system_io.cpp
@@ -1,34 +1,62 @@
#include <fcntl.h>
#include <unistd.h>
+
#include <system_wrappers/system_io.h>
-hidra2::FileData hidra2::SystemIO::GetDataFromFile(const std::string& fname, IOErrors* err) {
- int fd = open(fname.c_str(), O_RDONLY);
+namespace hidra2 {
+
+void ReadWholeFile(int fd, uint8_t* array, uint64_t fsize, IOErrors* err) {
+ ssize_t totalbytes = 0;
+ ssize_t readbytes = 0;
+ do {
+ readbytes = read(fd, array + totalbytes, fsize);
+ totalbytes += readbytes;
+ } while (readbytes > 0 && totalbytes < fsize);
+ if (totalbytes != fsize) {
+ *err = IOErrors::READ_ERROR;
+ }
+}
+
+FileData SystemIO::GetDataFromFile(const std::string& fname, uint64_t fsize, IOErrors* err) {
+ int fd = open(fname.c_str(), O_RDONLY);
*err = IOErrorFromErrno();
+ if (*err != IOErrors::NO_ERROR) {
+ return {};
+ }
+
+ FileData data(fsize);
+ ReadWholeFile(fd, &data[0], fsize, err);
if (*err != IOErrors::NO_ERROR) {
+ close(fd);
return {};
}
- *err = IOErrors::READ_ERROR;
+
close(fd);
- return {};
+ *err = IOErrorFromErrno();
+ if (*err != IOErrors::NO_ERROR) {
+ return {};
+ }
+ return data;
}
-int hidra2::SystemIO::open(const char* __file, int __oflag) {
+int SystemIO::open(const char* __file, int __oflag) {
return ::open(__file, __oflag);
}
-int hidra2::SystemIO::close(int __fd) {
+int SystemIO::close(int __fd) {
return ::close(__fd);
}
-ssize_t hidra2::SystemIO::read(int __fd, void* buf, size_t count) {
+ssize_t SystemIO::read(int __fd, void* buf, size_t count) {
return ::read(__fd, buf, count);
}
-ssize_t hidra2::SystemIO::write(int __fd, const void* __buf, size_t __n) {
+ssize_t SystemIO::write(int __fd, const void* __buf, size_t __n) {
return ::write(__fd, __buf, __n);
}
+
+}
\ No newline at end of file
diff --git a/common/cpp/src/system_io_linux.cpp b/common/cpp/src/system_io_linux.cpp
index 290ef2829..c4b3a5be8 100644
--- a/common/cpp/src/system_io_linux.cpp
+++ b/common/cpp/src/system_io_linux.cpp
@@ -41,15 +41,7 @@ bool IsDirectory(const struct dirent* entity) {
strstr(entity->d_name, ".") == nullptr;
}
-system_clock::time_point GetTimePointFromFile(const string& fname, IOErrors* err) {
-
- struct stat t_stat {};
- int res = stat(fname.c_str(), &t_stat);
- if (res < 0) {
- *err = IOErrorFromErrno();
- return system_clock::time_point{};
- }
-
+void SetModifyDate(const struct stat& t_stat, FileInfo* file_info) {
#ifdef __APPLE__
#define st_mtim st_mtimespec
#endif
@@ -59,7 +51,43 @@ system_clock::time_point GetTimePointFromFile(const string& fname, IOErrors* err
#undef st_mtim
#endif
- return system_clock::time_point {std::chrono::duration_cast<system_clock::duration>(d)};
+ file_info->modify_date = system_clock::time_point
+ {std::chrono::duration_cast<system_clock::duration>(d)};
+}
+
+void SetFileSize(const struct stat& t_stat, FileInfo* file_info) {
+ file_info->size = t_stat.st_size;
+}
+
+void SetFileName(const string& path, const string& name, FileInfo* file_info) {
+ file_info->relative_path = path;
+ file_info->base_name = name;
+}
+
+struct stat FileStat(const string& fname, IOErrors* err) {
+ struct stat t_stat {};
+ int res = stat(fname.c_str(), &t_stat);
+ if (res < 0) {
+ *err = IOErrorFromErrno();
+ }
+ return t_stat;
+}
+
+FileInfo GetFileInfo(const string& path, const string& name, IOErrors* err) {
+ FileInfo file_info;
+
+ SetFileName(path, name, &file_info);
+
+ auto t_stat = FileStat(path + "/" + name, err);
+ if (*err != IOErrors::NO_ERROR) {
+ return FileInfo{};
+ }
+
+ SetFileSize(t_stat, &file_info);
+
+ SetModifyDate(t_stat, &file_info);
+
+ return file_info;
}
void ProcessFileEntity(const struct dirent* entity, const std::string& path,
@@ -69,11 +97,8 @@ void ProcessFileEntity(const struct dirent* entity, const std::string& path,
if (entity->d_type != DT_REG) {
return;
}
- FileInfo file_info;
- file_info.relative_path = path;
- file_info.base_name = entity->d_name;
- file_info.modify_date = GetTimePointFromFile(path + "/" + entity->d_name, err);
+ FileInfo file_info = GetFileInfo(path, entity->d_name, err);
if (*err != IOErrors::NO_ERROR) {
return;
}
diff --git a/tests/common/cpp/include/testing.h b/tests/common/cpp/include/testing.h
index c3c24b6b8..a78cdba7e 100644
--- a/tests/common/cpp/include/testing.h
+++ b/tests/common/cpp/include/testing.h
@@ -6,6 +6,8 @@
namespace hidra2 {
void M_AssertEq(const std::string& expected, const std::string& got);
+void M_AssertEq(int expected, int got);
+
}
diff --git a/tests/common/cpp/src/testing.cpp b/tests/common/cpp/src/testing.cpp
index e9a9adae6..aa9283189 100644
--- a/tests/common/cpp/src/testing.cpp
+++ b/tests/common/cpp/src/testing.cpp
@@ -4,7 +4,8 @@
namespace hidra2 {
-void M_AssertEq(const std::string& expected, const std::string& got) {
+template<typename T>
+void T_AssertEq(const T& expected, const T& got) {
if (expected != got) {
std::cerr << "Assert failed:\n"
<< "Expected:\t'" << expected << "'\n"
@@ -12,5 +13,16 @@ void M_AssertEq(const std::string& expected, const std::string& got) {
abort();
}
}
+
+
+void M_AssertEq(const std::string& expected, const std::string& got) {
+ T_AssertEq(expected, got);
+}
+
+void M_AssertEq(int expected, int got) {
+ T_AssertEq(expected, got);
+}
+
+
}
diff --git a/tests/system_io/read_file_content/CMakeLists.txt b/tests/system_io/read_file_content/CMakeLists.txt
index 7c5c9a53d..21ad081c5 100644
--- a/tests/system_io/read_file_content/CMakeLists.txt
+++ b/tests/system_io/read_file_content/CMakeLists.txt
@@ -14,7 +14,7 @@ set_target_properties(${TARGET_NAME} PROPERTIES LINKER_LANGUAGE CXX)
################################
add_test_setup_cleanup(${TARGET_NAME})
-add_integration_test(${TARGET_NAME} readfile "test/1 readerror")
+add_integration_test(${TARGET_NAME} readfile "test/1 123")
add_integration_test(${TARGET_NAME} filenotfound "test_notexist notfound")
add_integration_test(${TARGET_NAME} filenoaccess "file_noaccess noaccess")
diff --git a/tests/system_io/read_file_content/read_file_content.cpp b/tests/system_io/read_file_content/read_file_content.cpp
index 9e6f6980b..f6af4619b 100644
--- a/tests/system_io/read_file_content/read_file_content.cpp
+++ b/tests/system_io/read_file_content/read_file_content.cpp
@@ -1,6 +1,8 @@
#include <iostream>
#include <system_wrappers/system_io.h>
+#include <memory>
+#include <string>
#include "testing.h"
using hidra2::SystemIO;
@@ -16,7 +18,7 @@ int main(int argc, char* argv[]) {
IOErrors err;
auto io = std::unique_ptr<SystemIO> {new SystemIO};
- auto data = io->GetDataFromFile(argv[1], &err);
+ auto data = io->GetDataFromFile(argv[1], expect.size(), &err);
std::string result;
@@ -27,6 +29,7 @@ int main(int argc, char* argv[]) {
case IOErrors::NO_ERROR:
for(auto symbol : data)
result += symbol;
+ M_AssertEq(expect.size(), result.size());
break;
case IOErrors::PERMISSIONS_DENIED:
result = "noaccess";
@@ -41,7 +44,5 @@ int main(int argc, char* argv[]) {
}
M_AssertEq(expect, result);
-
-
return 0;
}
diff --git a/tests/system_io/read_files_in_folder/read_folder_content.cpp b/tests/system_io/read_files_in_folder/read_folder_content.cpp
index bfa8e2d9f..62d69b20c 100644
--- a/tests/system_io/read_files_in_folder/read_folder_content.cpp
+++ b/tests/system_io/read_files_in_folder/read_folder_content.cpp
@@ -40,6 +40,5 @@ int main(int argc, char* argv[]) {
M_AssertEq(expect, result);
-
return 0;
}
diff --git a/worker/api/cpp/src/folder_data_broker.cpp b/worker/api/cpp/src/folder_data_broker.cpp
index 41ddb9769..a9b6ffd93 100644
--- a/worker/api/cpp/src/folder_data_broker.cpp
+++ b/worker/api/cpp/src/folder_data_broker.cpp
@@ -79,7 +79,7 @@ WorkerErrorCode FolderDataBroker::GetNext(FileInfo* info, FileData* data) {
IOErrors ioerr;
*data = io__->GetDataFromFile(base_path_ + "/" + info->relative_path +
(info->relative_path.empty() ? "" : "/") +
- info->base_name, &ioerr);
+ info->base_name, info->size, &ioerr);
return MapIOError(ioerr);
}
diff --git a/worker/api/cpp/unittests/test_folder_broker.cpp b/worker/api/cpp/unittests/test_folder_broker.cpp
index c5e6ce044..9060e2c72 100644
--- a/worker/api/cpp/unittests/test_folder_broker.cpp
+++ b/worker/api/cpp/unittests/test_folder_broker.cpp
@@ -36,7 +36,7 @@ TEST(FolderDataBroker, SetCorrectIO) {
class FakeIO: public IO {
public:
- FileData GetDataFromFile(const std::string& fname, IOErrors* err) {
+ FileData GetDataFromFile(const std::string& fname, uint64_t fsize, IOErrors* err)override {
*err = IOErrors::NO_ERROR;
return {};
};
@@ -58,6 +58,7 @@ class FakeIO: public IO {
*err = IOErrors::NO_ERROR;
std::vector<FileInfo> file_infos;
FileInfo fi;
+ fi.size = 100;
fi.base_name = "1";
file_infos.push_back(fi);
fi.base_name = "2";
@@ -95,7 +96,7 @@ class IOEmptyFodler: public FakeIO {
class IOCannotOpenFile: public FakeIO {
public:
- FileData GetDataFromFile(const std::string& fname, IOErrors* err) {
+ FileData GetDataFromFile(const std::string& fname, uint64_t fsize, IOErrors* err) {
*err = IOErrors::PERMISSIONS_DENIED;
return {};
};
@@ -168,6 +169,8 @@ TEST_F(FolderDataBrokerTests, GetNextReturnsFileInfo) {
ASSERT_THAT(err, Eq(WorkerErrorCode::OK));
ASSERT_THAT(fi.base_name, Eq("1"));
+ ASSERT_THAT(fi.size, Eq(100));
+
}
TEST_F(FolderDataBrokerTests, SecondNextReturnsAnotherFileInfo) {
@@ -201,53 +204,52 @@ TEST_F(FolderDataBrokerTests, GetNextReturnsErrorWhenFilePermissionsDenied) {
ASSERT_THAT(err, Eq(WorkerErrorCode::PERMISSIONS_DENIED));
}
+
class OpenFileMock : public FakeIO {
public:
- MOCK_METHOD2(GetDataFromFile, FileData(const std::string&, IOErrors*));
+ MOCK_METHOD3(GetDataFromFile, FileData(const std::string&, uint64_t, IOErrors*));
};
-TEST_F(FolderDataBrokerTests, GetNextCallsGetDataFileWithFileName) {
+
+class GetDataFromFileTests : public Test {
+ public:
+ std::unique_ptr<FolderDataBroker> data_broker;
OpenFileMock mock;
- data_broker->io__.reset(&mock);
- data_broker->Connect();
FileInfo fi;
FileData data;
+ void SetUp() override {
+ data_broker = std::unique_ptr<FolderDataBroker> {new FolderDataBroker("/path/to/file")};
+ data_broker->io__ = std::unique_ptr<IO> {&mock};
+ data_broker->Connect();
+ }
+ void TearDown() override {
+ data_broker->io__.release();
+ }
+};
+
+TEST_F(GetDataFromFileTests, GetNextCallsGetDataFileWithFileName) {
+ EXPECT_CALL(mock, GetDataFromFile("/path/to/file/1", _, _)).
+ WillOnce(DoAll(testing::SetArgPointee<2>(IOErrors::NO_ERROR), testing::Return(FileData{})));
- auto err = IOErrors::NO_ERROR;
- EXPECT_CALL(mock, GetDataFromFile("/path/to/file/1", _)).
- WillOnce(DoAll(testing::SetArgPointee<1>(IOErrors::NO_ERROR), testing::Return(FileData{})));
data_broker->GetNext(&fi, &data);
- data_broker->io__.release();
}
-TEST_F(FolderDataBrokerTests, GetNextReturnsData) {
- OpenFileMock mock;
- data_broker->io__.reset(&mock);
- data_broker->Connect();
- FileInfo fi;
- FileData data;
+TEST_F(GetDataFromFileTests, GetNextReturnsData) {
+ EXPECT_CALL(mock, GetDataFromFile(_, _, _)).
+ WillOnce(DoAll(testing::SetArgPointee<2>(IOErrors::NO_ERROR), testing::Return(FileData{'1'})));
- EXPECT_CALL(mock, GetDataFromFile(_, _)).
- WillOnce(DoAll(testing::SetArgPointee<1>(IOErrors::NO_ERROR), testing::Return(FileData{'1'})));
data_broker->GetNext(&fi, &data);
- data_broker->io__.release();
ASSERT_THAT(data[0], Eq('1'));
}
-TEST_F(FolderDataBrokerTests, GetNextReturnsErrorWhenCannotReadData) {
- OpenFileMock mock;
- data_broker->io__.reset(&mock);
- data_broker->Connect();
- FileInfo fi;
- FileData data;
+TEST_F(GetDataFromFileTests, GetNextReturnsErrorWhenCannotReadData) {
+ EXPECT_CALL(mock, GetDataFromFile(_, _, _)).
+ WillOnce(DoAll(testing::SetArgPointee<2>(IOErrors::READ_ERROR), testing::Return(FileData{})));
- EXPECT_CALL(mock, GetDataFromFile(_, _)).
- WillOnce(DoAll(testing::SetArgPointee<1>(IOErrors::READ_ERROR), testing::Return(FileData{})));
auto err = data_broker->GetNext(&fi, &data);
- data_broker->io__.release();
ASSERT_THAT(err, Eq(WorkerErrorCode::ERROR_READING_FROM_SOURCE));
ASSERT_TRUE(data.empty());
--
GitLab