fw: pull FaceTable construction out of Forwarder class

Refs: #4922, #4973
Change-Id: Ibbd4a8840cb0d01ebbd4cabf9c34fa78a1d23db1
diff --git a/daemon/face/face-system.cpp b/daemon/face/face-system.cpp
index 67738aa..5601057 100644
--- a/daemon/face/face-system.cpp
+++ b/daemon/face/face-system.cpp
@@ -34,15 +34,17 @@
 
 NFD_LOG_INIT(FaceSystem);
 
-static const std::string SECTION_GENERAL = "general";
-static const std::string SECTION_NETDEVBOUND = "netdev_bound";
+const std::string CFGSEC_FACESYSTEM = "face_system";
+const std::string CFGSEC_GENERAL = "general";
+const std::string CFGSEC_GENERAL_FQ = CFGSEC_FACESYSTEM + ".general";
+const std::string CFGSEC_NETDEVBOUND = "netdev_bound";
 
 FaceSystem::FaceSystem(FaceTable& faceTable, shared_ptr<ndn::net::NetworkMonitor> netmon)
   : m_faceTable(faceTable)
   , m_netmon(std::move(netmon))
 {
   auto pfCtorParams = this->makePFCtorParams();
-  for (const std::string& id : ProtocolFactory::listRegistered()) {
+  for (const auto& id : ProtocolFactory::listRegistered()) {
     NFD_LOG_TRACE("creating factory " << id);
     m_factories[id] = ProtocolFactory::create(id, pfCtorParams);
   }
@@ -53,7 +55,7 @@
 ProtocolFactoryCtorParams
 FaceSystem::makePFCtorParams()
 {
-  auto addFace = [&ft = m_faceTable] (auto face) { ft.add(std::move(face)); };
+  auto addFace = [this] (auto face) { m_faceTable.add(std::move(face)); };
   return {addFace, m_netmon};
 }
 
@@ -92,25 +94,25 @@
 void
 FaceSystem::setConfigFile(ConfigFile& configFile)
 {
-  configFile.addSectionHandler("face_system", bind(&FaceSystem::processConfig, this, _1, _2, _3));
+  configFile.addSectionHandler(CFGSEC_FACESYSTEM, bind(&FaceSystem::processConfig, this, _1, _2, _3));
 }
 
 void
-FaceSystem::processConfig(const ConfigSection& configSection, bool isDryRun, const std::string& filename)
+FaceSystem::processConfig(const ConfigSection& configSection, bool isDryRun, const std::string&)
 {
   ConfigContext context;
   context.isDryRun = isDryRun;
 
   // process general protocol factory config section
-  auto generalSection = configSection.get_child_optional(SECTION_GENERAL);
+  auto generalSection = configSection.get_child_optional(CFGSEC_GENERAL);
   if (generalSection) {
     for (const auto& pair : *generalSection) {
       const std::string& key = pair.first;
       if (key == "enable_congestion_marking") {
-        context.generalConfig.wantCongestionMarking = ConfigFile::parseYesNo(pair, "face_system.general");
+        context.generalConfig.wantCongestionMarking = ConfigFile::parseYesNo(pair, CFGSEC_GENERAL_FQ);
       }
       else {
-        NDN_THROW(ConfigFile::Error("Unrecognized option face_system.general." + key));
+        NDN_THROW(ConfigFile::Error("Unrecognized option " + CFGSEC_GENERAL_FQ + "." + key));
       }
     }
   }
@@ -140,7 +142,7 @@
   }
 
   // process netdev_bound section, after factories start providing *+dev schemes
-  auto netdevBoundSection = configSection.get_child_optional(SECTION_NETDEVBOUND);
+  auto netdevBoundSection = configSection.get_child_optional(CFGSEC_NETDEVBOUND);
   m_netdevBound->processConfig(netdevBoundSection, context);
 
   // process other sections
@@ -150,15 +152,15 @@
     // const ConfigSection& subSection = pair.second;
 
     if (!seenSections.insert(sectionName).second) {
-      NDN_THROW(ConfigFile::Error("Duplicate section face_system." + sectionName));
+      NDN_THROW(ConfigFile::Error("Duplicate section " + CFGSEC_FACESYSTEM + "." + sectionName));
     }
 
-    if (sectionName == SECTION_GENERAL || sectionName == SECTION_NETDEVBOUND ||
+    if (sectionName == CFGSEC_GENERAL || sectionName == CFGSEC_NETDEVBOUND ||
         m_factories.count(sectionName) > 0) {
       continue;
     }
 
-    NDN_THROW(ConfigFile::Error("Unrecognized option face_system." + sectionName));
+    NDN_THROW(ConfigFile::Error("Unrecognized option " + CFGSEC_FACESYSTEM + "." + sectionName));
   }
 }
 
diff --git a/daemon/fw/face-table.hpp b/daemon/fw/face-table.hpp
index f501813..d1c51c7 100644
--- a/daemon/fw/face-table.hpp
+++ b/daemon/fw/face-table.hpp
@@ -1,6 +1,6 @@
 /* -*- Mode:C++; c-file-style:"gnu"; indent-tabs-mode:nil; -*- */
-/**
- * Copyright (c) 2014-2017,  Regents of the University of California,
+/*
+ * Copyright (c) 2014-2019,  Regents of the University of California,
  *                           Arizona Board of Regents,
  *                           Colorado State University,
  *                           University Pierre & Marie Curie, Sorbonne University,
@@ -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>
 
@@ -79,15 +80,15 @@
   end() const;
 
 public: // signals
-  /** \brief fires after a face is added
+  /** \brief Fires immediately after a face is added.
    */
-  signal::Signal<FaceTable, Face&> afterAdd;
+  signal::Signal<FaceTable, Face> afterAdd;
 
-  /** \brief fires before a face is removed
+  /** \brief Fires immediately before a face is removed.
    *
-   *  When this signal is emitted, face is still in FaceTable and has valid FaceId.
+   *  When this signal is emitted, the face is still in FaceTable and has a valid FaceId.
    */
-  signal::Signal<FaceTable, Face&> beforeRemove;
+  signal::Signal<FaceTable, Face> beforeRemove;
 
 private:
   void
diff --git a/daemon/fw/forwarder.cpp b/daemon/fw/forwarder.cpp
index 4c91fb7..f59963e 100644
--- a/daemon/fw/forwarder.cpp
+++ b/daemon/fw/forwarder.cpp
@@ -44,14 +44,15 @@
   return fw::BestRouteStrategy2::getStrategyName();
 }
 
