rib: change FIB mocking mechanism

Previously, RIB test suites mock FIB updates by invoking RibManager
or Rib's private methods from a callback. In this commit, Rib class
exposes a `mockFibResponse` function variable, so that test cases
can directly specify FibUpdater's response without needing to invoke
private methods. This further allows replacing these private methods
with lambdas.

Also, logic for beginning RibUpdateBatch is abstracted out of control
command handlers, in preparation for PrefixAnnouncement-based RIB
update functions.

refs #4683

Change-Id: I0c95d8daa1458c704c7352a7809e6bed69134642
diff --git a/rib/rib-manager.cpp b/rib/rib-manager.cpp
index 1d9873b..a1707ef 100644
--- a/rib/rib-manager.cpp
+++ b/rib/rib-manager.cpp
@@ -103,40 +103,112 @@
 RibManager::enableLocalFields()
 {
   m_nfdController.start<ndn::nfd::FaceUpdateCommand>(
-    ControlParameters()
-      .setFlagBit(ndn::nfd::BIT_LOCAL_FIELDS_ENABLED, true),
-    bind(&RibManager::onEnableLocalFieldsSuccess, this),
-    bind(&RibManager::onEnableLocalFieldsError, this, _1));
+    ControlParameters().setFlagBit(ndn::nfd::BIT_LOCAL_FIELDS_ENABLED, true),
+    [] (const ControlParameters& res) {
+      NFD_LOG_DEBUG("Local fields enabled");
+    },
+    [] (const ControlResponse& res) {
+      BOOST_THROW_EXCEPTION(Error("Couldn't enable local fields (" + to_string(res.getCode()) +
+                                  " " + res.getText() + ")"));
+    });
 }
 
 void
-RibManager::onRibUpdateSuccess(const RibUpdate& update)
+RibManager::beginAddRoute(const Name& name, Route route, optional<time::nanoseconds> expires,
+                          const std::function<void(RibUpdateResult)>& done)
 {
-  NFD_LOG_DEBUG("RIB update succeeded for " << update);
+  if (expires) {
+    if (*expires <= 0_ns) {
+      done(RibUpdateResult::EXPIRED);
+      return;
+    }
+    route.expires = time::steady_clock::now() + *expires;
+  }
+  else if (route.expires) {
+    expires = *route.expires - time::steady_clock::now();
+    if (*expires <= 0_ns) {
+      done(RibUpdateResult::EXPIRED);
+      return;
+    }
+  }
+
+  NFD_LOG_INFO("Adding route " << name << " nexthop=" << route.faceId <<
+               " origin=" << route.origin << " cost=" << route.cost);
+
+  if (expires) {
+    route.setExpirationEvent(scheduler::schedule(
+      *expires, [=] { m_rib.onRouteExpiration(name, route); }));
+    NFD_LOG_TRACE("Scheduled unregistration at: " << *route.expires);
+  }
+
+  m_registeredFaces.insert(route.faceId);
+
+  RibUpdate update;
+  update.setAction(RibUpdate::REGISTER)
+        .setName(name)
+        .setRoute(route);
+  beginRibUpdate(update, done);
 }
 
 void
