rib: Cancel expiration on unregistration

refs: #1902

Change-Id: I85a75370ecc3677c8a9ebf3a48f3c0857598da2d
diff --git a/rib/rib-entry.cpp b/rib/rib-entry.cpp
index 5043233..3e24275 100644
--- a/rib/rib-entry.cpp
+++ b/rib/rib-entry.cpp
@@ -25,8 +25,12 @@
 
 #include "rib-entry.hpp"
 
+#include "core/logger.hpp"
+
 #include <ndn-cxx/management/nfd-control-command.hpp>
 
+NFD_LOG_INIT("RibEntry");
+
 namespace nfd {
 namespace rib {
 
@@ -57,28 +61,11 @@
     }
 }
 
-bool
+void
 RibEntry::eraseFace(const FaceEntry& face)
 {
   RibEntry::iterator it = std::find_if(begin(), end(), bind(&compareFaceIdAndOrigin, _1, face));
-
-  if (it != m_faces.end())
-    {
-      if (it->flags & ndn::nfd::ROUTE_FLAG_CAPTURE)
-        {
-          m_nFacesWithCaptureSet--;
-        }
-
-      //cancel any scheduled event
-      scheduler::cancel(it->getExpirationEvent());
-
-      m_faces.erase(it);
-      return true;
-    }
-  else
-    {
-      return false;
-    }
+  eraseFace(it);
 }
 
 bool
@@ -124,6 +111,10 @@
           m_nFacesWithCaptureSet--;
         }
 
+      //cancel any scheduled event
+      NFD_LOG_TRACE("Cancelling expiration eventId: " << face->getExpirationEvent());
+      scheduler::cancel(face->getExpirationEvent());
+
       return m_faces.erase(face);
     }
 
diff --git a/rib/rib-entry.hpp b/rib/rib-entry.hpp
index fc78624..b17085a 100644
--- a/rib/rib-entry.hpp
+++ b/rib/rib-entry.hpp
@@ -79,9 +79,8 @@
   insertFace(const FaceEntry& face);
 
   /** \brief erases a FaceEntry with the same faceId and origin
-   *  \return{ true if the face is removed, false otherwise }
    */
