fw: FaceTable signals use Face& instead of shared_ptr

refs #3205

Change-Id: I53ecd0d6a13d379eb1dfb872041bdde8501f05e7

daemon/fw/access-strategy.cpp

@@ -272,9 +272,9 @@ AccessStrategy::FaceInfo::FaceInfo()
 }
 
 void
-AccessStrategy::removeFaceInfo(shared_ptr<Face> face)
+AccessStrategy::removeFaceInfo(const Face& face)
 {
-  m_fit.erase(face->getId());
+  m_fit.erase(face.getId());
 }
 
 } // namespace fw

daemon/fw/access-strategy.hpp

@@ -123,7 +123,7 @@ private: // StrategyInfo
   typedef std::unordered_map<FaceId, FaceInfo> FaceInfoTable;
 
   void
-  removeFaceInfo(shared_ptr<Face> face);
+  removeFaceInfo(const Face& face);
 
 private: // forwarding procedures
   void

daemon/fw/face-table.cpp

@@ -93,26 +93,29 @@ FaceTable::addImpl(shared_ptr<Face> face, FaceId faceId)
   face->afterReceiveInterest.connect(bind(&Forwarder::startProcessInterest, &m_forwarder, ref(*face), _1));
   face->afterReceiveData.connect(bind(&Forwarder::startProcessData, &m_forwarder, ref(*face), _1));
   face->afterReceiveNack.connect(bind(&Forwarder::startProcessNack, &m_forwarder, ref(*face), _1));
-  connectFaceClosedSignal(*face, bind(&FaceTable::remove, this, face));
+  connectFaceClosedSignal(*face, bind(&FaceTable::remove, this, faceId));
 
-  this->afterAdd(face);
+  this->afterAdd(*face);
 }
 
 void
-FaceTable::remove(shared_ptr<Face> face)
+FaceTable::remove(FaceId faceId)
 {
-  this->beforeRemove(face);
+  auto i = m_faces.find(faceId);
+  BOOST_ASSERT(i != m_faces.end());
+  shared_ptr<Face> face = i->second;
 
-  FaceId faceId = face->getId();
-  m_faces.erase(faceId);
+  this->beforeRemove(*face);
+
+  m_forwarder.getFib().removeNextHopFromAllEntries(*face);
+
+  m_faces.erase(i);
   face->setId(face::INVALID_FACEID);
 
   NFD_LOG_INFO("Removed face id=" << faceId <<
                " remote=" << face->getRemoteUri() <<
                " local=" << face->getLocalUri());
 
-  m_forwarder.getFib().removeNextHopFromAllEntries(*face);
-
   // defer Face deallocation, so that Transport isn't deallocated during afterStateChange signal
   getGlobalIoService().post([face] {});
 }

daemon/fw/face-table.hpp

@@ -53,7 +53,7 @@ public:
   VIRTUAL_WITH_TESTS void
   add(shared_ptr<Face> face);
 
-  /** \brief add a special Face with a reserved FaceId
+  /** \brief add a special face with a reserved FaceId
    */
   VIRTUAL_WITH_TESTS void
   addReserved(shared_ptr<Face> face, FaceId faceId);
@@ -86,22 +86,22 @@ public: // enumeration
   end() const;
 
 public: // signals
-  /** \brief fires after a Face is added
+  /** \brief fires after a face is added
    */
-  signal::Signal<FaceTable, shared_ptr<Face>> afterAdd;
+  signal::Signal<FaceTable, Face&> afterAdd;
 
-  /** \brief fires before a Face is removed
+  /** \brief fires before a face is removed
    *
-   *  FaceId is valid when this event is fired
+   *  When this signal is emitted, face is still in FaceTable and has valid FaceId.
    */
-  signal::Signal<FaceTable, shared_ptr<Face>> beforeRemove;
+  signal::Signal<FaceTable, Face&> beforeRemove;
 
 private:
   void
   addImpl(shared_ptr<Face> face, FaceId faceId);
 
   void
-  remove(shared_ptr<Face> face);
+  remove(FaceId faceId);
 
   ForwardRange
   getForwardRange() const;

daemon/fw/strategy.hpp

@@ -178,8 +178,8 @@ protected: // accessors
   getFaceTable() const;
 
 protected: // accessors
-  signal::Signal<FaceTable, shared_ptr<Face>>& afterAddFace;
-  signal::Signal<FaceTable, shared_ptr<Face>>& beforeRemoveFace;
+  signal::Signal<FaceTable, Face&>& afterAddFace;
+  signal::Signal<FaceTable, Face&>& beforeRemoveFace;
 
 private:
   Name m_name;

daemon/mgmt/face-manager.cpp