-RibManager::onRibUpdateFailure(const RibUpdate& update, uint32_t code, const std::string& error)
+RibManager::beginRemoveRoute(const Name& name, const Route& route,
+                             const std::function<void(RibUpdateResult)>& done)
 {
-  NFD_LOG_DEBUG("RIB update failed for " << update << " (code: " << code
-                                                   << ", error: " << error << ")");
+  NFD_LOG_INFO("Removing route " << name << " nexthop=" << route.faceId <<
+               " origin=" << route.origin);
 
-  // Since the FIB rejected the update, clean up invalid routes
-  scheduleActiveFaceFetch(time::seconds(1));
+  RibUpdate update;
+  update.setAction(RibUpdate::UNREGISTER)
+        .setName(name)
+        .setRoute(route);
+  beginRibUpdate(update, done);
+}
+
+void
+RibManager::beginRibUpdate(const RibUpdate& update,
+                           const std::function<void(RibUpdateResult)>& done)
+{
+  m_rib.beginApplyUpdate(update,
+    [=] {
+      NFD_LOG_DEBUG("RIB update succeeded for " << update);
+      done(RibUpdateResult::OK);
+    },
+    [=] (uint32_t code, const std::string& error) {
+      NFD_LOG_DEBUG("RIB update failed for " << update << " (" << code << " " << error << ")");
+
+      // Since the FIB rejected the update, clean up invalid routes
+      scheduleActiveFaceFetch(1_s);
+
+      done(RibUpdateResult::ERROR);
+    });
 }
 
 void
 RibManager::registerTopPrefix(const Name& topPrefix)
 {
-  // register entry to the FIB
+  // add FIB nexthop
   m_nfdController.start<ndn::nfd::FibAddNextHopCommand>(
-    ControlParameters()
-        .setName(Name(topPrefix).append(MGMT_MODULE_NAME))
-        .setFaceId(0),
-    [=] (const auto& res) { this->onCommandPrefixAddNextHopSuccess(topPrefix, res); },
-    [=] (const auto& res) { this->onCommandPrefixAddNextHopError(topPrefix, res); });
+    ControlParameters().setName(Name(topPrefix).append(MGMT_MODULE_NAME))
+                       .setFaceId(0),
+    [=] (const ControlParameters& res) {
+      NFD_LOG_DEBUG("Successfully registered " << topPrefix << " with NFD");
 
-  // add top prefix to the dispatcher
+      // Routes must be inserted into the RIB so route flags can be applied
+      Route route;
+      route.faceId = res.getFaceId();
+      route.origin = ndn::nfd::ROUTE_ORIGIN_APP;
+      route.flags = ndn::nfd::ROUTE_FLAG_CHILD_INHERIT;
+
+      m_rib.insert(topPrefix, route);
+
+      m_registeredFaces.insert(route.faceId);
+    },
+    [=] (const ControlResponse& res) {
+      BOOST_THROW_EXCEPTION(Error("Cannot add FIB entry " + topPrefix.toUri() + " (" +
+                                  to_string(res.getCode()) + " " + res.getText() + ")"));
+    });
+
+  // add top prefix to the dispatcher without prefix registration
   m_dispatcher.addTopPrefix(topPrefix, false);
 }
 
@@ -162,38 +234,13 @@
   route.cost = parameters.getCost();
   route.flags = parameters.getFlags();
 
+  optional<time::nanoseconds> expires;
   if (parameters.hasExpirationPeriod() &&
       parameters.getExpirationPeriod() != time::milliseconds::max()) {
-    route.expires = time::steady_clock::now() + parameters.getExpirationPeriod();
-
-    // Schedule a new event, the old one will be cancelled during rib insertion.
-    scheduler::EventId eventId = scheduler::schedule(parameters.getExpirationPeriod(),
-      bind(&Rib::onRouteExpiration, &m_rib, parameters.getName(), route));
-
-    NFD_LOG_TRACE("Scheduled unregistration at: " << *route.expires <<
-                  " with EventId: " << eventId);
-
-    // Set the  NewEventId of this entry
-    route.setExpirationEvent(eventId);
-  }
-  else {
-    route.expires = nullopt;
+    expires = time::duration_cast<time::nanoseconds>(parameters.getExpirationPeriod());
   }
 
-  NFD_LOG_INFO("Adding route " << parameters.getName() << " nexthop=" << route.faceId
-                                                       << " origin=" << route.origin
-                                                       << " cost=" << route.cost);
-
-  RibUpdate update;
-  update.setAction(RibUpdate::REGISTER)
-        .setName(parameters.getName())
-        .setRoute(route);
-
-  m_rib.beginApplyUpdate(update,
-                         bind(&RibManager::onRibUpdateSuccess, this, update),
-                         bind(&RibManager::onRibUpdateFailure, this, update, _1, _2));
-
-  m_registeredFaces.insert(route.faceId);
+  beginAddRoute(parameters.getName(), std::move(route), expires, [] (RibUpdateResult) {});
 }
 
 void
@@ -210,17 +257,7 @@
   route.faceId = parameters.getFaceId();
   route.origin = parameters.getOrigin();
 
