Commit c7f86fda authored by Marcin Siodelski's avatar Marcin Siodelski
Browse files

[5317] Addressed review comments.

parent 5d5e1d52
......@@ -130,7 +130,7 @@ public:
// make the actual configuration text.
std::string config_txt = header + socket_path_ + footer;
server_.reset(new NakedControlledDhcpv4Srv());
ASSERT_NO_THROW(server_.reset(new NakedControlledDhcpv4Srv()));
ConstElementPtr config;
ASSERT_NO_THROW(config = parseDHCP4(config_txt));
......@@ -190,11 +190,14 @@ public:
client.reset(new UnixControlClient());
ASSERT_TRUE(client);
// Connect.
// Connect to the server. This is expected to trigger server's acceptor
// handler when IOService::poll() is run.
ASSERT_TRUE(client->connectToServer(socket_path_));
ASSERT_NO_THROW(getIOService()->poll());
// Send the command.
// Send the command. This will trigger server's handler which receives
// data over the unix domain socket. The server will start sending
// response to the client.
ASSERT_TRUE(client->sendCommand(command));
ASSERT_NO_THROW(getIOService()->poll());
......@@ -683,7 +686,7 @@ TEST_F(CtrlChannelDhcpv4SrvTest, configSet) {
EXPECT_EQ("{ \"result\": 0, \"text\": \"Configuration successful.\" }",
response);
/// Check that the config was indeed applied.
// Check that the config was indeed applied.
const Subnet4Collection* subnets =
CfgMgr::instance().getCurrentCfg()->getCfgSubnets4()->getAll();
EXPECT_EQ(1, subnets->size());
......@@ -1080,7 +1083,7 @@ TEST_F(CtrlChannelDhcpv4SrvTest, concurrentConnections) {
ASSERT_TRUE(client1);
boost::scoped_ptr<UnixControlClient> client2(new UnixControlClient());
ASSERT_TRUE(client1);
ASSERT_TRUE(client2);
// Client 1 connects.
ASSERT_TRUE(client1->connectToServer(socket_path_));
......
......@@ -200,11 +200,14 @@ public:
client.reset(new UnixControlClient());
ASSERT_TRUE(client);
// Connect.
// Connect to the server. This is expected to trigger server's acceptor
// handler when IOService::poll() is run.
ASSERT_TRUE(client->connectToServer(socket_path_));
ASSERT_NO_THROW(getIOService()->poll());
// Send the command.
// Send the command. This will trigger server's handler which receives
// data over the unix domain socket. The server will start sending
// response to the client.
ASSERT_TRUE(client->sendCommand(command));
ASSERT_NO_THROW(getIOService()->poll());
......@@ -1102,7 +1105,7 @@ TEST_F(CtrlChannelDhcpv6SrvTest, concurrentConnections) {
ASSERT_TRUE(client1);
boost::scoped_ptr<UnixControlClient> client2(new UnixControlClient());
ASSERT_TRUE(client1);
ASSERT_TRUE(client2);
// Client 1 connects.
ASSERT_TRUE(client1->connectToServer(socket_path_));
......
......@@ -22,6 +22,11 @@ namespace asiolink {
/// This is a wrapper class for ASIO acceptor service. Classes implementing
/// services for specific protocol types should derive from this class.
///
/// Acceptor is an IO object which accepts incoming connections into a socket
/// object. This socket is then used for data transmission from the client
/// to server and back. The acceptor is continued to be used to accept new
/// connections while the accepted connection is active.
///
/// @tparam ProtocolType ASIO protocol type, e.g. stream_protocol
/// @tparam CallbackType Callback function type which should have the following
/// signature: @c void(const boost::system::error_code&).
......
......@@ -19,6 +19,9 @@ namespace isc {
namespace asiolink {
/// @brief Implements acceptor service for @ref UnixDomainSocket.
///
/// This class is used to accept new incoming connections over unix domain
/// sockets.
class UnixDomainSocketAcceptor : public IOAcceptor<boost::asio::local::stream_protocol,
std::function<void(const boost::system::error_code&)> > {
public:
......
......@@ -18,7 +18,9 @@ namespace asiolink {
/// @brief Endpoint for @ref UnixDomainSocket.
///
/// This is a simple class encapsulating ASIO unix domain socket.
/// This is a simple class encapsulating ASIO unix domain socket endpoint.
/// It is used to represent endpoints taking part in communication via
/// unix domain sockets.
class UnixDomainSocketEndpoint {
public:
......
......@@ -200,32 +200,33 @@ Connection::receiveHandler(const boost::system::error_code& ec,
// No response generated. Connection will be closed.
if (!rsp) {
LOG_WARN(command_logger, COMMAND_RESPONSE_ERROR);
rsp = createAnswer(CONTROL_RESULT_ERROR,
"internal server error: no response generated");
} else {
}
// Let's convert JSON response to text. Note that at this stage
// the rsp pointer is always set.
std::string txt = rsp->str();
size_t len = txt.length();
if (len > 65535) {
// Hmm, our response is too large. Let's send the first
// 64KB and hope for the best.
LOG_ERROR(command_logger, COMMAND_SOCKET_RESPONSE_TOOLARGE).arg(len);
// Let's convert JSON response to text. Note that at this stage
// the rsp pointer is always set.
std::string txt = rsp->str();
size_t len = txt.length();
if (len > 65535) {
// Hmm, our response is too large. Let's send the first
// 64KB and hope for the best.
LOG_ERROR(command_logger, COMMAND_SOCKET_RESPONSE_TOOLARGE).arg(len);
len = 65535;
}
len = 65535;
}
try {
// Send the data back over socket.
socket_->write(txt.c_str(), len);
try {
// Send the data back over socket.
socket_->write(txt.c_str(), len);
} catch (const std::exception& ex) {
// Response transmission failed. Since the response failed, it doesn't
// make sense to send any status codes. Let's log it and be done with
// it.
LOG_ERROR(command_logger, COMMAND_SOCKET_WRITE_FAIL)
.arg(len).arg(socket_->getNative()).arg(ex.what());
}
} catch (const std::exception& ex) {
// Response transmission failed. Since the response failed, it doesn't
// make sense to send any status codes. Let's log it and be done with
// it.
LOG_ERROR(command_logger, COMMAND_SOCKET_WRITE_FAIL)
.arg(len).arg(socket_->getNative()).arg(ex.what());
}
connection_pool_.stop(shared_from_this());
......
......@@ -367,4 +367,11 @@ TEST_F(TimerMgrTest, callbackWithException) {
doWait(500);
}
// This test verifies that IO service specified for the TimerMgr
// must not be null.
TEST_F(TimerMgrTest, setIOService) {
EXPECT_THROW(timer_mgr_->setIOService(IOServicePtr()),
BadValue);
}
} // end of anonymous namespace
......@@ -160,6 +160,9 @@ TimerMgrImpl::TimerMgrImpl() :
void
TimerMgrImpl::setIOService(const IOServicePtr& io_service) {
if (!io_service) {
isc_throw(BadValue, "IO service object must not be null for TimerMgr");
}
io_service_ = io_service;
}
......
......@@ -24,7 +24,7 @@ class TimerMgr;
/// @brief Type definition of the shared pointer to @c TimerMgr.
typedef boost::shared_ptr<TimerMgr> TimerMgrPtr;
/// @brief Manages a pool of asynchronous interval timers for Kea server.
/// @brief Manages a pool of asynchronous interval timers.
///
/// This class holds a pool of asynchronous interval timers.
///
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment