pingserver: code cleanup
Change-Id: Ifdb75174f54aa26619594d4f1b919396a1012998
diff --git a/tests/ping/integrated.t.cpp b/tests/ping/integrated.t.cpp
index 9e929aa..84c13e5 100644
--- a/tests/ping/integrated.t.cpp
+++ b/tests/ping/integrated.t.cpp
@@ -1,6 +1,6 @@
/* -*- Mode:C++; c-file-style:"gnu"; indent-tabs-mode:nil; -*- */
-/**
- * Copyright (c) 2014-2016, Arizona Board of Regents.
+/*
+ * Copyright (c) 2015-2018, Arizona Board of Regents.
*
* This file is part of ndn-tools (Named Data Networking Essential Tools).
* See AUTHORS.md for complete list of ndn-tools authors and contributors.
@@ -74,11 +74,11 @@
{
server::Options serverOpts;
serverOpts.prefix = "ndn:/test-prefix";
- serverOpts.freshnessPeriod = time::milliseconds(5000);
+ serverOpts.freshnessPeriod = 5_s;
serverOpts.nMaxPings = 4;
- serverOpts.shouldPrintTimestamp = false;
+ serverOpts.wantTimestamp = false;
serverOpts.payloadSize = 0;
- server.reset(new server::PingServer(serverFace, m_keyChain, serverOpts));
+ server = make_unique<server::PingServer>(serverFace, m_keyChain, serverOpts);
BOOST_REQUIRE_EQUAL(0, server->getNPings());
server->start();
@@ -88,17 +88,17 @@
clientOpts.shouldGenerateRandomSeq = false;
clientOpts.shouldPrintTimestamp = false;
clientOpts.nPings = 4;
- clientOpts.interval = time::milliseconds(100);
- clientOpts.timeout = time::milliseconds(2000);
+ clientOpts.interval = 100_ms;
+ clientOpts.timeout = 2_s;
clientOpts.startSeq = 1000;
- client.reset(new client::Ping(clientFace, clientOpts));
+ client = make_unique<client::Ping>(clientFace, clientOpts);
client->afterFinish.connect(bind(&PingIntegratedFixture::onFinish, this));
client->start();
- this->advanceClocks(io, time::milliseconds(1), 400);
+ advanceClocks(io, 1_ms, 400);
io.run();
- BOOST_REQUIRE_EQUAL(4, server->getNPings());
+ BOOST_CHECK_EQUAL(4, server->getNPings());
}
BOOST_FIXTURE_TEST_CASE(Timeout, PingIntegratedFixture)
@@ -107,11 +107,11 @@
server::Options serverOpts;
serverOpts.prefix = "ndn:/test-prefix";
- serverOpts.freshnessPeriod = time::milliseconds(5000);
+ serverOpts.freshnessPeriod = 5_s;
serverOpts.nMaxPings = 4;
- serverOpts.shouldPrintTimestamp = false;
+ serverOpts.wantTimestamp = false;
serverOpts.payloadSize = 0;
- server.reset(new server::PingServer(serverFace, m_keyChain, serverOpts));
+ server = make_unique<server::PingServer>(serverFace, m_keyChain, serverOpts);
BOOST_REQUIRE_EQUAL(0, server->getNPings());
server->start();
@@ -121,17 +121,17 @@
clientOpts.shouldGenerateRandomSeq = false;
clientOpts.shouldPrintTimestamp = false;
clientOpts.nPings = 4;
- clientOpts.interval = time::milliseconds(100);
- clientOpts.timeout = time::milliseconds(500);
+ clientOpts.interval = 100_ms;
+ clientOpts.timeout = 500_ms;
clientOpts.startSeq = 1000;
- client.reset(new client::Ping(clientFace, clientOpts));
+ client = make_unique<client::Ping>(clientFace, clientOpts);
client->afterFinish.connect(bind(&PingIntegratedFixture::onFinish, this));
client->start();
- this->advanceClocks(io, time::milliseconds(1), 1000);
+ advanceClocks(io, 1_ms, 1000);
io.run();
- BOOST_REQUIRE_EQUAL(0, server->getNPings());
+ BOOST_CHECK_EQUAL(0, server->getNPings());
}
BOOST_AUTO_TEST_SUITE_END() // TestIntegrated
diff --git a/tests/ping/server/ping-server.t.cpp b/tests/ping/server/ping-server.t.cpp
index 0192008..497d4d6 100644
--- a/tests/ping/server/ping-server.t.cpp
+++ b/tests/ping/server/ping-server.t.cpp
@@ -1,6 +1,6 @@
/* -*- Mode:C++; c-file-style:"gnu"; indent-tabs-mode:nil; -*- */
-/**
- * Copyright (c) 2014-2016, Arizona Board of Regents.
+/*
+ * Copyright (c) 2014-2018, Arizona Board of Regents.
*
* This file is part of ndn-tools (Named Data Networking Essential Tools).
* See AUTHORS.md for complete list of ndn-tools authors and contributors.
@@ -18,11 +18,12 @@
*/
#include "tools/ping/server/ping-server.hpp"
-#include <ndn-cxx/util/dummy-client-face.hpp>
#include "tests/test-common.hpp"
#include "../../identity-management-fixture.hpp"
+#include <ndn-cxx/util/dummy-client-face.hpp>
+
namespace ndn {
namespace ping {
namespace server {
@@ -63,7 +64,7 @@
opt.prefix = "ndn:/test-prefix";
opt.freshnessPeriod = time::milliseconds(5000);
opt.nMaxPings = 2;
- opt.shouldPrintTimestamp = false;
+ opt.wantTimestamp = false;
opt.payloadSize = 0;
return opt;
}
@@ -80,14 +81,14 @@
BOOST_REQUIRE_EQUAL(0, pingServer.getNPings());
pingServer.start();
- this->advanceClocks(io, time::milliseconds(1), 200);
+ advanceClocks(io, 1_ms, 200);
face.receive(makePingInterest(1000));
face.receive(makePingInterest(1001));
io.run();
- BOOST_REQUIRE_EQUAL(2, pingServer.getNPings());
+ BOOST_CHECK_EQUAL(2, pingServer.getNPings());
}
BOOST_AUTO_TEST_SUITE_END() // TestPingServer
diff --git a/tools/ping/server/ndn-ping-server.cpp b/tools/ping/server/ndn-ping-server.cpp
index 2b9db41..a51519c 100644
--- a/tools/ping/server/ndn-ping-server.cpp
+++ b/tools/ping/server/ndn-ping-server.cpp
@@ -1,6 +1,6 @@
/* -*- Mode:C++; c-file-style:"gnu"; indent-tabs-mode:nil; -*- */
-/**
- * Copyright (c) 2015, Arizona Board of Regents.
+/*
+ * Copyright (c) 2015-2018, Arizona Board of Regents.
*
* This file is part of ndn-tools (Named Data Networking Essential Tools).
* See AUTHORS.md for complete list of ndn-tools authors and contributors.
@@ -30,6 +30,8 @@
namespace ping {
namespace server {
+namespace po = boost::program_options;
+
class Runner : noncopyable
{
public:
@@ -38,171 +40,142 @@
: m_options(options)
, m_pingServer(m_face, m_keyChain, options)
, m_tracer(m_pingServer, options)
- , m_signalSetInt(m_face.getIoService(), SIGINT)
+ , m_signalSet(m_face.getIoService(), SIGINT)
{
- m_signalSetInt.async_wait(bind(&Runner::afterIntSignal, this, _1));
-
m_pingServer.afterFinish.connect([this] {
- this->cancel();
- });
+ m_pingServer.stop();
+ m_signalSet.cancel();
+ });
+
+ m_signalSet.async_wait([this] (const auto& ec, auto) {
+ if (ec != boost::asio::error::operation_aborted) {
+ m_pingServer.stop();
+ }
+ });
}
int
run()
{
+ std::cout << "PING SERVER " << m_options.prefix << std::endl;
+
try {
m_pingServer.start();
m_face.processEvents();
}
- catch (std::exception& e) {
+ catch (const std::exception& e) {
std::cerr << "ERROR: " << e.what() << std::endl;
return 1;
}
- std::cout << "\n--- ping server " << m_options.prefix << " ---" << std::endl;
- std::cout << m_pingServer.getNPings() << " packets processed" << std::endl;
-
+ std::cout << "\n--- " << m_options.prefix << " ping server statistics ---\n"
+ << m_pingServer.getNPings() << " packets processed" << std::endl;
return 0;
}
private:
- void
- cancel()
- {
- m_signalSetInt.cancel();
- m_pingServer.stop();
- }
-
- void
- afterIntSignal(const boost::system::error_code& errorCode)
- {
- if (errorCode == boost::asio::error::operation_aborted) {
- return;
- }
-
- cancel();
- }
-
-private:
const Options& m_options;
+
Face m_face;
KeyChain m_keyChain;
PingServer m_pingServer;
Tracer m_tracer;
- boost::asio::signal_set m_signalSetInt;
+ boost::asio::signal_set m_signalSet;
};
-static time::milliseconds
-getMinimumFreshnessPeriod()
-{
- return time::milliseconds(1000);
-}
-
static void
-usage(const boost::program_options::options_description& options)
+usage(std::ostream& os, const std::string& programName, const po::options_description& options)
{
- std::cout << "Usage: ndnpingserver [options] ndn:/name/prefix\n"
- "\n"
- "Starts a NDN ping server that responds to Interests under name "
- "ndn:/name/prefix/ping.\n"
- "\n";
- std::cout << options;
- exit(2);
+ os << "Usage: " << programName << " [options] <prefix>\n"
+ << "\n"
+ << "Starts a NDN ping server that responds to Interests under name ndn:<prefix>/ping\n"
+ << "\n"
+ << options;
}
-int
+static int
main(int argc, char* argv[])
{
Options options;
- options.freshnessPeriod = getMinimumFreshnessPeriod();
- options.shouldLimitSatisfied = false;
- options.nMaxPings = 0;
- options.shouldPrintTimestamp = false;
- options.payloadSize = 0;
+ std::string prefix;
+ auto nMaxPings = static_cast<std::make_signed_t<size_t>>(options.nMaxPings);
+ auto payloadSize = static_cast<std::make_signed_t<size_t>>(options.payloadSize);
- namespace po = boost::program_options;
+ po::options_description visibleDesc("Options");
+ visibleDesc.add_options()
+ ("help,h", "print this help message and exit")
+ ("freshness,x", po::value<time::milliseconds::rep>()->default_value(options.freshnessPeriod.count()),
+ "FreshnessPeriod of the ping response, in milliseconds")
+ ("satisfy,p", po::value(&nMaxPings)->default_value(nMaxPings),
+ "maximum number of pings to be satisfied (0=infinite)")
+ ("size,s", po::value(&payloadSize)->default_value(payloadSize),
+ "size of response payload")
+ ("timestamp,t", po::bool_switch(&options.wantTimestamp), "log timestamp with responses")
+ ("version,V", "print program version and exit")
+ ;
- po::options_description visibleOptDesc("Allowed options");
- visibleOptDesc.add_options()
- ("help,h", "print this message and exit")
- ("version,V", "display version and exit")
- ("freshness,x", po::value<int>(),
- ("set freshness period in milliseconds (minimum " +
- std::to_string(getMinimumFreshnessPeriod().count()) + " ms)").c_str())
- ("satisfy,p", po::value<int>(&options.nMaxPings), "set maximum number of pings to be satisfied")
- ("timestamp,t", "log timestamp with responses")
- ("size,s", po::value<int>(&options.payloadSize), "specify size of response payload")
- ;
- po::options_description hiddenOptDesc("Hidden options");
- hiddenOptDesc.add_options()
- ("prefix", po::value<std::string>(), "prefix to register")
- ;
+ po::options_description hiddenDesc;
+ hiddenDesc.add_options()
+ ("prefix", po::value<std::string>(&prefix));
- po::options_description optDesc("Allowed options");
- optDesc.add(visibleOptDesc).add(hiddenOptDesc);
+ po::positional_options_description posDesc;
+ posDesc.add("prefix", -1);
+ po::options_description optDesc;
+ optDesc.add(visibleDesc).add(hiddenDesc);
+
+ po::variables_map vm;
try {
- po::positional_options_description optPos;
- optPos.add("prefix", -1);
-
- po::variables_map optVm;
- po::store(po::command_line_parser(argc, argv).options(optDesc).positional(optPos).run(), optVm);
- po::notify(optVm);
-
- if (optVm.count("help") > 0) {
- usage(visibleOptDesc);
- }
-
- if (optVm.count("version") > 0) {
- std::cout << "ndnpingserver " << tools::VERSION << std::endl;
- exit(0);
- }
-
- if (optVm.count("prefix") > 0) {
- options.prefix = Name(optVm["prefix"].as<std::string>());
- }
- else {
- std::cerr << "ERROR: No prefix specified" << std::endl;
- usage(visibleOptDesc);
- }
-
- if (optVm.count("freshness") > 0) {
- options.freshnessPeriod = time::milliseconds(optVm["freshness"].as<int>());
-
- if (options.freshnessPeriod.count() < getMinimumFreshnessPeriod().count()) {
- std::cerr << "ERROR: Specified FreshnessPeriod is less than the minimum "
- << getMinimumFreshnessPeriod() << std::endl;
- usage(visibleOptDesc);
- }
- }
-
- if (optVm.count("satisfy") > 0) {
- options.shouldLimitSatisfied = true;
-
- if (options.nMaxPings < 1) {
- std::cerr << "ERROR: Maximum number of pings to satisfy must be greater than 0" << std::endl;
- usage(visibleOptDesc);
- }
- }
-
- if (optVm.count("timestamp") > 0) {
- options.shouldPrintTimestamp = true;
- }
-
- if (optVm.count("size") > 0) {
- if (options.payloadSize < 0) {
- std::cerr << "ERROR: Payload size must be greater than or equal to 0" << std::endl;
- usage(visibleOptDesc);
- }
- }
+ po::store(po::command_line_parser(argc, argv).options(optDesc).positional(posDesc).run(), vm);
+ po::notify(vm);
}
catch (const po::error& e) {
- std::cerr << "ERROR: " << e.what() << std::endl;
- usage(visibleOptDesc);
+ std::cerr << "ERROR: " << e.what() << "\n\n";
+ usage(std::cerr, argv[0], visibleDesc);
+ return 2;
+ }
+ catch (const boost::bad_any_cast& e) {
+ std::cerr << "ERROR: " << e.what() << "\n\n";
+ usage(std::cerr, argv[0], visibleDesc);
+ return 2;
}
- std::cout << "PING SERVER " << options.prefix << std::endl;
+ if (vm.count("help") > 0) {
+ usage(std::cout, argv[0], visibleDesc);
+ return 0;
+ }
+
+ if (vm.count("version") > 0) {
+ std::cout << "ndnpingserver " << tools::VERSION << std::endl;
+ return 0;
+ }
+
+ if (prefix.empty()) {
+ std::cerr << "ERROR: no name prefix specified\n\n";
+ usage(std::cerr, argv[0], visibleDesc);
+ return 2;
+ }
+ options.prefix = prefix;
+
+ options.freshnessPeriod = time::milliseconds(vm["freshness"].as<time::milliseconds::rep>());
+ if (options.freshnessPeriod < 0_ms) {
+ std::cerr << "ERROR: FreshnessPeriod cannot be negative" << std::endl;
+ return 2;
+ }
+
+ if (nMaxPings < 0) {
+ std::cerr << "ERROR: maximum number of pings to satisfy cannot be negative" << std::endl;
+ return 2;
+ }
+ options.nMaxPings = static_cast<size_t>(nMaxPings);
+
+ if (payloadSize < 0) {
+ std::cerr << "ERROR: payload size cannot be negative" << std::endl;
+ return 2;
+ }
+ options.payloadSize = static_cast<size_t>(payloadSize);
+
return Runner(options).run();
}
diff --git a/tools/ping/server/ping-server.cpp b/tools/ping/server/ping-server.cpp
index 0a94d0f..9b1403d 100644
--- a/tools/ping/server/ping-server.cpp
+++ b/tools/ping/server/ping-server.cpp
@@ -30,37 +30,36 @@
PingServer::PingServer(Face& face, KeyChain& keyChain, const Options& options)
: m_options(options)
- , m_keyChain(keyChain)
- , m_name(options.prefix)
- , m_nPings(0)
, m_face(face)
- , m_registeredPrefixId(nullptr)
+ , m_keyChain(keyChain)
+ , m_nPings(0)
+ , m_regPrefixId(nullptr)
{
- shared_ptr<Buffer> b = make_shared<Buffer>();
+ auto b = make_shared<Buffer>();
b->assign(m_options.payloadSize, 'a');
- m_payload = Block(tlv::Content, b);
+ m_payload = Block(tlv::Content, std::move(b));
}
void
PingServer::start()
{
- m_name.append("ping");
- m_registeredPrefixId = m_face.setInterestFilter(m_name,
- bind(&PingServer::onInterest,
- this, _2),
- bind(&PingServer::onRegisterFailed,
- this, _2));
+ m_regPrefixId = m_face.setInterestFilter(
+ Name(m_options.prefix).append("ping"),
+ bind(&PingServer::onInterest, this, _2),
+ [] (const auto&, const auto& reason) {
+ BOOST_THROW_EXCEPTION(std::runtime_error("Failed to register prefix: " + reason));
+ });
}
void
PingServer::stop()
{
- if (m_registeredPrefixId != nullptr) {
- m_face.unsetInterestFilter(m_registeredPrefixId);
+ if (m_regPrefixId != nullptr) {
+ m_face.unsetInterestFilter(m_regPrefixId);
}
}
-int
+size_t
PingServer::getNPings() const
{
return m_nPings;
@@ -69,28 +68,20 @@
void
PingServer::onInterest(const Interest& interest)
{
- Name interestName = interest.getName();
+ afterReceive(interest.getName());
- afterReceive(interestName);
-
- shared_ptr<Data> data = make_shared<Data>(interestName);
+ auto data = make_shared<Data>(interest.getName());
data->setFreshnessPeriod(m_options.freshnessPeriod);
data->setContent(m_payload);
m_keyChain.sign(*data, signingWithSha256());
m_face.put(*data);
++m_nPings;
- if (m_options.shouldLimitSatisfied && m_options.nMaxPings > 0 && m_options.nMaxPings == m_nPings) {
+ if (m_options.nMaxPings > 0 && m_options.nMaxPings == m_nPings) {
afterFinish();
}
}
-void
-PingServer::onRegisterFailed(const std::string& reason)
-{
- throw "Failed to register prefix in local hub's daemon, REASON: " + reason;
-}
-
} // namespace server
} // namespace ping
} // namespace ndn
diff --git a/tools/ping/server/ping-server.hpp b/tools/ping/server/ping-server.hpp
index ff8e1c7..2d81e5f 100644
--- a/tools/ping/server/ping-server.hpp
+++ b/tools/ping/server/ping-server.hpp
@@ -1,6 +1,6 @@
/* -*- Mode:C++; c-file-style:"gnu"; indent-tabs-mode:nil; -*- */
-/**
- * Copyright (c) 2015, Arizona Board of Regents.
+/*
+ * Copyright (c) 2015-2018, Arizona Board of Regents.
*
* This file is part of ndn-tools (Named Data Networking Essential Tools).
* See AUTHORS.md for complete list of ndn-tools authors and contributors.
@@ -30,16 +30,15 @@
namespace server {
/**
- * @brief options for ndnping server
+ * @brief Options for PingServer
*/
struct Options
{
- Name prefix; //!< prefix to register
- time::milliseconds freshnessPeriod; //!< freshness period
- bool shouldLimitSatisfied; //!< should limit the number of pings satisfied
- int nMaxPings; //!< max number of pings to satisfy
- bool shouldPrintTimestamp; //!< print timestamp when response sent
- int payloadSize; //!< user specified payload size
+ Name prefix; //!< prefix to register
+ time::milliseconds freshnessPeriod = 1_s; //!< data freshness period
+ size_t nMaxPings = 0; //!< max number of pings to satisfy (0 == no limit)
+ size_t payloadSize = 0; //!< response payload size (0 == no payload)
+ bool wantTimestamp = false; //!< print timestamp when response sent
};
/**
@@ -87,7 +86,7 @@
/**
* @brief gets the number of pings received
*/
- int
+ size_t
getNPings() const;
private:
@@ -99,27 +98,17 @@
void
onInterest(const Interest& interest);
- /**
- * @brief Called when prefix registration failed
- *
- * @param reason reason for failure
- */
- void
- onRegisterFailed(const std::string& reason);
-
private:
const Options& m_options;
- KeyChain& m_keyChain;
- Name m_name;
- int m_nPings;
Face& m_face;
+ KeyChain& m_keyChain;
+ size_t m_nPings;
Block m_payload;
-
- const RegisteredPrefixId* m_registeredPrefixId;
+ const RegisteredPrefixId* m_regPrefixId;
};
} // namespace server
} // namespace ping
} // namespace ndn
-#endif //NDN_TOOLS_PING_SERVER_PING_SERVER_HPP
+#endif // NDN_TOOLS_PING_SERVER_PING_SERVER_HPP
diff --git a/tools/ping/server/tracer.cpp b/tools/ping/server/tracer.cpp
index 305fc5d..5f5150a 100644
--- a/tools/ping/server/tracer.cpp
+++ b/tools/ping/server/tracer.cpp
@@ -1,6 +1,6 @@
/* -*- Mode:C++; c-file-style:"gnu"; indent-tabs-mode:nil; -*- */
-/**
- * Copyright (c) 2015, Arizona Board of Regents.
+/*
+ * Copyright (c) 2015-2018, Arizona Board of Regents.
*
* This file is part of ndn-tools (Named Data Networking Essential Tools).
* See AUTHORS.md for complete list of ndn-tools authors and contributors.
@@ -28,17 +28,17 @@
Tracer::Tracer(PingServer& pingServer, const Options& options)
: m_options(options)
{
- pingServer.afterReceive.connect(bind(&Tracer::onReceive, this, _1));
+ pingServer.afterReceive.connect([this] (const Name& name) { onReceive(name); });
}
void
Tracer::onReceive(const Name& name)
{
- if (m_options.shouldPrintTimestamp) {
- std::cout << time::toIsoString(time::system_clock::now()) << " - ";
+ if (m_options.wantTimestamp) {
+ std::cout << time::toIsoString(time::system_clock::now()) << " - ";
}
- std::cout << "interest received: seq=" << name.at(-1).toUri() << std::endl;
+ std::cout << "interest received: seq=" << name.at(-1) << std::endl;
}
} // namespace server