From ab165f6479acd943a69f3ea20f8fbfc538ad6ca7 Mon Sep 17 00:00:00 2001
From: Sergey Yakubov <sergey.yakubov@desy.de>
Date: Mon, 24 Jun 2019 12:06:10 +0200
Subject: [PATCH] fix
---
.../content_multithread.cpp | 3 +-
worker/api/cpp/include/worker/data_broker.h | 2 +-
worker/api/cpp/src/folder_data_broker.cpp | 2 +-
worker/api/cpp/src/folder_data_broker.h | 2 +-
worker/api/cpp/src/server_data_broker.cpp | 19 +++----
worker/api/cpp/src/server_data_broker.h | 6 +--
.../api/cpp/unittests/test_folder_broker.cpp | 4 +-
.../api/cpp/unittests/test_server_broker.cpp | 54 +++++++++++++++++--
8 files changed, 70 insertions(+), 22 deletions(-)
diff --git a/tests/automatic/worker/connect_multithread/content_multithread.cpp b/tests/automatic/worker/connect_multithread/content_multithread.cpp
index 2ba338998..070c9c890 100644
--- a/tests/automatic/worker/connect_multithread/content_multithread.cpp
+++ b/tests/automatic/worker/connect_multithread/content_multithread.cpp
@@ -3,6 +3,7 @@
#include <thread>
#include <algorithm>
#include "worker/data_broker.h"
+#include "worker/worker_error.h"
#include "testing.h"
using asapo::M_AssertEq;
@@ -15,7 +16,7 @@ void Assert(std::vector<asapo::Error>& errors, int nthreads) {
int count_already_connected = 0;
for (auto& error : errors) {
if (!error) continue;
- if (error->Explain().find(asapo::WorkerErrorMessage::kSourceAlreadyConnected) != std::string::npos)
+ if (error == asapo::WorkerErrorTemplates::kSourceAlreadyConnected)
count_already_connected++;
}
diff --git a/worker/api/cpp/include/worker/data_broker.h b/worker/api/cpp/include/worker/data_broker.h
index e33f7a112..5f2358a52 100644
--- a/worker/api/cpp/include/worker/data_broker.h
+++ b/worker/api/cpp/include/worker/data_broker.h
@@ -81,7 +81,7 @@ class DataBroker {
\param err - will be set in case of error, nullptr otherwise
\return vector of image metadata matchiing to specified query. Empty if nothing found or error
*/
- virtual FileInfos QueryImages(std::string query,Error* err) = 0;
+ virtual FileInfos QueryImages(std::string query, Error* err) = 0;
virtual ~DataBroker() = default; // needed for unique_ptr to delete itself
};
diff --git a/worker/api/cpp/src/folder_data_broker.cpp b/worker/api/cpp/src/folder_data_broker.cpp
index 791561b8a..f64f140bb 100644
--- a/worker/api/cpp/src/folder_data_broker.cpp
+++ b/worker/api/cpp/src/folder_data_broker.cpp
@@ -100,7 +100,7 @@ std::string FolderDataBroker::GetBeamtimeMeta(Error* err) {
}
FileInfos FolderDataBroker::QueryImages(std::string query, Error* err) {
- *err=TextError("Not supported for folder data broker");
+ *err = TextError("Not supported for folder data broker");
return FileInfos{};
}
diff --git a/worker/api/cpp/src/folder_data_broker.h b/worker/api/cpp/src/folder_data_broker.h
index aa0dfca5d..9ffb04d3b 100644
--- a/worker/api/cpp/src/folder_data_broker.h
+++ b/worker/api/cpp/src/folder_data_broker.h
@@ -24,7 +24,7 @@ class FolderDataBroker final : public asapo::DataBroker {
uint64_t GetNDataSets(Error* err) override;
Error GetById(uint64_t id, FileInfo* info, std::string group_id, FileData* data) override;
std::unique_ptr<asapo::IO> io__; // modified in testings to mock system calls,otherwise do not touch
- FileInfos QueryImages(std::string query,Error* err) override;
+ FileInfos QueryImages(std::string query, Error* err) override;
private:
std::string base_path_;
bool is_connected_;
diff --git a/worker/api/cpp/src/server_data_broker.cpp b/worker/api/cpp/src/server_data_broker.cpp
index bb0b86ebb..58185be85 100644
--- a/worker/api/cpp/src/server_data_broker.cpp
+++ b/worker/api/cpp/src/server_data_broker.cpp
@@ -22,8 +22,8 @@ Error HttpCodeToWorkerError(const HttpCode& code) {
return nullptr;
case HttpCode::BadRequest:
return WorkerErrorTemplates::kWrongInput.Generate();
- case HttpCode::Unauthorized:
- return WorkerErrorTemplates::kAuthorizationError.Generate();
+ case HttpCode::Unauthorized:
+ return WorkerErrorTemplates::kAuthorizationError.Generate();
case HttpCode::InternalServerError:
return WorkerErrorTemplates::kInternalError.Generate();
case HttpCode::NotFound:
@@ -85,9 +85,10 @@ Error ServerDataBroker::ProcessRequest(std::string* response, const RequestInfo&
Error err;
HttpCode code;
if (request.post) {
- *response = httpclient__->Post(RequestWithToken(request.host+request.api) + request.extra_params, request.body, &code, &err);
+ *response = httpclient__->Post(RequestWithToken(request.host + request.api) + request.extra_params, request.body, &code,
+ &err);
} else {
- *response = httpclient__->Get(RequestWithToken(request.host+request.api) + request.extra_params, &code, &err);
+ *response = httpclient__->Get(RequestWithToken(request.host + request.api) + request.extra_params, &code, &err);
}
if (err != nullptr) {
current_broker_uri_ = "";
@@ -103,7 +104,7 @@ Error ServerDataBroker::GetBrokerUri() {
RequestInfo ri;
ri.host = server_uri_;
- ri.api= "/discovery/broker";
+ ri.api = "/discovery/broker";
Error err;
err = ProcessRequest(¤t_broker_uri_, ri);
@@ -135,7 +136,7 @@ Error ServerDataBroker::GetFileInfoFromServer(FileInfo* info, std::string group_
ProcessServerError(&err, response, &request_suffix);
if (elapsed_ms >= timeout_ms_) {
- err = IOErrorTemplates::kTimeout.Generate( ", last error: "+err->Explain());
+ err = IOErrorTemplates::kTimeout.Generate( ", last error: " + err->Explain());
return err;
}
std::this_thread::sleep_for(std::chrono::milliseconds(100));
@@ -224,7 +225,7 @@ std::string ServerDataBroker::GenerateNewGroupId(Error* err) {
std::string ServerDataBroker::AppendUri(std::string request_string) {
- return current_broker_uri_ + "/"+std::move(request_string);
+ return current_broker_uri_ + "/" + std::move(request_string);
}
@@ -244,7 +245,7 @@ std::string ServerDataBroker::BrokerRequestWithTimeout(RequestInfo request, Erro
std::this_thread::sleep_for(std::chrono::milliseconds(100));
elapsed_ms += 100;
}
- *err = IOErrorTemplates::kTimeout.Generate( ", last error: "+(*err)->Explain());
+ *err = IOErrorTemplates::kTimeout.Generate( ", last error: " + (*err)->Explain());
return "";
}
@@ -313,7 +314,7 @@ FileInfos ServerDataBroker::QueryImages(std::string query, Error* err) {
ri.body = std::move(query);
auto responce = BrokerRequestWithTimeout(ri, err);
- if (err) {
+ if (*err) {
(*err)->Append(responce);
}
diff --git a/worker/api/cpp/src/server_data_broker.h b/worker/api/cpp/src/server_data_broker.h
index 6662eaae0..a7d1afb96 100644
--- a/worker/api/cpp/src/server_data_broker.h
+++ b/worker/api/cpp/src/server_data_broker.h
@@ -37,7 +37,7 @@ class ServerDataBroker final : public asapo::DataBroker {
uint64_t GetNDataSets(Error* err) override;
Error GetById(uint64_t id, FileInfo* info, std::string group_id, FileData* data) override;
void SetTimeout(uint64_t timeout_ms) override;
- FileInfos QueryImages(std::string query,Error* err) override;
+ FileInfos QueryImages(std::string query, Error* err) override;
std::unique_ptr<IO> io__; // modified in testings to mock system calls,otherwise do not touch
std::unique_ptr<HttpClient> httpclient__;
@@ -53,10 +53,10 @@ class ServerDataBroker final : public asapo::DataBroker {
Error GetImageFromServer(GetImageServerOperation op, uint64_t id, std::string group_id, FileInfo* info, FileData* data);
bool DataCanBeInBuffer(const FileInfo* info);
Error TryGetDataFromBuffer(const FileInfo* info, FileData* data);
- std::string BrokerRequestWithTimeout(RequestInfo request,Error* err);
+ std::string BrokerRequestWithTimeout(RequestInfo request, Error* err);
std::string AppendUri(std::string request_string);
- std::string OpToUriCmd(GetImageServerOperation op);
+ std::string OpToUriCmd(GetImageServerOperation op);
std::string server_uri_;
std::string current_broker_uri_;
std::string source_path_;
diff --git a/worker/api/cpp/unittests/test_folder_broker.cpp b/worker/api/cpp/unittests/test_folder_broker.cpp
index ecbcfe5ca..9b9c25493 100644
--- a/worker/api/cpp/unittests/test_folder_broker.cpp
+++ b/worker/api/cpp/unittests/test_folder_broker.cpp
@@ -344,10 +344,10 @@ TEST_F(GetDataFromFileTests, GetMetaDataReturnsOK) {
}
TEST(FolderDataBroker, QueryImages) {
- auto data_broker = std::unique_ptr<FolderDataBroker>{new FolderDataBroker("test")};
+ auto data_broker = std::unique_ptr<FolderDataBroker> {new FolderDataBroker("test")};
Error err;
- auto infos = data_broker->QueryImages("bla",&err);
+ auto infos = data_broker->QueryImages("bla", &err);
ASSERT_THAT(err, Ne(nullptr));
ASSERT_THAT(infos.size(), Eq(0));
diff --git a/worker/api/cpp/unittests/test_server_broker.cpp b/worker/api/cpp/unittests/test_server_broker.cpp
index 00fd6a4d5..86854a0ec 100644
--- a/worker/api/cpp/unittests/test_server_broker.cpp
+++ b/worker/api/cpp/unittests/test_server_broker.cpp
@@ -499,20 +499,66 @@ TEST_F(ServerDataBrokerTests, QueryImagesReturnError) {
MockGetBrokerUri();
EXPECT_CALL(mock_http_client, Post_t(HasSubstr("queryimages"), expected_query_string, _, _)).WillOnce(DoAll(
- SetArgPointee<2>(HttpCode::BadRequest),
- SetArgPointee<3>(nullptr),
- Return("error in query")));
+ SetArgPointee<2>(HttpCode::BadRequest),
+ SetArgPointee<3>(nullptr),
+ Return("error in query")));
data_broker->SetTimeout(1000);
asapo::Error err;
- auto images = data_broker->QueryImages(expected_query_string,&err);
+ auto images = data_broker->QueryImages(expected_query_string, &err);
ASSERT_THAT(err, Eq(asapo::WorkerErrorTemplates::kWrongInput));
ASSERT_THAT(err->Explain(), HasSubstr("query"));
+ ASSERT_THAT(images.size(), Eq(0));
+}
+
+
+TEST_F(ServerDataBrokerTests, QueryImagesReturnEmptyResults) {
+ MockGetBrokerUri();
+
+ EXPECT_CALL(mock_http_client, Post_t(HasSubstr("queryimages"), expected_query_string, _, _)).WillOnce(DoAll(
+ SetArgPointee<2>(HttpCode::OK),
+ SetArgPointee<3>(nullptr),
+ Return("[]")));
+
+ data_broker->SetTimeout(100);
+ asapo::Error err;
+ auto images = data_broker->QueryImages(expected_query_string, &err);
+ ASSERT_THAT(err, Eq(nullptr));
ASSERT_THAT(images.size(), Eq(0));
}
+TEST_F(ServerDataBrokerTests, QueryImagesReturnRecords) {
+ return;
+ MockGetBrokerUri();
+
+ auto rec1 = CreateFI();
+ auto rec2 = CreateFI();
+ auto json1 = rec1.Json();
+ auto json2 = rec2.Json();
+ auto responce_string = "[" + json1 + "," + json2 + "]";
+
+
+ EXPECT_CALL(mock_http_client, Post_t(HasSubstr("queryimages"), expected_query_string, _, _)).WillOnce(DoAll(
+ SetArgPointee<2>(HttpCode::OK),
+ SetArgPointee<3>(nullptr),
+ Return(responce_string)));
+
+ data_broker->SetTimeout(100);
+ asapo::Error err;
+ auto images = data_broker->QueryImages(expected_query_string, &err);
+
+ ASSERT_THAT(err, Eq(nullptr));
+ ASSERT_THAT(images.size(), Eq(2));
+
+ ASSERT_THAT(images[0], Eq(rec1));
+ ASSERT_THAT(images[1], Eq(rec2));
+
+
+}
+
+
}
--
GitLab