-Forwarder::Forwarder()
-  : m_unsolicitedDataPolicy(make_unique<fw::DefaultUnsolicitedDataPolicy>())
+Forwarder::Forwarder(FaceTable& faceTable)
+  : m_faceTable(faceTable)
+  , m_unsolicitedDataPolicy(make_unique<fw::DefaultUnsolicitedDataPolicy>())
   , m_fib(m_nameTree)
   , m_pit(m_nameTree)
   , m_measurements(m_nameTree)
   , m_strategyChoice(*this)
 {
-  m_faceTable.afterAdd.connect([this] (Face& face) {
+  m_faceTable.afterAdd.connect([this] (const Face& face) {
     face.afterReceiveInterest.connect(
       [this, &face] (const Interest& interest, const EndpointId& endpointId) {
         this->startProcessInterest(FaceEndpoint(face, endpointId), interest);
@@ -70,7 +71,7 @@
       });
   });
 
-  m_faceTable.beforeRemove.connect([this] (Face& face) {
+  m_faceTable.beforeRemove.connect([this] (const Face& face) {
     cleanupOnFaceRemoval(m_nameTree, m_fib, m_pit, face);
   });
 
diff --git a/daemon/fw/forwarder.hpp b/daemon/fw/forwarder.hpp
index cc1d048..3b14a61 100644
--- a/daemon/fw/forwarder.hpp
+++ b/daemon/fw/forwarder.hpp
@@ -44,14 +44,15 @@
 class Strategy;
 } // namespace fw
 
-/** \brief Main class of NFD forwarding engine.
+/** \brief Main class of NFD's forwarding engine.
  *
- *  Forwarder owns all faces and tables, and implements the forwarding pipelines.
+ *  Forwarder owns all tables and implements the forwarding pipelines.
  */
 class Forwarder
 {
 public:
-  Forwarder();
+  explicit
+  Forwarder(FaceTable& faceTable);
 
   VIRTUAL_WITH_TESTS
   ~Forwarder();
@@ -62,33 +63,6 @@
     return m_counters;
   }
 
-public: // faces and policies
-  FaceTable&
-  getFaceTable()
-  {
-    return m_faceTable;
-  }
-
-  /** \brief get existing Face
-   *
-   *  shortcut to .getFaceTable().get(face)
-   */
-  Face*
-  getFace(FaceId id) const
-  {
-    return m_faceTable.get(id);
-  }
-
-  /** \brief add new Face
-   *
-   *  shortcut to .getFaceTable().add(face)
-   */
-  void
-  addFace(shared_ptr<Face> face)
-  {
-    m_faceTable.add(face);
-  }
-
   fw::UnsolicitedDataPolicy&
   getUnsolicitedDataPolicy() const
   {
@@ -274,7 +248,7 @@
 private:
   ForwarderCounters m_counters;
 
-  FaceTable m_faceTable;
+  FaceTable& m_faceTable;
   unique_ptr<fw::UnsolicitedDataPolicy> m_unsolicitedDataPolicy;
 
   NameTree           m_nameTree;
diff --git a/daemon/fw/strategy.cpp b/daemon/fw/strategy.cpp
index b0b9362..f9f4106 100644
--- a/daemon/fw/strategy.cpp
+++ b/daemon/fw/strategy.cpp
@@ -141,8 +141,8 @@
 }
 
 Strategy::Strategy(Forwarder& forwarder)
-  : afterAddFace(forwarder.getFaceTable().afterAdd)
-  , beforeRemoveFace(forwarder.getFaceTable().beforeRemove)
+  : afterAddFace(forwarder.m_faceTable.afterAdd)
+  , beforeRemoveFace(forwarder.m_faceTable.beforeRemove)
   , m_forwarder(forwarder)
   , m_measurements(m_forwarder.getMeasurements(), m_forwarder.getStrategyChoice(), *this)
 {
diff --git a/daemon/fw/strategy.hpp b/daemon/fw/strategy.hpp
index 8c62329..99e4826 100644
--- a/daemon/fw/strategy.hpp
+++ b/daemon/fw/strategy.hpp
@@ -322,13 +322,13 @@
   Face*
   getFace(FaceId id) const
   {
-    return m_forwarder.getFace(id);
+    return getFaceTable().get(id);
   }
 
   const FaceTable&
   getFaceTable() const
   {
-    return m_forwarder.getFaceTable();
+    return m_forwarder.m_faceTable;
   }
 
 protected: // instance name
@@ -379,8 +379,8 @@
   find(const Name& instanceName);
 
 protected: // accessors
-  signal::Signal<FaceTable, Face&>& afterAddFace;
-  signal::Signal<FaceTable, Face&>& beforeRemoveFace;
+  signal::Signal<FaceTable, Face>& afterAddFace;
+  signal::Signal<FaceTable, Face>& beforeRemoveFace;
 
 private: // instance fields
   Name m_name;
diff --git a/daemon/nfd.cpp b/daemon/nfd.cpp
index 63af6c7..e07b5d8 100644
--- a/daemon/nfd.cpp
+++ b/daemon/nfd.cpp
@@ -30,6 +30,7 @@
 #include "face/face-system.hpp"
 #include "face/internal-face.hpp"
 #include "face/null-face.hpp"
+#include "fw/face-table.hpp"
 #include "fw/forwarder.hpp"
 #include "mgmt/cs-manager.hpp"
 #include "mgmt/face-manager.hpp"
@@ -76,26 +77,25 @@
 {
   configureLogging();
 
-  m_forwarder = make_unique<Forwarder>();
+  m_faceTable = make_unique<FaceTable>();
+  m_faceTable->addReserved(face::makeNullFace(), face::FACEID_NULL);
+  m_faceTable->addReserved(face::makeNullFace(FaceUri("contentstore://")), face::FACEID_CONTENT_STORE);
 
-  FaceTable& faceTable = m_forwarder->getFaceTable();
-  faceTable.addReserved(face::makeNullFace(), face::FACEID_NULL);
-  faceTable.addReserved(face::makeNullFace(FaceUri("contentstore://")), face::FACEID_CONTENT_STORE);
-  m_faceSystem = make_unique<face::FaceSystem>(faceTable, m_netmon);
+  m_faceSystem = make_unique<face::FaceSystem>(*m_faceTable, m_netmon);
+  m_forwarder = make_unique<Forwarder>(*m_faceTable);
 
   initializeManagement();
 
   PrivilegeHelper::drop();
 
   m_netmon->onNetworkStateChanged.connect([this] {
-      // delay stages, so if multiple events are triggered in short sequence,
-      // only one auto-detection procedure is triggered
-      m_reloadConfigEvent = getScheduler().schedule(5_s,
-        [this] {
-          NFD_LOG_INFO("Network change detected, reloading face section of the config file...");
-          this->reloadConfigFileFaceSection();
-        });
+    // delay stages, so if multiple events are triggered in short sequence,
+    // only one auto-detection procedure is triggered
+    m_reloadConfigEvent = getScheduler().schedule(5_s, [this] {
+      NFD_LOG_INFO("Network change detected, reloading face section of the config file...");
+      reloadConfigFileFaceSection();
     });
+  });
 }
 
 void
@@ -133,14 +133,14 @@
 Nfd::initializeManagement()
 {
   std::tie(m_internalFace, m_internalClientFace) = face::makeInternalFace(m_keyChain);
-  m_forwarder->getFaceTable().addReserved(m_internalFace, face::FACEID_INTERNAL_FACE);
+  m_faceTable->addReserved(m_internalFace, face::FACEID_INTERNAL_FACE);
 
   m_dispatcher = make_unique<ndn::mgmt::Dispatcher>(*m_internalClientFace, m_keyChain);
   m_authenticator = CommandAuthenticator::create();
 
   m_forwarderStatusManager = make_unique<ForwarderStatusManager>(*m_forwarder, *m_dispatcher);
   m_faceManager = make_unique<FaceManager>(*m_faceSystem, *m_dispatcher, *m_authenticator);
-  m_fibManager = make_unique<FibManager>(m_forwarder->getFib(), m_forwarder->getFaceTable(),
+  m_fibManager = make_unique<FibManager>(m_forwarder->getFib(), *m_faceTable,
                                          *m_dispatcher, *m_authenticator);
   m_csManager = make_unique<CsManager>(m_forwarder->getCs(), m_forwarder->getCounters(),
                                        *m_dispatcher, *m_authenticator);
diff --git a/daemon/nfd.hpp b/daemon/nfd.hpp
index 508876c..787afb8 100644
--- a/daemon/nfd.hpp
+++ b/daemon/nfd.hpp
@@ -35,10 +35,10 @@
 
 namespace nfd {
 
+class FaceTable;
 class Forwarder;
-class CommandAuthenticator;
 
-// forward-declare management modules, in the order defined in management protocol
+class CommandAuthenticator;
 class ForwarderStatusManager;
 class FaceManager;
 class FibManager;
@@ -110,8 +110,9 @@
   std::string m_configFile;
   ConfigSection m_configSection;
 
-  unique_ptr<Forwarder> m_forwarder;
+  unique_ptr<FaceTable> m_faceTable;
   unique_ptr<face::FaceSystem> m_faceSystem;
+  unique_ptr<Forwarder> m_forwarder;
 
   ndn::KeyChain& m_keyChain;
   shared_ptr<face::Face> m_internalFace;