-  bool
+  void
   eraseFace(const FaceEntry& face);
 
   /** \brief erases a FaceEntry with the passed iterator
diff --git a/rib/rib-manager.cpp b/rib/rib-manager.cpp
index aa859e2..59e6230 100644
--- a/rib/rib-manager.cpp
+++ b/rib/rib-manager.cpp
@@ -286,10 +286,11 @@
 
       // Schedule a new event, the old one will be cancelled during rib insertion.
       EventId eventId;
-      NFD_LOG_TRACE("scheduling unregistration at: " << faceEntry.expires);
       eventId = scheduler::schedule(parameters.getExpirationPeriod(),
-                                    bind(&RibManager::unregisterEntry,
+                                    bind(&RibManager::expireEntry,
                                     this, shared_ptr<Interest>(), parameters));
+      NFD_LOG_TRACE("Scheduled unregistration at: " << faceEntry.expires <<
+                    " with EventId: " << eventId);
 
       //set the  NewEventId of this entry
       faceEntry.setExpirationEvent(eventId);
@@ -307,6 +308,19 @@
 }
 
 void
+RibManager::expireEntry(const shared_ptr<const Interest>& request, ControlParameters& params)
+{
+  FaceEntry face;
+  face.faceId = params.getFaceId();
+  face.origin = params.getOrigin();
+  face.cost = params.getCost();
+  face.flags = params.getFlags();
+
+  NFD_LOG_DEBUG(face << " for " << params.getName() << " has expired");
+  unregisterEntry(request, params);
+}
+
+void
 RibManager::unregisterEntry(const shared_ptr<const Interest>& request,
                             ControlParameters& params)
 {
diff --git a/rib/rib-manager.hpp b/rib/rib-manager.hpp
index d08ab50..dffb24a 100644
--- a/rib/rib-manager.hpp
+++ b/rib/rib-manager.hpp
@@ -114,6 +114,9 @@
                   ControlParameters& parameters);
 
   void
+  expireEntry(const shared_ptr<const Interest>& request, ControlParameters& params);
+
+  void
   onCommandValidated(const shared_ptr<const Interest>& request);
 
   void
@@ -169,6 +172,7 @@
 
   void
   onControlHeaderError(uint32_t code, const std::string& reason);
+
   static bool
   extractParameters(const Name::Component& parameterComponent,
                     ControlParameters& extractedParameters);
@@ -216,8 +220,10 @@
   void
   onFetchFaceStatusTimeout();
 
-private:
+PUBLIC_WITH_TESTS_ELSE_PRIVATE:
   Rib m_managedRib;
+
+private:
   ndn::Face& m_face;
   ndn::nfd::Controller m_nfdController;
   ndn::KeyChain m_keyChain;
diff --git a/rib/rib.cpp b/rib/rib.cpp
index 9901b77..c76940b 100644
--- a/rib/rib.cpp
+++ b/rib/rib.cpp
@@ -25,6 +25,10 @@
 
 #include "rib.hpp"
 
+#include "core/logger.hpp"
+
+NFD_LOG_INIT("Rib");
+
 namespace nfd {
 namespace rib {
 
@@ -127,7 +131,11 @@
         {
           // First cancel old scheduled event, if any, then set the EventId to new one
           if (static_cast<bool>(faceIt->getExpirationEvent()))
+            {
+              NFD_LOG_TRACE("Cancelling expiration event for " << entry->getName() << " "
+                                                               << *faceIt);
               scheduler::cancel(faceIt->getExpirationEvent());
+            }
 
           // No checks are required here as the iterator needs to be updated in all cases.
           faceIt->setExpirationEvent(face.getExpirationEvent());
@@ -211,6 +219,7 @@
           faceToErase.cost = faceIt->cost;
 
           entry->eraseFace(faceIt);
+
           m_nItems--;
 
           const bool captureWasTurnedOff = (hadCapture && !entry->hasCapture());
diff --git a/tests/rib/rib-manager.cpp b/tests/rib/rib-manager.cpp
index 593517f..0d43dcb 100644
--- a/tests/rib/rib-manager.cpp
+++ b/tests/rib/rib-manager.cpp
@@ -27,6 +27,7 @@
 
 #include "tests/test-common.hpp"
 #include "tests/dummy-client-face.hpp"
+#include "tests/limited-io.hpp"
 #include "rib/rib-status-publisher-common.hpp"
 
 namespace nfd {
@@ -255,6 +256,45 @@
   RibStatusPublisherFixture::decodeRibEntryBlock(face->m_sentDatas[0], name, entry);
 }
 
+BOOST_FIXTURE_TEST_CASE(CancelExpirationEvent, AuthorizedRibManager)
+{
+  // Register face
+  ControlParameters addParameters;
+  addParameters
+    .setName("/expire")
+    .setFaceId(1)
+    .setCost(10)
+    .setFlags(0)
+    .setOrigin(128)
+    .setExpirationPeriod(ndn::time::milliseconds(500));
+
+  Name registerName("/localhost/nfd/rib/register");
+
+  receiveCommandInterest(registerName, addParameters);
+  face->m_sentInterests.clear();
+
+  // Unregister face
+  ControlParameters removeParameters;
+  removeParameters
+    .setName("/expire")
+    .setFaceId(1)
+    .setOrigin(128);
+
+  Name unregisterName("/localhost/nfd/rib/unregister");
+
+  receiveCommandInterest(unregisterName, removeParameters);
+
+  // Reregister face
+  Name reRegisterName("/localhost/nfd/rib/register");
+  addParameters.setExpirationPeriod(ndn::time::milliseconds::max());
+  receiveCommandInterest(reRegisterName, addParameters);
+
+  nfd::tests::LimitedIo limitedIo;
+  limitedIo.run(nfd::tests::LimitedIo::UNLIMITED_OPS, time::seconds(1));
+
+  BOOST_REQUIRE_EQUAL(manager->m_managedRib.size(), 1);
+}
+
 BOOST_AUTO_TEST_SUITE_END()
 
 } // namespace tests
diff --git a/tests/rib/rib.cpp b/tests/rib/rib.cpp
index faa1845..3226e57 100644
--- a/tests/rib/rib.cpp
+++ b/tests/rib/rib.cpp
@@ -60,7 +60,9 @@
   entry.insertFace(face2);
   BOOST_CHECK_EQUAL(entry.getFaces().size(), 1);
 
-  BOOST_CHECK_EQUAL(entry.eraseFace(face1), false);
+  entry.eraseFace(face1);
+  BOOST_CHECK_EQUAL(entry.getFaces().size(), 1);
+  BOOST_CHECK(entry.findFace(face2) != entry.getFaces().end());
 }
 
 BOOST_AUTO_TEST_CASE(Parent)