-  NFD_LOG_INFO("Removing route " << parameters.getName() << " nexthop=" << route.faceId
-                                                         << " origin=" << route.origin);
-
-  RibUpdate update;
-  update.setAction(RibUpdate::UNREGISTER)
-        .setName(parameters.getName())
-        .setRoute(route);
-
-  m_rib.beginApplyUpdate(update,
-                         bind(&RibManager::onRibUpdateSuccess, this, update),
-                         bind(&RibManager::onRibUpdateFailure, this, update, _1, _2));
+  beginRemoveRoute(parameters.getName(), route, [] (RibUpdateResult) {});
 }
 
 void
@@ -348,44 +385,5 @@
   }
 }
 
-void
-RibManager::onCommandPrefixAddNextHopSuccess(const Name& prefix,
-                                             const ndn::nfd::ControlParameters& result)
-{
-  NFD_LOG_DEBUG("Successfully registered " + prefix.toUri() + " with NFD");
-
-  // Routes must be inserted into the RIB so route flags can be applied
-  Route route;
-  route.faceId = result.getFaceId();
-  route.origin = ndn::nfd::ROUTE_ORIGIN_APP;
-  route.expires = nullopt;
-  route.flags = ndn::nfd::ROUTE_FLAG_CHILD_INHERIT;
-
-  m_rib.insert(prefix, route);
-
-  m_registeredFaces.insert(route.faceId);
-}
-
-void
-RibManager::onCommandPrefixAddNextHopError(const Name& name,
-                                           const ndn::nfd::ControlResponse& response)
-{
-  BOOST_THROW_EXCEPTION(Error("Error in setting interest filter (" + name.toUri() +
-                              "): " + response.getText()));
-}
-
-void
-RibManager::onEnableLocalFieldsSuccess()
-{
-  NFD_LOG_DEBUG("Local fields enabled");
-}
-
-void
-RibManager::onEnableLocalFieldsError(const ndn::nfd::ControlResponse& response)
-{
-  BOOST_THROW_EXCEPTION(Error("Couldn't enable local fields (code: " +
-                              to_string(response.getCode()) + ", info: " + response.getText() + ")"));
-}
-
 } // namespace rib
 } // namespace nfd
diff --git a/rib/rib-manager.hpp b/rib/rib-manager.hpp
index 6ef6d6b..169c9ec 100644
--- a/rib/rib-manager.hpp
+++ b/rib/rib-manager.hpp
@@ -87,28 +87,56 @@
   void
   enableLocalFields();
 
-PUBLIC_WITH_TESTS_ELSE_PRIVATE:
+private: // RIB and FibUpdater actions
+  enum class RibUpdateResult
+  {
+    OK,
+    ERROR,
+    EXPIRED,
+  };
+
+  /** \brief Start adding a route to RIB and FIB.
+   *  \param name route name
+   *  \param route route parameters; may contain absolute expiration time
+   *  \param expires relative expiration time; if specified, overwrites \c route.expires
+   *  \param done completion callback
+   */
   void
-  onRibUpdateSuccess(const RibUpdate& update);
+  beginAddRoute(const Name& name, Route route, optional<time::nanoseconds> expires,
+                const std::function<void(RibUpdateResult)>& done);
+
+  /** \brief Start removing a route from RIB and FIB.
+   *  \param name route name
+   *  \param route route parameters
+   *  \param done completion callback
+   */
+  void
+  beginRemoveRoute(const Name& name, const Route& route,
+                   const std::function<void(RibUpdateResult)>& done);
 
   void
-  onRibUpdateFailure(const RibUpdate& update, uint32_t code, const std::string& error);
+  beginRibUpdate(const RibUpdate& update, const std::function<void(RibUpdateResult)>& done);
 
-private: // initialization helpers
+private: // management Dispatcher related
   void
   registerTopPrefix(const Name& topPrefix);
 
-private: // ControlCommand and StatusDataset
+  /** \brief Serve rib/register command.
+   */
   void
   registerEntry(const Name& topPrefix, const Interest& interest,
                 ControlParameters parameters,
                 const ndn::mgmt::CommandContinuation& done);
 
