From 491d754084a387f385718518832242e1e40682e1 Mon Sep 17 00:00:00 2001
From: Sergey Yakubov <sergey.yakubov@desy.de>
Date: Wed, 6 Dec 2017 18:20:00 +0100
Subject: [PATCH] refactoring, read filelist from a given folder
---
CMakeModules/astyle.cmake | 4 +-
common/cpp/CMakeLists.txt | 8 +-
common/cpp/include/system_wrappers/io.h | 17 ++-
.../cpp/include/system_wrappers/system_io.h | 9 +-
common/cpp/src/system_io.cpp | 6 +-
common/cpp/src/system_io_linux.cpp | 125 +++++++++++++++++-
producer/api/src/producer/producer.cpp | 2 +-
tests/CMakeLists.txt | 1 +
tests/system_io/CMakeLists.txt | 32 ++++-
tests/system_io/cleanup.sh | 4 +
tests/system_io/read_folder_content.cpp | 34 +++--
tests/system_io/setup.sh | 14 ++
worker/api/cpp/include/worker/data_broker.h | 4 +-
worker/api/cpp/src/data_broker.cpp | 2 +-
worker/api/cpp/src/folder_data_broker.cpp | 18 +--
worker/api/cpp/src/folder_data_broker.h | 2 +-
.../api/cpp/unittests/test_folder_broker.cpp | 27 ++--
17 files changed, 246 insertions(+), 63 deletions(-)
create mode 100644 tests/system_io/cleanup.sh
create mode 100644 tests/system_io/setup.sh
diff --git a/CMakeModules/astyle.cmake b/CMakeModules/astyle.cmake
index ad5a7695f..326e6697b 100644
--- a/CMakeModules/astyle.cmake
+++ b/CMakeModules/astyle.cmake
@@ -6,8 +6,8 @@ if(ASTYLE_EXECUTABLE)
COMMAND
${ASTYLE_EXECUTABLE} -i
--exclude=${PROJECT_BINARY_DIR}
- --recursive -n --style=google --indent=spaces=4
- --max-instatement-indent=120
+ --recursive -n --style=google --indent=spaces=4 --max-code-length=120
+ --max-instatement-indent=50 --pad-oper --align-pointer=type
"${PROJECT_SOURCE_DIR}/*.cpp" "${PROJECT_SOURCE_DIR}/*.h"
WORKING_DIRECTORY ${PROJECT_SOURCE_DIR}
VERBATIM
diff --git a/common/cpp/CMakeLists.txt b/common/cpp/CMakeLists.txt
index 9d5e62bad..1549c800f 100644
--- a/common/cpp/CMakeLists.txt
+++ b/common/cpp/CMakeLists.txt
@@ -4,8 +4,12 @@ set(SOURCE_FILES
include/common/networking.h
include/system_wrappers/io.h
include/system_wrappers/system_io.h
- src/system_io.cpp
- src/system_io_linux.cpp)
+ src/system_io.cpp)
+IF(WIN32)
+ # Do windows specific includes
+ELSEIF(UNIX)
+ set(SOURCE_FILES ${SOURCE_FILES} src/system_io_linux.cpp)
+ENDIF(WIN32)
################################
diff --git a/common/cpp/include/system_wrappers/io.h b/common/cpp/include/system_wrappers/io.h
index 371bae4ff..ac35b38f9 100644
--- a/common/cpp/include/system_wrappers/io.h
+++ b/common/cpp/include/system_wrappers/io.h
@@ -3,10 +3,17 @@
#include <string>
#include <vector>
-
+#include <chrono>
namespace hidra2 {
+struct FileInfo {
+ std::string base_name;
+ std::string relative_path;
+ std::chrono::system_clock::time_point modify_date;
+};
+
+
enum class IOErrors {
NO_ERROR,
FOLDER_NOT_FOUND,
@@ -16,13 +23,13 @@ enum class IOErrors {
class IO {
public:
- virtual int open(const char *__file, int __oflag) = 0;
+ virtual int open(const char* __file, int __oflag) = 0;
virtual int close(int __fd) = 0;
- virtual ssize_t read(int __fd, void *buf, size_t count) = 0;
- virtual ssize_t write(int __fd, const void *__buf, size_t __n) = 0;
+ virtual ssize_t read(int __fd, void* buf, size_t count) = 0;
+ virtual ssize_t write(int __fd, const void* __buf, size_t __n) = 0;
// this is not standard function - to be implemented differently in windows and linux
- virtual std::vector<std::string> FilesInFolder(std::string folder,IOErrors* err) = 0;
+ virtual std::vector<FileInfo> FilesInFolder(const std::string& folder, IOErrors* err) = 0;
};
}
diff --git a/common/cpp/include/system_wrappers/system_io.h b/common/cpp/include/system_wrappers/system_io.h
index 166068dd7..def69d206 100644
--- a/common/cpp/include/system_wrappers/system_io.h
+++ b/common/cpp/include/system_wrappers/system_io.h
@@ -4,13 +4,14 @@
#include "io.h"
namespace hidra2 {
+
class SystemIO final : public IO {
public:
- int open(const char *__file, int __oflag);
+ int open(const char* __file, int __oflag);
int close(int __fd);
- ssize_t read(int __fd, void *buf, size_t count);
- ssize_t write(int __fd, const void *__buf, size_t __n);
- std::vector<std::string> FilesInFolder(std::string folder,IOErrors* err);
+ ssize_t read(int __fd, void* buf, size_t count);
+ ssize_t write(int __fd, const void* __buf, size_t __n);
+ std::vector<FileInfo> FilesInFolder(const std::string& folder, IOErrors* err);
};
}
diff --git a/common/cpp/src/system_io.cpp b/common/cpp/src/system_io.cpp
index 96bee7116..34a78141e 100644
--- a/common/cpp/src/system_io.cpp
+++ b/common/cpp/src/system_io.cpp
@@ -2,7 +2,7 @@
#include <unistd.h>
#include <system_wrappers/system_io.h>
-int hidra2::SystemIO::open(const char *__file, int __oflag) {
+int hidra2::SystemIO::open(const char* __file, int __oflag) {
return ::open(__file, __oflag);
}
@@ -10,10 +10,10 @@ int hidra2::SystemIO::close(int __fd) {
return ::close(__fd);
}
-ssize_t hidra2::SystemIO::read(int __fd, void *buf, size_t count) {
+ssize_t hidra2::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 hidra2::SystemIO::write(int __fd, const void* __buf, size_t __n) {
return ::write(__fd, __buf, __n);
}
diff --git a/common/cpp/src/system_io_linux.cpp b/common/cpp/src/system_io_linux.cpp
index 21e4edfad..7ae3486c2 100644
--- a/common/cpp/src/system_io_linux.cpp
+++ b/common/cpp/src/system_io_linux.cpp
@@ -1,10 +1,127 @@
#include "system_wrappers/system_io.h"
+#include <cstring>
+
+#include <dirent.h>
+#include <sys/stat.h>
+#include <algorithm>
+
+using std::string;
+using std::vector;
+using std::chrono::system_clock;
+
namespace hidra2 {
-std::vector<std::string> SystemIO::FilesInFolder(std::string folder, IOErrors* err) {
- *err=IOErrors::FOLDER_NOT_FOUND;
- return {};
+IOErrors IOErrorFromErrno() {
+ IOErrors err;
+ switch (errno) {
+ case 0:
+ err = IOErrors::NO_ERROR;
+ break;
+ case ENOENT:
+ case ENOTDIR:
+ err = IOErrors::FOLDER_NOT_FOUND;
+ break;
+ case EACCES:
+ err = IOErrors::PERMISSIONS_DENIED;
+ break;
+ default:
+ err = IOErrors::UNKWOWN_ERROR;
+ break;
+ }
+ return err;
}
-}
\ No newline at end of file
+bool IsDirectory(const struct dirent* entity) {
+ return entity->d_type == DT_DIR &&
+ strstr(entity->d_name, "..") == nullptr &&
+ 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{};
+ }
+
+ std::chrono::nanoseconds d = std::chrono::nanoseconds{t_stat.st_mtim.tv_nsec} +
+ std::chrono::seconds{t_stat.st_mtim.tv_sec};
+ return system_clock::time_point {std::chrono::duration_cast<system_clock::duration>(d)};
+}
+
+void ProcessFileEntity(const struct dirent* entity, const std::string& path,
+ std::vector<FileInfo>& files, IOErrors* err) {
+
+ *err = IOErrors::NO_ERROR;
+ 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);
+ if (*err != IOErrors::NO_ERROR) {
+ return;
+ }
+
+ files.push_back(file_info);
+}
+
+void CollectFileInformationRecursivly(const std::string& path,
+ std::vector<FileInfo>& files, IOErrors* err) {
+ auto dir = opendir((path).c_str());
+ if (dir == nullptr) {
+ *err = IOErrorFromErrno();
+ return;
+ }
+
+ while (struct dirent* current_entity = readdir(dir)) {
+ if (IsDirectory(current_entity)) {
+ CollectFileInformationRecursivly(path + "/" + current_entity->d_name,
+ files, err);
+ if (*err != IOErrors::NO_ERROR) {
+ closedir(dir);
+ return;
+ }
+ }
+ ProcessFileEntity(current_entity, path, files, err);
+ if (*err != IOErrors::NO_ERROR) {
+ closedir(dir);
+ return;
+ }
+
+ }
+ *err = IOErrorFromErrno();
+ closedir(dir);
+}
+
+void SortFileList(std::vector<FileInfo>& file_list) {
+ std::sort(file_list.begin(), file_list.end(),
+ [](FileInfo const & a, FileInfo const & b) {
+ return a.modify_date < b.modify_date;
+ });
+}
+
+void StripBasePath(const string& folder, std::vector<FileInfo>& file_list) {
+ auto n_erase = folder.size() + 1;
+ for (auto& file : file_list) {
+ file.relative_path.erase(0, n_erase);
+ }
+}
+
+std::vector<FileInfo> SystemIO::FilesInFolder(const string& folder, IOErrors* err) {
+ std::vector<FileInfo> files{};
+ CollectFileInformationRecursivly(folder, files, err);
+ if (*err != IOErrors::NO_ERROR) {
+ return {};
+ }
+ StripBasePath(folder, files);
+ SortFileList(files);
+ return files;
+}
+
+}
diff --git a/producer/api/src/producer/producer.cpp b/producer/api/src/producer/producer.cpp
index 15a6fca30..f4373a91d 100644
--- a/producer/api/src/producer/producer.cpp
+++ b/producer/api/src/producer/producer.cpp
@@ -7,6 +7,6 @@ hidra2::Producer::Producer() {
kinit_count_++;
}
-hidra2::Producer *hidra2::Producer::CreateProducer(std::string receiver_address) {
+hidra2::Producer* hidra2::Producer::CreateProducer(std::string receiver_address) {
return new Producer();
}
diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt
index 0c4ded43f..a72ee6770 100644
--- a/tests/CMakeLists.txt
+++ b/tests/CMakeLists.txt
@@ -1,3 +1,4 @@
+CMAKE_MINIMUM_REQUIRED(VERSION 3.7) # needed for fixtures
add_subdirectory(system_io)
diff --git a/tests/system_io/CMakeLists.txt b/tests/system_io/CMakeLists.txt
index ed34b6946..15ed86b24 100644
--- a/tests/system_io/CMakeLists.txt
+++ b/tests/system_io/CMakeLists.txt
@@ -13,5 +13,33 @@ set_target_properties(${TARGET_NAME} PROPERTIES LINKER_LANGUAGE CXX)
# Testing
################################
-add_test(NAME test-${TARGET_NAME} COMMAND ${TARGET_NAME} test notfound)
-set_tests_properties(test-${TARGET_NAME} PROPERTIES LABELS "integration;all")
+add_test(NAME test-${TARGET_NAME}-create_list COMMAND ${TARGET_NAME} test 23subtest/subtest241)
+add_test(NAME test-${TARGET_NAME}-foldernotfound COMMAND ${TARGET_NAME} test_notexist notfound)
+add_test(NAME test-${TARGET_NAME}-foldernoaccess COMMAND ${TARGET_NAME} test_noaccess noaccess)
+
+
+set_tests_properties(test-${TARGET_NAME}-create_list PROPERTIES
+ LABELS "integration;all"
+ FIXTURES_REQUIRED ${TARGET_NAME}-fixture
+ )
+
+set_tests_properties(test-${TARGET_NAME}-foldernotfound PROPERTIES
+ LABELS "integration;all"
+# FIXTURES_REQUIRED ${TARGET_NAME}-fixture
+ )
+
+
+set_tests_properties(test-${TARGET_NAME}-foldernoaccess PROPERTIES
+ LABELS "integration;all"
+ FIXTURES_REQUIRED ${TARGET_NAME}-fixture
+ )
+
+
+add_test(NAME test-${TARGET_NAME}-setup COMMAND bash ${CMAKE_CURRENT_SOURCE_DIR}/setup.sh)
+add_test(NAME test-${TARGET_NAME}-cleanup COMMAND bash ${CMAKE_CURRENT_SOURCE_DIR}/cleanup.sh)
+set_tests_properties(test-${TARGET_NAME}-setup PROPERTIES FIXTURES_SETUP ${TARGET_NAME}-fixture)
+set_tests_properties(test-${TARGET_NAME}-cleanup PROPERTIES FIXTURES_CLEANUP ${TARGET_NAME}-fixture)
+
+
+
+
diff --git a/tests/system_io/cleanup.sh b/tests/system_io/cleanup.sh
new file mode 100644
index 000000000..dbb103528
--- /dev/null
+++ b/tests/system_io/cleanup.sh
@@ -0,0 +1,4 @@
+#!/usr/bin/env bash
+
+rm -rf test
+rm -rf test_noaccess
diff --git a/tests/system_io/read_folder_content.cpp b/tests/system_io/read_folder_content.cpp
index 0048fe286..cb4b24c10 100644
--- a/tests/system_io/read_folder_content.cpp
+++ b/tests/system_io/read_folder_content.cpp
@@ -1,43 +1,49 @@
#include <iostream>
-#include <string>
+#include <memory>
#include <system_wrappers/system_io.h>
using hidra2::SystemIO;
using hidra2::IOErrors;
-#
-
-void M_Assert(bool expr, std::string expected, std::string got, std::string msg) {
- if (!expr) {
- std::cerr << "Assert failed:\t" << msg << "\n"
+void M_AssertEq(const std::string& expected, const std::string& got) {
+ if (expected.compare(got) != 0) {
+ std::cerr << "Assert failed:\n"
<< "Expected:\t'" << expected << "'\n"
<< "Obtained:\t'" << got << "'\n";
abort();
}
}
-
int main(int argc, char* argv[]) {
- auto io = new SystemIO;
-
if (argc != 3) {
std::cout << "Wrong number of arguments" << std::endl;
return 1;
}
-
std::string expect{argv[2]};
IOErrors err;
-
+ auto io = std::unique_ptr<SystemIO> {new SystemIO};
auto files = io->FilesInFolder(argv[1], &err);
+ std::string result;
+
switch (err) {
case IOErrors::FOLDER_NOT_FOUND:
- M_Assert(expect.compare("notfound")==0,expect,"notfound","Folder not found");
- return 0;
+ result = "notfound";
+ break;
+ case IOErrors::NO_ERROR:
+ for(auto file_info : files)
+ result += file_info.relative_path + file_info.base_name;
+ break;
+ case IOErrors::PERMISSIONS_DENIED:
+ result = "noaccess";
+ break;
}
- return 1;
+ M_AssertEq(expect, result);
+
+
+ return 0;
}
diff --git a/tests/system_io/setup.sh b/tests/system_io/setup.sh
new file mode 100644
index 000000000..ad7a62176
--- /dev/null
+++ b/tests/system_io/setup.sh
@@ -0,0 +1,14 @@
+#!/usr/bin/env bash
+
+mkdir -p test/subtest/subtest2
+touch test/2
+sleep 0.01
+touch test/3
+sleep 0.01
+touch test/subtest/subtest2/4
+sleep 0.01
+touch test/1
+
+mkdir test_noaccess
+chmod -rx test_noaccess
+
diff --git a/worker/api/cpp/include/worker/data_broker.h b/worker/api/cpp/include/worker/data_broker.h
index 7898a8b06..38a263f36 100644
--- a/worker/api/cpp/include/worker/data_broker.h
+++ b/worker/api/cpp/include/worker/data_broker.h
@@ -17,12 +17,12 @@ enum class WorkerErrorCode {
class DataBroker {
public:
- virtual WorkerErrorCode Connect()=0;
+ virtual WorkerErrorCode Connect() = 0;
};
class DataBrokerFactory {
public:
- static std::unique_ptr<DataBroker> Create(const std::string &source_name, WorkerErrorCode* return_code) noexcept;
+ static std::unique_ptr<DataBroker> Create(const std::string& source_name, WorkerErrorCode* return_code) noexcept;
};
}
diff --git a/worker/api/cpp/src/data_broker.cpp b/worker/api/cpp/src/data_broker.cpp
index e99844ff9..f412eaf1c 100644
--- a/worker/api/cpp/src/data_broker.cpp
+++ b/worker/api/cpp/src/data_broker.cpp
@@ -4,7 +4,7 @@
namespace hidra2 {
std::unique_ptr<DataBroker> DataBrokerFactory::Create(const std::string& source_name,
- WorkerErrorCode* return_code) noexcept {
+ WorkerErrorCode* return_code) noexcept {
if (source_name.empty()) {
*return_code = WorkerErrorCode::ERR__EMPTY_DATASOURCE;
diff --git a/worker/api/cpp/src/folder_data_broker.cpp b/worker/api/cpp/src/folder_data_broker.cpp
index 1dc555936..e464ac107 100644
--- a/worker/api/cpp/src/folder_data_broker.cpp
+++ b/worker/api/cpp/src/folder_data_broker.cpp
@@ -9,15 +9,15 @@ namespace hidra2 {
WorkerErrorCode MapIOError(IOErrors io_err) {
WorkerErrorCode err;
switch (io_err) { // we do not use map due to performance reasons
- case IOErrors::NO_ERROR:
- err = WorkerErrorCode::ERR__NO_ERROR;
- break;
- case IOErrors::FOLDER_NOT_FOUND:
- err = WorkerErrorCode::SOURCE_NOT_FOUND;
- break;
- default:
- err = WorkerErrorCode::UNKNOWN_IO_ERROR;
- break;
+ case IOErrors::NO_ERROR:
+ err = WorkerErrorCode::ERR__NO_ERROR;
+ break;
+ case IOErrors::FOLDER_NOT_FOUND:
+ err = WorkerErrorCode::SOURCE_NOT_FOUND;
+ break;
+ default:
+ err = WorkerErrorCode::UNKNOWN_IO_ERROR;
+ break;
}
return err;
diff --git a/worker/api/cpp/src/folder_data_broker.h b/worker/api/cpp/src/folder_data_broker.h
index ed3010a23..bdecc943d 100644
--- a/worker/api/cpp/src/folder_data_broker.h
+++ b/worker/api/cpp/src/folder_data_broker.h
@@ -17,7 +17,7 @@ class FolderDataBroker final : public hidra2::DataBroker {
std::unique_ptr<hidra2::IO> io__; // modified in testings to mock system calls,otherwise do not touch
private:
std::string base_path_;
- std::vector<std::string> filelist_;
+ std::vector<FileInfo> filelist_;
};
}
diff --git a/worker/api/cpp/unittests/test_folder_broker.cpp b/worker/api/cpp/unittests/test_folder_broker.cpp
index f9b9b802a..b4b3ef926 100644
--- a/worker/api/cpp/unittests/test_folder_broker.cpp
+++ b/worker/api/cpp/unittests/test_folder_broker.cpp
@@ -13,6 +13,7 @@ using hidra2::FolderDataBroker;
using hidra2::WorkerErrorCode;
using hidra2::IO;
using hidra2::IOErrors;
+using hidra2::FileInfo;
using ::testing::Eq;
using ::testing::Ne;
@@ -21,7 +22,7 @@ using ::testing::Test;
namespace {
TEST(FolderDataBroker, SetCorrectIO) {
- auto data_broker=new FolderDataBroker("test");
+ auto data_broker = new FolderDataBroker("test");
ASSERT_THAT(dynamic_cast<hidra2::SystemIO*>(data_broker->io__.release()), Ne(nullptr));
}
@@ -29,36 +30,36 @@ TEST(FolderDataBroker, SetCorrectIO) {
class FakeIO: public IO {
public:
- int open(const char *__file, int __oflag) {
+ int open(const char* __file, int __oflag) {
return 0;
};
int close(int __fd) {
return 0;
};
- ssize_t read(int __fd, void *buf, size_t count) {
+ ssize_t read(int __fd, void* buf, size_t count) {
return 0;
};
- ssize_t write(int __fd, const void *__buf, size_t __n) {
+ ssize_t write(int __fd, const void* __buf, size_t __n) {
return 0;
};
- std::vector<std::string> FilesInFolder(std::string folder, IOErrors* err) {
- *err=IOErrors::NO_ERROR;
+ std::vector<FileInfo> FilesInFolder(const std::string& folder, IOErrors* err) {
+ *err = IOErrors::NO_ERROR;
return {};
}
};
class IOFolderNotFound: public FakeIO {
public:
- std::vector<std::string> FilesInFolder(std::string folder, IOErrors* err) {
- *err=IOErrors::FOLDER_NOT_FOUND;
+ std::vector<FileInfo> FilesInFolder(const std::string& folder, IOErrors* err) {
+ *err = IOErrors::FOLDER_NOT_FOUND;
return {};
}
};
class IOFodlerUnknownError: public FakeIO {
public:
- std::vector<std::string> FilesInFolder(std::string folder, IOErrors* err) {
- *err=IOErrors::UNKWOWN_ERROR;
+ std::vector<FileInfo> FilesInFolder(const std::string& folder, IOErrors* err) {
+ *err = IOErrors::UNKWOWN_ERROR;
return {};
}
};
@@ -69,7 +70,7 @@ class FolderDataBrokerTests : public Test {
std::unique_ptr<FolderDataBroker> data_broker;
void SetUp() override {
data_broker = std::unique_ptr<FolderDataBroker> {new FolderDataBroker("/path/to/file")};
- data_broker->io__=std::unique_ptr<IO> {new FakeIO()};
+ data_broker->io__ = std::unique_ptr<IO> {new FakeIO()};
}
void TearDown() override {
}
@@ -82,7 +83,7 @@ TEST_F(FolderDataBrokerTests, CanConnect) {
}
TEST_F(FolderDataBrokerTests, CannotConnectWhenNoFolder) {
- data_broker->io__=std::unique_ptr<IO> {new IOFolderNotFound()};
+ data_broker->io__ = std::unique_ptr<IO> {new IOFolderNotFound()};
auto return_code = data_broker->Connect();
@@ -90,7 +91,7 @@ TEST_F(FolderDataBrokerTests, CannotConnectWhenNoFolder) {
}
TEST_F(FolderDataBrokerTests, ConnectReturnsUnknownIOError) {
- data_broker->io__=std::unique_ptr<IO> {new IOFodlerUnknownError()};
+ data_broker->io__ = std::unique_ptr<IO> {new IOFodlerUnknownError()};
auto return_code = data_broker->Connect();
--
GitLab