diff --git a/CMakeModules/testing_go.cmake b/CMakeModules/testing_go.cmake index 715aab8fa84617deea80eba99619045744d7f0bb..3fcdb6e501075a3e234e2c95dab63b52be6fcd8a 100644 --- a/CMakeModules/testing_go.cmake +++ b/CMakeModules/testing_go.cmake @@ -3,7 +3,7 @@ if (BUILD_TESTS) endif () if (BUILD_TESTS) - set(HIDRA2_MINIMUM_COVERAGE 90) + set(HIDRA2_MINIMUM_COVERAGE 80) find_program(MEMORYCHECK_COMMAND valgrind) set(MEMORYCHECK_COMMAND_OPTIONS "--trace-children=yes --leak-check=full --error-exitcode=1 --suppressions=${CMAKE_SOURCE_DIR}/tests/valgrind.suppressions") diff --git a/broker/src/hidra2_broker/server/get_next.go b/broker/src/hidra2_broker/server/get_next.go index af1017798f460072b50ce1dd91b8030c95596f37..efdcf40746b1cfa586a0bf306f13961312dd9615 100644 --- a/broker/src/hidra2_broker/server/get_next.go +++ b/broker/src/hidra2_broker/server/get_next.go @@ -1,14 +1,16 @@ package server import ( + "github.com/gorilla/mux" "hidra2_broker/database" "hidra2_broker/utils" "net/http" ) func extractRequestParameters(r *http.Request) (string, bool) { - db_name := r.URL.Query().Get("database") - return db_name, db_name != "" + vars := mux.Vars(r) + db_name, ok := vars["dbname"] + return db_name, ok } func routeGetNext(w http.ResponseWriter, r *http.Request) { diff --git a/broker/src/hidra2_broker/server/get_next_test.go b/broker/src/hidra2_broker/server/get_next_test.go index 1f706218c8dc96d8590dc68eb5c32c911e2c639e..ba177fb4cdd6e437690accd99c59d55c12b9f6e2 100644 --- a/broker/src/hidra2_broker/server/get_next_test.go +++ b/broker/src/hidra2_broker/server/get_next_test.go @@ -26,8 +26,8 @@ func doRequest(path string) *httptest.ResponseRecorder { } func TestGetNextWithoutDatabaseName(t *testing.T) { - w := doRequest("/next") - assert.Equal(t, http.StatusBadRequest, w.Code, "no database name") + w := doRequest("/database/next") + assert.Equal(t, http.StatusNotFound, w.Code, "no database name") } func TestGetNextWithWrongDatabaseName(t *testing.T) { @@ -37,8 +37,8 @@ func TestGetNextWithWrongDatabaseName(t *testing.T) { mock_db.On("GetNextRecord", "foo").Return([]byte(""), &database.DBError{utils.StatusWrongInput, ""}) - w := doRequest("/next?database=foo") - assert.Equal(t, http.StatusBadRequest, w.Code, "no database name") + w := doRequest("/database/foo/next") + assert.Equal(t, http.StatusBadRequest, w.Code, "wrong database name") assertExpectations(t, mock_db) } @@ -48,7 +48,7 @@ func TestGetNextWithInternalDBError(t *testing.T) { defer func() { db = nil }() mock_db.On("GetNextRecord", "foo").Return([]byte(""), errors.New("")) - w := doRequest("/next?database=foo") + w := doRequest("/database/foo/next") assert.Equal(t, http.StatusInternalServerError, w.Code, "internal error") assertExpectations(t, mock_db) } @@ -57,9 +57,9 @@ func TestGetNextWithGoodDatabaseName(t *testing.T) { mock_db := new(database.MockedDatabase) db = mock_db defer func() { db = nil }() - mock_db.On("GetNextRecord", "database").Return([]byte("Hello"), nil) + mock_db.On("GetNextRecord", "dbname").Return([]byte("Hello"), nil) - w := doRequest("/next?database=database") + w := doRequest("/database/dbname/next") assert.Equal(t, http.StatusOK, w.Code, "GetNext OK") assert.Equal(t, "Hello", string(w.Body.Bytes()), "GetNext sends data") assertExpectations(t, mock_db) diff --git a/broker/src/hidra2_broker/server/listroutes.go b/broker/src/hidra2_broker/server/listroutes.go index 86baeb470e3f028cbd68c5f7719f0bdee20eec51..b9b48ee7efe99ac4251028191f9ab052601c7e05 100644 --- a/broker/src/hidra2_broker/server/listroutes.go +++ b/broker/src/hidra2_broker/server/listroutes.go @@ -8,7 +8,7 @@ var listRoutes = utils.Routes{ utils.Route{ "GetNext", "Get", - "/next", + "/database/{dbname}/next", routeGetNext, }, } diff --git a/common/cpp/include/common/data_structs.h b/common/cpp/include/common/data_structs.h index 93fbdb40b68fba9ee8dac590c230f3a4ca69f1b5..afa13ae0e64c2d07a9c57f9fb5a7c7ff3ccf1211 100644 --- a/common/cpp/include/common/data_structs.h +++ b/common/cpp/include/common/data_structs.h @@ -18,6 +18,7 @@ class FileInfo { uint64_t id{0}; std::string Json() const; bool SetFromJson(const std::string& json_string); + std::string FullName(const std::string& base_path); }; inline bool operator==(const FileInfo& lhs, const FileInfo& rhs) { diff --git a/common/cpp/src/data_structs/data_structs.cpp b/common/cpp/src/data_structs/data_structs.cpp index 9cde1a4a535d05058ce9d90a259df33ad914e9e7..0467bea2933fffee24cc1beebe31235898e744a3 100644 --- a/common/cpp/src/data_structs/data_structs.cpp +++ b/common/cpp/src/data_structs/data_structs.cpp @@ -75,4 +75,11 @@ bool FileInfo::SetFromJson(const std::string& json_string) { return true; } +std::string FileInfo::FullName(const std::string& base_path) { + std::string full_name; + full_name = base_path.empty() ? "" : base_path + "/"; + full_name += relative_path.empty() ? "" : relative_path + "/"; + return full_name + base_name; +} + } diff --git a/tests/broker/get_next/check_linux.sh b/tests/broker/get_next/check_linux.sh index 32d6ed6e0446afadc41a1ee69620230f5c2acb60..d21ae53e809a05d6cd4615d30fa6d96928e3cc73 100644 --- a/tests/broker/get_next/check_linux.sh +++ b/tests/broker/get_next/check_linux.sh @@ -22,7 +22,7 @@ $@ & sleep 0.3 brokerid=`echo $!` -curl -v --silent 127.0.0.1:5005/next?database=data --stderr - | grep '"_id":1' -curl -v --silent 127.0.0.1:5005/next?database=data --stderr - | grep '"_id":2' +curl -v --silent 127.0.0.1:5005/database/data/next --stderr - | grep '"_id":1' +curl -v --silent 127.0.0.1:5005/database/data/next --stderr - | grep '"_id":2' -curl -v --silent 127.0.0.1:5005/next?database=data --stderr - | grep "No Content" +curl -v --silent 127.0.0.1:5005/database/data/next --stderr - | grep "No Content" diff --git a/tests/broker/get_next/check_windows.bat b/tests/broker/get_next/check_windows.bat index 351a030364f89034b28442438b6cfa765e38dc7f..009f92c4df7e83d3feea8476d0b58cb2c0ee76b5 100644 --- a/tests/broker/get_next/check_windows.bat +++ b/tests/broker/get_next/check_windows.bat @@ -11,9 +11,9 @@ start /B "" "%full_name%" ping 1.0.0.0 -n 1 -w 100 > nul -C:\Curl\curl.exe -v --silent 127.0.0.1:5005/next?database=data --stderr - | findstr \"_id\":1 || goto :error -C:\Curl\curl.exe -v --silent 127.0.0.1:5005/next?database=data --stderr - | findstr \"_id\":2 || goto :error -C:\Curl\curl.exe -v --silent 127.0.0.1:5005/next?database=data --stderr - | findstr "No Content" || goto :error +C:\Curl\curl.exe -v --silent 127.0.0.1:5005/database/data/next --stderr - | findstr \"_id\":1 || goto :error +C:\Curl\curl.exe -v --silent 127.0.0.1:5005/database/data/next --stderr - | findstr \"_id\":2 || goto :error +C:\Curl\curl.exe -v --silent 127.0.0.1:5005/database/data/next --stderr - | findstr "No Content" || goto :error goto :clean diff --git a/worker/api/cpp/CMakeLists.txt b/worker/api/cpp/CMakeLists.txt index cc301d778a0bbfa86e077b8d0b519da57878c7d5..30f9c63cc8ff109457d93272dcc05293840a5c25 100644 --- a/worker/api/cpp/CMakeLists.txt +++ b/worker/api/cpp/CMakeLists.txt @@ -4,7 +4,7 @@ set(SOURCE_FILES src/data_broker.cpp src/folder_data_broker.cpp src/server_data_broker.cpp - src/io_map.cpp + src/broker_helpers.cpp src/curl_http_client.cpp) diff --git a/worker/api/cpp/src/io_map.cpp b/worker/api/cpp/src/broker_helpers.cpp similarity index 96% rename from worker/api/cpp/src/io_map.cpp rename to worker/api/cpp/src/broker_helpers.cpp index c6855d6fae06dabc37008f4aa2d5517088412c11..c9e8dc8e2f9d1a1d15d21bd5c2317717e6766d19 100644 --- a/worker/api/cpp/src/io_map.cpp +++ b/worker/api/cpp/src/broker_helpers.cpp @@ -1,4 +1,4 @@ -#include "io_map.h" +#include "broker_helpers.h" namespace hidra2 { diff --git a/worker/api/cpp/src/io_map.h b/worker/api/cpp/src/broker_helpers.h similarity index 100% rename from worker/api/cpp/src/io_map.h rename to worker/api/cpp/src/broker_helpers.h diff --git a/worker/api/cpp/src/folder_data_broker.cpp b/worker/api/cpp/src/folder_data_broker.cpp index f081fc9a27ac52934850d337755b4c313137bbfe..bc0801dfeace152c3c576d4c45a7304e2130e186 100644 --- a/worker/api/cpp/src/folder_data_broker.cpp +++ b/worker/api/cpp/src/folder_data_broker.cpp @@ -1,7 +1,7 @@ #include "folder_data_broker.h" #include "system_wrappers/system_io.h" -#include "io_map.h" +#include "broker_helpers.h" namespace hidra2 { @@ -31,7 +31,7 @@ WorkerErrorCode FolderDataBroker::CanGetData(FileInfo* info, FileData* data, int return WorkerErrorCode::kSourceNotConnected; } - if (info == nullptr && data == nullptr) { + if (info == nullptr) { return WorkerErrorCode::kWrongInput; } @@ -53,19 +53,14 @@ WorkerErrorCode FolderDataBroker::GetNext(FileInfo* info, FileData* data) { return err; } - FileInfo file_info = filelist_[nfile_to_get]; - if (info != nullptr) { - *info = file_info; - } + *info = filelist_[nfile_to_get]; if (data == nullptr) { return WorkerErrorCode::kOK; } IOErrors ioerr; - *data = io__->GetDataFromFile(base_path_ + "/" + file_info.relative_path + - (file_info.relative_path.empty() ? "" : "/") + - file_info.base_name, file_info.size, &ioerr); + *data = io__->GetDataFromFile(info->FullName(base_path_), info->size, &ioerr); return MapIOError(ioerr); } diff --git a/worker/api/cpp/src/server_data_broker.cpp b/worker/api/cpp/src/server_data_broker.cpp index ee712f8e07f47b65e91a0c9977ba646f396031fa..26e3b2fe2a66fbf439a08da58f540906cc57fa4f 100644 --- a/worker/api/cpp/src/server_data_broker.cpp +++ b/worker/api/cpp/src/server_data_broker.cpp @@ -1,7 +1,7 @@ #include "server_data_broker.h" #include "system_wrappers/system_io.h" #include "curl_http_client.h" -#include "io_map.h" +#include "broker_helpers.h" namespace hidra2 { @@ -15,25 +15,27 @@ WorkerErrorCode ServerDataBroker::Connect() { return WorkerErrorCode::kOK; } -WorkerErrorCode ServerDataBroker::GetNext(FileInfo* info, FileData* data) { - if (info == nullptr && data == nullptr) { - return WorkerErrorCode::kWrongInput; - } - - std::string full_uri = server_uri_ + "/next?database=" + source_name_; +WorkerErrorCode ServerDataBroker::GetFileInfoFromServer(FileInfo* info, const std::string& operation) { + std::string full_uri = server_uri_ + "/database/" + source_name_ + "/" + operation; WorkerErrorCode err; auto responce = httpclient__->Get(full_uri, &err); if (err != WorkerErrorCode::kOK) { return err; } - - FileInfo file_info; - if (!file_info.SetFromJson(responce)) { + if (!info->SetFromJson(responce)) { return WorkerErrorCode::kErrorReadingSource; } + return WorkerErrorCode::kOK; +} + +WorkerErrorCode ServerDataBroker::GetNext(FileInfo* info, FileData* data) { + if (info == nullptr) { + return WorkerErrorCode::kWrongInput; + } - if (info != nullptr) { - *info = file_info; + auto err = GetFileInfoFromServer(info, "next"); + if (err != WorkerErrorCode::kOK) { + return err; } if (data == nullptr) { @@ -41,10 +43,7 @@ WorkerErrorCode ServerDataBroker::GetNext(FileInfo* info, FileData* data) { } IOErrors ioerr; - *data = io__->GetDataFromFile(file_info.relative_path + - (file_info.relative_path.empty() ? "" : "/") + - file_info.base_name, file_info.size, &ioerr); - + *data = io__->GetDataFromFile(info->FullName(""), info->size, &ioerr); return hidra2::MapIOError(ioerr); } diff --git a/worker/api/cpp/src/server_data_broker.h b/worker/api/cpp/src/server_data_broker.h index 6cea3a104dc397178e14f3e38ff71ffa52f3911e..ad3bc604e06e16a6b55d6359a1eab86503c30b43 100644 --- a/worker/api/cpp/src/server_data_broker.h +++ b/worker/api/cpp/src/server_data_broker.h @@ -15,6 +15,7 @@ class ServerDataBroker final : public hidra2::DataBroker { std::unique_ptr<hidra2::IO> io__; // modified in testings to mock system calls,otherwise do not touch std::unique_ptr<hidra2::HttpClient> httpclient__; private: + WorkerErrorCode GetFileInfoFromServer(FileInfo* info, const std::string& operation); std::string server_uri_; std::string source_name_; }; diff --git a/worker/api/cpp/unittests/test_folder_broker.cpp b/worker/api/cpp/unittests/test_folder_broker.cpp index b7131c45efb68c40dfbdada3564203f573ddfc5f..e736d061ee91ee84b38e3542685e8a4dcbdac434 100644 --- a/worker/api/cpp/unittests/test_folder_broker.cpp +++ b/worker/api/cpp/unittests/test_folder_broker.cpp @@ -248,16 +248,6 @@ TEST_F(GetDataFromFileTests, GetNextReturnsDataAndInfo) { } -TEST_F(GetDataFromFileTests, GetNextReturnsOnlyData) { - EXPECT_CALL(mock, GetDataFromFileProxy(_, _, _)). - WillOnce(DoAll(testing::SetArgPointee<2>(IOErrors::kNoError), testing::Return(new uint8_t[1] {'1'}))); - - data_broker->GetNext(nullptr, &data); - - ASSERT_THAT(data[0], Eq('1')); -} - - TEST_F(GetDataFromFileTests, GetNextReturnsErrorWhenCannotReadData) { EXPECT_CALL(mock, GetDataFromFileProxy(_, _, _)). WillOnce(DoAll(testing::SetArgPointee<2>(IOErrors::kReadError), testing::Return(nullptr))); diff --git a/worker/api/cpp/unittests/test_server_broker.cpp b/worker/api/cpp/unittests/test_server_broker.cpp index 6e083a32d39597bb63299af2dc9f5913f4e78dcc..afe61a0dc5adfc1b4779a96ee615cc09eefb80d7 100644 --- a/worker/api/cpp/unittests/test_server_broker.cpp +++ b/worker/api/cpp/unittests/test_server_broker.cpp @@ -33,13 +33,14 @@ using ::testing::SetArgPointee; namespace { -TEST(FolderDataBroker, SetCorrectMembers) { - auto data_broker = std::unique_ptr<ServerDataBroker> {new ServerDataBroker("test", "database")}; +TEST(FolderDataBroker, SetCorrectIo) { + auto data_broker = std::unique_ptr<ServerDataBroker> {new ServerDataBroker("test", "dbname")}; ASSERT_THAT(dynamic_cast<hidra2::SystemIO*>(data_broker->io__.get()), Ne(nullptr)); } TEST(FolderDataBroker, SetCorrectHttpClient) { - auto data_broker = std::unique_ptr<ServerDataBroker> {new ServerDataBroker("test", "database")}; + auto data_broker = std::unique_ptr<ServerDataBroker> {new ServerDataBroker("test", "dbname")}; + ASSERT_THAT(dynamic_cast<hidra2::CurlHttpClient*>(data_broker->httpclient__.get()), Ne(nullptr)); } @@ -51,7 +52,7 @@ class ServerDataBrokerTests : public Test { FileInfo info; void SetUp() override { - data_broker = std::unique_ptr<ServerDataBroker> {new ServerDataBroker("test", "database")}; + data_broker = std::unique_ptr<ServerDataBroker> {new ServerDataBroker("test", "dbname")}; data_broker->io__ = std::unique_ptr<IO> {&mock_io}; data_broker->httpclient__ = std::unique_ptr<hidra2::HttpClient> {&mock_http_client}; } @@ -72,7 +73,7 @@ TEST_F(ServerDataBrokerTests, GetNextReturnsErrorOnWrongInput) { } TEST_F(ServerDataBrokerTests, GetNextUsesCorrectUri) { - EXPECT_CALL(mock_http_client, Get("test/next?database=database", _)).WillOnce(DoAll( + EXPECT_CALL(mock_http_client, Get("test/database/dbname/next", _)).WillOnce(DoAll( SetArgPointee<1>(WorkerErrorCode::kOK), Return(""))); data_broker->GetNext(&info, nullptr); @@ -104,8 +105,7 @@ TEST_F(ServerDataBrokerTests, GetNextReturnsFileInfo) { auto json = to_send.Json(); EXPECT_CALL(mock_http_client, Get(_, _)).WillOnce(DoAll( SetArgPointee<1>(WorkerErrorCode::kOK), - Return(json) - )); + Return(json))); auto err = data_broker->GetNext(&info, nullptr); @@ -134,8 +134,7 @@ TEST_F(ServerDataBrokerTests, GetNextReturnsParseError) { TEST_F(ServerDataBrokerTests, GetNextReturnsIfNoDtataNeeded) { EXPECT_CALL(mock_http_client, Get(_, _)).WillOnce(DoAll( SetArgPointee<1>(WorkerErrorCode::kOK), - Return("blabla") - )); + Return("blabla"))); EXPECT_CALL( mock_io, GetDataFromFile_t(_, _, _)).Times(0); data_broker->GetNext(&info, nullptr);