From 4c49806ba718b9b4926bb1c6539c3e7ccb00ae1d Mon Sep 17 00:00:00 2001
From: Carsten Patzke <carsten.patzke@desy.de>
Date: Wed, 14 Mar 2018 17:02:16 +0100
Subject: [PATCH] Changed the way request handlers are called
---
receiver/src/network_producer_peer_impl.cpp | 6 +-
receiver/src/network_producer_peer_impl.h | 21 ++---
.../network_producer_peer_impl_handlers.cpp | 21 +++--
.../test_network_producer_peer_impl.cpp | 87 ++++++++++++++++---
4 files changed, 103 insertions(+), 32 deletions(-)
diff --git a/receiver/src/network_producer_peer_impl.cpp b/receiver/src/network_producer_peer_impl.cpp
index 9b6c42b24..1211cb5f3 100644
--- a/receiver/src/network_producer_peer_impl.cpp
+++ b/receiver/src/network_producer_peer_impl.cpp
@@ -103,7 +103,7 @@ void NetworkProducerPeerImpl::StopPeerListener() {
}
size_t NetworkProducerPeerImpl::HandleGenericRequest(GenericNetworkRequest* request,
- GenericNetworkResponse* response, Error* err) {
+ GenericNetworkResponse* response, Error* err) noexcept {
if(!CheckIfValidNetworkOpCode(request->op_code)) {
*err = hidra2::ReceiverErrorTemplates::kInvalidOpCode.Generate();
return 0;
@@ -120,12 +120,12 @@ size_t NetworkProducerPeerImpl::HandleGenericRequest(GenericNetworkRequest* requ
io->Receive(socket_fd_, (uint8_t*)request + sizeof_generic_request,
handler_information.request_size - sizeof_generic_request, err);
- if(err) {
+ if(*err) {
return 0;
}
//Invoke the request handler which sets the response
- handler_information.handler(this, request, response);
+ handler_information.handler(this, request, response, err);
return handler_information.response_size;
}
diff --git a/receiver/src/network_producer_peer_impl.h b/receiver/src/network_producer_peer_impl.h
index d7ddf04c0..72b51b15c 100644
--- a/receiver/src/network_producer_peer_impl.h
+++ b/receiver/src/network_producer_peer_impl.h
@@ -15,15 +15,15 @@
namespace hidra2 {
class NetworkProducerPeerImpl : public NetworkProducerPeer, public HasIO {
- public:
+ public:
typedef void (* RequestHandler)(NetworkProducerPeerImpl* self, GenericNetworkRequest* request,
- GenericNetworkResponse* response);
+ GenericNetworkResponse* response, Error* err);
struct RequestHandlerInformation {
size_t request_size;
size_t response_size;
RequestHandler handler;
};
- private:
+ private:
uint32_t connection_id_;
std::string address_;
int socket_fd_;
@@ -35,7 +35,7 @@ class NetworkProducerPeerImpl : public NetworkProducerPeer, public HasIO {
void InternalPeerReceiverThreadEntryPoint();
void InternalPeerReceiverDoWork(GenericNetworkRequest* request, GenericNetworkResponse* response, Error* err);
void HandleRawRequestBuffer(GenericNetworkRequest* request, GenericNetworkResponse* response, Error* err);
- public:
+ public:
static const std::vector<RequestHandlerInformation> kRequestHandlers;
static size_t kRequestHandlerMaxBufferSize;
@@ -57,15 +57,16 @@ class NetworkProducerPeerImpl : public NetworkProducerPeer, public HasIO {
virtual bool CheckIfValidFileSize(size_t file_size) const noexcept;
virtual bool CheckIfValidNetworkOpCode(Opcode opcode) const noexcept;
- public:
+ public:
/*
* Private functions but opened for unittest
*/
- size_t HandleGenericRequest(GenericNetworkRequest* request, GenericNetworkResponse* response, Error* err);
- static void HandleSendDataRequest(NetworkProducerPeerImpl* self,
- const SendDataRequest* request,
- SendDataResponse* response);
-
+ virtual size_t HandleGenericRequest(GenericNetworkRequest* request, GenericNetworkResponse* response, Error* err) noexcept;
+ virtual void HandleSendDataRequest(const SendDataRequest* request, SendDataResponse* response, Error* err) noexcept;
+ private:
+ static void HandleSendDataRequestInternalCaller(NetworkProducerPeerImpl* self,
+ const SendDataRequest* request,
+ SendDataResponse* response, Error* err) noexcept;
};
}
diff --git a/receiver/src/network_producer_peer_impl_handlers.cpp b/receiver/src/network_producer_peer_impl_handlers.cpp
index 6ec5707df..a19a43a0d 100644
--- a/receiver/src/network_producer_peer_impl_handlers.cpp
+++ b/receiver/src/network_producer_peer_impl_handlers.cpp
@@ -12,7 +12,7 @@ NetworkProducerPeerImpl::StaticInitRequestHandlerList() {
vec[kNetOpcodeSendData] = {
sizeof(SendDataRequest),
sizeof(SendDataResponse),
- (NetworkProducerPeerImpl::RequestHandler)& NetworkProducerPeerImpl::HandleSendDataRequest
+ (NetworkProducerPeerImpl::RequestHandler) &NetworkProducerPeerImpl::HandleSendDataRequestInternalCaller
};
for(RequestHandlerInformation& handler_information : vec) {
@@ -29,21 +29,26 @@ NetworkProducerPeerImpl::StaticInitRequestHandlerList() {
return vec;
}
-void NetworkProducerPeerImpl::HandleSendDataRequest(NetworkProducerPeerImpl* self, const SendDataRequest* request,
- SendDataResponse* response) {
- Error err;
- self->ReceiveAndSaveFile(request->file_id, request->file_size, &err);
+void NetworkProducerPeerImpl::HandleSendDataRequestInternalCaller(NetworkProducerPeerImpl* self,
+ const SendDataRequest* request,
+ SendDataResponse* response,
+ Error* err) noexcept {
+ self->HandleSendDataRequest(request, response, err);
+}
+
+void NetworkProducerPeerImpl::HandleSendDataRequest(const SendDataRequest* request, SendDataResponse* response, Error* err) noexcept {
+ ReceiveAndSaveFile(request->file_id, request->file_size, err);
- if(!err) {
+ if(!*err) {
response->error_code = NET_ERR__NO_ERROR;
return;
}
- if(err == IOErrorTemplates::kFileAlreadyExists) {
+ if(*err == IOErrorTemplates::kFileAlreadyExists) {
response->error_code = NET_ERR__FILEID_ALREADY_IN_USE;
} else {
- std::cout << "[" << self->GetConnectionId() << "] Unexpected ReceiveAndSaveFile error " << err << std::endl;
+ std::cout << "[" << GetConnectionId() << "] Unexpected ReceiveAndSaveFile error " << *err << std::endl;
response->error_code = NET_ERR__INTERNAL_SERVER_ERROR;
//self->io->CloseSocket(self->socket_fd_, nullptr); TODO: Might want to close the connection?
}
diff --git a/receiver/unittests/test_network_producer_peer_impl.cpp b/receiver/unittests/test_network_producer_peer_impl.cpp
index 19e3e0138..ddcb886dd 100644
--- a/receiver/unittests/test_network_producer_peer_impl.cpp
+++ b/receiver/unittests/test_network_producer_peer_impl.cpp
@@ -16,6 +16,11 @@ using ::hidra2::Error;
using ::hidra2::ErrorInterface;
using ::hidra2::FileDescriptor;
using ::hidra2::SocketDescriptor;
+using ::hidra2::SendDataRequest;
+using ::hidra2::SendDataResponse;
+using ::hidra2::GenericNetworkRequest;
+using ::hidra2::GenericNetworkResponse;
+using ::hidra2::Opcode;
namespace {
@@ -225,6 +230,8 @@ class ReceiveAndSaveFileFixture : public testing::Test {
};
TEST_F(ReceiveAndSaveFileFixture, CheckFileSizeError) {
+ err = nullptr;
+
EXPECT_CALL(*networkProducerPeer, CheckIfValidFileSize_t(expected_file_size))
.WillOnce(
Return(false)
@@ -236,6 +243,8 @@ TEST_F(ReceiveAndSaveFileFixture, CheckFileSizeError) {
}
TEST_F(ReceiveAndSaveFileFixture, CheckErrorWhenFileAlreadyExists) {
+ err = nullptr;
+
EXPECT_CALL(*networkProducerPeer, CheckIfValidFileSize_t(expected_file_size))
.WillOnce(
Return(true)
@@ -255,6 +264,8 @@ TEST_F(ReceiveAndSaveFileFixture, CheckErrorWhenFileAlreadyExists) {
}
TEST_F(ReceiveAndSaveFileFixture, CheckErrorWhenUnknownErrorWhileOpenFile) {
+ err = nullptr;
+
EXPECT_CALL(*networkProducerPeer, CheckIfValidFileSize_t(expected_file_size))
.WillOnce(
Return(true)
@@ -272,6 +283,8 @@ TEST_F(ReceiveAndSaveFileFixture, CheckErrorWhenUnknownErrorWhileOpenFile) {
}
TEST_F(ReceiveAndSaveFileFixture, CheckErrorWhenUnknownErrorWhileReceivingData) {
+ err = nullptr;
+
EXPECT_CALL(*networkProducerPeer, CheckIfValidFileSize_t(expected_file_size))
.WillOnce(
Return(true)
@@ -295,6 +308,8 @@ TEST_F(ReceiveAndSaveFileFixture, CheckErrorWhenUnknownErrorWhileReceivingData)
}
TEST_F(ReceiveAndSaveFileFixture, CheckErrorWhenUnknownErrorWhileSavingData) {
+ err = nullptr;
+
EXPECT_CALL(*networkProducerPeer, CheckIfValidFileSize_t(expected_file_size))
.WillOnce(
Return(true)
@@ -324,6 +339,8 @@ TEST_F(ReceiveAndSaveFileFixture, CheckErrorWhenUnknownErrorWhileSavingData) {
}
TEST_F(ReceiveAndSaveFileFixture, Ok) {
+ err = nullptr;
+
EXPECT_CALL(*networkProducerPeer, CheckIfValidFileSize_t(expected_file_size))
.WillOnce(
Return(true)
@@ -369,13 +386,12 @@ class HandleSendDataRequestFixture : public ReceiveAndSaveFileFixture {
public:
std::unique_ptr<HandleSendDataRequestMock> networkProducerPeer;
- hidra2::SendDataRequest request{};
- hidra2::SendDataResponse response{};
+ SendDataRequest send_data_request{};
+ SendDataResponse send_data_response{};
void SetUp() override {
- request.file_id = expected_file_id;
- request.file_size = expected_file_size;
-
+ send_data_request.file_id = expected_file_id;
+ send_data_request.file_size = expected_file_size;
networkProducerPeer.reset(new HandleSendDataRequestMock(expected_socket_descriptor, expected_address));
networkProducerPeer->SetIO__(&mockIO);
@@ -383,36 +399,85 @@ class HandleSendDataRequestFixture : public ReceiveAndSaveFileFixture {
};
TEST_F(HandleSendDataRequestFixture, Ok) {
+ err = nullptr;
+
EXPECT_CALL(*networkProducerPeer, ReceiveAndSaveFile_t(expected_file_id, expected_file_size, _))
.WillOnce(
SetArgPointee<2>(nullptr)
);
- networkProducerPeer->HandleSendDataRequest(networkProducerPeer.get(), &request, &response);
+ networkProducerPeer->HandleSendDataRequest(&send_data_request, &send_data_response, &err);
- ASSERT_THAT(response.error_code, Eq(hidra2::NET_ERR__NO_ERROR));
+ ASSERT_THAT(send_data_response.error_code, Eq(hidra2::NET_ERR__NO_ERROR));
+ ASSERT_THAT(send_data_response.error_code, Eq(hidra2::NET_ERR__NO_ERROR));
}
TEST_F(HandleSendDataRequestFixture, CheckErrorCodeWhenFileIdIsAlreadyUsed) {
+ err = nullptr;
+
EXPECT_CALL(*networkProducerPeer, ReceiveAndSaveFile_t(expected_file_id, expected_file_size, _))
.WillOnce(
SetArgPointee<2>(hidra2::IOErrorTemplates::kFileAlreadyExists.Generate().release())
);
- networkProducerPeer->HandleSendDataRequest(networkProducerPeer.get(), &request, &response);
+ networkProducerPeer->HandleSendDataRequest(&send_data_request, &send_data_response, &err);
- ASSERT_THAT(response.error_code, Eq(hidra2::NET_ERR__FILEID_ALREADY_IN_USE));
+ ASSERT_THAT(send_data_response.error_code, Eq(hidra2::NET_ERR__FILEID_ALREADY_IN_USE));
}
TEST_F(HandleSendDataRequestFixture, CheckErrorCodeWhenUnexpectedError) {
+ err = nullptr;
+
EXPECT_CALL(*networkProducerPeer, ReceiveAndSaveFile_t(expected_file_id, expected_file_size, _))
.WillOnce(
SetArgPointee<2>(hidra2::IOErrorTemplates::kUnknownIOError.Generate().release())
);
- networkProducerPeer->HandleSendDataRequest(networkProducerPeer.get(), &request, &response);
+ networkProducerPeer->HandleSendDataRequest(&send_data_request, &send_data_response, &err);
- ASSERT_THAT(response.error_code, Eq(hidra2::NET_ERR__INTERNAL_SERVER_ERROR));
+ ASSERT_THAT(send_data_response.error_code, Eq(hidra2::NET_ERR__INTERNAL_SERVER_ERROR));
}
+class HandleGenericRequestMock : public HandleSendDataRequestMock {
+ public:
+ HandleGenericRequestMock(SocketDescriptor socket_fd, const std::string &address)
+ : HandleSendDataRequestMock(socket_fd, address) {}
+
+ size_t HandleGenericRequest(GenericNetworkRequest* request, GenericNetworkResponse* response, Error* err) noexcept override {
+ ErrorInterface* error = nullptr;
+ auto data = HandleGenericRequest_t(request, response, &error);
+ err->reset(error);
+ return data;
+ }
+ MOCK_METHOD3(HandleGenericRequest_t, size_t(GenericNetworkRequest* request, GenericNetworkResponse* response, ErrorInterface** err));
+
+
+ bool CheckIfValidNetworkOpCode(Opcode opcode) const noexcept override {
+ return HandleGenericRequest_t(opcode);
+ }
+ MOCK_CONST_METHOD1(HandleGenericRequest_t, bool(Opcode opcode));
+
+
+};
+
+class HandleGenericRequestFixture : public HandleSendDataRequestFixture {
+ public:
+ std::unique_ptr<HandleGenericRequestMock> networkProducerPeer;
+
+ SendDataRequest generic_request{};
+ SendDataResponse generic_response{};
+
+ void SetUp() override {
+ networkProducerPeer.reset(new HandleGenericRequestMock(expected_socket_descriptor, expected_address));
+ networkProducerPeer->SetIO__(&mockIO);
+ }
+};
+
+TEST_F(HandleGenericRequestFixture, DebugTest) {
+ err = nullptr;
+
+ ASSERT_TRUE(true);
+}
+
+
}
--
GitLab