From da98870ec3c4bf1b434c1f312356808cff3f8b59 Mon Sep 17 00:00:00 2001
From: Sergey Yakubov <sergey.yakubov@desy.de>
Date: Fri, 8 Dec 2017 17:28:42 +0100
Subject: [PATCH] added OpenFile and tests
---
CMakeModules/testing_cpp.cmake | 2 +-
common/cpp/include/common/file_info.h | 2 +
common/cpp/include/system_wrappers/io.h | 7 +-
.../cpp/include/system_wrappers/system_io.h | 1 +
common/cpp/src/system_io.cpp | 7 +
common/cpp/src/system_io_linux.cpp | 5 +-
tests/system_io/read_folder_content.cpp | 2 +-
worker/api/cpp/include/worker/data_broker.h | 16 ++-
worker/api/cpp/src/data_broker.cpp | 6 +-
worker/api/cpp/src/folder_data_broker.cpp | 59 +++++++-
worker/api/cpp/src/folder_data_broker.h | 6 +
.../api/cpp/unittests/test_folder_broker.cpp | 132 ++++++++++++++++--
worker/api/cpp/unittests/test_worker_api.cpp | 6 +-
13 files changed, 224 insertions(+), 27 deletions(-)
diff --git a/CMakeModules/testing_cpp.cmake b/CMakeModules/testing_cpp.cmake
index ab96564a5..7f20142f7 100644
--- a/CMakeModules/testing_cpp.cmake
+++ b/CMakeModules/testing_cpp.cmake
@@ -10,7 +10,7 @@ function(gtest target test_source_files test_libraries)
if (BUILD_TESTS)
include_directories(${gtest_SOURCE_DIR}/include ${gtest_SOURCE_DIR})
add_executable(test-${target} ${test_source_files})
- target_link_libraries(test-${target} gtest gtest_main ${CMAKE_THREAD_LIBS_INIT})
+ target_link_libraries(test-${target} gtest gmock gtest_main ${CMAKE_THREAD_LIBS_INIT})
if (NOT ${test_libraries} STREQUAL "")
target_link_libraries(test-${target} ${test_libraries})
endif ()
diff --git a/common/cpp/include/common/file_info.h b/common/cpp/include/common/file_info.h
index e75678442..fe123c60b 100644
--- a/common/cpp/include/common/file_info.h
+++ b/common/cpp/include/common/file_info.h
@@ -1,6 +1,8 @@
#ifndef HIDRA2_FILE_INFO_H
#define HIDRA2_FILE_INFO_H
+#include <chrono>
+
namespace hidra2 {
struct FileInfo {
diff --git a/common/cpp/include/system_wrappers/io.h b/common/cpp/include/system_wrappers/io.h
index ead7c2739..b4bdcd830 100644
--- a/common/cpp/include/system_wrappers/io.h
+++ b/common/cpp/include/system_wrappers/io.h
@@ -12,13 +12,18 @@ namespace hidra2 {
enum class IOErrors {
NO_ERROR,
- FOLDER_NOT_FOUND,
+ FILE_NOT_FOUND,
PERMISSIONS_DENIED,
UNKWOWN_ERROR
};
+IOErrors IOErrorFromErrno();
+
class IO {
public:
+
+ virtual int OpenFileToRead(const std::string& fname, IOErrors* err) = 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;
diff --git a/common/cpp/include/system_wrappers/system_io.h b/common/cpp/include/system_wrappers/system_io.h
index def69d206..13b1ea53c 100644
--- a/common/cpp/include/system_wrappers/system_io.h
+++ b/common/cpp/include/system_wrappers/system_io.h
@@ -7,6 +7,7 @@ namespace hidra2 {
class SystemIO final : public IO {
public:
+ int OpenFileToRead(const std::string& fname, IOErrors* err);
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 34a78141e..df63bab04 100644
--- a/common/cpp/src/system_io.cpp
+++ b/common/cpp/src/system_io.cpp
@@ -2,6 +2,13 @@
#include <unistd.h>
#include <system_wrappers/system_io.h>
+int hidra2::SystemIO::OpenFileToRead(const std::string& fname, IOErrors* err) {
+ int fd = ::open(fname.c_str(), O_RDONLY);
+ *err = IOErrorFromErrno();
+ return fd;
+}
+
+
int hidra2::SystemIO::open(const char* __file, int __oflag) {
return ::open(__file, __oflag);
}
diff --git a/common/cpp/src/system_io_linux.cpp b/common/cpp/src/system_io_linux.cpp
index e74694106..19e517b6d 100644
--- a/common/cpp/src/system_io_linux.cpp
+++ b/common/cpp/src/system_io_linux.cpp
@@ -20,7 +20,7 @@ IOErrors IOErrorFromErrno() {
break;
case ENOENT:
case ENOTDIR:
- err = IOErrors::FOLDER_NOT_FOUND;
+ err = IOErrors::FILE_NOT_FOUND;
break;
case EACCES:
err = IOErrors::PERMISSIONS_DENIED;
@@ -32,6 +32,7 @@ IOErrors IOErrorFromErrno() {
return err;
}
+
bool IsDirectory(const struct dirent* entity) {
return entity->d_type == DT_DIR &&
strstr(entity->d_name, "..") == nullptr &&
@@ -40,7 +41,7 @@ bool IsDirectory(const struct dirent* entity) {
system_clock::time_point GetTimePointFromFile(const string& fname, IOErrors* err) {
- struct stat t_stat{};
+ struct stat t_stat {};
int res = stat(fname.c_str(), &t_stat);
if (res < 0) {
*err = IOErrorFromErrno();
diff --git a/tests/system_io/read_folder_content.cpp b/tests/system_io/read_folder_content.cpp
index 8391c73bc..d924c8aa4 100644
--- a/tests/system_io/read_folder_content.cpp
+++ b/tests/system_io/read_folder_content.cpp
@@ -30,7 +30,7 @@ int main(int argc, char* argv[]) {
std::string result;
switch (err) {
- case IOErrors::FOLDER_NOT_FOUND:
+ case IOErrors::FILE_NOT_FOUND:
result = "notfound";
break;
case IOErrors::NO_ERROR:
diff --git a/worker/api/cpp/include/worker/data_broker.h b/worker/api/cpp/include/worker/data_broker.h
index df931e4ac..216cfe8e9 100644
--- a/worker/api/cpp/include/worker/data_broker.h
+++ b/worker/api/cpp/include/worker/data_broker.h
@@ -4,20 +4,30 @@
#include <memory>
#include <string>
#include <iostream>
+#include <common/file_info.h>
+#include <vector>
namespace hidra2 {
enum class WorkerErrorCode {
- ERR__NO_ERROR,
- ERR__MEMORY_ERROR,
- ERR__EMPTY_DATASOURCE,
+ OK,
+ MEMORY_ERROR,
+ EMPTY_DATASOURCE,
SOURCE_NOT_FOUND,
+ SOURCE_NOT_CONNECTED,
+ SOURCE_ALREADY_CONNECTED,
+ PERMISSIONS_DENIED,
+ NO_DATA,
+ WRONG_INPUT,
UNKNOWN_IO_ERROR
};
+typedef std::vector<char> FileData;
+
class DataBroker {
public:
virtual WorkerErrorCode Connect() = 0;
+ virtual WorkerErrorCode GetNext(FileInfo* info, FileData* data) = 0;
virtual ~DataBroker() = default; // needed for unique_ptr to delete itself
};
diff --git a/worker/api/cpp/src/data_broker.cpp b/worker/api/cpp/src/data_broker.cpp
index cdcd96684..39f158393 100644
--- a/worker/api/cpp/src/data_broker.cpp
+++ b/worker/api/cpp/src/data_broker.cpp
@@ -7,16 +7,16 @@ std::unique_ptr<DataBroker> DataBrokerFactory::Create(const std::string& source_
WorkerErrorCode* return_code) noexcept {
if (source_name.empty()) {
- *return_code = WorkerErrorCode::ERR__EMPTY_DATASOURCE;
+ *return_code = WorkerErrorCode::EMPTY_DATASOURCE;
return nullptr;
}
std::unique_ptr<DataBroker> p = nullptr;
try {
p.reset(new FolderDataBroker(source_name));
- *return_code = WorkerErrorCode::ERR__NO_ERROR;
+ *return_code = WorkerErrorCode::OK;
} catch (...) { // we do not test this part
- *return_code = WorkerErrorCode::ERR__MEMORY_ERROR;
+ *return_code = WorkerErrorCode::MEMORY_ERROR;
}
return p;
diff --git a/worker/api/cpp/src/folder_data_broker.cpp b/worker/api/cpp/src/folder_data_broker.cpp
index e464ac107..85be42d47 100644
--- a/worker/api/cpp/src/folder_data_broker.cpp
+++ b/worker/api/cpp/src/folder_data_broker.cpp
@@ -1,7 +1,5 @@
#include "folder_data_broker.h"
-#include <map>
-
#include "system_wrappers/system_io.h"
namespace hidra2 {
@@ -10,11 +8,14 @@ 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;
+ err = WorkerErrorCode::OK;
break;
- case IOErrors::FOLDER_NOT_FOUND:
+ case IOErrors::FILE_NOT_FOUND:
err = WorkerErrorCode::SOURCE_NOT_FOUND;
break;
+ case IOErrors::PERMISSIONS_DENIED:
+ err = WorkerErrorCode::PERMISSIONS_DENIED;
+ break;
default:
err = WorkerErrorCode::UNKNOWN_IO_ERROR;
break;
@@ -24,15 +25,61 @@ WorkerErrorCode MapIOError(IOErrors io_err) {
}
-FolderDataBroker::FolderDataBroker(const std::string& source_name) : base_path_{source_name},
- io__{new hidra2::SystemIO} {
+FolderDataBroker::FolderDataBroker(const std::string& source_name) :
+ base_path_{source_name}, io__{new hidra2::SystemIO}, is_connected_{false},
+current_file_{0} {
}
WorkerErrorCode FolderDataBroker::Connect() {
IOErrors io_err;
+ if (is_connected_) {
+ return WorkerErrorCode::SOURCE_ALREADY_CONNECTED;
+ }
+
filelist_ = io__->FilesInFolder(base_path_, &io_err);
+
+ if (io_err == IOErrors::NO_ERROR) {
+ is_connected_ = true;
+ }
return MapIOError(io_err);
}
+WorkerErrorCode FolderDataBroker::CheckCanGetData(FileInfo* info, FileData* data) {
+ if (!is_connected_) {
+ return WorkerErrorCode::SOURCE_NOT_CONNECTED;
+ }
+
+ if (info == nullptr && data == nullptr) {
+ return WorkerErrorCode::WRONG_INPUT;
+ }
+
+ if (current_file_ >= filelist_.size()) {
+ return WorkerErrorCode::NO_DATA;
+ }
+ return WorkerErrorCode::OK;
+}
+
+
+WorkerErrorCode FolderDataBroker::GetNext(FileInfo* info, FileData* data) {
+ auto err = CheckCanGetData(info, data);
+ if (err != WorkerErrorCode::OK) {
+ return err;
+ }
+
+ *info = filelist_[current_file_];
+ current_file_++;
+
+ if (data == nullptr) {
+ return WorkerErrorCode::OK;
+ }
+
+ IOErrors ioerr;
+ auto fd = io__->OpenFileToRead(base_path_+"/"+info->relative_path +
+ (info->relative_path.empty()?"":"/")+
+ info->base_name, &ioerr);
+
+ return MapIOError(ioerr);
+}
+
}
\ No newline at end of file
diff --git a/worker/api/cpp/src/folder_data_broker.h b/worker/api/cpp/src/folder_data_broker.h
index 564751158..3de03b058 100644
--- a/worker/api/cpp/src/folder_data_broker.h
+++ b/worker/api/cpp/src/folder_data_broker.h
@@ -14,11 +14,17 @@ class FolderDataBroker final : public hidra2::DataBroker {
public:
explicit FolderDataBroker(const std::string& source_name);
WorkerErrorCode Connect() override;
+ WorkerErrorCode GetNext(FileInfo* info, FileData* data) override;
+
std::unique_ptr<hidra2::IO> io__; // modified in testings to mock system calls,otherwise do not touch
private:
+ bool is_connected_;
+ int current_file_;
std::string base_path_;
std::vector<FileInfo> filelist_;
+ WorkerErrorCode CheckCanGetData(FileInfo* info, FileData* data);
+
};
}
diff --git a/worker/api/cpp/unittests/test_folder_broker.cpp b/worker/api/cpp/unittests/test_folder_broker.cpp
index a8b891b1e..8bec87e88 100644
--- a/worker/api/cpp/unittests/test_folder_broker.cpp
+++ b/worker/api/cpp/unittests/test_folder_broker.cpp
@@ -1,4 +1,6 @@
#include <gmock/gmock.h>
+#include "gtest/gtest.h"
+using ::testing::AtLeast;
#include "worker/data_broker.h"
#include "system_wrappers/io.h"
@@ -12,10 +14,15 @@ using hidra2::WorkerErrorCode;
using hidra2::IO;
using hidra2::IOErrors;
using hidra2::FileInfo;
+using hidra2::FileData;
+
using ::testing::Eq;
using ::testing::Ne;
using ::testing::Test;
+using ::testing::_;
+using ::testing::Mock;
+
namespace {
@@ -29,9 +36,15 @@ TEST(FolderDataBroker, SetCorrectIO) {
class FakeIO: public IO {
public:
+ int OpenFileToRead(const std::string& fname, IOErrors* err) {
+ *err = IOErrors::NO_ERROR;
+ return 1;
+ };
+
int open(const char* __file, int __oflag) {
return 0;
};
+
int close(int __fd) {
return 0;
};
@@ -43,14 +56,23 @@ class FakeIO: public IO {
};
std::vector<FileInfo> FilesInFolder(const std::string& folder, IOErrors* err) {
*err = IOErrors::NO_ERROR;
- return {};
+ std::vector<FileInfo> file_infos;
+ FileInfo fi;
+ fi.base_name = "1";
+ file_infos.push_back(fi);
+ fi.base_name = "2";
+ file_infos.push_back(fi);
+ fi.base_name = "3";
+ file_infos.push_back(fi);
+
+ return file_infos;
}
};
class IOFolderNotFound: public FakeIO {
public:
std::vector<FileInfo> FilesInFolder(const std::string& folder, IOErrors* err) {
- *err = IOErrors::FOLDER_NOT_FOUND;
+ *err = IOErrors::FILE_NOT_FOUND;
return {};
}
};
@@ -63,6 +85,24 @@ class IOFodlerUnknownError: public FakeIO {
}
};
+class IOEmptyFodler: public FakeIO {
+ public:
+ std::vector<FileInfo> FilesInFolder(const std::string& folder, IOErrors* err) {
+ *err = IOErrors::NO_ERROR;
+ return {};
+ }
+};
+
+class IOCannotOpenFile: public FakeIO {
+ public:
+ int OpenFileToRead(const std::string& fname, IOErrors* err) {
+ *err = IOErrors::PERMISSIONS_DENIED;
+ return 1;
+ };
+};
+
+
+
class FolderDataBrokerTests : public Test {
public:
std::unique_ptr<FolderDataBroker> data_broker;
@@ -77,9 +117,18 @@ class FolderDataBrokerTests : public Test {
TEST_F(FolderDataBrokerTests, CanConnect) {
auto return_code = data_broker->Connect();
- ASSERT_THAT(return_code, Eq(WorkerErrorCode::ERR__NO_ERROR));
+ ASSERT_THAT(return_code, Eq(WorkerErrorCode::OK));
+}
+
+TEST_F(FolderDataBrokerTests, CannotConnectTwice) {
+ data_broker->Connect();
+
+ auto return_code = data_broker->Connect();
+
+ ASSERT_THAT(return_code, Eq(WorkerErrorCode::SOURCE_ALREADY_CONNECTED));
}
+
TEST_F(FolderDataBrokerTests, CannotConnectWhenNoFolder) {
data_broker->io__ = std::unique_ptr<IO> {new IOFolderNotFound()};
@@ -96,13 +145,80 @@ TEST_F(FolderDataBrokerTests, ConnectReturnsUnknownIOError) {
ASSERT_THAT(return_code, Eq(WorkerErrorCode::UNKNOWN_IO_ERROR));
}
-/*TEST_F(FolderDataBrokerTests, GetNextWithouConnectReturnsError) {
- WorkerErrorCode err;
-
- auto err=data_broker->GetNext(nullptr,nullptr);
+TEST_F(FolderDataBrokerTests, GetNextWithoutConnectReturnsError) {
+ auto err = data_broker->GetNext(nullptr, nullptr);
ASSERT_THAT(err, Eq(WorkerErrorCode::SOURCE_NOT_CONNECTED));
}
-*/
+TEST_F(FolderDataBrokerTests, GetNextWithNullPointersReturnsError) {
+ data_broker->Connect();
+
+ auto err = data_broker->GetNext(nullptr, nullptr);
+
+ ASSERT_THAT(err, Eq(WorkerErrorCode::WRONG_INPUT));
+}
+
+
+TEST_F(FolderDataBrokerTests, GetNextReturnsFileInfo) {
+ data_broker->Connect();
+ FileInfo fi;
+
+ auto err = data_broker->GetNext(&fi, nullptr);
+
+ ASSERT_THAT(err, Eq(WorkerErrorCode::OK));
+ ASSERT_THAT(fi.base_name, Eq("1"));
+}
+
+TEST_F(FolderDataBrokerTests, SecondNextReturnsAnotherFileInfo) {
+ data_broker->Connect();
+ FileInfo fi;
+ data_broker->GetNext(&fi, nullptr);
+
+ auto err = data_broker->GetNext(&fi, nullptr);
+
+ ASSERT_THAT(err, Eq(WorkerErrorCode::OK));
+ ASSERT_THAT(fi.base_name, Eq("2"));
+}
+
+TEST_F(FolderDataBrokerTests, GetNextFromEmptyFolderReturnsError) {
+ data_broker->io__ = std::unique_ptr<IO> {new IOEmptyFodler()};
+ data_broker->Connect();
+ FileInfo fi;
+
+ auto err = data_broker->GetNext(&fi, nullptr);
+ ASSERT_THAT(err, Eq(WorkerErrorCode::NO_DATA));
+}
+
+
+TEST_F(FolderDataBrokerTests, GetNextReturnsErrorWhenFilePermissionsDenied) {
+ data_broker->io__ = std::unique_ptr<IO> {new IOCannotOpenFile()};
+ data_broker->Connect();
+ FileInfo fi;
+ FileData data;
+
+ auto err = data_broker->GetNext(&fi, &data);
+ ASSERT_THAT(err, Eq(WorkerErrorCode::PERMISSIONS_DENIED));
+}
+
+class OpenFileMock : public FakeIO {
+ public:
+ MOCK_METHOD2(OpenFileToRead, int(const std::string&, IOErrors*));
+};
+
+TEST_F(FolderDataBrokerTests, GetNextCallsOpenFileWithFileName) {
+ OpenFileMock* mock=new OpenFileMock;
+ data_broker->io__.reset(mock);
+ data_broker->Connect();
+ FileInfo fi;
+ FileData data;
+
+ EXPECT_CALL(*mock, OpenFileToRead("/path/to/file/1",_));
+ data_broker->GetNext(&fi, &data);
+
+ Mock::AllowLeak(mock);
+
+}
+
+
}
diff --git a/worker/api/cpp/unittests/test_worker_api.cpp b/worker/api/cpp/unittests/test_worker_api.cpp
index 681a99462..ea35b54ac 100644
--- a/worker/api/cpp/unittests/test_worker_api.cpp
+++ b/worker/api/cpp/unittests/test_worker_api.cpp
@@ -12,6 +12,8 @@ using ::testing::Eq;
using ::testing::Ne;
using ::testing::Test;
+
+
namespace {
TEST(DataBrokerFactoryTests, CreateFolderDataSource) {
@@ -19,7 +21,7 @@ TEST(DataBrokerFactoryTests, CreateFolderDataSource) {
auto data_broker = DataBrokerFactory::Create("path/to/file", &return_code);
- ASSERT_THAT(return_code, Eq(WorkerErrorCode::ERR__NO_ERROR));
+ ASSERT_THAT(return_code, Eq(WorkerErrorCode::OK));
ASSERT_THAT(dynamic_cast<FolderDataBroker*>(data_broker.get()), Ne(nullptr));
}
@@ -28,7 +30,7 @@ TEST(DataBrokerFactoryTests, FailCreateDataSourceWithEmptySource) {
auto data_broker = DataBrokerFactory::Create("", &return_code);
- ASSERT_THAT(return_code, Eq(WorkerErrorCode::ERR__EMPTY_DATASOURCE));
+ ASSERT_THAT(return_code, Eq(WorkerErrorCode::EMPTY_DATASOURCE));
ASSERT_THAT(data_broker.release(), Eq(nullptr));
}
--
GitLab