@@ -52,9 +52,7 @@ namespace nfd {
 
 NFD_LOG_INIT("FaceManager");
 
-FaceManager::FaceManager(FaceTable& faceTable,
-                         Dispatcher& dispatcher,
-                         CommandValidator& validator)
+FaceManager::FaceManager(FaceTable& faceTable, Dispatcher& dispatcher, CommandValidator& validator)
   : NfdManagerBase(dispatcher, validator, "faces")
   , m_faceTable(faceTable)
 {
@@ -74,11 +72,9 @@ FaceManager::FaceManager(FaceTable& faceTable,
   registerStatusDatasetHandler("channels", bind(&FaceManager::listChannels, this, _1, _2, _3));
   registerStatusDatasetHandler("query", bind(&FaceManager::queryFaces, this, _1, _2, _3));
 
-  auto postNotification = registerNotificationStream("events");
-  m_faceAddConn =
-    m_faceTable.afterAdd.connect(bind(&FaceManager::afterFaceAdded, this, _1, postNotification));
-  m_faceRemoveConn =
-    m_faceTable.beforeRemove.connect(bind(&FaceManager::afterFaceRemoved, this, _1, postNotification));
+  m_postNotification = registerNotificationStream("events");
+  m_faceAddConn = m_faceTable.afterAdd.connect(bind(&FaceManager::notifyAddFace, this, _1));
+  m_faceRemoveConn = m_faceTable.beforeRemove.connect(bind(&FaceManager::notifyRemoveFace, this, _1));
 }
 
 void
@@ -386,25 +382,23 @@ FaceManager::collectFaceProperties(const Face& face, FaceTraits& traits)
 }
 
 void
-FaceManager::afterFaceAdded(shared_ptr<Face> face,
-                            const ndn::mgmt::PostNotification& post)
+FaceManager::notifyAddFace(const Face& face)
 {
   ndn::nfd::FaceEventNotification notification;
   notification.setKind(ndn::nfd::FACE_EVENT_CREATED);
-  collectFaceProperties(*face, notification);
+  collectFaceProperties(face, notification);
 
-  post(notification.wireEncode());
+  m_postNotification(notification.wireEncode());
 }
 
 void
-FaceManager::afterFaceRemoved(shared_ptr<Face> face,
-                              const ndn::mgmt::PostNotification& post)
+FaceManager::notifyRemoveFace(const Face& face)
 {
   ndn::nfd::FaceEventNotification notification;
   notification.setKind(ndn::nfd::FACE_EVENT_DESTROYED);
-  collectFaceProperties(*face, notification);
+  collectFaceProperties(face, notification);
 
-  post(notification.wireEncode());
+  m_postNotification(notification.wireEncode());
 }
 
 void

daemon/mgmt/face-manager.hpp

@@ -121,12 +121,10 @@ private: // helpers for StatusDataset handler
 
 private: // NotificationStream
   void
-  afterFaceAdded(shared_ptr<Face> face,
-                 const ndn::mgmt::PostNotification& post);
+  notifyAddFace(const Face& face);
 
   void
-  afterFaceRemoved(shared_ptr<Face> face,
-                   const ndn::mgmt::PostNotification& post);
+  notifyRemoveFace(const Face& face);
 
 private: // configuration
   void
@@ -155,6 +153,7 @@ PUBLIC_WITH_TESTS_ELSE_PRIVATE:
 
 private:
   FaceTable& m_faceTable;
+  ndn::mgmt::PostNotification m_postNotification;
   signal::ScopedConnection m_faceAddConn;
   signal::ScopedConnection m_faceRemoveConn;
 };

tests/daemon/fw/face-table.t.cpp

@@ -42,8 +42,8 @@ BOOST_AUTO_TEST_CASE(AddRemove)
 
   std::vector<FaceId> addHistory;
   std::vector<FaceId> removeHistory;
-  faceTable.afterAdd.connect([&] (shared_ptr<Face> face) { addHistory.push_back(face->getId()); });
-  faceTable.beforeRemove.connect([&] (shared_ptr<Face> face) { removeHistory.push_back(face->getId()); });
+  faceTable.afterAdd.connect([&] (const Face& face) { addHistory.push_back(face.getId()); });
+  faceTable.beforeRemove.connect([&] (const Face& face) { removeHistory.push_back(face.getId()); });
 
   shared_ptr<Face> face1 = make_shared<DummyFace>();
   shared_ptr<Face> face2 = make_shared<DummyFace>();

tests/daemon/fw/strategy.t.cpp

@@ -48,11 +48,11 @@ public:
   FaceTableAccessTestStrategy(Forwarder& forwarder)
     : DummyStrategy(forwarder, Name("ndn:/strategy"))
   {
-    this->afterAddFace.connect([this] (shared_ptr<Face> face) {
-      this->addedFaces.push_back(face->getId());
+    this->afterAddFace.connect([this] (const Face& face) {
+      this->addedFaces.push_back(face.getId());
     });
-    this->beforeRemoveFace.connect([this] (shared_ptr<Face> face) {
-      this->removedFaces.push_back(face->getId());
+    this->beforeRemoveFace.connect([this] (const Face& face) {
+      this->removedFaces.push_back(face.getId());
     });
   }