+  /** \brief Serve rib/unregister command.
+   */
   void
   unregisterEntry(const Name& topPrefix, const Interest& interest,
                   ControlParameters parameters,
                   const ndn::mgmt::CommandContinuation& done);
 
+  /** \brief Serve rib/list dataset.
+   */
   void
   listEntries(const Name& topPrefix, const Interest& interest,
               ndn::mgmt::StatusDatasetContext& context);
@@ -150,19 +178,6 @@
   onNotification(const ndn::nfd::FaceEventNotification& notification);
 
 private:
-  void
-  onCommandPrefixAddNextHopSuccess(const Name& prefix, const ControlParameters& result);
-
-  void
-  onCommandPrefixAddNextHopError(const Name& name, const ControlResponse& response);
-
-  void
-  onEnableLocalFieldsSuccess();
-
-  void
-  onEnableLocalFieldsError(const ControlResponse& response);
-
-private:
   Rib& m_rib;
   ndn::nfd::Controller& m_nfdController;
   Dispatcher& m_dispatcher;
diff --git a/rib/rib.cpp b/rib/rib.cpp
index 6a221ea..542a582 100644
--- a/rib/rib.cpp
+++ b/rib/rib.cpp
@@ -396,18 +396,27 @@
   // Until task #1698, each RibUpdateBatch contains exactly one RIB update
   BOOST_ASSERT(batch.size() == 1);
 
-  const Rib::UpdateSuccessCallback& managerSuccessCallback = item.managerSuccessCallback;
-  const Rib::UpdateFailureCallback& managerFailureCallback = item.managerFailureCallback;
+  auto fibSuccessCb = bind(&Rib::onFibUpdateSuccess, this, batch, _1, item.managerSuccessCallback);
+  auto fibFailureCb = bind(&Rib::onFibUpdateFailure, this, item.managerFailureCallback, _1, _2);
 
-  m_fibUpdater->computeAndSendFibUpdates(batch,
-                                         bind(&Rib::onFibUpdateSuccess, this,
-                                              batch, _1, managerSuccessCallback),
-                                         bind(&Rib::onFibUpdateFailure, this,
-                                              managerFailureCallback, _1, _2));
-
-  if (m_onSendBatchFromQueue != nullptr) {
-    m_onSendBatchFromQueue(batch);
+#ifdef WITH_TESTS
+  if (mockFibResponse != nullptr) {
+    m_fibUpdater->computeAndSendFibUpdates(batch, bind([]{}), bind([]{}));
+    bool shouldFibSucceed = mockFibResponse(batch);
+    if (wantMockFibResponseOnce) {
+      mockFibResponse = nullptr;
+    }
+    if (shouldFibSucceed) {
+      fibSuccessCb(m_fibUpdater->m_inheritedRoutes);
+    }
+    else {
+      fibFailureCb(504, "mocked failure");
+    }
+    return;
   }
+#endif
+
+  m_fibUpdater->computeAndSendFibUpdates(batch, fibSuccessCb, fibFailureCb);
 }
 
 void
diff --git a/rib/rib.hpp b/rib/rib.hpp
index c8a0f5d..a2539b8 100644
--- a/rib/rib.hpp
+++ b/rib/rib.hpp
@@ -175,8 +175,16 @@
   sendBatchFromQueue();
 
 PUBLIC_WITH_TESTS_ELSE_PRIVATE:
-  // Used by RibManager unit-tests to get sent batch to simulate successful FIB update
-  std::function<void(RibUpdateBatch)> m_onSendBatchFromQueue;
+#ifdef WITH_TESTS
+  /** \brief In unit tests, mock FIB update result.
+   *
+   *  If the callback is not nullptr, sendBatchFromQueue() immediately succeeds or fails according
+   *  to the return value of callback function.
+   */
+  std::function<bool(const RibUpdateBatch&)> mockFibResponse;
+
+  bool wantMockFibResponseOnce = false; ///< if true, mockFibResponse is cleared after every use.
+#endif
 
   void
   erase(const Name& prefix, const Route& route);