fw: FaceTable iterator dereferences to Face& instead of shared_ptr
This commit also improves the speed and reliability of
Mgmt/TestFaceManager/CreateFace test suite.
refs #3205
Change-Id: Idd013488ced2d7f8072ef8a3d910f94da2e0c8ac
diff --git a/daemon/fw/face-table.cpp b/daemon/fw/face-table.cpp
index b2f1332..2d40bbc 100644
--- a/daemon/fw/face-table.cpp
+++ b/daemon/fw/face-table.cpp
@@ -120,7 +120,7 @@
FaceTable::ForwardRange
FaceTable::getForwardRange() const
{
- return m_faces | boost::adaptors::map_values;
+ return m_faces | boost::adaptors::map_values | boost::adaptors::indirected;
}
FaceTable::const_iterator
diff --git a/daemon/fw/face-table.hpp b/daemon/fw/face-table.hpp
index 19517b7..c8f93d0 100644
--- a/daemon/fw/face-table.hpp
+++ b/daemon/fw/face-table.hpp
@@ -27,6 +27,7 @@
#define NFD_DAEMON_FW_FACE_TABLE_HPP
#include "face/face.hpp"
+#include <boost/range/adaptor/indirected.hpp>
#include <boost/range/adaptor/map.hpp>
namespace nfd {
@@ -72,9 +73,9 @@
public: // enumeration
typedef std::map<FaceId, shared_ptr<Face>> FaceMap;
- typedef boost::select_second_const_range<FaceMap> ForwardRange;
+ typedef boost::indirected_range<const boost::select_second_const_range<FaceMap>> ForwardRange;
- /** \brief ForwardIterator for shared_ptr<Face>
+ /** \brief ForwardIterator for Face&
*/
typedef boost::range_iterator<ForwardRange>::type const_iterator;
diff --git a/daemon/mgmt/face-manager.cpp b/daemon/mgmt/face-manager.cpp
index ac9085c..157908c 100644
--- a/daemon/mgmt/face-manager.cpp
+++ b/daemon/mgmt/face-manager.cpp
@@ -249,8 +249,8 @@
ndn::mgmt::StatusDatasetContext& context)
{
auto now = time::steady_clock::now();
- for (const auto& face : m_faceTable) {
- ndn::nfd::FaceStatus status = collectFaceStatus(*face, now);
+ for (const Face& face : m_faceTable) {
+ ndn::nfd::FaceStatus status = collectFaceStatus(face, now);
context.append(status.wireEncode());
}
context.end();
@@ -294,11 +294,11 @@
}
auto now = time::steady_clock::now();
- for (const auto& face : m_faceTable) {
- if (!doesMatchFilter(faceFilter, face)) {
+ for (const Face& face : m_faceTable) {
+ if (!matchFilter(faceFilter, face)) {
continue;
}
- ndn::nfd::FaceStatus status = collectFaceStatus(*face, now);
+ ndn::nfd::FaceStatus status = collectFaceStatus(face, now);
context.append(status.wireEncode());
}
@@ -306,41 +306,41 @@
}
bool
-FaceManager::doesMatchFilter(const ndn::nfd::FaceQueryFilter& filter, shared_ptr<Face> face)
+FaceManager::matchFilter(const ndn::nfd::FaceQueryFilter& filter, const Face& face)
{
if (filter.hasFaceId() &&
- filter.getFaceId() != static_cast<uint64_t>(face->getId())) {
+ filter.getFaceId() != static_cast<uint64_t>(face.getId())) {
return false;
}
if (filter.hasUriScheme() &&
- filter.getUriScheme() != face->getRemoteUri().getScheme() &&
- filter.getUriScheme() != face->getLocalUri().getScheme()) {
+ filter.getUriScheme() != face.getRemoteUri().getScheme() &&
+ filter.getUriScheme() != face.getLocalUri().getScheme()) {
return false;
}
if (filter.hasRemoteUri() &&
- filter.getRemoteUri() != face->getRemoteUri().toString()) {
+ filter.getRemoteUri() != face.getRemoteUri().toString()) {
return false;
}
if (filter.hasLocalUri() &&
- filter.getLocalUri() != face->getLocalUri().toString()) {
+ filter.getLocalUri() != face.getLocalUri().toString()) {
return false;
}
if (filter.hasFaceScope() &&
- filter.getFaceScope() != face->getScope()) {
+ filter.getFaceScope() != face.getScope()) {
return false;
}
if (filter.hasFacePersistency() &&
- filter.getFacePersistency() != face->getPersistency()) {
+ filter.getFacePersistency() != face.getPersistency()) {
return false;
}
if (filter.hasLinkType() &&
- filter.getLinkType() != face->getLinkType()) {
+ filter.getLinkType() != face.getLinkType()) {
return false;
}
diff --git a/daemon/mgmt/face-manager.hpp b/daemon/mgmt/face-manager.hpp
index b97a629..aa0c05d 100644
--- a/daemon/mgmt/face-manager.hpp
+++ b/daemon/mgmt/face-manager.hpp
@@ -105,7 +105,7 @@
private: // helpers for StatusDataset handler
bool
- doesMatchFilter(const ndn::nfd::FaceQueryFilter& filter, shared_ptr<Face> face);
+ matchFilter(const ndn::nfd::FaceQueryFilter& filter, const Face& face);
/** \brief get status of face, including properties and counters
*/
diff --git a/tests/daemon/fw/face-table.t.cpp b/tests/daemon/fw/face-table.t.cpp
index 4616efb..e8962b8 100644
--- a/tests/daemon/fw/face-table.t.cpp
+++ b/tests/daemon/fw/face-table.t.cpp
@@ -113,41 +113,31 @@
BOOST_CHECK_EQUAL(std::distance(faceTable.begin(), faceTable.end()), faceTable.size());
hasFace1 = hasFace2 = false;
for (FaceTable::const_iterator it = faceTable.begin(); it != faceTable.end(); ++it) {
- if (*it == face1) {
- hasFace1 = true;
- }
+ hasFace1 = hasFace1 || &*it == face1.get();
}
- BOOST_CHECK(hasFace1);
+ BOOST_CHECK_EQUAL(hasFace1, true);
faceTable.add(face2);
BOOST_CHECK_EQUAL(faceTable.size(), 2);
BOOST_CHECK_EQUAL(std::distance(faceTable.begin(), faceTable.end()), faceTable.size());
hasFace1 = hasFace2 = false;
for (FaceTable::const_iterator it = faceTable.begin(); it != faceTable.end(); ++it) {
- if (*it == face1) {
- hasFace1 = true;
- }
- if (*it == face2) {
- hasFace2 = true;
- }
+ hasFace1 = hasFace1 || &*it == face1.get();
+ hasFace2 = hasFace2 || &*it == face2.get();
}
- BOOST_CHECK(hasFace1);
- BOOST_CHECK(hasFace2);
+ BOOST_CHECK_EQUAL(hasFace1, true);
+ BOOST_CHECK_EQUAL(hasFace2, true);
face1->close();
BOOST_CHECK_EQUAL(faceTable.size(), 1);
BOOST_CHECK_EQUAL(std::distance(faceTable.begin(), faceTable.end()), faceTable.size());
hasFace1 = hasFace2 = false;
for (FaceTable::const_iterator it = faceTable.begin(); it != faceTable.end(); ++it) {
- if (*it == face1) {
- hasFace1 = true;
- }
- if (*it == face2) {
- hasFace2 = true;
- }
+ hasFace1 = hasFace1 || &*it == face1.get();
+ hasFace2 = hasFace2 || &*it == face2.get();
}
- BOOST_CHECK(!hasFace1);
- BOOST_CHECK(hasFace2);
+ BOOST_CHECK_EQUAL(hasFace1, false);
+ BOOST_CHECK_EQUAL(hasFace2, true);
}
BOOST_AUTO_TEST_SUITE_END() // TestFaceTable
diff --git a/tests/daemon/fw/strategy.t.cpp b/tests/daemon/fw/strategy.t.cpp
index 90654d7..7a15df4 100644
--- a/tests/daemon/fw/strategy.t.cpp
+++ b/tests/daemon/fw/strategy.t.cpp
@@ -1,6 +1,6 @@
/* -*- Mode:C++; c-file-style:"gnu"; indent-tabs-mode:nil; -*- */
/**
- * Copyright (c) 2014-2015, Regents of the University of California,
+ * Copyright (c) 2014-2016, Regents of the University of California,
* Arizona Board of Regents,
* Colorado State University,
* University Pierre & Marie Curie, Sorbonne University,
@@ -27,6 +27,8 @@
#include "dummy-strategy.hpp"
#include "tests/daemon/face/dummy-face.hpp"
#include <boost/range/adaptor/filtered.hpp>
+#include <boost/range/adaptor/transformed.hpp>
+#include <boost/range/algorithm/copy.hpp>
#include "tests/test-common.hpp"
@@ -58,14 +60,15 @@
getLocalFaces()
{
auto enumerable = this->getFaceTable() |
- boost::adaptors::filtered([] (shared_ptr<Face> face) {
- return face->getScope() == ndn::nfd::FACE_SCOPE_LOCAL;
+ boost::adaptors::filtered([] (Face& face) {
+ return face.getScope() == ndn::nfd::FACE_SCOPE_LOCAL;
+ }) |
+ boost::adaptors::transformed([] (Face& face) {
+ return face.getId();
});
std::vector<FaceId> results;
- for (shared_ptr<Face> face : enumerable) {
- results.push_back(face->getId());
- }
+ boost::copy(enumerable, std::back_inserter(results));
return results;
}
diff --git a/tests/daemon/mgmt/face-manager-create-face.t.cpp b/tests/daemon/mgmt/face-manager-create-face.t.cpp
index fe6d147..8c67b3a 100644
--- a/tests/daemon/mgmt/face-manager-create-face.t.cpp
+++ b/tests/daemon/mgmt/face-manager-create-face.t.cpp
@@ -25,19 +25,18 @@
#include "mgmt/face-manager.hpp"
#include "fw/forwarder.hpp"
+#include <ndn-cxx/mgmt/dispatcher.hpp>
+#include <ndn-cxx/util/dummy-client-face.hpp>
+
+#include <thread>
+#include <boost/property_tree/info_parser.hpp>
#include "tests/test-common.hpp"
#include "tests/identity-management-fixture.hpp"
-#include <ndn-cxx/mgmt/dispatcher.hpp>
-#include <ndn-cxx/util/dummy-client-face.hpp>
-
-#include <boost/property_tree/info_parser.hpp>
-
namespace nfd {
namespace tests {
-
BOOST_AUTO_TEST_SUITE(Mgmt)
BOOST_AUTO_TEST_SUITE(TestFaceManager)
@@ -47,9 +46,10 @@
{
public:
FaceManagerNode(ndn::KeyChain& keyChain, const std::string& port = "6363")
- : face(getGlobalIoService(), keyChain, {true, true})
+ : faceTable(forwarder.getFaceTable())
+ , face(getGlobalIoService(), keyChain, {true, true})
, dispatcher(face, keyChain, ndn::security::SigningInfo())
- , manager(forwarder.getFaceTable(), dispatcher, validator)
+ , manager(faceTable, dispatcher, validator)
{
dispatcher.addTopPrefix("/localhost/nfd");
@@ -95,16 +95,17 @@
void
closeFaces()
{
- std::vector<shared_ptr<Face>> facesToClose;
+ std::vector<std::reference_wrapper<Face>> facesToClose;
std::copy(forwarder.getFaceTable().begin(), forwarder.getFaceTable().end(),
std::back_inserter(facesToClose));
- for (auto face : facesToClose) {
- face->close();
+ for (Face& face : facesToClose) {
+ face.close();
}
}
public:
Forwarder forwarder;
+ FaceTable& faceTable;
ndn::util::DummyClientFace face;
ndn::mgmt::Dispatcher dispatcher;
CommandValidator validator;
@@ -119,14 +120,16 @@
: node1(m_keyChain, "16363")
, node2(m_keyChain, "26363")
{
- advanceClocks(time::milliseconds(1), 100);
+ advanceClocks(time::milliseconds(1), 5);
}
~FaceManagerFixture()
{
+ // Explicitly closing faces is necessary. Otherwise, in a subsequent test case,
+ // incoming packets may be delivered to an old socket from previous test cases.
node1.closeFaces();
node2.closeFaces();
- advanceClocks(time::milliseconds(1), 100);
+ advanceClocks(time::milliseconds(1), 5);
}
public:
@@ -262,13 +265,11 @@
Name commandName("/localhost/nfd/faces");
commandName.append("create");
commandName.append(FaceType().getParameters().wireEncode());
-
- shared_ptr<Interest> command(make_shared<Interest>(commandName));
+ auto command = makeInterest(commandName);
m_keyChain.sign(*command);
bool hasCallbackFired = false;
this->node1.face.onSendData.connect([this, command, &hasCallbackFired] (const Data& response) {
- // std::cout << response << std::endl;
if (!command->getName().isPrefixOf(response.getName())) {
return;
}
@@ -289,7 +290,7 @@
});
this->node1.face.receive(*command);
- this->advanceClocks(time::milliseconds(1), 100);
+ this->advanceClocks(time::milliseconds(1), 5);
BOOST_CHECK(hasCallbackFired);
}
@@ -313,12 +314,11 @@
Name commandName("/localhost/nfd/faces");
commandName.append("create");
commandName.append(FaceType1().getParameters().wireEncode());
-
- shared_ptr<Interest> command(make_shared<Interest>(commandName));
+ auto command = makeInterest(commandName);
m_keyChain.sign(*command);
this->node1.face.receive(*command);
- this->advanceClocks(time::milliseconds(1), 10);
+ this->advanceClocks(time::milliseconds(1), 5);
}
//
@@ -328,8 +328,7 @@
Name commandName("/localhost/nfd/faces");
commandName.append("create");
commandName.append(FaceType2().getParameters().wireEncode());
-
- shared_ptr<Interest> command(make_shared<Interest>(commandName));
+ auto command = makeInterest(commandName);
m_keyChain.sign(*command);
bool hasCallbackFired = false;
@@ -349,7 +348,7 @@
});
this->node1.face.receive(*command);
- this->advanceClocks(time::milliseconds(1), 10);
+ this->advanceClocks(time::milliseconds(1), 5);
BOOST_CHECK(hasCallbackFired);
}
@@ -389,8 +388,7 @@
Name commandName("/localhost/nfd/faces");
commandName.append("create");
commandName.append(OtherNodeFace().getParameters().wireEncode());
-
- shared_ptr<Interest> command(make_shared<Interest>(commandName));
+ auto command = makeInterest(commandName);
m_keyChain.sign(*command);
ndn::util::signal::ScopedConnection connection =
@@ -411,19 +409,21 @@
});
this->node2.face.receive(*command);
- this->advanceClocks(time::milliseconds(1), 10);
+ this->advanceClocks(time::milliseconds(1), 5); // let node2 process command and send Interest
+ std::this_thread::sleep_for(std::chrono::milliseconds(100)); // allow wallclock time for socket IO
+ this->advanceClocks(time::milliseconds(1), 5); // let node1 accept Interest and create on-demand face
}
// make sure there is on-demand face
- bool onDemandFaceFound = false;
FaceUri onDemandFaceUri(FaceType().getParameters().getUri());
- for (auto face : this->node1.forwarder.getFaceTable()) {
- if (face->getRemoteUri() == onDemandFaceUri) {
- onDemandFaceFound = true;
+ const Face* foundFace = nullptr;
+ for (const Face& face : this->node1.faceTable) {
+ if (face.getRemoteUri() == onDemandFaceUri) {
+ foundFace = &face;
break;
}
}
- BOOST_REQUIRE(onDemandFaceFound);
+ BOOST_REQUIRE_MESSAGE(foundFace != nullptr, "on-demand face is not created");
//
{
@@ -432,12 +432,12 @@
Name commandName("/localhost/nfd/faces");
commandName.append("create");
commandName.append(FaceType().getParameters().wireEncode());
-
- shared_ptr<Interest> command(make_shared<Interest>(commandName));
+ auto command = makeInterest(commandName);
m_keyChain.sign(*command);
bool hasCallbackFired = false;
- this->node1.face.onSendData.connect([this, command, &hasCallbackFired] (const Data& response) {
+ this->node1.face.onSendData.connect(
+ [this, command, &hasCallbackFired, foundFace] (const Data& response) {
if (!command->getName().isPrefixOf(response.getName())) {
return;
}
@@ -447,13 +447,15 @@
ControlParameters expectedParams(FaceType().getParameters());
ControlParameters actualParams(actual.getBody());
- BOOST_CHECK_EQUAL(expectedParams.getFacePersistency(), actualParams.getFacePersistency());
+ BOOST_CHECK_EQUAL(actualParams.getFacePersistency(), expectedParams.getFacePersistency());
+ BOOST_CHECK_EQUAL(actualParams.getFaceId(), foundFace->getId());
+ BOOST_CHECK_EQUAL(foundFace->getPersistency(), expectedParams.getFacePersistency());
hasCallbackFired = true;
});
this->node1.face.receive(*command);
- this->advanceClocks(time::milliseconds(1), 10);
+ this->advanceClocks(time::milliseconds(1), 5);
BOOST_CHECK(hasCallbackFired);
}